foundry icon indicating copy to clipboard operation
foundry copied to clipboard

Ability to detect different "environments" in scripts/tests

Open 0xPhaze opened this issue 2 years ago • 7 comments

Component

Forge

Describe the feature you would like

I'm working on a more complex setup that includes a bunch of ffi sanity checks and post-deploy scripts. It would be great if I could somehow be able to detect when --broadcast is enabled so that I see the current script run as a "dry-run" and not actually overwrite my current deployments.

Other use-cases include disabling a bunch of these sanity checks (which take up a lot of extra time) by passing in some user-argument.

There actually seems to be no easy way to discern different "modes", like (e.g. dry-run, or other user-defined parameters). Running DRY_RUN=true && forge script deploy doesn't let me read DRY_RUN through vm.getEnv, because it's not in the default .env file. The closest I've gotten is to being able to detect --ffi through a try vm.ffi() {} catch {}, which somewhat works for my use-case for now, by always enforcing it to be set to true when broadcasting.

Some things that would be helpful for me include:

  • being able to detect --broadcast
  • being able to read env vars passed in as DRY_RUN=true && forge script deploy
  • being able to pass in a different environment --env .env-test
  • being able to read additional .toml settings (and then running via FOUNDRY_PROFILE=mainnet-dry-run && forge script deploy)

I think most helpful and straightforward would be nr2 or nr3. Just curious if anyone else has found the need for something like this.

Additional context

Edit: Decided to add some additional context, because I'm not sure it's really clear what I'm asking for. Below is my setup, and I'm basically using (abusing) --ffi as a way to determine DRY_RUN=true, since this has to be enabled anyway to run some of my sanity checks.

contract deploy is DeployScripts {
    function isFFIEnabled() internal returns (bool) {
        string[] memory script = new string[](1);
        script[0] = "echo";
        try vm.ffi(script) {
            return true;
        } catch {
            return false;
        }
    }

    function startBroadcastIfFFIEnabled() internal {
        if (isFFIEnabled()) {
            vm.startBroadcast();
        } else {
            console.log('FFI disabled: run again with `--ffi` to save deployments and run storage compatibility checks.');
            console.log('Disabling `broadcast`, continuing as a "dry-run".\n');

            __DEPLOY_SCRIPTS_DRY_RUN = true;

            // need to start prank instead now to be consistent in "dry-run"
            vm.stopBroadcast();
            vm.startPrank(msg.sender);
        }
    }

    function run() external {
        startBroadcastIfFFIEnabled();

        setUpContracts();

        if (isTestnet()) initContractsCITEST();
        else initContractsCI();

        vm.stopBroadcast();

        logRegisteredContracts();

        if (!__DEPLOY_SCRIPTS_DRY_RUN) {
            string memory json = getRegisteredContractsJson();

            vm.writeFile(getDeploymentsPath(string.concat("deploy-latest.json")), json);
            vm.writeFile(getDeploymentsPath(string.concat("deploy-", vm.toString(block.timestamp), ".json")), json);
        }
    }
}

0xPhaze avatar Aug 23 '22 13:08 0xPhaze

What foundry version are you on? Reading env vars works as expected for me with the latest nightly, forge 0.2.0 (4e11d1f 2022-08-24T00:12:55.883067Z), so I think you may just need to fix your setup a bit:

being able to detect --broadcast

This isn't natively supported, but you can work around with a command runner/script, e.g. if DRY_RUN=true, don't append --broadcast to the command, otherwise append it

being able to read env vars passed in as DRY_RUN=true && forge script

Remove the && and it will read your env var: DRY_RUN=true forge script <path>

Alternative, put the env var in a .env file and foundry picks it up automatically

being able to pass in a different environment --env .env-test

add export DRY_RUN=true to .env-test then run source .env-test; forge script <path> and it will pick up that env var

being able to read additional .toml settings (and then running via FOUNDRY_PROFILE=mainnet-dry-run && forge script deploy)

I'm not sure I understand the use case here / what you're trying to do 😅

