diff --git a/foundry.toml b/foundry.toml index 2754a5b2..23b1cb58 100644 --- a/foundry.toml +++ b/foundry.toml @@ -7,7 +7,6 @@ libs = ['lib'] out = 'out' optimizer = true optimizer_runs = 10_000 -ignored_error_codes = [3628] fs_permissions = [ { access = "read", path = "./out-optimized" } ] diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManager.sol similarity index 97% rename from src/account/PluginManagerInternals.sol rename to src/account/PluginManager.sol index 6f47c55a..0f5b209a 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManager.sol @@ -27,10 +27,12 @@ import { PluginManifest } from "../interfaces/IPlugin.sol"; -abstract contract PluginManagerInternals is IPluginManager { +contract PluginManager { using EnumerableMap for EnumerableMap.Bytes32ToUintMap; using EnumerableSet for EnumerableSet.AddressSet; + address private immutable _SELF; + error ArrayLengthMismatch(); error ExecutionFunctionAlreadySet(bytes4 selector); error InvalidDependenciesProvided(); @@ -38,6 +40,7 @@ abstract contract PluginManagerInternals is IPluginManager { error MissingPluginDependency(address dependency); error NullFunctionReference(); error NullPlugin(); + error OnlyDelegate(); error PluginAlreadyInstalled(address plugin); error PluginDependencyViolation(address plugin); error PluginInstallCallbackFailed(address plugin, bytes revertReason); @@ -48,6 +51,17 @@ abstract contract PluginManagerInternals is IPluginManager { error PluginApplyHookCallbackFailed(address providingPlugin, bytes revertReason); error PluginUnapplyHookCallbackFailed(address providingPlugin, bytes revertReason); + // Re-declare events from IPluginManager, since solidity doesn't support importing events from interfaces. + + event PluginInstalled( + address indexed plugin, + bytes32 manifestHash, + FunctionReference[] dependencies, + IPluginManager.InjectedHook[] injectedHooks + ); + + event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded); + modifier notNullFunction(FunctionReference functionReference) { if (functionReference.isEmpty()) { revert NullFunctionReference(); @@ -62,210 +76,24 @@ abstract contract PluginManagerInternals is IPluginManager { _; } - // Storage update operations - - function _setExecutionFunction(bytes4 selector, address plugin) internal notNullPlugin(plugin) { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - if (_selectorData.plugin != address(0)) { - revert ExecutionFunctionAlreadySet(selector); - } - - _selectorData.plugin = plugin; - } - - function _removeExecutionFunction(bytes4 selector) internal { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - _selectorData.plugin = address(0); - } - - function _addUserOpValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - if (!_selectorData.userOpValidation.isEmpty()) { - revert UserOpValidationFunctionAlreadySet(selector, validationFunction); - } - - _selectorData.userOpValidation = validationFunction; - } - - function _removeUserOpValidationFunction(bytes4 selector, FunctionReference validationFunction) - internal - notNullFunction(validationFunction) - { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - _selectorData.userOpValidation = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; - } - - 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 - { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - _addHooks(_selectorData.executionHooks, preExecHook, postExecHook); - } - - function _removeExecHooks(bytes4 selector, FunctionReference preExecHook, FunctionReference postExecHook) - internal - { - SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - - _removeHooks(_selectorData.executionHooks, preExecHook, postExecHook); - } - - function _enableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage) - internal - { - bytes24 key = getPermittedCallKey(plugin, selector); - - // If there are duplicates, this will just enable the flag again. This is not a problem, since the boolean - // will be set to false twice during uninstall, which is fine. - accountStorage.permittedCalls[key].callPermitted = true; - } - - function _disableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage) - internal - { - bytes24 key = getPermittedCallKey(plugin, selector); - accountStorage.permittedCalls[key].callPermitted = false; - } - - function _addPermittedCallHooks( - bytes4 selector, - address plugin, - FunctionReference preExecHook, - FunctionReference postExecHook - ) internal notNullPlugin(plugin) { - bytes24 permittedCallKey = getPermittedCallKey(plugin, selector); - PermittedCallData storage _permittedCalldata = getAccountStorage().permittedCalls[permittedCallKey]; - - _addHooks(_permittedCalldata.permittedCallHooks, preExecHook, postExecHook); - } - - function _removePermittedCallHooks( - bytes4 selector, - address plugin, - FunctionReference preExecHook, - FunctionReference postExecHook - ) internal notNullPlugin(plugin) { - bytes24 permittedCallKey = getPermittedCallKey(plugin, selector); - PermittedCallData storage _permittedCallData = getAccountStorage().permittedCalls[permittedCallKey]; - - _removeHooks(_permittedCallData.permittedCallHooks, preExecHook, postExecHook); - } - - function _addHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook) - internal - { - if (!preExecHook.isEmpty()) { - _addOrIncrement(hooks.preHooks, _toSetValue(preExecHook)); - - if (!postExecHook.isEmpty()) { - _addOrIncrement(hooks.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); - } - } else { - if (postExecHook.isEmpty()) { - // both pre and post hooks cannot be null - revert NullFunctionReference(); - } - - _addOrIncrement(hooks.postOnlyHooks, _toSetValue(postExecHook)); + modifier onlyDelegate() { + if (address(this) == _SELF) { + revert OnlyDelegate(); } + _; } - function _removeHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook) - internal - { - if (!preExecHook.isEmpty()) { - _removeOrDecrement(hooks.preHooks, _toSetValue(preExecHook)); - - if (!postExecHook.isEmpty()) { - _removeOrDecrement(hooks.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); - } - } else { - // The case where both pre and post hooks are null was checked during installation. - - // May ignore return value, as the manifest hash is validated to ensure that the hook exists. - _removeOrDecrement(hooks.postOnlyHooks, _toSetValue(postExecHook)); - } - } - - function _addPreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) - internal - notNullFunction(preUserOpValidationHook) - { - _addOrIncrement( - getAccountStorage().selectorData[selector].preUserOpValidationHooks, - _toSetValue(preUserOpValidationHook) - ); - } - - function _removePreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) - internal - notNullFunction(preUserOpValidationHook) - { - // May ignore return value, as the manifest hash is validated to ensure that the hook exists. - _removeOrDecrement( - getAccountStorage().selectorData[selector].preUserOpValidationHooks, - _toSetValue(preUserOpValidationHook) - ); - } - - function _addPreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) - internal - notNullFunction(preRuntimeValidationHook) - { - _addOrIncrement( - getAccountStorage().selectorData[selector].preRuntimeValidationHooks, - _toSetValue(preRuntimeValidationHook) - ); - } - - function _removePreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) - internal - notNullFunction(preRuntimeValidationHook) - { - // May ignore return value, as the manifest hash is validated to ensure that the hook exists. - _removeOrDecrement( - getAccountStorage().selectorData[selector].preRuntimeValidationHooks, - _toSetValue(preRuntimeValidationHook) - ); + constructor() { + _SELF = address(this); } - function _installPlugin( + function installPlugin( address plugin, bytes32 manifestHash, bytes memory pluginInitData, FunctionReference[] memory dependencies, - InjectedHook[] memory injectedHooks - ) internal { + IPluginManager.InjectedHook[] memory injectedHooks + ) external onlyDelegate { AccountStorage storage _storage = getAccountStorage(); // Check if the plugin exists. @@ -384,7 +212,7 @@ abstract contract PluginManagerInternals is IPluginManager { } for (uint256 i = 0; i < length;) { - InjectedHook memory hook = injectedHooks[i]; + IPluginManager.InjectedHook memory hook = injectedHooks[i]; _storage.pluginData[plugin].injectedHooks[i] = StoredInjectedHook({ providingPlugin: hook.providingPlugin, selector: hook.selector, @@ -536,7 +364,7 @@ abstract contract PluginManagerInternals is IPluginManager { length = injectedHooks.length; for (uint256 i = 0; i < length;) { - InjectedHook memory hook = injectedHooks[i]; + IPluginManager.InjectedHook memory hook = injectedHooks[i]; // not inlined in function call to avoid stack too deep error bytes memory onHookApplyData = injectedHooks[i].hookApplyData; /* solhint-disable no-empty-blocks */ @@ -560,12 +388,12 @@ abstract contract PluginManagerInternals is IPluginManager { emit PluginInstalled(plugin, manifestHash, dependencies, injectedHooks); } - function _uninstallPlugin( + function uninstallPlugin( address plugin, PluginManifest memory manifest, bytes memory uninstallData, bytes[] calldata hookUnapplyData - ) internal { + ) external onlyDelegate { AccountStorage storage _storage = getAccountStorage(); // Check if the plugin exists. @@ -808,7 +636,7 @@ abstract contract PluginManagerInternals is IPluginManager { /* solhint-disable no-empty-blocks */ try IPlugin(hook.providingPlugin).onHookUnapply( plugin, - InjectedHooksInfo({ + IPluginManager.InjectedHooksInfo({ preExecHookFunctionId: hook.preExecHookFunctionId, isPostHookUsed: hook.isPostHookUsed, postExecHookFunctionId: hook.postExecHookFunctionId @@ -838,6 +666,203 @@ abstract contract PluginManagerInternals is IPluginManager { emit PluginUninstalled(plugin, onUninstallSuccess); } + // Storage update operations + + function _setExecutionFunction(bytes4 selector, address plugin) internal notNullPlugin(plugin) { + SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; + + if (_selectorData.plugin != address(0)) { + revert ExecutionFunctionAlreadySet(selector); + } + + _selectorData.plugin = plugin; + } + + function _removeExecutionFunction(bytes4 selector) internal { + SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; + + _selectorData.plugin = address(0); + } + + function _addUserOpValidationFunction(bytes4 selector, FunctionReference validationFunction) + internal + notNullFunction(validationFunction) + { + SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; + + if (!_selectorData.userOpValidation.isEmpty()) { + revert UserOpValidationFunctionAlreadySet(selector, validationFunction); + } + + _selectorData.userOpValidation = validationFunction; + } + + function _removeUserOpValidationFunction(bytes4 selector, FunctionReference validationFunction) + internal + notNullFunction(validationFunction) + { + SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; + + _selectorData.userOpValidation = FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; + } + + 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 + { + SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; + + _addHooks(_selectorData.executionHooks, preExecHook, postExecHook); + } + + function _removeExecHooks(bytes4 selector, FunctionReference preExecHook, FunctionReference postExecHook) + internal + { + SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; + + _removeHooks(_selectorData.executionHooks, preExecHook, postExecHook); + } + + function _enableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage) + internal + { + bytes24 key = getPermittedCallKey(plugin, selector); + + // If there are duplicates, this will just enable the flag again. This is not a problem, since the boolean + // will be set to false twice during uninstall, which is fine. + accountStorage.permittedCalls[key].callPermitted = true; + } + + function _disableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage) + internal + { + bytes24 key = getPermittedCallKey(plugin, selector); + accountStorage.permittedCalls[key].callPermitted = false; + } + + function _addPermittedCallHooks( + bytes4 selector, + address plugin, + FunctionReference preExecHook, + FunctionReference postExecHook + ) internal notNullPlugin(plugin) { + bytes24 permittedCallKey = getPermittedCallKey(plugin, selector); + PermittedCallData storage _permittedCalldata = getAccountStorage().permittedCalls[permittedCallKey]; + + _addHooks(_permittedCalldata.permittedCallHooks, preExecHook, postExecHook); + } + + function _removePermittedCallHooks( + bytes4 selector, + address plugin, + FunctionReference preExecHook, + FunctionReference postExecHook + ) internal notNullPlugin(plugin) { + bytes24 permittedCallKey = getPermittedCallKey(plugin, selector); + PermittedCallData storage _permittedCallData = getAccountStorage().permittedCalls[permittedCallKey]; + + _removeHooks(_permittedCallData.permittedCallHooks, preExecHook, postExecHook); + } + + function _addHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook) + internal + { + if (!preExecHook.isEmpty()) { + _addOrIncrement(hooks.preHooks, _toSetValue(preExecHook)); + + if (!postExecHook.isEmpty()) { + _addOrIncrement(hooks.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); + } + } else { + if (postExecHook.isEmpty()) { + // both pre and post hooks cannot be null + revert NullFunctionReference(); + } + + _addOrIncrement(hooks.postOnlyHooks, _toSetValue(postExecHook)); + } + } + + function _removeHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook) + internal + { + if (!preExecHook.isEmpty()) { + _removeOrDecrement(hooks.preHooks, _toSetValue(preExecHook)); + + if (!postExecHook.isEmpty()) { + _removeOrDecrement(hooks.associatedPostHooks[preExecHook], _toSetValue(postExecHook)); + } + } else { + // The case where both pre and post hooks are null was checked during installation. + + // May ignore return value, as the manifest hash is validated to ensure that the hook exists. + _removeOrDecrement(hooks.postOnlyHooks, _toSetValue(postExecHook)); + } + } + + function _addPreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) + internal + notNullFunction(preUserOpValidationHook) + { + _addOrIncrement( + getAccountStorage().selectorData[selector].preUserOpValidationHooks, + _toSetValue(preUserOpValidationHook) + ); + } + + function _removePreUserOpValidationHook(bytes4 selector, FunctionReference preUserOpValidationHook) + internal + notNullFunction(preUserOpValidationHook) + { + // May ignore return value, as the manifest hash is validated to ensure that the hook exists. + _removeOrDecrement( + getAccountStorage().selectorData[selector].preUserOpValidationHooks, + _toSetValue(preUserOpValidationHook) + ); + } + + function _addPreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) + internal + notNullFunction(preRuntimeValidationHook) + { + _addOrIncrement( + getAccountStorage().selectorData[selector].preRuntimeValidationHooks, + _toSetValue(preRuntimeValidationHook) + ); + } + + function _removePreRuntimeValidationHook(bytes4 selector, FunctionReference preRuntimeValidationHook) + internal + notNullFunction(preRuntimeValidationHook) + { + // May ignore return value, as the manifest hash is validated to ensure that the hook exists. + _removeOrDecrement( + getAccountStorage().selectorData[selector].preRuntimeValidationHooks, + _toSetValue(preRuntimeValidationHook) + ); + } + function _addOrIncrement(EnumerableMap.Bytes32ToUintMap storage map, bytes32 key) internal { (bool success, uint256 value) = map.tryGet(key); map.set(key, success ? value + 1 : 0); diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 0caf3b13..8d624ba0 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -18,7 +18,7 @@ import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; import {IPluginExecutor} from "../interfaces/IPluginExecutor.sol"; import {IPluginManager} from "../interfaces/IPluginManager.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; -import {PluginManagerInternals} from "./PluginManagerInternals.sol"; +import {PluginManager} from "./PluginManager.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol"; contract UpgradeableModularAccount is @@ -29,7 +29,7 @@ contract UpgradeableModularAccount is IERC165, IPluginExecutor, IStandardExecutor, - PluginManagerInternals, + IPluginManager, UUPSUpgradeable { using EnumerableMap for EnumerableMap.Bytes32ToUintMap; @@ -41,6 +41,7 @@ contract UpgradeableModularAccount is } IEntryPoint private immutable _ENTRY_POINT; + PluginManager private immutable _PLUGIN_MANAGER; // As per the EIP-165 spec, no interface should ever match 0xffffffff bytes4 internal constant _INTERFACE_ID_INVALID = 0xffffffff; @@ -49,6 +50,7 @@ contract UpgradeableModularAccount is event ModularAccountInitialized(IEntryPoint indexed entryPoint); error AlwaysDenyRule(); + error ArrayLengthMismatch(); error AuthorizeUpgradeReverted(bytes revertReason); error ExecFromPluginNotPermitted(address plugin, bytes4 selector); error ExecFromPluginExternalNotPermitted(address plugin, address target, uint256 value, bytes data); @@ -75,8 +77,9 @@ contract UpgradeableModularAccount is _doCachedPostExecHooks(postExecHooks); } - constructor(IEntryPoint anEntryPoint) { + constructor(IEntryPoint anEntryPoint, PluginManager aPluginManager) { _ENTRY_POINT = anEntryPoint; + _PLUGIN_MANAGER = aPluginManager; _disableInitializers(); } @@ -279,7 +282,7 @@ contract UpgradeableModularAccount is bytes32 manifestHash, bytes calldata pluginInitData, FunctionReference[] calldata dependencies, - InjectedHook[] calldata injectedHooks + IPluginManager.InjectedHook[] calldata injectedHooks ) external override wrapNativeFunction { _installPlugin(plugin, manifestHash, pluginInitData, dependencies, injectedHooks); } @@ -299,7 +302,15 @@ contract UpgradeableModularAccount is manifest = IPlugin(plugin).pluginManifest(); } - _uninstallPlugin(plugin, manifest, pluginUninstallData, hookUnapplyData); + // Perform the action through the plugin manager contract. + bytes memory data = + abi.encodeCall(PluginManager.uninstallPlugin, (plugin, manifest, pluginUninstallData, hookUnapplyData)); + (bool success, bytes memory returndata) = address(_PLUGIN_MANAGER).delegatecall(data); + if (!success) { + assembly ("memory-safe") { + revert(add(returndata, 32), mload(returndata)) + } + } } /// @notice ERC165 introspection @@ -606,4 +617,26 @@ contract UpgradeableModularAccount is // solhint-disable-next-line no-empty-blocks function _authorizeUpgrade(address newImplementation) internal override {} + + function _toFunctionReference(bytes32 setValue) internal pure returns (FunctionReference) { + return FunctionReference.wrap(bytes21(setValue)); + } + + function _installPlugin( + address plugin, + bytes32 manifestHash, + bytes memory pluginInitData, + FunctionReference[] memory dependencies, + InjectedHook[] memory injectedHooks + ) internal { + bytes memory data = abi.encodeCall( + PluginManager.installPlugin, (plugin, manifestHash, pluginInitData, dependencies, injectedHooks) + ); + (bool success, bytes memory returndata) = address(_PLUGIN_MANAGER).delegatecall(data); + if (!success) { + assembly ("memory-safe") { + revert(add(returndata, 32), mload(returndata)) + } + } + } } diff --git a/test/account/ManifestValidity.t.sol b/test/account/ManifestValidity.t.sol index 18972d07..5c705a3f 100644 --- a/test/account/ManifestValidity.t.sol +++ b/test/account/ManifestValidity.t.sol @@ -5,7 +5,7 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; +import {PluginManager} from "../../src/account/PluginManager.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {FunctionReference} from "../../src/libraries/FunctionReferenceLib.sol"; @@ -46,7 +46,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -64,7 +64,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -82,7 +82,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -98,7 +98,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -114,7 +114,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -131,7 +131,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -148,7 +148,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, @@ -164,7 +164,7 @@ contract ManifestValidityTest is OptimizedTest { bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); account.installPlugin({ plugin: address(plugin), manifestHash: manifestHash, diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index 528eedd2..bbe7d0e7 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -7,7 +7,7 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; -import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; +import {PluginManager} from "../../src/account/PluginManager.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; @@ -319,7 +319,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { function test_installPlugin_invalidManifest() public { vm.startPrank(owner2); - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), manifestHash: bytes32(0), @@ -334,7 +334,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { address badPlugin = address(1); vm.expectRevert( - abi.encodeWithSelector(PluginManagerInternals.PluginInterfaceNotSupported.selector, address(badPlugin)) + abi.encodeWithSelector(PluginManager.PluginInterfaceNotSupported.selector, address(badPlugin)) ); IPluginManager(account2).installPlugin({ plugin: address(badPlugin), @@ -358,9 +358,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { }); vm.expectRevert( - abi.encodeWithSelector( - PluginManagerInternals.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin) - ) + abi.encodeWithSelector(PluginManager.PluginAlreadyInstalled.selector, address(tokenReceiverPlugin)) ); IPluginManager(account2).installPlugin({ plugin: address(tokenReceiverPlugin), @@ -441,7 +439,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { // Attempt to uninstall with a blank manifest PluginManifest memory blankManifest; - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); + vm.expectRevert(abi.encodeWithSelector(PluginManager.InvalidPluginManifest.selector)); IPluginManager(account2).uninstallPlugin({ plugin: address(plugin), config: abi.encode(blankManifest), @@ -571,7 +569,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { ); vm.expectRevert( - abi.encodeWithSelector(PluginManagerInternals.MissingPluginDependency.selector, address(hooksPlugin)) + abi.encodeWithSelector(PluginManager.MissingPluginDependency.selector, address(hooksPlugin)) ); vm.prank(owner2); IPluginManager(account2).installPlugin({ @@ -602,7 +600,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { vm.prank(owner2); vm.expectRevert( - abi.encodeWithSelector(PluginManagerInternals.PluginDependencyViolation.selector, address(hooksPlugin)) + abi.encodeWithSelector(PluginManager.PluginDependencyViolation.selector, address(hooksPlugin)) ); IPluginManager(account2).uninstallPlugin({ plugin: address(hooksPlugin), @@ -638,7 +636,7 @@ contract UpgradeableModularAccountTest is OptimizedTest { // length != installed hooks length bytes[] memory injectedHooksDatas = new bytes[](2); - vm.expectRevert(PluginManagerInternals.ArrayLengthMismatch.selector); + vm.expectRevert(PluginManager.ArrayLengthMismatch.selector); vm.prank(owner2); IPluginManager(account2).uninstallPlugin({ plugin: address(newPlugin), diff --git a/test/mocks/MockPlugin.sol b/test/mocks/MockPlugin.sol index 1a71e999..67891502 100644 --- a/test/mocks/MockPlugin.sol +++ b/test/mocks/MockPlugin.sol @@ -73,6 +73,8 @@ contract MockPlugin is ERC165 { return interfaceId == type(IPlugin).interfaceId || super.supportsInterface(interfaceId); } + receive() external payable {} + // solhint-disable-next-line no-complex-fallback fallback() external payable { emit ReceivedCall(msg.data, msg.value); diff --git a/test/utils/OptimizedTest.sol b/test/utils/OptimizedTest.sol index f9431acc..c0336708 100644 --- a/test/utils/OptimizedTest.sol +++ b/test/utils/OptimizedTest.sol @@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol"; import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {PluginManager} from "../../src/account/PluginManager.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {TokenReceiverPlugin} from "../../src/plugins/TokenReceiverPlugin.sol"; @@ -32,16 +33,22 @@ abstract contract OptimizedTest is Test { internal returns (UpgradeableModularAccount) { - return _isOptimizedTest() - ? UpgradeableModularAccount( + if (_isOptimizedTest()) { + PluginManager pluginManager = + PluginManager(deployCode("out-optimized/PluginManager.sol/PluginManager.json")); + + return UpgradeableModularAccount( payable( deployCode( "out-optimized/UpgradeableModularAccount.sol/UpgradeableModularAccount.json", - abi.encode(entryPoint) + abi.encode(entryPoint, pluginManager) ) ) - ) - : new UpgradeableModularAccount(entryPoint); + ); + } else { + PluginManager pluginManager = new PluginManager(); + return new UpgradeableModularAccount(entryPoint, pluginManager); + } } function _deploySingleOwnerPlugin() internal returns (SingleOwnerPlugin) {