orders: remove the per-type order request variants - #377
Open
userFRM wants to merge 4 commits into
Open
Conversation
`build_order_request` returned early into a standalone `SubmitAdjustableStop` request, which bypassed the extended-attributes path every other order type goes through. The dedicated encoder arm emitted neither tag 6107 nor 583 and hard-coded 59=0, so an adjustable stop used as a bracket child shipped unlinked from its parent, outside its OCA group and DAY. The bypass dropped the rest of `OrderAttrs` with them: outside-RTH, hidden, display size, trigger method, conditions and GTD expiry were all silently discarded on this path. The adjustable stop is now an `OrderKind::AdjustableStop` carried by `SubmitEx`, so it encodes through `send_order_ex` like every other kind and picks up the shared attribute block. The wire layout is unchanged: 40=3 and 99 sit with the other order-type tags, and the 6257/6261/6258/6259 group plus the conditional 6262 and 6260/6269 are appended after 204 and the attribute block, which is where the encoder being replaced put them. Tag order should not carry meaning, but this path had a shipped layout and there was no reason to move it as a side effect. `Context::submit_adjustable_stop` takes `tif` and `attrs` to match the other extended submitters. `Connection::for_test()` is new test-only plumbing: it hands back the peer socket so a test can assert on the bytes an encoder actually writes. Two regression tests use it: one pins 6107, 583 and 59 for a bracket child, the other pins the conditional 6262/6260/6269, the absence of 6107/583 when no parent or OCA is set, and the relative order of the whole group. The enum-level tests passed unchanged for the whole time the child was shipping naked, which is why these assert on the wire instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`build_order_request` computes the extended-attribute block partway down, and three order types returned before reaching it. Their request variants carried no attribute block at all, so `Order::attrs()` was never consulted on those paths and their encoders emitted none of it. Setting `outsideRth` on an adaptive, algo or what-if order was accepted by the API and silently ignored — the order went out regular-hours-only with nothing reporting the difference. The same early return bypassed the parent link and the OCA group, so an adaptive or algo order used as a bracket child was submitted unlinked and outside its group, which is what the attribute block exists to prevent. The tif was hard-coded to DAY on all three. Patching the missing tags into each dedicated encoder would leave the rest of the block — hidden, display size, min quantity, good-after, GTD expiry, discretionary amount, sweep-to-fill, all-or-none, trigger method, cash quantity, conditions — still dropped on those paths, and would put the emission in four places. These route instead, the way the adjustable stop does: `Adaptive`, `Algo` and `WhatIf` are now `OrderKind` variants carried by `SubmitEx`, and the three standalone encoder arms are gone. Their own tags keep their values and their position — after tag 204 and the attribute block, where the encoders this replaces put them: 18=e and the adaptive priority parameter, the algo strategy with 849 and its parameter pairs, and the what-if flag. A what-if is still tracked under its marker so the response is recognised as a preview. `OrderKind` is no longer `Copy`, because the algo parameters it now carries own their strings. `Context::submit_adaptive`, `submit_algo` and `submit_what_if` take a tif and an attribute block, as `submit_adjustable_stop` does. Closes deepentropy#318.
`build_order_request` chose per order type between a request that carries the extended attributes and a plain one that does not, and the engine had a separate encoder for each. Twenty-one order types were encoded twice, in two places, from two field lists. That duplication is where the attribute bugs come from. An order type whose own encoder drifts from the shared one ships without something the caller set: bracket children submitted unlinked and DAY (deepentropy#224), then the adjustable stop the same way (deepentropy#240), then adaptive, algo and what-if (deepentropy#318). Each was the same defect found again in a different arm. Every type now routes through the shared encoder. The plain-versus-extended choice is gone, so there is no arm for a type to drift into. A test encodes each type both ways and compares the frames, which is what makes that safe to do and what stops the two paths separating again while both exist. It found six that did not agree, all the same shape: an optional tag the per-type encoder appends after 204, emitted in among the order-type tags by the shared one. The mid-price cap on 44, the pegged offset on 211 with the two mid-offset tags beside it, and the initial trailing trigger on 6117 now sit where the per-type encoders put them, which is where the captures show them. The values and the conditions are unchanged; only the position moves, and only on the shared path. The remaining per-type request variants are now unreachable from the API surface and are removed separately. No wire change for an order with no extended attributes: the two encodings are byte-identical, tag for tag and value for value, for all twenty-one types.
With every order type encoding through the shared path, the per-type request variants and their encoders are unreachable from the API surface and carry no behaviour of their own. Twenty-three variants and their encoder arms are removed. The `Context` helpers that constructed them build the equivalent `SubmitEx` instead, with the same defaults they already implied: `submit_market`, `submit_limit`, `submit_stop` and the rest keep their signatures, so a caller of the low-level engine API sees no change. `OrderRequest::order_id` already answers what the integration harness was answering with a forty-line match over every submit variant; it calls the accessor now. The equivalence test goes with them. Its subject was the second encoding of each type, and it existed to establish that the two agreed before the second one was removed. What it was guarding against — one encoder gaining a tag the other does not — is no longer expressible. OrderRequest variants 42 → 15 send_fix sites in one file 42 → 17 order_builder.rs 2059 → 1443 lines Three of the diagnostics `ib_paper_compat` already produced were call sites constructing a trailing-stop variant without its optional trigger; migrating them supplied it, so that target now reports nine rather than twelve. The nine are the ones it reported before, unchanged. No behaviour change: the removed encoders were byte-identical to the shared one for every type they covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this removes
With every order type encoding through the shared path (#376), the per-type request variants and their encoders are unreachable from the API surface and carry no behaviour of their own.
Twenty-three variants and their encoder arms are gone.
What does not change
The
Contexthelpers that constructed them build the equivalentSubmitExinstead, with the same defaults they already implied.submit_market,submit_limit,submit_stopand the rest keep their signatures, so a caller of the low-level engine API sees no change at all.OrderRequest::order_idalready answers what the integration harness was answering with a forty-line match over every submit variant; it calls the accessor now.The equivalence test goes too
Its subject was the second encoding of each type, and it existed to establish that the two agreed before one of them was removed. What it guarded against — one encoder gaining a tag the other does not — is no longer expressible with a single encoder.
ib_paper_compatThree of the diagnostics that target already produced were call sites constructing a trailing-stop variant without its optional trigger. Migrating them supplied it, so the target now reports nine rather than twelve. The nine are the same
filtersomissions it reported before, unchanged.Verification
The removed encoders were byte-identical to the shared one for every type they covered, established by the equivalence test in #376 across all twenty-one types.
--lib,--lib --features python,--bins,--examplesand all eight integration targets check clean.Follows #376, which closes #375.
Test plan
Contexthelpers keep their signatures, so a caller of the low-level engine API sees no change.--binsincluded in the check sweep: two bench targets still constructed a removed variant and were migrated.ib_paper_compatreports nine diagnostics rather than twelve — the three that disappeared were trailing-stop initializers missing their optional trigger, supplied during the migration; the remaining nine are unchanged.cargo check --offlineclean on--lib,--lib --features python,--bins,--examples, and each integration target individually.tests/ib_paper_compatcompared against a clean checkout of the base commit — identical sorted diagnostic sets.cargo test --offline --lib— only the two knownconfig::expiry_testsfailures, which fail on the base commit for missing legacy tzdata (fixed separately in config: resolve the legacy timezone names IB states its times in (ibx#335) #336).