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

BSIP81: Simple maker-taker fees #2136

Merged
merged 22 commits into from
Apr 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2f2ec4d
BSIP81: Define taker fee
MichelSantos Mar 30, 2020
b252cf6
BSIP81: Use maker or taker fee when calculating market fee
MichelSantos Mar 30, 2020
a60feec
BSIP81: Define hardfork time
MichelSantos Mar 30, 2020
58767d9
BSIP81: Hardfork guards on asset operations
MichelSantos Mar 30, 2020
e9c8f59
BSIP81: Assign default taker fees at hardfork activation time
MichelSantos Mar 30, 2020
ebff7f1
BSIP81: Test the setting of taker fees
MichelSantos Apr 1, 2020
501cb1e
BSIP81: Test default values of taker fees after hardfork activation
MichelSantos Apr 1, 2020
fe590d1
BSIP81: Test taker fees when complete filling of UIA orders
MichelSantos Apr 4, 2020
c7ed43b
BSIP81: Hardfork guards on asset operations in proposals
MichelSantos Apr 9, 2020
36fbf2d
BSIP81: Test taker fees when complete filling of UIA orders
MichelSantos Apr 9, 2020
9d73860
BSIP81: Test taker fees when complete filling of a smart asset order
MichelSantos Apr 9, 2020
9b9ed84
BSIP81: Test maker and taker fees for partially filled limit orders
MichelSantos Apr 10, 2020
bab6d23
BSIP81: Comments
MichelSantos Apr 13, 2020
86a7233
BSIP81: Remove unused public function from evaluator
MichelSantos Apr 13, 2020
8e221be
BSIP81: Fix bug of zero taker fees whenever maker fee is set to 0%
MichelSantos Apr 13, 2020
4f339c6
BSIP81: Refactor taker_fee_percent into asset_options.extensions
MichelSantos Apr 14, 2020
2be77d9
BSIP81: Refactor unit tests to obtain the final asset objects from th…
MichelSantos Apr 14, 2020
759c91a
BSIP81: Comments
MichelSantos Apr 15, 2020
6d750c6
BSIP81: Taker orders should be charged the taker fee if defined, othe…
MichelSantos Apr 15, 2020
3408aac
BSIP81: Merge resolution with Issue 1780
MichelSantos Apr 18, 2020
f99d81f
BSIP81: Test improvements
MichelSantos Apr 21, 2020
0f712cc
BSIP81: Test default taker fee usage after hardfork if the asset owne…
MichelSantos Apr 21, 2020
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
30 changes: 25 additions & 5 deletions libraries/chain/asset_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,22 @@ namespace detail {
"Asset extension reward percent must be less than 100% till HARDFORK_1774_TIME!");
}
}

// TODO review and remove code below and links to it after HARDFORK_BSIP_81_TIME
void check_asset_options_hf_bsip81(const fc::time_point_sec& block_time, const asset_options& options)
{
if (block_time < HARDFORK_BSIP_81_TIME) {
// Taker fees should be zero until activation of BSIP81
FC_ASSERT(!options.extensions.value.taker_fee_percent.valid(),
"Taker fee percent should not be defined before HARDFORK_BSIP_81_TIME");
}
}
}

void_result asset_create_evaluator::do_evaluate( const asset_create_operation& op )
{ try {

database& d = db();
const database& d = db();

const auto& chain_parameters = d.get_global_properties().parameters;
FC_ASSERT( op.common_options.whitelist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );
Expand All @@ -68,8 +78,10 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
auto asset_symbol_itr = asset_indx.find( op.symbol );
FC_ASSERT( asset_symbol_itr == asset_indx.end() );

// Define now from the current block time
const time_point_sec now = d.head_block_time();
MichelSantos marked this conversation as resolved.
Show resolved Hide resolved
// This must remain due to "BOND.CNY" being allowed before this HF
if( d.head_block_time() > HARDFORK_385_TIME )
if( now > HARDFORK_385_TIME )
{
auto dotpos = op.symbol.rfind( '.' );
if( dotpos != std::string::npos )
Expand Down Expand Up @@ -107,6 +119,9 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
FC_ASSERT( op.precision == op.bitasset_opts->short_backing_asset(d).precision );
}

// Check the taker fee percent
detail::check_asset_options_hf_bsip81(now, op.common_options);

return void_result();
} FC_CAPTURE_AND_RETHROW( (op) ) }

