From 1c74bba60dbf339926c46b4a4ac8de856818b713 Mon Sep 17 00:00:00 2001 From: Apr Team Date: Thu, 5 Jul 2018 12:58:59 +0300 Subject: [PATCH] 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 ) {