feat(cheatcodes) vm.prompt: Prompt user for interactive input

[Tudmotu]

Implementing #5509. I am reviving this, took me a while to get to this, apologies. This PR will add two cheatcodes: vm.prompt() and vm.promptSecret(). These cheatcodes will let users enter dynamic values that can be consumed by Forge scripts.

The description below is long. It mostly explains the issues I encountered. TLDR:

• I could not implement a timeout due to limitation in Dialoguer
• I did not write tests because I couldn't figure out how to test user input
• Some open questions at the bottom

Motivation

It would be useful to pass arguments to scripts in a dynamic & user-friendly way. Today the options for passing arguments are either environment variables or using the --sig argument. Both of these are cumbersome and in addition, if the parameter is some secret (password, private key, api key, etc), it exposes it in the shell history. Adding the vm.prompt() and vm.promptSecret() cheatcodes will allow script authors to write better scripts and more easily generalize their arguments. For example, scripts could prompt for a fork alias to pass to vm.createSelectFork(), or the address of a contract they interact with, etc.

Implementation

We decided to keep the interface simple and only allow string memory responses, letting the script author handle any necessary parsing using the usual string parsing methods. The implementation is based on the Dialoguer crate which is already used elsewhere and it is quite straightforward.

Timeout

Adding a timeout was suggested in order to prevent CI environments from hanging unexpectedly. These timeouts would be handled by script authors using try / catch in Solidity if desired. Unfortunately, Dialoguer does not support timeouts for prompts. Looking at other prompt libraries it does not seem they support a timeout either. I was able to hack together a partial solution by running Dialoguer in a thread and using mpsc::channel to wait for its response up to a certain timeout. This works for Input prompts but not for Password prompts. See this comment I left on the timeout feature request on Dialoguer repo.

Therefore I decided to leave the timeout feature out of the initial PR so we can discuss other approaches (see "Open questions").

Testing

Since this cheat requires interactivity it cannot be tested in Solidity ― only in Rust. But unfortunately I couldn't figure out exactly how to test it. The main problem is that the One option I thought we could try is mocking one of the underlying components Dialoguer uses ― Term ― in order to mock user input. But after looking at some Rust libraries it seems you cannot mock a struct from external libraries. Another idea I had was to try and interact with the Term object via threads: run prompt in main thread, write into Term in a different thread. While visually it works (the terminal display the prompt & answer) it seems that Dialoguer does not register the input from the other thread.

Open questions

1. For the CI hangups issue, we could explore other approaches:
   1. Use a different package which might work with the mpsc::channel solution
   2. Detect interactive environment with atty (not sure if this would actually work)
   3. Follow the --non-interactive flag forge script already has. This would require implementing a similar flag for forge test
   4. Decide to ignore this and let script authors handle timeouts externally (e.g. put a timeout on their CI job)
2. Currently the error returned from this cheatcode is only catchable with catch (bytes memory) and not with catch Error(string memory). Is there a way to fix this?
3. Testing ― as mentioned above, I couldn't figure out testing. Anyone got an idea?

Dependent PRs

• [ ] Foundry Book
  • [ ] Document @klkvr suggestion as best practice for parameterized scripts running in CI
  • [ ] Open PR
• [ ] Forge-Std

[klkvr]

this looks great! thanks for implementing this

Currently the error returned from this cheatcode is only catchable with catch (bytes memory) and not with catch Error(string memory). Is there a way to fix this?

Error(string) is a selector for Solidity errors coming from revert/require and only such errors are catchable by catch Error(string). Cheatcode errors are encoded as a CheatcodeError(string), so it's fine and expected that they can only be catched via catch (bytes)

I was able to hack together a partial solution by running Dialoguer in a thread and using mpsc::channel to wait for its response up to a certain timeout. This works for Input prompts but not for Password prompts.

Your code from https://github.com/console-rs/dialoguer/issues/266#issuecomment-1925432193 with some small modifications seems to be working for me

fn prompt_secret(prompt_text: String) -> Result<String> {
    let (send, recv) = mpsc::channel();

    thread::spawn(move || {
        send.send(Password::new().with_prompt(prompt_text).interact()).unwrap();
    });

    match recv.recv_timeout(Duration::from_secs(5)) {
        Ok(res) => res.map_err(|_| "i/o error occured".into()),
        Err(_) => Err("timeout".into())
    }
}

