diff --git a/src/account/AccountLoupe.sol b/src/account/AccountLoupe.sol index ef679e4e..c537fcdb 100644 --- a/src/account/AccountLoupe.sol +++ b/src/account/AccountLoupe.sol @@ -88,9 +88,4 @@ abstract contract AccountLoupe is IAccountLoupe { { preValidationHooks = getAccountStorage().validationData[validationFunction].preValidationHooks; } - - /// @inheritdoc IAccountLoupe - function getInstalledPlugins() external view override returns (address[] memory pluginAddresses) { - pluginAddresses = getAccountStorage().pluginManifestHashes.keys(); - } } diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index ea2f4551..7fcbee95 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.25; -import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; @@ -42,8 +41,6 @@ struct AccountStorage { // AccountStorageInitializable variables uint8 initialized; bool initializing; - // Plugin metadata storage - EnumerableMap.AddressToUintMap pluginManifestHashes; // Execution functions and their associated functions mapping(bytes4 => SelectorData) selectorData; mapping(PluginEntity validationFunction => ValidationData) validationData; diff --git a/src/account/PluginManager2.sol b/src/account/PluginManager2.sol deleted file mode 100644 index b77ae1bf..00000000 --- a/src/account/PluginManager2.sol +++ /dev/null @@ -1,148 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.25; - -import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; - -import {PluginEntityLib} from "../helpers/PluginEntityLib.sol"; -import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; - -import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; -import {IPlugin} from "../interfaces/IPlugin.sol"; -import {PluginEntity, ValidationConfig} from "../interfaces/IPluginManager.sol"; -import {ValidationData, getAccountStorage, toSetValue} from "./AccountStorage.sol"; - -// Temporary additional functions for a user-controlled install flow for validation functions. -abstract contract PluginManager2 { - using EnumerableSet for EnumerableSet.Bytes32Set; - using ValidationConfigLib for ValidationConfig; - - // Index marking the start of the data for the validation function. - uint8 internal constant _RESERVED_VALIDATION_DATA_INDEX = 255; - uint32 internal constant _SELF_PERMIT_VALIDATION_FUNCTIONID = type(uint32).max; - - error PreValidationAlreadySet(PluginEntity validationFunction, PluginEntity preValidationFunction); - error ValidationAlreadySet(bytes4 selector, PluginEntity validationFunction); - error ValidationNotSet(bytes4 selector, PluginEntity validationFunction); - error PermissionAlreadySet(PluginEntity validationFunction, ExecutionHook hook); - error PreValidationHookLimitExceeded(); - - function _installValidation( - ValidationConfig validationConfig, - bytes4[] memory selectors, - bytes calldata installData, - bytes memory preValidationHooks, - bytes memory permissionHooks - ) internal { - ValidationData storage _validationData = - getAccountStorage().validationData[validationConfig.pluginEntity()]; - - if (preValidationHooks.length > 0) { - (PluginEntity[] memory preValidationFunctions, bytes[] memory initDatas) = - abi.decode(preValidationHooks, (PluginEntity[], bytes[])); - - for (uint256 i = 0; i < preValidationFunctions.length; ++i) { - PluginEntity preValidationFunction = preValidationFunctions[i]; - - _validationData.preValidationHooks.push(preValidationFunction); - - if (initDatas[i].length > 0) { - (address preValidationPlugin,) = PluginEntityLib.unpack(preValidationFunction); - IPlugin(preValidationPlugin).onInstall(initDatas[i]); - } - } - - // Avoid collision between reserved index and actual indices - if (_validationData.preValidationHooks.length > _RESERVED_VALIDATION_DATA_INDEX) { - revert PreValidationHookLimitExceeded(); - } - } - - if (permissionHooks.length > 0) { - (ExecutionHook[] memory permissionFunctions, bytes[] memory initDatas) = - abi.decode(permissionHooks, (ExecutionHook[], bytes[])); - - for (uint256 i = 0; i < permissionFunctions.length; ++i) { - ExecutionHook memory permissionFunction = permissionFunctions[i]; - - if (!_validationData.permissionHooks.add(toSetValue(permissionFunction))) { - revert PermissionAlreadySet(validationConfig.pluginEntity(), permissionFunction); - } - - if (initDatas[i].length > 0) { - (address executionPlugin,) = PluginEntityLib.unpack(permissionFunction.hookFunction); - IPlugin(executionPlugin).onInstall(initDatas[i]); - } - } - } - - for (uint256 i = 0; i < selectors.length; ++i) { - bytes4 selector = selectors[i]; - if (!_validationData.selectors.add(toSetValue(selector))) { - revert ValidationAlreadySet(selector, validationConfig.pluginEntity()); - } - } - - if (validationConfig.entityId() != _SELF_PERMIT_VALIDATION_FUNCTIONID) { - // Only allow global validations and signature validations if they're not direct-call validations. - - _validationData.isGlobal = validationConfig.isGlobal(); - _validationData.isSignatureValidation = validationConfig.isSignatureValidation(); - if (installData.length > 0) { - IPlugin(validationConfig.plugin()).onInstall(installData); - } - } - } - - function _uninstallValidation( - PluginEntity validationFunction, - bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData, - bytes calldata permissionHookUninstallData - ) internal { - ValidationData storage _validationData = getAccountStorage().validationData[validationFunction]; - - _validationData.isGlobal = false; - _validationData.isSignatureValidation = false; - - { - bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[])); - - // Clear pre validation hooks - PluginEntity[] storage preValidationHooks = _validationData.preValidationHooks; - for (uint256 i = 0; i < preValidationHooks.length; ++i) { - PluginEntity preValidationFunction = preValidationHooks[i]; - if (preValidationHookUninstallDatas[0].length > 0) { - (address preValidationPlugin,) = PluginEntityLib.unpack(preValidationFunction); - IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]); - } - } - delete _validationData.preValidationHooks; - } - - { - bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[])); - - // Clear permission hooks - EnumerableSet.Bytes32Set storage permissionHooks = _validationData.permissionHooks; - uint256 permissionHookLen = permissionHooks.length(); - for (uint256 i = 0; i < permissionHookLen; ++i) { - bytes32 permissionHook = permissionHooks.at(0); - permissionHooks.remove(permissionHook); - address permissionHookPlugin = address(uint160(bytes20(permissionHook))); - IPlugin(permissionHookPlugin).onUninstall(permissionHookUninstallDatas[i]); - } - } - - // Clear selectors - uint256 selectorLen = _validationData.selectors.length(); - for (uint256 i = 0; i < selectorLen; ++i) { - bytes32 selectorSetValue = _validationData.selectors.at(0); - _validationData.selectors.remove(selectorSetValue); - } - - if (uninstallData.length > 0) { - (address plugin,) = PluginEntityLib.unpack(validationFunction); - IPlugin(plugin).onUninstall(uninstallData); - } - } -} diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManagerInternals.sol index e473730e..1b8aac96 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManagerInternals.sol @@ -3,34 +3,34 @@ pragma solidity ^0.8.25; import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; -import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {RESERVED_VALIDATION_DATA_INDEX, SELF_PERMIT_VALIDATION_FUNCTIONID} from "../helpers/Constants.sol"; import {KnownSelectors} from "../helpers/KnownSelectors.sol"; import {PluginEntityLib} from "../helpers/PluginEntityLib.sol"; +import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; -import {IPlugin, ManifestExecutionHook, ManifestValidation, PluginManifest} from "../interfaces/IPlugin.sol"; -import {IPluginManager, PluginEntity} from "../interfaces/IPluginManager.sol"; -import {AccountStorage, SelectorData, getAccountStorage, toSetValue} from "./AccountStorage.sol"; +import {IPlugin, ManifestExecutionHook, PluginManifest} from "../interfaces/IPlugin.sol"; +import {IPluginManager, PluginEntity, ValidationConfig} from "../interfaces/IPluginManager.sol"; +import {AccountStorage, SelectorData, ValidationData, getAccountStorage, toSetValue} from "./AccountStorage.sol"; abstract contract PluginManagerInternals is IPluginManager { using EnumerableSet for EnumerableSet.Bytes32Set; - using EnumerableMap for EnumerableMap.AddressToUintMap; using PluginEntityLib for PluginEntity; + using ValidationConfigLib for ValidationConfig; error ArrayLengthMismatch(); error Erc4337FunctionNotAllowed(bytes4 selector); error ExecutionFunctionAlreadySet(bytes4 selector); - error InvalidPluginManifest(); error IPluginFunctionNotAllowed(bytes4 selector); error NativeFunctionNotAllowed(bytes4 selector); - error NullPluginEntity(); error NullPlugin(); - error PluginAlreadyInstalled(address plugin); + error PermissionAlreadySet(PluginEntity validationFunction, ExecutionHook hook); error PluginInstallCallbackFailed(address plugin, bytes revertReason); error PluginInterfaceNotSupported(address plugin); error PluginNotInstalled(address plugin); - error ValidationFunctionAlreadySet(bytes4 selector, PluginEntity validationFunction); + error PreValidationHookLimitExceeded(); + error ValidationAlreadySet(bytes4 selector, PluginEntity validationFunction); // Storage update operations @@ -76,39 +76,35 @@ abstract contract PluginManagerInternals is IPluginManager { _selectorData.allowGlobalValidation = false; } - function _addValidationFunction(address plugin, ManifestValidation memory mv) internal { - AccountStorage storage _storage = getAccountStorage(); - - PluginEntity validationFunction = PluginEntityLib.pack(plugin, mv.entityId); + function _addValidationFunction(ValidationConfig validationConfig, bytes4[] memory selectors) internal { + ValidationData storage _validationData = + getAccountStorage().validationData[validationConfig.pluginEntity()]; - if (mv.isDefault) { - _storage.validationData[validationFunction].isGlobal = true; + if (validationConfig.isGlobal()) { + _validationData.isGlobal = true; } - if (mv.isSignatureValidation) { - _storage.validationData[validationFunction].isSignatureValidation = true; + if (validationConfig.isSignatureValidation()) { + _validationData.isSignatureValidation = true; } // Add the validation function to the selectors. - uint256 length = mv.selectors.length; + uint256 length = selectors.length; for (uint256 i = 0; i < length; ++i) { - bytes4 selector = mv.selectors[i]; - _storage.validationData[validationFunction].selectors.add(toSetValue(selector)); + _validationData.selectors.add(toSetValue(selectors[i])); } } - function _removeValidationFunction(address plugin, ManifestValidation memory mv) internal { - AccountStorage storage _storage = getAccountStorage(); - - PluginEntity validationFunction = PluginEntityLib.pack(plugin, mv.entityId); + function _removeValidationFunction(PluginEntity validationFunction) internal { + ValidationData storage _validationData = getAccountStorage().validationData[validationFunction]; - _storage.validationData[validationFunction].isGlobal = false; - _storage.validationData[validationFunction].isSignatureValidation = false; + _validationData.isGlobal = false; + _validationData.isSignatureValidation = false; // Clear the selectors - while (_storage.validationData[validationFunction].selectors.length() > 0) { - bytes32 selector = _storage.validationData[validationFunction].selectors.at(0); - _storage.validationData[validationFunction].selectors.remove(selector); + uint256 length = _validationData.selectors.length(); + for (uint256 i = 0; i < length; ++i) { + _validationData.selectors.remove(_validationData.selectors.at(0)); } } @@ -138,32 +134,21 @@ abstract contract PluginManagerInternals is IPluginManager { ); } - function _installPlugin(address plugin, bytes32 manifestHash, bytes memory pluginInstallData) internal { + function _installPlugin(address plugin, PluginManifest calldata manifest, bytes memory pluginInstallData) + internal + { AccountStorage storage _storage = getAccountStorage(); if (plugin == address(0)) { revert NullPlugin(); } - // Check if the plugin exists. - if (_storage.pluginManifestHashes.contains(plugin)) { - revert PluginAlreadyInstalled(plugin); - } - + // TODO: do we need this check? Or switch to a non-165 checking function? // Check that the plugin supports the IPlugin interface. if (!ERC165Checker.supportsInterface(plugin, type(IPlugin).interfaceId)) { revert PluginInterfaceNotSupported(plugin); } - // Check manifest hash. - PluginManifest memory manifest = IPlugin(plugin).pluginManifest(); - if (!_isValidPluginManifest(manifest, manifestHash)) { - revert InvalidPluginManifest(); - } - - // Add the plugin metadata to the account - _storage.pluginManifestHashes.set(plugin, uint256(manifestHash)); - // Update components according to the manifest. uint256 length = manifest.executionFunctions.length; for (uint256 i = 0; i < length; ++i) { @@ -173,12 +158,19 @@ abstract contract PluginManagerInternals is IPluginManager { _setExecutionFunction(selector, isPublic, allowGlobalValidation, plugin); } - length = manifest.validationFunctions.length; - for (uint256 i = 0; i < length; ++i) { - // Todo: limit this to only "direct runtime call" validation path (old EFP), - // and add a way for the user to specify permission/pre-val hooks here. - _addValidationFunction(plugin, manifest.validationFunctions[i]); - } + // Install direct call validation, if any, from the manifest + + // Todo: add a way for the user to specify permission/pre-val hooks here. + + ValidationConfig directCallValidation = ValidationConfigLib.pack({ + _plugin: plugin, + _entityId: SELF_PERMIT_VALIDATION_FUNCTIONID, + _isGlobal: manifest.globalDirectCallValidation, + // Direct call validation is never a signature validation + _isSignatureValidation: false + }); + + _addValidationFunction(directCallValidation, manifest.directCallValidationSelectors); length = manifest.executionHooks.length; for (uint256 i = 0; i < length; ++i) { @@ -200,25 +192,14 @@ abstract contract PluginManagerInternals is IPluginManager { revert PluginInstallCallbackFailed(plugin, revertReason); } - emit PluginInstalled(plugin, manifestHash); + emit PluginInstalled(plugin); } - function _uninstallPlugin(address plugin, PluginManifest memory manifest, bytes memory uninstallData) + function _uninstallPlugin(address plugin, PluginManifest calldata manifest, bytes memory uninstallData) internal { AccountStorage storage _storage = getAccountStorage(); - // Check if the plugin exists. - if (!_storage.pluginManifestHashes.contains(plugin)) { - revert PluginNotInstalled(plugin); - } - - // Check manifest hash. - bytes32 manifestHash = bytes32(_storage.pluginManifestHashes.get(plugin)); - if (!_isValidPluginManifest(manifest, manifestHash)) { - revert InvalidPluginManifest(); - } - // Remove components according to the manifest, in reverse order (by component type) of their installation. uint256 length = manifest.executionHooks.length; @@ -229,9 +210,10 @@ abstract contract PluginManagerInternals is IPluginManager { _removeExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook); } - length = manifest.validationFunctions.length; - for (uint256 i = 0; i < length; ++i) { - _removeValidationFunction(plugin, manifest.validationFunctions[i]); + if (manifest.globalDirectCallValidation || manifest.directCallValidationSelectors.length > 0) { + PluginEntity directCallValidation = PluginEntityLib.pack(plugin, SELF_PERMIT_VALIDATION_FUNCTIONID); + + _removeValidationFunction(directCallValidation); } length = manifest.executionFunctions.length; @@ -245,9 +227,6 @@ abstract contract PluginManagerInternals is IPluginManager { _storage.supportedIfaces[manifest.interfaceIds[i]] -= 1; } - // Remove the plugin metadata from the account. - _storage.pluginManifestHashes.remove(plugin); - // Clear the plugin storage for the account. bool onUninstallSuccess = true; // solhint-disable-next-line no-empty-blocks @@ -259,11 +238,122 @@ abstract contract PluginManagerInternals is IPluginManager { emit PluginUninstalled(plugin, onUninstallSuccess); } - function _isValidPluginManifest(PluginManifest memory manifest, bytes32 manifestHash) - internal - pure - returns (bool) - { - return manifestHash == keccak256(abi.encode(manifest)); + function _installValidation( + ValidationConfig validationConfig, + bytes4[] memory selectors, + bytes calldata installData, + bytes memory preValidationHooks, + bytes memory permissionHooks + ) internal { + ValidationData storage _validationData = + getAccountStorage().validationData[validationConfig.pluginEntity()]; + + if (preValidationHooks.length > 0) { + (PluginEntity[] memory preValidationFunctions, bytes[] memory initDatas) = + abi.decode(preValidationHooks, (PluginEntity[], bytes[])); + + for (uint256 i = 0; i < preValidationFunctions.length; ++i) { + PluginEntity preValidationFunction = preValidationFunctions[i]; + + _validationData.preValidationHooks.push(preValidationFunction); + + if (initDatas[i].length > 0) { + (address preValidationPlugin,) = PluginEntityLib.unpack(preValidationFunction); + IPlugin(preValidationPlugin).onInstall(initDatas[i]); + } + } + + // Avoid collision between reserved index and actual indices + if (_validationData.preValidationHooks.length > RESERVED_VALIDATION_DATA_INDEX) { + revert PreValidationHookLimitExceeded(); + } + } + + if (permissionHooks.length > 0) { + (ExecutionHook[] memory permissionFunctions, bytes[] memory initDatas) = + abi.decode(permissionHooks, (ExecutionHook[], bytes[])); + + for (uint256 i = 0; i < permissionFunctions.length; ++i) { + ExecutionHook memory permissionFunction = permissionFunctions[i]; + + if (!_validationData.permissionHooks.add(toSetValue(permissionFunction))) { + revert PermissionAlreadySet(validationConfig.pluginEntity(), permissionFunction); + } + + if (initDatas[i].length > 0) { + (address executionPlugin,) = PluginEntityLib.unpack(permissionFunction.hookFunction); + IPlugin(executionPlugin).onInstall(initDatas[i]); + } + } + } + + for (uint256 i = 0; i < selectors.length; ++i) { + bytes4 selector = selectors[i]; + if (!_validationData.selectors.add(toSetValue(selector))) { + revert ValidationAlreadySet(selector, validationConfig.pluginEntity()); + } + } + + if (validationConfig.entityId() != SELF_PERMIT_VALIDATION_FUNCTIONID) { + // Only allow global validations and signature validations if they're not direct-call validations. + + _validationData.isGlobal = validationConfig.isGlobal(); + _validationData.isSignatureValidation = validationConfig.isSignatureValidation(); + if (installData.length > 0) { + IPlugin(validationConfig.plugin()).onInstall(installData); + } + } + } + + function _uninstallValidation( + PluginEntity validationFunction, + bytes calldata uninstallData, + bytes calldata preValidationHookUninstallData, + bytes calldata permissionHookUninstallData + ) internal { + ValidationData storage _validationData = getAccountStorage().validationData[validationFunction]; + + _removeValidationFunction(validationFunction); + + { + bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[])); + + // Clear pre validation hooks + PluginEntity[] storage preValidationHooks = _validationData.preValidationHooks; + for (uint256 i = 0; i < preValidationHooks.length; ++i) { + PluginEntity preValidationFunction = preValidationHooks[i]; + if (preValidationHookUninstallDatas[0].length > 0) { + (address preValidationPlugin,) = PluginEntityLib.unpack(preValidationFunction); + IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]); + } + } + delete _validationData.preValidationHooks; + } + + { + bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[])); + + // Clear permission hooks + EnumerableSet.Bytes32Set storage permissionHooks = _validationData.permissionHooks; + uint256 permissionHookLen = permissionHooks.length(); + for (uint256 i = 0; i < permissionHookLen; ++i) { + bytes32 permissionHook = permissionHooks.at(0); + permissionHooks.remove(permissionHook); + address permissionHookPlugin = address(uint160(bytes20(permissionHook))); + IPlugin(permissionHookPlugin).onUninstall(permissionHookUninstallDatas[i]); + } + } + + // Clear selectors + uint256 selectorLen = _validationData.selectors.length(); + for (uint256 i = 0; i < selectorLen; ++i) { + bytes32 selectorSetValue = _validationData.selectors.at(0); + _validationData.selectors.remove(selectorSetValue); + } + + if (uninstallData.length > 0) { + (address plugin,) = PluginEntityLib.unpack(validationFunction); + IPlugin(plugin).onUninstall(uninstallData); + } } } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 4f64cdd3..b15718c7 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -18,8 +18,9 @@ import {SparseCalldataSegmentLib} from "../helpers/SparseCalldataSegmentLib.sol" import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationResHelpers.sol"; +import {RESERVED_VALIDATION_DATA_INDEX, SELF_PERMIT_VALIDATION_FUNCTIONID} from "../helpers/Constants.sol"; import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; -import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; +import {PluginManifest} from "../interfaces/IPlugin.sol"; import {IPluginManager, PluginEntity, ValidationConfig} from "../interfaces/IPluginManager.sol"; import {Call, IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; import {IValidation} from "../interfaces/IValidation.sol"; @@ -28,8 +29,6 @@ import {AccountExecutor} from "./AccountExecutor.sol"; import {AccountLoupe} from "./AccountLoupe.sol"; import {AccountStorage, getAccountStorage, toExecutionHook, toSetValue} from "./AccountStorage.sol"; import {AccountStorageInitializable} from "./AccountStorageInitializable.sol"; - -import {PluginManager2} from "./PluginManager2.sol"; import {PluginManagerInternals} from "./PluginManagerInternals.sol"; contract UpgradeableModularAccount is @@ -42,7 +41,6 @@ contract UpgradeableModularAccount is IStandardExecutor, IAccountExecute, PluginManagerInternals, - PluginManager2, UUPSUpgradeable { using EnumerableSet for EnumerableSet.Bytes32Set; @@ -109,21 +107,21 @@ contract UpgradeableModularAccount is /// @notice Initializes the account with a set of plugins /// @param plugins The plugins to install - /// @param manifestHashes The manifest hashes of the plugins to install + /// @param manifests The manifests of the plugins to install /// @param pluginInstallDatas The plugin install datas of the plugins to install function initialize( address[] memory plugins, - bytes32[] memory manifestHashes, + PluginManifest[] calldata manifests, bytes[] memory pluginInstallDatas ) external initializer { uint256 length = plugins.length; - if (length != manifestHashes.length || length != pluginInstallDatas.length) { + if (length != manifests.length || length != pluginInstallDatas.length) { revert ArrayLengthMismatch(); } for (uint256 i = 0; i < length; ++i) { - _installPlugin(plugins[i], manifestHashes[i], pluginInstallDatas[i]); + _installPlugin(plugins[i], manifests[i], pluginInstallDatas[i]); } emit ModularAccountInitialized(_ENTRY_POINT); @@ -249,29 +247,21 @@ contract UpgradeableModularAccount is /// @inheritdoc IPluginManager /// @notice May be validated by a global validation. - function installPlugin(address plugin, bytes32 manifestHash, bytes calldata pluginInstallData) + function installPlugin(address plugin, PluginManifest calldata manifest, bytes calldata pluginInstallData) external override wrapNativeFunction { - _installPlugin(plugin, manifestHash, pluginInstallData); + _installPlugin(plugin, manifest, pluginInstallData); } /// @inheritdoc IPluginManager /// @notice May be validated by a global validation. - function uninstallPlugin(address plugin, bytes calldata config, bytes calldata pluginUninstallData) + function uninstallPlugin(address plugin, PluginManifest calldata manifest, bytes calldata pluginUninstallData) external override wrapNativeFunction { - PluginManifest memory manifest; - - if (config.length > 0) { - manifest = abi.decode(config, (PluginManifest)); - } else { - manifest = IPlugin(plugin).pluginManifest(); - } - _uninstallPlugin(plugin, manifest, pluginUninstallData); } @@ -451,7 +441,7 @@ contract UpgradeableModularAccount is // Run the user op validationFunction { - if (signatureSegment.getIndex() != _RESERVED_VALIDATION_DATA_INDEX) { + if (signatureSegment.getIndex() != RESERVED_VALIDATION_DATA_INDEX) { revert ValidationSignatureSegmentMissing(); } @@ -507,7 +497,7 @@ contract UpgradeableModularAccount is _doPreRuntimeValidationHook(preRuntimeValidationHooks[i], callData, currentAuthData); } - if (authSegment.getIndex() != _RESERVED_VALIDATION_DATA_INDEX) { + if (authSegment.getIndex() != RESERVED_VALIDATION_DATA_INDEX) { revert ValidationSignatureSegmentMissing(); } @@ -645,7 +635,7 @@ contract UpgradeableModularAccount is return (new PostExecToRun[](0), new PostExecToRun[](0)); } - PluginEntity directCallValidationKey = PluginEntityLib.pack(msg.sender, _SELF_PERMIT_VALIDATION_FUNCTIONID); + PluginEntity directCallValidationKey = PluginEntityLib.pack(msg.sender, SELF_PERMIT_VALIDATION_FUNCTIONID); _checkIfValidationAppliesCallData(msg.data, directCallValidationKey, false); diff --git a/src/helpers/Constants.sol b/src/helpers/Constants.sol new file mode 100644 index 00000000..6a9f5209 --- /dev/null +++ b/src/helpers/Constants.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.25; + +// Index marking the start of the data for the validation function. +uint8 constant RESERVED_VALIDATION_DATA_INDEX = 255; + +// Magic value for the Entity ID of direct call validation. +uint32 constant SELF_PERMIT_VALIDATION_FUNCTIONID = type(uint32).max; diff --git a/src/helpers/KnownSelectors.sol b/src/helpers/KnownSelectors.sol index ae91f3b7..46143e52 100644 --- a/src/helpers/KnownSelectors.sol +++ b/src/helpers/KnownSelectors.sol @@ -35,8 +35,7 @@ library KnownSelectors { // check against IAccountLoupe methods || selector == IAccountLoupe.getExecutionFunctionHandler.selector || selector == IAccountLoupe.getSelectors.selector || selector == IAccountLoupe.getExecutionHooks.selector - || selector == IAccountLoupe.getPreValidationHooks.selector - || selector == IAccountLoupe.getInstalledPlugins.selector; + || selector == IAccountLoupe.getPreValidationHooks.selector; } function isErc4337Function(bytes4 selector) internal pure returns (bool) { diff --git a/src/interfaces/IAccountLoupe.sol b/src/interfaces/IAccountLoupe.sol index 3e7d9f11..731e2aac 100644 --- a/src/interfaces/IAccountLoupe.sol +++ b/src/interfaces/IAccountLoupe.sol @@ -40,8 +40,4 @@ interface IAccountLoupe { external view returns (PluginEntity[] memory preValidationHooks); - - /// @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 824a1ddf..62d253e7 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -13,14 +13,6 @@ struct ManifestExecutionFunction { bool allowGlobalValidation; } -// todo: do we need these at all? Or do we fully switch to `installValidation`? -struct ManifestValidation { - uint32 entityId; - bool isDefault; - bool isSignatureValidation; - bytes4[] selectors; -} - struct ManifestExecutionHook { // TODO(erc6900 spec): These fields can be packed into a single word bytes4 executionSelector; @@ -54,7 +46,8 @@ struct PluginMetadata { struct PluginManifest { // Execution functions defined in this plugin to be installed on the MSCA. ManifestExecutionFunction[] executionFunctions; - ManifestValidation[] validationFunctions; + bool globalDirectCallValidation; + bytes4[] directCallValidationSelectors; ManifestExecutionHook[] executionHooks; // List of ERC-165 interface IDs to add to account to support introspection checks. This MUST NOT include // IPlugin's interface ID. diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index d2df72b5..1d360737 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -1,21 +1,24 @@ // SPDX-License-Identifier: CC0-1.0 pragma solidity ^0.8.25; +import {PluginManifest} from "./IPlugin.sol"; + type PluginEntity is bytes24; type ValidationConfig is bytes26; interface IPluginManager { - event PluginInstalled(address indexed plugin, bytes32 manifestHash); + event PluginInstalled(address indexed plugin); 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 manifest the manifest describing functions to install /// @param pluginInstallData Optional data to be decoded and used by the plugin to setup initial plugin data /// for the modular account. - function installPlugin(address plugin, bytes32 manifestHash, bytes calldata pluginInstallData) external; + function installPlugin(address plugin, PluginManifest calldata manifest, bytes calldata pluginInstallData) + external; /// @notice Temporary install function - pending a different user-supplied install config & manifest validation /// path. @@ -53,9 +56,9 @@ interface IPluginManager { /// @notice Uninstall a plugin from the modular account. /// @param plugin The plugin to uninstall. - /// @param config An optional, implementation-specific field that accounts may use to ensure consistency - /// guarantees. + /// @param manifest the manifest describing functions to uninstall. /// @param pluginUninstallData Optional data to be decoded and used by the plugin to clear plugin data for the /// modular account. - function uninstallPlugin(address plugin, bytes calldata config, bytes calldata pluginUninstallData) external; + function uninstallPlugin(address plugin, PluginManifest calldata manifest, bytes calldata pluginUninstallData) + external; } diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index ba3a2098..625fce85 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -14,8 +14,6 @@ import {AccountTestBase} from "../utils/AccountTestBase.sol"; contract AccountExecHooksTest is AccountTestBase { MockPlugin public mockPlugin1; - bytes32 public manifestHash1; - bytes32 public manifestHash2; bytes4 internal constant _EXEC_SELECTOR = bytes4(uint32(1)); uint32 internal constant _PRE_HOOK_FUNCTION_ID_1 = 1; @@ -24,7 +22,7 @@ contract AccountExecHooksTest is AccountTestBase { PluginManifest internal _m1; - event PluginInstalled(address indexed plugin, bytes32 manifestHash); + event PluginInstalled(address indexed plugin); event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); // emitted by MockPlugin event ReceivedCall(bytes msgData, uint256 msgValue); @@ -162,19 +160,19 @@ contract AccountExecHooksTest is AccountTestBase { function _installPlugin1WithHooks(ManifestExecutionHook memory execHooks) internal { _m1.executionHooks.push(execHooks); mockPlugin1 = new MockPlugin(_m1); - manifestHash1 = keccak256(abi.encode(mockPlugin1.pluginManifest())); vm.expectEmit(true, true, true, true); emit ReceivedCall(abi.encodeCall(IPlugin.onInstall, (bytes(""))), 0); vm.expectEmit(true, true, true, true); - emit PluginInstalled(address(mockPlugin1), manifestHash1); + emit PluginInstalled(address(mockPlugin1)); - vm.prank(address(entryPoint)); + vm.startPrank(address(entryPoint)); account1.installPlugin({ plugin: address(mockPlugin1), - manifestHash: manifestHash1, + manifest: mockPlugin1.pluginManifest(), pluginInstallData: bytes("") }); + vm.stopPrank(); } function _uninstallPlugin(MockPlugin plugin) internal { @@ -183,7 +181,8 @@ contract AccountExecHooksTest is AccountTestBase { vm.expectEmit(true, true, true, true); emit PluginUninstalled(address(plugin), true); - vm.prank(address(entryPoint)); - account1.uninstallPlugin(address(plugin), bytes(""), bytes("")); + vm.startPrank(address(entryPoint)); + account1.uninstallPlugin(address(plugin), plugin.pluginManifest(), bytes("")); + vm.stopPrank(); } } diff --git a/test/account/AccountLoupe.t.sol b/test/account/AccountLoupe.t.sol index f0670848..9f837bd8 100644 --- a/test/account/AccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -16,22 +16,18 @@ contract AccountLoupeTest is CustomValidationTestBase { event ReceivedCall(bytes msgData, uint256 msgValue); + PluginEntity public comprehensivePluginValidation; + function setUp() public { comprehensivePlugin = new ComprehensivePlugin(); + comprehensivePluginValidation = + PluginEntityLib.pack(address(comprehensivePlugin), uint32(ComprehensivePlugin.EntityId.VALIDATION)); _customValidationSetup(); - bytes32 manifestHash = keccak256(abi.encode(comprehensivePlugin.pluginManifest())); - vm.prank(address(entryPoint)); - account1.installPlugin(address(comprehensivePlugin), manifestHash, ""); - } - - function test_pluginLoupe_getInstalledPlugins_initial() public { - address[] memory plugins = account1.getInstalledPlugins(); - - assertEq(plugins.length, 1); - - assertEq(plugins[0], address(comprehensivePlugin)); + vm.startPrank(address(entryPoint)); + account1.installPlugin(address(comprehensivePlugin), comprehensivePlugin.pluginManifest(), ""); + vm.stopPrank(); } function test_pluginLoupe_getExecutionFunctionHandler_native() public { @@ -69,9 +65,6 @@ contract AccountLoupeTest is CustomValidationTestBase { } function test_pluginLoupe_getSelectors() public { - PluginEntity comprehensivePluginValidation = - PluginEntityLib.pack(address(comprehensivePlugin), uint32(ComprehensivePlugin.EntityId.VALIDATION)); - bytes4[] memory selectors = account1.getSelectors(comprehensivePluginValidation); assertEq(selectors.length, 1); @@ -115,7 +108,7 @@ contract AccountLoupeTest is CustomValidationTestBase { } function test_pluginLoupe_getValidationHooks() public { - PluginEntity[] memory hooks = account1.getPreValidationHooks(_signerValidation); + PluginEntity[] memory hooks = account1.getPreValidationHooks(comprehensivePluginValidation); assertEq(hooks.length, 2); assertEq( @@ -152,12 +145,15 @@ contract AccountLoupeTest is CustomValidationTestBase { address(comprehensivePlugin), uint32(ComprehensivePlugin.EntityId.PRE_VALIDATION_HOOK_2) ); + bytes4[] memory selectors = new bytes4[](1); + selectors[0] = comprehensivePlugin.foo.selector; + bytes[] memory installDatas = new bytes[](2); return ( - _signerValidation, + comprehensivePluginValidation, true, true, - new bytes4[](0), + selectors, bytes(""), abi.encode(preValidationHooks, installDatas), "" diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index fa8bdf86..d1f8493b 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -26,21 +26,19 @@ contract AccountReturnDataTest is AccountTestBase { resultConsumerPlugin = new ResultConsumerPlugin(resultCreatorPlugin, regularResultContract); // Add the result creator plugin to the account - bytes32 resultCreatorManifestHash = keccak256(abi.encode(resultCreatorPlugin.pluginManifest())); - vm.prank(address(entryPoint)); + vm.startPrank(address(entryPoint)); account1.installPlugin({ plugin: address(resultCreatorPlugin), - manifestHash: resultCreatorManifestHash, + manifest: resultCreatorPlugin.pluginManifest(), pluginInstallData: "" }); // Add the result consumer plugin to the account - bytes32 resultConsumerManifestHash = keccak256(abi.encode(resultConsumerPlugin.pluginManifest())); - vm.prank(address(entryPoint)); account1.installPlugin({ plugin: address(resultConsumerPlugin), - manifestHash: resultConsumerManifestHash, + manifest: resultConsumerPlugin.pluginManifest(), pluginInstallData: "" }); + vm.stopPrank(); } // Tests the ability to read the result of plugin execution functions via the account's fallback diff --git a/test/account/PermittedCallPermissions.t.sol b/test/account/PermittedCallPermissions.t.sol index 607c50b4..b0a96fb2 100644 --- a/test/account/PermittedCallPermissions.t.sol +++ b/test/account/PermittedCallPermissions.t.sol @@ -21,21 +21,19 @@ contract PermittedCallPermissionsTest is AccountTestBase { permittedCallerPlugin = new PermittedCallerPlugin(); // Add the result creator plugin to the account - bytes32 resultCreatorManifestHash = keccak256(abi.encode(resultCreatorPlugin.pluginManifest())); - vm.prank(address(entryPoint)); + vm.startPrank(address(entryPoint)); account1.installPlugin({ plugin: address(resultCreatorPlugin), - manifestHash: resultCreatorManifestHash, + manifest: resultCreatorPlugin.pluginManifest(), pluginInstallData: "" }); // Add the permitted caller plugin to the account - bytes32 permittedCallerManifestHash = keccak256(abi.encode(permittedCallerPlugin.pluginManifest())); - vm.prank(address(entryPoint)); account1.installPlugin({ plugin: address(permittedCallerPlugin), - manifestHash: permittedCallerManifestHash, + manifest: permittedCallerPlugin.pluginManifest(), pluginInstallData: "" }); + vm.stopPrank(); } function test_permittedCall_Allowed() public { diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index 743a0241..b966b734 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -24,12 +24,18 @@ contract SelfCallAuthorizationTest is AccountTestBase { comprehensivePlugin = new ComprehensivePlugin(); - bytes32 manifestHash = keccak256(abi.encode(comprehensivePlugin.pluginManifest())); - vm.prank(address(entryPoint)); - account1.installPlugin(address(comprehensivePlugin), manifestHash, ""); - comprehensivePluginValidation = PluginEntityLib.pack(address(comprehensivePlugin), uint32(ComprehensivePlugin.EntityId.VALIDATION)); + + bytes4[] memory validationSelectors = new bytes4[](1); + validationSelectors[0] = ComprehensivePlugin.foo.selector; + + vm.startPrank(address(entryPoint)); + account1.installPlugin(address(comprehensivePlugin), comprehensivePlugin.pluginManifest(), ""); + account1.installValidation( + ValidationConfigLib.pack(comprehensivePluginValidation, false, false), validationSelectors, "", "", "" + ); + vm.stopPrank(); } function test_selfCallFails_userOp() public { diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 8b77332f..8225b995 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -42,7 +42,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { Counter public counter; PluginManifest internal _manifest; - event PluginInstalled(address indexed plugin, bytes32 manifestHash); + event PluginInstalled(address indexed plugin); event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); event ReceivedCall(bytes msgData, uint256 msgValue); @@ -239,19 +239,17 @@ contract UpgradeableModularAccountTest is AccountTestBase { function test_installPlugin() public { vm.startPrank(address(entryPoint)); - bytes32 manifestHash = keccak256(abi.encode(tokenReceiverPlugin.pluginManifest())); - vm.expectEmit(true, true, true, true); - emit PluginInstalled(address(tokenReceiverPlugin), manifestHash); + emit PluginInstalled(address(tokenReceiverPlugin)); IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), - manifestHash: manifestHash, + manifest: tokenReceiverPlugin.pluginManifest(), pluginInstallData: abi.encode(uint48(1 days)) }); - address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 1); - assertEq(plugins[0], address(tokenReceiverPlugin)); + address handler = + IAccountLoupe(account1).getExecutionFunctionHandler(TokenReceiverPlugin.onERC721Received.selector); + assertEq(handler, address(tokenReceiverPlugin)); } function test_installPlugin_PermittedCallSelectorNotInstalled() public { @@ -260,26 +258,14 @@ contract UpgradeableModularAccountTest is AccountTestBase { PluginManifest memory m; MockPlugin mockPluginWithBadPermittedExec = new MockPlugin(m); - bytes32 manifestHash = keccak256(abi.encode(mockPluginWithBadPermittedExec.pluginManifest())); IPluginManager(account1).installPlugin({ plugin: address(mockPluginWithBadPermittedExec), - manifestHash: manifestHash, + manifest: mockPluginWithBadPermittedExec.pluginManifest(), pluginInstallData: "" }); } - function test_installPlugin_invalidManifest() public { - vm.startPrank(address(entryPoint)); - - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); - IPluginManager(account1).installPlugin({ - plugin: address(tokenReceiverPlugin), - manifestHash: bytes32(0), - pluginInstallData: abi.encode(uint48(1 days)) - }); - } - function test_installPlugin_interfaceNotSupported() public { vm.startPrank(address(entryPoint)); @@ -287,31 +273,32 @@ contract UpgradeableModularAccountTest is AccountTestBase { vm.expectRevert( abi.encodeWithSelector(PluginManagerInternals.PluginInterfaceNotSupported.selector, address(badPlugin)) ); - IPluginManager(account1).installPlugin({ - plugin: address(badPlugin), - manifestHash: bytes32(0), - pluginInstallData: "" - }); + + PluginManifest memory m; + + IPluginManager(account1).installPlugin({plugin: address(badPlugin), manifest: m, pluginInstallData: ""}); } function test_installPlugin_alreadyInstalled() public { - vm.startPrank(address(entryPoint)); + PluginManifest memory m = tokenReceiverPlugin.pluginManifest(); - bytes32 manifestHash = keccak256(abi.encode(tokenReceiverPlugin.pluginManifest())); + vm.prank(address(entryPoint)); IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), - manifestHash: manifestHash, + manifest: m, pluginInstallData: abi.encode(uint48(1 days)) }); + vm.prank(address(entryPoint)); vm.expectRevert( abi.encodeWithSelector( - PluginManagerInternals.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin) + PluginManagerInternals.ExecutionFunctionAlreadySet.selector, + TokenReceiverPlugin.onERC721Received.selector ) ); IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), - manifestHash: manifestHash, + manifest: m, pluginInstallData: abi.encode(uint48(1 days)) }); } @@ -320,29 +307,9 @@ contract UpgradeableModularAccountTest is AccountTestBase { vm.startPrank(address(entryPoint)); ComprehensivePlugin plugin = new ComprehensivePlugin(); - bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - IPluginManager(account1).installPlugin({ - plugin: address(plugin), - manifestHash: manifestHash, - pluginInstallData: "" - }); - - vm.expectEmit(true, true, true, true); - emit PluginUninstalled(address(plugin), true); - IPluginManager(account1).uninstallPlugin({plugin: address(plugin), config: "", pluginUninstallData: ""}); - address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 0); - } - - function test_uninstallPlugin_manifestParameter() public { - vm.startPrank(address(entryPoint)); - - ComprehensivePlugin plugin = new ComprehensivePlugin(); - bytes memory serializedManifest = abi.encode(plugin.pluginManifest()); - bytes32 manifestHash = keccak256(serializedManifest); IPluginManager(account1).installPlugin({ plugin: address(plugin), - manifestHash: manifestHash, + manifest: plugin.pluginManifest(), pluginInstallData: "" }); @@ -350,48 +317,22 @@ contract UpgradeableModularAccountTest is AccountTestBase { emit PluginUninstalled(address(plugin), true); IPluginManager(account1).uninstallPlugin({ plugin: address(plugin), - config: serializedManifest, + manifest: plugin.pluginManifest(), pluginUninstallData: "" }); - address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 0); - } - - function test_uninstallPlugin_invalidManifestFails() public { - vm.startPrank(address(entryPoint)); - - ComprehensivePlugin plugin = new ComprehensivePlugin(); - bytes memory serializedManifest = abi.encode(plugin.pluginManifest()); - bytes32 manifestHash = keccak256(serializedManifest); - IPluginManager(account1).installPlugin({ - plugin: address(plugin), - manifestHash: manifestHash, - pluginInstallData: "" - }); - // Attempt to uninstall with a blank _manifest - PluginManifest memory blankManifest; - - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); - IPluginManager(account1).uninstallPlugin({ - plugin: address(plugin), - config: abi.encode(blankManifest), - pluginUninstallData: "" - }); - address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); - assertEq(plugins.length, 1); - assertEq(plugins[0], address(plugin)); + address handler = IAccountLoupe(account1).getExecutionFunctionHandler(plugin.foo.selector); + assertEq(handler, address(0)); } function _installPluginWithExecHooks() internal returns (MockPlugin plugin) { vm.startPrank(address(entryPoint)); plugin = new MockPlugin(_manifest); - bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); IPluginManager(account1).installPlugin({ plugin: address(plugin), - manifestHash: manifestHash, + manifest: plugin.pluginManifest(), pluginInstallData: "" }); diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 3bdaa1b6..eed74dc0 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -46,19 +46,21 @@ contract ValidationIntersectionTest is AccountTestBase { entityId: uint32(MockBaseUserOpValidationPlugin.EntityId.USER_OP_VALIDATION) }); + bytes4[] memory validationSelectors = new bytes4[](1); + validationSelectors[0] = MockUserOpValidationPlugin.foo.selector; + vm.startPrank(address(entryPoint)); - account1.installPlugin({ - plugin: address(noHookPlugin), - manifestHash: keccak256(abi.encode(noHookPlugin.pluginManifest())), - pluginInstallData: "" - }); - account1.installPlugin({ - plugin: address(oneHookPlugin), - manifestHash: keccak256(abi.encode(oneHookPlugin.pluginManifest())), - pluginInstallData: "" - }); - // TODO: change with new install flow - // temporary fix to add the pre-validation hook + // Install noHookValidation + account1.installValidation( + ValidationConfigLib.pack(noHookValidation, true, true), + validationSelectors, + bytes(""), + bytes(""), + bytes("") + ); + + // Install oneHookValidation + validationSelectors[0] = MockUserOpValidation1HookPlugin.bar.selector; PluginEntity[] memory preValidationHooks = new PluginEntity[](1); preValidationHooks[0] = PluginEntityLib.pack({ addr: address(oneHookPlugin), @@ -67,17 +69,14 @@ contract ValidationIntersectionTest is AccountTestBase { bytes[] memory installDatas = new bytes[](1); account1.installValidation( ValidationConfigLib.pack(oneHookValidation, true, true), - new bytes4[](0), + validationSelectors, bytes(""), abi.encode(preValidationHooks, installDatas), bytes("") ); - account1.installPlugin({ - plugin: address(twoHookPlugin), - manifestHash: keccak256(abi.encode(twoHookPlugin.pluginManifest())), - pluginInstallData: "" - }); - // temporary fix to add the pre-validation hook + + // Install twoHookValidation + validationSelectors[0] = MockUserOpValidation2HookPlugin.baz.selector; preValidationHooks = new PluginEntity[](2); preValidationHooks[0] = PluginEntityLib.pack({ addr: address(twoHookPlugin), @@ -90,7 +89,7 @@ contract ValidationIntersectionTest is AccountTestBase { installDatas = new bytes[](2); account1.installValidation( ValidationConfigLib.pack(twoHookValidation, true, true), - new bytes4[](0), + validationSelectors, bytes(""), abi.encode(preValidationHooks, installDatas), bytes("") diff --git a/test/mocks/plugins/ComprehensivePlugin.sol b/test/mocks/plugins/ComprehensivePlugin.sol index e4233cf2..f2689279 100644 --- a/test/mocks/plugins/ComprehensivePlugin.sol +++ b/test/mocks/plugins/ComprehensivePlugin.sol @@ -7,7 +7,6 @@ import {IExecutionHook} from "../../../src/interfaces/IExecutionHook.sol"; import { ManifestExecutionFunction, ManifestExecutionHook, - ManifestValidation, PluginManifest, PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; @@ -140,17 +139,6 @@ contract ComprehensivePlugin is IValidation, IValidationHook, IExecutionHook, Ba allowGlobalValidation: false }); - bytes4[] memory validationSelectors = new bytes4[](1); - validationSelectors[0] = this.foo.selector; - - manifest.validationFunctions = new ManifestValidation[](1); - manifest.validationFunctions[0] = ManifestValidation({ - entityId: uint32(EntityId.VALIDATION), - isDefault: true, - isSignatureValidation: false, - selectors: validationSelectors - }); - manifest.executionHooks = new ManifestExecutionHook[](3); manifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: this.foo.selector, diff --git a/test/mocks/plugins/ReturnDataPluginMocks.sol b/test/mocks/plugins/ReturnDataPluginMocks.sol index 96a77ffe..43e0dfa4 100644 --- a/test/mocks/plugins/ReturnDataPluginMocks.sol +++ b/test/mocks/plugins/ReturnDataPluginMocks.sol @@ -3,12 +3,9 @@ pragma solidity ^0.8.19; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; -import { - ManifestExecutionFunction, - ManifestValidation, - PluginManifest, - PluginMetadata -} from "../../../src/interfaces/IPlugin.sol"; +import {ManifestExecutionFunction, PluginManifest, PluginMetadata} from "../../../src/interfaces/IPlugin.sol"; + +import {SELF_PERMIT_VALIDATION_FUNCTIONID} from "../../../src/helpers/Constants.sol"; import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {IValidation} from "../../../src/interfaces/IValidation.sol"; @@ -103,7 +100,8 @@ contract ResultConsumerPlugin is BasePlugin, IValidation { // This result should be allowed based on the manifest permission request bytes memory returnData = IStandardExecutor(msg.sender).executeWithAuthorization( abi.encodeCall(IStandardExecutor.execute, (target, 0, abi.encodeCall(RegularResultContract.foo, ()))), - abi.encodePacked(this, uint32(0), uint8(0), uint32(1), uint8(255)) // Validation function of self, + abi.encodePacked(this, SELF_PERMIT_VALIDATION_FUNCTIONID, uint8(0), uint32(1), uint8(255)) // Validation + // function of self, // selector-associated, with no auth data ); @@ -119,17 +117,8 @@ contract ResultConsumerPlugin is BasePlugin, IValidation { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - // todo: this is the exact workflow that would benefit from a "permiteed call" setup in the manifest. - bytes4[] memory validationSelectors = new bytes4[](1); - validationSelectors[0] = IStandardExecutor.execute.selector; - - manifest.validationFunctions = new ManifestValidation[](1); - manifest.validationFunctions[0] = ManifestValidation({ - entityId: 0, - isDefault: true, - isSignatureValidation: false, - selectors: validationSelectors - }); + manifest.directCallValidationSelectors = new bytes4[](1); + manifest.directCallValidationSelectors[0] = IStandardExecutor.execute.selector; manifest.executionFunctions = new ManifestExecutionFunction[](2); manifest.executionFunctions[0] = ManifestExecutionFunction({ diff --git a/test/mocks/plugins/ValidationPluginMocks.sol b/test/mocks/plugins/ValidationPluginMocks.sol index 3cfe1d09..b3532581 100644 --- a/test/mocks/plugins/ValidationPluginMocks.sol +++ b/test/mocks/plugins/ValidationPluginMocks.sol @@ -3,12 +3,7 @@ pragma solidity ^0.8.19; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; -import { - ManifestExecutionFunction, - ManifestValidation, - PluginManifest, - PluginMetadata -} from "../../../src/interfaces/IPlugin.sol"; +import {ManifestExecutionFunction, PluginManifest, PluginMetadata} from "../../../src/interfaces/IPlugin.sol"; import {IValidation} from "../../../src/interfaces/IValidation.sol"; import {IValidationHook} from "../../../src/interfaces/IValidationHook.sol"; import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; @@ -112,17 +107,6 @@ contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { allowGlobalValidation: false }); - bytes4[] memory validationSelectors = new bytes4[](1); - validationSelectors[0] = this.foo.selector; - - manifest.validationFunctions = new ManifestValidation[](1); - manifest.validationFunctions[0] = ManifestValidation({ - entityId: uint32(EntityId.USER_OP_VALIDATION), - isDefault: false, - isSignatureValidation: false, - selectors: validationSelectors - }); - return manifest; } } @@ -155,17 +139,6 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { allowGlobalValidation: false }); - bytes4[] memory validationSelectors = new bytes4[](1); - validationSelectors[0] = this.bar.selector; - - manifest.validationFunctions = new ManifestValidation[](2); - manifest.validationFunctions[0] = ManifestValidation({ - entityId: uint32(EntityId.USER_OP_VALIDATION), - isDefault: false, - isSignatureValidation: false, - selectors: validationSelectors - }); - return manifest; } } @@ -201,17 +174,6 @@ contract MockUserOpValidation2HookPlugin is MockBaseUserOpValidationPlugin { allowGlobalValidation: false }); - bytes4[] memory validationSelectors = new bytes4[](1); - validationSelectors[0] = this.baz.selector; - - manifest.validationFunctions = new ManifestValidation[](1); - manifest.validationFunctions[0] = ManifestValidation({ - entityId: uint32(EntityId.USER_OP_VALIDATION), - isDefault: false, - isSignatureValidation: false, - selectors: validationSelectors - }); - return manifest; } } diff --git a/test/plugin/TokenReceiverPlugin.t.sol b/test/plugin/TokenReceiverPlugin.t.sol index 32fa2a9a..15a4da5f 100644 --- a/test/plugin/TokenReceiverPlugin.t.sol +++ b/test/plugin/TokenReceiverPlugin.t.sol @@ -55,10 +55,9 @@ contract TokenReceiverPluginTest is OptimizedTest, IERC1155Receiver { } function _initPlugin() internal { - bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - - vm.prank(address(entryPoint)); - acct.installPlugin(address(plugin), manifestHash, ""); + vm.startPrank(address(entryPoint)); + acct.installPlugin(address(plugin), plugin.pluginManifest(), ""); + vm.stopPrank(); } function test_failERC721Transfer() public {