foundry icon indicating copy to clipboard operation
foundry copied to clipboard

feat: support environment variables in `foundry.toml`

Open mds1 opened this issue 3 years ago • 31 comments

Component

Forge

Describe the feature you would like

A common use case is configuring test params with environment variables, with the RPC URL used for forking tests being a popular example. Right now there is now way to specify in the config file to use a certain environment variable for fork URLs, and instead you'd have to pass this in to the test command, e.g. forge test --fork-url $MY_RPC_URL

Additional context

No response

mds1 avatar Mar 24 '22 01:03 mds1

It should be the case that all things in foundry.toml can be configured using FOUNDRY_*, e.g. FOUNDRY_ETH_RPC_URL in your case

Edit: Ah, you want to override the environment variable used?

onbjerg avatar Mar 24 '22 02:03 onbjerg

Yea I want someone to be able to clone a repo, configure a .env file, and have forge test run with those environment variables which get loaded because they're used the config file. Currently you have to run forge test --fork-url $RPC_URL which obviously isn't a huge deal, but being able to do everything in the config file, instead of splitting between config file and command line, is better UX IMO

mds1 avatar Mar 24 '22 02:03 mds1

+1 not having to change tracked files makes for better DX

rpedroni avatar Mar 25 '22 17:03 rpedroni

Still unsure about this, I don't know of any tools that allow this other than DappTools because they used a full bash script for configuration, and I don't think there's a clean way to do this in TOML either. Any reason you don't just do the renaming in your environment script that we could autoload instead?

onbjerg avatar Mar 27 '22 05:03 onbjerg

You could argue hardhat does allow for this: In hardhat.config.ts you can access env vars like normal with process.env.MY_ENV_VAR. So a common approach is something like this:

  • Commit a file called .env.example with the contents FORK_RPC_URL=mainnetRpcUrlHere.
  • Readme instructs users to run cp .env.example .env and add their RPC URL.
  • The hardhat config defines the hardhat network configuration to fork form process.env.FORK_RPC_URL.
  • Now, running the standard hardhat test command reads the env var in the config and runs tests against that fork.

Though to your point I don't think there's a way to do this natively in TOML files. I think the easiest way is to define a custom syntax like env:MY_ENV_VAR or $MY_ENV_VAR and manually replace that string with the corresponding env var at runtime.

Any reason you don't just do the renaming in your environment script that we could autoload instead?

Sorry, not sure I follow what environment script forge would autoload. Do you mean e.g. using a makefile to define test commands with env vars and running make test instead of forge test?

mds1 avatar Mar 27 '22 13:03 mds1

I mean if Forge auto-loaded .env for example

onbjerg avatar Mar 28 '22 11:03 onbjerg

Ahh interesting! Let me make sure I'm understanding: Since every param in foundry.toml can also be set with FOUNDRY_PARAM_NAME, you're suggestion the way to use env vars would be define those params in .env instead of foundry.toml, then add a PR which auto-loads .env at the beginning of a forge command? And presumably the env file would always take precedence over the config?

I'd say that's an acceptable short term solution, with the reasons I don't love it being that:

  1. If an env var isn't set, tests will still run but likely fail (e.g. because they're not running against a forked network), which isn't very intuitive behavior. Whereas with the "env vars in config file" approach we can have an explicit ENV_VAR not set error
  2. It still fragments config so it's partially in the config file and partially in the env file
  3. You can't use different env vars for different profiles (admittedly I don't have a use case in mind for this, it just seems like a nice property)

mds1 avatar Mar 28 '22 22:03 mds1

Yes, that's what I mean.

I think the main reason I don't like the env vars in foundry.toml approach is that it's non-standard TOML and requires us to do a little hacking, which I'd rather avoid if possible.

onbjerg avatar Apr 07 '22 10:04 onbjerg

Makes sense. Maybe a good solution to mitigate items 1 and 2 is adding a field to the config file called something like env-vars=['FOUNDRY_FORK_URL', 'FOUNDRY_FORK_BLOCK'], and foundry auto-loads those env vars specifically? (Those might not be the right names for this example, I don't see them in the foundry book config reference page and don't remember offhand). This way if an env var is missing, foundry knows and can show a warning or error.

Not sure whether it'd be better to specify the env var names specifically there, or the config file field names which foundry would convert to the env var format

mds1 avatar Apr 07 '22 12:04 mds1

Why not use something like Justfile? eg https://github.com/sambacha/foundry-scripts/blob/master/justfile

This loads env file, etc

