fix(safe): refuse execution of future-nonce proposals by default (EXSC-690)

[0xDEnYO]

Which Linear task belongs to this PR?

EXSC-690

Why did I implement it this way?

confirm-safe-tx.ts printed a GS026 warning for a proposal whose nonce is ahead of the Safe's
on-chain nonce and then still offered Proceed anyway? (will revert with GS026). Taking that
option broadcasts a transaction that cannot succeed — gas is spent on a guaranteed revert and the
resulting failure is confusing to read back. Five lines above, the stale-nonce branch (the same
class of guaranteed revert) already hard-refuses with no override, so the two branches disagreed
about the same problem.

Consistency with the stale-nonce branch. The future-nonce execute path now refuses by default,
matching the stale branch. The stale branch itself is unchanged in behaviour.

Escape hatch for the stale-RPC case. The one legitimate reason to broadcast anyway is a lagging
RPC under-reporting the Safe's on-chain nonce, which makes an already-executable proposal look like
a future one. ALLOW_FUTURE_NONCE_EXECUTION=true re-enables execution for that case only; it does
not unlock the stale branch, because a stale reading would require an RPC reporting a nonce ahead
of consensus. The flag follows the existing DRAIN_PARKED_TASKS / isDrainEnabled() convention —
a small predicate helper (isFutureNonceExecutionAllowed()), not an inline process.env read — and
is documented in .env.example.

Predicate placed in safe-utils for testability. confirm-safe-tx.ts has no test file and is not
unit-testable as written (interactive prompts, MongoDB and RPC side effects). The decision is
therefore a pure exported function in safe-utils.ts, which has a colocated
safe-utils.test.ts:

canExecuteWithNonceStatus(status, { allowFutureNonce })
  // -> { canExecute: true;  reason: 'nonce-current' | 'future-nonce-override' }
  // -> { canExecute: false; reason: 'stale-nonce'   | 'future-nonce' }

The console UX (messages, colours, the blocking-proposal lookup) stays in confirm-safe-tx.ts,
which just switches on reason. safe-utils.ts gains no import from safe-decode-utils.ts
(rule 201 unaffected).

Signing is still permitted — only execution is refused. The action list is built before the
gate and always offers Sign for an unsigned proposal, independent of nonce status. Only the four
execute actions consult the predicate, so signatures can keep accumulating on a future-nonce
proposal while the blocking one is still pending. The refusal message says so explicitly and points
at the override flag.

Governance impact (rule 105): none. This does not change any Safe threshold, timelock delay,
role, owner set, or transaction authorization path. It only prevents broadcasting a transaction
that would revert on-chain; which transactions are authorized, and by whom, is untouched.

Verification (worktree, origin/main @ 358c2b9):

• bun test script/deploy/safe/safe-utils.test.ts -> 42 pass, 0 fail (8 new)
• bunx eslint on the three changed .ts files -> exit 0
• bunx tsc-files --noEmit on the three changed .ts files -> exit 0

Checklist before requesting a review

• I have performed a self-review of my code
• This pull request is as small as possible and only tackles one problem
• I have added tests that cover the functionality / test the bug
• For new facets: I have checked all points from this list: https://www.notion.so/lifi/New-Facet-Contract-Checklist-157f0ff14ac78095a2b8f999d655622e
• I have updated any required documentation

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

• I have checked that any arbitrary calls to external contracts are validated and or restricted
• I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
• I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

[coderabbitai]

Warning

Review limit reached

@0xDEnYO, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 27 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f8ee9e8d-feec-46a2-8b4d-153bf4ad7e61

📥 Commits

Reviewing files that changed from the base of the PR and between ac64042 and 8f582da.

📒 Files selected for processing (2)

• script/deploy/safe/confirm-safe-tx.ts
• script/deploy/safe/safe-utils.test.ts

Walkthrough

Changes

Safe nonce execution gating

Layer / File(s)	Summary
Nonce decision contract and coverage .env.example, script/deploy/safe/safe-utils.ts, script/deploy/safe/safe-utils.test.ts	Defines nonce execution decisions, adds the future-nonce environment override, and tests stale, current, future, and environment-flag behavior.
Confirmation and execution flow script/deploy/safe/confirm-safe-tx.ts	Uses nonce decisions for execute actions, blocks refused broadcasts, updates GS026 messaging, and preserves signing instructions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: refusing future-nonce Safe executions by default.
Description check	✅ Passed	The description covers the Linear task, rationale, implementation approach, tests, and documentation updates.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/exsc-690-refuse-future-nonce-execution

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

script/deploy/safe/confirm-safe-tx.ts (1)

663-714: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not report a guaranteed revert when the override permits execution.

future-nonce-override enters this branch with canExecute: true, yet the output still says execution “WILL REVERT” and recommends recreating a blocking proposal. Split the refusal diagnostics from the override path; the latter should state that the configured RPC reports a nonce gap and execution is proceeding by explicit override.

🤖 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 `@script/deploy/safe/confirm-safe-tx.ts` around lines 663 - 714, Update the
nonceDecision handling around the `GS026` warnings so the
guaranteed-revert/refusal diagnostics are emitted only when
`nonceDecision.canExecute` is false. For the `canExecute` override path, report
that the configured RPC indicates a nonce gap and execution is proceeding
because `ALLOW_FUTURE_NONCE_EXECUTION=true`, without recommending a blocking
proposal or claiming the transaction will revert.

🧹 Nitpick comments (1)

script/deploy/safe/safe-utils.test.ts (1)

551-584: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Parameterize the nonce-decision matrix.

These cases repeat the same invocation/assertion shape. Use a table-driven test so each status/override/result is one row and the test follows Arrange-Act-Assert consistently.

As per coding guidelines, “Use parameterized tests to reduce code duplication when testing multiple scenarios” and “Follow Arrange-Act-Assert (AAA) pattern.”

🤖 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 `@script/deploy/safe/safe-utils.test.ts` around lines 551 - 584, Refactor the
canExecuteWithNonceStatus test suite into one table-driven test containing a row
for each status, allowFutureNonce setting, and expected result. For each row,
arrange the inputs, invoke canExecuteWithNonceStatus, then assert the result,
preserving all existing stale, future, override, and current-nonce outcomes.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@script/deploy/safe/confirm-safe-tx.ts`:
- Around line 663-714: Update the nonceDecision handling around the `GS026`
warnings so the guaranteed-revert/refusal diagnostics are emitted only when
`nonceDecision.canExecute` is false. For the `canExecute` override path, report
that the configured RPC indicates a nonce gap and execution is proceeding
because `ALLOW_FUTURE_NONCE_EXECUTION=true`, without recommending a blocking
proposal or claiming the transaction will revert.

---

Nitpick comments:
In `@script/deploy/safe/safe-utils.test.ts`:
- Around line 551-584: Refactor the canExecuteWithNonceStatus test suite into
one table-driven test containing a row for each status, allowFutureNonce
setting, and expected result. For each row, arrange the inputs, invoke
canExecuteWithNonceStatus, then assert the result, preserving all existing
stale, future, override, and current-nonce outcomes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 644e71eb-3468-4984-83cf-c9ed73000c04

📥 Commits

Reviewing files that changed from the base of the PR and between 358c2b9 and ac64042.

📒 Files selected for processing (4)

• .env.example
• script/deploy/safe/confirm-safe-tx.ts
• script/deploy/safe/safe-utils.test.ts
• script/deploy/safe/safe-utils.ts