Issue867 Add CLI command to add signatures to a partially signed transaction

libraries/wallet/include/graphene/wallet/wallet.hpp

@@ -1638,6 +1638,20 @@ class wallet_api
 
       order_book get_order_book( const string& base, const string& quote, unsigned limit = 50);
 
+      /** Signs a transaction.
+       *
+       * Given a fully-formed transaction with or without signatures, signs
+       * the transaction with the owned keys and optionally broadcasts the
+       * transaction.
+       *
+       * @param tx the unsigned transaction
+       * @param broadcast true if you wish to broadcast the transaction
+       *
+       * @return the signed transaction
+       */
+      signed_transaction add_transaction_signature( signed_transaction tx,
+                                                    bool broadcast = false );
+
       void dbg_make_uia(string creator, string symbol);
       void dbg_make_mia(string creator, string symbol);
       void dbg_push_blocks( std::string src_filename, uint32_t count );
@@ -1828,6 +1842,7 @@ FC_API( graphene::wallet::wallet_api,
         (save_wallet_file)
         (serialize_transaction)
         (sign_transaction)
+        (add_transaction_signature)
         (get_prototype_operation)
         (propose_parameter_change)
         (propose_fee_change)

libraries/wallet/wallet.cpp

@@ -789,6 +789,74 @@ class wallet_api_impl
       return true;
    }
 
+   /**
+    * Get the required public keys to sign the transaction which had been
+    * owned by us
+    *
+    * NOTE, if `erase_existing_sigs` set to true, the original trasaction's
+    * signatures will be erased
+    *
+    * @param tx           The transaction to be signed
+    * @param erase_existing_sigs
+    *        The transaction could have been partially signed already,
+    *        if set to false, the corresponding public key of existing
+    *        signatures won't be returned.
+    *        If set to true, the existing signatures will be erased and
+    *        all required keys returned.
+   */
+   set<public_key_type> get_owned_required_keys( signed_transaction &tx,
+                                                    bool erase_existing_sigs = true)
+   {
+      set<public_key_type> pks = _remote_db->get_potential_signatures( tx );
+      flat_set<public_key_type> owned_keys;
+      owned_keys.reserve( pks.size() );
+      std::copy_if( pks.begin(), pks.end(),
+                    std::inserter( owned_keys, owned_keys.end() ),
+                    [this]( const public_key_type &pk ) {
+                       return _keys.find( pk ) != _keys.end();
+                    } );
+
+      if ( erase_existing_sigs )
+         tx.signatures.clear();
+
+      return _remote_db->get_required_signatures( tx, owned_keys );
+   }
+
+   signed_transaction add_transaction_signature( signed_transaction tx,
+                                                 bool broadcast )
+   {
+      set<public_key_type> approving_key_set = get_owned_required_keys(tx, false);
+
+      if ( ( ( tx.ref_block_num == 0 && tx.ref_block_prefix == 0 ) ||
+             tx.expiration == fc::time_point_sec() ) &&
+           tx.signatures.empty() )
+      {
+         auto dyn_props = get_dynamic_global_properties();
+         auto parameters = get_global_properties().parameters;
+         fc::time_point_sec now = dyn_props.time;
+         tx.set_reference_block( dyn_props.head_block_id );
+         tx.set_expiration( now + parameters.maximum_time_until_expiration );
+      }
+      for ( const public_key_type &key : approving_key_set )
+         tx.sign( get_private_key( key ), _chain_id );
+
+      if ( broadcast )
+      {
+         try
+         {
+            _remote_net_broadcast->broadcast_transaction( tx );
+         }
+         catch ( const fc::exception &e )
+         {
+            elog( "Caught exception while broadcasting tx ${id}:  ${e}",
+                  ( "id", tx.id().str() )( "e", e.to_detail_string() ) );
+            FC_THROW( "Caught exception while broadcasting tx" );
+         }
+      }
+
+      return tx;
+   }
+
     void quit()
     {
         ilog( "Quitting Cli Wallet ..." );
@@ -976,7 +1044,6 @@ class wallet_api_impl
       _builder_transactions.erase(handle);
    }
 
-
    signed_transaction register_account(string name,
                                        public_key_type owner,
                                        public_key_type active,
@@ -1044,7 +1111,6 @@ class wallet_api_impl
       return tx;
    } FC_CAPTURE_AND_RETHROW( (name)(owner)(active)(registrar_account)(referrer_account)(referrer_percent)(broadcast) ) }
 
-
    signed_transaction upgrade_account(string name, bool broadcast)
    { try {
       FC_ASSERT( !self.is_locked() );
@@ -1062,7 +1128,6 @@ class wallet_api_impl
       return sign_transaction( tx, broadcast );
    } FC_CAPTURE_AND_RETHROW( (name) ) }
 
