diff --git a/libraries/app/api.cpp b/libraries/app/api.cpp index 077ad0d1..0190868a 100644 --- a/libraries/app/api.cpp +++ b/libraries/app/api.cpp @@ -169,12 +169,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 bcf79cfb..66126dbf 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -36,9 +36,45 @@ #include #include #include +#include #include +namespace { + + 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; + }; + + 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 { bool database::is_known_block( const block_id_type& id )const @@ -104,6 +140,32 @@ 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 = 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) + { + auto proposed_operations_digests = gather_proposed_operations_digests(pending_transaction); + existed_operations_digests.insert(proposed_operations_digests.begin(), proposed_operations_digests.end()); + } + + 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."); + } +} /** * 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 new file mode 100644 index 00000000..77120e4b --- /dev/null +++ b/tests/tests/network_broadcast_api_tests.cpp @@ -0,0 +1,352 @@ +#include + +#include +#include +#include +#include +#include +#include + +#include "../common/database_fixture.hpp" + +using namespace graphene::chain; +using namespace graphene::chain::test; + +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; + } + + 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; + set_expiration(fixture.db, transaction); + + transaction.operations = operations; + + fixture.db.create([&](proposal_object& proposal) + { + proposal.proposed_transaction = transaction; + }); + } + + signed_transaction make_signed_transaction_with_proposed_operation(database_fixture& fixture, const std::vector& operations) + { + proposal_create_operation operation_proposal; + + for (auto& operation: operations) + { + operation_proposal.proposed_ops.push_back(op_wrapper(operation)); + } + + signed_transaction transaction; + set_expiration(fixture.db, transaction); + transaction.operations = {operation_proposal}; + + 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 ) + +BOOST_AUTO_TEST_CASE( test_exception_throwing_for_the_same_operation_proposed_twice ) +{ + 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(500))}); + 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_without_duplication ) +{ + try + { + ACTORS((alice)) + + 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 ) + { + edump((e.to_detail_string())); + throw; + } +} + +BOOST_AUTO_TEST_CASE( check_passes_for_the_same_operation_with_different_assets ) +{ + 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))}); + 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_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_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 + { + 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 + { + 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_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_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()