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

Fix #1270 Call price is inconsistent when MCR changed #1324

Merged
merged 36 commits into from
Feb 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
84c7218
BitAsset obj: added maintenance_collateralization
abitmore Aug 31, 2018
6254076
Refactored feed_price::max_short_squeeze_price()
abitmore Aug 31, 2018
9b41a92
Updated comment in price::call_price()
abitmore Aug 31, 2018
5308ea0
Iterates by collateral in globally_settle_asset()
abitmore Aug 31, 2018
c912886
Iterate call orders by collateral in apply_order()
abitmore Aug 31, 2018
469a499
Iterate calls by collateral in check_call_orders()
abitmore Sep 1, 2018
cb7bd0c
db_market: stop using call_price after hf #1270
abitmore Sep 1, 2018
0ba6cb1
Do not update current_maint_collateral before hf
abitmore Sep 4, 2018
92a3b71
check revive: stop using call_price after hf #1270
abitmore Sep 4, 2018
077f4e0
update call: stop using call_price after hf #1270
abitmore Sep 4, 2018
68c702b
Update and match call orders at hf #1270
abitmore Sep 9, 2018
8bcf38d
Check for blackswan by collateral after hf #1270
abitmore Sep 9, 2018
fd1c091
get_call_orders() database API: use by_collateral
abitmore Sep 9, 2018
129e89c
Minor coding style update
abitmore Sep 13, 2018
9af57a8
Bump DB_VERSION due to call_order_object change
abitmore Sep 13, 2018
b5e63a3
Target CR: stop using call_price after hf #1270
abitmore Sep 22, 2018
a56562a
update median feeds when crossing hf1270
oxarbitrage Dec 15, 2018
d7306ab
add testcases for mcr bug(hf1270)
oxarbitrage Dec 15, 2018
fa6b12e
Merge pull request #1469 from oxarbitrage/1270_testcases
abitmore Dec 15, 2018
bf9537f
add support for existing tests after hf1270
oxarbitrage Dec 24, 2018
9f06364
print call orders instead of market in target_cr_test_limit_call
oxarbitrage Dec 25, 2018
8c0152c
Fix target CR calculation
abitmore Dec 25, 2018
613f414
Merge branch '1270-fix-call-price-cache' of https://github.com/bitsha…
oxarbitrage Dec 25, 2018
3aec53d
remove prints after fix
oxarbitrage Dec 25, 2018
46e2943
add hf1270 swan tests
oxarbitrage Dec 27, 2018
4e2514c
skip black_swan_issue_346 test after hf1270
oxarbitrage Jan 16, 2019
7abf7c5
update bitasset tests for hf1270
oxarbitrage Jan 17, 2019
a2d5fac
accurate block generation for hf1270 in market and swan tests
oxarbitrage Jan 24, 2019
8466e19
changes in bitasset_tests/hf_935_test
oxarbitrage Jan 25, 2019
ece99b6
workaround hf_935_test
oxarbitrage Jan 29, 2019
1aad691
use affected_by_hf_343 flag
oxarbitrage Jan 30, 2019
ca1e045
remove trailing white spaces
oxarbitrage Jan 30, 2019
6b22b91
Merge pull request #1493 from oxarbitrage/1270_existing_testcases
abitmore Jan 30, 2019
54c4c02
Merge hardfork branch
abitmore Jan 30, 2019
b494609
Revert to by_price index when globally settling
abitmore Feb 1, 2019
a22f92e
Simplify lambda
abitmore Feb 4, 2019
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
8 changes: 4 additions & 4 deletions libraries/app/database_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1174,13 +1174,13 @@ vector<call_order_object> database_api::get_call_orders(asset_id_type a, uint32_

vector<call_order_object> database_api_impl::get_call_orders(asset_id_type a, uint32_t limit)const
{
const auto& call_index = _db.get_index_type<call_order_index>().indices().get<by_price>();
const auto& call_index = _db.get_index_type<call_order_index>().indices().get<by_collateral>();
const asset_object& mia = _db.get(a);
price index_price = price::min(mia.bitasset_data(_db).options.short_backing_asset, mia.get_id());
price index_price = price::min( mia.bitasset_data(_db).options.short_backing_asset, a );

vector< call_order_object> result;
auto itr_min = call_index.lower_bound(index_price.min());
auto itr_max = call_index.lower_bound(index_price.max());
auto itr_min = call_index.lower_bound(index_price);
auto itr_max = call_index.upper_bound(index_price.max());
while( itr_min != itr_max && result.size() < limit )
{
result.emplace_back(*itr_min);
Expand Down
48 changes: 35 additions & 13 deletions libraries/chain/asset_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ static bool update_bitasset_object_options(
const asset_update_bitasset_operation& op, database& db,
asset_bitasset_data_object& bdo, const asset_object& asset_to_update )
{
const fc::time_point_sec& next_maint_time = db.get_dynamic_global_properties().next_maintenance_time;
const fc::time_point_sec next_maint_time = db.get_dynamic_global_properties().next_maintenance_time;
pmconrad marked this conversation as resolved.
Show resolved Hide resolved
bool after_hf_core_868_890 = ( next_maint_time > HARDFORK_CORE_868_890_TIME );

// If the minimum number of feeds to calculate a median has changed, we need to recalculate the median
Expand Down Expand Up @@ -707,7 +707,7 @@ static bool update_bitasset_object_options(
if( should_update_feeds )
{
const auto old_feed = bdo.current_feed;
bdo.update_median_feeds( db.head_block_time() );
bdo.update_median_feeds( db.head_block_time(), next_maint_time );

// TODO review and refactor / cleanup after hard fork:
// 1. if hf_core_868_890 and core-935 occurred at same time
Expand Down Expand Up @@ -784,8 +784,9 @@ void_result asset_update_feed_producers_evaluator::do_apply(const asset_update_f
{ try {
database& d = db();
const auto head_time = d.head_block_time();
const auto next_maint_time = d.get_dynamic_global_properties().next_maintenance_time;
const asset_bitasset_data_object& bitasset_to_update = asset_to_update->bitasset_data(d);
d.modify( bitasset_to_update, [&o,head_time](asset_bitasset_data_object& a) {
d.modify( bitasset_to_update, [&o,head_time,next_maint_time](asset_bitasset_data_object& a) {
//This is tricky because I have a set of publishers coming in, but a map of publisher to feed is stored.
//I need to update the map such that the keys match the new publishers, but not munge the old price feeds from
//publishers who are being kept.
Expand All @@ -809,7 +810,7 @@ void_result asset_update_feed_producers_evaluator::do_apply(const asset_update_f
{
a.feeds[acc];
}
a.update_median_feeds( head_time );
a.update_median_feeds( head_time, next_maint_time );
});
// Process margin calls, allow black swan, not for a new limit order
d.check_call_orders( *asset_to_update, true, false, &bitasset_to_update );
Expand Down Expand Up @@ -987,27 +988,48 @@ void_result asset_publish_feeds_evaluator::do_apply(const asset_publish_feed_ope
{ try {

database& d = db();
const auto head_time = d.head_block_time();
const auto next_maint_time = d.get_dynamic_global_properties().next_maintenance_time;

const asset_object& base = *asset_ptr;
const asset_bitasset_data_object& bad = *bitasset_ptr;

auto old_feed = bad.current_feed;
// Store medians for this asset
d.modify(bad , [&o,&d](asset_bitasset_data_object& a) {
a.feeds[o.publisher] = make_pair(d.head_block_time(), o.feed);
a.update_median_feeds(d.head_block_time());
d.modify( bad , [&o,head_time,next_maint_time](asset_bitasset_data_object& a) {
a.feeds[o.publisher] = make_pair( head_time, o.feed );
a.update_median_feeds( head_time, next_maint_time );
});

if( !(old_feed == bad.current_feed) )
{
if( bad.has_settlement() ) // implies head_block_time > HARDFORK_CORE_216_TIME
// Check whether need to revive the asset and proceed if need
if( bad.has_settlement() // has globally settled, implies head_block_time > HARDFORK_CORE_216_TIME
&& !bad.current_feed.settlement_price.is_null() ) // has a valid feed
{
bool should_revive = false;
const auto& mia_dyn = base.dynamic_asset_data_id(d);
if( !bad.current_feed.settlement_price.is_null()
&& ( mia_dyn.current_supply == 0
|| ~price::call_price(asset(mia_dyn.current_supply, o.asset_id),
asset(bad.settlement_fund, bad.options.short_backing_asset),
bad.current_feed.maintenance_collateral_ratio ) < bad.current_feed.settlement_price ) )
if( mia_dyn.current_supply == 0 ) // if current supply is zero, revive the asset
should_revive = true;
else // if current supply is not zero, when collateral ratio of settlement fund is greater than MCR, revive the asset
{
if( next_maint_time <= HARDFORK_CORE_1270_TIME )
{
// before core-1270 hard fork, calculate call_price and compare to median feed
if( ~price::call_price( asset(mia_dyn.current_supply, o.asset_id),
asset(bad.settlement_fund, bad.options.short_backing_asset),
bad.current_feed.maintenance_collateral_ratio ) < bad.current_feed.settlement_price )
should_revive = true;
}
else
{
// after core-1270 hard fork, calculate collateralization and compare to maintenance_collateralization
if( price( asset( bad.settlement_fund, bad.options.short_backing_asset ),
asset( mia_dyn.current_supply, o.asset_id ) ) > bad.current_maintenance_collateralization )
should_revive = true;
}
}
if( should_revive )
d.revive_bitasset(base);
}
// Process margin calls, allow black swan, not for a new limit order
Expand Down
25 changes: 14 additions & 11 deletions libraries/chain/asset_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/
#include <graphene/chain/asset_object.hpp>
#include <graphene/chain/database.hpp>
#include <graphene/chain/hardfork.hpp>

#include <fc/uint128.hpp>

Expand All @@ -43,16 +44,10 @@ share_type asset_bitasset_data_object::max_force_settlement_volume(share_type cu
return volume.to_uint64();
}

/******
* @brief calculate the median feed
*
* This calculates the median feed. It sets the current_feed_publication_time
* and current_feed member variables
*
* @param current_time the time to use in the calculations
*/
void graphene::chain::asset_bitasset_data_object::update_median_feeds(time_point_sec current_time)
void graphene::chain::asset_bitasset_data_object::update_median_feeds( time_point_sec current_time,
time_point_sec next_maintenance_time )
{
bool after_core_hardfork_1270 = ( next_maintenance_time > HARDFORK_CORE_1270_TIME ); // call price caching issue
current_feed_publication_time = current_time;
vector<std::reference_wrapper<const price_feed>> current_feeds;
// find feeds that were alive at current_time
Expand All @@ -73,13 +68,18 @@ void graphene::chain::asset_bitasset_data_object::update_median_feeds(time_point
feed_cer_updated = false; // new median cer is null, won't update asset_object anyway, set to false for better performance
current_feed_publication_time = current_time;
current_feed = price_feed();
if( after_core_hardfork_1270 )
current_maintenance_collateralization = price();
return;
}
if( current_feeds.size() == 1 )
{
if( current_feed.core_exchange_rate != current_feeds.front().get().core_exchange_rate )
feed_cer_updated = true;
current_feed = std::move(current_feeds.front());
current_feed = current_feeds.front();
// Note: perhaps can defer updating current_maintenance_collateralization for better performance
if( after_core_hardfork_1270 )
current_maintenance_collateralization = current_feed.maintenance_collateralization();
return;
}

Expand All @@ -100,6 +100,9 @@ void graphene::chain::asset_bitasset_data_object::update_median_feeds(time_point
if( current_feed.core_exchange_rate != median_feed.core_exchange_rate )
feed_cer_updated = true;
current_feed = median_feed;
// Note: perhaps can defer updating current_maintenance_collateralization for better performance
if( after_core_hardfork_1270 )
current_maintenance_collateralization = current_feed.maintenance_collateralization();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unconditionally, i. e. always, setting this will simplify the code significantly. I believe the performance penalty will be insignificant since you get rid of a method parameter and several checks at the same time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there are several 128-bit multiplications and divisions in maintenance_collateralization(), I thought that it costs more. Anyway we can benchmark it (I haven't done).

}


Expand Down Expand Up @@ -157,7 +160,7 @@ asset asset_object::amount_from_string(string amount_string) const
satoshis *= -1;

return amount(satoshis);
} FC_CAPTURE_AND_RETHROW( (amount_string) ) }
} FC_CAPTURE_AND_RETHROW( (amount_string) ) }

string asset_object::amount_to_string(share_type amount) const
{
Expand Down
82 changes: 70 additions & 12 deletions libraries/chain/db_maint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,9 @@ void database::process_bids( const asset_bitasset_data_object& bad )
_cancel_bids_and_revive_mpa( to_revive, bad );
}

void update_and_match_call_orders( database& db )
/// Reset call_price of all call orders according to their remaining collateral and debt.
/// Do not update orders of prediction markets because we're sure they're up to date.
void update_call_orders_hf_343( database& db )
{
// Update call_price
wlog( "Updating all call orders for hardfork core-343 at block ${n}", ("n",db.head_block_num()) );
Expand All @@ -868,7 +870,30 @@ void update_and_match_call_orders( database& db )
abd->current_feed.maintenance_collateral_ratio );
});
}
wlog( "Done updating all call orders for hardfork core-343 at block ${n}", ("n",db.head_block_num()) );
}

/// Reset call_price of all call orders to (1,1) since it won't be used in the future.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is superfluous, since as you say it won't used anymore in the future. Getting rid of this and not tying the hf to a maintenance interval will simplify the PR significantly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concerns:

  1. For UI. If old positions' call_price are unchanged but new positions' are 1, people may get confused.
  2. I guess it's faster to compare 1 with 1 when inserting new data (when creating new positions) so it benefits in the long run with the cost of one-time process.

BTW as you've noticed later we need to tie the hf to a maintenance interval anyway.

/// Update PMs as well.
void update_call_orders_hf_1270( database& db )
{
// Update call_price
wlog( "Updating all call orders for hardfork core-1270 at block ${n}", ("n",db.head_block_num()) );
for( const auto& call_obj : db.get_index_type<call_order_index>().indices().get<by_id>() )
{
db.modify( call_obj, []( call_order_object& call ) {
call.call_price.base.amount = 1;
call.call_price.quote.amount = 1;
});
}
wlog( "Done updating all call orders for hardfork core-1270 at block ${n}", ("n",db.head_block_num()) );
}

/// Match call orders for all bitAssets, including PMs.
void match_call_orders( database& db )
{
// Match call orders
wlog( "Matching call orders at block ${n}", ("n",db.head_block_num()) );
const auto& asset_idx = db.get_index_type<asset_index>().indices().get<by_type>();
auto itr = asset_idx.lower_bound( true /** market issued */ );
while( itr != asset_idx.end() )
Expand All @@ -878,7 +903,7 @@ void update_and_match_call_orders( database& db )
// be here, next_maintenance_time should have been updated already
db.check_call_orders( a, true, false ); // allow black swan, and call orders are taker
}
wlog( "Done updating all call orders for hardfork core-343 at block ${n}", ("n",db.head_block_num()) );
wlog( "Done matching call orders at block ${n}", ("n",db.head_block_num()) );
}

void database::process_bitassets()
Expand Down Expand Up @@ -946,6 +971,22 @@ void process_hf_1465( database& db )
}
}

void update_median_feeds(database& db)
{
time_point_sec head_time = db.head_block_time();
time_point_sec next_maint_time = db.get_dynamic_global_properties().next_maintenance_time;

const auto update_bitasset = [head_time, next_maint_time]( asset_bitasset_data_object &o )
{
o.update_median_feeds( head_time, next_maint_time );
};

for( const auto& d : db.get_index_type<asset_bitasset_data_index>().indices() )
{
db.modify( d, update_bitasset );
}
}

/******
* @brief one-time data process for hard fork core-868-890
*
Expand All @@ -967,6 +1008,7 @@ void process_hf_1465( database& db )
// * NOTE: the removal can't be applied to testnet
void process_hf_868_890( database& db, bool skip_check_call_orders )
{
const auto next_maint_time = db.get_dynamic_global_properties().next_maintenance_time;
const auto head_time = db.head_block_time();
const auto head_num = db.head_block_num();
wlog( "Processing hard fork core-868-890 at block ${n}", ("n",head_num) );
Expand Down Expand Up @@ -1026,8 +1068,8 @@ void process_hf_868_890( database& db, bool skip_check_call_orders )
}

// always update the median feed due to https://github.com/bitshares/bitshares-core/issues/890
db.modify( bitasset_data, [&head_time]( asset_bitasset_data_object &obj ) {
obj.update_median_feeds( head_time );
db.modify( bitasset_data, [head_time,next_maint_time]( asset_bitasset_data_object &obj ) {
obj.update_median_feeds( head_time, next_maint_time );
});

bool median_changed = ( old_feed.settlement_price != bitasset_data.current_feed.settlement_price );
Expand Down Expand Up @@ -1238,20 +1280,25 @@ void database::perform_chain_maintenance(const signed_block& next_block, const g
if( (dgpo.next_maintenance_time < HARDFORK_613_TIME) && (next_maintenance_time >= HARDFORK_613_TIME) )
deprecate_annual_members(*this);

// To reset call_price of all call orders, then match by new rule
bool to_update_and_match_call_orders = false;
// To reset call_price of all call orders, then match by new rule, for hard fork core-343
bool to_update_and_match_call_orders_for_hf_343 = false;
if( (dgpo.next_maintenance_time <= HARDFORK_CORE_343_TIME) && (next_maintenance_time > HARDFORK_CORE_343_TIME) )
to_update_and_match_call_orders = true;
to_update_and_match_call_orders_for_hf_343 = true;

// Process inconsistent price feeds
if( (dgpo.next_maintenance_time <= HARDFORK_CORE_868_890_TIME) && (next_maintenance_time > HARDFORK_CORE_868_890_TIME) )
process_hf_868_890( *this, to_update_and_match_call_orders );
process_hf_868_890( *this, to_update_and_match_call_orders_for_hf_343 );

// Explicitly call check_call_orders of all markets
if( (dgpo.next_maintenance_time <= HARDFORK_CORE_935_TIME) && (next_maintenance_time > HARDFORK_CORE_935_TIME)
&& !to_update_and_match_call_orders )
&& !to_update_and_match_call_orders_for_hf_343 )
process_hf_935( *this );

// To reset call_price of all call orders, then match by new rule, for hard fork core-1270
bool to_update_and_match_call_orders_for_hf_1270 = false;
if( (dgpo.next_maintenance_time <= HARDFORK_CORE_1270_TIME) && (next_maintenance_time > HARDFORK_CORE_1270_TIME) )
to_update_and_match_call_orders_for_hf_1270 = true;

// make sure current_supply is less than or equal to max_supply
if ( dgpo.next_maintenance_time <= HARDFORK_CORE_1465_TIME && next_maintenance_time > HARDFORK_CORE_1465_TIME )
process_hf_1465(*this);
Expand All @@ -1261,9 +1308,20 @@ void database::perform_chain_maintenance(const signed_block& next_block, const g
d.accounts_registered_this_interval = 0;
});

// We need to do it after updated next_maintenance_time, to apply new rules here
if( to_update_and_match_call_orders )
update_and_match_call_orders(*this);
// We need to do it after updated next_maintenance_time, to apply new rules here, for hard fork core-343
if( to_update_and_match_call_orders_for_hf_343 )
{
update_call_orders_hf_343(*this);
match_call_orders(*this);
}

// We need to do it after updated next_maintenance_time, to apply new rules here, for hard fork core-1270.
if( to_update_and_match_call_orders_for_hf_1270 )
{
update_call_orders_hf_1270(*this);
update_median_feeds(*this);
match_call_orders(*this);
abitmore marked this conversation as resolved.
Show resolved Hide resolved
}

process_bitassets();

Expand Down
Loading