-
    // This function generates derived keys starting with index 0 and keeps incrementing
    // the index until it finds a key that isn't registered in the block chain.  To be
    // safer, it continues checking for a few more keys to make sure there wasn't a short gap
@@ -1950,13 +2015,8 @@ class wallet_api_impl
 
    signed_transaction sign_transaction(signed_transaction tx, bool broadcast = false)
    {
-      set<public_key_type> pks = _remote_db->get_potential_signatures( tx );
-      flat_set<public_key_type> owned_keys;
-      owned_keys.reserve( pks.size() );
-      std::copy_if( pks.begin(), pks.end(), std::inserter(owned_keys, owned_keys.end()),
-                    [this](const public_key_type& pk){ return _keys.find(pk) != _keys.end(); } );
-      tx.clear_signatures();
-      set<public_key_type> approving_key_set = _remote_db->get_required_signatures( tx, owned_keys );
+
+      set<public_key_type> approving_key_set = get_owned_required_keys(tx);
 
       auto dyn_props = get_dynamic_global_properties();
       tx.set_reference_block( dyn_props.head_block_id );
@@ -3851,6 +3911,12 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const
    return my->get_dynamic_global_properties();
 }
 
+signed_transaction wallet_api::add_transaction_signature( signed_transaction tx,
+                                                          bool broadcast )
+{
+   return my->add_transaction_signature( tx, broadcast );
+}
+
 string wallet_api::help()const
 {
    std::vector<std::string> method_names = my->method_documentation.get_method_names();

tests/cli/main.cpp

@@ -583,6 +583,138 @@ BOOST_FIXTURE_TEST_CASE( account_history_pagination, cli_fixture )
    }
 }
 
