fix: unify names of supported chains

[shawnharmsen]

issue: #1564

Was using references:
https://users.rust-lang.org/t/structopt-enum-like-args/67414/2
https://github.com/TeXitoi/structopt/blob/master/examples/enum_in_args_with_strum.rs

Motivation

Solution

Unsure if I should be adding strum

foundry/cli/Cargo.toml

Line 73 in c8872a9

strum = { version = "0.22.0", features = ["derive"] }

I was a bit too hurried and didn't try using this:

foundry/cli/src/opts/mod.rs

Lines 40 to 57 in ed4eb2d

	possible_values = [
	"mainnet",
	"ropsten",
	"rinkeby",
	"goerli",
	"kovan",
	"xdai",
	"polygon",
	"polygon_mumbai",
	"avalanche",
	"avalanche_fuji",
	"sepolia",
	"moonbeam",
	"moonbeam_dev",
	"moonriver",
	"optimism",
	"optimism-kovan"
	])]

Will try again and update

[mattsse]

thanks,
I was wondering since we have a Chain enum on ethers-rs already, can we instead use that?

[mattsse]

can we even replace this with the native ethers-rs Chain?

[shawnharmsen]

I'm having a lot of trouble figuring out how to use the Chain enum from
use ethers::prelude::Chain;

instead of

foundry/cli/src/cmd/forge/cache.rs

Lines 42 to 68 in 62c728a

	pub enum PossibleChains {
	All,
	Mainnet,
	Ropsten,
	Rinkeby,
	Goerli,
	Kovan,
	Xdai,
	Polygon,
	PolygonMumbai,
	Avalanche,
	AvalancheFuji,
	Sepolia,
	Moonbeam,
	MoonbeamDev,
	Moonriver,
	Optimism,
	OptimismKovan,
	Fantom,
	FantomTestnet,
	Arbitrum,
	ArbitrumTestnet,
	Bsc,
	BscTestnet,
	Cronos,
	CronosTestnet,
	}

Do you have any pointers/search terms for google fu?

I will keep on at it until I grok it

[mattsse]

looks like the https://docs.rs/strum/latest/strum/derive.EnumVariantNames.html macro simply creates a VARIANTS constant,

we probably should add this here instead: https://github.com/gakonst/ethers-rs/blob/842f4d260f7f1f7479eaadd7ebf7f98b329d0da5/ethers-core/src/types/chain.rs#L16

so we don't have duplicate Chain types,
we can either add the VARIANTS manually or use strum there

you can test changes to ethers-rs by patching the ethers-rs dependency against a local checkout:

foundry/Cargo.toml

Line 52 in b11b776

## Patch ethers-rs with a local checkout then run `cargo update -p ethers`

[shawnharmsen]

awesome will try that

I may have run into a blocker

Getting build error when making these updates here - updated with pr
gakonst/ethers-rs#1249

➜  ethers-rs git:(add-strum-for-chain) cargo build
   Compiling ethers-core v0.6.0 (/Users/m1/CLionProjects/ethers-rs/ethers-core)
warning: unused import: `VariantNames`
 --> ethers-core/src/types/chain.rs:6:43
  |
6 | use strum::{EnumString, EnumVariantNames, VariantNames};
  |                                           ^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error[E0119]: conflicting implementations of trait `std::str::FromStr` for type `types::chain::Chain`
   --> ethers-core/src/types/chain.rs:15:64
    |
15  | #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Deserialize, EnumString, EnumVariantNames)]
    |                                                                ^^^^^^^^^^ conflicting implementation for `types::chain::Chain`