Expand Down Expand Up @@ -267,7 +282,8 @@ static void validate_new_issuer( const database& d, const asset_object& a, accou

void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
{ try {
database& d = db();
const database& d = db();
const time_point_sec now = d.head_block_time();

const asset_object& a = o.asset_to_update(d);
auto a_copy = a;
Expand All @@ -276,7 +292,7 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)

if( o.new_issuer )
{
FC_ASSERT( d.head_block_time() < HARDFORK_CORE_199_TIME,
FC_ASSERT( now < HARDFORK_CORE_199_TIME,
"Since Hardfork #199, updating issuer requires the use of asset_update_issuer_operation.");
validate_new_issuer( d, a, *o.new_issuer );
}
Expand Down Expand Up @@ -308,6 +324,9 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
for( auto id : o.new_options.blacklist_authorities )
d.get_object(id);

// Check the taker fee percent
detail::check_asset_options_hf_bsip81(now, o.new_options);

return void_result();
} FC_CAPTURE_AND_RETHROW((o)) }

Expand Down Expand Up @@ -761,7 +780,8 @@ operation_result asset_settle_evaluator::do_apply(const asset_settle_evaluator::
// performance loss. Needs testing.
if( d.head_block_time() >= HARDFORK_CORE_1780_TIME )
{
auto issuer_fees = d.pay_market_fees( fee_paying_account, settled_amount.asset_id(d), settled_amount );
const bool is_maker = false; // Settlement orders are takers
auto issuer_fees = d.pay_market_fees( fee_paying_account, settled_amount.asset_id(d), settled_amount , is_maker );
settled_amount -= issuer_fees;
}

Expand Down
1 change: 1 addition & 0 deletions libraries/chain/db_maint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,7 @@ void process_hf_868_890( database& db, bool skip_check_call_orders )
} // for each market issued asset
}


abitmore marked this conversation as resolved.
Show resolved Hide resolved
/**
* @brief Remove any custom active authorities whose expiration dates are in the past
* @param db A mutable database reference
Expand Down
30 changes: 23 additions & 7 deletions libraries/chain/db_market.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ bool database::fill_limit_order( const limit_order_object& order, const asset& p
const account_object& seller = order.seller(*this);
const asset_object& recv_asset = receives.asset_id(*this);

auto issuer_fees = pay_market_fees(&seller, recv_asset, receives);
auto issuer_fees = pay_market_fees(&seller, recv_asset, receives, is_maker);

pay_order( seller, receives - issuer_fees, pays );

Expand Down Expand Up @@ -925,7 +925,7 @@ bool database::fill_settle_order( const force_settlement_object& settle, const a
if( head_block_time() >= HARDFORK_CORE_1780_TIME )
settle_owner_ptr = &settle.owner(*this);

auto issuer_fees = pay_market_fees( settle_owner_ptr, get(receives.asset_id), receives );
auto issuer_fees = pay_market_fees( settle_owner_ptr, get(receives.asset_id), receives, is_maker );

if( pays < settle.balance )
{
Expand Down Expand Up @@ -1174,16 +1174,31 @@ void database::pay_order( const account_object& receiver, const asset& receives,
adjust_balance(receiver.get_id(), receives);
}

asset database::calculate_market_fee( const asset_object& trade_asset, const asset& trade_amount )
asset database::calculate_market_fee( const asset_object& trade_asset, const asset& trade_amount, const bool& is_maker)
{
assert( trade_asset.id == trade_amount.asset_id );

if( !trade_asset.charges_market_fees() )
return trade_asset.amount(0);
if( trade_asset.options.market_fee_percent == 0 )
// Optimization: The fee is zero if the order is a maker, and the maker fee percent is 0%
if( is_maker && trade_asset.options.market_fee_percent == 0 )
return trade_asset.amount(0);

auto value = detail::calculate_percent(trade_amount.amount, trade_asset.options.market_fee_percent);
// Optimization: The fee is zero if the order is a taker, and the taker fee percent is 0%
const optional<uint16_t>& taker_fee_percent = trade_asset.options.extensions.value.taker_fee_percent;
if(!is_maker && taker_fee_percent.valid() && *taker_fee_percent == 0)
return trade_asset.amount(0);

uint16_t fee_percent;
if (is_maker) {
// Maker orders are charged the maker fee percent
fee_percent = trade_asset.options.market_fee_percent;
} else {
// Taker orders are charged the taker fee percent if they are valid. Otherwise, the maker fee percent.
fee_percent = taker_fee_percent.valid() ? *taker_fee_percent : trade_asset.options.market_fee_percent;
}

auto value = detail::calculate_percent(trade_amount.amount, fee_percent);
asset percent_fee = trade_asset.amount(value);

if( percent_fee.amount > trade_asset.options.max_market_fee )
Expand All @@ -1192,9 +1207,10 @@ asset database::calculate_market_fee( const asset_object& trade_asset, const ass
return percent_fee;
}

asset database::pay_market_fees(const account_object* seller, const asset_object& recv_asset, const asset& receives )
asset database::pay_market_fees(const account_object* seller, const asset_object& recv_asset, const asset& receives,
const bool& is_maker)
{
const auto market_fees = calculate_market_fee( recv_asset, receives );
const auto market_fees = calculate_market_fee( recv_asset, receives, is_maker );
auto issuer_fees = market_fees;
FC_ASSERT( issuer_fees <= receives, "Market fee shouldn't be greater than receives");
//Don't dirty undo state if not actually collecting any fees
Expand Down
5 changes: 5 additions & 0 deletions libraries/chain/hardfork.d/BSIP_81.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// BSIP 81 (Simple Maker-Taker Market Fees) hardfork check
#ifndef HARDFORK_BSIP_81_TIME
// Jan 1 2030, midnight this is a dummy date until a hardfork date is scheduled
#define HARDFORK_BSIP_81_TIME (fc::time_point_sec( 1893456000 ))
#endif
5 changes: 3 additions & 2 deletions libraries/chain/include/graphene/chain/database.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,9 @@ namespace graphene { namespace chain {
// helpers to fill_order
void pay_order( const account_object& receiver, const asset& receives, const asset& pays );

asset calculate_market_fee(const asset_object& recv_asset, const asset& trade_amount);
asset pay_market_fees(const account_object* seller, const asset_object& recv_asset, const asset& receives );
asset calculate_market_fee(const asset_object& recv_asset, const asset& trade_amount, const bool& is_maker);
asset pay_market_fees(const account_object* seller, const asset_object& recv_asset, const asset& receives,
const bool& is_maker);
///@}


Expand Down
2 changes: 0 additions & 2 deletions libraries/chain/include/graphene/chain/market_evaluator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ namespace graphene { namespace chain {
void_result do_evaluate( const limit_order_create_operation& o );
object_id_type do_apply( const limit_order_create_operation& o );

asset calculate_market_fee( const asset_object* aobj, const asset& trade_amount );

/** override the default behavior defined by generic_evalautor
*/
virtual void convert_fee() override;
Expand Down
12 changes: 10 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_1774(const fc::time_point_sec& block_time, const asset_options& options);
void check_asset_options_hf_bsip81(const fc::time_point_sec& block_time, const asset_options& options);
}

