fix(ingest): scope MBO book keys by channel_id by ben-dz · Pull Request #110 · malbeclabs/doublezero-edge-connect

ben-dz · 2026-08-07T22:09:49Z

The edge-feed-spec is explicit that an instrument's unique key is (channel_id, instrument_id) — instrument_id is a u32 scoped to its channel and need not be unique across channels. MboProcessor keyed its books on (publisher, instrument_id).

What breaks

Two channels on one multicast group both carrying instrument id 7 have their books merged into one: each channel's deltas applied to the other's state. Every per-publisher sequence check still passes, so nothing catches it — it surfaces as a corrupt book with no error anywhere. Latent today: the live publishers put everything on channel_id = 0.

The fix

BookKey = (publisher, channel_id, instrument_id) — the same three axes MbpProcessor's PriceBookKey already carries — taking the raw wire channel, since producer-side state must stay separately sequenced while consumer-facing identity keeps going through DatagramCtx::canonical_channel. Three traps a reviewer should check:

Every map keyed off books moves with it: books_order, last_top, emitted_symbol, revealed, synced_reported, reveal_rebaselined_ns. One left on the 2-tuple outgrows the map it is bounded against — and for revealed it is worse than a leak: evicting one channel's book un-reveals an instrument a sibling channel is still serving, silently stopping its depth and its definition.
pending_channel is deleted, not re-documented. Its only rationale was that the book key carried no channel. The reveal now reads the channel from the key and canonicalizes at the emission site, as MBP does.
SnapshotOrder routing narrows to the channel instance (publisher + channel; the port is implicit, one processor serves one port block). Either component alone lets an order leak into another instance's same-snapshot_id building book.

EndOfSession still resets every book and clears the depth floors feed-wide.

Known gap, out of scope: reference data is still channel-flat, so two channels sharing an id resolve one definition and publish one symbol. The venue-wide depth floor then drops whichever channel trails each tick, so the consumer sees that symbol flip-flopping between two books, not merely sharing a name. Recorded in the BookKey doc and CLAUDE.md.

Verified

mbo_books_are_keyed_per_channel pins the merged book, mbo_book_eviction_does_not_unreveal_a_sibling_channel the un-reveal; both were checked failing against the channel-blind key. Full suite (852 lib + 84 integration), clippy --all-targets -D warnings, rustfmt clean. No runtime verification — there is no multi-channel feed to point it at.

The edge-feed-spec keys an instrument on (channel_id, instrument_id): instrument_id is scoped to its channel and need not be unique across channels. MboProcessor keyed its books on (publisher, instrument_id) alone, so two channels carried on one multicast group both using instrument id 7 would have their books merged - each channel's deltas applied to the other's state. Every per-publisher sequence check still passes, so nothing upstream catches it; it surfaces as a corrupt book with no error. Widen the key to (publisher, channel_id, instrument_id), taken from the frame header rather than any message body, and move the three sibling maps in lockstep - books_order, last_top and emitted_symbol - or they outgrow the book map they are bounded against. SnapshotOrder routing narrows to the same (publisher, channel): the spec only forbids interleaving snapshot groups within a channel, so with more than one channel two books can be building at once. Latent today: the live publishers put everything on channel_id = 0, so the key is degenerate.

armcconnell

Spec: FAIL
Verdict: REQUEST_CHANGES

[Important] src/ingest/processor.rs — Branch is 12 commits behind main and conflicts in all three changed files; main's book_for since gained a fourth map evicted in lockstep with books, revealed: HashMap<(IpAddr, u32), u16>, the Source-ID gate that blocks all emission for a key. A rebase that re-keys only books/books_order/last_top/emitted_symbol leaves revealed on the 2-tuple, so evicting one channel's book un-reveals an instrument a sibling channel is still serving and silently stops its depth and definition — the same class of silent corruption this PR fixes. Fix: rebase onto main, carry channel_id into revealed, and re-check pending_channel's "MBO's book key carries no channel dimension" rationale, which this change invalidates.

armcconnell · 2026-08-20T11:24:56Z

+/// two channels carrying the same id have their books merged and each channel's deltas applied to
+/// the other's state. The publisher component is separate: see [`MboProcessor::books`].
+///
+/// The reference-data state is NOT channel-scoped, so two channels sharing an id still resolve one


[Minor] Understates the residual gap: both channels emit depth under one (venue, symbol), so the venue-wide depth floor drops whichever channel trails each tick — a consumer sees one symbol flip-flopping between two books, not merely a shared name (the new test's own comment shows it). Say that here.

Fixed in 6cd6fba. The BookKey doc and CLAUDE.md now say it outright: the venue-wide depth floor drops whichever channel trails each tick, so the consumer sees one symbol flip-flopping between two books, not merely sharing a name.

armcconnell · 2026-08-20T11:24:56Z

+  and need not be unique across channels. Two channels on one group both carrying instrument id 7
+  had their books merged, applying each channel's deltas to the other's state; every per-publisher
+  sequence check still passed, so it surfaced only as a silently corrupt book. The book key and the
+  `SnapshotOrder` routing filter now both carry `channel_id`, taken from the frame header. Latent —


[docs] GLOSSARY bans frame for our own traffic — channel_id comes from the 24-byte datagram header. s/frame header/datagram header/.

Fixed in 6cd6fba — the CHANGELOG entry was rewritten against main and says "the datagram header".

armcconnell · 2026-08-20T11:24:56Z

-  sequences collide, so the books can't be merged. `SnapshotOrder` carries only a `snapshot_id` (no
-  instrument id) and routes **only to the originating publisher's** building book. `emit_depth` stamps
+  `MboProcessor` reconstructs an **independent book per `BookKey` = `(publisher, channel_id,
+  instrument_id)`**. *Publisher* (the datagram source IP): two publishers mirror one feed but their


