Skip to content

Commit d6e70fb

Browse files
sitalkediasitalkedia
authored
[Orderbook] Allow order validation API to return failure reason and o… (#17811)
1 parent 590893e commit d6e70fb

File tree

6 files changed

+228
-72
lines changed

6 files changed

+228
-72
lines changed

‎aptos-move/framework/aptos-experimental/doc/market_types.md‎

Lines changed: 145 additions & 22 deletions
Large diffs are not rendered by default.

‎aptos-move/framework/aptos-experimental/doc/order_placement.md‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,7 @@ of fill limit violation in the previous transaction and the order is just a con
13901390
<b>let</b> is_taker_order =
13911391
market.get_order_book().is_taker_order(limit_price, is_bid, trigger_condition);
13921392

1393+
<b>let</b> callback_results = <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_empty">vector::empty</a>();
13931394
<b>if</b> (emit_taker_order_open && trigger_condition.is_none()) {
13941395
// We don't emit order open events for orders <b>with</b> trigger conditions <b>as</b> they are not
13951396
// actually placed in the order book until they are triggered.
@@ -1412,20 +1413,20 @@ of fill limit violation in the previous transaction and the order is just a con
14121413
);
14131414
};
14141415

1415-
<b>if</b> (
1416-
!callbacks.validate_order_placement(
1417-
new_clearinghouse_order_info(
1418-
user_addr,
1419-
order_id,
1420-
client_order_id,
1421-
is_bid,
1422-
limit_price,
1423-
time_in_force,
1424-
metadata
1425-
),
1426-
is_taker_order, // is_taker
1427-
remaining_size,
1428-
)) {
1416+
<b>let</b> validation_result = callbacks.validate_order_placement(
1417+
new_clearinghouse_order_info(
1418+
user_addr,
1419+
order_id,
1420+
client_order_id,
1421+
is_bid,
1422+
limit_price,
1423+
time_in_force,
1424+
metadata
1425+
),
1426+
is_taker_order, // is_taker
1427+
remaining_size,
1428+
);
1429+
<b>if</b> (!is_validation_result_valid(&validation_result)) {
14291430
<b>return</b> <a href="order_placement.md#0x7_order_placement_cancel_single_order_internal">cancel_single_order_internal</a>(
14301431
market,
14311432
user_addr,
@@ -1439,14 +1440,19 @@ of fill limit violation in the previous transaction and the order is just a con
14391440
is_bid,
14401441
is_taker_order, // is_taker
14411442
OrderCancellationReason::PositionUpdateViolation,
1442-
std::string::utf8(b"Position Update violation"),
1443+
validation_result.get_validation_cancellation_reason().destroy_some(),
14431444
metadata,
14441445
time_in_force,
14451446
callbacks,
1446-
<a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>[]
1447+
<a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>[],
14471448
);
14481449
};
14491450

1451+
<b>let</b> validation_actions = validation_result.get_validation_actions();
1452+
<b>if</b> (validation_actions.is_some()) {
1453+
callback_results.push_back(validation_actions.destroy_some());
1454+
};
1455+
14501456
<b>if</b> (client_order_id.is_some()) {
14511457
<b>if</b> (market.get_order_book().client_order_id_exists(user_addr, client_order_id.destroy_some())) {
14521458
// Client provided a client order id that already <b>exists</b> in the order book
@@ -1544,7 +1550,6 @@ of fill limit violation in the previous transaction and the order is just a con
15441550
};
15451551
<b>let</b> fill_sizes = <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_empty">vector::empty</a>();
15461552
<b>let</b> match_count = 0;
1547-
<b>let</b> callback_results = <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_empty">vector::empty</a>();
15481553
<b>loop</b> {
15491554
match_count += 1;
15501555
<b>let</b> (taker_cancellation_reason, callback_result) =

‎aptos-move/framework/aptos-experimental/sources/trading/market/market_types.move‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,20 @@ module aptos_experimental::market_types {
9494
}
9595
}
9696

97+
enum ValidationResult<R: store + copy + drop> has drop, copy {
98+
V1 {
99+
// If valid this is none, else contains the reason for invalidity
100+
cancellation_reason: Option<String>,
101+
validation_actions: Option<R>
102+
}
103+
}
104+
97105
enum MarketClearinghouseCallbacks<M: store + copy + drop, R: store + copy + drop> has drop {
98106
V1 {
99107
/// settle_trade_f arguments: market, taker, maker, fill_id, settled_price, settled_size,
100108
settle_trade_f: |&mut Market<M>, MarketClearinghouseOrderInfo<M>, MarketClearinghouseOrderInfo<M>, u64, u64, u64| SettleTradeResult<R> has drop + copy,
101109
/// validate_settlement_update_f arguments: order_info, is_taker, size
102-
validate_order_placement_f: | MarketClearinghouseOrderInfo<M>, bool, u64| bool has drop + copy,
110+
validate_order_placement_f: | MarketClearinghouseOrderInfo<M>, bool, u64| ValidationResult<R> has drop + copy,
103111
/// Validate the bulk order placement arguments: account, bids_prices, bids_sizes, asks_prices, asks_sizes
104112
validate_bulk_order_placement_f: |address, vector<u64>, vector<u64>, vector<u64>, vector<u64>, M| bool has drop + copy,
105113
/// place_maker_order_f arguments: order_info, size
@@ -129,9 +137,19 @@ module aptos_experimental::market_types {
129137
}
130138
}
131139

140+
public fun new_validation_result<R: store + copy + drop>(
141+
cancellation_reason: Option<String>,
142+
validation_actions: Option<R>
143+
): ValidationResult<R> {
144+
ValidationResult::V1 {
145+
cancellation_reason,
146+
validation_actions
147+
}
148+
}
149+
132150
public fun new_market_clearinghouse_callbacks<M: store + copy + drop, R: store + copy + drop>(
133151
settle_trade_f: |&mut Market<M>, MarketClearinghouseOrderInfo<M>, MarketClearinghouseOrderInfo<M>, u64, u64, u64| SettleTradeResult<R> has drop + copy,
134-
validate_order_placement_f: | MarketClearinghouseOrderInfo<M>, bool, u64| bool has drop + copy,
152+
validate_order_placement_f: | MarketClearinghouseOrderInfo<M>, bool, u64| ValidationResult<R> has drop + copy,
135153
validate_bulk_order_placement_f: |address, vector<u64>, vector<u64>, vector<u64>, vector<u64>, M| bool has drop + copy,
136154
place_maker_order_f: |MarketClearinghouseOrderInfo<M>, u64| has drop + copy,
137155
cleanup_order_f: |MarketClearinghouseOrderInfo<M>, u64| has drop + copy,
@@ -167,6 +185,18 @@ module aptos_experimental::market_types {
167185
&self.callback_result
168186
}
169187

188+
public fun is_validation_result_valid<R: store + copy + drop>(self: &ValidationResult<R>): bool {
189+
self.cancellation_reason.is_none()
190+
}
191+
192+
public fun get_validation_cancellation_reason<R: store + copy + drop>(self: &ValidationResult<R>): Option<String> {
193+
self.cancellation_reason
194+
}
195+
196+
public fun get_validation_actions<R: store + copy + drop>(self: &ValidationResult<R>): Option<R> {
197+
self.validation_actions
198+
}
199+
170200
public fun extract_results<R: store + copy + drop>(self: CallbackResult<R>): Option<R> {
171201
match (self) {
172202
CallbackResult::NOT_AVAILABLE => option::none(),
@@ -175,14 +205,6 @@ module aptos_experimental::market_types {
175205
}
176206
}
177207

178-
public fun should_continue_matching<R: store + copy + drop>(self: &CallbackResult<R>): bool {
179-
match (self) {
180-
CallbackResult::CONTINUE_MATCHING { result: _ } => true,
181-
CallbackResult::STOP_MATCHING { result: _ } => false,
182-
CallbackResult::NOT_AVAILABLE => true,
183-
}
184-
}
185-
186208
public fun should_stop_matching<R: store + copy + drop>(self: &CallbackResult<R>): bool {
187209
match (self) {
188210
CallbackResult::CONTINUE_MATCHING { result: _ } => false,
@@ -218,7 +240,7 @@ module aptos_experimental::market_types {
218240
self: &MarketClearinghouseCallbacks<M, R>,
219241
order_info: MarketClearinghouseOrderInfo<M>,
220242
is_taker: bool,
221-
size: u64): bool {
243+
size: u64): ValidationResult<R> {
222244
(self.validate_order_placement_f)(order_info, is_taker, size)
223245
}
224246

‎aptos-move/framework/aptos-experimental/sources/trading/market/order_placement.move‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ module aptos_experimental::order_placement {
7070
use aptos_experimental::market_types::{
7171
Self,
7272
MarketClearinghouseCallbacks,
73-
Market, CallbackResult, new_callback_result_not_available, emit_event_for_bulk_order_modified,
73+
Market, CallbackResult, new_callback_result_not_available,
74+
is_validation_result_valid, get_validation_cancellation_reason,
7475
};
7576

7677
// Error codes
@@ -732,6 +733,7 @@ module aptos_experimental::order_placement {
732733
let is_taker_order =
733734
market.get_order_book().is_taker_order(limit_price, is_bid, trigger_condition);
734735

736+
let callback_results = vector::empty();
735737
if (emit_taker_order_open && trigger_condition.is_none()) {
736738
// We don't emit order open events for orders with trigger conditions as they are not
737739
// actually placed in the order book until they are triggered.
@@ -754,20 +756,20 @@ module aptos_experimental::order_placement {
754756
);
755757
};
756758

757-
if (
758-
!callbacks.validate_order_placement(
759-
new_clearinghouse_order_info(
760-
user_addr,
761-
order_id,
762-
client_order_id,
763-
is_bid,
764-
limit_price,
765-
time_in_force,
766-
metadata
767-
),
768-
is_taker_order, // is_taker
769-
remaining_size,
770-
)) {
759+
let validation_result = callbacks.validate_order_placement(
760+
new_clearinghouse_order_info(
761+
user_addr,
762+
order_id,
763+
client_order_id,
764+
is_bid,
765+
limit_price,
766+
time_in_force,
767+
metadata
768+
),
769+
is_taker_order, // is_taker
770+
remaining_size,
771+
);
772+
if (!is_validation_result_valid(&validation_result)) {
771773
return cancel_single_order_internal(
772774
market,
773775
user_addr,
@@ -781,14 +783,19 @@ module aptos_experimental::order_placement {
781783
is_bid,
782784
is_taker_order, // is_taker
783785
OrderCancellationReason::PositionUpdateViolation,
784-
std::string::utf8(b"Position Update violation"),
786+
validation_result.get_validation_cancellation_reason().destroy_some(),
785787
metadata,
786788
time_in_force,
787789
callbacks,
788-
vector[]
790+
vector[],
789791
);
790792
};
791793

794+
let validation_actions = validation_result.get_validation_actions();
795+
if (validation_actions.is_some()) {
796+
callback_results.push_back(validation_actions.destroy_some());
797+
};
798+
792799
if (client_order_id.is_some()) {
793800
if (market.get_order_book().client_order_id_exists(user_addr, client_order_id.destroy_some())) {
794801
// Client provided a client order id that already exists in the order book
@@ -886,7 +893,6 @@ module aptos_experimental::order_placement {
886893
};
887894
let fill_sizes = vector::empty();
888895
let match_count = 0;
889-
let callback_results = vector::empty();
890896
loop {
891897
match_count += 1;
892898
let (taker_cancellation_reason, callback_result) =

‎aptos-move/framework/aptos-experimental/sources/trading/tests/market/clearinghouse_test.move‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ module aptos_experimental::clearinghouse_test {
1313
MarketClearinghouseCallbacks,
1414
new_market_clearinghouse_callbacks,
1515
new_callback_result_continue_matching,
16-
new_callback_result_stop_matching
16+
new_callback_result_stop_matching, ValidationResult, new_validation_result
1717
};
1818

1919
const EINVALID_ADDRESS: u64 = 1;
@@ -65,14 +65,14 @@ module aptos_experimental::clearinghouse_test {
6565
);
6666
}
6767

68-
public(package) fun validate_order_placement(order_id: OrderIdType): bool acquires GlobalState {
68+
public(package) fun validate_order_placement(order_id: OrderIdType): ValidationResult<u64> acquires GlobalState {
6969
let open_orders = &mut borrow_global_mut<GlobalState>(@0x1).open_orders;
7070
assert!(
7171
!open_orders.contains(order_id),
7272
error::invalid_argument(E_DUPLICATE_ORDER)
7373
);
7474
open_orders.add(order_id, true);
75-
return true
75+
return new_validation_result(option::none(), option::none())
7676
}
7777

7878
public(package) fun validate_bulk_order_placement(account: address): bool acquires GlobalState {

‎aptos-move/framework/aptos-experimental/sources/trading/tests/market/market_bulk_order_tests.move‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ module aptos_experimental::market_bulk_order_tests {
2525
test_gtc_taker_partially_filled_helper,
2626
test_post_only_success_helper,
2727
test_post_only_failure_helper,
28-
test_taker_partial_cancelled_maker_reinserted_helper, test_self_matching_not_allowed_helper,
28+
test_taker_partial_cancelled_maker_reinserted_helper,
2929
test_self_matching_allowed_helper,
3030
};
3131

0 commit comments

Comments
 (0)