diff --git a/src/api/client/orders.rs b/src/api/client/orders.rs index 79fc300b..991066e5 100644 --- a/src/api/client/orders.rs +++ b/src/api/client/orders.rs @@ -38,6 +38,7 @@ impl EClient { order_id: oid, price, qty, + outside_rth: order.outside_rth, }) } else { ClientCore::build_order_request(order, oid, instrument)? diff --git a/src/api/client/tests.rs b/src/api/client/tests.rs index 98f2fe81..e21e5d7a 100644 --- a/src/api/client/tests.rs +++ b/src/api/client/tests.rs @@ -392,6 +392,42 @@ fn place_order_adjustable_trail_carries_trailing_amount_and_unit() { } } +#[test] +fn modify_carries_outside_rth_from_the_resubmitted_order() { + // ibx#247: the replace asserted 6433=1 unconditionally, so an order placed + // with outside_rth=false came back outside-RTH after any modify. The flag + // has to travel with the modify, since the tracked record has no field for + // it. + let (client, rx, shared) = test_client(); + shared.market.set_instrument_count(1); + let order = Order { + action: "BUY".into(), total_quantity: 1.0, order_type: "LMT".into(), + lmt_price: 100.0, outside_rth: false, ..Default::default() + }; + client.place_order(70, &spy(), &order).unwrap(); + let _submit = rx.try_recv().unwrap(); + + // Same id -> modify. Caller still says outside_rth=false. + let reprice = Order { lmt_price: 101.0, ..order.clone() }; + client.place_order(70, &spy(), &reprice).unwrap(); + match rx.try_recv().unwrap() { + ControlCommand::Order(OrderRequest::Modify { outside_rth, .. }) => { + assert!(!outside_rth, "a modify must not opt the order into the extended session"); + } + cmd => panic!("expected Modify, got {:?}", cmd), + } + + // And it survives when the caller does want it. + let rth_out = Order { lmt_price: 102.0, outside_rth: true, ..order.clone() }; + client.place_order(70, &spy(), &rth_out).unwrap(); + match rx.try_recv().unwrap() { + ControlCommand::Order(OrderRequest::Modify { outside_rth, .. }) => { + assert!(outside_rth, "an explicit outside_rth=true must reach the replace"); + } + cmd => panic!("expected Modify, got {:?}", cmd), + } +} + #[test] fn place_order_adjustable_trail_percent_unit_passes_through() { // Percent unit (100) must survive; the trailing amount is a percent value. diff --git a/src/bin/bench_order_modify.rs b/src/bin/bench_order_modify.rs index 0c8b1e0a..ab71134a 100644 --- a/src/bin/bench_order_modify.rs +++ b/src/bin/bench_order_modify.rs @@ -111,6 +111,8 @@ fn main() { order_id: current_order_id, price: new_price, qty: 1, + // Matches the outside_rth=true the order was placed with above. + outside_rth: true, }); // Wait for ack on new_order_id diff --git a/src/engine/context.rs b/src/engine/context.rs index 0c39e6a1..c1637b58 100644 --- a/src/engine/context.rs +++ b/src/engine/context.rs @@ -853,7 +853,9 @@ impl Context { .push(OrderRequest::CancelAll { instrument }); } - pub fn modify(&mut self, order_id: OrderId, price: Price, qty: u32) -> OrderId { + /// `outside_rth` is asserted on the replace: the tracked record has no + /// field for it, so it has to come from the caller (ibx#247). + pub fn modify(&mut self, order_id: OrderId, price: Price, qty: u32, outside_rth: bool) -> OrderId { let new_id = self.next_order_id; self.next_order_id += 1; self.pending_orders.push(OrderRequest::Modify { @@ -861,6 +863,7 @@ impl Context { order_id, price, qty, + outside_rth, }); new_id } @@ -1067,7 +1070,7 @@ mod tests { #[test] fn modify_drains_correctly() { let mut ctx = Context::new(); - ctx.modify(7, 200 * PRICE_SCALE, 50); + ctx.modify(7, 200 * PRICE_SCALE, 50, false); let orders: Vec<_> = ctx.drain_pending_orders().collect(); match orders[0] { diff --git a/src/engine/hot_loop/order_builder.rs b/src/engine/hot_loop/order_builder.rs index bb7cb4b2..c1bbedaa 100644 --- a/src/engine/hot_loop/order_builder.rs +++ b/src/engine/hot_loop/order_builder.rs @@ -1442,7 +1442,7 @@ pub(crate) fn drain_and_send_orders( } last_result } - OrderRequest::Modify { new_order_id, order_id, price, qty } => { + OrderRequest::Modify { new_order_id, order_id, price, qty, outside_rth } => { let orig = context.order(order_id).copied(); // Modify carries no instrument; resolve it from the tracked // order to snap the new price to the tick grid (ibx#216). @@ -1491,7 +1491,15 @@ pub(crate) fn drain_and_send_orders( (44, &price_str), // Price (1, account_id), // Account (6122, "c"), // Client version - (6433, "1"), // OutsideRTH (preserve from original) + ]; + // OutsideRTH, from the order the caller resubmitted rather than + // hard-coded: the tracked record cannot express it, and asserting + // 1 unconditionally opted every modified order into the extended + // session (ibx#247). Same position it held in the capture. + if outside_rth { + fields.push((6433, "1")); + } + let rest: [(u32, &str); 11] = [ (38, &qty_str), // OrderQty (54, side_str), // Side (40, &ord_type_str), // OrdType @@ -1504,6 +1512,7 @@ pub(crate) fn drain_and_send_orders( (6211, ""), // Empty (matches reference) (6238, ""), // Empty (matches reference) ]; + fields.extend(rest); // Include stop price for order types that need it let stop_str; if let Some(o) = orig { @@ -2057,3 +2066,49 @@ mod tests { assert!(shared.orders.drain_order_updates().is_empty()); } } + +#[cfg(test)] +mod modify_wire_tests { + use super::*; + use crate::protocol::connection::Connection; + use std::io::Read; + + /// Drive the Modify arm and read what actually reaches the socket. + fn replace_bytes(outside_rth: bool) -> String { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let client = std::net::TcpStream::connect(addr).unwrap(); + let (mut peer, _) = listener.accept().unwrap(); + peer.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap(); + let mut conn = Some(Connection::new_raw(client).unwrap()); + + let mut context = Context::new(); + context.insert_order(crate::types::Order::new( + 7, 0, Side::Buy, 1, 100 * crate::types::PRICE_SCALE, b'2', b'0', 0, + )); + context.modify(7, 200 * crate::types::PRICE_SCALE, 50, outside_rth); + + let shared = std::sync::Arc::new(SharedState::new()); + let mut hb = HeartbeatState::new(); + drain_and_send_orders(&mut conn, &mut context, "DU111111", &mut hb, false, &shared); + + let mut buf = [0u8; 4096]; + let n = peer.read(&mut buf).unwrap(); + String::from_utf8_lossy(&buf[..n]).replace('\u{1}', "|") + } + + /// ibx#247: the replace asserted 6433=1 unconditionally, so an RTH-only + /// order was opted into the extended session by its first modify. Pins the + /// 6122/6433/38 neighbourhood in both polarities — presence and captured + /// position when the caller sets the flag, absence when it does not. + #[test] + fn modify_emits_outside_rth_only_when_the_caller_set_it() { + let on = replace_bytes(true); + assert!(on.contains("|6122=c|6433=1|38=50|"), + "6433 must keep its captured position between 6122 and 38: {}", on); + + let off = replace_bytes(false); + assert!(!off.contains("|6433="), "an RTH-only order must not assert 6433: {}", off); + assert!(off.contains("|6122=c|38=50|"), "the rest of the message is unchanged: {}", off); + } +} diff --git a/src/python/compat/client/orders.rs b/src/python/compat/client/orders.rs index 80a445b0..df35a83d 100644 --- a/src/python/compat/client/orders.rs +++ b/src/python/compat/client/orders.rs @@ -44,6 +44,7 @@ impl EClient { order_id: oid, price, qty, + outside_rth: api_order.outside_rth, }) } else { ClientCore::build_order_request(&api_order, oid, instrument) diff --git a/src/types.rs b/src/types.rs index fe09f6b5..08e3a981 100644 --- a/src/types.rs +++ b/src/types.rs @@ -865,6 +865,10 @@ pub enum OrderRequest { order_id: OrderId, price: Price, qty: u32, + /// Outside-RTH flag from the order the caller resubmitted. The replace + /// asserts tag 6433 from this rather than from the tracked record, + /// which has no field for it (ibx#247). + outside_rth: bool, }, } @@ -1620,6 +1624,7 @@ mod tests { order_id: 1, price: 100 * PRICE_SCALE, qty: 200, + outside_rth: false, }; let req2 = req.clone(); match (req, req2) { @@ -1868,7 +1873,7 @@ mod tests { assert_eq!(req.instrument(), Some(7)); assert_eq!(OrderRequest::Cancel { order_id: 1 }.instrument(), None); assert_eq!( - OrderRequest::Modify { new_order_id: 2, order_id: 1, price: 0, qty: 1 }.instrument(), + OrderRequest::Modify { new_order_id: 2, order_id: 1, price: 0, qty: 1, outside_rth: false }.instrument(), None ); } @@ -1893,7 +1898,7 @@ mod tests { #[test] fn order_request_modify_fields() { - let req = OrderRequest::Modify { new_order_id: 100, order_id: 99, price: 200 * PRICE_SCALE, qty: 10 }; + let req = OrderRequest::Modify { new_order_id: 100, order_id: 99, price: 200 * PRICE_SCALE, qty: 10, outside_rth: false }; match req { OrderRequest::Modify { order_id, price, qty, .. } => { assert_eq!(order_id, 99); diff --git a/tests/ib_paper_compat/orders.rs b/tests/ib_paper_compat/orders.rs index eba8d7c7..59617c2b 100644 --- a/tests/ib_paper_compat/orders.rs +++ b/tests/ib_paper_compat/orders.rs @@ -231,7 +231,7 @@ pub(super) fn phase_modify_order(conns: Conns) -> Conns { } else if !order_acked { order_acked = true; control_tx.send(ControlCommand::Order(OrderRequest::Modify { - order_id, new_order_id, price: 2_00_000_000, qty: 1, + order_id, new_order_id, price: 2_00_000_000, qty: 1, outside_rth: false, })).unwrap(); modify_sent = true; } @@ -459,7 +459,7 @@ pub(super) fn phase_modify_qty(conns: Conns) -> Conns { } else if !order_acked { order_acked = true; control_tx.send(ControlCommand::Order(OrderRequest::Modify { - order_id, new_order_id, price: 1_00_000_000, qty: 2, + order_id, new_order_id, price: 1_00_000_000, qty: 2, outside_rth: false, })).unwrap(); modify_sent = true; } @@ -1729,7 +1729,7 @@ pub(super) fn phase_modify_price_and_qty(conns: Conns) -> Conns { order_acked = true; // Modify BOTH price ($1→$2) and qty (1→3) in a single Modify control_tx.send(ControlCommand::Order(OrderRequest::Modify { - order_id, new_order_id, price: 2_00_000_000, qty: 3, + order_id, new_order_id, price: 2_00_000_000, qty: 3, outside_rth: false, })).unwrap(); modify_sent = true; } @@ -1797,14 +1797,14 @@ pub(super) fn phase_double_modify(conns: Conns) -> Conns { 0 => { // Original order acked → modify to $2 control_tx.send(ControlCommand::Order(OrderRequest::Modify { - order_id, new_order_id: modify_id_1, price: 2_00_000_000, qty: 1, + order_id, new_order_id: modify_id_1, price: 2_00_000_000, qty: 1, outside_rth: false, })).unwrap(); phase = 1; } 1 => { // First modify acked → modify again to $3 control_tx.send(ControlCommand::Order(OrderRequest::Modify { - order_id: modify_id_1, new_order_id: modify_id_2, price: 3_00_000_000, qty: 1, + order_id: modify_id_1, new_order_id: modify_id_2, price: 3_00_000_000, qty: 1, outside_rth: false, })).unwrap(); phase = 2; } @@ -1877,7 +1877,7 @@ pub(super) fn phase_cancel_during_modify(conns: Conns) -> Conns { order_acked = true; // Send modify AND cancel back-to-back — no waiting control_tx.send(ControlCommand::Order(OrderRequest::Modify { - order_id, new_order_id, price: 2_00_000_000, qty: 1, + order_id, new_order_id, price: 2_00_000_000, qty: 1, outside_rth: false, })).unwrap(); control_tx.send(ControlCommand::Order(OrderRequest::Cancel { order_id })).unwrap(); control_tx.send(ControlCommand::Order(OrderRequest::Cancel { order_id: new_order_id })).unwrap();