foundry icon indicating copy to clipboard operation
foundry copied to clipboard

script regressions

Open sakulstra opened this issue 9 months ago • 5 comments

Component

Forge

Have you ensured that all of these are up to date?

  • [X] Foundry
  • [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (cafc260 2024-05-02T00:22:11.188280000Z)

What command(s) is the bug in?

forge script

Operating System

macOS (Apple Silicon)

Describe the bug

There seems to be a few regressions on scripts introduced in the last few weeks.

  1. when running a script it will no longer properly show the number of txns
==========================

Chain 1

Estimated gas price: 28.900907541 gwei

Estimated total gas used for script: 11250722

Estimated amount required: 0.325156076291494602 ETH

==========================
##
Sending transactions [0 - 0]. // in earlier versions this would have shown 0-4 (given there are 5txns in that script)
  1. when running a script with --resume it will no longer show any gas information, but immediately start with:
##
Sending transactions [0 - 0].

omitting info like estimated amount required.

  1. when running a script with --resume after doing some txns the script will fail as the pre-calculated nonce changed.
Context:
- EOA nonce changed unexpectedly while sending transactions. Expected 107 got 108 from provider.

Not sure if that ever was not an issue, but I guess it should be solved anyways as it's quite common (run a script, gas spikes and a txn is not executed, swap some assets, try to resume). The issue appears to be that the script will try to use the hardcoded nonce from run-latest. I guess the "proper" way would be the recalculate the execution path based on txns that already have a hash.

sakulstra avatar May 02 '24 12:05 sakulstra

when running a script it will no longer properly show the number of txns

Are you running the script with --slow flag when this occurs?

klkvr avatar May 02 '24 12:05 klkvr

@klkvr yes, forge script scripts/Deploy.s.sol:Deploy --rpc-url mainnet --broadcast --ledger --mnemonic-indexes 1 --sender <redacted> --verify -vvvv --slow was the full command

sakulstra avatar May 02 '24 12:05 sakulstra

Not sure if that ever was not an issue, but I guess it should be solved anyways as it's quite common (run a script, gas spikes and a txn is not executed, swap some assets, try to resume).

So the issue occurs only when you firstly run scirpt, then do some transactions from script sender which change sender nonce, and then resume script? Or it is the case even when script is just resumed immidiately without sender nonce being changed?

klkvr avatar May 02 '24 13:05 klkvr

@klkvr only when resuming after doing some txns elsewhere(at least didn't face it before).

sakulstra avatar May 02 '24 13:05 sakulstra

I don't think this is something we should/can do.

Currently --resume assumes that nonce did not change, thus it does not re-execute script and just sends transactions which were not sent before.

If we'd account for nonce change after --resume, this would require us to at least partialy re-execute script as new nonce might result in other contract addresses -> other transactions. Such re-execution is not really trivial as we'd probably have to firstly execute script from the beginning, then determine once we've reached the tx we stopped broadcasting at, then apply state changeset between this tx and current state, and then continue executing and collecting new txses. And this would probably get trickier with multiple senders

Also, such --resume functionality might result in footguns for scripts like this which precompute addreses for contracts dependent on each-other:

uint256 nonce = vm.getNonce(sender);
address expectedContract2 = vm.computeCreateAddress(sender, nonce + 1);
address contract1 = address(new Contract1(expectedContract2));
// arbitrary txs sent here
new Contract2(contract1); // contract2 has different address than the one Contract1 saw in constructor

klkvr avatar May 02 '24 14:05 klkvr

Marking the ticket as won't fix per the conversation above

zerosnacks avatar Jul 15 '24 13:07 zerosnacks