orders: encode every order type through one path (ibx#375) - #376
Open
userFRM wants to merge 3 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.
7 tasks
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.
Problem
build_order_requestchose 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 (#224), then the adjustable stop the same way (#240), then adaptive, algo and what-if (#318). Each was the same defect found again in a different arm.
What this changes
Every type 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.
MIDPXPEG MKTPEG MIDTRAIL,TRAIL LIMIT,TRAIL PCTThose 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 other fifteen types were already byte-identical.
The remaining per-type request variants are unreachable from the API surface after this and are removed separately.
Wire impact
None 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.
For an order of those six types that does carry an extended attribute or a non-DAY time-in-force, the affected tag moves to the position the per-type encoder used.
Tests
the_shared_encoder_restates_every_type_exactly_as_its_own_variant_doesencodes all twenty-one types through both paths and compares the frames with the sequence number, timestamps, body length and checksum removed. It fails by name when a moved tag is put back, and when a tag is added to only one of the two encoders.Closes #375.
Test plan
the_shared_encoder_restates_every_type_exactly_as_its_own_variant_doesencodes all twenty-one types through both paths and compares the frames with sequence number, timestamps, body length and checksum removed.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).