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

Implement Limit Order Modification Feature #2743

Merged
merged 31 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
61d85de
Progress #1604: Basic implementation
nathanielhourt Feb 21, 2019
f6bbe4b
Progress #1604: Tests and Fixes
nathanielhourt Feb 22, 2019
5484c9f
Ref #1604: Add matching logic
nathanielhourt Feb 22, 2019
3d24f7c
Progress #1604: Add dust check
nathanielhourt Feb 26, 2019
073ecbe
Progress #1604: Add hardfork guards
nathanielhourt Feb 26, 2019
a34c855
Ref #1604: Another test case
nathanielhourt Feb 26, 2019
fba5bac
Progress #1604: Do not allow sooner expiration
nathanielhourt Feb 27, 2019
8a82d92
Add fee_schedule::exists<op_type>()
nathanielhourt Feb 27, 2019
6180946
Ref #1604: Add missing hardfork guard
nathanielhourt Feb 27, 2019
55dc688
Eliminate variable fee
nathanielhourt Oct 7, 2019
a71cfe4
Merge hardfork branch, fix build and tests
abitmore Apr 29, 2023
a03b4ce
Fix issues about limit_order_update_evaluator
abitmore May 8, 2023
44e6de4
Do not allow inappropriate price manipulation
abitmore May 9, 2023
2387df3
Remove redundant checks for order matching
abitmore May 9, 2023
2b3bcd3
Remove redundant checks for account balance
abitmore May 9, 2023
e8e9d0d
Simplify try-catch wrappers in tests
abitmore May 9, 2023
136007e
Update tests to adapt code changes
abitmore May 10, 2023
cf2fca6
Remove mostly useless try/catch wrappers
abitmore May 10, 2023
9160a7e
Add tests for limit_order_update_operation
abitmore May 10, 2023
c58db0b
Fix potential dereferencing issues in tests
abitmore May 10, 2023
85bef43
Add asset authorization tests for order_update op
abitmore May 10, 2023
afcff0d
Update a comment and an assertion message
abitmore May 10, 2023
dfce7c6
Update default order update fee
abitmore May 11, 2023
8898b5a
Process operation fees
abitmore May 11, 2023
269a16d
Reduce log level of exceptions in order_update_op
abitmore May 11, 2023
032f125
Update a comment
abitmore May 12, 2023
663ecd9
Move deferred fee processing code into a function
abitmore May 12, 2023
d381110
Add tests for order_update fee if paid in CORE
abitmore May 12, 2023
1195df5
Add tests for order_update fee if not paid in CORE
abitmore May 12, 2023
2a34e1b
Update variable names to fix variable shadowing
abitmore May 15, 2023
126987e
Update a comment
abitmore May 15, 2023
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
1 change: 1 addition & 0 deletions libraries/chain/db_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ void database::initialize_evaluators()
register_evaluator<asset_global_settle_evaluator>();
register_evaluator<assert_evaluator>();
register_evaluator<limit_order_create_evaluator>();
register_evaluator<limit_order_update_evaluator>();
register_evaluator<limit_order_cancel_evaluator>();
register_evaluator<call_order_update_evaluator>();
register_evaluator<bid_collateral_evaluator>();
Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/db_notify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ struct get_impacted_account_visitor
{
_impacted.insert( op.fee_payer() ); // seller
}
void operator()(const limit_order_update_operation& op)
{
_impacted.insert(op.fee_payer()); // seller
}
void operator()( const limit_order_cancel_operation& op )
{
_impacted.insert( op.fee_payer() ); // fee_paying_account
Expand Down
5 changes: 5 additions & 0 deletions libraries/chain/hardfork.d/CORE_1604.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// bitshares-core issue #1604 Operation to modify existing limit order
#ifndef HARDFORK_CORE_1604_TIME
#define HARDFORK_CORE_1604_TIME (fc::time_point_sec(1893456000)) // Jan 1 00:00:00 2030 (Not yet scheduled)
#define HARDFORK_CORE_1604_PASSED(head_block_time) (head_block_time >= HARDFORK_CORE_1604_TIME)
#endif
4 changes: 4 additions & 0 deletions libraries/chain/include/graphene/chain/hardfork_visitor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct hardfork_visitor {
using BSIP_40_ops = fc::typelist::list< protocol::custom_authority_create_operation,
protocol::custom_authority_update_operation,
protocol::custom_authority_delete_operation>;
using hf1604_ops = fc::typelist::list< protocol::limit_order_update_operation>;
using hf2103_ops = fc::typelist::list< protocol::ticket_create_operation,
protocol::ticket_update_operation>;
using liquidity_pool_ops = fc::typelist::list< protocol::liquidity_pool_create_operation,
Expand Down Expand Up @@ -82,6 +83,9 @@ struct hardfork_visitor {
std::enable_if_t<fc::typelist::contains<BSIP_40_ops, Op>(), bool>
visit() { return HARDFORK_BSIP_40_PASSED(now); }
template<typename Op>
std::enable_if_t<fc::typelist::contains<hf1604_ops, Op>(), bool>
visit() { return HARDFORK_CORE_1604_PASSED(now); }
template<typename Op>
std::enable_if_t<fc::typelist::contains<hf2103_ops, Op>(), bool>
visit() { return HARDFORK_CORE_2103_PASSED(now); }
template<typename Op>
Expand Down
14 changes: 13 additions & 1 deletion libraries/chain/include/graphene/chain/market_evaluator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ namespace graphene { namespace chain {
const asset_object* _receive_asset = nullptr;
};

class limit_order_update_evaluator : public evaluator<limit_order_update_evaluator>
{
public:
using operation_type = limit_order_update_operation;

void_result do_evaluate(const limit_order_update_operation& o);
void_result do_apply(const limit_order_update_operation& o);

const limit_order_object* _order = nullptr;
bool should_match_orders = false;
};

class limit_order_cancel_evaluator : public evaluator<limit_order_cancel_evaluator>
{
public:
Expand All @@ -68,7 +80,7 @@ namespace graphene { namespace chain {
asset do_apply( const limit_order_cancel_operation& o ) const;

private:
const limit_order_object* _order;
const limit_order_object* _order = nullptr;
};

class call_order_update_evaluator : public evaluator<call_order_update_evaluator>
Expand Down
87 changes: 87 additions & 0 deletions libraries/chain/market_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,93 @@ object_id_type limit_order_create_evaluator::do_apply(const limit_order_create_o
return order_id;
} FC_CAPTURE_AND_RETHROW( (op) ) }

void_result limit_order_update_evaluator::do_evaluate(const limit_order_update_operation& o)
{ try {
const database& d = db();
FC_ASSERT(d.head_block_time() >= HARDFORK_CORE_1604_TIME, "Operation has not activated yet");
_order = &o.order(d);

// Check this is my order
FC_ASSERT(o.seller == _order->seller, "Cannot update someone else's order");

// Check new price is compatible, and determine whether it becomes the best offer on the market
if (o.new_price) {
auto base_id = o.new_price->base.asset_id;
auto quote_id = o.new_price->quote.asset_id;
FC_ASSERT(base_id == _order->sell_price.base.asset_id && quote_id == _order->sell_price.quote.asset_id,
"Cannot update limit order with incompatible price");
const auto& order_index = d.get_index_type<limit_order_index>().indices().get<by_price>();
auto top_of_book = order_index.upper_bound(boost::make_tuple(price::max(base_id, quote_id)));
FC_ASSERT(top_of_book->sell_price.base.asset_id == base_id && top_of_book->sell_price.quote.asset_id == quote_id,
"Paradox: attempting to update an order in a market that has no orders? There's a logic error somewhere.");

// If the new price of our order is greater than the price of the order at the top of the book, we should match orders at the end.
// Otherwise, we can skip matching because there's no way this change could trigger orders to fill.
should_match_orders = (*o.new_price > top_of_book->sell_price);
}

// Check delta asset is compatible
if (o.delta_amount_to_sell) {
const auto& delta = *o.delta_amount_to_sell;
FC_ASSERT(delta.asset_id == _order->sell_price.base.asset_id,
"Cannot update limit order with incompatible asset");
if (delta.amount > 0)
FC_ASSERT(d.get_balance(o.seller, delta.asset_id) > delta,
"Insufficient balance to increase order amount");
else
FC_ASSERT(_order->for_sale > -delta.amount,
"Cannot deduct more from order than order contains");
}

// Check dust
if (o.new_price || (o.delta_amount_to_sell && o.delta_amount_to_sell->amount < 0)) {
auto new_price = o.new_price? *o.new_price : _order->sell_price;
auto new_amount = _order->amount_for_sale();
if (o.delta_amount_to_sell)
new_amount += *o.delta_amount_to_sell;
auto new_amount_to_receive = new_amount * new_price;

FC_ASSERT(new_amount_to_receive.amount > 0, "Cannot update limit order: order becomes too small; cancel order instead");
}

// Check expiration is in the future
if (o.new_expiration)
FC_ASSERT(*o.new_expiration > _order->expiration,
"Cannot update limit order to expire sooner; new expiration must be later than old one.");

return {};
} FC_CAPTURE_AND_RETHROW((o)) }

void_result limit_order_update_evaluator::do_apply(const limit_order_update_operation& o)
{ try {
database& d = db();

// Adjust account balance
const auto& seller_stats = o.seller(d).statistics(d);
if (o.delta_amount_to_sell && o.delta_amount_to_sell->asset_id == asset_id_type())
db().modify(seller_stats, [&o](account_statistics_object& bal) {
bal.total_core_in_orders += o.delta_amount_to_sell->amount;
});
if (o.delta_amount_to_sell)
d.adjust_balance(o.seller, -*o.delta_amount_to_sell);

// Update order
d.modify(*_order, [&o](limit_order_object& loo) {
if (o.new_price)
loo.sell_price = *o.new_price;
if (o.delta_amount_to_sell)
loo.for_sale += o.delta_amount_to_sell->amount;
if (o.new_expiration)
loo.expiration = *o.new_expiration;
});

// Perform order matching if necessary
if (should_match_orders)
d.apply_order(*_order);

return {};
} FC_CAPTURE_AND_RETHROW((o)) }

void_result limit_order_cancel_evaluator::do_evaluate(const limit_order_cancel_operation& o)
{ try {
const database& d = db();
Expand Down
10 changes: 10 additions & 0 deletions libraries/chain/proposal_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ struct proposal_operation_hardfork_visitor
template<typename T>
void operator()(const T &v) const {}

// TODO review and cleanup code below after hard fork
// hf_1604
void operator()(const graphene::chain::limit_order_update_operation &) const {
FC_ASSERT(block_time >= HARDFORK_CORE_1604_TIME, "Operation is not enabled yet");
}

void operator()(const graphene::chain::asset_create_operation &v) const {
detail::check_asset_options_hf_1774(block_time, v.common_options);
detail::check_asset_options_hf_bsip_48_75(block_time, v.common_options);
Expand Down Expand Up @@ -143,6 +149,10 @@ struct proposal_operation_hardfork_visitor
FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_redeem_operation>());
FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_extend_operation>());
}
if (block_time < HARDFORK_CORE_1604_TIME) {
FC_ASSERT(!op.new_parameters.current_fees->exists<limit_order_update_operation>(),
"Cannot set fees for limit_order_update_operation before its hardfork time");
}
if (!HARDFORK_BSIP_40_PASSED(block_time)) {
FC_ASSERT(!op.new_parameters.extensions.value.custom_authority_options.valid(),
"Unable to set Custom Authority Options before hardfork BSIP 40");
Expand Down
31 changes: 31 additions & 0 deletions libraries/protocol/include/graphene/protocol/market.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,32 @@ namespace graphene { namespace protocol {
price get_price()const { return amount_to_sell / min_to_receive; }
};

/**
* @ingroup operations
* Used to update an existing limit order.
*/
struct limit_order_update_operation : public base_operation
{
struct fee_params_t {
uint64_t fee = GRAPHENE_BLOCKCHAIN_PRECISION / 2;
};

asset fee;
account_id_type seller;
limit_order_id_type order;
optional<price> new_price;
optional<asset> delta_amount_to_sell;
optional<time_point_sec> new_expiration;

extensions_type extensions;

account_id_type fee_payer() const { return seller; }
void validate() const;
share_type calculate_fee(const fee_params_t& k) const {
return k.fee;
}
};

/**
* @ingroup operations
* Used to cancel an existing limit order. Both fee_pay_account and the
Expand Down Expand Up @@ -219,6 +245,7 @@ namespace graphene { namespace protocol {
} } // graphene::protocol

FC_REFLECT( graphene::protocol::limit_order_create_operation::fee_params_t, (fee) )
FC_REFLECT( graphene::protocol::limit_order_update_operation::fee_params_t, (fee) )
FC_REFLECT( graphene::protocol::limit_order_cancel_operation::fee_params_t, (fee) )
FC_REFLECT( graphene::protocol::call_order_update_operation::fee_params_t, (fee) )
FC_REFLECT( graphene::protocol::bid_collateral_operation::fee_params_t, (fee) )
Expand All @@ -229,6 +256,8 @@ FC_REFLECT( graphene::protocol::call_order_update_operation::options_type, (targ

FC_REFLECT( graphene::protocol::limit_order_create_operation,
(fee)(seller)(amount_to_sell)(min_to_receive)(expiration)(fill_or_kill)(extensions))
FC_REFLECT( graphene::protocol::limit_order_update_operation,
(fee)(seller)(order)(new_price)(delta_amount_to_sell)(new_expiration)(extensions))
FC_REFLECT( graphene::protocol::limit_order_cancel_operation,
(fee)(fee_paying_account)(order)(extensions) )
FC_REFLECT( graphene::protocol::call_order_update_operation,
Expand All @@ -242,10 +271,12 @@ FC_REFLECT( graphene::protocol::execute_bid_operation,

GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::call_order_update_operation::options_type )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_create_operation::fee_params_t )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_update_operation::fee_params_t )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_cancel_operation::fee_params_t )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::call_order_update_operation::fee_params_t )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::bid_collateral_operation::fee_params_t )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_create_operation )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_update_operation )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_cancel_operation )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::call_order_update_operation )
GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::bid_collateral_operation )
Expand Down
3 changes: 2 additions & 1 deletion libraries/protocol/include/graphene/protocol/operations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ namespace graphene { namespace protocol {
/* 73 */ credit_deal_repay_operation,
/* 74 */ credit_deal_expired_operation, // VIRTUAL
/* 75 */ liquidity_pool_update_operation,
/* 76 */ credit_deal_update_operation
/* 76 */ credit_deal_update_operation,
/* 77 */ limit_order_update_operation
>;

/// @} // operations group
Expand Down
13 changes: 13 additions & 0 deletions libraries/protocol/market.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ void limit_order_create_operation::validate()const
FC_ASSERT( min_to_receive.amount > 0 );
}

void limit_order_update_operation::validate() const
{ try {
FC_ASSERT(fee.amount >= 0, "Fee must not be negative");
FC_ASSERT(new_price || delta_amount_to_sell || new_expiration,
"Cannot update limit order if nothing is specified to update");
if (new_price)
new_price->validate();
if (delta_amount_to_sell)
FC_ASSERT(delta_amount_to_sell->amount != 0, "Cannot change limit order amount by zero");
} FC_CAPTURE_AND_RETHROW((*this)) }

void limit_order_cancel_operation::validate()const
{
FC_ASSERT( fee.amount >= 0 );
Expand All @@ -60,10 +71,12 @@ void bid_collateral_operation::validate()const

GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::call_order_update_operation::options_type )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_create_operation::fee_params_t )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_update_operation::fee_params_t )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_cancel_operation::fee_params_t )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::call_order_update_operation::fee_params_t )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::bid_collateral_operation::fee_params_t )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_create_operation )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_update_operation )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::limit_order_cancel_operation )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::call_order_update_operation )
GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::bid_collateral_operation )
Expand Down
29 changes: 27 additions & 2 deletions tests/common/database_fixture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ const limit_order_object* database_fixture_base::create_sell_order( const accoun
buy_order.amount_to_sell = amount;
buy_order.min_to_receive = recv;
buy_order.expiration = order_expiration;
trx.operations.push_back(buy_order);
trx.operations = {buy_order};
for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op, fee_core_exchange_rate);
trx.validate();
auto processed = PUSH_TX(db, trx, ~0);
Expand All @@ -1194,6 +1194,31 @@ const limit_order_object* database_fixture_base::create_sell_order( const accoun
return db.find<limit_order_object>( processed.operation_results[0].get<object_id_type>() );
}

