diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 20555aed..205af107 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -35,6 +35,10 @@ struct SelectorData { // The plugin that implements this execution function. // If this is a native function, the address must remain address(0). address plugin; + // Whether or not the function needs runtime validation, or can be called by anyone. + // Note that even if this is set to true, user op validation will still be required, otherwise anyone could + // drain the account of native tokens by wasting gas. + bool isPublic; // How many times a `PRE_HOOK_ALWAYS_DENY` has been added for this function. // Since that is the only type of hook that may overlap, we can use this to track the number of times it has // been applied, and whether or not the deny should apply. The size `uint48` was chosen somewhat arbitrarily, diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManagerInternals.sol index 1a946e1d..7ded3176 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManagerInternals.sol @@ -59,7 +59,10 @@ abstract contract PluginManagerInternals is IPluginManager { // Storage update operations - function _setExecutionFunction(bytes4 selector, address plugin) internal notNullPlugin(plugin) { + function _setExecutionFunction(bytes4 selector, bool isPublic, address plugin) + internal + notNullPlugin(plugin) + { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; if (_selectorData.plugin != address(0)) { @@ -67,12 +70,14 @@ abstract contract PluginManagerInternals is IPluginManager { } _selectorData.plugin = plugin; + _selectorData.isPublic = isPublic; } function _removeExecutionFunction(bytes4 selector) internal { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; _selectorData.plugin = address(0); + _selectorData.isPublic = false; } function _addValidationFunction(bytes4 selector, FunctionReference validationFunction) @@ -211,7 +216,9 @@ abstract contract PluginManagerInternals is IPluginManager { length = manifest.executionFunctions.length; for (uint256 i = 0; i < length; ++i) { - _setExecutionFunction(manifest.executionFunctions[i], plugin); + bytes4 selector = manifest.executionFunctions[i].executionSelector; + bool isPublic = manifest.executionFunctions[i].isPublic; + _setExecutionFunction(selector, isPublic, plugin); } // Add installed plugin and selectors this plugin can call @@ -253,10 +260,7 @@ abstract contract PluginManagerInternals is IPluginManager { _addValidationFunction( mv.executionSelector, _resolveManifestFunction( - mv.associatedFunction, - plugin, - dependencies, - ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW + mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE ) ); } @@ -378,10 +382,7 @@ abstract contract PluginManagerInternals is IPluginManager { _removeValidationFunction( mv.executionSelector, _resolveManifestFunction( - mv.associatedFunction, - plugin, - dependencies, - ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW + mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE ) ); } @@ -421,7 +422,8 @@ abstract contract PluginManagerInternals is IPluginManager { length = manifest.executionFunctions.length; for (uint256 i = 0; i < length; ++i) { - _removeExecutionFunction(manifest.executionFunctions[i]); + bytes4 selector = manifest.executionFunctions[i].executionSelector; + _removeExecutionFunction(selector); } length = manifest.interfaceIds.length; @@ -465,13 +467,6 @@ abstract contract PluginManagerInternals is IPluginManager { revert InvalidPluginManifest(); } return 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; - } 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; diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index ba671941..1cd0d98e 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -372,10 +372,8 @@ contract UpgradeableModularAccount is PackedUserOperation calldata userOp, bytes32 userOpHash ) internal returns (uint256 validationData) { - if (userOpValidationFunction.isEmptyOrMagicValue()) { + if (userOpValidationFunction.isEmpty()) { // If the validation function is empty, then the call cannot proceed. - // Alternatively, the validation function may be set to the RUNTIME_VALIDATION_ALWAYS_ALLOW magic - // value, in which case we also revert. revert UserOpValidationFunctionMissing(selector); } @@ -443,18 +441,20 @@ contract UpgradeableModularAccount is // Identifier scope limiting { - if (!runtimeValidationFunction.isEmptyOrMagicValue()) { - (address plugin, uint8 functionId) = runtimeValidationFunction.unpack(); - // solhint-disable-next-line no-empty-blocks - try IValidation(plugin).validateRuntime(functionId, msg.sender, msg.value, msg.data) {} - catch (bytes memory revertReason) { - revert RuntimeValidationFunctionReverted(plugin, functionId, revertReason); - } - } else { - if (runtimeValidationFunction.isEmpty()) { - revert RuntimeValidationFunctionMissing(msg.sig); - } - // If _RUNTIME_VALIDATION_ALWAYS_ALLOW, just let the function finish. + if (_storage.selectorData[msg.sig].isPublic) { + // If the function is public, we don't need to check the runtime validation function. + return; + } + + if (runtimeValidationFunction.isEmpty()) { + revert RuntimeValidationFunctionMissing(msg.sig); + } + + (address plugin, uint8 functionId) = runtimeValidationFunction.unpack(); + // solhint-disable-next-line no-empty-blocks + try IValidation(plugin).validateRuntime(functionId, msg.sender, msg.value, msg.data) {} + catch (bytes memory revertReason) { + revert RuntimeValidationFunctionReverted(plugin, functionId, revertReason); } } } diff --git a/src/helpers/FunctionReferenceLib.sol b/src/helpers/FunctionReferenceLib.sol index e80992c9..7bddd94b 100644 --- a/src/helpers/FunctionReferenceLib.sol +++ b/src/helpers/FunctionReferenceLib.sol @@ -6,9 +6,6 @@ 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))); @@ -30,10 +27,6 @@ library FunctionReferenceLib { 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); } diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index a229f51b..c20225dd 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -13,11 +13,6 @@ enum ManifestAssociatedFunctionType { SELF, // Function belongs to an external plugin provided as a dependency during plugin installation. DEPENDENCY, - // Resolves to a magic value to always bypass runtime validation for a given function. - // This is only assignable on runtime validation functions. If it were to be used on a user op validation function, - // it would risk burning gas from the account. When used as a hook in any hook location, it is equivalent to not - // setting a hook and is therefore disallowed. - RUNTIME_VALIDATION_ALWAYS_ALLOW, // Resolves to a magic value to always fail in a hook for a given function. // This is only assignable to pre execution hooks. It should not be used on validation functions themselves, because // this is equivalent to leaving the validation functions unset. It should not be used in post-exec hooks, because @@ -26,6 +21,14 @@ enum ManifestAssociatedFunctionType { } // forgefmt: disable-end +struct ManifestExecutionFunction { + // TODO(erc6900 spec): These fields can be packed into a single word + // The selector to install + bytes4 executionSelector; + // If true, the function won't need runtime validaiton, and can be called by anyone. + bool isPublic; +} + /// @dev For functions of type `ManifestAssociatedFunctionType.DEPENDENCY`, the MSCA MUST find the plugin address /// of the function at `dependencies[dependencyIndex]` during the call to `installPlugin(config)`. struct ManifestFunction { @@ -82,7 +85,7 @@ struct PluginManifest { // structs used in the manifest. bytes4[] dependencyInterfaceIds; // Execution functions defined in this plugin to be installed on the MSCA. - bytes4[] executionFunctions; + ManifestExecutionFunction[] executionFunctions; // Plugin execution functions already installed on the MSCA that this plugin will be able to call. bytes4[] permittedExecutionSelectors; // Boolean to indicate whether the plugin can call any external address. diff --git a/src/plugins/TokenReceiverPlugin.sol b/src/plugins/TokenReceiverPlugin.sol index 1a9e8feb..f3379f1a 100644 --- a/src/plugins/TokenReceiverPlugin.sol +++ b/src/plugins/TokenReceiverPlugin.sol @@ -4,14 +4,7 @@ pragma solidity ^0.8.25; import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; import {IERC1155Receiver} from "@openzeppelin/contracts/interfaces/IERC1155Receiver.sol"; -import { - IPlugin, - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, - PluginManifest, - PluginMetadata -} from "../interfaces/IPlugin.sol"; +import {IPlugin, ManifestExecutionFunction, PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; import {BasePlugin} from "./BasePlugin.sol"; /// @title Token Receiver Plugin @@ -65,30 +58,13 @@ contract TokenReceiverPlugin is BasePlugin, IERC721Receiver, IERC1155Receiver { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](3); - manifest.executionFunctions[0] = this.onERC721Received.selector; - manifest.executionFunctions[1] = this.onERC1155Received.selector; - manifest.executionFunctions[2] = this.onERC1155BatchReceived.selector; - - // Only runtime validationFunction is needed since callbacks come from token contracts only - ManifestFunction memory alwaysAllowRuntime = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, // Unused. - dependencyIndex: 0 // Unused. - }); - manifest.validationFunctions = new ManifestAssociatedFunction[](3); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.onERC721Received.selector, - associatedFunction: alwaysAllowRuntime - }); - manifest.validationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: this.onERC1155Received.selector, - associatedFunction: alwaysAllowRuntime - }); - manifest.validationFunctions[2] = ManifestAssociatedFunction({ - executionSelector: this.onERC1155BatchReceived.selector, - associatedFunction: alwaysAllowRuntime - }); + manifest.executionFunctions = new ManifestExecutionFunction[](3); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.onERC721Received.selector, isPublic: true}); + manifest.executionFunctions[1] = + ManifestExecutionFunction({executionSelector: this.onERC1155Received.selector, isPublic: true}); + manifest.executionFunctions[2] = + ManifestExecutionFunction({executionSelector: this.onERC1155BatchReceived.selector, isPublic: true}); manifest.interfaceIds = new bytes4[](2); manifest.interfaceIds[0] = type(IERC721Receiver).interfaceId; diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index a1db86c6..3605ddd5 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -6,6 +6,7 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, ManifestExecutionHook, + ManifestExecutionFunction, ManifestFunction, PluginManifest } from "../../src/interfaces/IPlugin.sol"; @@ -38,18 +39,7 @@ contract AccountExecHooksTest is AccountTestBase { function setUp() public { _transferOwnershipToTest(); - m1.executionFunctions.push(_EXEC_SELECTOR); - - m1.validationFunctions.push( - ManifestAssociatedFunction({ - executionSelector: _EXEC_SELECTOR, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, - dependencyIndex: 0 - }) - }) - ); + m1.executionFunctions.push(ManifestExecutionFunction({executionSelector: _EXEC_SELECTOR, isPublic: true})); } function test_preExecHook_install() public { diff --git a/test/account/ManifestValidity.t.sol b/test/account/ManifestValidity.t.sol index 21023ef3..8200a017 100644 --- a/test/account/ManifestValidity.t.sol +++ b/test/account/ManifestValidity.t.sol @@ -4,11 +4,7 @@ pragma solidity ^0.8.19; import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol"; import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; -import { - BadValidationMagicValue_PreValidationHook_Plugin, - BadHookMagicValue_UserOpValidationFunction_Plugin, - BadHookMagicValue_RuntimeValidationFunction_Plugin -} from "../mocks/plugins/ManifestValidityMocks.sol"; +import {BadHookMagicValue_ValidationFunction_Plugin} from "../mocks/plugins/ManifestValidityMocks.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; contract ManifestValidityTest is AccountTestBase { @@ -16,43 +12,9 @@ contract ManifestValidityTest is AccountTestBase { _transferOwnershipToTest(); } - // Tests that the plugin manager rejects a plugin with a pre-runtime validation hook set to "validation always - // allow" - function test_ManifestValidity_invalid_ValidationAlwaysAllow_PreValidationHook() public { - BadValidationMagicValue_PreValidationHook_Plugin plugin = - new BadValidationMagicValue_PreValidationHook_Plugin(); - - bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); - account1.installPlugin({ - plugin: address(plugin), - manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](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(); - - bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); - - vm.expectRevert(abi.encodeWithSelector(PluginManagerInternals.InvalidPluginManifest.selector)); - account1.installPlugin({ - plugin: address(plugin), - manifestHash: manifestHash, - pluginInstallData: "", - dependencies: new FunctionReference[](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 validation function set to "hook always deny" + function test_ManifestValidity_invalid_HookAlwaysDeny_Validation() public { + BadHookMagicValue_ValidationFunction_Plugin plugin = new BadHookMagicValue_ValidationFunction_Plugin(); bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest())); diff --git a/test/mocks/plugins/BadTransferOwnershipPlugin.sol b/test/mocks/plugins/BadTransferOwnershipPlugin.sol index 9d07687e..6561bb1b 100644 --- a/test/mocks/plugins/BadTransferOwnershipPlugin.sol +++ b/test/mocks/plugins/BadTransferOwnershipPlugin.sol @@ -1,13 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import { - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, - PluginManifest, - PluginMetadata -} from "../../../src/interfaces/IPlugin.sol"; +import {ManifestExecutionFunction, PluginManifest, PluginMetadata} from "../../../src/interfaces/IPlugin.sol"; import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; import {ISingleOwnerPlugin} from "../../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {IPluginExecutor} from "../../../src/interfaces/IPluginExecutor.sol"; @@ -38,22 +32,13 @@ contract BadTransferOwnershipPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.evilTransferOwnership.selector; + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.evilTransferOwnership.selector, isPublic: true}); manifest.permittedExecutionSelectors = new bytes4[](1); manifest.permittedExecutionSelectors[0] = ISingleOwnerPlugin.transferOwnership.selector; - 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. - }) - }); - return manifest; } diff --git a/test/mocks/plugins/ComprehensivePlugin.sol b/test/mocks/plugins/ComprehensivePlugin.sol index bdb82499..9aac5545 100644 --- a/test/mocks/plugins/ComprehensivePlugin.sol +++ b/test/mocks/plugins/ComprehensivePlugin.sol @@ -5,6 +5,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import { ManifestExecutionHook, + ManifestExecutionFunction, ManifestFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, @@ -127,8 +128,9 @@ contract ComprehensivePlugin is IValidation, IValidationHook, IExecutionHook, Ba function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.foo.selector; + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.foo.selector, isPublic: false}); ManifestFunction memory fooValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, diff --git a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol index ae775dd0..11970dfa 100644 --- a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol +++ b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol @@ -2,9 +2,7 @@ pragma solidity ^0.8.19; import { - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, + ManifestExecutionFunction, ManifestExternalCallPermission, PluginManifest, PluginMetadata @@ -36,32 +34,21 @@ contract EFPCallerPlugin is BasePlugin { function _getManifest() internal view returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](11); - manifest.executionFunctions[0] = this.useEFPPermissionAllowed.selector; - manifest.executionFunctions[1] = this.useEFPPermissionNotAllowed.selector; - manifest.executionFunctions[2] = this.setNumberCounter1.selector; - manifest.executionFunctions[3] = this.getNumberCounter1.selector; - manifest.executionFunctions[4] = this.incrementCounter1.selector; - manifest.executionFunctions[5] = this.setNumberCounter2.selector; - manifest.executionFunctions[6] = this.getNumberCounter2.selector; - manifest.executionFunctions[7] = this.incrementCounter2.selector; - manifest.executionFunctions[8] = this.setNumberCounter3.selector; - manifest.executionFunctions[9] = this.getNumberCounter3.selector; - manifest.executionFunctions[10] = this.incrementCounter3.selector; - - manifest.validationFunctions = new ManifestAssociatedFunction[](11); - - ManifestFunction memory alwaysAllowValidationFunction = ManifestFunction({ - functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, - dependencyIndex: 0 - }); + manifest.executionFunctions = new ManifestExecutionFunction[](11); + manifest.executionFunctions[0].executionSelector = this.useEFPPermissionAllowed.selector; + manifest.executionFunctions[1].executionSelector = this.useEFPPermissionNotAllowed.selector; + manifest.executionFunctions[2].executionSelector = this.setNumberCounter1.selector; + manifest.executionFunctions[3].executionSelector = this.getNumberCounter1.selector; + manifest.executionFunctions[4].executionSelector = this.incrementCounter1.selector; + manifest.executionFunctions[5].executionSelector = this.setNumberCounter2.selector; + manifest.executionFunctions[6].executionSelector = this.getNumberCounter2.selector; + manifest.executionFunctions[7].executionSelector = this.incrementCounter2.selector; + manifest.executionFunctions[8].executionSelector = this.setNumberCounter3.selector; + manifest.executionFunctions[9].executionSelector = this.getNumberCounter3.selector; + manifest.executionFunctions[10].executionSelector = this.incrementCounter3.selector; for (uint256 i = 0; i < manifest.executionFunctions.length; i++) { - manifest.validationFunctions[i] = ManifestAssociatedFunction({ - executionSelector: manifest.executionFunctions[i], - associatedFunction: alwaysAllowValidationFunction - }); + manifest.executionFunctions[i].isPublic = true; } // Request permission only for "foo", but not "bar", from ResultCreatorPlugin @@ -198,18 +185,9 @@ contract EFPCallerPluginAnyExternal is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.passthroughExecute.selector; - - 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 - }) - }); + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.passthroughExecute.selector, isPublic: true}); manifest.permitAnyExternalAddress = true; diff --git a/test/mocks/plugins/ManifestValidityMocks.sol b/test/mocks/plugins/ManifestValidityMocks.sol index 2ea94f3c..302226c0 100644 --- a/test/mocks/plugins/ManifestValidityMocks.sol +++ b/test/mocks/plugins/ManifestValidityMocks.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.19; import { ManifestFunction, + ManifestExecutionFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, @@ -12,7 +13,7 @@ import { import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; // solhint-disable-next-line contract-name-camelcase -contract BadValidationMagicValue_PreValidationHook_Plugin is BasePlugin { +contract BadHookMagicValue_ValidationFunction_Plugin is BasePlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -24,83 +25,9 @@ contract BadValidationMagicValue_PreValidationHook_Plugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.foo.selector; - - manifest.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.SELF, - functionId: 0, - dependencyIndex: 0 - }) - }); - - manifest.preValidationHooks = new ManifestAssociatedFunction[](1); - // Illegal assignment: validation always allow only usable on runtime validation functions - manifest.preValidationHooks[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, - dependencyIndex: 0 - }) - }); - - return manifest; - } - - function pluginMetadata() external pure override returns (PluginMetadata memory) {} -} - -// solhint-disable-next-line contract-name-camelcase -contract BadHookMagicValue_UserOpValidationFunction_Plugin is BasePlugin { - 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.validationFunctions = new ManifestAssociatedFunction[](1); - manifest.validationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.foo.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY, - functionId: 0, - dependencyIndex: 0 - }) - }); - - return manifest; - } - - function pluginMetadata() external pure override returns (PluginMetadata memory) {} -} - -// solhint-disable-next-line contract-name-camelcase -contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BasePlugin { - 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.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.foo.selector, isPublic: false}); manifest.validationFunctions = new ManifestAssociatedFunction[](1); manifest.validationFunctions[0] = ManifestAssociatedFunction({ diff --git a/test/mocks/plugins/ReturnDataPluginMocks.sol b/test/mocks/plugins/ReturnDataPluginMocks.sol index 766bb903..788cb4c7 100644 --- a/test/mocks/plugins/ReturnDataPluginMocks.sol +++ b/test/mocks/plugins/ReturnDataPluginMocks.sol @@ -2,9 +2,7 @@ pragma solidity ^0.8.19; import { - ManifestFunction, - ManifestAssociatedFunctionType, - ManifestAssociatedFunction, + ManifestExecutionFunction, ManifestExternalCallPermission, PluginManifest, PluginMetadata @@ -39,19 +37,11 @@ contract ResultCreatorPlugin is BasePlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](2); - manifest.executionFunctions[0] = this.foo.selector; - manifest.executionFunctions[1] = this.bar.selector; - - 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 - }) - }); + manifest.executionFunctions = new ManifestExecutionFunction[](2); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.foo.selector, isPublic: true}); + manifest.executionFunctions[1] = + ManifestExecutionFunction({executionSelector: this.bar.selector, isPublic: false}); return manifest; } @@ -117,27 +107,11 @@ contract ResultConsumerPlugin is BasePlugin { function _innerPluginManifest() internal view returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](2); - manifest.executionFunctions[0] = this.checkResultEFPFallback.selector; - manifest.executionFunctions[1] = this.checkResultEFPExternal.selector; - - 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.validationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: this.checkResultEFPExternal.selector, - associatedFunction: ManifestFunction({ - functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, - functionId: 0, - dependencyIndex: 0 - }) - }); + manifest.executionFunctions = new ManifestExecutionFunction[](2); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.checkResultEFPFallback.selector, isPublic: true}); + manifest.executionFunctions[1] = + ManifestExecutionFunction({executionSelector: this.checkResultEFPExternal.selector, isPublic: true}); manifest.permittedExecutionSelectors = new bytes4[](1); manifest.permittedExecutionSelectors[0] = ResultCreatorPlugin.foo.selector; diff --git a/test/mocks/plugins/ValidationPluginMocks.sol b/test/mocks/plugins/ValidationPluginMocks.sol index 9e7909df..554f589f 100644 --- a/test/mocks/plugins/ValidationPluginMocks.sol +++ b/test/mocks/plugins/ValidationPluginMocks.sol @@ -5,6 +5,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import { ManifestFunction, + ManifestExecutionFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginMetadata, @@ -93,8 +94,9 @@ contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.foo.selector; + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.foo.selector, isPublic: false}); manifest.validationFunctions = new ManifestAssociatedFunction[](1); manifest.validationFunctions[0] = ManifestAssociatedFunction({ @@ -131,8 +133,9 @@ contract MockUserOpValidation1HookPlugin is MockBaseUserOpValidationPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](1); - manifest.executionFunctions[0] = this.bar.selector; + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.bar.selector, isPublic: false}); ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF, @@ -183,8 +186,9 @@ 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; + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = + ManifestExecutionFunction({executionSelector: this.baz.selector, isPublic: false}); ManifestFunction memory userOpValidationFunctionRef = ManifestFunction({ functionType: ManifestAssociatedFunctionType.SELF,