Add verify_preflight_check for script sequences

[klkvr]

Motivation

Closes #6368

Solution

Adds ScriptSequence::verify_preflight_check method which is being called between on-chain simulation and actual transactions broadcasting in cases when self.verify == true.

The same naming is used in forge create CreateArgs::verify_preflight_check so it might make sense to change it, but I didn't figure out another suitable name for it.

This might break some scenarios when --verify flag is used for multichain deployments when only subset of chains support etherscan verification.

[mattsse]

this now only deploys with --verify and all used chains have a valid etherscan API key.

I think this is reasonable.

wdyt @mds1

[klkvr]

@mattsse did CI failed because of my code? I don't understand what exactly have happened there

upd: rebased

[mds1]

Previous behavior: If no etherscan API key was set but user specified to verify, verification was silently skipped and everything else proceeds as normal

New behavior: If no etherscan API key was set but user specified to verify, forge will exit with an error after the simulation but before broadcasting.

Is that correct? If so sgtm, but may be better UX to fail fast before simulation?

[klkvr]

@mds1 Yes, that's correct

To make it fail before simulation the implementation would have to be less cleaner, because the logic will be different for newly created and resumed scripts. I would be happy to improve it that way, but considering that on-chain simulation usually doesn't take much time (usually less than script execution which will run anyway) it may not be really needed.

btw I have been testing it for multichain scripts and #5047 seems to be the case. Is there any working example of --resume --multi? I am always getting panic to have been filled with a proper rpc

[klkvr]

Also, in case when script fails because of missing etherscan key, transactions are getting saved into ./broadcast and can be restarted via --resume, thus skipping both simulation and execution which wouldn't be possible if it failed before simulation.

[Evalir]

lgtm—i think it's reasonable to include this. cc @mattsse for merging unless we haven't settled on the ux discussion on when to fail?

[mds1]

To make it fail before simulation the implementation would have to be less cleaner, because the logic will be different for newly created and resumed scripts. I would be happy to improve it that way, but considering that on-chain simulation usually doesn't take much time (usually less than script execution which will run anyway) it may not be really needed.

Got it, this seems ok to me then. @Evalir @mattsse if scripting ever gets refactored moving this to fail earlier is a nice UX change to implement if you want to track that somewhere (I've definitely had some scripts that can take 30+ seconds to simulate due to lots of RPC calls)