Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 1774: Too restrictive check in market fee sharing #1848

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions libraries/chain/asset_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand All @@ -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 )
Expand Down Expand Up @@ -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) )
{
Expand Down
7 changes: 6 additions & 1 deletion libraries/chain/db_market.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,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 ){
abitmore marked this conversation as resolved.
Show resolved Hide resolved
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 )
Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/hardfork.d/CORE_1774.hf
Original file line number Diff line number Diff line change
@@ -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
7 changes: 5 additions & 2 deletions libraries/chain/proposal_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion libraries/protocol/asset_ops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
118 changes: 115 additions & 3 deletions tests/tests/market_fee_sharing_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand All @@ -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
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()