Merge pull request #1848 from openledger/issue-1774

Issue 1774: Too restrictive check in market fee sharing

libraries/chain/asset_evaluator.cpp

@@ -48,6 +48,17 @@ namespace detail {
             "Asset extension whitelist_market_fee_sharing is only available after HARDFORK_1268_TIME!");
       }
    }
+
+   // TODO review and remove code below and links to it after hf_1774
+   void check_asset_options_hf_1774(const fc::time_point_sec& block_time, const asset_options& options)
+   {
+      if(  block_time >= HARDFORK_1268_TIME && block_time < HARDFORK_1774_TIME )
+      {
+         FC_ASSERT( !options.extensions.value.reward_percent.valid() ||
+                    *options.extensions.value.reward_percent < GRAPHENE_100_PERCENT,
+            "Asset extension reward percent must be less than 100% till HARDFORK_1774_TIME!");
+      }
+   }
 }
 
 void_result asset_create_evaluator::do_evaluate( const asset_create_operation& op )
@@ -60,6 +71,7 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
    FC_ASSERT( op.common_options.blacklist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );
 
    detail::check_asset_options_hf_1268(d.head_block_time(), op.common_options);
+   detail::check_asset_options_hf_1774(d.head_block_time(), op.common_options);
 
    // Check that all authorities do exist
    for( auto id : op.common_options.whitelist_authorities )
@@ -289,6 +301,7 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
    }
 
    detail::check_asset_options_hf_1268(d.head_block_time(), o.new_options);
