diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 47192abd..96c33ce4 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -11,6 +11,9 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol"; import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; +import {IValidation} from "../interfaces/IValidation.sol"; +import {IValidationHook} from "../interfaces/IValidationHook.sol"; +import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; import {IPluginExecutor} from "../interfaces/IPluginExecutor.sol"; import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; @@ -364,7 +367,7 @@ contract UpgradeableModularAccount is FunctionReference preUserOpValidationHook = toFunctionReference(key); (address plugin, uint8 functionId) = preUserOpValidationHook.unpack(); - currentValidationData = IPlugin(plugin).preUserOpValidationHook(functionId, userOp, userOpHash); + currentValidationData = IValidationHook(plugin).preUserOpValidationHook(functionId, userOp, userOpHash); if (uint160(currentValidationData) > 1) { // If the aggregator is not 0 or 1, it is an unexpected value @@ -376,7 +379,7 @@ contract UpgradeableModularAccount is // Run the user op validationFunction { (address plugin, uint8 functionId) = userOpValidationFunction.unpack(); - currentValidationData = IPlugin(plugin).userOpValidationFunction(functionId, userOp, userOpHash); + currentValidationData = IValidation(plugin).validateUserOp(functionId, userOp, userOpHash); if (preUserOpValidationHooksLength != 0) { // If we have other validation data we need to coalesce with @@ -408,7 +411,7 @@ contract UpgradeableModularAccount is (address plugin, uint8 functionId) = preRuntimeValidationHook.unpack(); // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).preRuntimeValidationHook(functionId, msg.sender, msg.value, msg.data) {} + try IValidationHook(plugin).preRuntimeValidationHook(functionId, msg.sender, msg.value, msg.data) {} catch (bytes memory revertReason) { revert PreRuntimeValidationHookFailed(plugin, functionId, revertReason); } @@ -419,7 +422,7 @@ contract UpgradeableModularAccount is if (!runtimeValidationFunction.isEmptyOrMagicValue()) { (address plugin, uint8 functionId) = runtimeValidationFunction.unpack(); // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).runtimeValidationFunction(functionId, msg.sender, msg.value, msg.data) {} + try IValidation(plugin).validateRuntime(functionId, msg.sender, msg.value, msg.data) {} catch (bytes memory revertReason) { revert RuntimeValidationFunctionReverted(plugin, functionId, revertReason); } @@ -475,7 +478,7 @@ contract UpgradeableModularAccount is returns (bytes memory preExecHookReturnData) { (address plugin, uint8 functionId) = preExecHook.unpack(); - try IPlugin(plugin).preExecutionHook(functionId, msg.sender, msg.value, data) returns ( + try IExecutionHook(plugin).preExecutionHook(functionId, msg.sender, msg.value, data) returns ( bytes memory returnData ) { preExecHookReturnData = returnData; @@ -500,7 +503,7 @@ contract UpgradeableModularAccount is (address plugin, uint8 functionId) = postHookToRun.postExecHook.unpack(); // solhint-disable-next-line no-empty-blocks - try IPlugin(plugin).postExecutionHook(functionId, postHookToRun.preExecHookReturnData) {} + try IExecutionHook(plugin).postExecutionHook(functionId, postHookToRun.preExecHookReturnData) {} catch (bytes memory revertReason) { revert PostExecHookReverted(plugin, functionId, revertReason); } diff --git a/src/interfaces/IExecutionHook.sol b/src/interfaces/IExecutionHook.sol new file mode 100644 index 00000000..3240c489 --- /dev/null +++ b/src/interfaces/IExecutionHook.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: CC0-1.0 +pragma solidity ^0.8.25; + +import {IPlugin} from "./IPlugin.sol"; + +interface IExecutionHook is IPlugin { + /// @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) + external + returns (bytes memory); + + /// @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; +} diff --git a/src/interfaces/IPlugin.sol b/src/interfaces/IPlugin.sol index d3163730..cf28c5d2 100644 --- a/src/interfaces/IPlugin.sol +++ b/src/interfaces/IPlugin.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: CC0-1.0 pragma solidity ^0.8.25; -import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; // Forge formatter will displace the first comment for the enum field out of the enum itself, // so annotating here to prevent that. @@ -96,7 +96,7 @@ struct PluginManifest { ManifestExecutionHook[] executionHooks; } -interface IPlugin { +interface IPlugin is IERC165 { /// @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 @@ -109,66 +109,6 @@ interface IPlugin { /// account. function onUninstall(bytes calldata data) external; - /// @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, PackedUserOperation 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. - /// @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, PackedUserOperation calldata userOp, bytes32 userOpHash) - external - returns (uint256); - - /// @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; - - /// @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; - - /// @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) - external - returns (bytes memory); - - /// @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; - /// @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. diff --git a/src/interfaces/IValidation.sol b/src/interfaces/IValidation.sol new file mode 100644 index 00000000..ac9c68e7 --- /dev/null +++ b/src/interfaces/IValidation.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: CC0-1.0 +pragma solidity ^0.8.25; + +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; + +import {IPlugin} from "./IPlugin.sol"; + +interface IValidation is IPlugin { + /// @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 validateUserOp(uint8 functionId, PackedUserOperation calldata userOp, bytes32 userOpHash) + external + returns (uint256); + + /// @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 validateRuntime(uint8 functionId, address sender, uint256 value, bytes calldata data) external; +} diff --git a/src/interfaces/IValidationHook.sol b/src/interfaces/IValidationHook.sol new file mode 100644 index 00000000..4cbee5e7 --- /dev/null +++ b/src/interfaces/IValidationHook.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: CC0-1.0 +pragma solidity ^0.8.25; + +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; + +import {IPlugin} from "./IPlugin.sol"; + +interface IValidationHook is IPlugin { + /// @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, PackedUserOperation calldata userOp, bytes32 userOpHash) + external + returns (uint256); + + /// @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; +} diff --git a/src/plugins/BasePlugin.sol b/src/plugins/BasePlugin.sol index 44145286..3b0fd521 100644 --- a/src/plugins/BasePlugin.sol +++ b/src/plugins/BasePlugin.sol @@ -1,10 +1,9 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.25; -import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; -import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +import {ERC165, IERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import {IPlugin, PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; +import {IPlugin} from "../interfaces/IPlugin.sol"; /// @title Base contract for plugins /// @dev Implements ERC-165 to support IPlugin's interface, which is a requirement @@ -13,124 +12,6 @@ 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. - 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. - 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, PackedUserOperation calldata userOp, bytes32 userOpHash) - external - virtual - returns (uint256) - { - (functionId, 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, PackedUserOperation calldata userOp, bytes32 userOpHash) - external - virtual - returns (uint256) - { - (functionId, 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); - 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); - 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) - external - virtual - returns (bytes memory) - { - (functionId, 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); - 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. - 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. - function pluginMetadata() external pure virtual returns (PluginMetadata memory); - /// @dev Returns true if this contract implements the interface defined by /// `interfaceId`. See the corresponding /// https://eips.ethereum.org/EIPS/eip-165#how-interfaces-are-identified[EIP section] @@ -143,7 +24,7 @@ abstract contract BasePlugin is ERC165, IPlugin { /// making calls to plugins. /// @param interfaceId The interface ID to check for support. /// @return True if the contract supports `interfaceId`. - function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { return interfaceId == type(IPlugin).interfaceId || super.supportsInterface(interfaceId); } } diff --git a/src/plugins/TokenReceiverPlugin.sol b/src/plugins/TokenReceiverPlugin.sol index 980d7397..1a9e8feb 100644 --- a/src/plugins/TokenReceiverPlugin.sol +++ b/src/plugins/TokenReceiverPlugin.sol @@ -5,6 +5,7 @@ import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Recei import {IERC1155Receiver} from "@openzeppelin/contracts/interfaces/IERC1155Receiver.sol"; import { + IPlugin, ManifestFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, @@ -52,15 +53,15 @@ contract TokenReceiverPlugin is BasePlugin, IERC721Receiver, IERC1155Receiver { // ┃ Plugin interface functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin // solhint-disable-next-line no-empty-blocks function onInstall(bytes calldata) external pure override {} - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin // solhint-disable-next-line no-empty-blocks function onUninstall(bytes calldata) external pure override {} - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; @@ -96,7 +97,7 @@ contract TokenReceiverPlugin is BasePlugin, IERC721Receiver, IERC1155Receiver { return manifest; } - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { PluginMetadata memory metadata; metadata.name = NAME; diff --git a/src/plugins/owner/ISingleOwnerPlugin.sol b/src/plugins/owner/ISingleOwnerPlugin.sol index d20296ef..0c3d8255 100644 --- a/src/plugins/owner/ISingleOwnerPlugin.sol +++ b/src/plugins/owner/ISingleOwnerPlugin.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.25; -interface ISingleOwnerPlugin { +import {IValidation} from "../../interfaces/IValidation.sol"; + +interface ISingleOwnerPlugin is IValidation { enum FunctionId { VALIDATION_OWNER_OR_SELF } diff --git a/src/plugins/owner/SingleOwnerPlugin.sol b/src/plugins/owner/SingleOwnerPlugin.sol index 38f16872..dc5bae13 100644 --- a/src/plugins/owner/SingleOwnerPlugin.sol +++ b/src/plugins/owner/SingleOwnerPlugin.sol @@ -18,7 +18,9 @@ import { SelectorPermission } from "../../interfaces/IPlugin.sol"; import {IStandardExecutor} from "../../interfaces/IStandardExecutor.sol"; -import {BasePlugin} from "../BasePlugin.sol"; +import {IPlugin} from "../../interfaces/IPlugin.sol"; +import {IValidation} from "../../interfaces/IValidation.sol"; +import {BasePlugin, IERC165} from "../BasePlugin.sol"; import {ISingleOwnerPlugin} from "./ISingleOwnerPlugin.sol"; /// @title Single Owner Plugin @@ -37,7 +39,7 @@ import {ISingleOwnerPlugin} from "./ISingleOwnerPlugin.sol"; /// the account, violating storage access rules. This also means that the /// owner of a modular account may not be another modular account if you want to /// send user operations through a bundler. -contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { +contract SingleOwnerPlugin is ISingleOwnerPlugin, BasePlugin, IERC1271 { using ECDSA for bytes32; using MessageHashUtils for bytes32; @@ -66,22 +68,18 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { // ┃ Plugin interface functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin function onInstall(bytes calldata data) external override { _transferOwnership(abi.decode(data, (address))); } - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin function onUninstall(bytes calldata) external override { _transferOwnership(address(0)); } - /// @inheritdoc BasePlugin - function runtimeValidationFunction(uint8 functionId, address sender, uint256, bytes calldata) - external - view - override - { + /// @inheritdoc IValidation + function validateRuntime(uint8 functionId, address sender, uint256, bytes calldata) external view override { if (functionId == uint8(FunctionId.VALIDATION_OWNER_OR_SELF)) { // Validate that the sender is the owner of the account or self. if (sender != _owners[msg.sender] && sender != msg.sender) { @@ -92,8 +90,8 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { revert NotImplemented(); } - /// @inheritdoc BasePlugin - function userOpValidationFunction(uint8 functionId, PackedUserOperation calldata userOp, bytes32 userOpHash) + /// @inheritdoc IValidation + function validateUserOp(uint8 functionId, PackedUserOperation calldata userOp, bytes32 userOpHash) external view override @@ -142,7 +140,7 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { return _owners[account]; } - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; @@ -199,7 +197,7 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { return manifest; } - /// @inheritdoc BasePlugin + /// @inheritdoc IPlugin function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { PluginMetadata memory metadata; metadata.name = NAME; @@ -224,7 +222,7 @@ contract SingleOwnerPlugin is BasePlugin, ISingleOwnerPlugin, IERC1271 { // ┗━━━━━━━━━━━━━━━┛ /// @inheritdoc BasePlugin - function supportsInterface(bytes4 interfaceId) public view override returns (bool) { + function supportsInterface(bytes4 interfaceId) public view override(BasePlugin, IERC165) returns (bool) { return interfaceId == type(ISingleOwnerPlugin).interfaceId || super.supportsInterface(interfaceId); } diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index 9b7cde45..a1db86c6 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -9,6 +9,7 @@ import { ManifestFunction, PluginManifest } from "../../src/interfaces/IPlugin.sol"; +import {IExecutionHook} from "../../src/interfaces/IExecutionHook.sol"; import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; @@ -70,7 +71,7 @@ contract AccountExecHooksTest is AccountTestBase { vm.expectEmit(true, true, true, true); emit ReceivedCall( abi.encodeWithSelector( - IPlugin.preExecutionHook.selector, + IExecutionHook.preExecutionHook.selector, _PRE_HOOK_FUNCTION_ID_1, address(this), // caller 0, // msg.value in call to account @@ -109,7 +110,7 @@ contract AccountExecHooksTest is AccountTestBase { // pre hook call emit ReceivedCall( abi.encodeWithSelector( - IPlugin.preExecutionHook.selector, + IExecutionHook.preExecutionHook.selector, _BOTH_HOOKS_FUNCTION_ID_3, address(this), // caller 0, // msg.value in call to account @@ -123,7 +124,7 @@ contract AccountExecHooksTest is AccountTestBase { vm.expectEmit(true, true, true, true); // post hook call emit ReceivedCall( - abi.encodeCall(IPlugin.postExecutionHook, (_BOTH_HOOKS_FUNCTION_ID_3, "")), + abi.encodeCall(IExecutionHook.postExecutionHook, (_BOTH_HOOKS_FUNCTION_ID_3, "")), 0 // msg value in call to plugin ); @@ -155,7 +156,7 @@ contract AccountExecHooksTest is AccountTestBase { vm.expectEmit(true, true, true, true); emit ReceivedCall( - abi.encodeCall(IPlugin.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), + abi.encodeCall(IExecutionHook.postExecutionHook, (_POST_HOOK_FUNCTION_ID_2, "")), 0 // msg value in call to plugin ); diff --git a/test/mocks/MockPlugin.sol b/test/mocks/MockPlugin.sol index 67891502..3a2bf984 100644 --- a/test/mocks/MockPlugin.sol +++ b/test/mocks/MockPlugin.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.19; import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import {PluginManifest, IPlugin, PluginMetadata} from "../../src/interfaces/IPlugin.sol"; +import {IValidation} from "../../src/interfaces/IValidation.sol"; +import {IExecutionHook} from "../../src/interfaces/IExecutionHook.sol"; contract MockPlugin is ERC165 { // It's super inefficient to hold the entire abi-encoded manifest in storage, but this is fine since it's @@ -79,9 +81,8 @@ 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.preExecutionHook.selector + msg.sig == IValidation.validateUserOp.selector || msg.sig == IValidation.validateRuntime.selector + || msg.sig == IExecutionHook.preExecutionHook.selector ) { // return 0 for userOp/runtimeVal case, return bytes("") for preExecutionHook case assembly ("memory-safe") { diff --git a/test/mocks/plugins/BaseTestPlugin.sol b/test/mocks/plugins/BaseTestPlugin.sol deleted file mode 100644 index 508845c1..00000000 --- a/test/mocks/plugins/BaseTestPlugin.sol +++ /dev/null @@ -1,12 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; - -import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; -import {PluginMetadata} from "../../../src/interfaces/IPlugin.sol"; - -contract BaseTestPlugin is BasePlugin { - // Don't need to implement this in each context - function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { - revert NotImplemented(); - } -} diff --git a/test/mocks/plugins/ComprehensivePlugin.sol b/test/mocks/plugins/ComprehensivePlugin.sol index 87f85210..edc8379e 100644 --- a/test/mocks/plugins/ComprehensivePlugin.sol +++ b/test/mocks/plugins/ComprehensivePlugin.sol @@ -11,10 +11,15 @@ import { PluginManifest, PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; +import {PluginManifest} from "../../../src/interfaces/IPlugin.sol"; +import {IValidation} from "../../../src/interfaces/IValidation.sol"; +import {IValidationHook} from "../../../src/interfaces/IValidationHook.sol"; +import {IExecutionHook} from "../../../src/interfaces/IExecutionHook.sol"; + import {IStandardExecutor} from "../../../src/interfaces/IStandardExecutor.sol"; import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; -contract ComprehensivePlugin is BasePlugin { +contract ComprehensivePlugin is IValidation, IValidationHook, IExecutionHook, BasePlugin { enum FunctionId { PRE_VALIDATION_HOOK_1, PRE_VALIDATION_HOOK_2, @@ -56,7 +61,7 @@ contract ComprehensivePlugin is BasePlugin { revert NotImplemented(); } - function userOpValidationFunction(uint8 functionId, PackedUserOperation calldata, bytes32) + function validateUserOp(uint8 functionId, PackedUserOperation calldata, bytes32) external pure override @@ -77,11 +82,7 @@ contract ComprehensivePlugin is BasePlugin { revert NotImplemented(); } - function runtimeValidationFunction(uint8 functionId, address, uint256, bytes calldata) - external - pure - override - { + function validateRuntime(uint8 functionId, address, uint256, bytes calldata) external pure override { if (functionId == uint8(FunctionId.VALIDATION)) { return; } diff --git a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol index 72c812bb..ae775dd0 100644 --- a/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol +++ b/test/mocks/plugins/ExecFromPluginPermissionsMocks.sol @@ -6,15 +6,16 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, ManifestExternalCallPermission, - PluginManifest + PluginManifest, + PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; import {IPluginExecutor} from "../../../src/interfaces/IPluginExecutor.sol"; -import {BaseTestPlugin} from "./BaseTestPlugin.sol"; +import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; import {ResultCreatorPlugin} from "./ReturnDataPluginMocks.sol"; import {Counter} from "../Counter.sol"; -contract EFPCallerPlugin is BaseTestPlugin { +contract EFPCallerPlugin is BasePlugin { // Store the counters as immutables, and use the view -> pure cast to get the manifest // solhint-disable private-vars-leading-underscore, immutable-vars-naming address private immutable counter1; @@ -107,6 +108,8 @@ contract EFPCallerPlugin is BaseTestPlugin { return _castToPure(_getManifest)(); } + function pluginMetadata() external pure override returns (PluginMetadata memory) {} + // The manifest requested access to use the plugin-defined method "foo" function useEFPPermissionAllowed() external returns (bytes memory) { return IPluginExecutor(msg.sender).executeFromPlugin(abi.encodeCall(ResultCreatorPlugin.foo, ())); @@ -187,7 +190,7 @@ contract EFPCallerPlugin is BaseTestPlugin { } } -contract EFPCallerPluginAnyExternal is BaseTestPlugin { +contract EFPCallerPluginAnyExternal is BasePlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -213,6 +216,8 @@ contract EFPCallerPluginAnyExternal is BaseTestPlugin { return manifest; } + function pluginMetadata() external pure override returns (PluginMetadata memory) {} + function passthroughExecute(address target, uint256 value, bytes calldata data) external payable diff --git a/test/mocks/plugins/ManifestValidityMocks.sol b/test/mocks/plugins/ManifestValidityMocks.sol index ed5370fb..2ea94f3c 100644 --- a/test/mocks/plugins/ManifestValidityMocks.sol +++ b/test/mocks/plugins/ManifestValidityMocks.sol @@ -5,13 +5,14 @@ import { ManifestFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, - PluginManifest + PluginManifest, + PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; -import {BaseTestPlugin} from "./BaseTestPlugin.sol"; +import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; // solhint-disable-next-line contract-name-camelcase -contract BadValidationMagicValue_PreValidationHook_Plugin is BaseTestPlugin { +contract BadValidationMagicValue_PreValidationHook_Plugin is BasePlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -49,10 +50,12 @@ contract BadValidationMagicValue_PreValidationHook_Plugin is BaseTestPlugin { return manifest; } + + function pluginMetadata() external pure override returns (PluginMetadata memory) {} } // solhint-disable-next-line contract-name-camelcase -contract BadHookMagicValue_UserOpValidationFunction_Plugin is BaseTestPlugin { +contract BadHookMagicValue_UserOpValidationFunction_Plugin is BasePlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -79,10 +82,12 @@ contract BadHookMagicValue_UserOpValidationFunction_Plugin is BaseTestPlugin { return manifest; } + + function pluginMetadata() external pure override returns (PluginMetadata memory) {} } // solhint-disable-next-line contract-name-camelcase -contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BaseTestPlugin { +contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BasePlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -109,4 +114,6 @@ contract BadHookMagicValue_RuntimeValidationFunction_Plugin is BaseTestPlugin { return manifest; } + + function pluginMetadata() external pure override returns (PluginMetadata memory) {} } diff --git a/test/mocks/plugins/ReturnDataPluginMocks.sol b/test/mocks/plugins/ReturnDataPluginMocks.sol index 153a4340..766bb903 100644 --- a/test/mocks/plugins/ReturnDataPluginMocks.sol +++ b/test/mocks/plugins/ReturnDataPluginMocks.sol @@ -6,11 +6,12 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, ManifestExternalCallPermission, - PluginManifest + PluginManifest, + PluginMetadata } from "../../../src/interfaces/IPlugin.sol"; import {IPluginExecutor} from "../../../src/interfaces/IPluginExecutor.sol"; -import {BaseTestPlugin} from "./BaseTestPlugin.sol"; +import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; contract RegularResultContract { function foo() external pure returns (bytes32) { @@ -22,7 +23,7 @@ contract RegularResultContract { } } -contract ResultCreatorPlugin is BaseTestPlugin { +contract ResultCreatorPlugin is BasePlugin { function onInstall(bytes calldata) external override {} function onUninstall(bytes calldata) external override {} @@ -54,9 +55,11 @@ contract ResultCreatorPlugin is BaseTestPlugin { return manifest; } + + function pluginMetadata() external pure override returns (PluginMetadata memory) {} } -contract ResultConsumerPlugin is BaseTestPlugin { +contract ResultConsumerPlugin is BasePlugin { ResultCreatorPlugin public immutable RESULT_CREATOR; RegularResultContract public immutable REGULAR_RESULT_CONTRACT; @@ -151,4 +154,6 @@ contract ResultConsumerPlugin is BaseTestPlugin { return manifest; } + + function pluginMetadata() external pure override returns (PluginMetadata memory) {} } diff --git a/test/mocks/plugins/ValidationPluginMocks.sol b/test/mocks/plugins/ValidationPluginMocks.sol index 937e5c65..48ca0ea1 100644 --- a/test/mocks/plugins/ValidationPluginMocks.sol +++ b/test/mocks/plugins/ValidationPluginMocks.sol @@ -7,11 +7,14 @@ import { ManifestFunction, ManifestAssociatedFunctionType, ManifestAssociatedFunction, + PluginMetadata, PluginManifest } from "../../../src/interfaces/IPlugin.sol"; -import {BaseTestPlugin} from "./BaseTestPlugin.sol"; +import {IValidation} from "../../../src/interfaces/IValidation.sol"; +import {IValidationHook} from "../../../src/interfaces/IValidationHook.sol"; +import {BasePlugin} from "../../../src/plugins/BasePlugin.sol"; -abstract contract MockBaseUserOpValidationPlugin is BaseTestPlugin { +abstract contract MockBaseUserOpValidationPlugin is IValidation, IValidationHook, BasePlugin { enum FunctionId { USER_OP_VALIDATION, PRE_VALIDATION_HOOK_1, @@ -44,7 +47,7 @@ abstract contract MockBaseUserOpValidationPlugin is BaseTestPlugin { revert NotImplemented(); } - function userOpValidationFunction(uint8 functionId, PackedUserOperation calldata, bytes32) + function validateUserOp(uint8 functionId, PackedUserOperation calldata, bytes32) external view override @@ -55,6 +58,17 @@ abstract contract MockBaseUserOpValidationPlugin is BaseTestPlugin { } revert NotImplemented(); } + + // Empty stubs + function pluginMetadata() external pure override returns (PluginMetadata memory) {} + + function preRuntimeValidationHook(uint8, address, uint256, bytes calldata) external pure override { + revert NotImplemented(); + } + + function validateRuntime(uint8, address, uint256, bytes calldata) external pure override { + revert NotImplemented(); + } } contract MockUserOpValidationPlugin is MockBaseUserOpValidationPlugin { diff --git a/test/plugin/SingleOwnerPlugin.t.sol b/test/plugin/SingleOwnerPlugin.t.sol index dea5b2f9..5c2ecb63 100644 --- a/test/plugin/SingleOwnerPlugin.t.sol +++ b/test/plugin/SingleOwnerPlugin.t.sol @@ -114,15 +114,11 @@ contract SingleOwnerPluginTest is OptimizedTest { assertEq(address(0), plugin.owner()); plugin.transferOwnership(owner1); assertEq(owner1, plugin.owner()); - plugin.runtimeValidationFunction( - uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER_OR_SELF), owner1, 0, "" - ); + plugin.validateRuntime(uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER_OR_SELF), owner1, 0, ""); vm.startPrank(b); vm.expectRevert(ISingleOwnerPlugin.NotAuthorized.selector); - plugin.runtimeValidationFunction( - uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER_OR_SELF), owner1, 0, "" - ); + plugin.validateRuntime(uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER_OR_SELF), owner1, 0, ""); } function testFuzz_validateUserOpSig(string memory salt, PackedUserOperation memory userOp) public { @@ -137,7 +133,7 @@ contract SingleOwnerPluginTest is OptimizedTest { userOp.signature = abi.encodePacked(r, s, v); // sig check should fail - uint256 success = plugin.userOpValidationFunction( + uint256 success = plugin.validateUserOp( uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER_OR_SELF), userOp, userOpHash ); assertEq(success, 1); @@ -147,7 +143,7 @@ contract SingleOwnerPluginTest is OptimizedTest { assertEq(signer, plugin.owner()); // sig check should pass - success = plugin.userOpValidationFunction( + success = plugin.validateUserOp( uint8(ISingleOwnerPlugin.FunctionId.VALIDATION_OWNER_OR_SELF), userOp, userOpHash ); assertEq(success, 0);