From 2ba497873eb0ac854c976f611054ff85ebf8e936 Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Mon, 22 Jan 2024 01:49:41 -0500 Subject: [PATCH 1/4] feat: update spec for v0.7.0 --- src/account/PluginManagerInternals.sol | 4 +- src/account/UpgradeableModularAccount.sol | 4 +- src/interfaces/IAccountLoupe.sol | 32 +-- src/interfaces/IPlugin.sol | 42 ++-- src/interfaces/IPluginExecutor.sol | 22 ++- src/interfaces/IPluginManager.sol | 12 +- src/interfaces/IStandardExecutor.sol | 13 +- standard/ERCs/erc-6900.md | 182 ++++++++++-------- test/account/AccountExecHooks.t.sol | 6 +- test/account/AccountReturnData.t.sol | 4 +- .../ExecuteFromPluginPermissions.t.sol | 6 +- test/account/ManifestValidity.t.sol | 16 +- test/account/UpgradeableModularAccount.t.sol | 20 +- test/account/ValidationIntersection.t.sol | 6 +- test/mocks/MSCAFactoryFixture.sol | 6 +- 15 files changed, 197 insertions(+), 178 deletions(-) diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManagerInternals.sol index 2c0b44e0..c77fb301 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManagerInternals.sol @@ -218,7 +218,7 @@ abstract contract PluginManagerInternals is IPluginManager { function _installPlugin( address plugin, bytes32 manifestHash, - bytes memory pluginInitData, + bytes memory pluginInstallData, FunctionReference[] memory dependencies ) internal { AccountStorage storage _storage = getAccountStorage(); @@ -431,7 +431,7 @@ abstract contract PluginManagerInternals is IPluginManager { // Initialize the plugin storage for the account. // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).onInstall(pluginInitData) {} + try IPlugin(plugin).onInstall(pluginInstallData) {} catch (bytes memory revertReason) { revert PluginInstallCallbackFailed(plugin, revertReason); } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index c4553734..e26db165 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -262,10 +262,10 @@ contract UpgradeableModularAccount is function installPlugin( address plugin, bytes32 manifestHash, - bytes calldata pluginInitData, + bytes calldata pluginInstallData, FunctionReference[] calldata dependencies ) external override wrapNativeFunction { - _installPlugin(plugin, manifestHash, pluginInitData, dependencies); + _installPlugin(plugin, manifestHash, pluginInstallData, dependencies); } /// @inheritdoc IPluginManager diff --git a/src/interfaces/IAccountLoupe.sol b/src/interfaces/IAccountLoupe.sol index 6c4f15bd..25d50487 100644 --- a/src/interfaces/IAccountLoupe.sol +++ b/src/interfaces/IAccountLoupe.sol @@ -4,35 +4,35 @@ pragma solidity ^0.8.19; import {FunctionReference} from "../interfaces/IPluginManager.sol"; interface IAccountLoupe { - /// @notice Config for an execution function, given a selector + /// @notice Config for an execution function, given a selector. struct ExecutionFunctionConfig { address plugin; FunctionReference userOpValidationFunction; FunctionReference runtimeValidationFunction; } - /// @notice Pre and post hooks for a given selector - /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty + /// @notice Pre and post hooks for a given selector. + /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty. struct ExecutionHooks { FunctionReference preExecHook; FunctionReference postExecHook; } - /// @notice Gets the validation functions and plugin address for a selector - /// @dev If the selector is a native function, the plugin address will be the address of the account - /// @param selector The selector to get the configuration for - /// @return The configuration for this selector + /// @notice Get the validation functions and plugin address for a selector. + /// @dev If the selector is a native function, the plugin address will be the address of the account. + /// @param selector The selector to get the configuration for. + /// @return The configuration for this selector. function getExecutionFunctionConfig(bytes4 selector) external view returns (ExecutionFunctionConfig memory); - /// @notice Gets the pre and post execution hooks for a selector - /// @param selector The selector to get the hooks for - /// @return The pre and post execution hooks for this selector + /// @notice Get the pre and post execution hooks for a selector. + /// @param selector The selector to get the hooks for. + /// @return The pre and post execution hooks for this selector. function getExecutionHooks(bytes4 selector) external view returns (ExecutionHooks[] memory); - /// @notice Gets the pre user op and runtime validation hooks associated with a selector - /// @param selector The selector to get the hooks for - /// @return preUserOpValidationHooks The pre user op validation hooks for this selector - /// @return preRuntimeValidationHooks The pre runtime validation hooks for this selector + /// @notice Get the pre user op and runtime validation hooks associated with a selector. + /// @param selector The selector to get the hooks for. + /// @return preUserOpValidationHooks The pre user op validation hooks for this selector. + /// @return preRuntimeValidationHooks The pre runtime validation hooks for this selector. function getPreValidationHooks(bytes4 selector) external view @@ -41,7 +41,7 @@ interface IAccountLoupe { FunctionReference[] memory preRuntimeValidationHooks ); - /// @notice Gets an array of all installed plugins - /// @return The addresses of all installed plugins + /// @notice Get an array of all installed plugins. + /// @return The addresses of all installed plugins. function getInstalledPlugins() external view returns (address[] memory); } diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index c02bc207..f2d095fb 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -7,28 +7,28 @@ import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/User // so annotating here to prevent that. // forgefmt: disable-start enum ManifestAssociatedFunctionType { - /// @notice Function is not defined. + // Function is not defined. NONE, - /// @notice Function belongs to this plugin. + // Function belongs to this plugin. SELF, - /// @notice Function belongs to an external plugin provided as a dependency during plugin installation. + // Function belongs to an external plugin provided as a dependency during plugin installation. DEPENDENCY, - /// @notice Resolves to a magic value to always bypass runtime validation for a given function. - /// This is only assignable on runtime validation functions. If it were to be used on a user op validationFunction, - /// it would risk burning gas from the account. When used as a hook in any hook location, it is equivalent to not - /// setting a hook and is therefore disallowed. + // Resolves to a magic value to always bypass runtime validation for a given function. + // This is only assignable on runtime validation functions. If it were to be used on a user op validationFunction, + // it would risk burning gas from the account. When used as a hook in any hook location, it is equivalent to not + // setting a hook and is therefore disallowed. RUNTIME_VALIDATION_ALWAYS_ALLOW, - /// @notice Resolves to a magic value to always fail in a hook for a given function. - /// This is only assignable to pre hooks (pre validation and pre execution). It should not be used on - /// validation functions themselves, because this is equivalent to leaving the validation functions unset. - /// It should not be used in post-exec hooks, because if it is known to always revert, that should happen - /// as early as possible to save gas. + // Resolves to a magic value to always fail in a hook for a given function. + // This is only assignable to pre hooks (pre validation and pre execution). It should not be used on + // validation functions themselves, because this is equivalent to leaving the validation functions unset. + // It should not be used in post-exec hooks, because if it is known to always revert, that should happen + // as early as possible to save gas. PRE_HOOK_ALWAYS_DENY } // forgefmt: disable-end -// For functions of type `ManifestAssociatedFunctionType.DEPENDENCY`, the MSCA MUST find the plugin address -// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. +/// @dev For functions of type `ManifestAssociatedFunctionType.DEPENDENCY`, the MSCA MUST find the plugin address +/// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. struct ManifestFunction { ManifestAssociatedFunctionType functionType; uint8 functionId; @@ -73,19 +73,21 @@ struct PluginMetadata { /// @dev A struct describing how the plugin should be installed on a modular account. struct PluginManifest { - // List of ERC-165 interfaceIds to add to account to support introspection checks. + // List of ERC-165 interface IDs to add to account to support introspection checks. This MUST NOT include + // IPlugin's interface ID. bytes4[] interfaceIds; - // If this plugin depends on other plugins' validation functions and/or hooks, the interface IDs of - // those plugins MUST be provided here, with its position in the array matching the `dependencyIndex` - // members of `ManifestFunction` structs used in the manifest. + // If this plugin depends on other plugins' validation functions, the interface IDs of those plugins MUST be + // provided here, with its position in the array matching the `dependencyIndex` members of `ManifestFunction` + // structs used in the manifest. bytes4[] dependencyInterfaceIds; // Execution functions defined in this plugin to be installed on the MSCA. bytes4[] executionFunctions; // Plugin execution functions already installed on the MSCA that this plugin will be able to call. bytes4[] permittedExecutionSelectors; - // Boolean to indicate whether the plugin can call any external contract addresses. + // Boolean to indicate whether the plugin can call any external address. bool permitAnyExternalAddress; - // Boolean to indicate whether the plugin needs access to spend native tokens of the account. + // Boolean to indicate whether the plugin needs access to spend native tokens of the account. If false, the + // plugin MUST still be able to spend up to the balance that it sends to the account in the same call. bool canSpendNativeToken; ManifestExternalCallPermission[] permittedExternalCalls; ManifestAssociatedFunction[] userOpValidationFunctions; diff --git a/src/interfaces/IPluginExecutor.sol b/src/interfaces/IPluginExecutor.sol index b868b33a..d18ba665 100644 --- a/src/interfaces/IPluginExecutor.sol +++ b/src/interfaces/IPluginExecutor.sol @@ -2,18 +2,20 @@ pragma solidity ^0.8.19; interface IPluginExecutor { - /// @notice Executes a call from a plugin to another plugin. - /// @dev Permissions must be granted to the calling plugin for the call to go through. - /// @param data calldata to send to the plugin - /// @return The result of the call + /// @notice Execute a call from a plugin to another plugin, via an execution function installed on the account. + /// @dev Plugins are not allowed to call native functions on the account. Permissions must be granted to the + /// calling plugin for the call to go through. + /// @param data The calldata to send to the plugin. + /// @return The return data from the call. function executeFromPlugin(bytes calldata data) external payable returns (bytes memory); - /// @notice Executes a call from a plugin to a non-plugin address. - /// @dev Permissions must be granted to the calling plugin for the call to go through. - /// @param target address of the target to call - /// @param value value to send with the call - /// @param data calldata to send to the target - /// @return The result of the call + /// @notice Execute a call from a plugin to a non-plugin address. + /// @dev If the target is a plugin, the call SHOULD revert. Permissions must be granted to the calling plugin + /// for the call to go through. + /// @param target The address to be called. + /// @param value The value to send with the call. + /// @param data The calldata to send to the target. + /// @return The return data from the call. function executeFromPluginExternal(address target, uint256 value, bytes calldata data) external payable diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index 48f069df..21054d27 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -3,22 +3,22 @@ pragma solidity ^0.8.19; type FunctionReference is bytes21; -/// @title Plugin Manager Interface interface IPluginManager { event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); - event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); + event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded); /// @notice Install a plugin to the modular account. /// @param plugin The plugin to install. /// @param manifestHash The hash of the plugin manifest. - /// @param pluginInitData Optional data to be decoded and used by the plugin to setup initial plugin data for - /// the modular account. - /// @param dependencies The dependencies of the plugin, as described in the manifest. + /// @param pluginInstallData Optional data to be decoded and used by the plugin to setup initial plugin data + /// for the modular account. + /// @param dependencies The dependencies of the plugin, as described in the manifest. Each FunctionReference + /// MUST be composed of an installed plugin's address and a function ID of its validation function. function installPlugin( address plugin, bytes32 manifestHash, - bytes calldata pluginInitData, + bytes calldata pluginInstallData, FunctionReference[] calldata dependencies ) external; diff --git a/src/interfaces/IStandardExecutor.sol b/src/interfaces/IStandardExecutor.sol index 5f49930d..a43da646 100644 --- a/src/interfaces/IStandardExecutor.sol +++ b/src/interfaces/IStandardExecutor.sol @@ -2,27 +2,26 @@ pragma solidity ^0.8.19; struct Call { - // The target address for account to call. + // The target address for the account to call. address target; // The value sent with the call. uint256 value; - // The call data for the call. + // The calldata for the call. bytes data; } -/// @title Standard Executor Interface interface IStandardExecutor { /// @notice Standard execute method. /// @dev If the target is a plugin, the call SHOULD revert. - /// @param target The target address for account to call. + /// @param target The target address for the account to call. /// @param value The value sent with the call. - /// @param data The call data for the call. + /// @param data The calldata for the call. /// @return The return data from the call. function execute(address target, uint256 value, bytes calldata data) external payable returns (bytes memory); /// @notice Standard executeBatch method. - /// @dev If the target is a plugin, the call SHOULD revert. If any of the transactions revert, the entire batch - /// reverts. + /// @dev If the target is a plugin, the call SHOULD revert. If any of the calls revert, the entire batch MUST + /// revert. /// @param calls The array of calls. /// @return An array containing the return data from the calls. function executeBatch(Call[] calldata calls) external payable returns (bytes[] memory); diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index 8487d6ef..4c377f30 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -1,7 +1,7 @@ --- eip: 6900 title: Modular Smart Contract Accounts and Plugins -description: Interfaces for composable contract accounts optionally supporting upgradability and introspection +description: Interfaces for composable contract accounts optionally supporting upgradeability and introspection author: Adam Egyed (@adamegyed), Fangting Liu (@trinity-0111), Jay Paik (@jaypaik), Yoav Weiss (@yoavw) discussions-to: https://ethereum-magicians.org/t/eip-modular-smart-contract-accounts-and-plugins/13885 status: Draft @@ -77,7 +77,7 @@ A call to the smart contract account can be broken down into the steps as shown The following diagram shows permitted plugin execution flows. During a plugin's execution step from the above diagram, the plugin may perform a "Plugin Execution Function", using either `executeFromPlugin` or `executeFromPluginExternal`. These can be used by plugins to execute using the account's context. - `executeFromPlugin` handles calls to other installed plugin's execution function on the modular account. -- `executeFromPluginExternal` handles calls to external contracts. +- `executeFromPluginExternal` handles calls to external addresses. ![diagram showing a plugin execution flow](../assets/eip-6900/Plugin_Execution_Flow.svg) @@ -87,7 +87,7 @@ Each step is modular, supporting different implementations for each execution fu **Modular Smart Contract Accounts** **MUST** implement -- `IAccount` from [ERC-4337](./eip-4337.md). +- `IAccount.sol` from [ERC-4337](./eip-4337.md). - `IPluginManager.sol` to support installing and uninstalling plugins. - `IStandardExecutor.sol` to support open-ended execution. **Calls to plugins through this SHOULD revert.** - `IPluginExecutor.sol` to support execution from plugins. **Calls to plugins through `executeFromPluginExternal` SHOULD revert.** @@ -98,28 +98,32 @@ Each step is modular, supporting different implementations for each execution fu **Plugins** **MUST** implement -- `IPlugin` interface described below and implement [ERC-165](./eip-165.md) for `IPlugin`. +- `IPlugin.sol` described below and implement [ERC-165](./eip-165.md) for `IPlugin`. #### `IPluginManager.sol` Plugin manager interface. Modular Smart Contract Accounts **MUST** implement this interface to support installing and uninstalling plugins. ```solidity +// Treats the first 20 bytes as an address, and the last byte as a function identifier. +type FunctionReference is bytes21; + interface IPluginManager { event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); - event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); + event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded); /// @notice Install a plugin to the modular account. /// @param plugin The plugin to install. /// @param manifestHash The hash of the plugin manifest. - /// @param pluginInitData Optional data to be decoded and used by the plugin to setup initial plugin data for - /// the modular account. - /// @param dependencies The dependencies of the plugin, as described in the manifest. + /// @param pluginInstallData Optional data to be decoded and used by the plugin to setup initial plugin data + /// for the modular account. + /// @param dependencies The dependencies of the plugin, as described in the manifest. Each FunctionReference + /// MUST be composed of an installed plugin's address and a function ID of its validation function. function installPlugin( address plugin, bytes32 manifestHash, - bytes calldata pluginInitData, + bytes calldata pluginInstallData, FunctionReference[] calldata dependencies ) external; @@ -144,25 +148,26 @@ Standard execute functions SHOULD check whether the call's target implements the ```solidity struct Call { - // The target contract for account to execute. + // The target address for the account to call. address target; - // The value for the call. + // The value sent with the call. uint256 value; - // The call data for the call. + // The calldata for the call. bytes data; } interface IStandardExecutor { /// @notice Standard execute method. /// @dev If the target is a plugin, the call SHOULD revert. - /// @param target The target contract for account to execute. - /// @param value The value for the call. - /// @param data The call data for the call. + /// @param target The target address for account to call. + /// @param value The value sent with the call. + /// @param data The calldata for the call. /// @return The return data from the call. function execute(address target, uint256 value, bytes calldata data) external payable returns (bytes memory); /// @notice Standard executeBatch method. - /// @dev If the target is a plugin, the call SHOULD revert. + /// @dev If the target is a plugin, the call SHOULD revert. If any of the calls revert, the entire batch MUST + /// revert. /// @param calls The array of calls. /// @return An array containing the return data from the calls. function executeBatch(Call[] calldata calls) external payable returns (bytes[] memory); @@ -181,17 +186,22 @@ This prevents accidental misconfiguration or misuse of plugins (both installed a ```solidity interface IPluginExecutor { - /// @notice Method from calls made from plugins. - /// @param data The call data for the call. + /// @notice Execute a call from a plugin to another plugin, via an execution function installed on the account. + /// @dev Permissions must be granted to the calling plugin for the call to go through. + /// @param data Calldata to send to the plugin. /// @return The return data from the call. function executeFromPlugin(bytes calldata data) external payable returns (bytes memory); - /// @notice Method from cals made from plugins. - /// @dev If the target is a plugin, the call SHOULD revert. - /// @param target The target of the external contract to be called. - /// @param value The value to pass. - /// @param data The data to pass. - function executeFromPluginExternal(address target, uint256 value, bytes calldata data) external payable; + /// @notice Execute a call from a plugin to a non-plugin address. + /// @dev Permissions must be granted to the calling plugin for the call to go through. + /// @param target Address of the target to call. + /// @param value Value to send with the call. + /// @param data Calldata to send to the target. + /// @return The return data from the call. + function executeFromPluginExternal(address target, uint256 value, bytes calldata data) + external + payable + returns (bytes memory); } ``` @@ -200,39 +210,36 @@ interface IPluginExecutor { Plugin inspection interface. Modular Smart Contract Accounts **MAY** implement this interface to support visibility in plugin configuration on-chain. ```solidity -// Treats the first 20 bytes as an address, and the last byte as a function identifier. -type FunctionReference is bytes21; - interface IAccountLoupe { - /// @notice Config for an execution function, given a selector + /// @notice Config for an execution function, given a selector. struct ExecutionFunctionConfig { address plugin; FunctionReference userOpValidationFunction; FunctionReference runtimeValidationFunction; } - /// @notice Pre and post hooks for a given selector - /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty + /// @notice Pre and post hooks for a given selector. + /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty. struct ExecutionHooks { FunctionReference preExecHook; FunctionReference postExecHook; } - /// @notice Gets the validation functions and plugin address for a selector - /// @dev If the selector is a native function, the plugin address will be the address of the account - /// @param selector The selector to get the configuration for - /// @return The configuration for this selector + /// @notice Get the validation functions and plugin address for a selector. + /// @dev If the selector is a native function, the plugin address will be the address of the account. + /// @param selector The selector to get the configuration for. + /// @return The configuration for this selector. function getExecutionFunctionConfig(bytes4 selector) external view returns (ExecutionFunctionConfig memory); - /// @notice Gets the pre and post execution hooks for a selector - /// @param selector The selector to get the hooks for - /// @return The pre and post execution hooks for this selector + /// @notice Get the pre and post execution hooks for a selector. + /// @param selector The selector to get the hooks for. + /// @return The pre and post execution hooks for this selector. function getExecutionHooks(bytes4 selector) external view returns (ExecutionHooks[] memory); - /// @notice Gets the pre user op and runtime validation hooks associated with a selector - /// @param selector The selector to get the hooks for - /// @return preUserOpValidationHooks The pre user op validation hooks for this selector - /// @return preRuntimeValidationHooks The pre runtime validation hooks for this selector + /// @notice Get the pre user op and runtime validation hooks associated with a selector. + /// @param selector The selector to get the hooks for. + /// @return preUserOpValidationHooks The pre user op validation hooks for this selector. + /// @return preRuntimeValidationHooks The pre runtime validation hooks for this selector. function getPreValidationHooks(bytes4 selector) external view @@ -241,8 +248,8 @@ interface IAccountLoupe { FunctionReference[] memory preRuntimeValidationHooks ); - /// @notice Gets an array of all installed plugins - /// @return The addresses of all installed plugins + /// @notice Get an array of all installed plugins. + /// @return The addresses of all installed plugins. function getInstalledPlugins() external view returns (address[] memory); } ``` @@ -332,27 +339,28 @@ The plugin manifest is responsible for describing the execution functions, valid ```solidity enum ManifestAssociatedFunctionType { - /// @notice Function is not defined. + // Function is not defined. NONE, - /// @notice Function belongs to this plugin. + // Function belongs to this plugin. SELF, - /// @notice Function belongs to an external plugin provided as a dependency during plugin installation. + // Function belongs to an external plugin provided as a dependency during plugin installation. Plugins MAY depend + // on external validation functions. It MUST NOT depend on external hooks, or installation will fail. DEPENDENCY, - /// @notice Resolves to a magic value to always bypass runtime validation for a given function. - /// This is only assignable on runtime validation functions. If it were to be used on a user op validationFunction, - /// it would risk burning gas from the account. When used as a hook in any hook location, it is equivalent to not - /// setting a hook and is therefore disallowed. + // Resolves to a magic value to always bypass runtime validation for a given function. + // This is only assignable on runtime validation functions. If it were to be used on a user op validationFunction, + // it would risk burning gas from the account. When used as a hook in any hook location, it is equivalent to not + // setting a hook and is therefore disallowed. RUNTIME_VALIDATION_ALWAYS_ALLOW, - /// @notice Resolves to a magic value to always fail in a hook for a given function. - /// This is only assignable to pre hooks (pre validation and pre execution). It should not be used on - /// validation functions themselves, because this is equivalent to leaving the validation functions unset. - /// It should not be used in post-exec hooks, because if it is known to always revert, that should happen - /// as early as possible to save gas. + // Resolves to a magic value to always fail in a hook for a given function. + // This is only assignable to pre hooks (pre validation and pre execution). It should not be used on + // validation functions themselves, because this is equivalent to leaving the validation functions unset. + // It should not be used in post-exec hooks, because if it is known to always revert, that should happen + // as early as possible to save gas. PRE_HOOK_ALWAYS_DENY } -// For functions of type `ManifestAssociatedFunctionType.DEPENDENCY`, the MSCA MUST find the plugin address -// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. +/// @dev For functions of type `ManifestAssociatedFunctionType.DEPENDENCY`, the MSCA MUST find the plugin address +/// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. struct ManifestFunction { ManifestAssociatedFunctionType functionType; uint8 functionId; @@ -390,26 +398,28 @@ struct PluginMetadata { // The author field SHOULD be a username representing the identity of the user or organization // that created this plugin. string author; - // String desciptions of the relative sensitivity of specific functions. The selectors MUST be selectors for + // String descriptions of the relative sensitivity of specific functions. The selectors MUST be selectors for // functions implemented by this plugin. SelectorPermission[] permissionDescriptors; } /// @dev A struct describing how the plugin should be installed on a modular account. struct PluginManifest { - // List of ERC-165 interfaceIds to add to account to support introspection checks. + // List of ERC-165 interface IDs to add to account to support introspection checks. This MUST NOT include + // IPlugin's interface ID. bytes4[] interfaceIds; - // If this plugin depends on other plugins' validation functions and/or hooks, the interface IDs of - // those plugins MUST be provided here, with its position in the array matching the `dependencyIndex` - // members of `ManifestFunction` structs used in the manifest. + // If this plugin depends on other plugins' validation functions, the interface IDs of those plugins MUST be + // provided here, with its position in the array matching the `dependencyIndex` members of `ManifestFunction` + // structs used in the manifest. bytes4[] dependencyInterfaceIds; // Execution functions defined in this plugin to be installed on the MSCA. bytes4[] executionFunctions; // Plugin execution functions already installed on the MSCA that this plugin will be able to call. bytes4[] permittedExecutionSelectors; - // Boolean to indicate whether the plugin can call any external contract addresses. + // Boolean to indicate whether the plugin can call any external address. bool permitAnyExternalAddress; - // Boolean to indicate whether the plugin needs access to spend native tokens of the account. + // Boolean to indicate whether the plugin needs access to spend native tokens of the account. If false, the + // plugin MUST still be able to spend up to the balance that it sends to the account in the same call. bool canSpendNativeToken; ManifestExternalCallPermission[] permittedExternalCalls; ManifestAssociatedFunction[] userOpValidationFunctions; @@ -425,11 +435,11 @@ struct PluginManifest { #### Responsibilties of `StandardExecutor` and `PluginExecutor` -`StandardExecutor` functions are mainly used for open-ended execution of external contracts. +`StandardExecutor` functions are mainly used for open-ended execution against external addresses. `PluginExecutor` functions are specifically used by plugins to request the account to execute with account's context. Explicit permissions are required for plugins to use `PluginExecutor`. -The following behavior must be followed: +The following behavior MUST be followed: - `StandardExecutor` can NOT call plugin execution functions and/or `PluginExecutor`. This is guaranteed by checking whether the call's target implements the `IPlugin` interface via ERC-165 as required. - `StandardExecutor` can NOT be called by plugin execution functions and/or `PluginExecutor`. @@ -448,18 +458,20 @@ The function MUST perform the following preliminary checks: - Revert if `manifestHash` does not match the computed Keccak-256 hash of the plugin's returned manifest. This prevents installation of plugins that attempt to install a different plugin configuration than the one that was approved by the client. - Revert if any address in `dependencies` does not support the interface at its matching index in the manifest's `dependencyInterfaceIds`, or if the two array lengths do not match, or if any of the dependencies are not already installed on the modular account. -The function MUST record the manifest hash and dependencies that were used for the plugin's installation. Each dependency's record MUST also be updated to reflect that it has a new dependent. These records MUST be used to ensure calls to `uninstallPlugin` are comprehensive and undo all editted configuration state from installation. The mechanism by which these records are stored and validated is up to the implementation. +The function MUST record the manifest hash and dependencies that were used for the plugin's installation. Each dependency's record MUST also be updated to reflect that it has a new dependent. These records MUST be used to ensure calls to `uninstallPlugin` are comprehensive and undo all edited configuration state from installation. The mechanism by which these records are stored and validated is up to the implementation. -The function MUST store the plugin's permitted function selectors and external contract calls to be able to validate calls to `executeFromPlugin` and `executeFromPluginExternal`. +The function MUST store the plugin's permitted function selectors, permitted external calls, and whether it can spend the account's native tokens, to be able to validate calls to `executeFromPlugin` and `executeFromPluginExternal`. The function MUST parse through the execution functions, validation functions, and hooks in the manifest and add them to the modular account after resolving each `ManifestFunction` type. -- Each function selector MUST be added as a valid execution function on the modular account. If the function selector has already been added or matches the selector of a native function, the function SHOULD revert. +- Each execution function selector MUST be added as a valid execution function on the modular account. If the function selector has already been added or matches the selector of a native function, the function SHOULD revert. - If a validation function is to be added to a selector that already has that type of validation function, the function SHOULD revert. -Next, the function MUST call the plugin's `onInstall` callback with the data provided in the `installData` parameter. This serves to initialize the plugin state for the modular account. If `onInstall` reverts, the `installPlugin` function MUST revert. +The function MAY store the interface IDs provided in the manifest's `interfaceIds` and update its `supportsInterface` behavior accordingly. -Finally, the function must emit the event `PluginInstalled` with the plugin's address and a hash of its manifest. +Next, the function MUST call the plugin's `onInstall` callback with the data provided in the `pluginInstallData` parameter. This serves to initialize the plugin state for the modular account. If `onInstall` reverts, the `installPlugin` function MUST revert. + +Finally, the function MUST emit the event `PluginInstalled` with the plugin's address, the hash of its manifest, and the dependencies that were used. > **⚠️ The ability to install and uninstall plugins is very powerful. The security of these functions determines the security of the account. It is critical for modular account implementers to make sure the implementation of the functions in `IPluginManager` have the proper security consideration and access control in place.** @@ -472,23 +484,27 @@ The function MUST revert if the plugin is not installed on the modular account. The function SHOULD perform the following checks: - Revert if the hash of the manifest used at install time does not match the computed Keccak-256 hash of the plugin's current manifest. This prevents unclean removal of plugins that attempt to force a removal of a different plugin configuration than the one that was originally approved by the client for installation. To allow for removal of such plugins, the modular account MAY implement the capability for the manifest to be encoded in the config field as a parameter. -- Revert if there is at least 1 other installed plugin that depends on validation functions or hooks added by this plugin. Plugins used as dependencies must not be uninstalled while dependent plugins exist. +- Revert if there is at least 1 other installed plugin that depends on validation functions added by this plugin. Plugins used as dependencies SHOULD NOT be uninstalled while dependent plugins exist. The function SHOULD update account storage to reflect the uninstall via inspection functions, such as those defined by `IAccountLoupe`. Each dependency's record SHOULD also be updated to reflect that it has no longer has this plugin as a dependent. -The function MUST remove records for the plugin's dependencies, permitted function selectors, and permitted external calls. The hooks to remove MUST be exactly the same as what was provided during installation. It is up to the implementing modular account to decide how to keep this invariant. The config parameter field MAY be used. +The function MUST remove records for the plugin's manifest hash, dependencies, permitted function selectors, permitted external calls, and whether it can spend the account's native tokens. + +The function MUST parse through the execution functions, validation functions, and hooks in the manifest and remove them from the modular account after resolving each `ManifestFunction` type. If multiple plugins added the same hook, it MUST persist until the last plugin is uninstalled. + +If the account stored the interface IDs provided in the manifest's `interfaceIds` during installation, it MUST remove them and update its `supportsInterface` behavior accordingly. If multiple plugins added the same interface ID, it MUST persist until the last plugin is uninstalled. -The function MUST parse through the execution functions, validation functions, and hooks in the manifest and remove them from the modular account after resolving each `ManifestFunction` type. +Next, the function MUST call the plugin's `onUninstall` callback with the data provided in the `pluginUninstallData` parameter. This serves to clear the plugin state for the modular account. If `onUninstall` reverts, execution SHOULD continue to allow the uninstall to complete. -Finally, the function MUST call the plugin's `onUninstall` callback with the data provided in the `uninstallData` parameter. This serves to clear the plugin state for the modular account. If `onUninstall` reverts, execution SHOULD continue to allow the uninstall to complete. +Finally, the function MUST emit the event `PluginUninstalled` with the plugin's address and whether the `onUninstall` callback succeeded. -> **⚠️ Incorrectly uninstalled plugins can prevent uninstallation of their dependencies. Therefore, some form of validation that the uninstall step completely and correctly removes the plugin and its usage of dependencies is required.** +> **⚠️ Incorrectly uninstalled plugins can prevent uninstalls of their dependencies. Therefore, some form of validation that the uninstall step completely and correctly removes the plugin and its usage of dependencies is required.** #### Calls to `validateUserOp` When the function `validateUserOp` is called on modular account by the `EntryPoint`, it MUST find the user operation validation function associated to the function selector in the first four bytes of `userOp.callData`. If there is no function defined for the selector, or if `userOp.callData.length < 4`, then execution MUST revert. -If the function selector has associated pre user operation validation hooks, then those hooks MUST be run sequentially. If any revert, the outer call MUST revert. If any are set to `PRE_HOOK_ALWAYS_DENY`, the call must revert. If any return an `authorizer` value other than 0 or 1, execution MUST revert. If any return an `authorizer` value of 1, indicating an invalid signature, the returned validation data of the outer call MUST also be 1. If any return time-bounded validation by specifying either a `validUntil` or `validBefore` value, the resulting validation data MUST be the intersection of all time bounds provided. +If the function selector has associated pre user operation validation hooks, then those hooks MUST be run sequentially. If any revert, the outer call MUST revert. If any are set to `PRE_HOOK_ALWAYS_DENY`, the call MUST revert. If any return an `authorizer` value other than 0 or 1, execution MUST revert. If any return an `authorizer` value of 1, indicating an invalid signature, the returned validation data of the outer call MUST also be 1. If any return time-bounded validation by specifying either a `validUntil` or `validBefore` value, the resulting validation data MUST be the intersection of all time bounds provided. Then, the modular account MUST execute the validation function with the user operation and its hash as parameters using the `call` opcode. The returned validation data from the user operation validation function MUST be updated, if necessary, by the return values of any pre user operation validation hooks, then returned by `validateUserOp`. @@ -500,18 +516,18 @@ Additionally, when the modular account natively implements functions in `IPlugin The steps to perform are: -- If the call is not from the `EntryPoint`, then find an associated runtime validation function. If one does not exist, execution MUST revert. The modular account MUST execute all pre runtime validation hooks, then the runtime validation function, with the `call` opcode. All of these functions MUST receive the caller, value, and execution function's calldata as parameters. If any of these functions revert, execution MUST revert. If any are set to `PRE_HOOK_ALWAYS_DENY`, execution MUST revert. If any are set to `RUNTIME_VALIDATION_ALWAYS_ALLOW`, those must not be run and treated as though they did not revert. -- If there are pre execution hooks defined for the execution function, execute those hooks with the caller, value, and execution function's calldata as parameters. If any of these hooks returns data, it MUST be preserved until the call to the post execution hook. The operation MUST be done with the `call` opcode. If any of these functions revert, execution MUST revert. +- If the call is not from the `EntryPoint`, then find an associated runtime validation function. If one does not exist, execution MUST revert. The modular account MUST execute all pre runtime validation hooks, then the runtime validation function, with the `call` opcode. All of these functions MUST receive the caller, value, and execution function's calldata as parameters. If any of these functions revert, execution MUST revert. If any pre runtime validation hooks are set to `PRE_HOOK_ALWAYS_DENY`, execution MUST revert. If any runtime validation functions are set to `RUNTIME_VALIDATION_ALWAYS_ALLOW`, the validation function MUST be bypassed. +- If there are pre execution hooks defined for the execution function, execute those hooks with the caller, value, and execution function's calldata as parameters. If any of these hooks returns data, it MUST be preserved until the call to the post execution hook. The operation MUST be done with the `call` opcode. If there are duplicate pre execution hooks applied to the same selector, run the hook only once. If any of these functions revert, execution MUST revert. - Run the execution function. -- If any associated post execution hooks are defined, run the functions. If a pre execution hook returned data to the account, that data MUST be passed as a parameter to the associated post execution hook. The operation MUST be done with the `call` opcode. If any of these functions revert, execution MUST revert. Notably, for the `uninstallPlugin` native function, the post execution hooks defined for it prior to the uninstall MUST run afterwards. +- If any post execution hooks are defined, run the functions. If a pre execution hook returned data to the account, that data MUST be passed as a parameter to the associated post execution hook. The operation MUST be done with the `call` opcode. If there are duplicate post execution hooks applied to the same selector, run them once for each unique associated pre execution hook. For post execution hooks without an associated pre execution hook, run the hook only once. If any of these functions revert, execution MUST revert. Notably, for the `uninstallPlugin` native function, the post execution hooks defined for it prior to the uninstall MUST run afterwards. #### Calls made from plugins -Plugins MAY interact with other plugins and external contracts through the modular account using the functions defined in the `IPluginExecutor` interface. These functions MAY be called without a defined validation function, but the modular account MUST enforce these checks and behaviors: +Plugins MAY interact with other plugins and external addresses through the modular account using the functions defined in the `IPluginExecutor` interface. These functions MAY be called without a defined validation function, but the modular account MUST enforce these checks and behaviors: The `executeFromPlugin` function MUST allow plugins to call execution functions installed by plugins on the modular account. Hooks matching the function selector provided in `data` MUST be called. If the calling plugin's manifest did not include the provided function selector within `permittedExecutionSelectors` at the time of installation, execution MUST revert. -The `executeFromPluginExternal` function MUST allow plugins to call external contracts as specified by its parameters on behalf of the modular account. If the calling plugin's manifest did not explicitly allow the external contract call within `permittedExternalCalls` at the time of installation, execution MUST revert. +The `executeFromPluginExternal` function MUST allow plugins to call external addresses as specified by its parameters on behalf of the modular account. If the calling plugin's manifest did not explicitly allow the external call within `permittedExternalCalls` at the time of installation, execution MUST revert. ## Rationale @@ -519,7 +535,7 @@ ERC-4337 compatible accounts must implement the `IAccount` interface, which cons The function routing pattern of ERC-2535 is the logical starting point for achieving this extension into multi-functional accounts. It also meets our other primary design rationale of generalizing execution calls across multiple implementing contracts. However, a strict diamond pattern is constrained by its inability to customize validation schemes for specific execution functions in the context of `validateUserOp`, and its requirement of `delegatecall`. -This proposal includes several interfaces that build on ERC-4337 and are inspired by ERC-2535. First, we standardize a set of modular functions that allow smart contract developers greater flexibility in bundling validation, execution, and hook logic. We also propose interfaces that take inspiration from the diamond standard and provide methods for querying execution functions, validation functions, and hooks on a modular account. The rest of the interfaces describe a plugin's methods for exposing its modular functions and desired configuration, and the modular account's methods for installing and removing plugins and allowing execution across plugins and external contracts. +This proposal includes several interfaces that build on ERC-4337 and are inspired by ERC-2535. First, we standardize a set of modular functions that allow smart contract developers greater flexibility in bundling validation, execution, and hook logic. We also propose interfaces that take inspiration from the diamond standard and provide methods for querying execution functions, validation functions, and hooks on a modular account. The rest of the interfaces describe a plugin's methods for exposing its modular functions and desired configuration, and the modular account's methods for installing and removing plugins and allowing execution across plugins and external addresses. ## Backwards Compatibility diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index 3666f777..a9b23875 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -305,7 +305,7 @@ contract AccountExecHooksTest is OptimizedTest { account.installPlugin({ plugin: address(mockPlugin1), manifestHash: manifestHash1, - pluginInitData: bytes(""), + pluginInstallData: bytes(""), dependencies: new FunctionReference[](0) }); } @@ -336,7 +336,7 @@ contract AccountExecHooksTest is OptimizedTest { account.installPlugin({ plugin: address(mockPlugin2), manifestHash: manifestHash2, - pluginInitData: bytes(""), + pluginInstallData: bytes(""), dependencies: dependencies }); } @@ -364,7 +364,7 @@ contract AccountExecHooksTest is OptimizedTest { account.installPlugin({ plugin: address(mockPlugin2), manifestHash: manifestHash2, - pluginInitData: bytes(""), + pluginInstallData: bytes(""), dependencies: dependencies }); } diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index 808f6d2a..2837b904 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -46,7 +46,7 @@ contract AccountReturnDataTest is OptimizedTest { account.installPlugin({ plugin: address(resultCreatorPlugin), manifestHash: resultCreatorManifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); // Add the result consumer plugin to the account @@ -54,7 +54,7 @@ contract AccountReturnDataTest is OptimizedTest { account.installPlugin({ plugin: address(resultConsumerPlugin), manifestHash: resultConsumerManifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } diff --git a/test/account/ExecuteFromPluginPermissions.t.sol b/test/account/ExecuteFromPluginPermissions.t.sol index 46cdbaef..88ce0c98 100644 --- a/test/account/ExecuteFromPluginPermissions.t.sol +++ b/test/account/ExecuteFromPluginPermissions.t.sol @@ -54,7 +54,7 @@ contract ExecuteFromPluginPermissionsTest is OptimizedTest { account.installPlugin({ plugin: address(resultCreatorPlugin), manifestHash: resultCreatorManifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); // Add the EFP caller plugin to the account @@ -62,7 +62,7 @@ contract ExecuteFromPluginPermissionsTest is OptimizedTest { account.installPlugin({ plugin: address(efpCallerPlugin), manifestHash: efpCallerManifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); @@ -72,7 +72,7 @@ contract ExecuteFromPluginPermissionsTest is OptimizedTest { account.installPlugin({ plugin: address(efpCallerPluginAnyExternal), manifestHash: efpCallerAnyExternalManifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } diff --git a/test/account/ManifestValidity.t.sol b/test/account/ManifestValidity.t.sol index e620bf28..55b2c9ba 100644 --- a/test/account/ManifestValidity.t.sol +++ b/test/account/ManifestValidity.t.sol @@ -50,7 +50,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -67,7 +67,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -84,7 +84,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -99,7 +99,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -114,7 +114,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -130,7 +130,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -146,7 +146,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -161,7 +161,7 @@ contract ManifestValidityTest is OptimizedTest { account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 02a5b3b1..1005cdf2 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -269,7 +269,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, - pluginInitData: abi.encode(uint48(1 days)), + pluginInstallData: abi.encode(uint48(1 days)), dependencies: new FunctionReference[](0) }); @@ -292,7 +292,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(mockPluginWithBadPermittedExec), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -304,7 +304,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: bytes32(0), - pluginInitData: abi.encode(uint48(1 days)), + pluginInstallData: abi.encode(uint48(1 days)), dependencies: new FunctionReference[](0) }); } @@ -319,7 +319,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(badPlugin), manifestHash: bytes32(0), - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); } @@ -331,7 +331,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, - pluginInitData: abi.encode(uint48(1 days)), + pluginInstallData: abi.encode(uint48(1 days)), dependencies: new FunctionReference[](0) }); @@ -343,7 +343,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, - pluginInitData: abi.encode(uint48(1 days)), + pluginInstallData: abi.encode(uint48(1 days)), dependencies: new FunctionReference[](0) }); } @@ -356,7 +356,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); @@ -377,7 +377,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); @@ -402,7 +402,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); @@ -430,7 +430,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { IPluginManager(account2).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index ea8f5bcc..b5f001f7 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -47,19 +47,19 @@ contract ValidationIntersectionTest is OptimizedTest { account1.installPlugin({ plugin: address(noHookPlugin), manifestHash: keccak256(abi.encode(noHookPlugin.pluginManifest())), - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); account1.installPlugin({ plugin: address(oneHookPlugin), manifestHash: keccak256(abi.encode(oneHookPlugin.pluginManifest())), - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); account1.installPlugin({ plugin: address(twoHookPlugin), manifestHash: keccak256(abi.encode(twoHookPlugin.pluginManifest())), - pluginInitData: "", + pluginInstallData: "", dependencies: new FunctionReference[](0) }); vm.stopPrank(); diff --git a/test/mocks/MSCAFactoryFixture.sol b/test/mocks/MSCAFactoryFixture.sol index 03b90736..17be2cc0 100644 --- a/test/mocks/MSCAFactoryFixture.sol +++ b/test/mocks/MSCAFactoryFixture.sol @@ -57,13 +57,13 @@ contract MSCAFactoryFixture is OptimizedTest { plugins[0] = address(singleOwnerPlugin); bytes32[] memory pluginManifestHashes = new bytes32[](1); pluginManifestHashes[0] = keccak256(abi.encode(singleOwnerPlugin.pluginManifest())); - bytes[] memory pluginInitData = new bytes[](1); - pluginInitData[0] = abi.encode(owner); + bytes[] memory pluginInstallData = new bytes[](1); + pluginInstallData[0] = abi.encode(owner); // not necessary to check return addr since next call will fail if so new ERC1967Proxy{salt: getSalt(owner, salt)}(address(accountImplementation), ""); // point proxy to actual implementation and init plugins - UpgradeableModularAccount(payable(addr)).initialize(plugins, pluginManifestHashes, pluginInitData); + UpgradeableModularAccount(payable(addr)).initialize(plugins, pluginManifestHashes, pluginInstallData); } return UpgradeableModularAccount(payable(addr)); From 4338c4db4468e8bfa906f5e21181a5c68daa7fc5 Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Wed, 24 Jan 2024 02:13:12 -0500 Subject: [PATCH 2/4] fix: address comments --- src/account/UpgradeableModularAccount.sol | 2 +- standard/ERCs/erc-6900.md | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index e26db165..c2de3656 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -84,7 +84,7 @@ contract UpgradeableModularAccount is // EXTERNAL FUNCTIONS /// @notice Initializes the account with a set of plugins - /// @dev No dependencies or hooks can be injected with this installation + /// @dev No dependencies may be provided with this installation. /// @param plugins The plugins to install /// @param manifestHashes The manifest hashes of the plugins to install /// @param pluginInstallDatas The plugin install datas of the plugins to install diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index 4c377f30..0302abac 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -1,7 +1,7 @@ --- eip: 6900 title: Modular Smart Contract Accounts and Plugins -description: Interfaces for composable contract accounts optionally supporting upgradeability and introspection +description: Interfaces for composable contract accounts optionally supporting upgradability and introspection author: Adam Egyed (@adamegyed), Fangting Liu (@trinity-0111), Jay Paik (@jaypaik), Yoav Weiss (@yoavw) discussions-to: https://ethereum-magicians.org/t/eip-modular-smart-contract-accounts-and-plugins/13885 status: Draft @@ -435,7 +435,7 @@ struct PluginManifest { #### Responsibilties of `StandardExecutor` and `PluginExecutor` -`StandardExecutor` functions are mainly used for open-ended execution against external addresses. +`StandardExecutor` functions are mainly used for open-ended calls to external addresses. `PluginExecutor` functions are specifically used by plugins to request the account to execute with account's context. Explicit permissions are required for plugins to use `PluginExecutor`. @@ -516,10 +516,10 @@ Additionally, when the modular account natively implements functions in `IPlugin The steps to perform are: -- If the call is not from the `EntryPoint`, then find an associated runtime validation function. If one does not exist, execution MUST revert. The modular account MUST execute all pre runtime validation hooks, then the runtime validation function, with the `call` opcode. All of these functions MUST receive the caller, value, and execution function's calldata as parameters. If any of these functions revert, execution MUST revert. If any pre runtime validation hooks are set to `PRE_HOOK_ALWAYS_DENY`, execution MUST revert. If any runtime validation functions are set to `RUNTIME_VALIDATION_ALWAYS_ALLOW`, the validation function MUST be bypassed. -- If there are pre execution hooks defined for the execution function, execute those hooks with the caller, value, and execution function's calldata as parameters. If any of these hooks returns data, it MUST be preserved until the call to the post execution hook. The operation MUST be done with the `call` opcode. If there are duplicate pre execution hooks applied to the same selector, run the hook only once. If any of these functions revert, execution MUST revert. +- If the call is not from the `EntryPoint`, then find an associated runtime validation function. If one does not exist, execution MUST revert. The modular account MUST execute all pre runtime validation hooks, then the runtime validation function, with the `call` opcode. All of these functions MUST receive the caller, value, and execution function's calldata as parameters. If any of these functions revert, execution MUST revert. If any pre runtime validation hooks are set to `PRE_HOOK_ALWAYS_DENY`, execution MUST revert. If the runtime validation function is set to `RUNTIME_VALIDATION_ALWAYS_ALLOW`, the validation function MUST be bypassed. +- If there are pre execution hooks defined for the execution function, execute those hooks with the caller, value, and execution function's calldata as parameters. If any of these hooks returns data, it MUST be preserved until the call to the post execution hook. The operation MUST be done with the `call` opcode. If there is more than one instance of the same pre execution hook (i.e., the hooks have the same function ID and are from the same plugin) applied to the same selector, run the hook only once. If any of these functions revert, execution MUST revert. - Run the execution function. -- If any post execution hooks are defined, run the functions. If a pre execution hook returned data to the account, that data MUST be passed as a parameter to the associated post execution hook. The operation MUST be done with the `call` opcode. If there are duplicate post execution hooks applied to the same selector, run them once for each unique associated pre execution hook. For post execution hooks without an associated pre execution hook, run the hook only once. If any of these functions revert, execution MUST revert. Notably, for the `uninstallPlugin` native function, the post execution hooks defined for it prior to the uninstall MUST run afterwards. +- If any post execution hooks are defined, run the functions. If a pre execution hook returned data to the account, that data MUST be passed as a parameter to the associated post execution hook. The operation MUST be done with the `call` opcode. If there is more than one instance of the same post execution hook applied to the same selector, run them once for each unique associated pre execution hook. For post execution hooks without an associated pre execution hook, run the hook only once. If any of these functions revert, execution MUST revert. Notably, for the `uninstallPlugin` native function, the post execution hooks defined for it prior to the uninstall MUST run afterwards. #### Calls made from plugins From 3078fc9dffec2ffd5c716a0900b4cbc590a60174 Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Wed, 24 Jan 2024 13:42:10 -0500 Subject: [PATCH 3/4] fix: nits and update duplicate hooks description --- standard/ERCs/erc-6900.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index 0302abac..76869999 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -435,7 +435,7 @@ struct PluginManifest { #### Responsibilties of `StandardExecutor` and `PluginExecutor` -`StandardExecutor` functions are mainly used for open-ended calls to external addresses. +`StandardExecutor` functions are used for open-ended calls to external addresses. `PluginExecutor` functions are specifically used by plugins to request the account to execute with account's context. Explicit permissions are required for plugins to use `PluginExecutor`. @@ -517,9 +517,9 @@ Additionally, when the modular account natively implements functions in `IPlugin The steps to perform are: - If the call is not from the `EntryPoint`, then find an associated runtime validation function. If one does not exist, execution MUST revert. The modular account MUST execute all pre runtime validation hooks, then the runtime validation function, with the `call` opcode. All of these functions MUST receive the caller, value, and execution function's calldata as parameters. If any of these functions revert, execution MUST revert. If any pre runtime validation hooks are set to `PRE_HOOK_ALWAYS_DENY`, execution MUST revert. If the runtime validation function is set to `RUNTIME_VALIDATION_ALWAYS_ALLOW`, the validation function MUST be bypassed. -- If there are pre execution hooks defined for the execution function, execute those hooks with the caller, value, and execution function's calldata as parameters. If any of these hooks returns data, it MUST be preserved until the call to the post execution hook. The operation MUST be done with the `call` opcode. If there is more than one instance of the same pre execution hook (i.e., the hooks have the same function ID and are from the same plugin) applied to the same selector, run the hook only once. If any of these functions revert, execution MUST revert. +- If there are pre execution hooks defined for the execution function, execute those hooks with the caller, value, and execution function's calldata as parameters. If any of these hooks returns data, it MUST be preserved until the call to the post execution hook. The operation MUST be done with the `call` opcode. If there are duplicate pre execution hooks (i.e., hooks with identical `FunctionReference`s), run the hook only once. If any of these functions revert, execution MUST revert. - Run the execution function. -- If any post execution hooks are defined, run the functions. If a pre execution hook returned data to the account, that data MUST be passed as a parameter to the associated post execution hook. The operation MUST be done with the `call` opcode. If there is more than one instance of the same post execution hook applied to the same selector, run them once for each unique associated pre execution hook. For post execution hooks without an associated pre execution hook, run the hook only once. If any of these functions revert, execution MUST revert. Notably, for the `uninstallPlugin` native function, the post execution hooks defined for it prior to the uninstall MUST run afterwards. +- If any post execution hooks are defined, run the functions. If a pre execution hook returned data to the account, that data MUST be passed as a parameter to the associated post execution hook. The operation MUST be done with the `call` opcode. If there are duplicate post execution hooks, run them once for each unique associated pre execution hook. For post execution hooks without an associated pre execution hook, run the hook only once. If any of these functions revert, execution MUST revert. Notably, for the `uninstallPlugin` native function, the post execution hooks defined for it prior to the uninstall MUST run afterwards. #### Calls made from plugins From b05d9ff4aa7c55e2936684019412755787c3c0ca Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Wed, 24 Jan 2024 17:29:37 -0500 Subject: [PATCH 4/4] fix: value comments fix --- src/interfaces/IPlugin.sol | 2 +- src/interfaces/IStandardExecutor.sol | 4 ++-- standard/ERCs/erc-6900.md | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index f2d095fb..cf74fd89 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -14,7 +14,7 @@ enum ManifestAssociatedFunctionType { // Function belongs to an external plugin provided as a dependency during plugin installation. DEPENDENCY, // Resolves to a magic value to always bypass runtime validation for a given function. - // This is only assignable on runtime validation functions. If it were to be used on a user op validationFunction, + // This is only assignable on runtime validation functions. If it were to be used on a user op validation function, // it would risk burning gas from the account. When used as a hook in any hook location, it is equivalent to not // setting a hook and is therefore disallowed. RUNTIME_VALIDATION_ALWAYS_ALLOW, diff --git a/src/interfaces/IStandardExecutor.sol b/src/interfaces/IStandardExecutor.sol index a43da646..a5618be6 100644 --- a/src/interfaces/IStandardExecutor.sol +++ b/src/interfaces/IStandardExecutor.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.19; struct Call { // The target address for the account to call. address target; - // The value sent with the call. + // The value to send with the call. uint256 value; // The calldata for the call. bytes data; @@ -14,7 +14,7 @@ interface IStandardExecutor { /// @notice Standard execute method. /// @dev If the target is a plugin, the call SHOULD revert. /// @param target The target address for the account to call. - /// @param value The value sent with the call. + /// @param value The value to send with the call. /// @param data The calldata for the call. /// @return The return data from the call. function execute(address target, uint256 value, bytes calldata data) external payable returns (bytes memory); diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index 76869999..2af1858c 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -150,7 +150,7 @@ Standard execute functions SHOULD check whether the call's target implements the struct Call { // The target address for the account to call. address target; - // The value sent with the call. + // The value to send with the call. uint256 value; // The calldata for the call. bytes data; @@ -160,7 +160,7 @@ interface IStandardExecutor { /// @notice Standard execute method. /// @dev If the target is a plugin, the call SHOULD revert. /// @param target The target address for account to call. - /// @param value The value sent with the call. + /// @param value The value to send with the call. /// @param data The calldata for the call. /// @return The return data from the call. function execute(address target, uint256 value, bytes calldata data) external payable returns (bytes memory);