Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/account/AccountLoupe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet
import {IAccountLoupe, ExecutionHook} from "../interfaces/IAccountLoupe.sol";
import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol";
import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
import {getAccountStorage, SelectorData, toFunctionReferenceArray, toExecutionHook} from "./AccountStorage.sol";
import {getAccountStorage, toFunctionReferenceArray, toExecutionHook} from "./AccountStorage.sol";

abstract contract AccountLoupe is IAccountLoupe {
using EnumerableSet for EnumerableSet.Bytes32Set;
Expand Down Expand Up @@ -39,18 +39,36 @@ abstract contract AccountLoupe is IAccountLoupe {
override
returns (ExecutionHook[] memory execHooks)
{
SelectorData storage selectorData = getAccountStorage().selectorData[selector];
uint256 executionHooksLength = selectorData.executionHooks.length();
EnumerableSet.Bytes32Set storage hooks = getAccountStorage().selectorData[selector].executionHooks;
uint256 executionHooksLength = hooks.length();

execHooks = new ExecutionHook[](executionHooksLength);

for (uint256 i = 0; i < executionHooksLength; ++i) {
bytes32 key = selectorData.executionHooks.at(i);
bytes32 key = hooks.at(i);
ExecutionHook memory execHook = execHooks[i];
(execHook.hookFunction, execHook.isPreHook, execHook.isPostHook) = toExecutionHook(key);
}
}

/// @inheritdoc IAccountLoupe
function getPermissionHooks(FunctionReference validationFunction)
external
view
override
returns (ExecutionHook[] memory permissionHooks)
{
EnumerableSet.Bytes32Set storage hooks =
getAccountStorage().validationData[validationFunction].permissionHooks;
uint256 executionHooksLength = hooks.length();
permissionHooks = new ExecutionHook[](executionHooksLength);
for (uint256 i = 0; i < executionHooksLength; ++i) {
bytes32 key = hooks.at(i);
ExecutionHook memory execHook = permissionHooks[i];
(execHook.hookFunction, execHook.isPreHook, execHook.isPostHook) = toExecutionHook(key);
}
}

/// @inheritdoc IAccountLoupe
function getPreValidationHooks(FunctionReference validationFunction)
external
Expand Down
5 changes: 4 additions & 1 deletion src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@ struct ValidationData {
bool isDefault;
// Whether or not this validation is a signature validator.
bool isSignatureValidation;
// How many execution hooks require the UO context.
uint8 requireUOHookCount;
// The pre validation hooks for this function selector.
EnumerableSet.Bytes32Set preValidationHooks;
// Permission hooks for this validation function.
EnumerableSet.Bytes32Set permissionHooks;
}

struct AccountStorage {
Expand All @@ -52,7 +56,6 @@ struct AccountStorage {
// Execution functions and their associated functions
mapping(bytes4 => SelectorData) selectorData;
mapping(FunctionReference validationFunction => ValidationData) validationData;
mapping(address caller => mapping(bytes4 selector => bool)) callPermitted;
// For ERC165 introspection
mapping(bytes4 => uint256) supportedIfaces;
}
Expand Down
28 changes: 8 additions & 20 deletions src/account/PluginManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,25 +125,25 @@ abstract contract PluginManagerInternals is IPluginManager {
}

function _addExecHooks(
bytes4 selector,
EnumerableSet.Bytes32Set storage hooks,
FunctionReference hookFunction,
bool isPreExecHook,
bool isPostExecHook
) internal {
getAccountStorage().selectorData[selector].executionHooks.add(
hooks.add(
toSetValue(
ExecutionHook({hookFunction: hookFunction, isPreHook: isPreExecHook, isPostHook: isPostExecHook})
)
);
}

function _removeExecHooks(
bytes4 selector,
EnumerableSet.Bytes32Set storage hooks,
FunctionReference hookFunction,
bool isPreExecHook,
bool isPostExecHook
) internal {
getAccountStorage().selectorData[selector].executionHooks.remove(
hooks.remove(
toSetValue(
ExecutionHook({hookFunction: hookFunction, isPreHook: isPreExecHook, isPostHook: isPostExecHook})
)
Expand Down Expand Up @@ -212,14 +212,6 @@ abstract contract PluginManagerInternals is IPluginManager {
_setExecutionFunction(selector, isPublic, allowDefaultValidation, plugin);
}

// Add installed plugin and selectors this plugin can call
length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length; ++i) {
// If there are duplicates, this will just enable the flag again. This is not a problem, since the
// boolean will be set to false twice during uninstall, which is fine.
_storage.callPermitted[plugin][manifest.permittedExecutionSelectors[i]] = true;
}

length = manifest.validationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mv = manifest.validationFunctions[i];
Expand All @@ -238,8 +230,9 @@ abstract contract PluginManagerInternals is IPluginManager {
length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks;
FunctionReference hookFunction = FunctionReferenceLib.pack(plugin, mh.functionId);
_addExecHooks(mh.executionSelector, hookFunction, mh.isPreHook, mh.isPostHook);
_addExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook);
}

length = manifest.interfaceIds.length;
Expand Down Expand Up @@ -290,12 +283,12 @@ abstract contract PluginManagerInternals is IPluginManager {
}

// Remove components according to the manifest, in reverse order (by component type) of their installation.

length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
FunctionReference hookFunction = FunctionReferenceLib.pack(plugin, mh.functionId);
_removeExecHooks(mh.executionSelector, hookFunction, mh.isPreHook, mh.isPostHook);
EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks;
_removeExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook);
}

length = manifest.signatureValidationFunctions.length;
Expand All @@ -313,11 +306,6 @@ abstract contract PluginManagerInternals is IPluginManager {
);
}

length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length; ++i) {
_storage.callPermitted[plugin][manifest.permittedExecutionSelectors[i]] = false;
}

length = manifest.executionFunctions.length;
for (uint256 i = 0; i < length; ++i) {
bytes4 selector = manifest.executionFunctions[i].executionSelector;
Expand Down
36 changes: 22 additions & 14 deletions src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {AccountLoupe} from "./AccountLoupe.sol";
import {
AccountStorage,
getAccountStorage,
SelectorData,
toSetValue,
toFunctionReference,
toExecutionHook
Expand Down Expand Up @@ -70,6 +69,7 @@ contract UpgradeableModularAccount is
error PostExecHookReverted(address plugin, uint8 functionId, bytes revertReason);
error PreExecHookReverted(address plugin, uint8 functionId, bytes revertReason);
error PreRuntimeValidationHookFailed(address plugin, uint8 functionId, bytes revertReason);
error RequireUserOperationContext();
error RuntimeValidationFunctionMissing(bytes4 selector);
error RuntimeValidationFunctionReverted(address plugin, uint8 functionId, bytes revertReason);
error SignatureValidationInvalid(address plugin, uint8 functionId);
Expand All @@ -83,7 +83,8 @@ contract UpgradeableModularAccount is
modifier wrapNativeFunction() {
_checkPermittedCallerIfNotFromEP();

PostExecToRun[] memory postExecHooks = _doPreExecHooks(msg.sig, msg.data);
PostExecToRun[] memory postExecHooks =
_doPreExecHooks(getAccountStorage().selectorData[msg.sig].executionHooks, msg.data);

_;

Expand Down Expand Up @@ -137,7 +138,7 @@ contract UpgradeableModularAccount is

PostExecToRun[] memory postExecHooks;
// Cache post-exec hooks in memory
postExecHooks = _doPreExecHooks(msg.sig, msg.data);
postExecHooks = _doPreExecHooks(getAccountStorage().selectorData[msg.sig].executionHooks, msg.data);

// execute the function, bubbling up any reverts
(bool execSuccess, bytes memory execReturnData) = execPlugin.call(msg.data);
Expand Down Expand Up @@ -350,6 +351,17 @@ contract UpgradeableModularAccount is

_checkIfValidationApplies(selector, userOpValidationFunction, isDefaultValidation);

// Check if there are exec hooks associated with the validator that require UO context, and revert if the
// 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
*/
revert RequireUserOperationContext();
}

validationData =
_doUserOpValidation(selector, userOpValidationFunction, userOp, userOp.signature[22:], userOpHash);
}
Expand Down Expand Up @@ -441,21 +453,19 @@ contract UpgradeableModularAccount is
}
}

