From 79178c1d5ce47489bf27e0d09b7b2c49dda810b4 Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Thu, 22 Aug 2024 15:16:03 -0400 Subject: [PATCH] feat: add moduleId --- src/helpers/KnownSelectors.sol | 2 +- src/interfaces/IModule.sol | 29 +++---------------- src/modules/ERC20TokenLimitModule.sol | 13 ++------- src/modules/NativeTokenLimitModule.sol | 18 ++---------- src/modules/TokenReceiverModule.sol | 14 ++------- .../permissionhooks/AllowlistModule.sol | 12 +++----- .../SingleSignerValidationModule.sol | 14 ++------- test/libraries/KnowSelectors.t.sol | 2 +- test/mocks/MockModule.sol | 14 ++------- test/mocks/modules/ComprehensiveModule.sol | 9 ++---- test/mocks/modules/DirectCallModule.sol | 5 ++-- .../modules/MockAccessControlHookModule.sol | 5 ++-- test/mocks/modules/PermittedCallMocks.sol | 5 ++-- test/mocks/modules/ReturnDataModuleMocks.sol | 9 ++++-- test/mocks/modules/ValidationModuleMocks.sol | 7 +++-- 15 files changed, 46 insertions(+), 112 deletions(-) diff --git a/src/helpers/KnownSelectors.sol b/src/helpers/KnownSelectors.sol index ba772d66..2da40efe 100644 --- a/src/helpers/KnownSelectors.sol +++ b/src/helpers/KnownSelectors.sol @@ -49,7 +49,7 @@ library KnownSelectors { function isIModuleFunction(bytes4 selector) internal pure returns (bool) { return selector == IModule.onInstall.selector || selector == IModule.onUninstall.selector - || selector == IExecutionModule.executionManifest.selector || selector == IModule.moduleMetadata.selector + || selector == IModule.moduleId.selector || selector == IExecutionModule.executionManifest.selector || selector == IExecutionHookModule.preExecutionHook.selector || selector == IExecutionHookModule.postExecutionHook.selector || selector == IValidationModule.validateUserOp.selector diff --git a/src/interfaces/IModule.sol b/src/interfaces/IModule.sol index 10358619..df070f6f 100644 --- a/src/interfaces/IModule.sol +++ b/src/interfaces/IModule.sol @@ -3,27 +3,6 @@ pragma solidity ^0.8.25; import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; -struct SelectorPermission { - bytes4 functionSelector; - string permissionDescription; -} - -/// @dev A struct holding fields to describe the module in a purely view context. Intended for front end clients. -struct ModuleMetadata { - // A human-readable name of the module. - string name; - // The version of the module, following the semantic versioning scheme. - string version; - // The author field SHOULD be a username representing the identity of the user or organization - // that created this module. - string author; - // String desciptions of the relative sensitivity of specific functions. The selectors MUST be selectors for - // functions implemented by this module. - SelectorPermission[] permissionDescriptors; - // A list of all ERC-7715 permission strings that the module could possibly use - string[] permissionRequest; -} - interface IModule is IERC165 { /// @notice Initialize module data for the modular account. /// @dev Called by the modular account during `installExecution`. @@ -37,8 +16,8 @@ interface IModule is IERC165 { /// account. function onUninstall(bytes calldata data) external; - /// @notice Describe the metadata of the module. - /// @dev This metadata MUST stay constant over time. - /// @return A metadata struct describing the module. - function moduleMetadata() external pure returns (ModuleMetadata memory); + /// @notice Return a unique identifier for the module. + /// @dev This function MUST return a string in the format "vendor/module/semver". + /// @return The module ID. + function moduleId() external view returns (string memory); } diff --git a/src/modules/ERC20TokenLimitModule.sol b/src/modules/ERC20TokenLimitModule.sol index 790a67a2..406937b1 100644 --- a/src/modules/ERC20TokenLimitModule.sol +++ b/src/modules/ERC20TokenLimitModule.sol @@ -14,7 +14,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet import {IExecutionHookModule} from "../interfaces/IExecutionHookModule.sol"; import {Call, IModularAccount} from "../interfaces/IModularAccount.sol"; -import {IModule, ModuleMetadata} from "../interfaces/IModule.sol"; +import {IModule} from "../interfaces/IModule.sol"; import {BaseModule, IERC165} from "./BaseModule.sol"; @@ -114,15 +114,8 @@ contract ERC20TokenLimitModule is BaseModule, IExecutionHookModule { } /// @inheritdoc IModule - function moduleMetadata() external pure virtual override returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = _NAME; - metadata.version = _VERSION; - metadata.author = _AUTHOR; - - metadata.permissionRequest = new string[](1); - metadata.permissionRequest[0] = "erc20-token-limit"; - return metadata; + function moduleId() external pure returns (string memory) { + return "erc6900/erc20-token-limit-module/1.0.0"; } /// @inheritdoc BaseModule diff --git a/src/modules/NativeTokenLimitModule.sol b/src/modules/NativeTokenLimitModule.sol index 568fac62..4d0a076b 100644 --- a/src/modules/NativeTokenLimitModule.sol +++ b/src/modules/NativeTokenLimitModule.sol @@ -7,7 +7,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet import {IExecutionHookModule} from "../interfaces/IExecutionHookModule.sol"; import {Call, IModularAccount} from "../interfaces/IModularAccount.sol"; -import {IModule, ModuleMetadata} from "../interfaces/IModule.sol"; +import {IModule} from "../interfaces/IModule.sol"; import {IValidationHookModule} from "../interfaces/IValidationHookModule.sol"; import {BaseModule, IERC165} from "./BaseModule.sol"; @@ -21,10 +21,6 @@ contract NativeTokenLimitModule is BaseModule, IExecutionHookModule, IValidation using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.Bytes32Set; - string internal constant _NAME = "Native Token Limit"; - string internal constant _VERSION = "1.0.0"; - string internal constant _AUTHOR = "ERC-6900 Authors"; - mapping(uint256 funcIds => mapping(address account => uint256 limit)) public limits; // Accounts should add paymasters that still use the accounts tokens here // E.g. ERC20 paymasters that pull funds from the account @@ -119,16 +115,8 @@ contract NativeTokenLimitModule is BaseModule, IExecutionHookModule, IValidation function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external pure override {} /// @inheritdoc IModule - function moduleMetadata() external pure virtual override returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = _NAME; - metadata.version = _VERSION; - metadata.author = _AUTHOR; - - metadata.permissionRequest = new string[](2); - metadata.permissionRequest[0] = "native-token-limit"; - metadata.permissionRequest[1] = "gas-limit"; - return metadata; + function moduleId() external pure returns (string memory) { + return "erc6900/native-token-limit-module/1.0.0"; } // ┏━━━━━━━━━━━━━━━┓ diff --git a/src/modules/TokenReceiverModule.sol b/src/modules/TokenReceiverModule.sol index 11692254..51f06fcb 100644 --- a/src/modules/TokenReceiverModule.sol +++ b/src/modules/TokenReceiverModule.sol @@ -6,7 +6,7 @@ import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Recei import {ExecutionManifest, ManifestExecutionFunction} from "../interfaces/IExecutionModule.sol"; import {ExecutionManifest, IExecutionModule} from "../interfaces/IExecutionModule.sol"; -import {IModule, ModuleMetadata} from "../interfaces/IModule.sol"; +import {IModule} from "../interfaces/IModule.sol"; import {BaseModule} from "./BaseModule.sol"; /// @title Token Receiver Module @@ -14,10 +14,6 @@ import {BaseModule} from "./BaseModule.sol"; /// @notice This module allows modular accounts to receive various types of tokens by implementing /// required token receiver interfaces. contract TokenReceiverModule is BaseModule, IExecutionModule, IERC721Receiver, IERC1155Receiver { - string internal constant _NAME = "Token Receiver Module"; - string internal constant _VERSION = "1.0.0"; - string internal constant _AUTHOR = "ERC-6900 Authors"; - // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ @@ -85,11 +81,7 @@ contract TokenReceiverModule is BaseModule, IExecutionModule, IERC721Receiver, I } /// @inheritdoc IModule - function moduleMetadata() external pure virtual override returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = _NAME; - metadata.version = _VERSION; - metadata.author = _AUTHOR; - return metadata; + function moduleId() external pure returns (string memory) { + return "erc6900/token-receiver-module/1.0.0"; } } diff --git a/src/modules/permissionhooks/AllowlistModule.sol b/src/modules/permissionhooks/AllowlistModule.sol index 9ccc4e61..3e4edb04 100644 --- a/src/modules/permissionhooks/AllowlistModule.sol +++ b/src/modules/permissionhooks/AllowlistModule.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.25; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; -import {ModuleMetadata} from "../../interfaces/IModule.sol"; +import {IModule} from "../../interfaces/IModule.sol"; import {Call, IModularAccount} from "../../interfaces/IModularAccount.sol"; import {IValidationHookModule} from "../../interfaces/IValidationHookModule.sol"; @@ -88,13 +88,9 @@ contract AllowlistModule is IValidationHookModule, BaseModule { // solhint-disable-next-line no-empty-blocks function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external pure override {} - function moduleMetadata() external pure override returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = "Allowlist Module"; - metadata.version = "v0.0.1"; - metadata.author = "ERC-6900 Working Group"; - - return metadata; + /// @inheritdoc IModule + function moduleId() external pure returns (string memory) { + return "erc6900/allowlist-module/0.0.1"; } function setAllowlistTarget(uint32 entityId, address target, bool allowed, bool hasSelectorAllowlist) public { diff --git a/src/modules/validation/SingleSignerValidationModule.sol b/src/modules/validation/SingleSignerValidationModule.sol index c9d2b938..d3dd71da 100644 --- a/src/modules/validation/SingleSignerValidationModule.sol +++ b/src/modules/validation/SingleSignerValidationModule.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.25; -import {IModule, ModuleMetadata} from "../../interfaces/IModule.sol"; +import {IModule} from "../../interfaces/IModule.sol"; import {IValidationModule} from "../../interfaces/IValidationModule.sol"; import {BaseModule} from "../BaseModule.sol"; @@ -26,10 +26,6 @@ import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/Signa contract SingleSignerValidationModule is ISingleSignerValidationModule, ReplaySafeWrapper, BaseModule { using MessageHashUtils for bytes32; - string internal constant _NAME = "SingleSigner Validation"; - string internal constant _VERSION = "1.0.0"; - string internal constant _AUTHOR = "ERC-6900 Authors"; - uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; @@ -115,12 +111,8 @@ contract SingleSignerValidationModule is ISingleSignerValidationModule, ReplaySa // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc IModule - function moduleMetadata() external pure virtual override returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = _NAME; - metadata.version = _VERSION; - metadata.author = _AUTHOR; - return metadata; + function moduleId() external pure returns (string memory) { + return "erc6900/single-signer-validation-module/1.0.0"; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ diff --git a/test/libraries/KnowSelectors.t.sol b/test/libraries/KnowSelectors.t.sol index 48bd17e9..76242372 100644 --- a/test/libraries/KnowSelectors.t.sol +++ b/test/libraries/KnowSelectors.t.sol @@ -18,6 +18,6 @@ contract KnownSelectorsTest is Test { } function test_isIModuleFunction() public { - assertTrue(KnownSelectors.isIModuleFunction(IModule.moduleMetadata.selector)); + assertTrue(KnownSelectors.isIModuleFunction(IModule.moduleId.selector)); } } diff --git a/test/mocks/MockModule.sol b/test/mocks/MockModule.sol index a6b63bec..354373e1 100644 --- a/test/mocks/MockModule.sol +++ b/test/mocks/MockModule.sol @@ -5,7 +5,7 @@ import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import {IExecutionHookModule} from "../../src/interfaces/IExecutionHookModule.sol"; import {ExecutionManifest} from "../../src/interfaces/IExecutionModule.sol"; -import {IModule, ModuleMetadata} from "../../src/interfaces/IModule.sol"; +import {IModule} from "../../src/interfaces/IModule.sol"; import {IValidationModule} from "../../src/interfaces/IValidationModule.sol"; contract MockModule is ERC165 { @@ -18,10 +18,6 @@ contract MockModule is ERC165 { // struct ManifestAssociatedFunction memory[] memory to storage not yet supported. bytes internal _manifest; - string internal constant _NAME = "Mock Module Modifiable"; - string internal constant _VERSION = "1.0.0"; - string internal constant _AUTHOR = "ERC-6900 Authors"; - event ReceivedCall(bytes msgData, uint256 msgValue); // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -51,12 +47,8 @@ contract MockModule is ERC165 { return _castToPure(_getManifest)(); } - function moduleMetadata() external pure returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = _NAME; - metadata.version = _VERSION; - metadata.author = _AUTHOR; - return metadata; + function moduleId() external pure returns (string memory) { + return "erc6900/mock-module/1.0.0"; } /// @dev Returns true if this contract implements the interface defined by diff --git a/test/mocks/modules/ComprehensiveModule.sol b/test/mocks/modules/ComprehensiveModule.sol index 3fb98041..623c4cf9 100644 --- a/test/mocks/modules/ComprehensiveModule.sol +++ b/test/mocks/modules/ComprehensiveModule.sol @@ -12,7 +12,6 @@ import { import {IExecutionHookModule} from "../../../src/interfaces/IExecutionHookModule.sol"; import {IExecutionModule} from "../../../src/interfaces/IExecutionModule.sol"; -import {ModuleMetadata} from "../../../src/interfaces/IModule.sol"; import {IValidationHookModule} from "../../../src/interfaces/IValidationHookModule.sol"; import {IValidationModule} from "../../../src/interfaces/IValidationModule.sol"; @@ -175,11 +174,7 @@ contract ComprehensiveModule is return manifest; } - function moduleMetadata() external pure virtual override returns (ModuleMetadata memory) { - ModuleMetadata memory metadata; - metadata.name = _NAME; - metadata.version = _VERSION; - metadata.author = _AUTHOR; - return metadata; + function moduleId() external pure returns (string memory) { + return "erc6900/comprehensive-module/1.0.0"; } } diff --git a/test/mocks/modules/DirectCallModule.sol b/test/mocks/modules/DirectCallModule.sol index d3303a55..76505dde 100644 --- a/test/mocks/modules/DirectCallModule.sol +++ b/test/mocks/modules/DirectCallModule.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.19; import {IExecutionHookModule} from "../../../src/interfaces/IExecutionHookModule.sol"; import {IModularAccount} from "../../../src/interfaces/IModularAccount.sol"; -import {ModuleMetadata} from "../../../src/interfaces/IModule.sol"; import {BaseModule} from "../../../src/modules/BaseModule.sol"; contract DirectCallModule is BaseModule, IExecutionHookModule { @@ -22,7 +21,9 @@ contract DirectCallModule is BaseModule, IExecutionHookModule { return hex"04546b"; } - function moduleMetadata() external pure override returns (ModuleMetadata memory) {} + function moduleId() external pure returns (string memory) { + return "erc6900/direct-call-module/1.0.0"; + } function preExecutionHook(uint32, address sender, uint256, bytes calldata) external diff --git a/test/mocks/modules/MockAccessControlHookModule.sol b/test/mocks/modules/MockAccessControlHookModule.sol index 03f5471c..e37e9630 100644 --- a/test/mocks/modules/MockAccessControlHookModule.sol +++ b/test/mocks/modules/MockAccessControlHookModule.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.25; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {IModularAccount} from "../../../src/interfaces/IModularAccount.sol"; -import {ModuleMetadata} from "../../../src/interfaces/IModule.sol"; import {IValidationHookModule} from "../../../src/interfaces/IValidationHookModule.sol"; import {BaseModule} from "../../../src/modules/BaseModule.sol"; @@ -84,5 +83,7 @@ contract MockAccessControlHookModule is IValidationHookModule, BaseModule { return; } - function moduleMetadata() external pure override returns (ModuleMetadata memory) {} + function moduleId() external pure returns (string memory) { + return "erc6900/mock-access-control-hook-module/1.0.0"; + } } diff --git a/test/mocks/modules/PermittedCallMocks.sol b/test/mocks/modules/PermittedCallMocks.sol index 9904a1ea..0981160f 100644 --- a/test/mocks/modules/PermittedCallMocks.sol +++ b/test/mocks/modules/PermittedCallMocks.sol @@ -6,7 +6,6 @@ import { IExecutionModule, ManifestExecutionFunction } from "../../../src/interfaces/IExecutionModule.sol"; -import {ModuleMetadata} from "../../../src/interfaces/IModule.sol"; import {BaseModule} from "../../../src/modules/BaseModule.sol"; import {ResultCreatorModule} from "./ReturnDataModuleMocks.sol"; @@ -30,7 +29,9 @@ contract PermittedCallerModule is IExecutionModule, BaseModule { return manifest; } - function moduleMetadata() external pure override returns (ModuleMetadata memory) {} + function moduleId() external pure returns (string memory) { + return "erc6900/permitted-caller-module/1.0.0"; + } // The manifest requested access to use the module-defined method "foo" function usePermittedCallAllowed() external view returns (bytes memory) { diff --git a/test/mocks/modules/ReturnDataModuleMocks.sol b/test/mocks/modules/ReturnDataModuleMocks.sol index 13672766..3eb81fa7 100644 --- a/test/mocks/modules/ReturnDataModuleMocks.sol +++ b/test/mocks/modules/ReturnDataModuleMocks.sol @@ -8,7 +8,6 @@ import { IExecutionModule, ManifestExecutionFunction } from "../../../src/interfaces/IExecutionModule.sol"; -import {ModuleMetadata} from "../../../src/interfaces/IModule.sol"; import {DIRECT_CALL_VALIDATION_ENTITYID} from "../../../src/helpers/Constants.sol"; @@ -58,7 +57,9 @@ contract ResultCreatorModule is IExecutionModule, BaseModule { return manifest; } - function moduleMetadata() external pure override returns (ModuleMetadata memory) {} + function moduleId() external pure returns (string memory) { + return "erc6900/result-creator-module/1.0.0"; + } } contract ResultConsumerModule is IExecutionModule, BaseModule, IValidationModule { @@ -137,5 +138,7 @@ contract ResultConsumerModule is IExecutionModule, BaseModule, IValidationModule return manifest; } - function moduleMetadata() external pure override returns (ModuleMetadata memory) {} + function moduleId() external pure returns (string memory) { + return "erc6900/result-consumer-module/1.0.0"; + } } diff --git a/test/mocks/modules/ValidationModuleMocks.sol b/test/mocks/modules/ValidationModuleMocks.sol index a143e03a..3521e3fc 100644 --- a/test/mocks/modules/ValidationModuleMocks.sol +++ b/test/mocks/modules/ValidationModuleMocks.sol @@ -8,7 +8,6 @@ import { IExecutionModule, ManifestExecutionFunction } from "../../../src/interfaces/IExecutionModule.sol"; -import {ModuleMetadata} from "../../../src/interfaces/IModule.sol"; import {IValidationHookModule} from "../../../src/interfaces/IValidationHookModule.sol"; import {IValidationModule} from "../../../src/interfaces/IValidationModule.sol"; @@ -75,9 +74,11 @@ abstract contract MockBaseUserOpValidationModule is revert NotImplemented(); } - // Empty stubs - function moduleMetadata() external pure override returns (ModuleMetadata memory) {} + function moduleId() external pure returns (string memory) { + return "erc6900/mock-user-op-validation-module/1.0.0"; + } + // Empty stubs function preRuntimeValidationHook(uint32, address, uint256, bytes calldata, bytes calldata) external pure