diff --git a/libraries/chain/account_object.cpp b/libraries/chain/account_object.cpp index 255ba5b1..a68c8260 100644 --- a/libraries/chain/account_object.cpp +++ b/libraries/chain/account_object.cpp @@ -128,6 +128,14 @@ void account_statistics_object::process_fees(const account_object& a, database& } } +void account_statistics_object::pay_fee( share_type core_fee, share_type cashback_vesting_threshold ) +{ + if( core_fee > cashback_vesting_threshold ) + pending_fees += core_fee; + else + pending_vested_fees += core_fee; +} + void account_object::options_type::validate() const { auto needed_witnesses = num_witness; diff --git a/libraries/chain/db_market.cpp b/libraries/chain/db_market.cpp index f45fed6d..c1d1456f 100644 --- a/libraries/chain/db_market.cpp +++ b/libraries/chain/db_market.cpp @@ -116,6 +116,7 @@ void database::cancel_order( const limit_order_object& order, bool create_virtua } }); adjust_balance(order.seller, refunded); + adjust_balance(order.seller, order.deferred_fee); if( create_virtual_op ) { @@ -273,6 +274,15 @@ bool database::fill_order( const limit_order_object& order, const asset& pays, c assert( pays.asset_id != receives.asset_id ); push_applied_operation( fill_order_operation( order.id, order.seller, pays, receives, issuer_fees ) ); + // conditional because cheap integer comparison may allow us to avoid two expensive modify() and object lookups + if( order.deferred_fee > 0 ) + { + modify( seller.statistics(*this), [&]( account_statistics_object& statistics ) + { + statistics.pay_fee( order.deferred_fee, get_global_properties().parameters.cashback_vesting_threshold ); + } ); + } + if( pays == order.amount_for_sale() ) { remove( order ); @@ -282,6 +292,7 @@ bool database::fill_order( const limit_order_object& order, const asset& pays, c { modify( order, [&]( limit_order_object& b ) { b.for_sale -= pays.amount; + b.deferred_fee = 0; }); /** * There are times when the AMOUNT_FOR_SALE * SALE_PRICE == 0 which means that we diff --git a/libraries/chain/evaluator.cpp b/libraries/chain/evaluator.cpp index 878d98cb..ee2f1580 100644 --- a/libraries/chain/evaluator.cpp +++ b/libraries/chain/evaluator.cpp @@ -74,18 +74,25 @@ database& generic_evaluator::db()const { return trx_state->db(); } } } - void generic_evaluator::pay_fee() - { try { + 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; }); - db().modify(*fee_paying_account_statistics, [&](account_statistics_object& s) { - if( core_fee_paid > db().get_global_properties().parameters.cashback_vesting_threshold ) - s.pending_fees += core_fee_paid; - else - s.pending_vested_fees += 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 ); }); } FC_CAPTURE_AND_RETHROW() } diff --git a/libraries/chain/include/graphene/chain/account_object.hpp b/libraries/chain/include/graphene/chain/account_object.hpp index 5067292d..102b1071 100644 --- a/libraries/chain/include/graphene/chain/account_object.hpp +++ b/libraries/chain/include/graphene/chain/account_object.hpp @@ -77,6 +77,11 @@ namespace graphene { namespace chain { /// @brief Split up and pay out @ref pending_fees and @ref pending_vested_fees void process_fees(const account_object& a, database& d) const; + + /** + * Core fees are paid into the account_statistics_object by this method + */ + void pay_fee( share_type core_fee, share_type cashback_vesting_threshold ); }; /** diff --git a/libraries/chain/include/graphene/chain/evaluator.hpp b/libraries/chain/include/graphene/chain/evaluator.hpp index 015f10a5..cfbbaea2 100644 --- a/libraries/chain/include/graphene/chain/evaluator.hpp +++ b/libraries/chain/include/graphene/chain/evaluator.hpp @@ -86,6 +86,22 @@ namespace graphene { namespace chain { virtual operation_result evaluate(const operation& op) = 0; virtual operation_result apply(const operation& op) = 0; + /** + * Routes the fee to where it needs to go. The default implementation + * routes the fee to the account_statistics_object of the fee_paying_account. + * + * Before pay_fee() is called, the fee is computed by prepare_fee() and has been + * moved out of the fee_paying_account and (if paid in a non-CORE asset) converted + * by the asset's fee pool. + * + * Therefore, when pay_fee() is called, the fee only exists in this->core_fee_paid. + * So pay_fee() need only increment the receiving balance. + * + * The default implementation simply calls account_statistics_object->pay_fee() to + * increment pending_fees or pending_vested_fees. + */ + virtual void pay_fee(); + database& db()const; //void check_required_authorities(const operation& op); @@ -97,10 +113,24 @@ namespace graphene { namespace chain { * * This method verifies that the fee is valid and sets the object pointer members and the fee fields. It should * be called during do_evaluate. + * + * In particular, core_fee_paid field is set by prepare_fee(). */ void prepare_fee(account_id_type account_id, asset fee); - /// Pays the fee and returns the number of CORE asset that were paid. - void pay_fee(); + + /** + * Convert the fee into BTS through the exchange pool. + * + * Reads core_fee_paid field for how much CORE is deducted from the exchange pool, + * and fee_from_account for how much USD is added to the pool. + * + * Since prepare_fee() does the validation checks ensuring the account and fee pool + * have sufficient balance and the exchange rate is correct, + * those validation checks are not replicated here. + * + * Rather than returning a value, this method fills in core_fee_paid field. + */ + void convert_fee(); object_id_type get_relative_id( object_id_type rel_id )const; @@ -201,11 +231,13 @@ namespace graphene { namespace chain { return eval->do_evaluate(op); } + virtual operation_result apply(const operation& o) final override { auto* eval = static_cast(this); const auto& op = o.get(); + convert_fee(); pay_fee(); auto result = eval->do_apply(op); diff --git a/libraries/chain/include/graphene/chain/hardfork.hpp b/libraries/chain/include/graphene/chain/hardfork.hpp index 7f5da1a8..1eb263f6 100644 --- a/libraries/chain/include/graphene/chain/hardfork.hpp +++ b/libraries/chain/include/graphene/chain/hardfork.hpp @@ -28,3 +28,7 @@ #define HARDFORK_416_TIME (fc::time_point_sec( 1446652800 )) #define HARDFORK_419_TIME (fc::time_point_sec( 1446652800 )) #define HARDFORK_436_TIME (fc::time_point_sec( 2000000000 )) + +// #445 Refund create order fees on cancel +// 2015-12-02 17:00:00 UTC / 12:00:00 EST +#define HARDFORK_445_TIME (fc::time_point_sec( 1449075600 )) diff --git a/libraries/chain/include/graphene/chain/market_evaluator.hpp b/libraries/chain/include/graphene/chain/market_evaluator.hpp index f6dfa4a0..53094940 100644 --- a/libraries/chain/include/graphene/chain/market_evaluator.hpp +++ b/libraries/chain/include/graphene/chain/market_evaluator.hpp @@ -45,6 +45,7 @@ namespace graphene { namespace chain { account_id_type seller; share_type for_sale; ///< asset id is sell_price.base.asset_id price sell_price; + share_type deferred_fee; pair get_market()const { @@ -189,6 +190,12 @@ namespace graphene { namespace chain { asset calculate_market_fee( const asset_object* aobj, const asset& trade_amount ); + /** override the default behavior defined by generic_evalautor which is to + * post the fee to fee_paying_account_stats.pending_fees + */ + virtual void pay_fee() override; + + share_type _deferred_fee = 0; const limit_order_create_operation* _op = nullptr; const account_object* _seller = nullptr; const asset_object* _sell_asset = nullptr; @@ -225,7 +232,7 @@ namespace graphene { namespace chain { FC_REFLECT_DERIVED( graphene::chain::limit_order_object, (graphene::db::object), - (expiration)(seller)(for_sale)(sell_price) + (expiration)(seller)(for_sale)(sell_price)(deferred_fee) ) FC_REFLECT_DERIVED( graphene::chain::call_order_object, (graphene::db::object), diff --git a/libraries/chain/market_evaluator.cpp b/libraries/chain/market_evaluator.cpp index 8be4829f..d782c39e 100644 --- a/libraries/chain/market_evaluator.cpp +++ b/libraries/chain/market_evaluator.cpp @@ -18,12 +18,14 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * */ -#include #include #include #include +#include + #include #include + #include namespace graphene { namespace chain { @@ -59,6 +61,14 @@ void_result limit_order_create_evaluator::do_evaluate(const limit_order_create_o return void_result(); } FC_CAPTURE_AND_RETHROW( (op) ) } +void limit_order_create_evaluator::pay_fee() +{ + if( db().head_block_time() <= HARDFORK_445_TIME ) + generic_evaluator::pay_fee(); + else + _deferred_fee = core_fee_paid; +} + object_id_type limit_order_create_evaluator::do_apply(const limit_order_create_operation& op) { try { const auto& seller_stats = _seller->statistics(db()); @@ -76,6 +86,7 @@ object_id_type limit_order_create_evaluator::do_apply(const limit_order_create_o obj.for_sale = op.amount_to_sell.amount; obj.sell_price = op.get_price(); obj.expiration = op.expiration; + obj.deferred_fee = _deferred_fee; }); limit_order_id_type order_id = new_order_object.id; // save this because we may remove the object by filling it bool filled = db().apply_order(new_order_object); @@ -103,28 +114,6 @@ asset limit_order_cancel_evaluator::do_apply(const limit_order_cancel_operation& auto quote_asset = _order->sell_price.quote.asset_id; auto refunded = _order->amount_for_sale(); - /// TODO... implement this refund in a better way - if( true ) // HARD FORK d.head_block_time() > - { - const auto& fees = d.current_fee_schedule(); - asset create_fee = fees.calculate_fee( limit_order_create_operation() ); - - // then the create fee was only the network fee and not the - // full create_fee - const auto& gprops = d.get_global_properties(); - auto cashback_percent = GRAPHENE_100_PERCENT - gprops.parameters.network_percent_of_fee; - auto cashback_amount = (create_fee.amount * cashback_percent) / GRAPHENE_100_PERCENT; - create_fee.amount -= cashback_amount; - - const auto& core_asset_data = asset_id_type(0)(d).dynamic_asset_data_id(d); - d.modify( core_asset_data, [&]( asset_dynamic_data_object& addo ) { - addo.accumulated_fees -= create_fee.amount; - }); - - d.adjust_balance( o.fee_paying_account, create_fee ); - } - - d.cancel_order(*_order, false /* don't create a virtual op*/); // Possible optimization: order can be called by canceling a limit order iff the canceled order was at the top of the book. diff --git a/tests/common/database_fixture.cpp b/tests/common/database_fixture.cpp index f7431f40..7e8e41f4 100644 --- a/tests/common/database_fixture.cpp +++ b/tests/common/database_fixture.cpp @@ -169,6 +169,7 @@ void database_fixture::verify_asset_supplies( const database& db ) asset for_sale = o.amount_for_sale(); if( for_sale.asset_id == asset_id_type() ) core_in_orders += for_sale.amount; total_balances[for_sale.asset_id] += for_sale.amount; + total_balances[asset_id_type()] += o.deferred_fee; } for( const call_order_object& o : db.get_index_type().indices() ) { @@ -522,6 +523,41 @@ void database_fixture::issue_uia( const account_object& recipient, asset amount trx.operations.clear(); } +void database_fixture::issue_uia( account_id_type recipient_id, asset amount ) +{ + issue_uia( recipient_id(db), amount ); +} + +void database_fixture::change_fees( + const flat_set< fee_parameters >& new_params, + uint32_t new_scale /* = 0 */ + ) +{ + const chain_parameters& current_chain_params = db.get_global_properties().parameters; + const fee_schedule& current_fees = *(current_chain_params.current_fees); + + flat_map< int, fee_parameters > fee_map; + fee_map.reserve( current_fees.parameters.size() ); + for( const fee_parameters& op_fee : current_fees.parameters ) + fee_map[ op_fee.which() ] = op_fee; + for( const fee_parameters& new_fee : new_params ) + fee_map[ new_fee.which() ] = new_fee; + + fee_schedule_type new_fees; + + for( const std::pair< int, fee_parameters >& item : fee_map ) + new_fees.parameters.insert( item.second ); + if( new_scale != 0 ) + new_fees.scale = new_scale; + + chain_parameters new_chain_params = current_chain_params; + new_chain_params.current_fees = new_fees; + + db.modify(db.get_global_properties(), [&](global_property_object& p) { + p.parameters = new_chain_params; + }); +} + const account_object& database_fixture::create_account( const string& name, const public_key_type& key /* = public_key_type() */ diff --git a/tests/common/database_fixture.hpp b/tests/common/database_fixture.hpp index 9bb218a9..40087fa9 100644 --- a/tests/common/database_fixture.hpp +++ b/tests/common/database_fixture.hpp @@ -225,7 +225,7 @@ struct database_fixture { const account_object& issuer, uint16_t flags ); void issue_uia( const account_object& recipient, asset amount ); - + void issue_uia( account_id_type recipient_id, asset amount ); const account_object& create_account( const string& name, @@ -263,6 +263,7 @@ struct database_fixture { void transfer( const account_object& from, const account_object& to, const asset& amount, const asset& fee = asset() ); void fund_fee_pool( const account_object& from, const asset_object& asset_to_fund, const share_type amount ); void enable_fees(); + void change_fees( const flat_set< fee_parameters >& new_params, uint32_t new_scale = 0 ); void upgrade_to_lifetime_member( account_id_type account ); void upgrade_to_lifetime_member( const account_object& account ); void upgrade_to_annual_member( account_id_type account ); diff --git a/tests/tests/fee_tests.cpp b/tests/tests/fee_tests.cpp index 643caaf5..601a2584 100644 --- a/tests/tests/fee_tests.cpp +++ b/tests/tests/fee_tests.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -592,4 +593,129 @@ BOOST_AUTO_TEST_CASE( account_create_fee_scaling ) BOOST_CHECK_EQUAL(db.get_global_properties().parameters.current_fees->get().basic_fee, 1); } FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( fee_refund_test ) +{ + try + { + ACTORS((alice)(bob)(izzy)); + + int64_t alice_b0 = 1000000, bob_b0 = 1000000; + + transfer( account_id_type(), alice_id, asset(alice_b0) ); + transfer( account_id_type(), bob_id, asset(bob_b0) ); + + asset_id_type core_id = asset_id_type(); + asset_id_type usd_id = create_user_issued_asset( "IZZYUSD", izzy_id(db), charge_market_fee ).id; + issue_uia( alice_id, asset( alice_b0, usd_id ) ); + issue_uia( bob_id, asset( bob_b0, usd_id ) ); + + int64_t order_create_fee = 537; + int64_t order_cancel_fee = 129; + + generate_block(); + + for( int i=0; i<2; i++ ) + { + if( i == 1 ) + { + generate_blocks( HARDFORK_445_TIME ); + generate_block(); + } + + // enable_fees() and change_fees() modifies DB directly, and results will be overwritten by block generation + // so we have to do it every time we stop generating/popping blocks and start doing tx's + enable_fees(); + /* + change_fees({ + limit_order_create_operation::fee_parameters_type { order_create_fee }, + limit_order_cancel_operation::fee_parameters_type { order_cancel_fee } + }); + */ + // C++ -- The above commented out statement doesn't work, I don't know why + // so we will use the following rather lengthy initialization instead + { + flat_set< fee_parameters > new_fees; + { + limit_order_create_operation::fee_parameters_type create_fee_params; + create_fee_params.fee = order_create_fee; + new_fees.insert( create_fee_params ); + } + { + limit_order_cancel_operation::fee_parameters_type cancel_fee_params; + cancel_fee_params.fee = order_cancel_fee; + new_fees.insert( cancel_fee_params ); + } + change_fees( new_fees ); + } + + // Alice creates order + // Bob creates order which doesn't match + + // AAAAGGHH create_sell_order reads trx.expiration #469 + set_expiration( db, trx ); + + // Check non-overlapping + + limit_order_id_type ao1_id = create_sell_order( alice_id, asset(1000), asset(1000, usd_id) )->id; + limit_order_id_type bo1_id = create_sell_order( bob_id, asset(500, usd_id), asset(1000) )->id; + + BOOST_CHECK_EQUAL( get_balance( alice_id, core_id ), alice_b0 - 1000 - order_create_fee ); + BOOST_CHECK_EQUAL( get_balance( alice_id, usd_id ), alice_b0 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, core_id ), bob_b0 - order_create_fee ); + BOOST_CHECK_EQUAL( get_balance( bob_id, usd_id ), bob_b0 - 500 ); + + // Bob cancels order + cancel_limit_order( bo1_id(db) ); + + int64_t cancel_net_fee; + if( db.head_block_time() >= HARDFORK_445_TIME ) + cancel_net_fee = order_cancel_fee; + else + cancel_net_fee = order_create_fee + order_cancel_fee; + + BOOST_CHECK_EQUAL( get_balance( alice_id, core_id ), alice_b0 - 1000 - order_create_fee ); + BOOST_CHECK_EQUAL( get_balance( alice_id, usd_id ), alice_b0 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, core_id ), bob_b0 - cancel_net_fee ); + BOOST_CHECK_EQUAL( get_balance( bob_id, usd_id ), bob_b0 ); + + // Alice cancels order + cancel_limit_order( ao1_id(db) ); + + BOOST_CHECK_EQUAL( get_balance( alice_id, core_id ), alice_b0 - cancel_net_fee ); + BOOST_CHECK_EQUAL( get_balance( alice_id, usd_id ), alice_b0 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, core_id ), bob_b0 - cancel_net_fee ); + BOOST_CHECK_EQUAL( get_balance( bob_id, usd_id ), bob_b0 ); + + // Check partial fill + const limit_order_object* ao2 = create_sell_order( alice_id, asset(1000), asset(200, usd_id) ); + const limit_order_object* bo2 = create_sell_order( bob_id, asset(100, usd_id), asset(500) ); + + BOOST_CHECK( ao2 != nullptr ); + BOOST_CHECK( bo2 == nullptr ); + + BOOST_CHECK_EQUAL( get_balance( alice_id, core_id ), alice_b0 - cancel_net_fee - order_create_fee - 1000 ); + BOOST_CHECK_EQUAL( get_balance( alice_id, usd_id ), alice_b0 + 100 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, core_id ), bob_b0 - cancel_net_fee - order_create_fee + 500 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, usd_id ), bob_b0 - 100 ); + + // cancel Alice order, show that entire deferred_fee was consumed by partial match + cancel_limit_order( *ao2 ); + + BOOST_CHECK_EQUAL( get_balance( alice_id, core_id ), alice_b0 - cancel_net_fee - order_create_fee - 500 - order_cancel_fee ); + BOOST_CHECK_EQUAL( get_balance( alice_id, usd_id ), alice_b0 + 100 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, core_id ), bob_b0 - cancel_net_fee - order_create_fee + 500 ); + BOOST_CHECK_EQUAL( get_balance( bob_id, usd_id ), bob_b0 - 100 ); + + // TODO: Check multiple fill + // there really should be a test case involving Alice creating multiple orders matched by single Bob order + // but we'll save that for future cleanup + + // undo above tx's and reset + generate_block(); + db.pop_block(); + } + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_SUITE_END()