Merge remote-tracking branch 'origin/broadcast-api-operations-duplication-check' into beatrice

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<fc::variant>::ptr prom( new fc::promise<fc::variant>() );
        broadcast_transaction_with_callback( [=]( const fc::variant& v ){
         prom->set_value(v);

libraries/chain/db_block.cpp

@@ -36,9 +36,45 @@
 #include <graphene/chain/protocol/fee_schedule.hpp>
 #include <graphene/chain/exceptions.hpp>
 #include <graphene/chain/evaluator.hpp>
+#include <fc/crypto/digest.hpp>
 
 #include <fc/smart_ref_impl.hpp>
 
+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<class T>
+      void operator()(const T&) 
+      {}
+      
+      std::vector<fc::sha256> proposed_operations_digests;
+   };
+   
+   std::vector<fc::sha256> 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<block_id_type> 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<proposal_object>();
+   std::set<fc::sha256> existed_operations_digests;
+   
+   proposal_index.inspect_all_objects( [&](const object& obj){
+      const proposal_object& proposal = static_cast<const proposal_object&>(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

libraries/chain/include/graphene/chain/database.hpp

@@ -136,6 +136,8 @@ namespace graphene { namespace chain {
          void                              add_checkpoints( const flat_map<uint32_t,block_id_type>& checkpts );
          const flat_map<uint32_t,block_id_type> 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 );

tests/tests/network_broadcast_api_tests.cpp

@@ -0,0 +1,352 @@
+#include <boost/test/unit_test.hpp>
+
+#include <graphene/chain/exceptions.hpp>
+#include <graphene/chain/protocol/proposal.hpp>
+#include <graphene/chain/proposal_object.hpp>
+#include <graphene/chain/witness_object.hpp>
+#include <graphene/chain/protocol/committee_member.hpp>
+#include <fc/crypto/digest.hpp>
+
+#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<operation>& operations)
+    {
+        signed_transaction transaction;
+        set_expiration(fixture.db, transaction);
+        
+        transaction.operations = operations;
+        
+        fixture.db.create<proposal_object>([&](proposal_object& proposal)
+        {
+            proposal.proposed_transaction = transaction;
+        });
+    }
+    
+    signed_transaction make_signed_transaction_with_proposed_operation(database_fixture& fixture, const std::vector<operation>& 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<operation>& 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()