Merge pull request #2161 from bitshares/jmj_bsip_64_pt2

Fix line length, add preimage to HTLC virtual operation
bitshares · Apr 28, 2020 · ca06127 · ca06127
2 parents bb8493d + ccef9fd
commit ca06127
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 54 deletions.
diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp
@@ -61,7 +61,8 @@ namespace graphene {
             // attempted on an HTLC with a 0 preimage size before the hardfork date.
             if ( htlc_obj->conditions.hash_lock.preimage_size > 0U || 
                   block_time < HARDFORK_CORE_BSIP64_TIME )
-               FC_ASSERT(op.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size, "Preimage size mismatch.");
+               FC_ASSERT(op.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size, 
+                     "Preimage size mismatch.");
          }
       } // end of graphene::chain::details
 
@@ -78,9 +79,11 @@ namespace graphene {
          FC_ASSERT(htlc_options, "HTLC Committee options are not set.");
 
          // make sure the expiration is reasonable
-         FC_ASSERT( o.claim_period_seconds <= htlc_options->max_timeout_secs, "HTLC Timeout exceeds allowed length" );
+         FC_ASSERT( o.claim_period_seconds <= htlc_options->max_timeout_secs, 
+               "HTLC Timeout exceeds allowed length" );
          // make sure the preimage length is reasonable
-         FC_ASSERT( o.preimage_size <= htlc_options->max_preimage_size, "HTLC preimage length exceeds allowed length" );
+         FC_ASSERT( o.preimage_size <= htlc_options->max_preimage_size, 
+               "HTLC preimage length exceeds allowed length" );
          // make sure the sender has the funds for the HTLC
          FC_ASSERT( d.get_balance( o.from, o.amount.asset_id) >= (o.amount), "Insufficient funds") ;
          const auto& asset_to_transfer = o.amount.asset_id( d );
@@ -154,7 +157,8 @@ namespace graphene {
          db().adjust_balance(htlc_obj->transfer.to, amount);
          // notify related parties
          htlc_redeemed_operation virt_op( htlc_obj->id, htlc_obj->transfer.from, htlc_obj->transfer.to, o.redeemer,
-               amount, htlc_obj->conditions.hash_lock.preimage_hash, htlc_obj->conditions.hash_lock.preimage_size );
+               amount, htlc_obj->conditions.hash_lock.preimage_hash, htlc_obj->conditions.hash_lock.preimage_size,
+               o.preimage );
          db().push_applied_operation( virt_op );
          db().remove(*htlc_obj);
          return void_result();

diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp
@@ -63,7 +63,8 @@ struct proposal_operation_hardfork_visitor
 
    void operator()(const graphene::chain::committee_member_update_global_parameters_operation &op) const {
       if (block_time < HARDFORK_CORE_1468_TIME) {
-         FC_ASSERT(!op.new_parameters.extensions.value.updatable_htlc_options.valid(), "Unable to set HTLC options before hardfork 1468");
+         FC_ASSERT(!op.new_parameters.extensions.value.updatable_htlc_options.valid(), 
+               "Unable to set HTLC options before hardfork 1468");
          FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_create_operation>());
          FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_redeem_operation>());
          FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_extend_operation>());
@@ -126,7 +127,8 @@ struct proposal_operation_hardfork_visitor
          // Do not allow more than 1 proposal_update in a proposal
          if ( op.op.is_type<proposal_update_operation>() )
          {
-            FC_ASSERT( !already_contains_proposal_update, "At most one proposal update can be nested in a proposal!" );
+            FC_ASSERT( !already_contains_proposal_update, 
+                  "At most one proposal update can be nested in a proposal!" );
             already_contains_proposal_update = true;
          }
       }
