Repository navigation
fix(op-stack): use the withdrawal's L2 timestamp for super-root games - #5302
Conversation
Without l2Timestamp, getWithdrawalStatus, getTimeToProve and waitToProve matched super-root games (L2 timestamps) against the receipt's L2 block number, so every withdrawal looked provable at once. For super-root game types, take the timestamp from the receipt logs' blockTimestamp, or throw L2TimestampRequiredError when it is missing.
…mestamp check `getWithdrawalStatus` threw `L2TimestampRequiredError` before reading `finalizedWithdrawals`, so a finalized withdrawal on a super-root chain failed when no L2 timestamp was available. The timestamp lookup now only fails the game lookup. Also resets fake timers in `afterEach` in the `getTimeToProve` tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQrn3pHwhyRp35QAdpXpB1
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: d39198a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d582caf22b
ℹ️ 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".
| functionName: 'respectedGameType', | ||
| }) | ||
| if (!isSuperGameType(gameType)) return receipt.blockNumber |
There was a problem hiding this comment.
When l2Timestamp is omitted, this converts the receipt position to either a block number or timestamp only once before waitForNextGame begins its potentially long poll. If respectedGameType changes between legacy and super-root games while that poll is active, subsequent getGame calls read the new game type but keep comparing against the old unit, so the wait can either return a super-root game that predates the withdrawal or fail to match new legacy games indefinitely. The unit selection needs to follow the game type on each polling lookup rather than being frozen here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this as is. The failure needs respectedGameType to switch between a block-number and a super-root game type while one waitToProve call is polling. That only happens in a governance upgrade, a rare one-off event. Fixing it would mean reworking waitForNextGame/getGame to re-derive the unit on every poll, which is a larger change than this fix warrants. A caller can also pass l2Timestamp to pin the unit. Happy to follow up separately if maintainers want it.
Generated by Claude Code
- Order `l2Timestamp` after `l2OutputOracleAddress` on the action pages and document that its default only applies to super-root games. - Cover the `L2TimestampRequiredError` path in `getTimeToProve`. - Drop the faked clock from the `getTimeToProve` super-root test. - Separate execution from assertions in the new `getWithdrawalStatus` tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQrn3pHwhyRp35QAdpXpB1
…-root-l2-timestamp
Supersedes #5296 by @Anirudh-712. Their commit is kept as-is, with follow-up commits on top.
Problem
When the respected dispute game anchors on super roots (OP Mainnet, game type 9),
getGamesstores the game's L2 timestamp inl2BlockNumber. Whenl2Timestampisn't passed,getWithdrawalStatus,getTimeToProveandwaitToProvecompare those timestamps againstreceipt.blockNumber. A block number is always smaller than a timestamp, so every game appears to cover the withdrawal. As a result:getWithdrawalStatusreturnsready-to-provetoo early.getTimeToProvereturnsseconds: 0.waitToProvereturns a game whose L2 state predates the withdrawal.Change
From #5296:
l2Timestampisn't passed and the portal is v3+, the actions readrespectedGameType().blockTimestamp. If that is missing, they throwL2TimestampRequiredError.l2Timestampare unchanged.l2Timestampon the three action pages, plus a patch changeset.Follow-ups:
getWithdrawalStatus, the timestamp lookup now runs inside the settled batch. Before, it could throw beforefinalizedWithdrawalswas checked. A finalized withdrawal on a super-root chain therefore still returnsfinalizedin two cases where it previously threw:{ withdrawalHash, sender, l2BlockNumber }form, which has no receipt;blockTimestamp.getTimeToProveerror path.getTimeToProvesuper-root test no longer fakes the clock.l2Timestampis ordered alphabetically, and the docs say its default only applies to super-root games.Tests
All tests are fork tests at L1 block 26270300 and OP block 158668700.
maingetWithdrawalStatus0xdab8…05f6waiting-to-proveready-to-provegetWithdrawalStatus0xee9f…3e5d(covered by the latest game)ready-to-proveready-to-provegetWithdrawalStatusl2TimestampfinalizedfinalizedgetWithdrawalStatusblockTimestampL2TimestampRequiredErrorready-to-provegetTimeToProve0xdab8…05f6seconds > 0seconds: 0getTimeToProveblockTimestampL2TimestampRequiredErrorseconds: 0waitToProveblockTimestampL2TimestampRequiredErrorChecked via RPC:
respectedGameType()is9at L1 block 26270300, andfinalizedWithdrawals(0x1cd5…8e55)istruethere.tsc -p src, a typecheck of the three test files, andbiome checkare clean. I couldn't run the fork tests for the follow-up commits locally because anvil isn't available, so CI is their first run.mainis merged in, including #5298 (getTimeToFinalizewaits for the dispute game); it doesn't overlap with this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01VQrn3pHwhyRp35QAdpXpB1