fix(collector): separate Binance event and trade clocks

[proerror77]

Issue relationship

Refs #846. The runtime outcome remains open until its own Shadow and OSS evidence passes.

What changed

• use only event time E for the 30-second source-to-receive gate
• keep depth T<=E, monotonicity, and sequence checks without treating T as network latency
• keep the existing trade-age rejection for aggTrade, raw trade, and forceOrder, but name it separately
• record recv-E, E-T, and recv-T; preserve the rejected raw-trade frame and shard identity
• avoid cloning every normal raw-trade frame on the hot path

The 30-second threshold, fail-closed trade/depth behavior, session policy, deployment, Gate, cutover, and Live behavior are unchanged.

Evidence

• red: fresh E plus old T depth fixture was rejected by the previous shared receive-clock check
• green: hft-data 81/81
• green: Linux collector test target cargo check
• green: scoped hft-data and collector Clippy with -D warnings
• green: git diff --check
• local macOS execution of the collector test remains blocked by the existing libc::RUSAGE_THREAD portability failure; exact-head Linux CI is required

Sources

Binance documents E as event time and T as trade/transaction time; Spot bookTicker carries neither field.

Summary by CodeRabbit

• Bug Fixes
  • Improved market-data timing validation for depth, aggregate-trade, raw-trade, and force-order events.
  • Fresh events are now accepted when transaction timestamps are old but event receipt timing is valid.
  • Trades exceeding allowed receipt-to-trade delays are rejected consistently.
  • Parsing errors now include clearer timing, stream, symbol, and source details to simplify troubleshooting.
• Tests
  • Added regression coverage for timing validation and detailed stale-trade error reporting.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change centralizes Binance clock validation across market-tape parsers. It allows fresh events with older transaction times and reports event, trade, and receive ages. Raw-trade archiving now uses a shared parser and preserves detailed failure context.

Changes

Binance clock validation and raw-trade flow

Layer / File(s)	Summary
Centralized clock validation rust_hft/data-pipelines/core/src/binance_market_tape.rs	Depth parsing validates event time only. Aggregate-trade, raw-trade, and force-order parsing use validate_trade_clocks. Tests verify accepted old transaction times and detailed age diagnostics.
Raw-trade archiver diagnostics rust_hft/tools/collector/src/bin/binance-lob-archiver.rs	Raw-trade frame parsing is shared across dispatch paths. Zero-price handling remains supported. Parse errors include stream, symbol, clock, producer, and original-frame details. Regression tests cover event-time and trade-time violations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• proerror77/monday#104: Introduced clock-validation logic that this change extends.
• proerror77/monday#811: Refactored related raw-trade parsing and USD-M zero-price handling.
• proerror77/monday#816: Modified related raw-trade validation and zero-price sentinel handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: separating Binance event and trade clock handling.
Description check	✅ Passed	The description explains the behavior changes, issue reference, validation evidence, and runtime impact, although headings differ from the template.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/raw-trade-stale-diagnostics-846

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

rust_hft/data-pipelines/core/src/binance_market_tape.rs (1)

1099-1103: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid the per-frame allocation in validate_trade_clocks.

format!("{kind} E") allocates for every aggregate-trade, raw-trade, and force-order frame. Raw trades use this hot path. Pass the static event-clock label from each caller.

Proposed refactor

 fn validate_trade_clocks(
     event_time_ms: u64,
     trade_time_ms: u64,
     received_at_ns: u64,
     kind: &str,
+    event_clock_kind: &str,
 ) -> Result<()> {
@@
-    if let Err(error) = validate_receive_clock(event_time_ms, received_at_ns, &format!("{kind} E"))
+    if let Err(error) = validate_receive_clock(event_time_ms, received_at_ns, event_clock_kind)

-        validate_trade_clocks(event_time_ms, trade_time_ms, received_at_ns, "raw trade")?;
+        validate_trade_clocks(
+            event_time_ms,
+            trade_time_ms,
+            received_at_ns,
+            "raw trade",
+            "raw trade E",
+        )?;

🤖 Prompt for 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.

In `@rust_hft/data-pipelines/core/src/binance_market_tape.rs` around lines 1099 -
1103, Update validate_trade_clocks and its callers to accept a static
event-clock label, passing the appropriate literal from each aggregate-trade,
raw-trade, and force-order path. Replace the per-frame format!("{kind} E")
construction in validate_receive_clock with that label while preserving the
existing validation and error context.

🤖 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.

Nitpick comments:
In `@rust_hft/data-pipelines/core/src/binance_market_tape.rs`:
- Around line 1099-1103: Update validate_trade_clocks and its callers to accept
a static event-clock label, passing the appropriate literal from each
aggregate-trade, raw-trade, and force-order path. Replace the per-frame
format!("{kind} E") construction in validate_receive_clock with that label while
preserving the existing validation and error context.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f9b90f23-0a11-4a7b-9f1a-0e3d6fa7d860

📥 Commits

Reviewing files that changed from the base of the PR and between a5bd25c and 4229e28.

📒 Files selected for processing (2)

• rust_hft/data-pipelines/core/src/binance_market_tape.rs
• rust_hft/tools/collector/src/bin/binance-lob-archiver.rs