diff --git a/src/account/AccountLoupe.sol b/src/account/AccountLoupe.sol index 32721550..d652f45c 100644 --- a/src/account/AccountLoupe.sol +++ b/src/account/AccountLoupe.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.25; import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {IAccountLoupe, ExecutionHook} from "../interfaces/IAccountLoupe.sol"; import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; @@ -11,7 +12,7 @@ import {getAccountStorage, toExecutionHook, toSelector} from "./AccountStorage.s abstract contract AccountLoupe is IAccountLoupe { using EnumerableSet for EnumerableSet.Bytes32Set; - using EnumerableSet for EnumerableSet.AddressSet; + using EnumerableMap for EnumerableMap.AddressToUintMap; /// @inheritdoc IAccountLoupe function getExecutionFunctionHandler(bytes4 selector) external view override returns (address plugin) { @@ -89,6 +90,6 @@ abstract contract AccountLoupe is IAccountLoupe { /// @inheritdoc IAccountLoupe function getInstalledPlugins() external view override returns (address[] memory pluginAddresses) { - pluginAddresses = getAccountStorage().plugins.values(); + pluginAddresses = getAccountStorage().pluginManifestHashes.keys(); } } diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 4f90169f..a7645a42 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.25; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; import {FunctionReference} from "../interfaces/IPluginManager.sol"; @@ -9,13 +10,6 @@ import {FunctionReference} from "../interfaces/IPluginManager.sol"; // bytes = keccak256("ERC6900.UpgradeableModularAccount.Storage") bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c401499164f368baef687948c315d9073e40; -struct PluginData { - bytes32 manifestHash; - FunctionReference[] dependencies; - // Tracks the number of times this plugin has been used as a dependency function - uint256 dependentCount; -} - // Represents data associated with a specifc function selector. struct SelectorData { // The plugin that implements this execution function. @@ -51,8 +45,7 @@ struct AccountStorage { uint8 initialized; bool initializing; // Plugin metadata storage - EnumerableSet.AddressSet plugins; - mapping(address => PluginData) pluginData; + EnumerableMap.AddressToUintMap pluginManifestHashes; // Execution functions and their associated functions mapping(bytes4 => SelectorData) selectorData; mapping(FunctionReference validationFunction => ValidationData) validationData; diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManagerInternals.sol index 42868585..40bb4971 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManagerInternals.sol @@ -3,16 +3,10 @@ pragma solidity ^0.8.25; import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; -import { - IPlugin, - ManifestExecutionHook, - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, - PluginManifest -} from "../interfaces/IPlugin.sol"; +import {IPlugin, ManifestExecutionHook, ManifestValidation, PluginManifest} from "../interfaces/IPlugin.sol"; import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; import {KnownSelectors} from "../helpers/KnownSelectors.sol"; @@ -20,45 +14,27 @@ import {AccountStorage, getAccountStorage, SelectorData, toSetValue} from "./Acc abstract contract PluginManagerInternals is IPluginManager { using EnumerableSet for EnumerableSet.Bytes32Set; - using EnumerableSet for EnumerableSet.AddressSet; + using EnumerableMap for EnumerableMap.AddressToUintMap; using FunctionReferenceLib for FunctionReference; error ArrayLengthMismatch(); error Erc4337FunctionNotAllowed(bytes4 selector); error ExecutionFunctionAlreadySet(bytes4 selector); - error InvalidDependenciesProvided(); error InvalidPluginManifest(); error IPluginFunctionNotAllowed(bytes4 selector); - error MissingPluginDependency(address dependency); error NativeFunctionNotAllowed(bytes4 selector); error NullFunctionReference(); error NullPlugin(); error PluginAlreadyInstalled(address plugin); - error PluginDependencyViolation(address plugin); error PluginInstallCallbackFailed(address plugin, bytes revertReason); error PluginInterfaceNotSupported(address plugin); error PluginNotInstalled(address plugin); error ValidationFunctionAlreadySet(bytes4 selector, FunctionReference validationFunction); - modifier notNullFunction(FunctionReference functionReference) { - if (functionReference.isEmpty()) { - revert NullFunctionReference(); - } - _; - } - - modifier notNullPlugin(address plugin) { - if (plugin == address(0)) { - revert NullPlugin(); - } - _; - } - // Storage update operations function _setExecutionFunction(bytes4 selector, bool isPublic, bool allowDefaultValidation, address plugin) internal - notNullPlugin(plugin) { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; @@ -99,25 +75,40 @@ abstract contract PluginManagerInternals is IPluginManager { _selectorData.allowDefaultValidation = false; } - function _addValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { - // Fail on duplicate validation functions. Otherwise, dependency validation functions could shadow - // non-depdency validation functions. Then, if a either plugin is uninstalled, it would cause a partial - // uninstall of the other. - if (!getAccountStorage().validationData[validationFunction].selectors.add(toSetValue(selector))) { - revert ValidationFunctionAlreadySet(selector, validationFunction); + function _addValidationFunction(address plugin, ManifestValidation memory mv) internal { + AccountStorage storage _storage = getAccountStorage(); + + FunctionReference validationFunction = FunctionReferenceLib.pack(plugin, mv.functionId); + + if (mv.isDefault) { + _storage.validationData[validationFunction].isDefault = true; + } + + if (mv.isSignatureValidation) { + _storage.validationData[validationFunction].isSignatureValidation = true; + } + + // Add the validation function to the selectors. + uint256 length = mv.selectors.length; + for (uint256 i = 0; i < length; ++i) { + bytes4 selector = mv.selectors[i]; + _storage.validationData[validationFunction].selectors.add(toSetValue(selector)); } } - function _removeValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { - // May ignore return value, as the manifest hash is validated to ensure that the validation function - // exists. - getAccountStorage().validationData[validationFunction].selectors.remove(toSetValue(selector)); + function _removeValidationFunction(address plugin, ManifestValidation memory mv) internal { + AccountStorage storage _storage = getAccountStorage(); + + FunctionReference validationFunction = FunctionReferenceLib.pack(plugin, mv.functionId); + + _storage.validationData[validationFunction].isDefault = false; + _storage.validationData[validationFunction].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); + } } function _addExecHooks( @@ -146,16 +137,15 @@ abstract contract PluginManagerInternals is IPluginManager { ); } - function _installPlugin( - address plugin, - bytes32 manifestHash, - bytes memory pluginInstallData, - FunctionReference[] memory dependencies - ) internal { + function _installPlugin(address plugin, bytes32 manifestHash, bytes memory pluginInstallData) internal { AccountStorage storage _storage = getAccountStorage(); + if (plugin == address(0)) { + revert NullPlugin(); + } + // Check if the plugin exists. - if (!_storage.plugins.add(plugin)) { + if (_storage.pluginManifestHashes.contains(plugin)) { revert PluginAlreadyInstalled(plugin); } @@ -170,37 +160,11 @@ abstract contract PluginManagerInternals is IPluginManager { revert InvalidPluginManifest(); } - // Check that the dependencies match the manifest. - if (dependencies.length != manifest.dependencyInterfaceIds.length) { - revert InvalidDependenciesProvided(); - } - - uint256 length = dependencies.length; - for (uint256 i = 0; i < length; ++i) { - // Check the dependency interface id over the address of the dependency. - (address dependencyAddr,) = dependencies[i].unpack(); - - // Check that the dependency is installed. - if (_storage.pluginData[dependencyAddr].manifestHash == bytes32(0)) { - revert MissingPluginDependency(dependencyAddr); - } - - // Check that the dependency supports the expected interface. - if (!ERC165Checker.supportsInterface(dependencyAddr, manifest.dependencyInterfaceIds[i])) { - revert InvalidDependenciesProvided(); - } - - // Increment the dependency's dependents counter. - _storage.pluginData[dependencyAddr].dependentCount += 1; - } - // Add the plugin metadata to the account - _storage.pluginData[plugin].manifestHash = manifestHash; - _storage.pluginData[plugin].dependencies = dependencies; + _storage.pluginManifestHashes.set(plugin, uint256(manifestHash)); // Update components according to the manifest. - - length = manifest.executionFunctions.length; + uint256 length = manifest.executionFunctions.length; for (uint256 i = 0; i < length; ++i) { bytes4 selector = manifest.executionFunctions[i].executionSelector; bool isPublic = manifest.executionFunctions[i].isPublic; @@ -210,17 +174,9 @@ abstract contract PluginManagerInternals is IPluginManager { length = manifest.validationFunctions.length; for (uint256 i = 0; i < length; ++i) { - ManifestAssociatedFunction memory mv = manifest.validationFunctions[i]; - _addValidationFunction( - mv.executionSelector, _resolveManifestFunction(mv.associatedFunction, plugin, dependencies) - ); - } - - length = manifest.signatureValidationFunctions.length; - for (uint256 i = 0; i < length; ++i) { - FunctionReference signatureValidationFunction = - FunctionReferenceLib.pack(plugin, manifest.signatureValidationFunctions[i]); - _storage.validationData[signatureValidationFunction].isSignatureValidation = true; + // 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]); } length = manifest.executionHooks.length; @@ -243,7 +199,7 @@ abstract contract PluginManagerInternals is IPluginManager { revert PluginInstallCallbackFailed(plugin, revertReason); } - emit PluginInstalled(plugin, manifestHash, dependencies); + emit PluginInstalled(plugin, manifestHash); } function _uninstallPlugin(address plugin, PluginManifest memory manifest, bytes memory uninstallData) @@ -252,34 +208,19 @@ abstract contract PluginManagerInternals is IPluginManager { AccountStorage storage _storage = getAccountStorage(); // Check if the plugin exists. - if (!_storage.plugins.remove(plugin)) { + if (!_storage.pluginManifestHashes.contains(plugin)) { revert PluginNotInstalled(plugin); } // Check manifest hash. - bytes32 manifestHash = _storage.pluginData[plugin].manifestHash; + bytes32 manifestHash = bytes32(_storage.pluginManifestHashes.get(plugin)); if (!_isValidPluginManifest(manifest, manifestHash)) { revert InvalidPluginManifest(); } - // Ensure that there are no dependent plugins. - if (_storage.pluginData[plugin].dependentCount != 0) { - revert PluginDependencyViolation(plugin); - } - - // Remove this plugin as a dependent from its dependencies. - FunctionReference[] memory dependencies = _storage.pluginData[plugin].dependencies; - uint256 length = dependencies.length; - for (uint256 i = 0; i < length; ++i) { - FunctionReference dependency = dependencies[i]; - (address dependencyAddr,) = dependency.unpack(); - - // Decrement the dependent count for the dependency function. - _storage.pluginData[dependencyAddr].dependentCount -= 1; - } - // Remove components according to the manifest, in reverse order (by component type) of their installation. - length = manifest.executionHooks.length; + + uint256 length = manifest.executionHooks.length; for (uint256 i = 0; i < length; ++i) { ManifestExecutionHook memory mh = manifest.executionHooks[i]; FunctionReference hookFunction = FunctionReferenceLib.pack(plugin, mh.functionId); @@ -287,19 +228,9 @@ abstract contract PluginManagerInternals is IPluginManager { _removeExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook); } - length = manifest.signatureValidationFunctions.length; - for (uint256 i = 0; i < length; ++i) { - FunctionReference signatureValidationFunction = - FunctionReferenceLib.pack(plugin, manifest.signatureValidationFunctions[i]); - _storage.validationData[signatureValidationFunction].isSignatureValidation = false; - } - length = manifest.validationFunctions.length; for (uint256 i = 0; i < length; ++i) { - ManifestAssociatedFunction memory mv = manifest.validationFunctions[i]; - _removeValidationFunction( - mv.executionSelector, _resolveManifestFunction(mv.associatedFunction, plugin, dependencies) - ); + _removeValidationFunction(plugin, manifest.validationFunctions[i]); } length = manifest.executionFunctions.length; @@ -314,7 +245,7 @@ abstract contract PluginManagerInternals is IPluginManager { } // Remove the plugin metadata from the account. - delete _storage.pluginData[plugin]; + _storage.pluginManifestHashes.remove(plugin); // Clear the plugin storage for the account. bool onUninstallSuccess = true; @@ -334,20 +265,4 @@ abstract contract PluginManagerInternals is IPluginManager { { return manifestHash == keccak256(abi.encode(manifest)); } - - function _resolveManifestFunction( - ManifestFunction memory manifestFunction, - address plugin, - FunctionReference[] memory dependencies - ) internal pure returns (FunctionReference) { - if (manifestFunction.functionType == ManifestAssociatedFunctionType.SELF) { - return FunctionReferenceLib.pack(plugin, manifestFunction.functionId); - } else if (manifestFunction.functionType == ManifestAssociatedFunctionType.DEPENDENCY) { - if (manifestFunction.dependencyIndex >= dependencies.length) { - revert InvalidPluginManifest(); - } - return dependencies[manifestFunction.dependencyIndex]; - } - return FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; // Empty checks are done elsewhere - } } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index aa6dfe19..7e59544d 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -102,7 +102,6 @@ contract UpgradeableModularAccount is // EXTERNAL FUNCTIONS /// @notice Initializes the account with a set of plugins - /// @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 @@ -117,10 +116,8 @@ contract UpgradeableModularAccount is revert ArrayLengthMismatch(); } - FunctionReference[] memory emptyDependencies = new FunctionReference[](0); - for (uint256 i = 0; i < length; ++i) { - _installPlugin(plugins[i], manifestHashes[i], pluginInstallDatas[i], emptyDependencies); + _installPlugin(plugins[i], manifestHashes[i], pluginInstallDatas[i]); } emit ModularAccountInitialized(_ENTRY_POINT); @@ -246,13 +243,12 @@ contract UpgradeableModularAccount is /// @inheritdoc IPluginManager /// @notice May be validated by a default validation. - function installPlugin( - address plugin, - bytes32 manifestHash, - bytes calldata pluginInstallData, - FunctionReference[] calldata dependencies - ) external override wrapNativeFunction { - _installPlugin(plugin, manifestHash, pluginInstallData, dependencies); + function installPlugin(address plugin, bytes32 manifestHash, bytes calldata pluginInstallData) + external + override + wrapNativeFunction + { + _installPlugin(plugin, manifestHash, pluginInstallData); } /// @inheritdoc IPluginManager diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index 7259d748..c7ffabde 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -3,19 +3,6 @@ pragma solidity ^0.8.25; import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; -// Forge formatter will displace the first comment for the enum field out of the enum itself, -// so annotating here to prevent that. -// forgefmt: disable-start -enum ManifestAssociatedFunctionType { - // Function is not defined. - NONE, - // Function belongs to this plugin. - SELF, - // Function belongs to an external plugin provided as a dependency during plugin installation. - DEPENDENCY -} -// forgefmt: disable-end - struct ManifestExecutionFunction { // TODO(erc6900 spec): These fields can be packed into a single word // The selector to install @@ -26,17 +13,12 @@ struct ManifestExecutionFunction { bool allowDefaultValidation; } -/// @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; +// todo: do we need these at all? Or do we fully switch to `installValidation`? +struct ManifestValidation { uint8 functionId; - uint256 dependencyIndex; -} - -struct ManifestAssociatedFunction { - bytes4 executionSelector; - ManifestFunction associatedFunction; + bool isDefault; + bool isSignatureValidation; + bytes4[] selectors; } struct ManifestExecutionHook { @@ -72,16 +54,11 @@ struct PluginMetadata { struct PluginManifest { // Execution functions defined in this plugin to be installed on the MSCA. ManifestExecutionFunction[] executionFunctions; - ManifestAssociatedFunction[] validationFunctions; + ManifestValidation[] validationFunctions; ManifestExecutionHook[] executionHooks; - uint8[] signatureValidationFunctions; // 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, 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; } interface IPlugin is IERC165 { diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index d98badbf..dd035b81 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.25; type FunctionReference is bytes21; interface IPluginManager { - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded); @@ -13,14 +13,7 @@ interface IPluginManager { /// @param manifestHash The hash of the plugin 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 pluginInstallData, - FunctionReference[] calldata dependencies - ) external; + function installPlugin(address plugin, bytes32 manifestHash, bytes calldata pluginInstallData) external; /// @notice Temporary install function - pending a different user-supplied install config & manifest validation /// path. diff --git a/src/plugins/owner/ISingleOwnerPlugin.sol b/src/plugins/owner/ISingleOwnerPlugin.sol index 57bcac80..c8f0df43 100644 --- a/src/plugins/owner/ISingleOwnerPlugin.sol +++ b/src/plugins/owner/ISingleOwnerPlugin.sol @@ -5,8 +5,7 @@ import {IValidation} from "../../interfaces/IValidation.sol"; interface ISingleOwnerPlugin is IValidation { enum FunctionId { - VALIDATION_OWNER, - SIG_VALIDATION + VALIDATION_OWNER } /// @notice This event is emitted when ownership of the account changes. diff --git a/src/plugins/owner/SingleOwnerPlugin.sol b/src/plugins/owner/SingleOwnerPlugin.sol index dbdd41b2..e335708b 100644 --- a/src/plugins/owner/SingleOwnerPlugin.sol +++ b/src/plugins/owner/SingleOwnerPlugin.sol @@ -9,12 +9,7 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; import {IPluginManager} from "../../interfaces/IPluginManager.sol"; import { - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, - PluginManifest, - PluginMetadata, - SelectorPermission + PluginManifest, ManifestValidation, PluginMetadata, SelectorPermission } from "../../interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../interfaces/IStandardExecutor.sol"; import {IPlugin} from "../../interfaces/IPlugin.sol"; @@ -129,7 +124,7 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { override returns (bytes4) { - if (functionId == uint8(FunctionId.SIG_VALIDATION)) { + if (functionId == uint8(FunctionId.VALIDATION_OWNER)) { if (SignatureChecker.isValidSignatureNow(_owners[msg.sender], digest, signature)) { return _1271_MAGIC_VALUE; } @@ -156,35 +151,23 @@ contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - ManifestFunction memory ownerValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, + // TODO: use default validation instead + bytes4[] memory accountSelectors = new bytes4[](5); + accountSelectors[0] = IStandardExecutor.execute.selector; + accountSelectors[1] = IStandardExecutor.executeBatch.selector; + accountSelectors[2] = IPluginManager.installPlugin.selector; + accountSelectors[3] = IPluginManager.uninstallPlugin.selector; + accountSelectors[4] = UUPSUpgradeable.upgradeToAndCall.selector; + + ManifestValidation memory ownerValidationFunction = ManifestValidation({ functionId: uint8(FunctionId.VALIDATION_OWNER), - dependencyIndex: 0 // Unused. - }); - manifest.validationFunctions = new ManifestAssociatedFunction[](5); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.execute.selector, - associatedFunction: ownerValidationFunction - }); - manifest.validationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.executeBatch.selector, - associatedFunction: ownerValidationFunction - }); - manifest.validationFunctions[2] = ManifestAssociatedFunction({ - executionSelector: IPluginManager.installPlugin.selector, - associatedFunction: ownerValidationFunction - }); - manifest.validationFunctions[3] = ManifestAssociatedFunction({ - executionSelector: IPluginManager.uninstallPlugin.selector, - associatedFunction: ownerValidationFunction - }); - manifest.validationFunctions[4] = ManifestAssociatedFunction({ - executionSelector: UUPSUpgradeable.upgradeToAndCall.selector, - associatedFunction: ownerValidationFunction + isDefault: false, + isSignatureValidation: true, + selectors: accountSelectors }); - manifest.signatureValidationFunctions = new uint8[](1); - manifest.signatureValidationFunctions[0] = uint8(FunctionId.SIG_VALIDATION); + manifest.validationFunctions = new ManifestValidation[](1); + manifest.validationFunctions[0] = ownerValidationFunction; return manifest; } diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index f2e6391f..5e5049f8 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -8,7 +8,6 @@ import { PluginManifest } from "../../src/interfaces/IPlugin.sol"; import {IExecutionHook} from "../../src/interfaces/IExecutionHook.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {MockPlugin} from "../mocks/MockPlugin.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; @@ -25,7 +24,7 @@ contract AccountExecHooksTest is AccountTestBase { PluginManifest internal _m1; - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash); event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); // emitted by MockPlugin event ReceivedCall(bytes msgData, uint256 msgValue); @@ -168,14 +167,13 @@ contract AccountExecHooksTest is AccountTestBase { 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, new FunctionReference[](0)); + emit PluginInstalled(address(mockPlugin1), manifestHash1); vm.prank(address(entryPoint)); account1.installPlugin({ plugin: address(mockPlugin1), manifestHash: manifestHash1, - pluginInstallData: bytes(""), - dependencies: new FunctionReference[](0) + pluginInstallData: bytes("") }); } diff --git a/test/account/AccountLoupe.t.sol b/test/account/AccountLoupe.t.sol index 4c0ddb88..15803fa8 100644 --- a/test/account/AccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -23,7 +23,7 @@ contract AccountLoupeTest is AccountTestBase { bytes32 manifestHash = keccak256(abi.encode(comprehensivePlugin.pluginManifest())); vm.prank(address(entryPoint)); - account1.installPlugin(address(comprehensivePlugin), manifestHash, "", new FunctionReference[](0)); + account1.installPlugin(address(comprehensivePlugin), manifestHash, ""); FunctionReference[] memory preValidationHooks = new FunctionReference[](2); preValidationHooks[0] = FunctionReferenceLib.pack( diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index fc9fd615..99a8b516 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; +import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; import {Call} from "../../src/interfaces/IStandardExecutor.sol"; import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; @@ -31,8 +31,7 @@ contract AccountReturnDataTest is AccountTestBase { account1.installPlugin({ plugin: address(resultCreatorPlugin), manifestHash: resultCreatorManifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); // Add the result consumer plugin to the account bytes32 resultConsumerManifestHash = keccak256(abi.encode(resultConsumerPlugin.pluginManifest())); @@ -40,8 +39,7 @@ contract AccountReturnDataTest is AccountTestBase { account1.installPlugin({ plugin: address(resultConsumerPlugin), manifestHash: resultConsumerManifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); } diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index 9c79be9d..e0e69a37 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -34,7 +34,7 @@ contract MultiValidationTest is AccountTestBase { function test_overlappingValidationInstall() public { bytes32 manifestHash = keccak256(abi.encode(validator2.pluginManifest())); vm.prank(address(entryPoint)); - account1.installPlugin(address(validator2), manifestHash, abi.encode(owner2), new FunctionReference[](0)); + account1.installPlugin(address(validator2), manifestHash, abi.encode(owner2)); FunctionReference[] memory validations = new FunctionReference[](2); validations[0] = FunctionReferenceLib.pack( diff --git a/test/account/PermittedCallPermissions.t.sol b/test/account/PermittedCallPermissions.t.sol index 517b2080..18257955 100644 --- a/test/account/PermittedCallPermissions.t.sol +++ b/test/account/PermittedCallPermissions.t.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.19; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {ResultCreatorPlugin} from "../mocks/plugins/ReturnDataPluginMocks.sol"; import {PermittedCallerPlugin} from "../mocks/plugins/PermittedCallMocks.sol"; @@ -27,8 +26,7 @@ contract PermittedCallPermissionsTest is AccountTestBase { account1.installPlugin({ plugin: address(resultCreatorPlugin), manifestHash: resultCreatorManifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); // Add the permitted caller plugin to the account bytes32 permittedCallerManifestHash = keccak256(abi.encode(permittedCallerPlugin.pluginManifest())); @@ -36,8 +34,7 @@ contract PermittedCallPermissionsTest is AccountTestBase { account1.installPlugin({ plugin: address(permittedCallerPlugin), manifestHash: permittedCallerManifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); } diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index 840f268a..fda2061a 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -33,7 +33,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { bytes32 manifestHash = keccak256(abi.encode(comprehensivePlugin.pluginManifest())); vm.prank(address(entryPoint)); - account1.installPlugin(address(comprehensivePlugin), manifestHash, "", new FunctionReference[](0)); + account1.installPlugin(address(comprehensivePlugin), manifestHash, ""); comprehensivePluginValidation = FunctionReferenceLib.pack( address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.VALIDATION) diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 668852b8..de3fda30 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -10,7 +10,6 @@ import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {PluginManifest} from "../../src/interfaces/IPlugin.sol"; import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; @@ -39,7 +38,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { Counter public counter; PluginManifest internal _manifest; - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash); event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); event ReceivedCall(bytes msgData, uint256 msgValue); @@ -241,12 +240,11 @@ contract UpgradeableModularAccountTest is AccountTestBase { bytes32 manifestHash = keccak256(abi.encode(tokenReceiverPlugin.pluginManifest())); vm.expectEmit(true, true, true, true); - emit PluginInstalled(address(tokenReceiverPlugin), manifestHash, new FunctionReference[](0)); + emit PluginInstalled(address(tokenReceiverPlugin), manifestHash); IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, - pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + pluginInstallData: abi.encode(uint48(1 days)) }); address[] memory plugins = IAccountLoupe(account1).getInstalledPlugins(); @@ -266,8 +264,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(mockPluginWithBadPermittedExec), manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); } @@ -278,8 +275,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: bytes32(0), - pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + pluginInstallData: abi.encode(uint48(1 days)) }); } @@ -293,8 +289,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(badPlugin), manifestHash: bytes32(0), - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); } @@ -305,8 +300,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, - pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + pluginInstallData: abi.encode(uint48(1 days)) }); vm.expectRevert( @@ -317,8 +311,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, - pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + pluginInstallData: abi.encode(uint48(1 days)) }); } @@ -330,8 +323,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); vm.expectEmit(true, true, true, true); @@ -351,8 +343,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); vm.expectEmit(true, true, true, true); @@ -376,8 +367,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); // Attempt to uninstall with a blank _manifest @@ -404,8 +394,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { IPluginManager(account1).installPlugin({ plugin: address(plugin), manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); vm.stopPrank(); @@ -444,7 +433,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { // singleOwnerPlugin.ownerOf(address(account1)); bytes memory signature = abi.encodePacked( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.SIG_VALIDATION), r, s, v + address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), r, s, v ); bytes4 validationResult = IERC1271(address(account1)).isValidSignature(message, signature); diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index faa24074..d3fce6d8 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -49,14 +49,12 @@ contract ValidationIntersectionTest is AccountTestBase { account1.installPlugin({ plugin: address(noHookPlugin), manifestHash: keccak256(abi.encode(noHookPlugin.pluginManifest())), - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); account1.installPlugin({ plugin: address(oneHookPlugin), manifestHash: keccak256(abi.encode(oneHookPlugin.pluginManifest())), - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); // TODO: change with new install flow // temporary fix to add the pre-validation hook @@ -77,8 +75,7 @@ contract ValidationIntersectionTest is AccountTestBase { account1.installPlugin({ plugin: address(twoHookPlugin), manifestHash: keccak256(abi.encode(twoHookPlugin.pluginManifest())), - pluginInstallData: "", - dependencies: new FunctionReference[](0) + pluginInstallData: "" }); // temporary fix to add the pre-validation hook preValidationHooks = new FunctionReference[](2); diff --git a/test/mocks/plugins/ComprehensivePlugin.sol b/test/mocks/plugins/ComprehensivePlugin.sol index 4062218b..c901c81a 100644 --- a/test/mocks/plugins/ComprehensivePlugin.sol +++ b/test/mocks/plugins/ComprehensivePlugin.sol @@ -6,9 +6,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import { ManifestExecutionHook, ManifestExecutionFunction, - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, + ManifestValidation, PluginManifest, PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; @@ -142,15 +140,15 @@ contract ComprehensivePlugin is IValidation, IValidationHook, IExecutionHook, Ba allowDefaultValidation: false }); - ManifestFunction memory fooValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, + bytes4[] memory validationSelectors = new bytes4[](1); + validationSelectors[0] = this.foo.selector; + + manifest.validationFunctions = new ManifestValidation[](1); + manifest.validationFunctions[0] = ManifestValidation({ functionId: uint8(FunctionId.VALIDATION), - dependencyIndex: 0 // Unused. - }); - manifest.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: fooValidationFunction + isDefault: true, + isSignatureValidation: false, + selectors: validationSelectors }); manifest.executionHooks = new ManifestExecutionHook[](3); diff --git a/test/mocks/plugins/ReturnDataPluginMocks.sol b/test/mocks/plugins/ReturnDataPluginMocks.sol index c0fc0cfe..d7de107c 100644 --- a/test/mocks/plugins/ReturnDataPluginMocks.sol +++ b/test/mocks/plugins/ReturnDataPluginMocks.sol @@ -5,9 +5,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import { ManifestExecutionFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, - ManifestFunction, + ManifestValidation, PluginManifest, PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; @@ -117,14 +115,16 @@ contract ResultConsumerPlugin is BasePlugin, IValidation { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.execute.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(0), - dependencyIndex: 0 - }) + // 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({ + functionId: 0, + isDefault: true, + isSignatureValidation: false, + selectors: validationSelectors }); manifest.executionFunctions = new ManifestExecutionFunction[](2); diff --git a/test/mocks/plugins/ValidationPluginMocks.sol b/test/mocks/plugins/ValidationPluginMocks.sol index f6ed4a5f..c9bf91f2 100644 --- a/test/mocks/plugins/ValidationPluginMocks.sol +++ b/test/mocks/plugins/ValidationPluginMocks.sol @@ -4,10 +4,8 @@ pragma solidity ^0.8.19; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import { - ManifestFunction, ManifestExecutionFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, + ManifestValidation, PluginMetadata, PluginManifest } from "../../../src/interfaces/IPlugin.sol"; @@ -105,14 +103,15 @@ contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { allowDefaultValidation: false }); - manifest.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.USER_OP_VALIDATION), - dependencyIndex: 0 // Unused. - }) + bytes4[] memory validationSelectors = new bytes4[](1); + validationSelectors[0] = this.foo.selector; + + manifest.validationFunctions = new ManifestValidation[](1); + manifest.validationFunctions[0] = ManifestValidation({ + functionId: uint8(FunctionId.USER_OP_VALIDATION), + isDefault: false, + isSignatureValidation: false, + selectors: validationSelectors }); return manifest; @@ -147,15 +146,15 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { allowDefaultValidation: false }); - ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, + bytes4[] memory validationSelectors = new bytes4[](1); + validationSelectors[0] = this.bar.selector; + + manifest.validationFunctions = new ManifestValidation[](2); + manifest.validationFunctions[0] = ManifestValidation({ functionId: uint8(FunctionId.USER_OP_VALIDATION), - dependencyIndex: 0 // Unused. - }); - manifest.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.bar.selector, - associatedFunction: userOpValidationFunctionRef + isDefault: false, + isSignatureValidation: false, + selectors: validationSelectors }); return manifest; @@ -193,15 +192,15 @@ contract MockUserOpValidation2HookPlugin is MockBaseUserOpValidationPlugin { allowDefaultValidation: false }); - ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, + bytes4[] memory validationSelectors = new bytes4[](1); + validationSelectors[0] = this.baz.selector; + + manifest.validationFunctions = new ManifestValidation[](1); + manifest.validationFunctions[0] = ManifestValidation({ functionId: uint8(FunctionId.USER_OP_VALIDATION), - dependencyIndex: 0 // Unused. - }); - manifest.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.baz.selector, - associatedFunction: userOpValidationFunctionRef + isDefault: false, + isSignatureValidation: false, + selectors: validationSelectors }); return manifest; diff --git a/test/plugin/SingleOwnerPlugin.t.sol b/test/plugin/SingleOwnerPlugin.t.sol index 41997591..f86a29f4 100644 --- a/test/plugin/SingleOwnerPlugin.t.sol +++ b/test/plugin/SingleOwnerPlugin.t.sol @@ -156,7 +156,7 @@ contract SingleOwnerPluginTest is OptimizedTest { // sig check should fail assertEq( plugin.validateSignature( - uint8(ISingleOwnerPlugin.FunctionId.SIG_VALIDATION), + uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), address(this), digest, abi.encodePacked(r, s, v) @@ -171,7 +171,7 @@ contract SingleOwnerPluginTest is OptimizedTest { // sig check should pass assertEq( plugin.validateSignature( - uint8(ISingleOwnerPlugin.FunctionId.SIG_VALIDATION), + uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), address(this), digest, abi.encodePacked(r, s, v) @@ -186,7 +186,7 @@ contract SingleOwnerPluginTest is OptimizedTest { bytes memory signature = contractOwner.sign(digest); assertEq( plugin.validateSignature( - uint8(ISingleOwnerPlugin.FunctionId.SIG_VALIDATION), address(this), digest, signature + uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER), address(this), digest, signature ), _1271_MAGIC_VALUE ); diff --git a/test/plugin/TokenReceiverPlugin.t.sol b/test/plugin/TokenReceiverPlugin.t.sol index 0e111020..2f52a988 100644 --- a/test/plugin/TokenReceiverPlugin.t.sol +++ b/test/plugin/TokenReceiverPlugin.t.sol @@ -6,7 +6,6 @@ import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Recei import {IERC1155Receiver} from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; @@ -56,7 +55,7 @@ contract TokenReceiverPluginTest is OptimizedTest, IERC1155Receiver { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); vm.prank(address(entryPoint)); - acct.installPlugin(address(plugin), manifestHash, "", new FunctionReference[](0)); + acct.installPlugin(address(plugin), manifestHash, ""); } function test_failERC721Transfer() public {