Add `--sender-nonce <INITIAL_NONCE>` flag to forge script for manual nonce management

[sam-goldman]

Component

Forge

Describe the feature you would like

Context

Forge scripts provide a convenient way to collect transaction data that can be executed through a multisig wallet like a Gnosis Safe. We collect transactions in this manner at Sphinx. Specifically, here's the (non-standard) way that we use Forge scripts:

1. The user defines their Forge script. In the script, the user's Gnosis Safe is impersonated, i.e. the sender of the user's transactions is the Safe.
2. We dry run the script to collect the user's transactions. It's possible to collect the transactions through the dry run file written to the broadcast folder, or by using the state diff cheatcode.
3. After the multisig owners have approved the transaction data, an EOA executes the transactions on-chain. Specifically, the EOA calls the Gnosis Safe, which executes the user's transactions. Crucially, the sender of all the collected transactions is the Gnosis Safe. (I.e. the msg.sender in the user's transactions is the Gnosis Safe, not the EOA that initiated the transaction).

If you're wondering why we execute transactions through a Gnosis Safe at all, it's to provide features like gas abstraction and secure deployments/upgrades.

Problem

Collecting transactions in this manner works well for contract deployments and standard transactions that are defined in the Forge script. However, if the user's multisig wallet hasn't been deployed yet, Foundry uses an incorrect sender nonce to deploy linked libraries, which causes the addresses of the libraries to be incorrect.

Specifically, here's why the issue happens:

1. Foundry fetches the nonce of the Gnosis Safe (i.e. the sender) when forge script is called. Since the Gnosis Safe hasn't been deployed yet, the initial nonce at its address is 0. This means the address of the first library deployed in the script will be calculated with a sender nonce of 0.
2. When the Gnosis Safe is deployed on-chain, its initial nonce is 1. (The EVM requires that all contracts have an initial nonce of 1 as of EIP-161). This means the address of the first library deployed on-chain will be calculated with a sender nonce of 1 instead of 0, resulting in a different library address. This is dangerous because the incorrect library address (i.e. the address calculated w/ a sender nonce of 0) is already injected into the initcode of any contract that depends on the linked library.

This limitation is preventing Sphinx from supporting linked libraries.

This issue doesn't occur if the user's multisig is already deployed because Foundry fetches the correct Gnosis Safe nonce, resulting in consistent library addresses.

Solutions

Preferred Solution

@RPate97 and I think the most straightforward solution is to add a new --contract-sender flag to forge script. This flag would simply set the initial nonce of the sender to 1 if its nonce on-chain is currently 0. This flag should also be available in forge test so that the user can get the same library addresses in their tests.

We acknowledge that this flag only makes sense in the context of collecting transactions to be executed later through a multisig wallet. This flag doesn't make sense when broadcasting transactions onto a live network via forge script because a smart contract can't sign a transaction.

We see this flag as a patch that satisfies our use case with minimal effort. In the future, it may make more sense to have some sort of forge collect command built specifically for this purpose.

Alternative Solution 1

The --contract-sender flag is for a rather specific use case. A more general solution could be a --sender-nonce <INITIAL_NONCE> flag, which could potentially be useful for EOAs too, if users need to manually specify an initial nonce for whatever reason. I'm not sure if that's a valid use case though, since presumably Foundry handles the sender's nonce internally.

Alternative Solution 2

An alternative solution is to calculate the correct library addresses outside of the Forge script, then deploy these libraries manually inside of the script. However, this would require us to re-implement Foundry's logic to resolve library dependencies, which is non-trivial and would lead to quite a bit of redundancy between our systems. It makes a lot more sense to leverage Foundry's existing logic instead of re-implementing it.

Alternative Solution 3

Another solution is to deploy the multisig in advance so that Foundry recognizes that its nonce is 1. This isn't an acceptable solution because the user should be able to fully simulate their script before any transactions are sent on-chain, which is one of Forge script's most compelling features.

[mattsse]

wdyt @klkvr ?

[klkvr]

I like the --sender-nonce <INITIAL_NONCE> approach more, and it should be straightforward to implement

This issue feels a little bit like an edge case, but in general it highlights the need for extending forge script with an API which would allow "preparing" EVM state for scripts/tests execution by executing arbitrary logic. E.g. increasing sender nonce in this case or pre-etching precompile mocks for L2s. Those state modifications should persist during both default and on-chain simulation.

[sam-goldman]

I like the --sender-nonce <INITIAL_NONCE> approach more, and it should be straightforward to implement

We're happy to implement this solution. Should we go ahead and make the PR, or should we wait for input from the other Foundry core devs? @mattsse @klkvr

[sam-goldman]

I like the --sender-nonce <INITIAL_NONCE> approach more, and it should be straightforward to implement

We're happy to implement this solution. Should we go ahead and make the PR, or should we wait for input from the other Foundry core devs? @mattsse @klkvr

Hey @mattsse @klkvr, just following up on this. Again, we're happy to implement this solution - just need confirmation on your side that it'll be accepted :)

[klkvr]

hey @sam-goldman feel free to create a PR implementing this!

however, we are performing a script refactoring currently in #7247, so it's probably better to wait until it will be merged (probably today)

[zerosnacks]

Hi @sam-goldman, https://github.com/foundry-rs/foundry/pull/7247 has been merged. Would you still be interested in implementing this?

[sam-goldman]

Hi @sam-goldman, #7247 has been merged. Would you still be interested in implementing this?

Hey @zerosnacks, it's no longer relevant to us, so I'm not going to be implementing it

[zerosnacks]

No problem, I've updated the title to reflect the feedback and will leave this ticket open as it seems like a useful feature to me