diff --git a/foundry.toml b/foundry.toml index 2aa76ba2..0252d4e5 100644 --- a/foundry.toml +++ b/foundry.toml @@ -38,7 +38,7 @@ runs = 5000 depth = 32 [profile.deep.fuzz] -runs = 10000 +runs = 100000 [profile.deep.invariant] runs = 5000 diff --git a/src/account/AccountFactory.sol b/src/account/AccountFactory.sol index 67fcd8c6..ca15e815 100644 --- a/src/account/AccountFactory.sol +++ b/src/account/AccountFactory.sol @@ -51,8 +51,7 @@ contract AccountFactory is Ownable { ValidationConfigLib.pack(SINGLE_SIGNER_VALIDATION, entityId, true, true), new bytes4[](0), pluginInstallData, - "", - "" + new bytes[](0) ); } diff --git a/src/account/AccountLoupe.sol b/src/account/AccountLoupe.sol index f0afc1b2..bcd6dbcd 100644 --- a/src/account/AccountLoupe.sol +++ b/src/account/AccountLoupe.sol @@ -6,14 +6,16 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeab import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {HookConfigLib} from "../helpers/HookConfigLib.sol"; import {ExecutionHook, IAccountLoupe} from "../interfaces/IAccountLoupe.sol"; -import {IModuleManager, ModuleEntity} from "../interfaces/IModuleManager.sol"; +import {HookConfig, IModuleManager, ModuleEntity} from "../interfaces/IModuleManager.sol"; import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; -import {getAccountStorage, toExecutionHook, toSelector} from "./AccountStorage.sol"; +import {getAccountStorage, toHookConfig, toSelector} from "./AccountStorage.sol"; abstract contract AccountLoupe is IAccountLoupe { using EnumerableSet for EnumerableSet.Bytes32Set; using EnumerableMap for EnumerableMap.AddressToUintMap; + using HookConfigLib for HookConfig; /// @inheritdoc IAccountLoupe function getExecutionFunctionHandler(bytes4 selector) external view override returns (address module) { @@ -56,8 +58,12 @@ abstract contract AccountLoupe is IAccountLoupe { for (uint256 i = 0; i < executionHooksLength; ++i) { bytes32 key = hooks.at(i); - ExecutionHook memory execHook = execHooks[i]; - (execHook.hookFunction, execHook.isPreHook, execHook.isPostHook) = toExecutionHook(key); + HookConfig hookConfig = toHookConfig(key); + execHooks[i] = ExecutionHook({ + hookFunction: hookConfig.moduleEntity(), + isPreHook: hookConfig.hasPreHook(), + isPostHook: hookConfig.hasPostHook() + }); } } @@ -74,8 +80,12 @@ abstract contract AccountLoupe is IAccountLoupe { permissionHooks = new ExecutionHook[](executionHooksLength); for (uint256 i = 0; i < executionHooksLength; ++i) { bytes32 key = hooks.at(i); - ExecutionHook memory execHook = permissionHooks[i]; - (execHook.hookFunction, execHook.isPreHook, execHook.isPostHook) = toExecutionHook(key); + HookConfig hookConfig = toHookConfig(key); + permissionHooks[i] = ExecutionHook({ + hookFunction: hookConfig.moduleEntity(), + isPreHook: hookConfig.hasPreHook(), + isPostHook: hookConfig.hasPostHook() + }); } } diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index c9845a8e..f7d279e4 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -3,8 +3,7 @@ pragma solidity ^0.8.25; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; -import {ModuleEntity} from "../interfaces/IModuleManager.sol"; +import {HookConfig, ModuleEntity} from "../interfaces/IModuleManager.sol"; // bytes = keccak256("ERC6900.UpgradeableModularAccount.Storage") bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c401499164f368baef687948c315d9073e40; @@ -70,19 +69,12 @@ function toModuleEntity(bytes32 setValue) pure returns (ModuleEntity) { // 0x________________________________________________AA____________________ is pre hook // 0x__________________________________________________BB__________________ is post hook -function toSetValue(ExecutionHook memory executionHook) pure returns (bytes32) { - return bytes32(ModuleEntity.unwrap(executionHook.hookFunction)) - | bytes32(executionHook.isPreHook ? uint256(1) << 56 : 0) - | bytes32(executionHook.isPostHook ? uint256(1) << 48 : 0); +function toSetValue(HookConfig hookConfig) pure returns (bytes32) { + return bytes32(HookConfig.unwrap(hookConfig)); } -function toExecutionHook(bytes32 setValue) - pure - returns (ModuleEntity hookFunction, bool isPreHook, bool isPostHook) -{ - hookFunction = ModuleEntity.wrap(bytes24(setValue)); - isPreHook = (uint256(setValue) >> 56) & 0xFF == 1; - isPostHook = (uint256(setValue) >> 48) & 0xFF == 1; +function toHookConfig(bytes32 setValue) pure returns (HookConfig) { + return HookConfig.wrap(bytes26(setValue)); } function toSetValue(bytes4 selector) pure returns (bytes32) { diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index 5eeeb77b..1c5dc6db 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -5,19 +5,27 @@ import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165C import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import {DIRECT_CALL_VALIDATION_ENTITYID, MAX_PRE_VALIDATION_HOOKS} from "../helpers/Constants.sol"; +import {MAX_PRE_VALIDATION_HOOKS} from "../helpers/Constants.sol"; +import {HookConfigLib} from "../helpers/HookConfigLib.sol"; import {KnownSelectors} from "../helpers/KnownSelectors.sol"; import {ModuleEntityLib} from "../helpers/ModuleEntityLib.sol"; import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; -import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; import {IModule, ManifestExecutionHook, ModuleManifest} from "../interfaces/IModule.sol"; -import {IModuleManager, ModuleEntity, ValidationConfig} from "../interfaces/IModuleManager.sol"; -import {AccountStorage, SelectorData, ValidationData, getAccountStorage, toSetValue} from "./AccountStorage.sol"; +import {HookConfig, IModuleManager, ModuleEntity, ValidationConfig} from "../interfaces/IModuleManager.sol"; +import { + AccountStorage, + SelectorData, + ValidationData, + getAccountStorage, + toModuleEntity, + toSetValue +} from "./AccountStorage.sol"; abstract contract ModuleManagerInternals is IModuleManager { using EnumerableSet for EnumerableSet.Bytes32Set; using ModuleEntityLib for ModuleEntity; using ValidationConfigLib for ValidationConfig; + using HookConfigLib for HookConfig; error ArrayLengthMismatch(); error Erc4337FunctionNotAllowed(bytes4 selector); @@ -25,7 +33,7 @@ abstract contract ModuleManagerInternals is IModuleManager { error IModuleFunctionNotAllowed(bytes4 selector); error NativeFunctionNotAllowed(bytes4 selector); error NullModule(); - error PermissionAlreadySet(ModuleEntity validationFunction, ExecutionHook hook); + error PermissionAlreadySet(ModuleEntity validationFunction, HookConfig hookConfig); error ModuleInstallCallbackFailed(address module, bytes revertReason); error ModuleInterfaceNotSupported(address module); error ModuleNotInstalled(address module); @@ -108,30 +116,12 @@ abstract contract ModuleManagerInternals is IModuleManager { } } - function _addExecHooks( - EnumerableSet.Bytes32Set storage hooks, - ModuleEntity hookFunction, - bool isPreExecHook, - bool isPostExecHook - ) internal { - hooks.add( - toSetValue( - ExecutionHook({hookFunction: hookFunction, isPreHook: isPreExecHook, isPostHook: isPostExecHook}) - ) - ); + function _addExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal { + hooks.add(toSetValue(hookConfig)); } - function _removeExecHooks( - EnumerableSet.Bytes32Set storage hooks, - ModuleEntity hookFunction, - bool isPreExecHook, - bool isPostExecHook - ) internal { - hooks.remove( - toSetValue( - ExecutionHook({hookFunction: hookFunction, isPreHook: isPreExecHook, isPostHook: isPostExecHook}) - ) - ); + function _removeExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal { + hooks.remove(toSetValue(hookConfig)); } function _installModule(address module, ModuleManifest calldata manifest, bytes memory moduleInstallData) @@ -162,8 +152,13 @@ abstract contract ModuleManagerInternals is IModuleManager { for (uint256 i = 0; i < length; ++i) { ManifestExecutionHook memory mh = manifest.executionHooks[i]; EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks; - ModuleEntity hookFunction = ModuleEntityLib.pack(module, mh.entityId); - _addExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook); + HookConfig hookConfig = HookConfigLib.packExecHook({ + _module: module, + _entityId: mh.entityId, + _hasPre: mh.isPreHook, + _hasPost: mh.isPostHook + }); + _addExecHooks(execHooks, hookConfig); } length = manifest.interfaceIds.length; @@ -191,9 +186,14 @@ abstract contract ModuleManagerInternals is IModuleManager { uint256 length = manifest.executionHooks.length; for (uint256 i = 0; i < length; ++i) { ManifestExecutionHook memory mh = manifest.executionHooks[i]; - ModuleEntity hookFunction = ModuleEntityLib.pack(module, mh.entityId); EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks; - _removeExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook); + HookConfig hookConfig = HookConfigLib.packExecHook({ + _module: module, + _entityId: mh.entityId, + _hasPre: mh.isPreHook, + _hasPost: mh.isPostHook + }); + _removeExecHooks(execHooks, hookConfig); } length = manifest.executionFunctions.length; @@ -218,53 +218,44 @@ abstract contract ModuleManagerInternals is IModuleManager { emit ModuleUninstalled(module, onUninstallSuccess); } + function _onInstall(address module, bytes calldata data) internal { + if (data.length > 0) { + IModule(module).onInstall(data); + } + } + + function _onUninstall(address module, bytes calldata data) internal { + if (data.length > 0) { + IModule(module).onUninstall(data); + } + } + function _installValidation( ValidationConfig validationConfig, - bytes4[] memory selectors, + bytes4[] calldata selectors, bytes calldata installData, - bytes memory preValidationHooks, - bytes memory permissionHooks + bytes[] calldata hooks ) internal { ValidationData storage _validationData = getAccountStorage().validationData[validationConfig.moduleEntity()]; - if (preValidationHooks.length > 0) { - (ModuleEntity[] memory preValidationFunctions, bytes[] memory initDatas) = - abi.decode(preValidationHooks, (ModuleEntity[], bytes[])); - - for (uint256 i = 0; i < preValidationFunctions.length; ++i) { - ModuleEntity preValidationFunction = preValidationFunctions[i]; + for (uint256 i = 0; i < hooks.length; ++i) { + HookConfig hookConfig = HookConfig.wrap(bytes26(hooks[i][:26])); + bytes calldata hookData = hooks[i][26:]; - _validationData.preValidationHooks.push(preValidationFunction); + if (hookConfig.isValidationHook()) { + _validationData.preValidationHooks.push(hookConfig.moduleEntity()); - if (initDatas[i].length > 0) { - (address preValidationModule,) = ModuleEntityLib.unpack(preValidationFunction); - IModule(preValidationModule).onInstall(initDatas[i]); + // Avoid collision between reserved index and actual indices + if (_validationData.preValidationHooks.length > MAX_PRE_VALIDATION_HOOKS) { + revert PreValidationHookLimitExceeded(); } + } // Hook is an execution hook + else if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) { + revert PermissionAlreadySet(validationConfig.moduleEntity(), hookConfig); } - // Avoid collision between reserved index and actual indices - if (_validationData.preValidationHooks.length > MAX_PRE_VALIDATION_HOOKS) { - revert PreValidationHookLimitExceeded(); - } - } - - if (permissionHooks.length > 0) { - (ExecutionHook[] memory permissionFunctions, bytes[] memory initDatas) = - abi.decode(permissionHooks, (ExecutionHook[], bytes[])); - - for (uint256 i = 0; i < permissionFunctions.length; ++i) { - ExecutionHook memory permissionFunction = permissionFunctions[i]; - - if (!_validationData.permissionHooks.add(toSetValue(permissionFunction))) { - revert PermissionAlreadySet(validationConfig.moduleEntity(), permissionFunction); - } - - if (initDatas[i].length > 0) { - (address executionModule,) = ModuleEntityLib.unpack(permissionFunction.hookFunction); - IModule(executionModule).onInstall(initDatas[i]); - } - } + _onInstall(hookConfig.module(), hookData); } for (uint256 i = 0; i < selectors.length; ++i) { @@ -274,56 +265,59 @@ abstract contract ModuleManagerInternals is IModuleManager { } } - if (validationConfig.entityId() != DIRECT_CALL_VALIDATION_ENTITYID) { - // Only allow global validations and signature validations if they're not direct-call validations. + _validationData.isGlobal = validationConfig.isGlobal(); + _validationData.isSignatureValidation = validationConfig.isSignatureValidation(); - _validationData.isGlobal = validationConfig.isGlobal(); - _validationData.isSignatureValidation = validationConfig.isSignatureValidation(); - if (installData.length > 0) { - IModule(validationConfig.module()).onInstall(installData); - } - } + _onInstall(validationConfig.module(), installData); } function _uninstallValidation( ModuleEntity validationFunction, bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData, - bytes calldata permissionHookUninstallData + bytes[] calldata hookUninstallDatas ) internal { ValidationData storage _validationData = getAccountStorage().validationData[validationFunction]; _removeValidationFunction(validationFunction); - { - bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[])); + // Send `onUninstall` to hooks + if (hookUninstallDatas.length > 0) { + // If any uninstall data is provided, assert it is of the correct length. + if ( + hookUninstallDatas.length + != _validationData.preValidationHooks.length + _validationData.permissionHooks.length() + ) { + revert ArrayLengthMismatch(); + } - // Clear pre validation hooks - ModuleEntity[] storage preValidationHooks = _validationData.preValidationHooks; - for (uint256 i = 0; i < preValidationHooks.length; ++i) { - ModuleEntity preValidationFunction = preValidationHooks[i]; - if (preValidationHookUninstallDatas[0].length > 0) { - (address preValidationModule,) = ModuleEntityLib.unpack(preValidationFunction); - IModule(preValidationModule).onUninstall(preValidationHookUninstallDatas[0]); - } + // Hook uninstall data is provided in the order of pre-validation hooks, then permission hooks. + uint256 hookIndex = 0; + for (uint256 i = 0; i < _validationData.preValidationHooks.length; ++i) { + bytes calldata hookData = hookUninstallDatas[hookIndex]; + (address hookModule,) = ModuleEntityLib.unpack(_validationData.preValidationHooks[i]); + _onUninstall(hookModule, hookData); + hookIndex++; } - delete _validationData.preValidationHooks; - } - { - bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[])); - - // Clear permission hooks - EnumerableSet.Bytes32Set storage permissionHooks = _validationData.permissionHooks; - uint256 permissionHookLen = permissionHooks.length(); - for (uint256 i = 0; i < permissionHookLen; ++i) { - bytes32 permissionHook = permissionHooks.at(0); - permissionHooks.remove(permissionHook); - address permissionHookModule = address(uint160(bytes20(permissionHook))); - IModule(permissionHookModule).onUninstall(permissionHookUninstallDatas[i]); + for (uint256 i = 0; i < _validationData.permissionHooks.length(); ++i) { + bytes calldata hookData = hookUninstallDatas[hookIndex]; + (address hookModule,) = + ModuleEntityLib.unpack(toModuleEntity(_validationData.permissionHooks.at(i))); + _onUninstall(hookModule, hookData); + hookIndex++; } } + // Clear all stored hooks + delete _validationData.preValidationHooks; + + EnumerableSet.Bytes32Set storage permissionHooks = _validationData.permissionHooks; + uint256 permissionHookLen = permissionHooks.length(); + for (uint256 i = 0; i < permissionHookLen; ++i) { + bytes32 permissionHook = permissionHooks.at(0); + permissionHooks.remove(permissionHook); + } + // Clear selectors uint256 selectorLen = _validationData.selectors.length(); for (uint256 i = 0; i < selectorLen; ++i) { @@ -331,9 +325,7 @@ abstract contract ModuleManagerInternals is IModuleManager { _validationData.selectors.remove(selectorSetValue); } - if (uninstallData.length > 0) { - (address module,) = ModuleEntityLib.unpack(validationFunction); - IModule(module).onUninstall(uninstallData); - } + (address module,) = ModuleEntityLib.unpack(validationFunction); + _onUninstall(module, uninstallData); } } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 87379241..f8545051 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -12,6 +12,7 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeab import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {HookConfig, HookConfigLib} from "../helpers/HookConfigLib.sol"; import {ModuleEntityLib} from "../helpers/ModuleEntityLib.sol"; import {SparseCalldataSegmentLib} from "../helpers/SparseCalldataSegmentLib.sol"; @@ -27,7 +28,7 @@ import {IValidation} from "../interfaces/IValidation.sol"; import {IValidationHook} from "../interfaces/IValidationHook.sol"; import {AccountExecutor} from "./AccountExecutor.sol"; import {AccountLoupe} from "./AccountLoupe.sol"; -import {AccountStorage, getAccountStorage, toExecutionHook, toSetValue} from "./AccountStorage.sol"; +import {AccountStorage, getAccountStorage, toHookConfig, toSetValue} from "./AccountStorage.sol"; import {AccountStorageInitializable} from "./AccountStorageInitializable.sol"; import {ModuleManagerInternals} from "./ModuleManagerInternals.sol"; @@ -46,6 +47,7 @@ contract UpgradeableModularAccount is using EnumerableSet for EnumerableSet.Bytes32Set; using ModuleEntityLib for ModuleEntity; using ValidationConfigLib for ValidationConfig; + using HookConfigLib for HookConfig; using SparseCalldataSegmentLib for bytes; struct PostExecToRun { @@ -249,12 +251,11 @@ contract UpgradeableModularAccount is /// @dev This function is only callable once, and only by the EntryPoint. function initializeWithValidation( ValidationConfig validationConfig, - bytes4[] memory selectors, + bytes4[] calldata selectors, bytes calldata installData, - bytes calldata preValidationHooks, - bytes calldata permissionHooks + bytes[] calldata hooks ) external initializer { - _installValidation(validationConfig, selectors, installData, preValidationHooks, permissionHooks); + _installValidation(validationConfig, selectors, installData, hooks); emit ModularAccountInitialized(_ENTRY_POINT); } @@ -262,12 +263,11 @@ contract UpgradeableModularAccount is /// @notice May be validated by a global validation. function installValidation( ValidationConfig validationConfig, - bytes4[] memory selectors, + bytes4[] calldata selectors, bytes calldata installData, - bytes calldata preValidationHooks, - bytes calldata permissionHooks + bytes[] calldata hooks ) external wrapNativeFunction { - _installValidation(validationConfig, selectors, installData, preValidationHooks, permissionHooks); + _installValidation(validationConfig, selectors, installData, hooks); } /// @inheritdoc IModuleManager @@ -275,12 +275,9 @@ contract UpgradeableModularAccount is function uninstallValidation( ModuleEntity validationFunction, bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData, - bytes calldata permissionHookUninstallData + bytes[] calldata hookUninstallData ) external wrapNativeFunction { - _uninstallValidation( - validationFunction, uninstallData, preValidationHookUninstallData, permissionHookUninstallData - ); + _uninstallValidation(validationFunction, uninstallData, hookUninstallData); } /// @notice ERC165 introspection @@ -503,26 +500,24 @@ contract UpgradeableModularAccount is // Copy all post hooks to the array. This happens before any pre hooks are run, so we can // be sure that the set of hooks to run will not be affected by state changes mid-execution. for (uint256 i = 0; i < hooksLength; ++i) { - bytes32 key = executionHooks.at(i); - (ModuleEntity hookFunction,, bool isPostHook) = toExecutionHook(key); - if (isPostHook) { - postHooksToRun[i].postExecHook = hookFunction; + HookConfig hookConfig = toHookConfig(executionHooks.at(i)); + if (hookConfig.hasPostHook()) { + postHooksToRun[i].postExecHook = hookConfig.moduleEntity(); } } // Run the pre hooks and copy their return data to the post hooks array, if an associated post-exec hook // exists. for (uint256 i = 0; i < hooksLength; ++i) { - bytes32 key = executionHooks.at(i); - (ModuleEntity hookFunction, bool isPreHook, bool isPostHook) = toExecutionHook(key); + HookConfig hookConfig = toHookConfig(executionHooks.at(i)); - if (isPreHook) { + if (hookConfig.hasPreHook()) { bytes memory preExecHookReturnData; - preExecHookReturnData = _runPreExecHook(hookFunction, data); + preExecHookReturnData = _runPreExecHook(hookConfig.moduleEntity(), data); // If there is an associated post-exec hook, save the return data. - if (isPostHook) { + if (hookConfig.hasPostHook()) { postHooksToRun[i].preExecHookReturnData = preExecHookReturnData; } } diff --git a/src/helpers/HookConfigLib.sol b/src/helpers/HookConfigLib.sol new file mode 100644 index 00000000..9c94e4bd --- /dev/null +++ b/src/helpers/HookConfigLib.sol @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.25; + +import {HookConfig, ModuleEntity} from "../interfaces/IModuleManager.sol"; + +// Hook types: +// Exec hook: bools for hasPre, hasPost +// Validation hook: no bools + +// Hook fields: +// module address +// entity ID +// hook type +// if exec hook: hasPre, hasPost + +// Hook config is a packed representation of a hook function and flags for its configuration. +// Layout: +// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address +// 0x________________________________________BBBBBBBB________________ // Entity ID +// 0x________________________________________________CC______________ // Type +// 0x__________________________________________________DD____________ // exec hook flags +// + +// Hook types: +// 0x00 // Exec (selector and validation associated) +// 0x01 // Validation + +// Exec hook flags layout: +// 0b000000__ // unused +// 0b______A_ // hasPre +// 0b_______B // hasPost + +library HookConfigLib { + // Hook type constants + // Exec has no bits set + bytes32 internal constant _HOOK_TYPE_EXEC = bytes32(uint256(0)); + // Validation has 1 in the 25th byte + bytes32 internal constant _HOOK_TYPE_VALIDATION = bytes32(uint256(1) << 56); + + // Exec hook flags constants + // Pre hook has 1 in 2's bit in the 26th byte + bytes32 internal constant _EXEC_HOOK_HAS_PRE = bytes32(uint256(1) << 49); + // Post hook has 1 in 1's bit in the 26th byte + bytes32 internal constant _EXEC_HOOK_HAS_POST = bytes32(uint256(1) << 48); + + function packValidationHook(ModuleEntity _hookFunction) internal pure returns (HookConfig) { + return + HookConfig.wrap(bytes26(bytes26(ModuleEntity.unwrap(_hookFunction)) | bytes26(_HOOK_TYPE_VALIDATION))); + } + + function packValidationHook(address _module, uint32 _entityId) internal pure returns (HookConfig) { + return HookConfig.wrap( + bytes25( + // module address stored in the first 20 bytes + bytes25(bytes20(_module)) + // entityId stored in the 21st - 24th byte + | bytes25(bytes24(uint192(_entityId))) | bytes25(_HOOK_TYPE_VALIDATION) + ) + ); + } + + function packExecHook(ModuleEntity _hookFunction, bool _hasPre, bool _hasPost) + internal + pure + returns (HookConfig) + { + return HookConfig.wrap( + bytes26( + bytes26(ModuleEntity.unwrap(_hookFunction)) + // | bytes26(_HOOK_TYPE_EXEC) // Can omit because exec type is 0 + | bytes26(_hasPre ? _EXEC_HOOK_HAS_PRE : bytes32(0)) + | bytes26(_hasPost ? _EXEC_HOOK_HAS_POST : bytes32(0)) + ) + ); + } + + function packExecHook(address _module, uint32 _entityId, bool _hasPre, bool _hasPost) + internal + pure + returns (HookConfig) + { + return HookConfig.wrap( + bytes26( + // module address stored in the first 20 bytes + bytes26(bytes20(_module)) + // entityId stored in the 21st - 24th byte + | bytes26(bytes24(uint192(_entityId))) + // | bytes26(_HOOK_TYPE_EXEC) // Can omit because exec type is 0 + | bytes26(_hasPre ? _EXEC_HOOK_HAS_PRE : bytes32(0)) + | bytes26(_hasPost ? _EXEC_HOOK_HAS_POST : bytes32(0)) + ) + ); + } + + function unpackValidationHook(HookConfig _config) internal pure returns (ModuleEntity _hookFunction) { + bytes26 configBytes = HookConfig.unwrap(_config); + _hookFunction = ModuleEntity.wrap(bytes24(configBytes)); + } + + function unpackExecHook(HookConfig _config) + internal + pure + returns (ModuleEntity _hookFunction, bool _hasPre, bool _hasPost) + { + bytes26 configBytes = HookConfig.unwrap(_config); + _hookFunction = ModuleEntity.wrap(bytes24(configBytes)); + _hasPre = configBytes & _EXEC_HOOK_HAS_PRE != 0; + _hasPost = configBytes & _EXEC_HOOK_HAS_POST != 0; + } + + function module(HookConfig _config) internal pure returns (address) { + return address(bytes20(HookConfig.unwrap(_config))); + } + + function entityId(HookConfig _config) internal pure returns (uint32) { + return uint32(bytes4(HookConfig.unwrap(_config) << 160)); + } + + function moduleEntity(HookConfig _config) internal pure returns (ModuleEntity) { + return ModuleEntity.wrap(bytes24(HookConfig.unwrap(_config))); + } + + // Check if the hook is a validation hook + // If false, it is an exec hook + function isValidationHook(HookConfig _config) internal pure returns (bool) { + return HookConfig.unwrap(_config) & _HOOK_TYPE_VALIDATION != 0; + } + + // Check if the exec hook has a pre hook + // Undefined behavior if the hook is not an exec hook + function hasPreHook(HookConfig _config) internal pure returns (bool) { + return HookConfig.unwrap(_config) & _EXEC_HOOK_HAS_PRE != 0; + } + + // Check if the exec hook has a post hook + // Undefined behavior if the hook is not an exec hook + function hasPostHook(HookConfig _config) internal pure returns (bool) { + return HookConfig.unwrap(_config) & _EXEC_HOOK_HAS_POST != 0; + } +} diff --git a/src/interfaces/IModuleManager.sol b/src/interfaces/IModuleManager.sol index 9923bd34..d2e63f74 100644 --- a/src/interfaces/IModuleManager.sol +++ b/src/interfaces/IModuleManager.sol @@ -7,6 +7,8 @@ type ModuleEntity is bytes24; type ValidationConfig is bytes26; +type HookConfig is bytes26; + interface IModuleManager { event ModuleInstalled(address indexed module); @@ -29,14 +31,14 @@ interface IModuleManager { /// @param validationConfig The validation function to install, along with configuration flags. /// @param selectors The selectors to install the validation function for. /// @param installData Optional data to be decoded and used by the module to setup initial module state. - /// @param preValidationHooks Optional pre-validation hooks to install for the validation function. - /// @param permissionHooks Optional permission hooks to install for the validation function. + /// @param hooks Optional hooks to install, associated with the validation function. These may be + /// pre-validation hooks or execution hooks. The expected format is a bytes26 HookConfig, followed by the + /// install data, if any. function installValidation( ValidationConfig validationConfig, - bytes4[] memory selectors, + bytes4[] calldata selectors, bytes calldata installData, - bytes calldata preValidationHooks, - bytes calldata permissionHooks + bytes[] calldata hooks ) external; /// @notice Uninstall a validation function from a set of execution selectors. @@ -44,14 +46,13 @@ interface IModuleManager { /// @param validationFunction The validation function to uninstall. /// @param uninstallData Optional data to be decoded and used by the module to clear module data for the /// account. - /// @param preValidationHookUninstallData Optional data to be decoded and used by the module to clear account - /// data - /// @param permissionHookUninstallData Optional data to be decoded and used by the module to clear account data + /// @param hookUninstallData Optional data to be used by hooks for cleanup. If any are provided, the array must + /// be of a length equal to existing pre-validation hooks plus permission hooks. Hooks are indexed by + /// pre-validation hook order first, then permission hooks. function uninstallValidation( ModuleEntity validationFunction, bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData, - bytes calldata permissionHookUninstallData + bytes[] calldata hookUninstallData ) external; /// @notice Uninstall a module from the modular account. diff --git a/test/account/AccountLoupe.t.sol b/test/account/AccountLoupe.t.sol index 8b03d86f..988e3dd5 100644 --- a/test/account/AccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.19; import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; import {ModuleEntity, ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; import {ExecutionHook} from "../../src/interfaces/IAccountLoupe.sol"; import {IModuleManager} from "../../src/interfaces/IModuleManager.sol"; @@ -135,28 +136,23 @@ contract AccountLoupeTest is CustomValidationTestBase { internal virtual override - returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) + returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) { - ModuleEntity[] memory preValidationHooks = new ModuleEntity[](2); - preValidationHooks[0] = ModuleEntityLib.pack( - address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.PRE_VALIDATION_HOOK_1) + bytes[] memory hooks = new bytes[](2); + hooks[0] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.PRE_VALIDATION_HOOK_1) + ) ); - preValidationHooks[1] = ModuleEntityLib.pack( - address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.PRE_VALIDATION_HOOK_2) + hooks[1] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.PRE_VALIDATION_HOOK_2) + ) ); bytes4[] memory selectors = new bytes4[](1); selectors[0] = comprehensiveModule.foo.selector; - bytes[] memory installDatas = new bytes[](2); - return ( - comprehensiveModuleValidation, - true, - true, - selectors, - bytes(""), - abi.encode(preValidationHooks, installDatas), - "" - ); + return (comprehensiveModuleValidation, true, true, selectors, bytes(""), hooks); } } diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index d6834af6..4f6fef9d 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -48,8 +48,7 @@ contract AccountReturnDataTest is AccountTestBase { ValidationConfigLib.pack(address(resultConsumerModule), DIRECT_CALL_VALIDATION_ENTITYID, false, false), selectors, "", - "", - "" + new bytes[](0) ); vm.stopPrank(); } diff --git a/test/account/DirectCallsFromModule.t.sol b/test/account/DirectCallsFromModule.t.sol index b8d39728..6d108540 100644 --- a/test/account/DirectCallsFromModule.t.sol +++ b/test/account/DirectCallsFromModule.t.sol @@ -1,9 +1,10 @@ pragma solidity ^0.8.19; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; + +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; import {ModuleEntity, ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; import {ValidationConfig, ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; -import {ExecutionHook} from "../../src/interfaces/IAccountLoupe.sol"; import {Call, IStandardExecutor} from "../../src/interfaces/IStandardExecutor.sol"; import {DirectCallModule} from "../mocks/modules/DirectCallModule.sol"; @@ -108,23 +109,22 @@ contract DirectCallsFromModuleTest is AccountTestBase { bytes4[] memory selectors = new bytes4[](1); selectors[0] = IStandardExecutor.execute.selector; - ExecutionHook[] memory permissionHooks = new ExecutionHook[](1); - bytes[] memory permissionHookInitDatas = new bytes[](1); - - permissionHooks[0] = ExecutionHook({hookFunction: _moduleEntity, isPreHook: true, isPostHook: true}); - - bytes memory encodedPermissionHooks = abi.encode(permissionHooks, permissionHookInitDatas); + bytes[] memory hooks = new bytes[](1); + hooks[0] = abi.encodePacked( + HookConfigLib.packExecHook({_hookFunction: _moduleEntity, _hasPre: true, _hasPost: true}), + hex"00" // onInstall data + ); vm.prank(address(entryPoint)); ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, false, false); - account1.installValidation(validationConfig, selectors, "", "", encodedPermissionHooks); + account1.installValidation(validationConfig, selectors, "", hooks); } function _uninstallModule() internal { vm.prank(address(entryPoint)); - account1.uninstallValidation(_moduleEntity, "", abi.encode(new bytes[](0)), abi.encode(new bytes[](1))); + account1.uninstallValidation(_moduleEntity, "", new bytes[](1)); } function _buildDirectCallDisallowedError(bytes4 selector) internal pure returns (bytes memory) { diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index 772011bf..3e927ee6 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -39,8 +39,7 @@ contract MultiValidationTest is AccountTestBase { ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true), new bytes4[](0), abi.encode(TEST_DEFAULT_VALIDATION_ENTITY_ID, owner2), - "", - "" + new bytes[](0) ); ModuleEntity[] memory validations = new ModuleEntity[](2); diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index 15cefbd9..cc3f3415 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -6,7 +6,9 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {ModuleEntity, ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; + +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; +import {ModuleEntity} from "../../src/helpers/ModuleEntityLib.sol"; import {Counter} from "../mocks/Counter.sol"; import {MockAccessControlHookModule} from "../mocks/modules/MockAccessControlHookModule.sol"; @@ -330,29 +332,23 @@ contract PerHookDataTest is CustomValidationTestBase { internal virtual override - returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) + returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) { - ModuleEntity accessControlHook = ModuleEntityLib.pack( - address(_accessControlHookModule), uint32(MockAccessControlHookModule.EntityId.PRE_VALIDATION_HOOK) + bytes[] memory hooks = new bytes[](1); + hooks[0] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(_accessControlHookModule), uint32(MockAccessControlHookModule.EntityId.PRE_VALIDATION_HOOK) + ), + abi.encode(_counter) ); - ModuleEntity[] memory preValidationHooks = new ModuleEntity[](1); - preValidationHooks[0] = accessControlHook; - - bytes[] memory preValidationHookData = new bytes[](1); - // Access control is restricted to only the counter - preValidationHookData[0] = abi.encode(_counter); - - bytes memory packedPreValidationHooks = abi.encode(preValidationHooks, preValidationHookData); - return ( _signerValidation, true, true, new bytes4[](0), abi.encode(TEST_DEFAULT_VALIDATION_ENTITY_ID, owner1), - packedPreValidationHooks, - "" + hooks ); } } diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index 4dd8a595..de2839c7 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -33,7 +33,10 @@ contract SelfCallAuthorizationTest is AccountTestBase { vm.startPrank(address(entryPoint)); account1.installModule(address(comprehensiveModule), comprehensiveModule.moduleManifest(), ""); account1.installValidation( - ValidationConfigLib.pack(comprehensiveModuleValidation, false, false), validationSelectors, "", "", "" + ValidationConfigLib.pack(comprehensiveModuleValidation, false, false), + validationSelectors, + "", + new bytes[](0) ); vm.stopPrank(); } @@ -302,7 +305,12 @@ contract SelfCallAuthorizationTest is AccountTestBase { account1.executeWithAuthorization( abi.encodeCall( UpgradeableModularAccount.installValidation, - (ValidationConfigLib.pack(comprehensiveModuleValidation, false, false), selectors, "", "", "") + ( + ValidationConfigLib.pack(comprehensiveModuleValidation, false, false), + selectors, + "", + new bytes[](0) + ) ), _encodeSignature(_signerValidation, GLOBAL_VALIDATION, "") ); diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index c9c1a882..73deb89f 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.19; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; + +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; import {ModuleEntity, ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; @@ -52,47 +54,36 @@ contract ValidationIntersectionTest is AccountTestBase { vm.startPrank(address(entryPoint)); // Install noHookValidation account1.installValidation( - ValidationConfigLib.pack(noHookValidation, true, true), - validationSelectors, - bytes(""), - bytes(""), - bytes("") + ValidationConfigLib.pack(noHookValidation, true, true), validationSelectors, bytes(""), new bytes[](0) ); // Install oneHookValidation validationSelectors[0] = MockUserOpValidation1HookModule.bar.selector; - ModuleEntity[] memory preValidationHooks = new ModuleEntity[](1); - preValidationHooks[0] = ModuleEntityLib.pack({ - addr: address(oneHookModule), - entityId: uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_1) - }); - bytes[] memory installDatas = new bytes[](1); + bytes[] memory hooks = new bytes[](1); + hooks[0] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(oneHookModule), uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_1) + ) + ); account1.installValidation( - ValidationConfigLib.pack(oneHookValidation, true, true), - validationSelectors, - bytes(""), - abi.encode(preValidationHooks, installDatas), - bytes("") + ValidationConfigLib.pack(oneHookValidation, true, true), validationSelectors, bytes(""), hooks ); // Install twoHookValidation validationSelectors[0] = MockUserOpValidation2HookModule.baz.selector; - preValidationHooks = new ModuleEntity[](2); - preValidationHooks[0] = ModuleEntityLib.pack({ - addr: address(twoHookModule), - entityId: uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_1) - }); - preValidationHooks[1] = ModuleEntityLib.pack({ - addr: address(twoHookModule), - entityId: uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_2) - }); - installDatas = new bytes[](2); + hooks = new bytes[](2); + hooks[0] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(twoHookModule), uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_1) + ) + ); + hooks[1] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(twoHookModule), uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_2) + ) + ); account1.installValidation( - ValidationConfigLib.pack(twoHookValidation, true, true), - validationSelectors, - bytes(""), - abi.encode(preValidationHooks, installDatas), - bytes("") + ValidationConfigLib.pack(twoHookValidation, true, true), validationSelectors, bytes(""), hooks ); vm.stopPrank(); } diff --git a/test/libraries/HookConfigLib.t.sol b/test/libraries/HookConfigLib.t.sol new file mode 100644 index 00000000..7a4671b8 --- /dev/null +++ b/test/libraries/HookConfigLib.t.sol @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.25; + +import {Test} from "forge-std/Test.sol"; + +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; +import {ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; +import {HookConfig, ModuleEntity} from "../../src/interfaces/IModuleManager.sol"; + +contract HookConfigLibTest is Test { + using ModuleEntityLib for ModuleEntity; + using HookConfigLib for HookConfig; + + // Tests the packing and unpacking of a hook config with a randomized state + + function testFuzz_hookConfig_packingUnderlying( + address addr, + uint32 entityId, + bool isValidation, + bool hasPre, + bool hasPost + ) public { + HookConfig hookConfig; + + if (isValidation) { + hookConfig = HookConfigLib.packValidationHook(addr, entityId); + } else { + hookConfig = HookConfigLib.packExecHook(addr, entityId, hasPre, hasPost); + } + + assertEq(hookConfig.module(), addr, "module mismatch"); + assertEq(hookConfig.entityId(), entityId, "entityId mismatch"); + assertEq(hookConfig.isValidationHook(), isValidation, "isValidation mismatch"); + + if (!isValidation) { + assertEq(hookConfig.hasPreHook(), hasPre, "hasPre mismatch"); + assertEq(hookConfig.hasPostHook(), hasPost, "hasPost mismatch"); + } + } + + function testFuzz_hookConfig_packingModuleEntity( + ModuleEntity hookFunction, + bool isValidation, + bool hasPre, + bool hasPost + ) public { + HookConfig hookConfig; + + if (isValidation) { + hookConfig = HookConfigLib.packValidationHook(hookFunction); + } else { + hookConfig = HookConfigLib.packExecHook(hookFunction, hasPre, hasPost); + } + + assertEq( + ModuleEntity.unwrap(hookConfig.moduleEntity()), + ModuleEntity.unwrap(hookFunction), + "moduleEntity mismatch" + ); + assertEq(hookConfig.isValidationHook(), isValidation, "isValidation mismatch"); + + if (!isValidation) { + assertEq(hookConfig.hasPreHook(), hasPre, "hasPre mismatch"); + assertEq(hookConfig.hasPostHook(), hasPost, "hasPost mismatch"); + } + } +} diff --git a/test/mocks/SingleSignerFactoryFixture.sol b/test/mocks/SingleSignerFactoryFixture.sol index 98dfc0b8..b4a3a4ff 100644 --- a/test/mocks/SingleSignerFactoryFixture.sol +++ b/test/mocks/SingleSignerFactoryFixture.sol @@ -56,8 +56,7 @@ contract SingleSignerFactoryFixture is OptimizedTest { ), new bytes4[](0), moduleInstallData, - "", - "" + new bytes[](0) ); } diff --git a/test/module/AllowlistModule.t.sol b/test/module/AllowlistModule.t.sol index 136ae211..b0e09c90 100644 --- a/test/module/AllowlistModule.t.sol +++ b/test/module/AllowlistModule.t.sol @@ -4,7 +4,9 @@ pragma solidity ^0.8.25; import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {ModuleEntity, ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; + +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; +import {ModuleEntity} from "../../src/helpers/ModuleEntityLib.sol"; import {Call} from "../../src/interfaces/IStandardExecutor.sol"; import {AllowlistModule} from "../../src/modules/permissionhooks/AllowlistModule.sol"; @@ -291,19 +293,15 @@ contract AllowlistModuleTest is CustomValidationTestBase { internal virtual override - returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes memory, bytes memory) + returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) { - ModuleEntity accessControlHook = - ModuleEntityLib.pack(address(allowlistModule), uint32(AllowlistModule.EntityId.PRE_VALIDATION_HOOK)); - - ModuleEntity[] memory preValidationHooks = new ModuleEntity[](1); - preValidationHooks[0] = accessControlHook; - - bytes[] memory preValidationHookData = new bytes[](1); - // Access control is restricted to only the counter - preValidationHookData[0] = abi.encode(allowlistInit); - - bytes memory packedPreValidationHooks = abi.encode(preValidationHooks, preValidationHookData); + bytes[] memory hooks = new bytes[](1); + hooks[0] = abi.encodePacked( + HookConfigLib.packValidationHook( + address(allowlistModule), uint32(AllowlistModule.EntityId.PRE_VALIDATION_HOOK) + ), + abi.encode(allowlistInit) + ); return ( _signerValidation, @@ -311,8 +309,7 @@ contract AllowlistModuleTest is CustomValidationTestBase { true, new bytes4[](0), abi.encode(TEST_DEFAULT_VALIDATION_ENTITY_ID, owner1), - packedPreValidationHooks, - "" + hooks ); } diff --git a/test/module/ERC20TokenLimitModule.t.sol b/test/module/ERC20TokenLimitModule.t.sol index e88a4527..c0aa8719 100644 --- a/test/module/ERC20TokenLimitModule.t.sol +++ b/test/module/ERC20TokenLimitModule.t.sol @@ -8,6 +8,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {ModuleEntity} from "../../src/helpers/ModuleEntityLib.sol"; +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; import {ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; @@ -52,16 +53,15 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { ERC20TokenLimitModule.ERC20SpendLimit[] memory limit = new ERC20TokenLimitModule.ERC20SpendLimit[](1); limit[0] = ERC20TokenLimitModule.ERC20SpendLimit({token: address(erc20), limits: limits}); - bytes[] memory permissionInitDatas = new bytes[](1); - permissionInitDatas[0] = abi.encode(uint8(0), limit); + bytes[] memory hooks = new bytes[](1); + hooks[0] = abi.encodePacked( + HookConfigLib.packExecHook({_module: address(module), _entityId: 0, _hasPre: true, _hasPost: false}), + abi.encode(uint32(0), limit) + ); vm.prank(address(acct)); acct.installValidation( - ValidationConfigLib.pack(address(validationModule), 0, true, true), - new bytes4[](0), - new bytes(0), - new bytes(0), - abi.encode(permissionHooks, permissionInitDatas) + ValidationConfigLib.pack(address(validationModule), 0, true, true), new bytes4[](0), "", hooks ); validationFunction = ModuleEntityLib.pack(address(validationModule), 0); diff --git a/test/module/NativeTokenLimitModule.t.sol b/test/module/NativeTokenLimitModule.t.sol index a5e20389..540dc902 100644 --- a/test/module/NativeTokenLimitModule.t.sol +++ b/test/module/NativeTokenLimitModule.t.sol @@ -8,8 +8,8 @@ import {ModuleEntity} from "../../src/helpers/ModuleEntityLib.sol"; import {ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; +import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol"; import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; -import {ExecutionHook} from "../../src/interfaces/IAccountLoupe.sol"; import {ModuleManifest} from "../../src/interfaces/IModule.sol"; import {Call, IStandardExecutor} from "../../src/interfaces/IStandardExecutor.sol"; import {NativeTokenLimitModule} from "../../src/modules/NativeTokenLimitModule.sol"; @@ -38,29 +38,24 @@ contract NativeTokenLimitModuleTest is AccountTestBase { ModuleEntity[] memory preValidationHooks = new ModuleEntity[](1); preValidationHooks[0] = ModuleEntityLib.pack(address(module), 0); - ExecutionHook[] memory permissionHooks = new ExecutionHook[](1); - permissionHooks[0] = ExecutionHook({ - hookFunction: ModuleEntityLib.pack(address(module), 0), - isPreHook: true, - isPostHook: false - }); - uint256[] memory spendLimits = new uint256[](1); spendLimits[0] = spendLimit; - bytes[] memory preValHooksInitDatas = new bytes[](1); - preValHooksInitDatas[0] = ""; + bytes[] memory hooks = new bytes[](2); + hooks[0] = abi.encodePacked(HookConfigLib.packValidationHook({_module: address(module), _entityId: 0})); + // No init data for pre validation - bytes[] memory permissionInitDatas = new bytes[](1); - permissionInitDatas[0] = abi.encode(0, spendLimits); + hooks[1] = abi.encodePacked( + HookConfigLib.packExecHook({_module: address(module), _entityId: 0, _hasPre: true, _hasPost: false}), + abi.encode(0, spendLimits) + ); vm.prank(address(acct)); acct.installValidation( ValidationConfigLib.pack(address(validationModule), 0, true, true), new bytes4[](0), new bytes(0), - abi.encode(preValidationHooks, preValHooksInitDatas), - abi.encode(permissionHooks, permissionInitDatas) + hooks ); validationFunction = ModuleEntityLib.pack(address(validationModule), 0); @@ -120,7 +115,7 @@ contract NativeTokenLimitModuleTest is AccountTestBase { UpgradeableModularAccount.PreExecHookReverted.selector, abi.encode( address(module), - uint8(0), + uint32(0), abi.encodePacked(NativeTokenLimitModule.ExceededNativeTokenLimit.selector) ) ) diff --git a/test/module/SingleSignerValidation.t.sol b/test/module/SingleSignerValidation.t.sol index ae3be4e5..9358b59f 100644 --- a/test/module/SingleSignerValidation.t.sol +++ b/test/module/SingleSignerValidation.t.sol @@ -83,8 +83,7 @@ contract SingleSignerValidationTest is AccountTestBase { ValidationConfigLib.pack(address(singleSignerValidation), newEntityId, true, false), new bytes4[](0), abi.encode(newEntityId, owner2), - "", - "" + new bytes[](0) ); vm.prank(owner2); diff --git a/test/utils/CustomValidationTestBase.sol b/test/utils/CustomValidationTestBase.sol index 18fd6ae4..3b313039 100644 --- a/test/utils/CustomValidationTestBase.sol +++ b/test/utils/CustomValidationTestBase.sol @@ -21,8 +21,7 @@ abstract contract CustomValidationTestBase is AccountTestBase { bool isSignatureValidation, bytes4[] memory selectors, bytes memory installData, - bytes memory preValidationHooks, - bytes memory permissionHooks + bytes[] memory hooks ) = _initialValidationConfig(); address accountImplementation = address(factory.accountImplementation()); @@ -33,8 +32,7 @@ abstract contract CustomValidationTestBase is AccountTestBase { ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation), selectors, installData, - preValidationHooks, - permissionHooks + hooks ); vm.deal(address(account1), 100 ether); @@ -49,7 +47,6 @@ abstract contract CustomValidationTestBase is AccountTestBase { bool isSignatureValidation, bytes4[] memory selectors, bytes memory installData, - bytes memory preValidationHooks, - bytes memory permissionHooks + bytes[] memory hooks ); }