diff --git a/src/account/AccountLoupe.sol b/src/account/AccountLoupe.sol index 8459c65a..b9135b43 100644 --- a/src/account/AccountLoupe.sol +++ b/src/account/AccountLoupe.sol @@ -6,9 +6,10 @@ import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {IAccountLoupe} from "../interfaces/IAccountLoupe.sol"; -import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; +import {IPlugin} from "../interfaces/IPlugin.sol"; +import {IPluginManager} from "../interfaces/IPluginManager.sol"; import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; -import {AccountStorage, getAccountStorage, SelectorData, toFunctionReferenceArray} from "./AccountStorage.sol"; +import {AccountStorage, getAccountStorage, toAddressArray, toPlugin, SelectorData} from "./AccountStorage.sol"; abstract contract AccountLoupe is IAccountLoupe { using EnumerableMap for EnumerableMap.Bytes32ToUintMap; @@ -36,9 +37,7 @@ abstract contract AccountLoupe is IAccountLoupe { config.plugin = _storage.selectorData[selector].plugin; } - config.userOpValidationFunction = _storage.selectorData[selector].userOpValidation; - - config.runtimeValidationFunction = _storage.selectorData[selector].runtimeValidation; + config.validationPlugin = address(_storage.selectorData[selector].validation); } /// @inheritdoc IAccountLoupe @@ -46,16 +45,7 @@ abstract contract AccountLoupe is IAccountLoupe { SelectorData storage selectorData = getAccountStorage().selectorData[selector]; uint256 preExecHooksLength = selectorData.preHooks.length(); uint256 postOnlyExecHooksLength = selectorData.postOnlyHooks.length(); - uint256 maxExecHooksLength = postOnlyExecHooksLength; - - // There can only be as many associated post hooks to run as there are pre hooks. - for (uint256 i = 0; i < preExecHooksLength;) { - (, uint256 count) = selectorData.preHooks.at(i); - unchecked { - maxExecHooksLength += (count + 1); - ++i; - } - } + uint256 maxExecHooksLength = postOnlyExecHooksLength + preExecHooksLength; // Overallocate on length - not all of this may get filled up. We set the correct length later. execHooks = new ExecutionHooks[](maxExecHooksLength); @@ -63,36 +53,23 @@ abstract contract AccountLoupe is IAccountLoupe { for (uint256 i = 0; i < preExecHooksLength;) { (bytes32 key,) = selectorData.preHooks.at(i); - FunctionReference preExecHook = FunctionReference.wrap(bytes21(key)); - - uint256 associatedPostExecHooksLength = selectorData.associatedPostHooks[preExecHook].length(); - if (associatedPostExecHooksLength > 0) { - for (uint256 j = 0; j < associatedPostExecHooksLength;) { - execHooks[actualExecHooksLength].preExecHook = preExecHook; - (key,) = selectorData.associatedPostHooks[preExecHook].at(j); - execHooks[actualExecHooksLength].postExecHook = FunctionReference.wrap(bytes21(key)); - - unchecked { - ++actualExecHooksLength; - ++j; - } - } - } else { - execHooks[actualExecHooksLength].preExecHook = preExecHook; - - unchecked { - ++actualExecHooksLength; - } + IPlugin preExecHookPlugin = toPlugin(key); + + execHooks[actualExecHooksLength].preExecHookPlugin = address(preExecHookPlugin); + + if (selectorData.hasAssociatedPostHook[preExecHookPlugin]) { + execHooks[actualExecHooksLength].postExecHookPlugin = address(preExecHookPlugin); } unchecked { + ++actualExecHooksLength; ++i; } } for (uint256 i = 0; i < postOnlyExecHooksLength;) { (bytes32 key,) = selectorData.postOnlyHooks.at(i); - execHooks[actualExecHooksLength].postExecHook = FunctionReference.wrap(bytes21(key)); + execHooks[actualExecHooksLength].postExecHookPlugin = address(toPlugin(key)); unchecked { ++actualExecHooksLength; @@ -110,15 +87,12 @@ abstract contract AccountLoupe is IAccountLoupe { function getPreValidationHooks(bytes4 selector) external view - returns ( - FunctionReference[] memory preUserOpValidationHooks, - FunctionReference[] memory preRuntimeValidationHooks - ) + returns (address[] memory preUserOpValidationHooks, address[] memory preRuntimeValidationHooks) { preUserOpValidationHooks = - toFunctionReferenceArray(getAccountStorage().selectorData[selector].preUserOpValidationHooks); + toAddressArray(getAccountStorage().selectorData[selector].preUserOpValidationHooks); preRuntimeValidationHooks = - toFunctionReferenceArray(getAccountStorage().selectorData[selector].preRuntimeValidationHooks); + toAddressArray(getAccountStorage().selectorData[selector].preRuntimeValidationHooks); } /// @inheritdoc IAccountLoupe diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 0fea6771..3a8eb0d1 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -5,7 +5,6 @@ import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {IPlugin} from "../interfaces/IPlugin.sol"; -import {FunctionReference} from "../interfaces/IPluginManager.sol"; // bytes = keccak256("ERC6900.UpgradeableModularAccount.Storage") bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c401499164f368baef687948c315d9073e40; @@ -15,7 +14,7 @@ struct PluginData { // boolean to indicate if the plugin can spend native tokens from the account. bool canSpendNativeToken; bytes32 manifestHash; - FunctionReference[] dependencies; + address[] dependencies; // Tracks the number of times this plugin has been used as a dependency function uint256 dependentCount; } @@ -35,15 +34,15 @@ struct SelectorData { // The plugin that implements this execution function. // If this is a native function, the address must remain address(0). address plugin; - FunctionReference userOpValidation; - FunctionReference runtimeValidation; + // User operation validation and runtime validation share a function reference. + IPlugin validation; // The pre validation hooks for this function selector. EnumerableMap.Bytes32ToUintMap preUserOpValidationHooks; EnumerableMap.Bytes32ToUintMap preRuntimeValidationHooks; // The execution hooks for this function selector. EnumerableMap.Bytes32ToUintMap preHooks; - // bytes21 key = pre hook function reference - mapping(FunctionReference => EnumerableMap.Bytes32ToUintMap) associatedPostHooks; + // bytes20 key = pre hook plugin address + mapping(IPlugin => bool) hasAssociatedPostHook; EnumerableMap.Bytes32ToUintMap postOnlyHooks; } @@ -53,7 +52,7 @@ struct AccountStorage { bool initializing; // Plugin metadata storage EnumerableSet.AddressSet plugins; - mapping(address => PluginData) pluginData; + mapping(IPlugin => PluginData) pluginData; // Execution functions and their associated functions mapping(bytes4 => SelectorData) selectorData; // bytes24 key = address(calling plugin) || bytes4(selector of execution function) @@ -74,18 +73,19 @@ function getPermittedCallKey(address addr, bytes4 selector) pure returns (bytes2 return bytes24(bytes20(addr)) | (bytes24(selector) >> 160); } +function toPlugin(bytes32 key) pure returns (IPlugin) { + return IPlugin(address(bytes20(key))); +} + // Helper function to get all elements of a set into memory. using EnumerableMap for EnumerableMap.Bytes32ToUintMap; -function toFunctionReferenceArray(EnumerableMap.Bytes32ToUintMap storage map) - view - returns (FunctionReference[] memory) -{ +function toAddressArray(EnumerableMap.Bytes32ToUintMap storage map) view returns (address[] memory) { uint256 length = map.length(); - FunctionReference[] memory result = new FunctionReference[](length); + address[] memory result = new address[](length); for (uint256 i = 0; i < length;) { (bytes32 key,) = map.at(i); - result[i] = FunctionReference.wrap(bytes21(key)); + result[i] = address(bytes20(key)); unchecked { ++i; diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManagerInternals.sol index e3062661..f97946c9 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManagerInternals.sol @@ -5,7 +5,6 @@ import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165C import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; import { IPlugin, ManifestExecutionHook, @@ -15,7 +14,7 @@ import { ManifestExternalCallPermission, PluginManifest } from "../interfaces/IPlugin.sol"; -import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; +import {IPluginManager} from "../interfaces/IPluginManager.sol"; import { AccountStorage, getAccountStorage, @@ -27,7 +26,10 @@ import { abstract contract PluginManagerInternals is IPluginManager { using EnumerableMap for EnumerableMap.Bytes32ToUintMap; using EnumerableSet for EnumerableSet.AddressSet; - using FunctionReferenceLib for FunctionReference; + + IPlugin internal constant _NULL_PLUGIN = IPlugin(address(0)); + IPlugin internal constant _RUNTIME_VALIDATION_ALWAYS_ALLOW = IPlugin(address(1)); + IPlugin internal constant _PRE_HOOK_ALWAYS_DENY = IPlugin(address(2)); error ArrayLengthMismatch(); error ExecutionFunctionAlreadySet(bytes4 selector); @@ -41,18 +43,10 @@ abstract contract PluginManagerInternals is IPluginManager { error PluginInstallCallbackFailed(address plugin, bytes revertReason); error PluginInterfaceNotSupported(address plugin); error PluginNotInstalled(address plugin); - error RuntimeValidationFunctionAlreadySet(bytes4 selector, FunctionReference validationFunction); - error UserOpValidationFunctionAlreadySet(bytes4 selector, FunctionReference validationFunction); + error ValidationFunctionAlreadySet(bytes4 selector, IPlugin validation); - modifier notNullFunction(FunctionReference functionReference) { - if (functionReference.isEmpty()) { - revert NullFunctionReference(); - } - _; - } - - modifier notNullPlugin(address plugin) { - if (plugin == address(0)) { + modifier notNullPlugin(IPlugin plugin) { + if (address(plugin) == address(0)) { revert NullPlugin(); } _; @@ -60,7 +54,7 @@ abstract contract PluginManagerInternals is IPluginManager { // Storage update operations - function _setExecutionFunction(bytes4 selector, address plugin) internal notNullPlugin(plugin) { + function _setExecutionFunction(bytes4 selector, address plugin) internal notNullPlugin(IPlugin(plugin)) { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; if (_selectorData.plugin != address(0)) { @@ -76,81 +70,53 @@ abstract contract PluginManagerInternals is IPluginManager { _selectorData.plugin = address(0); } - function _addUserOpValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { + function _addValidationFunction(bytes4 selector, IPlugin validation) internal notNullPlugin(validation) { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (!_selectorData.userOpValidation.isEmpty()) { - revert UserOpValidationFunctionAlreadySet(selector, validationFunction); + if (_selectorData.validation != _NULL_PLUGIN) { + revert ValidationFunctionAlreadySet(selector, validation); } - _selectorData.userOpValidation = validationFunction; + _selectorData.validation = validation; } - function _removeUserOpValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { + function _removeValidationFunction(bytes4 selector, IPlugin validation) internal notNullPlugin(validation) { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - _selectorData.userOpValidation = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; + _selectorData.validation = _NULL_PLUGIN; } - function _addRuntimeValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - if (!_selectorData.runtimeValidation.isEmpty()) { - revert RuntimeValidationFunctionAlreadySet(selector, validationFunction); - } - - _selectorData.runtimeValidation = validationFunction; - } - - function _removeRuntimeValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - _selectorData.runtimeValidation = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; - } - - function _addExecHooks(bytes4 selector, FunctionReference preExecHook, FunctionReference postExecHook) - internal - { + function _addExecHooks(bytes4 selector, IPlugin preExecHook, IPlugin postExecHook) internal { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (!preExecHook.isEmpty()) { + if (preExecHook != _NULL_PLUGIN) { _addOrIncrement(_selectorData.preHooks, _toSetValue(preExecHook)); - if (!postExecHook.isEmpty()) { - _addOrIncrement(_selectorData.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); + if (postExecHook != _NULL_PLUGIN) { + if (preExecHook == _PRE_HOOK_ALWAYS_DENY) { + // todo: more elegant handling of this case. + revert InvalidPluginManifest(); + } + _selectorData.hasAssociatedPostHook[preExecHook] = true; } } else { - if (postExecHook.isEmpty()) { + if (postExecHook == _NULL_PLUGIN) { // both pre and post hooks cannot be null - revert NullFunctionReference(); + revert NullPlugin(); } _addOrIncrement(_selectorData.postOnlyHooks, _toSetValue(postExecHook)); } } - function _removeExecHooks(bytes4 selector, FunctionReference preExecHook, FunctionReference postExecHook) - internal - { + function _removeExecHooks(bytes4 selector, IPlugin preExecHook, IPlugin postExecHook) internal { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - if (!preExecHook.isEmpty()) { + if (preExecHook != _NULL_PLUGIN) { _removeOrDecrement(_selectorData.preHooks, _toSetValue(preExecHook)); - if (!postExecHook.isEmpty()) { - _removeOrDecrement(_selectorData.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); + if (postExecHook != _NULL_PLUGIN) { + _selectorData.hasAssociatedPostHook[preExecHook] = false; } } else { // The case where both pre and post hooks are null was checked during installation. @@ -160,9 +126,9 @@ abstract contract PluginManagerInternals is IPluginManager { } } - function _addPreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) + function _addPreUserOpValidationHook(bytes4 selector, IPlugin preUserOpValidationHook) internal - notNullFunction(preUserOpValidationHook) + notNullPlugin(preUserOpValidationHook) { _addOrIncrement( getAccountStorage().selectorData[selector].preUserOpValidationHooks, @@ -170,9 +136,9 @@ abstract contract PluginManagerInternals is IPluginManager { ); } - function _removePreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) + function _removePreUserOpValidationHook(bytes4 selector, IPlugin preUserOpValidationHook) internal - notNullFunction(preUserOpValidationHook) + notNullPlugin(preUserOpValidationHook) { // May ignore return value, as the manifest hash is validated to ensure that the hook exists. _removeOrDecrement( @@ -181,9 +147,9 @@ abstract contract PluginManagerInternals is IPluginManager { ); } - function _addPreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) + function _addPreRuntimeValidationHook(bytes4 selector, IPlugin preRuntimeValidationHook) internal - notNullFunction(preRuntimeValidationHook) + notNullPlugin(preRuntimeValidationHook) { _addOrIncrement( getAccountStorage().selectorData[selector].preRuntimeValidationHooks, @@ -191,9 +157,9 @@ abstract contract PluginManagerInternals is IPluginManager { ); } - function _removePreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) + function _removePreRuntimeValidationHook(bytes4 selector, IPlugin preRuntimeValidationHook) internal - notNullFunction(preRuntimeValidationHook) + notNullPlugin(preRuntimeValidationHook) { // May ignore return value, as the manifest hash is validated to ensure that the hook exists. _removeOrDecrement( @@ -206,7 +172,7 @@ abstract contract PluginManagerInternals is IPluginManager { address plugin, bytes32 manifestHash, bytes memory pluginInstallData, - FunctionReference[] memory dependencies + address[] memory dependencies ) internal { AccountStorage storage _storage = getAccountStorage(); @@ -234,15 +200,15 @@ abstract contract PluginManagerInternals is IPluginManager { uint256 length = dependencies.length; for (uint256 i = 0; i < length;) { // Check the dependency interface id over the address of the dependency. - (address dependencyAddr,) = dependencies[i].unpack(); + IPlugin dependencyAddr = IPlugin(dependencies[i]); // Check that the dependency is installed. if (_storage.pluginData[dependencyAddr].manifestHash == bytes32(0)) { - revert MissingPluginDependency(dependencyAddr); + revert MissingPluginDependency(address(dependencyAddr)); } // Check that the dependency supports the expected interface. - if (!ERC165Checker.supportsInterface(dependencyAddr, manifest.dependencyInterfaceIds[i])) { + if (!ERC165Checker.supportsInterface(address(dependencyAddr), manifest.dependencyInterfaceIds[i])) { revert InvalidDependenciesProvided(); } @@ -255,14 +221,14 @@ abstract contract PluginManagerInternals is IPluginManager { } // Add the plugin metadata to the account - _storage.pluginData[plugin].manifestHash = manifestHash; - _storage.pluginData[plugin].dependencies = dependencies; + _storage.pluginData[IPlugin(plugin)].manifestHash = manifestHash; + _storage.pluginData[IPlugin(plugin)].dependencies = dependencies; // Update components according to the manifest. // Mark whether or not this plugin may spend native token amounts if (manifest.canSpendNativeToken) { - _storage.pluginData[plugin].canSpendNativeToken = true; + _storage.pluginData[IPlugin(plugin)].canSpendNativeToken = true; } length = manifest.executionFunctions.length; @@ -288,7 +254,7 @@ abstract contract PluginManagerInternals is IPluginManager { // Add the permitted external calls to the account. if (manifest.permitAnyExternalAddress) { - _storage.pluginData[plugin].anyExternalExecPermitted = true; + _storage.pluginData[IPlugin(plugin)].anyExternalExecPermitted = true; } else { // Only store the specific permitted external calls if "permit any" flag was not set. length = manifest.permittedExternalCalls.length; @@ -319,25 +285,10 @@ abstract contract PluginManagerInternals is IPluginManager { } } - length = manifest.userOpValidationFunctions.length; + length = manifest.validationFunctions.length; for (uint256 i = 0; i < length;) { - ManifestAssociatedFunction memory mv = manifest.userOpValidationFunctions[i]; - _addUserOpValidationFunction( - mv.executionSelector, - _resolveManifestFunction( - mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE - ) - ); - - unchecked { - ++i; - } - } - - length = manifest.runtimeValidationFunctions.length; - for (uint256 i = 0; i < length;) { - ManifestAssociatedFunction memory mv = manifest.runtimeValidationFunctions[i]; - _addRuntimeValidationFunction( + ManifestAssociatedFunction memory mv = manifest.validationFunctions[i]; + _addValidationFunction( mv.executionSelector, _resolveManifestFunction( mv.associatedFunction, @@ -353,7 +304,7 @@ abstract contract PluginManagerInternals is IPluginManager { } // Hooks are not allowed to be provided as dependencies, so we use an empty array for resolving them. - FunctionReference[] memory emptyDependencies; + address[] memory emptyDependencies; length = manifest.preUserOpValidationHooks.length; for (uint256 i = 0; i < length;) { @@ -437,25 +388,24 @@ abstract contract PluginManagerInternals is IPluginManager { } // Check manifest hash. - bytes32 manifestHash = _storage.pluginData[plugin].manifestHash; + bytes32 manifestHash = _storage.pluginData[IPlugin(plugin)].manifestHash; if (!_isValidPluginManifest(manifest, manifestHash)) { revert InvalidPluginManifest(); } // Ensure that there are no dependent plugins. - if (_storage.pluginData[plugin].dependentCount != 0) { + if (_storage.pluginData[IPlugin(plugin)].dependentCount != 0) { revert PluginDependencyViolation(plugin); } // Remove this plugin as a dependent from its dependencies. - FunctionReference[] memory dependencies = _storage.pluginData[plugin].dependencies; + address[] memory dependencies = _storage.pluginData[IPlugin(plugin)].dependencies; uint256 length = dependencies.length; for (uint256 i = 0; i < length;) { - FunctionReference dependency = dependencies[i]; - (address dependencyAddr,) = dependency.unpack(); + address dependency = dependencies[i]; // Decrement the dependent count for the dependency function. - _storage.pluginData[dependencyAddr].dependentCount -= 1; + _storage.pluginData[IPlugin(dependency)].dependentCount -= 1; unchecked { ++i; @@ -465,7 +415,7 @@ abstract contract PluginManagerInternals is IPluginManager { // Remove components according to the manifest, in reverse order (by component type) of their installation. // Hooks are not allowed to be provided as dependencies, so we use an empty array for resolving them. - FunctionReference[] memory emptyDependencies; + address[] memory emptyDependencies; length = manifest.executionHooks.length; for (uint256 i = 0; i < length;) { @@ -521,10 +471,10 @@ abstract contract PluginManagerInternals is IPluginManager { } } - length = manifest.runtimeValidationFunctions.length; + length = manifest.validationFunctions.length; for (uint256 i = 0; i < length;) { - ManifestAssociatedFunction memory mv = manifest.runtimeValidationFunctions[i]; - _removeRuntimeValidationFunction( + ManifestAssociatedFunction memory mv = manifest.validationFunctions[i]; + _removeValidationFunction( mv.executionSelector, _resolveManifestFunction( mv.associatedFunction, @@ -539,26 +489,11 @@ abstract contract PluginManagerInternals is IPluginManager { } } - length = manifest.userOpValidationFunctions.length; - for (uint256 i = 0; i < length;) { - ManifestAssociatedFunction memory mv = manifest.userOpValidationFunctions[i]; - _removeUserOpValidationFunction( - mv.executionSelector, - _resolveManifestFunction( - mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE - ) - ); - - unchecked { - ++i; - } - } - // remove external call permissions if (manifest.permitAnyExternalAddress) { // Only clear if it was set during install time - _storage.pluginData[plugin].anyExternalExecPermitted = false; + _storage.pluginData[IPlugin(plugin)].anyExternalExecPermitted = false; } else { // Only clear the specific permitted external calls if "permit any" flag was not set. length = manifest.permittedExternalCalls.length; @@ -617,7 +552,7 @@ abstract contract PluginManagerInternals is IPluginManager { } // Remove the plugin metadata from the account. - delete _storage.pluginData[plugin]; + delete _storage.pluginData[IPlugin(plugin)]; // Clear the plugin storage for the account. bool onUninstallSuccess = true; @@ -649,12 +584,12 @@ abstract contract PluginManagerInternals is IPluginManager { return true; } - function _toSetValue(FunctionReference functionReference) internal pure returns (bytes32) { - return bytes32(FunctionReference.unwrap(functionReference)); + function _toSetValue(IPlugin plugin) internal pure returns (bytes32) { + return bytes32(bytes20(address(plugin))); } - function _toFunctionReference(bytes32 setValue) internal pure returns (FunctionReference) { - return FunctionReference.wrap(bytes21(setValue)); + function _isEmptyOrMagicValue(IPlugin plugin) internal pure returns (bool) { + return address(plugin) <= address(2); } function _isValidPluginManifest(PluginManifest memory manifest, bytes32 manifestHash) @@ -668,31 +603,31 @@ abstract contract PluginManagerInternals is IPluginManager { function _resolveManifestFunction( ManifestFunction memory manifestFunction, address plugin, - FunctionReference[] memory dependencies, + address[] memory dependencies, // Indicates which magic value, if any, is permissible for the function to resolve. ManifestAssociatedFunctionType allowedMagicValue - ) internal pure returns (FunctionReference) { + ) internal pure returns (IPlugin) { if (manifestFunction.functionType == ManifestAssociatedFunctionType.SELF) { - return FunctionReferenceLib.pack(plugin, manifestFunction.functionId); + return IPlugin(plugin); } else if (manifestFunction.functionType == ManifestAssociatedFunctionType.DEPENDENCY) { if (manifestFunction.dependencyIndex >= dependencies.length) { revert InvalidPluginManifest(); } - return dependencies[manifestFunction.dependencyIndex]; + return IPlugin(dependencies[manifestFunction.dependencyIndex]); } else if (manifestFunction.functionType == ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW) { if (allowedMagicValue == ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW) { - return FunctionReferenceLib._RUNTIME_VALIDATION_ALWAYS_ALLOW; + return _RUNTIME_VALIDATION_ALWAYS_ALLOW; } else { revert InvalidPluginManifest(); } } else if (manifestFunction.functionType == ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY) { if (allowedMagicValue == ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY) { - return FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY; + return _PRE_HOOK_ALWAYS_DENY; } else { revert InvalidPluginManifest(); } } - return FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; // Empty checks are done elsewhere + return IPlugin(address(0)); // Empty checks are done elsewhere } } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index a449ae99..1fb60f13 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -9,15 +9,16 @@ import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol"; import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; import {IPluginExecutor} from "../interfaces/IPluginExecutor.sol"; -import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; +import {IPluginManager} from "../interfaces/IPluginManager.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; import {AccountExecutor} from "./AccountExecutor.sol"; import {AccountLoupe} from "./AccountLoupe.sol"; -import {AccountStorage, getAccountStorage, getPermittedCallKey, SelectorData} from "./AccountStorage.sol"; +import { + AccountStorage, getAccountStorage, getPermittedCallKey, toPlugin, SelectorData +} from "./AccountStorage.sol"; import {AccountStorageInitializable} from "./AccountStorageInitializable.sol"; import {PluginManagerInternals} from "./PluginManagerInternals.sol"; @@ -34,11 +35,10 @@ contract UpgradeableModularAccount is { using EnumerableMap for EnumerableMap.Bytes32ToUintMap; using EnumerableSet for EnumerableSet.Bytes32Set; - using FunctionReferenceLib for FunctionReference; struct PostExecToRun { bytes preExecHookReturnData; - FunctionReference postExecHook; + IPlugin postExecHookPlugin; } IEntryPoint private immutable _ENTRY_POINT; @@ -55,14 +55,14 @@ contract UpgradeableModularAccount is error ExecFromPluginExternalNotPermitted(address plugin, address target, uint256 value, bytes data); error InvalidConfiguration(); error NativeTokenSpendingNotPermitted(address plugin); - error PostExecHookReverted(address plugin, uint8 functionId, bytes revertReason); - error PreExecHookReverted(address plugin, uint8 functionId, bytes revertReason); - error PreRuntimeValidationHookFailed(address plugin, uint8 functionId, bytes revertReason); - error RuntimeValidationFunctionMissing(bytes4 selector); - error RuntimeValidationFunctionReverted(address plugin, uint8 functionId, bytes revertReason); - error UnexpectedAggregator(address plugin, uint8 functionId, address aggregator); + error PostExecHookReverted(address plugin, bytes revertReason); + error PreExecHookReverted(address plugin, bytes revertReason); + error PreRuntimeValidationHookFailed(address plugin, bytes revertReason); + error RuntimeValidationMissing(bytes4 selector); + error RuntimeValidationReverted(address plugin, bytes revertReason); + error UnexpectedAggregator(address plugin, address aggregator); error UnrecognizedFunction(bytes4 selector); - error UserOpValidationFunctionMissing(bytes4 selector); + error UserOpValidationMissing(bytes4 selector); // Wraps execution of a native function with runtime validation and hooks // Used for upgradeTo, upgradeToAndCall, execute, executeBatch, installPlugin, uninstallPlugin @@ -99,7 +99,7 @@ contract UpgradeableModularAccount is revert ArrayLengthMismatch(); } - FunctionReference[] memory emptyDependencies = new FunctionReference[](0); + address[] memory emptyDependencies = new address[](0); for (uint256 i = 0; i < length;) { _installPlugin(plugins[i], manifestHashes[i], pluginInstallDatas[i], emptyDependencies); @@ -219,7 +219,7 @@ contract UpgradeableModularAccount is AccountStorage storage _storage = getAccountStorage(); // Make sure plugin is allowed to spend native token. - if (value > 0 && value > msg.value && !_storage.pluginData[msg.sender].canSpendNativeToken) { + if (value > 0 && value > msg.value && !_storage.pluginData[IPlugin(msg.sender)].canSpendNativeToken) { revert NativeTokenSpendingNotPermitted(msg.sender); } @@ -241,7 +241,7 @@ contract UpgradeableModularAccount is // If the target contract is not permitted, check if the caller plugin is permitted to make any external // calls. - if (!(targetContractPermittedCall || _storage.pluginData[msg.sender].anyExternalExecPermitted)) { + if (!(targetContractPermittedCall || _storage.pluginData[IPlugin(msg.sender)].anyExternalExecPermitted)) { revert ExecFromPluginExternalNotPermitted(msg.sender, target, value, data); } @@ -263,7 +263,7 @@ contract UpgradeableModularAccount is address plugin, bytes32 manifestHash, bytes calldata pluginInstallData, - FunctionReference[] calldata dependencies + address[] calldata dependencies ) external override wrapNativeFunction { _installPlugin(plugin, manifestHash, pluginInstallData, dependencies); } @@ -336,20 +336,20 @@ contract UpgradeableModularAccount is } bytes4 selector = bytes4(userOp.callData); - FunctionReference userOpValidationFunction = getAccountStorage().selectorData[selector].userOpValidation; + IPlugin userOpValidation = getAccountStorage().selectorData[selector].validation; - validationData = _doUserOpValidation(selector, userOpValidationFunction, userOp, userOpHash); + validationData = _doUserOpValidation(selector, userOpValidation, userOp, userOpHash); } // To support gas estimation, we don't fail early when the failure is caused by a signature failure function _doUserOpValidation( bytes4 selector, - FunctionReference userOpValidationFunction, + IPlugin userOpValidationPlugin, UserOperation calldata userOp, bytes32 userOpHash ) internal returns (uint256 validationData) { - if (userOpValidationFunction.isEmpty()) { - revert UserOpValidationFunctionMissing(selector); + if (userOpValidationPlugin == _NULL_PLUGIN) { + revert UserOpValidationMissing(selector); } uint256 currentValidationData; @@ -361,15 +361,16 @@ contract UpgradeableModularAccount is uint256 preUserOpValidationHooksLength = preUserOpValidationHooks.length(); for (uint256 i = 0; i < preUserOpValidationHooksLength;) { (bytes32 key,) = preUserOpValidationHooks.at(i); - FunctionReference preUserOpValidationHook = _toFunctionReference(key); + IPlugin preUserOpValidationHookPlugin = toPlugin(key); - if (!preUserOpValidationHook.isEmptyOrMagicValue()) { - (address plugin, uint8 functionId) = preUserOpValidationHook.unpack(); - currentValidationData = IPlugin(plugin).preUserOpValidationHook(functionId, userOp, userOpHash); + if (!_isEmptyOrMagicValue(preUserOpValidationHookPlugin)) { + currentValidationData = preUserOpValidationHookPlugin.preUserOpValidationHook(userOp, userOpHash); if (uint160(currentValidationData) > 1) { // If the aggregator is not 0 or 1, it is an unexpected value - revert UnexpectedAggregator(plugin, functionId, address(uint160(currentValidationData))); + revert UnexpectedAggregator( + address(preUserOpValidationHookPlugin), address(uint160(currentValidationData)) + ); } validationData = _coalescePreValidation(validationData, currentValidationData); } else { @@ -384,9 +385,8 @@ contract UpgradeableModularAccount is // Run the user op validationFunction { - if (!userOpValidationFunction.isEmptyOrMagicValue()) { - (address plugin, uint8 functionId) = userOpValidationFunction.unpack(); - currentValidationData = IPlugin(plugin).userOpValidationFunction(functionId, userOp, userOpHash); + if (!_isEmptyOrMagicValue(userOpValidationPlugin)) { + currentValidationData = IPlugin(userOpValidationPlugin).validateUserOp(userOp, userOpHash); if (preUserOpValidationHooksLength != 0) { // If we have other validation data we need to coalesce with @@ -395,7 +395,8 @@ contract UpgradeableModularAccount is validationData = currentValidationData; } } else { - // _RUNTIME_VALIDATION_ALWAYS_ALLOW and _PRE_HOOK_ALWAYS_DENY is not permitted here. + // _PRE_HOOK_ALWAYS_DENY is not permitted here. + // If this is _RUNTIME_VALIDATION_ALWAYS_ALLOW, the call should revert. revert InvalidConfiguration(); } } @@ -405,7 +406,7 @@ contract UpgradeableModularAccount is if (msg.sender == address(_ENTRY_POINT)) return; AccountStorage storage _storage = getAccountStorage(); - FunctionReference runtimeValidationFunction = _storage.selectorData[msg.sig].runtimeValidation; + IPlugin runtimeValidationPlugin = _storage.selectorData[msg.sig].validation; // run all preRuntimeValidation hooks EnumerableMap.Bytes32ToUintMap storage preRuntimeValidationHooks = getAccountStorage().selectorData[msg.sig].preRuntimeValidationHooks; @@ -413,21 +414,20 @@ contract UpgradeableModularAccount is uint256 preRuntimeValidationHooksLength = preRuntimeValidationHooks.length(); for (uint256 i = 0; i < preRuntimeValidationHooksLength;) { (bytes32 key,) = preRuntimeValidationHooks.at(i); - FunctionReference preRuntimeValidationHook = _toFunctionReference(key); + IPlugin preRuntimeValidationHookPlugin = toPlugin(key); - if (!preRuntimeValidationHook.isEmptyOrMagicValue()) { - (address plugin, uint8 functionId) = preRuntimeValidationHook.unpack(); + if (!_isEmptyOrMagicValue(preRuntimeValidationHookPlugin)) { // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).preRuntimeValidationHook(functionId, msg.sender, msg.value, msg.data) {} + try preRuntimeValidationHookPlugin.preRuntimeValidationHook(msg.sender, msg.value, msg.data) {} catch (bytes memory revertReason) { - revert PreRuntimeValidationHookFailed(plugin, functionId, revertReason); + revert PreRuntimeValidationHookFailed(address(preRuntimeValidationHookPlugin), revertReason); } unchecked { ++i; } } else { - if (preRuntimeValidationHook.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) { + if (preRuntimeValidationHookPlugin == _PRE_HOOK_ALWAYS_DENY) { revert AlwaysDenyRule(); } // Function reference cannot be 0 or _RUNTIME_VALIDATION_ALWAYS_ALLOW. @@ -437,17 +437,16 @@ contract UpgradeableModularAccount is // Identifier scope limiting { - if (!runtimeValidationFunction.isEmptyOrMagicValue()) { - (address plugin, uint8 functionId) = runtimeValidationFunction.unpack(); + if (!_isEmptyOrMagicValue(runtimeValidationPlugin)) { // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).runtimeValidationFunction(functionId, msg.sender, msg.value, msg.data) {} + try runtimeValidationPlugin.validateRuntime(msg.sender, msg.value, msg.data) {} catch (bytes memory revertReason) { - revert RuntimeValidationFunctionReverted(plugin, functionId, revertReason); + revert RuntimeValidationReverted(address(runtimeValidationPlugin), revertReason); } } else { - if (runtimeValidationFunction.isEmpty()) { - revert RuntimeValidationFunctionMissing(msg.sig); - } else if (runtimeValidationFunction.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) { + if (runtimeValidationPlugin == _NULL_PLUGIN) { + revert RuntimeValidationMissing(msg.sig); + } else if (runtimeValidationPlugin == _PRE_HOOK_ALWAYS_DENY) { revert InvalidConfiguration(); } // If _RUNTIME_VALIDATION_ALWAYS_ALLOW, just let the function finish. @@ -462,16 +461,7 @@ contract UpgradeableModularAccount is SelectorData storage selectorData = getAccountStorage().selectorData[selector]; uint256 preExecHooksLength = selectorData.preHooks.length(); uint256 postOnlyHooksLength = selectorData.postOnlyHooks.length(); - uint256 maxPostExecHooksLength = postOnlyHooksLength; - - // There can only be as many associated post hooks to run as there are pre hooks. - for (uint256 i = 0; i < preExecHooksLength;) { - (, uint256 count) = selectorData.preHooks.at(i); - unchecked { - maxPostExecHooksLength += (count + 1); - ++i; - } - } + uint256 maxPostExecHooksLength = postOnlyHooksLength + preExecHooksLength; // Overallocate on length - not all of this may get filled up. We set the correct length later. postHooksToRun = new PostExecToRun[](maxPostExecHooksLength); @@ -480,7 +470,7 @@ contract UpgradeableModularAccount is // Copy post-only hooks to the array. for (uint256 i = 0; i < postOnlyHooksLength;) { (bytes32 key,) = selectorData.postOnlyHooks.at(i); - postHooksToRun[actualPostHooksToRunLength].postExecHook = _toFunctionReference(key); + postHooksToRun[actualPostHooksToRunLength].postExecHookPlugin = toPlugin(key); unchecked { ++actualPostHooksToRunLength; ++i; @@ -491,27 +481,22 @@ contract UpgradeableModularAccount is // the array. for (uint256 i = 0; i < preExecHooksLength;) { (bytes32 key,) = selectorData.preHooks.at(i); - FunctionReference preExecHook = _toFunctionReference(key); + IPlugin preExecHookPlugin = toPlugin(key); - if (preExecHook.isEmptyOrMagicValue()) { + if (_isEmptyOrMagicValue(preExecHookPlugin)) { // The function reference must be PRE_HOOK_ALWAYS_DENY in this case, because zero and any other // magic value is unassignable here. revert AlwaysDenyRule(); } - bytes memory preExecHookReturnData = _runPreExecHook(preExecHook, data); + bytes memory preExecHookReturnData = _runPreExecHook(preExecHookPlugin, data); - uint256 associatedPostExecHooksLength = selectorData.associatedPostHooks[preExecHook].length(); - if (associatedPostExecHooksLength > 0) { - for (uint256 j = 0; j < associatedPostExecHooksLength;) { - (key,) = selectorData.associatedPostHooks[preExecHook].at(j); - postHooksToRun[actualPostHooksToRunLength].postExecHook = _toFunctionReference(key); - postHooksToRun[actualPostHooksToRunLength].preExecHookReturnData = preExecHookReturnData; + if (selectorData.hasAssociatedPostHook[preExecHookPlugin]) { + postHooksToRun[actualPostHooksToRunLength].postExecHookPlugin = preExecHookPlugin; + postHooksToRun[actualPostHooksToRunLength].preExecHookReturnData = preExecHookReturnData; - unchecked { - ++actualPostHooksToRunLength; - ++j; - } + unchecked { + ++actualPostHooksToRunLength; } } @@ -526,17 +511,14 @@ contract UpgradeableModularAccount is } } - function _runPreExecHook(FunctionReference preExecHook, bytes calldata data) + function _runPreExecHook(IPlugin preExecHookPlugin, bytes calldata data) internal returns (bytes memory preExecHookReturnData) { - (address plugin, uint8 functionId) = preExecHook.unpack(); - try IPlugin(plugin).preExecutionHook(functionId, msg.sender, msg.value, data) returns ( - bytes memory returnData - ) { + try preExecHookPlugin.preExecutionHook(msg.sender, msg.value, data) returns (bytes memory returnData) { preExecHookReturnData = returnData; } catch (bytes memory revertReason) { - revert PreExecHookReverted(plugin, functionId, revertReason); + revert PreExecHookReverted(address(preExecHookPlugin), revertReason); } } @@ -549,11 +531,10 @@ contract UpgradeableModularAccount is } PostExecToRun memory postHookToRun = postHooksToRun[i]; - (address plugin, uint8 functionId) = postHookToRun.postExecHook.unpack(); // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).postExecutionHook(functionId, postHookToRun.preExecHookReturnData) {} + try postHookToRun.postExecHookPlugin.postExecutionHook(postHookToRun.preExecHookReturnData) {} catch (bytes memory revertReason) { - revert PostExecHookReverted(plugin, functionId, revertReason); + revert PostExecHookReverted(address(postHookToRun.postExecHookPlugin), revertReason); } } } diff --git a/src/helpers/FunctionReferenceLib.sol b/src/helpers/FunctionReferenceLib.sol deleted file mode 100644 index 07a0abbc..00000000 --- a/src/helpers/FunctionReferenceLib.sol +++ /dev/null @@ -1,40 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.8.19; - -import {FunctionReference} from "../interfaces/IPluginManager.sol"; - -library FunctionReferenceLib { - // Empty or unset function reference. - FunctionReference internal constant _EMPTY_FUNCTION_REFERENCE = FunctionReference.wrap(bytes21(0)); - // Magic value for runtime validation functions that always allow access. - FunctionReference internal constant _RUNTIME_VALIDATION_ALWAYS_ALLOW = - FunctionReference.wrap(bytes21(uint168(1))); - // Magic value for hooks that should always revert. - FunctionReference internal constant _PRE_HOOK_ALWAYS_DENY = FunctionReference.wrap(bytes21(uint168(2))); - - function pack(address addr, uint8 functionId) internal pure returns (FunctionReference) { - return FunctionReference.wrap(bytes21(bytes20(addr)) | bytes21(uint168(functionId))); - } - - function unpack(FunctionReference fr) internal pure returns (address addr, uint8 functionId) { - bytes21 underlying = FunctionReference.unwrap(fr); - addr = address(bytes20(underlying)); - functionId = uint8(bytes1(underlying << 160)); - } - - function isEmpty(FunctionReference fr) internal pure returns (bool) { - return FunctionReference.unwrap(fr) == bytes21(0); - } - - function isEmptyOrMagicValue(FunctionReference fr) internal pure returns (bool) { - return FunctionReference.unwrap(fr) <= bytes21(uint168(2)); - } - - function eq(FunctionReference a, FunctionReference b) internal pure returns (bool) { - return FunctionReference.unwrap(a) == FunctionReference.unwrap(b); - } - - function notEq(FunctionReference a, FunctionReference b) internal pure returns (bool) { - return FunctionReference.unwrap(a) != FunctionReference.unwrap(b); - } -} diff --git a/src/interfaces/IAccountLoupe.sol b/src/interfaces/IAccountLoupe.sol index 25d50487..13be4c19 100644 --- a/src/interfaces/IAccountLoupe.sol +++ b/src/interfaces/IAccountLoupe.sol @@ -1,21 +1,18 @@ // SPDX-License-Identifier: CC0-1.0 pragma solidity ^0.8.19; -import {FunctionReference} from "../interfaces/IPluginManager.sol"; - interface IAccountLoupe { /// @notice Config for an execution function, given a selector. struct ExecutionFunctionConfig { address plugin; - FunctionReference userOpValidationFunction; - FunctionReference runtimeValidationFunction; + address validationPlugin; } /// @notice Pre and post hooks for a given selector. /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty. struct ExecutionHooks { - FunctionReference preExecHook; - FunctionReference postExecHook; + address preExecHookPlugin; + address postExecHookPlugin; } /// @notice Get the validation functions and plugin address for a selector. @@ -36,10 +33,7 @@ interface IAccountLoupe { function getPreValidationHooks(bytes4 selector) external view - returns ( - FunctionReference[] memory preUserOpValidationHooks, - FunctionReference[] memory preRuntimeValidationHooks - ); + returns (address[] memory preUserOpValidationHooks, address[] memory preRuntimeValidationHooks); /// @notice Get an array of all installed plugins. /// @return The addresses of all installed plugins. diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index cf74fd89..fff4066a 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -31,7 +31,6 @@ enum ManifestAssociatedFunctionType { /// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. struct ManifestFunction { ManifestAssociatedFunctionType functionType; - uint8 functionId; uint256 dependencyIndex; } @@ -66,7 +65,7 @@ struct PluginMetadata { // The author field SHOULD be a username representing the identity of the user or organization // that created this plugin. string author; - // String desciptions of the relative sensitivity of specific functions. The selectors MUST be selectors for + // String descriptions of the relative sensitivity of specific functions. The selectors MUST be selectors for // functions implemented by this plugin. SelectorPermission[] permissionDescriptors; } @@ -90,8 +89,7 @@ struct PluginManifest { // plugin MUST still be able to spend up to the balance that it sends to the account in the same call. bool canSpendNativeToken; ManifestExternalCallPermission[] permittedExternalCalls; - ManifestAssociatedFunction[] userOpValidationFunctions; - ManifestAssociatedFunction[] runtimeValidationFunctions; + ManifestAssociatedFunction[] validationFunctions; ManifestAssociatedFunction[] preUserOpValidationHooks; ManifestAssociatedFunction[] preRuntimeValidationHooks; ManifestExecutionHook[] executionHooks; @@ -110,65 +108,49 @@ interface IPlugin { /// account. function onUninstall(bytes calldata data) external; - /// @notice Run the pre user operation validation hook specified by the `functionId`. + /// @notice Run the pre user operation validation hook. /// @dev Pre user operation validation hooks MUST NOT return an authorizer value other than 0 or 1. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. /// @param userOp The user operation. /// @param userOpHash The user operation hash. /// @return Packed validation data for validAfter (6 bytes), validUntil (6 bytes), and authorizer (20 bytes). - function preUserOpValidationHook(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) + function preUserOpValidationHook(UserOperation calldata userOp, bytes32 userOpHash) external returns (uint256); - /// @notice Run the user operation validationFunction specified by the `functionId`. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. + /// @notice Run the user operation validation function. /// @param userOp The user operation. /// @param userOpHash The user operation hash. /// @return Packed validation data for validAfter (6 bytes), validUntil (6 bytes), and authorizer (20 bytes). - function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) - external - returns (uint256); + function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash) external returns (uint256); - /// @notice Run the pre runtime validation hook specified by the `functionId`. + /// @notice Run the pre runtime validation hook. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. /// @param sender The caller address. /// @param value The call value. /// @param data The calldata sent. - function preRuntimeValidationHook(uint8 functionId, address sender, uint256 value, bytes calldata data) - external; + function preRuntimeValidationHook(address sender, uint256 value, bytes calldata data) external; - /// @notice Run the runtime validationFunction specified by the `functionId`. + /// @notice Run the runtime validation function. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. /// @param sender The caller address. /// @param value The call value. /// @param data The calldata sent. - function runtimeValidationFunction(uint8 functionId, address sender, uint256 value, bytes calldata data) - external; + function validateRuntime(address sender, uint256 value, bytes calldata data) external; - /// @notice Run the pre execution hook specified by the `functionId`. + /// @notice Run the pre execution hook. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. /// @param sender The caller address. /// @param value The call value. /// @param data The calldata sent. /// @return Context to pass to a post execution hook, if present. An empty bytes array MAY be returned. - function preExecutionHook(uint8 functionId, address sender, uint256 value, bytes calldata data) + function preExecutionHook(address sender, uint256 value, bytes calldata data) external returns (bytes memory); - /// @notice Run the post execution hook specified by the `functionId`. + /// @notice Run the post execution hook. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. /// @param preExecHookData The context returned by its associated pre execution hook. - function postExecutionHook(uint8 functionId, bytes calldata preExecHookData) external; + function postExecutionHook(bytes calldata preExecHookData) external; /// @notice Describe the contents and intended configuration of the plugin. /// @dev This manifest MUST stay constant over time. diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index 21054d27..d2c6c36c 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.19; type FunctionReference is bytes21; interface IPluginManager { - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash, address[] dependencies); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded); @@ -19,7 +19,7 @@ interface IPluginManager { address plugin, bytes32 manifestHash, bytes calldata pluginInstallData, - FunctionReference[] calldata dependencies + address[] calldata dependencies ) external; /// @notice Uninstall a plugin from the modular account. diff --git a/src/plugins/BasePlugin.sol b/src/plugins/BasePlugin.sol index 5dbcb2b3..ffb15d74 100644 --- a/src/plugins/BasePlugin.sol +++ b/src/plugins/BasePlugin.sol @@ -13,122 +13,72 @@ import {IPlugin, PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol abstract contract BasePlugin is ERC165, IPlugin { error NotImplemented(); - /// @notice Initialize plugin data for the modular account. - /// @dev Called by the modular account during `installPlugin`. - /// @param data Optional bytes array to be decoded and used by the plugin to setup initial plugin data for the - /// modular account. + /// @inheritdoc IPlugin function onInstall(bytes calldata data) external virtual { (data); revert NotImplemented(); } - /// @notice Clear plugin data for the modular account. - /// @dev Called by the modular account during `uninstallPlugin`. - /// @param data Optional bytes array to be decoded and used by the plugin to clear plugin data for the modular - /// account. + /// @inheritdoc IPlugin function onUninstall(bytes calldata data) external virtual { (data); revert NotImplemented(); } - /// @notice Run the pre user operation validation hook specified by the `functionId`. - /// @dev Pre user operation validation hooks MUST NOT return an authorizer value other than 0 or 1. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. - /// @param userOp The user operation. - /// @param userOpHash The user operation hash. - /// @return Packed validation data for validAfter (6 bytes), validUntil (6 bytes), and authorizer (20 bytes). - function preUserOpValidationHook(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) + /// @inheritdoc IPlugin + function preUserOpValidationHook(UserOperation calldata userOp, bytes32 userOpHash) external virtual returns (uint256) { - (functionId, userOp, userOpHash); + (userOp, userOpHash); revert NotImplemented(); } - /// @notice Run the user operation validationFunction specified by the `functionId`. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. - /// @param userOp The user operation. - /// @param userOpHash The user operation hash. - /// @return Packed validation data for validAfter (6 bytes), validUntil (6 bytes), and authorizer (20 bytes). - function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) + /// @inheritdoc IPlugin + function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash) external virtual returns (uint256) { - (functionId, userOp, userOpHash); + (userOp, userOpHash); revert NotImplemented(); } - /// @notice Run the pre runtime validation hook specified by the `functionId`. - /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. - /// @param sender The caller address. - /// @param value The call value. - /// @param data The calldata sent. - function preRuntimeValidationHook(uint8 functionId, address sender, uint256 value, bytes calldata data) - external - virtual - { - (functionId, sender, value, data); + /// @inheritdoc IPlugin + function preRuntimeValidationHook(address sender, uint256 value, bytes calldata data) external virtual { + (sender, value, data); revert NotImplemented(); } - /// @notice Run the runtime validationFunction specified by the `functionId`. - /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. - /// @param sender The caller address. - /// @param value The call value. - /// @param data The calldata sent. - function runtimeValidationFunction(uint8 functionId, address sender, uint256 value, bytes calldata data) - external - virtual - { - (functionId, sender, value, data); + /// @inheritdoc IPlugin + function validateRuntime(address sender, uint256 value, bytes calldata data) external virtual { + (sender, value, data); revert NotImplemented(); } - /// @notice Run the pre execution hook specified by the `functionId`. - /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. - /// @param sender The caller address. - /// @param value The call value. - /// @param data The calldata sent. - /// @return Context to pass to a post execution hook, if present. An empty bytes array MAY be returned. - function preExecutionHook(uint8 functionId, address sender, uint256 value, bytes calldata data) + /// @inheritdoc IPlugin + function preExecutionHook(address sender, uint256 value, bytes calldata data) external virtual returns (bytes memory) { - (functionId, sender, value, data); + (sender, value, data); revert NotImplemented(); } - /// @notice Run the post execution hook specified by the `functionId`. - /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. - /// @param preExecHookData The context returned by its associated pre execution hook. - function postExecutionHook(uint8 functionId, bytes calldata preExecHookData) external virtual { - (functionId, preExecHookData); + /// @inheritdoc IPlugin + function postExecutionHook(bytes calldata preExecHookData) external virtual { + (preExecHookData); revert NotImplemented(); } - /// @notice Describe the contents and intended configuration of the plugin. - /// @dev This manifest MUST stay constant over time. - /// @return A manifest describing the contents and intended configuration of the plugin. + /// @inheritdoc IPlugin function pluginManifest() external pure virtual returns (PluginManifest memory) { revert NotImplemented(); } - /// @notice Describe the metadata of the plugin. - /// @dev This metadata MUST stay constant over time. - /// @return A metadata struct describing the plugin. + /// @inheritdoc IPlugin function pluginMetadata() external pure virtual returns (PluginMetadata memory); /// @dev Returns true if this contract implements the interface defined by diff --git a/src/plugins/TokenReceiverPlugin.sol b/src/plugins/TokenReceiverPlugin.sol index ac9335fd..b9237179 100644 --- a/src/plugins/TokenReceiverPlugin.sol +++ b/src/plugins/TokenReceiverPlugin.sol @@ -81,23 +81,22 @@ contract TokenReceiverPlugin is BasePlugin, IERC721Receiver, IERC777Recipient, I // Only runtime validationFunction is needed since callbacks come from token contracts only ManifestFunction memory alwaysAllowFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, // Unused. dependencyIndex: 0 // Unused. }); - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](4); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](4); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.tokensReceived.selector, associatedFunction: alwaysAllowFunction }); - manifest.runtimeValidationFunctions[1] = ManifestAssociatedFunction({ + manifest.validationFunctions[1] = ManifestAssociatedFunction({ executionSelector: this.onERC721Received.selector, associatedFunction: alwaysAllowFunction }); - manifest.runtimeValidationFunctions[2] = ManifestAssociatedFunction({ + manifest.validationFunctions[2] = ManifestAssociatedFunction({ executionSelector: this.onERC1155Received.selector, associatedFunction: alwaysAllowFunction }); - manifest.runtimeValidationFunctions[3] = ManifestAssociatedFunction({ + manifest.validationFunctions[3] = ManifestAssociatedFunction({ executionSelector: this.onERC1155BatchReceived.selector, associatedFunction: alwaysAllowFunction }); diff --git a/src/plugins/owner/ISingleOwnerPlugin.sol b/src/plugins/owner/ISingleOwnerPlugin.sol index c75ab8a3..02cb0168 100644 --- a/src/plugins/owner/ISingleOwnerPlugin.sol +++ b/src/plugins/owner/ISingleOwnerPlugin.sol @@ -4,11 +4,6 @@ pragma solidity ^0.8.19; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; interface ISingleOwnerPlugin { - enum FunctionId { - RUNTIME_VALIDATION_OWNER_OR_SELF, - USER_OP_VALIDATION_OWNER - } - /// @notice This event is emitted when ownership of the account changes. /// @param account The account whose ownership changed. /// @param previousOwner The address of the previous owner. diff --git a/src/plugins/owner/SingleOwnerPlugin.sol b/src/plugins/owner/SingleOwnerPlugin.sol index fa8ed177..b6427c3f 100644 --- a/src/plugins/owner/SingleOwnerPlugin.sol +++ b/src/plugins/owner/SingleOwnerPlugin.sol @@ -103,37 +103,27 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { } /// @inheritdoc BasePlugin - function runtimeValidationFunction(uint8 functionId, address sender, uint256, bytes calldata) - external - view - override - { - if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)) { - // Validate that the sender is the owner of the account or self. - if (sender != _owners[msg.sender] && sender != msg.sender) { - revert NotAuthorized(); - } - return; + function validateRuntime(address sender, uint256, bytes calldata) external view override { + // Validate that the sender is the owner of the account or self. + if (sender != _owners[msg.sender] && sender != msg.sender) { + revert NotAuthorized(); } - revert NotImplemented(); + return; } /// @inheritdoc BasePlugin - function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) + function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash) external view override returns (uint256) { - if (functionId == uint8(FunctionId.USER_OP_VALIDATION_OWNER)) { - // Validate the user op signature against the owner. - (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); - if (signer == address(0) || signer != _owners[msg.sender]) { - return _SIG_VALIDATION_FAILED; - } - return _SIG_VALIDATION_PASSED; + // Validate the user op signature against the owner. + (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); + if (signer == address(0) || signer != _owners[msg.sender]) { + return _SIG_VALIDATION_FAILED; } - revert NotImplemented(); + return _SIG_VALIDATION_PASSED; } /// @inheritdoc BasePlugin @@ -145,85 +135,45 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { manifest.executionFunctions[1] = this.isValidSignature.selector; manifest.executionFunctions[2] = this.owner.selector; - ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ + ManifestFunction memory ownerValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.USER_OP_VALIDATION_OWNER), dependencyIndex: 0 // Unused. }); - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](7); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](8); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.transferOwnership.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[1] = ManifestAssociatedFunction({ + manifest.validationFunctions[1] = ManifestAssociatedFunction({ executionSelector: IStandardExecutor.execute.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[2] = ManifestAssociatedFunction({ + manifest.validationFunctions[2] = ManifestAssociatedFunction({ executionSelector: IStandardExecutor.executeBatch.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[3] = ManifestAssociatedFunction({ + manifest.validationFunctions[3] = ManifestAssociatedFunction({ executionSelector: IPluginManager.installPlugin.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[4] = ManifestAssociatedFunction({ + manifest.validationFunctions[4] = ManifestAssociatedFunction({ executionSelector: IPluginManager.uninstallPlugin.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[5] = ManifestAssociatedFunction({ + manifest.validationFunctions[5] = ManifestAssociatedFunction({ executionSelector: UUPSUpgradeable.upgradeTo.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[6] = ManifestAssociatedFunction({ + manifest.validationFunctions[6] = ManifestAssociatedFunction({ executionSelector: UUPSUpgradeable.upgradeToAndCall.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - ManifestFunction memory ownerOrSelfRuntimeValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF), - dependencyIndex: 0 // Unused. - }); ManifestFunction memory alwaysAllowFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, // Unused. dependencyIndex: 0 // Unused. }); - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](9); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.transferOwnership.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: this.owner.selector, - associatedFunction: alwaysAllowFunction - }); - manifest.runtimeValidationFunctions[2] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.execute.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[3] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.executeBatch.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[4] = ManifestAssociatedFunction({ - executionSelector: IPluginManager.installPlugin.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[5] = ManifestAssociatedFunction({ - executionSelector: IPluginManager.uninstallPlugin.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[6] = ManifestAssociatedFunction({ - executionSelector: UUPSUpgradeable.upgradeTo.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[7] = ManifestAssociatedFunction({ - executionSelector: UUPSUpgradeable.upgradeToAndCall.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[8] = ManifestAssociatedFunction({ + manifest.validationFunctions[7] = ManifestAssociatedFunction({ executionSelector: this.isValidSignature.selector, associatedFunction: alwaysAllowFunction }); diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index e4a4d02b..47b9cd99 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -186,53 +186,42 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { } /// @inheritdoc BasePlugin - function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) + function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash) external view override returns (uint256) { - if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { - (address signer, ECDSA.RecoverError err) = - userOpHash.toEthSignedMessageHash().tryRecover(userOp.signature); - if (err != ECDSA.RecoverError.NoError) { - revert InvalidSignature(); - } - bytes4 selector = bytes4(userOp.callData[0:4]); - bytes memory key = msg.sender.allocateAssociatedStorageKey(0, 1); - StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(signer, selector))); - SessionInfo storage duration = _castPtrToStruct(ptr); - uint48 validAfter = duration.validAfter; - uint48 validUntil = duration.validUntil; - - return _packValidationData(validUntil == 0, validUntil, validAfter); + (address signer, ECDSA.RecoverError err) = userOpHash.toEthSignedMessageHash().tryRecover(userOp.signature); + if (err != ECDSA.RecoverError.NoError) { + revert InvalidSignature(); } - revert NotImplemented(); + bytes4 selector = bytes4(userOp.callData[0:4]); + bytes memory key = msg.sender.allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(signer, selector))); + SessionInfo storage duration = _castPtrToStruct(ptr); + uint48 validAfter = duration.validAfter; + uint48 validUntil = duration.validUntil; + + return _packValidationData(validUntil == 0, validUntil, validAfter); } /// @inheritdoc BasePlugin - function runtimeValidationFunction(uint8 functionId, address sender, uint256, bytes calldata data) - external - view - override - { - if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER)) { - bytes4 selector = bytes4(data[0:4]); - bytes memory key = msg.sender.allocateAssociatedStorageKey(0, 1); - StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sender, selector))); - SessionInfo storage duration = _castPtrToStruct(ptr); - uint48 validAfter = duration.validAfter; - uint48 validUntil = duration.validUntil; - - if (validUntil != 0) { - if (block.timestamp < validAfter || block.timestamp > validUntil) { - revert WrongTimeRangeForSession(); - } - return; + function validateRuntime(address sender, uint256, bytes calldata data) external view override { + bytes4 selector = bytes4(data[0:4]); + bytes memory key = msg.sender.allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sender, selector))); + SessionInfo storage duration = _castPtrToStruct(ptr); + uint48 validAfter = duration.validAfter; + uint48 validUntil = duration.validUntil; + + if (validUntil != 0) { + if (block.timestamp < validAfter || block.timestamp > validUntil) { + revert WrongTimeRangeForSession(); } - revert NotAuthorized(); + return; } - revert NotImplemented(); + revert NotAuthorized(); } /// @inheritdoc BasePlugin @@ -245,65 +234,40 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { manifest.executionFunctions[2] = this.addSessionKeyBatch.selector; manifest.executionFunctions[3] = this.removeSessionKeyBatch.selector; - ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ + ManifestFunction memory ownerValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, // Unused. dependencyIndex: 0 // Used as first index. }); - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](4); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](5); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.addSessionKey.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[1] = ManifestAssociatedFunction({ + manifest.validationFunctions[1] = ManifestAssociatedFunction({ executionSelector: this.removeSessionKey.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[2] = ManifestAssociatedFunction({ + manifest.validationFunctions[2] = ManifestAssociatedFunction({ executionSelector: this.addSessionKeyBatch.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - manifest.userOpValidationFunctions[3] = ManifestAssociatedFunction({ + manifest.validationFunctions[3] = ManifestAssociatedFunction({ executionSelector: this.removeSessionKeyBatch.selector, - associatedFunction: ownerUserOpValidationFunction + associatedFunction: ownerValidationFunction }); - ManifestFunction memory ownerOrSelfRuntimeValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, // Unused. - dependencyIndex: 1 - }); ManifestFunction memory alwaysAllowFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, // Unused. dependencyIndex: 0 // Unused. }); - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](5); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.addSessionKey.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: this.removeSessionKey.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[2] = ManifestAssociatedFunction({ - executionSelector: this.addSessionKeyBatch.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[3] = ManifestAssociatedFunction({ - executionSelector: this.removeSessionKeyBatch.selector, - associatedFunction: ownerOrSelfRuntimeValidationFunction - }); - manifest.runtimeValidationFunctions[4] = ManifestAssociatedFunction({ + manifest.validationFunctions[4] = ManifestAssociatedFunction({ executionSelector: this.getSessionDuration.selector, associatedFunction: alwaysAllowFunction }); - manifest.dependencyInterfaceIds = new bytes4[](2); + manifest.dependencyInterfaceIds = new bytes4[](1); manifest.dependencyInterfaceIds[0] = type(ISingleOwnerPlugin).interfaceId; - manifest.dependencyInterfaceIds[1] = type(ISingleOwnerPlugin).interfaceId; return manifest; } diff --git a/src/samples/plugins/TokenSessionKeyPlugin.sol b/src/samples/plugins/TokenSessionKeyPlugin.sol index 9e2e8abd..e4361d27 100644 --- a/src/samples/plugins/TokenSessionKeyPlugin.sol +++ b/src/samples/plugins/TokenSessionKeyPlugin.sol @@ -68,32 +68,19 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.transferFromSessionKey.selector; - ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ + ManifestFunction memory tempOwnerValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, // Unused dependencyIndex: 0 // Used as first index }); - ManifestFunction memory tempOwnerRuntimeValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, // Unused - dependencyIndex: 1 // Used as second index - }); - - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.transferFromSessionKey.selector, - associatedFunction: tempOwnerUserOpValidationFunction - }); - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.transferFromSessionKey.selector, - associatedFunction: tempOwnerRuntimeValidationFunction + associatedFunction: tempOwnerValidationFunction }); - manifest.dependencyInterfaceIds = new bytes4[](2); + manifest.dependencyInterfaceIds = new bytes4[](1); manifest.dependencyInterfaceIds[0] = type(IModularSessionKeyPlugin).interfaceId; - manifest.dependencyInterfaceIds[1] = type(IModularSessionKeyPlugin).interfaceId; bytes4[] memory permittedExternalSelectors = new bytes4[](1); permittedExternalSelectors[0] = TRANSFERFROM_SELECTOR; diff --git a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol index 88d31f1a..9beacffc 100644 --- a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -4,11 +4,6 @@ pragma solidity ^0.8.19; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; interface IModularSessionKeyPlugin { - enum FunctionId { - RUNTIME_VALIDATION_TEMPORARY_OWNER, - USER_OP_VALIDATION_TEMPORARY_OWNER - } - /// @notice This event is emitted when a session key is added to the account. /// @param account The account whose session key is updated. /// @param sessionKey The address of the session key. diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index 9f9de2ce..f56860ea 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -105,11 +105,8 @@ Each step is modular, supporting different implementations for each execution fu Plugin manager interface. Modular Smart Contract Accounts **MUST** implement this interface to support installing and uninstalling plugins. ```solidity -// Treats the first 20 bytes as an address, and the last byte as a function identifier. -type FunctionReference is bytes21; - interface IPluginManager { - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash, address[] dependencies); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSucceeded); @@ -124,7 +121,7 @@ interface IPluginManager { address plugin, bytes32 manifestHash, bytes calldata pluginInstallData, - FunctionReference[] calldata dependencies + address[] calldata dependencies ) external; /// @notice Uninstall a plugin from the modular account. @@ -135,7 +132,6 @@ interface IPluginManager { /// modular account. function uninstallPlugin(address plugin, bytes calldata config, bytes calldata pluginUninstallData) external; } - ``` #### `IStandardExecutor.sol` @@ -215,15 +211,14 @@ interface IAccountLoupe { /// @notice Config for an execution function, given a selector. struct ExecutionFunctionConfig { address plugin; - FunctionReference userOpValidationFunction; - FunctionReference runtimeValidationFunction; + address validationPlugin; } /// @notice Pre and post hooks for a given selector. /// @dev It's possible for one of either `preExecHook` or `postExecHook` to be empty. struct ExecutionHooks { - FunctionReference preExecHook; - FunctionReference postExecHook; + address preExecHookPlugin; + address postExecHookPlugin; } /// @notice Get the validation functions and plugin address for a selector. @@ -244,10 +239,7 @@ interface IAccountLoupe { function getPreValidationHooks(bytes4 selector) external view - returns ( - FunctionReference[] memory preUserOpValidationHooks, - FunctionReference[] memory preRuntimeValidationHooks - ); + returns (address[] memory preUserOpValidationHooks, address[] memory preRuntimeValidationHooks); /// @notice Get an array of all installed plugins. /// @return The addresses of all installed plugins. @@ -271,56 +263,45 @@ interface IPlugin { /// @param data Optional bytes array to be decoded and used by the plugin to clear plugin data for the modular account. function onUninstall(bytes calldata data) external; - /// @notice Run the pre user operation validation hook specified by the `functionId`. + /// @notice Run the pre user operation validation hook. /// @dev Pre user operation validation hooks MUST NOT return an authorizer value other than 0 or 1. - /// @param functionId An identifier that routes the call to different internal implementations, should there be more than one. /// @param userOp The user operation. /// @param userOpHash The user operation hash. /// @return Packed validation data for validAfter (6 bytes), validUntil (6 bytes), and authorizer (20 bytes). - function preUserOpValidationHook(uint8 functionId, UserOperation memory userOp, bytes32 userOpHash) external returns (uint256); + function preUserOpValidationHook(UserOperation calldata userOp, bytes32 userOpHash) external returns (uint256); - /// @notice Run the user operation validationFunction specified by the `functionId`. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. + /// @notice Run the user operation validation function. /// @param userOp The user operation. /// @param userOpHash The user operation hash. /// @return Packed validation data for validAfter (6 bytes), validUntil (6 bytes), and authorizer (20 bytes). - function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) - external - returns (uint256); + function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash) external returns (uint256); - /// @notice Run the pre runtime validation hook specified by the `functionId`. + /// @notice Run the pre runtime validation hook. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be more than one. /// @param sender The caller address. /// @param value The call value. /// @param data The calldata sent. - function preRuntimeValidationHook(uint8 functionId, address sender, uint256 value, bytes calldata data) external; + function preRuntimeValidationHook(address sender, uint256 value, bytes calldata data) external; - /// @notice Run the runtime validationFunction specified by the `functionId`. + /// @notice Run the runtime validation function. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be - /// more than one. /// @param sender The caller address. /// @param value The call value. /// @param data The calldata sent. - function runtimeValidationFunction(uint8 functionId, address sender, uint256 value, bytes calldata data) - external; + function validateRuntime(address sender, uint256 value, bytes calldata data) external; - /// @notice Run the pre execution hook specified by the `functionId`. + /// @notice Run the pre execution hook. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be more than one. /// @param sender The caller address. /// @param value The call value. /// @param data The calldata sent. /// @return Context to pass to a post execution hook, if present. An empty bytes array MAY be returned. - function preExecutionHook(uint8 functionId, address sender, uint256 value, bytes calldata data) external returns (bytes memory); + function preExecutionHook(address sender, uint256 value, bytes calldata data) external returns (bytes memory); - /// @notice Run the post execution hook specified by the `functionId`. + /// @notice Run the post execution hook. /// @dev To indicate the entire call should revert, the function MUST revert. - /// @param functionId An identifier that routes the call to different internal implementations, should there be more than one. /// @param preExecHookData The context returned by its associated pre execution hook. - function postExecutionHook(uint8 functionId, bytes calldata preExecHookData) external; + function postExecutionHook(bytes calldata preExecHookData) external; /// @notice Describe the contents and intended configuration of the plugin. /// @dev This manifest MUST stay constant over time. @@ -364,7 +345,6 @@ enum ManifestAssociatedFunctionType { /// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. struct ManifestFunction { ManifestAssociatedFunctionType functionType; - uint8 functionId; uint256 dependencyIndex; } @@ -423,8 +403,7 @@ struct PluginManifest { // plugin MUST still be able to spend up to the balance that it sends to the account in the same call. bool canSpendNativeToken; ManifestExternalCallPermission[] permittedExternalCalls; - ManifestAssociatedFunction[] userOpValidationFunctions; - ManifestAssociatedFunction[] runtimeValidationFunctions; + ManifestAssociatedFunction[] validationFunctions; ManifestAssociatedFunction[] preUserOpValidationHooks; ManifestAssociatedFunction[] preRuntimeValidationHooks; ManifestExecutionHook[] executionHooks; diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index a9b23875..f5f71adb 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -15,7 +15,6 @@ import { import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; import {MockPlugin} from "../mocks/MockPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; @@ -36,15 +35,11 @@ contract AccountExecHooksTest is OptimizedTest { bytes32 public manifestHash2; bytes4 internal constant _EXEC_SELECTOR = bytes4(uint32(1)); - uint8 internal constant _PRE_HOOK_FUNCTION_ID_1 = 1; - uint8 internal constant _POST_HOOK_FUNCTION_ID_2 = 2; - uint8 internal constant _PRE_HOOK_FUNCTION_ID_3 = 3; - uint8 internal constant _POST_HOOK_FUNCTION_ID_4 = 4; PluginManifest public m1; PluginManifest public m2; - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash, address[] dependencies); event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); // emitted by MockPlugin event ReceivedCall(bytes msgData, uint256 msgValue); @@ -60,12 +55,11 @@ contract AccountExecHooksTest is OptimizedTest { m1.executionFunctions.push(_EXEC_SELECTOR); - m1.runtimeValidationFunctions.push( + m1.validationFunctions.push( ManifestAssociatedFunction({ executionSelector: _EXEC_SELECTOR, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }) @@ -75,12 +69,8 @@ contract AccountExecHooksTest is OptimizedTest { function test_preExecHook_install() public { _installPlugin1WithHooks( _EXEC_SELECTOR, - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: _PRE_HOOK_FUNCTION_ID_1, - dependencyIndex: 0 - }), - ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, functionId: 0, dependencyIndex: 0}) + ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}), + ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, dependencyIndex: 0}) ); } @@ -93,7 +83,6 @@ contract AccountExecHooksTest is OptimizedTest { emit ReceivedCall( abi.encodeWithSelector( IPlugin.preExecutionHook.selector, - _PRE_HOOK_FUNCTION_ID_1, address(this), // caller 0, // msg.value in call to account abi.encodeWithSelector(_EXEC_SELECTOR) @@ -114,16 +103,8 @@ contract AccountExecHooksTest is OptimizedTest { function test_execHookPair_install() public { _installPlugin1WithHooks( _EXEC_SELECTOR, - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: _PRE_HOOK_FUNCTION_ID_1, - dependencyIndex: 0 - }), - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: _POST_HOOK_FUNCTION_ID_2, - dependencyIndex: 0 - }) + ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}), + ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}) ); } @@ -137,7 +118,6 @@ contract AccountExecHooksTest is OptimizedTest { emit ReceivedCall( abi.encodeWithSelector( IPlugin.preExecutionHook.selector, - _PRE_HOOK_FUNCTION_ID_1, address(this), // caller 0, // msg.value in call to account abi.encodeWithSelector(_EXEC_SELECTOR) @@ -150,7 +130,7 @@ contract AccountExecHooksTest is OptimizedTest { vm.expectEmit(true, true, true, true); // post hook call emit ReceivedCall( - abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + abi.encodeCall(IPlugin.postExecutionHook, ("")), 0 // msg value in call to plugin ); @@ -167,12 +147,8 @@ contract AccountExecHooksTest is OptimizedTest { function test_postOnlyExecHook_install() public { _installPlugin1WithHooks( _EXEC_SELECTOR, - ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, functionId: 0, dependencyIndex: 0}), - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: _POST_HOOK_FUNCTION_ID_2, - dependencyIndex: 0 - }) + ManifestFunction({functionType: ManifestAssociatedFunctionType.NONE, dependencyIndex: 0}), + ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}) ); } @@ -183,7 +159,7 @@ contract AccountExecHooksTest is OptimizedTest { vm.expectEmit(true, true, true, true); emit ReceivedCall( - abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + abi.encodeCall(IPlugin.postExecutionHook, ("")), 0 // msg value in call to plugin ); @@ -203,10 +179,9 @@ contract AccountExecHooksTest is OptimizedTest { _EXEC_SELECTOR, ManifestFunction({ functionType: ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY, - functionId: 0, dependencyIndex: 0 }), - ManifestFunction(ManifestAssociatedFunctionType.NONE, 0, 0) + ManifestFunction(ManifestAssociatedFunctionType.NONE, 0) ); // Install a second plugin that applies the same pre hook on the same selector. @@ -214,11 +189,10 @@ contract AccountExecHooksTest is OptimizedTest { _EXEC_SELECTOR, ManifestFunction({ functionType: ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY, - functionId: 0, dependencyIndex: 0 }), - ManifestFunction(ManifestAssociatedFunctionType.NONE, 0, 0), - new FunctionReference[](0) + ManifestFunction(ManifestAssociatedFunctionType.NONE, 0), + new address[](0) ); vm.stopPrank(); @@ -251,36 +225,20 @@ contract AccountExecHooksTest is OptimizedTest { // Install the first plugin. _installPlugin1WithHooks( _EXEC_SELECTOR, - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: _PRE_HOOK_FUNCTION_ID_1, - dependencyIndex: 0 - }), - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: _POST_HOOK_FUNCTION_ID_2, - dependencyIndex: 0 - }) + ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}), + ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}) ); // Attempt to install a second plugin that applies the first plugin's hook pair (as dependencies) to the // same selector. This should revert. - FunctionReference[] memory dependencies = new FunctionReference[](2); - dependencies[0] = FunctionReferenceLib.pack(address(mockPlugin1), _PRE_HOOK_FUNCTION_ID_1); - dependencies[1] = FunctionReferenceLib.pack(address(mockPlugin1), _POST_HOOK_FUNCTION_ID_2); + address[] memory dependencies = new address[](2); + dependencies[0] = address(mockPlugin1); + dependencies[1] = address(mockPlugin1); _installPlugin2WithHooksExpectFail( _EXEC_SELECTOR, - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, - dependencyIndex: 0 - }), - ManifestFunction({ - functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, - dependencyIndex: 1 - }), + ManifestFunction({functionType: ManifestAssociatedFunctionType.DEPENDENCY, dependencyIndex: 0}), + ManifestFunction({functionType: ManifestAssociatedFunctionType.DEPENDENCY, dependencyIndex: 1}), dependencies, abi.encodePacked(PluginManagerInternals.InvalidPluginManifest.selector) ); @@ -300,13 +258,13 @@ contract AccountExecHooksTest is OptimizedTest { 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, new address[](0)); account.installPlugin({ plugin: address(mockPlugin1), manifestHash: manifestHash1, pluginInstallData: bytes(""), - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -314,7 +272,7 @@ contract AccountExecHooksTest is OptimizedTest { bytes4 selector, ManifestFunction memory preHook, ManifestFunction memory postHook, - FunctionReference[] memory dependencies + address[] memory dependencies ) internal { if (preHook.functionType == ManifestAssociatedFunctionType.DEPENDENCY) { m2.dependencyInterfaceIds.push(type(IPlugin).interfaceId); @@ -345,7 +303,7 @@ contract AccountExecHooksTest is OptimizedTest { bytes4 selector, ManifestFunction memory preHook, ManifestFunction memory postHook, - FunctionReference[] memory dependencies, + address[] memory dependencies, bytes memory revertData ) internal { if (preHook.functionType == ManifestAssociatedFunctionType.DEPENDENCY) { diff --git a/test/account/AccountLoupe.t.sol b/test/account/AccountLoupe.t.sol index 17f66788..5ad64802 100644 --- a/test/account/AccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -5,7 +5,6 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeab import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; import { ManifestAssociatedFunctionType, ManifestExecutionHook, @@ -13,9 +12,9 @@ import { PluginManifest } from "../../src/interfaces/IPlugin.sol"; import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol"; +import {IPlugin} from "../../src/interfaces/IPlugin.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {IStandardExecutor} from "../../src/interfaces/IStandardExecutor.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; @@ -31,8 +30,7 @@ contract AccountLoupeTest is OptimizedTest { UpgradeableModularAccount public account1; - FunctionReference public ownerUserOpValidation; - FunctionReference public ownerRuntimeValidation; + IPlugin public ownerValidation; event ReceivedCall(bytes msgData, uint256 msgValue); @@ -46,14 +44,9 @@ contract AccountLoupeTest is OptimizedTest { account1 = factory.createAccount(address(this), 0); bytes32 manifestHash = keccak256(abi.encode(comprehensivePlugin.pluginManifest())); - account1.installPlugin(address(comprehensivePlugin), manifestHash, "", new FunctionReference[](0)); - - ownerUserOpValidation = FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER) - ); - ownerRuntimeValidation = FunctionReferenceLib.pack( - address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF) - ); + account1.installPlugin(address(comprehensivePlugin), manifestHash, "", new address[](0)); + + ownerValidation = IPlugin(singleOwnerPlugin); } function test_pluginLoupe_getInstalledPlugins_initial() public { @@ -67,78 +60,51 @@ contract AccountLoupeTest is OptimizedTest { function test_pluginLoupe_getExecutionFunctionConfig_native() public { bytes4[] memory selectorsToCheck = new bytes4[](5); - FunctionReference[] memory expectedUserOpValidations = new FunctionReference[](5); - FunctionReference[] memory expectedRuntimeValidations = new FunctionReference[](5); + IPlugin[] memory expectedValidations = new IPlugin[](5); selectorsToCheck[0] = IStandardExecutor.execute.selector; - expectedUserOpValidations[0] = ownerUserOpValidation; - expectedRuntimeValidations[0] = ownerRuntimeValidation; + expectedValidations[0] = ownerValidation; selectorsToCheck[1] = IStandardExecutor.executeBatch.selector; - expectedUserOpValidations[1] = ownerUserOpValidation; - expectedRuntimeValidations[1] = ownerRuntimeValidation; + expectedValidations[1] = ownerValidation; selectorsToCheck[2] = UUPSUpgradeable.upgradeToAndCall.selector; - expectedUserOpValidations[2] = ownerUserOpValidation; - expectedRuntimeValidations[2] = ownerRuntimeValidation; + expectedValidations[2] = ownerValidation; selectorsToCheck[3] = IPluginManager.installPlugin.selector; - expectedUserOpValidations[3] = ownerUserOpValidation; - expectedRuntimeValidations[3] = ownerRuntimeValidation; + expectedValidations[3] = ownerValidation; selectorsToCheck[4] = IPluginManager.uninstallPlugin.selector; - expectedUserOpValidations[4] = ownerUserOpValidation; - expectedRuntimeValidations[4] = ownerRuntimeValidation; + expectedValidations[4] = ownerValidation; for (uint256 i = 0; i < selectorsToCheck.length; i++) { IAccountLoupe.ExecutionFunctionConfig memory config = account1.getExecutionFunctionConfig(selectorsToCheck[i]); assertEq(config.plugin, address(account1)); - assertEq( - FunctionReference.unwrap(config.userOpValidationFunction), - FunctionReference.unwrap(expectedUserOpValidations[i]) - ); - assertEq( - FunctionReference.unwrap(config.runtimeValidationFunction), - FunctionReference.unwrap(expectedRuntimeValidations[i]) - ); + assertEq(config.validationPlugin, address(expectedValidations[i])); } } function test_pluginLoupe_getExecutionFunctionConfig_plugin() public { bytes4[] memory selectorsToCheck = new bytes4[](2); address[] memory expectedPluginAddress = new address[](2); - FunctionReference[] memory expectedUserOpValidations = new FunctionReference[](2); - FunctionReference[] memory expectedRuntimeValidations = new FunctionReference[](2); + IPlugin[] memory expectedValidations = new IPlugin[](2); selectorsToCheck[0] = comprehensivePlugin.foo.selector; expectedPluginAddress[0] = address(comprehensivePlugin); - expectedUserOpValidations[0] = FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.USER_OP_VALIDATION) - ); - expectedRuntimeValidations[0] = FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.RUNTIME_VALIDATION) - ); + expectedValidations[0] = IPlugin(comprehensivePlugin); selectorsToCheck[1] = singleOwnerPlugin.transferOwnership.selector; expectedPluginAddress[1] = address(singleOwnerPlugin); - expectedUserOpValidations[1] = ownerUserOpValidation; - expectedRuntimeValidations[1] = ownerRuntimeValidation; + expectedValidations[1] = ownerValidation; for (uint256 i = 0; i < selectorsToCheck.length; i++) { IAccountLoupe.ExecutionFunctionConfig memory config = account1.getExecutionFunctionConfig(selectorsToCheck[i]); assertEq(config.plugin, expectedPluginAddress[i]); - assertEq( - FunctionReference.unwrap(config.userOpValidationFunction), - FunctionReference.unwrap(expectedUserOpValidations[i]) - ); - assertEq( - FunctionReference.unwrap(config.runtimeValidationFunction), - FunctionReference.unwrap(expectedRuntimeValidations[i]) - ); + assertEq(config.validationPlugin, address(expectedValidations[i])); } } @@ -146,22 +112,8 @@ contract AccountLoupeTest is OptimizedTest { IAccountLoupe.ExecutionHooks[] memory hooks = account1.getExecutionHooks(comprehensivePlugin.foo.selector); assertEq(hooks.length, 1); - assertEq( - FunctionReference.unwrap(hooks[0].preExecHook), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.PRE_EXECUTION_HOOK) - ) - ) - ); - assertEq( - FunctionReference.unwrap(hooks[0].postExecHook), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.POST_EXECUTION_HOOK) - ) - ) - ); + assertEq(hooks[0].preExecHookPlugin, address(comprehensivePlugin)); + assertEq(hooks[0].postExecHookPlugin, address(comprehensivePlugin)); } function test_pluginLoupe_getHooks_multiple() public { @@ -173,99 +125,47 @@ contract AccountLoupeTest is OptimizedTest { mockPluginManifest.executionHooks = new ManifestExecutionHook[](1); mockPluginManifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: ComprehensivePlugin.foo.selector, - preExecHook: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, - dependencyIndex: 0 - }), - postExecHook: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, - dependencyIndex: 0 - }) + preExecHook: ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}), + postExecHook: ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}) }); MockPlugin mockPlugin = new MockPlugin(mockPluginManifest); bytes32 manifestHash = keccak256(abi.encode(mockPlugin.pluginManifest())); - account1.installPlugin(address(mockPlugin), manifestHash, "", new FunctionReference[](0)); + account1.installPlugin(address(mockPlugin), manifestHash, "", new address[](0)); // Assert that the returned execution hooks are what is expected IAccountLoupe.ExecutionHooks[] memory hooks = account1.getExecutionHooks(comprehensivePlugin.foo.selector); assertEq(hooks.length, 2); - assertEq( - FunctionReference.unwrap(hooks[0].preExecHook), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.PRE_EXECUTION_HOOK) - ) - ) - ); - assertEq( - FunctionReference.unwrap(hooks[0].postExecHook), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), uint8(ComprehensivePlugin.FunctionId.POST_EXECUTION_HOOK) - ) - ) - ); - assertEq( - FunctionReference.unwrap(hooks[1].preExecHook), - FunctionReference.unwrap(FunctionReferenceLib.pack(address(mockPlugin), uint8(0))) - ); - assertEq( - FunctionReference.unwrap(hooks[1].postExecHook), - FunctionReference.unwrap(FunctionReferenceLib.pack(address(mockPlugin), uint8(0))) - ); + assertEq(hooks[0].preExecHookPlugin, address(comprehensivePlugin)); + assertEq(hooks[0].postExecHookPlugin, address(comprehensivePlugin)); + assertEq(hooks[1].preExecHookPlugin, address(mockPlugin)); + assertEq(hooks[1].postExecHookPlugin, address(mockPlugin)); } function test_pluginLoupe_getPreUserOpValidationHooks() public { - (FunctionReference[] memory hooks,) = account1.getPreValidationHooks(comprehensivePlugin.foo.selector); + (address[] memory hooks,) = account1.getPreValidationHooks(comprehensivePlugin.foo.selector); - assertEq(hooks.length, 2); - assertEq( - FunctionReference.unwrap(hooks[0]), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), - uint8(ComprehensivePlugin.FunctionId.PRE_USER_OP_VALIDATION_HOOK_1) - ) - ) - ); - assertEq( - FunctionReference.unwrap(hooks[1]), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), - uint8(ComprehensivePlugin.FunctionId.PRE_USER_OP_VALIDATION_HOOK_2) - ) - ) - ); + assertEq(hooks.length, 1); + assertEq(hooks[0], address(comprehensivePlugin)); + // todo: add a second hook to measure here + // assertEq( + // hooks[1], + // address(comprehensivePlugin) + // ); } function test_pluginLoupe_getPreRuntimeValidationHooks() public { - (, FunctionReference[] memory hooks) = account1.getPreValidationHooks(comprehensivePlugin.foo.selector); + (, address[] memory hooks) = account1.getPreValidationHooks(comprehensivePlugin.foo.selector); - assertEq(hooks.length, 2); - assertEq( - FunctionReference.unwrap(hooks[0]), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), - uint8(ComprehensivePlugin.FunctionId.PRE_RUNTIME_VALIDATION_HOOK_1) - ) - ) - ); - assertEq( - FunctionReference.unwrap(hooks[1]), - FunctionReference.unwrap( - FunctionReferenceLib.pack( - address(comprehensivePlugin), - uint8(ComprehensivePlugin.FunctionId.PRE_RUNTIME_VALIDATION_HOOK_2) - ) - ) - ); + assertEq(hooks.length, 1); + assertEq(hooks[0], address(comprehensivePlugin)); + // todo: add a second hook to measure here + // assertEq( + // hooks[1], + // address(comprehensivePlugin) + // ); } } diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index 2837b904..4ca8a7d1 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.19; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {Call} from "../../src/interfaces/IStandardExecutor.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; @@ -47,7 +46,7 @@ contract AccountReturnDataTest is OptimizedTest { plugin: address(resultCreatorPlugin), manifestHash: resultCreatorManifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); // Add the result consumer plugin to the account bytes32 resultConsumerManifestHash = keccak256(abi.encode(resultConsumerPlugin.pluginManifest())); @@ -55,7 +54,7 @@ contract AccountReturnDataTest is OptimizedTest { plugin: address(resultConsumerPlugin), manifestHash: resultConsumerManifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } diff --git a/test/account/ExecuteFromPluginPermissions.t.sol b/test/account/ExecuteFromPluginPermissions.t.sol index 88ce0c98..ba74ced3 100644 --- a/test/account/ExecuteFromPluginPermissions.t.sol +++ b/test/account/ExecuteFromPluginPermissions.t.sol @@ -6,7 +6,6 @@ import {console} from "forge-std/Test.sol"; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; @@ -55,7 +54,7 @@ contract ExecuteFromPluginPermissionsTest is OptimizedTest { plugin: address(resultCreatorPlugin), manifestHash: resultCreatorManifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); // Add the EFP caller plugin to the account bytes32 efpCallerManifestHash = keccak256(abi.encode(efpCallerPlugin.pluginManifest())); @@ -63,7 +62,7 @@ contract ExecuteFromPluginPermissionsTest is OptimizedTest { plugin: address(efpCallerPlugin), manifestHash: efpCallerManifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); // Add the EFP caller plugin with any external permissions to the account @@ -73,7 +72,7 @@ contract ExecuteFromPluginPermissionsTest is OptimizedTest { plugin: address(efpCallerPluginAnyExternal), manifestHash: efpCallerAnyExternalManifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } diff --git a/test/account/ManifestValidity.t.sol b/test/account/ManifestValidity.t.sol index 55b2c9ba..21cb3863 100644 --- a/test/account/ManifestValidity.t.sol +++ b/test/account/ManifestValidity.t.sol @@ -6,18 +6,16 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; import { - BadValidationMagicValue_UserOp_Plugin, BadValidationMagicValue_PreRuntimeValidationHook_Plugin, BadValidationMagicValue_PreUserOpValidationHook_Plugin, BadValidationMagicValue_PreExecHook_Plugin, BadValidationMagicValue_PostExecHook_Plugin, - BadHookMagicValue_UserOpValidationFunction_Plugin, - BadHookMagicValue_RuntimeValidationFunction_Plugin, + BadHookMagicValue_UserOpValidation_Plugin, + BadHookMagicValue_RuntimeValidation_Plugin, BadHookMagicValue_PostExecHook_Plugin } from "../mocks/plugins/ManifestValidityMocks.sol"; import {OptimizedTest} from "../utils/OptimizedTest.sol"; @@ -39,22 +37,6 @@ contract ManifestValidityTest is OptimizedTest { account = factory.createAccount(address(this), 0); } - // Tests that the plugin manager rejects a plugin with a user op validationFunction set to "validation always - // allow" - function test_ManifestValidity_invalid_ValidationAlwaysAllow_UserOpValidationFunction() public { - BadValidationMagicValue_UserOp_Plugin plugin = new BadValidationMagicValue_UserOp_Plugin(); - - bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); - account.installPlugin({ - plugin: address(plugin), - manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](0) - }); - } - // Tests that the plugin manager rejects a plugin with a pre-runtime validation hook set to "validation always // allow" function test_ManifestValidity_invalid_ValidationAlwaysAllow_PreRuntimeValidationHook() public { @@ -68,7 +50,7 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -85,7 +67,7 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -100,7 +82,7 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -115,14 +97,13 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } // Tests that the plugin manager rejects a plugin with a user op validationFunction set to "hook always deny" function test_ManifestValidity_invalid_HookAlwaysDeny_UserOpValidation() public { - BadHookMagicValue_UserOpValidationFunction_Plugin plugin = - new BadHookMagicValue_UserOpValidationFunction_Plugin(); + BadHookMagicValue_UserOpValidation_Plugin plugin = new BadHookMagicValue_UserOpValidation_Plugin(); bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); @@ -131,14 +112,13 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } - // Tests that the plugin manager rejects a plugin with a runtime validationFunction set to "hook always deny" - function test_ManifestValidity_invalid_HookAlwaysDeny_RuntimeValidationFunction() public { - BadHookMagicValue_RuntimeValidationFunction_Plugin plugin = - new BadHookMagicValue_RuntimeValidationFunction_Plugin(); + // Tests that the plugin manager rejects a plugin with a runtime validation function set to "hook always deny" + function test_ManifestValidity_invalid_HookAlwaysDeny_RuntimeValidation() public { + BadHookMagicValue_RuntimeValidation_Plugin plugin = new BadHookMagicValue_RuntimeValidation_Plugin(); bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); @@ -147,7 +127,7 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -162,7 +142,7 @@ contract ManifestValidityTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } } diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 1005cdf2..94dd2d0e 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -9,7 +9,6 @@ import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/User import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {IPlugin, PluginManifest} from "../../src/interfaces/IPlugin.sol"; import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol"; import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; @@ -47,7 +46,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { uint256 public constant CALL_GAS_LIMIT = 50000; uint256 public constant VERIFICATION_GAS_LIMIT = 1200000; - event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies); + event PluginInstalled(address indexed plugin, bytes32 manifestHash, address[] dependencies); event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); event ReceivedCall(bytes msgData, uint256 msgValue); @@ -265,12 +264,12 @@ contract UpgradeableModularAccountTest is OptimizedTest { 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, new address[](0)); IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); address[] memory plugins = IAccountLoupe(account2).getInstalledPlugins(); @@ -293,7 +292,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(mockPluginWithBadPermittedExec), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -305,7 +304,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(tokenReceiverPlugin), manifestHash: bytes32(0), pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -320,7 +319,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(badPlugin), manifestHash: bytes32(0), pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -332,7 +331,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); vm.expectRevert( @@ -344,7 +343,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(tokenReceiverPlugin), manifestHash: manifestHash, pluginInstallData: abi.encode(uint48(1 days)), - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); } @@ -357,7 +356,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); vm.expectEmit(true, true, true, true); @@ -378,7 +377,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); vm.expectEmit(true, true, true, true); @@ -403,7 +402,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); // Attempt to uninstall with a blank manifest @@ -431,7 +430,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { plugin: address(plugin), manifestHash: manifestHash, pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); vm.stopPrank(); diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index b5f001f7..643b07a3 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -5,14 +5,12 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; import { - MockBaseUserOpValidationPlugin, - MockUserOpValidation1HookPlugin, - MockUserOpValidation2HookPlugin, + MockUserOpValidationWithPreHookPlugin, + MockOnlyPreUserOpValidationHookPlugin, MockUserOpValidationPlugin } from "../mocks/plugins/ValidationPluginMocks.sol"; import {OptimizedTest} from "../utils/OptimizedTest.sol"; @@ -25,9 +23,9 @@ contract ValidationIntersectionTest is OptimizedTest { address public owner1; uint256 public owner1Key; UpgradeableModularAccount public account1; - MockUserOpValidationPlugin public noHookPlugin; - MockUserOpValidation1HookPlugin public oneHookPlugin; - MockUserOpValidation2HookPlugin public twoHookPlugin; + MockUserOpValidationPlugin public uoPlugin; + MockUserOpValidationWithPreHookPlugin public uoPvhPlugin; + MockOnlyPreUserOpValidationHookPlugin public pvhPlugin; function setUp() public { entryPoint = new EntryPoint(); @@ -39,37 +37,37 @@ contract ValidationIntersectionTest is OptimizedTest { account1 = factory.createAccount(owner1, 0); vm.deal(address(account1), 1 ether); - noHookPlugin = new MockUserOpValidationPlugin(); - oneHookPlugin = new MockUserOpValidation1HookPlugin(); - twoHookPlugin = new MockUserOpValidation2HookPlugin(); + uoPlugin = new MockUserOpValidationPlugin(); + uoPvhPlugin = new MockUserOpValidationWithPreHookPlugin(); + pvhPlugin = new MockOnlyPreUserOpValidationHookPlugin(); vm.startPrank(address(owner1)); account1.installPlugin({ - plugin: address(noHookPlugin), - manifestHash: keccak256(abi.encode(noHookPlugin.pluginManifest())), + plugin: address(uoPlugin), + manifestHash: keccak256(abi.encode(uoPlugin.pluginManifest())), pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); account1.installPlugin({ - plugin: address(oneHookPlugin), - manifestHash: keccak256(abi.encode(oneHookPlugin.pluginManifest())), + plugin: address(uoPvhPlugin), + manifestHash: keccak256(abi.encode(uoPvhPlugin.pluginManifest())), pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); account1.installPlugin({ - plugin: address(twoHookPlugin), - manifestHash: keccak256(abi.encode(twoHookPlugin.pluginManifest())), + plugin: address(pvhPlugin), + manifestHash: keccak256(abi.encode(pvhPlugin.pluginManifest())), pluginInstallData: "", - dependencies: new FunctionReference[](0) + dependencies: new address[](0) }); vm.stopPrank(); } function testFuzz_validationIntersect_single(uint256 validationData) public { - noHookPlugin.setValidationData(validationData); + uoPlugin.setValidationData(validationData); UserOperation memory userOp; - userOp.callData = bytes.concat(noHookPlugin.foo.selector); + userOp.callData = bytes.concat(uoPlugin.foo.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -79,13 +77,13 @@ contract ValidationIntersectionTest is OptimizedTest { } function test_validationIntersect_authorizer_sigfail_validationFunction() public { - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( _SIG_VALIDATION_FAILED, 0 // returns OK ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -96,13 +94,13 @@ contract ValidationIntersectionTest is OptimizedTest { } function test_validationIntersect_authorizer_sigfail_hook() public { - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( 0, // returns OK _SIG_VALIDATION_FAILED ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -119,12 +117,12 @@ contract ValidationIntersectionTest is OptimizedTest { uint48 start2 = uint48(15); uint48 end2 = uint48(25); - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( _packValidationData(address(0), start1, end1), _packValidationData(address(0), start2, end2) ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -140,12 +138,12 @@ contract ValidationIntersectionTest is OptimizedTest { uint48 start2 = uint48(15); uint48 end2 = uint48(25); - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( _packValidationData(address(0), start2, end2), _packValidationData(address(0), start1, end1) ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -157,23 +155,20 @@ contract ValidationIntersectionTest is OptimizedTest { function test_validationIntersect_revert_unexpectedAuthorizer() public { address badAuthorizer = makeAddr("badAuthorizer"); - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( 0, // returns OK uint256(uint160(badAuthorizer)) // returns an aggregator, which preValidation hooks are not allowed to // do. ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); vm.expectRevert( abi.encodeWithSelector( - UpgradeableModularAccount.UnexpectedAggregator.selector, - address(oneHookPlugin), - MockBaseUserOpValidationPlugin.FunctionId.PRE_USER_OP_VALIDATION_HOOK_1, - badAuthorizer + UpgradeableModularAccount.UnexpectedAggregator.selector, address(uoPvhPlugin), badAuthorizer ) ); account1.validateUserOp(userOp, uoHash, 1 wei); @@ -182,13 +177,13 @@ contract ValidationIntersectionTest is OptimizedTest { function test_validationIntersect_validAuthorizer() public { address goodAuthorizer = makeAddr("goodAuthorizer"); - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( uint256(uint160(goodAuthorizer)), // returns a valid aggregator 0 // returns OK ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -206,12 +201,12 @@ contract ValidationIntersectionTest is OptimizedTest { address goodAuthorizer = makeAddr("goodAuthorizer"); - oneHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( _packValidationData(goodAuthorizer, start1, end1), _packValidationData(address(0), start2, end2) ); UserOperation memory userOp; - userOp.callData = bytes.concat(oneHookPlugin.bar.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -227,14 +222,15 @@ contract ValidationIntersectionTest is OptimizedTest { uint48 start2 = uint48(15); uint48 end2 = uint48(25); - twoHookPlugin.setValidationData( + uoPvhPlugin.setValidationData( 0, // returns OK - _packValidationData(address(0), start1, end1), - _packValidationData(address(0), start2, end2) + _packValidationData(address(0), start1, end1) ); + pvhPlugin.setValidationData(_packValidationData(address(0), start2, end2)); + UserOperation memory userOp; - userOp.callData = bytes.concat(twoHookPlugin.baz.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); vm.prank(address(entryPoint)); @@ -244,14 +240,15 @@ contract ValidationIntersectionTest is OptimizedTest { } function test_validationIntersect_multiplePreValidationHooksSigFail() public { - twoHookPlugin.setValidationData( - 0, // returns OK + uoPvhPlugin.setValidationData( 0, // returns OK - _SIG_VALIDATION_FAILED + 0 // returns OK ); + pvhPlugin.setValidationData(_SIG_VALIDATION_FAILED); + UserOperation memory userOp; - userOp.callData = bytes.concat(twoHookPlugin.baz.selector); + userOp.callData = bytes.concat(uoPvhPlugin.bar.selector); bytes32 uoHash = entryPoint.getUserOpHash(userOp); diff --git a/test/libraries/FunctionReferenceLib.t.sol b/test/libraries/FunctionReferenceLib.t.sol deleted file mode 100644 index 6471fbd0..00000000 --- a/test/libraries/FunctionReferenceLib.t.sol +++ /dev/null @@ -1,40 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; - -import {Test} from "forge-std/Test.sol"; - -import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; -import {FunctionReference} from "../../src/interfaces/IPluginManager.sol"; - -contract FunctionReferenceLibTest is Test { - using FunctionReferenceLib for FunctionReference; - - function testFuzz_functionReference_packing(address addr, uint8 functionId) public { - // console.log("addr: ", addr); - // console.log("functionId: ", vm.toString(functionId)); - FunctionReference fr = FunctionReferenceLib.pack(addr, functionId); - // console.log("packed: ", vm.toString(FunctionReference.unwrap(fr))); - (address addr2, uint8 functionId2) = FunctionReferenceLib.unpack(fr); - // console.log("addr2: ", addr2); - // console.log("functionId2: ", vm.toString(functionId2)); - assertEq(addr, addr2); - assertEq(functionId, functionId2); - } - - function testFuzz_functionReference_operators(FunctionReference a, FunctionReference b) public { - assertTrue(a.eq(a)); - assertTrue(b.eq(b)); - - if (FunctionReference.unwrap(a) == FunctionReference.unwrap(b)) { - assertTrue(a.eq(b)); - assertTrue(b.eq(a)); - assertFalse(a.notEq(b)); - assertFalse(b.notEq(a)); - } else { - assertTrue(a.notEq(b)); - assertTrue(b.notEq(a)); - assertFalse(a.eq(b)); - assertFalse(b.eq(a)); - } - } -} diff --git a/test/mocks/MockPlugin.sol b/test/mocks/MockPlugin.sol index 67891502..4b0ffd71 100644 --- a/test/mocks/MockPlugin.sol +++ b/test/mocks/MockPlugin.sol @@ -79,8 +79,7 @@ contract MockPlugin is ERC165 { fallback() external payable { emit ReceivedCall(msg.data, msg.value); if ( - msg.sig == IPlugin.userOpValidationFunction.selector - || msg.sig == IPlugin.runtimeValidationFunction.selector + msg.sig == IPlugin.validateUserOp.selector || msg.sig == IPlugin.validateRuntime.selector || msg.sig == IPlugin.preExecutionHook.selector ) { // return 0 for userOp/runtimeVal case, return bytes("") for preExecutionHook case diff --git a/test/mocks/plugins/BadTransferOwnershipPlugin.sol b/test/mocks/plugins/BadTransferOwnershipPlugin.sol index 0b291c03..7725c8e5 100644 --- a/test/mocks/plugins/BadTransferOwnershipPlugin.sol +++ b/test/mocks/plugins/BadTransferOwnershipPlugin.sol @@ -47,12 +47,11 @@ contract BadTransferOwnershipPlugin is BasePlugin { manifest.permittedExecutionSelectors = new bytes4[](1); manifest.permittedExecutionSelectors[0] = ISingleOwnerPlugin.transferOwnership.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.evilTransferOwnership.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, // Unused. dependencyIndex: 0 // Unused. }) }); diff --git a/test/mocks/plugins/ComprehensivePlugin.sol b/test/mocks/plugins/ComprehensivePlugin.sol index 30b656cd..2b93b9bf 100644 --- a/test/mocks/plugins/ComprehensivePlugin.sol +++ b/test/mocks/plugins/ComprehensivePlugin.sol @@ -15,18 +15,18 @@ import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; contract ComprehensivePlugin is BasePlugin { - enum FunctionId { - PRE_USER_OP_VALIDATION_HOOK_1, - PRE_USER_OP_VALIDATION_HOOK_2, - USER_OP_VALIDATION, - PRE_RUNTIME_VALIDATION_HOOK_1, - PRE_RUNTIME_VALIDATION_HOOK_2, - RUNTIME_VALIDATION, - PRE_EXECUTION_HOOK, - PRE_PERMITTED_CALL_EXECUTION_HOOK, - POST_EXECUTION_HOOK, - POST_PERMITTED_CALL_EXECUTION_HOOK - } + // todo: remove + // enum FunctionId { + // PRE_USER_OP_VALIDATION_HOOK_1, + // PRE_USER_OP_VALIDATION_HOOK_2, + // PRE_RUNTIME_VALIDATION_HOOK_1, + // PRE_RUNTIME_VALIDATION_HOOK_2, + // VALIDATION, + // PRE_EXECUTION_HOOK, + // PRE_PERMITTED_CALL_EXECUTION_HOOK, + // POST_EXECUTION_HOOK, + // POST_PERMITTED_CALL_EXECUTION_HOOK + // } string public constant NAME = "Comprehensive Plugin"; string public constant VERSION = "1.0.0"; @@ -46,73 +46,56 @@ contract ComprehensivePlugin is BasePlugin { function onUninstall(bytes calldata) external override {} - function preUserOpValidationHook(uint8 functionId, UserOperation calldata, bytes32) - external - pure - override - returns (uint256) - { - if (functionId == uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1)) { - return 0; - } else if (functionId == uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_2)) { - return 0; - } - revert NotImplemented(); + function preUserOpValidationHook(UserOperation calldata, bytes32) external pure override returns (uint256) { + // if (functionId == uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1)) { + // return 0; + // } else if (functionId == uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_2)) { + // return 0; + // } + // revert NotImplemented(); + // Todo: is there a logic step missing here, with the two different hooks? + return 0; } - function userOpValidationFunction(uint8 functionId, UserOperation calldata, bytes32) - external - pure - override - returns (uint256) - { - if (functionId == uint8(FunctionId.USER_OP_VALIDATION)) { - return 0; - } - revert NotImplemented(); + function validateUserOp(UserOperation calldata, bytes32) external pure override returns (uint256) { + return 0; } - function preRuntimeValidationHook(uint8 functionId, address, uint256, bytes calldata) external pure override { - if (functionId == uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_1)) { - return; - } else if (functionId == uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_2)) { - return; - } - revert NotImplemented(); + function preRuntimeValidationHook(address, uint256, bytes calldata) external pure override { + // if (functionId == uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_1)) { + // return; + // } else if (functionId == uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_2)) { + // return; + // } + // revert NotImplemented(); + // Todo: is there a logic step missing here, with the two different hooks? + return; } - function runtimeValidationFunction(uint8 functionId, address, uint256, bytes calldata) - external - pure - override - { - if (functionId == uint8(FunctionId.RUNTIME_VALIDATION)) { - return; - } - revert NotImplemented(); + function validateRuntime(address, uint256, bytes calldata) external pure override { + return; } - function preExecutionHook(uint8 functionId, address, uint256, bytes calldata) - external - pure - override - returns (bytes memory) - { - if (functionId == uint8(FunctionId.PRE_EXECUTION_HOOK)) { - return ""; - } else if (functionId == uint8(FunctionId.PRE_PERMITTED_CALL_EXECUTION_HOOK)) { - return ""; - } - revert NotImplemented(); + function preExecutionHook(address, uint256, bytes calldata) external pure override returns (bytes memory) { + // if (functionId == uint8(FunctionId.PRE_EXECUTION_HOOK)) { + // return ""; + // } else if (functionId == uint8(FunctionId.PRE_PERMITTED_CALL_EXECUTION_HOOK)) { + // return ""; + // } + // revert NotImplemented(); + // Todo: is there a logic step missing here, with the two different hooks? + return ""; } - function postExecutionHook(uint8 functionId, bytes calldata) external pure override { - if (functionId == uint8(FunctionId.POST_EXECUTION_HOOK)) { - return; - } else if (functionId == uint8(FunctionId.POST_PERMITTED_CALL_EXECUTION_HOOK)) { - return; - } - revert NotImplemented(); + function postExecutionHook(bytes calldata) external pure override { + // if (functionId == uint8(FunctionId.POST_EXECUTION_HOOK)) { + // return; + // } else if (functionId == uint8(FunctionId.POST_PERMITTED_CALL_EXECUTION_HOOK)) { + // return; + // } + // revert NotImplemented(); + // Todo: is there a logic step missing here, with the two different hooks? + return; } function pluginManifest() external pure override returns (PluginManifest memory) { @@ -121,107 +104,61 @@ contract ComprehensivePlugin is BasePlugin { manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.foo.selector; - ManifestFunction memory fooUserOpValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.USER_OP_VALIDATION), - dependencyIndex: 0 // Unused. - }); - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: fooUserOpValidationFunction - }); - - ManifestFunction memory fooRuntimeValidationFunction = ManifestFunction({ + ManifestFunction memory fooValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.RUNTIME_VALIDATION), dependencyIndex: 0 // Unused. }); - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, - associatedFunction: fooRuntimeValidationFunction + associatedFunction: fooValidationFunction }); - manifest.preUserOpValidationHooks = new ManifestAssociatedFunction[](4); + manifest.preUserOpValidationHooks = new ManifestAssociatedFunction[](2); manifest.preUserOpValidationHooks[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1), dependencyIndex: 0 // Unused. }) }); + // todo: manifest.preUserOpValidationHooks[1] = manifest.preUserOpValidationHooks[1] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_2), - dependencyIndex: 0 // Unused. - }) - }); - manifest.preUserOpValidationHooks[2] = ManifestAssociatedFunction({ executionSelector: IStandardExecutor.execute.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1), - dependencyIndex: 0 // Unused. - }) - }); - manifest.preUserOpValidationHooks[3] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.execute.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_2), dependencyIndex: 0 // Unused. }) }); + // todo: manifest.preUserOpValidationHooks[3] = - manifest.preRuntimeValidationHooks = new ManifestAssociatedFunction[](4); + manifest.preRuntimeValidationHooks = new ManifestAssociatedFunction[](2); manifest.preRuntimeValidationHooks[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_1), dependencyIndex: 0 // Unused. }) }); + // todo: manifest.preRuntimeValidationHooks[1] = manifest.preRuntimeValidationHooks[1] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_2), - dependencyIndex: 0 // Unused. - }) - }); - manifest.preRuntimeValidationHooks[2] = ManifestAssociatedFunction({ - executionSelector: IStandardExecutor.execute.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_1), - dependencyIndex: 0 // Unused. - }) - }); - manifest.preRuntimeValidationHooks[3] = ManifestAssociatedFunction({ executionSelector: IStandardExecutor.execute.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_RUNTIME_VALIDATION_HOOK_2), dependencyIndex: 0 // Unused. }) }); + // todo: manifest.preRuntimeValidationHooks[3] = manifest.executionHooks = new ManifestExecutionHook[](1); manifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: this.foo.selector, preExecHook: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_EXECUTION_HOOK), dependencyIndex: 0 // Unused. }), postExecHook: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.POST_EXECUTION_HOOK), dependencyIndex: 0 // Unused. }) }); diff --git a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol index e858d8ee..8125cb76 100644 --- a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol +++ b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {FunctionReference} from "../../../src/helpers/FunctionReferenceLib.sol"; import { ManifestFunction, ManifestAssociatedFunctionType, @@ -45,16 +44,15 @@ contract EFPCallerPlugin is BaseTestPlugin { manifest.executionFunctions[9] = this.getNumberCounter3.selector; manifest.executionFunctions[10] = this.incrementCounter3.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](11); + manifest.validationFunctions = new ManifestAssociatedFunction[](11); ManifestFunction memory alwaysAllowValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }); for (uint256 i = 0; i < manifest.executionFunctions.length; i++) { - manifest.runtimeValidationFunctions[i] = ManifestAssociatedFunction({ + manifest.validationFunctions[i] = ManifestAssociatedFunction({ executionSelector: manifest.executionFunctions[i], associatedFunction: alwaysAllowValidationFunction }); @@ -181,12 +179,11 @@ contract EFPCallerPluginAnyExternal is BaseTestPlugin { manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.passthroughExecute.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.passthroughExecute.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); diff --git a/test/mocks/plugins/ManifestValidityMocks.sol b/test/mocks/plugins/ManifestValidityMocks.sol index 27243f79..15622cfb 100644 --- a/test/mocks/plugins/ManifestValidityMocks.sol +++ b/test/mocks/plugins/ManifestValidityMocks.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {FunctionReference} from "../../../src/helpers/FunctionReferenceLib.sol"; import { ManifestFunction, ManifestAssociatedFunctionType, @@ -16,36 +15,6 @@ import {IPlugin} from "../../../src/interfaces/IPlugin.sol"; import {BaseTestPlugin} from "./BaseTestPlugin.sol"; -contract BadValidationMagicValue_UserOp_Plugin is BaseTestPlugin { - function onInstall(bytes calldata) external override {} - - function onUninstall(bytes calldata) external override {} - - function foo() external pure returns (bytes32) { - return keccak256("bar"); - } - - function pluginManifest() external pure override returns (PluginManifest memory) { - PluginManifest memory manifest; - - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.foo.selector; - - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - // Illegal assignment: validation always allow only usable on runtime validation functions - functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, - dependencyIndex: 0 - }) - }); - - return manifest; - } -} - contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} @@ -61,12 +30,11 @@ contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BaseTestPlug manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.foo.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, dependencyIndex: 0 }) }); @@ -77,7 +45,6 @@ contract BadValidationMagicValue_PreRuntimeValidationHook_Plugin is BaseTestPlug executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); @@ -101,12 +68,11 @@ contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BaseTestPlugi manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.foo.selector; - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, dependencyIndex: 0 }) }); @@ -117,7 +83,6 @@ contract BadValidationMagicValue_PreUserOpValidationHook_Plugin is BaseTestPlugi executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); @@ -148,14 +113,9 @@ contract BadValidationMagicValue_PreExecHook_Plugin is BaseTestPlugin { executionSelector: this.foo.selector, preExecHook: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }), - postExecHook: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, // Dummy unimplemented function id, but can be added correctly - dependencyIndex: 0 - }) + postExecHook: ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}) }); return manifest; @@ -181,14 +141,9 @@ contract BadValidationMagicValue_PostExecHook_Plugin is BaseTestPlugin { // Illegal assignment: validation always allow only usable on runtime validation functions manifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: this.foo.selector, - preExecHook: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, // Dummy unimplemented function id, but can be added correctly - dependencyIndex: 0 - }), + preExecHook: ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}), postExecHook: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); @@ -197,7 +152,7 @@ contract BadValidationMagicValue_PostExecHook_Plugin is BaseTestPlugin { } } -contract BadHookMagicValue_UserOpValidationFunction_Plugin is BaseTestPlugin { +contract BadHookMagicValue_UserOpValidation_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -212,12 +167,11 @@ contract BadHookMagicValue_UserOpValidationFunction_Plugin is BaseTestPlugin { manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.foo.selector; - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY, - functionId: 0, dependencyIndex: 0 }) }); @@ -226,7 +180,7 @@ contract BadHookMagicValue_UserOpValidationFunction_Plugin is BaseTestPlugin { } } -contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BaseTestPlugin { +contract BadHookMagicValue_RuntimeValidation_Plugin is BaseTestPlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -241,12 +195,11 @@ contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BaseTestPlugin { manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.foo.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY, - functionId: 0, dependencyIndex: 0 }) }); @@ -274,14 +227,9 @@ contract BadHookMagicValue_PostExecHook_Plugin is BaseTestPlugin { // Illegal assignment: hook always deny only usable on runtime validation functions manifest.executionHooks[0] = ManifestExecutionHook({ executionSelector: this.foo.selector, - preExecHook: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, // Dummy unimplemented function id, but can be added correctly - dependencyIndex: 0 - }), + preExecHook: ManifestFunction({functionType: ManifestAssociatedFunctionType.SELF, dependencyIndex: 0}), postExecHook: ManifestFunction({ functionType: ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY, - functionId: 0, dependencyIndex: 0 }) }); diff --git a/test/mocks/plugins/ReturnDataPluginMocks.sol b/test/mocks/plugins/ReturnDataPluginMocks.sol index 34ce8e28..b92c3f39 100644 --- a/test/mocks/plugins/ReturnDataPluginMocks.sol +++ b/test/mocks/plugins/ReturnDataPluginMocks.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {FunctionReference} from "../../../src/helpers/FunctionReferenceLib.sol"; import { ManifestFunction, ManifestAssociatedFunctionType, @@ -45,12 +44,11 @@ contract ResultCreatorPlugin is BaseTestPlugin { manifest.executionFunctions[0] = this.foo.selector; manifest.executionFunctions[1] = this.bar.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.foo.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); @@ -121,20 +119,18 @@ contract ResultConsumerPlugin is BaseTestPlugin { manifest.executionFunctions[0] = this.checkResultEFPFallback.selector; manifest.executionFunctions[1] = this.checkResultEFPExternal.selector; - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](2); - manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](2); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.checkResultEFPFallback.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); - manifest.runtimeValidationFunctions[1] = ManifestAssociatedFunction({ + manifest.validationFunctions[1] = ManifestAssociatedFunction({ executionSelector: this.checkResultEFPExternal.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, dependencyIndex: 0 }) }); diff --git a/test/mocks/plugins/ValidationPluginMocks.sol b/test/mocks/plugins/ValidationPluginMocks.sol index 1e6cc941..5c5c7e9f 100644 --- a/test/mocks/plugins/ValidationPluginMocks.sol +++ b/test/mocks/plugins/ValidationPluginMocks.sol @@ -12,15 +12,8 @@ import { import {BaseTestPlugin} from "./BaseTestPlugin.sol"; abstract contract MockBaseUserOpValidationPlugin is BaseTestPlugin { - enum FunctionId { - USER_OP_VALIDATION, - PRE_USER_OP_VALIDATION_HOOK_1, - PRE_USER_OP_VALIDATION_HOOK_2 - } - uint256 internal _userOpValidationFunctionData; - uint256 internal _preUserOpValidationHook1Data; - uint256 internal _preUserOpValidationHook2Data; + uint256 internal _preUserOpValidationHookData; // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Plugin interface functions ┃ @@ -30,30 +23,13 @@ abstract contract MockBaseUserOpValidationPlugin is BaseTestPlugin { function onUninstall(bytes calldata) external override {} - function preUserOpValidationHook(uint8 functionId, UserOperation calldata, bytes32) - external - view - override - returns (uint256) - { - if (functionId == uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1)) { - return _preUserOpValidationHook1Data; - } else if (functionId == uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_2)) { - return _preUserOpValidationHook2Data; - } - revert NotImplemented(); + function preUserOpValidationHook(UserOperation calldata, bytes32) external view override returns (uint256) { + // todo: is there a test case we don't cover by not having multiple hooks? + return _preUserOpValidationHookData; } - function userOpValidationFunction(uint8 functionId, UserOperation calldata, bytes32) - external - view - override - returns (uint256) - { - if (functionId == uint8(FunctionId.USER_OP_VALIDATION)) { - return _userOpValidationFunctionData; - } - revert NotImplemented(); + function validateUserOp(UserOperation calldata, bytes32) external view override returns (uint256) { + return _userOpValidationFunctionData; } } @@ -78,12 +54,11 @@ contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { manifest.executionFunctions = new bytes4[](1); manifest.executionFunctions[0] = this.foo.selector; - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + 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. }) }); @@ -92,12 +67,12 @@ contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { } } -contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { - function setValidationData(uint256 userOpValidationFunctionData, uint256 preUserOpValidationHook1Data) +contract MockUserOpValidationWithPreHookPlugin is MockBaseUserOpValidationPlugin { + function setValidationData(uint256 userOpValidationFunctionData, uint256 preUserOpValidationHookData) external { _userOpValidationFunctionData = userOpValidationFunctionData; - _preUserOpValidationHook1Data = preUserOpValidationHook1Data; + _preUserOpValidationHookData = preUserOpValidationHookData; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -118,11 +93,10 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.USER_OP_VALIDATION), dependencyIndex: 0 // Unused. }); - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + manifest.validationFunctions = new ManifestAssociatedFunction[](1); + manifest.validationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.bar.selector, associatedFunction: userOpValidationFunctionRef }); @@ -132,7 +106,6 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { executionSelector: this.bar.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1), dependencyIndex: 0 // Unused. }) }); @@ -141,23 +114,12 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { } } -contract MockUserOpValidation2HookPlugin is MockBaseUserOpValidationPlugin { - function setValidationData( - uint256 userOpValidationFunctionData, - uint256 preUserOpValidationHook1Data, - uint256 preUserOpValidationHook2Data - ) external { - _userOpValidationFunctionData = userOpValidationFunctionData; - _preUserOpValidationHook1Data = preUserOpValidationHook1Data; - _preUserOpValidationHook2Data = preUserOpValidationHook2Data; +// Applies a second pre validation hook over the `bar()` function from MockUserOpValidationWithPreHookPlugin. +contract MockOnlyPreUserOpValidationHookPlugin is MockBaseUserOpValidationPlugin { + function setValidationData(uint256 preUserOpValidationHookData) external { + _preUserOpValidationHookData = preUserOpValidationHookData; } - // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ - // ┃ Execution functions ┃ - // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - - function baz() external {} - // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Plugin interface functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ @@ -165,34 +127,11 @@ contract MockUserOpValidation2HookPlugin is MockBaseUserOpValidationPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.baz.selector; - - ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.USER_OP_VALIDATION), - dependencyIndex: 0 // Unused. - }); - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); - manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.baz.selector, - associatedFunction: userOpValidationFunctionRef - }); - - manifest.preUserOpValidationHooks = new ManifestAssociatedFunction[](2); + manifest.preUserOpValidationHooks = new ManifestAssociatedFunction[](1); manifest.preUserOpValidationHooks[0] = ManifestAssociatedFunction({ - executionSelector: this.baz.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_1), - dependencyIndex: 0 // Unused. - }) - }); - manifest.preUserOpValidationHooks[1] = ManifestAssociatedFunction({ - executionSelector: this.baz.selector, + executionSelector: MockUserOpValidationWithPreHookPlugin.bar.selector, associatedFunction: ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, - functionId: uint8(FunctionId.PRE_USER_OP_VALIDATION_HOOK_2), dependencyIndex: 0 // Unused. }) }); diff --git a/test/plugin/SingleOwnerPlugin.t.sol b/test/plugin/SingleOwnerPlugin.t.sol index 090e5be1..cb709fa4 100644 --- a/test/plugin/SingleOwnerPlugin.t.sol +++ b/test/plugin/SingleOwnerPlugin.t.sol @@ -112,15 +112,11 @@ contract SingleOwnerPluginTest is OptimizedTest { assertEq(address(0), plugin.owner()); plugin.transferOwnership(owner1); assertEq(owner1, plugin.owner()); - plugin.runtimeValidationFunction( - uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF), owner1, 0, "" - ); + plugin.validateRuntime(owner1, 0, ""); vm.startPrank(b); vm.expectRevert(ISingleOwnerPlugin.NotAuthorized.selector); - plugin.runtimeValidationFunction( - uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF), owner1, 0, "" - ); + plugin.validateRuntime(owner1, 0, ""); } function testFuzz_validateUserOpSig(string memory salt, UserOperation memory userOp) public { @@ -135,9 +131,7 @@ contract SingleOwnerPluginTest is OptimizedTest { userOp.signature = abi.encodePacked(r, s, v); // sig check should fail - uint256 success = plugin.userOpValidationFunction( - uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER), userOp, userOpHash - ); + uint256 success = plugin.validateUserOp(userOp, userOpHash); assertEq(success, 1); // transfer ownership to signer @@ -145,9 +139,7 @@ contract SingleOwnerPluginTest is OptimizedTest { assertEq(signer, plugin.owner()); // sig check should pass - success = plugin.userOpValidationFunction( - uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER), userOp, userOpHash - ); + success = plugin.validateUserOp(userOp, userOpHash); assertEq(success, 0); } diff --git a/test/plugin/TokenReceiverPlugin.t.sol b/test/plugin/TokenReceiverPlugin.t.sol index 49692e34..f31a6f1a 100644 --- a/test/plugin/TokenReceiverPlugin.t.sol +++ b/test/plugin/TokenReceiverPlugin.t.sol @@ -9,7 +9,6 @@ import {IERC777Recipient} from "@openzeppelin/contracts/token/ERC777/IERC777Reci 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"; @@ -60,7 +59,7 @@ contract TokenReceiverPluginTest is OptimizedTest, IERC1155Receiver { function _initPlugin() internal { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - acct.installPlugin(address(plugin), manifestHash, "", new FunctionReference[](0)); + acct.installPlugin(address(plugin), manifestHash, "", new address[](0)); } function test_failERC721Transfer() public { diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index bdbcbfff..19a5cf0a 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -16,7 +16,6 @@ import {ITokenSessionKeyPlugin} from "../../../src/samples/plugins/interfaces/IT import {UpgradeableModularAccount} from "../../../src/account/UpgradeableModularAccount.sol"; import {MSCAFactoryFixture} from "../../mocks/MSCAFactoryFixture.sol"; -import {FunctionReference, FunctionReferenceLib} from "../../../src/helpers/FunctionReferenceLib.sol"; import {IPluginManager} from "../../../src/interfaces/IPluginManager.sol"; import {MockERC20} from "../../mocks/MockERC20.sol"; @@ -99,13 +98,8 @@ contract ModularSessionKeyPluginTest is Test { vm.deal(address(account), 1 ether); vm.startPrank(owner); - FunctionReference[] memory modularSessionDependency = new FunctionReference[](2); - modularSessionDependency[0] = FunctionReferenceLib.pack( - address(ownerPlugin), uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER) - ); - modularSessionDependency[1] = FunctionReferenceLib.pack( - address(ownerPlugin), uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF) - ); + address[] memory modularSessionDependency = new address[](1); + modularSessionDependency[0] = address(ownerPlugin); bytes32 modularSessionKeyManifestHash = keccak256(abi.encode(modularSessionKeyPlugin.pluginManifest())); @@ -130,15 +124,8 @@ contract ModularSessionKeyPluginTest is Test { dependencies: modularSessionDependency }); - FunctionReference[] memory tokenSessionDependency = new FunctionReference[](2); - tokenSessionDependency[0] = FunctionReferenceLib.pack( - address(modularSessionKeyPlugin), - uint8(IModularSessionKeyPlugin.FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER) - ); - tokenSessionDependency[1] = FunctionReferenceLib.pack( - address(modularSessionKeyPlugin), - uint8(IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) - ); + address[] memory tokenSessionDependency = new address[](1); + tokenSessionDependency[0] = address(modularSessionKeyPlugin); bytes32 tokenSessionKeyManifestHash = keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); account.installPlugin({ @@ -243,9 +230,8 @@ contract ModularSessionKeyPluginTest is Test { bytes memory revertReason = abi.encodeWithSelector(IModularSessionKeyPlugin.NotAuthorized.selector); vm.expectRevert( abi.encodeWithSelector( - UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + UpgradeableModularAccount.RuntimeValidationReverted.selector, address(modularSessionKeyPlugin), - IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) ); @@ -284,9 +270,8 @@ contract ModularSessionKeyPluginTest is Test { vm.expectRevert( abi.encodeWithSelector( - UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + UpgradeableModularAccount.RuntimeValidationReverted.selector, address(modularSessionKeyPlugin), - IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) ); @@ -306,9 +291,8 @@ contract ModularSessionKeyPluginTest is Test { vm.expectRevert( abi.encodeWithSelector( - UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + UpgradeableModularAccount.RuntimeValidationReverted.selector, address(modularSessionKeyPlugin), - IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) );