feat(runtime): emit signed arrival-cost evidence - #787
Conversation
📝 WalkthroughWalkthroughThe change adds optional arrival-price data to order lifecycles and execution events. Market orders capture the resolved executable quote. Execution paths propagate the value. Runtime attribution preserves it across duplicates and records arrival-price slippage. ChangesArrival-price instrumentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MarketIntentProcessor
participant ExecutionWorker
participant ExecutionEvent
participant RuntimeAttribution
MarketIntentProcessor->>ExecutionWorker: resolve and store lifecycle arrival_price
ExecutionWorker->>ExecutionEvent: emit OrderNew with arrival_price
ExecutionEvent->>RuntimeAttribution: process order event
RuntimeAttribution->>RuntimeAttribution: preserve metadata and calculate arrival slippage
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
* feat(research): define CEX replay snapshot V2 * fix(research): bind V2 snapshot evidence timelines * fix(research): keep V1 writer until consumers migrate * fix(research): harden V2 PIT evidence bounds * fix(research): bind complete PIT evidence sets * fix(research): close V2 evidence coverage gaps * fix(research): bind fee evidence to account * feat(research): materialize aggregate trade flow * feat(research): verify real lifecycle latency evidence * fix(research): bind V2 series identity * chore: refresh trade feature stack base * chore: refresh trade feature stack base * fix(research): bind latency evidence to fee account * refactor(research): pass prebuilt source lineage * chore: refresh latency evidence stack base * test(research): keep aggregate trade ids contiguous * chore: refresh latency evidence stack base * fix(research): bind real arrival latency evidence * fix(research): authenticate runtime instrument market --------- Co-authored-by: Sonic Shih <sonic.shih@mandonothing.com>
This reverts commit 26890d6.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee77203dcf
ℹ️ 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".
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…-instrumentation # Conflicts: # agent-worktree.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57463cffb6
ℹ️ 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".
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust_hft/apps/live/src/runtime_attribution.rs (1)
348-390: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake conflicting duplicate invalidation terminal.
A duplicate with a different
arrival_priceis accepted because the conflict predicate does not compare it. The new value then overwrites the prior arrival evidence.After a detected conflict, this code removes metadata but stores no rejected-order marker. A later matching
OrderNewcan recreate metadata and make subsequent fills attributable again.Treat a populated, different
arrival_priceas a conflict. Retain an invalid-order tombstone after any conflict. Add a test with a conflicting event, a later matching duplicate, and a fill.Also applies to: 1384-1410
🤖 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/apps/live/src/runtime_attribution.rs` around lines 348 - 390, Update the duplicate-order conflict checks in the shown handler and corresponding flow around the second occurrence to compare populated, differing arrival_price values. When any conflict is detected, preserve a rejected-order tombstone in state so later matching OrderNew events cannot recreate metadata or attribute fills; add coverage for a conflicting event, a subsequent matching duplicate, and its fill.
🤖 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/market-core/engine/src/lib.rs`:
- Around line 1658-1674: Update the retain_mut closure in the market-intent
processing block so non-market envelopes preserve their existing
lifecycle.arrival_price. Only assign arrival_price from the enriched
envelope.intent.price when a market intent is successfully accepted; otherwise
leave it unchanged, while retaining the current rejection behavior for failed
market enrichment.
---
Outside diff comments:
In `@rust_hft/apps/live/src/runtime_attribution.rs`:
- Around line 348-390: Update the duplicate-order conflict checks in the shown
handler and corresponding flow around the second occurrence to compare
populated, differing arrival_price values. When any conflict is detected,
preserve a rejected-order tombstone in state so later matching OrderNew events
cannot recreate metadata or attribute fills; add coverage for a conflicting
event, a subsequent matching duplicate, and its fill.
🪄 Autofix
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 Plus
Run ID: 039bbd59-4527-41b8-8aea-ff5e6f29be30
📒 Files selected for processing (7)
agent-worktree.ymlrust_hft/apps/live/src/runtime_attribution.rsrust_hft/execution-gateway/adapters/adapter-backpack/src/lib.rsrust_hft/market-core/engine/src/execution_worker.rsrust_hft/market-core/engine/src/lib.rsrust_hft/market-core/ports/src/events.rsrust_hft/market-core/runtime/src/system_builder/simulated_execution.rs
…-instrumentation # Conflicts: # agent-worktree.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58494e6124
ℹ️ 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".
|
Exact-head review requested for 258c540. IOC now captures the side-specific executable quote without replacing its protection limit; unbound Market/IOC intents bind to the event-sequenced venue, and the unsafe any-venue fallback is removed. Focused engine tests pass. Please review this exact head. |
…-instrumentation # Conflicts: # agent-worktree.yml
|
Exact-head review requested for afc533b after integrating current main (including merged #791). The arrival-quote fix remains unchanged from 258c540: IOC quote capture preserves its protection limit, unbound intents bind to the event-sequenced venue, and no any-venue quote can be signed. Please review this exact head. |
There was a problem hiding this comment.
💡 Codex Review
monday/rust_hft/market-core/engine/src/lib.rs
Lines 2063 to 2065 in afc533b
Reject stale fallback quotes before signing arrival evidence
When the event-specific book is unavailable or does not match, a venue-bound IOC uses this MarketView fallback without checking the snapshot timestamp. In the engine path inspected, AggregationEngine::cleanup_stale_data has no caller, so a disconnected venue's top-of-book can remain indefinitely and be recorded as the executable arrival quote while other events continue triggering orders. Validate the selected snapshot's freshness against the capture boundary before carrying it into signed fill evidence.
AGENTS.md reference: AGENTS.md:L65-L66
ℹ️ 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".
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Materialize verified Binance Spot/USD-M market tapes into the V3 outer materialization carrying CexReplaySnapshotV2: PIT instrument rules, account-bound maker/taker fees, independent funding and OI timelines, aggregate-trade flow, and signed runtime lifecycle latency evidence. Rebased onto origin/main be9e746, which already contains the signed arrival-cost evidence (#787), its verification (#788), fee-account binding (#789), and fee snapshot timers (#793). Refs #794.
* feat(research): publish verified CEX ResearchSnapshot V2 Materialize verified Binance Spot/USD-M market tapes into the V3 outer materialization carrying CexReplaySnapshotV2: PIT instrument rules, account-bound maker/taker fees, independent funding and OI timelines, aggregate-trade flow, and signed runtime lifecycle latency evidence. Rebased onto origin/main be9e746, which already contains the signed arrival-cost evidence (#787), its verification (#788), fee-account binding (#789), and fee snapshot timers (#793). Refs #794. * fix(research): close lob-pit-materializer review threads - Delegate runtime latency verification to the hardened alpha_domain::runtime_latency_evidence verifier instead of a drifted collector-local copy: event IDs are deduplicated before filtering (conflicting duplicates fail closed), every fill must carry an authenticated instrument_market_{market} identity, and USD-M fails closed until a derivatives execution path exists. - Republish verified fee and reference evidence triplets into the materialization artifact directory under content-addressed names so the digests recorded in the snapshot resolve to immutable bytes. - Restructure the test fixture per market: the happy path is Spot (signed LiveSmall latency evidence exists only for Spot), a new test proves USD-M materialization fails closed, and the aggregate-trade assertions now match the (previous, current] bucket semantics. * fix(research): anchor latency cohort and holding-interval funding - Publish the complete digest-anchored runtime feedback log and the trusted-key document into the artifact directory under their content digests, so independent readers can reverify every signed lifecycle event and no resolvable-evidence gap remains for the key document. - Charge USD-M funding settlements over each row's actual (current, future] holding interval, matching how the evaluator applies funding_bps to the held position; a settlement that precedes the holding interval is no longer charged and multi-bucket horizons collect every settlement. --------- Co-authored-by: Sonic Shih <sonic.shih@mandonothing.com>
Change contract
Emit signed, PIT-available arrival-price and instrument-market evidence for Market and IOC fills so research can measure real execution latency cost.
Issue relationship
Closes #795
Out of scope
Research verification, derivatives execution, fee-account binding, materialization, deployment activation, and order placement.
Dependencies and merge order
None. Merge before the separate research verifier.
Focused validation
cargo test -p hft-engine arrival_quote_binds_ioc_to_the_sequenced_venue_without_replacing_its_limit --lockedcargo test -p hft-engine arrival_price_requires_engine_quote_provenance --lockedcargo test -p hft-engine conflicting_duplicate_order_invalidates_prior_metadata --lockedRollout and rollback
Code-only runtime instrumentation; no service restart or trading activation in this PR. Revert the squash commit to remove the added event field and signed metrics.
Scope exception
None.