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

BSIP31-34 market engine improvements #829

Merged
merged 29 commits into from
Apr 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
10298ca
Test case reproduces #338 #343 #453 #606 #625 #649
abitmore Feb 6, 2018
ceb480a
Update tests to get around old feed expiration bug
abitmore Feb 7, 2018
b2a86dd
Market engine improvements.
abitmore Feb 3, 2018
adebe1e
Refactor and fix new limit order matching. #625
abitmore Feb 5, 2018
c3d0acc
Minor tweak for fill_order(call_order_object)
abitmore Feb 7, 2018
ea12aa8
Change hard fork to occur at maintenance interval
abitmore Feb 7, 2018
0b6bbca
Update black swan event detection for #338
abitmore Feb 8, 2018
8005788
Fix typo
abitmore Feb 8, 2018
3f699c1
Skip checking call orders on manually cancel #606
abitmore Feb 8, 2018
29925b1
Skip checking calls on limit order expiration #606
abitmore Feb 8, 2018
462cf57
Rename fill_order funtions to indicate order types
abitmore Feb 13, 2018
3e3c457
Remove unnecessary template match() funtion
abitmore Feb 13, 2018
dbb2dce
Update comments, remove code that is commented out
abitmore Feb 15, 2018
145b0ad
Apply_order(): also match calls with CR == MCR.
abitmore Feb 15, 2018
c049b53
Remove cull_small_test for easier merging
abitmore Apr 7, 2018
e5fc5d4
Test case for #338 #343 #453 #606 #625 #649 fixes
abitmore Feb 7, 2018
82765cb
Remove unused CORE_338.hf file
abitmore Feb 7, 2018
8ea398d
Update tests for hard fork on maintenance interval
abitmore Feb 7, 2018
9ab327a
Re-add CORE_338.hf
abitmore Feb 8, 2018
a8391d8
Add cross-hardfork tests #338 #343 #453 #606 #649
abitmore Feb 8, 2018
98a2310
Update tests for global settlement price
abitmore Feb 8, 2018
43d3795
Add tests for multiple assets crossing hardfork
abitmore Feb 9, 2018
22a2ecc
Update tests about matching calls with CR == MCR
abitmore Feb 15, 2018
62b3a7b
Recover check_call_order_cull_small_test
abitmore Apr 7, 2018
e311769
Avoid dereferencing invalidated iterator after hf
abitmore Apr 7, 2018
cd7bb4b
Minor code tweak
abitmore Apr 9, 2018
5eb43de
Add big limit order test case after hard fork #625
abitmore Apr 9, 2018
d6cdc89
Minor logging format tweak
abitmore Apr 9, 2018
8dcd9ca
Add FC_ASSERT and change some assert to FC_ASSERT
abitmore Apr 12, 2018
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
43 changes: 43 additions & 0 deletions libraries/chain/db_maint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,40 @@ 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 )
{
// Update call_price
wlog( "Updating all call orders for hardfork core-343 at block ${n}", ("n",db.head_block_num()) );
asset_id_type current_asset;
const asset_bitasset_data_object* abd = nullptr;
// by_collateral index won't change after call_price updated, so it's safe to iterate
for( const auto& call_obj : db.get_index_type<call_order_index>().indices().get<by_collateral>() )
{
if( current_asset != call_obj.debt_type() ) // debt type won't be asset_id_type(), abd will always get initialized
{
current_asset = call_obj.debt_type();
abd = &current_asset(db).bitasset_data(db);
}
if( !abd || abd->is_prediction_market ) // nothing to do with PM's; check !abd just to be safe
continue;
db.modify( call_obj, [abd]( call_order_object& call ) {
call.call_price = price::call_price( call.get_debt(), call.get_collateral(),
abd->current_feed.maintenance_collateral_ratio );
});
}
// Match call orders
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() )
{
const asset_object& a = *itr;
++itr;
// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps skip over PM's here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whether to skip over PM's here, the only logic difference might be the check_black_swan() call in check_call_orders().

Since we didn't fix #460 in this release (no BSIP for it so far), PM's will still suffer black swan events when there is a price feed change or matching logic change.

Although we didn't update call_prices of PM's, but we changed logic in check_black_swan() to check with MSSP, which is possible to trigger a black swan event. So the outcome is different if skip over PM's here.

I'd suggest we trigger black swan events here (if there would be some) to be consistent (rather than triggering on a new but same price feed).

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps keep the old logic for PM's, don't trigger black swan event when there is no matching limit order?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we agreed that #460 is a bug and doesn't need a BSIP.
We could fix #460 quickly by adding a HF-protected check in check_for_black_swan, then don't need to skip here.

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 opinion on #460 is here: #460 (comment). Aka not really a bug and not a quick fix to me.

So my recommendation in this release is to let PMs (still) have same behavior (on black swan events) as other MPAs, that said, we changed behavior of other MPAs, which will affect PMs as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

}
wlog( "Done updating all call orders for hardfork core-343 at block ${n}", ("n",db.head_block_num()) );
}

