From 626c5720d45a29d019dbef7723e3f9295046fe2c Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 4 Apr 2019 06:23:36 -0500 Subject: [PATCH 1/7] Add htlc_extend validation --- libraries/chain/htlc_evaluator.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp index 9bd71e630b..61eed3db1e 100644 --- a/libraries/chain/htlc_evaluator.cpp +++ b/libraries/chain/htlc_evaluator.cpp @@ -123,6 +123,9 @@ namespace graphene { void_result htlc_extend_evaluator::do_evaluate(const htlc_extend_operation& o) { htlc_obj = &db().get(o.htlc_id); + FC_ASSERT(o.fee_payer() == htlc_obj->transfer.from, "HTLC may only be extended by its creator."); + FC_ASSERT(o.seconds_to_add < fc::time_point_sec::maximum().sec_since_epoch() + - htlc_obj->conditions.time_lock.expiration.sec_since_epoch(), "Invalid number of seconds" ); return void_result(); } From 69f5aa79e8ac751425756718a43328d3feef778c Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 4 Apr 2019 07:32:44 -0500 Subject: [PATCH 2/7] Added tests for htlc_extend --- libraries/chain/htlc_evaluator.cpp | 4 +-- tests/tests/htlc_tests.cpp | 52 +++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp index 61eed3db1e..ea9f051588 100644 --- a/libraries/chain/htlc_evaluator.cpp +++ b/libraries/chain/htlc_evaluator.cpp @@ -123,8 +123,8 @@ namespace graphene { void_result htlc_extend_evaluator::do_evaluate(const htlc_extend_operation& o) { htlc_obj = &db().get(o.htlc_id); - FC_ASSERT(o.fee_payer() == htlc_obj->transfer.from, "HTLC may only be extended by its creator."); - FC_ASSERT(o.seconds_to_add < fc::time_point_sec::maximum().sec_since_epoch() + FC_ASSERT(o.update_issuer == htlc_obj->transfer.from, "HTLC may only be extended by its creator."); + FC_ASSERT(o.seconds_to_add <= fc::time_point_sec::maximum().sec_since_epoch() - htlc_obj->conditions.time_lock.expiration.sec_since_epoch(), "Invalid number of seconds" ); return void_result(); } diff --git a/tests/tests/htlc_tests.cpp b/tests/tests/htlc_tests.cpp index 826cc7e1a2..1595ea6a0a 100644 --- a/tests/tests/htlc_tests.cpp +++ b/tests/tests/htlc_tests.cpp @@ -210,10 +210,60 @@ try { auto obj = db_api.get_objects( {alice_htlc_id }).front(); graphene::chain::htlc_object htlc = obj.template as(GRAPHENE_MAX_NESTED_OBJECTS); + // someone else attempts to extend it (bob says he's alice, but he's not) + { + graphene::chain::htlc_extend_operation bad_extend; + bad_extend.htlc_id = alice_htlc_id; + bad_extend.seconds_to_add = 10; + bad_extend.fee = db.get_global_properties().parameters.current_fees->calculate_fee(bad_extend); + bad_extend.update_issuer = alice_id; + trx.operations.push_back(bad_extend); + sign(trx, bob_private_key); + GRAPHENE_CHECK_THROW( PUSH_TX(db, trx, database::skip_nothing ), fc::exception ); + trx.clear(); + } + // someone else attempts to extend it (bob wants to extend Alice's contract) + { + graphene::chain::htlc_extend_operation bad_extend; + bad_extend.htlc_id = alice_htlc_id; + bad_extend.seconds_to_add = 10; + bad_extend.fee = db.get_global_properties().parameters.current_fees->calculate_fee(bad_extend); + bad_extend.update_issuer = bob_id; + trx.operations.push_back(bad_extend); + sign(trx, bob_private_key); + GRAPHENE_CHECK_THROW( PUSH_TX(db, trx, ~0 ), fc::exception ); + trx.clear(); + } + // attempt to extend it with too much time + { + graphene::chain::htlc_extend_operation big_extend; + big_extend.htlc_id = alice_htlc_id; + big_extend.seconds_to_add = (fc::time_point_sec::maximum() - 59).sec_since_epoch(); + big_extend.fee = db.get_global_properties().parameters.current_fees->calculate_fee(big_extend); + big_extend.update_issuer = alice_id; + trx.operations.push_back(big_extend); + sign(trx, alice_private_key); + GRAPHENE_CHECK_THROW( PUSH_TX(db, trx, ~0), fc::exception ); + trx.clear(); + } + + // attempt to extend properly + { + graphene::chain::htlc_extend_operation extend; + extend.htlc_id = alice_htlc_id; + extend.seconds_to_add = 10; + extend.fee = db.get_global_properties().parameters.current_fees->calculate_fee(extend); + extend.update_issuer = alice_id; + trx.operations.push_back(extend); + sign(trx, alice_private_key); + PUSH_TX(db, trx, ~0); + trx.clear(); + } + // let it expire (wait for timeout) generate_blocks( db.head_block_time() + fc::seconds(120) ); // verify funds return (minus the fees) - BOOST_CHECK_EQUAL( get_balance(alice_id, graphene::chain::asset_id_type()), 96 * GRAPHENE_BLOCKCHAIN_PRECISION ); + BOOST_CHECK_EQUAL( get_balance(alice_id, graphene::chain::asset_id_type()), 92 * GRAPHENE_BLOCKCHAIN_PRECISION ); // verify Bob cannot execute the contract after the fact } FC_LOG_AND_RETHROW() } From 24ffef2f2f07fe679942c342bec8c56969f26182 Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 4 Apr 2019 14:13:13 -0500 Subject: [PATCH 3/7] Correctly validate HTLC extension date --- libraries/chain/htlc_evaluator.cpp | 7 +++++-- tests/tests/htlc_tests.cpp | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp index ea9f051588..9fd9bbbc8e 100644 --- a/libraries/chain/htlc_evaluator.cpp +++ b/libraries/chain/htlc_evaluator.cpp @@ -124,8 +124,11 @@ namespace graphene { { htlc_obj = &db().get(o.htlc_id); FC_ASSERT(o.update_issuer == htlc_obj->transfer.from, "HTLC may only be extended by its creator."); - FC_ASSERT(o.seconds_to_add <= fc::time_point_sec::maximum().sec_since_epoch() - - htlc_obj->conditions.time_lock.expiration.sec_since_epoch(), "Invalid number of seconds" ); + optional htlc_options = get_committee_htlc_options(db()); + FC_ASSERT( htlc_obj->conditions.time_lock.expiration.sec_since_epoch() + + static_cast(o.seconds_to_add) < fc::time_point_sec::maximum().sec_since_epoch(), "Extension would cause an invalid date"); + FC_ASSERT( htlc_obj->conditions.time_lock.expiration + o.seconds_to_add + <= db().head_block_time() + htlc_options->max_timeout_secs, "Extension pushes contract too far into the future" ); return void_result(); } diff --git a/tests/tests/htlc_tests.cpp b/tests/tests/htlc_tests.cpp index 1595ea6a0a..0b5f42f606 100644 --- a/tests/tests/htlc_tests.cpp +++ b/tests/tests/htlc_tests.cpp @@ -238,7 +238,7 @@ try { { graphene::chain::htlc_extend_operation big_extend; big_extend.htlc_id = alice_htlc_id; - big_extend.seconds_to_add = (fc::time_point_sec::maximum() - 59).sec_since_epoch(); + big_extend.seconds_to_add = db.get_global_properties().parameters.extensions.value.updatable_htlc_options->max_timeout_secs + 10; big_extend.fee = db.get_global_properties().parameters.current_fees->calculate_fee(big_extend); big_extend.update_issuer = alice_id; trx.operations.push_back(big_extend); From 82fa2f8892dfdaf8dce933e41036d072cda6fb20 Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 4 Apr 2019 18:41:27 -0500 Subject: [PATCH 4/7] Only allow HTLC recipient to call redeem --- libraries/chain/htlc_evaluator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp index 9fd9bbbc8e..2aaeca4112 100644 --- a/libraries/chain/htlc_evaluator.cpp +++ b/libraries/chain/htlc_evaluator.cpp @@ -102,7 +102,7 @@ namespace graphene { htlc_obj = &db().get(o.htlc_id); FC_ASSERT(o.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size, "Preimage size mismatch."); - + FC_ASSERT(o.redeemer == htlc_obj->transfer.to, "Only the recipient can redeem."); const htlc_redeem_visitor vtor( o.preimage ); FC_ASSERT( htlc_obj->conditions.hash_lock.preimage_hash.visit( vtor ), "Provided preimage does not generate correct hash."); From 510932fd9cd1e3590e8738b2e89c69c8d799752a Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 4 Apr 2019 18:44:12 -0500 Subject: [PATCH 5/7] Shorten long lines --- libraries/chain/htlc_evaluator.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp index 2aaeca4112..8d7e6824f3 100644 --- a/libraries/chain/htlc_evaluator.cpp +++ b/libraries/chain/htlc_evaluator.cpp @@ -45,7 +45,7 @@ namespace graphene { // make sure the expiration is reasonable 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 ); @@ -104,7 +104,8 @@ namespace graphene { FC_ASSERT(o.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size, "Preimage size mismatch."); FC_ASSERT(o.redeemer == htlc_obj->transfer.to, "Only the recipient can redeem."); const htlc_redeem_visitor vtor( o.preimage ); - FC_ASSERT( htlc_obj->conditions.hash_lock.preimage_hash.visit( vtor ), "Provided preimage does not generate correct hash."); + FC_ASSERT( htlc_obj->conditions.hash_lock.preimage_hash.visit( vtor ), + "Provided preimage does not generate correct hash."); return void_result(); } @@ -126,9 +127,11 @@ namespace graphene { FC_ASSERT(o.update_issuer == htlc_obj->transfer.from, "HTLC may only be extended by its creator."); optional htlc_options = get_committee_htlc_options(db()); FC_ASSERT( htlc_obj->conditions.time_lock.expiration.sec_since_epoch() - + static_cast(o.seconds_to_add) < fc::time_point_sec::maximum().sec_since_epoch(), "Extension would cause an invalid date"); + + static_cast(o.seconds_to_add) < fc::time_point_sec::maximum().sec_since_epoch(), + "Extension would cause an invalid date"); FC_ASSERT( htlc_obj->conditions.time_lock.expiration + o.seconds_to_add - <= db().head_block_time() + htlc_options->max_timeout_secs, "Extension pushes contract too far into the future" ); + <= db().head_block_time() + htlc_options->max_timeout_secs, + "Extension pushes contract too far into the future" ); return void_result(); } From 8f5dddd3a6c2cdf9197d9ffb0b7a699a7c8b3034 Mon Sep 17 00:00:00 2001 From: John Jones Date: Fri, 5 Apr 2019 06:22:37 -0500 Subject: [PATCH 6/7] Allow 3rd party to redeem, notify all --- libraries/chain/db_notify.cpp | 4 +++- libraries/chain/htlc_evaluator.cpp | 5 ++--- .../include/graphene/chain/protocol/htlc.hpp | 8 +++---- tests/tests/htlc_tests.cpp | 22 ++++++++++++++----- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index 1347026428..bcf8c6a674 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -264,11 +264,13 @@ struct get_impacted_account_visitor } void operator()( const htlc_redeem_operation& op ) { - _impacted.insert( op.fee_payer() ); + _impacted.insert( op.fee_payer() ); } void operator()( const htlc_redeemed_operation& op ) { _impacted.insert( op.from ); + if ( op.to != op.redeemer ) + _impacted.insert( op.to ); } void operator()( const htlc_extend_operation& op ) { diff --git a/libraries/chain/htlc_evaluator.cpp b/libraries/chain/htlc_evaluator.cpp index 8d7e6824f3..1009a75c53 100644 --- a/libraries/chain/htlc_evaluator.cpp +++ b/libraries/chain/htlc_evaluator.cpp @@ -102,7 +102,6 @@ namespace graphene { htlc_obj = &db().get(o.htlc_id); FC_ASSERT(o.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size, "Preimage size mismatch."); - FC_ASSERT(o.redeemer == htlc_obj->transfer.to, "Only the recipient can redeem."); const htlc_redeem_visitor vtor( o.preimage ); FC_ASSERT( htlc_obj->conditions.hash_lock.preimage_hash.visit( vtor ), "Provided preimage does not generate correct hash."); @@ -114,8 +113,8 @@ namespace graphene { { db().adjust_balance(htlc_obj->transfer.to, asset(htlc_obj->transfer.amount, htlc_obj->transfer.asset_id) ); // notify related parties - htlc_redeemed_operation virt_op( htlc_obj->id, htlc_obj->transfer.from, htlc_obj->transfer.to, - asset(htlc_obj->transfer.amount, htlc_obj->transfer.asset_id ) ); + htlc_redeemed_operation virt_op( htlc_obj->id, htlc_obj->transfer.from, htlc_obj->transfer.to, + o.redeemer, asset(htlc_obj->transfer.amount, htlc_obj->transfer.asset_id ) ); db().push_applied_operation( virt_op ); db().remove(*htlc_obj); return void_result(); diff --git a/libraries/chain/include/graphene/chain/protocol/htlc.hpp b/libraries/chain/include/graphene/chain/protocol/htlc.hpp index 5e9ab847a8..7e4e713368 100644 --- a/libraries/chain/include/graphene/chain/protocol/htlc.hpp +++ b/libraries/chain/include/graphene/chain/protocol/htlc.hpp @@ -126,8 +126,8 @@ namespace graphene { struct fee_parameters_type {}; htlc_redeemed_operation() {} - htlc_redeemed_operation( htlc_id_type htlc_id, account_id_type from, account_id_type to, asset amount) : - htlc_id(htlc_id), from(from), to(to), amount(amount) {} + htlc_redeemed_operation( htlc_id_type htlc_id, account_id_type from, account_id_type to, account_id_type redeemer, asset amount ) : + htlc_id(htlc_id), from(from), to(to), redeemer(redeemer), amount(amount) {} account_id_type fee_payer()const { return to; } void validate()const { FC_ASSERT( !"virtual operation" ); } @@ -135,7 +135,7 @@ namespace graphene { share_type calculate_fee(const fee_parameters_type& k)const { return 0; } htlc_id_type htlc_id; - account_id_type from, to; + account_id_type from, to, redeemer; asset amount; asset fee; }; @@ -206,6 +206,6 @@ FC_REFLECT( graphene::chain::htlc_refund_operation::fee_parameters_type, ) // VI FC_REFLECT( graphene::chain::htlc_create_operation, (fee)(from)(to)(amount)(preimage_hash)(preimage_size)(claim_period_seconds)(extensions)) FC_REFLECT( graphene::chain::htlc_redeem_operation, (fee)(htlc_id)(redeemer)(preimage)(extensions)) -FC_REFLECT( graphene::chain::htlc_redeemed_operation, (fee)(htlc_id)(from)(to)(amount) ) +FC_REFLECT( graphene::chain::htlc_redeemed_operation, (fee)(htlc_id)(from)(to)(redeemer)(amount) ) FC_REFLECT( graphene::chain::htlc_extend_operation, (fee)(htlc_id)(update_issuer)(seconds_to_add)(extensions)) FC_REFLECT( graphene::chain::htlc_refund_operation, (fee)(htlc_id)(to)) diff --git a/tests/tests/htlc_tests.cpp b/tests/tests/htlc_tests.cpp index 0b5f42f606..6a7f192084 100644 --- a/tests/tests/htlc_tests.cpp +++ b/tests/tests/htlc_tests.cpp @@ -271,12 +271,13 @@ try { BOOST_AUTO_TEST_CASE( htlc_fulfilled ) { try { - ACTORS((alice)(bob)); + ACTORS((alice)(bob)(joker)); int64_t init_balance(100 * GRAPHENE_BLOCKCHAIN_PRECISION); transfer( committee_account, alice_id, graphene::chain::asset(init_balance) ); transfer( committee_account, bob_id, graphene::chain::asset(init_balance) ); + transfer( committee_account, joker_id, graphene::chain::asset(init_balance) ); advance_past_hardfork(this); @@ -329,23 +330,32 @@ try { // make sure Alice's money is still on hold, and account for extra fee BOOST_CHECK_EQUAL( get_balance( alice_id, graphene::chain::asset_id_type()), 72 * GRAPHENE_BLOCKCHAIN_PRECISION ); - // send a redeem operation to claim the funds + // grab number of history objects to make sure everyone gets notified + size_t alice_num_history = get_operation_history(alice_id).size(); + size_t bob_num_history = get_operation_history(bob_id).size(); + size_t joker_num_history = get_operation_history(joker_id).size(); + + // joker sends a redeem operation to claim the funds for bob { graphene::chain::htlc_redeem_operation update_operation; - update_operation.redeemer = bob_id; + update_operation.redeemer = joker_id; update_operation.htlc_id = alice_htlc_id; update_operation.preimage = pre_image; update_operation.fee = db.current_fee_schedule().calculate_fee( update_operation ); trx.operations.push_back( update_operation ); - sign(trx, bob_private_key); + sign(trx, joker_private_key); PUSH_TX( db, trx, ~0 ); generate_block(); trx.clear(); } - // verify funds end up in Bob's account (100 + 20 - 4(fee) ) - BOOST_CHECK_EQUAL( get_balance(bob_id, graphene::chain::asset_id_type()), 116 * GRAPHENE_BLOCKCHAIN_PRECISION ); + // verify funds end up in Bob's account (100 + 20 ) + BOOST_CHECK_EQUAL( get_balance(bob_id, graphene::chain::asset_id_type()), 120 * GRAPHENE_BLOCKCHAIN_PRECISION ); // verify funds remain out of Alice's acount ( 100 - 20 - 4 ) BOOST_CHECK_EQUAL( get_balance(alice_id, graphene::chain::asset_id_type()), 72 * GRAPHENE_BLOCKCHAIN_PRECISION ); + // verify all three get notified + BOOST_CHECK_EQUAL( get_operation_history(alice_id).size(), alice_num_history + 1); + BOOST_CHECK_EQUAL( get_operation_history(bob_id).size(), bob_num_history + 1); + BOOST_CHECK_EQUAL( get_operation_history(joker_id).size(), joker_num_history + 1); } FC_LOG_AND_RETHROW() } From f734f2d7fda987ec0d29cd8dec876765704f9792 Mon Sep 17 00:00:00 2001 From: John Jones Date: Mon, 8 Apr 2019 11:35:47 -0500 Subject: [PATCH 7/7] bump db_version --- libraries/chain/include/graphene/chain/config.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/graphene/chain/config.hpp b/libraries/chain/include/graphene/chain/config.hpp index aee67ff74d..40259cec80 100644 --- a/libraries/chain/include/graphene/chain/config.hpp +++ b/libraries/chain/include/graphene/chain/config.hpp @@ -121,7 +121,7 @@ #define GRAPHENE_RECENTLY_MISSED_COUNT_INCREMENT 4 #define GRAPHENE_RECENTLY_MISSED_COUNT_DECREMENT 3 -#define GRAPHENE_CURRENT_DB_VERSION "20190323" +#define GRAPHENE_CURRENT_DB_VERSION "20190408" #define GRAPHENE_IRREVERSIBLE_THRESHOLD (70 * GRAPHENE_1_PERCENT)