Skip to content

Commit 328f2aa

Browse files
nathanielhourtnathanielhourt
committed
Progress bitshares#1604: Tests and Fixes
Some major facepalms in here... haha! but overall, it's looking good.
1 parent 123556f commit 328f2aa

File tree

10 files changed

+183
-41
lines changed

10 files changed

+183
-41
lines changed

‎libraries/chain/db_init.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ void database::initialize_evaluators()
148148
register_evaluator<asset_global_settle_evaluator>();
149149
register_evaluator<assert_evaluator>();
150150
register_evaluator<limit_order_create_evaluator>();
151+
register_evaluator<limit_order_update_evaluator>();
151152
register_evaluator<limit_order_cancel_evaluator>();
152153
register_evaluator<call_order_update_evaluator>();
153154
register_evaluator<bid_collateral_evaluator>();

‎libraries/chain/db_notify.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ struct get_impacted_account_visitor
4343
{
4444
_impacted.insert( op.fee_payer() ); // seller
4545
}
46+
void operator()(const limit_order_update_operation& op)
47+
{
48+
_impacted.insert(op.fee_payer()); // seller
49+
}
4650
void operator()( const limit_order_cancel_operation& op )
4751
{
4852
_impacted.insert( op.fee_payer() ); // fee_paying_account

‎libraries/chain/include/graphene/chain/market_evaluator.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ namespace graphene { namespace chain {
6363
const asset_object* _receive_asset = nullptr;
6464
};
6565

66-
class limit_order_update_evaluator : public evaluator<limit_order_update_operation>
66+
class limit_order_update_evaluator : public evaluator<limit_order_update_evaluator>
6767
{
6868
public:
6969
using operation_type = limit_order_update_operation;

‎libraries/chain/include/graphene/chain/protocol/market.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ namespace graphene { namespace chain {
9696

9797
extensions_type extensions;
9898

99-
account_id_type fee_payer() { return seller; }
99+
account_id_type fee_payer() const { return seller; }
100100
void validate() const;
101101
share_type calculate_fee(const fee_parameters_type& k) const {
102102
return delta_amount_to_sell? k.amount_fee : k.price_fee;

‎libraries/chain/include/graphene/chain/protocol/operations.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ namespace graphene { namespace chain {
9595
bid_collateral_operation,
9696
execute_bid_operation, // VIRTUAL
9797
asset_claim_pool_operation,
98-
asset_update_issuer_operation
98+
asset_update_issuer_operation,
99+
limit_order_update_operation
99100
> operation;
100101

101102
/// @} // operations group

‎libraries/chain/market_evaluator.cpp

+41-30
Original file line numberDiff line numberDiff line change
@@ -128,44 +128,51 @@ object_id_type limit_order_create_evaluator::do_apply(const limit_order_create_o
128128

129129
void_result limit_order_update_evaluator::do_evaluate(const limit_order_update_operation& o)
130130
{ try {
131-
const database& d = db();
132-
_order = &o.order(d);
133-
134-
// Check this is my order
135-
FC_ASSERT(o.seller == _order->seller, "Cannot update someone else's order");
136-
137-
// Check new price is compatible
138-
if (o.new_price)
139-
FC_ASSERT(o.new_price->base.asset_id == _order->sell_price.base.asset_id &&
140-
o.new_price->quote.asset_id == _order->sell_price.quote.asset_id,
141-
"Cannot update limit order with incompatible price");
142-
143-
// Check delta asset is compatible
144-
if (o.delta_amount_to_sell) {
145-
const auto& delta = *o.delta_amount_to_sell;
146-
FC_ASSERT(delta.asset_id == _order->sell_price.base.asset_id,
147-
"Cannot update limit order with incompatible asset");
148-
if (delta.amount > 0)
149-
FC_ASSERT(d.get_balance(o.seller, delta.asset_id) > delta,
150-
"Insufficient balance to increase order amount");
151-
else
152-
FC_ASSERT(_order->for_sale > -delta.amount,
153-
"Cannot deduct more from order than order contains");
154-
}
155-
156-
// Check expiration is in the future
157-
if (o.new_expiration)
158-
FC_ASSERT(*o.new_expiration >= d.head_block_time(),
159-
"Cannot update limit order with past expiration");
131+
const database& d = db();
132+
_order = &o.order(d);
133+
134+
// Check this is my order
135+
FC_ASSERT(o.seller == _order->seller, "Cannot update someone else's order");
136+
137+
// Check new price is compatible
138+
if (o.new_price)
139+
FC_ASSERT(o.new_price->base.asset_id == _order->sell_price.base.asset_id &&
140+
o.new_price->quote.asset_id == _order->sell_price.quote.asset_id,
141+
"Cannot update limit order with incompatible price");
142+
143+
// Check delta asset is compatible
144+
if (o.delta_amount_to_sell) {
145+
const auto& delta = *o.delta_amount_to_sell;
146+
FC_ASSERT(delta.asset_id == _order->sell_price.base.asset_id,
147+
"Cannot update limit order with incompatible asset");
148+
if (delta.amount > 0)
149+
FC_ASSERT(d.get_balance(o.seller, delta.asset_id) > delta,
150+
"Insufficient balance to increase order amount");
151+
else
152+
FC_ASSERT(_order->for_sale > -delta.amount,
153+
"Cannot deduct more from order than order contains");
154+
}
155+
156+
// Check expiration is in the future
157+
if (o.new_expiration)
158+
FC_ASSERT(*o.new_expiration >= d.head_block_time(),
159+
"Cannot update limit order with past expiration");
160+
161+
return void_result();
160162
} FC_CAPTURE_AND_RETHROW((o)) }
161163

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

166168
// Adjust account balance
169+
const auto& seller_stats = o.seller(d).statistics(d);
170+
if (o.delta_amount_to_sell && o.delta_amount_to_sell->asset_id == asset_id_type())
171+
db().modify(seller_stats, [&o](account_statistics_object& bal) {
172+
bal.total_core_in_orders += o.delta_amount_to_sell->amount;
173+
});
167174
if (o.delta_amount_to_sell)
168-
d.adjust_balance(o.seller, *o.delta_amount_to_sell);
175+
d.adjust_balance(o.seller, -*o.delta_amount_to_sell);
169176

170177
// Update order
171178
d.modify(*_order, [&o](limit_order_object& loo) {
@@ -176,6 +183,10 @@ void_result limit_order_update_evaluator::do_apply(const limit_order_update_oper
176183
if (o.new_expiration)
177184
loo.expiration = *o.new_expiration;
178185
});
186+
187+
// TODO: check if order is at front of book and price moved in favor of buyer; if so, trigger matching
188+
189+
return void_result();
179190
} FC_CAPTURE_AND_RETHROW((o)) }
180191

181192
void_result limit_order_cancel_evaluator::do_evaluate(const limit_order_cancel_operation& o)

‎libraries/chain/protocol/market.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ void limit_order_create_operation::validate()const
3434
}
3535

3636
void limit_order_update_operation::validate() const
37-
{
37+
{ try {
3838
FC_ASSERT(fee.amount >= 0, "Fee must not be negative");
3939
FC_ASSERT(new_price || delta_amount_to_sell || new_expiration,
40-
"Cannot update limit order if nothing is specified to update");
40+
"Cannot update limit order if nothing is specified to update");
4141
if (new_price)
42-
new_price->validate();
42+
new_price->validate();
4343
if (delta_amount_to_sell)
44-
FC_ASSERT(delta_amount_to_sell->amount != 0, "Cannot change limit order amount by zero");
45-
}
44+
FC_ASSERT(delta_amount_to_sell->amount != 0, "Cannot change limit order amount by zero");
45+
} FC_CAPTURE_AND_RETHROW((*this)) }
4646

4747
void limit_order_cancel_operation::validate()const
4848
{

‎tests/common/database_fixture.cpp

+28-3
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ const limit_order_object* database_fixture::create_sell_order( const account_obj
752752
buy_order.amount_to_sell = amount;
753753
buy_order.min_to_receive = recv;
754754
buy_order.expiration = order_expiration;
755-
trx.operations.push_back(buy_order);
755+
trx.operations = {buy_order};
756756
for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op, fee_core_exchange_rate);
757757
trx.validate();
758758
auto processed = PUSH_TX(db, trx, ~0);
@@ -761,17 +761,42 @@ const limit_order_object* database_fixture::create_sell_order( const account_obj
761761
return db.find<limit_order_object>( processed.operation_results[0].get<object_id_type>() );
762762
}
763763

764+
void database_fixture::update_limit_order(const limit_order_object& order,
765+
fc::optional<price> new_price,
766+
fc::optional<asset> delta_amount,
767+
fc::optional<time_point_sec> new_expiration) {
768+
limit_order_update_operation update_order;
769+
update_order.seller = order.seller;
770+
update_order.order = order.id;
771+
update_order.new_price = new_price;
772+
update_order.delta_amount_to_sell = delta_amount;
773+
update_order.new_expiration = new_expiration;
774+
trx.operations = {update_order};
775+
for(auto& op : trx.operations) db.current_fee_schedule().set_fee(op);
776+
trx.validate();
777+
auto processed = PUSH_TX(db, trx, ~0);
778+
trx.operations.clear();
779+
verify_asset_supplies(db);
780+
}
781+
782+
void database_fixture::update_limit_order(limit_order_id_type order_id,
783+
fc::optional<price> new_price,
784+
fc::optional<asset> delta_amount,
785+
fc::optional<time_point_sec> new_expiration) {
786+
update_limit_order(order_id(db), new_price, delta_amount, new_expiration);
787+
}
788+
764789
asset database_fixture::cancel_limit_order( const limit_order_object& order )
765790
{
766791
limit_order_cancel_operation cancel_order;
767792
cancel_order.fee_paying_account = order.seller;
768793
cancel_order.order = order.id;
769-
trx.operations.push_back(cancel_order);
794+
trx.operations = {cancel_order};
770795
for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op);
771796
trx.validate();
772797
auto processed = PUSH_TX(db, trx, ~0);
773798
trx.operations.clear();
774-
verify_asset_supplies(db);
799+
verify_asset_supplies(db);
775800
return processed.operation_results[0].get<asset>();
776801
}
777802

‎tests/common/database_fixture.hpp

+8
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,14 @@ struct database_fixture {
332332
const limit_order_object* create_sell_order( const account_object& user, const asset& amount, const asset& recv,
333333
const time_point_sec order_expiration = time_point_sec::maximum(),
334334
const price& fee_core_exchange_rate = price::unit_price() );
335+
void update_limit_order(const limit_order_object& order,
336+
fc::optional<price> new_price = {},
337+
fc::optional<asset> delta_amount = {},
338+
fc::optional<time_point_sec> new_expiration = {});
339+
void update_limit_order(limit_order_id_type order_id,
340+
fc::optional<price> new_price = {},
341+
fc::optional<asset> delta_amount = {},
342+
fc::optional<time_point_sec> new_expiration = {});
335343
asset cancel_limit_order( const limit_order_object& order );
336344
void transfer( account_id_type from, account_id_type to, const asset& amount, const asset& fee = asset() );
337345
void transfer( const account_object& from, const account_object& to, const asset& amount, const asset& fee = asset() );

‎tests/tests/operation_tests.cpp

+92
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,98 @@ BOOST_AUTO_TEST_CASE( feed_limit_logic_test )
8080
}
8181
}
8282

83+
BOOST_AUTO_TEST_CASE(limit_order_update_test)
84+
{
85+
try {
86+
ACTORS((nathan));
87+
const auto& bitusd = create_bitasset("USDBIT", nathan.id);
88+
const auto& munee = create_user_issued_asset("MUNEE");
89+
const auto& core = asset_id_type()(db);
90+
91+
update_feed_producers(bitusd, {nathan_id});
92+
price_feed current_feed;
93+
current_feed.settlement_price = bitusd.amount(200) / core.amount(100);
94+
current_feed.maintenance_collateral_ratio = 1750;
95+
publish_feed(bitusd, nathan, current_feed);
96+
97+
transfer(committee_account, nathan_id, asset(1500));
98+
issue_uia(nathan, munee.amount(100));
99+
borrow(nathan_id, bitusd.amount(100), asset(500));
100+
101+
auto expiration = db.head_block_time() + 1000;
102+
auto sell_price = price(asset(500), bitusd.amount(1000));
103+
limit_order_id_type order_id = create_sell_order(nathan, asset(500), bitusd.amount(1000), expiration)->id;
104+
BOOST_REQUIRE_EQUAL(order_id(db).for_sale.value, 500);
105+
BOOST_REQUIRE_EQUAL(fc::json::to_string(order_id(db).sell_price), fc::json::to_string(sell_price));
106+
BOOST_REQUIRE_EQUAL(order_id(db).expiration.sec_since_epoch(), expiration.sec_since_epoch());
107+
108+
// Cannot update order without changing anything
109+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id), fc::assert_exception);
110+
// Cannot update order to use inverted price assets
111+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, price(bitusd.amount(2), asset(1))), fc::assert_exception);
112+
// Cannot update order to use different assets
113+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, price(bitusd.amount(2), munee.amount(1))), fc::assert_exception);
114+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, price(munee.amount(2), bitusd.amount(1))), fc::assert_exception);
115+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, price(asset(2), munee.amount(1))), fc::assert_exception);
116+
// Cannot update order to expire in the past
117+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, {}, db.head_block_time() - 10), fc::assert_exception);
118+
// Cannot update order to add more funds than seller has
119+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, asset(501)), fc::assert_exception);
120+
// Cannot update order to remove more funds than order has
121+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, asset(-501)), fc::assert_exception);
122+
// Cannot update order to remove all funds in order
123+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, asset(-500)), fc::assert_exception);
124+
// Cannot update order to add or remove different kind of funds
125+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, bitusd.amount(50)), fc::assert_exception);
126+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, bitusd.amount(-50)), fc::assert_exception);
127+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, munee.amount(50)), fc::assert_exception);
128+
GRAPHENE_REQUIRE_THROW(update_limit_order(order_id, {}, munee.amount(-50)), fc::assert_exception);
129+
130+
// Try changing price
131+
sell_price.base = asset(501);
132+
update_limit_order(order_id, sell_price);
133+
BOOST_REQUIRE_EQUAL(fc::json::to_string(order_id(db).sell_price), fc::json::to_string(sell_price));
134+
sell_price.base = asset(500);
135+
sell_price.quote = bitusd.amount(999);
136+
update_limit_order(order_id, sell_price);
137+
BOOST_REQUIRE_EQUAL(fc::json::to_string(order_id(db).sell_price), fc::json::to_string(sell_price));
138+
sell_price.quote = bitusd.amount(1000);
139+
update_limit_order(order_id, sell_price);
140+
BOOST_REQUIRE_EQUAL(fc::json::to_string(order_id(db).sell_price), fc::json::to_string(sell_price));
141+
142+
// Try changing expiration
143+
expiration += 50;
144+
update_limit_order(order_id, {}, {}, expiration);
145+
BOOST_REQUIRE_EQUAL(order_id(db).expiration.sec_since_epoch(), expiration.sec_since_epoch());
146+
expiration -= 100;
147+
update_limit_order(order_id, {}, {}, expiration);
148+
BOOST_REQUIRE_EQUAL(order_id(db).expiration.sec_since_epoch(), expiration.sec_since_epoch());
149+
150+
// Try adding funds
151+
update_limit_order(order_id, {}, asset(50));
152+
BOOST_REQUIRE_EQUAL(order_id(db).amount_for_sale().amount.value, 550);
153+
BOOST_REQUIRE_EQUAL(db.get_balance(nathan_id, core.get_id()).amount.value, 450);
154+
155+
// Try removing funds
156+
update_limit_order(order_id, {}, asset(-100));
157+
BOOST_REQUIRE_EQUAL(order_id(db).amount_for_sale().amount.value, 450);
158+
BOOST_REQUIRE_EQUAL(db.get_balance(nathan_id, core.get_id()).amount.value, 550);
159+
160+
// Try changing everything at once
161+
expiration += 50;
162+
sell_price.base = asset(499);
163+
sell_price.quote = bitusd.amount(1001);
164+
update_limit_order(order_id, sell_price, 50, expiration);
165+
BOOST_REQUIRE_EQUAL(fc::json::to_string(order_id(db).sell_price), fc::json::to_string(sell_price));
166+
BOOST_REQUIRE_EQUAL(order_id(db).expiration.sec_since_epoch(), expiration.sec_since_epoch());
167+
BOOST_REQUIRE_EQUAL(order_id(db).amount_for_sale().amount.value, 500);
168+
BOOST_REQUIRE_EQUAL(db.get_balance(nathan_id, core.get_id()).amount.value, 500);
169+
} catch (fc::exception& e) {
170+
edump((e.to_detail_string()));
171+
throw;
172+
}
173+
}
174+
83175
BOOST_AUTO_TEST_CASE( call_order_update_test )
84176
{
85177
try {

0 commit comments

Comments
 (0)