[WIP][early draft] orders index #18360
Conversation
igor-aptos
requested review from
georgemitenkov,
vgao1996,
wrwg and
zekun000
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| let is_bid = safely_pop_arg!(args, bool); | ||
| let price = safely_pop_arg!(args, u64); | ||
|
|
||
| println!("price: {}, is_bid: {}, trigger: {:?}", price, is_bid, trigger_condition); |
There was a problem hiding this comment.
Debug println statement left in production code
A println!() debug statement is present in native_is_taker_order that logs price, is_bid, and trigger condition. This debug output will pollute logs in production and may leak internal order information.
| panic!( | ||
| "Struct {}::{} doesn't yet exist in the cache", | ||
| module_id, struct_name | ||
| ); |
There was a problem hiding this comment.
Panic statements will crash instead of returning errors
Multiple panic!() calls have been added before return Err(...) statements. Since panic occurs before the return, the code will always crash instead of gracefully returning an error. This affects lookup failure handling in find_struct and import_call_by_name, which will crash the process instead of propagating errors.
Additional Locations (2)
| if Some(ModuleHandleIndex(idx as u16)) != self_module | ||
| && !context.dependency_map.contains_key(&module_id) | ||
| { | ||
| panic!("Couldn't find {} in dependency map", module_id); |
There was a problem hiding this comment.
Panic statements in bytecode verifier cause crashes
Multiple panic!() calls added before error returns in verify_imported_modules, verify_imported_structs, and verify_imported_functions. These panics execute before the return statements, causing the verifier to crash when encountering missing dependencies instead of returning proper verification errors. This can crash nodes during module verification.
Additional Locations (2)
| } | ||
| } | ||
|
|
||
| static ORDERS_INDEXES_NATIVE_STATE: Lazy<Mutex<OrdersIndexesNativeState>> = Lazy::new(|| Mutex::new(OrdersIndexesNativeState { orders_indexes: BTreeMap::new() })); |
There was a problem hiding this comment.
Global static state causes incorrect blockchain state isolation
ORDERS_INDEXES_NATIVE_STATE uses a global static Lazy<Mutex<...>> to store order book state. In a blockchain context, this breaks state isolation between transactions, persists state incorrectly across block boundaries, and can cause data inconsistency. Native functions should interact with the Move VM's storage layer rather than using process-global state.
| // do something | ||
| } else { | ||
| self.place_ready_maker_order_with_unique_idx(order_req, ascending_idx)?; | ||
| } |
There was a problem hiding this comment.
Orders with trigger conditions silently discarded
When order_req.trigger_condition.is_some() is true, the place_maker_or_pending_order function has an empty block with just a // do something comment, then returns Ok(()). This means orders with trigger conditions (like stop-loss or time-based orders) are silently discarded while appearing to succeed. The order is never stored and is lost, but the caller receives no error indication.
Description
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist
Note
Introduce
aptos-trading-nativesand experimental0x7::orders_index(native order placement/matching), wire into VM, and add workloads/tests and docs.0x7::orders_indexmodule (enumOrdersIndex, native fns:place_maker_order,is_taker_order,get_single_match_for_taker).orders_indexand include in experimental overview.aptos-trading-nativeswithorders_indexandorder_book_typesnatives (order parsing/matching, packing helpers).aptos-vm-environment(underEXPERIMENTAL_CODE_ADDRESS) and add deps inaptos-vm,aptos-vm-environment,aptos-vm-profiling.safely_get_struct_variant_field_as!macro.order_index_example.moveand wire new transaction types (OrderIndex*) intotransaction-workloads-libandsingle_node_performance.py.EntryPoints::OrderBooktoorder_index_examplewhenuse_order_index.Written by Cursor Bugbot for commit 66d385c. This will update automatically on new commits. Configure here.