function _doPreExecHooks(bytes4 selector, bytes calldata data)
function _doPreExecHooks(EnumerableSet.Bytes32Set storage executionHooks, bytes calldata data)
internal
returns (PostExecToRun[] memory postHooksToRun)
{
SelectorData storage selectorData = getAccountStorage().selectorData[selector];

uint256 hooksLength = selectorData.executionHooks.length();
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);

// Copy all post hooks to the array. This happens before any pre hooks are run, so we can
// 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 = selectorData.executionHooks.at(i);
bytes32 key = executionHooks.at(i);
(FunctionReference hookFunction,, bool isPostHook) = toExecutionHook(key);
if (isPostHook) {
postHooksToRun[i].postExecHook = hookFunction;
Expand All @@ -465,7 +475,7 @@ contract UpgradeableModularAccount is
// Run the pre hooks and copy their return data to the post hooks array, if an associated post-exec hook
// exists.
for (uint256 i = 0; i < hooksLength; ++i) {
bytes32 key = selectorData.executionHooks.at(i);
bytes32 key = executionHooks.at(i);
(FunctionReference hookFunction, bool isPreHook, bool isPostHook) = toExecutionHook(key);

if (isPreHook) {
Expand Down Expand Up @@ -555,11 +565,9 @@ contract UpgradeableModularAccount is
AccountStorage storage _storage = getAccountStorage();

if (
msg.sender == address(_ENTRY_POINT) || msg.sender == address(this)
|| _storage.selectorData[msg.sig].isPublic
) return;

if (!_storage.callPermitted[msg.sender][msg.sig]) {
msg.sender != address(_ENTRY_POINT) && msg.sender != address(this)
&& !_storage.selectorData[msg.sig].isPublic
) {
revert ExecFromPluginNotPermitted(msg.sender, msg.sig);
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/interfaces/IAccountLoupe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ interface IAccountLoupe {
/// @return The pre and post execution hooks for this selector.
function getExecutionHooks(bytes4 selector) external view returns (ExecutionHook[] memory);

/// @notice Get the pre and post execution hooks for a validation function.
/// @param validationFunction The validation function to get the hooks for.
/// @return The pre and post execution hooks for this validation function.
function getPermissionHooks(FunctionReference validationFunction)
external
view
returns (ExecutionHook[] memory);

/// @notice Get the pre user op and runtime validation hooks associated with a selector.
/// @param validationFunction The validation function to get the hooks for.
/// @return preValidationHooks The pre validation hooks for this selector.
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/IPlugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ struct PluginMetadata {
// String desciptions of the relative sensitivity of specific functions. The selectors MUST be selectors for
// functions implemented by this plugin.
SelectorPermission[] permissionDescriptors;
// A list of all ERC-7715 permission strings that the plugin could possibly use
string[] permissionRequest;
}

/// @dev A struct describing how the plugin should be installed on a modular account.
Expand All @@ -73,8 +75,6 @@ struct PluginManifest {
ManifestAssociatedFunction[] validationFunctions;
ManifestExecutionHook[] executionHooks;
uint8[] signatureValidationFunctions;
// Plugin execution functions already installed on the MSCA that this plugin will be able to call.
bytes4[] permittedExecutionSelectors;
// List of ERC-165 interface IDs to add to account to support introspection checks. This MUST NOT include
// IPlugin's interface ID.
bytes4[] interfaceIds;
Expand Down
6 changes: 2 additions & 4 deletions test/account/AccountExecHooks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {AccountTestBase} from "../utils/AccountTestBase.sol";

contract AccountExecHooksTest is AccountTestBase {
MockPlugin public mockPlugin1;
MockPlugin public mockPlugin2;
bytes32 public manifestHash1;
bytes32 public manifestHash2;

Expand All @@ -25,7 +24,6 @@ contract AccountExecHooksTest is AccountTestBase {
uint8 internal constant _BOTH_HOOKS_FUNCTION_ID_3 = 3;

PluginManifest internal _m1;
PluginManifest internal _m2;

event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
Expand Down Expand Up @@ -66,7 +64,7 @@ contract AccountExecHooksTest is AccountTestBase {
IExecutionHook.preExecutionHook.selector,
_PRE_HOOK_FUNCTION_ID_1,
address(this), // caller
0, // msg.value in call to account
uint256(0), // msg.value in call to account
abi.encodeWithSelector(_EXEC_SELECTOR)
),
0 // msg value in call to plugin
Expand Down Expand Up @@ -105,7 +103,7 @@ contract AccountExecHooksTest is AccountTestBase {
IExecutionHook.preExecutionHook.selector,
_BOTH_HOOKS_FUNCTION_ID_3,
address(this), // caller
0, // msg.value in call to account
uint256(0), // msg.value in call to account
abi.encodeWithSelector(_EXEC_SELECTOR)
),
0 // msg value in call to plugin
Expand Down
4 changes: 1 addition & 3 deletions test/account/UpgradeableModularAccount.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol";
import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol";
import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol";
import {IPlugin, PluginManifest} from "../../src/interfaces/IPlugin.sol";
import {PluginManifest} from "../../src/interfaces/IPlugin.sol";
import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol";
import {IPluginManager} from "../../src/interfaces/IPluginManager.sol";
import {Call} from "../../src/interfaces/IStandardExecutor.sol";
Expand Down Expand Up @@ -262,8 +262,6 @@ contract UpgradeableModularAccountTest is AccountTestBase {
vm.startPrank(address(entryPoint));

PluginManifest memory m;
m.permittedExecutionSelectors = new bytes4[](1);
m.permittedExecutionSelectors[0] = IPlugin.onInstall.selector;

MockPlugin mockPluginWithBadPermittedExec = new MockPlugin(m);
bytes32 manifestHash = keccak256(abi.encode(mockPluginWithBadPermittedExec.pluginManifest()));
Expand Down
4 changes: 0 additions & 4 deletions test/mocks/plugins/PermittedCallMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ contract PermittedCallerPlugin is BasePlugin {
manifest.executionFunctions[i].isPublic = true;
}

// Request permission only for "foo", but not "bar", from ResultCreatorPlugin
manifest.permittedExecutionSelectors = new bytes4[](1);
manifest.permittedExecutionSelectors[0] = ResultCreatorPlugin.foo.selector;

return manifest;
}

Expand Down
3 changes: 0 additions & 3 deletions test/mocks/plugins/ReturnDataPluginMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,6 @@ contract ResultConsumerPlugin is BasePlugin, IValidation {
allowDefaultValidation: false
});

manifest.permittedExecutionSelectors = new bytes4[](1);
manifest.permittedExecutionSelectors[0] = ResultCreatorPlugin.foo.selector;

return manifest;
}

Expand Down