From 46233d8d199106af07a009a640aa9f78a24b3929 Mon Sep 17 00:00:00 2001 From: Uxio Fuentefria <6909403+Uxio0@users.noreply.github.com> Date: Wed, 20 Mar 2024 14:18:38 +0100 Subject: [PATCH] Refactor SafeOperation - Make class inmutable - Add proper caches - Return `valid_after` and `valid_until` as datetime --- gnosis/eth/tests/mocks/mock_bundler.py | 30 ++++ .../account_abstraction/safe_operation.py | 136 ++++++++++-------- .../test_safe_operation.py | 42 +++++- 3 files changed, 147 insertions(+), 61 deletions(-) diff --git a/gnosis/eth/tests/mocks/mock_bundler.py b/gnosis/eth/tests/mocks/mock_bundler.py index 2e6cc9283..aee1a7756 100644 --- a/gnosis/eth/tests/mocks/mock_bundler.py +++ b/gnosis/eth/tests/mocks/mock_bundler.py @@ -393,3 +393,33 @@ "0x0000000071727De22E5E9d8BAf0edAc6f37da032", ], } + + +user_operation_with_valid_dates_hash_mock = HexBytes( + "0xdab376e240f65500714fbf103e644dc15709189addcf71f16d5e1d2831d75073" +) + +# It contains `valid_after` and `valid_until` values set in the signature +user_operation_with_valid_dates_mock = { + "jsonrpc": "2.0", + "id": 1, + "result": { + "userOperation": { + "sender": "0xb17E6F96085b443389E30eEAc243DA37A71cbd42", + "nonce": "0x0", + "initCode": "0x4e1dcf7ad4e460cfd30791ccc4f9c8a4f820ec671688f0b900000000000000000000000029fcb43b46531bca003ddc8fcb67ffe91900c76200000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000069e3500000000000000000000000000000000000000000000000000000000000001e4b63e800d000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000010000000000000000000000008ecd4ec46d4d2a6b64fe960b3d64e8b94b2234eb0000000000000000000000000000000000000000000000000000000000000140000000000000000000000000a581c4a4db7175302464ff3c06380bc3270b40370000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000d725e11588f040d86c4c49d8236e32a5868549f000000000000000000000000000000000000000000000000000000000000000648d0dc49f00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a581c4a4db7175302464ff3c06380bc3270b40370000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "callData": "0x7bb3742800000000000000000000000038869bf66a61cf6bdb996a6ae40d5853fd43b52600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000001848d80ff0a00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000132001c7d4b196cb0c7b01d743fbc6116a902379c723800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044a9059cbb000000000000000000000000b17e6f96085b443389e30eeac243da37a71cbd4200000000000000000000000000000000000000000000000000000000000186a0001c7d4b196cb0c7b01d743fbc6116a902379c723800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044a9059cbb000000000000000000000000b17e6f96085b443389e30eeac243da37a71cbd4200000000000000000000000000000000000000000000000000000000000186a0000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "callGasLimit": "0x1d006", + "verificationGasLimit": "0x840a0", + "preVerificationGas": "0xe20c", + "maxFeePerGas": "0x5c17448f", + "maxPriorityFeePerGas": "0x52412100", + "paymasterAndData": "0xdff7fa1077bce740a6a212b3995990682c0ba66d0000000000000000000000000000000000000000000000000000000065f97c070000000000000000000000000000000000000000000000000000000000000000e3fbfff23a1aba3bb40cc4e4d7eb82407ddbf881e82d7d31eb8af58086fa676824910cf9ddd36be8f799acdfcb7ac09afa7ad388f34d50d3596ece4f0e5051fa1c", + "signature": "0x000065f979a8000065fa6408b5fec8be96ee50b27e45b26fa2ee2e8093e075d7db7921425db40f5ea68295b66e30a84e32c300c0376c3b5b880e492deb012e2e009cc054e09c080f060f419c1b", + }, + "entryPoint": "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789", + "blockNumber": "0x543047", + "blockHash": "0x94d81e926531f77063fe9a8672cb52c4525dfca23bc5b18b2e09348441ee2122", + "transactionHash": "0xe2dc67ebcfc3215eaf3809931529ea9a4c657d1931f9b8e0ec5714d71d0a8387", + }, +} diff --git a/gnosis/safe/account_abstraction/safe_operation.py b/gnosis/safe/account_abstraction/safe_operation.py index 473314f4c..b4344fc54 100644 --- a/gnosis/safe/account_abstraction/safe_operation.py +++ b/gnosis/safe/account_abstraction/safe_operation.py @@ -1,5 +1,9 @@ import dataclasses +import datetime +import logging +from functools import cache, cached_property from typing import Optional +from zoneinfo import ZoneInfo from eth_abi import encode as abi_encode from eth_abi.packed import encode_packed @@ -9,10 +13,12 @@ from gnosis.eth.account_abstraction import UserOperation from gnosis.eth.utils import fast_keccak +logger = logging.getLogger(__name__) + _domain_separator_cache = {} -@dataclasses.dataclass +@dataclasses.dataclass(eq=True, frozen=True) class SafeOperation: """ Safe EIP4337 operation @@ -54,7 +60,6 @@ class SafeOperation: "0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218" ) # bytes32 safe_operation_hash: Optional[bytes] = None - safe_operation_hash_preimage: Optional[bytes] = None @classmethod def from_user_operation(cls, user_operation: UserOperation): @@ -75,6 +80,63 @@ def from_user_operation(cls, user_operation: UserOperation): user_operation.signature[12:], ) + @staticmethod + def _parse_epoch(epoch: int) -> Optional[datetime.datetime]: + if not epoch: + return None + + try: + return datetime.datetime.fromtimestamp(epoch, ZoneInfo("UTC")) + except (OverflowError, ValueError) as exc: + logger.warning("Invalid epoch %d: %s", epoch, exc) + return None + + @cached_property + def valid_after_as_datetime(self) -> Optional[datetime.datetime]: + return self._parse_epoch(self.valid_after) + + @cached_property + def valid_until_as_datetime(self) -> Optional[datetime.datetime]: + return self._parse_epoch(self.valid_until) + + @cached_property + def safe_operation_hash_preimage(self) -> bytes: + encoded_safe_op_struct = abi_encode( + [ + "bytes32", + "address", + "uint256", + "bytes32", + "bytes32", + "uint256", + "uint256", + "uint256", + "uint256", + "uint256", + "bytes32", + "uint48", + "uint48", + "address", + ], + [ + self.TYPE_HASH, + self.safe, + self.nonce, + self.init_code_hash, + self.call_data_hash, + self.call_gas_limit, + self.verification_gas_limit, + self.pre_verification_gas, + self.max_fee_per_gas, + self.max_priority_fee_per_gas, + self.paymaster_and_data_hash, + self.valid_after, + self.valid_until, + self.entry_point, + ], + ) + return fast_keccak(encoded_safe_op_struct) + def get_domain_separator( self, chain_id: int, module_address: ChecksumAddress ) -> bytes: @@ -88,64 +150,18 @@ def get_domain_separator( ) return _domain_separator_cache[key] - def get_safe_operation_hash_preimage( - self, chain_id: int, module_address: ChecksumAddress - ) -> bytes: - # Cache safe_operation_hash - if not self.safe_operation_hash_preimage: - encoded_safe_op_struct = abi_encode( - [ - "bytes32", - "address", - "uint256", - "bytes32", - "bytes32", - "uint256", - "uint256", - "uint256", - "uint256", - "uint256", - "bytes32", - "uint48", - "uint48", - "address", - ], - [ - self.TYPE_HASH, - self.safe, - self.nonce, - self.init_code_hash, - self.call_data_hash, - self.call_gas_limit, - self.verification_gas_limit, - self.pre_verification_gas, - self.max_fee_per_gas, - self.max_priority_fee_per_gas, - self.paymaster_and_data_hash, - self.valid_after, - self.valid_until, - self.entry_point, - ], - ) - self.safe_operation_hash_preimage = fast_keccak(encoded_safe_op_struct) - return self.safe_operation_hash_preimage - + @cache def get_safe_operation_hash( self, chain_id: int, module_address: ChecksumAddress ) -> bytes: - # Cache safe_operation_hash - if not self.safe_operation_hash: - safe_op_struct_hash = self.get_safe_operation_hash_preimage( - chain_id, module_address - ) - operation_data = encode_packed( - ["bytes1", "bytes1", "bytes32", "bytes32"], - [ - bytes.fromhex("19"), - bytes.fromhex("01"), - self.get_domain_separator(chain_id, module_address), - safe_op_struct_hash, - ], - ) - self.safe_operation_hash = fast_keccak(operation_data) - return self.safe_operation_hash + safe_op_struct_hash = self.safe_operation_hash_preimage + operation_data = encode_packed( + ["bytes1", "bytes1", "bytes32", "bytes32"], + [ + bytes.fromhex("19"), + bytes.fromhex("01"), + self.get_domain_separator(chain_id, module_address), + safe_op_struct_hash, + ], + ) + return fast_keccak(operation_data) diff --git a/gnosis/safe/tests/account_abstraction/test_safe_operation.py b/gnosis/safe/tests/account_abstraction/test_safe_operation.py index 2183f0700..1f76a5230 100644 --- a/gnosis/safe/tests/account_abstraction/test_safe_operation.py +++ b/gnosis/safe/tests/account_abstraction/test_safe_operation.py @@ -1,3 +1,6 @@ +import dataclasses +import datetime +import zoneinfo from unittest import TestCase from gnosis.eth.account_abstraction import UserOperation @@ -8,6 +11,8 @@ safe_4337_safe_operation_hash_mock, safe_4337_user_operation_hash_mock, user_operation_mock, + user_operation_with_valid_dates_hash_mock, + user_operation_with_valid_dates_mock, ) from ...account_abstraction import SafeOperation @@ -21,7 +26,7 @@ def setUp(self): def tearDown(self): _domain_separator_cache.clear() - def test_safe_operation(self): + def test_from_user_operation(self): safe_operation = SafeOperation.from_user_operation( UserOperation.from_bundler_response( safe_4337_user_operation_hash_mock, user_operation_mock["result"] @@ -61,3 +66,38 @@ def test_safe_operation(self): ): safe_4337_module_domain_separator_mock }, ) + + self.assertIsNone(safe_operation.valid_after_as_datetime) + self.assertIsNone(safe_operation.valid_until_as_datetime) + + def test_datetime_parse(self): + safe_operation = SafeOperation.from_user_operation( + UserOperation.from_bundler_response( + user_operation_with_valid_dates_hash_mock, + user_operation_with_valid_dates_mock["result"], + ) + ) + + self.assertEqual(safe_operation.valid_after, 1710848424) + self.assertEqual( + safe_operation.valid_after_as_datetime, + datetime.datetime( + 2024, 3, 19, 11, 40, 24, tzinfo=zoneinfo.ZoneInfo(key="UTC") + ), + ) + self.assertEqual(safe_operation.valid_until, 1710908424) + self.assertEqual( + safe_operation.valid_until_as_datetime, + datetime.datetime( + 2024, 3, 20, 4, 20, 24, tzinfo=zoneinfo.ZoneInfo(key="UTC") + ), + ) + + # Test invalid value cannot be parsed as datetime + invalid_safe_operation = dataclasses.replace( + safe_operation, + valid_after=5555555555555555555555, + valid_until=666666666666666666666, + ) + self.assertIsNone(invalid_safe_operation.valid_after_as_datetime) + self.assertIsNone(invalid_safe_operation.valid_until_as_datetime)