Skip to content

fix(data): tolerate bounded Binance source clock lead - #121

Merged
proerror77 merged 1 commit into
mainfrom
codex/binance-source-clock-tolerance
Jul 17, 2026
Merged

proerror77 merged 1 commit into
mainfrom
codex/binance-source-clock-tolerance

Conversation

@proerror77

@proerror77 proerror77 commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Change contract

Accept Binance depth and aggregate-trade source timestamps that lead the local receive clock by at most 1,000 ms while preserving the existing 30,000 ms stale-delay ceiling and rejecting larger leads fail closed.

Out of scope

  • Aggregate-trade ID or raw-trade-range continuity
  • Capture epoch and segment rotation semantics
  • Artifact verification, research assembly, deployment, or ECS changes

Dependency or merge order

PR #118 is already merged. This PR is otherwise independent and must merge before the Binance artifact verifier is rebased and promoted.

Focused validation

  • cargo test --manifest-path rust_hft/Cargo.toml -p hft-data binance_market_tape — 14 passed
  • cargo clippy --manifest-path rust_hft/Cargo.toml -p hft-data --all-targets -- -D warnings — passed
  • cargo fmt --manifest-path rust_hft/Cargo.toml -p hft-data -- --check — passed
  • git diff --check origin/main...HEAD — passed
  • Counterexample coverage: +1,000 ms is accepted and +1,001 ms is rejected for both depth and aggTrade frames; the 30,000 ms delay guard remains covered.

Rollout / rollback impact

No runtime switch is performed by this PR. Rollout is the shared parser behavior change; rollback is a single-commit revert that restores zero positive clock lead tolerance. Existing fail-closed replay safety from #118 remains in effect.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of Binance market events when source timestamps are slightly ahead of the local receive clock.
    • Events may now lead the receive clock by up to 1 second; larger differences are rejected with a clear validation error.
    • Preserved safeguards against excessive source-to-receive delays.
    • Updated validation coverage for depth and aggregate-trade events.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 86cc9408-7039-40d4-8970-0298f094008e

📥 Commits

Reviewing files that changed from the base of the PR and between 27afc2e and 0a83705.

📒 Files selected for processing (1)
  • rust_hft/data-pipelines/core/src/binance_market_tape.rs

📝 Walkthrough

Walkthrough

Binance receive-clock validation now permits source timestamps to lead the local receive clock by up to 1,000 milliseconds, while retaining the maximum delay check. Depth and aggregate-trade tests cover acceptance at the boundary and rejection beyond it.

Changes

Binance receive-clock validation

Layer / File(s) Summary
Bounded source-clock validation
rust_hft/data-pipelines/core/src/binance_market_tape.rs
Adds MAX_SOURCE_LEAD_MS and uses saturating arithmetic to enforce bounded source lead and source-to-receive delay checks.
Depth and aggregate-trade boundary tests
rust_hft/data-pipelines/core/src/binance_market_tape.rs
Updates the depth error assertion and adds depth and aggregate-trade tests for lead acceptance and rejection boundaries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • proerror77/monday#104: Modifies the same Binance receive-clock validation logic and governed timing checks.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: allowing a bounded Binance source clock lead.
Description check ✅ Passed It covers the required contract, out-of-scope, dependency, validation, and rollout details, with only minor template heading deviations.
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/binance-source-clock-tolerance

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

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a837056b7

ℹ️ 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".

let received_at_ms = received_at_ns / 1_000_000;
if event_time_ms > received_at_ms {
anyhow::bail!("{kind} source and receive clocks are reversed");
if event_time_ms.saturating_sub(received_at_ms) > MAX_SOURCE_LEAD_MS {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Carry the clock-lead tolerance into replay filters

For accepted Binance aggTrade/depth frames whose source timestamp is 1–1000 ms ahead of receive time, this parser now permits rows with received_at < trade_time/event_time; however, checked rust_hft/prediction-markets/crates/ploy-feed-loaders/src/database.rs and the sampled replay queries still require received_at >= trade_time (lines 90–94) and received_at >= event_time (lines 121–126). Those newly accepted rows are therefore silently excluded from historical replay/features, so the tolerant capture contract does not carry through to experiments.

Useful? React with 👍 / 👎.

@proerror77
proerror77 merged commit 843de0a into main Jul 17, 2026
19 checks passed
@proerror77
proerror77 deleted the codex/binance-source-clock-tolerance branch July 17, 2026 23:33
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.

1 participant