From e90e9b1b0031f8000471cf9eb444f2f6a57ec125 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 31 May 2018 01:55:37 +0800 Subject: [PATCH 01/29] #867, Add CLI command to add signatures to a partially signed transaction --- libraries/wallet/wallet.cpp | 89 +++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index d3986cc763..e8b176a6ee 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1876,6 +1876,95 @@ class wallet_api_impl return tx; } + signed_transaction sign_transaction2(singed_transaction &tx, bool broadcast = false) + { + set pks = _remote_db->get_potential_signatures(tx); + flat_set 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(); + }); + + set approving_key_set = + _remote_db->get_required_signatures(tx, owned_keys); + + // The first sign should also set TaPoS field. In general, cold wallet + // won't be first sign since it has no idea of the head block id + if (tx.signatures.empty()) + { + auto dyn_props = get_dynamic_global_properties(); + + tx.set_reference_block(dyn_props.head_block_id); + + // first, some bookkeeping, expire old items from + // _recently_generated_transactions since transactions include the head + // block id, we just need the index for keeping transactions unique + // when there are multiple transactions in the same block. choose a + // time period that should be at least one block long, even in the + // worst case. 2 minutes ought to be plenty. + fc::time_point_sec oldest_transaction_ids_to_track(dyn_props.time - + fc::minutes(2)); + auto oldest_transaction_record_iter = + _recently_generated_transactions.get().lower_bound( + oldest_transaction_ids_to_track); + auto begin_iter = + _recently_generated_transactions.get().begin(); + _recently_generated_transactions.get().erase( + begin_iter, oldest_transaction_record_iter); + + uint32_t expiration_time_offset = 0; + for (;;) + { + tx.set_expiration(dyn_props.time + + fc::seconds(30 + expiration_time_offset)); + tx.signatures.clear(); + + for (const public_key_type &key : approving_key_set) + tx.sign(get_private_key(key), _chain_id); + + graphene::chain::transaction_id_type this_transaction_id = tx.id(); + auto iter = + _recently_generated_transactions.find(this_transaction_id); + if (iter == _recently_generated_transactions.end()) + { + // we haven't generated this transaction before, the usual case + recently_generated_transaction_record this_transaction_record; + this_transaction_record.generation_time = dyn_props.time; + this_transaction_record.transaction_id = this_transaction_id; + _recently_generated_transactions.insert(this_transaction_record); + break; + } + + // else we've generated a dupe, increment expiration time and + // re-sign it + ++expiration_time_offset; + } + } + else + { + 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())); + throw; + } + } + + return tx; + } + memo_data sign_memo(string from, string to, string memo) { FC_ASSERT( !self.is_locked() ); From c17049174f07606663e664cb88de35e6fb70bdb6 Mon Sep 17 00:00:00 2001 From: cifer Date: Sun, 3 Jun 2018 18:53:11 +0800 Subject: [PATCH 02/29] add offline transaction signing method --- libraries/wallet/wallet.cpp | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index e8b176a6ee..cc050995ae 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1876,19 +1876,11 @@ class wallet_api_impl return tx; } - signed_transaction sign_transaction2(singed_transaction &tx, bool broadcast = false) + signed_transaction sign_transaction_offline(signed_transaction& tx, public_key_type& key) { - set pks = _remote_db->get_potential_signatures(tx); - flat_set 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(); - }); - - set approving_key_set = - _remote_db->get_required_signatures(tx, owned_keys); + FC_ASSERT((tx.ref_block_num && tx.ref_block_prefix) && + (tx.expiration != fc::time_point_sec()), + "Must first set TaPOS and expiration field"); // The first sign should also set TaPoS field. In general, cold wallet // won't be first sign since it has no idea of the head block id @@ -1948,20 +1940,6 @@ class wallet_api_impl 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())); - throw; - } - } - return tx; } From 1b294370a105f497acd0ae6e5163af0197d26ed9 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 7 Jun 2018 14:46:25 +0800 Subject: [PATCH 03/29] rough implementation of multi-sig transaction --- .../wallet/include/graphene/wallet/wallet.hpp | 24 +- libraries/wallet/wallet.cpp | 207 ++++++++++++------ 2 files changed, 162 insertions(+), 69 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index da191b8d66..bf32e8aa4b 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -252,8 +252,15 @@ struct vesting_balance_object_with_info : public vesting_balance_object fc::time_point_sec allowed_withdraw_time; }; -namespace detail { -class wallet_api_impl; +struct multisig_trx_data +{ + signed_transaction tx; + set required_keys; +}; + +namespace detail +{ + class wallet_api_impl; } /*** @@ -1538,6 +1545,13 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); + bool multisig_mode(string on_or_off, string tx_filename = ""); + multisig_trx_data + multisig_import_transaction(string tx_filename); + multisig_trx_data + multisig_sign_transaction(const vector& wif_keys, + 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 ); @@ -1633,6 +1647,9 @@ FC_REFLECT(graphene::wallet::operation_detail_ex, FC_REFLECT( graphene::wallet::account_history_operation_detail, (total_count)(result_count)(details)) +FC_REFLECT( graphene::wallet::multisig_trx_data, + (tx)(required_keys)) + FC_API( graphene::wallet::wallet_api, (help) (gethelp) @@ -1751,4 +1768,7 @@ FC_API( graphene::wallet::wallet_api, (blind_history) (receive_blind_transfer) (get_order_book) + (multisig_mode) + (multisig_import_transaction) + (multisig_sign_transaction) ) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index cc050995ae..595361fb8c 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -758,6 +758,120 @@ class wallet_api_impl return true; } + + bool multisig_mode(string on_or_off, string tx_filename) + { + if (on_or_off == "on") + { + if (tx_filename == "") + FC_THROW("Must provide transaction file name"); + else + _tx_filename = tx_filename; + + _multisig_mode = true; + } + else + _multisig_mode = false; + + return _multisig_mode; + } + + multisig_trx_data multisig_import_transaction(string tx_filename) + { + if (!fc::exists(tx_filename)) + return _trx_data; + + _trx_data = fc::json::from_file(tx_filename) + .as(2 * GRAPHENE_MAX_NESTED_OBJECTS); + + return _trx_data; + } + + multisig_trx_data multisig_sign_transaction(const vector& wif_keys, bool broadcast) + { + // TODO + // 1. show which key was signed, which was not + + // use const reference to call the const variant of `sign()` + const signed_transaction& trx = _trx_data.tx; + + for (const string &wif_key : wif_keys) + { + fc::optional optional_private_key = + wif_to_key(wif_key); + + if (!optional_private_key) + FC_THROW("Invalid private key: ${i}", ("i", wif_key)); + + signature_type sig = + optional_private_key->sign_compact(trx.sig_digest(_chain_id)); + const vector &sigs = trx.signatures; + if (std::find(sigs.begin(), sigs.end(), sig) != sigs.end()) + { + wlog( "signature of ${fn} had already exist, skip", ("fn", wif_key) ); + } + else + { + _trx_data.tx.sign(*optional_private_key, _chain_id); + } + } + + if (broadcast) + { + try + { + _remote_net_broadcast->broadcast_transaction(_trx_data.tx); + } + catch (const fc::exception &e) + { + elog("Caught exception while broadcasting tx ${id}: ${e}", + ("id", _trx_data.tx.id().str())("e", e.to_detail_string())); + throw; + } + } + + return _trx_data; + } + + void save_transaction_to_file(signed_transaction& tx, set required_keys) + { + // + // Serialize in memory, then save to disk + // + // This approach lessens the risk of a partially written wallet + // if exceptions are thrown in serialization + // + + //encrypt_keys(); + // + _trx_data.tx = tx; + _trx_data.required_keys = required_keys; + + ilog( "saving wallet to file ${fn}", ("fn", _tx_filename) ); + + string tx_data = fc::json::to_pretty_string( _trx_data ); + try + { + enable_umask_protection(); + // + // Parentheses on the following declaration fails to compile, + // due to the Most Vexing Parse. Thanks, C++ + // + // http://en.wikipedia.org/wiki/Most_vexing_parse + // + fc::ofstream outfile{ fc::path( _tx_filename ) }; + outfile.write( tx_data.c_str(), tx_data.length() ); + outfile.flush(); + outfile.close(); + disable_umask_protection(); + } + catch(...) + { + disable_umask_protection(); + throw; + } + } + void save_wallet_file(string wallet_filename = "") { // @@ -1860,6 +1974,11 @@ class wallet_api_impl ++expiration_time_offset; } + if (_multisig_mode) + { + save_transaction_to_file(tx, pks); + } + if( broadcast ) { try @@ -1876,73 +1995,6 @@ class wallet_api_impl return tx; } - signed_transaction sign_transaction_offline(signed_transaction& tx, public_key_type& key) - { - FC_ASSERT((tx.ref_block_num && tx.ref_block_prefix) && - (tx.expiration != fc::time_point_sec()), - "Must first set TaPOS and expiration field"); - - // The first sign should also set TaPoS field. In general, cold wallet - // won't be first sign since it has no idea of the head block id - if (tx.signatures.empty()) - { - auto dyn_props = get_dynamic_global_properties(); - - tx.set_reference_block(dyn_props.head_block_id); - - // first, some bookkeeping, expire old items from - // _recently_generated_transactions since transactions include the head - // block id, we just need the index for keeping transactions unique - // when there are multiple transactions in the same block. choose a - // time period that should be at least one block long, even in the - // worst case. 2 minutes ought to be plenty. - fc::time_point_sec oldest_transaction_ids_to_track(dyn_props.time - - fc::minutes(2)); - auto oldest_transaction_record_iter = - _recently_generated_transactions.get().lower_bound( - oldest_transaction_ids_to_track); - auto begin_iter = - _recently_generated_transactions.get().begin(); - _recently_generated_transactions.get().erase( - begin_iter, oldest_transaction_record_iter); - - uint32_t expiration_time_offset = 0; - for (;;) - { - tx.set_expiration(dyn_props.time + - fc::seconds(30 + expiration_time_offset)); - tx.signatures.clear(); - - for (const public_key_type &key : approving_key_set) - tx.sign(get_private_key(key), _chain_id); - - graphene::chain::transaction_id_type this_transaction_id = tx.id(); - auto iter = - _recently_generated_transactions.find(this_transaction_id); - if (iter == _recently_generated_transactions.end()) - { - // we haven't generated this transaction before, the usual case - recently_generated_transaction_record this_transaction_record; - this_transaction_record.generation_time = dyn_props.time; - this_transaction_record.transaction_id = this_transaction_id; - _recently_generated_transactions.insert(this_transaction_record); - break; - } - - // else we've generated a dupe, increment expiration time and - // re-sign it - ++expiration_time_offset; - } - } - else - { - for (const public_key_type &key : approving_key_set) - tx.sign(get_private_key(key), _chain_id); - } - - return tx; - } - memo_data sign_memo(string from, string to, string memo) { FC_ASSERT( !self.is_locked() ); @@ -2677,6 +2729,10 @@ class wallet_api_impl return it->second; } + bool _multisig_mode = false; + multisig_trx_data _trx_data; + string _tx_filename; + string _wallet_filename; wallet_data _wallet; @@ -3654,6 +3710,23 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } +bool wallet_api::multisig_mode(string on_or_off, string tx_filename) +{ + return my->multisig_mode(on_or_off, tx_filename); +} + +multisig_trx_data wallet_api::multisig_import_transaction(string tx_filename) +{ + return my->multisig_import_transaction(tx_filename); +} + +multisig_trx_data +wallet_api::multisig_sign_transaction(const vector &wif_keys, + bool broadcast) +{ + return my->multisig_sign_transaction(wif_keys, broadcast); +} + string wallet_api::help()const { std::vector method_names = my->method_documentation.get_method_names(); From a218ef69bd5046ec1def40fe41fe358d6278f883 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 7 Jun 2018 15:18:38 +0800 Subject: [PATCH 04/29] make filename optional on multisig_mode command --- libraries/wallet/include/graphene/wallet/wallet.hpp | 2 +- libraries/wallet/wallet.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index bf32e8aa4b..a8fa430c07 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1545,7 +1545,7 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); - bool multisig_mode(string on_or_off, string tx_filename = ""); + bool multisig_mode(string on_or_off, fc::optional tx_filename); multisig_trx_data multisig_import_transaction(string tx_filename); multisig_trx_data diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 595361fb8c..24a53eaef1 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -759,14 +759,14 @@ class wallet_api_impl return true; } - bool multisig_mode(string on_or_off, string tx_filename) + bool multisig_mode(string on_or_off, fc::optional tx_filename) { if (on_or_off == "on") { - if (tx_filename == "") + if (! tx_filename) FC_THROW("Must provide transaction file name"); else - _tx_filename = tx_filename; + _tx_filename = *tx_filename; _multisig_mode = true; } @@ -3710,7 +3710,7 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } -bool wallet_api::multisig_mode(string on_or_off, string tx_filename) +bool wallet_api::multisig_mode(string on_or_off, fc::optional tx_filename) { return my->multisig_mode(on_or_off, tx_filename); } From dcd7150928d5881558bc1c5d151a366635069ca5 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 7 Jun 2018 15:49:30 +0800 Subject: [PATCH 05/29] validate TaPoS and expiration field for imported transaction --- libraries/wallet/wallet.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 24a53eaef1..b51dc52f55 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -778,12 +778,24 @@ class wallet_api_impl multisig_trx_data multisig_import_transaction(string tx_filename) { + multisig_trx_data tmp; + if (!fc::exists(tx_filename)) - return _trx_data; + return tmp; - _trx_data = fc::json::from_file(tx_filename) + tmp = fc::json::from_file(tx_filename) .as(2 * GRAPHENE_MAX_NESTED_OBJECTS); + if (!tmp.tx.ref_block_num || !tmp.tx.ref_block_prefix || + tmp.tx.expiration == fc::time_point_sec()) + { + elog("Must fill TaPoS and expiration field in online machine!"); + } + else + { + _trx_data = tmp; + } + return _trx_data; } From 47569a007dcc15eef3cfc4b780f8b25d769408c3 Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 00:04:34 +0800 Subject: [PATCH 06/29] simplify implementation according to Peter's reviews --- .../wallet/include/graphene/wallet/wallet.hpp | 17 +--- libraries/wallet/wallet.cpp | 96 +++++++++---------- 2 files changed, 50 insertions(+), 63 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index a8fa430c07..2db31b9895 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -252,12 +252,6 @@ struct vesting_balance_object_with_info : public vesting_balance_object fc::time_point_sec allowed_withdraw_time; }; -struct multisig_trx_data -{ - signed_transaction tx; - set required_keys; -}; - namespace detail { class wallet_api_impl; @@ -1546,11 +1540,9 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); bool multisig_mode(string on_or_off, fc::optional tx_filename); - multisig_trx_data - multisig_import_transaction(string tx_filename); - multisig_trx_data - multisig_sign_transaction(const vector& wif_keys, - bool broadcast = false); + optional multisig_import_transaction( string tx_filename ); + signed_transaction multisig_sign_transaction( const vector &wif_keys, + bool broadcast = false ); void dbg_make_uia(string creator, string symbol); void dbg_make_mia(string creator, string symbol); @@ -1647,9 +1639,6 @@ FC_REFLECT(graphene::wallet::operation_detail_ex, FC_REFLECT( graphene::wallet::account_history_operation_detail, (total_count)(result_count)(details)) -FC_REFLECT( graphene::wallet::multisig_trx_data, - (tx)(required_keys)) - FC_API( graphene::wallet::wallet_api, (help) (gethelp) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index b51dc52f55..a6db1b3d7e 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -776,76 +776,78 @@ class wallet_api_impl return _multisig_mode; } - multisig_trx_data multisig_import_transaction(string tx_filename) + optional multisig_import_transaction(string tx_filename) { - multisig_trx_data tmp; + signed_transaction tmp; - if (!fc::exists(tx_filename)) - return tmp; + if ( !fc::exists( tx_filename ) ) + { + elog( "Transaction file not exists" ); + return optional(); + } - tmp = fc::json::from_file(tx_filename) - .as(2 * GRAPHENE_MAX_NESTED_OBJECTS); + tmp = fc::json::from_file( tx_filename ) + .as( 2 * GRAPHENE_MAX_NESTED_OBJECTS ); - if (!tmp.tx.ref_block_num || !tmp.tx.ref_block_prefix || - tmp.tx.expiration == fc::time_point_sec()) + if ( !tmp.ref_block_num || !tmp.ref_block_prefix || tmp.expiration == fc::time_point_sec() ) { - elog("Must fill TaPoS and expiration field in online machine!"); + elog( "Must fill TaPoS and expiration field in online machine" ); + return optional(); } else { - _trx_data = tmp; + _multisig_trx = tmp; + return _multisig_trx; } - - return _trx_data; } - multisig_trx_data multisig_sign_transaction(const vector& wif_keys, bool broadcast) + signed_transaction multisig_sign_transaction( const vector &wif_keys, bool broadcast ) { - // TODO - // 1. show which key was signed, which was not - // use const reference to call the const variant of `sign()` - const signed_transaction& trx = _trx_data.tx; + vector private_keys; - for (const string &wif_key : wif_keys) + for ( const string &wif_key : wif_keys ) { - fc::optional optional_private_key = - wif_to_key(wif_key); + fc::optional optional_private_key = wif_to_key( wif_key ); + + if ( !optional_private_key ) + FC_THROW( "Invalid private key: ${i}", ( "i", wif_key ) ); - if (!optional_private_key) - FC_THROW("Invalid private key: ${i}", ("i", wif_key)); + private_keys.push_back(*optional_private_key); + } - signature_type sig = - optional_private_key->sign_compact(trx.sig_digest(_chain_id)); - const vector &sigs = trx.signatures; - if (std::find(sigs.begin(), sigs.end(), sig) != sigs.end()) + for ( const fc::ecc::private_key& privkey : private_keys) + { + signature_type sig = privkey.sign_compact( _multisig_trx.sig_digest( _chain_id ) ); + const vector &sigs = _multisig_trx.signatures; + if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { - wlog( "signature of ${fn} had already exist, skip", ("fn", wif_key) ); + wlog( "signature of ${fn} had already exist, skip", ( "fn", key_to_wif( privkey ) ) ); } else { - _trx_data.tx.sign(*optional_private_key, _chain_id); + _multisig_trx.sign( privkey, _chain_id ); } } - if (broadcast) + if ( broadcast ) { try { - _remote_net_broadcast->broadcast_transaction(_trx_data.tx); + _remote_net_broadcast->broadcast_transaction( _multisig_trx ); } - catch (const fc::exception &e) + catch ( const fc::exception &e ) { - elog("Caught exception while broadcasting tx ${id}: ${e}", - ("id", _trx_data.tx.id().str())("e", e.to_detail_string())); + elog( "Caught exception while broadcasting tx ${id}: ${e}", + ( "id", _multisig_trx.id().str() )( "e", e.to_detail_string() ) ); throw; } } - return _trx_data; + return _multisig_trx; } - void save_transaction_to_file(signed_transaction& tx, set required_keys) + void save_transaction_to_file(signed_transaction& tx) { // // Serialize in memory, then save to disk @@ -854,14 +856,9 @@ class wallet_api_impl // if exceptions are thrown in serialization // - //encrypt_keys(); - // - _trx_data.tx = tx; - _trx_data.required_keys = required_keys; - ilog( "saving wallet to file ${fn}", ("fn", _tx_filename) ); - string tx_data = fc::json::to_pretty_string( _trx_data ); + string tx_data = fc::json::to_pretty_string( tx ); try { enable_umask_protection(); @@ -882,6 +879,8 @@ class wallet_api_impl disable_umask_protection(); throw; } + + _multisig_trx = tx; } void save_wallet_file(string wallet_filename = "") @@ -1988,7 +1987,7 @@ class wallet_api_impl if (_multisig_mode) { - save_transaction_to_file(tx, pks); + save_transaction_to_file(tx); } if( broadcast ) @@ -2742,7 +2741,7 @@ class wallet_api_impl } bool _multisig_mode = false; - multisig_trx_data _trx_data; + signed_transaction _multisig_trx; string _tx_filename; string _wallet_filename; @@ -3727,16 +3726,15 @@ bool wallet_api::multisig_mode(string on_or_off, fc::optional tx_filenam return my->multisig_mode(on_or_off, tx_filename); } -multisig_trx_data wallet_api::multisig_import_transaction(string tx_filename) +optional wallet_api::multisig_import_transaction( string tx_filename ) { - return my->multisig_import_transaction(tx_filename); + return my->multisig_import_transaction( tx_filename ); } -multisig_trx_data -wallet_api::multisig_sign_transaction(const vector &wif_keys, - bool broadcast) +signed_transaction wallet_api::multisig_sign_transaction( const vector &wif_keys, + bool broadcast ) { - return my->multisig_sign_transaction(wif_keys, broadcast); + return my->multisig_sign_transaction( wif_keys, broadcast ); } string wallet_api::help()const From dc265ab538702fb650c139dfde0b23c2e261e30a Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 19:59:25 +0800 Subject: [PATCH 07/29] make variable name clearer --- libraries/wallet/wallet.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index a6db1b3d7e..1452680041 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -766,7 +766,7 @@ class wallet_api_impl if (! tx_filename) FC_THROW("Must provide transaction file name"); else - _tx_filename = *tx_filename; + _multisig_trx_file = *tx_filename; _multisig_mode = true; } @@ -797,6 +797,7 @@ class wallet_api_impl else { _multisig_trx = tmp; + _multisig_trx_file = tx_filename; return _multisig_trx; } } @@ -856,7 +857,7 @@ class wallet_api_impl // if exceptions are thrown in serialization // - ilog( "saving wallet to file ${fn}", ("fn", _tx_filename) ); + ilog( "saving wallet to file ${fn}", ("fn", _multisig_trx_file) ); string tx_data = fc::json::to_pretty_string( tx ); try @@ -868,7 +869,7 @@ class wallet_api_impl // // http://en.wikipedia.org/wiki/Most_vexing_parse // - fc::ofstream outfile{ fc::path( _tx_filename ) }; + fc::ofstream outfile{ fc::path( _multisig_trx_file ) }; outfile.write( tx_data.c_str(), tx_data.length() ); outfile.flush(); outfile.close(); @@ -2742,7 +2743,7 @@ class wallet_api_impl bool _multisig_mode = false; signed_transaction _multisig_trx; - string _tx_filename; + string _multisig_trx_file; string _wallet_filename; wallet_data _wallet; From efff736793133c5002df120b60ee52b707430fa8 Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 21:46:05 +0800 Subject: [PATCH 08/29] simplify again according to Peter's reviews --- .../wallet/include/graphene/wallet/wallet.hpp | 9 +- libraries/wallet/wallet.cpp | 123 ++---------------- 2 files changed, 16 insertions(+), 116 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 2db31b9895..f4a64d7252 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1539,10 +1539,9 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); - bool multisig_mode(string on_or_off, fc::optional tx_filename); - optional multisig_import_transaction( string tx_filename ); - signed_transaction multisig_sign_transaction( const vector &wif_keys, - bool broadcast = false ); + signed_transaction multisig_sign_transaction( signed_transaction tx, + const vector &wif_keys, + bool broadcast = false ); void dbg_make_uia(string creator, string symbol); void dbg_make_mia(string creator, string symbol); @@ -1757,7 +1756,5 @@ FC_API( graphene::wallet::wallet_api, (blind_history) (receive_blind_transfer) (get_order_book) - (multisig_mode) - (multisig_import_transaction) (multisig_sign_transaction) ) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 1452680041..29340efe4f 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -759,52 +759,9 @@ class wallet_api_impl return true; } - bool multisig_mode(string on_or_off, fc::optional tx_filename) + signed_transaction multisig_sign_transaction( signed_transaction tx, + const vector &wif_keys, bool broadcast ) { - if (on_or_off == "on") - { - if (! tx_filename) - FC_THROW("Must provide transaction file name"); - else - _multisig_trx_file = *tx_filename; - - _multisig_mode = true; - } - else - _multisig_mode = false; - - return _multisig_mode; - } - - optional multisig_import_transaction(string tx_filename) - { - signed_transaction tmp; - - if ( !fc::exists( tx_filename ) ) - { - elog( "Transaction file not exists" ); - return optional(); - } - - tmp = fc::json::from_file( tx_filename ) - .as( 2 * GRAPHENE_MAX_NESTED_OBJECTS ); - - if ( !tmp.ref_block_num || !tmp.ref_block_prefix || tmp.expiration == fc::time_point_sec() ) - { - elog( "Must fill TaPoS and expiration field in online machine" ); - return optional(); - } - else - { - _multisig_trx = tmp; - _multisig_trx_file = tx_filename; - return _multisig_trx; - } - } - - signed_transaction multisig_sign_transaction( const vector &wif_keys, bool broadcast ) - { - // use const reference to call the const variant of `sign()` vector private_keys; for ( const string &wif_key : wif_keys ) @@ -814,20 +771,20 @@ class wallet_api_impl if ( !optional_private_key ) FC_THROW( "Invalid private key: ${i}", ( "i", wif_key ) ); - private_keys.push_back(*optional_private_key); + private_keys.push_back( *optional_private_key ); } - for ( const fc::ecc::private_key& privkey : private_keys) + for ( const fc::ecc::private_key &privkey : private_keys ) { - signature_type sig = privkey.sign_compact( _multisig_trx.sig_digest( _chain_id ) ); - const vector &sigs = _multisig_trx.signatures; + signature_type sig = privkey.sign_compact( tx.sig_digest( _chain_id ) ); + const vector &sigs = tx.signatures; if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { wlog( "signature of ${fn} had already exist, skip", ( "fn", key_to_wif( privkey ) ) ); } else { - _multisig_trx.sign( privkey, _chain_id ); + tx.sign( privkey, _chain_id ); } } @@ -835,53 +792,17 @@ class wallet_api_impl { try { - _remote_net_broadcast->broadcast_transaction( _multisig_trx ); + _remote_net_broadcast->broadcast_transaction( tx ); } catch ( const fc::exception &e ) { elog( "Caught exception while broadcasting tx ${id}: ${e}", - ( "id", _multisig_trx.id().str() )( "e", e.to_detail_string() ) ); + ( "id", tx.id().str() )( "e", e.to_detail_string() ) ); throw; } } - return _multisig_trx; - } - - void save_transaction_to_file(signed_transaction& tx) - { - // - // Serialize in memory, then save to disk - // - // This approach lessens the risk of a partially written wallet - // if exceptions are thrown in serialization - // - - ilog( "saving wallet to file ${fn}", ("fn", _multisig_trx_file) ); - - string tx_data = fc::json::to_pretty_string( tx ); - try - { - enable_umask_protection(); - // - // Parentheses on the following declaration fails to compile, - // due to the Most Vexing Parse. Thanks, C++ - // - // http://en.wikipedia.org/wiki/Most_vexing_parse - // - fc::ofstream outfile{ fc::path( _multisig_trx_file ) }; - outfile.write( tx_data.c_str(), tx_data.length() ); - outfile.flush(); - outfile.close(); - disable_umask_protection(); - } - catch(...) - { - disable_umask_protection(); - throw; - } - - _multisig_trx = tx; + return tx; } void save_wallet_file(string wallet_filename = "") @@ -1986,11 +1907,6 @@ class wallet_api_impl ++expiration_time_offset; } - if (_multisig_mode) - { - save_transaction_to_file(tx); - } - if( broadcast ) { try @@ -2741,10 +2657,6 @@ class wallet_api_impl return it->second; } - bool _multisig_mode = false; - signed_transaction _multisig_trx; - string _multisig_trx_file; - string _wallet_filename; wallet_data _wallet; @@ -3722,20 +3634,11 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } -bool wallet_api::multisig_mode(string on_or_off, fc::optional tx_filename) -{ - return my->multisig_mode(on_or_off, tx_filename); -} - -optional wallet_api::multisig_import_transaction( string tx_filename ) -{ - return my->multisig_import_transaction( tx_filename ); -} - -signed_transaction wallet_api::multisig_sign_transaction( const vector &wif_keys, +signed_transaction wallet_api::multisig_sign_transaction( signed_transaction tx, + const vector &wif_keys, bool broadcast ) { - return my->multisig_sign_transaction( wif_keys, broadcast ); + return my->multisig_sign_transaction( tx, wif_keys, broadcast ); } string wallet_api::help()const From c4d598ebb8fa326d9be27ad5ffff0df3f3265f9c Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 21:50:10 +0800 Subject: [PATCH 09/29] correct grammer and revert unrelated code style changes --- libraries/wallet/include/graphene/wallet/wallet.hpp | 5 ++--- libraries/wallet/wallet.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index f4a64d7252..52aece5063 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -252,9 +252,8 @@ struct vesting_balance_object_with_info : public vesting_balance_object fc::time_point_sec allowed_withdraw_time; }; -namespace detail -{ - class wallet_api_impl; +namespace detail { +class wallet_api_impl; } /*** diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 29340efe4f..eb6ed8dd81 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -780,7 +780,7 @@ class wallet_api_impl const vector &sigs = tx.signatures; if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { - wlog( "signature of ${fn} had already exist, skip", ( "fn", key_to_wif( privkey ) ) ); + wlog( "signature of ${fn} had already exist, skipping", ( "fn", key_to_wif( privkey ) ) ); } else { From c0207fce8cbb6f62d92a4d78ea3ebebd7b4f1030 Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 22:58:50 +0800 Subject: [PATCH 10/29] validate TaPoS and expiration field and change error level so looks more clearer --- libraries/wallet/wallet.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index eb6ed8dd81..f307bb8baf 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -764,12 +764,21 @@ class wallet_api_impl { vector private_keys; + if ( !tx.ref_block_num || !tx.ref_block_prefix || tx.expiration == fc::time_point_sec() ) + { + elog( "Must fill TaPoS and expiration field in an online machine" ); + FC_THROW(""); + } + for ( const string &wif_key : wif_keys ) { fc::optional optional_private_key = wif_to_key( wif_key ); if ( !optional_private_key ) - FC_THROW( "Invalid private key: ${i}", ( "i", wif_key ) ); + { + elog( "Invalid private key: ${i}", ( "i", wif_key ) ); + FC_THROW(""); + } private_keys.push_back( *optional_private_key ); } @@ -798,7 +807,7 @@ class wallet_api_impl { elog( "Caught exception while broadcasting tx ${id}: ${e}", ( "id", tx.id().str() )( "e", e.to_detail_string() ) ); - throw; + FC_THROW(""); } } From 4a10ecc3419914445275ba10dfe015c13ef88797 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 31 May 2018 01:55:37 +0800 Subject: [PATCH 11/29] #867, Add CLI command to add signatures to a partially signed transaction --- libraries/wallet/wallet.cpp | 89 +++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index b360058e57..591b004cfa 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1925,6 +1925,95 @@ class wallet_api_impl return tx; } + signed_transaction sign_transaction2(singed_transaction &tx, bool broadcast = false) + { + set pks = _remote_db->get_potential_signatures(tx); + flat_set 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(); + }); + + set approving_key_set = + _remote_db->get_required_signatures(tx, owned_keys); + + // The first sign should also set TaPoS field. In general, cold wallet + // won't be first sign since it has no idea of the head block id + if (tx.signatures.empty()) + { + auto dyn_props = get_dynamic_global_properties(); + + tx.set_reference_block(dyn_props.head_block_id); + + // first, some bookkeeping, expire old items from + // _recently_generated_transactions since transactions include the head + // block id, we just need the index for keeping transactions unique + // when there are multiple transactions in the same block. choose a + // time period that should be at least one block long, even in the + // worst case. 2 minutes ought to be plenty. + fc::time_point_sec oldest_transaction_ids_to_track(dyn_props.time - + fc::minutes(2)); + auto oldest_transaction_record_iter = + _recently_generated_transactions.get().lower_bound( + oldest_transaction_ids_to_track); + auto begin_iter = + _recently_generated_transactions.get().begin(); + _recently_generated_transactions.get().erase( + begin_iter, oldest_transaction_record_iter); + + uint32_t expiration_time_offset = 0; + for (;;) + { + tx.set_expiration(dyn_props.time + + fc::seconds(30 + expiration_time_offset)); + tx.signatures.clear(); + + for (const public_key_type &key : approving_key_set) + tx.sign(get_private_key(key), _chain_id); + + graphene::chain::transaction_id_type this_transaction_id = tx.id(); + auto iter = + _recently_generated_transactions.find(this_transaction_id); + if (iter == _recently_generated_transactions.end()) + { + // we haven't generated this transaction before, the usual case + recently_generated_transaction_record this_transaction_record; + this_transaction_record.generation_time = dyn_props.time; + this_transaction_record.transaction_id = this_transaction_id; + _recently_generated_transactions.insert(this_transaction_record); + break; + } + + // else we've generated a dupe, increment expiration time and + // re-sign it + ++expiration_time_offset; + } + } + else + { + 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())); + throw; + } + } + + return tx; + } + memo_data sign_memo(string from, string to, string memo) { FC_ASSERT( !self.is_locked() ); From 7cb75198b8e5b1347f0cd8248c88953614493688 Mon Sep 17 00:00:00 2001 From: cifer Date: Sun, 3 Jun 2018 18:53:11 +0800 Subject: [PATCH 12/29] add offline transaction signing method --- libraries/wallet/wallet.cpp | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 591b004cfa..7e7a11499e 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1925,19 +1925,11 @@ class wallet_api_impl return tx; } - signed_transaction sign_transaction2(singed_transaction &tx, bool broadcast = false) + signed_transaction sign_transaction_offline(signed_transaction& tx, public_key_type& key) { - set pks = _remote_db->get_potential_signatures(tx); - flat_set 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(); - }); - - set approving_key_set = - _remote_db->get_required_signatures(tx, owned_keys); + FC_ASSERT((tx.ref_block_num && tx.ref_block_prefix) && + (tx.expiration != fc::time_point_sec()), + "Must first set TaPOS and expiration field"); // The first sign should also set TaPoS field. In general, cold wallet // won't be first sign since it has no idea of the head block id @@ -1997,20 +1989,6 @@ class wallet_api_impl 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())); - throw; - } - } - return tx; } From 8652bd3b45e3e9e1ea1cc5c0ae7ac13d2bc75e88 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 7 Jun 2018 14:46:25 +0800 Subject: [PATCH 13/29] rough implementation of multi-sig transaction --- .../wallet/include/graphene/wallet/wallet.hpp | 24 +- libraries/wallet/wallet.cpp | 207 ++++++++++++------ 2 files changed, 162 insertions(+), 69 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 85b069c900..9afbbdb4c6 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -252,8 +252,15 @@ struct vesting_balance_object_with_info : public vesting_balance_object fc::time_point_sec allowed_withdraw_time; }; -namespace detail { -class wallet_api_impl; +struct multisig_trx_data +{ + signed_transaction tx; + set required_keys; +}; + +namespace detail +{ + class wallet_api_impl; } /*** @@ -1599,6 +1606,13 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); + bool multisig_mode(string on_or_off, string tx_filename = ""); + multisig_trx_data + multisig_import_transaction(string tx_filename); + multisig_trx_data + multisig_sign_transaction(const vector& wif_keys, + 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 ); @@ -1694,6 +1708,9 @@ FC_REFLECT(graphene::wallet::operation_detail_ex, FC_REFLECT( graphene::wallet::account_history_operation_detail, (total_count)(result_count)(details)) +FC_REFLECT( graphene::wallet::multisig_trx_data, + (tx)(required_keys)) + FC_API( graphene::wallet::wallet_api, (help) (gethelp) @@ -1815,4 +1832,7 @@ FC_API( graphene::wallet::wallet_api, (blind_history) (receive_blind_transfer) (get_order_book) + (multisig_mode) + (multisig_import_transaction) + (multisig_sign_transaction) ) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 7e7a11499e..e94871ba9c 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -759,6 +759,120 @@ class wallet_api_impl return true; } + + bool multisig_mode(string on_or_off, string tx_filename) + { + if (on_or_off == "on") + { + if (tx_filename == "") + FC_THROW("Must provide transaction file name"); + else + _tx_filename = tx_filename; + + _multisig_mode = true; + } + else + _multisig_mode = false; + + return _multisig_mode; + } + + multisig_trx_data multisig_import_transaction(string tx_filename) + { + if (!fc::exists(tx_filename)) + return _trx_data; + + _trx_data = fc::json::from_file(tx_filename) + .as(2 * GRAPHENE_MAX_NESTED_OBJECTS); + + return _trx_data; + } + + multisig_trx_data multisig_sign_transaction(const vector& wif_keys, bool broadcast) + { + // TODO + // 1. show which key was signed, which was not + + // use const reference to call the const variant of `sign()` + const signed_transaction& trx = _trx_data.tx; + + for (const string &wif_key : wif_keys) + { + fc::optional optional_private_key = + wif_to_key(wif_key); + + if (!optional_private_key) + FC_THROW("Invalid private key: ${i}", ("i", wif_key)); + + signature_type sig = + optional_private_key->sign_compact(trx.sig_digest(_chain_id)); + const vector &sigs = trx.signatures; + if (std::find(sigs.begin(), sigs.end(), sig) != sigs.end()) + { + wlog( "signature of ${fn} had already exist, skip", ("fn", wif_key) ); + } + else + { + _trx_data.tx.sign(*optional_private_key, _chain_id); + } + } + + if (broadcast) + { + try + { + _remote_net_broadcast->broadcast_transaction(_trx_data.tx); + } + catch (const fc::exception &e) + { + elog("Caught exception while broadcasting tx ${id}: ${e}", + ("id", _trx_data.tx.id().str())("e", e.to_detail_string())); + throw; + } + } + + return _trx_data; + } + + void save_transaction_to_file(signed_transaction& tx, set required_keys) + { + // + // Serialize in memory, then save to disk + // + // This approach lessens the risk of a partially written wallet + // if exceptions are thrown in serialization + // + + //encrypt_keys(); + // + _trx_data.tx = tx; + _trx_data.required_keys = required_keys; + + ilog( "saving wallet to file ${fn}", ("fn", _tx_filename) ); + + string tx_data = fc::json::to_pretty_string( _trx_data ); + try + { + enable_umask_protection(); + // + // Parentheses on the following declaration fails to compile, + // due to the Most Vexing Parse. Thanks, C++ + // + // http://en.wikipedia.org/wiki/Most_vexing_parse + // + fc::ofstream outfile{ fc::path( _tx_filename ) }; + outfile.write( tx_data.c_str(), tx_data.length() ); + outfile.flush(); + outfile.close(); + disable_umask_protection(); + } + catch(...) + { + disable_umask_protection(); + throw; + } + } + void save_wallet_file(string wallet_filename = "") { // @@ -1909,6 +2023,11 @@ class wallet_api_impl ++expiration_time_offset; } + if (_multisig_mode) + { + save_transaction_to_file(tx, pks); + } + if( broadcast ) { try @@ -1925,73 +2044,6 @@ class wallet_api_impl return tx; } - signed_transaction sign_transaction_offline(signed_transaction& tx, public_key_type& key) - { - FC_ASSERT((tx.ref_block_num && tx.ref_block_prefix) && - (tx.expiration != fc::time_point_sec()), - "Must first set TaPOS and expiration field"); - - // The first sign should also set TaPoS field. In general, cold wallet - // won't be first sign since it has no idea of the head block id - if (tx.signatures.empty()) - { - auto dyn_props = get_dynamic_global_properties(); - - tx.set_reference_block(dyn_props.head_block_id); - - // first, some bookkeeping, expire old items from - // _recently_generated_transactions since transactions include the head - // block id, we just need the index for keeping transactions unique - // when there are multiple transactions in the same block. choose a - // time period that should be at least one block long, even in the - // worst case. 2 minutes ought to be plenty. - fc::time_point_sec oldest_transaction_ids_to_track(dyn_props.time - - fc::minutes(2)); - auto oldest_transaction_record_iter = - _recently_generated_transactions.get().lower_bound( - oldest_transaction_ids_to_track); - auto begin_iter = - _recently_generated_transactions.get().begin(); - _recently_generated_transactions.get().erase( - begin_iter, oldest_transaction_record_iter); - - uint32_t expiration_time_offset = 0; - for (;;) - { - tx.set_expiration(dyn_props.time + - fc::seconds(30 + expiration_time_offset)); - tx.signatures.clear(); - - for (const public_key_type &key : approving_key_set) - tx.sign(get_private_key(key), _chain_id); - - graphene::chain::transaction_id_type this_transaction_id = tx.id(); - auto iter = - _recently_generated_transactions.find(this_transaction_id); - if (iter == _recently_generated_transactions.end()) - { - // we haven't generated this transaction before, the usual case - recently_generated_transaction_record this_transaction_record; - this_transaction_record.generation_time = dyn_props.time; - this_transaction_record.transaction_id = this_transaction_id; - _recently_generated_transactions.insert(this_transaction_record); - break; - } - - // else we've generated a dupe, increment expiration time and - // re-sign it - ++expiration_time_offset; - } - } - else - { - for (const public_key_type &key : approving_key_set) - tx.sign(get_private_key(key), _chain_id); - } - - return tx; - } - memo_data sign_memo(string from, string to, string memo) { FC_ASSERT( !self.is_locked() ); @@ -2751,6 +2803,10 @@ class wallet_api_impl return it->second; } + bool _multisig_mode = false; + multisig_trx_data _trx_data; + string _tx_filename; + string _wallet_filename; wallet_data _wallet; @@ -3747,6 +3803,23 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } +bool wallet_api::multisig_mode(string on_or_off, string tx_filename) +{ + return my->multisig_mode(on_or_off, tx_filename); +} + +multisig_trx_data wallet_api::multisig_import_transaction(string tx_filename) +{ + return my->multisig_import_transaction(tx_filename); +} + +multisig_trx_data +wallet_api::multisig_sign_transaction(const vector &wif_keys, + bool broadcast) +{ + return my->multisig_sign_transaction(wif_keys, broadcast); +} + string wallet_api::help()const { std::vector method_names = my->method_documentation.get_method_names(); From 35597c97eeec14fc1dcdb8499108b525f209fe9b Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 7 Jun 2018 15:18:38 +0800 Subject: [PATCH 14/29] make filename optional on multisig_mode command --- libraries/wallet/include/graphene/wallet/wallet.hpp | 2 +- libraries/wallet/wallet.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 9afbbdb4c6..410410b496 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1606,7 +1606,7 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); - bool multisig_mode(string on_or_off, string tx_filename = ""); + bool multisig_mode(string on_or_off, fc::optional tx_filename); multisig_trx_data multisig_import_transaction(string tx_filename); multisig_trx_data diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index e94871ba9c..9c53946cca 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -760,14 +760,14 @@ class wallet_api_impl return true; } - bool multisig_mode(string on_or_off, string tx_filename) + bool multisig_mode(string on_or_off, fc::optional tx_filename) { if (on_or_off == "on") { - if (tx_filename == "") + if (! tx_filename) FC_THROW("Must provide transaction file name"); else - _tx_filename = tx_filename; + _tx_filename = *tx_filename; _multisig_mode = true; } @@ -3803,7 +3803,7 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } -bool wallet_api::multisig_mode(string on_or_off, string tx_filename) +bool wallet_api::multisig_mode(string on_or_off, fc::optional tx_filename) { return my->multisig_mode(on_or_off, tx_filename); } From a616f511c78af77ad8d6ff0564385672fabbe6e8 Mon Sep 17 00:00:00 2001 From: cifer Date: Thu, 7 Jun 2018 15:49:30 +0800 Subject: [PATCH 15/29] validate TaPoS and expiration field for imported transaction --- libraries/wallet/wallet.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 9c53946cca..d162223644 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -779,12 +779,24 @@ class wallet_api_impl multisig_trx_data multisig_import_transaction(string tx_filename) { + multisig_trx_data tmp; + if (!fc::exists(tx_filename)) - return _trx_data; + return tmp; - _trx_data = fc::json::from_file(tx_filename) + tmp = fc::json::from_file(tx_filename) .as(2 * GRAPHENE_MAX_NESTED_OBJECTS); + if (!tmp.tx.ref_block_num || !tmp.tx.ref_block_prefix || + tmp.tx.expiration == fc::time_point_sec()) + { + elog("Must fill TaPoS and expiration field in online machine!"); + } + else + { + _trx_data = tmp; + } + return _trx_data; } From 0ccf3e7cbe817df4c7ce4c30717829c8ce9eb7a0 Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 00:04:34 +0800 Subject: [PATCH 16/29] simplify implementation according to Peter's reviews --- .../wallet/include/graphene/wallet/wallet.hpp | 17 +--- libraries/wallet/wallet.cpp | 96 +++++++++---------- 2 files changed, 50 insertions(+), 63 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 410410b496..dd6f86255f 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -252,12 +252,6 @@ struct vesting_balance_object_with_info : public vesting_balance_object fc::time_point_sec allowed_withdraw_time; }; -struct multisig_trx_data -{ - signed_transaction tx; - set required_keys; -}; - namespace detail { class wallet_api_impl; @@ -1607,11 +1601,9 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); bool multisig_mode(string on_or_off, fc::optional tx_filename); - multisig_trx_data - multisig_import_transaction(string tx_filename); - multisig_trx_data - multisig_sign_transaction(const vector& wif_keys, - bool broadcast = false); + optional multisig_import_transaction( string tx_filename ); + signed_transaction multisig_sign_transaction( const vector &wif_keys, + bool broadcast = false ); void dbg_make_uia(string creator, string symbol); void dbg_make_mia(string creator, string symbol); @@ -1708,9 +1700,6 @@ FC_REFLECT(graphene::wallet::operation_detail_ex, FC_REFLECT( graphene::wallet::account_history_operation_detail, (total_count)(result_count)(details)) -FC_REFLECT( graphene::wallet::multisig_trx_data, - (tx)(required_keys)) - FC_API( graphene::wallet::wallet_api, (help) (gethelp) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index d162223644..c97950bc7c 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -777,76 +777,78 @@ class wallet_api_impl return _multisig_mode; } - multisig_trx_data multisig_import_transaction(string tx_filename) + optional multisig_import_transaction(string tx_filename) { - multisig_trx_data tmp; + signed_transaction tmp; - if (!fc::exists(tx_filename)) - return tmp; + if ( !fc::exists( tx_filename ) ) + { + elog( "Transaction file not exists" ); + return optional(); + } - tmp = fc::json::from_file(tx_filename) - .as(2 * GRAPHENE_MAX_NESTED_OBJECTS); + tmp = fc::json::from_file( tx_filename ) + .as( 2 * GRAPHENE_MAX_NESTED_OBJECTS ); - if (!tmp.tx.ref_block_num || !tmp.tx.ref_block_prefix || - tmp.tx.expiration == fc::time_point_sec()) + if ( !tmp.ref_block_num || !tmp.ref_block_prefix || tmp.expiration == fc::time_point_sec() ) { - elog("Must fill TaPoS and expiration field in online machine!"); + elog( "Must fill TaPoS and expiration field in online machine" ); + return optional(); } else { - _trx_data = tmp; + _multisig_trx = tmp; + return _multisig_trx; } - - return _trx_data; } - multisig_trx_data multisig_sign_transaction(const vector& wif_keys, bool broadcast) + signed_transaction multisig_sign_transaction( const vector &wif_keys, bool broadcast ) { - // TODO - // 1. show which key was signed, which was not - // use const reference to call the const variant of `sign()` - const signed_transaction& trx = _trx_data.tx; + vector private_keys; - for (const string &wif_key : wif_keys) + for ( const string &wif_key : wif_keys ) { - fc::optional optional_private_key = - wif_to_key(wif_key); + fc::optional optional_private_key = wif_to_key( wif_key ); + + if ( !optional_private_key ) + FC_THROW( "Invalid private key: ${i}", ( "i", wif_key ) ); - if (!optional_private_key) - FC_THROW("Invalid private key: ${i}", ("i", wif_key)); + private_keys.push_back(*optional_private_key); + } - signature_type sig = - optional_private_key->sign_compact(trx.sig_digest(_chain_id)); - const vector &sigs = trx.signatures; - if (std::find(sigs.begin(), sigs.end(), sig) != sigs.end()) + for ( const fc::ecc::private_key& privkey : private_keys) + { + signature_type sig = privkey.sign_compact( _multisig_trx.sig_digest( _chain_id ) ); + const vector &sigs = _multisig_trx.signatures; + if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { - wlog( "signature of ${fn} had already exist, skip", ("fn", wif_key) ); + wlog( "signature of ${fn} had already exist, skip", ( "fn", key_to_wif( privkey ) ) ); } else { - _trx_data.tx.sign(*optional_private_key, _chain_id); + _multisig_trx.sign( privkey, _chain_id ); } } - if (broadcast) + if ( broadcast ) { try { - _remote_net_broadcast->broadcast_transaction(_trx_data.tx); + _remote_net_broadcast->broadcast_transaction( _multisig_trx ); } - catch (const fc::exception &e) + catch ( const fc::exception &e ) { - elog("Caught exception while broadcasting tx ${id}: ${e}", - ("id", _trx_data.tx.id().str())("e", e.to_detail_string())); + elog( "Caught exception while broadcasting tx ${id}: ${e}", + ( "id", _multisig_trx.id().str() )( "e", e.to_detail_string() ) ); throw; } } - return _trx_data; + return _multisig_trx; } - void save_transaction_to_file(signed_transaction& tx, set required_keys) + void save_transaction_to_file(signed_transaction& tx) { // // Serialize in memory, then save to disk @@ -855,14 +857,9 @@ class wallet_api_impl // if exceptions are thrown in serialization // - //encrypt_keys(); - // - _trx_data.tx = tx; - _trx_data.required_keys = required_keys; - ilog( "saving wallet to file ${fn}", ("fn", _tx_filename) ); - string tx_data = fc::json::to_pretty_string( _trx_data ); + string tx_data = fc::json::to_pretty_string( tx ); try { enable_umask_protection(); @@ -883,6 +880,8 @@ class wallet_api_impl disable_umask_protection(); throw; } + + _multisig_trx = tx; } void save_wallet_file(string wallet_filename = "") @@ -2037,7 +2036,7 @@ class wallet_api_impl if (_multisig_mode) { - save_transaction_to_file(tx, pks); + save_transaction_to_file(tx); } if( broadcast ) @@ -2816,7 +2815,7 @@ class wallet_api_impl } bool _multisig_mode = false; - multisig_trx_data _trx_data; + signed_transaction _multisig_trx; string _tx_filename; string _wallet_filename; @@ -3820,16 +3819,15 @@ bool wallet_api::multisig_mode(string on_or_off, fc::optional tx_filenam return my->multisig_mode(on_or_off, tx_filename); } -multisig_trx_data wallet_api::multisig_import_transaction(string tx_filename) +optional wallet_api::multisig_import_transaction( string tx_filename ) { - return my->multisig_import_transaction(tx_filename); + return my->multisig_import_transaction( tx_filename ); } -multisig_trx_data -wallet_api::multisig_sign_transaction(const vector &wif_keys, - bool broadcast) +signed_transaction wallet_api::multisig_sign_transaction( const vector &wif_keys, + bool broadcast ) { - return my->multisig_sign_transaction(wif_keys, broadcast); + return my->multisig_sign_transaction( wif_keys, broadcast ); } string wallet_api::help()const From c27c9754525736a9338e3907d49b07f7eea8b92e Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 19:59:25 +0800 Subject: [PATCH 17/29] make variable name clearer --- libraries/wallet/wallet.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index c97950bc7c..6e314e2d60 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -767,7 +767,7 @@ class wallet_api_impl if (! tx_filename) FC_THROW("Must provide transaction file name"); else - _tx_filename = *tx_filename; + _multisig_trx_file = *tx_filename; _multisig_mode = true; } @@ -798,6 +798,7 @@ class wallet_api_impl else { _multisig_trx = tmp; + _multisig_trx_file = tx_filename; return _multisig_trx; } } @@ -857,7 +858,7 @@ class wallet_api_impl // if exceptions are thrown in serialization // - ilog( "saving wallet to file ${fn}", ("fn", _tx_filename) ); + ilog( "saving wallet to file ${fn}", ("fn", _multisig_trx_file) ); string tx_data = fc::json::to_pretty_string( tx ); try @@ -869,7 +870,7 @@ class wallet_api_impl // // http://en.wikipedia.org/wiki/Most_vexing_parse // - fc::ofstream outfile{ fc::path( _tx_filename ) }; + fc::ofstream outfile{ fc::path( _multisig_trx_file ) }; outfile.write( tx_data.c_str(), tx_data.length() ); outfile.flush(); outfile.close(); @@ -2816,7 +2817,7 @@ class wallet_api_impl bool _multisig_mode = false; signed_transaction _multisig_trx; - string _tx_filename; + string _multisig_trx_file; string _wallet_filename; wallet_data _wallet; From 8e99d7c1f7683193585082dee318e93d57e7d104 Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 21:46:05 +0800 Subject: [PATCH 18/29] simplify again according to Peter's reviews --- .../wallet/include/graphene/wallet/wallet.hpp | 9 +- libraries/wallet/wallet.cpp | 123 ++---------------- 2 files changed, 16 insertions(+), 116 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index dd6f86255f..50fda85fdc 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1600,10 +1600,9 @@ class wallet_api order_book get_order_book( const string& base, const string& quote, unsigned limit = 50); - bool multisig_mode(string on_or_off, fc::optional tx_filename); - optional multisig_import_transaction( string tx_filename ); - signed_transaction multisig_sign_transaction( const vector &wif_keys, - bool broadcast = false ); + signed_transaction multisig_sign_transaction( signed_transaction tx, + const vector &wif_keys, + bool broadcast = false ); void dbg_make_uia(string creator, string symbol); void dbg_make_mia(string creator, string symbol); @@ -1821,7 +1820,5 @@ FC_API( graphene::wallet::wallet_api, (blind_history) (receive_blind_transfer) (get_order_book) - (multisig_mode) - (multisig_import_transaction) (multisig_sign_transaction) ) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 6e314e2d60..9ccbc281b3 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -760,52 +760,9 @@ class wallet_api_impl return true; } - bool multisig_mode(string on_or_off, fc::optional tx_filename) + signed_transaction multisig_sign_transaction( signed_transaction tx, + const vector &wif_keys, bool broadcast ) { - if (on_or_off == "on") - { - if (! tx_filename) - FC_THROW("Must provide transaction file name"); - else - _multisig_trx_file = *tx_filename; - - _multisig_mode = true; - } - else - _multisig_mode = false; - - return _multisig_mode; - } - - optional multisig_import_transaction(string tx_filename) - { - signed_transaction tmp; - - if ( !fc::exists( tx_filename ) ) - { - elog( "Transaction file not exists" ); - return optional(); - } - - tmp = fc::json::from_file( tx_filename ) - .as( 2 * GRAPHENE_MAX_NESTED_OBJECTS ); - - if ( !tmp.ref_block_num || !tmp.ref_block_prefix || tmp.expiration == fc::time_point_sec() ) - { - elog( "Must fill TaPoS and expiration field in online machine" ); - return optional(); - } - else - { - _multisig_trx = tmp; - _multisig_trx_file = tx_filename; - return _multisig_trx; - } - } - - signed_transaction multisig_sign_transaction( const vector &wif_keys, bool broadcast ) - { - // use const reference to call the const variant of `sign()` vector private_keys; for ( const string &wif_key : wif_keys ) @@ -815,20 +772,20 @@ class wallet_api_impl if ( !optional_private_key ) FC_THROW( "Invalid private key: ${i}", ( "i", wif_key ) ); - private_keys.push_back(*optional_private_key); + private_keys.push_back( *optional_private_key ); } - for ( const fc::ecc::private_key& privkey : private_keys) + for ( const fc::ecc::private_key &privkey : private_keys ) { - signature_type sig = privkey.sign_compact( _multisig_trx.sig_digest( _chain_id ) ); - const vector &sigs = _multisig_trx.signatures; + signature_type sig = privkey.sign_compact( tx.sig_digest( _chain_id ) ); + const vector &sigs = tx.signatures; if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { wlog( "signature of ${fn} had already exist, skip", ( "fn", key_to_wif( privkey ) ) ); } else { - _multisig_trx.sign( privkey, _chain_id ); + tx.sign( privkey, _chain_id ); } } @@ -836,53 +793,17 @@ class wallet_api_impl { try { - _remote_net_broadcast->broadcast_transaction( _multisig_trx ); + _remote_net_broadcast->broadcast_transaction( tx ); } catch ( const fc::exception &e ) { elog( "Caught exception while broadcasting tx ${id}: ${e}", - ( "id", _multisig_trx.id().str() )( "e", e.to_detail_string() ) ); + ( "id", tx.id().str() )( "e", e.to_detail_string() ) ); throw; } } - return _multisig_trx; - } - - void save_transaction_to_file(signed_transaction& tx) - { - // - // Serialize in memory, then save to disk - // - // This approach lessens the risk of a partially written wallet - // if exceptions are thrown in serialization - // - - ilog( "saving wallet to file ${fn}", ("fn", _multisig_trx_file) ); - - string tx_data = fc::json::to_pretty_string( tx ); - try - { - enable_umask_protection(); - // - // Parentheses on the following declaration fails to compile, - // due to the Most Vexing Parse. Thanks, C++ - // - // http://en.wikipedia.org/wiki/Most_vexing_parse - // - fc::ofstream outfile{ fc::path( _multisig_trx_file ) }; - outfile.write( tx_data.c_str(), tx_data.length() ); - outfile.flush(); - outfile.close(); - disable_umask_protection(); - } - catch(...) - { - disable_umask_protection(); - throw; - } - - _multisig_trx = tx; + return tx; } void save_wallet_file(string wallet_filename = "") @@ -2035,11 +1956,6 @@ class wallet_api_impl ++expiration_time_offset; } - if (_multisig_mode) - { - save_transaction_to_file(tx); - } - if( broadcast ) { try @@ -2815,10 +2731,6 @@ class wallet_api_impl return it->second; } - bool _multisig_mode = false; - signed_transaction _multisig_trx; - string _multisig_trx_file; - string _wallet_filename; wallet_data _wallet; @@ -3815,20 +3727,11 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } -bool wallet_api::multisig_mode(string on_or_off, fc::optional tx_filename) -{ - return my->multisig_mode(on_or_off, tx_filename); -} - -optional wallet_api::multisig_import_transaction( string tx_filename ) -{ - return my->multisig_import_transaction( tx_filename ); -} - -signed_transaction wallet_api::multisig_sign_transaction( const vector &wif_keys, +signed_transaction wallet_api::multisig_sign_transaction( signed_transaction tx, + const vector &wif_keys, bool broadcast ) { - return my->multisig_sign_transaction( wif_keys, broadcast ); + return my->multisig_sign_transaction( tx, wif_keys, broadcast ); } string wallet_api::help()const From 8074954f6393b27fa725ec14c2d4a9e81801a4ac Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 21:50:10 +0800 Subject: [PATCH 19/29] correct grammer and revert unrelated code style changes --- libraries/wallet/include/graphene/wallet/wallet.hpp | 5 ++--- libraries/wallet/wallet.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 50fda85fdc..4b1d598eb3 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -252,9 +252,8 @@ struct vesting_balance_object_with_info : public vesting_balance_object fc::time_point_sec allowed_withdraw_time; }; -namespace detail -{ - class wallet_api_impl; +namespace detail { +class wallet_api_impl; } /*** diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 9ccbc281b3..668b73aff2 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -781,7 +781,7 @@ class wallet_api_impl const vector &sigs = tx.signatures; if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { - wlog( "signature of ${fn} had already exist, skip", ( "fn", key_to_wif( privkey ) ) ); + wlog( "signature of ${fn} had already exist, skipping", ( "fn", key_to_wif( privkey ) ) ); } else { From e099865fb2a2ba20cc3b13bf4ce7e940238b4898 Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 13 Jun 2018 22:58:50 +0800 Subject: [PATCH 20/29] validate TaPoS and expiration field and change error level so looks more clearer --- libraries/wallet/wallet.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 668b73aff2..0299618b08 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -765,12 +765,21 @@ class wallet_api_impl { vector private_keys; + if ( !tx.ref_block_num || !tx.ref_block_prefix || tx.expiration == fc::time_point_sec() ) + { + elog( "Must fill TaPoS and expiration field in an online machine" ); + FC_THROW(""); + } + for ( const string &wif_key : wif_keys ) { fc::optional optional_private_key = wif_to_key( wif_key ); if ( !optional_private_key ) - FC_THROW( "Invalid private key: ${i}", ( "i", wif_key ) ); + { + elog( "Invalid private key: ${i}", ( "i", wif_key ) ); + FC_THROW(""); + } private_keys.push_back( *optional_private_key ); } @@ -799,7 +808,7 @@ class wallet_api_impl { elog( "Caught exception while broadcasting tx ${id}: ${e}", ( "id", tx.id().str() )( "e", e.to_detail_string() ) ); - throw; + FC_THROW(""); } } From 0c2a7ddd864ebe39194712ad09da03497ce93400 Mon Sep 17 00:00:00 2001 From: cifer Date: Sat, 16 Jun 2018 18:52:41 +0800 Subject: [PATCH 21/29] Add test cases and remove unneeded field check --- libraries/wallet/wallet.cpp | 9 --- tests/cli/main.cpp | 127 ++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 9 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 0299618b08..02212dfe7d 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -765,12 +765,6 @@ class wallet_api_impl { vector private_keys; - if ( !tx.ref_block_num || !tx.ref_block_prefix || tx.expiration == fc::time_point_sec() ) - { - elog( "Must fill TaPoS and expiration field in an online machine" ); - FC_THROW(""); - } - for ( const string &wif_key : wif_keys ) { fc::optional optional_private_key = wif_to_key( wif_key ); @@ -968,7 +962,6 @@ class wallet_api_impl _builder_transactions.erase(handle); } - signed_transaction register_account(string name, public_key_type owner, public_key_type active, @@ -1037,7 +1030,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() ); @@ -1055,7 +1047,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 diff --git a/tests/cli/main.cpp b/tests/cli/main.cpp index 53f702aea1..5d659ce214 100644 --- a/tests/cli/main.cpp +++ b/tests/cli/main.cpp @@ -388,3 +388,130 @@ BOOST_AUTO_TEST_CASE( cli_set_voting_proxy ) } app1->shutdown(); } + +/////////////////////// +// 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 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 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 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(), (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); + + // save the private key for this new account in the wallet file + BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki2.wif_priv_key)); + con.wallet_api_ptr->save_wallet_file(con.wallet_filename); + + // 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); + transfer_tx2.set_reference_block(dyn_props.head_block_id); + transfer_tx2.set_expiration(dyn_props.time + fc::seconds(30)); + + // case1: broadcast without signature + // expect: exception with missing active authority + BOOST_CHECK_THROW(con.wallet_api_ptr->broadcast_transaction(transfer_tx2), fc::exception); + + // case2: sign with invalid private key + // expect: exception with invaid private key + BOOST_CHECK_THROW(con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {"invalid private key"}), fc::exception); + + // case3: broadcast with partial signatures + // expect: exception with missing active authority + transfer_tx2 = con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {bki2.wif_priv_key}); + BOOST_CHECK_THROW(con.wallet_api_ptr->broadcast_transaction(transfer_tx2), fc::exception); + + // case4: sign with duplicated private key + // expect: num of signatures not increase + transfer_tx2 = con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {bki2.wif_priv_key}); + BOOST_CHECK_EQUAL(transfer_tx2.signatures.size(), 1); + + // case5: broadcast without full signatures + // expect: transaction broadcast successfully + con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {bki3.wif_priv_key}, 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(); +} From edf3291dcfb7cc9033a9cab8de1f91bf8477333b Mon Sep 17 00:00:00 2001 From: cifer Date: Tue, 19 Jun 2018 00:03:17 +0800 Subject: [PATCH 22/29] partially updated per to Peter's review --- libraries/wallet/include/graphene/wallet/wallet.hpp | 12 ++++++++++++ libraries/wallet/wallet.cpp | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 4b1d598eb3..8dc5a507e3 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1599,6 +1599,18 @@ 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, this signs + * the transaction with the provided keys and optionally broadcasts the + * transaction + * + * @param tx the unsigned transaction + * @param wif_keys private keys to be used to sign the transaction + * @param broadcast true if you wish to broadcast the transaction + * + * @return the signed version of the transaction + */ signed_transaction multisig_sign_transaction( signed_transaction tx, const vector &wif_keys, bool broadcast = false ); diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 02212dfe7d..b5a84414e6 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -784,7 +784,7 @@ class wallet_api_impl const vector &sigs = tx.signatures; if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) { - wlog( "signature of ${fn} had already exist, skipping", ( "fn", key_to_wif( privkey ) ) ); + wlog( "signature of ${fn} already exist, skipping", ( "fn", public_key_type( privkey.get_public_key() ) ) ); } else { From 5ad1612d5c1c1167be35338e7b05d82009cbe227 Mon Sep 17 00:00:00 2001 From: cifer Date: Tue, 19 Jun 2018 20:16:12 +0800 Subject: [PATCH 23/29] fixed the logic of wether a private key had signed the transaction --- libraries/wallet/wallet.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index b5a84414e6..333da1d1e0 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -778,13 +778,13 @@ class wallet_api_impl private_keys.push_back( *optional_private_key ); } + const flat_set &sig_keys = tx.get_signature_keys( _chain_id ); for ( const fc::ecc::private_key &privkey : private_keys ) { - signature_type sig = privkey.sign_compact( tx.sig_digest( _chain_id ) ); - const vector &sigs = tx.signatures; - if ( std::find( sigs.begin(), sigs.end(), sig ) != sigs.end() ) + public_key_type pubkey = privkey.get_public_key(); + if ( std::find( sig_keys.begin(), sig_keys.end(), pubkey ) != sig_keys.end() ) { - wlog( "signature of ${fn} already exist, skipping", ( "fn", public_key_type( privkey.get_public_key() ) ) ); + wlog( "signature of ${fn} already exist, skipping", ( "fn", pubkey ) ); } else { From 777cf13a8e575c8a13ae682859742d0fd34c84de Mon Sep 17 00:00:00 2001 From: cifer Date: Sat, 23 Jun 2018 00:59:12 +0800 Subject: [PATCH 24/29] use private keys in wallet instead of param list and fill TaPoS/expiration fields --- .../wallet/include/graphene/wallet/wallet.hpp | 10 +-- libraries/wallet/wallet.cpp | 89 +++++++++++++------ tests/cli/main.cpp | 40 +++++---- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 8dc5a507e3..3c24465274 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1601,18 +1601,16 @@ class wallet_api /** Signs a transaction. * - * Given a fully-formed transaction with or without signatures, this signs - * the transaction with the provided keys and optionally broadcasts the - * 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 wif_keys private keys to be used to sign the transaction * @param broadcast true if you wish to broadcast the transaction * - * @return the signed version of the transaction + * @return the signed transaction */ signed_transaction multisig_sign_transaction( signed_transaction tx, - const vector &wif_keys, bool broadcast = false ); void dbg_make_uia(string creator, string symbol); diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 333da1d1e0..3224f9073b 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -760,37 +760,77 @@ class wallet_api_impl return true; } - signed_transaction multisig_sign_transaction( signed_transaction tx, - const vector &wif_keys, bool broadcast ) + signed_transaction multisig_sign_transaction( signed_transaction tx, bool broadcast ) { - vector private_keys; - - for ( const string &wif_key : wif_keys ) + set pks = _remote_db->get_potential_signatures( tx ); + flat_set 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(); + } ); + + set approving_key_set = + _remote_db->get_required_signatures( tx, owned_keys ); + + if ( ( ( tx.ref_block_num == 0 && tx.ref_block_prefix == 0 ) || + tx.expiration == fc::time_point_sec() ) && + tx.signatures.empty() ) { - fc::optional optional_private_key = wif_to_key( wif_key ); + auto dyn_props = get_dynamic_global_properties(); + tx.set_reference_block( dyn_props.head_block_id ); - if ( !optional_private_key ) + // first, some bookkeeping, expire old items from + // _recently_generated_transactions since transactions include the head + // block id, we just need the index for keeping transactions unique + // when there are multiple transactions in the same block. choose a + // time period that should be at least one block long, even in the + // worst case. 2 minutes ought to be plenty. + fc::time_point_sec oldest_transaction_ids_to_track( dyn_props.time - + fc::minutes( 2 ) ); + auto oldest_transaction_record_iter = + _recently_generated_transactions.get().lower_bound( + oldest_transaction_ids_to_track ); + auto begin_iter = + _recently_generated_transactions.get().begin(); + _recently_generated_transactions.get().erase( + begin_iter, oldest_transaction_record_iter ); + + uint32_t expiration_time_offset = 0; + for ( ;; ) { - elog( "Invalid private key: ${i}", ( "i", wif_key ) ); - FC_THROW(""); - } + tx.set_expiration( dyn_props.time + + fc::seconds( 30 + expiration_time_offset ) ); + tx.signatures.clear(); - private_keys.push_back( *optional_private_key ); - } + for ( const public_key_type &key : approving_key_set ) + tx.sign( get_private_key( key ), _chain_id ); - const flat_set &sig_keys = tx.get_signature_keys( _chain_id ); - for ( const fc::ecc::private_key &privkey : private_keys ) - { - public_key_type pubkey = privkey.get_public_key(); - if ( std::find( sig_keys.begin(), sig_keys.end(), pubkey ) != sig_keys.end() ) - { - wlog( "signature of ${fn} already exist, skipping", ( "fn", pubkey ) ); - } - else - { - tx.sign( privkey, _chain_id ); + graphene::chain::transaction_id_type this_transaction_id = tx.id(); + auto iter = + _recently_generated_transactions.find( this_transaction_id ); + if ( iter == _recently_generated_transactions.end() ) + { + // we haven't generated this transaction before, the usual case + recently_generated_transaction_record this_transaction_record; + this_transaction_record.generation_time = dyn_props.time; + this_transaction_record.transaction_id = this_transaction_id; + _recently_generated_transactions.insert( + this_transaction_record ); + break; + } + + // else we've generated a dupe, increment expiration time and + // re-sign it + ++expiration_time_offset; } } + else + { + for ( const public_key_type &key : approving_key_set ) + tx.sign( get_private_key( key ), _chain_id ); + } if ( broadcast ) { @@ -3728,10 +3768,9 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const } signed_transaction wallet_api::multisig_sign_transaction( signed_transaction tx, - const vector &wif_keys, bool broadcast ) { - return my->multisig_sign_transaction( tx, wif_keys, broadcast ); + return my->multisig_sign_transaction( tx, broadcast ); } string wallet_api::help()const diff --git a/tests/cli/main.cpp b/tests/cli/main.cpp index 5d659ce214..543a691a94 100644 --- a/tests/cli/main.cpp +++ b/tests/cli/main.cpp @@ -455,10 +455,6 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) create_multisig_acct_tx.operations.push_back(account_create_op); con.wallet_api_ptr->sign_transaction(create_multisig_acct_tx, true); - // save the private key for this new account in the wallet file - BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki2.wif_priv_key)); - con.wallet_api_ptr->save_wallet_file(con.wallet_filename); - // 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); @@ -476,30 +472,38 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) xfer_op.amount = asset(100000000); xfer_op.fee = asset(3000000); // should be enough for transfer transfer_tx2.operations.push_back(xfer_op); - transfer_tx2.set_reference_block(dyn_props.head_block_id); - transfer_tx2.set_expiration(dyn_props.time + fc::seconds(30)); - // case1: broadcast without signature + // case1: sign a transaction without TaPoS and expiration fields + // expect: return a transaction with TaPoS and expiration filled + transfer_tx2 = + con.wallet_api_ptr->multisig_sign_transaction( 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); - // case2: sign with invalid private key - // expect: exception with invaid private key - BOOST_CHECK_THROW(con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {"invalid private key"}), fc::exception); - - // case3: broadcast with partial signatures + // 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 - transfer_tx2 = con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {bki2.wif_priv_key}); - BOOST_CHECK_THROW(con.wallet_api_ptr->broadcast_transaction(transfer_tx2), fc::exception); + BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki2.wif_priv_key)); + BOOST_CHECK_THROW(con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, true), fc::exception); - // case4: sign with duplicated private key + // case4: sign again as signature exists // expect: num of signatures not increase - transfer_tx2 = con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {bki2.wif_priv_key}); + transfer_tx2 = con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, false); BOOST_CHECK_EQUAL(transfer_tx2.signatures.size(), 1); - // case5: broadcast without full signatures + // case5: + // import another private key, sign and broadcast without full signatures + // // expect: transaction broadcast successfully - con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, {bki3.wif_priv_key}, true); + BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki3.wif_priv_key)); + con.wallet_api_ptr->multisig_sign_transaction(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()) { From 3a3993805e80bb20914e44babb180ff742b270e5 Mon Sep 17 00:00:00 2001 From: cifer Date: Sun, 24 Jun 2018 23:41:55 +0800 Subject: [PATCH 25/29] extend expiration and remove dupe check --- libraries/wallet/wallet.cpp | 58 ++++--------------------------------- 1 file changed, 6 insertions(+), 52 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 3224f9073b..bc32cb7330 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -760,7 +760,8 @@ class wallet_api_impl return true; } - signed_transaction multisig_sign_transaction( signed_transaction tx, bool broadcast ) + signed_transaction multisig_sign_transaction( signed_transaction tx, + bool broadcast ) { set pks = _remote_db->get_potential_signatures( tx ); flat_set owned_keys; @@ -780,57 +781,10 @@ class wallet_api_impl { auto dyn_props = get_dynamic_global_properties(); tx.set_reference_block( dyn_props.head_block_id ); - - // first, some bookkeeping, expire old items from - // _recently_generated_transactions since transactions include the head - // block id, we just need the index for keeping transactions unique - // when there are multiple transactions in the same block. choose a - // time period that should be at least one block long, even in the - // worst case. 2 minutes ought to be plenty. - fc::time_point_sec oldest_transaction_ids_to_track( dyn_props.time - - fc::minutes( 2 ) ); - auto oldest_transaction_record_iter = - _recently_generated_transactions.get().lower_bound( - oldest_transaction_ids_to_track ); - auto begin_iter = - _recently_generated_transactions.get().begin(); - _recently_generated_transactions.get().erase( - begin_iter, oldest_transaction_record_iter ); - - uint32_t expiration_time_offset = 0; - for ( ;; ) - { - tx.set_expiration( dyn_props.time + - fc::seconds( 30 + expiration_time_offset ) ); - tx.signatures.clear(); - - for ( const public_key_type &key : approving_key_set ) - tx.sign( get_private_key( key ), _chain_id ); - - graphene::chain::transaction_id_type this_transaction_id = tx.id(); - auto iter = - _recently_generated_transactions.find( this_transaction_id ); - if ( iter == _recently_generated_transactions.end() ) - { - // we haven't generated this transaction before, the usual case - recently_generated_transaction_record this_transaction_record; - this_transaction_record.generation_time = dyn_props.time; - this_transaction_record.transaction_id = this_transaction_id; - _recently_generated_transactions.insert( - this_transaction_record ); - break; - } - - // else we've generated a dupe, increment expiration time and - // re-sign it - ++expiration_time_offset; - } - } - else - { - for ( const public_key_type &key : approving_key_set ) - tx.sign( get_private_key( key ), _chain_id ); + tx.set_expiration( fc::time_point_sec::maximum() ); } + for ( const public_key_type &key : approving_key_set ) + tx.sign( get_private_key( key ), _chain_id ); if ( broadcast ) { @@ -842,7 +796,7 @@ class wallet_api_impl { elog( "Caught exception while broadcasting tx ${id}: ${e}", ( "id", tx.id().str() )( "e", e.to_detail_string() ) ); - FC_THROW(""); + FC_THROW( "" ); } } From f61cb7540699635e0860765ca23136309b7d5d7f Mon Sep 17 00:00:00 2001 From: cifer Date: Sun, 22 Jul 2018 23:04:44 +0800 Subject: [PATCH 26/29] get max transaction expiration time from global property object --- libraries/wallet/wallet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index bc32cb7330..3d6cbc6b7f 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -780,8 +780,10 @@ class wallet_api_impl tx.signatures.empty() ) { auto dyn_props = get_dynamic_global_properties(); + auto props = get_global_properties(); tx.set_reference_block( dyn_props.head_block_id ); - tx.set_expiration( fc::time_point_sec::maximum() ); + tx.set_expiration( fc::time_point_sec( + props.parameters.maximum_time_until_expiration ) ); } for ( const public_key_type &key : approving_key_set ) tx.sign( get_private_key( key ), _chain_id ); From 0a30317436a135363e64f222a0cb713e2c316e0d Mon Sep 17 00:00:00 2001 From: cifer Date: Wed, 25 Jul 2018 00:46:25 +0800 Subject: [PATCH 27/29] fix transaction expiration to absolute time from now --- libraries/wallet/wallet.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 3d6cbc6b7f..04eee17768 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -780,10 +780,10 @@ class wallet_api_impl tx.signatures.empty() ) { auto dyn_props = get_dynamic_global_properties(); - auto props = get_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( fc::time_point_sec( - props.parameters.maximum_time_until_expiration ) ); + 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 ); From 6cf8fed729b1c70b642178e86ebfda08ce65f665 Mon Sep 17 00:00:00 2001 From: Cifer Date: Tue, 9 Oct 2018 14:03:10 +0800 Subject: [PATCH 28/29] refactor redundent codes and change name to sign_transaction2 --- .../wallet/include/graphene/wallet/wallet.hpp | 4 +- libraries/wallet/wallet.cpp | 43 +++++++++++++------ tests/cli/main.cpp | 8 ++-- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 3c24465274..a8eaca7dcd 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1610,7 +1610,7 @@ class wallet_api * * @return the signed transaction */ - signed_transaction multisig_sign_transaction( signed_transaction tx, + signed_transaction sign_transaction2( signed_transaction tx, bool broadcast = false ); void dbg_make_uia(string creator, string symbol); @@ -1801,6 +1801,7 @@ FC_API( graphene::wallet::wallet_api, (save_wallet_file) (serialize_transaction) (sign_transaction) + (sign_transaction2) (get_prototype_operation) (propose_parameter_change) (propose_fee_change) @@ -1829,5 +1830,4 @@ FC_API( graphene::wallet::wallet_api, (blind_history) (receive_blind_transfer) (get_order_book) - (multisig_sign_transaction) ) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 04eee17768..a1187a0b1d 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -760,8 +760,23 @@ class wallet_api_impl return true; } - signed_transaction multisig_sign_transaction( signed_transaction tx, - bool broadcast ) + /** + * 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 erase and + * all required keys returned. + */ + set get_owned_required_keys( signed_transaction &tx, + bool erase_existing_sigs = true) { set pks = _remote_db->get_potential_signatures( tx ); flat_set owned_keys; @@ -772,8 +787,17 @@ class wallet_api_impl return _keys.find( pk ) != _keys.end(); } ); + if ( erase_existing_sigs ) + tx.signatures.clear(); + + return _remote_db->get_required_signatures( tx, owned_keys ); + } + + signed_transaction sign_transaction2( signed_transaction tx, + bool broadcast ) + { set approving_key_set = - _remote_db->get_required_signatures( tx, owned_keys ); + std::move( get_owned_required_keys( tx, false ) ); if ( ( ( tx.ref_block_num == 0 && tx.ref_block_prefix == 0 ) || tx.expiration == fc::time_point_sec() ) && @@ -1907,13 +1931,8 @@ class wallet_api_impl signed_transaction sign_transaction(signed_transaction tx, bool broadcast = false) { - set pks = _remote_db->get_potential_signatures( tx ); - flat_set 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.signatures.clear(); - set approving_key_set = _remote_db->get_required_signatures( tx, owned_keys ); + set approving_key_set = + std::move( get_owned_required_keys( tx ) ); auto dyn_props = get_dynamic_global_properties(); tx.set_reference_block( dyn_props.head_block_id ); @@ -3723,10 +3742,10 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } -signed_transaction wallet_api::multisig_sign_transaction( signed_transaction tx, +signed_transaction wallet_api::sign_transaction2( signed_transaction tx, bool broadcast ) { - return my->multisig_sign_transaction( tx, broadcast ); + return my->sign_transaction2( tx, broadcast ); } string wallet_api::help()const diff --git a/tests/cli/main.cpp b/tests/cli/main.cpp index 543a691a94..378bc311cd 100644 --- a/tests/cli/main.cpp +++ b/tests/cli/main.cpp @@ -476,7 +476,7 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) // case1: sign a transaction without TaPoS and expiration fields // expect: return a transaction with TaPoS and expiration filled transfer_tx2 = - con.wallet_api_ptr->multisig_sign_transaction( transfer_tx2, false ); + con.wallet_api_ptr->sign_transaction2( transfer_tx2, false ); BOOST_CHECK( ( transfer_tx2.ref_block_num != 0 && transfer_tx2.ref_block_prefix != 0 ) || ( transfer_tx2.expiration != fc::time_point_sec() ) ); @@ -491,11 +491,11 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) // // 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->multisig_sign_transaction(transfer_tx2, true), fc::exception); + BOOST_CHECK_THROW(con.wallet_api_ptr->sign_transaction2(transfer_tx2, true), fc::exception); // case4: sign again as signature exists // expect: num of signatures not increase - transfer_tx2 = con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, false); + transfer_tx2 = con.wallet_api_ptr->sign_transaction2(transfer_tx2, false); BOOST_CHECK_EQUAL(transfer_tx2.signatures.size(), 1); // case5: @@ -503,7 +503,7 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) // // expect: transaction broadcast successfully BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki3.wif_priv_key)); - con.wallet_api_ptr->multisig_sign_transaction(transfer_tx2, true); + con.wallet_api_ptr->sign_transaction2(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()) { From e3a3f25b5fb4af7b45f14a7930a5bf2faed8fd44 Mon Sep 17 00:00:00 2001 From: lixiaodongcifer Date: Sat, 16 Feb 2019 14:54:21 +0800 Subject: [PATCH 29/29] Fix issues according to review comments --- .../wallet/include/graphene/wallet/wallet.hpp | 4 ++-- libraries/wallet/wallet.cpp | 16 +++++++--------- tests/cli/main.cpp | 8 ++++---- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index a8eaca7dcd..97191010b4 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -1610,7 +1610,7 @@ class wallet_api * * @return the signed transaction */ - signed_transaction sign_transaction2( signed_transaction tx, + signed_transaction add_transaction_signature( signed_transaction tx, bool broadcast = false ); void dbg_make_uia(string creator, string symbol); @@ -1801,7 +1801,7 @@ FC_API( graphene::wallet::wallet_api, (save_wallet_file) (serialize_transaction) (sign_transaction) - (sign_transaction2) + (add_transaction_signature) (get_prototype_operation) (propose_parameter_change) (propose_fee_change) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index a1187a0b1d..36773b73b3 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -772,7 +772,7 @@ class wallet_api_impl * 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 erase and + * If set to true, the existing signatures will be erased and * all required keys returned. */ set get_owned_required_keys( signed_transaction &tx, @@ -793,11 +793,10 @@ class wallet_api_impl return _remote_db->get_required_signatures( tx, owned_keys ); } - signed_transaction sign_transaction2( signed_transaction tx, + signed_transaction add_transaction_signature( signed_transaction tx, bool broadcast ) { - set approving_key_set = - std::move( get_owned_required_keys( tx, false ) ); + set 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() ) && @@ -822,7 +821,7 @@ class wallet_api_impl { elog( "Caught exception while broadcasting tx ${id}: ${e}", ( "id", tx.id().str() )( "e", e.to_detail_string() ) ); - FC_THROW( "" ); + FC_THROW( "Caught exception while broadcasting tx" ); } } @@ -1931,8 +1930,7 @@ class wallet_api_impl signed_transaction sign_transaction(signed_transaction tx, bool broadcast = false) { - set approving_key_set = - std::move( get_owned_required_keys( tx ) ); + set approving_key_set = get_owned_required_keys(tx); auto dyn_props = get_dynamic_global_properties(); tx.set_reference_block( dyn_props.head_block_id ); @@ -3742,10 +3740,10 @@ dynamic_global_property_object wallet_api::get_dynamic_global_properties() const return my->get_dynamic_global_properties(); } -signed_transaction wallet_api::sign_transaction2( signed_transaction tx, +signed_transaction wallet_api::add_transaction_signature( signed_transaction tx, bool broadcast ) { - return my->sign_transaction2( tx, broadcast ); + return my->add_transaction_signature( tx, broadcast ); } string wallet_api::help()const diff --git a/tests/cli/main.cpp b/tests/cli/main.cpp index 378bc311cd..60aaf9d28f 100644 --- a/tests/cli/main.cpp +++ b/tests/cli/main.cpp @@ -476,7 +476,7 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) // case1: sign a transaction without TaPoS and expiration fields // expect: return a transaction with TaPoS and expiration filled transfer_tx2 = - con.wallet_api_ptr->sign_transaction2( transfer_tx2, false ); + 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() ) ); @@ -491,11 +491,11 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) // // 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->sign_transaction2(transfer_tx2, true), fc::exception); + 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->sign_transaction2(transfer_tx2, false); + transfer_tx2 = con.wallet_api_ptr->add_transaction_signature(transfer_tx2, false); BOOST_CHECK_EQUAL(transfer_tx2.signatures.size(), 1); // case5: @@ -503,7 +503,7 @@ BOOST_AUTO_TEST_CASE( cli_multisig_transaction ) // // expect: transaction broadcast successfully BOOST_CHECK(con.wallet_api_ptr->import_key("cifer.test", bki3.wif_priv_key)); - con.wallet_api_ptr->sign_transaction2(transfer_tx2, true); + 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()) {