-
Notifications
You must be signed in to change notification settings - Fork 156
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
Calculate SafeOperation preimage #818
Conversation
Uxio0
commented
Mar 5, 2024
- It's needed for validating EIP1271 signatures
- It's needed for validating EIP1271 signatures
@@ -87,11 +88,11 @@ def get_domain_separator( | |||
) | |||
return _domain_separator_cache[key] | |||
|
|||
def get_safe_operation_hash( | |||
def get_safe_operation_hash_preimage( |
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.
Could be cached_property and avoid to check if exists the pre-image?
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.
Dataclasses are not hashable by default, so cache decorators cannot be used. Also a property cannot be used as it receives parameters
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.
Maybe we should use namedtuple for this case https://stackoverflow.com/questions/51671699/data-classes-vs-typing-namedtuple-primary-use-cases
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.
Dataclasses are not hashable by default, so cache decorators cannot be used. Also a property cannot be used as it receives parameters
Got it.
My concern is if you create an instance of UserOperation
, maybe you are not aware that the useroperation_hash
is None
until call the get function.
Maybe we can add chain
and module
as atributes of UserOperation
like DOMAIN_HASH_SEPARATOR
is and remove safe_operation_hash
and safe_operation_hash_preimage
as parameters and create the property functions.
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.
Chain and module are not available when retrieving the UserOperation, that's why I don't want to force to define then as properties as they are not mandatory for working with the UserOperation and only needed when calculating the hash