Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(forge): add fork_url as an alias for eth_rpc_url in foundry.toml #1414

Closed
OliverNChalk opened this issue Apr 26, 2022 · 6 comments
Closed
Labels
C-forge Command: forge D-easy Difficulty: easy first issue A good way to start contributing P-low Priority: low T-feature Type: feature

Comments

@OliverNChalk
Copy link
Contributor

OliverNChalk commented Apr 26, 2022

Component

Forge

Describe the feature you would like

Brief

A user is able to setup fork scenarios using the following declaration:

[arbitrum]
eth_rpc_url = "https://rpc.ankr.com/arbitrum"
match_contract = "Saddle"

If we want to continue the convention of letting config set CLI options, then it would make sense to alias fork_url to eth_rpc_url, as shown:

[arbitrum]
fork_url = "https://rpc.ankr.com/arbitrum"
match_contract = "Saddle"

This was prompted after testing the changes in: #1378

Scope

  • Are eth_rpc_url in foundry.toml and forge test --fork-url already equivalent? Is priority given to the command line flag?
  • Is it as simple as adding fork_url as an alias for eth_rpc_url, do we want to reverse the relationship and make eth_rpc_url an alias for fork_url?

Additional context

No response

@onbjerg
Copy link
Member

onbjerg commented Apr 26, 2022

Are eth_rpc_url in foundty.toml and forge test --fork-url already equivalent? Is priority given to the command line flag?

Yes, and yes.

Is it as simple as adding fork_url as an alias for eth_rpc_url, do we want to reverse the relationship and make eth_rpc_url an alias for fork_url?

I think it's easier to alias fork_url since Cast also depends on eth_rpc_url

@onbjerg onbjerg added first issue A good way to start contributing C-forge Command: forge D-easy Difficulty: easy P-low Priority: low labels Apr 26, 2022
@malawadd
Copy link

how would I approach this if i wanted to work on this issue

@onbjerg
Copy link
Member

onbjerg commented Apr 27, 2022

Actually, I think we might not want to create this alias since that would also run all tests in fork mode if ETH_RPC_URL is set globally. cc @mds1 for thoughts on this, I know this was not expected behavior when I implemented it the last time

@mds1
Copy link
Collaborator

mds1 commented Apr 27, 2022

I'm not sure I totally follow the suggesting aliasing and how it would affect config options/defaults, but:

  • Allowing users to specify fork_url in the config as an alias for eth_rpc_url seems reasonable to me.
  • Automatically forking if a FOUNDRY_FORK_URL env var is found also seems ok.
  • The main thing we want to prevent is: the presence of an ETH_RPC_URL env var should not lead to tests forking from that URL. Having a globally defined ETH_RPC_URL is common for cast users, so we don't want that to affect tests.

Now that cast supports config files though, I wonder if this alias puts us in a weird spot: I might want tests to not fork, but cast to use e.g. Optimism by specifying an Optimism RPC in the config file's eth_rpc_url. AFAIK that is not currently doable because specifying eth_rpc_url will result in tests also forking?

So perhaps we want to decouple the two and have something like this?

  • fork_url used to determine the network for forge
  • eth_rpc_url to determine the network for cast, which defaults to fork_url if present, and ETH_RPC_URL env var if fork_url isn't present

@OliverNChalk
Copy link
Contributor Author

To second @mds1's comments, the following scenario is currently not supported:

  • Specify eth_rpc_url in config
  • Pass in --fork-block-number on command line

In this scenario the following error will be returned:
image

@onbjerg
Copy link
Member

onbjerg commented Aug 15, 2022

Will close this as this is sort of supported now - you can add RPC URLs in the [rpc_endpoints] section of foundry.toml and alias one of them to ETH_RPC_URL, e.g.:

[rpc_endpoints]
mainnet = "${ETH_RPC_URL}"

I think that should cover this functionality, but please let me know if it didn't quite cover what you were asking for 🙂

@onbjerg onbjerg closed this as completed Aug 15, 2022
Repository owner moved this from Todo to Done in Foundry Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge D-easy Difficulty: easy first issue A good way to start contributing P-low Priority: low T-feature Type: feature
Projects
Archived in project
Development

No branches or pull requests

4 participants