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

Distribute Asset Market Fees to Referral Program #1419

Merged
merged 17 commits into from Jan 29, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -96,6 +96,7 @@ class database_api_impl : public std::enable_shared_from_this<database_api_impl>
vector<balance_object> get_balance_objects( const vector<address>& addrs )const;
vector<asset> get_vested_balances( const vector<balance_id_type>& objs )const;
vector<vesting_balance_object> get_vesting_balances( const std::string account_id_or_name )const;
vector<vesting_balance_object> get_mfs_vesting_balances( const std::string account_id_or_name )const;

// Assets
vector<optional<asset_object>> get_assets(const vector<asset_id_type>& asset_ids)const;
@@ -1026,6 +1027,11 @@ vector<vesting_balance_object> database_api::get_vesting_balances( const std::st
return my->get_vesting_balances( account_id_or_name );
}

vector<vesting_balance_object> database_api::get_mfs_vesting_balances( const std::string account_id_or_name )const
{
return my->get_mfs_vesting_balances( account_id_or_name );
}

vector<vesting_balance_object> database_api_impl::get_vesting_balances( const std::string account_id_or_name )const
{
try
@@ -1042,6 +1048,27 @@ vector<vesting_balance_object> database_api_impl::get_vesting_balances( const st
FC_CAPTURE_AND_RETHROW( (account_id_or_name) );
}

vector<vesting_balance_object> database_api_impl::get_mfs_vesting_balances( const std::string account_id_or_name )const
{
try
{
const account_id_type account_id = get_account_from_string(account_id_or_name)->id;
vector<vesting_balance_object> result;

auto& vesting_balances = _db.get_index_type<vesting_balance_index>().indices().get<by_vesting_type>();
auto key = boost::make_tuple(account_id, vesting_balance_type::market_fee_sharing);
auto mfs_vesting_range = vesting_balances.equal_range(key);

std::for_each(mfs_vesting_range.first, mfs_vesting_range.second,
[&result](const vesting_balance_object& balance) {
result.emplace_back(balance);
});

return result;
}
FC_CAPTURE_AND_RETHROW( (account_id_or_name) );
}

//////////////////////////////////////////////////////////////////////
// //
// Assets //
@@ -364,6 +364,8 @@ class database_api

vector<vesting_balance_object> get_vesting_balances( const std::string account_id_or_name )const;

vector<vesting_balance_object> get_mfs_vesting_balances( const std::string account_id_or_name )const;

/**
* @brief Get the total number of accounts registered with the blockchain
*/
@@ -784,6 +786,7 @@ FC_API(graphene::app::database_api,
(get_balance_objects)
(get_vested_balances)
(get_vesting_balances)
(get_mfs_vesting_balances)

// Assets
(get_assets)
@@ -45,6 +45,12 @@ 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 );

if( d.head_block_time() < HARDFORK_1268_TIME )
{
FC_ASSERT(!op.common_options.additional_options.value.null_ext.valid());
FC_ASSERT(!op.common_options.additional_options.value.reward_percent.valid());
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please change this like you did it in asset_update, with a nice error message.

}

// Check that all authorities do exist
for( auto id : op.common_options.whitelist_authorities )
d.get_object(id);
@@ -277,6 +283,12 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
validate_new_issuer( d, a, *o.new_issuer );
}

if ( o.new_options.additional_options.value.reward_percent.valid() )
{
FC_ASSERT( d.head_block_time() >= HARDFORK_1268_TIME,
"Referrer percent is only available after HARDFORK_1268_TIME!");
}

