Skip to content

orders: restate OutsideRTH on a modify instead of asserting it (ibx#312) - #319

Closed
userFRM wants to merge 1 commit into
deepentropy:mainfrom
userFRM:fix/modify-forces-outside-rth
Closed

orders: restate OutsideRTH on a modify instead of asserting it (ibx#312)#319
userFRM wants to merge 1 commit into
deepentropy:mainfrom
userFRM:fix/modify-forces-outside-rth

Conversation

@userFRM

@userFRM userFRM commented Jul 29, 2026

Copy link
Copy Markdown

Summary

  • Every replace carried 6433=1. The comment beside it read "OutsideRTH (preserve from original)", but the value was a literal and there was nothing to preserve it from — types::Order had no field for the flag, so what the order was submitted with was gone by the time a modify was built.
  • The submit path is correct: it emits 6433 only when the caller asks, and omits the tag otherwise. So the default order is regular-hours-only, and the first modify silently changes that.
  • Modifying an order's price or quantity turns on extended-hours eligibility regardless of how it was placed. A resting stop submitted for regular hours becomes eligible to trigger outside them as soon as it is touched. The gateway accepts the flag, the replace is acknowledged normally, and the order reads back as intended apart from the one field the caller never set.
  • Both entry points reach it: EClient::place_order on an already-tracked id, and Context::modify.

Closes #312.

Change

  • Order carries the flag, set from the submit request and from the CCP recovery record, where the server restates the order and the flag is on the wire.
  • The replace restates it, and omits the tag when the order is not eligible — which is exactly what the submit path does for the same order, so the two messages agree rather than contradicting each other.
  • A replace also builds a fresh tracked order from the original, so the flag is carried across that too. Without it the first replace is right and every later one wrong — the shape that survives a casual test.

Omitting the tag rather than sending 6433=0 also leaves the server's own setting alone in the case where the original order is not tracked.

Wire shape

For an order submitted outside RTH the replace is byte-identical to before, including the tag's position in the message. The tag list moved into a free function so that can be asserted without a live connection.

Tests

The tests drive the real send path over a loopback pair and read back what was written, so they cover both what the engine records and what it puts on the wire. Each of these fails them:

  • passing the flag through as false at the callsite
  • dropping it at any of the three GTC submits
  • dropping it across a replace, so only the second modify is wrong

cargo test --lib: 808 pass. The two config::expiry_tests failures are pre-existing and unrelated (timezone data). No new clippy warnings.

Related

#318 — four order types (adaptive, algo, what-if, adjustable stop) return before the extended-attribute block and never record the flag at all, along with the parent link and OCA group. Pre-existing and separate; found while tracing where the flag comes from.

Every replace carried `6433=1`. The comment beside it said "preserve from original", but the value was a literal and there was nothing to preserve from: `Order` did not track OutsideRTH, so the flag the order was submitted with was gone by the time a modify was built.

The result is that modifying any order turns on extended-hours eligibility, whichever way the order was placed. A resting stop submitted for regular hours only — the default, since the submit path omits 6433 unless asked for it — becomes eligible to trigger outside them the first time its price or quantity is touched. Nothing reports the change: the gateway accepts the flag, the replace is acknowledged normally, and the order reads back as the caller intended apart from the one field they never set. The exposure is a thin pre- or post-session print taking out a stop at a level the regular session never traded.

`Order` now carries the flag, set from the submit request and from the recovery record on reconnect, and the replace restates it. A replace also builds a fresh tracked order from the original, so the flag is carried across that too — without it the first replace would be right and every later one wrong, which is the shape that survives a casual test.

When the order is not eligible the tag is omitted, which is what the submit path does for the same order, so the two agree rather than contradicting each other one message apart.

The replace tag list moved into a free function so the wire shape can be tested without a live connection. Its position in the message is unchanged, and an order submitted outside RTH produces the same bytes it did before.

The tests drive the real send path over a loopback pair and read back what was written, so they cover what the engine records and what it puts on the wire. Passing the flag straight through as false at the callsite, dropping it at any of the submits, or dropping it across a replace each fail them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@userFRM
userFRM force-pushed the fix/modify-forces-outside-rth branch from a9b69c2 to 371f891 Compare July 29, 2026 16:45
@userFRM

userFRM commented Jul 29, 2026

Copy link
Copy Markdown
Author

Three mutations were slipping through, and the most important one covered the path every public-client order actually takes.

send_order_ex is where an order carrying outsideRth from either client ends up — the routing predicate sends them all there. Dropping the flag on that one path passed the whole suite, because the only test covered submit_limit_gtc. The resting stop this change exists for was untested for the same reason, and so was the recovery record's own flag: a recovered extended-hours order would have silently narrowed to regular hours on its next modify, since the replace restates what the engine recorded. All three are covered now and each mutation fails.

Also fixed a placement slip of my own: paired_for_test was inserted between send_fixcomp and its doc comment, so the compress/sign documentation ended up describing the test helper. Same mistake I made in #333 — put back with its function.

Independent verification worth recording: the outside-RTH submit and modify are byte-identical to the base across both commits, and for an ordinary RTH replace the only difference is the removed 6433=1. The reviewer also noted that at base every replace carried that flag regardless of order type, so the widening applied to the whole book, not just the types tested here.

@userFRM

userFRM commented Jul 30, 2026

Copy link
Copy Markdown
Author

Closing in favour of #249, which fixes the same defect. The underlying bug was filed twice — #247 and #312 — and each attracted its own PR.

#249 is the one to keep, and not only because it is a third the size. The two take opposite positions on where the flag comes from, and that difference outlives the outside-RTH question:

In this API a modify is a full resubmission — place_order with a tracked id is the modify entry point, mirroring TWS, where a replace restates the whole order. Under that reading the caller's current value is the answer, and restating the placed value means a caller who changes the flag on a modify is silently ignored. That is the same defect shape as #349, so this approach would have fixed one instance of it while entrenching another.

The recovery-record hunk here exists to feed the stored field, so it goes with the approach rather than being an independent fix. If a recovered order's 6433 needs to reach callers on the status path, that deserves its own issue on its own merits.

One thing worth keeping from this branch: build_replace_fields made the wire shape testable without a socket. #249 gets the same coverage more directly, by reading the bytes off a loopback pair in both polarities, so the extraction is not needed to test it — and a fourteen-argument function behind an #[allow(clippy::too_many_arguments)] is not the shape to upstream it in.

@userFRM userFRM closed this Jul 30, 2026
@userFRM
userFRM deleted the fix/modify-forces-outside-rth branch August 4, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orders: a modify always sets OutsideRTH, so any replace makes an RTH-only order eligible outside regular hours

1 participant