Skip to content

Commit f6bbe4b

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

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
@@ -147,6 +147,7 @@ void database::initialize_evaluators()
147147
register_evaluator<asset_global_settle_evaluator>();
148148
register_evaluator<assert_evaluator>();
149149
register_evaluator<limit_order_create_evaluator>();
150+
register_evaluator<limit_order_update_evaluator>();
150151
register_evaluator<limit_order_cancel_evaluator>();
151152
register_evaluator<call_order_update_evaluator>();
152153
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
@@ -126,44 +126,51 @@ object_id_type limit_order_create_evaluator::do_apply(const limit_order_create_o
126126

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

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

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

168175
// Update order
169176
d.modify(*_order, [&o](limit_order_object& loo) {
@@ -174,6 +181,10 @@ void_result limit_order_update_evaluator::do_apply(const limit_order_update_oper
174181
if (o.new_expiration)
175182
loo.expiration = *o.new_expiration;
176183
});
184+
185+
// TODO: check if order is at front of book and price moved in favor of buyer; if so, trigger matching
186+
187+
return void_result();
177188
} FC_CAPTURE_AND_RETHROW((o)) }
178189

179190
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
@@ -751,7 +751,7 @@ const limit_order_object* database_fixture::create_sell_order( const account_obj
751751
buy_order.amount_to_sell = amount;
752752
buy_order.min_to_receive = recv;
753753
buy_order.expiration = order_expiration;
754-
trx.operations.push_back(buy_order);
754+
trx.operations = {buy_order};
755755
for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op, fee_core_exchange_rate);
756756
trx.validate();
757757
auto processed = PUSH_TX(db, trx, ~0);
@@ -760,17 +760,42 @@ const limit_order_object* database_fixture::create_sell_order( const account_obj
760760
return db.find<limit_order_object>( processed.operation_results[0].get<object_id_type>() );
761761
}
762762

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

‎tests/common/database_fixture.hpp

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