if( (d.head_block_time() < HARDFORK_572_TIME) || (a.dynamic_asset_data_id(d).current_supply != 0) )
{
// new issuer_permissions must be subset of old issuer permissions
@@ -28,6 +28,7 @@
#include <graphene/chain/asset_object.hpp>
#include <graphene/chain/vesting_balance_object.hpp>
#include <graphene/chain/witness_object.hpp>
#include <boost/range/algorithm.hpp>

namespace graphene { namespace chain {

@@ -80,6 +81,40 @@ void database::adjust_balance(account_id_type account, asset delta )

} FC_CAPTURE_AND_RETHROW( (account)(delta) ) }

void database::deposit_market_fee_vesting_balance(const account_id_type &account_id, const asset &delta)
{ try {
FC_ASSERT( delta.amount > 0, "Invalid negative value for balance");
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

>=, or the next if doesn't make sense


if( delta.amount == 0 )
return;

auto& vesting_balances = get_index_type<vesting_balance_index>().indices().get<by_vesting_type>();
auto market_vesting_balances = vesting_balances.equal_range(boost::make_tuple(account_id, vesting_balance_type::market_fee_sharing));
auto market_balance = boost::range::find_if(market_vesting_balances,
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

This is very inefficient, should make asset_id part of the index.

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 9, 2018

Author Contributor

We are going to start an optimisation a bit later.

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 27, 2018

Author Contributor

Done.

[&delta](const vesting_balance_object& vbo) { return vbo.balance.asset_id == delta.asset_id;}
);

if(market_balance == boost::end(market_vesting_balances) )
{
create<vesting_balance_object>([&](vesting_balance_object &vbo) {
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please capture required variables explicitly.

vbo.owner = account_id;
vbo.balance = delta;
vbo.balance_type = vesting_balance_type::market_fee_sharing;
cdd_vesting_policy policy;
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please use linear vesting instead, it requires much less computation on balance changes.

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 9, 2018

Author Contributor

What do you think about creating a new type of policy (e.g. "vested_policy") that doesn't do any computation connected with time and
change get_allowed_withdraw method to return total amount allowed for withdrawal (everything that is on a vesting_balance_object::balance)

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 10, 2018

Contributor

sounds good.

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 16, 2018

Author Contributor

Done.

policy.vesting_seconds = { 0 };
policy.coin_seconds_earned = vbo.balance.amount.value;
policy.coin_seconds_earned_last_update = head_block_time();
vbo.policy = policy;
});
} else {
modify( *market_balance, [&]( vesting_balance_object& vbo )
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please capture required variables explicitly.

{
vbo.deposit_vested(head_block_time(), delta);
});
}

} FC_CAPTURE_AND_RETHROW( (account_id)(delta) ) }

optional< vesting_balance_id_type > database::deposit_lazy_vesting(
const optional< vesting_balance_id_type >& ovbid,
share_type amount, uint32_t req_vesting_seconds,
@@ -31,7 +31,17 @@

#include <fc/uint128.hpp>

namespace graphene { namespace chain {
namespace graphene { namespace chain { namespace detail {

fc::uint128 calculate_percent(const share_type& value, uint16_t percent)
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Perhaps change return value to uint64_t so you don't always have to call to_uint64?

{
fc::uint128 a(value.value);
a *= percent;
a /= GRAPHENE_100_PERCENT;
return a;
}

} //detail

/**
* All margin positions are force closed at the swan price
@@ -775,7 +785,12 @@ 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( recv_asset, receives );
//auto issuer_fees = pay_market_fees( recv_asset, receives );
//auto issuer_fees = pay_market_fees(seller, recv_asset, receives);
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please remove commented-out lines

auto issuer_fees = ( head_block_time() < HARDFORK_1268_TIME ) ?
This conversation was marked as resolved by oxarbitrage

This comment has been minimized.

Copy link
@oxarbitrage

oxarbitrage Dec 19, 2018

Member

please also add comment here so we can just leave pay_market_fees(seller, recv_asset, receives); after hardfork. thanks.

This comment has been minimized.

Copy link
@oxarbitrage

oxarbitrage Dec 19, 2018

Member

sorry, in this one is not possible to remove it, my mistake, please ignore.

pay_market_fees(recv_asset, receives) :
pay_market_fees(seller, recv_asset, receives);

pay_order( seller, receives - issuer_fees, pays );

assert( pays.asset_id != receives.asset_id );
@@ -1109,10 +1124,8 @@ asset database::calculate_market_fee( const asset_object& trade_asset, const ass
if( trade_asset.options.market_fee_percent == 0 )
return trade_asset.amount(0);

fc::uint128 a(trade_amount.amount.value);
a *= trade_asset.options.market_fee_percent;
a /= GRAPHENE_100_PERCENT;
asset percent_fee = trade_asset.amount(a.to_uint64());
auto value = detail::calculate_percent(trade_amount.amount, trade_asset.options.market_fee_percent);
asset percent_fee = trade_asset.amount(value.to_uint64());

if( percent_fee.amount > trade_asset.options.max_market_fee )
percent_fee.amount = trade_asset.options.max_market_fee;
@@ -1138,4 +1151,53 @@ asset database::pay_market_fees( const asset_object& recv_asset, const asset& re
return issuer_fees;
}

asset database::pay_market_fees(const account_object& seller, const asset_object& recv_asset, const asset& receives )
{
FC_ASSERT( head_block_time() >= HARDFORK_1268_TIME );
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

can remove this


const auto issuer_fees = calculate_market_fee( recv_asset, receives );

assert(issuer_fees <= receives );
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please make this an FC_ASSERT


//Don't dirty undo state if not actually collecting any fees
if( issuer_fees.amount > 0 )
{
// calculate and pay rewards
asset reward = recv_asset.amount(0);

const auto reward_percent = recv_asset.options.additional_options.value.reward_percent;

if ( reward_percent && *reward_percent )
{
const auto reward_value = detail::calculate_percent(issuer_fees.amount, *reward_percent);

if ( reward_value > 0 )
{
reward = recv_asset.amount(reward_value.to_uint64());

assert( reward < issuer_fees );
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please make this an FC_ASSERT

// cut referrer percent from reward
const auto referrer_rewards_percentage = seller.referrer_rewards_percentage;
const auto referrer_rewards_value = detail::calculate_percent(reward.amount, referrer_rewards_percentage);

This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please add an FC_ASSERT that referrer_rewards_value <= reward

auto registrar_reward = reward;
if ( referrer_rewards_value > 0 )
{
const asset referrer_reward = recv_asset.amount(referrer_rewards_value.to_uint64());
registrar_reward -= referrer_reward;
deposit_market_fee_vesting_balance(seller.referrer, referrer_reward);
}
deposit_market_fee_vesting_balance(seller.registrar, registrar_reward);
}
}

const auto& recv_dyn_data = recv_asset.dynamic_asset_data_id(*this);
modify( recv_dyn_data, [&]( asset_dynamic_data_object& obj ){
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please capture required variables explicitly.

obj.accumulated_fees += issuer_fees.amount - reward.amount;
});
}

return issuer_fees;
}

} }
@@ -0,0 +1,4 @@
// #1268 Distribute Asset Market Fees to Referral Program
#ifndef HARDFORK_1268_TIME
#define HARDFORK_1268_TIME (fc::time_point_sec( 1530705600 )) // Wednesday, July 4, 2018 12:00:00 PM
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please move this timestamp into the far future.
And add a linefeed to the last line.

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 9, 2018

Author Contributor

Of course, we will change it during the last iteration. Right now the current value is more suitable for us because our smoke tests cover the market fee sharing logic after HARDFORK_1268_TIME

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Jan 24, 2019

Author Contributor

Done.

#endif
@@ -303,6 +303,8 @@ namespace graphene { namespace chain {
*/
void adjust_balance(account_id_type account, asset delta);

void deposit_market_fee_vesting_balance(const account_id_type &account, const asset &delta);

/**
* @brief Helper to make lazy deposit to CDD VBO.
*
@@ -395,6 +397,7 @@ namespace graphene { namespace chain {

asset calculate_market_fee(const asset_object& recv_asset, const asset& trade_amount);
asset pay_market_fees( const asset_object& recv_asset, const asset& receives );
asset pay_market_fees( const account_object& seller, const asset_object& recv_asset, const asset& receives );


///@{
@@ -27,6 +27,13 @@

namespace graphene { namespace chain {

struct additional_asset_options
{
fc::optional<void_t> null_ext;
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Can remove this.

fc::optional<uint16_t> reward_percent;
};
typedef extension<additional_asset_options> additional_asset_options_t;

bool is_valid_symbol( const string& symbol );

/**
@@ -75,7 +82,7 @@ namespace graphene { namespace chain {
* size of description.
*/
string description;
extensions_type extensions;
additional_asset_options_t additional_options;

/// Perform internal consistency checks.
/// @throws fc::exception if any check fails
@@ -523,7 +530,7 @@ FC_REFLECT( graphene::chain::asset_options,
(whitelist_markets)
(blacklist_markets)
(description)
(extensions)
(additional_options)
)
FC_REFLECT( graphene::chain::bitasset_options,
(feed_lifetime_sec)
@@ -535,7 +542,7 @@ FC_REFLECT( graphene::chain::bitasset_options,
(extensions)
)


FC_REFLECT( graphene::chain::additional_asset_options, (null_ext)(reward_percent) )
FC_REFLECT( graphene::chain::asset_create_operation::fee_parameters_type, (symbol3)(symbol4)(long_symbol)(price_per_kbyte) )
FC_REFLECT( graphene::chain::asset_global_settle_operation::fee_parameters_type, (fee) )
FC_REFLECT( graphene::chain::asset_settle_operation::fee_parameters_type, (fee) )
@@ -26,6 +26,7 @@
#include <graphene/chain/protocol/asset.hpp>
#include <graphene/db/object.hpp>
#include <graphene/db/generic_index.hpp>
#include <boost/multi_index/composite_key.hpp>

#include <fc/static_variant.hpp>
#include <fc/uint128.hpp>
@@ -124,6 +125,9 @@ namespace graphene { namespace chain {
cdd_vesting_policy
> vesting_policy;

enum class vesting_balance_type { unspecified,
worker,
market_fee_sharing };
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Please add types for witness and cashback.

/**
* Vesting balance object is a balance that is locked by the blockchain for a period of time.
*/
@@ -140,6 +144,8 @@ namespace graphene { namespace chain {
asset balance;
/// The vesting policy stores details on when funds vest, and controls when they may be withdrawn
vesting_policy policy;
/// type of the vesting balance
vesting_balance_type balance_type = vesting_balance_type::unspecified;

vesting_balance_object() {}

@@ -171,12 +177,22 @@ namespace graphene { namespace chain {
* @ingroup object_index
*/
struct by_account;
struct by_vesting_type;
This conversation was marked as resolved by pmconrad

This comment has been minimized.

Copy link
@pmconrad

pmconrad Jan 8, 2019

Contributor

Please add a comment that the by_vesting_type index MUST NOT be used for iterating because order is not well-defined.


typedef multi_index_container<
vesting_balance_object,
indexed_by<
ordered_unique< tag<by_id>, member< object, object_id_type, &object::id > >,
ordered_unique< tag<by_id>, member< object, object_id_type, &object::id >
>,
ordered_non_unique< tag<by_account>,
member<vesting_balance_object, account_id_type, &vesting_balance_object::owner>
>,
ordered_non_unique< tag<by_vesting_type>,
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

This must be made unique, for example by adding object::id as the last key component.
(Order must be well-defined so that consensus logic always agrees which balance is affected.)

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 27, 2018

Author Contributor

Done.

composite_key<
vesting_balance_object,
member<vesting_balance_object, account_id_type, &vesting_balance_object::owner>,
member<vesting_balance_object, vesting_balance_type, &vesting_balance_object::balance_type>
This conversation was marked as resolved by OpenLedgerApp

This comment has been minimized.

Copy link
@pmconrad

pmconrad Nov 7, 2018

Contributor

Also, add asset_id for efficiency (see above).

This comment has been minimized.

Copy link
@OpenLedgerApp

OpenLedgerApp Nov 28, 2018

Author Contributor

Done.
See:
asset database::get_market_fee_vesting_balance() db_balance.cpp:113

>
>
>
> vesting_balance_multi_index_type;
@@ -1455,6 +1455,13 @@ class wallet_api
*/
vector< vesting_balance_object_with_info > get_vesting_balances( string account_name );

/** List account's market fee sharing vesting balances.
* Each account can have multiple market fee sharing vesting balances.
* @param account_name_or_id the name or id of the account whose balances you want
* @returns a list of the given account's market fee sharing vesting balances
*/
vector< vesting_balance_object_with_info > get_mfs_vesting_balances(string account_name_or_id);

/**
* Withdraw a vesting balance.
*
@@ -1799,6 +1806,7 @@ FC_API( graphene::wallet::wallet_api,
(create_worker)
(update_worker_votes)
(get_vesting_balances)
(get_mfs_vesting_balances)
(withdraw_vesting)
(vote_for_committee_member)
(vote_for_witness)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.