From bee7f31f07570daf0ae867c2c0dd516d87110507 Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Fri, 20 Oct 2017 22:30:38 +0200 Subject: [PATCH 1/6] Added unit test for #429 --- tests/tests/fee_tests.cpp | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/tests/fee_tests.cpp b/tests/tests/fee_tests.cpp index 934d9fc1..2521f1db 100644 --- a/tests/tests/fee_tests.cpp +++ b/tests/tests/fee_tests.cpp @@ -947,4 +947,54 @@ BOOST_AUTO_TEST_CASE( stealth_fba_test ) } } +BOOST_AUTO_TEST_CASE( issue_429_test ) +{ + try + { + ACTORS((alice)); + + transfer( committee_account, alice_id, asset( 1000000 * asset::scaled_precision( asset_id_type()(db).precision ) ) ); + + // make sure the database requires our fee to be nonzero + enable_fees(); + + auto fees_to_pay = db.get_global_properties().parameters.current_fees->get(); + + { + signed_transaction tx; + asset_create_operation op; + op.issuer = alice_id; + op.symbol = "ALICE"; + op.common_options.core_exchange_rate = asset( 1 ) / asset( 1, asset_id_type( 1 ) ); + op.fee = asset( (fees_to_pay.long_symbol + fees_to_pay.price_per_kbyte) & (~1) ); + tx.operations.push_back( op ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + PUSH_TX( db, tx ); + } + + verify_asset_supplies( db ); + + { + signed_transaction tx; + asset_create_operation op; + op.issuer = alice_id; + op.symbol = "ALICE.ODD"; + op.common_options.core_exchange_rate = asset( 1 ) / asset( 1, asset_id_type( 1 ) ); + op.fee = asset((fees_to_pay.long_symbol + fees_to_pay.price_per_kbyte) | 1); + tx.operations.push_back( op ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + PUSH_TX( db, tx ); + } + + verify_asset_supplies( db ); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From bb813a96b096357239eb1b5000cc409da6e7a9cd Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Fri, 20 Oct 2017 23:29:57 +0200 Subject: [PATCH 2/6] Added test for #433 --- tests/tests/fee_tests.cpp | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/tests/fee_tests.cpp b/tests/tests/fee_tests.cpp index 2521f1db..e07ec77b 100644 --- a/tests/tests/fee_tests.cpp +++ b/tests/tests/fee_tests.cpp @@ -997,4 +997,44 @@ BOOST_AUTO_TEST_CASE( issue_429_test ) } } +BOOST_AUTO_TEST_CASE( issue_433_test ) +{ + try + { + ACTORS((alice)); + + auto& core = asset_id_type()(db); + + transfer( committee_account, alice_id, asset( 1000000 * asset::scaled_precision( core.precision ) ) ); + + const auto& myusd = create_user_issued_asset( "MYUSD", alice, 0 ); + issue_uia( alice, myusd.amount( 1000000000 ) ); + + // make sure the database requires our fee to be nonzero + enable_fees(); + + auto fees_to_pay = db.get_global_properties().parameters.current_fees->get(); + + fund_fee_pool( alice, myusd, 2*fees_to_pay.long_symbol ); + + signed_transaction tx; + asset_create_operation op; + op.issuer = alice_id; + op.symbol = "ALICE"; + op.common_options.core_exchange_rate = asset( 1 ) / asset( 1, asset_id_type( 1 ) ); + op.fee = myusd.amount( ((fees_to_pay.long_symbol + fees_to_pay.price_per_kbyte) & (~1)) ); + tx.operations.push_back( op ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + PUSH_TX( db, tx ); + + verify_asset_supplies( db ); + } + catch( const fc::exception& e ) + { + edump((e.to_detail_string())); + throw; + } +} + BOOST_AUTO_TEST_SUITE_END() From a6f1ae0bf1a5ecbb068a0c4420dad771d23adeb7 Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Sat, 21 Oct 2017 08:47:32 +0200 Subject: [PATCH 3/6] Added test case for proposal --- tests/tests/fee_tests.cpp | 55 ++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/tests/tests/fee_tests.cpp b/tests/tests/fee_tests.cpp index e07ec77b..f5545232 100644 --- a/tests/tests/fee_tests.cpp +++ b/tests/tests/fee_tests.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include @@ -1008,25 +1009,61 @@ BOOST_AUTO_TEST_CASE( issue_433_test ) transfer( committee_account, alice_id, asset( 1000000 * asset::scaled_precision( core.precision ) ) ); const auto& myusd = create_user_issued_asset( "MYUSD", alice, 0 ); - issue_uia( alice, myusd.amount( 1000000000 ) ); + issue_uia( alice, myusd.amount( 2000000000 ) ); // make sure the database requires our fee to be nonzero enable_fees(); - auto fees_to_pay = db.get_global_properties().parameters.current_fees->get(); + const auto& fees = *db.get_global_properties().parameters.current_fees; + const auto asset_create_fees = fees.get(); - fund_fee_pool( alice, myusd, 2*fees_to_pay.long_symbol ); + fund_fee_pool( alice, myusd, 5*asset_create_fees.long_symbol ); - signed_transaction tx; asset_create_operation op; op.issuer = alice_id; op.symbol = "ALICE"; op.common_options.core_exchange_rate = asset( 1 ) / asset( 1, asset_id_type( 1 ) ); - op.fee = myusd.amount( ((fees_to_pay.long_symbol + fees_to_pay.price_per_kbyte) & (~1)) ); - tx.operations.push_back( op ); - set_expiration( db, tx ); - sign( tx, alice_private_key ); - PUSH_TX( db, tx ); + op.fee = myusd.amount( ((asset_create_fees.long_symbol + asset_create_fees.price_per_kbyte) & (~1)) ); + { + signed_transaction tx; + tx.operations.push_back( op ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + PUSH_TX( db, tx ); + } + + verify_asset_supplies( db ); + + const auto proposal_create_fees = fees.get(); + proposal_create_operation prop; + op.symbol = "ALICE.PROP"; + prop.fee_paying_account = alice_id; + prop.proposed_ops.emplace_back( op ); + prop.expiration_time = db.head_block_time() + fc::days(1); + prop.fee = asset( proposal_create_fees.fee + proposal_create_fees.price_per_kbyte ); + object_id_type proposal_id; + { + signed_transaction tx; + tx.operations.push_back( prop ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + proposal_id = PUSH_TX( db, tx ).operation_results.front().get(); + } + const proposal_object& proposal = db.get( proposal_id ); + + const auto proposal_update_fees = fees.get(); + proposal_update_operation pup; + pup.proposal = proposal.id; + pup.fee_paying_account = alice_id; + pup.active_approvals_to_add.insert(alice_id); + pup.fee = asset( proposal_update_fees.fee + proposal_update_fees.price_per_kbyte ); + { + signed_transaction tx; + tx.operations.push_back( pup ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + PUSH_TX( db, tx ); + } verify_asset_supplies( db ); } From 8d8b84cc37cf6862fdd30191bcb859ccbd0c6042 Mon Sep 17 00:00:00 2001 From: abitmore Date: Wed, 18 Oct 2017 22:01:32 +0000 Subject: [PATCH 4/6] Proof of concept fix for asset creation fee issue --- libraries/chain/asset_evaluator.cpp | 8 ++++++-- .../chain/include/graphene/chain/asset_evaluator.hpp | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/libraries/chain/asset_evaluator.cpp b/libraries/chain/asset_evaluator.cpp index 4e3e9b96..91da528d 100644 --- a/libraries/chain/asset_evaluator.cpp +++ b/libraries/chain/asset_evaluator.cpp @@ -93,8 +93,6 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o wlog( "Asset ${s} has a name which requires hardfork 385", ("s",op.symbol) ); } - core_fee_paid -= core_fee_paid.value/2; - if( op.bitasset_opts ) { const asset_object& backing = op.bitasset_opts->short_backing_asset(d); @@ -121,6 +119,12 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o return void_result(); } FC_CAPTURE_AND_RETHROW( (op) ) } +void asset_create_evaluator::pay_fee() +{ + core_fee_paid -= core_fee_paid.value/2; + generic_evaluator::pay_fee(); +} + object_id_type asset_create_evaluator::do_apply( const asset_create_operation& op ) { try { const asset_dynamic_data_object& dyn_asset = diff --git a/libraries/chain/include/graphene/chain/asset_evaluator.hpp b/libraries/chain/include/graphene/chain/asset_evaluator.hpp index 234a60d7..26605a4f 100644 --- a/libraries/chain/include/graphene/chain/asset_evaluator.hpp +++ b/libraries/chain/include/graphene/chain/asset_evaluator.hpp @@ -35,6 +35,11 @@ namespace graphene { namespace chain { void_result do_evaluate( const asset_create_operation& o ); object_id_type do_apply( const asset_create_operation& o ); + + /** 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; }; class asset_issue_evaluator : public evaluator From 4b0579b475c4efb75e0e054ae7791afb4217358c Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Sat, 21 Oct 2017 13:11:21 +0200 Subject: [PATCH 5/6] Fixed #429 --- libraries/chain/asset_evaluator.cpp | 12 +++++++++++- libraries/chain/hardfork.d/CORE_429.hf | 4 ++++ .../include/graphene/chain/asset_evaluator.hpp | 2 ++ tests/tests/fee_tests.cpp | 17 +++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 libraries/chain/hardfork.d/CORE_429.hf diff --git a/libraries/chain/asset_evaluator.cpp b/libraries/chain/asset_evaluator.cpp index 91da528d..8103e2b5 100644 --- a/libraries/chain/asset_evaluator.cpp +++ b/libraries/chain/asset_evaluator.cpp @@ -121,17 +121,27 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o void asset_create_evaluator::pay_fee() { + fee_is_odd = core_fee_paid.value & 1; core_fee_paid -= core_fee_paid.value/2; generic_evaluator::pay_fee(); } object_id_type asset_create_evaluator::do_apply( const asset_create_operation& op ) { try { + bool hf_429 = fee_is_odd && db().head_block_time() > HARDFORK_CORE_429_TIME; + const asset_dynamic_data_object& dyn_asset = db().create( [&]( asset_dynamic_data_object& a ) { a.current_supply = 0; - a.fee_pool = core_fee_paid; //op.calculate_fee(db().current_fee_schedule()).value / 2; + a.fee_pool = core_fee_paid - (hf_429 ? 1 : 0); }); + if( fee_is_odd && !hf_429 ) + { + const auto& core_dd = db().get( asset_id_type() ).dynamic_data( db() ); + db().modify( core_dd, [=]( asset_dynamic_data_object& dd ) { + dd.current_supply++; + }); + } asset_bitasset_data_id_type bit_asset_id; if( op.bitasset_opts.valid() ) diff --git a/libraries/chain/hardfork.d/CORE_429.hf b/libraries/chain/hardfork.d/CORE_429.hf new file mode 100644 index 00000000..748f10f9 --- /dev/null +++ b/libraries/chain/hardfork.d/CORE_429.hf @@ -0,0 +1,4 @@ +// bitshares-core #429 rounding issue when creating assets +#ifndef HARDFORK_CORE_429_TIME +#define HARDFORK_CORE_429_TIME (fc::time_point_sec( 1511793600 )) +#endif diff --git a/libraries/chain/include/graphene/chain/asset_evaluator.hpp b/libraries/chain/include/graphene/chain/asset_evaluator.hpp index 26605a4f..27fb1aa2 100644 --- a/libraries/chain/include/graphene/chain/asset_evaluator.hpp +++ b/libraries/chain/include/graphene/chain/asset_evaluator.hpp @@ -40,6 +40,8 @@ namespace graphene { namespace chain { * post the fee to fee_paying_account_stats.pending_fees */ virtual void pay_fee() override; + private: + bool fee_is_odd; }; class asset_issue_evaluator : public evaluator diff --git a/tests/tests/fee_tests.cpp b/tests/tests/fee_tests.cpp index f5545232..affa1bff 100644 --- a/tests/tests/fee_tests.cpp +++ b/tests/tests/fee_tests.cpp @@ -990,6 +990,23 @@ BOOST_AUTO_TEST_CASE( issue_429_test ) } verify_asset_supplies( db ); + + generate_blocks( HARDFORK_CORE_429_TIME + 10 ); + + { + signed_transaction tx; + asset_create_operation op; + op.issuer = alice_id; + op.symbol = "ALICE.ODDER"; + op.common_options.core_exchange_rate = asset( 1 ) / asset( 1, asset_id_type( 1 ) ); + op.fee = asset((fees_to_pay.long_symbol + fees_to_pay.price_per_kbyte) | 1); + tx.operations.push_back( op ); + set_expiration( db, tx ); + sign( tx, alice_private_key ); + PUSH_TX( db, tx ); + } + + verify_asset_supplies( db ); } catch( const fc::exception& e ) { From e75f18ba571aa952adf289c92dae89c52c3713dd Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Fri, 23 Mar 2018 15:13:45 +0100 Subject: [PATCH 6/6] Removed hardfork code for CORE_429 --- libraries/chain/asset_evaluator.cpp | 11 +---------- libraries/chain/hardfork.d/CORE_429.hf | 4 ---- tests/tests/fee_tests.cpp | 2 -- 3 files changed, 1 insertion(+), 16 deletions(-) delete mode 100644 libraries/chain/hardfork.d/CORE_429.hf diff --git a/libraries/chain/asset_evaluator.cpp b/libraries/chain/asset_evaluator.cpp index 8103e2b5..1346584c 100644 --- a/libraries/chain/asset_evaluator.cpp +++ b/libraries/chain/asset_evaluator.cpp @@ -128,20 +128,11 @@ void asset_create_evaluator::pay_fee() object_id_type asset_create_evaluator::do_apply( const asset_create_operation& op ) { try { - bool hf_429 = fee_is_odd && db().head_block_time() > HARDFORK_CORE_429_TIME; - const asset_dynamic_data_object& dyn_asset = db().create( [&]( asset_dynamic_data_object& a ) { a.current_supply = 0; - a.fee_pool = core_fee_paid - (hf_429 ? 1 : 0); + a.fee_pool = core_fee_paid - (fee_is_odd ? 1 : 0); }); - if( fee_is_odd && !hf_429 ) - { - const auto& core_dd = db().get( asset_id_type() ).dynamic_data( db() ); - db().modify( core_dd, [=]( asset_dynamic_data_object& dd ) { - dd.current_supply++; - }); - } asset_bitasset_data_id_type bit_asset_id; if( op.bitasset_opts.valid() ) diff --git a/libraries/chain/hardfork.d/CORE_429.hf b/libraries/chain/hardfork.d/CORE_429.hf deleted file mode 100644 index 748f10f9..00000000 --- a/libraries/chain/hardfork.d/CORE_429.hf +++ /dev/null @@ -1,4 +0,0 @@ -// bitshares-core #429 rounding issue when creating assets -#ifndef HARDFORK_CORE_429_TIME -#define HARDFORK_CORE_429_TIME (fc::time_point_sec( 1511793600 )) -#endif diff --git a/tests/tests/fee_tests.cpp b/tests/tests/fee_tests.cpp index affa1bff..6d2d489d 100644 --- a/tests/tests/fee_tests.cpp +++ b/tests/tests/fee_tests.cpp @@ -991,8 +991,6 @@ BOOST_AUTO_TEST_CASE( issue_429_test ) verify_asset_supplies( db ); - generate_blocks( HARDFORK_CORE_429_TIME + 10 ); - { signed_transaction tx; asset_create_operation op;