feat: add interface checks for validations and hooks

src/account/ModuleManagerInternals.sol

@@ -10,9 +10,13 @@ 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 {IExecutionHookModule} from "../interfaces/IExecutionHookModule.sol";
 import {ExecutionManifest, ManifestExecutionHook} from "../interfaces/IExecutionModule.sol";
 import {HookConfig, IModularAccount, ModuleEntity, ValidationConfig} from "../interfaces/IModularAccount.sol";
 import {IModule} from "../interfaces/IModule.sol";
+import {IValidationHookModule} from "../interfaces/IValidationHookModule.sol";
+import {IValidationModule} from "../interfaces/IValidationModule.sol";
+
 import {
     AccountStorage,
     ExecutionData,
@@ -32,11 +36,11 @@ abstract contract ModuleManagerInternals is IModularAccount {
     error Erc4337FunctionNotAllowed(bytes4 selector);
     error ExecutionFunctionAlreadySet(bytes4 selector);
     error IModuleFunctionNotAllowed(bytes4 selector);
+    error InterfaceNotSupported(address module);
     error NativeFunctionNotAllowed(bytes4 selector);
     error NullModule();
     error PermissionAlreadySet(ModuleEntity validationFunction, HookConfig hookConfig);
     error ModuleInstallCallbackFailed(address module, bytes revertReason);
-    error ModuleInterfaceNotSupported(address module);
     error ModuleNotInstalled(address module);
     error PreValidationHookLimitExceeded();
     error ValidationAlreadySet(bytes4 selector, ModuleEntity validationFunction);
@@ -134,10 +138,8 @@ abstract contract ModuleManagerInternals is IModularAccount {
             revert NullModule();
         }
 
-        // TODO: do we need this check? Or switch to a non-165 checking function?
-        // Check that the module supports the IModule interface.
         if (!ERC165Checker.supportsInterface(module, type(IModule).interfaceId)) {
-            revert ModuleInterfaceNotSupported(module);
+            revert InterfaceNotSupported(module);
         }
 
         // Update components according to the manifest.
@@ -228,6 +230,16 @@ abstract contract ModuleManagerInternals is IModularAccount {
         }
     }
 
+    function _onInstall(address module, bytes calldata data, bytes4 interfaceId) internal {
+        if (data.length > 0) {
+            if (!ERC165Checker.supportsInterface(module, interfaceId)) {
+                revert InterfaceNotSupported(module);
+            }
+
+            IModule(module).onInstall(data);
+        }
+    }
+
     function _onUninstall(address module, bytes calldata data) internal returns (bool onUninstallSuccess) {
         onUninstallSuccess = true;
         if (data.length > 0) {
@@ -261,12 +273,17 @@ abstract contract ModuleManagerInternals is IModularAccount {
                 if (_validationData.preValidationHooks.length > MAX_PRE_VALIDATION_HOOKS) {
                     revert PreValidationHookLimitExceeded();
                 }
-            } // Hook is an execution hook
-            else if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) {
+
+                _onInstall(hookConfig.module(), hookData, type(IValidationHookModule).interfaceId);
+
+                continue;
+            }
+            // Hook is a permission hook
+            if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) {
                 revert PermissionAlreadySet(moduleEntity, hookConfig);
             }
 
-            _onInstall(hookConfig.module(), hookData);
+            _onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId);
         }
 
         for (uint256 i = 0; i < selectors.length; ++i) {
@@ -279,7 +296,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
         _validationData.isGlobal = validationConfig.isGlobal();
         _validationData.isSignatureValidation = validationConfig.isSignatureValidation();
 
-        _onInstall(validationConfig.module(), installData);
+        _onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId);
         emit ValidationInstalled(validationConfig.module(), validationConfig.entityId());
     }
 

src/modules/ERC20TokenLimitModule.sol

