Repository navigation
fix(script): read the Safe nonce from the real chain state after simulating - #60
Merged
MalteHerrmann merged 2 commits intoOct 5, 2026
Conversation
|
|
Changes to gas cost
🧾 Summary (20% most significant diffs)
Full diff report 👇
|
LCOV of commit
|
…lating `_simulateBatch` executes the batch for real on the local fork. That advances the Safe nonce by one. `SafeNonce.next` then read the advanced nonce. Every script that simulates before proposing queried the service one nonce too high and proposed one slot past the free one. Wrap the simulation in a state snapshot so the proposal sees the chain as it is. Also: - Split the response parsing out of the HTTP call as `SafeNonce.fromResponse`. Fixture pages now cover the status check and the JSON paths. - Each propose helper initializes the Safe client itself. `SafeNonce.next` no longer does it as a side effect. - Add explicit-nonce overloads to the timelock propose helpers. They mirror the batch helper and bypass the transaction service. - Drop the unreachable "pending below on-chain" guard. The query already filters on `nonce__gte`. - Log the chosen nonce the same way on every propose path.
PierrickGT
force-pushed
the
fix/safe-nonce-after-simulation
branch
from
October 5, 2026 17:11
3a86c47 to
9183901
Compare
…imulation * `MultiSigBatchBase._simulateBatch` uses `vm.revertToStateAndDelete` and requires its result. A failed restore now reverts, and the snapshot is deleted. * `TimelockBatchBase._simulateBatch` restores the chain state after the simulation. The batch's effects, e.g. a new minimum delay, do not leak into the proposal.
MalteHerrmann
merged commit Oct 5, 2026
ec34fe3
into
bb/check-this-handoff-document-and-implement-the-ch-thr_mm3mdzctsw
2 checks passed
MalteHerrmann
added a commit
that referenced
this pull request
Oct 7, 2026
## What - Add the `SafeNonce` library (`script/SafeNonce.sol`). `next` reads the pending proposals from the Safe tx service and returns one above the highest pending nonce, or the on-chain nonce if none is pending. It reverts if the service does not answer with a 2xx status. The parsing is a pure `fromResponse`, covered by fixture pages and a fuzz test. - `MultiSigBatchBase._proposeBatch(safe, sender)` and the `SafeTimelockBatchBase` propose helpers now propose at this nonce. Each has an explicit-nonce overload, for when the service cannot be queried. - `_simulateBatch` in `MultiSigBatchBase` and `TimelockBatchBase` now restores the chain state after the simulation. The simulation executes the batch for real on the local fork, so the Safe nonce read afterwards was one too high: with no pending proposals, the proposal landed one slot past the free nonce. A mock-Safe test fails without the restore. - Move `lib/safe-utils` from upstream `Recon-Fuzz/safe-utils` to the `m0-platform/safe-utils` fork (`a2cc7c2`). The fork adds tx-service URLs for more chains. ## Why The Safe on-chain nonce advances only on execution. Thus, a new proposal can use the same nonce as a pending proposal, and only one of the two can execute. This moves the fix from `evm-m-suite-deployment` to `common`, so that all repos that propose Safe transactions get it (INT-437). Includes #60.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Stacked on #59. The simulation in
MultiSigBatchBase._simulateBatchrunsexecTransactionfor real on the local fork. By the timeSafeNonce.nextreads the Safe's local nonce, it is one higher than on chain. Every script in evm-m-extensions simulates before it proposes, so the service was queried withnonce__gteone too high and the proposal landed one slot past the free nonce. With a pending proposal at the real nonce, the query also hid it, which is the collision #59 sets out to prevent. The simulation is now wrapped invm.snapshotStateandvm.revertToState. A newMultiSigBatchBasetest with a mock Safe fails without the restore and passes with it. The service parsing is split into a purefromResponseand covered by fixture pages for the no-pending, pending and non-2xx cases. All 8 suites pass locally on Foundry 1.8.3. The snapshot path has not been exercised in a liveforge scriptdry run yet.Left for a follow-up: moving
nextinto the safe-utils fork so the nonce-less safe-utils entry points get the same behaviour, and pinning the fork to a tag instead ofmain.What changed
_simulateBatchrestores the chain state after the simulation, so the Safe nonce, the owners' dealt balances and the batch's effects do not leak into the proposal.SafeNonce.nexttakes an initialized client and no longer initializes it; each propose helper initializes the client itself.SafeNonce.fromResponseholds the 2xx check and the JSON parsing, and is tested through a harness.nextFromand its unreachable below-on-chain branch are gone._proposeScheduleBatchand_proposeCancelgain explicit-nonce overloads, and the nonce NatSpec points to them as the bypass when the service is down.[nonce] proposing at nonceonce.