However, there were several times when it didn't react on any input and failed with timeout, but this looked like a shell bug and simple restart of the shell fixed it. I can't reliably reproduce this behavior.

Testing ― as mentioned above, I couldn't figure out testing. Anyone got an idea?

I can imagine some kind of InputSource abstraction which would be a wrapper around Dialoguer usually and which we can mock in tests. I think @DaniPopes or @mattsse might have some better ideas

Let's also hide values received from promptSecret in traces as it's done with env variables here: https://github.com/foundry-rs/foundry/blob/7922fd5482f9561699e0fe5a903c90b3fa1fc50d/crates/evm/traces/src/decoder/mod.rs#L460-L470

[Tudmotu]

Thank you @klkvr for the quick response! :pray:

1. Regarding the error: Ok, got it, thanks :slightly_smiling_face:
2. Regarding the timeout: that's awesome, I will try it out! Interesting what you're saying about a shell bug. I am using tmux, so might be related? I will test it out and report back
3. Regarding testing, are you suggesting a sort of DI? Let me know if I understand your suggestion correctly:
   1. Create a new struct, InputSource, which implements the same API as Dialoguer but with traits, and delegates the function calls to an internal instance of Dialoguer
   2. Change prompt and prompt_secret so they accept an instance of this new trait
   3. In the tests module, create MockInputSource which implements the trait but does not actually interact with the terminal, simply returns a hard-coded value
4. Regarding hiding values in traces: good catch! I will do that

[klkvr]

3. Yeah, I meant DI approach here, however not sure if it really worth the effort (we would have to somehow pass the mocked InputSource all the way down to the Cheatcodes when running tests), as we won't actually test any stdin interactions that way.

Let's figure out how to deal with timeouts first, and after that we can add tests for timeouts causing reverts which seems to be more important and won't require any mocking

[mds1]

One thing we also should figure out is how can scripts be used in a non-interactive/test environment with this cheat. It's best practice to test scripts, and many use scripts to setup their test state by running the script in setUp. I think the value of this cheat goes down significantly compared to a confirmContinue approach if this cannot be used in a test environment.

One approach might to add two extra args: the default value, and a bool of whether or not to immediately use that value (i.e. no timeout). This is also nice because the default arg gives us stronger return typing for free. This would fit really nicely with https://github.com/foundry-rs/foundry/issues/2900 to remove the need for an env var. Examples

// new cheat signature
prompt(string calldata promptText, uint256 default, bool useDefault) external returns (uint256 value);

// usage with env var config
uint256 myUint = vm.prompt("enter uint", 42, vm.envOr("TEST_MODE", false));

// with forge environment detection
uint256 myUint = vm.prompt("enter uint", 42, vm.isTestContext());

[klkvr]

@mds1 Good point. IMO, having a single default value which is used in test environment will not always be enough. I believe that for testing scripts with prompt cheats such pattern would make sense:

contract Script {
    function run() public {
        uint256 myUint = vm.parseUint(vm.prompt("enter uint"));
        run(myUint);
    }

    function run(uint256 myUint) public {
        // actual logic
    }
}

