Skip to content

Conversation

@iainnash
Copy link
Contributor

@iainnash iainnash commented Apr 30, 2025

To add Etherscan API Version types for reasonable parsing and serialization into the toml configuration files, these functions were added to block_explorers.

See: foundry-rs/foundry#10298

@iainnash
Copy link
Contributor Author

iainnash commented May 1, 2025

the deny cli task fails but all tests / fmt / clippy succeed.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense,

a few nits

pub enum EtherscanApiVersion {
#[default]
V1,
#[default]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grandizzy can we make this default already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be to not specify a default here and specify a default on the forge library.

Comment on lines -117 to -118
/// Create a new client with the correct endpoint with the chain and chosen API v2 version
pub fn new_v2_from_env(chain: Chain) -> Result<Self> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep this so that this doesn't break?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure — it only makes sense imo if the default is V1 and maybe it should still be. Not sure about other tools using this package.

.with_api_key(api_key)
.chain(chain)?
.build()
pub fn new_with_api_version(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a one liner doc

@mattsse mattsse merged commit 18100fb into foundry-rs:main May 5, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants