Skip to content

fix(dia): require strictly-positive cross-epoch margin + clamp round timestamps - #292

Merged
hardyjosh merged 1 commit into
mainfrom
fix/dia-cross-epoch-skew
Aug 10, 2026
Merged

hardyjosh merged 1 commit into
mainfrom
fix/dia-cross-epoch-skew

Conversation

@hardyjosh

@hardyjosh hardyjosh commented Aug 10, 2026 •

Copy link
Copy Markdown
Collaborator

C1 (HIGH) — the cross-epoch invariant #280 called "airtight" at pauseTimeAfter == maxAge is defeated by DIA feed forward clock skew. Staleness ages a push by its own SOURCE timestamp while the pause is measured in block.timestamp; a feed running even 2s fast stamps a pre-split observation 2s after effectiveTime, which then passes staleness at pause-lift and pairs with the post-split ratio — serving 200e8 for a true 100e8 (verified repro). The margin pauseTimeAfter - maxAge is exactly the forward skew a config tolerates, and equality tolerates zero.

Fix: init now requires pauseTimeAfter > maxAge strictly (was >=), so a zero-margin config can't deploy, and the NatSpec states operators must size the margin above the feed's worst-case forward skew (prod's 1h margin is the reference) rather than claiming equality is airtight. testForwardSkewIsRejectedWithinMargin pins the exact tolerance boundary and the operator-owned residual beyond it.

Also on this file:

  • C3 (LOW) — latestRoundData returned updatedAt/startedAt == the raw push timestamp, which for an accepted future-dated push exceeds block.timestamp and underflow-reverts a Chainlink-style consumer. Now clamped to block.timestamp (fresh future push reports age 0); roundId still tracks the raw timestamp.
  • C2/C4/C5 (doc) — corrected NatSpec the code contradicted: roundId is a freshness token, not monotonic; a uint256-range overflow surfaces as FixedDecimalOverflow, not VaultSharePriceOverflow; a non-zero ERC-4626 decimals offset is mispriced, not merely un-pausable.

script/ untouched. Stacked on #291. 191 tests; fmt/slither/reuse/single-contract clean. The margin-enforcement approach (strict > + operator-sized margin, no baked-in skew constant) was your call.

🤖 Generated with Claude Code

hardyjosh commented Aug 10, 2026 •

Copy link
Copy Markdown
Collaborator Author

Merge activity

  • Aug 10, 4:33 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 10, 4:35 PM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 10, 4:36 PM UTC: @hardyjosh merged this pull request with Graphite.

@hardyjosh
hardyjosh changed the base branch from audit/mutation-coverage-2026-08-10 to graphite-base/292 August 10, 2026 16:33
hardyjosh added a commit that referenced this pull request Aug 10, 2026
…s) (#291)

Coverage half of the adversarial-mutation-test run (skill 0.30.0) against `main @ c371a23`. 4 group workers (Opus), 340 behaviours probed; the majority were already killed by existing tests. This commits only the gaps — 18 discriminating tests, each verified pass-on-clean / fail-under-mutation. `src/` and `script/` byte-identical to c371a23.

The adversarial half's findings land as code fixes stacked on top (starting [#292](https://app.graphite.com/github/pr/ST0x-Technology/st0x.oracle/292)).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@hardyjosh
hardyjosh changed the base branch from graphite-base/292 to main August 10, 2026 16:33
…timestamps

Adversarial-pass findings on DIAVaultOracle (mutation-test run @ c371a23).

C1 (HIGH) — the cross-epoch invariant #280 called 'airtight' at
pauseTimeAfter == maxAge is defeated by DIA feed forward clock skew. Staleness
is aged from the push's own SOURCE timestamp, but the pause is measured in
block.timestamp; a feed running even 2s fast stamps a pre-split observation
2s after effectiveTime, which then passes staleness at the pause-lift instant
and pairs with the post-split ratio — serving 200e8 for a true 100e8 (verified
repro). The margin pauseTimeAfter - maxAge is exactly the forward skew a config
tolerates, and equality tolerates zero. Fix: init now requires
pauseTimeAfter > maxAge STRICTLY (was >=), so a zero-margin config can no
longer deploy, and the NatSpec now states the margin must exceed the feed's
worst-case forward skew (operators size it; prod's 1h margin is the reference)
rather than claiming equality is airtight. New tests:
testInitRejectsPauseAfterEqualToMaxAge, testForwardSkewIsRejectedWithinMargin
(pins the exact tolerance boundary and the operator-owned residual beyond it);
existing equality-boundary tests and configs updated to a positive margin.

C3 (LOW) — latestRoundData returned updatedAt/startedAt == the raw push
timestamp, which for an accepted future-dated push exceeds block.timestamp and
underflow-reverts a Chainlink-style consumer computing block.timestamp -
updatedAt. Fix: clamp both to block.timestamp (a fresh future push reports age
0); roundId still tracks the raw timestamp. New test
testLatestRoundDataClampsFutureTimestamp.

C2/C4/C5 (LOW/doc) — corrected NatSpec that was contradicted by the code:
roundId is a freshness token, NOT monotonic (it regresses if DIA republishes a
lower source timestamp — diff for inequality, don't assert >); a uint256-range
overflow surfaces as LibDecimalFloat's FixedDecimalOverflow, not this
contract's VaultSharePriceOverflow; and a non-zero ERC-4626 decimals offset is
mispriced (not merely un-pausable), a second reason arbitrary vaults are
unsupported.

script/ untouched. 191 tests green; fmt/slither/reuse/single-contract clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hi8wq9YkjWPcGACXCEKeL
@hardyjosh
hardyjosh force-pushed the fix/dia-cross-epoch-skew branch 2 times, most recently from 8639bbd to b4cef4c Compare August 10, 2026 16:35
@hardyjosh
hardyjosh merged commit 06e9edd into main Aug 10, 2026
11 checks passed
hardyjosh added a commit that referenced this pull request Aug 10, 2026
…yer NatSpec (#293)

Deploy-path adversarial findings (mutation-test run @ c371a23). Stacked on [#292](https://app.graphite.com/github/pr/ST0x-Technology/st0x.oracle/292).

- **C1 (MED)** — `deploySignedPriceStack` guarded `ST0X_ADMIN`/`ST0X_ORACLE_ADMIN` against the hot deploy key but not `ST0X_SIGNER`, which controls every served price more directly (permissionless `updatePrice`). Adds the matching `require(signer != deployer)`, mutation-verified.
- **C2/C3 (doc)** — `iCentral` NatSpec no longer claims 'fixed for the beacon's whole life' (an owner beacon-upgrade can change it); the `Deployment` event `caller` docs now note minting is permissionless/front-runnable and direct monitoring to the deterministic proxy address.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
hardyjosh added a commit that referenced this pull request Aug 10, 2026
#296)

Doc consistency follow-up surfaced by the **post-fix mutation run** (@ `c7c22c3`), which found **no code defects and no coverage gaps** — every non-equivalent mutant is killed by the existing suite — but flagged four doc sites the fix PRs ([#292](https://app.graphite.com/github/pr/ST0x-Technology/st0x.oracle/292)–[#295](https://app.graphite.com/github/pr/ST0x-Technology/st0x.oracle/295)) left contradicting the hardened code:

- **README** cross-epoch section still documented `pauseTimeAfter >= maxAge` and called `== maxAge` 'airtight' — a config it calls valid now reverts at init. Rewritten to the strict `> maxAge` rule + forward-skew-margin rationale.
- **`maxAge` struct-field NatSpec** said 'MUST be `<= pauseTimeAfter`'; init enforces strict `<`. Corrected.
- **`iCentral` immutable comment** still said 'fixed for the beacon's whole life', contradicting the struct doc corrected in [#293](https://app.graphite.com/github/pr/ST0x-Technology/st0x.oracle/293).
- Two **test comments** with stale `>=` / 'airtight at equality' phrasing.

Doc/comment + README only — **zero code lines changed**. 192 tests; fmt/slither/reuse clean. Last doc loose end before the pre-audit evidence is regenerated over the final tip.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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