void database::perform_chain_maintenance(const signed_block& next_block, const global_property_object& global_props)
{
const auto& gpo = get_global_properties();
Expand Down Expand Up @@ -917,11 +951,20 @@ 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;
if( (dgpo.next_maintenance_time <= HARDFORK_CORE_343_TIME) && (next_maintenance_time > HARDFORK_CORE_343_TIME) )
to_update_and_match_call_orders = true;

modify(dgpo, [next_maintenance_time](dynamic_global_property_object& d) {
d.next_maintenance_time = next_maintenance_time;
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);

// Reset all BitAsset force settlement volumes to zero
for( const auto& d : get_index_type<asset_bitasset_data_index>().indices() )
{
Expand Down
274 changes: 216 additions & 58 deletions libraries/chain/db_market.cpp

Large diffs are not rendered by default.

62 changes: 39 additions & 23 deletions libraries/chain/db_update.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,20 @@ bool database::check_for_blackswan( const asset_object& mia, bool enable_black_s
const call_order_index& call_index = get_index_type<call_order_index>();
const auto& call_price_index = call_index.indices().get<by_price>();

auto call_min = price::min( bitasset.options.short_backing_asset, mia.id );
auto call_max = price::max( bitasset.options.short_backing_asset, mia.id );
auto call_itr = call_price_index.lower_bound( call_min );
auto call_end = call_price_index.upper_bound( call_max );

if( call_itr == call_end ) return false; // no call orders

price highest = settle_price;

auto maint_time = get_dynamic_global_properties().next_maintenance_time;
if( maint_time > HARDFORK_CORE_338_TIME )
// due to #338, we won't check for black swan on incoming limit order, so need to check with MSSP here
highest = bitasset.current_feed.max_short_squeeze_price();

const limit_order_index& limit_index = get_index_type<limit_order_index>();
const auto& limit_price_index = limit_index.indices().get<by_price>();

Expand All @@ -217,38 +231,35 @@ bool database::check_for_blackswan( const asset_object& mia, bool enable_black_s
// stop when limit orders are selling too little USD for too much CORE
auto lowest_possible_bid = price::min( mia.id, bitasset.options.short_backing_asset );

assert( highest_possible_bid.base.asset_id == lowest_possible_bid.base.asset_id );
FC_ASSERT( highest_possible_bid.base.asset_id == lowest_possible_bid.base.asset_id );
// NOTE limit_price_index is sorted from greatest to least
auto limit_itr = limit_price_index.lower_bound( highest_possible_bid );
auto limit_end = limit_price_index.upper_bound( lowest_possible_bid );

auto call_min = price::min( bitasset.options.short_backing_asset, mia.id );
auto call_max = price::max( bitasset.options.short_backing_asset, mia.id );
auto call_itr = call_price_index.lower_bound( call_min );
auto call_end = call_price_index.upper_bound( call_max );

if( call_itr == call_end ) return false; // no call orders

price highest = settle_price;
if( limit_itr != limit_end ) {
assert( settle_price.base.asset_id == limit_itr->sell_price.base.asset_id );
highest = std::max( limit_itr->sell_price, settle_price );
FC_ASSERT( highest.base.asset_id == limit_itr->sell_price.base.asset_id );
highest = std::max( limit_itr->sell_price, highest );
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that still correct after HF_CORE_338?
The call will always be matched at the squeeze price, even if the limit order offers a better price. A call could have sufficient collateral to fill the limit order at its price, but insufficient collateral to fill at MSQP.

Important for understanding: check_for_blackswan is only called from check_call_orders.
check_call_orders currently always uses limit order price for filling call. That shouldn't be the case, though, if I understand BSIP-32 correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Answer to previous remark:
After HF_625, check_call_orders is only ever called with the maker flag set to false. IOW after HF 338, check_call_orders is always taker, which means using the limit order price is correct, assuming that HF 338 and HF 625 happen simultaneously.

Copy link
Member Author

Choose a reason for hiding this comment

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

In short, this change is correct.

}

auto least_collateral = call_itr->collateralization();
if( ~least_collateral >= highest )
{
wdump( (*call_itr) );
elog( "Black Swan detected: \n"
" Least collateralized call: ${lc} ${~lc}\n"
// " Highest Bid: ${hb} ${~hb}\n"
" Settle Price: ${sp} ${~sp}\n"
" Max: ${h} ${~h}\n",
" Settle Price: ${~sp} ${sp}\n"
" Max: ${~h} ${h}\n",
("lc",least_collateral.to_real())("~lc",(~least_collateral).to_real())
// ("hb",limit_itr->sell_price.to_real())("~hb",(~limit_itr->sell_price).to_real())
("sp",settle_price.to_real())("~sp",(~settle_price).to_real())
("h",highest.to_real())("~h",(~highest).to_real()) );
FC_ASSERT( enable_black_swan, "Black swan was detected during a margin update which is not allowed to trigger a blackswan" );
globally_settle_asset(mia, ~least_collateral );
if( maint_time > HARDFORK_CORE_338_TIME && ~least_collateral <= settle_price )
// global settle at feed price if possible
globally_settle_asset(mia, settle_price );
Copy link
Contributor

Choose a reason for hiding this comment

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

Still struggling with the legitimacy of this, but tending to think it's ok.

else
globally_settle_asset(mia, ~least_collateral );
return true;
}
return false;
Expand All @@ -257,20 +268,25 @@ bool database::check_for_blackswan( const asset_object& mia, bool enable_black_s
void database::clear_expired_orders()
{ try {
//Cancel expired limit orders
auto head_time = head_block_time();
auto maint_time = get_dynamic_global_properties().next_maintenance_time;
auto& limit_index = get_index_type<limit_order_index>().indices().get<by_expiration>();
while( !limit_index.empty() && limit_index.begin()->expiration <= head_block_time() )
while( !limit_index.empty() && limit_index.begin()->expiration <= head_time )
{
const limit_order_object& order = *limit_index.begin();
auto base_asset = order.sell_price.base.asset_id;
auto quote_asset = order.sell_price.quote.asset_id;
cancel_limit_order( order );
// check call orders
// Comments below are copied from limit_order_cancel_evaluator::do_apply(...)
// Possible optimization: order can be called by cancelling a limit order
// if the canceled order was at the top of the book.
// Do I need to check calls in both assets?
check_call_orders( base_asset( *this ) );
check_call_orders( quote_asset( *this ) );
if( maint_time <= HARDFORK_CORE_606_TIME )
{
// check call orders
// Comments below are copied from limit_order_cancel_evaluator::do_apply(...)
// Possible optimization: order can be called by cancelling a limit order
// if the canceled order was at the top of the book.
// Do I need to check calls in both assets?
check_call_orders( base_asset( *this ) );
check_call_orders( quote_asset( *this ) );
}
}

//Process expired force settlement orders
Expand Down Expand Up @@ -330,7 +346,7 @@ void database::clear_expired_orders()
}

// Has this order not reached its settlement date?
if( order.settlement_date > head_block_time() )
if( order.settlement_date > head_time )
{
if( next_asset() )
{
Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/hardfork.d/CORE_338.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// bitshares-core issue #338 Fix "margin call order fills at price of matching limit_order"
#ifndef HARDFORK_CORE_338_TIME
#define HARDFORK_CORE_338_TIME (fc::time_point_sec( 1600000000 ))
#endif
5 changes: 5 additions & 0 deletions libraries/chain/hardfork.d/CORE_343.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// bitshares-core issue #343
// Fix "Inconsistent sorting of call orders between matching against a limit order and a force settle order"
#ifndef HARDFORK_CORE_343_TIME
#define HARDFORK_CORE_343_TIME (fc::time_point_sec( 1600000000 ))
#endif
4 changes: 4 additions & 0 deletions libraries/chain/hardfork.d/CORE_453.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// bitshares-core issue #453 Fix "Multiple limit order and call order matching issue"
#ifndef HARDFORK_CORE_453_TIME
#define HARDFORK_CORE_453_TIME (fc::time_point_sec( 1600000000 ))
#endif
4 changes: 4 additions & 0 deletions libraries/chain/hardfork.d/CORE_606.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// bitshares-core issue #606 Fix "Undercollateralized short positions should be called regardless of asks"
#ifndef HARDFORK_CORE_606_TIME
#define HARDFORK_CORE_606_TIME (fc::time_point_sec( 1600000000 ))
#endif
4 changes: 4 additions & 0 deletions libraries/chain/hardfork.d/CORE_625.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// bitshares-core issue #625 Fix "Potential erratic order matching issue involving margin call orders"
#ifndef HARDFORK_CORE_625_TIME
#define HARDFORK_CORE_625_TIME (fc::time_point_sec( 1600000000 ))
#endif
24 changes: 12 additions & 12 deletions libraries/chain/include/graphene/chain/database.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,22 +351,22 @@ namespace graphene { namespace chain {
* This function takes a new limit order, and runs the markets attempting to match it with existing orders
* already on the books.
*/
bool apply_order_before_hardfork_625(const limit_order_object& new_order_object, bool allow_black_swan = true);
bool apply_order(const limit_order_object& new_order_object, bool allow_black_swan = true);

/**
* Matches the two orders,
* Matches the two orders, the first parameter is taker, the second is maker.
*
* @return a bit field indicating which orders were filled (and thus removed)
*
* 0 - no orders were matched
* 1 - bid was filled
* 2 - ask was filled
* 1 - taker was filled
* 2 - maker was filled
* 3 - both were filled
*/
///@{
template<typename OrderType>
int match( const limit_order_object& bid, const OrderType& ask, const price& match_price );
int match( const limit_order_object& bid, const limit_order_object& ask, const price& trade_price );
int match( const limit_order_object& taker, const limit_order_object& maker, const price& trade_price );
int match( const limit_order_object& taker, const call_order_object& maker, const price& trade_price );
/// @return the amount of asset settled
asset match(const call_order_object& call,
const force_settlement_object& settle,
Expand All @@ -378,12 +378,12 @@ namespace graphene { namespace chain {
/**
* @return true if the order was completely filled and thus freed.
*/
bool fill_order( const limit_order_object& order, const asset& pays, const asset& receives, bool cull_if_small,
const price& fill_price, const bool is_maker );
bool fill_order( const call_order_object& order, const asset& pays, const asset& receives,
const price& fill_price, const bool is_maker );
bool fill_order( const force_settlement_object& settle, const asset& pays, const asset& receives,
const price& fill_price, const bool is_maker );
bool fill_limit_order( const limit_order_object& order, const asset& pays, const asset& receives, bool cull_if_small,
const price& fill_price, const bool is_maker );
bool fill_call_order( const call_order_object& order, const asset& pays, const asset& receives,
const price& fill_price, const bool is_maker );
bool fill_settle_order( const force_settlement_object& settle, const asset& pays, const asset& receives,
const price& fill_price, const bool is_maker );

bool check_call_orders( const asset_object& mia, bool enable_black_swan = true, bool for_new_limit_order = false );

Expand Down
7 changes: 5 additions & 2 deletions libraries/chain/include/graphene/chain/market_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ class limit_order_object : public abstract_object<limit_order_object>

asset amount_for_sale()const { return asset( for_sale, sell_price.base.asset_id ); }
asset amount_to_receive()const { return amount_for_sale() * sell_price; }
asset_id_type sell_asset_id()const { return sell_price.base.asset_id; }
asset_id_type receive_asset_id()const { return sell_price.quote.asset_id; }
};

struct by_id;
Expand Down Expand Up @@ -115,12 +117,13 @@ class call_order_object : public abstract_object<call_order_object>
asset get_debt()const { return asset( debt, debt_type() ); }
asset amount_to_receive()const { return get_debt(); }
asset_id_type debt_type()const { return call_price.quote.asset_id; }
asset_id_type collateral_type()const { return call_price.base.asset_id; }
price collateralization()const { return get_collateral() / get_debt(); }

account_id_type borrower;
share_type collateral; ///< call_price.base.asset_id, access via get_collateral
share_type debt; ///< call_price.quote.asset_id, access via get_collateral
price call_price; ///< Debt / Collateral
share_type debt; ///< call_price.quote.asset_id, access via get_debt
price call_price; ///< Collateral / Debt

pair<asset_id_type,asset_id_type> get_market()const
{
Expand Down
17 changes: 12 additions & 5 deletions libraries/chain/market_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,11 @@ object_id_type limit_order_create_evaluator::do_apply(const limit_order_create_o
obj.deferred_paid_fee = _deferred_paid_fee;
});
limit_order_id_type order_id = new_order_object.id; // save this because we may remove the object by filling it
bool filled = db().apply_order(new_order_object);
bool filled;
if( db().get_dynamic_global_properties().next_maintenance_time <= HARDFORK_CORE_625_TIME )
filled = db().apply_order_before_hardfork_625( new_order_object );
else
filled = db().apply_order( new_order_object );

FC_ASSERT( !op.fill_or_kill || filled );

Expand All @@ -138,10 +142,13 @@ asset limit_order_cancel_evaluator::do_apply(const limit_order_cancel_operation&

d.cancel_limit_order(*_order, false /* don't create a virtual op*/);

// Possible optimization: order can be called by canceling a limit order iff the canceled order was at the top of the book.
// Do I need to check calls in both assets?
d.check_call_orders(base_asset(d));
d.check_call_orders(quote_asset(d));
if( d.get_dynamic_global_properties().next_maintenance_time <= HARDFORK_CORE_606_TIME )
{
// Possible optimization: order can be called by canceling a limit order iff the canceled order was at the top of the book.
// Do I need to check calls in both assets?
d.check_call_orders(base_asset(d));
d.check_call_orders(quote_asset(d));
}

return refunded;
} FC_CAPTURE_AND_RETHROW( (o) ) }
Expand Down
Loading