feat(data): verify Binance market tape artifacts - #120
Conversation
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✨ Finishing Touches🧪 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 |
674217c to
780ad17
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 780ad174f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if aggregate_trades.is_empty() { | ||
| bail!("verified market-tape has no aggregate trades"); |
There was a problem hiding this comment.
For multi-symbol artifacts, this only checks that at least one aggregate trade exists anywhere in the sealed set, so a replay-safe segment containing BTC and ETH checkpoints/books but only a BTC agg_trade can still produce a verified handle with no ETH price surface. I checked the collector path: replay_checkpoint_ready only marks a segment replay-safe after every state key appears in aggregate_trade_symbols (rust_hft/tools/collector/src/bin/binance-lob-archiver.rs:1083-1085), so the verifier should reconstruct that per-symbol coverage before accepting the manifest’s latest aggregate trade price contract.
Useful? React with 👍 / 👎.
| } | ||
| checkpoints.insert(symbol.clone()); | ||
| } | ||
| observe_replay(&mut replay, &symbol, event_type, raw, received_at_ns)?; |
There was a problem hiding this comment.
When the sealed set starts at a segment_open/checkpoint-only segment, this call lets the existing replay validator install that collector-generated checkpoint as the initial book state; I checked ReplaySequenceValidator::observe, and with no prior state it accepts a checkpoint at rust_hft/data-pipelines/core/src/binance_lob_replay.rs:214-220. That means an anchored file with just a bridged checkpoint plus one aggregate trade can verify without any raw Binance snapshot/diff evidence for the book, so leading checkpoints should be rejected unless an earlier sealed segment has already replayed the state they summarize.
Useful? React with 👍 / 👎.
| if book.bids.is_empty() || book.asks.is_empty() { | ||
| bail!("verified market-tape contains an empty replayed book"); |
There was a problem hiding this comment.
If an anchored tape carries a snapshot/checkpoint/diff level with a zero or negative quantity (or non-positive price), the replay path only parses the decimals (rust_hft/data-pipelines/core/src/binance_lob_replay.rs:364-368) and this final check treats any non-empty side as valid. In that scenario VerifiedBinanceMarketTape can expose a replayed book containing impossible live levels, so the verifier should reject non-positive final book levels rather than only checking that each side is non-empty.
Useful? React with 👍 / 👎.
Change contract
Externally authenticated content and manifest SHA-256 values seal bound Binance market-tape file descriptors; only one capture session whose manifests, immutable archived-row envelopes, clocks, LOB replay, aggregate trades, and cross-segment sequences all validate can produce an opaque
VerifiedBinanceMarketTapewith typed read-only projections. Manifest and archived-row envelopes are closed-world; nestedframeandsnapshotvalues remain authenticated, venue-owned raw Binance payloads and may carry additional venue fields, while only validated identity, clock, sequence, numeric, and replay surfaces enter the verified projections.Out of scope
Collector production, downloader or OSS selector, trade backfill, research mapping, snapshot assembly, deployment, database import, Polymarket or ECS changes. Versioning a closed-world Binance raw-payload schema is also out of scope.
Dependency or merge order
#104, #118, #121, and #124 are merged. This PR is rebased on current main
9ae1ef9eand has no remaining prerequisite. It does not depend on a downloader.Focused validation
cargo test --locked -p hft-data: 22 passedcargo test --locked -p hft-data binance_market_tape_artifact -- --nocapture: 5 passedcargo clippy --locked -p hft-data --all-targets -- -D warningscargo fmt --package hft-data -- --checkgit diff --checkCargo.locklines, below the 750-line split threshold.Rollout or rollback impact
No runtime rollout. This adds only a fail-closed verifier seam and opaque verified handle. Rollback removes Binance market-tape eligibility but does not alter collection, deployment, execution, or live trading.