feat: simulation when broadcast not specified#30
Conversation
|
Hi there, Thanks for your submission. I will review this until the end of the week and we'll get this merge soon |
|
@codex review |
|
@chasebrownn I've added a few comments and asked codex to review can you double check? otherwise, looks good |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df69f210ce
ℹ️ 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".
Adds simulation support to safe-utils, allowing Safe transactions to be tested against a local fork without hardware wallet signing or Safe API interaction. Safe.sol additions: - isBroadcastMode() / isSimulationMode(): detect --broadcast flag via vm.isContext, with SAFE_BROADCAST env var override - simulateTransaction(): executes a pre-built tx via vm.prank + execTransaction - simulateTransactionNoSign() / simulateTransactionsNoSign(): no-HW-wallet simulation using vm.store to mark approvedHashes[sender][txHash] = 1 - simulateTransactionMultiSigNoSign() / simulateTransactionsMultiSigNoSign(): multi-sig simulation; signers are sorted ascending as Safe requires - proposeTransactionWithSignature(..., uint256 nonce): additive overload for sequential proposals in one script run (on-chain nonce not yet advanced) - proposeTransactionsWithSignature(..., uint256 nonce): same for batch SafeScriptBase.sol (new file): - Abstract base for forge scripts; auto-detects mode and routes to simulate or propose. Tracks currentNonce across sequential transactions. - _proposeTransaction / _proposeTransactions / _proposeTransactionWithVerification - Single-sig and multi-sig init via SIGNER_ADDRESS / SIGNER_ADDRESS_0..N
Move SAFE_BROADCAST env-var check to execute before vm.isContext so it can override detection in all contexts (not just catch paths). Add test/SafeSimulation.t.sol with 11 tests covering: mode detection, single-signer simulation, batch simulation, multi-sig simulation, and explicit-nonce propose overloads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SimulationFailed was declared but never used — simulate functions return bool by design so callers decide how to handle failures. Add README docs for simulateTransactionNoSign, multi-sig variants, SafeScriptBase, mode detection helpers, and the SAFE_BROADCAST env var. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
df69f21 to
98fc29c
Compare
|
@aviggiano All great findings. Everything should be addressed now. Please let me know if you need anything else. |
Simulation support + SafeScriptBase
Motivation
At Telcoin Association we do a lot of on-chain executions via governance scripts and find the
safe-utilsvery helpful. One thing we noticed is the lack of support around the traditional workflow of simulation when--broadcastis not specified. This PR adds simulation support so you can catch reverts, verify deployment addresses, and confirm state changes without touching a hardware wallet or proposing to the multisig.What's added
Safe.sol— simulation functionsisSimulationMode()/isBroadcastMode()— detects whether the script is running with--broadcast. Falls back to theSAFE_BROADCASTenv var for environments wherevm.isContextis unavailable (e.g. test suites).simulateTransactionNoSign(target, data, signer)— simulates a singleCalltransaction. Writes directly to the Safe'sapprovedHashesstorage slot viavm.storeso no real signature is needed.simulateTransactionsNoSign(targets, datas, signer)— batch variant via MultiSend.simulateTransactionMultiSigNoSign(target, data, signers[])— multi-sig variant; sorts signers ascending as required by Safe'scheckNSignatures.simulateTransactionsMultiSigNoSign(targets, datas, signers[])— batch multi-sig variant.bool(never throw) so callers decide how to handle failures.proposeTransactionWithSignature(..., nonce)andproposeTransactionsWithSignature(..., nonce)— explicit-nonce overloads for scripts that propose multiple transactions in one run (the Safe's on-chain nonce only advances on execution, so sequential proposes need a manually incremented nonce).SafeScriptBase.sol— new fileAbstract Foundry script base that auto-routes to simulate or propose based on mode detection. Eliminates boilerplate for the common pattern of "dry-run first, broadcast second":
_initializeSafe()/_initializeSafeMultiSig()— reads addresses and derivation path from env._proposeTransaction()/_proposeTransactions()— simulate or propose, incrementcurrentNonce._proposeTransactionWithVerification()— adds a post-simulation code-length check for deployment transactions.test/SafeSimulation.t.sol— new test file11 tests against a real Base mainnet fork using a Safe whose owners are the default Foundry accounts (no private keys or storage hacks needed):