chore: rename permissions hook to execution hook (#174)

Author: howydev

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;
 }

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();

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(HookConfig hookConfig);
     error ModuleInstallCallbackFailed(address module, bytes revertReason);
     error ModuleNotInstalled(address module);
     error PreValidationHookLimitExceeded();
@@ -106,7 +106,9 @@ abstract contract ModuleManagerInternals is IModularAccount {
     }
 
     function _addExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
-        hooks.add(toSetValue(hookConfig));
+        if (!hooks.add(toSetValue(hookConfig))) {
+            revert ExecutionHookAlreadySet(hookConfig);
+        }
     }
 
     function _removeExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
@@ -247,10 +249,8 @@ 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
+            _addExecHooks(_validationData.executionHooks, hookConfig);
 
             _onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId);
         }
@@ -285,12 +285,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];
@@ -299,10 +299,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++;
             }
@@ -311,11 +311,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

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(

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,

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;
 }

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);
     }

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;

test/mocks/modules/DirectCallModule.sol

@@ -32,15 +32,15 @@ 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"));
     }
 
     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;
     }