From 8ef66a297fbf053d819b4cbbcf3abe6d61b008a6 Mon Sep 17 00:00:00 2001 From: adam-alchemy <127769144+adam-alchemy@users.noreply.github.com> Date: Thu, 14 Mar 2024 14:31:09 -0700 Subject: [PATCH] feat(v2): Cleanup and comments (#39) --- src/LightAccount.sol | 2 +- src/LightAccountFactory.sol | 5 +++++ src/MultiOwnerLightAccount.sol | 4 ++-- src/MultiOwnerLightAccountFactory.sol | 4 ++++ src/common/BaseLightAccount.sol | 3 +-- src/common/CustomSlotInitializable.sol | 2 +- test/LightAccount.t.sol | 2 +- test/MultiOwnerLightAccount.t.sol | 2 +- 8 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/LightAccount.sol b/src/LightAccount.sol index 74ff0c0..28e7fdf 100644 --- a/src/LightAccount.sol +++ b/src/LightAccount.sol @@ -166,7 +166,7 @@ contract LightAccount is BaseLightAccount, CustomSlotInitializable { function _getStorage() internal pure returns (LightAccountStorage storage storageStruct) { bytes32 position = _STORAGE_POSITION; - assembly { + assembly ("memory-safe") { storageStruct.slot := position } } diff --git a/src/LightAccountFactory.sol b/src/LightAccountFactory.sol index e91dca9..6b84c6b 100644 --- a/src/LightAccountFactory.sol +++ b/src/LightAccountFactory.sol @@ -45,7 +45,12 @@ contract LightAccountFactory { ); } + /// @notice Compute the hash of the owner and salt in scratch space memory. + /// @param owner The owner of the account to be created. + /// @param salt A salt, which can be changed to create multiple accounts with the same owner. + /// @return combinedSalt The hash of the owner and salt. function _getCombinedSalt(address owner, uint256 salt) internal pure returns (bytes32 combinedSalt) { + // Compute the hash of the owner and salt in scratch space memory. assembly ("memory-safe") { mstore(0x00, owner) mstore(0x20, salt) diff --git a/src/MultiOwnerLightAccount.sol b/src/MultiOwnerLightAccount.sol index 8e1e199..e10bfde 100644 --- a/src/MultiOwnerLightAccount.sol +++ b/src/MultiOwnerLightAccount.sol @@ -172,7 +172,7 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable { returns (bool) { (address recovered, ECDSA.RecoverError error,) = derivedHash.tryRecover(trimmedSignature); - return (error == ECDSA.RecoverError.NoError && _getStorage().owners.contains(CastLib.toSetValue(recovered))) + return (error == ECDSA.RecoverError.NoError && _getStorage().owners.contains(recovered.toSetValue())) || _isValidERC1271SignatureNow(derivedHash, trimmedSignature); } @@ -193,7 +193,7 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable { function _getStorage() internal pure returns (LightAccountStorage storage storageStruct) { bytes32 position = _STORAGE_POSITION; - assembly { + assembly ("memory-safe") { storageStruct.slot := position } } diff --git a/src/MultiOwnerLightAccountFactory.sol b/src/MultiOwnerLightAccountFactory.sol index 14e3110..eae43be 100644 --- a/src/MultiOwnerLightAccountFactory.sol +++ b/src/MultiOwnerLightAccountFactory.sol @@ -75,6 +75,10 @@ contract MultiOwnerLightAccountFactory { ); } + /// @notice Compute the hash of the owner and salt in scratch space memory. + /// @param owners The owners of the account to be created. + /// @param salt A salt, which can be changed to create multiple accounts with the same owner. + /// @return combinedSalt The hash of the owner and salt. function _getCombinedSalt(address[] memory owners, uint256 salt) internal pure returns (bytes32) { return keccak256(abi.encode(owners, salt)); } diff --git a/src/common/BaseLightAccount.sol b/src/common/BaseLightAccount.sol index ca6be88..a203f89 100644 --- a/src/common/BaseLightAccount.sol +++ b/src/common/BaseLightAccount.sol @@ -11,7 +11,6 @@ import {UUPSUpgradeable} from "../../ext/solady/UUPSUpgradeable.sol"; import {ERC1271} from "./ERC1271.sol"; abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, ERC1271 { - bytes4 internal constant _1271_MAGIC_VALUE = bytes4(keccak256("isValidSignature(bytes32,bytes)")); // 0x1626ba7e IEntryPoint internal immutable _ENTRY_POINT; /// @dev The length of the array does not match the expected length. @@ -109,7 +108,7 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg function _call(address target, uint256 value, bytes memory data) internal { (bool success, bytes memory result) = target.call{value: value}(data); if (!success) { - assembly { + assembly ("memory-safe") { revert(add(result, 32), mload(result)) } } diff --git a/src/common/CustomSlotInitializable.sol b/src/common/CustomSlotInitializable.sol index d793027..0348392 100644 --- a/src/common/CustomSlotInitializable.sol +++ b/src/common/CustomSlotInitializable.sol @@ -182,7 +182,7 @@ abstract contract CustomSlotInitializable { function _getInitializableStorage() private view returns (CustomSlotInitializableStorage storage _storage) { bytes32 position = _storagePosition; - assembly { + assembly ("memory-safe") { _storage.slot := position } } diff --git a/test/LightAccount.t.sol b/test/LightAccount.t.sol index 2834f3a..0cd74c8 100644 --- a/test/LightAccount.t.sol +++ b/test/LightAccount.t.sol @@ -451,7 +451,7 @@ contract LightAccountTest is Test { bytes32(uint256(uint160(0x0000000071727De22E5E9d8BAf0edAc6f37da032))) ) ), - 0x7448a966519c6db16b5148dde40a37c19b5fe204acc51ef0cbfe865ea110a15d + 0x6e81714395a37e7d98764ff024ec24f3978f47157f8551fcbc0143548f39c8ef ); } diff --git a/test/MultiOwnerLightAccount.t.sol b/test/MultiOwnerLightAccount.t.sol index b828803..3d5a4af 100644 --- a/test/MultiOwnerLightAccount.t.sol +++ b/test/MultiOwnerLightAccount.t.sol @@ -499,7 +499,7 @@ contract MultiOwnerLightAccountTest is Test { bytes32(uint256(uint160(0x0000000071727De22E5E9d8BAf0edAc6f37da032))) ) ), - 0x5d40c20ca8014ab1be19cd220e039820dbbd860cd89b76eeaa27a65813ecf83e + 0x3765ef442bfa3b5ef7a30073b39949186919dd2344bb5aa0736a0e0be66ebfe1 ); }