fix: accept ethlive as a chain name #2268
Conversation
ethers-core/src/types/chain.rs
Outdated
| @@ -47,6 +47,7 @@ pub type ParseChainError = TryFromPrimitiveError<Chain>; | |||
| #[strum(serialize_all = "kebab-case")] | |||
| #[repr(u64)] | |||
| pub enum Chain { | |||
| #[strum(serialize = "ethlive")] | |||
There was a problem hiding this comment.
this will change replace "mainnet" whith ethlive.
imo mainnet is more meaningful.
we could add this as alias, but dunno how with strum tbh.
There was a problem hiding this comment.
yeah it's weird, I learnt this not too long ago: you can have only one "to_string" attr, which is the main one. Then aliases use serialize = ...
I'll open a pr to add that to the comments above
There was a problem hiding this comment.
I checked cast chain, and the reason we use ethlive is because of eth classic
ethers-core/src/types/chain.rs
Outdated
| @@ -47,6 +47,7 @@ pub type ParseChainError = TryFromPrimitiveError<Chain>; | |||
| #[strum(serialize_all = "kebab-case")] | |||
| #[repr(u64)] | |||
| pub enum Chain { | |||
| #[strum(serialize = "ethlive")] | |||
There was a problem hiding this comment.
strum is weird and you have to this
| #[strum(serialize = "ethlive")] | |
| #[strum(to_string = "mainnet", serialize = "ethlive")] |
There was a problem hiding this comment.
That's right, the proposed PR breaks the string representation. I've fixed it by adding another serialize parameter, it matches what other enum variants do. I've added tests checking that, but I see that in #2270 you've solved the tests coverage once and for all, so I'll remove them.
Motivation
I'm coming from Foundry. The result of
cast chainfor Ethereum mainnet returnsethlive. This value is then rejected as a--chainargument forforge verify-contract, maybe other commands too. The correct value for--chainismainnet.Solution
This PR adds support for parsing the
ethlivename of a chain. This name was also used in dapptools, maybe also somewhere else, but it doesn't seem to be a bug on Foundry side to use it, it's just a less known value.PR Checklist