...
132 | impl FromStr for Chain {
    | ---------------------- first implementation here
    |
    = note: this error originates in the derive macro `EnumString` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0119`.
warning: `ethers-core` (lib) generated 1 warning
error: could not compile `ethers-core` due to previous error; 1 warning emitted
warning: build failed, waiting for other jobs to finish...
warning: `ethers-core` (lib) generated 1 warning (1 duplicate)
error: build failed
➜  ethers-rs git:(add-strum-for-chain) 

I will lookup adding VARIANTS manually

[shawnharmsen]

Tests should pass after ethers-rs pr gets approved
gakonst/ethers-rs#1249

[onbjerg]

Hmm, AFAIK we already have a ClapChain enum somewhere that does this without strum - we should just use that. Also, we should do that everywhere we accept a chain name.

Edit: Found it:

foundry/cli/src/opts/mod.rs

Line 33 in eb98846

pub struct ClapChain {

But there is also a Chain enum in the config crate.. https://github.com/foundry-rs/foundry/blob/eb9884639c4fb2b976f34f033d60b2cc0432e3f3/config/src/chain.rs

[shawnharmsen]

oh whoops, will take a look at using this

[shawnharmsen]

I am a bit hard stuck here and would appreciate some guidance

From what I understand the solution should be

• instead of using strum to use Chain::VARIANTS from ethers-rs (pr: fix: unify name of supported chains with strum gakonst/ethers-rs#1249)
• Use the chain enum on the config crate

pub enum Chain {
    /// Contains a known chain
    #[serde(serialize_with = "super::from_str_lowercase::serialize")]
    Named(ethers_core::types::Chain),
    /// Contains the id of a chain
    Id(u64),
}

one

where I am stuck is

If I try to get a possible_value from Chain from config/src/chain.rs

use foundry_config::{cache, Chain as FoundryConfigChain, Config};

#[derive(Debug, Parser)]
pub struct LsArgs {
    // TODO refactor to dedup shared logic with ClapChain in opts/mod
    #[clap(
        env = "CHAIN",
        default_value = "all",
        possible_value = "all",
        possible_value = FoundryConfigChain::Named(Chain::Mainnet)
    )]
    chains: Vec<ChainOrAll>,
}

I get the error below

error[E0277]: the trait bound `PossibleValue<'_>: std::convert::From<foundry_config::Chain>` is not satisfied
    --> cli/src/cmd/forge/cache.rs:67:26
     |
67   |         possible_value = FoundryConfigChain::Named(Chain::Mainnet)
     |         --------------   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<foundry_config::Chain>` is not implemented for `PossibleValue<'_>`
     |         |
     |         required by a bound introduced by this call
     |
     = help: the following implementations were found:
               <PossibleValue<'help> as std::convert::From<&'help &'help str>>
               <PossibleValue<'help> as std::convert::From<&'help str>>
     = note: required because of the requirements on the impl of `Into<PossibleValue<'_>>` for `foundry_config::Chain`
note: required by a bound in `Arg::<'help>::possible_value`
    --> /Users/m1/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-3.1.12/src/build/arg.rs:1720:12
     |
1720 |         T: Into<PossibleValue<'help>>,
     |            ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Arg::<'help>::possible_value`

For more information about this error, try `rustc --explain E0277`.
warning: `foundry-cli` (bin "forge") generated 2 warnings
error: could not compile `foundry-cli` due to previous error; 2 warnings emitted
warning: build failed, waiting for other jobs to finish...
warning: `foundry-cli` (bin "cast") generated 2 warnings (2 duplicates)
error: build failed

two

Or trying with

possible_value = FoundryConfigChain::from_str("mainnet")

getting error

error[E0277]: the trait bound `PossibleValue<'_>: std::convert::From<Result<foundry_config::Chain, std::string::String>>` is not satisfied
    --> cli/src/cmd/forge/cache.rs:67:26
     |
67   |         possible_value = FoundryConfigChain::from_str("mainnet")
     |         --------------   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<Result<foundry_config::Chain, std::string::String>>` is not implemented for `PossibleValue<'_>`
     |         |
     |         required by a bound introduced by this call
     |
     = help: the following implementations were found:
               <PossibleValue<'help> as std::convert::From<&'help &'help str>>
               <PossibleValue<'help> as std::convert::From<&'help str>>
     = note: required because of the requirements on the impl of `Into<PossibleValue<'_>>` for `Result<foundry_config::Chain, std::string::String>`
note: required by a bound in `Arg::<'help>::possible_value`
    --> /Users/m1/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-3.1.12/src/build/arg.rs:1720:12
     |
1720 |         T: Into<PossibleValue<'help>>,
     |            ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Arg::<'help>::possible_value`

For more information about this error, try `rustc --explain E0277`.
warning: `foundry-cli` (bin "forge") generated 2 warnings
error: could not compile `foundry-cli` due to previous error; 2 warnings emitted
warning: build failed, waiting for other jobs to finish...
warning: `foundry-cli` (bin "cast") generated 2 warnings (2 duplicates)
error: build failed

[onbjerg]

@shawnharmsen I think using ClapChains might be preferable here because you need to derive ArgEnum for the enum like so:

#[derive(ArgEnum)]
pub enum MyEnum {
  OptionOne,
  OptionTwo
}

If you use the one in the config crate then we'd need to add clap as a dependency of config which is not ideal :(

Doing that it just automagically kind of works. See the CoverageReportKind enum I have in the coverage PR (in cli/src/cmd/forge/coverage.rs)

[shawnharmsen]

Apologies but ran into another problem

Latest commit I see how argEnum works

pub struct LsArgs {
\\ ...
#[clap(long, arg_enum, default_value = "all", help = "Name of chain")]
    chains2: ChainOptions,
}

#[derive(Debug, Clone, ArgEnum)]
pub enum ChainOptions {
    All,
    Mainnet,
    Goerli,
}

and the --help shows the new --chains2 with possible values

foundry git:(feat-unify-names-supported-chains) target/debug/forge cache ls -h
forge-cache-ls 
Shows cached data from ~/.foundry.

USAGE:
    forge cache ls [OPTIONS] [CHAINS]...

ARGS:
    <CHAINS>...    [env: CHAIN=] [default: all] [possible values: all, mainnet, ropsten, rinkeby, goerli, kovan, xdai, polygon, polygon-mumbai, avalanche, avalanche-fuji, sepolia,
                   moonbeam, moonbeam-dev, moonriver, optimism, optimism-kovan, fantom, fantom-testnet, arbitrum, arbitrum-testnet, bsc, bsc-testnet, cronos]

OPTIONS:
        --chains2 <CHAINS2>    Name of chain [default: all] [possible values: all, mainnet, goerli]
    -h, --help                 Print help information

When I try to use argEnum on enum ChainOrAll I get ArgEnum only supports non-unit variants

#[derive(Debug, Clone, ArgEnum)]
pub enum ChainOrAll {
    Chain(Chain),
    All,
}

 foundry git:(feat-unify-names-supported-chains) ✗ cargo build
   Compiling foundry-cli v0.2.0 (/Users/m1/CLionProjects/foundry/cli)
error: `#[derive(ArgEnum)]` only supports non-unit variants, unless they are skipped
  --> cli/src/cmd/forge/cache.rs:20:5
   |
20 |     Chain(Chain),
   |     ^^^^^

adding arg_enum to ClapChain gives the ethers::prelude::Chain does not have ArgEnum

• so maybe ArgEnum on ethers-rs is the simplest solution?

#[derive(Debug, Clone, Parser)]
pub struct ClapChain {
    #[clap(
        arg_enum,

error[E0277]: the trait bound `ethers::prelude::Chain: ArgEnum` is not satisfied
  --> cli/src/opts/mod.rs:66:16
   |
66 |     pub inner: Chain,
   |                ^^^^^ the trait `ArgEnum` is not implemented for `ethers::prelude::Chain`

error[E0277]: the trait bound `ethers::prelude::Chain: ArgEnum` is not satisfied
  --> cli/src/opts/mod.rs:34:5
   |
34 | /     #[clap(
35 | |         arg_enum,
36 | |         short = 'c',
37 | |         long = "chain",
...  |
65 | |         ])]
66 | |     pub inner: Chain,
   | |____________________^ the trait `ArgEnum` is not implemented for `ethers::prelude::Chain`

[gakonst]

Yeah you are getting this error because of:

error: `#[derive(ArgEnum)]` only supports non-unit variants, unless they are skipped
  --> cli/src/cmd/forge/cache.rs:20:5
   |
20 |     Chain(Chain),
   |     ^^^^^

[shawnharmsen]

I've opened a new pr because I've been getting build errors that I think stem from cargo.lock

Will close this one and use this: #1636