sambacha avatar Apr 07 '22 20:04 sambacha

Here's one idea:

  1. We autoload .env if present (does this still work on Windows?) to set env vars
  2. We allow the env("<VALUE>") syntax inside foundry.toml. Before running anything, we search + replace any instances of env("FOO") with the value of the environment value FOO.

Example:

[default]
src = 'src'
test = 'test'
eth-rpc-url = 'https://mainnet.infura.io/v3/env("INFURA_API")'

[rinkeby]
eth-rpc-url = 'https://rinkeby.infura.io/v3/env("INFURA_API")'

Edge case 1

Hardhat will error if no env var is present (because process.env.INFURA_API will return undefined and it'll try to read an undefined value), so there's a world where we do that. There's also a world where if a parameter needs an env var but that env var is not present, then we simply ignore that parameter and use the default value from the config (e.g. if INFURA_API isn't set above, we just consider eth-rpc-url as unset, but if it's set, forge test will assume that it's called as forge test --rpc-url)

gakonst avatar May 17 '22 23:05 gakonst

Personally I like that approach, though I know above @onbjerg mentioned he didn't love the idea of non-standard TOML, but I do think it provides the best and simplest UX.

If you specify an env var in your config but it's not present, we should probably error. I suspect most cases (such as fork URLs and API keys) would end up erroring anyway, or resulting in unexpected behavior, if you tried to fall back to some default value.

mds1 avatar May 18 '22 12:05 mds1

@onbjerg curious for any strong opinions re: my comment above?

gakonst avatar May 18 '22 15:05 gakonst

I think if we want inline env vars I'd rather go with something standard like ${ENV}. Additionally, there is a question around how this would work for non-string config since the TOML library we use parse stuff strongly typed, so it probably wouldn't work without a custom deserializer

onbjerg avatar May 19 '22 16:05 onbjerg

I'm :+1: with calling it ${ENV}

gakonst avatar May 19 '22 17:05 gakonst

This comment might be relevant: https://github.com/foundry-rs/foundry/issues/1457#issuecomment-1147407539

onbjerg avatar Jun 09 '22 14:06 onbjerg

Here's one idea:

  1. We autoload .env if present (does this still work on Windows?) to set env vars
  2. We allow the env("<VALUE>") syntax inside foundry.toml. Before running anything, we search + replace any instances of env("FOO") with the value of the environment value FOO.

Example:

[default]
src = 'src'
test = 'test'
eth-rpc-url = 'https://mainnet.infura.io/v3/env("INFURA_API")'

[rinkeby]
eth-rpc-url = 'https://rinkeby.infura.io/v3/env("INFURA_API")'

Quoting from my https://github.com/foundry-rs/foundry/issues/1457#issuecomment-1147407539 comment an enhancement to your suggestion (that's the same thing I wanted to implement on the other thread) would be to also allow the developer to have different .env files.

.env is the default one, and it acts like the default profile for foundry. You can have multiple .env file, each one for a profile you have defined on the foundry.toml config file.

If you specify a profile when running a foundry script, foundry will load the .env file and the corresponding .env.profilename file, merge them (the more specific one will override when possible the generic one) and at that point you will apply those env variables to the script.

Would this make sense?

Side question: how can I specify the profile when launching forge test? There's no way to specify it via --profile but only via an ENV var, so I would need to have the script like this to make it work?

{
  "scripts": {
    "test": "FOUNDRY_PROFILE=rinkeby forge test"
  }
}

StErMi avatar Jun 10 '22 09:06 StErMi

I think I prefer the approach mentioned above with ${ENV}. Don't like forcing dotenv files to the user.

Side question: how can I specify the profile when launching forge test? There's no way to specify it via --profile but only via an ENV var, so I would need to have the script like this to make it work?

Yep.

gakonst avatar Jun 11 '22 15:06 gakonst

I think I prefer the approach mentioned above with ${ENV}. Don't like forcing dotenv files to the user.

Side question: how can I specify the profile when launching forge test? There's no way to specify it via --profile but only via an ENV var, so I would need to have the script like this to make it work?

Yep.

Maybe I didn't understand it, how can I pass locally those env vars without the support of a .env standard?

StErMi avatar Jun 12 '22 06:06 StErMi

It depends on your shell, but in bash you would prepend the variables like so:

FOO=bar ./my-program

Alternatively you can source your .env before running commands:

source .env
./my-program

onbjerg avatar Jun 13 '22 09:06 onbjerg

