From 7e14d10b29126b598671a04bc1d7694f7a4cfbc4 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Fri, 11 Oct 2024 19:26:19 -0400 Subject: [PATCH] feat: add erc7739 compatibility chore: improve comments and dedupe constants Co-authored-by: Adam Egyed <5456061+adamegyed@users.noreply.github.com> Update src/helpers/ERC7739ReplaySafeWrapper.sol Update src/helpers/ERC7739ReplaySafeWrapper.sol Co-authored-by: Zer0dot Update src/helpers/ERC7739ReplaySafeWrapper.sol Co-authored-by: Zer0dot Co-authored-by: Adam Egyed <5456061+adamegyed@users.noreply.github.com> chore: gas bad chore: fix test name Co-authored-by: Adam Egyed <5456061+adamegyed@users.noreply.github.com> --- ...odularAccount_Runtime_AccountCreation.snap | 2 +- .../ModularAccount_Runtime_Erc20Transfer.snap | 2 +- ...count_Runtime_InstallSessionKey_Case1.snap | 2 +- ...ModularAccount_Runtime_NativeTransfer.snap | 2 +- ...t_Runtime_UseSessionKey_Case1_Counter.snap | 2 +- ...unt_Runtime_UseSessionKey_Case1_Token.snap | 2 +- ...ccount_UserOp_InstallSessionKey_Case1.snap | 2 +- ...ularAccount_UserOp_deferredValidation.snap | 2 +- ...iModularAccount_Runtime_Erc20Transfer.snap | 2 +- ...count_Runtime_InstallSessionKey_Case1.snap | 2 +- ...ModularAccount_Runtime_NativeTransfer.snap | 2 +- ...t_Runtime_UseSessionKey_Case1_Counter.snap | 2 +- ...unt_Runtime_UseSessionKey_Case1_Token.snap | 2 +- ...ccount_UserOp_InstallSessionKey_Case1.snap | 2 +- ...ularAccount_UserOp_deferredValidation.snap | 2 +- gas/modular-account/ModularAccount.gas.t.sol | 24 +- .../SemiModularAccount.gas.t.sol | 16 +- src/account/SemiModularAccountBase.sol | 40 +-- src/helpers/ERC7739ReplaySafeWrapper.sol | 282 ++++++++++++++++++ src/libraries/ERC7739ReplaySafeWrapper.sol | 278 +++++++++++++++++ .../SemiModularKnownSelectorsLib.sol | 3 +- .../validation/ECDSAValidationModule.sol | 9 +- .../validation/WebauthnValidationModule.sol | 3 +- test/account/AccountExecHooks.t.sol | 1 + test/account/AccountReturnData.t.sol | 1 + test/account/DeferredValidation.t.sol | 1 + test/account/DirectCallsFromModule.t.sol | 1 + test/account/GlobalValidationTest.t.sol | 1 + test/account/HookOrdering.t.sol | 4 +- test/account/ModularAccount.t.sol | 41 ++- test/account/ModularAccountView.t.sol | 1 + test/account/MultiValidation.t.sol | 1 + test/account/PerHookData.t.sol | 39 ++- test/account/PermittedCallPermissions.t.sol | 1 + test/account/ReplaceModule.t.sol | 6 +- test/account/SelfCallAuthorization.t.sol | 1 + .../SemiModularAccountDirectCall.t.sol | 1 + test/account/TokenReceiver.t.sol | 1 + test/account/UpgradeToSma.t.sol | 1 + test/account/ValidationIntersection.t.sol | 1 + .../modules/MockAccessControlHookModule.sol | 2 +- test/modules/AllowlistERC20TokenLimit.t.sol | 1 + test/modules/AllowlistModule.t.sol | 1 + test/modules/NativeTokenLimitModule.t.sol | 1 + test/modules/PaymasterGuardModule.t.sol | 1 + test/modules/TimeRangeModule.t.sol | 1 + test/modules/WebauthnValidationModule.t.sol | 15 +- test/utils/AccountTestBase.sol | 52 ++-- test/utils/ModuleSignatureUtils.sol | 184 +++++++++--- 49 files changed, 884 insertions(+), 162 deletions(-) create mode 100644 src/helpers/ERC7739ReplaySafeWrapper.sol create mode 100644 src/libraries/ERC7739ReplaySafeWrapper.sol diff --git a/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap b/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap index ad7ee641c..1f55f020b 100644 --- a/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap +++ b/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap @@ -1 +1 @@ -176044 \ No newline at end of file +176022 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap b/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap index 81aabe9bd..fbaf4d02c 100644 --- a/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap +++ b/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap @@ -1 +1 @@ -79423 \ No newline at end of file +79401 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap b/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap index a269b5119..13bdc794f 100644 --- a/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -424078 \ No newline at end of file +424034 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap b/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap index 83e1116e8..8345f666c 100644 --- a/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap +++ b/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap @@ -1 +1 @@ -55545 \ No newline at end of file +55523 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index 345ce0264..89d617951 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -80392 \ No newline at end of file +80370 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap index c135b6166..9fdf10f1f 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -113969 \ No newline at end of file +113947 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap index 942d5cfed..59ca84f94 100644 --- a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -532471 \ No newline at end of file +532449 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap index cc4a61ee1..3a5119665 100644 --- a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -258153 \ No newline at end of file +262095 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap index 5b4c3029c..ce6fc708a 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap @@ -1 +1 @@ -75869 \ No newline at end of file +75847 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap index c940e75b8..e17f8dbcb 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -423008 \ No newline at end of file +422964 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap index bb0cb6723..778cb9315 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap @@ -1 +1 @@ -51991 \ No newline at end of file +51969 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index 636ea3208..c664b8aa3 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -80830 \ No newline at end of file +80786 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap index 5affe5a51..b12b841a8 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -114407 \ No newline at end of file +114363 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap index f799eb8f1..06a446a0a 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -530046 \ No newline at end of file +530024 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap index c315ad3c5..d56b9a9ae 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -254593 \ No newline at end of file +258633 \ No newline at end of file diff --git a/gas/modular-account/ModularAccount.gas.t.sol b/gas/modular-account/ModularAccount.gas.t.sol index 33e2dea71..41a3a9305 100644 --- a/gas/modular-account/ModularAccount.gas.t.sol +++ b/gas/modular-account/ModularAccount.gas.t.sol @@ -198,18 +198,26 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount") uint256 deferredInstallNonce = 0; uint48 deferredInstallDeadline = 0; - bytes memory deferredValidationSig = _packFinalSignature( + (bytes32 structHash, bytes32 digest, bytes32 domainSeparator) = _getDeferredInstallStructAndHash( + account1, deferredInstallNonce, deferredInstallDeadline, deferredValidationInstallCall + ); + + bytes memory deferredValidationSig = _packFinal1271Signature( _signRawHash( vm, owner1Key, - _getECDSAReplaySafeHash( - account1, - ecdsaValidationModule, - _getDeferredInstallHash( - account1, deferredInstallNonce, deferredInstallDeadline, deferredValidationInstallCall - ) + _getModuleReplaySafeHash( + address(account1), + address(ecdsaValidationModule), + domainSeparator, + structHash, + digest, + _DEFERRED_INSTALL_CONTENTS_TYPE ) - ) + ), + domainSeparator, + structHash, + _DEFERRED_INSTALL_CONTENTS_TYPE ); userOp.signature = _encodeDeferredInstallUOSignature( diff --git a/gas/modular-account/SemiModularAccount.gas.t.sol b/gas/modular-account/SemiModularAccount.gas.t.sol index cf4efd417..6d7354264 100644 --- a/gas/modular-account/SemiModularAccount.gas.t.sol +++ b/gas/modular-account/SemiModularAccount.gas.t.sol @@ -193,17 +193,21 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun uint256 deferredInstallNonce = 0; uint48 deferredInstallDeadline = 0; - bytes memory deferredValidationSig = _packFinalSignature( + (bytes32 structHash, bytes32 digest, bytes32 domainSeparator) = _getDeferredInstallStructAndHash( + account1, deferredInstallNonce, deferredInstallDeadline, deferredValidationInstallCall + ); + + bytes memory deferredValidationSig = _packFinal1271Signature( _signRawHash( vm, owner1Key, _getSMAReplaySafeHash( - account1, - _getDeferredInstallHash( - account1, deferredInstallNonce, deferredInstallDeadline, deferredValidationInstallCall - ) + address(account1), domainSeparator, structHash, digest, _DEFERRED_INSTALL_CONTENTS_TYPE ) - ) + ), + domainSeparator, + structHash, + _DEFERRED_INSTALL_CONTENTS_TYPE ); userOp.signature = _encodeDeferredInstallUOSignature( diff --git a/src/account/SemiModularAccountBase.sol b/src/account/SemiModularAccountBase.sol index b9d8f4521..5a354d744 100644 --- a/src/account/SemiModularAccountBase.sol +++ b/src/account/SemiModularAccountBase.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.26; -import {IModularAccount, ModuleEntity} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; import {IModularAccount, ModuleEntity} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; import {ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; @@ -11,6 +10,8 @@ import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/Signa import {DIRECT_CALL_VALIDATION_ENTITYID, FALLBACK_VALIDATION} from "../helpers/Constants.sol"; import {SignatureType} from "../helpers/SignatureType.sol"; + +import {ERC7739ReplaySafeWrapper} from "../libraries/ERC7739ReplaySafeWrapper.sol"; import {RTCallBuffer, UOCallBuffer} from "../libraries/ExecutionLib.sol"; import {SemiModularKnownSelectorsLib} from "../libraries/SemiModularKnownSelectorsLib.sol"; import {ModularAccountBase} from "./ModularAccountBase.sol"; @@ -18,6 +19,7 @@ import {ModularAccountBase} from "./ModularAccountBase.sol"; abstract contract SemiModularAccountBase is ModularAccountBase { using MessageHashUtils for bytes32; using ModuleEntityLib for ModuleEntity; + using ERC7739ReplaySafeWrapper for address; struct SemiModularAccountStorage { address fallbackSigner; @@ -28,10 +30,6 @@ abstract contract SemiModularAccountBase is ModularAccountBase { uint256 internal constant _SEMI_MODULAR_ACCOUNT_STORAGE_SLOT = 0x5b9dc9aa943f8fa2653ceceda5e3798f0686455280432166ba472eca0bc17a32; - // keccak256("ReplaySafeHash(bytes32 hash)") - bytes32 private constant _REPLAY_SAFE_HASH_TYPEHASH = - 0x294a8735843d4afb4f017c76faf3b7731def145ed0025fc9b1d5ce30adf113ff; - uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; @@ -82,24 +80,6 @@ abstract contract SemiModularAccountBase is ModularAccountBase { return "alchemy.semi-modular-account.0.0.1"; } - /// @notice Returns the replay-safe hash generated from the passed typed data hash for 1271 validation. - /// @param hash The typed data hash to wrap in a replay-safe hash. - /// @return The replay-safe hash, to be used for 1271 signature generation. - /// - /// @dev Generates a replay-safe hash to wrap a standard typed data hash. This prevents replay attacks by - /// enforcing the domain separator, which includes this contract's address and the chainId. This is only - /// relevant for 1271 validation because UserOp validation relies on the UO hash and the Entrypoint has - /// safeguards. - /// - /// NOTE: Like in signature-based validation modules, the returned hash should be used to generate signatures, - /// but the original hash should be passed to the external-facing function for 1271 validation. - function replaySafeHash(bytes32 hash) public view virtual returns (bytes32) { - return MessageHashUtils.toTypedDataHash({ - domainSeparator: _domainSeparator(), - structHash: _hashStructReplaySafeHash(hash) - }); - } - function _execUserOpValidation( ModuleEntity userOpValidationFunction, bytes32 userOpHash, @@ -143,7 +123,9 @@ abstract contract SemiModularAccountBase is ModularAccountBase { if (sigValidation.eq(FALLBACK_VALIDATION)) { address fallbackSigner = _getFallbackSigner(); - if (_checkSignature(fallbackSigner, replaySafeHash(hash), signature)) { + (bytes32 digest, bytes calldata innerSignature) = + address(this).validateERC7739SigFormatForAccount(hash, signature); + if (_checkSignature(fallbackSigner, digest, innerSignature)) { return _1271_MAGIC_VALUE; } return _1271_INVALID; @@ -232,16 +214,6 @@ abstract contract SemiModularAccountBase is ModularAccountBase { return _storage; } - function _hashStructReplaySafeHash(bytes32 hash) internal pure virtual returns (bytes32) { - bytes32 res; - assembly ("memory-safe") { - mstore(0x00, _REPLAY_SAFE_HASH_TYPEHASH) - mstore(0x20, hash) - res := keccak256(0, 0x40) - } - return res; - } - // Overrides ModuleManagerInternals function _isNativeFunction(bytes4 selector) internal pure override returns (bool) { return SemiModularKnownSelectorsLib.isNativeFunction(selector); diff --git a/src/helpers/ERC7739ReplaySafeWrapper.sol b/src/helpers/ERC7739ReplaySafeWrapper.sol new file mode 100644 index 000000000..9316b6d14 --- /dev/null +++ b/src/helpers/ERC7739ReplaySafeWrapper.sol @@ -0,0 +1,282 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.26; + +// A contract mixin ERC7739-compliant nested EIP-712 wrappers over EIP-1271 digests. +// This allows for efficient, readable ERC-1271 signature schemes for smart contract accounts. +library ERC7739ReplaySafeWrapper { + // Points to a location in memory with EIP-712 formatted `encodeType(TypedDataSign)`, excluding the first two + // words for typeHash(TypedDataSign)` and `typeHash(contents)`. Does not store the length, because this has a + // fixed size of `0x120` (9 words). + // Used to compute `hashStruct(typedDataSign)` + type TypeStruct is bytes32; + + /// @dev `keccak256("PersonalSign(bytes prefixed)")`. + bytes32 internal constant _PERSONAL_SIGN_TYPEHASH = + 0x983e65e5148e570cd828ead231ee759a8d7958721a768f93bc4483ba005c32de; + + // The difference between a module hash and an account hash is: + // Account domains include only chainId and verifyingContract of itself (not the implementation) + // Module domains include chainId, verifyingContract of the module, and uses the optional salt param, using the + // account address + + // keccak256("EIP712Domain(uint256 chainId,address verifyingContract,bytes32 salt)") + bytes32 internal constant _DOMAIN_SEPARATOR_TYPEHASH_MODULE = + 0x71062c282d40422f744945d587dbf4ecfd4f9cfad1d35d62c944373009d96162; + + // keccak256("EIP712Domain(uint256 chainId,address verifyingContract)") + bytes32 internal constant _DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT = + 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218; + + // keccak256(abi.encode("")) + bytes32 internal constant _HASH_EMPTY_BYTES = + 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470; + + /// @notice Helper function to obtain the ERC-7739 digest on an incoming EIP-712 digest + /// @dev This should be used in accounts + /// @param account The account address + /// @param hash The incoming app digest. This should be generated via a EIP-712 process + /// @param signature The full ERC-7739 signature + /// @return bytes32 The ERC-7739 digest + /// @return bytes The inner signature + function validateERC7739SigFormatForAccount(address account, bytes32 hash, bytes calldata signature) + internal + view + returns (bytes32, bytes calldata) + { + TypeStruct t; + (t, hash, signature) = _validateERC7739SigFormat(typedDataSignFieldsForAccount(account), hash, signature); + if (isZero(t)) hash = hashTypedDataForAccount(account, hash); // `PersonalSign` workflow. + return (hash, signature); + } + + /// @notice Helper function to obtain and arrange the eip712 fields for an account + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L283 + /// which is based on + /// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L253 + /// @param account The account address + /// @return m The location of all the types + function typedDataSignFieldsForAccount(address account) internal view returns (TypeStruct m) { + bytes1 fields = bytes1(hex"0C"); // 001100 + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = address(this); + // !bytes32 salt; + // !uint256[] memory extensions; + assembly ("memory-safe") { + m := mload(0x40) // Grab the free memory pointer. + mstore(0x40, add(m, 0x120)) // Allocate the memory. + // Skip 2 words for the `typedDataSignTypehash` and `contents` struct hash. + mstore(add(m, 0x40), shl(248, fields)) + mstore(add(m, 0x60), _HASH_EMPTY_BYTES) + mstore(add(m, 0x80), _HASH_EMPTY_BYTES) + mstore(add(m, 0xa0), chainid()) + mstore(add(m, 0xc0), account) + mstore(add(m, 0xe0), 0) + mstore(add(m, 0x100), _HASH_EMPTY_BYTES) + } + } + + /// @notice Helper function to build the full personal sign hash, for an account + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L312 + /// @param account The account address + /// @param structHash the hashStruct of arguments provided for an EIP-712 struct + /// @return digest The ERC-7739 digest + function hashTypedDataForAccount(address account, bytes32 structHash) internal view returns (bytes32 digest) { + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = address(this); + // !bytes32 salt; + // !uint256[] memory extensions; + /// @solidity memory-safe-assembly + assembly { + //Rebuild domain separator out of 712 domain + let m := mload(0x40) // Load the free memory pointer. + mstore(m, _DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT) + mstore(add(m, 0x20), _HASH_EMPTY_BYTES) // Name hash. + mstore(add(m, 0x40), _HASH_EMPTY_BYTES) // Version hash. + mstore(add(m, 0x60), chainid()) + mstore(add(m, 0x80), account) + digest := keccak256(m, 0xa0) //domain separator + + // Hash typed data + mstore(m, 0x1901) // Store "\x19\x01". + mstore(add(m, 0x20), digest) // Store the domain separator. + mstore(add(m, 0x40), structHash) // Store the struct hash. + digest := keccak256(add(m, 0x1e), 0x42) + } + } + + /// @notice Helper function to obtain the ERC-7739 digest on an incoming EIP-712 digest + /// @dev This should be used in modules + /// @param account The account address + /// @param module The module address + /// @param hash The incoming app digest. This should be generated via a EIP-712 process + /// @param signature The full ERC-7739 signature + /// @return bytes32 The ERC-7739 digest + /// @return bytes The inner signature + function validateERC7739SigFormatForModule( + address account, + address module, + bytes32 hash, + bytes calldata signature + ) internal view returns (bytes32, bytes calldata) { + TypeStruct t; + (t, hash, signature) = + _validateERC7739SigFormat(typedDataSignFieldsForModule(account, module), hash, signature); + if (isZero(t)) hash = hashTypedDataForModule(account, module, hash); // `PersonalSign` workflow. + return (hash, signature); + } + + /// @notice Helper function to obtain and arrange the eip712 fields for a module + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L283 + /// which is based on + /// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L253 + /// @param account The account address + /// @param module The module address + /// @return m The location of all the types + function typedDataSignFieldsForModule(address account, address module) internal view returns (TypeStruct m) { + bytes1 fields = bytes1(hex"0E"); // 001110 + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = module; + // bytes32 salt = account; + // !uint256[] memory extensions; + /// @solidity memory-safe-assembly + assembly { + m := mload(0x40) // Grab the free memory pointer. + mstore(0x40, add(m, 0x120)) // Allocate the memory. + // Skip 2 words for the `typedDataSignTypehash` and `contents` struct hash. + mstore(add(m, 0x40), shl(248, fields)) + mstore(add(m, 0x60), _HASH_EMPTY_BYTES) + mstore(add(m, 0x80), _HASH_EMPTY_BYTES) + mstore(add(m, 0xa0), chainid()) + mstore(add(m, 0xc0), module) + mstore(add(m, 0xe0), account) + mstore(add(m, 0x100), _HASH_EMPTY_BYTES) + } + } + + /// @notice Helper function to build the full personal sign hash for a module + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L312 + /// @param account The account address + /// @param module The module address + /// @param structHash the hashStruct of arguments provided for an EIP-712 struct + /// @return digest The ERC-7739 digest + function hashTypedDataForModule(address account, address module, bytes32 structHash) + internal + view + returns (bytes32 digest) + { + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = address(this); + // bytes32 salt = account; + // !uint256[] memory extensions; + /// @solidity memory-safe-assembly + assembly { + //Rebuild domain separator out of 712 domain + let m := mload(0x40) // Load the free memory pointer. + mstore(m, _DOMAIN_SEPARATOR_TYPEHASH_MODULE) + mstore(add(m, 0x20), _HASH_EMPTY_BYTES) // Name hash. + mstore(add(m, 0x40), _HASH_EMPTY_BYTES) // Version hash. + mstore(add(m, 0x60), chainid()) + mstore(add(m, 0x80), module) + mstore(add(m, 0xa0), account) + digest := keccak256(m, 0xa0) //domain separator + + // Hash typed data + mstore(m, 0x1901) // Store "\x19\x01". + mstore(add(m, 0x20), digest) // Store the domain separator. + mstore(add(m, 0x40), structHash) // Store the struct hash. + digest := keccak256(add(m, 0x1e), 0x42) + } + } + + function isZero(TypeStruct t) internal pure returns (bool) { + return TypeStruct.unwrap(t) == bytes32(0); + } + + /// @notice Helper function to validate ERC7739 compatible nested EIP712 structs to guard against signature + /// replay + /// @dev Parses out the inner signature from the full signature and returns it + /// @dev Implementation is lifted from + /// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L191 + /// @dev Also see: + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L22 + /// @param t Type to use + /// @param hash The incoming app digest. This should be generated through an EIP-712 process + /// @return TypeStruct the location of all the types + /// @return bytes32 Replay-safe hash, computed by wrapping the input hash in an EIP-712 struct. + /// @return bytes Inner signature to use for verification. + function _validateERC7739SigFormat(TypeStruct t, bytes32 hash, bytes calldata signature) + private + pure + returns (TypeStruct, bytes32, bytes calldata) + { + /// @solidity memory-safe-assembly + assembly { + let m := mload(0x40) // Cache the free memory pointer since we overwrite it below. + // `c` is `contentsType.length`, which is stored in the last 2 bytes of the signature. + let c := shr(240, calldataload(add(signature.offset, sub(signature.length, 2)))) + for {} 1 {} { + let l := add(0x42, c) // Total length of appended data (32 + 32 + c + 2). + let o := add(signature.offset, sub(signature.length, l)) // Offset of appended data. + mstore(0x00, 0x1901) // Store the "\x19\x01" prefix. + calldatacopy(0x20, o, 0x40) // Copy the `APP_DOMAIN_SEPARATOR` and `contents` struct hash. + + // Check the reconstructed hash doesn't match or if the appended data is invalid, i.e. + // `appendedData.length > signature.length || contentsType.length == 0`. If so, we use + // the `PersonalSign` workflow. + if or(xor(keccak256(0x1e, 0x42), hash), or(lt(signature.length, l), iszero(c))) { + t := 0 // Set `t` to 0, denoting that we need to `hash = _hashTypedData(hash)`. + mstore(t, _PERSONAL_SIGN_TYPEHASH) + mstore(0x20, hash) // Store the `prefixed`. + hash := keccak256(t, 0x40) // Compute the `PersonalSign` struct hash. + break + } + + // Else, use the `TypedDataSign` workflow. Here, we build the full 7739 type which has form + // `TypedDataSign({ContentsName} contents,bytes1 fields,...){ContentsType}`. + // E.g. for Permit(uint256 nonce,address spender,uint256 expiry,bool allowed), "Permit" is + // the ContentsName and the full string is the ContentsType. + mstore(m, "TypedDataSign(") // Store the start of `TypedDataSign`'s type encoding. + let p := add(m, 0x0e) // Advance 14 bytes to skip "TypedDataSign(". + calldatacopy(p, add(o, 0x40), c) // Copy `contentsType` to extract `contentsName`. + // `d & 1 == 1` means that `contentsName` is invalid. + let d := shr(byte(0, mload(p)), 0x7fffffe000000000000010000000000) // Starts with `[a-z(]`. + // Store the end sentinel '(', and advance `p` until we encounter a '(' byte. + for { mstore(add(p, c), 40) } iszero(eq(byte(0, mload(p)), 40)) { p := add(p, 1) } { + d := or(shr(byte(0, mload(p)), 0x120100000001), d) // Has a byte in ", )\x00". + } + mstore(p, " contents,bytes1 fields,string n") // Store the rest of the encoding. + mstore(add(p, 0x20), "ame,string version,uint256 chain") + mstore(add(p, 0x40), "Id,address verifyingContract,byt") + mstore(add(p, 0x60), "es32 salt,uint256[] extensions)") + p := add(p, 0x7f) + calldatacopy(p, add(o, 0x40), c) // Copy `contentsType`. + + // We then build and hash the ERC-7739 digest + // Fill in the missing fields of the `TypedDataSign`. + calldatacopy(t, o, 0x40) // Copy the `contents` struct hash to `add(t, 0x20)`. + mstore(t, keccak256(m, sub(add(p, c), m))) // Store `typedDataSignTypehash`. + // The "\x19\x01" prefix is already at 0x00. + // `APP_DOMAIN_SEPARATOR` is already at 0x20. + mstore(0x40, keccak256(t, 0x120)) // `hashStruct(typedDataSign)`. + // Compute the final hash, corrupted if `contentsName` is invalid. + hash := keccak256(0x1e, add(0x42, and(1, d))) + signature.length := sub(signature.length, l) // Truncate the signature. + break + } + mstore(0x40, m) // Restore the free memory pointer. + } + return (t, hash, signature); + } +} diff --git a/src/libraries/ERC7739ReplaySafeWrapper.sol b/src/libraries/ERC7739ReplaySafeWrapper.sol new file mode 100644 index 000000000..7deeafc3f --- /dev/null +++ b/src/libraries/ERC7739ReplaySafeWrapper.sol @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.26; + +// A library for ERC7739-compliant nested EIP-712 wrappers over EIP-1271 digests. +// This allows for efficient, readable ERC-1271 signature schemes for smart contract accounts. +library ERC7739ReplaySafeWrapper { + type TypeStruct is bytes32; + + /// @dev `keccak256("PersonalSign(bytes prefixed)")`. + bytes32 internal constant _PERSONAL_SIGN_TYPEHASH = + 0x983e65e5148e570cd828ead231ee759a8d7958721a768f93bc4483ba005c32de; + + // The difference between a module hash and an account hash is: + // Account domains include only chainId and verifyingContract of itself (not the implementation) + // Module domains include chainId, verifyingContract of the module, and uses the optional salt param, using the + // account address + + // keccak256("EIP712Domain(uint256 chainId,address verifyingContract,bytes32 salt)") + bytes32 internal constant _DOMAIN_SEPARATOR_TYPEHASH_MODULE = + 0x71062c282d40422f744945d587dbf4ecfd4f9cfad1d35d62c944373009d96162; + + // keccak256("EIP712Domain(uint256 chainId,address verifyingContract)") + bytes32 internal constant _DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT = + 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218; + + // keccak256("") + bytes32 internal constant _HASH_EMPTY_BYTES = + 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470; + + /// @notice Helper function to obtain the ERC-7739 digest on an incoming EIP-712 digest + /// @dev This should be used in accounts + /// @param account The account address + /// @param hash The incoming app digest. This should be generated via a EIP-712 process + /// @param signature The full ERC-7739 signature + /// @return bytes32 The ERC-7739 digest + /// @return bytes The inner signature + function validateERC7739SigFormatForAccount(address account, bytes32 hash, bytes calldata signature) + internal + view + returns (bytes32, bytes calldata) + { + TypeStruct t; + (t, hash, signature) = _validateERC7739SigFormat(typedDataSignFieldsForAccount(account), hash, signature); + if (isZero(t)) hash = hashTypedDataForAccount(account, hash); // `PersonalSign` workflow. + return (hash, signature); + } + + /// @notice Helper function to obtain and arrange the eip712 fields for an account + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L283 + /// which is based on + /// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L253 + /// @param account The account address + /// @return m The location of all the types + function typedDataSignFieldsForAccount(address account) internal view returns (TypeStruct m) { + bytes1 fields = bytes1(hex"0C"); // 001100 + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = address(this); + // !bytes32 salt; + // !uint256[] memory extensions; + assembly ("memory-safe") { + m := mload(0x40) // Grab the free memory pointer. + mstore(0x40, add(m, 0x120)) // Allocate the memory. + // Skip 2 words for the `typedDataSignTypehash` and `contents` struct hash. + mstore(add(m, 0x40), fields) + mstore(add(m, 0x60), _HASH_EMPTY_BYTES) + mstore(add(m, 0x80), _HASH_EMPTY_BYTES) + mstore(add(m, 0xa0), chainid()) + mstore(add(m, 0xc0), account) + mstore(add(m, 0xe0), 0) + mstore(add(m, 0x100), _HASH_EMPTY_BYTES) + } + } + + /// @notice Helper function to build the full personal sign hash, for an account + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L312 + /// @param account The account address + /// @param structHash the hashStruct of arguments provided for an EIP-712 struct + /// @return digest The ERC-7739 digest + function hashTypedDataForAccount(address account, bytes32 structHash) internal view returns (bytes32 digest) { + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = address(this); + // !bytes32 salt; + // !uint256[] memory extensions; + /// @solidity memory-safe-assembly + assembly { + //Rebuild domain separator out of 712 domain + let m := mload(0x40) // Load the free memory pointer. + mstore(m, _DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT) + mstore(add(m, 0x20), _HASH_EMPTY_BYTES) // Name hash. + mstore(add(m, 0x40), _HASH_EMPTY_BYTES) // Version hash. + mstore(add(m, 0x60), chainid()) + mstore(add(m, 0x80), account) + digest := keccak256(m, 0xa0) //domain separator + + // Hash typed data + mstore(m, 0x1901) // Store "\x19\x01". + mstore(add(m, 0x20), digest) // Store the domain separator. + mstore(add(m, 0x40), structHash) // Store the struct hash. + digest := keccak256(add(m, 0x1e), 0x42) + } + } + + /// @notice Helper function to obtain the ERC-7739 digest on an incoming EIP-712 digest + /// @dev This should be used in modules + /// @param account The account address + /// @param module The module address + /// @param hash The incoming app digest. This should be generated via a EIP-712 process + /// @param signature The full ERC-7739 signature + /// @return bytes32 The ERC-7739 digest + /// @return bytes The inner signature + function validateERC7739SigFormatForModule( + address account, + address module, + bytes32 hash, + bytes calldata signature + ) internal view returns (bytes32, bytes calldata) { + TypeStruct t; + (t, hash, signature) = + _validateERC7739SigFormat(typedDataSignFieldsForModule(account, module), hash, signature); + if (isZero(t)) hash = hashTypedDataForModule(account, module, hash); // `PersonalSign` workflow. + return (hash, signature); + } + + /// @notice Helper function to obtain and arrange the eip712 fields for a module + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L283 + /// which is based on + /// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L253 + /// @param account The account address + /// @param module The module address + /// @return m The location of all the types + function typedDataSignFieldsForModule(address account, address module) internal view returns (TypeStruct m) { + bytes1 fields = bytes1(hex"0E"); // 001110 + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = module; + // bytes32 salt = account; + // !uint256[] memory extensions; + /// @solidity memory-safe-assembly + assembly { + m := mload(0x40) // Grab the free memory pointer. + mstore(0x40, add(m, 0x120)) // Allocate the memory. + // Skip 2 words for the `typedDataSignTypehash` and `contents` struct hash. + mstore(add(m, 0x40), fields) + mstore(add(m, 0x60), _HASH_EMPTY_BYTES) + mstore(add(m, 0x80), _HASH_EMPTY_BYTES) + mstore(add(m, 0xa0), chainid()) + mstore(add(m, 0xc0), module) + mstore(add(m, 0xe0), account) + mstore(add(m, 0x100), _HASH_EMPTY_BYTES) + } + } + + /// @notice Helper function to build the full personal sign hash for a module + /// @dev From + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L312 + /// @param account The account address + /// @param module The module address + /// @param structHash the hashStruct of arguments provided for an EIP-712 struct + /// @return digest The ERC-7739 digest + function hashTypedDataForModule(address account, address module, bytes32 structHash) + internal + view + returns (bytes32 digest) + { + // !string memory name; + // !string memory version; + // uint256 chainId = chainId; + // address verifyingContract = address(this); + // bytes32 salt = account; + // !uint256[] memory extensions; + /// @solidity memory-safe-assembly + assembly { + //Rebuild domain separator out of 712 domain + let m := mload(0x40) // Load the free memory pointer. + mstore(m, _DOMAIN_SEPARATOR_TYPEHASH_MODULE) + mstore(add(m, 0x20), _HASH_EMPTY_BYTES) // Name hash. + mstore(add(m, 0x40), _HASH_EMPTY_BYTES) // Version hash. + mstore(add(m, 0x60), chainid()) + mstore(add(m, 0x80), module) + mstore(add(m, 0xa0), account) + digest := keccak256(m, 0xa0) //domain separator + + // Hash typed data + mstore(m, 0x1901) // Store "\x19\x01". + mstore(add(m, 0x20), digest) // Store the domain separator. + mstore(add(m, 0x40), structHash) // Store the struct hash. + digest := keccak256(add(m, 0x1e), 0x42) + } + } + + function isZero(TypeStruct t) internal pure returns (bool) { + return TypeStruct.unwrap(t) == bytes32(0); + } + + /// @notice Helper function to validate ERC7739 compatible nested EIP712 structs to guard against signature + /// replay + /// @dev Parses out the inner signature from the full signature and returns it + /// @dev Implementation is lifted from + /// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L191 + /// @dev Also see: + /// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L22 + /// @param t Type to use + /// @param hash The incoming app digest. This should be generated through an EIP-712 process + /// @return TypeStruct the location of all the types + /// @return bytes32 Replay-safe hash, computed by wrapping the input hash in an EIP-712 struct. + /// @return bytes Inner signature to use for verification. + function _validateERC7739SigFormat(TypeStruct t, bytes32 hash, bytes calldata signature) + private + pure + returns (TypeStruct, bytes32, bytes calldata) + { + /// @solidity memory-safe-assembly + assembly { + let m := mload(0x40) // Cache the free memory pointer since we overwrite it below. + // `c` is `contentsType.length`, which is stored in the last 2 bytes of the signature. + let c := shr(240, calldataload(add(signature.offset, sub(signature.length, 2)))) + for {} 1 {} { + let l := add(0x42, c) // Total length of appended data (32 + 32 + c + 2). + let o := add(signature.offset, sub(signature.length, l)) // Offset of appended data. + mstore(0x00, 0x1901) // Store the "\x19\x01" prefix. + calldatacopy(0x20, o, 0x40) // Copy the `APP_DOMAIN_SEPARATOR` and `contents` struct hash. + + // Check the reconstructed hash doesn't match or if the appended data is invalid, i.e. + // `appendedData.length > signature.length || contentsType.length == 0`. If so, we use + // the `PersonalSign` workflow. + if or(xor(keccak256(0x1e, 0x42), hash), or(lt(signature.length, l), iszero(c))) { + t := 0 // Set `t` to 0, denoting that we need to `hash = _hashTypedData(hash)`. + mstore(t, _PERSONAL_SIGN_TYPEHASH) + mstore(0x20, hash) // Store the `prefixed`. + hash := keccak256(t, 0x40) // Compute the `PersonalSign` struct hash. + break + } + + // Else, use the `TypedDataSign` workflow. Here, we build the full 7739 type which has form + // `TypedDataSign({ContentsName} contents,bytes1 fields,...){ContentsType}`. + // E.g. for Permit(uint256 nonce,address spender,uint256 expiry,bool allowed), "Permit" is + // the ContentsName and the full string is the ContentsType. + mstore(m, "TypedDataSign(") // Store the start of `TypedDataSign`'s type encoding. + let p := add(m, 0x0e) // Advance 14 bytes to skip "TypedDataSign(". + calldatacopy(p, add(o, 0x40), c) // Copy `contentsType` to extract `contentsName`. + // `d & 1 == 1` means that `contentsName` is invalid. + let d := shr(byte(0, mload(p)), 0x7fffffe000000000000010000000000) // Starts with `[a-z(]`. + // Store the end sentinel '(', and advance `p` until we encounter a '(' byte. + for { mstore(add(p, c), 40) } iszero(eq(byte(0, mload(p)), 40)) { p := add(p, 1) } { + d := or(shr(byte(0, mload(p)), 0x120100000001), d) // Has a byte in ", )\x00". + } + mstore(p, " contents,bytes1 fields,string n") // Store the rest of the encoding. + mstore(add(p, 0x20), "ame,string version,uint256 chain") + mstore(add(p, 0x40), "Id,address verifyingContract,byt") + mstore(add(p, 0x60), "es32 salt,uint256[] extensions)") + p := add(p, 0x7f) + calldatacopy(p, add(o, 0x40), c) // Copy `contentsType`. + + // We then build and hash the ERC-7739 digest + // Fill in the missing fields of the `TypedDataSign`. + calldatacopy(t, o, 0x40) // Copy the `contents` struct hash to `add(t, 0x20)`. + mstore(t, keccak256(m, sub(add(p, c), m))) // Store `typedDataSignTypehash`. + // The "\x19\x01" prefix is already at 0x00. + // `APP_DOMAIN_SEPARATOR` is already at 0x20. + mstore(0x40, keccak256(t, 0x120)) // `hashStruct(typedDataSign)`. + // Compute the final hash, corrupted if `contentsName` is invalid. + hash := keccak256(0x1e, add(0x42, and(1, d))) + signature.length := sub(signature.length, l) // Truncate the signature. + break + } + mstore(0x40, m) // Restore the free memory pointer. + } + return (t, hash, signature); + } +} diff --git a/src/libraries/SemiModularKnownSelectorsLib.sol b/src/libraries/SemiModularKnownSelectorsLib.sol index b84d6cb08..584a26f87 100644 --- a/src/libraries/SemiModularKnownSelectorsLib.sol +++ b/src/libraries/SemiModularKnownSelectorsLib.sol @@ -12,7 +12,6 @@ library SemiModularKnownSelectorsLib { || selector == SemiModularAccountBase.updateFallbackSigner.selector || selector == SemiModularAccountBase.setFallbackSignerDisabled.selector || selector == SemiModularAccountBase.isFallbackSignerDisabled.selector - || selector == SemiModularAccountBase.getFallbackSigner.selector - || selector == SemiModularAccountBase.replaySafeHash.selector; + || selector == SemiModularAccountBase.getFallbackSigner.selector; } } diff --git a/src/modules/validation/ECDSAValidationModule.sol b/src/modules/validation/ECDSAValidationModule.sol index b8700be48..12acbda79 100644 --- a/src/modules/validation/ECDSAValidationModule.sol +++ b/src/modules/validation/ECDSAValidationModule.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.26; -import {ReplaySafeWrapper} from "@erc6900/reference-implementation/modules/ReplaySafeWrapper.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; @@ -12,6 +11,7 @@ import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol" import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; import {SignatureType} from "../../helpers/SignatureType.sol"; +import {ERC7739ReplaySafeWrapper} from "../../libraries/ERC7739ReplaySafeWrapper.sol"; import {BaseModule} from "../BaseModule.sol"; /// @title ECSDA Validation @@ -26,8 +26,9 @@ import {BaseModule} from "../BaseModule.sol"; /// - This validation supports ERC-1271. The signature is valid if it is signed by the owner's private key. /// - This validation supports composition that other validation can relay on entities in this validation /// to validate partially or fully. -contract ECDSAValidationModule is IValidationModule, ReplaySafeWrapper, BaseModule { +contract ECDSAValidationModule is IValidationModule, BaseModule { using MessageHashUtils for bytes32; + using ERC7739ReplaySafeWrapper for address; uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; @@ -111,8 +112,8 @@ contract ECDSAValidationModule is IValidationModule, ReplaySafeWrapper, BaseModu override returns (bytes4) { - bytes32 _replaySafeHash = replaySafeHash(account, digest); - if (_checkSig(signers[entityId][account], _replaySafeHash, signature)) { + (digest, signature) = account.validateERC7739SigFormatForModule(address(this), digest, signature); + if (_checkSig(signers[entityId][account], digest, signature)) { return _1271_MAGIC_VALUE; } return _1271_INVALID; diff --git a/src/modules/validation/WebauthnValidationModule.sol b/src/modules/validation/WebauthnValidationModule.sol index 86f02d307..89341208e 100644 --- a/src/modules/validation/WebauthnValidationModule.sol +++ b/src/modules/validation/WebauthnValidationModule.sol @@ -9,7 +9,8 @@ import {WebAuthn} from "webauthn-sol/src/WebAuthn.sol"; import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; -import {BaseModule} from "@erc6900/reference-implementation/modules/BaseModule.sol"; + +import {BaseModule} from "../BaseModule.sol"; /// @title Webauthn Validation /// @author Alchemy diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index 56f09ee06..46227cfc9 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -28,6 +28,7 @@ contract AccountExecHooksTest is AccountTestBase { event ReceivedCall(bytes msgData, uint256 msgValue); function setUp() public override { + _revertSnapshot = vm.snapshot(); _allowTestDirectCalls(); _m1.executionFunctions.push( diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index e48e6a97f..7f8077c18 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -21,6 +21,7 @@ contract AccountReturnDataTest is AccountTestBase { ResultConsumerModule public resultConsumerModule; function setUp() public override { + _revertSnapshot = vm.snapshot(); _transferOwnershipToTest(); regularResultContract = new RegularResultContract(); diff --git a/test/account/DeferredValidation.t.sol b/test/account/DeferredValidation.t.sol index 83c495e68..21969fd50 100644 --- a/test/account/DeferredValidation.t.sol +++ b/test/account/DeferredValidation.t.sol @@ -29,6 +29,7 @@ contract DeferredValidationTest is AccountTestBase { uint256 internal _newSignerKey; function setUp() public override { + _revertSnapshot = vm.snapshot(); _encodedCall = abi.encodeCall(ModularAccountBase.execute, (makeAddr("dead"), 0, "")); _deferredValidation = ModuleEntityLib.pack(address(_deployECDSAValidationModule()), 0); uint32 entityId = 0; diff --git a/test/account/DirectCallsFromModule.t.sol b/test/account/DirectCallsFromModule.t.sol index 4e4a96848..1ccfc76fb 100644 --- a/test/account/DirectCallsFromModule.t.sol +++ b/test/account/DirectCallsFromModule.t.sol @@ -35,6 +35,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { } function setUp() public override { + _revertSnapshot = vm.snapshot(); _module = new DirectCallModule(); assertFalse(_module.preHookRan()); assertFalse(_module.postHookRan()); diff --git a/test/account/GlobalValidationTest.t.sol b/test/account/GlobalValidationTest.t.sol index 895695380..6c149b7b3 100644 --- a/test/account/GlobalValidationTest.t.sol +++ b/test/account/GlobalValidationTest.t.sol @@ -21,6 +21,7 @@ contract GlobalValidationTest is AccountTestBase { ModularAccount public account2; function setUp() public override { + _revertSnapshot = vm.snapshot(); (owner2, owner2Key) = makeAddrAndKey("owner2"); // Compute counterfactual address diff --git a/test/account/HookOrdering.t.sol b/test/account/HookOrdering.t.sol index ab431b48d..cf5352048 100644 --- a/test/account/HookOrdering.t.sol +++ b/test/account/HookOrdering.t.sol @@ -37,6 +37,7 @@ contract HookOrderingTest is AccountTestBase { ModuleEntity public orderCheckerValidationEntity; function setUp() public override { + _revertSnapshot = vm.snapshot(); hookOrderChecker = new HookOrderCheckerModule(); } @@ -337,7 +338,8 @@ contract HookOrderingTest is AccountTestBase { // because it will be invoked with `staticcall`, so we call `isValidSignature` directly with `call`. bytes memory callData = abi.encodeCall( - account1.isValidSignature, (bytes32(0), _encode1271Signature(orderCheckerValidationEntity, hex"")) + account1.isValidSignature, + (bytes32(0), _encode1271Signature(orderCheckerValidationEntity, hex"", bytes32(0))) ); (bool success,) = address(account1).call(callData); diff --git a/test/account/ModularAccount.t.sol b/test/account/ModularAccount.t.sol index 42f32b6b8..6fd8e0141 100644 --- a/test/account/ModularAccount.t.sol +++ b/test/account/ModularAccount.t.sol @@ -47,6 +47,7 @@ contract ModularAccountTest is AccountTestBase { event ReceivedCall(bytes msgData, uint256 msgValue); function setUp() public override { + _revertSnapshot = vm.snapshot(); mockExecutionInstallationModule = new MockExecutionInstallationModule(); (owner2, owner2Key) = makeAddrAndKey("owner2"); @@ -422,16 +423,27 @@ contract ModularAccountTest is AccountTestBase { function test_isValidSignature() public withSMATest { bytes32 message = keccak256("hello world"); + (bytes32 mockAppStructHash, bytes32 mockAppDigest) = _getMockApp712Contents(message); bytes32 replaySafeHash = _isSMATest - ? SemiModularAccountBytecode(payable(account1)).replaySafeHash(message) - : ecdsaValidationModule.replaySafeHash(address(account1), message); + ? _getSMAReplaySafeHash( + address(account1), _MOCK_APP_DOMAIN, mockAppStructHash, mockAppDigest, _MOCK_APP_CONTENTS_TYPE + ) + : _getModuleReplaySafeHash( + address(account1), + address(ecdsaValidationModule), + _MOCK_APP_DOMAIN, + mockAppStructHash, + mockAppDigest, + _MOCK_APP_CONTENTS_TYPE + ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, replaySafeHash); - bytes memory signature = - _encode1271Signature(_signerValidation, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)); + bytes memory signature = _encode1271Signature( + _signerValidation, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v), mockAppStructHash + ); - bytes4 validationResult = IERC1271(address(account1)).isValidSignature(message, signature); + bytes4 validationResult = IERC1271(address(account1)).isValidSignature(mockAppDigest, signature); assertEq(validationResult, bytes4(0x1626ba7e)); } @@ -449,15 +461,26 @@ contract ModularAccountTest is AccountTestBase { ); bytes32 message = keccak256("hello world"); - + (bytes32 mockAppStructHash, bytes32 mockAppDigest) = _getMockApp712Contents(message); bytes32 replaySafeHash = _isSMATest - ? SemiModularAccountBytecode(payable(account1)).replaySafeHash(message) - : ecdsaValidationModule.replaySafeHash(address(account1), message); + ? _getSMAReplaySafeHash( + address(account1), _MOCK_APP_DOMAIN, mockAppStructHash, mockAppDigest, _MOCK_APP_CONTENTS_TYPE + ) + : _getModuleReplaySafeHash( + address(account1), + address(ecdsaValidationModule), + _MOCK_APP_DOMAIN, + mockAppStructHash, + mockAppDigest, + _MOCK_APP_CONTENTS_TYPE + ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, replaySafeHash); bytes memory signature = _encode1271Signature( - ModuleEntityLib.pack(address(ecdsaValidationModule), newEntityId), abi.encodePacked(r, s, v) + ModuleEntityLib.pack(address(ecdsaValidationModule), newEntityId), + abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v), + message ); vm.expectRevert( diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol index ab582a690..9b778bacd 100644 --- a/test/account/ModularAccountView.t.sol +++ b/test/account/ModularAccountView.t.sol @@ -25,6 +25,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { ModuleEntity public comprehensiveModuleValidation; function setUp() public override { + _revertSnapshot = vm.snapshot(); comprehensiveModule = new ComprehensiveModule(); comprehensiveModuleValidation = ModuleEntityLib.pack(address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.VALIDATION)); diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index d01f6a131..fc29bcab7 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -25,6 +25,7 @@ contract MultiValidationTest is AccountTestBase { uint256 public owner2Key; function setUp() public override { + _revertSnapshot = vm.snapshot(); validator2 = new ECDSAValidationModule(); (owner2, owner2Key) = makeAddrAndKey("owner2"); diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index 2e9110eab..502827a9b 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -28,6 +28,7 @@ contract PerHookDataTest is CustomValidationTestBase { uint32 internal constant _PRE_HOOK_ENTITY_ID_2 = 1; function setUp() public override { + _revertSnapshot = vm.snapshot(); _counter = new Counter(); _accessControlHookModule = new MockAccessControlHookModule(); @@ -400,20 +401,35 @@ contract PerHookDataTest is CustomValidationTestBase { } function test_pass1271AccessControl() public withSMATest { - string memory message = "Hello, world!"; - - bytes32 messageHash = keccak256(abi.encodePacked(message)); + bytes32 messageHash = keccak256(abi.encodePacked("Hello, world!")); + + (bytes32 mockAppStructHash, bytes32 mockAppDigest) = _getMockApp712Contents(messageHash); + // we use module validation for both cases + bytes32 replaySafeHash = _getModuleReplaySafeHash( + address(account1), + address(ecdsaValidationModule), + _MOCK_APP_DOMAIN, + mockAppStructHash, + mockAppDigest, + _MOCK_APP_CONTENTS_TYPE + ); - bytes32 replaySafeHash = ecdsaValidationModule.replaySafeHash(address(account1), messageHash); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, replaySafeHash); PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); - preValidationHookData[0] = PreValidationHookData({index: 0, validationData: abi.encodePacked(message)}); + preValidationHookData[0] = PreValidationHookData({ + index: 0, + validationData: abi.encodePacked(hex"1901", _MOCK_APP_DOMAIN, mockAppStructHash) // preimage of + // appDigest + }); bytes4 result = account1.isValidSignature( - messageHash, + mockAppDigest, _encode1271Signature( - _signerValidation, preValidationHookData, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v) + _signerValidation, + preValidationHookData, + abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v), + mockAppStructHash ) ); @@ -437,7 +453,10 @@ contract PerHookDataTest is CustomValidationTestBase { account1.isValidSignature( messageHash, _encode1271Signature( - _signerValidation, preValidationHookData, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v) + _signerValidation, + preValidationHookData, + abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v), + messageHash ) ); } @@ -450,7 +469,9 @@ contract PerHookDataTest is CustomValidationTestBase { (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, messageHash); vm.expectRevert("Preimage not provided"); - account1.isValidSignature(messageHash, _encode1271Signature(_signerValidation, abi.encodePacked(r, s, v))); + account1.isValidSignature( + messageHash, _encode1271Signature(_signerValidation, abi.encodePacked(r, s, v), messageHash) + ); } function _installSecondPreHook() internal { diff --git a/test/account/PermittedCallPermissions.t.sol b/test/account/PermittedCallPermissions.t.sol index bce71affa..60b5b5cd6 100644 --- a/test/account/PermittedCallPermissions.t.sol +++ b/test/account/PermittedCallPermissions.t.sol @@ -13,6 +13,7 @@ contract PermittedCallPermissionsTest is AccountTestBase { PermittedCallerModule public permittedCallerModule; function setUp() public override { + _revertSnapshot = vm.snapshot(); _transferOwnershipToTest(); resultCreatorModule = new ResultCreatorModule(); diff --git a/test/account/ReplaceModule.t.sol b/test/account/ReplaceModule.t.sol index 787ad5c57..cf3826848 100644 --- a/test/account/ReplaceModule.t.sol +++ b/test/account/ReplaceModule.t.sol @@ -33,6 +33,10 @@ contract UpgradeModuleTest is AccountTestBase { // From MockModule event ReceivedCall(bytes msgData, uint256 msgValue); + function setUp() public override { + _revertSnapshot = vm.snapshot(); + } + function test_upgradeModuleExecutionFunction() public withSMATest { ExecutionManifest memory m; ManifestExecutionFunction[] memory executionFunctions = new ManifestExecutionFunction[](1); @@ -98,7 +102,7 @@ contract UpgradeModuleTest is AccountTestBase { } function test_upgradeModuleValidationFunction() public withSMATest { - // Setup new validaiton with pre validation and execution hooks associated with a validator + // Setup new validation with pre validation and execution hooks associated with a validator ECDSAValidationModule validation1 = new ECDSAValidationModule(); ECDSAValidationModule validation2 = new ECDSAValidationModule(); uint32 validationEntityId1 = 10; diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index c643a4a81..5f52aef6e 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -20,6 +20,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ModuleEntity public comprehensiveModuleValidation; function setUp() public override { + _revertSnapshot = vm.snapshot(); // install the comprehensive module to get new exec functions with different validations configured. comprehensiveModule = new ComprehensiveModule(); diff --git a/test/account/SemiModularAccountDirectCall.t.sol b/test/account/SemiModularAccountDirectCall.t.sol index b29dd0982..535c22e62 100644 --- a/test/account/SemiModularAccountDirectCall.t.sol +++ b/test/account/SemiModularAccountDirectCall.t.sol @@ -21,6 +21,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { ModuleEntity internal _fallbackDirectCallModuleEntity; function setUp() public override { + _revertSnapshot = vm.snapshot(); _module = new MockSMADirectFallbackModule(); _directCallModuleEntity = ModuleEntityLib.pack(address(_module), DIRECT_CALL_VALIDATION_ENTITYID); diff --git a/test/account/TokenReceiver.t.sol b/test/account/TokenReceiver.t.sol index b4d968041..3392a5718 100644 --- a/test/account/TokenReceiver.t.sol +++ b/test/account/TokenReceiver.t.sol @@ -15,6 +15,7 @@ contract TokenReceiverTest is AccountTestBase { uint256 internal constant _NFT_TOKEN_COUNT = 10; function setUp() public override { + _revertSnapshot = vm.snapshot(); // Compute counterfactual address // account1 = factory.createAccount(owner1, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID); // vm.deal(address(account1), 100 ether); diff --git a/test/account/UpgradeToSma.t.sol b/test/account/UpgradeToSma.t.sol index aacf2e232..d783364d0 100644 --- a/test/account/UpgradeToSma.t.sol +++ b/test/account/UpgradeToSma.t.sol @@ -29,6 +29,7 @@ contract UpgradeToSmaTest is AccountTestBase { uint256 public transferAmount; function setUp() public override { + _revertSnapshot = vm.snapshot(); smaStorageImpl = address(new SemiModularAccountStorageOnly(entryPoint)); (owner2, owner2Key) = makeAddrAndKey("owner2"); transferAmount = 0.1 ether; diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 33f706e44..65da007ed 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -29,6 +29,7 @@ contract ValidationIntersectionTest is AccountTestBase { ModuleEntity public twoHookValidation; function setUp() public override { + _revertSnapshot = vm.snapshot(); noHookModule = new MockUserOpValidationModule(); oneHookModule = new MockUserOpValidation1HookModule(); twoHookModule = new MockUserOpValidation2HookModule(); diff --git a/test/mocks/modules/MockAccessControlHookModule.sol b/test/mocks/modules/MockAccessControlHookModule.sol index ea8a23a34..698a42e8d 100644 --- a/test/mocks/modules/MockAccessControlHookModule.sol +++ b/test/mocks/modules/MockAccessControlHookModule.sol @@ -8,7 +8,7 @@ import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; import {BaseModule} from "../../../src/modules/BaseModule.sol"; -// A pre validaiton hook module that uses per-hook data. +// A pre validation hook module that uses per-hook data. // This example enforces that the target of an `execute` call must only be the previously specified address. // This is just a mock - it does not enforce this over `executeBatch` and other methods of making calls, and should // not be used in production.. diff --git a/test/modules/AllowlistERC20TokenLimit.t.sol b/test/modules/AllowlistERC20TokenLimit.t.sol index f8a472fe3..431b32293 100644 --- a/test/modules/AllowlistERC20TokenLimit.t.sol +++ b/test/modules/AllowlistERC20TokenLimit.t.sol @@ -33,6 +33,7 @@ contract AllowlistERC20TokenLimitTest is AccountTestBase { uint32 public constant ENTITY_ID = 1; function setUp() public override { + _revertSnapshot = vm.snapshot(); // Set up a validator with hooks from the erc20 spend limit module attached erc20 = new MockERC20(); diff --git a/test/modules/AllowlistModule.t.sol b/test/modules/AllowlistModule.t.sol index 5897d5f87..b0ac5aa7b 100644 --- a/test/modules/AllowlistModule.t.sol +++ b/test/modules/AllowlistModule.t.sol @@ -33,6 +33,7 @@ contract AllowlistModuleTest is CustomValidationTestBase { ); function setUp() public override { + _revertSnapshot = vm.snapshot(); _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID); allowlistModule = new AllowlistModule(); diff --git a/test/modules/NativeTokenLimitModule.t.sol b/test/modules/NativeTokenLimitModule.t.sol index be149d5df..6c983eeaa 100644 --- a/test/modules/NativeTokenLimitModule.t.sol +++ b/test/modules/NativeTokenLimitModule.t.sol @@ -31,6 +31,7 @@ contract NativeTokenLimitModuleTest is AccountTestBase { uint32 public entityId = 0; function setUp() public override { + _revertSnapshot = vm.snapshot(); // Set up a validator with hooks from the gas spend limit module attached acct = factory.createAccount(address(this), 0, entityId); diff --git a/test/modules/PaymasterGuardModule.t.sol b/test/modules/PaymasterGuardModule.t.sol index 57eaa19da..a7f9681bf 100644 --- a/test/modules/PaymasterGuardModule.t.sol +++ b/test/modules/PaymasterGuardModule.t.sol @@ -16,6 +16,7 @@ contract PaymasterGuardModuleTest is AccountTestBase { uint32 public constant ENTITY_ID = 1; function setUp() public override { + _revertSnapshot = vm.snapshot(); account = payable(makeAddr("account")); paymaster1 = payable(makeAddr("paymaster1")); paymaster2 = payable(makeAddr("paymaster2")); diff --git a/test/modules/TimeRangeModule.t.sol b/test/modules/TimeRangeModule.t.sol index d803a04a5..924265241 100644 --- a/test/modules/TimeRangeModule.t.sol +++ b/test/modules/TimeRangeModule.t.sol @@ -27,6 +27,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { uint48 public validAfter; function setUp() public override { + _revertSnapshot = vm.snapshot(); _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID); timeRangeModule = new TimeRangeModule(); diff --git a/test/modules/WebauthnValidationModule.t.sol b/test/modules/WebauthnValidationModule.t.sol index c78d74717..4b79e65fe 100644 --- a/test/modules/WebauthnValidationModule.t.sol +++ b/test/modules/WebauthnValidationModule.t.sol @@ -32,6 +32,7 @@ contract WebauthnValidationModuleTest is AccountTestBase { uint256 internal constant _SIG_VALIDATION_FAILED = 1; function setUp() public override { + _revertSnapshot = vm.snapshot(); module = new WebauthnValidationModule(); account = payable(account1); vm.prank(address(entryPoint)); @@ -53,6 +54,17 @@ contract WebauthnValidationModuleTest is AccountTestBase { ); } + // fuzz message + function testFuzz_pass_isValidSignature(bytes32 message) public view { + bytes32 challenge = module.replaySafeHash(account, message); + + assertTrue( + ModularAccountBase(account).isValidSignature(message, _get1271SigForChallenge(challenge, 0, 0)) + == 0x1626ba7e + ); + } + + // Fuzz sig function testFuzz_fail_isValidSignature(bytes32 message, uint256 sigR, uint256 sigS) external view { bytes32 challenge = module.replaySafeHash(account, message); @@ -97,7 +109,6 @@ contract WebauthnValidationModuleTest is AccountTestBase { PackedUserOperation memory uo; uo.sender = account; uo.callData = abi.encodeCall(ModularAccountBase.execute, (CODELESS_ADDRESS, 0, new bytes(0))); - bytes32 uoHash = entryPoint.getUserOpHash(uo); uo.signature = _getUOSigForChallenge(uoHash.toEthSignedMessageHash(), 0, 0); @@ -105,7 +116,7 @@ contract WebauthnValidationModuleTest is AccountTestBase { assertEq(ModularAccountBase(account).validateUserOp(uo, uoHash, 0), _SIG_VALIDATION_PASSED); } - function testFuzz_uoValidaton_shouldFail(uint256 sigR, uint256 sigS) external { + function testFuzz_uoValidation_shouldFail(uint256 sigR, uint256 sigS) external { PackedUserOperation memory uo; uo.sender = account; uo.callData = abi.encodeCall(ModularAccountBase.execute, (CODELESS_ADDRESS, 0, new bytes(0))); diff --git a/test/utils/AccountTestBase.sol b/test/utils/AccountTestBase.sol index 26764868e..721333b28 100644 --- a/test/utils/AccountTestBase.sol +++ b/test/utils/AccountTestBase.sol @@ -101,8 +101,6 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { vm.deal(address(account1), 100 ether); _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID); - - _revertSnapshot = vm.snapshot(); } function _runExecUserOp(address target, bytes memory callData) internal { @@ -274,15 +272,31 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { uint256 signingKey, bytes memory uoSig ) internal view returns (bytes memory) { - bytes memory deferredValidationSig = _packFinalSignature( - _signEcdsaAccountAgnostic( - _getDeferredInstallHash( - account, deferredInstallNonce, deferredInstallDeadline, deferredValidationInstallCall - ), - signingKey, - account, - ecdsaValidationModule - ) + (bytes32 structHash, bytes32 digest, bytes32 domainSeparator) = _getDeferredInstallStructAndHash( + account, deferredInstallNonce, deferredInstallDeadline, deferredValidationInstallCall + ); + + bytes32 replaySafeHash; + if (_isSMATest) { + replaySafeHash = _getSMAReplaySafeHash( + address(account), domainSeparator, structHash, digest, _DEFERRED_INSTALL_CONTENTS_TYPE + ); + } else { + replaySafeHash = _getModuleReplaySafeHash( + address(account), + address(ecdsaValidationModule), + domainSeparator, + structHash, + digest, + _DEFERRED_INSTALL_CONTENTS_TYPE + ); + } + + bytes memory deferredValidationSig = _packFinal1271Signature( + _signRawHash(vm, signingKey, replaySafeHash), + domainSeparator, + structHash, + _DEFERRED_INSTALL_CONTENTS_TYPE ); return _encodeDeferredInstallUOSignature( @@ -294,22 +308,6 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { ); } - function _signEcdsaAccountAgnostic( - bytes32 hash, - uint256 signingKey, - ModularAccount account, - ECDSAValidationModule validationModule - ) internal view returns (bytes memory) { - bytes32 replaySafeHash; - if (_isSMATest) { - replaySafeHash = _getSMAReplaySafeHash(account, hash); - } else { - replaySafeHash = validationModule.replaySafeHash(address(account), hash); - } - - return _signRawHash(vm, signingKey, replaySafeHash); - } - // helper function to compress 2 gas values into a single bytes32 function _encodeGas(uint256 g1, uint256 g2) internal pure returns (bytes32) { return bytes32(uint256((g1 << 128) + uint128(g2))); diff --git a/test/utils/ModuleSignatureUtils.sol b/test/utils/ModuleSignatureUtils.sol index e81d8d46b..bb4219961 100644 --- a/test/utils/ModuleSignatureUtils.sol +++ b/test/utils/ModuleSignatureUtils.sol @@ -7,14 +7,14 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa import {Vm} from "forge-std/src/Vm.sol"; import {ModularAccount} from "../../src/account/ModularAccount.sol"; -import {SemiModularAccountBytecode} from "../../src/account/SemiModularAccountBytecode.sol"; import {RESERVED_VALIDATION_DATA_INDEX} from "../../src/helpers/Constants.sol"; -import {ECDSAValidationModule} from "../../src/modules/validation/ECDSAValidationModule.sol"; +import {ERC7739ReplaySafeWrapper} from "../../src/libraries/ERC7739ReplaySafeWrapper.sol"; /// @dev Utilities for encoding signatures for modular account validation. Used for encoding user op, runtime, and /// 1271 signatures. contract ModuleSignatureUtils { using ModuleEntityLib for ModuleEntity; + using ERC7739ReplaySafeWrapper for address; struct PreValidationHookData { uint8 index; @@ -28,17 +28,84 @@ contract ModuleSignatureUtils { bytes32 private constant _INSTALL_VALIDATION_TYPEHASH = keccak256("InstallValidation(uint256 nonce,uint48 deadline,bytes validationInstall)"); + // 712 for a Mock App + string internal constant _MOCK_APP_CONTENTS_TYPE = "Message(string message)"; // len 23 + bytes32 internal constant _MOCK_APP_DOMAIN = 0x71062c282d40422f744945d587dbf4ecfd4f9cfad1d35d62c944373009d96162; + + string internal constant _DEFERRED_INSTALL_CONTENTS_TYPE = + "InstallValidation(uint256 nonce,uint48 deadline,bytes validationInstall)"; // len 36 + + function _getMockApp712Contents(bytes32 _digest) + internal + pure + returns (bytes32 mockAppStructHash, bytes32 mockAppDigest) + { + mockAppStructHash = keccak256(abi.encode(keccak256(abi.encodePacked(_MOCK_APP_CONTENTS_TYPE)), _digest)); + mockAppDigest = keccak256(abi.encodePacked(bytes2(hex"1901"), _MOCK_APP_DOMAIN, mockAppStructHash)); + } + + function generate1271DigestForModule( + address account, + address module, + bytes32 mockAppDigest, + bytes calldata sig + ) external view returns (bytes32 digest) { + (digest,) = account.validateERC7739SigFormatForModule(module, mockAppDigest, sig); + } + + function generate1271DigestForAccount(address account, bytes32 mockAppDigest, bytes calldata sig) + external + view + returns (bytes32 digest) + { + (digest,) = account.validateERC7739SigFormatForAccount(mockAppDigest, sig); + } + + function _encode1271Signature(ModuleEntity validationFunction, bytes memory validationData, bytes32 structHash) + internal + pure + returns (bytes memory) + { + return _encode1271Signature( + validationFunction, + new PreValidationHookData[](0), + validationData, + _MOCK_APP_DOMAIN, + structHash, + _MOCK_APP_CONTENTS_TYPE + ); + } + + function _encode1271Signature( + ModuleEntity validationFunction, + PreValidationHookData[] memory preValidationHookData, + bytes memory validationData, + bytes32 mockAppStructHash + ) internal pure returns (bytes memory) { + return _encode1271Signature( + validationFunction, + preValidationHookData, + validationData, + _MOCK_APP_DOMAIN, + mockAppStructHash, + _MOCK_APP_CONTENTS_TYPE + ); + } + // helper function to encode a 1271 signature, according to the per-hook and per-validation data format. function _encode1271Signature( ModuleEntity validationFunction, PreValidationHookData[] memory preValidationHookData, - bytes memory validationData + bytes memory validationData, + bytes32 appDomain, + bytes32 appContents, + string memory contentsType ) internal pure returns (bytes memory) { bytes memory sig = abi.encodePacked(validationFunction); sig = abi.encodePacked(sig, _packPreHookDatas(preValidationHookData)); - sig = abi.encodePacked(sig, _packFinalSignature(validationData)); + sig = abi.encodePacked(sig, _packFinal1271Signature(validationData, appDomain, appContents, contentsType)); return sig; } @@ -70,13 +137,33 @@ contract ModuleSignatureUtils { } // overload for the case where there are no pre validation hooks + function _encode1271Signature( + ModuleEntity validationFunction, + bytes memory validationData, + bytes32 domainSeparator, + bytes32 contents, + string memory contentsType + ) internal pure returns (bytes memory) { + PreValidationHookData[] memory emptyPreValidationHookData = new PreValidationHookData[](0); + return _encode1271Signature( + validationFunction, emptyPreValidationHookData, validationData, domainSeparator, contents, contentsType + ); + } + + // Helper function for webauthn plugin. This provides a 1271 sig for a ReplaySafeWrapper, instead of the + // ERC7739 sig function _encode1271Signature(ModuleEntity validationFunction, bytes memory validationData) internal pure returns (bytes memory) { - PreValidationHookData[] memory emptyPreValidationHookData = new PreValidationHookData[](0); - return _encode1271Signature(validationFunction, emptyPreValidationHookData, validationData); + bytes memory sig = abi.encodePacked(validationFunction); + + sig = abi.encodePacked(sig, _packPreHookDatas(new PreValidationHookData[](0))); + + sig = abi.encodePacked(sig, _packFinalSignature(validationData)); + + return sig; } // helper function to pack pre validation hook datas, according to the sparse calldata segment spec. @@ -143,30 +230,49 @@ contract ModuleSignatureUtils { return abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v); } - function _getECDSAReplaySafeHash( - ModularAccount account, - ECDSAValidationModule validationModule, - bytes32 typedDataHash + // From + // github/Vectorized/solady/blob/4676345386dab5728e2da9a6540f6cd308a9f4d5/src/accounts/ERC1271.sol#L157-L158 + // The signature will be `r ‖ s ‖ v ‖ + // APP_DOMAIN_SEPARATOR ‖ contents ‖ contentsType ‖ uint16(contentsType.length)`, + function _packFinal1271Signature( + bytes memory sig, + bytes32 appDomain, + bytes32 appStructHash, + string memory contentsType + ) internal pure returns (bytes memory) { + return abi.encodePacked( + RESERVED_VALIDATION_DATA_INDEX, + sig, + appDomain, + appStructHash, + contentsType, + uint16(bytes(contentsType).length) + ); + } + + function _getModuleReplaySafeHash( + address account, + address validationModule, + bytes32 domainSeparator, + bytes32 appStructHash, + bytes32 digest, + string memory contentsType ) internal view returns (bytes32) { - return validationModule.replaySafeHash(address(account), typedDataHash); + bytes memory sig = + abi.encodePacked(domainSeparator, appStructHash, contentsType, uint16(bytes(contentsType).length)); + return this.generate1271DigestForModule(account, validationModule, digest, sig); } - function _getSMAReplaySafeHash(ModularAccount account, bytes32 typedDataHash) - internal - view - returns (bytes32) - { - if (address(account).code.length > 0) { - return SemiModularAccountBytecode(payable(account)).replaySafeHash(typedDataHash); - } else { - // precompute it as the SMA is not yet deployed - // for SMA, the domain separator used for the deferred validation installation is the same as the one - // used to compute the replay safe hash. - return MessageHashUtils.toTypedDataHash({ - domainSeparator: _computeDomainSeparator(account), - structHash: _hashStructReplaySafeHash(typedDataHash) - }); - } + function _getSMAReplaySafeHash( + address account, + bytes32 domainSeparator, + bytes32 appStructHash, + bytes32 digest, + string memory contentsType + ) internal view returns (bytes32) { + bytes memory sig = + abi.encodePacked(domainSeparator, appStructHash, contentsType, uint16(bytes(contentsType).length)); + return this.generate1271DigestForAccount(account, digest, sig); } // Deferred validation helpers @@ -203,32 +309,26 @@ contract ModuleSignatureUtils { return deferredInstallData; } - function _getDeferredInstallHash( + function _getDeferredInstallStructAndHash( ModularAccount account, uint256 nonce, uint48 deadline, bytes memory installCall - ) internal view returns (bytes32) { - bytes32 domainSeparator = _computeDomainSeparator(account); + ) internal view returns (bytes32 structHash, bytes32 typedDataHash, bytes32 domainSeparator) { + domainSeparator = _computeDomainSeparator(address(account)); bytes32 installCallHash = keccak256(installCall); - bytes32 structHash = keccak256(abi.encode(_INSTALL_VALIDATION_TYPEHASH, nonce, deadline, installCallHash)); - - bytes32 typedDataHash = MessageHashUtils.toTypedDataHash(domainSeparator, structHash); + structHash = keccak256(abi.encode(_INSTALL_VALIDATION_TYPEHASH, nonce, deadline, installCallHash)); - return typedDataHash; + typedDataHash = MessageHashUtils.toTypedDataHash(domainSeparator, structHash); } // EIP-712 helpers - function _computeDomainSeparator(ModularAccount account) internal view returns (bytes32) { - bytes32 domainSeparatorTypehash = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)"); - return keccak256(abi.encode(domainSeparatorTypehash, block.chainid, address(account))); - } - - function _hashStructReplaySafeHash(bytes32 hash) internal pure returns (bytes32) { - bytes32 replaySafeTypehash = keccak256("ReplaySafeHash(bytes32 hash)"); // const 0x.. in contract - return keccak256(abi.encode(replaySafeTypehash, hash)); + function _computeDomainSeparator(address account) internal view returns (bytes32) { + return keccak256( + abi.encode(ERC7739ReplaySafeWrapper._DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT, block.chainid, account) + ); } }