+
+///////////////////////
+// Create a multi-sig account and verify that only when all signatures are
+// signed, the transaction could be broadcast
+///////////////////////
+BOOST_AUTO_TEST_CASE( cli_multisig_transaction )
+{
+   using namespace graphene::chain;
+   using namespace graphene::app;
+   std::shared_ptr<graphene::app::application> app1;
+   try {
+      fc::temp_directory app_dir( graphene::utilities::temp_directory_path() );
+
+      int server_port_number = 0;
+      app1 = start_application(app_dir, server_port_number);
+
+      // connect to the server
+      client_connection con(app1, app_dir, server_port_number);
+
+      BOOST_TEST_MESSAGE("Setting wallet password");
+      con.wallet_api_ptr->set_password("supersecret");
+      con.wallet_api_ptr->unlock("supersecret");
+
+      // import Nathan account
+      BOOST_TEST_MESSAGE("Importing nathan key");
+      std::vector<std::string> nathan_keys{"5KQwrPbwdL6PhXujxW37FSSQZ1JiwsST4cqQzDeyXtP79zkvFD3"};
+      BOOST_CHECK_EQUAL(nathan_keys[0], "5KQwrPbwdL6PhXujxW37FSSQZ1JiwsST4cqQzDeyXtP79zkvFD3");
+      BOOST_CHECK(con.wallet_api_ptr->import_key("nathan", nathan_keys[0]));
+
+      BOOST_TEST_MESSAGE("Importing nathan's balance");
+      std::vector<signed_transaction> import_txs = con.wallet_api_ptr->import_balance("nathan", nathan_keys, true);
+      account_object nathan_acct_before_upgrade = con.wallet_api_ptr->get_account("nathan");
+
+      // upgrade nathan
+      BOOST_TEST_MESSAGE("Upgrading Nathan to LTM");
+      signed_transaction upgrade_tx = con.wallet_api_ptr->upgrade_account("nathan", true);
+      account_object nathan_acct_after_upgrade = con.wallet_api_ptr->get_account("nathan");
+
+      // verify that the upgrade was successful
+      BOOST_CHECK_PREDICATE( std::not_equal_to<uint32_t>(), (nathan_acct_before_upgrade.membership_expiration_date.sec_since_epoch())(nathan_acct_after_upgrade.membership_expiration_date.sec_since_epoch()) );
+      BOOST_CHECK(nathan_acct_after_upgrade.is_lifetime_member());
+
+      // create a new multisig account
+      graphene::wallet::brain_key_info bki1 = con.wallet_api_ptr->suggest_brain_key();
+      graphene::wallet::brain_key_info bki2 = con.wallet_api_ptr->suggest_brain_key();
+      graphene::wallet::brain_key_info bki3 = con.wallet_api_ptr->suggest_brain_key();
+      graphene::wallet::brain_key_info bki4 = con.wallet_api_ptr->suggest_brain_key();
+      BOOST_CHECK(!bki1.brain_priv_key.empty());
+      BOOST_CHECK(!bki2.brain_priv_key.empty());
+      BOOST_CHECK(!bki3.brain_priv_key.empty());
+      BOOST_CHECK(!bki4.brain_priv_key.empty());
+
+      signed_transaction create_multisig_acct_tx;
+      account_create_operation account_create_op;
+
+      account_create_op.referrer = nathan_acct_after_upgrade.id;
+      account_create_op.referrer_percent = nathan_acct_after_upgrade.referrer_rewards_percentage;
+      account_create_op.registrar = nathan_acct_after_upgrade.id;
+      account_create_op.name = "cifer.test";
+      account_create_op.owner = authority(1, bki1.pub_key, 1);
+      account_create_op.active = authority(2, bki2.pub_key, 1, bki3.pub_key, 1);
+      account_create_op.options.memo_key = bki4.pub_key;
+      account_create_op.fee = asset(1000000);  // should be enough for creating account
+
+      create_multisig_acct_tx.operations.push_back(account_create_op);
+      con.wallet_api_ptr->sign_transaction(create_multisig_acct_tx, true);
+
+      // attempt to give cifer.test some bitsahres
+      BOOST_TEST_MESSAGE("Transferring bitshares from Nathan to cifer.test");
+      signed_transaction transfer_tx1 = con.wallet_api_ptr->transfer("nathan", "cifer.test", "10000", "1.3.0", "Here are some BTS for your new account", true);
+
+      // transfer bts from cifer.test to nathan
+      BOOST_TEST_MESSAGE("Transferring bitshares from cifer.test to nathan");
+      auto dyn_props = app1->chain_database()->get_dynamic_global_properties();
+      account_object cifer_test = con.wallet_api_ptr->get_account("cifer.test");
+
+      // construct a transfer transaction
+      signed_transaction transfer_tx2;
+      transfer_operation xfer_op;
+      xfer_op.from = cifer_test.id;
+      xfer_op.to = nathan_acct_after_upgrade.id;
+      xfer_op.amount = asset(100000000);
+      xfer_op.fee = asset(3000000);  // should be enough for transfer
+      transfer_tx2.operations.push_back(xfer_op);
+
+      // case1: sign a transaction without TaPoS and expiration fields
+      // expect: return a transaction with TaPoS and expiration filled
+      transfer_tx2 =
+         con.wallet_api_ptr->add_transaction_signature( transfer_tx2, false );
+      BOOST_CHECK( ( transfer_tx2.ref_block_num != 0 &&
+                     transfer_tx2.ref_block_prefix != 0 ) ||
+                   ( transfer_tx2.expiration != fc::time_point_sec() ) );
+
+      // case2: broadcast without signature
+      // expect: exception with missing active authority
+      BOOST_CHECK_THROW(con.wallet_api_ptr->broadcast_transaction(transfer_tx2), fc::exception);
+
+      // case3:
+      // import one of the private keys for this new account in the wallet file,
+      // sign and broadcast with partial signatures
+      //
+      // expect: exception with missing active authority
+      BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki2.wif_priv_key));
+      BOOST_CHECK_THROW(con.wallet_api_ptr->add_transaction_signature(transfer_tx2, true), fc::exception);
+
+      // case4: sign again as signature exists
+      // expect: num of signatures not increase
+      transfer_tx2 = con.wallet_api_ptr->add_transaction_signature(transfer_tx2, false);
+      BOOST_CHECK_EQUAL(transfer_tx2.signatures.size(), 1);
+
+      // case5:
+      // import another private key, sign and broadcast without full signatures
+      //
+      // expect: transaction broadcast successfully
+      BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki3.wif_priv_key));
+      con.wallet_api_ptr->add_transaction_signature(transfer_tx2, true);
+      auto balances = con.wallet_api_ptr->list_account_balances( "cifer.test" );
+      for (auto b : balances) {
+         if (b.asset_id == asset_id_type()) {
+            BOOST_ASSERT(b == asset(900000000 - 3000000));
+         }
+      }
+
+      // wait for everything to finish up
+      fc::usleep(fc::seconds(1));
+   } catch( fc::exception& e ) {
+      edump((e.to_detail_string()));
+      throw;
+   }
+   app1->shutdown();
+}
+
 graphene::wallet::plain_keys decrypt_keys( const std::string& password, const vector<char>& cipher_keys )
 {
    auto pw = fc::sha512::hash( password.c_str(), password.size() );