That way, we are keeping UX gain (don't have to provide --sig argument when running script), but tests can set any value to myUint and not just hardcoded default.

[Tudmotu]

@mds1 would you like me to implement #2399 as part of this PR? Should be simple enough I think. Dialoguer supports a confirm-type prompt: https://docs.rs/dialoguer/latest/dialoguer/struct.Confirm.html

Regarding a "default value" ― script authors could catch the timeout and provide a default for themselves:

function promptWithDefault (
    string memory prompt,
    string memory defaultValue
) internal returns (string memory input) {
    try vm.prompt(prompt) returns (string memory res) {
        if (bytes(res).length == 0) input = defaultValue;
        else input = res;
    }
    catch (bytes memory) {
        input = defaultValue;
    }
}

Not sure if that truly answers your concerns, but something to consider.

[Tudmotu]

I'm still working on the timeout, I added it in the config but it seems I broke some tests. Looking into it.

[Tudmotu]

@klkvr @mds1 @DaniPopes I see I have tests that are failing. I am very certain there is an actual issue because the original commit for the PR fully passed the tests. When I try to run the tests locally using e.g.:

$ cargo nextest run -E 'kind(test) & !test(/issue|forge_std|ext_integration/)' --partition count:2/3

I get different tests failing :thinking: (specifically one forge::cli cmd::can_install_missing_deps_build).

One noticeable difference is that it seems these tests are running on Windows while my machine is Linux.

What would be the best way to try and reproduce these failures locally?

[mds1]

@mds1 would you like me to implement https://github.com/foundry-rs/foundry/issues/2399 as part of this PR? Should be simple enough I think. Dialoguer supports a confirm-type prompt: https://docs.rs/dialoguer/latest/dialoguer/struct.Confirm.html

I would do this as a separate PR, just to manage scope, and ensure one feature doesn't block the other

@mds1 Good point. IMO, having a single default value which is used in test environment will not always be enough. I believe that for testing scripts with prompt cheats such pattern would make sense:

contract Script {
    function run() public {
        uint256 myUint = vm.parseUint(vm.prompt("enter uint"));
        run(myUint);
    }

    function run(uint256 myUint) public {
        // actual logic
    }
}

That way, we are keeping UX gain (don't have to provide --sig argument when running script), but tests can set any value to myUint and not just hardcoded default.

This is a good idea, I'd be ok with this as the solution, and we should make sure to document this pattern in the book. Let's go with your workflow here to keep things simpler, as my suggestion can always be implemented in the future in a backwards compatible way

[DaniPopes]

I see I have tests that are failing

@Tudmotu Some tests are very flaky unfortunately. I've re-ran the failing jobs and they pass now.

[Tudmotu]

I see I have tests that are failing

@Tudmotu Some tests are very flaky unfortunately. I've re-ran the failing jobs and they pass now.

Ah, thank you @DaniPopes :pray: I was pretty certain I broke something because my local tests were also failing, albeit other ones. But I guess for some other reason.

[Tudmotu]

Looking at my code I am pretty sure I did not implement the config option correctly. It works, but it feels wrong to modify evm/core for this, right?

[Tudmotu]

I pushed another change that removes the unnecessary changes I made to evm/core.

I would like to push this PR forward.

Regarding tests: Would we like to follow the approach suggested by @klkvr? I describe it here: https://github.com/foundry-rs/foundry/pull/7012#issuecomment-1926476629

[klkvr]

I think it's fine to just have a couple tests like

vm._expectCheatcodeRevert("Prompt timed out");
vm.prompt(...);

vm._expectCheatcodeRevert("Prompt timed out");
vm.promptSecret(...);

we have default timeout set to 0 for testing, so it shouldn't cause any issues with our CI

@mattsse wdyt?

[Tudmotu]

It seems using vm.prompt fails with a different error because Dialoguer detects it is not running in an interactive shell:

Looking into how to make it work.

[Tudmotu]

After looking into it, it seems to be caused by nextest capturing stdout. Running with --no-capture allows Dialoguer to run but it hangs for some reason. The hangup might be solvable, but I'm more uncertain about the --no-capture flag, since it requires running the tests sequentially.

[Tudmotu]

Hi all, I would like to get a temp check on this. Is there anything I can do to push this feature forward?

[Tudmotu]

Pushed a change which propagates the error message. Also added tests that make sure this error exists.

Will prepare PRs for Foundry Book & Forge Std.

Let me know if there is anything else you'd like me to fix here.

[Tudmotu]

Prepared branches for Book: https://github.com/foundry-rs/book/compare/master...Tudmotu:foundry-book:tudmotu/vm.prompt?expand=1

And Forge-Std: https://github.com/foundry-rs/forge-std/compare/master...Tudmotu:tudmotu/vm.prompt?expand=1

[Tudmotu]

Whoops, I was certain I already made these changes but I guess I messed something up with my rebases :sweat_smile:

I believe everything should be addressed now :slightly_smiling_face:

[mattsse]

@Tudmotu

we can now add it to forge-std

[Tudmotu]

Thank you @mattsse 🙏

I will open the Book & Std PRs soon.

[Tudmotu]

Thank you all for the review, effort and patience 🙂 🙏