Skip to content

Commit

Permalink
Merge pull request #2131 from bitshares/old-hardfork-for-issue-1774
Browse files Browse the repository at this point in the history
Fix issue 1774: Too restrictive check in market fee sharing
  • Loading branch information
abitmore authored Apr 6, 2020
2 parents eba093c + 64fdcdd commit 45aceb1
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 7 deletions.
19 changes: 17 additions & 2 deletions libraries/chain/asset_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,20 @@

#include <functional>

#include <locale>

namespace graphene { namespace chain {
namespace detail {

// 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_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 )
{ try {
Expand All @@ -45,6 +56,8 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
FC_ASSERT( op.common_options.whitelist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );
FC_ASSERT( op.common_options.blacklist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );

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 )
d.get_object(id);
Expand Down Expand Up @@ -268,6 +281,8 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
validate_new_issuer( d, a, *o.new_issuer );
}

detail::check_asset_options_hf_1774(d.head_block_time(), o.new_options);

if( a.dynamic_asset_data_id(d).current_supply != 0 )
{
// new issuer_permissions must be subset of old issuer permissions
Expand Down
11 changes: 9 additions & 2 deletions libraries/chain/db_market.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,13 @@ 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");
// TODO after hf_1774, remove the `if` check, keep the code in `else`
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 not be greater than issuer fees");
}
// cut referrer percent from reward
auto registrar_reward = reward;
if( seller->referrer != seller->registrar )
Expand All @@ -1228,7 +1234,8 @@ asset database::pay_market_fees(const account_object* seller, const asset_object
deposit_market_fee_vesting_balance(seller->referrer, referrer_reward);
}
}
deposit_market_fee_vesting_balance(seller->registrar, registrar_reward);
if( registrar_reward.amount > 0 )
deposit_market_fee_vesting_balance(seller->registrar, registrar_reward);
}
}
}
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
13 changes: 13 additions & 0 deletions libraries/chain/proposal_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@

namespace graphene { namespace chain {

namespace detail {
void check_asset_options_hf_1774(const fc::time_point_sec& block_time, const asset_options& options);
}

struct proposal_operation_hardfork_visitor
{
typedef void result_type;
Expand All @@ -41,6 +45,15 @@ struct proposal_operation_hardfork_visitor
template<typename T>
void operator()(const T &v) const {}

// hf_1774
void operator()(const graphene::chain::asset_create_operation &v) const {
detail::check_asset_options_hf_1774(block_time, v.common_options);
}
// hf_1774
void operator()(const graphene::chain::asset_update_operation &v) const {
detail::check_asset_options_hf_1774(block_time, v.new_options);
}

void operator()(const graphene::chain::committee_member_update_global_parameters_operation &op) const {
if (block_time < HARDFORK_CORE_1468_TIME) {
FC_ASSERT(!op.new_parameters.extensions.value.updatable_htlc_options.valid(), "Unable to set HTLC options before hardfork 1468");
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
117 changes: 115 additions & 2 deletions tests/tests/market_fee_sharing_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ struct reward_database_fixture : database_fixture
PUSH_TX( db, tx);
}

void generate_blocks_past_hf1774()
{
generate_blocks( HARDFORK_1774_TIME );
generate_block();
set_expiration(db, trx);
}

asset core_asset(int64_t x )
{
return asset( x*core_precision );
Expand All @@ -108,7 +115,7 @@ struct reward_database_fixture : database_fixture

BOOST_FIXTURE_TEST_SUITE( fee_sharing_tests, reward_database_fixture )

BOOST_AUTO_TEST_CASE(create_asset_with_additional_options_after_hf)
BOOST_AUTO_TEST_CASE(cannot_create_asset_with_reward_percent_of_100_before_hf1774)
{
try
{
Expand Down Expand Up @@ -160,7 +167,7 @@ BOOST_AUTO_TEST_CASE(create_asset_with_additional_options_after_hf)
FC_LOG_AND_RETHROW()
}

BOOST_AUTO_TEST_CASE(update_additional_options_after_hf)
BOOST_AUTO_TEST_CASE(cannot_set_reward_percent_to_100_before_hf1774)
{
try
{
Expand Down Expand Up @@ -190,6 +197,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
{
generate_blocks_past_hf1774();

ACTOR(issuer);

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); // make a copy

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 @@ -884,4 +944,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()

0 comments on commit 45aceb1

Please sign in to comment.