mds1 avatar Aug 24 '22 13:08 mds1

Ok, I don't know when I started using the && in the script. Yeah, you're right, like that reading the environment vars works as expected and that pretty much solves my main issue of being able to tell the script to have a different "program".

I still think it would be great to be able to detect when --broadcast is on, because only then do I want to overwrite existing deployment data. Not sure if this is possible. And if not many others see value in this, I'll close this.

0xPhaze avatar Aug 24 '22 17:08 0xPhaze

I'd like to resurrect this issue, being able to detect '--broadcast' can be useful for choosing whether to write to a file or not.

adhusson avatar Sep 20 '22 13:09 adhusson

@mds1 I'd like to chime and say detecting whether --broadcast is enabled is useful.

If we take a step back and look at the big picture, we write scripts in foundry to automate some processes on the blockchain. As part of these processes we might have local side effects. For example, writing out the addresses of deployed contracts to a file. If --broadcast was off and we only did a simulation, then some local side effects are moot. --broadcast is a low-level lever for the high level goal of a "simulation mode", and right now it's up to the user to coordinate "simulation mode" flags for their script, as well as "simulation mode" for forge itself. This is error prone.

I would say there needs to be a "holistic" way of determining a simulation mode and scripts be aware of it. Presence of the --broadcast flag is the most direct way right now, but maybe there's other ways to coordinate this.

As an aside, my team is using the upgrade scripts written by @0xPhaze , and the very first thing we ran into was this issue. See https://github.com/0xPhaze/upgrade-scripts/issues/1 .

KholdStare avatar Sep 21 '22 14:09 KholdStare

This seems reasonable to me, I can see the use case. There's currently no script-only cheat codes so maybe setting an env var is the way to go?

