fix(FET-3203): don't surface transient CommitmentTooNew as a revert

[v1rtl]

Fixes FET-3203.

Root cause

The error data from the ticket decodes cleanly:

$ cast error-decode 0x74480cc9ae290bba...69d61c9f...69d61c93
CommitmentTooNew(bytes32,uint256,uint256)
0xae290bbaa3282c9bf6ccbc240c630120d38c5edda4089fe86e3afc462b7a6a06
1775639711   # minimumCommitmentTimestamp
1775639699   # currentTimestamp  <- 12s behind, exactly one slot

The 60s gate on the register step is wall-clock based: commitTx.finaliseTime is the commit block header timestamp (transactionStore.ts -> reducer.ts -> Transactions.tsx), and thorin's CountdownCircle fires setCommitComplete(true) at commitBlockTs + 60 per Date.now(). The pre-flight in the modal (eth_createAccessList + estimateGas) then executes against a block header that can be up to one slot behind wall clock, so block.timestamp < commitment + minCommitmentAge and the controller reverts.

The commitment is valid - the chain just hasn't caught up yet. Click the instant the circle hits zero and you hit it; hesitate a few seconds and you don't, which is why it reproduces intermittently.

Three separate things made this user-visible:

1. getReadableError threw the detail away. It short-circuited on EstimateGasExecutionError and returned null for anything that wasn't "insufficient funds", never reaching the decodeErrorResult branch below it - so every custom ENS contract revert collapsed into viem's generic "execution reverted".
2. Nothing retried. createTransactionRequestQueryFn swallows failures into { data: null, error }, which react-query treats as success - so no retry, and the failure was persisted to IndexedDB.
3. The modal treated it as terminal. The confirm button is hard-disabled on !!requestError, so the only escape was refetchOnMount: 'always' - i.e. literally closing and reopening the modal, exactly as the ticket describes.

Changes

src/utils/errors.ts - decode revert data before the EstimateGasExecutionError / RpcRequestError branches. This matters because eth_createAccessList uses a raw client.request, so its revert arrives as an RpcRequestError whose details used to win over the decodable data. Adds decodeContractError + isCommitmentTooNewError.

Also fixes a latent crash: decodeErrorResult throws on an unrecognised selector rather than returning nullish, so the old if (!decodedError) return null guard was dead code and getReadableError could throw on any unknown revert. Now guarded.

stage/query.ts - rethrow CommitmentTooNew only, so it becomes a real react-query error. All other errors keep the existing swallow-into-data.error behaviour.

stage/TransactionStageModal.tsx - retry that error 8x at 3s. A normal slot is 12s but a missed slot pushes the next block to 24s, so this covers the worst case; useInvalidateOnBlock remains the primary recovery path and this is cover for a slow block watcher. isWaitingForCommitment reads both failureReason (set while retries are in flight) and the settled error so the state is stable for the whole window. While waiting: disabled button + spinner reading "Waiting for confirmation", confirm-stage copy explaining we're waiting for on-chain confirmation, and the red Helper suppressed. If it genuinely never resolves it falls through to the normal error path - now naming CommitmentTooNew rather than "execution reverted".

public/locales/en/common.json - two new keys. fallbackLng: 'en' so the other 7 locales inherit them.

Testing

4 regression tests in src/utils/errors.test.ts using the real payload from the ticket: detection through RpcRequestError (the createAccessList path) and EstimateGasExecutionError (the estimateGas path), an insufficient-funds guard for the reorder, and the unknown-selector no-throw case.

pnpm vitest - 47 existing TransactionDialogManager tests + 4 new pass. pnpm lint clean. pnpm lint:types reports 5 errors, all pre-existing in metadataCache.test.ts / routes.test.ts, neither touched here.

Notes for the reviewer