+   detail::check_asset_options_hf_1774(d.head_block_time(), o.new_options);
 
    if( (d.head_block_time() < HARDFORK_572_TIME) || (a.dynamic_asset_data_id(d).current_supply != 0) )
    {

libraries/chain/db_market.cpp

@@ -1278,7 +1278,12 @@ asset database::pay_market_fees(const account_object& seller, const asset_object
             if ( reward_value > 0 && is_authorized_asset(*this, seller.registrar(*this), recv_asset) )
             {
                reward = recv_asset.amount(reward_value);
-               FC_ASSERT( reward < issuer_fees, "Market reward should be less than issuer fees");
+               if( head_block_time() < HARDFORK_1774_TIME ){
+                  FC_ASSERT( reward < issuer_fees, "Market reward should be less than issuer fees");
+               }
+               else{
+                  FC_ASSERT( reward <= issuer_fees, "Market reward should be less or equal than issuer fees");
+               }
                // cut referrer percent from reward
                auto registrar_reward = reward;
                if( seller.referrer != seller.registrar )

libraries/chain/hardfork.d/CORE_1774.hf

@@ -0,0 +1,4 @@
+// #1774 Too restrictive check in market fee sharing
+#ifndef HARDFORK_1774_TIME
+#define HARDFORK_1774_TIME (fc::time_point_sec( 1600000000 ) ) // September 13, 2020 12:26:40 PM
+#endif

libraries/chain/proposal_evaluator.cpp

@@ -30,6 +30,7 @@ namespace graphene { namespace chain {
 
 namespace detail {
    void check_asset_options_hf_1268(const fc::time_point_sec& block_time, const asset_options& options);
+   void check_asset_options_hf_1774(const fc::time_point_sec& block_time, const asset_options& options);   
    void check_vesting_balance_policy_hf_1268(const fc::time_point_sec& block_time, const vesting_policy_initializer& policy);
 }
 
@@ -57,13 +58,15 @@ struct proposal_operation_hardfork_visitor
             && (*(v.delta_debt.asset_id( db ).bitasset_data_id))( db ).is_prediction_market )
             , "Soft fork - preventing proposal with call_order_update!" );
    }
-   // hf_1268
+   // hf_1268 + hf_1774
    void operator()(const graphene::chain::asset_create_operation &v) const {
       detail::check_asset_options_hf_1268(block_time, v.common_options);
+      detail::check_asset_options_hf_1774(block_time, v.common_options);
    }
-   // hf_1268
+   // hf_1268 + hf_1774
    void operator()(const graphene::chain::asset_update_operation &v) const {
       detail::check_asset_options_hf_1268(block_time, v.new_options);
+      detail::check_asset_options_hf_1774(block_time, v.new_options);
    }
    // hf_1268
    void operator()(const graphene::chain::vesting_balance_create_operation &v) const {

libraries/protocol/asset_ops.cpp

@@ -239,7 +239,7 @@ void asset_options::validate()const
       FC_ASSERT( whitelist_markets.find(item) == whitelist_markets.end() );
    }
    if( extensions.value.reward_percent.valid() )
-      FC_ASSERT( *extensions.value.reward_percent < GRAPHENE_100_PERCENT );
+      FC_ASSERT( *extensions.value.reward_percent <= GRAPHENE_100_PERCENT );
 }
 
 void asset_claim_fees_operation::validate()const {

tests/tests/market_fee_sharing_tests.cpp

@@ -96,6 +96,12 @@ struct reward_database_fixture : database_fixture
       database_fixture::generate_block();
    }
 
+   void generate_blocks_past_hf1774()
+   {
+      database_fixture::generate_blocks( HARDFORK_1774_TIME );
+      database_fixture::generate_block();
+   }
+
    asset core_asset(int64_t x )
    {
        return asset( x*core_precision );
@@ -139,7 +145,7 @@ BOOST_AUTO_TEST_CASE(cannot_create_asset_with_additional_options_before_hf)
    FC_LOG_AND_RETHROW()
 }
 
-BOOST_AUTO_TEST_CASE(create_asset_with_additional_options_after_hf)
+BOOST_AUTO_TEST_CASE(create_asset_with_additional_options_after_hf1268)
 {
    try
    {
@@ -193,7 +199,7 @@ BOOST_AUTO_TEST_CASE(create_asset_with_additional_options_after_hf)
    FC_LOG_AND_RETHROW()
 }
 
-BOOST_AUTO_TEST_CASE(cannot_update_additional_options_before_hf)
+BOOST_AUTO_TEST_CASE(cannot_update_additional_options_before_hf1268)
 {
    try
    {
@@ -209,7 +215,7 @@ BOOST_AUTO_TEST_CASE(cannot_update_additional_options_before_hf)
    FC_LOG_AND_RETHROW()
 }
 
-BOOST_AUTO_TEST_CASE(update_additional_options_after_hf)
+BOOST_AUTO_TEST_CASE(update_additional_options_after_hf1268)
 {
    try
    {
@@ -241,6 +247,59 @@ BOOST_AUTO_TEST_CASE(update_additional_options_after_hf)
    FC_LOG_AND_RETHROW()
 }
 
+BOOST_AUTO_TEST_CASE(create_asset_with_reward_percent_of_100_after_hf1774)
+{
+   try
+   {
+      ACTOR(issuer);
+
+      generate_blocks_past_hf1774();
+
+      uint16_t reward_percent = GRAPHENE_100_PERCENT; // 100.00%
+      flat_set<account_id_type> whitelist = {issuer_id};
+      price price(asset(1, asset_id_type(1)), asset(1));
+      uint16_t market_fee_percent = 100;
+
+      additional_asset_options_t options;
+      options.value.reward_percent = reward_percent;
+      options.value.whitelist_market_fee_sharing = whitelist;
+
+      asset_object usd_asset = create_user_issued_asset("USD",
+                                                        issuer,
+                                                        charge_market_fee,
+                                                        price,
+                                                        2,
+                                                        market_fee_percent,
+                                                        options);
+
+      additional_asset_options usd_options = usd_asset.options.extensions.value;
+      BOOST_CHECK_EQUAL(reward_percent, *usd_options.reward_percent);
+      BOOST_CHECK(whitelist == *usd_options.whitelist_market_fee_sharing);
+   }
+   FC_LOG_AND_RETHROW()
+}
+
+BOOST_AUTO_TEST_CASE(set_reward_percent_to_100_after_hf1774)
+{
+   try
+   {
+      ACTOR(issuer);
+
+      asset_object usd_asset = create_user_issued_asset("USD", issuer, charge_market_fee);
+
+      generate_blocks_past_hf1774();
+
+      uint16_t reward_percent = GRAPHENE_100_PERCENT; // 100.00%
+      flat_set<account_id_type> whitelist = {issuer_id};
+      update_asset(issuer_id, issuer_private_key, usd_asset.get_id(), reward_percent, whitelist);
+
+      additional_asset_options options = usd_asset.get_id()(db).options.extensions.value;
+      BOOST_CHECK_EQUAL(reward_percent, *options.reward_percent);
+      BOOST_CHECK(whitelist == *options.whitelist_market_fee_sharing);
+   }
+   FC_LOG_AND_RETHROW()
+}
+
 BOOST_AUTO_TEST_CASE(asset_rewards_test)
 {
    try
@@ -1002,4 +1061,57 @@ BOOST_AUTO_TEST_CASE( create_vesting_balance_object_test )
 
 } FC_LOG_AND_RETHROW() }
 
+
+BOOST_AUTO_TEST_CASE( possibility_to_set_100_reward_percent_after_hf1774 )
+{
+   try
+   {
+      INVOKE(create_actors);
+
+      generate_blocks_past_hf1774();
+      GET_ACTOR(jill);
+
+      constexpr auto jillcoin_reward_percent = 100*GRAPHENE_1_PERCENT;
+      const asset_object &jillcoin = get_asset("JCOIN");
+
+      update_asset(jill_id, jill_private_key, jillcoin.get_id(), jillcoin_reward_percent);
+
+      GET_ACTOR(izzyregistrar);
+      GET_ACTOR(izzyreferrer);
+      BOOST_CHECK_EQUAL( get_market_fee_reward( izzyregistrar, jillcoin ), 0 );
+      BOOST_CHECK_EQUAL( get_market_fee_reward( izzyreferrer, jillcoin ), 0 );
+
+      GET_ACTOR(alice);
+      GET_ACTOR(bob);
+
+      const share_type sell_amount = 100000;
+
+      // Alice and Bob place orders which match
+      create_sell_order( alice, jillcoin.amount(sell_amount), core_asset(1) );
+      create_sell_order( bob, core_asset(1), jillcoin.amount(sell_amount) );
+
+      const auto izzyregistrar_reward = get_market_fee_reward( izzyregistrar, jillcoin );
+      const auto izzyreferrer_reward = get_market_fee_reward( izzyreferrer, jillcoin );
+
+      BOOST_CHECK_GT(izzyregistrar_reward , 0);
+      BOOST_CHECK_GT(izzyreferrer_reward , 0);
+
+      auto calculate_percent = [](const share_type& value, uint16_t percent)
+      {
+         auto a(value.value);
+         a *= percent;
+         a /= GRAPHENE_100_PERCENT;
+         return a;
+      };
+      //jillcoin has 20% market fee percent, see create_actors
+      //all market fees are distributed between registrar and referrer
+      auto acc_rewards = izzyregistrar_reward + izzyreferrer_reward;
+      BOOST_CHECK_EQUAL( calculate_percent(sell_amount, 20*GRAPHENE_1_PERCENT), acc_rewards);
+
+      //check referrer reward
+      BOOST_CHECK_EQUAL( calculate_percent(acc_rewards, alice.referrer_rewards_percentage), izzyreferrer_reward);
+   }
+   FC_LOG_AND_RETHROW()
+}
+
 BOOST_AUTO_TEST_SUITE_END()