Is there value to generalizing this so this feature (whether a cheat code, env var that's set, etc.) reports the overall state, including:

  • Which forge command is being run, such as forge snapshot, forge test, or forge script
  • Any relevant options such as broadcast or not broadcast, are there others?

I also don't want to overcomplicate it, so maybe just something like FOUNDRY_MODE is set which can be test, snapshot, script-dry-run, script-broadcast, etc.

right now it's up to the user to coordinate "simulation mode" flags for their script, as well as "simulation mode" for forge itself. This is error prone.

Can you expand on what you mean here, not sure I follow?

One last comment is there's also https://github.com/foundry-rs/foundry/issues/3298, which is interesting—if you can inject arbitrary output data into the broadcast logs, is there still a use case for writing your own custom output files? The dry runs are automatically nested into a dry-run folder that's gitignore'd, so maybe #3298 removes the need for this feature?

mds1 avatar Sep 21 '22 15:09 mds1

right now it's up to the user to coordinate "simulation mode" flags for their script, as well as "simulation mode" for forge itself. This is error prone.

Can you expand on what you mean here, not sure I follow?

If we think about the same situation of "run script to deploy contracts, and write results to a file". @0xPhaze mentioned this at the top:

if (!__DEPLOY_SCRIPTS_DRY_RUN) {
    string memory json = getRegisteredContractsJson();

    vm.writeFile(getDeploymentsPath(string.concat("deploy-latest.json")), json);
}

If __DEPLOY_SCRIPTS_DRY_RUN depends on a user supplied environment variable, it is independent of the --broadcast flag. What can happen is, the user forgets to set __DEPLOY_SCRIPTS_DRY_RUN, but leaves off the --broadcast flag. In that case the deployment doesn't actually happen, but the "results" are written as if they did. There is no way for the script to know that.

This can be "worked around" by wrapping calls in a shell script or something else, but I think that introduces extra complexity on the usability of the tool.

Regarding https://github.com/foundry-rs/foundry/issues/3298, I'll defer to @0xPhaze on the specifics, but my general feeling is that that is a "low-level lever". It gets the information written out somewhere, but may need further processing outside of forge to be useful. I.e. some other script or program that parses those outputs and creates the final "report" or script "output". IMHO in the longer term, if we want an ecosystem of tooling built with/around foundry, we need some higher-level solutions as well, otherwise we can have too many "low-level levers" that everyone has to make work together in their own way. Sorry if that's super vague - I think the those features are great, but there needs to be higher-level strategy to grow the ecosystem to go with them.

KholdStare avatar Sep 21 '22 17:09 KholdStare

I did think that it could be useful to detect the difference in "mode" between test/script, but there are some workarounds to achieve this. Nonetheless, I think having a FOUNDRY_MODE env-variable would be great and wouldn't require adding any new cheat-codes.

I also saw #3298 and thought it was very interesting. I can imagine this would be useful along with vm.parseJson being able to parse broadcast files.

The custom output files are there to keep an up-to-date list of all deployments. Say 3/4 contracts haven't changed, and only one needs to be re-deployed, I imagine this would be hard to keep track of over multiple files even with #3298. I also keep track of the contract's creation-code-hash instead of just a code-hash (because of contracts that use immutables) and the storage-layout. If there could be options to keep track and store of these by default, that would be amazing. Also, it would be great if there was a cheat-code to get the latest deployment/transaction data for a broadcasted transaction containing some arbitrary tag.

0xPhaze avatar Sep 21 '22 17:09 0xPhaze

It sounds like we can summarize this issues feature request as adding:

  • A FOUNDRY_MODE env var that's set with any forge invocation (or just ones that run solidity code?)
  • IMO we should make it a 32 byte hex string with values like keccack256(abi.encode("test")) (easier than strings for comparison purposes)
  • Then it can be accessed in tests/scripts with vm.envBytes32("FOUNDRY_MODE"), and forge-std can have a StdMode library so you can just do things like this:
bytes mode = vm.envBytes32("FOUNDRY_MODE");

if (mode == stdMode.Test) doSomething();
else if (mode == stdMode.ScriptDryRun) doSomethingElse();
else if (mode == stdMode.ScriptBroadcast) doAnotherThing();

@onbjerg @mattsse lmk what you think about this

There's a few other things discussed above, but @0xPhaze @KholdStare I'd suggest pulling them into separate issues to continue discussion to avoid mixing things here

mds1 avatar Sep 23 '22 14:09 mds1

I can see how this would be useful.

what if we introduce something like

 enum ForgeContext {
     Test,ScriptDryRun,ScriptBroadcast
}

and add cheatcodes vm.context() returns (ForgeContext) and vm.is_ffi()?

mattsse avatar Sep 23 '22 16:09 mattsse

That works for me, I think we'd also want Snapshot and Coverage as additional enum values?

mds1 avatar Sep 23 '22 16:09 mds1

@mattsse Any update on this? Was also wondering if it should be more of a "bitmask" of enabled features rather than an enum. I.e. Could have Script bit and Broadcast bit. For things like Snapshot and Coverage those could also be independent bits from Test. Just food for thought

KholdStare avatar Nov 29 '22 20:11 KholdStare

@mds1 closeable? i think #4884 was related to this

Evalir avatar Aug 21 '23 23:08 Evalir

This one is not yet closable, the idea is a cheatcode to let you determine what environment you're code is running in, such as:

if (vm.forgeContext() == ForgeContext.Script) {
  console.log("in a script");
} else if (vm.forgeContext() == ForgeContext.Test) {
  console.log("in a test");
} else {
  console.log("other");
}

We do need some thought on UX here before implementing, e.g. returning an array of contexts, or a bitmask as suggested by @KholdStare above, would probably offer better UX. In these cases we'd probably also want helper cheats, such as:

// Vm.sol
enum ForgeContext {
    Test,
    Script,
    Coverage,
    Snapshot,
    DryRun,
    Broadcast
}

function forgeContext() external returns (ForgeContext[] memory contexts);
function isTestContext() external returns (bool isTest);
function isScriptContext() external returns (bool isScript);
// etc.

#4844 is part of the way there, but a different goal. Not yet sure if they should be considered independent or if we should try to merge the two. Initial reaction is that they should be independent

mds1 avatar Aug 22 '23 02:08 mds1