From 45e717c1811e931e19faa9c3683494d32f4e521a Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Thu, 28 Jan 2016 20:02:37 -0500 Subject: [PATCH 1/3] HARDFORK: fix for hung cancel order --- libraries/chain/db_block.cpp | 4 +-- libraries/chain/db_update.cpp | 9 +++--- libraries/chain/evaluator.cpp | 29 ++++++++++--------- .../chain/transaction_evaluation_state.hpp | 1 + 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 770951b6..d0ce18b4 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -279,14 +279,14 @@ signed_block database::generate_block( const fc::ecc::private_key& block_signing_private_key, uint32_t skip /* = 0 */ ) -{ +{ try { signed_block result; detail::with_skip_flags( *this, skip, [&]() { result = _generate_block( when, witness_id, block_signing_private_key ); } ); return result; -} +} FC_CAPTURE_AND_RETHROW() } signed_block database::_generate_block( fc::time_point_sec when, diff --git a/libraries/chain/db_update.cpp b/libraries/chain/db_update.cpp index 2b6ba2e7..8a5ea106 100644 --- a/libraries/chain/db_update.cpp +++ b/libraries/chain/db_update.cpp @@ -152,14 +152,14 @@ void database::update_last_irreversible_block() } void database::clear_expired_transactions() -{ +{ try { //Look for expired transactions in the deduplication list, and remove them. //Transactions must have expired by at least two forking windows in order to be removed. auto& transaction_idx = static_cast(get_mutable_index(implementation_ids, impl_transaction_object_type)); const auto& dedupe_index = transaction_idx.indices().get(); while( (!dedupe_index.empty()) && (head_block_time() > dedupe_index.rbegin()->trx.expiration) ) transaction_idx.remove(*dedupe_index.rbegin()); -} +} FC_CAPTURE_AND_RETHROW() } void database::clear_expired_proposals() { @@ -250,7 +250,7 @@ bool database::check_for_blackswan( const asset_object& mia, bool enable_black_s } void database::clear_expired_orders() -{ +{ try { detail::with_skip_flags( *this, get_node_properties().skip_flags | skip_authority_check, [&](){ transaction_evaluation_state cancel_context(this); @@ -263,6 +263,7 @@ void database::clear_expired_orders() const limit_order_object& order = *limit_index.begin(); canceler.fee_paying_account = order.seller; canceler.order = order.id; + cancel_context.skip_fee = true; apply_operation(cancel_context, canceler); } }); @@ -362,7 +363,7 @@ void database::clear_expired_orders() }); } } -} +} FC_CAPTURE_AND_RETHROW() } void database::update_expired_feeds() { diff --git a/libraries/chain/evaluator.cpp b/libraries/chain/evaluator.cpp index ee2f1580..4f763efa 100644 --- a/libraries/chain/evaluator.cpp +++ b/libraries/chain/evaluator.cpp @@ -76,24 +76,27 @@ database& generic_evaluator::db()const { return trx_state->db(); } void generic_evaluator::convert_fee() { - if( fee_asset->get_id() != asset_id_type() ) - { - db().modify(*fee_asset_dyn_data, [this](asset_dynamic_data_object& d) { - d.accumulated_fees += fee_from_account.amount; - d.fee_pool -= core_fee_paid; - }); + if( !trx_state->skip_fee ) { + if( fee_asset->get_id() != asset_id_type() ) + { + db().modify(*fee_asset_dyn_data, [this](asset_dynamic_data_object& d) { + d.accumulated_fees += fee_from_account.amount; + d.fee_pool -= core_fee_paid; + }); + } } - return; } void generic_evaluator::pay_fee() { try { - database& d = db(); - /// TODO: db().pay_fee( account_id, core_fee ); - d.modify(*fee_paying_account_statistics, [&](account_statistics_object& s) - { - s.pay_fee( core_fee_paid, d.get_global_properties().parameters.cashback_vesting_threshold ); - }); + if( !trx_state->skip_fee ) { + database& d = db(); + /// TODO: db().pay_fee( account_id, core_fee ); + d.modify(*fee_paying_account_statistics, [&](account_statistics_object& s) + { + s.pay_fee( core_fee_paid, d.get_global_properties().parameters.cashback_vesting_threshold ); + }); + } } FC_CAPTURE_AND_RETHROW() } } } diff --git a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp index f31cc93c..4c28e122 100644 --- a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp +++ b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp @@ -42,5 +42,6 @@ namespace graphene { namespace chain { const signed_transaction* _trx = nullptr; database* _db = nullptr; bool _is_proposed_trx = false; + bool skip_fee = false; }; } } // namespace graphene::chain From 3646754fe59149ae688f82243b4fe41486c96ba3 Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Thu, 28 Jan 2016 20:11:46 -0500 Subject: [PATCH 2/3] HARDFORK - auto canceled orders still pay fee --- libraries/chain/db_update.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/db_update.cpp b/libraries/chain/db_update.cpp index 8a5ea106..76dc1df9 100644 --- a/libraries/chain/db_update.cpp +++ b/libraries/chain/db_update.cpp @@ -263,7 +263,7 @@ void database::clear_expired_orders() const limit_order_object& order = *limit_index.begin(); canceler.fee_paying_account = order.seller; canceler.order = order.id; - cancel_context.skip_fee = true; + canceler.fee = current_fee_schedule().calculate_fee( canceler ); apply_operation(cancel_context, canceler); } }); From 59503acde9eef6520fc177c1cde71fe486d72533 Mon Sep 17 00:00:00 2001 From: theoreticalbts Date: Tue, 2 Feb 2016 13:58:58 -0500 Subject: [PATCH 3/3] Cap auto-cancel fees at deferred_fee #549 --- libraries/chain/db_update.cpp | 11 +++++++++++ libraries/chain/include/graphene/chain/evaluator.hpp | 12 ++++++++---- .../graphene/chain/transaction_evaluation_state.hpp | 1 + 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/libraries/chain/db_update.cpp b/libraries/chain/db_update.cpp index 76dc1df9..3855c50b 100644 --- a/libraries/chain/db_update.cpp +++ b/libraries/chain/db_update.cpp @@ -264,6 +264,17 @@ void database::clear_expired_orders() canceler.fee_paying_account = order.seller; canceler.order = order.id; canceler.fee = current_fee_schedule().calculate_fee( canceler ); + if( canceler.fee.amount > order.deferred_fee ) + { + // Cap auto-cancel fees at deferred_fee; see #549 + wlog( "At block ${b}, fee for clearing expired order ${oid} was capped at deferred_fee ${fee}", ("b", head_block_num())("oid", order.id)("fee", order.deferred_fee) ); + canceler.fee = asset( order.deferred_fee, asset_id_type() ); + } + // we know the fee for this op is set correctly since it is set by the chain. + // this allows us to avoid a hung chain: + // - if #549 case above triggers + // - if the fee is incorrect, which may happen due to #435 (although since cancel is a fixed-fee op, it shouldn't) + cancel_context.skip_fee_schedule_check = true; apply_operation(cancel_context, canceler); } }); diff --git a/libraries/chain/include/graphene/chain/evaluator.hpp b/libraries/chain/include/graphene/chain/evaluator.hpp index cfbbaea2..a80a2df8 100644 --- a/libraries/chain/include/graphene/chain/evaluator.hpp +++ b/libraries/chain/include/graphene/chain/evaluator.hpp @@ -20,6 +20,7 @@ */ #pragma once #include +#include #include namespace graphene { namespace chain { @@ -224,10 +225,13 @@ namespace graphene { namespace chain { const auto& op = o.get(); prepare_fee(op.fee_payer(), op.fee); - GRAPHENE_ASSERT( core_fee_paid >= db().current_fee_schedule().calculate_fee( op ).amount, - insufficient_fee, - "Insufficient Fee Paid", - ("core_fee_paid",core_fee_paid)("required",db().current_fee_schedule().calculate_fee( op ).amount) ); + if( !trx_state->skip_fee_schedule_check ) + { + GRAPHENE_ASSERT( core_fee_paid >= db().current_fee_schedule().calculate_fee( op ).amount, + insufficient_fee, + "Insufficient Fee Paid", + ("core_fee_paid",core_fee_paid)("required",db().current_fee_schedule().calculate_fee( op ).amount) ); + } return eval->do_evaluate(op); } diff --git a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp index 4c28e122..4a5b5183 100644 --- a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp +++ b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp @@ -43,5 +43,6 @@ namespace graphene { namespace chain { database* _db = nullptr; bool _is_proposed_trx = false; bool skip_fee = false; + bool skip_fee_schedule_check = false; }; } } // namespace graphene::chain