@@ -188,8 +190,9 @@ void_result proposal_create_evaluator::do_evaluate( const proposal_create_operat
    FC_ASSERT( o.expiration_time > block_time, "Proposal has already expired on creation." );
    FC_ASSERT( o.expiration_time <= block_time + global_parameters.maximum_proposal_lifetime,
               "Proposal expiration time is too far in the future." );
-   FC_ASSERT( !o.review_period_seconds || fc::seconds( *o.review_period_seconds ) < ( o.expiration_time - block_time ),
-              "Proposal review period must be less than its overall lifetime." );
+   FC_ASSERT( !o.review_period_seconds || 
+         fc::seconds( *o.review_period_seconds ) < ( o.expiration_time - block_time ),
+         "Proposal review period must be less than its overall lifetime." );
 
    // Find all authorities required by the proposed operations
    flat_set<account_id_type> tmp_required_active_auths;
@@ -199,8 +202,8 @@ void_result proposal_create_evaluator::do_evaluate( const proposal_create_operat
       operation_get_required_authorities( op.op, tmp_required_active_auths, _required_owner_auths, other,
                                           MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( block_time ) );
    }
-   // All accounts which must provide both owner and active authority should be omitted from the active authority set;
-   // owner authority approval implies active authority approval.
+   // All accounts which must provide both owner and active authority should be omitted from the 
+   // active authority set; owner authority approval implies active authority approval.
    std::set_difference( tmp_required_active_auths.begin(), tmp_required_active_auths.end(),
                         _required_owner_auths.begin(), _required_owner_auths.end(),
                         std::inserter( _required_active_auths, _required_active_auths.begin() ) );
@@ -295,9 +298,9 @@ void_result proposal_update_evaluator::do_apply(const proposal_update_operation&
 { try {
    database& d = db();
 
-   // Potential optimization: if _executed_proposal is true, we can skip the modify step and make push_proposal skip
-   // signature checks. This isn't done now because I just wrote all the proposals code, and I'm not yet 100% sure the
-   // required approvals are sufficient to authorize the transaction.
+   // Potential optimization: if _executed_proposal is true, we can skip the modify step and make push_proposal 
+   // skip signature checks. This isn't done now because I just wrote all the proposals code, and I'm not yet 
+   // 100% sure the required approvals are sufficient to authorize the transaction.
    d.modify(*_proposal, [&o](proposal_object& p) {
       p.available_active_approvals.insert(o.active_approvals_to_add.begin(), o.active_approvals_to_add.end());
       p.available_owner_approvals.insert(o.owner_approvals_to_add.begin(), o.owner_approvals_to_add.end());

diff --git a/libraries/protocol/htlc.cpp b/libraries/protocol/htlc.cpp
@@ -34,7 +34,8 @@ namespace graphene { namespace protocol {
       FC_ASSERT( amount.amount > 0, "HTLC amount should be greater than zero" );
    }
 
-   share_type htlc_create_operation::calculate_fee( const fee_parameters_type& fee_params, uint32_t fee_per_kb )const
+   share_type htlc_create_operation::calculate_fee( const fee_parameters_type& fee_params, 
+         uint32_t fee_per_kb )const
    {
       uint64_t days = ( claim_period_seconds + SECONDS_PER_DAY - 1 ) / SECONDS_PER_DAY;
       // multiply with overflow check

diff --git a/libraries/protocol/include/graphene/protocol/htlc.hpp b/libraries/protocol/include/graphene/protocol/htlc.hpp
@@ -130,9 +130,10 @@ namespace graphene { namespace protocol {
 
          htlc_redeemed_operation() {}
          htlc_redeemed_operation( htlc_id_type htlc_id, account_id_type from, account_id_type to,
-               account_id_type redeemer, asset amount, const htlc_hash& preimage_hash, uint16_t preimage_size ) :
+               account_id_type redeemer, asset amount, const htlc_hash& preimage_hash, uint16_t preimage_size,
+               const std::vector<char>& preimage ) :
                htlc_id(htlc_id), from(from), to(to), redeemer(redeemer), amount(amount),
-               htlc_preimage_hash(preimage_hash), htlc_preimage_size(preimage_size) {}
+               htlc_preimage_hash(preimage_hash), htlc_preimage_size(preimage_size), preimage(preimage) {}
 
          account_id_type fee_payer()const { return to; }
          void validate()const { FC_ASSERT( !"virtual operation" ); }

diff --git a/libraries/wallet/operation_printer.cpp b/libraries/wallet/operation_printer.cpp
@@ -95,13 +95,13 @@ void operation_printer::print_preimage(const std::vector<char>& preimage)const
    if (preimage.size() == 0)
       return;
    out << " with preimage \"";
-   // cut it at 50 bytes max
+   // cut it at 300 bytes max
    auto flags = out.flags();
    out << std::hex << setw(2) << setfill('0');
-   for (size_t i = 0; i < std::min<size_t>(50, preimage.size()); i++)
+   for (size_t i = 0; i < std::min<size_t>(300, preimage.size()); i++)
       out << +preimage[i];
    out.flags(flags);
-   if (preimage.size() > 50)
+   if (preimage.size() > 300)
       out << "...(truncated due to size)";
    out << "\"";
 }

diff --git a/libraries/wallet/wallet_api_impl.hpp b/libraries/wallet/wallet_api_impl.hpp
@@ -220,8 +220,7 @@ class wallet_api_impl
    transaction preview_builder_transaction(transaction_handle_type handle);
    signed_transaction sign_builder_transaction(transaction_handle_type transaction_handle, bool broadcast = true);
    signed_transaction sign_builder_transaction2(transaction_handle_type transaction_handle,
-                                               const vector<public_key_type>& signing_keys = vector<public_key_type>(),
-                                               bool broadcast = true);
+         const vector<public_key_type>& signing_keys = vector<public_key_type>(), bool broadcast = true);
 
    pair<transaction_id_type,signed_transaction> broadcast_transaction(signed_transaction tx);
 
@@ -302,7 +301,8 @@ class wallet_api_impl
          string hash_algorithm, const std::string& preimage_hash, uint32_t preimage_size,
          const uint32_t claim_period_seconds, const std::string& memo, bool broadcast = false );
 
-   signed_transaction htlc_redeem( string htlc_id, string issuer, const std::vector<char>& preimage, bool broadcast );
+   signed_transaction htlc_redeem( string htlc_id, string issuer, const std::vector<char>& preimage, 
+         bool broadcast );
 
    signed_transaction htlc_extend ( string htlc_id, string issuer, const uint32_t seconds_to_add, bool broadcast);
 

diff --git a/libraries/wallet/wallet_transfer.cpp b/libraries/wallet/wallet_transfer.cpp
@@ -83,8 +83,8 @@ namespace graphene { namespace wallet { namespace detail {
       return sign_transaction(tx, broadcast);
    } FC_CAPTURE_AND_RETHROW( (from)(to)(amount)(asset_symbol)(memo)(broadcast) ) }
 
-   signed_transaction wallet_api_impl::htlc_create( string source, string destination, string amount, string asset_symbol,
-         string hash_algorithm, const std::string& preimage_hash, uint32_t preimage_size,
+   signed_transaction wallet_api_impl::htlc_create( string source, string destination, string amount,
+         string asset_symbol, string hash_algorithm, const std::string& preimage_hash, uint32_t preimage_size,
          const uint32_t claim_period_seconds, const std::string& memo, bool broadcast )
    {
       try
@@ -122,8 +122,8 @@ namespace graphene { namespace wallet { namespace detail {
             (preimage_hash)(preimage_size)(claim_period_seconds)(broadcast) )
    }
 
-   signed_transaction wallet_api_impl::htlc_redeem( string htlc_id, string issuer, const std::vector<char>& preimage, 
-         bool broadcast )
+   signed_transaction wallet_api_impl::htlc_redeem( string htlc_id, string issuer,
+         const std::vector<char>& preimage, bool broadcast )
    {
       try
       {
@@ -147,7 +147,7 @@ namespace graphene { namespace wallet { namespace detail {
       } FC_CAPTURE_AND_RETHROW( (htlc_id)(issuer)(preimage)(broadcast) )
    }
 
-   signed_transaction wallet_api_impl::htlc_extend ( string htlc_id, string issuer, const uint32_t seconds_to_add, 
+   signed_transaction wallet_api_impl::htlc_extend ( string htlc_id, string issuer, const uint32_t seconds_to_add,
          bool broadcast)
    {
       try

diff --git a/tests/cli/main.cpp b/tests/cli/main.cpp
@@ -1601,7 +1601,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
       int server_port_number = 0;
       app1 = start_application(app_dir, server_port_number);
       // set committee parameters
-      app1->chain_database()->modify(app1->chain_database()->get_global_properties(), [](global_property_object& p) {
+      app1->chain_database()->modify(app1->chain_database()->get_global_properties(), [](global_property_object& p)
+      {
          graphene::chain::htlc_options params;
          params.max_preimage_size = 1024;
          params.max_timeout_secs = 60 * 60 * 24 * 28;
@@ -1634,7 +1635,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
       account_object nathan_acct_after_upgrade = con.wallet_api_ptr->get_account("nathan");
 
       // verify that the upgrade was successful
-      BOOST_CHECK_PREDICATE( std::not_equal_to<uint32_t>(), (nathan_acct_before_upgrade.membership_expiration_date.sec_since_epoch())
+      BOOST_CHECK_PREDICATE( std::not_equal_to<uint32_t>(), 
+            (nathan_acct_before_upgrade.membership_expiration_date.sec_since_epoch())
             (nathan_acct_after_upgrade.membership_expiration_date.sec_since_epoch()) );
       BOOST_CHECK(nathan_acct_after_upgrade.is_lifetime_member());
 
@@ -1660,8 +1662,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
       {
          graphene::wallet::brain_key_info bki = con.wallet_api_ptr->suggest_brain_key();
          BOOST_CHECK(!bki.brain_priv_key.empty());
-         signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key, "alice", 
-               "nathan", "nathan", true);
+         signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key,
+               "alice", "nathan", "nathan", true);
          con.wallet_api_ptr->save_wallet_file(con.wallet_filename);
          // attempt to give alice some bitshares
          BOOST_TEST_MESSAGE("Transferring bitshares from Nathan to alice");
@@ -1673,8 +1675,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
       {
          graphene::wallet::brain_key_info bki = con.wallet_api_ptr->suggest_brain_key();
          BOOST_CHECK(!bki.brain_priv_key.empty());
-         signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key, "bob", 
-               "nathan", "nathan", true);
+         signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key,
+               "bob", "nathan", "nathan", true);
          // this should cause resync which will import the keys of alice and bob
          generate_block(app1);
          // attempt to give bob some bitshares
@@ -1710,7 +1712,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
          BOOST_CHECK(generate_block(app1, result_block));
 
          // get the ID:
-         htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1].operation_results[0].get<object_id_type>();
+         htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1]
+               .operation_results[0].get<object_id_type>();
          alice_htlc_id_as_string = (std::string)(object_id_type)htlc_id;
          BOOST_TEST_MESSAGE("Alice shares the HTLC ID with Bob. The HTLC ID is: " + alice_htlc_id_as_string);
       }
@@ -1722,7 +1725,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
 
       // Bob likes what he sees, so he creates an HTLC, using the info he retrieved from Alice's HTLC
       con.wallet_api_ptr->htlc_create("bob", "alice",
-            "3", "BOBCOIN", "HASH160", hash_str, preimage_string.size(), fc::hours(12).to_seconds(), "Bob to Alice", true);
+            "3", "BOBCOIN", "HASH160", hash_str, preimage_string.size(), fc::hours(12).to_seconds(),
+            "Bob to Alice", true);
 
       // normally, a wallet would watch block production, and find the transaction. Here, we can cheat:
       std::string bob_htlc_id_as_string;
@@ -1732,7 +1736,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
          BOOST_CHECK(generate_block(app1, result_block));
 
          // get the ID:
-         htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1].operation_results[0].get<object_id_type>();
+         htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1]
+               .operation_results[0].get<object_id_type>();
          bob_htlc_id_as_string = (std::string)(object_id_type)htlc_id;
          BOOST_TEST_MESSAGE("Bob shares the HTLC ID with Alice. The HTLC ID is: " + bob_htlc_id_as_string);
       }
@@ -1750,7 +1755,20 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
          BOOST_CHECK(generate_block(app1));
       }
 
-      // TODO: Bob can look at Alice's history to see her preimage
+      // Bob can look at Alice's history to see her preimage
+      {
+         BOOST_TEST_MESSAGE("Bob can look at the history of Alice to see the preimage");
+         std::vector<graphene::wallet::operation_detail> hist = con.wallet_api_ptr->get_account_history("alice", 1);
+         BOOST_CHECK( hist[0].description.find("with preimage \"4d792") != hist[0].description.npos);
+      }
+
+      // Bob can also look at his own history to see Alice's preimage
+      {
+         BOOST_TEST_MESSAGE("Bob can look at his own history to see the preimage");
+         std::vector<graphene::wallet::operation_detail> hist = con.wallet_api_ptr->get_account_history("bob", 1);
+         BOOST_CHECK( hist[0].description.find("with preimage \"4d792") != hist[0].description.npos);
+      }
+
       // Bob can use the preimage to retrieve his BTS
       {
          BOOST_TEST_MESSAGE("Bob uses Alice's preimage to retrieve the BOBCOIN");