-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(cast): strip WalletOpts and EtherscanOpts from cast subcommands that dont expect a signer or need etherscan api #12705
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
Conversation
crates/cast/src/cmd/estimate.rs
Outdated
| let provider = utils::get_provider(&config)?; | ||
| let sender = SenderKind::from_wallet_opts(eth.wallet).await?; | ||
|
|
||
| let sender = if let Some(from) = from { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk if this is right, we can probably keep WalletOpts too since we accept it already? @mattsse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're actually right because I just should've removed EtherscanOpts that was used in EthereumOpts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the change!
|
@DaniPopes , any help with the test failure? I don't see any issue when I run them locally |
|
unfortunately it's just flaky |
|
Thanks! |
Closes #12394
Motivation
Went through cast subcommands that don't require WalletOpts and EtherscanOpts and removed EthereumOpts.
RpcOpts is the only type that is being used.
Solution
Here's where EthereumOpts is still required:

PR Checklist