From 93dbef64eab2553c5368a757e63c23447b3e35f7 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 10 Jun 2024 11:40:00 -0400 Subject: [PATCH 1/4] feat: add execute user op --- src/account/UpgradeableModularAccount.sol | 76 ++++++++++++++++++----- 1 file changed, 60 insertions(+), 16 deletions(-) diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index ecdae1d4..e20880f7 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.25; import {BaseAccount} from "@eth-infinitism/account-abstraction/core/BaseAccount.sol"; import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {IAccountExecute} from "@eth-infinitism/account-abstraction/interfaces/IAccountExecute.sol"; import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol"; @@ -38,6 +39,7 @@ contract UpgradeableModularAccount is IERC165, IERC1271, IStandardExecutor, + IAccountExecute, PluginManagerInternals, PluginManager2, UUPSUpgradeable @@ -66,6 +68,7 @@ contract UpgradeableModularAccount is error ExecFromPluginNotPermitted(address plugin, bytes4 selector); error ExecFromPluginExternalNotPermitted(address plugin, address target, uint256 value, bytes data); error NativeTokenSpendingNotPermitted(address plugin); + error NotEntryPoint(); error PostExecHookReverted(address plugin, uint8 functionId, bytes revertReason); error PreExecHookReverted(address plugin, uint8 functionId, bytes revertReason); error PreRuntimeValidationHookFailed(address plugin, uint8 functionId, bytes revertReason); @@ -84,7 +87,7 @@ contract UpgradeableModularAccount is _checkPermittedCallerIfNotFromEP(); PostExecToRun[] memory postExecHooks = - _doPreExecHooks(getAccountStorage().selectorData[msg.sig].executionHooks, msg.data); + _doPreHooks(getAccountStorage().selectorData[msg.sig].executionHooks, msg.data, false); _; @@ -138,7 +141,7 @@ contract UpgradeableModularAccount is PostExecToRun[] memory postExecHooks; // Cache post-exec hooks in memory - postExecHooks = _doPreExecHooks(getAccountStorage().selectorData[msg.sig].executionHooks, msg.data); + postExecHooks = _doPreHooks(getAccountStorage().selectorData[msg.sig].executionHooks, msg.data, false); // execute the function, bubbling up any reverts (bool execSuccess, bytes memory execReturnData) = execPlugin.call(msg.data); @@ -155,6 +158,36 @@ contract UpgradeableModularAccount is return execReturnData; } + /// @notice Execution function that allows UO context to be passed to execution hooks + /// @dev This function is only callable by the EntryPoint + function executeUserOp(PackedUserOperation calldata userOp, bytes32) external { + if (msg.sender != address(_ENTRY_POINT)) { + revert NotEntryPoint(); + } + + FunctionReference userOpValidationFunction = FunctionReference.wrap(bytes21(userOp.signature[:21])); + + PostExecToRun[] memory postExecHooks = _doPreHooks( + getAccountStorage().selectorData[bytes4(userOp.callData[4:8])].executionHooks, abi.encode(userOp), true + ); + + PostExecToRun[] memory postPermissionHooks = _doPreHooks( + getAccountStorage().validationData[userOpValidationFunction].permissionHooks, abi.encode(userOp), true + ); + + (bool success, bytes memory result) = address(this).call(userOp.callData[4:]); + + if (!success) { + // Directly bubble up revert messages + assembly ("memory-safe") { + revert(add(result, 32), mload(result)) + } + } + + _doCachedPostExecHooks(postExecHooks); + _doCachedPostExecHooks(postPermissionHooks); + } + /// @inheritdoc IStandardExecutor function execute(address target, uint256 value, bytes calldata data) external @@ -444,12 +477,11 @@ contract UpgradeableModularAccount is } } - function _doPreExecHooks(EnumerableSet.Bytes32Set storage executionHooks, bytes calldata data) + function _doPreHooks(EnumerableSet.Bytes32Set storage executionHooks, bytes memory data, bool isPackedUO) internal returns (PostExecToRun[] memory postHooksToRun) { uint256 hooksLength = executionHooks.length(); - // Overallocate on length - not all of this may get filled up. We set the correct length later. postHooksToRun = new PostExecToRun[](hooksLength); @@ -457,13 +489,7 @@ contract UpgradeableModularAccount is // be sure that the set of hooks to run will not be affected by state changes mid-execution. for (uint256 i = 0; i < hooksLength; ++i) { bytes32 key = executionHooks.at(i); - (FunctionReference hookFunction,, bool isPostHook, bool requireUOContext) = toExecutionHook(key); - if (requireUOContext) { - /** - * && msg.sig != this.executeUserOp.selector - */ - revert RequireUserOperationContext(); - } + (FunctionReference hookFunction,, bool isPostHook,) = toExecutionHook(key); if (isPostHook) { postHooksToRun[i].postExecHook = hookFunction; } @@ -471,12 +497,31 @@ contract UpgradeableModularAccount is // Run the pre hooks and copy their return data to the post hooks array, if an associated post-exec hook // exists. + bool callNotToExecuteUserOp = msg.sig != this.executeUserOp.selector; for (uint256 i = 0; i < hooksLength; ++i) { bytes32 key = executionHooks.at(i); - (FunctionReference hookFunction, bool isPreHook, bool isPostHook,) = toExecutionHook(key); + (FunctionReference hookFunction, bool isPreHook, bool isPostHook, bool requireUOContext) = + toExecutionHook(key); + + if (requireUOContext && callNotToExecuteUserOp) { + revert RequireUserOperationContext(); + } if (isPreHook) { - bytes memory preExecHookReturnData = _runPreExecHook(hookFunction, data); + bytes memory preExecHookReturnData; + + if (isPackedUO) { + if (requireUOContext) { + preExecHookReturnData = _runPreExecHook(hookFunction, data); + } else { + PackedUserOperation memory uo = abi.decode(data, (PackedUserOperation)); + preExecHookReturnData = + _runPreExecHook(hookFunction, abi.encodePacked(msg.sender, msg.value, uo.callData)); + } + } else { + preExecHookReturnData = + _runPreExecHook(hookFunction, abi.encodePacked(msg.sender, msg.value, data)); + } // If there is an associated post-exec hook, save the return data. if (isPostHook) { @@ -486,13 +531,12 @@ contract UpgradeableModularAccount is } } - function _runPreExecHook(FunctionReference preExecHook, bytes calldata data) + function _runPreExecHook(FunctionReference preExecHook, bytes memory data) internal returns (bytes memory preExecHookReturnData) { (address plugin, uint8 functionId) = preExecHook.unpack(); - try IExecutionHook(plugin).preExecutionHook(functionId, abi.encodePacked(msg.sender, msg.value, data)) - returns (bytes memory returnData) { + try IExecutionHook(plugin).preExecutionHook(functionId, data) returns (bytes memory returnData) { preExecHookReturnData = returnData; } catch (bytes memory revertReason) { revert PreExecHookReverted(plugin, functionId, revertReason); From 6396656157123bf687a7808f14a66771bbc96c7e Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:35:52 -0400 Subject: [PATCH 2/4] fix: add selector check back into _validateSignature --- src/account/UpgradeableModularAccount.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index e20880f7..7dc36973 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -379,10 +379,10 @@ contract UpgradeableModularAccount is // call isn't to `executeUserOp` // This check must be here because if context isn't passed, we wouldn't be able to get the exec hooks // associated with the validator - if (getAccountStorage().validationData[userOpValidationFunction].requireUOHookCount > 0) { - /** - * && msg.sig != this.executeUserOp.selector - */ + if ( + getAccountStorage().validationData[userOpValidationFunction].requireUOHookCount > 0 + && bytes4(userOp.callData[:4]) != this.executeUserOp.selector + ) { revert RequireUserOperationContext(); } From 7c2b1df817bc34ba94cb23fe27b1f7422f0b3fef Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:44:31 -0400 Subject: [PATCH 3/4] chore: simplify --- src/account/UpgradeableModularAccount.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 7dc36973..8c8aaf90 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -167,14 +167,14 @@ contract UpgradeableModularAccount is FunctionReference userOpValidationFunction = FunctionReference.wrap(bytes21(userOp.signature[:21])); - PostExecToRun[] memory postExecHooks = _doPreHooks( - getAccountStorage().selectorData[bytes4(userOp.callData[4:8])].executionHooks, abi.encode(userOp), true - ); - PostExecToRun[] memory postPermissionHooks = _doPreHooks( getAccountStorage().validationData[userOpValidationFunction].permissionHooks, abi.encode(userOp), true ); + PostExecToRun[] memory postExecHooks = _doPreHooks( + getAccountStorage().selectorData[bytes4(userOp.callData[4:8])].executionHooks, abi.encode(userOp), true + ); + (bool success, bytes memory result) = address(this).call(userOp.callData[4:]); if (!success) { @@ -497,13 +497,12 @@ contract UpgradeableModularAccount is // Run the pre hooks and copy their return data to the post hooks array, if an associated post-exec hook // exists. - bool callNotToExecuteUserOp = msg.sig != this.executeUserOp.selector; for (uint256 i = 0; i < hooksLength; ++i) { bytes32 key = executionHooks.at(i); (FunctionReference hookFunction, bool isPreHook, bool isPostHook, bool requireUOContext) = toExecutionHook(key); - if (requireUOContext && callNotToExecuteUserOp) { + if (!isPackedUO && requireUOContext) { revert RequireUserOperationContext(); } From eb191850f70300ef52a72cf80e7e3523b660756d Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 17 Jun 2024 19:31:35 -0400 Subject: [PATCH 4/4] feat: permission hooks installation via installValidation --- src/account/PluginManager2.sol | 68 ++++++++++++++++++----- src/account/UpgradeableModularAccount.sol | 29 ++++++++-- src/interfaces/IPluginManager.sol | 11 +++- test/account/AccountLoupe.t.sol | 7 ++- test/account/ValidationIntersection.t.sol | 14 ++++- 5 files changed, 105 insertions(+), 24 deletions(-) diff --git a/src/account/PluginManager2.sol b/src/account/PluginManager2.sol index 1b1745b0..afa2963e 100644 --- a/src/account/PluginManager2.sol +++ b/src/account/PluginManager2.sol @@ -7,6 +7,7 @@ import {IPlugin} from "../interfaces/IPlugin.sol"; import {FunctionReference} from "../interfaces/IPluginManager.sol"; import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol"; import {AccountStorage, getAccountStorage, toSetValue, toFunctionReference} from "./AccountStorage.sol"; +import {ExecutionHook} from "../interfaces/IAccountLoupe.sol"; abstract contract PluginManager2 { using EnumerableSet for EnumerableSet.Bytes32Set; @@ -15,13 +16,15 @@ abstract contract PluginManager2 { error ValidationAlreadySet(bytes4 selector, FunctionReference validationFunction); error PreValidationAlreadySet(FunctionReference validationFunction, FunctionReference preValidationFunction); error ValidationNotSet(bytes4 selector, FunctionReference validationFunction); + error PermissionAlreadySet(FunctionReference validationFunction, ExecutionHook hook); function _installValidation( FunctionReference validationFunction, bool shared, bytes4[] memory selectors, bytes calldata installData, - bytes memory preValidationHooks + bytes memory preValidationHooks, + bytes memory permissionHooks ) // TODO: flag for signature validation internal @@ -50,6 +53,26 @@ abstract contract PluginManager2 { } } + if (permissionHooks.length > 0) { + (ExecutionHook[] memory permissionFunctions, bytes[] memory initDatas) = + abi.decode(permissionHooks, (ExecutionHook[], bytes[])); + + for (uint256 i = 0; i < permissionFunctions.length; ++i) { + ExecutionHook memory permissionFunction = permissionFunctions[i]; + + if ( + !_storage.validationData[validationFunction].permissionHooks.add(toSetValue(permissionFunction)) + ) { + revert PermissionAlreadySet(validationFunction, permissionFunction); + } + + if (initDatas[i].length > 0) { + (address executionPlugin,) = FunctionReferenceLib.unpack(permissionFunction.hookFunction); + IPlugin(executionPlugin).onInstall(initDatas[i]); + } + } + } + if (shared) { if (_storage.validationData[validationFunction].isShared) { revert DefaultValidationAlreadySet(validationFunction); @@ -74,24 +97,43 @@ abstract contract PluginManager2 { FunctionReference validationFunction, bytes4[] calldata selectors, bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData + bytes calldata preValidationHookUninstallData, + bytes calldata permissionHookUninstallData ) internal { AccountStorage storage _storage = getAccountStorage(); _storage.validationData[validationFunction].isShared = false; _storage.validationData[validationFunction].isSignatureValidation = false; - bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[])); - - // Clear pre validation hooks - EnumerableSet.Bytes32Set storage preValidationHooks = - _storage.validationData[validationFunction].preValidationHooks; - while (preValidationHooks.length() > 0) { - FunctionReference preValidationFunction = toFunctionReference(preValidationHooks.at(0)); - preValidationHooks.remove(toSetValue(preValidationFunction)); - (address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction); - if (preValidationHookUninstallDatas[0].length > 0) { - IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]); + { + bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[])); + + // Clear pre validation hooks + EnumerableSet.Bytes32Set storage preValidationHooks = + _storage.validationData[validationFunction].preValidationHooks; + while (preValidationHooks.length() > 0) { + FunctionReference preValidationFunction = toFunctionReference(preValidationHooks.at(0)); + preValidationHooks.remove(toSetValue(preValidationFunction)); + (address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction); + if (preValidationHookUninstallDatas[0].length > 0) { + IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]); + } + } + } + + { + bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[])); + + // Clear permission hooks + EnumerableSet.Bytes32Set storage permissionHooks = + _storage.validationData[validationFunction].permissionHooks; + while (permissionHooks.length() > 0) { + FunctionReference permissionHook = toFunctionReference(permissionHooks.at(0)); + permissionHooks.remove(toSetValue(permissionHook)); + (address permissionHookPlugin,) = FunctionReferenceLib.unpack(permissionHook); + if (permissionHookUninstallDatas[0].length > 0) { + IPlugin(permissionHookPlugin).onUninstall(permissionHookUninstallDatas[0]); + } } } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 8c8aaf90..aa74ecd2 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -231,8 +231,11 @@ contract UpgradeableModularAccount is _doRuntimeValidation(runtimeValidationFunction, data, authorization[22:]); - // If runtime validation passes, execute the call + // If runtime validation passes, do runtime permission checks + PostExecToRun[] memory postPermissionHooks = + _doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].permissionHooks, data, false); + // Execute the call (bool success, bytes memory returnData) = address(this).call(data); if (!success) { @@ -241,6 +244,8 @@ contract UpgradeableModularAccount is } } + _doCachedPostExecHooks(postPermissionHooks); + return returnData; } @@ -280,7 +285,7 @@ contract UpgradeableModularAccount is external initializer { - _installValidation(validationFunction, true, new bytes4[](0), installData, bytes("")); + _installValidation(validationFunction, true, new bytes4[](0), installData, bytes(""), bytes("")); emit ModularAccountInitialized(_ENTRY_POINT); } @@ -290,9 +295,10 @@ contract UpgradeableModularAccount is bool shared, bytes4[] memory selectors, bytes calldata installData, - bytes calldata preValidationHooks + bytes calldata preValidationHooks, + bytes calldata permissionHooks ) external wrapNativeFunction { - _installValidation(validationFunction, shared, selectors, installData, preValidationHooks); + _installValidation(validationFunction, shared, selectors, installData, preValidationHooks, permissionHooks); } /// @inheritdoc IPluginManager @@ -300,9 +306,16 @@ contract UpgradeableModularAccount is FunctionReference validationFunction, bytes4[] calldata selectors, bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData + bytes calldata preValidationHookUninstallData, + bytes calldata permissionHookUninstallData ) external wrapNativeFunction { - _uninstallValidation(validationFunction, selectors, uninstallData, preValidationHookUninstallData); + _uninstallValidation( + validationFunction, + selectors, + uninstallData, + preValidationHookUninstallData, + permissionHookUninstallData + ); } /// @notice ERC165 introspection @@ -369,6 +382,9 @@ contract UpgradeableModularAccount is revert UnrecognizedFunction(bytes4(userOp.callData)); } bytes4 selector = bytes4(userOp.callData); + if (selector == this.executeUserOp.selector) { + selector = bytes4(userOp.callData[4:8]); + } FunctionReference userOpValidationFunction = FunctionReference.wrap(bytes21(userOp.signature[:21])); bool isSharedValidation = uint8(userOp.signature[21]) == 1; @@ -538,6 +554,7 @@ contract UpgradeableModularAccount is try IExecutionHook(plugin).preExecutionHook(functionId, data) returns (bytes memory returnData) { preExecHookReturnData = returnData; } catch (bytes memory revertReason) { + // TODO: same issue with EP0.6 - we can't do bytes4 error codes in plugins revert PreExecHookReverted(plugin, functionId, revertReason); } } diff --git a/src/interfaces/IPluginManager.sol b/src/interfaces/IPluginManager.sol index f3809211..f7e7220b 100644 --- a/src/interfaces/IPluginManager.sol +++ b/src/interfaces/IPluginManager.sol @@ -32,12 +32,15 @@ interface IPluginManager { /// @param shared Whether the validation function is shared across all selectors in the default pool. /// @param selectors The selectors to install the validation function for. /// @param installData Optional data to be decoded and used by the plugin to setup initial plugin state. + /// @param preValidationHooks Optional pre-validation hooks to install for the validation function. + /// @param permissionHooks Optional permission hooks to install for the validation function. function installValidation( FunctionReference validationFunction, bool shared, bytes4[] memory selectors, bytes calldata installData, - bytes calldata preValidationHooks + bytes calldata preValidationHooks, + bytes calldata permissionHooks ) external; /// @notice Uninstall a validation function from a set of execution selectors. @@ -46,11 +49,15 @@ interface IPluginManager { /// @param selectors The selectors to uninstall the validation function for. /// @param uninstallData Optional data to be decoded and used by the plugin to clear plugin data for the /// account. + /// @param preValidationHookUninstallData Optional data to be decoded and used by the plugin to clear account + /// data + /// @param permissionHookUninstallData Optional data to be decoded and used by the plugin to clear account data function uninstallValidation( FunctionReference validationFunction, bytes4[] calldata selectors, bytes calldata uninstallData, - bytes calldata preValidationHookUninstallData + bytes calldata preValidationHookUninstallData, + bytes calldata permissionHookUninstallData ) external; /// @notice Uninstall a plugin from the modular account. diff --git a/test/account/AccountLoupe.t.sol b/test/account/AccountLoupe.t.sol index e21f232f..aebc65af 100644 --- a/test/account/AccountLoupe.t.sol +++ b/test/account/AccountLoupe.t.sol @@ -43,7 +43,12 @@ contract AccountLoupeTest is AccountTestBase { bytes[] memory installDatas = new bytes[](2); vm.prank(address(entryPoint)); account1.installValidation( - ownerValidation, true, new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas) + ownerValidation, + true, + new bytes4[](0), + bytes(""), + abi.encode(preValidationHooks, installDatas), + bytes("") ); } diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 877f8ff9..6f451a16 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -67,7 +67,12 @@ contract ValidationIntersectionTest is AccountTestBase { }); bytes[] memory installDatas = new bytes[](1); account1.installValidation( - oneHookValidation, true, new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas) + oneHookValidation, + true, + new bytes4[](0), + bytes(""), + abi.encode(preValidationHooks, installDatas), + bytes("") ); account1.installPlugin({ plugin: address(twoHookPlugin), @@ -87,7 +92,12 @@ contract ValidationIntersectionTest is AccountTestBase { }); installDatas = new bytes[](2); account1.installValidation( - twoHookValidation, true, new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas) + twoHookValidation, + true, + new bytes4[](0), + bytes(""), + abi.encode(preValidationHooks, installDatas), + bytes("") ); vm.stopPrank(); }