struct proposal_operation_hardfork_visitor
Expand All @@ -45,13 +46,19 @@ 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 {
// hf_1774
detail::check_asset_options_hf_1774(block_time, v.common_options);

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

// HARDFORK_BSIP_81
detail::check_asset_options_hf_bsip81(block_time, v.new_options);
}

void operator()(const graphene::chain::committee_member_update_global_parameters_operation &op) const {
Expand Down Expand Up @@ -94,6 +101,7 @@ struct proposal_operation_hardfork_visitor
void operator()(const graphene::chain::custom_authority_delete_operation&) const {
FC_ASSERT( HARDFORK_BSIP_40_PASSED(block_time), "Not allowed until hardfork BSIP 40" );
}

// loop and self visit in proposals
void operator()(const graphene::chain::proposal_create_operation &v) const {
bool already_contains_proposal_update = false;
Expand Down
6 changes: 6 additions & 0 deletions libraries/protocol/asset_ops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,13 @@ void asset_options::validate()const
{
FC_ASSERT( max_supply > 0 );
FC_ASSERT( max_supply <= GRAPHENE_MAX_SHARE_SUPPLY );
// The non-negative maker fee must be less than or equal to 100%
FC_ASSERT( market_fee_percent <= GRAPHENE_100_PERCENT );

// The non-negative taker fee must be less than or equal to 100%
if( extensions.value.taker_fee_percent.valid() )
FC_ASSERT( *extensions.value.taker_fee_percent <= GRAPHENE_100_PERCENT );

FC_ASSERT( max_market_fee >= 0 && max_market_fee <= GRAPHENE_MAX_SHARE_SUPPLY );
// There must be no high bits in permissions whose meaning is not known.
FC_ASSERT( !(issuer_permissions & ~ASSET_ISSUER_PERMISSION_MASK) );
Expand Down
6 changes: 5 additions & 1 deletion libraries/protocol/include/graphene/protocol/asset_ops.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ namespace graphene { namespace protocol {
{
fc::optional<uint16_t> reward_percent;
fc::optional<flat_set<account_id_type>> whitelist_market_fee_sharing;
// After BSIP81 activation, taker_fee_percent is the taker fee
fc::optional<uint16_t> taker_fee_percent;
};
typedef extension<additional_asset_options> additional_asset_options_t;

Expand All @@ -49,6 +51,8 @@ namespace graphene { namespace protocol {
/// When this asset is traded on the markets, this percentage of the total traded will be exacted and paid
/// to the issuer. This is a fixed point value, representing hundredths of a percent, i.e. a value of 100
/// in this field means a 1% fee is charged on market trades of this asset.
// BSIP81: Asset owners may specify different market fee rate for maker orders and taker orders
// After BSIP81 activation, market_fee_percent is the maker fee
uint16_t market_fee_percent = 0;
/// Market fees calculated as @ref market_fee_percent of the traded volume are capped to this value
share_type max_market_fee = GRAPHENE_MAX_SHARE_SUPPLY;
Expand Down Expand Up @@ -544,7 +548,7 @@ FC_REFLECT( graphene::protocol::bitasset_options,
(extensions)
)

FC_REFLECT( graphene::protocol::additional_asset_options, (reward_percent)(whitelist_market_fee_sharing) )
FC_REFLECT( graphene::protocol::additional_asset_options, (reward_percent)(whitelist_market_fee_sharing)(taker_fee_percent) )
FC_REFLECT( graphene::protocol::asset_create_operation::fee_parameters_type, (symbol3)(symbol4)(long_symbol)(price_per_kbyte) )
FC_REFLECT( graphene::protocol::asset_global_settle_operation::fee_parameters_type, (fee) )
FC_REFLECT( graphene::protocol::asset_settle_operation::fee_parameters_type, (fee) )
Expand Down
Loading