It depends on your shell, but in bash you would prepend the variables like so:

FOO=bar ./my-program

Alternatively you can source your .env before running commands:

source .env
./my-program

Ok, so you can't make them to have the same npm script to act the same on both CI and local dev env.

StErMi avatar Jun 13 '22 09:06 StErMi

I'm not entirely sure what you mean - it is definitely possible to run source .env in a CI environment before running the test as well

onbjerg avatar Jun 13 '22 09:06 onbjerg

@mattsse Did https://github.com/foundry-rs/foundry/pull/2334 add general support for this, or only for certain config keys?

mds1 avatar Jul 21 '22 19:07 mds1

Only for RPC endpoints in the [rpc_endpoints] section

onbjerg avatar Jul 21 '22 19:07 onbjerg

Where else do you want it @mds1 ?

gakonst avatar Jul 25 '22 01:07 gakonst

Mainly anywhere you might have API keys, which would also include eth_rpc_url and etherscan_api_key. But more generally, it feels inconsistent to only support this in one section or only for certain keys—why not support env var interpolation for all config keys?

Related to this—and maybe this is a foundry book issue—I'm unclear on the recommend way to run a portion of tests as fork tests within your test suite. Right now we use source .env && forge test along with vm.createFork(vm.envString("OPTIMISM_RPC_URL")) (and the related fork cheatcodes). It seems an alternative is to still use source .env && forge test but instead use the [rpc_endpoints] section to define an optimism RPC and then use vm.createFork(vm. rpcUrl("optimism"))? Is there a tradeoff here or are these two identical?

mds1 avatar Jul 30 '22 18:07 mds1

Tradeoff is that it's easier to figure out what environment variables to set if you use [rpc_endpoints] since it's all in the config vs having to read through all of the tests to find vm.envString calls :) Other than that they are equivalent

onbjerg avatar Jul 30 '22 22:07 onbjerg

Related to this—and maybe this is a foundry book issue—I'm unclear on the recommend way to run a portion of tests as fork tests within your test suite. Right now we use source .env && forge test along with vm.createFork(vm.envString("OPTIMISM_RPC_URL")) (and the related fork cheatcodes). It seems an alternative is to still use source .env && forge test but instead use the [rpc_endpoints] section to define an optimism RPC and then use vm.createFork(vm. rpcUrl("optimism"))? Is there a tradeoff here or are these two identical?

They're basically identical, and I prefer the latter to the former because you put all your env vars in the foundry toml and then refer only by aliases in the code.

We could also allow etherscan_api_key = ${NAME_OF_YOUR_ENV_VAR} although that's already read by default via the ETHERSCAN_API_KEY env var (or CLI option), so not sure how helpful that is?

gakonst avatar Jul 30 '22 23:07 gakonst

Heh thought that might be the answer. IMO best practice is to have a committed .env.template type of file committed to a repo so devs know which env vars they need, which means you don't really need the [rpc_endpoints] section to find all env vars. Additionally, there might be env vars part of e.g. helper scripts in the repo, so you can't guarantee all env vars in the toml are the only ones you need.

But the vm.rpcUrl is already implemented so fine to keep and is a bit more readable, so mainly just personal preference.

We could also allow etherscan_api_key = ${NAME_OF_YOUR_ENV_VAR} although that's already read by default via the ETHERSCAN_API_KEY env var (or CLI option), so not sure how helpful that is?

Etherscan API keys differ by network, so a single ETHERSCAN_API_KEY env var isn't sufficient for multichain cases. I'm not sure of what flow people use for multi-chain verification of contracts / if toml env var interpolation would be helpful there, though. Similar comment/question for eth_rpc_url.

Perhaps we want an [etherscan_api_keys] section to map chain IDs/network names to API keys, and that way foundry can automatically use the right one based on the specified chain? And for that section we'd probably want env var interpolation, unless we choose to hardcode env var names that foundry looks for and remove the need for that section (e.g. hardcode MAINNET_ETHERSCAN_API_KEY as what foundry looks for on mainnet, OPTIMISM_ETHERSCAN_API_KEY for optimism, etc. (need to confirm which networks share API keys))

mds1 avatar Aug 03 '22 15:08 mds1

Perhaps we want an [etherscan_api_keys] section to map chain IDs/network names to API keys, and that way foundry can automatically use the right one based on the specified chain?

Yep I like that. @mattsse wdyt? also relevant in the context of multichain deployments' verifications (cc @joshieDo)

gakonst avatar Aug 03 '22 18:08 gakonst