From 140ecd753268ce742e276b561b8b5480140fae4a Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 5 Jul 2018 13:58:36 +0300 Subject: [PATCH] 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()