From 9da8900535c81d965fab2a38afdd7f9885217bdd Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Fri, 30 Aug 2024 14:16:29 -0400 Subject: [PATCH] chore: rename permissions hook to execution hook --- src/account/AccountStorage.sol | 4 +- src/account/ModularAccountView.sol | 8 ++-- src/account/ModuleManagerInternals.sol | 26 ++++++------ src/account/ReferenceModularAccount.sol | 56 ++++++++++++------------- src/interfaces/IModularAccount.sol | 4 +- src/interfaces/IModularAccountView.sol | 4 +- test/account/ModularAccountView.t.sol | 2 +- test/account/ReplaceModule.t.sol | 2 +- test/mocks/modules/DirectCallModule.sol | 4 +- 9 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 79393208..940c1dfb 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -33,8 +33,8 @@ struct ValidationData { bool isUserOpValidation; // The pre validation hooks for this validation function. ModuleEntity[] preValidationHooks; - // Permission hooks for this validation function. - EnumerableSet.Bytes32Set permissionHooks; + // Execution hooks to run with this validation function. + EnumerableSet.Bytes32Set executionHooks; // The set of selectors that may be validated by this validation function. EnumerableSet.Bytes32Set selectors; } diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol index 36150fde..bb93aaad 100644 --- a/src/account/ModularAccountView.sol +++ b/src/account/ModularAccountView.sol @@ -52,10 +52,10 @@ abstract contract ModularAccountView is IModularAccountView { data.isUserOpValidation = validationData.isUserOpValidation; data.preValidationHooks = validationData.preValidationHooks; - uint256 permissionHooksLen = validationData.permissionHooks.length(); - data.permissionHooks = new HookConfig[](permissionHooksLen); - for (uint256 i = 0; i < permissionHooksLen; ++i) { - data.permissionHooks[i] = toHookConfig(validationData.permissionHooks.at(i)); + uint256 execHooksLen = validationData.executionHooks.length(); + data.executionHooks = new HookConfig[](execHooksLen); + for (uint256 i = 0; i < execHooksLen; ++i) { + data.executionHooks[i] = toHookConfig(validationData.executionHooks.at(i)); } bytes32[] memory selectors = validationData.selectors.values(); diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index 9f6ee1a2..93456e5b 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -39,7 +39,7 @@ abstract contract ModuleManagerInternals is IModularAccount { error InterfaceNotSupported(address module); error NativeFunctionNotAllowed(bytes4 selector); error NullModule(); - error PermissionAlreadySet(ModuleEntity validationFunction, HookConfig hookConfig); + error ExecutionHookAlreadySet(ModuleEntity validationFunction, HookConfig hookConfig); error ModuleInstallCallbackFailed(address module, bytes revertReason); error ModuleNotInstalled(address module); error PreValidationHookLimitExceeded(); @@ -244,9 +244,9 @@ abstract contract ModuleManagerInternals is IModularAccount { continue; } - // Hook is a permission hook - if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) { - revert PermissionAlreadySet(moduleEntity, hookConfig); + // Hook is an execution hook + if (!_validationData.executionHooks.add(toSetValue(hookConfig))) { + revert ExecutionHookAlreadySet(moduleEntity, hookConfig); } _onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId); @@ -282,12 +282,12 @@ abstract contract ModuleManagerInternals is IModularAccount { // If any uninstall data is provided, assert it is of the correct length. if ( hookUninstallDatas.length - != _validationData.preValidationHooks.length + _validationData.permissionHooks.length() + != _validationData.preValidationHooks.length + _validationData.executionHooks.length() ) { revert ArrayLengthMismatch(); } - // Hook uninstall data is provided in the order of pre validation hooks, then permission hooks. + // Hook uninstall data is provided in the order of pre validation hooks, then execution hooks. uint256 hookIndex = 0; for (uint256 i = 0; i < _validationData.preValidationHooks.length; ++i) { bytes calldata hookData = hookUninstallDatas[hookIndex]; @@ -296,10 +296,10 @@ abstract contract ModuleManagerInternals is IModularAccount { hookIndex++; } - for (uint256 i = 0; i < _validationData.permissionHooks.length(); ++i) { + for (uint256 i = 0; i < _validationData.executionHooks.length(); ++i) { bytes calldata hookData = hookUninstallDatas[hookIndex]; (address hookModule,) = - ModuleEntityLib.unpack(toModuleEntity(_validationData.permissionHooks.at(i))); + ModuleEntityLib.unpack(toModuleEntity(_validationData.executionHooks.at(i))); onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData); hookIndex++; } @@ -308,11 +308,11 @@ abstract contract ModuleManagerInternals is IModularAccount { // 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); + EnumerableSet.Bytes32Set storage executionHooks = _validationData.executionHooks; + uint256 executionHookLen = executionHooks.length(); + for (uint256 i = 0; i < executionHookLen; ++i) { + bytes32 executionHook = executionHooks.at(0); + executionHooks.remove(executionHook); } // Clear selectors diff --git a/src/account/ReferenceModularAccount.sol b/src/account/ReferenceModularAccount.sol index d142e8ef..6f485ef7 100644 --- a/src/account/ReferenceModularAccount.sol +++ b/src/account/ReferenceModularAccount.sol @@ -84,13 +84,13 @@ contract ReferenceModularAccount is // Wraps execution of a native function with runtime validation and hooks // Used for upgradeTo, upgradeToAndCall, execute, executeBatch, installExecution, uninstallExecution modifier wrapNativeFunction() { - (PostExecToRun[] memory postPermissionHooks, PostExecToRun[] memory postExecHooks) = + (PostExecToRun[] memory postValidatorExecHooks, PostExecToRun[] memory postSelectorExecHooks) = _checkPermittedCallerAndAssociatedHooks(); _; - _doCachedPostExecHooks(postExecHooks); - _doCachedPostExecHooks(postPermissionHooks); + _doCachedPostExecHooks(postSelectorExecHooks); + _doCachedPostExecHooks(postValidatorExecHooks); } constructor(IEntryPoint anEntryPoint) { @@ -110,7 +110,7 @@ contract ReferenceModularAccount is if (execModule == address(0)) { revert UnrecognizedFunction(msg.sig); } - (PostExecToRun[] memory postPermissionHooks, PostExecToRun[] memory postExecHooks) = + (PostExecToRun[] memory postValidatorExecHooks, PostExecToRun[] memory postSelectorExecHooks) = _checkPermittedCallerAndAssociatedHooks(); // execute the function, bubbling up any reverts @@ -123,8 +123,8 @@ contract ReferenceModularAccount is } } - _doCachedPostExecHooks(postExecHooks); - _doCachedPostExecHooks(postPermissionHooks); + _doCachedPostExecHooks(postSelectorExecHooks); + _doCachedPostExecHooks(postValidatorExecHooks); return execReturnData; } @@ -139,8 +139,8 @@ contract ReferenceModularAccount is ModuleEntity userOpValidationFunction = ModuleEntity.wrap(bytes24(userOp.signature[:24])); - PostExecToRun[] memory postPermissionHooks = - _doPreHooks(getAccountStorage().validationData[userOpValidationFunction].permissionHooks, msg.data); + PostExecToRun[] memory postValidatorExecHooks = + _doPreHooks(getAccountStorage().validationData[userOpValidationFunction].executionHooks, msg.data); (bool success, bytes memory result) = address(this).call(userOp.callData[4:]); @@ -151,7 +151,7 @@ contract ReferenceModularAccount is } } - _doCachedPostExecHooks(postPermissionHooks); + _doCachedPostExecHooks(postValidatorExecHooks); } /// @inheritdoc IModularAccount @@ -202,9 +202,9 @@ contract ReferenceModularAccount is _doRuntimeValidation(runtimeValidationFunction, data, authorization[25:]); - // If runtime validation passes, do runtime permission checks - PostExecToRun[] memory postPermissionHooks = - _doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].permissionHooks, data); + // If runtime validation passes, run exec hooks associated with the validator + PostExecToRun[] memory postValidatorExecHooks = + _doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].executionHooks, data); // Execute the call (bool success, bytes memory returnData) = address(this).call(data); @@ -215,7 +215,7 @@ contract ReferenceModularAccount is } } - _doCachedPostExecHooks(postPermissionHooks); + _doCachedPostExecHooks(postValidatorExecHooks); return returnData; } @@ -254,7 +254,7 @@ contract ReferenceModularAccount is /// @inheritdoc IModularAccount /// @notice May be validated by a global validation. /// @dev This function can be used to update (to a certain degree) previously installed validation functions. - /// - preValidationHook, permissionHook, and selectors can be added later. Though they won't be deleted. + /// - preValidationHook, executionHooks, and selectors can be added later. Though they won't be deleted. /// - isGlobal and isSignatureValidation can also be updated later. function installValidation( ValidationConfig validationConfig, @@ -360,12 +360,12 @@ contract ReferenceModularAccount is isGlobalValidation ? ValidationCheckingType.GLOBAL : ValidationCheckingType.SELECTOR ); - // Check if there are permission hooks associated with the validator, and revert if the call isn't to + // Check if there are execution hooks associated with the validator, and revert if the call isn't to // `executeUserOp` // This check must be here because if context isn't passed, we can't tell in execution which hooks should // have ran if ( - getAccountStorage().validationData[userOpValidationFunction].permissionHooks.length() > 0 + getAccountStorage().validationData[userOpValidationFunction].executionHooks.length() > 0 && bytes4(userOp.callData[:4]) != this.executeUserOp.selector ) { revert RequireUserOperationContext(); @@ -538,24 +538,24 @@ contract ReferenceModularAccount is /** * Order of operations: * 1. Check if the sender is the entry point, the account itself, or the selector called is public. - * - Yes: Return an empty array, there are no post permissionHooks. + * - Yes: Return an empty array, there are no post executionHooks. * - No: Continue * 2. Check if the called selector (msg.sig) is included in the set of selectors the msg.sender can * directly call. * - Yes: Continue * - No: Revert, the caller is not allowed to call this selector * 3. If there are runtime validation hooks associated with this caller-sig combination, run them. - * 4. Run the pre permissionHooks associated with this caller-sig combination, and return the - * post permissionHooks to run later. + * 4. Run the pre executionHooks associated with this caller-sig combination, and return the + * post executionHooks to run later. */ function _checkPermittedCallerAndAssociatedHooks() internal returns (PostExecToRun[] memory, PostExecToRun[] memory) { AccountStorage storage _storage = getAccountStorage(); - PostExecToRun[] memory postPermissionHooks; + PostExecToRun[] memory postValidatorExecutionHooks; - // We only need to handle permission hooks when the sender is not the entry point or the account itself, + // We only need to handle execution hooks when the sender is not the entry point or the account itself, // and the selector isn't public. if ( msg.sender != address(_ENTRY_POINT) && msg.sender != address(this) @@ -566,7 +566,7 @@ contract ReferenceModularAccount is _checkIfValidationAppliesCallData(msg.data, directCallValidationKey, ValidationCheckingType.EITHER); - // Direct call is allowed, run associated permission & validation hooks + // Direct call is allowed, run associated execution & validation hooks // Validation hooks ModuleEntity[] memory preRuntimeValidationHooks = @@ -577,16 +577,16 @@ contract ReferenceModularAccount is _doPreRuntimeValidationHook(preRuntimeValidationHooks[i], msg.data, ""); } - // Permission hooks - postPermissionHooks = - _doPreHooks(_storage.validationData[directCallValidationKey].permissionHooks, msg.data); + // Execution hooks associated with the validator + postValidatorExecutionHooks = + _doPreHooks(_storage.validationData[directCallValidationKey].executionHooks, msg.data); } - // Exec hooks - PostExecToRun[] memory postExecutionHooks = + // Exec hooks associated with the selector + PostExecToRun[] memory postSelectorExecutionHooks = _doPreHooks(_storage.executionData[msg.sig].executionHooks, msg.data); - return (postPermissionHooks, postExecutionHooks); + return (postValidatorExecutionHooks, postSelectorExecutionHooks); } function _execUserOpValidation( diff --git a/src/interfaces/IModularAccount.sol b/src/interfaces/IModularAccount.sol index 25f43465..c133dce7 100644 --- a/src/interfaces/IModularAccount.sol +++ b/src/interfaces/IModularAccount.sol @@ -107,8 +107,8 @@ interface IModularAccount { /// @param uninstallData Optional data to be decoded and used by the module to clear module data for the /// account. /// @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. + /// be of a length equal to existing pre validation hooks plus execution hooks. Hooks are indexed by + /// pre validation hook order first, then execution hooks. function uninstallValidation( ModuleEntity validationFunction, bytes calldata uninstallData, diff --git a/src/interfaces/IModularAccountView.sol b/src/interfaces/IModularAccountView.sol index 8b27cd97..8fd0aabe 100644 --- a/src/interfaces/IModularAccountView.sol +++ b/src/interfaces/IModularAccountView.sol @@ -28,8 +28,8 @@ struct ValidationDataView { bool isUserOpValidation; // The pre validation hooks for this validation function. ModuleEntity[] preValidationHooks; - // Permission hooks for this validation function. - HookConfig[] permissionHooks; + // Execution hooks to run with this validation function. + HookConfig[] executionHooks; // The set of selectors that may be validated by this validation function. bytes4[] selectors; } diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol index 9806116b..23f5e90d 100644 --- a/test/account/ModularAccountView.t.sol +++ b/test/account/ModularAccountView.t.sol @@ -121,7 +121,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { ) ); - assertEq(data.permissionHooks.length, 0); + assertEq(data.executionHooks.length, 0); assertEq(selectors.length, 1); assertEq(selectors[0], comprehensiveModule.foo.selector); } diff --git a/test/account/ReplaceModule.t.sol b/test/account/ReplaceModule.t.sol index 050a9e09..cc5c6dd3 100644 --- a/test/account/ReplaceModule.t.sol +++ b/test/account/ReplaceModule.t.sol @@ -98,7 +98,7 @@ contract UpgradeModuleTest is AccountTestBase { } function test_upgradeModuleValidationFunction() public { - // Setup new validaiton with pre validation and permission hooks + // Setup new validaiton with pre validation and execution hooks associated with a validator SingleSignerValidationModule validation1 = new SingleSignerValidationModule(); SingleSignerValidationModule validation2 = new SingleSignerValidationModule(); uint32 validationEntityId1 = 10; diff --git a/test/mocks/modules/DirectCallModule.sol b/test/mocks/modules/DirectCallModule.sol index c81c9a18..a8342367 100644 --- a/test/mocks/modules/DirectCallModule.sol +++ b/test/mocks/modules/DirectCallModule.sol @@ -32,7 +32,7 @@ contract DirectCallModule is BaseModule, IExecutionHookModule { override returns (bytes memory) { - require(sender == address(this), "mock direct call pre permission hook failed"); + require(sender == address(this), "mock direct call pre execution hook failed"); preHookRan = true; return abi.encode(keccak256(hex"04546b")); } @@ -40,7 +40,7 @@ contract DirectCallModule is BaseModule, IExecutionHookModule { function postExecutionHook(uint32, bytes calldata preExecHookData) external override { require( abi.decode(preExecHookData, (bytes32)) == keccak256(hex"04546b"), - "mock direct call post permission hook failed" + "mock direct call post execution hook failed" ); postHookRan = true; }