feat: vm.confirmContinue cheat code

[mds1]

Component

Forge

Describe the feature you would like

// Prints the provided string to the console, prompts the user to enter Y/N to
// continue execution, and reverts if N is entered, (infoString) => ()
vm.confirmContinue(string calldata info) external;

This cheatcode prints the full string entered and asks you to continue with a Y/N prompt, and reverts if you enter N. The use case here is to confirm deploy parameters in a script in a table or similar before executing the script.

The vm.toString() cheatcodes along with string.concat() make it simple to put the string together. It should support things like \n, or some other common syntax used for aligning lines such as rust's format! macro.

The benefit to making this a cheatcode instead of putting it in forge-std is to avoid requiring ffi for it.

Additional context

No response

[onbjerg]

Unclear how this should work with forge test since all of them run in parallel and we'd have to block the execution... Makes it super hard to test deploy scritps that use the cheatcode, no?

[mds1]

That's a good point. And now that I think about it, I don't think doing this with ffi would since ffi doesn't open/expose a shell for you.

What if we scope the cheatcode to only work when invoked with forge script? I think that is the best solution, since you don't want/need this in tests, and it's only useful for scripts anyway (though from an implementation perspective I'm not sure how feasible that is).

[onbjerg]

It's feasible-ish, but still some edge cases with forge script now that I think about it, e.g. when using the debugger it should also not work etc. Don't have a general stance on whether the cheatcode is good/useful so will leave that up for discussion, just trying to map out edge cases 🤠

[mds1]

True good call. Personally this is something we always add to our deploy scripts to add assurances around the deploy parameters, and just the other week we deployed something via forge script with the wrong params partly because we did not have something like this.

But happy to hear other opinions / alternative ideas as well and will let others chime in 🙂

[onbjerg]

@mds1 Can we not achieve the same result by having the user confirm before broadcast instead? Possibly also using decoded transaction data?

[mds1]

One downside is that if you have e.g. a bug in the config for tx 2, you already sent tx 1 by the time you notice.

But in general I think it's a good idea. Maybe an upfront summary of all decoded txs, like a partial trace, where only the top-level calls of broadcasted txs are shown (or two levels deep for create2 to show the actual contract creation)

[onbjerg]

@mds1 We already have all of the transactions before we broadcast them, so my idea was that we would display them all decoded so you can inspect them if you want before broadcasting any of them?

[mds1]

Cool, yea I like that idea. So I imagine the UX being something like:

• A new flag to enable/disable the "confirm continue" terminal prompt
• If enabled, show the decoded txs, perhaps in a view that looks similar to the tx traces? (I'm mostly indifferent as to the actual format here)
  • For normal txs we can decode like usual.
  • For create we'd want to decode contract name and constructor args.
  • For create2 txs, we'll need to show one level deeper (since top-level calldata is a tx to the create2 deployer, which then makes the create2 call).

Does that line up with what you're thinking?

[onbjerg]

Yep!

[Dendrimer]

Hey, wondering if there's been any progress on this issue. If no one is actively working/planning to work on this, I could take a crack at it, though this would be my first issue in Foundry so a code pointer or two would be much appreciated.

[onbjerg]

Feel free to take a stab at it :)

Let me know if you end up not being able to complete it and I will unassign you!

[mds1]

@Dendrimer I unassigned for you now, but if you do still plan to work on this let me know and I can reassign you

[sakulstra]

Hello, just adding my two 2ct here.
We're building lot's of scripts and utilize logs assertions and even some ffi to give users a clear picture of what they are going to submit before actually doing it.

While this works fine with ppl using ledger (as the flow is stuck till they confirm) we noticed especially ppl using pk mess things up, as there's no confirmation needed. Even if they see sth is wrong it's usually to late (ofc, there's dry run etc, but that's not the point here).

Therefore i'm wondering if instead of having a vm.confirmContinue cheatcode it would make more sense to just have a --requireConfirm on the script command which would force you to enter before every tx broadcast?

I assume ppl either always or never want this, so using a cheatcode seems a bit overkill to me.

[mds1]

Yea the title is outdated, the current UX that was agreed on is a flag as described in this comment: #2399 (comment)

[klkvr]

@mds1 should we close this considering that we now have vm.prompt* cheats?