diff --git a/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap b/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap index 8c8d3d55..95af41f4 100644 --- a/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap +++ b/.forge-snapshots/ModularAccount_Runtime_AccountCreation.snap @@ -1 +1 @@ -176008 \ No newline at end of file +175965 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_BatchTransfers.snap b/.forge-snapshots/ModularAccount_Runtime_BatchTransfers.snap index 44fd5c06..17f77273 100644 --- a/.forge-snapshots/ModularAccount_Runtime_BatchTransfers.snap +++ b/.forge-snapshots/ModularAccount_Runtime_BatchTransfers.snap @@ -1 +1 @@ -93101 \ No newline at end of file +92963 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap b/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap index ee18c5c9..7df10e2e 100644 --- a/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap +++ b/.forge-snapshots/ModularAccount_Runtime_Erc20Transfer.snap @@ -1 +1 @@ -78526 \ No newline at end of file +78372 \ 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 4f5d99f0..4a513a4b 100644 --- a/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -423482 \ No newline at end of file +422934 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap b/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap index 2b4248a2..778a1160 100644 --- a/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap +++ b/.forge-snapshots/ModularAccount_Runtime_NativeTransfer.snap @@ -1 +1 @@ -54675 \ No newline at end of file +54521 \ 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 80017d24..3d7c9312 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -78838 \ No newline at end of file +78584 \ 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 37f58ff6..69b5c3f0 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -112223 \ No newline at end of file +111897 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap b/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap index 531657a5..41e7355a 100644 --- a/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap +++ b/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap @@ -1 +1 @@ -198437 \ No newline at end of file +198297 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap b/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap index 1c39110f..9ba046bb 100644 --- a/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap +++ b/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap @@ -1 +1 @@ -185406 \ No newline at end of file +185250 \ 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 79070d24..ab0c8cf7 100644 --- a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -531716 \ No newline at end of file +531166 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap b/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap index d60bdac3..0e41b6e7 100644 --- a/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap +++ b/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap @@ -1 +1 @@ -161687 \ No newline at end of file +161531 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index 788c162e..5af0b409 100644 --- a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -195559 \ No newline at end of file +195236 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap index ef671140..419b595a 100644 --- a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -226591 \ No newline at end of file +226220 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap index e6aadc95..b1541951 100644 --- a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -261264 \ No newline at end of file +260935 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap b/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap index 1994258c..04b7e1c0 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap @@ -1 +1 @@ -89199 \ No newline at end of file +88968 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap index 06d89cec..fc53eb44 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap @@ -1 +1 @@ -74716 \ No newline at end of file +74425 \ 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 f51cd88c..7a2fc0ca 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -422162 \ No newline at end of file +421505 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap index 8f9a66ea..4dae62a6 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap @@ -1 +1 @@ -50875 \ No newline at end of file +50584 \ 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 dfca98c2..51f865bd 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -79254 \ No newline at end of file +78934 \ 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 367e1c2f..ed89336e 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -112639 \ No newline at end of file +112247 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap b/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap index b12e3ecd..98b956d3 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap @@ -1 +1 @@ -193639 \ No newline at end of file +193462 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap index ba360fee..dcb9e6d6 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap @@ -1 +1 @@ -180735 \ No newline at end of file +180498 \ 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 90f24a8c..b023b7b4 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -529289 \ No newline at end of file +528686 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap index b10db97a..92859895 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap @@ -1 +1 @@ -157010 \ No newline at end of file +156773 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index 541cafae..e574b3ae 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -195853 \ No newline at end of file +195464 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap index d15eabfa..49d32bc9 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -226897 \ No newline at end of file +226460 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap index 4bdce39f..289566e0 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -257961 \ No newline at end of file +257489 \ No newline at end of file diff --git a/foundry.toml b/foundry.toml index 5623af0d..e3d1e3b8 100644 --- a/foundry.toml +++ b/foundry.toml @@ -29,7 +29,7 @@ depth = 10 deny_warnings = true via_ir = true test = 'src' -optimizer_runs = 10000 +optimizer_runs = 15000 out = 'out-optimized' cache_path = 'cache-optimized' @@ -59,7 +59,7 @@ depth = 32 via_ir = true deny_warnings = true test = 'gas' -optimizer_runs = 10000 +optimizer_runs = 15000 out = 'out-optimized' cache_path = 'cache-optimized' ffi = true diff --git a/src/account/ModularAccountBase.sol b/src/account/ModularAccountBase.sol index 828b4ad6..22aced91 100644 --- a/src/account/ModularAccountBase.sol +++ b/src/account/ModularAccountBase.sol @@ -25,6 +25,7 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {UUPSUpgradeable} from "solady/utils/UUPSUpgradeable.sol"; +import {ExecutionInstallDelegate} from "../helpers/ExecutionInstallDelegate.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationResHelpers.sol"; import { DensePostHookData, @@ -89,6 +90,8 @@ abstract contract ModularAccountBase is uint8 internal constant _IS_GLOBAL_VALIDATION_BIT = 1; uint8 internal constant _HAS_DEFERRED_ACTION_BIT = 2; + address internal immutable _EXECUTION_INSTALL_DELEGATE; + event DeferredActionNonceInvalidated(uint256 nonce); error CreateFailed(); @@ -115,6 +118,8 @@ abstract contract ModularAccountBase is constructor(IEntryPoint anEntryPoint) AccountBase(anEntryPoint) { _disableInitializers(); + + _EXECUTION_INSTALL_DELEGATE = address(new ExecutionInstallDelegate()); } // EXTERNAL FUNCTIONS @@ -122,8 +127,10 @@ abstract contract ModularAccountBase is receive() external payable {} /// @notice Fallback function - /// @dev We route calls to execution functions based on incoming msg.sig - /// @dev If there's no module associated with this function selector, revert + /// @dev Routes calls to execution functions based on the incoming msg.sig. If there's no module associated + /// with this function selector, revert. + /// + /// @return The raw returned data from the invoked execution function. fallback(bytes calldata) external payable returns (bytes memory) { address execModule = getAccountStorage().executionStorage[msg.sig].module; if (execModule == address(0)) { @@ -144,40 +151,7 @@ abstract contract ModularAccountBase is /// @param value The value to send to the new contract constructor /// @param initCode The initCode to deploy. /// @return createdAddr The created contract address. - function performCreate(uint256 value, bytes calldata initCode) - external - payable - virtual - wrapNativeFunction - returns (address createdAddr) - { - assembly ("memory-safe") { - // Load the free memory pointer. - let fmp := mload(0x40) - - // Get the initCode length. - let len := initCode.length - - // Copy the initCode from callata to memory at the free memory pointer. - calldatacopy(fmp, initCode.offset, len) - - // Create the contract. - createdAddr := create(value, fmp, len) - - if iszero(createdAddr) { - // If creation failed (the address returned is zero), revert with CreateFailed(). - mstore(0x00, 0x7e16b8cd) - revert(0x1c, 0x04) - } - } - } - - /// @notice Creates a contract using create2 deterministic deployment. - /// @param value The value to send to the new contract constructor. - /// @param initCode The initCode to deploy. - /// @param salt The salt to use for the create2 operation. - /// @return createdAddr The created contract address. - function performCreate2(uint256 value, bytes calldata initCode, bytes32 salt) + function performCreate(uint256 value, bytes calldata initCode, bool isCreate2, bytes32 salt) external payable virtual @@ -194,8 +168,9 @@ abstract contract ModularAccountBase is // Copy the initCode from callata to memory at the free memory pointer. calldatacopy(fmp, initCode.offset, len) - // Create the contract using Create2 with the passed salt parameter. - createdAddr := create2(value, fmp, len, salt) + switch isCreate2 + case 1 { createdAddr := create2(value, fmp, len, salt) } + default { createdAddr := create(value, fmp, len) } if iszero(createdAddr) { // If creation failed (the address returned is zero), revert with CreateFailed(). @@ -320,7 +295,10 @@ abstract contract ModularAccountBase is ExecutionManifest calldata manifest, bytes calldata moduleInstallData ) external override wrapNativeFunction { - _installExecution(module, manifest, moduleInstallData); + // Access params to prevent compiler unused parameter flags. + (module, manifest, moduleInstallData); + address delegate = _EXECUTION_INSTALL_DELEGATE; + ExecutionLib.delegatecallBubbleOnRevertTransient(delegate); } /// @inheritdoc IModularAccount @@ -330,7 +308,10 @@ abstract contract ModularAccountBase is ExecutionManifest calldata manifest, bytes calldata moduleUninstallData ) external override wrapNativeFunction { - _uninstallExecution(module, manifest, moduleUninstallData); + // Access params to prevent compiler unused parameter flags. + (module, manifest, moduleUninstallData); + address delegate = _EXECUTION_INSTALL_DELEGATE; + ExecutionLib.delegatecallBubbleOnRevertTransient(delegate); } /// @inheritdoc IModularAccount @@ -797,7 +778,7 @@ abstract contract ModularAccountBase is || selector == this.installValidation.selector || selector == this.uninstallValidation.selector || selector == this.upgradeToAndCall.selector || selector == this.invalidateDeferredValidationInstallNonce.selector - || selector == this.performCreate.selector || selector == this.performCreate2.selector + || selector == this.performCreate.selector ) { return true; } diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol index 5c93e897..ac5bfc96 100644 --- a/src/account/ModularAccountView.sol +++ b/src/account/ModularAccountView.sol @@ -8,7 +8,7 @@ import { ValidationDataView } from "@erc6900/reference-implementation/interfaces/IModularAccountView.sol"; -import {KnownSelectorsLib} from "../libraries/KnownSelectorsLib.sol"; +import {NativeFunctionDelegate} from "../helpers/NativeFunctionDelegate.sol"; import {MemManagementLib} from "../libraries/MemManagementLib.sol"; import {ExecutionStorage, ValidationStorage, getAccountStorage} from "./AccountStorage.sol"; @@ -17,11 +17,18 @@ import {ExecutionStorage, ValidationStorage, getAccountStorage} from "./AccountS /// @notice This abstract contract implements the two view functions to get validation and execution data for an /// account. abstract contract ModularAccountView is IModularAccountView { + NativeFunctionDelegate internal immutable _NATIVE_FUNCTION_DELEGATE; + + constructor() { + _NATIVE_FUNCTION_DELEGATE = new NativeFunctionDelegate(); + } + /// @inheritdoc IModularAccountView function getExecutionData(bytes4 selector) external view override returns (ExecutionDataView memory data) { + // return ModularAccountViewLib.getExecutionData(selector, _isNativeFunction(selector)); ExecutionStorage storage executionStorage = getAccountStorage().executionStorage[selector]; - if (KnownSelectorsLib.isNativeFunction(selector)) { + if (_isNativeFunction(uint32(selector))) { data.module = address(this); data.allowGlobalValidation = true; } else { @@ -57,4 +64,8 @@ abstract contract ModularAccountView is IModularAccountView { MemManagementLib.reverseArr(selectors); data.selectors = selectors; } + + function _isNativeFunction(uint32 selector) internal view virtual returns (bool) { + return _NATIVE_FUNCTION_DELEGATE.isNativeFunction(selector); + } } diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index 568cac81..c823f824 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -3,35 +3,22 @@ pragma solidity ^0.8.26; import {MAX_VALIDATION_ASSOC_HOOKS} from "@erc6900/reference-implementation/helpers/Constants.sol"; import {IExecutionHookModule} from "@erc6900/reference-implementation/interfaces/IExecutionHookModule.sol"; -import { - ExecutionManifest, - ManifestExecutionHook -} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; import { HookConfig, IModularAccount, ModuleEntity, ValidationConfig } from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; -import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; import {HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; import {ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; -import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; -import {ExecutionLib} from "../libraries/ExecutionLib.sol"; -import {KnownSelectorsLib} from "../libraries/KnownSelectorsLib.sol"; import {LinkedListSet, LinkedListSetLib} from "../libraries/LinkedListSetLib.sol"; import {MemManagementLib} from "../libraries/MemManagementLib.sol"; -import { - AccountStorage, - ExecutionStorage, - ValidationStorage, - getAccountStorage, - toSetValue -} from "./AccountStorage.sol"; +import {ModuleInstallCommons} from "../libraries/ModuleInstallCommons.sol"; +import {ValidationStorage, getAccountStorage, toSetValue} from "./AccountStorage.sol"; /// @title Modular Manager Internal Methods /// @author Alchemy @@ -44,64 +31,10 @@ abstract contract ModuleManagerInternals is IModularAccount { using HookConfigLib for HookConfig; error ArrayLengthMismatch(); - error Erc4337FunctionNotAllowed(bytes4 selector); - error ExecutionFunctionAlreadySet(bytes4 selector); - error IModuleFunctionNotAllowed(bytes4 selector); - error InterfaceNotSupported(address module); - error NativeFunctionNotAllowed(bytes4 selector); - error NullModule(); - error ExecutionHookAlreadySet(HookConfig hookConfig); - error ModuleInstallCallbackFailed(address module, bytes revertReason); - error ModuleNotInstalled(address module); error PreValidationHookDuplicate(); error ValidationAlreadySet(bytes4 selector, ModuleEntity validationFunction); error ValidationAssocHookLimitExceeded(); - function _setExecutionFunction( - bytes4 selector, - bool skipRuntimeValidation, - bool allowGlobalValidation, - address module - ) internal { - ExecutionStorage storage _executionStorage = getAccountStorage().executionStorage[selector]; - - if (_executionStorage.module != address(0)) { - revert ExecutionFunctionAlreadySet(selector); - } - - // Make sure incoming execution function does not collide with any native functions (data are stored on the - // account implementation contract) - if (_isNativeFunction(selector)) { - revert NativeFunctionNotAllowed(selector); - } - - // Make sure incoming execution function is not a function in IModule - if (KnownSelectorsLib.isIModuleFunction(selector)) { - revert IModuleFunctionNotAllowed(selector); - } - - // Also make sure it doesn't collide with functions defined by ERC-4337 - // and called by the entry point. This prevents a malicious module from - // sneaking in a function with the same selector as e.g. - // `validatePaymasterUserOp` and turning the account into their own - // personal paymaster. - if (KnownSelectorsLib.isErc4337Function(selector)) { - revert Erc4337FunctionNotAllowed(selector); - } - - _executionStorage.module = module; - _executionStorage.skipRuntimeValidation = skipRuntimeValidation; - _executionStorage.allowGlobalValidation = allowGlobalValidation; - } - - function _removeExecutionFunction(bytes4 selector) internal { - ExecutionStorage storage _executionStorage = getAccountStorage().executionStorage[selector]; - - _executionStorage.module = address(0); - _executionStorage.skipRuntimeValidation = false; - _executionStorage.allowGlobalValidation = false; - } - function _removeValidationFunction(ModuleEntity validationFunction) internal { ValidationStorage storage _validationStorage = getAccountStorage().validationStorage[validationFunction]; @@ -110,127 +43,6 @@ abstract contract ModuleManagerInternals is IModularAccount { _validationStorage.isUserOpValidation = false; } - function _addExecHooks(LinkedListSet storage hooks, HookConfig hookConfig) internal { - if (!hooks.tryAdd(toSetValue(hookConfig))) { - revert ExecutionHookAlreadySet(hookConfig); - } - } - - function _removeExecHooks(LinkedListSet storage hooks, HookConfig hookConfig) internal { - hooks.tryRemove(toSetValue(hookConfig)); - } - - /// @notice Update components according to the manifest. - function _installExecution( - address module, - ExecutionManifest calldata manifest, - bytes calldata moduleInstallData - ) internal { - AccountStorage storage _storage = getAccountStorage(); - - if (module == address(0)) { - revert NullModule(); - } - - uint256 length = manifest.executionFunctions.length; - for (uint256 i = 0; i < length; ++i) { - bytes4 selector = manifest.executionFunctions[i].executionSelector; - bool skipRuntimeValidation = manifest.executionFunctions[i].skipRuntimeValidation; - bool allowGlobalValidation = manifest.executionFunctions[i].allowGlobalValidation; - _setExecutionFunction(selector, skipRuntimeValidation, allowGlobalValidation, module); - } - - length = manifest.executionHooks.length; - for (uint256 i = 0; i < length; ++i) { - ManifestExecutionHook memory mh = manifest.executionHooks[i]; - LinkedListSet storage executionHooks = _storage.executionStorage[mh.executionSelector].executionHooks; - HookConfig hookConfig = HookConfigLib.packExecHook({ - _module: module, - _entityId: mh.entityId, - _hasPre: mh.isPreHook, - _hasPost: mh.isPostHook - }); - _addExecHooks(executionHooks, hookConfig); - } - - length = manifest.interfaceIds.length; - for (uint256 i = 0; i < length; ++i) { - _storage.supportedIfaces[manifest.interfaceIds[i]] += 1; - } - - _onInstall(module, moduleInstallData, type(IModule).interfaceId); - - emit ExecutionInstalled(module, manifest); - } - - /// @notice Remove components according to the manifest, in reverse order (by component type) of their - /// installation. - function _uninstallExecution(address module, ExecutionManifest calldata manifest, bytes calldata uninstallData) - internal - { - AccountStorage storage _storage = getAccountStorage(); - - uint256 length = manifest.executionHooks.length; - for (uint256 i = 0; i < length; ++i) { - ManifestExecutionHook memory mh = manifest.executionHooks[i]; - LinkedListSet storage executionHooks = _storage.executionStorage[mh.executionSelector].executionHooks; - HookConfig hookConfig = HookConfigLib.packExecHook({ - _module: module, - _entityId: mh.entityId, - _hasPre: mh.isPreHook, - _hasPost: mh.isPostHook - }); - _removeExecHooks(executionHooks, hookConfig); - } - - length = manifest.executionFunctions.length; - for (uint256 i = 0; i < length; ++i) { - bytes4 selector = manifest.executionFunctions[i].executionSelector; - _removeExecutionFunction(selector); - } - - length = manifest.interfaceIds.length; - for (uint256 i = 0; i < length; ++i) { - _storage.supportedIfaces[manifest.interfaceIds[i]] -= 1; - } - - // Clear the module storage for the account. - bool onUninstallSuccess = _onUninstall(module, uninstallData); - - emit ExecutionUninstalled(module, onUninstallSuccess, manifest); - } - - /// @dev setup the module storage for the account, reverts are bubbled up into a custom - /// ModuleInstallCallbackFailed - function _onInstall(address module, bytes calldata data, bytes4 interfaceId) internal { - if (data.length > 0) { - if (!ERC165Checker.supportsERC165InterfaceUnchecked(module, interfaceId)) { - revert InterfaceNotSupported(module); - } - // solhint-disable-next-line no-empty-blocks - try IModule(module).onInstall(data) {} - catch { - bytes memory revertReason = ExecutionLib.collectReturnData(); - revert ModuleInstallCallbackFailed(module, revertReason); - } - } - } - - /// @dev clear the module storage for the account, reverts are IGNORED. Status is included in emitted event. - function _onUninstall(address module, bytes calldata data) internal returns (bool onUninstallSuccess) { - onUninstallSuccess = true; - if (data.length > 0) { - // Clear the module storage for the account. - // solhint-disable-next-line no-empty-blocks - try IModule(module).onUninstall(data) {} - catch { - onUninstallSuccess = false; - } - } - } - - /// @dev install a validation function for the account. If the validation function is already installed, - /// certain fields may be updated; more (not duplicated) hooks and selectors can be added. function _installValidation( ValidationConfig validationConfig, bytes4[] calldata selectors, @@ -263,7 +75,9 @@ abstract contract ModuleManagerInternals is IModularAccount { revert PreValidationHookDuplicate(); } - _onInstall(hookConfig.module(), hookData, type(IValidationHookModule).interfaceId); + ModuleInstallCommons.onInstall( + hookConfig.module(), hookData, type(IValidationHookModule).interfaceId + ); } else { // Hook is an execution hook @@ -277,8 +91,10 @@ abstract contract ModuleManagerInternals is IModularAccount { ++_validationStorage.executionHookCount; } - _addExecHooks(_validationStorage.executionHooks, hookConfig); - _onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId); + ModuleInstallCommons.addExecHooks(_validationStorage.executionHooks, hookConfig); + ModuleInstallCommons.onInstall( + hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId + ); } } @@ -294,7 +110,7 @@ abstract contract ModuleManagerInternals is IModularAccount { _validationStorage.isSignatureValidation = validationConfig.isSignatureValidation(); _validationStorage.isUserOpValidation = validationConfig.isUserOpValidation(); - _onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId); + ModuleInstallCommons.onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId); emit ValidationInstalled(validationConfig.module(), validationConfig.entityId()); } @@ -324,7 +140,7 @@ abstract contract ModuleManagerInternals is IModularAccount { for (uint256 i = 0; i < length; ++i) { bytes calldata hookData = hookUninstallDatas[hookIndex]; (address hookModule,) = ModuleEntityLib.unpack(validationHooks[i].moduleEntity()); - onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData); + onUninstallSuccess = onUninstallSuccess && ModuleInstallCommons.onUninstall(hookModule, hookData); hookIndex++; } @@ -332,7 +148,7 @@ abstract contract ModuleManagerInternals is IModularAccount { for (uint256 i = 0; i < length; ++i) { bytes calldata hookData = hookUninstallDatas[hookIndex]; address hookModule = execHooks[i].module(); - onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData); + onUninstallSuccess = onUninstallSuccess && ModuleInstallCommons.onUninstall(hookModule, hookData); hookIndex++; } } @@ -348,12 +164,8 @@ abstract contract ModuleManagerInternals is IModularAccount { _validationStorage.selectors.clear(); (address module, uint32 entityId) = ModuleEntityLib.unpack(validationFunction); - onUninstallSuccess = onUninstallSuccess && _onUninstall(module, uninstallData); + onUninstallSuccess = onUninstallSuccess && ModuleInstallCommons.onUninstall(module, uninstallData); emit ValidationUninstalled(module, entityId, onUninstallSuccess); } - - function _isNativeFunction(bytes4 selector) internal pure virtual returns (bool) { - return KnownSelectorsLib.isNativeFunction(selector); - } } diff --git a/src/account/SemiModularAccountBase.sol b/src/account/SemiModularAccountBase.sol index 858a96c5..ebbba9ce 100644 --- a/src/account/SemiModularAccountBase.sol +++ b/src/account/SemiModularAccountBase.sol @@ -13,9 +13,16 @@ import {FALLBACK_VALIDATION} from "../helpers/Constants.sol"; import {SignatureType} from "../helpers/SignatureType.sol"; import {ERC7739ReplaySafeWrapperLib} from "../libraries/ERC7739ReplaySafeWrapperLib.sol"; import {RTCallBuffer, SigCallBuffer, UOCallBuffer} from "../libraries/ExecutionLib.sol"; -import {SemiModularKnownSelectorsLib} from "../libraries/SemiModularKnownSelectorsLib.sol"; import {ModularAccountBase} from "./ModularAccountBase.sol"; +/// @title SemiModularAccountBase +/// @author Alchemy +/// +/// @notice Abstract base contract for the Alchemy Semi-Modular Account variants. Includes fallback signer +/// functionality. +/// +/// @dev Inherits ModularAccountBase. Overrides certain functionality from ModularAccountBase, and exposes an +/// internal virtual getter for the fallback signer. abstract contract SemiModularAccountBase is ModularAccountBase { using MessageHashUtils for bytes32; using ModuleEntityLib for ModuleEntity; @@ -33,8 +40,7 @@ abstract contract SemiModularAccountBase is ModularAccountBase { uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; - event FallbackSignerSet(address indexed previousFallbackSigner, address indexed newFallbackSigner); - event FallbackSignerDisabledSet(bool prevDisabled, bool newDisabled); + event FallbackSignerUpdated(address indexed newFallbackSigner, bool isDisabled); error FallbackSignerMismatch(); error FallbackSignerDisabled(); @@ -43,36 +49,24 @@ abstract contract SemiModularAccountBase is ModularAccountBase { constructor(IEntryPoint anEntryPoint) ModularAccountBase(anEntryPoint) {} - /// @notice Updates the fallback signer address in storage. + /// @notice Updates the fallback signer data in storage. /// @param fallbackSigner The new signer to set. - function updateFallbackSigner(address fallbackSigner) external wrapNativeFunction { + /// @param isDisabled Whether to disable fallback signing entirely. + function updateFallbackSignerData(address fallbackSigner, bool isDisabled) external wrapNativeFunction { SemiModularAccountStorage storage _storage = _getSemiModularAccountStorage(); - emit FallbackSignerSet(_storage.fallbackSigner, fallbackSigner); _storage.fallbackSigner = fallbackSigner; - } - - /// @notice Sets whether the fallback signer validation should be enabled or disabled. - /// @dev Due to being initially zero, we need to store "disabled" rather than "enabled" in storage. - /// @param isDisabled True to disable fallback signer validation, false to enable it. - function setFallbackSignerDisabled(bool isDisabled) external wrapNativeFunction { - SemiModularAccountStorage storage _storage = _getSemiModularAccountStorage(); - emit FallbackSignerDisabledSet(_storage.fallbackSignerDisabled, isDisabled); - _storage.fallbackSignerDisabled = isDisabled; - } - /// @notice Returns whether the fallback signer validation is disabled. - /// @return True if the fallback signer validation is disabled, false if it is enabled. - function isFallbackSignerDisabled() external view returns (bool) { - return _getSemiModularAccountStorage().fallbackSignerDisabled; + emit FallbackSignerUpdated(fallbackSigner, isDisabled); } - /// @notice Returns the fallback signer associated with this account, regardless if the fallback signer - /// validation is enabled or not. - /// @return The fallback signer address, either overriden in storage, or read from bytecode. - function getFallbackSigner() external view returns (address) { - return _retrieveFallbackSignerUnchecked(_getSemiModularAccountStorage()); + /// @notice Returns the fallback signer data in storage. + /// @return The fallback signer and a boolean, true if the fallback signer validation is disabled, false if it + /// is enabled. + function getFallbackSignerData() external view returns (address, bool) { + SemiModularAccountStorage storage _storage = _getSemiModularAccountStorage(); + return (_retrieveFallbackSignerUnchecked(_storage), _storage.fallbackSignerDisabled); } function _execUserOpValidation( @@ -147,8 +141,7 @@ abstract contract SemiModularAccountBase is ModularAccountBase { } function _globalValidationAllowed(bytes4 selector) internal view override returns (bool) { - return selector == this.setFallbackSignerDisabled.selector - || selector == this.updateFallbackSigner.selector || super._globalValidationAllowed(selector); + return selector == this.updateFallbackSignerData.selector || super._globalValidationAllowed(selector); } function _isValidationGlobal(ModuleEntity validationFunction) internal view override returns (bool) { @@ -201,6 +194,12 @@ abstract contract SemiModularAccountBase is ModularAccountBase { return _storage.fallbackSigner; } + // overrides ModularAccountView + function _isNativeFunction(uint32 selector) internal view override returns (bool) { + return super._isNativeFunction(selector) || selector == uint32(this.updateFallbackSignerData.selector) + || selector == uint32(this.getFallbackSignerData.selector); + } + function _getSemiModularAccountStorage() internal pure returns (SemiModularAccountStorage storage) { SemiModularAccountStorage storage _storage; assembly ("memory-safe") { @@ -209,11 +208,6 @@ abstract contract SemiModularAccountBase is ModularAccountBase { return _storage; } - // Overrides ModuleManagerInternals - function _isNativeFunction(bytes4 selector) internal pure override returns (bool) { - return SemiModularKnownSelectorsLib.isNativeFunction(selector); - } - // Conditionally skip allocation of call buffers. function _validationIsNative(ModuleEntity validationFunction) internal pure virtual override returns (bool) { return validationFunction.eq(FALLBACK_VALIDATION); diff --git a/src/account/SemiModularAccountBytecode.sol b/src/account/SemiModularAccountBytecode.sol index 17dbcd53..fd5ff003 100644 --- a/src/account/SemiModularAccountBytecode.sol +++ b/src/account/SemiModularAccountBytecode.sol @@ -10,10 +10,11 @@ import {SemiModularAccountBase} from "./SemiModularAccountBase.sol"; /// @title SemiModularAccountBytecode /// @author Alchemy /// -/// @notice An implementation of a semi-modular account with a fallback that reads the signer from proxy bytecode. +/// @notice An implementation of a semi-modular account which reads the signer from proxy bytecode if it is not +/// disabled and zero in storage. /// -/// @dev This account requires that its proxy is compliant with Solady's LibClone ERC1967WithImmutableArgs -/// bytecode. +/// @dev Inherits SemiModularAccountBase. This account requires that its proxy is compliant with Solady's LibClone +/// ERC1967WithImmutableArgs bytecode with a bytecode-appended address to be used as the fallback signer. contract SemiModularAccountBytecode is SemiModularAccountBase { constructor(IEntryPoint anEntryPoint) SemiModularAccountBase(anEntryPoint) {} diff --git a/src/account/SemiModularAccountStorageOnly.sol b/src/account/SemiModularAccountStorageOnly.sol index ffa6351a..5c4e9405 100644 --- a/src/account/SemiModularAccountStorageOnly.sol +++ b/src/account/SemiModularAccountStorageOnly.sol @@ -9,10 +9,12 @@ import {SemiModularAccountBase} from "./SemiModularAccountBase.sol"; /// @title SemiModularAccountStorageOnly /// @author Alchemy /// -/// @notice A basic Semi-Modular Account with an initializer to set the fallback signer in storage. +/// @notice An implementation of a semi-modular account which includes an initializer to set the fallback signer in +/// storage upon initialization. /// -/// @dev Note that the initializer has no access control and should be called via `upgradeToAndCall()`. -/// It's recommended to opt for the variant `SemiModularAccountBytecode` instead for new accounts. +/// @dev Inherits SemiModularAccountBase. Note that the initializer has no access control and should be called via +/// `upgradeToAndCall()`. Use the `SemiModularAccountBytecode` instead for new accounts, this implementation should +/// only be used for account upgrades. contract SemiModularAccountStorageOnly is SemiModularAccountBase { constructor(IEntryPoint anEntryPoint) SemiModularAccountBase(anEntryPoint) {} @@ -22,9 +24,7 @@ contract SemiModularAccountStorageOnly is SemiModularAccountBase { smaStorage.fallbackSigner = initialSigner; smaStorage.fallbackSignerDisabled = false; - // Note that it's technically possible for the fallback signer in storage to be nonzero before - // initialization. However, reading it here would add costs in the vast majority of cases. - emit FallbackSignerSet(address(0), initialSigner); + emit FallbackSignerUpdated(initialSigner, false); } /// @inheritdoc IModularAccount diff --git a/src/helpers/ExecutionInstallDelegate.sol b/src/helpers/ExecutionInstallDelegate.sol new file mode 100644 index 00000000..aded6aa1 --- /dev/null +++ b/src/helpers/ExecutionInstallDelegate.sol @@ -0,0 +1,174 @@ +// SPDX-License-Identifier: UNLICENSED + +pragma solidity ^0.8.26; + +import { + ExecutionManifest, + ManifestExecutionHook +} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; +import {HookConfig, IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; +import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; +import {HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; + +import {AccountStorage, ExecutionStorage, getAccountStorage, toSetValue} from "../account/AccountStorage.sol"; +import {KnownSelectorsLib} from "../libraries/KnownSelectorsLib.sol"; +import {LinkedListSet, LinkedListSetLib} from "../libraries/LinkedListSetLib.sol"; +import {ModuleInstallCommons} from "../libraries/ModuleInstallCommons.sol"; + +/// @title ExecutionInstallDelegate +/// @author Alchemy +/// +/// @notice This contract acts as an external library which is meant to handle Execution function installations and +/// uninstallations via delegatecall. +contract ExecutionInstallDelegate { + using LinkedListSetLib for LinkedListSet; + + address internal immutable _THIS_ADDRESS; + + error OnlyDelegateCall(); + error NullModule(); + error ExecutionFunctionAlreadySet(); + error IModuleFunctionNotAllowed(); + error Erc4337FunctionNotAllowed(); + + modifier onlyDelegateCall() { + if (address(this) == _THIS_ADDRESS) { + revert OnlyDelegateCall(); + } + _; + } + + constructor() { + _THIS_ADDRESS = address(this); + } + + // External Functions + + /// @notice Update components according to the manifest. + function installExecution( + address module, + ExecutionManifest calldata manifest, + bytes calldata moduleInstallData + ) external onlyDelegateCall { + AccountStorage storage _storage = getAccountStorage(); + + if (module == address(0)) { + revert NullModule(); + } + + // Update components according to the manifest. + uint256 length = manifest.executionFunctions.length; + for (uint256 i = 0; i < length; ++i) { + bytes4 selector = manifest.executionFunctions[i].executionSelector; + bool skipRuntimeValidation = manifest.executionFunctions[i].skipRuntimeValidation; + bool allowGlobalValidation = manifest.executionFunctions[i].allowGlobalValidation; + _setExecutionFunction(selector, skipRuntimeValidation, allowGlobalValidation, module); + } + + length = manifest.executionHooks.length; + for (uint256 i = 0; i < length; ++i) { + ManifestExecutionHook memory mh = manifest.executionHooks[i]; + LinkedListSet storage executionHooks = _storage.executionStorage[mh.executionSelector].executionHooks; + HookConfig hookConfig = HookConfigLib.packExecHook({ + _module: module, + _entityId: mh.entityId, + _hasPre: mh.isPreHook, + _hasPost: mh.isPostHook + }); + ModuleInstallCommons.addExecHooks(executionHooks, hookConfig); + } + + length = manifest.interfaceIds.length; + for (uint256 i = 0; i < length; ++i) { + _storage.supportedIfaces[manifest.interfaceIds[i]] += 1; + } + + ModuleInstallCommons.onInstall(module, moduleInstallData, type(IModule).interfaceId); + + emit IModularAccount.ExecutionInstalled(module, manifest); + } + + /// @notice Remove components according to the manifest, in reverse order (by component type) of their + /// installation. + function uninstallExecution(address module, ExecutionManifest calldata manifest, bytes calldata uninstallData) + external + onlyDelegateCall + { + AccountStorage storage _storage = getAccountStorage(); + + uint256 length = manifest.executionHooks.length; + for (uint256 i = 0; i < length; ++i) { + ManifestExecutionHook memory mh = manifest.executionHooks[i]; + LinkedListSet storage executionHooks = _storage.executionStorage[mh.executionSelector].executionHooks; + HookConfig hookConfig = HookConfigLib.packExecHook({ + _module: module, + _entityId: mh.entityId, + _hasPre: mh.isPreHook, + _hasPost: mh.isPostHook + }); + _removeExecHooks(executionHooks, hookConfig); + } + + length = manifest.executionFunctions.length; + for (uint256 i = 0; i < length; ++i) { + bytes4 selector = manifest.executionFunctions[i].executionSelector; + _removeExecutionFunction(selector); + } + + length = manifest.interfaceIds.length; + for (uint256 i = 0; i < length; ++i) { + _storage.supportedIfaces[manifest.interfaceIds[i]] -= 1; + } + + // Clear the module storage for the account. + bool onUninstallSuccess = ModuleInstallCommons.onUninstall(module, uninstallData); + + emit IModularAccount.ExecutionUninstalled(module, onUninstallSuccess, manifest); + } + + // Private Functions + + function _setExecutionFunction( + bytes4 selector, + bool skipRuntimeValidation, + bool allowGlobalValidation, + address module + ) internal { + ExecutionStorage storage _executionStorage = getAccountStorage().executionStorage[bytes4(selector)]; + + if (_executionStorage.module != address(0)) { + revert ExecutionFunctionAlreadySet(); + } + + // Note that there is no check for native function selectors. Installing a function with a colliding + // selector will lead to the installed function being unreachable. + + // Make sure incoming execution function is not a function in IModule + if (KnownSelectorsLib.isIModuleFunction(uint32(selector))) { + revert IModuleFunctionNotAllowed(); + } + + // Also make sure it doesn't collide with functions defined by ERC-4337 and called by the entry point. This + // prevents a malicious module from sneaking in a function with the same selector as e.g. + // `validatePaymasterUserOp` and turning the account into their own personal paymaster. + if (KnownSelectorsLib.isErc4337Function(uint32(selector))) { + revert Erc4337FunctionNotAllowed(); + } + + _executionStorage.module = module; + _executionStorage.skipRuntimeValidation = skipRuntimeValidation; + _executionStorage.allowGlobalValidation = allowGlobalValidation; + } + + function _removeExecutionFunction(bytes4 selector) internal { + ExecutionStorage storage _executionStorage = getAccountStorage().executionStorage[selector]; + + _executionStorage.module = address(0); + _executionStorage.skipRuntimeValidation = false; + _executionStorage.allowGlobalValidation = false; + } + + function _removeExecHooks(LinkedListSet storage hooks, HookConfig hookConfig) internal { + hooks.tryRemove(toSetValue(hookConfig)); + } +} diff --git a/src/helpers/NativeFunctionDelegate.sol b/src/helpers/NativeFunctionDelegate.sol new file mode 100644 index 00000000..3dca69a9 --- /dev/null +++ b/src/helpers/NativeFunctionDelegate.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.26; + +import {IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; +import {IModularAccountView} from "@erc6900/reference-implementation/interfaces/IModularAccountView.sol"; +import {IAccount} from "@eth-infinitism/account-abstraction/interfaces/IAccount.sol"; +import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; + +import {ModularAccountBase} from "../account/ModularAccountBase.sol"; + +/// @title NativeFunctionDelegate +/// @author Alchemy +/// +/// @dev This is a simple contract meant to be delegatecalled to determine whether a ModularAccountBase function +/// selector is native to the account implementation. +contract NativeFunctionDelegate { + function isNativeFunction(uint32 selector) external pure returns (bool) { + return + // check against IAccount methods + selector == uint32(IAccount.validateUserOp.selector) + // check against IModularAccount methods + || selector == uint32(IModularAccount.installExecution.selector) + || selector == uint32(IModularAccount.uninstallExecution.selector) + || selector == uint32(IModularAccount.installValidation.selector) + || selector == uint32(IModularAccount.uninstallValidation.selector) + || selector == uint32(IModularAccount.execute.selector) + || selector == uint32(IModularAccount.executeBatch.selector) + || selector == uint32(IModularAccount.executeWithRuntimeValidation.selector) + || selector == uint32(IModularAccount.accountId.selector) + // check against IERC165 methods + || selector == uint32(IERC165.supportsInterface.selector) + // check against UUPSUpgradeable methods + || selector == uint32(UUPSUpgradeable.proxiableUUID.selector) + || selector == uint32(UUPSUpgradeable.upgradeToAndCall.selector) + // check against IModularAccountView methods + || selector == uint32(IModularAccountView.getExecutionData.selector) + || selector == uint32(IModularAccountView.getValidationData.selector) + // check against ModularAccount methods + || selector == uint32(ModularAccountBase.performCreate.selector); + } +} diff --git a/src/libraries/ExecutionLib.sol b/src/libraries/ExecutionLib.sol index 36793daa..91f12cf2 100644 --- a/src/libraries/ExecutionLib.sol +++ b/src/libraries/ExecutionLib.sol @@ -71,17 +71,53 @@ library ExecutionLib { // Transiently copy the call data to a memory, and perform a self-call. function callBubbleOnRevertTransient(address target, uint256 value, bytes calldata callData) internal { - bytes memory callToSelf; + bytes memory encodedCall; assembly ("memory-safe") { // Store the length of the call - callToSelf := mload(0x40) - mstore(callToSelf, callData.length) + encodedCall := mload(0x40) + mstore(encodedCall, callData.length) // Copy in the calldata - calldatacopy(add(callToSelf, 0x20), callData.offset, callData.length) + calldatacopy(add(encodedCall, 0x20), callData.offset, callData.length) } - callBubbleOnRevert(target, value, callToSelf); + callBubbleOnRevert(target, value, encodedCall); + // Memory is discarded afterwards + } + + // Transiently copy the call data to a memory, and perform a self-call. + function delegatecallBubbleOnRevertTransient(address target) internal { + assembly ("memory-safe") { + // Store the length of the call + let fmp := mload(0x40) + + // Copy in the entire calldata + calldatacopy(fmp, 0, calldatasize()) + + let success := + delegatecall( + gas(), + target, + /*argOffset*/ + fmp, + /*argSize*/ + calldatasize(), + /*retOffset*/ + codesize(), + /*retSize*/ + 0 + ) + + // directly bubble up revert messages, if any. + if iszero(success) { + // For memory safety, copy this revert data to scratch space past the end of used memory. Because + // we immediately revert, we can omit storing the length as we normally would for a `bytes memory` + // type, as well as omit finalizing the allocation by updating the free memory pointer. + let revertDataLocation := mload(0x40) + returndatacopy(revertDataLocation, 0, returndatasize()) + revert(revertDataLocation, returndatasize()) + } + } // Memory is discarded afterwards } diff --git a/src/libraries/KnownSelectorsLib.sol b/src/libraries/KnownSelectorsLib.sol index 96742dc4..0ccd0b21 100644 --- a/src/libraries/KnownSelectorsLib.sol +++ b/src/libraries/KnownSelectorsLib.sol @@ -1,65 +1,35 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.26; -import {IAccount} from "@eth-infinitism/account-abstraction/interfaces/IAccount.sol"; -import {IAggregator} from "@eth-infinitism/account-abstraction/interfaces/IAggregator.sol"; -import {IPaymaster} from "@eth-infinitism/account-abstraction/interfaces/IPaymaster.sol"; -import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; -import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; - -import {ModularAccountBase} from "../account/ModularAccountBase.sol"; import {IExecutionHookModule} from "@erc6900/reference-implementation/interfaces/IExecutionHookModule.sol"; import {IExecutionModule} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; -import {IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; -import {IModularAccountView} from "@erc6900/reference-implementation/interfaces/IModularAccountView.sol"; import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; +import {IAggregator} from "@eth-infinitism/account-abstraction/interfaces/IAggregator.sol"; +import {IPaymaster} from "@eth-infinitism/account-abstraction/interfaces/IPaymaster.sol"; /// @dev Library to help to check if a selector is a know function selector of the modular account or ERC-4337 /// contract. library KnownSelectorsLib { - function isNativeFunction(bytes4 selector) internal pure returns (bool) { - return - // check against IAccount methods - selector == IAccount.validateUserOp.selector - // check against IModularAccount methods - || selector == IModularAccount.installExecution.selector - || selector == IModularAccount.uninstallExecution.selector - || selector == IModularAccount.installValidation.selector - || selector == IModularAccount.uninstallValidation.selector || selector == IModularAccount.execute.selector - || selector == IModularAccount.executeBatch.selector - || selector == IModularAccount.executeWithRuntimeValidation.selector - || selector == IModularAccount.accountId.selector - // check against IERC165 methods - || selector == IERC165.supportsInterface.selector - // check against UUPSUpgradeable methods - || selector == UUPSUpgradeable.proxiableUUID.selector - || selector == UUPSUpgradeable.upgradeToAndCall.selector - // check against IModularAccountView methods - || selector == IModularAccountView.getExecutionData.selector - || selector == IModularAccountView.getValidationData.selector - // check against ModularAccount methods - || selector == ModularAccountBase.performCreate.selector - || selector == ModularAccountBase.performCreate2.selector; - } - - function isErc4337Function(bytes4 selector) internal pure returns (bool) { - return selector == IAggregator.validateSignatures.selector - || selector == IAggregator.validateUserOpSignature.selector - || selector == IAggregator.aggregateSignatures.selector - || selector == IPaymaster.validatePaymasterUserOp.selector || selector == IPaymaster.postOp.selector; + function isErc4337Function(uint32 selector) internal pure returns (bool) { + return selector == uint32(IAggregator.validateSignatures.selector) + || selector == uint32(IAggregator.validateUserOpSignature.selector) + || selector == uint32(IAggregator.aggregateSignatures.selector) + || selector == uint32(IPaymaster.validatePaymasterUserOp.selector) + || selector == uint32(IPaymaster.postOp.selector); } - function isIModuleFunction(bytes4 selector) internal pure returns (bool) { - return selector == IModule.onInstall.selector || selector == IModule.onUninstall.selector - || selector == IModule.moduleId.selector || selector == IExecutionModule.executionManifest.selector - || selector == IExecutionHookModule.preExecutionHook.selector - || selector == IExecutionHookModule.postExecutionHook.selector - || selector == IValidationModule.validateUserOp.selector - || selector == IValidationModule.validateRuntime.selector - || selector == IValidationModule.validateSignature.selector - || selector == IValidationHookModule.preUserOpValidationHook.selector - || selector == IValidationHookModule.preRuntimeValidationHook.selector; + function isIModuleFunction(uint32 selector) internal pure returns (bool) { + return selector == uint32(IModule.onInstall.selector) || selector == uint32(IModule.onUninstall.selector) + || selector == uint32(IModule.moduleId.selector) + || selector == uint32(IExecutionModule.executionManifest.selector) + || selector == uint32(IExecutionHookModule.preExecutionHook.selector) + || selector == uint32(IExecutionHookModule.postExecutionHook.selector) + || selector == uint32(IValidationModule.validateUserOp.selector) + || selector == uint32(IValidationModule.validateRuntime.selector) + || selector == uint32(IValidationModule.validateSignature.selector) + || selector == uint32(IValidationHookModule.preUserOpValidationHook.selector) + || selector == uint32(IValidationHookModule.preRuntimeValidationHook.selector); } } diff --git a/src/libraries/ModuleInstallCommons.sol b/src/libraries/ModuleInstallCommons.sol new file mode 100644 index 00000000..f0e03d46 --- /dev/null +++ b/src/libraries/ModuleInstallCommons.sol @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: UNLICENSED + +pragma solidity ^0.8.26; + +import {HookConfig} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; +import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; +import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; + +import {toSetValue} from "../account/AccountStorage.sol"; +import {ExecutionLib} from "./ExecutionLib.sol"; +import {LinkedListSet, LinkedListSetLib} from "./LinkedListSetLib.sol"; + +/// @title ModuleInstallCommons +/// @author Alchemy +/// +/// @notice This is an internal library which holds module installation-related functions relevant to both the +/// ExecutionInstallDelegate and the ModuleManagerInternals contracts. +library ModuleInstallCommons { + using LinkedListSetLib for LinkedListSet; + + error InterfaceNotSupported(address module); + error ModuleInstallCallbackFailed(address module, bytes revertReason); + error ExecutionHookAlreadySet(HookConfig hookConfig); + + // Internal Functions + + // We don't need to bring the exec hook removal function here since it's only ever used in the + // ExecutionInstallLib + + /// @dev adds an execution hook to a specific set of hooks. + function addExecHooks(LinkedListSet storage hooks, HookConfig hookConfig) internal { + if (!hooks.tryAdd(toSetValue(hookConfig))) { + revert ExecutionHookAlreadySet(hookConfig); + } + } + + /// @dev setup the module storage for the account, reverts are bubbled up into a custom + /// ModuleInstallCallbackFailed + function onInstall(address module, bytes calldata data, bytes4 interfaceId) internal { + if (data.length > 0) { + if (!ERC165Checker.supportsERC165InterfaceUnchecked(module, interfaceId)) { + revert InterfaceNotSupported(module); + } + // solhint-disable-next-line no-empty-blocks + try IModule(module).onInstall(data) {} + catch { + bytes memory revertReason = ExecutionLib.collectReturnData(); + revert ModuleInstallCallbackFailed(module, revertReason); + } + } + } + + /// @dev clear the module storage for the account, reverts are IGNORED. Status is included in emitted event. + function onUninstall(address module, bytes calldata data) internal returns (bool onUninstallSuccess) { + onUninstallSuccess = true; + if (data.length > 0) { + // Clear the module storage for the account. + // solhint-disable-next-line no-empty-blocks + try IModule(module).onUninstall(data) {} + catch { + onUninstallSuccess = false; + } + } + } +} diff --git a/src/libraries/SemiModularKnownSelectorsLib.sol b/src/libraries/SemiModularKnownSelectorsLib.sol deleted file mode 100644 index 584a26f8..00000000 --- a/src/libraries/SemiModularKnownSelectorsLib.sol +++ /dev/null @@ -1,17 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.26; - -import {SemiModularAccountBase} from "../account/SemiModularAccountBase.sol"; -import {KnownSelectorsLib} from "./KnownSelectorsLib.sol"; - -/// @dev Library to help to check if a selector is a know function selector of the SemiModularAccountBase or -/// ModularAccount contract -library SemiModularKnownSelectorsLib { - function isNativeFunction(bytes4 selector) internal pure returns (bool) { - return KnownSelectorsLib.isNativeFunction(selector) - || selector == SemiModularAccountBase.updateFallbackSigner.selector - || selector == SemiModularAccountBase.setFallbackSignerDisabled.selector - || selector == SemiModularAccountBase.isFallbackSignerDisabled.selector - || selector == SemiModularAccountBase.getFallbackSigner.selector; - } -} diff --git a/src/modules/permissions/NativeTokenLimitModule.sol b/src/modules/permissions/NativeTokenLimitModule.sol index 3186ac86..ab7b0b2e 100644 --- a/src/modules/permissions/NativeTokenLimitModule.sol +++ b/src/modules/permissions/NativeTokenLimitModule.sol @@ -109,10 +109,7 @@ contract NativeTokenLimitModule is ModuleBase, IExecutionHookModule, IValidation for (uint256 i = 0; i < calls.length; i++) { value += calls[i].value; } - } else if ( - selector == ModularAccountBase.performCreate.selector - || selector == ModularAccountBase.performCreate2.selector - ) { + } else if (selector == ModularAccountBase.performCreate.selector) { value = abi.decode(callData, (uint256)); } diff --git a/test/account/ExecutionInstallDelegate.t.sol b/test/account/ExecutionInstallDelegate.t.sol new file mode 100644 index 00000000..328e73bf --- /dev/null +++ b/test/account/ExecutionInstallDelegate.t.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.26; + +import {ExecutionInstallDelegate} from "../../src/helpers/ExecutionInstallDelegate.sol"; +import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {ExecutionManifest} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; + +contract ExecutionInstallDelegateTest is AccountTestBase { + function test_fail_directCall_delegateCallOnly() public { + ExecutionInstallDelegate delegate = new ExecutionInstallDelegate(); + ExecutionManifest memory emptyManifest; + + vm.expectRevert(ExecutionInstallDelegate.OnlyDelegateCall.selector); + delegate.installExecution({module: address(0), manifest: emptyManifest, moduleInstallData: ""}); + } +} diff --git a/test/account/ModularAccount.t.sol b/test/account/ModularAccount.t.sol index f0bf47f0..d0668d12 100644 --- a/test/account/ModularAccount.t.sol +++ b/test/account/ModularAccount.t.sol @@ -38,8 +38,9 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa import {AccountBase} from "../../src/account/AccountBase.sol"; import {ModularAccount} from "../../src/account/ModularAccount.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; -import {ModuleManagerInternals} from "../../src/account/ModuleManagerInternals.sol"; import {SemiModularAccountBytecode} from "../../src/account/SemiModularAccountBytecode.sol"; +import {ExecutionInstallDelegate} from "../../src/helpers/ExecutionInstallDelegate.sol"; +import {ModuleInstallCommons} from "../../src/libraries/ModuleInstallCommons.sol"; import {SingleSignerValidationModule} from "../../src/modules/validation/SingleSignerValidationModule.sol"; import {Counter} from "../mocks/Counter.sol"; @@ -124,7 +125,7 @@ contract ModularAccountTest is AccountTestBase { function test_basicUserOp_withInitCode() public withSMATest { bytes memory callData = _isSMATest - ? abi.encodeCall(SemiModularAccountBytecode(payable(account1)).updateFallbackSigner, (owner2)) + ? abi.encodeCall(SemiModularAccountBytecode(payable(account1)).updateFallbackSignerData, (owner2, false)) : abi.encodeCall( ModularAccountBase.execute, ( @@ -334,7 +335,7 @@ contract ModularAccountTest is AccountTestBase { address badModule = CODELESS_ADDRESS; vm.expectRevert( - abi.encodeWithSelector(ModuleManagerInternals.InterfaceNotSupported.selector, address(badModule)) + abi.encodeWithSelector(ModuleInstallCommons.InterfaceNotSupported.selector, address(badModule)) ); ExecutionManifest memory m; @@ -354,12 +355,7 @@ contract ModularAccountTest is AccountTestBase { }); vm.prank(address(entryPoint)); - vm.expectRevert( - abi.encodeWithSelector( - ModuleManagerInternals.ExecutionFunctionAlreadySet.selector, - MockExecutionInstallationModule.executionInstallationExecute.selector - ) - ); + vm.expectRevert(ExecutionInstallDelegate.ExecutionFunctionAlreadySet.selector); account1.installExecution({ module: address(mockExecutionInstallationModule), manifest: m, @@ -424,7 +420,6 @@ contract ModularAccountTest is AccountTestBase { vm.stopPrank(); } - // TODO: Consider if this test belongs here or in the tests specific to the SingleSignerValidationModule function test_transferOwnership() public withSMATest { if (_isSMATest) { // Note: replaced "owner1" with address(0), this doesn't actually affect the account, but allows the @@ -590,7 +585,10 @@ contract ModularAccountTest is AccountTestBase { address expectedAddr = vm.computeCreateAddress(address(account1), vm.getNonce(address(account1))); vm.prank(address(entryPoint)); address returnedAddr = account1.performCreate( - 0, abi.encodePacked(type(ModularAccount).creationCode, abi.encode(address(entryPoint))) + 0, + abi.encodePacked(type(ModularAccount).creationCode, abi.encode(address(entryPoint))), + false, + bytes32(0) ); assertEq(returnedAddr, expectedAddr); @@ -599,7 +597,7 @@ contract ModularAccountTest is AccountTestBase { // Assert a reverting constructor causes a `CreateFailed` error vm.prank(address(entryPoint)); vm.expectRevert(ModularAccountBase.CreateFailed.selector); - account1.performCreate(0, abi.encodePacked(type(MockRevertingConstructor).creationCode)); + account1.performCreate(0, abi.encodePacked(type(MockRevertingConstructor).creationCode), false, 0); } function test_performCreate2() public withSMATest { @@ -610,7 +608,7 @@ contract ModularAccountTest is AccountTestBase { address expectedAddr = vm.computeCreate2Address(salt, initCodeHash, address(account1)); vm.prank(address(entryPoint)); - address returnedAddr = account1.performCreate2(0, initCode, salt); + address returnedAddr = account1.performCreate(0, initCode, true, salt); assertEq(returnedAddr, expectedAddr); assertEq(address(ModularAccount(payable(expectedAddr)).entryPoint()), address(entryPoint)); @@ -618,7 +616,7 @@ contract ModularAccountTest is AccountTestBase { vm.expectRevert(ModularAccountBase.CreateFailed.selector); // re-deploying with same salt should revert vm.prank(address(entryPoint)); - account1.performCreate2(0, initCode, salt); + account1.performCreate(0, initCode, true, salt); } function test_assertCallerEntryPoint() public withSMATest { diff --git a/test/account/SMASpecific.t.sol b/test/account/SMASpecific.t.sol new file mode 100644 index 00000000..38497dfd --- /dev/null +++ b/test/account/SMASpecific.t.sol @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.26; + +import {ModuleEntity} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; +import {HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; +import {ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; +import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; + +import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; +import {FALLBACK_VALIDATION} from "../../src/helpers/Constants.sol"; + +import {MockCountModule} from "../mocks/modules/MockCountModule.sol"; +import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {CODELESS_ADDRESS} from "../utils/TestConstants.sol"; + +contract SMASpecificTest is AccountTestBase { + using ModuleEntityLib for ModuleEntity; + using MessageHashUtils for bytes32; + + address public mockCountModule; + uint256 public transferAmount; + + function setUp() public override { + // smaStorageImpl = address(new SemiModularAccountStorageOnly(entryPoint)); + // (owner2, owner2Key) = makeAddrAndKey("owner2"); + _switchToSMA(); + transferAmount = 0.1 ether; + mockCountModule = address(new MockCountModule()); + } + + function testFuzz_fallbackValidation_hooksFlow( + uint32 validationHookCount, + uint32 valAssocExecHookCount, + bool[254] calldata execHooksHavePost + ) public { + validationHookCount = uint32(bound(validationHookCount, 0, 254)); + valAssocExecHookCount = uint32(bound(valAssocExecHookCount, 0, 254)); + + _installPreValidationHooks(validationHookCount); + + uint256 totalPostHooks = _installValidationAssociatedExecHooks(valAssocExecHookCount, execHooksHavePost); + + _runtimeTransfer(0); + _userOpTransfer(transferAmount); + + // Post run validation, ensuring hooks have run + assertEq(MockCountModule(mockCountModule).runtimeValidationHookRunCount(), validationHookCount); + assertEq(MockCountModule(mockCountModule).userOpValidationHookRunCount(), validationHookCount); + assertEq(MockCountModule(mockCountModule).preExecutionHookRunCount(), valAssocExecHookCount * 2); + assertEq(MockCountModule(mockCountModule).postExecutionHookRunCount(), totalPostHooks * 2); + + bytes[] memory hookUninstallDatas = new bytes[](validationHookCount + valAssocExecHookCount); + if (hookUninstallDatas.length > 0) { + hookUninstallDatas[0] = "a"; // mock nonzero length data to call `onUninstall()` once. + } + vm.prank(address(entryPoint)); + account1.uninstallValidation(_signerValidation, "", hookUninstallDatas); + + // Ensure the fallback validation works after uninstalling the hooks. + _runtimeTransfer(transferAmount * 2); + _userOpTransfer(transferAmount * 3); + + // After uninstallation, the hooks should not have run at all. + assertEq(MockCountModule(mockCountModule).runtimeValidationHookRunCount(), 0); + assertEq(MockCountModule(mockCountModule).userOpValidationHookRunCount(), 0); + assertEq(MockCountModule(mockCountModule).preExecutionHookRunCount(), 0); + assertEq(MockCountModule(mockCountModule).postExecutionHookRunCount(), 0); + } + + // Internal helpers + + function _runtimeTransfer(uint256 initialBalance) internal { + deal(address(account1), 1 ether); + address target = CODELESS_ADDRESS; + vm.prank(owner1); + account1.executeWithRuntimeValidation( + abi.encodeCall(ModularAccountBase.execute, (target, transferAmount, "")), + _encodeSignature(_signerValidation, GLOBAL_VALIDATION, "") + ); + assertEq(target.balance, transferAmount + initialBalance, "Target missing balance from runtime transfer"); + } + + function _userOpTransfer(uint256 initialBalance) internal { + // Pre-fund the account, arbitrarily high amount to cover arbitrarily high gas. + deal(address(account1), type(uint128).max); + + // Generate a target and ensure it has no balance. + address target = CODELESS_ADDRESS; + assertEq(target.balance, initialBalance, "Target has balance when it shouldn't"); + + // Encode a transfer to the target. + // bytes memory encodedCall = abi.encodeCall(ModularAccountBase.execute, (target, transferAmount, "")); + bytes memory encodedCall = abi.encodePacked( + ModularAccountBase.executeUserOp.selector, + abi.encodeCall(ModularAccountBase.execute, (target, transferAmount, "")) + ); + + // Run a UO with the encoded call. + _runUserOpWithFallbackValidation(encodedCall); + + assertEq(target.balance, transferAmount + initialBalance, "Target missing balance from UO transfer"); + } + + function _runUserOpWithFallbackValidation(bytes memory encodedCall) internal { + uint256 nonce = entryPoint.getNonce(address(account1), 0); + + PackedUserOperation memory userOp = PackedUserOperation({ + sender: address(account1), + nonce: nonce, + initCode: hex"", + callData: encodedCall, + accountGasLimits: _encodeGas(type(uint24).max, type(uint24).max), + preVerificationGas: 0, + gasFees: _encodeGas(1, 1), + paymasterAndData: hex"", + signature: hex"" + }); + + bytes32 userOpHash = entryPoint.getUserOpHash(userOp); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); + + userOp.signature = + _encodeSignature(FALLBACK_VALIDATION, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)); + + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); + userOps[0] = userOp; + + entryPoint.handleOps(userOps, beneficiary); + } + + function _installPreValidationHooks(uint32 count) internal { + bytes[] memory hooks = new bytes[](count); + + for (uint32 i = 0; i < count; ++i) { + hooks[i] = abi.encodePacked(HookConfigLib.packValidationHook(mockCountModule, i), ""); + } + + vm.prank(address(entryPoint)); + + account1.installValidation( + ValidationConfigLib.pack(_signerValidation, true, true, true), new bytes4[](0), "", hooks + ); + } + + function _installValidationAssociatedExecHooks(uint32 count, bool[254] memory execHooksHavePost) + internal + returns (uint256) + { + bytes[] memory hooks = new bytes[](count); + uint256 totalPostHookCount = 0; + + for (uint32 i = 0; i < count; ++i) { + bool hasPost = execHooksHavePost[i]; + if (hasPost) { + ++totalPostHookCount; + } + hooks[i] = + abi.encodePacked(HookConfigLib.packExecHook(mockCountModule, i, true, execHooksHavePost[i]), ""); + } + + vm.prank(address(entryPoint)); + + account1.installValidation( + ValidationConfigLib.pack(_signerValidation, true, true, true), new bytes4[](0), "", hooks + ); + return totalPostHookCount; + } +} diff --git a/test/account/SemiModularAccountDirectCall.t.sol b/test/account/SemiModularAccountDirectCall.t.sol index 5b463f21..043ba009 100644 --- a/test/account/SemiModularAccountDirectCall.t.sol +++ b/test/account/SemiModularAccountDirectCall.t.sol @@ -50,9 +50,9 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { // Negatives - function test_fail_smaDirectCall_disabledFallbackSigner() external withSMATest { + function test_fail_smaDirectCall_disabledFallbackSigner() external { vm.prank(owner1); - SemiModularAccountBase(payable(account1)).setFallbackSignerDisabled(true); + SemiModularAccountBase(payable(account1)).updateFallbackSignerData(address(0), true); bytes memory expectedRevertData = abi.encodeWithSelector( ModularAccountBase.ValidationFunctionMissing.selector, ModularAccountBase.execute.selector @@ -63,7 +63,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { account1.execute(_target, 0, ""); } - function test_fail_smaDirectCall_notFallbackSigner() external withSMATest { + function test_fail_smaDirectCall_notFallbackSigner() external { bytes memory expectedRevertData = abi.encodeWithSelector( ModularAccountBase.ValidationFunctionMissing.selector, ModularAccountBase.execute.selector ); @@ -75,7 +75,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { // Positives - function test_Flow_smaDirectCall_installedHooksUninstalled() external withSMATest { + function test_Flow_smaDirectCall_installedHooksUninstalled() external { // We install the validation as if it were a direct call validation, but we pass hooks from the // DirectCallModule, and while the validation remains the fallback signer + direct call entityId. bytes[] memory hooks = new bytes[](2); @@ -126,7 +126,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { assertFalse(_module.validationHookRan()); } - function test_smaDirectCall() external withSMATest { + function test_smaDirectCall() external { vm.prank(owner1); account1.execute(_target, 0, ""); } diff --git a/test/account/UpgradeToSma.t.sol b/test/account/UpgradeToSma.t.sol index 2bb10131..6cd3437d 100644 --- a/test/account/UpgradeToSma.t.sol +++ b/test/account/UpgradeToSma.t.sol @@ -68,7 +68,7 @@ contract UpgradeToSmaTest is AccountTestBase { // We call `updateFallbackSigner()` to upgrade from an MA to an SMA-S, because we are already initialized. vm.prank(address(entryPoint)); account1.upgradeToAndCall( - smaStorageImpl, abi.encodeCall(SemiModularAccountBase.updateFallbackSigner, owner2) + smaStorageImpl, abi.encodeCall(SemiModularAccountBase.updateFallbackSignerData, (owner2, false)) ); // The previous owner1 validation is still installed, so this should not revert. @@ -163,7 +163,7 @@ contract UpgradeToSmaTest is AccountTestBase { bytes memory encodedCall, bytes memory expectedRevertData ) internal { - uint256 nonce = entryPoint.getNonce(address(account), 0); + uint256 nonce = entryPoint.getNonce(account, 0); PackedUserOperation memory userOp = PackedUserOperation({ sender: account, diff --git a/test/libraries/KnowSelectors.t.sol b/test/libraries/KnowSelectors.t.sol deleted file mode 100644 index 8a469a03..00000000 --- a/test/libraries/KnowSelectors.t.sol +++ /dev/null @@ -1,51 +0,0 @@ -// This file is part of Modular Account. -// -// Copyright 2024 Alchemy Insights, Inc. -// -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This program is free software: you can redistribute it and/or modify it under the terms of the GNU General -// Public License as published by the Free Software Foundation, either version 3 of the License, or (at your -// option) any later version. -// -// This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the -// implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -// more details. -// -// You should have received a copy of the GNU General Public License along with this program. If not, see -// . - -pragma solidity ^0.8.26; - -import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; -import {IAccount} from "@eth-infinitism/account-abstraction/interfaces/IAccount.sol"; -import {IPaymaster} from "@eth-infinitism/account-abstraction/interfaces/IPaymaster.sol"; -import {Test} from "forge-std/src/Test.sol"; - -import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; -import {SemiModularAccountBase} from "../../src/account/SemiModularAccountBase.sol"; -import {KnownSelectorsLib} from "../../src/libraries/KnownSelectorsLib.sol"; -import {SemiModularKnownSelectorsLib} from "../../src/libraries/SemiModularKnownSelectorsLib.sol"; - -contract KnownSelectorsTest is Test { - function test_isNativeFunction() public pure { - assertTrue(KnownSelectorsLib.isNativeFunction(IAccount.validateUserOp.selector)); - assertTrue(KnownSelectorsLib.isNativeFunction(ModularAccountBase.installValidation.selector)); - } - - function test_sma_isNativeFunction() public pure { - assertTrue(SemiModularKnownSelectorsLib.isNativeFunction(IAccount.validateUserOp.selector)); - assertTrue( - SemiModularKnownSelectorsLib.isNativeFunction(SemiModularAccountBase.getFallbackSigner.selector) - ); - assertTrue(SemiModularKnownSelectorsLib.isNativeFunction(ModularAccountBase.installValidation.selector)); - } - - function test_isErc4337Function() public pure { - assertTrue(KnownSelectorsLib.isErc4337Function(IPaymaster.validatePaymasterUserOp.selector)); - } - - function test_isIModuleFunction() public pure { - assertTrue(KnownSelectorsLib.isIModuleFunction(IModule.moduleId.selector)); - } -} diff --git a/test/libraries/KnownSelectorsLib.t.sol b/test/libraries/KnownSelectorsLib.t.sol new file mode 100644 index 00000000..02d06933 --- /dev/null +++ b/test/libraries/KnownSelectorsLib.t.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.26; + +import {IExecutionHookModule} from "@erc6900/reference-implementation/interfaces/IExecutionHookModule.sol"; +import {IModule} from "@erc6900/reference-implementation/interfaces/IModule.sol"; +import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; +import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; + +import {IAccount} from "@eth-infinitism/account-abstraction/interfaces/IAccount.sol"; +import {IAggregator} from "@eth-infinitism/account-abstraction/interfaces/IAggregator.sol"; +import {IPaymaster} from "@eth-infinitism/account-abstraction/interfaces/IPaymaster.sol"; +import {Test} from "forge-std/src/Test.sol"; + +import {KnownSelectorsLib} from "../../src/libraries/KnownSelectorsLib.sol"; + +contract KnownSelectorsLibTest is Test { + function test_isErc4337Function() public pure { + assertTrue(KnownSelectorsLib.isErc4337Function(uint32(IAggregator.validateSignatures.selector))); + assertTrue(KnownSelectorsLib.isErc4337Function(uint32(IAggregator.validateUserOpSignature.selector))); + assertTrue(KnownSelectorsLib.isErc4337Function(uint32(IAggregator.aggregateSignatures.selector))); + assertTrue(KnownSelectorsLib.isErc4337Function(uint32(IPaymaster.validatePaymasterUserOp.selector))); + assertTrue(KnownSelectorsLib.isErc4337Function(uint32(IPaymaster.postOp.selector))); + + assertFalse(KnownSelectorsLib.isErc4337Function(uint32(IAccount.validateUserOp.selector))); + } + + function test_isIModuleFunction() public pure { + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IModule.onInstall.selector))); + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IModule.onUninstall.selector))); + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IModule.moduleId.selector))); + assertTrue( + KnownSelectorsLib.isIModuleFunction(uint32(IValidationHookModule.preUserOpValidationHook.selector)) + ); + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IValidationModule.validateUserOp.selector))); + assertTrue( + KnownSelectorsLib.isIModuleFunction(uint32(IValidationHookModule.preRuntimeValidationHook.selector)) + ); + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IValidationModule.validateRuntime.selector))); + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IExecutionHookModule.preExecutionHook.selector))); + assertTrue(KnownSelectorsLib.isIModuleFunction(uint32(IExecutionHookModule.postExecutionHook.selector))); + + assertFalse(KnownSelectorsLib.isIModuleFunction(uint32(IPaymaster.postOp.selector))); + } +} diff --git a/test/mocks/modules/MockCountModule.sol b/test/mocks/modules/MockCountModule.sol new file mode 100644 index 00000000..d22643bd --- /dev/null +++ b/test/mocks/modules/MockCountModule.sol @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.26; + +import {IExecutionHookModule} from "@erc6900/reference-implementation/interfaces/IExecutionHookModule.sol"; +import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; +import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; + +import {ModuleBase} from "../../../src/modules/ModuleBase.sol"; + +contract MockCountModule is ModuleBase, IExecutionHookModule, IValidationHookModule { + uint256 public preExecutionHookRunCount = 0; + uint256 public postExecutionHookRunCount = 0; + uint256 public runtimeValidationHookRunCount = 0; + uint256 public userOpValidationHookRunCount = 0; + uint256 public signatureValidationHookRunCount = 0; + + function onInstall(bytes calldata) external override {} + + function onUninstall(bytes calldata) external override { + preExecutionHookRunCount = 0; + postExecutionHookRunCount = 0; + runtimeValidationHookRunCount = 0; + userOpValidationHookRunCount = 0; + signatureValidationHookRunCount = 0; + } + + function preRuntimeValidationHook(uint32, address, uint256, bytes calldata, bytes calldata) external { + runtimeValidationHookRunCount++; + } + + function preExecutionHook(uint32, address, uint256, bytes calldata) external override returns (bytes memory) { + preExecutionHookRunCount++; + return abi.encode(keccak256(hex"04546b")); + } + + function postExecutionHook(uint32, bytes calldata preExecHookData) external override { + require( + abi.decode(preExecHookData, (bytes32)) == keccak256(hex"04546b"), + "mock direct call post execution hook failed" + ); + postExecutionHookRunCount++; + } + + function moduleId() external pure override returns (string memory) { + return "erc6900.direct-call-module.1.0.0"; + } + + function supportsInterface(bytes4 interfaceId) + public + view + virtual + override(ModuleBase, IERC165) + returns (bool) + { + return interfaceId == type(IExecutionHookModule).interfaceId + || interfaceId == type(IValidationHookModule).interfaceId || super.supportsInterface(interfaceId); + } + + function preUserOpValidationHook(uint32, PackedUserOperation calldata, bytes32) + external + override + returns (uint256) + { + userOpValidationHookRunCount++; + return 0; + } + + function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external view override {} +} diff --git a/test/mocks/modules/MockSMADirectFallbackModule.sol b/test/mocks/modules/MockSMADirectFallbackModule.sol index f5adb868..87e977eb 100644 --- a/test/mocks/modules/MockSMADirectFallbackModule.sol +++ b/test/mocks/modules/MockSMADirectFallbackModule.sol @@ -54,7 +54,7 @@ contract MockSMADirectFallbackModule is ModuleBase, IExecutionHookModule, IValid override returns (bytes memory) { - address fallbackSigner = SemiModularAccountBase(payable(msg.sender)).getFallbackSigner(); + (address fallbackSigner,) = SemiModularAccountBase(payable(msg.sender)).getFallbackSignerData(); require(sender == fallbackSigner, "mock SMA fallback direct call call pre execution hook failed"); preHookRan = true; return abi.encode(keccak256(hex"04546b")); diff --git a/test/modules/NativeTokenLimitModule.t.sol b/test/modules/NativeTokenLimitModule.t.sol index 78d9f7c3..8bd81d69 100644 --- a/test/modules/NativeTokenLimitModule.t.sol +++ b/test/modules/NativeTokenLimitModule.t.sol @@ -85,11 +85,15 @@ contract NativeTokenLimitModuleTest is AccountTestBase { } function _getPerformCreateCalldata(uint256 value) internal pure returns (bytes memory) { - return abi.encodeCall(ModularAccountBase.performCreate, (value, type(MockDeployment).creationCode)); + return abi.encodeCall( + ModularAccountBase.performCreate, (value, type(MockDeployment).creationCode, false, bytes32(0)) + ); } function _getPerformCreate2Calldata(uint256 value, bytes32 salt) internal pure returns (bytes memory) { - return abi.encodeCall(ModularAccountBase.performCreate2, (value, type(MockDeployment).creationCode, salt)); + return abi.encodeCall( + ModularAccountBase.performCreate, (value, type(MockDeployment).creationCode, true, salt) + ); } function _getPackedUO(uint256 gas1, uint256 gas2, uint256 gas3, uint256 gasPrice, bytes memory callData) diff --git a/test/utils/AccountTestBase.sol b/test/utils/AccountTestBase.sol index 4b5d36d6..dbecfe2b 100644 --- a/test/utils/AccountTestBase.sol +++ b/test/utils/AccountTestBase.sol @@ -238,7 +238,9 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { if (_isSMATest) { account1.executeWithRuntimeValidation( - abi.encodeCall(SemiModularAccountBytecode(payable(account1)).updateFallbackSigner, (address(this))), + abi.encodeCall( + SemiModularAccountBytecode(payable(account1)).updateFallbackSignerData, (address(this), false) + ), _encodeSignature(_signerValidation, GLOBAL_VALIDATION, "") ); return;