• The race still exists; this makes it invisible. The countdown is deliberately untouched - it still fires on wall clock, so the pre-flight will keep reverting for up to a slot and the modal now absorbs it. Gating the button on chain time would mean it stays disabled for up to 12s after the circle visibly hits zero, which is worse UX for a condition that resolves itself.
• The ticket reports this only happens with "use as primary name" enabled. I could not find a code path that makes it conditional on reverseRecord - nothing branches on it in registerName.ts, query.ts or the estimation hooks, and the controller checks CommitmentTooNew before it touches records. Most likely a timing correlation (the extra profile step shifts when the commit lands relative to slot boundaries). The fix is correct either way, but flagging that I haven't fully explained that detail.
• Separate latent bug, not fixed here: finaliseTime is milliseconds at transactionStore.ts:446 but seconds at transactionStore.ts:126 (the Etherscan-recovery path used for replaced/sped-up txs). If a commit is recovered that way, commitTimestamp + 60000 < Date.now() is instantly true, the countdown completes immediately, and CommitmentTooNew is guaranteed with a delta far larger than the 24s retry window here would cover. Worth its own ticket.

[cloudflare-workers-and-pages]

Deploying ens-app-v3 with    Cloudflare Pages

Latest commit:	f3adf23
Status:	✅  Deploy successful!
Preview URL:	https://6cb53c08.ens-app-v3.pages.dev
Branch Preview URL:	https://feature-fet-3203-execution-r.ens-app-v3.pages.dev

View logs

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[v1rtl]

Reviewed with a focus on the error-handling changes. Two things I'd want addressed before merge — both verified against the real viem / react-query paths rather than by reading.

1. getViemRevertErrorData throws on data: null — now on two paths

src/utils/errors.ts:26:

return typeof error.data === 'object' ? error.data.data : error.data

typeof null === 'object', so error.data === null evaluates null.data → TypeError. The call sits outside the try in decodeContractError, so the new guard doesn't cover it.

There's no normalization between the node and this line: viem's http transport throws new RpcRequestError({ body, error, url }) with error being the raw JSON-RPC error member (clients/transports/http.js:51), RpcRequestError sets cause: error (errors/request.js), and walk() stops on that raw object. Whatever the node sends as "data" arrives here untouched.

Driving the real http transport with { code: -32000, message: 'execution reverted', data: null }:

path	error class	pre-PR	post-PR
createAccessList (raw client.request)	InvalidInputRpcError	already threw	throws
estimateGas (viem action)	EstimateGasExecutionError	returned null, safe	throws

The estimateGas row is a regression from the reorder — the old code short-circuited before reaching getViemRevertErrorData. The createAccessList row is pre-existing (InvalidInputRpcError is not instanceof RpcRequestError, so it never matched any of the three guards), but this PR is the natural place to close it.

Impact is worse than a bad message, because isCommitmentTooNewError is now called in three places that can't absorb a throw:

• TransactionStageModal.tsx:451-453 — computed in the component body, so it throws during render
• TransactionStageModal.tsx:434 — inside the retry callback, i.e. inside the retryer's .catch()
• query.ts:277 — inside catch (e), so the TypeError replaces the real error

One-line fix: error.data && typeof error.data === 'object' ? … : …, or move getViemRevertErrorData inside the try. Worth a test next to the unknown-selector one.

Related: the description's mechanism for this bullet doesn't match what I measured

its revert arrives as an RpcRequestError whose details used to win over the decodable data

It arrives as InvalidInputRpcError, which isn't instanceof RpcRequestError, so details never won — the pre-PR code already decoded that path correctly:

[createAccessList + valid data]  OLD getReadableError -> {"message":"CommitmentTooNew","type":"contract"}
[estimateGas + valid data]       OLD getReadableError -> null
                                 OLD fallback shortMessage: "Execution reverted for an unknown reason."

The ticket's message came from estimateGas, via the EstimateGasExecutionError short-circuit — which is exactly what the reorder fixes. So the change is right, just described wrong. The new test constructs a bare RpcRequestError under a comment saying it's the createAccessList shape; it isn't, so that test passes without covering the shape production actually produces.

