-
Notifications
You must be signed in to change notification settings - Fork 2.3k
chore(anvil): migrate cheatsmanager to alloy
#6767
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
mattsse
left a comment
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.
smol nit
| #[cfg(feature = "impersonated-tx")] | ||
| pub fn is_impersonated(&self) -> bool { | ||
| self.signature() == IMPERSONATED_SIGNATURE | ||
| to_alloy_signature(self.signature()) == IMPERSONATED_SIGNATURE |
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.
make self.signature() return an alloy signature imo, any reason why we cannot port the typed transaction fields to be alloy?
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.
thought about doing this, but migrating typedtransaction so early would make us migrate most of anvil in one go with a longer running pr—ideally we want to avoid this.
TypedTransaction, along with other base types are getting migrated on this PR: #6778
| //! Support for "cheat codes" / bypass functions | ||
| use alloy_primitives::Address; | ||
| use alloy_rpc_types::Signature; |
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.
@DaniPopes are we mixing types here? or should we pull signature into primitives?
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 do have alloy_primitives::Signature! we'll pull it in once needed when we get into actually moving types that can recover their signer (TypedTransaction, e.g) here #6778. For now we can use this type, as right now we just need an equivalence check
onbjerg
left a comment
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.
lgtm
Motivation
Part of #6715.
Solution
Migrates the cheats manager and pushes any conversions straight to the signing step, which will then be removed once signers are migrated. While it doesn't remove many conversions, it makes easier to remove them later with the new signers.