Skip to content

orders: route an order carrying only conditions through the extended encoder (ibx#325) - #326

Open
userFRM wants to merge 1 commit into
deepentropy:mainfrom
userFRM:fix/conditions-only-orders-drop-them
Open

orders: route an order carrying only conditions through the extended encoder (ibx#325)#326
userFRM wants to merge 1 commit into
deepentropy:mainfrom
userFRM:fix/conditions-only-orders-drop-them

Conversation

@userFRM

@userFRM userFRM commented Jul 29, 2026

Copy link
Copy Markdown

Summary

  • has_extended_attrs() decides whether an order reaches the encoder that emits the attribute block. It named thirteen fields, and conditions was not one of them — nor were conditions_cancel_order, conditions_ignore_rth or oca_type.
  • All four are copied into OrderAttrs by attrs(). They were simply never reached: the order had already been routed to the plain encoder, which emits no condition tags at all. Copied, then thrown away.
  • So a conditional order went out unconditional. A DAY limit whose only extra attribute is a price condition that is currently false carries no condition tags, the gateway routes it immediately, and it fills — the position is taken now, at the current price, in the direction the caller said they were not ready to trade. No error, and nothing in the message showing a condition was ever requested.

Closes #325.

Why it hides

Setting any one of the thirteen listed attributes, or a TIF other than DAY, makes it behave correctly. Any test using a realistic attribute bundle passes.

Reachable from EClient::place_order and the Python client. Not from Context::submit_*_ex, which takes OrderAttrs directly and always routes through the shared encoder.

Change

The predicate names everything attrs() carries. The two have to agree field for field — anything copied into OrderAttrs and not named by the predicate is dropped in silence.

Test

One entry per carried attribute, each set alone on an otherwise default order. Dropping any of the four added arms fails it. It doubles as the checklist: a field added to attrs() without being added to the predicate fails the test rather than shipping.

cargo test --lib: 803 pass. The two config::expiry_tests failures are pre-existing and unrelated (timezone data).

Test plan

🤖 Generated with Claude Code

@userFRM
userFRM force-pushed the fix/conditions-only-orders-drop-them branch from fd3f59a to e72b6dd Compare July 29, 2026 15:56
@userFRM

userFRM commented Jul 29, 2026

Copy link
Copy Markdown
Author

Two corrections after a closer look at what the test actually guarantees.

The forward guard is now real. The commit said a field added to attrs() without being added to the predicate would fail the test. That was not true — the test is a hand-written list, and a new field would have left it compiling and passing. The test now destructures OrderAttrs without a rest pattern, so adding a field there stops it compiling until it is accounted for in both places. Adding a probe field to the struct produces:

error[E0063]: missing field `probe_field` in initializer of `types::OrderAttrs`
help: include the missing field in the pattern

The Python client made the oca_type arm inert. Order::to_api() copies the two condition flags across but omitted oca_type, so an OCA type set from Python fell to the default and was replaced by the gateway's before reaching any of this. One line, and the new arm is reachable from that client.

Also confirmed while checking: good_after_time is named by the predicate but never carried — attrs() hardcodes it to 0 pending a wire capture (#199). That is pre-existing and deliberate, and the only remaining asymmetry in either direction.

…encoder (ibx#325)

`has_extended_attrs()` decides whether an order reaches the encoder that emits the attribute block, and it named thirteen fields. `conditions` was not one of them, nor were `conditions_cancel_order`, `conditions_ignore_rth` or `oca_type` — all four are copied into `OrderAttrs` by `attrs()` and were then thrown away, because the order had already been routed to the plain encoder that emits no condition tags at all.

A conditional order therefore went out unconditional. A DAY limit whose only extra attribute is a price condition that is currently false carries no condition tags, so the gateway routes it immediately and it fills — the position is taken now, at the current price, in the direction the caller said they were not ready to trade. Nothing in the message shows a condition was ever asked for, and no error is raised.

Setting any one of the thirteen listed attributes, or a TIF other than DAY, made it behave correctly, which is why an order with a realistic attribute bundle never showed it.

The predicate now names everything `attrs()` carries. The two have to agree field for field: anything copied into `OrderAttrs` and not named here is dropped in silence.

The test has one entry per carried attribute and sets each alone, so removing any arm fails it. It also destructures `OrderAttrs` without a rest pattern, which is the part that survives the next change: adding a field there stops the test compiling until it is accounted for both in the predicate and in the list.

The Python client copies the two condition flags across to the Rust order but not `oca_type`, so an OCA type set from Python was replaced by the gateway default before reaching any of this. It crosses now, which is what makes the new arm reachable from that client at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@userFRM

userFRM commented Jul 29, 2026

Copy link
Copy Markdown
Author

A second omission in the same Python function, found by the same comparison that turned up oca_type.

Order::to_api() also dropped trail_stop_price, so a Python TRAIL or TRAIL LIMIT order silently lost its initial stop trigger — the field is defined on the Python class and read by the request builder, and simply never crossed. One line, same shape as the oca_type fix, and an exhaustive field-by-field comparison confirms nothing else consumed on the outbound path is missing. conditions is the intentional exception: to_api leaves it empty and the placement call converts it separately.

On the test: good_after_time is named by the predicate but is not actually carried — attrs() hardcodes it to 0 pending a wire capture (#199) — so that entry pins the routing rather than an emitted tag. Marked as such in the list rather than left to read as though it were carried.

Two reviewers built plain and extended messages for all 21 encoder shapes and compared 210 pairs: every extended tag set is a superset of the plain one, with matching order type, TIF, quantity, price, routing, currency, side and account. The condition arm is the one that changes broker behaviour; the two flag arms and oca_type route without emitting anything extra, since the flags only appear inside a non-empty condition block and 6209 only alongside an OCA group. Wasted routes, byte-identical wire.

@userFRM
userFRM force-pushed the fix/conditions-only-orders-drop-them branch from e72b6dd to 83f8fa1 Compare July 29, 2026 16:39
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: an order whose only extended attribute is conditions has them silently dropped and routes immediately

1 participant