From 4a9ca67b600a0a2650ad3bef0fee078a82729beb Mon Sep 17 00:00:00 2001 From: Apr Team Date: Wed, 4 Jul 2018 19:31:32 +0300 Subject: [PATCH 01/11] Added check for operation existing in pending proposals. --- libraries/chain/db_maint.cpp | 6 +- tests/tests/network_broadcast_api_tests.cpp | 173 ++++++++++++++++++++ 2 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 tests/tests/network_broadcast_api_tests.cpp diff --git a/libraries/chain/db_maint.cpp b/libraries/chain/db_maint.cpp index ade0504c..10e6cb3e 100644 --- a/libraries/chain/db_maint.cpp +++ b/libraries/chain/db_maint.cpp @@ -791,9 +791,9 @@ void schedule_pending_dividend_balances(database& db, auto current_distribution_account_balance_iter = current_distribution_account_balance_range.first; auto previous_distribution_account_balance_iter = previous_distribution_account_balance_range.first; - dlog("Current balances in distribution account: ${current}, Previous balances: ${previous}", - ("current", std::distance(current_distribution_account_balance_range.first, current_distribution_account_balance_range.second)) - ("previous", std::distance(previous_distribution_account_balance_range.first, previous_distribution_account_balance_range.second))); +// dlog("Current balances in distribution account: ${current}, Previous balances: ${previous}", +// ("current", std::distance(current_distribution_account_balance_range.first, current_distribution_account_balance_range.second)) +// ("previous", std::distance(previous_distribution_account_balance_range.first, previous_distribution_account_balance_range.second))); // when we pay out the dividends to the holders, we need to know the total balance of the dividend asset in all // accounts other than the distribution account (it would be silly to distribute dividends back to diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp new file mode 100644 index 00000000..5b87c3ab --- /dev/null +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -0,0 +1,173 @@ + + +/* + * Copyright (c) 2015 Cryptonomex, Inc., and contributors. + * + * The MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include + +#include +#include + +#include + +#include + +#include "../common/database_fixture.hpp" + +using namespace graphene::chain; +using namespace graphene::chain::test; + +//void database_fixture::transfer( +// account_id_type from, +// account_id_type to, +// const asset& amount, +// const asset& fee /* = asset() */ +//) +//{ +// transfer(from(db), to(db), amount, fee); +//} +// +//void database_fixture::transfer( +// const account_object& from, +// const account_object& to, +// const asset& amount, +// const asset& fee /* = asset() */ ) +//{ +// try +// { +// set_expiration( db, trx ); +// transfer_operation trans; +// trans.from = from.id; +// trans.to = to.id; +// trans.amount = amount; +// trx.operations.push_back(trans); +// +// if( fee == asset() ) +// { +// for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op); +// } +// trx.validate(); +// db.push_transaction(trx, ~0); +// verify_asset_supplies(db); +// trx.operations.clear(); +// } FC_CAPTURE_AND_RETHROW( (from.id)(to.id)(amount)(fee) ) +//} + +namespace +{ + transfer_operation make_transfer_operation(const account_id_type& from, const account_id_type& to, const asset& amount) + { + transfer_operation transfer; + transfer.from = from; + transfer.to = to; + transfer.amount = amount; + + return transfer; + } + + void propose_operation(database_fixture& fixture, const operation& op) + { + signed_transaction transaction; + set_expiration(fixture.db, transaction); + + transaction.operations = {op}; + + fixture.db.create([&](proposal_object& proposal) + { + proposal.proposed_transaction = transaction; + }); + } + + struct proposed_operations_digest_accumulator + { + typedef void result_type; + + template + void operator()(const T&) const + {} + + void operator()(const proposal_create_operation& proposal) const + { + for (auto& operation: proposal.proposed_ops) + { + proposed_operations_digests.push_back(fc::digest(operation.op)); + } + } + + mutable std::vector proposed_operations_digests; + }; + + void check_transactions_duplicates(database& db, const signed_transaction& transaction) + { + const auto& proposal_index = db.get_index(); + std::set existed_operations_digests; + + proposal_index.inspect_all_objects( [&](const object& obj){ + const proposal_object& proposal = static_cast(obj); + for (auto& operation: proposal.proposed_transaction.operations) + { + existed_operations_digests.insert(fc::digest(operation)); + } + }); + + proposed_operations_digest_accumulator digest_accumulator; + for (auto& operation: transaction.operations) + { + operation.visit(digest_accumulator); + } + + for (auto& digest: digest_accumulator.proposed_operations_digests) + { + FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation already pending for approval."); + } + } +} + +BOOST_FIXTURE_TEST_SUITE( check_transactions_duplicates, database_fixture ) + +BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_twice ) +{ + try + { + ACTORS((alice)(bob)) + + ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + + proposal_create_operation operation_proposal; + operation_proposal.proposed_ops = {op_wrapper(make_transfer_operation(account_id_type(), alice_id, asset(500)))}; + + signed_transaction trx; + set_expiration( db, trx ); + trx.operations = {operation_proposal}; + + BOOST_CHECK_THROW(check_transactions_duplicates(db, trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_SUITE_END() From 9fd4fe6713e0ba027b32dd0013cebdf16f6b8794 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Wed, 4 Jul 2018 19:49:05 +0300 Subject: [PATCH 02/11] Added couple of tests. --- tests/tests/network_broadcast_api_tests.cpp | 96 +++++++++++---------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index 5b87c3ab..d29f2ac8 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -24,56 +24,18 @@ * THE SOFTWARE. */ -#include +#include #include -#include - +#include +#include #include -#include - #include "../common/database_fixture.hpp" using namespace graphene::chain; using namespace graphene::chain::test; -//void database_fixture::transfer( -// account_id_type from, -// account_id_type to, -// const asset& amount, -// const asset& fee /* = asset() */ -//) -//{ -// transfer(from(db), to(db), amount, fee); -//} -// -//void database_fixture::transfer( -// const account_object& from, -// const account_object& to, -// const asset& amount, -// const asset& fee /* = asset() */ ) -//{ -// try -// { -// set_expiration( db, trx ); -// transfer_operation trans; -// trans.from = from.id; -// trans.to = to.id; -// trans.amount = amount; -// trx.operations.push_back(trans); -// -// if( fee == asset() ) -// { -// for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op); -// } -// trx.validate(); -// db.push_transaction(trx, ~0); -// verify_asset_supplies(db); -// trx.operations.clear(); -// } FC_CAPTURE_AND_RETHROW( (from.id)(to.id)(amount)(fee) ) -//} - namespace { transfer_operation make_transfer_operation(const account_id_type& from, const account_id_type& to, const asset& amount) @@ -139,18 +101,18 @@ namespace for (auto& digest: digest_accumulator.proposed_operations_digests) { - FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation already pending for approval."); + FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); } } } -BOOST_FIXTURE_TEST_SUITE( check_transactions_duplicates, database_fixture ) +BOOST_FIXTURE_TEST_SUITE( check_transactions_duplicates_tests, database_fixture ) BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_twice ) { try { - ACTORS((alice)(bob)) + ACTORS((alice)) ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); @@ -170,4 +132,50 @@ BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_tw } } +BOOST_AUTO_TEST_CASE( test_check_passes_without_duplication ) +{ + try + { + ACTORS((alice)) + + proposal_create_operation operation_proposal; + operation_proposal.proposed_ops = {op_wrapper(make_transfer_operation(account_id_type(), alice_id, asset(500)))}; + + signed_transaction trx; + set_expiration( db, trx ); + trx.operations = {operation_proposal}; + + BOOST_CHECK_NO_THROW(check_transactions_duplicates(db, trx)); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_with_different_assets ) +{ + try + { + ACTORS((alice)) + + ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + + proposal_create_operation operation_proposal; + operation_proposal.proposed_ops = {op_wrapper(make_transfer_operation(account_id_type(), alice_id, asset(501)))}; + + signed_transaction trx; + set_expiration( db, trx ); + trx.operations = {operation_proposal}; + + BOOST_CHECK_NO_THROW(check_transactions_duplicates(db, trx)); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From 1c74bba60dbf339926c46b4a4ac8de856818b713 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 5 Jul 2018 12:58:59 +0300 Subject: [PATCH 03/11] Moved check for duplicates to the database. Removed code duplication from unit-tests --- libraries/chain/db_block.cpp | 48 ++++++++++++ .../chain/include/graphene/chain/database.hpp | 2 + tests/tests/network_broadcast_api_tests.cpp | 78 ++++--------------- 3 files changed, 63 insertions(+), 65 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index b63dfde4..94468c2e 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -36,9 +36,32 @@ #include #include #include +#include #include +namespace { + + struct proposed_operations_digest_accumulator + { + typedef void result_type; + + template + void operator()(const T&) + {} + + void operator()(const graphene::chain::proposal_create_operation& proposal) + { + for (auto& operation: proposal.proposed_ops) + { + proposed_operations_digests.push_back(fc::digest(operation.op)); + } + } + + std::vector proposed_operations_digests; + }; +} + namespace graphene { namespace chain { bool database::is_known_block( const block_id_type& id )const @@ -104,6 +127,31 @@ std::vector database::get_block_ids_on_fork(block_id_type head_of result.emplace_back(branches.first.back()->previous_id()); return result; } + +void database::check_tansaction_for_duplicated_operations(const signed_transaction& trx) +{ + const auto& proposal_index = this->get_index(); + std::set existed_operations_digests; + + proposal_index.inspect_all_objects( [&](const object& obj){ + const proposal_object& proposal = static_cast(obj); + for (auto& operation: proposal.proposed_transaction.operations) + { + existed_operations_digests.insert(fc::digest(operation)); + } + }); + + proposed_operations_digest_accumulator digest_accumulator; + for (auto& operation: trx.operations) + { + operation.visit(digest_accumulator); + } + + for (auto& digest: digest_accumulator.proposed_operations_digests) + { + FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); + } +} /** * Push block "may fail" in which case every partial change is unwound. After diff --git a/libraries/chain/include/graphene/chain/database.hpp b/libraries/chain/include/graphene/chain/database.hpp index 02fe64f6..af50a94b 100644 --- a/libraries/chain/include/graphene/chain/database.hpp +++ b/libraries/chain/include/graphene/chain/database.hpp @@ -136,6 +136,8 @@ namespace graphene { namespace chain { void add_checkpoints( const flat_map& checkpts ); const flat_map get_checkpoints()const { return _checkpoints; } bool before_last_checkpoint()const; + + void check_tansaction_for_duplicated_operations(const signed_transaction& trx); bool push_block( const signed_block& b, uint32_t skip = skip_nothing ); processed_transaction push_transaction( const signed_transaction& trx, uint32_t skip = skip_nothing ); diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index d29f2ac8..ec5a4485 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -1,5 +1,3 @@ - - /* * Copyright (c) 2015 Cryptonomex, Inc., and contributors. * @@ -61,48 +59,16 @@ namespace }); } - struct proposed_operations_digest_accumulator + signed_transaction make_signed_transaction_with_proposed_operation(database_fixture& fixture, const operation& op) { - typedef void result_type; + proposal_create_operation operation_proposal; + operation_proposal.proposed_ops = {op_wrapper(op)}; - template - void operator()(const T&) const - {} + signed_transaction transaction; + set_expiration( fixture.db, transaction ); + transaction.operations = {operation_proposal}; - void operator()(const proposal_create_operation& proposal) const - { - for (auto& operation: proposal.proposed_ops) - { - proposed_operations_digests.push_back(fc::digest(operation.op)); - } - } - - mutable std::vector proposed_operations_digests; - }; - - void check_transactions_duplicates(database& db, const signed_transaction& transaction) - { - const auto& proposal_index = db.get_index(); - std::set existed_operations_digests; - - proposal_index.inspect_all_objects( [&](const object& obj){ - const proposal_object& proposal = static_cast(obj); - for (auto& operation: proposal.proposed_transaction.operations) - { - existed_operations_digests.insert(fc::digest(operation)); - } - }); - - proposed_operations_digest_accumulator digest_accumulator; - for (auto& operation: transaction.operations) - { - operation.visit(digest_accumulator); - } - - for (auto& digest: digest_accumulator.proposed_operations_digests) - { - FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); - } + return transaction; } } @@ -116,14 +82,8 @@ BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_tw ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); - proposal_create_operation operation_proposal; - operation_proposal.proposed_ops = {op_wrapper(make_transfer_operation(account_id_type(), alice_id, asset(500)))}; - - signed_transaction trx; - set_expiration( db, trx ); - trx.operations = {operation_proposal}; - - BOOST_CHECK_THROW(check_transactions_duplicates(db, trx), fc::exception); + auto trx = make_signed_transaction_with_proposed_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); } catch( const fc::exception& e ) { @@ -138,14 +98,8 @@ BOOST_AUTO_TEST_CASE( test_check_passes_without_duplication ) { ACTORS((alice)) - proposal_create_operation operation_proposal; - operation_proposal.proposed_ops = {op_wrapper(make_transfer_operation(account_id_type(), alice_id, asset(500)))}; - - signed_transaction trx; - set_expiration( db, trx ); - trx.operations = {operation_proposal}; - - BOOST_CHECK_NO_THROW(check_transactions_duplicates(db, trx)); + auto trx = make_signed_transaction_with_proposed_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); } catch( const fc::exception& e ) { @@ -162,14 +116,8 @@ BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_with_differ ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); - proposal_create_operation operation_proposal; - operation_proposal.proposed_ops = {op_wrapper(make_transfer_operation(account_id_type(), alice_id, asset(501)))}; - - signed_transaction trx; - set_expiration( db, trx ); - trx.operations = {operation_proposal}; - - BOOST_CHECK_NO_THROW(check_transactions_duplicates(db, trx)); + auto trx = make_signed_transaction_with_proposed_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(501))); + BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); } catch( const fc::exception& e ) { From 140ecd753268ce742e276b561b8b5480140fae4a Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 5 Jul 2018 13:58:36 +0300 Subject: [PATCH 04/11] Added more tests. Fixed offsets in the db file. Pathced the network_broadcast_api. --- libraries/app/api.cpp | 3 + libraries/chain/db_block.cpp | 50 +++++++----- tests/tests/network_broadcast_api_tests.cpp | 88 ++++++++++++++++++--- 3 files changed, 107 insertions(+), 34 deletions(-) diff --git a/libraries/app/api.cpp b/libraries/app/api.cpp index 6c6359c2..442a29aa 100644 --- a/libraries/app/api.cpp +++ b/libraries/app/api.cpp @@ -163,12 +163,15 @@ namespace graphene { namespace app { void network_broadcast_api::broadcast_transaction(const signed_transaction& trx) { trx.validate(); + _app.chain_database()->check_tansaction_for_duplicated_operations(trx); _app.chain_database()->push_transaction(trx); _app.p2p_node()->broadcast_transaction(trx); } fc::variant network_broadcast_api::broadcast_transaction_synchronous(const signed_transaction& trx) { + _app.chain_database()->check_tansaction_for_duplicated_operations(trx); + fc::promise::ptr prom( new fc::promise() ); broadcast_transaction_with_callback( [=]( const fc::variant& v ){ prom->set_value(v); diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 94468c2e..82f21dbb 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -130,27 +130,35 @@ std::vector database::get_block_ids_on_fork(block_id_type head_of void database::check_tansaction_for_duplicated_operations(const signed_transaction& trx) { - const auto& proposal_index = this->get_index(); - std::set existed_operations_digests; - - proposal_index.inspect_all_objects( [&](const object& obj){ - const proposal_object& proposal = static_cast(obj); - for (auto& operation: proposal.proposed_transaction.operations) - { - existed_operations_digests.insert(fc::digest(operation)); - } - }); - - proposed_operations_digest_accumulator digest_accumulator; - for (auto& operation: trx.operations) - { - operation.visit(digest_accumulator); - } - - for (auto& digest: digest_accumulator.proposed_operations_digests) - { - FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); - } + const auto& proposal_index = get_index(); + std::set existed_operations_digests; + + proposal_index.inspect_all_objects( [&](const object& obj){ + const proposal_object& proposal = static_cast(obj); + for (auto& operation: proposal.proposed_transaction.operations) + { + existed_operations_digests.insert(fc::digest(operation)); + } + }); + + for (auto& pending_transaction: _pending_tx) + { + for (auto& operation: pending_transaction.operations) + { + existed_operations_digests.insert(fc::digest(operation)); + } + } + + proposed_operations_digest_accumulator digest_accumulator; + for (auto& operation: trx.operations) + { + operation.visit(digest_accumulator); + } + + for (auto& digest: digest_accumulator.proposed_operations_digests) + { + FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); + } } /** diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index ec5a4485..dd073cfa 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -46,12 +46,12 @@ namespace return transfer; } - void propose_operation(database_fixture& fixture, const operation& op) + void create_proposal(database_fixture& fixture, const std::vector& operations) { signed_transaction transaction; set_expiration(fixture.db, transaction); - transaction.operations = {op}; + transaction.operations = operations; fixture.db.create([&](proposal_object& proposal) { @@ -59,20 +59,24 @@ namespace }); } - signed_transaction make_signed_transaction_with_proposed_operation(database_fixture& fixture, const operation& op) + signed_transaction make_signed_transaction_with_proposed_operation(database_fixture& fixture, const std::vector& operations) { proposal_create_operation operation_proposal; - operation_proposal.proposed_ops = {op_wrapper(op)}; + + for (auto& operation: operations) + { + operation_proposal.proposed_ops = {op_wrapper(operation)}; + } signed_transaction transaction; - set_expiration( fixture.db, transaction ); + set_expiration(fixture.db, transaction); transaction.operations = {operation_proposal}; return transaction; } } -BOOST_FIXTURE_TEST_SUITE( check_transactions_duplicates_tests, database_fixture ) +BOOST_FIXTURE_TEST_SUITE( check_tansaction_for_duplicated_operations, database_fixture ) BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_twice ) { @@ -80,9 +84,9 @@ BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_tw { ACTORS((alice)) - ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); - auto trx = make_signed_transaction_with_proposed_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); } catch( const fc::exception& e ) @@ -92,13 +96,13 @@ BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_tw } } -BOOST_AUTO_TEST_CASE( test_check_passes_without_duplication ) +BOOST_AUTO_TEST_CASE( check_passes_without_duplication ) { try { ACTORS((alice)) - auto trx = make_signed_transaction_with_proposed_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); } catch( const fc::exception& e ) @@ -108,15 +112,15 @@ BOOST_AUTO_TEST_CASE( test_check_passes_without_duplication ) } } -BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_with_different_assets ) +BOOST_AUTO_TEST_CASE( check_passes_for_the_same_operation_with_different_assets ) { try { ACTORS((alice)) - ::propose_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(500))); + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); - auto trx = make_signed_transaction_with_proposed_operation(*this, make_transfer_operation(account_id_type(), alice_id, asset(501))); + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(501))}); BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); } catch( const fc::exception& e ) @@ -126,4 +130,62 @@ BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_with_differ } } +BOOST_AUTO_TEST_CASE( check_fails_for_duplication_in_transaction_with_several_operations ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(501)), + make_transfer_operation(account_id_type(), alice_id, asset(500))}); //duplicated one + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_fails_for_duplicated_operation_in_existed_proposal_with_several_operations_and_transaction_with_several_operations ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(499)), + make_transfer_operation(account_id_type(), alice_id, asset(500))}); //duplicated one + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(501)), + make_transfer_operation(account_id_type(), alice_id, asset(500))}); //duplicated one + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_fails_for_duplicated_operation_in_existed_proposal_with_several_operations ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(499)), + make_transfer_operation(account_id_type(), alice_id, asset(500))}); //duplicated one + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); //duplicated one + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From a94dd371c1351e079b96e3e8645b40873d8928f7 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 5 Jul 2018 14:41:00 +0300 Subject: [PATCH 05/11] Reverted commented code. --- libraries/chain/db_maint.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/chain/db_maint.cpp b/libraries/chain/db_maint.cpp index 10e6cb3e..ade0504c 100644 --- a/libraries/chain/db_maint.cpp +++ b/libraries/chain/db_maint.cpp @@ -791,9 +791,9 @@ void schedule_pending_dividend_balances(database& db, auto current_distribution_account_balance_iter = current_distribution_account_balance_range.first; auto previous_distribution_account_balance_iter = previous_distribution_account_balance_range.first; -// dlog("Current balances in distribution account: ${current}, Previous balances: ${previous}", -// ("current", std::distance(current_distribution_account_balance_range.first, current_distribution_account_balance_range.second)) -// ("previous", std::distance(previous_distribution_account_balance_range.first, previous_distribution_account_balance_range.second))); + dlog("Current balances in distribution account: ${current}, Previous balances: ${previous}", + ("current", std::distance(current_distribution_account_balance_range.first, current_distribution_account_balance_range.second)) + ("previous", std::distance(previous_distribution_account_balance_range.first, previous_distribution_account_balance_range.second))); // when we pay out the dividends to the holders, we need to know the total balance of the dividend asset in all // accounts other than the distribution account (it would be silly to distribute dividends back to From e6f56af1b6ec79295043993cf451a2cd2c03fce3 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 5 Jul 2018 16:16:16 +0300 Subject: [PATCH 06/11] Fixed small issue in test + removed explicit check for duplicates in pending transactions. They are covered by proposal storage check. --- libraries/chain/db_block.cpp | 8 -------- tests/tests/network_broadcast_api_tests.cpp | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 82f21dbb..15c95378 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -141,14 +141,6 @@ void database::check_tansaction_for_duplicated_operations(const signed_transacti } }); - for (auto& pending_transaction: _pending_tx) - { - for (auto& operation: pending_transaction.operations) - { - existed_operations_digests.insert(fc::digest(operation)); - } - } - proposed_operations_digest_accumulator digest_accumulator; for (auto& operation: trx.operations) { diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index dd073cfa..f50b61e5 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -65,7 +65,7 @@ namespace for (auto& operation: operations) { - operation_proposal.proposed_ops = {op_wrapper(operation)}; + operation_proposal.proposed_ops.push_back(op_wrapper(operation)); } signed_transaction transaction; From 8094431613094da75813245bf7ba8f4130f70742 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Fri, 6 Jul 2018 15:29:15 +0300 Subject: [PATCH 07/11] Refactoring after review. --- libraries/chain/db_block.cpp | 38 ++++++------ tests/tests/network_broadcast_api_tests.cpp | 66 +++++++++++++++++++++ 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 15c95378..29b0eeaf 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -42,24 +42,26 @@ namespace { - struct proposed_operations_digest_accumulator - { - typedef void result_type; - - template - void operator()(const T&) - {} - - void operator()(const graphene::chain::proposal_create_operation& proposal) - { - for (auto& operation: proposal.proposed_ops) - { - proposed_operations_digests.push_back(fc::digest(operation.op)); - } - } - - std::vector proposed_operations_digests; - }; + struct proposed_operations_digest_accumulator + { + typedef void result_type; + + void operator()(const graphene::chain::proposal_create_operation& proposal) + { + for (auto& operation: proposal.proposed_ops) + { + proposed_operations_digests.push_back(fc::digest(operation.op)); + } + } + + //empty template method is needed for all other operation types + //we can ignore them, we are interested in only proposal_create_operation + template + void operator()(const T&) + {} + + std::vector proposed_operations_digests; + }; } namespace graphene { namespace chain { diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index f50b61e5..e438e76b 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include "../common/database_fixture.hpp" @@ -46,6 +48,16 @@ namespace return transfer; } + committee_member_create_operation make_committee_member_create_operation(const asset& fee, const account_id_type& member, const string& url) + { + committee_member_create_operation member_create_operation; + member_create_operation.fee = fee; + member_create_operation.committee_member_account = member; + member_create_operation.url = url; + + return member_create_operation; + } + void create_proposal(database_fixture& fixture, const std::vector& operations) { signed_transaction transaction; @@ -188,4 +200,58 @@ BOOST_AUTO_TEST_CASE( check_fails_for_duplicated_operation_in_existed_proposal_w } } +BOOST_AUTO_TEST_CASE( check_passes_for_different_operations_types ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500))}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); + BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_fails_for_same_member_create_operations ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_passes_for_different_member_create_operations ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_committee_member_create_operation(asset(1001), account_id_type(), "test url")}); + BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From 7c47df8a6b3ca4e8034fa70bb8adc6a98849d6f4 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Fri, 6 Jul 2018 16:40:05 +0300 Subject: [PATCH 08/11] Added test for serveral proposals with several operations of mixed types. --- tests/tests/network_broadcast_api_tests.cpp | 28 ++++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index e438e76b..a4aad6aa 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -222,8 +222,6 @@ BOOST_AUTO_TEST_CASE( check_fails_for_same_member_create_operations ) { try { - ACTORS((alice)) - create_proposal(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); auto trx = make_signed_transaction_with_proposed_operation(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); @@ -240,8 +238,6 @@ BOOST_AUTO_TEST_CASE( check_passes_for_different_member_create_operations ) { try { - ACTORS((alice)) - create_proposal(*this, {make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); auto trx = make_signed_transaction_with_proposed_operation(*this, {make_committee_member_create_operation(asset(1001), account_id_type(), "test url")}); @@ -254,4 +250,28 @@ BOOST_AUTO_TEST_CASE( check_passes_for_different_member_create_operations ) } } +BOOST_AUTO_TEST_CASE( check_failes_for_several_operations_of_mixed_type ) +{ + try + { + ACTORS((alice)) + + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(500)), + make_committee_member_create_operation(asset(1000), account_id_type(), "test url")}); + + create_proposal(*this, {make_transfer_operation(account_id_type(), alice_id, asset(501)), //duplicate + make_committee_member_create_operation(asset(1001), account_id_type(), "test url")}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(account_id_type(), alice_id, asset(501)), //duplicate + make_committee_member_create_operation(asset(1002), account_id_type(), "test url")}); + + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From 63f4c7f2c5921f097a6b706f924ba6ad4e3da213 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 12 Jul 2018 17:18:55 +0300 Subject: [PATCH 09/11] Added pending list check. --- libraries/chain/db_block.cpp | 13 ++- tests/tests/network_broadcast_api_tests.cpp | 99 +++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 29b0eeaf..bd917960 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -143,12 +143,23 @@ void database::check_tansaction_for_duplicated_operations(const signed_transacti } }); + for (auto& pending_transaction: _pending_tx) + { + proposed_operations_digest_accumulator digest_accumulator; + for (auto& operation: pending_transaction.operations) + { + operation.visit(digest_accumulator); + } + + existed_operations_digests.insert(digest_accumulator.proposed_operations_digests.begin(), digest_accumulator.proposed_operations_digests.end()); + } + proposed_operations_digest_accumulator digest_accumulator; for (auto& operation: trx.operations) { operation.visit(digest_accumulator); } - + for (auto& digest: digest_accumulator.proposed_operations_digests) { FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index a4aad6aa..b0fc81b0 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -86,6 +86,26 @@ namespace return transaction; } + + void push_proposal(database_fixture& fixture, const account_object& fee_payer, const std::vector& operations) + { + proposal_create_operation operation_proposal; + operation_proposal.fee_paying_account = fee_payer.id; + + for (auto& operation: operations) + { + operation_proposal.proposed_ops.push_back(op_wrapper(operation)); + } + + operation_proposal.expiration_time = fixture.db.head_block_time() + fc::days(1); + + signed_transaction transaction; + transaction.operations.push_back(operation_proposal); + set_expiration( fixture.db, transaction ); + + fixture.sign( transaction, fixture.init_account_priv_key ); + PUSH_TX( fixture.db, transaction ); + } } BOOST_FIXTURE_TEST_SUITE( check_tansaction_for_duplicated_operations, database_fixture ) @@ -274,4 +294,83 @@ BOOST_AUTO_TEST_CASE( check_failes_for_several_operations_of_mixed_type ) } } +BOOST_AUTO_TEST_CASE( check_failes_for_duplicates_in_pending_transactions_list ) +{ + try + { + ACTORS((alice)) + + fc::ecc::private_key committee_key = init_account_priv_key; + + const account_object& moneyman = create_account("moneyman", init_account_pub_key); + const asset_object& core = asset_id_type()(db); + + transfer(account_id_type()(db), moneyman, core.amount(1000000)); + + auto duplicate = make_transfer_operation(alice.id, moneyman.get_id(), asset(100)); + push_proposal(*this, moneyman, {duplicate}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {duplicate}); + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_passes_for_no_duplicates_in_pending_transactions_list ) +{ + try + { + ACTORS((alice)) + + fc::ecc::private_key committee_key = init_account_priv_key; + + const account_object& moneyman = create_account("moneyman", init_account_pub_key); + const asset_object& core = asset_id_type()(db); + + transfer(account_id_type()(db), moneyman, core.amount(1000000)); + + push_proposal(*this, moneyman, {make_transfer_operation(alice.id, moneyman.get_id(), asset(100))}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {make_transfer_operation(alice.id, moneyman.get_id(), asset(101))}); + BOOST_CHECK_NO_THROW(db.check_tansaction_for_duplicated_operations(trx)); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_fails_for_several_trĐ°nsactions_with_duplicates_in_pending_list ) +{ + try + { + ACTORS((alice)) + + fc::ecc::private_key committee_key = init_account_priv_key; + + const account_object& moneyman = create_account("moneyman", init_account_pub_key); + const asset_object& core = asset_id_type()(db); + + transfer(account_id_type()(db), moneyman, core.amount(1000000)); + + auto duplicate = make_transfer_operation(alice.id, moneyman.get_id(), asset(100)); + push_proposal(*this, moneyman, {make_transfer_operation(alice.id, moneyman.get_id(), asset(101)), + duplicate}); + + auto trx = make_signed_transaction_with_proposed_operation(*this, {duplicate, + make_transfer_operation(alice.id, moneyman.get_id(), asset(102))}); + BOOST_CHECK_THROW(db.check_tansaction_for_duplicated_operations(trx), fc::exception); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From b28783c3ec6cf3a14b2000deff9837dc1a2dcd75 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 12 Jul 2018 17:53:15 +0300 Subject: [PATCH 10/11] Refactoring after review. Removed code duplication. --- libraries/chain/db_block.cpp | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index bd917960..e11c50f7 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -62,6 +62,17 @@ namespace { std::vector proposed_operations_digests; }; + + std::vector gather_proposed_operations_digests(const graphene::chain::transaction& trx) + { + proposed_operations_digest_accumulator digest_accumulator; + for (auto& operation: trx.operations) + { + operation.visit(digest_accumulator); + } + + return digest_accumulator.proposed_operations_digests; + } } namespace graphene { namespace chain { @@ -145,22 +156,12 @@ void database::check_tansaction_for_duplicated_operations(const signed_transacti for (auto& pending_transaction: _pending_tx) { - proposed_operations_digest_accumulator digest_accumulator; - for (auto& operation: pending_transaction.operations) - { - operation.visit(digest_accumulator); - } - - existed_operations_digests.insert(digest_accumulator.proposed_operations_digests.begin(), digest_accumulator.proposed_operations_digests.end()); + auto proposed_operations_digests = gather_proposed_operations_digests(pending_transaction); + existed_operations_digests.insert(proposed_operations_digests.begin(), proposed_operations_digests.end()); } - proposed_operations_digest_accumulator digest_accumulator; - for (auto& operation: trx.operations) - { - operation.visit(digest_accumulator); - } - - for (auto& digest: digest_accumulator.proposed_operations_digests) + auto proposed_operations_digests = gather_proposed_operations_digests(trx); + for (auto& digest: proposed_operations_digests) { FC_ASSERT(existed_operations_digests.count(digest) == 0, "Proposed operation is already pending for approval."); } From 6d1538166a320e12f42a53e2026ff8e72caf2a3e Mon Sep 17 00:00:00 2001 From: Apr Team Date: Fri, 13 Jul 2018 13:13:55 +0300 Subject: [PATCH 11/11] Removed licence header from unit-tests. --- tests/tests/network_broadcast_api_tests.cpp | 24 --------------------- 1 file changed, 24 deletions(-) diff --git a/tests/tests/network_broadcast_api_tests.cpp b/tests/tests/network_broadcast_api_tests.cpp index b0fc81b0..77120e4b 100644 --- a/tests/tests/network_broadcast_api_tests.cpp +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -1,27 +1,3 @@ -/* - * Copyright (c) 2015 Cryptonomex, Inc., and contributors. - * - * The MIT License - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - #include #include