[docs] GLOSSARY: bare source is banned (qualified form is source IP address), and frame is banned for our own traffic — so "datagram source IP" → "source IP address", and "frame header" on line 240 → "datagram header".

Fixed in 6cd6fba: "source IP address" and "datagram header" throughout the rewritten CLAUDE.md paragraph.

armcconnell · 2026-08-20T11:24:56Z

+/// What identifies one reconstructed book: `(publisher, channel_id, instrument_id)`.
+///
+/// The edge-feed-spec scopes `instrument_id` to its channel — it need not be unique across
+/// channels — so `channel_id` (from the *frame header*, never a message body) is part of the key or


[docs] GLOSSARY bans frame for our own traffic; the field lives in the 24-byte datagram header. Same at :714 ("One frame is one channel") and :1749.

Fixed in 6cd6fba. main had already renamed frame→datagram across the file; the re-applied change uses "datagram header" everywhere.

armcconnell · 2026-08-20T11:24:56Z

-    ///    `books`. An evicted legitimate book simply re-syncs from the next snapshot.
-    fn book_for(&mut self, instrument_id: u32, ctx: &FrameCtx) -> Option<&mut BookState> {
+    /// 2. Bounds the map to [`MAX_BOOKS`] [`BookKey`]s with least-recently-inserted eviction, so even
+    ///    a flood of *defined* forged instrument_ids (the source IP and channel are also spoofable —


[docs] GLOSSARY bans bare source; the qualified form is source IP address.

Fixed in 6cd6fba — book_for's doc now reads "the channel and the source IP address are spoofable too".

armcconnell · 2026-08-20T11:24:56Z

+                    // snapshot. snapshot_id is monotonic per (channel, instrument) - not globally
+                    // unique, and certainly not across publishers - but the spec forbids
+                    // interleaving snapshot groups across instruments per channel, so at most one
+                    // book per (publisher, channel) is `building` at a time. Both key components


[docs] GLOSSARY already names (publisher, channel) a channel instance — (source IP address, Channel ID, destination port), the unit that owns a snapshot cycle. Use the term, and note the port is implicit: one processor serves one port block.

Fixed in 6cd6fba. The SnapshotOrder comment now uses "channel instance" and notes the port is implicit — one processor serves one port block.

`main`'s MBO processor gained three more maps evicted in lockstep with `books` — `revealed`, `synced_reported`, `reveal_rebaselined_ns` — plus `pending_channel`, a memo whose only reason to exist was that the book key carried no channel. Re-keying `books` alone would have left `revealed` on the 2-tuple, so evicting one channel's book un-revealed an instrument a sibling channel was still serving and silently stopped its depth and definition. Every map keyed off `books` now carries the raw wire `channel_id`, and `pending_channel` is gone: with the channel in the key, `reveal_if_needed` reads it directly and canonicalizes at the emission site, exactly as `MbpProcessor` already does. Consumer-facing identity still goes through `DatagramCtx::canonical_channel`, so a mirror path's two channels collapse to one market while their books stay separately sequenced. `mbo_book_eviction_does_not_unreveal_a_sibling_channel` replaces the `pending_channel` eviction test and pins the un-reveal directly.

ben-dz · 2026-08-28T01:50:09Z

Merged origin/main in 6cd6fba. You were right about revealed: it now carries channel_id along with synced_reported and reveal_rebaselined_ns, and mbo_book_eviction_does_not_unreveal_a_sibling_channel pins it — verified failing when revealed is evicted channel-blind.

pending_channel is deleted rather than re-documented: its only rationale was the missing channel dimension, so with the channel in the key the reveal reads it directly and canonicalizes at the emission site, matching MbpProcessor. Producer-side state stays on the raw wire channel; consumer-facing identity still goes through canonical_channel, so the MBO mirror test is unchanged and now asserts real producer-side separation.

Full suite, clippy -D warnings and rustfmt clean. Still no runtime verification, as there is no multi-channel feed. PR description updated to match.

ben-dz added 2 commits August 7, 2026 18:09

docs(changelog): note the MBO channel-scoped book key fix

f6d5ddd

ben-dz requested review from armcconnell and juan-malbeclabs August 7, 2026 22:18

armcconnell requested changes Aug 20, 2026

View reviewed changes

ben-dz requested a review from armcconnell August 28, 2026 01:51

Uh oh!

Conversation

ben-dz commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What breaks

The fix

Verified

Uh oh!

armcconnell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben-dz commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ben-dz commented Aug 7, 2026 •

edited

Loading