Skip to content

Commit

Permalink
feat: remove onchain 165 interface check during install
Browse files Browse the repository at this point in the history
  • Loading branch information
adamegyed committed Feb 12, 2025
1 parent 2b2c061 commit cafc5a7
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 25 deletions.
17 changes: 5 additions & 12 deletions src/account/ModuleManagerInternals.sol
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.20;

import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import {collectReturnData} from "../helpers/CollectReturnData.sol";
import {MAX_VALIDATION_ASSOC_HOOKS} from "../helpers/Constants.sol";
import {IExecutionHookModule} from "../interfaces/IExecutionHookModule.sol";
import {ExecutionManifest, ManifestExecutionHook} from "../interfaces/IExecutionModule.sol";
import {
HookConfig,
Expand All @@ -16,8 +14,6 @@ import {
ValidationFlags
} from "../interfaces/IModularAccount.sol";
import {IModule} from "../interfaces/IModule.sol";
import {IValidationHookModule} from "../interfaces/IValidationHookModule.sol";
import {IValidationModule} from "../interfaces/IValidationModule.sol";
import {HookConfigLib} from "../libraries/HookConfigLib.sol";
import {KnownSelectorsLib} from "../libraries/KnownSelectorsLib.sol";
import {ModuleEntityLib} from "../libraries/ModuleEntityLib.sol";
Expand Down Expand Up @@ -152,7 +148,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
_storage.supportedIfaces[manifest.interfaceIds[i]] += 1;
}

_onInstall(module, moduleInstallData, type(IModule).interfaceId);
_onInstall(module, moduleInstallData);

emit ExecutionInstalled(module, manifest);
}
Expand Down Expand Up @@ -195,11 +191,8 @@ abstract contract ModuleManagerInternals is IModularAccount {
emit ExecutionUninstalled(module, onUninstallSuccess, manifest);
}

function _onInstall(address module, bytes calldata data, bytes4 interfaceId) internal {
function _onInstall(address module, bytes calldata data) internal {
if (data.length > 0) {
if (!ERC165Checker.supportsInterface(module, interfaceId)) {
revert InterfaceNotSupported(module);
}
// solhint-disable-next-line no-empty-blocks
try IModule(module).onInstall(data) {}
catch {
Expand Down Expand Up @@ -243,14 +236,14 @@ abstract contract ModuleManagerInternals is IModularAccount {
revert PreValidationHookLimitExceeded();
}

_onInstall(hookConfig.module(), hookData, type(IValidationHookModule).interfaceId);
_onInstall(hookConfig.module(), hookData);

continue;
}
// Hook is an execution hook
_addExecHooks(_validationStorage.executionHooks, hookConfig);

_onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId);
_onInstall(hookConfig.module(), hookData);
}

for (uint256 i = 0; i < selectors.length; ++i) {
Expand All @@ -262,7 +255,7 @@ abstract contract ModuleManagerInternals is IModularAccount {

_validationStorage.validationFlags = validationFlags;

_onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId);
_onInstall(validationConfig.module(), installData);
emit ValidationInstalled(validationConfig.module(), validationConfig.entityId());
}

Expand Down
13 changes: 0 additions & 13 deletions test/account/ReferenceModularAccount.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -280,19 +280,6 @@ contract ReferenceModularAccountTest is AccountTestBase {
});
}

function test_installExecution_interfaceNotSupported() public {
vm.startPrank(address(entryPoint));

address badModule = address(1);
vm.expectRevert(
abi.encodeWithSelector(ModuleManagerInternals.InterfaceNotSupported.selector, address(badModule))
);

ExecutionManifest memory m;

account1.installExecution({module: address(badModule), manifest: m, moduleInstallData: "a"});
}

function test_installExecution_alreadyInstalled() public {
ExecutionManifest memory m = tokenReceiverModule.executionManifest();

Expand Down

0 comments on commit cafc5a7

Please sign in to comment.