Worth checking separately: createAccessList runs first in calculateGasLimit and the old code decoded its errors fine, so for "execution reverted" to have reached users it can't have thrown. Geth reports eth_createAccessList reverts as an error field in the result object, not as a JSON-RPC error — and AccessListResponse in createAccessList.ts has no error field, so that signal is dropped. I haven't tested against a real node, but it fits the evidence.

2. It never falls back to the error path

If it genuinely never resolves it falls through to the normal error path

It doesn't. query-core retains fetchFailureReason when the retry budget is exhausted (query.js:349 — the error reducer copies fetchFailureReason: error alongside status: 'error'). So isWaitingForCommitment stays true, the button stays a disabled spinner, and TransactionStageModal.tsx:673's && !isWaitingForCommitment keeps the Helper suppressed.

QueryObserver with this PR's exact retry config:

total queryFn attempts: 18
after retries exhausted    status=error   fetchStatus=idle     failureCount=9 isWaitingForCommitment=true
immediately after block    status=pending fetchStatus=fetching failureCount=1 isWaitingForCommitment=true
after 2nd round settles    status=error   fetchStatus=idle     failureCount=9 isWaitingForCommitment=true

The budget itself is right (9 attempts, 8×3s ≈ two slots). But useInvalidateOnBlock buys a fresh budget every block, and the flag reads true at every point across both rounds — so the error is suppressed continuously and CommitmentTooNew never reaches the screen. Rendering the modal with createAccessList rejecting on the ticket's payload agrees: 9 attempts, button still waitingForBlock, the string absent from the DOM.

The only escape is closing the dialog — the same escape hatch the PR set out to remove.

Reachable in practice via the finaliseTime bug the description already flags:

• transactionStore.ts:126 (etherscanDataToMinedData) → parseInt(timeStamp, 10), seconds — used at 198 / 226 / 249
• transactionStore.ts:446 → Number(timestamp) * 1000, ms
• both → useCallbackOnTransaction → TransactionFlowProvider.tsx:131 → setTransactionStageFromUpdate → reducer.ts:179 finaliseTime = minedData?.timestamp
• Transactions.tsx:270 → commitTimestamp + 60000 < Date.now(), ms semantics

On the Etherscan path that's ~1.7e9 against ~1.7e12, so commitComplete starts true, the pre-flight fires immediately, and the delta runs to ~60s — past the 24s budget. That's the case where this lands as a permanent spinner with no text at all, which is worse than what's on main today.

Suggested direction: derive the waiting state from something that clears — e.g. gate on transactionRequestQuery.status !== 'error' rather than the retained failureReason — or latch an explicit "gave up" flag. Also worth gating isWaitingForCommitment on stage === 'confirm': as written, a live failureReason in stage === 'failed' suppresses a genuine transactionError too, leaving a disabled "Try again" with no explanation.

Smaller

• retry: (failureCount, error) => isCommitmentTooNewError(error) && failureCount < 8 returns false for the two guard throws in the query fn (connectorClient is required / address does not match connector), which previously got react-query's default 3 retries. Both are guarded by enabled so likely inert, but it's an unannounced change.
• No tests for the two riskiest changes: the rethrow in stage/query.ts and the modal's waiting/retry state. stage/query.test.ts and TransactionStageModal.test.tsx both already exist.
• Decoding first is a genuine readability win — execution reverted → CommitmentTooNew on both paths — and I checked the merged ABI for selector collisions (27 error entries, 27 unique selectors, no duplicate names). Only nit: TransactionStageModal.tsx:664 renders {t(attemptedTransactionError.message)} while :674 renders it raw, so one site is wired for translated copy and the other isn't. Follow-up at most.

Confirmed green locally: the 47 existing TransactionDialogManager tests plus the 4 new ones pass, and tsc --noEmit reports nothing in the touched files.

---

Generated by Claude Code