fix(data): honor Binance USD-M pu continuity - #125
Conversation
📝 WalkthroughWalkthroughThe depth sequence validator now computes the expected update ID before handling previous final update IDs. A unit test verifies that futures depth continuity is accepted when the next first update ID is non-consecutive but its previous final update ID establishes continuity. ChangesDepth sequence validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust_hft/data-pipelines/core/src/binance_market_tape.rs`:
- Around line 109-119: Extend DepthDiff to retain previous_final_update_id from
each Binance depth event, and populate it when constructing diffs in the
market-tape pipeline. Update LocalBook::apply_diff to use this preserved value
when validating non-consecutive update IDs, allowing valid Binance bridge diffs
while still rejecting genuine gaps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: daf5f579-798b-4605-90e1-3b09fed8e713
📒 Files selected for processing (1)
rust_hft/data-pipelines/core/src/binance_market_tape.rs
Change contract
Accept Binance USD-M diff-depth frames when
puexactly matches the previous event'su, even whenUis nonconsecutive, while preserving fail-closed no-puadjacency, previous-ID mismatch, final-ID rollback, and source-clock checks.Why
An isolated ECS capture on the production host reproduced the failure with consecutive authenticated frames: previous
u=11074967403842; nextpu=11074967403842,U=11074967403847,u=11074967407986. The existing validator rejected the second frame solely becauseU > previous_u + 1, although Binance USD-M continuity is defined bypu == previous u.Official contract: https://developers.binance.com/en/docs/products/derivatives-trading-usds-futures/websocket-market-streams/How-to-manage-a-local-order-book-correctly
Out of scope
No collector retry/lifecycle changes, artifact schema or trust-anchor changes, evaluator/snapshot work, deployment/cutover changes, production service changes, or live-trading enablement.
Dependency / merge order
None. Based directly on
mainafter #120. Merge this PR before rebuilding and rerunning the isolated ECS Binance candidate.Focused validation
pu/u/Ucounterexample failed withdepth sequence gap expected=11074967403843 received=11074967403847.cargo test -p hft-data: 23 passed.cargo test -p hft-collector --bin binance-lob-archiver: 36 passed.cargo clippy -p hft-data --all-targets -- -D warnings: passed.cargo clippy -p hft-collector --bin binance-lob-archiver -- -D warnings: passed.rustfmt --check: passed.pugap/rollback, no-pugap, final-ID rollback, and source-time counterexamples.The repository-wide
cargo fmt --all -- --checkstill reports unrelated pre-existing formatting drift outside this PR; no unrelated files were reformatted.Rollout / rollback impact
No automatic deployment. After merge, publish a new immutable Linux collector bundle, rerun the isolated BTC/SOL ECS capture, and require zero sequence gaps before using the tape. Roll back by reverting this commit; the current production collectors remain untouched and live trading remains disabled.
Summary by CodeRabbit