transaction.cpp: Implement minimize_required_signatures and nonminimal_sig_test

libraries/chain/include/graphene/chain/protocol/transaction.hpp

@@ -118,19 +118,37 @@ namespace graphene { namespace chain {
       signature_type sign( const private_key_type& key )const;
 
       /**
-       *  The purpose of this method is to identify the minimal subset of @ref available_keys that are
+       *  The purpose of this method is to identify some subset of
-       *  required to sign given the signatures that are already provided.
+       *  @ref available_keys that will produce sufficient signatures
+       *  for a transaction.  The result is not always a minimal set of
+       *  signatures, but any non-minimal result will still pass
+       *  validation.
        */
-      set<public_key_type> get_required_signatures( const flat_set<public_key_type>& available_keys,
+      set<public_key_type> get_required_signatures(
-                                                    const std::function<const authority*(account_id_type)>& get_active,
+         const flat_set<public_key_type>& available_keys,
-                                                    const std::function<const authority*(account_id_type)>& get_owner,
+         const std::function<const authority*(account_id_type)>& get_active,
-                                                    uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
+         const std::function<const authority*(account_id_type)>& get_owner,
-                                                    )const;
+         uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
+         )const;
 
       void verify_authority( const std::function<const authority*(account_id_type)>& get_active,
                              const std::function<const authority*(account_id_type)>& get_owner,
                              uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const;
 
+      /**
+       * This is a slower replacement for get_required_signatures()
+       * which returns a minimal set in all cases, including
+       * some cases where get_required_signatures() returns a
+       * non-minimal set.
+       */
+
+      set<public_key_type> minimize_required_signatures(
+         const flat_set<public_key_type>& available_keys,
+         const std::function<const authority*(account_id_type)>& get_active,
+         const std::function<const authority*(account_id_type)>& get_owner,
+         uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
+         ) const;
+
       flat_set<public_key_type> get_signature_keys()const;
 
       vector<signature_type> signatures;

libraries/chain/protocol/transaction.cpp

@@ -267,6 +267,31 @@ set<public_key_type> signed_transaction::get_required_signatures( const flat_set
    return result;
 }
 
+set<public_key_type> signed_transaction::minimize_required_signatures(
+   const flat_set<public_key_type>& available_keys,
+   const std::function<const authority*(account_id_type)>& get_active,
+   const std::function<const authority*(account_id_type)>& get_owner,
+   uint32_t max_recursion
+   ) const
+{
+   set< public_key_type > s = get_required_signatures( available_keys, get_active, get_owner, max_recursion );
+   flat_set< public_key_type > result( s.begin(), s.end() );
+
+   for( const public_key_type& k : s )
+   {
+      result.erase( k );
+      try
+      {
+         graphene::chain::verify_authority( operations, result, get_active, get_owner, max_recursion );
+         continue;  // element stays erased if verify_authority is ok
+      }
+      catch( const tx_missing_owner_auth& e ) {}
+      catch( const tx_missing_active_auth& e ) {}
+      catch( const tx_missing_other_auth& e ) {}
+      result.insert( k );
+   }
+   return set<public_key_type>( result.begin(), result.end() );
+}
 
 void signed_transaction::verify_authority( const std::function<const authority*(account_id_type)>& get_active,
                                            const std::function<const authority*(account_id_type)>& get_owner,

tests/tests/authority_tests.cpp

@@ -1085,7 +1085,7 @@ BOOST_FIXTURE_TEST_CASE( voting_account, database_fixture )
                          vikram_committee_member) != db.get_global_properties().active_committee_members.end());
 } FC_LOG_AND_RETHROW() }
 
-/**
+/*
  * Simple corporate accounts:
  *
  * Well Corp.       Alice 50, Bob 50             T=60
@@ -1205,4 +1205,102 @@ BOOST_FIXTURE_TEST_CASE( get_required_signatures_test, database_fixture )
    }
 }
 
+/*
+ * Pathological case
+ *
+ *      Roco(T=2)
+ *    1/         \2
+ *   Styx(T=2)   Thud(T=1)
+ *  1/     \1       |1
+ * Alice  Bob     Alice
+ */
+
+BOOST_FIXTURE_TEST_CASE( nonminimal_sig_test, database_fixture )
+{
+   try
+   {
+      ACTORS(
+         (alice)(bob)
+         (roco)
+         (styx)(thud)
+         );
+
+      auto set_auth = [&](
+         account_id_type aid,
+         const authority& auth
+         )
+      {
+         signed_transaction tx;
+         account_update_operation op;
+         op.account = aid;
+         op.active = auth;
+         op.owner = auth;
+         tx.operations.push_back( op );
+         tx.set_expiration( db.head_block_time() + fc::minutes( 5 ) );
+         PUSH_TX( db, tx, database::skip_transaction_signatures | database::skip_authority_check );
+      } ;
+
+      auto get_active = [&](
+         account_id_type aid
+         ) -> const authority*
+      {
+         return &(aid(db).active);
+      } ;
+
+      auto get_owner = [&](
+         account_id_type aid
+         ) -> const authority*
+      {
+         return &(aid(db).owner);
+      } ;
+
+      auto chk = [&](
+         const signed_transaction& tx,
+         flat_set<public_key_type> available_keys,
+         set<public_key_type> ref_set
+         ) -> bool
+      {
+         //wdump( (tx)(available_keys) );
+         set<public_key_type> result_set = tx.get_required_signatures( available_keys, get_active, get_owner );
+         //wdump( (result_set)(ref_set) );
+         return result_set == ref_set;
+      } ;
+
+      auto chk_min = [&](
+         const signed_transaction& tx,
+         flat_set<public_key_type> available_keys,
+         set<public_key_type> ref_set
+         ) -> bool
+      {
+         //wdump( (tx)(available_keys) );
+         set<public_key_type> result_set = tx.minimize_required_signatures( available_keys, get_active, get_owner );
+         //wdump( (result_set)(ref_set) );
+         return result_set == ref_set;
+      } ;
+
+      set_auth( roco_id, authority( 2, styx_id, 1, thud_id, 2 ) );
+      set_auth( styx_id, authority( 2, alice_id, 1, bob_id, 1 ) );
+      set_auth( thud_id, authority( 1, alice_id, 1 ) );
+
+      signed_transaction tx;
+      transfer_operation op;
+      op.from = roco_id;
+      op.to = bob_id;
+      op.amount = asset(1);
+      tx.operations.push_back( op );
+
+      BOOST_CHECK( chk( tx, { alice_public_key, bob_public_key }, { alice_public_key, bob_public_key } ) );
+      BOOST_CHECK( chk_min( tx, { alice_public_key, bob_public_key }, { alice_public_key } ) );
+
+      GRAPHENE_REQUIRE_THROW( tx.verify_authority( get_active, get_owner ), fc::exception );
+      tx.sign( alice_private_key );
+      tx.verify_authority( get_active, get_owner );
+   }
+   catch(fc::exception& e)
+   {
+      edump((e.to_detail_string()));
+      throw;
+   }
+}
+
 BOOST_AUTO_TEST_SUITE_END()