-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[cast] Fix cast wallet verify #7215
[cast] Fix cast wallet verify #7215
Conversation
This addresses #7214 |
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.
Thanks! Indeed, this is a fix, but not the one we want—we want to keep the old types and not import any additional ethers types.
The fix would be to switch to using recover_address_from_msg
instead of recover_address_from_prehash
.
crates/cast/bin/cmd/wallet/mod.rs
Outdated
println!("Validation succeeded. Address {address} signed this message."); | ||
} else { | ||
println!("Validation failed. Address {address} did not sign this message."); | ||
match signature.verify(Self::hex_str_to_bytes(&message)?, address.to_ethers()) { |
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.
We should not be using the old ethers type here. We should actually use recover_address_from_msg
from the alloy Signature
type.
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've added a commit that applies the fix described—thanks!
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.
Can you please add a test
@DaniPopes yep was just gonna push one |
* Fix cast wallet verify * fmt * fix(cast): use recover_address_from_msg * chore: add test, abstract addr recovery for testing --------- Co-authored-by: Enrique Ortiz <hi@enriqueortiz.dev>
Motivation
#7088 broke
cast wallet verify
. The following worked previously and no longer works:Error:
Solution
This PR reverts a subset of the PR above which fixes the issue. Happy to take suggestions on alternative paths. Thanks!
Closes #7088