@@ -120,7 +120,7 @@ contract ERC20TokenLimitModule is BaseModule, IExecutionHookModule {
 
     /// @inheritdoc BaseModule
     function supportsInterface(bytes4 interfaceId) public view override(BaseModule, IERC165) returns (bool) {
-        return super.supportsInterface(interfaceId);
+        return interfaceId == type(IExecutionHookModule).interfaceId || super.supportsInterface(interfaceId);
     }
 
     function _decrementLimit(uint32 entityId, address token, bytes memory innerCalldata) internal {

src/modules/permissionhooks/AllowlistModule.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.25;
 
 import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
+import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";
 
 import {IModule} from "../../interfaces/IModule.sol";
 
@@ -118,6 +119,16 @@ contract AllowlistModule is IValidationHookModule, BaseModule {
         }
     }
 
+    function supportsInterface(bytes4 interfaceId)
+        public
+        view
+        virtual
+        override(BaseModule, IERC165)
+        returns (bool)
+    {
+        return interfaceId == type(IValidationHookModule).interfaceId || super.supportsInterface(interfaceId);
+    }
+
     function _checkCallPermission(uint32 entityId, address account, address target, bytes memory data)
         internal
         view

src/modules/validation/SingleSignerValidationModule.sol

@@ -1,15 +1,17 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.25;
 
+import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
+import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";
+import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
+import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
+
 import {IModule} from "../../interfaces/IModule.sol";
 import {IValidationModule} from "../../interfaces/IValidationModule.sol";
 import {BaseModule} from "../BaseModule.sol";
 
 import {ReplaySafeWrapper} from "../ReplaySafeWrapper.sol";
 import {ISingleSignerValidationModule} from "./ISingleSignerValidationModule.sol";
-import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
-import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
-import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
 
 /// @title ECSDA Validation
 /// @author ERC-6900 Authors
@@ -115,6 +117,16 @@ contract SingleSignerValidationModule is ISingleSignerValidationModule, ReplaySa
         return "erc6900/single-signer-validation-module/1.0.0";
     }
 
+    function supportsInterface(bytes4 interfaceId)
+        public
+        view
+        virtual
+        override(BaseModule, IERC165)
+        returns (bool)
+    {
+        return (interfaceId == type(IValidationModule).interfaceId || super.supportsInterface(interfaceId));
+    }
+
     // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
     // ┃    Internal / Private functions    ┃
     // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

test/account/UpgradeableModularAccount.t.sol

@@ -288,7 +288,7 @@ contract UpgradeableModularAccountTest is AccountTestBase {
 
         address badModule = address(1);
         vm.expectRevert(
-            abi.encodeWithSelector(ModuleManagerInternals.ModuleInterfaceNotSupported.selector, address(badModule))
+            abi.encodeWithSelector(ModuleManagerInternals.InterfaceNotSupported.selector, address(badModule))
         );
 
         ExecutionManifest memory m;

test/mocks/modules/DirectCallModule.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.19;
 
+import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";
+
 import {IExecutionHookModule} from "../../../src/interfaces/IExecutionHookModule.sol";
 import {IModularAccount} from "../../../src/interfaces/IModularAccount.sol";
 import {BaseModule} from "../../../src/modules/BaseModule.sol";
@@ -42,4 +44,14 @@ contract DirectCallModule is BaseModule, IExecutionHookModule {
         );
         postHookRan = true;
     }
+
+    function supportsInterface(bytes4 interfaceId)
+        public
+        view
+        virtual
+        override(BaseModule, IERC165)
+        returns (bool)
+    {
+        return interfaceId == type(IExecutionHookModule).interfaceId || super.supportsInterface(interfaceId);
+    }
 }

test/mocks/modules/MockAccessControlHookModule.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.25;
 
 import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
+import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";
 
 import {IModularAccount} from "../../../src/interfaces/IModularAccount.sol";
 import {IValidationHookModule} from "../../../src/interfaces/IValidationHookModule.sol";
@@ -86,4 +87,14 @@ contract MockAccessControlHookModule is IValidationHookModule, BaseModule {
     function moduleId() external pure returns (string memory) {
         return "erc6900/mock-access-control-hook-module/1.0.0";
     }
+
+    function supportsInterface(bytes4 interfaceId)
+        public
+        view
+        virtual
+        override(BaseModule, IERC165)
+        returns (bool)
+    {
+        return interfaceId == type(IValidationHookModule).interfaceId || super.supportsInterface(interfaceId);
+    }
 }