void database_fixture_base::update_limit_order(const limit_order_object& order,
fc::optional<price> new_price,
fc::optional<asset> delta_amount,
fc::optional<time_point_sec> new_expiration) {
limit_order_update_operation update_order;
update_order.seller = order.seller;
update_order.order = order.id;
update_order.new_price = new_price;
update_order.delta_amount_to_sell = delta_amount;
update_order.new_expiration = new_expiration;
trx.operations = {update_order};
for(auto& op : trx.operations) db.current_fee_schedule().set_fee(op);
trx.validate();
auto processed = PUSH_TX(db, trx, ~0);
trx.operations.clear();
verify_asset_supplies(db);
}

void database_fixture_base::update_limit_order(limit_order_id_type order_id,
fc::optional<price> new_price,
fc::optional<asset> delta_amount,
fc::optional<time_point_sec> new_expiration) {
update_limit_order(order_id(db), new_price, delta_amount, new_expiration);
}

asset database_fixture_base::cancel_limit_order( const limit_order_object& order )
{
limit_order_cancel_operation cancel_order;
Expand All @@ -1205,7 +1230,7 @@ asset database_fixture_base::cancel_limit_order( const limit_order_object& order
trx.validate();
auto processed = PUSH_TX(db, trx, ~0);
trx.operations.clear();
verify_asset_supplies(db);
verify_asset_supplies(db);
return processed.operation_results[0].get<asset>();
}

Expand Down
8 changes: 8 additions & 0 deletions tests/common/database_fixture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,14 @@ struct database_fixture_base {
const limit_order_object* create_sell_order( const account_object& user, const asset& amount, const asset& recv,
const time_point_sec order_expiration = time_point_sec::maximum(),
const price& fee_core_exchange_rate = price::unit_price() );
void update_limit_order(const limit_order_object& order,
fc::optional<price> new_price = {},
fc::optional<asset> delta_amount = {},
fc::optional<time_point_sec> new_expiration = {});
void update_limit_order(limit_order_id_type order_id,
fc::optional<price> new_price = {},
fc::optional<asset> delta_amount = {},
fc::optional<time_point_sec> new_expiration = {});
asset cancel_limit_order( const limit_order_object& order );
void transfer( account_id_type from, account_id_type to, const asset& amount, const asset& fee = asset() );
void transfer( const account_object& from, const account_object& to, const asset& amount, const asset& fee = asset() );
Expand Down
Loading