Skip to content

fix: support millisecond quote stamps in simulations - #4880

Open
6heLL66 wants to merge 1 commit into
cowprotocol:mainfrom
6heLL66:fix/metric-oracle-restamping
Open

fix: support millisecond quote stamps in simulations#4880
6heLL66 wants to merge 1 commit into
cowprotocol:mainfrom
6heLL66:fix/metric-oracle-restamping

Conversation

@6heLL66

@6heLL66 6heLL66 commented Sep 9, 2026

Copy link
Copy Markdown

Description

Titan quotes can contain uint48 millisecond timestamps at bytes 25..31, as in the captured Metric quote below. The stream currently recognizes only leading uint32 seconds, so these quotes retain the builder's future timestamp and can revert when simulated at the parent block.

Recognize the millisecond layout by an exact match to its frame's projected timestamp, independently of oracle or venue addresses. Each format selects its newest timestamp separately: a newer millisecond quote cannot change which seconds quotes are restamped. The original seconds recognition, maximum selection and restamping body are preserved.

Track millisecond eligibility per winning account, slot and storage field, so overwritten or unrecognized frames cannot borrow freshness. Seconds-format matches retain priority, including dual-format words that later become stale. Millisecond timestamps are never moved forward; all other bytes and account fields are preserved. Both state and stateDiff are supported, with no public API or configuration changes.

For the captured quote targeting block 25,925,280, simulating at its parent changes 01a07bb2de38 (1788781715000 ms) to 01a07bb2af58 (1788781703000 ms). The remaining 26 bytes stay identical.

Follow-up to #4691.

Validation

No new tests are included in the final diff. Existing test fixtures and calls only receive the metadata/signature adjustments required by the implementation.

  • Linux CI passed using production source identical to PR commit 1cb2f81e3, with four additional regressions kept in the validation branch: 23 simulator tests passed, 8 external-service/replay tests skipped. Clippy passed for all simulator features and targets with -D warnings (existing unreachable-Alloy-path configuration warnings remain).
  • Regression checks fail before the respective fixes: missing millisecond support on the original implementation, and the clock-isolation case on the preceding shared-clock version. Both pass with the final code.
  • Local checks passed 60,000 legacy-only comparisons, 46,080 mixed-format comparisons, and 144,000 outputs across 24,000 successive venue replacements. The mixed-input checks compare the original seconds selector and protected words against the exact base implementation on identical input histories, plus an independent reference for separate clocks and winning-word provenance. They also cover both clock orderings, millisecond-only snapshots, stale dual-format words, and integer boundaries. These checks use std-only substitutes for Alloy containers and remain outside the PR.
  • Nightly formatting and independent correctness and standards reviews passed. The original seconds selection and restamping body were also compared byte-for-byte with the base.

The skipped external-service/replay tests remain unverified; the in-process websocket/reconnect test passed in CI.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@6heLL66 6heLL66 changed the title fix: restamp Metric oracle quotes for simulation fix: support millisecond quote stamps in simulations Sep 9, 2026
@6heLL66
6heLL66 force-pushed the fix/metric-oracle-restamping branch 2 times, most recently from 79395d4 to eb452eb Compare September 9, 2026 12:09
@6heLL66
6heLL66 force-pushed the fix/metric-oracle-restamping branch from eb452eb to 1cb2f81 Compare September 9, 2026 13:15
@6heLL66
6heLL66 marked this pull request as ready for review September 9, 2026 14:13
@6heLL66
6heLL66 requested a review from a team as a code owner September 9, 2026 14:13
@6heLL66

6heLL66 commented Sep 9, 2026

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 9, 2026

@fleupold fleupold left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for your contribution!

Can you please rework the PR description to give a bit more background knowledge for people with less context on the matter (please don't AI generate).

Is the stamp format documented somewhere and expected to be stable or will other issues like this come up in the future?

Also, I feel a concrete example + test of the current limitation and proof of the fix is missing to give us confidence in accepting this change.

/// Newest stamp any venue quoted for. Only words carrying it belong to a
/// lane a maker is quoting for `block_number`.
stamp: Option<u32>,
millisecond_stamps: MillisecondStamps,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need a new type here? Can we not capture stamp in the existing type (but use the Metric specific encoding for detection)?

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.

2 participants