Skip to content

chore: rename permissions hook to execution hook #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 30, 2024
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
4 changes: 2 additions & 2 deletions src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ struct ValidationData {
bool isUserOpValidation;
// The pre validation hooks for this validation function.
ModuleEntity[] preValidationHooks;
// Permission hooks for this validation function.
EnumerableSet.Bytes32Set permissionHooks;
// Execution hooks to run with this validation function.
EnumerableSet.Bytes32Set executionHooks;
// The set of selectors that may be validated by this validation function.
EnumerableSet.Bytes32Set selectors;
}
Expand Down
8 changes: 4 additions & 4 deletions src/account/ModularAccountView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ abstract contract ModularAccountView is IModularAccountView {
data.isUserOpValidation = validationData.isUserOpValidation;
data.preValidationHooks = validationData.preValidationHooks;

uint256 permissionHooksLen = validationData.permissionHooks.length();
data.permissionHooks = new HookConfig[](permissionHooksLen);
for (uint256 i = 0; i < permissionHooksLen; ++i) {
data.permissionHooks[i] = toHookConfig(validationData.permissionHooks.at(i));
uint256 execHooksLen = validationData.executionHooks.length();
data.executionHooks = new HookConfig[](execHooksLen);
for (uint256 i = 0; i < execHooksLen; ++i) {
data.executionHooks[i] = toHookConfig(validationData.executionHooks.at(i));
}

bytes32[] memory selectors = validationData.selectors.values();
Expand Down
30 changes: 15 additions & 15 deletions src/account/ModuleManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
error InterfaceNotSupported(address module);
error NativeFunctionNotAllowed(bytes4 selector);
error NullModule();
error PermissionAlreadySet(ModuleEntity validationFunction, HookConfig hookConfig);
error ExecutionHookAlreadySet(HookConfig hookConfig);
error ModuleInstallCallbackFailed(address module, bytes revertReason);
error ModuleNotInstalled(address module);
error PreValidationHookLimitExceeded();
Expand Down Expand Up @@ -103,7 +103,9 @@ abstract contract ModuleManagerInternals is IModularAccount {
}

function _addExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
hooks.add(toSetValue(hookConfig));
if (!hooks.add(toSetValue(hookConfig))) {
revert ExecutionHookAlreadySet(hookConfig);
}
}

function _removeExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
Expand Down Expand Up @@ -244,10 +246,8 @@ abstract contract ModuleManagerInternals is IModularAccount {

continue;
}
// Hook is a permission hook
if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) {
revert PermissionAlreadySet(moduleEntity, hookConfig);
}
// Hook is an execution hook
_addExecHooks(_validationData.executionHooks, hookConfig);

_onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId);
}
Expand Down Expand Up @@ -282,12 +282,12 @@ abstract contract ModuleManagerInternals is IModularAccount {
// If any uninstall data is provided, assert it is of the correct length.
if (
hookUninstallDatas.length
!= _validationData.preValidationHooks.length + _validationData.permissionHooks.length()
!= _validationData.preValidationHooks.length + _validationData.executionHooks.length()
) {
revert ArrayLengthMismatch();
}

// Hook uninstall data is provided in the order of pre validation hooks, then permission hooks.
// Hook uninstall data is provided in the order of pre validation hooks, then execution hooks.
uint256 hookIndex = 0;
for (uint256 i = 0; i < _validationData.preValidationHooks.length; ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
Expand All @@ -296,10 +296,10 @@ abstract contract ModuleManagerInternals is IModularAccount {
hookIndex++;
}

for (uint256 i = 0; i < _validationData.permissionHooks.length(); ++i) {
for (uint256 i = 0; i < _validationData.executionHooks.length(); ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) =
ModuleEntityLib.unpack(toModuleEntity(_validationData.permissionHooks.at(i)));
ModuleEntityLib.unpack(toModuleEntity(_validationData.executionHooks.at(i)));
onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData);
hookIndex++;
}
Expand All @@ -308,11 +308,11 @@ abstract contract ModuleManagerInternals is IModularAccount {
// Clear all stored hooks
delete _validationData.preValidationHooks;

EnumerableSet.Bytes32Set storage permissionHooks = _validationData.permissionHooks;
uint256 permissionHookLen = permissionHooks.length();
for (uint256 i = 0; i < permissionHookLen; ++i) {
bytes32 permissionHook = permissionHooks.at(0);
permissionHooks.remove(permissionHook);
EnumerableSet.Bytes32Set storage executionHooks = _validationData.executionHooks;
uint256 executionHookLen = executionHooks.length();
for (uint256 i = 0; i < executionHookLen; ++i) {
bytes32 executionHook = executionHooks.at(0);
executionHooks.remove(executionHook);
}

// Clear selectors
Expand Down
56 changes: 28 additions & 28 deletions src/account/ReferenceModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ contract ReferenceModularAccount is
// Wraps execution of a native function with runtime validation and hooks
// Used for upgradeTo, upgradeToAndCall, execute, executeBatch, installExecution, uninstallExecution
modifier wrapNativeFunction() {
(PostExecToRun[] memory postPermissionHooks, PostExecToRun[] memory postExecHooks) =
(PostExecToRun[] memory postValidatorExecHooks, PostExecToRun[] memory postSelectorExecHooks) =
_checkPermittedCallerAndAssociatedHooks();

_;

_doCachedPostExecHooks(postExecHooks);
_doCachedPostExecHooks(postPermissionHooks);
_doCachedPostExecHooks(postSelectorExecHooks);
_doCachedPostExecHooks(postValidatorExecHooks);
}

constructor(IEntryPoint anEntryPoint) {
Expand All @@ -110,7 +110,7 @@ contract ReferenceModularAccount is
if (execModule == address(0)) {
revert UnrecognizedFunction(msg.sig);
}
(PostExecToRun[] memory postPermissionHooks, PostExecToRun[] memory postExecHooks) =
(PostExecToRun[] memory postValidatorExecHooks, PostExecToRun[] memory postSelectorExecHooks) =
_checkPermittedCallerAndAssociatedHooks();

// execute the function, bubbling up any reverts
Expand All @@ -123,8 +123,8 @@ contract ReferenceModularAccount is
}
}

_doCachedPostExecHooks(postExecHooks);
_doCachedPostExecHooks(postPermissionHooks);
_doCachedPostExecHooks(postSelectorExecHooks);
_doCachedPostExecHooks(postValidatorExecHooks);

return execReturnData;
}
Expand All @@ -139,8 +139,8 @@ contract ReferenceModularAccount is

ModuleEntity userOpValidationFunction = ModuleEntity.wrap(bytes24(userOp.signature[:24]));

PostExecToRun[] memory postPermissionHooks =
_doPreHooks(getAccountStorage().validationData[userOpValidationFunction].permissionHooks, msg.data);
PostExecToRun[] memory postValidatorExecHooks =
_doPreHooks(getAccountStorage().validationData[userOpValidationFunction].executionHooks, msg.data);

(bool success, bytes memory result) = address(this).call(userOp.callData[4:]);

Expand All @@ -151,7 +151,7 @@ contract ReferenceModularAccount is
}
}

_doCachedPostExecHooks(postPermissionHooks);
_doCachedPostExecHooks(postValidatorExecHooks);
}

/// @inheritdoc IModularAccount
Expand Down Expand Up @@ -202,9 +202,9 @@ contract ReferenceModularAccount is

_doRuntimeValidation(runtimeValidationFunction, data, authorization[25:]);

// If runtime validation passes, do runtime permission checks
PostExecToRun[] memory postPermissionHooks =
_doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].permissionHooks, data);
// If runtime validation passes, run exec hooks associated with the validator
PostExecToRun[] memory postValidatorExecHooks =
_doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].executionHooks, data);

// Execute the call
(bool success, bytes memory returnData) = address(this).call(data);
Expand All @@ -215,7 +215,7 @@ contract ReferenceModularAccount is
}
}

_doCachedPostExecHooks(postPermissionHooks);
_doCachedPostExecHooks(postValidatorExecHooks);

return returnData;
}
Expand Down Expand Up @@ -254,7 +254,7 @@ contract ReferenceModularAccount is
/// @inheritdoc IModularAccount
/// @notice May be validated by a global validation.
/// @dev This function can be used to update (to a certain degree) previously installed validation functions.
/// - preValidationHook, permissionHook, and selectors can be added later. Though they won't be deleted.
/// - preValidationHook, executionHooks, and selectors can be added later. Though they won't be deleted.
/// - isGlobal and isSignatureValidation can also be updated later.
function installValidation(
ValidationConfig validationConfig,
Expand Down Expand Up @@ -360,12 +360,12 @@ contract ReferenceModularAccount is
isGlobalValidation ? ValidationCheckingType.GLOBAL : ValidationCheckingType.SELECTOR
);

// Check if there are permission hooks associated with the validator, and revert if the call isn't to
// Check if there are execution hooks associated with the validator, and revert if the call isn't to
// `executeUserOp`
// This check must be here because if context isn't passed, we can't tell in execution which hooks should
// have ran
if (
getAccountStorage().validationData[userOpValidationFunction].permissionHooks.length() > 0
getAccountStorage().validationData[userOpValidationFunction].executionHooks.length() > 0
&& bytes4(userOp.callData[:4]) != this.executeUserOp.selector
) {
revert RequireUserOperationContext();
Expand Down Expand Up @@ -538,24 +538,24 @@ contract ReferenceModularAccount is
/**
* Order of operations:
* 1. Check if the sender is the entry point, the account itself, or the selector called is public.
* - Yes: Return an empty array, there are no post permissionHooks.
* - Yes: Return an empty array, there are no post executionHooks.
* - No: Continue
* 2. Check if the called selector (msg.sig) is included in the set of selectors the msg.sender can
* directly call.
* - Yes: Continue
* - No: Revert, the caller is not allowed to call this selector
* 3. If there are runtime validation hooks associated with this caller-sig combination, run them.
* 4. Run the pre permissionHooks associated with this caller-sig combination, and return the
* post permissionHooks to run later.
* 4. Run the pre executionHooks associated with this caller-sig combination, and return the
* post executionHooks to run later.
*/
function _checkPermittedCallerAndAssociatedHooks()
internal
returns (PostExecToRun[] memory, PostExecToRun[] memory)
{
AccountStorage storage _storage = getAccountStorage();
PostExecToRun[] memory postPermissionHooks;
PostExecToRun[] memory postValidatorExecutionHooks;

// We only need to handle permission hooks when the sender is not the entry point or the account itself,
// We only need to handle execution hooks when the sender is not the entry point or the account itself,
// and the selector isn't public.
if (
msg.sender != address(_ENTRY_POINT) && msg.sender != address(this)
Expand All @@ -566,7 +566,7 @@ contract ReferenceModularAccount is

_checkIfValidationAppliesCallData(msg.data, directCallValidationKey, ValidationCheckingType.EITHER);

// Direct call is allowed, run associated permission & validation hooks
// Direct call is allowed, run associated execution & validation hooks

// Validation hooks
ModuleEntity[] memory preRuntimeValidationHooks =
Expand All @@ -577,16 +577,16 @@ contract ReferenceModularAccount is
_doPreRuntimeValidationHook(preRuntimeValidationHooks[i], msg.data, "");
}

// Permission hooks
postPermissionHooks =
_doPreHooks(_storage.validationData[directCallValidationKey].permissionHooks, msg.data);
// Execution hooks associated with the validator
postValidatorExecutionHooks =
_doPreHooks(_storage.validationData[directCallValidationKey].executionHooks, msg.data);
}

// Exec hooks
PostExecToRun[] memory postExecutionHooks =
// Exec hooks associated with the selector
PostExecToRun[] memory postSelectorExecutionHooks =
_doPreHooks(_storage.executionData[msg.sig].executionHooks, msg.data);

return (postPermissionHooks, postExecutionHooks);
return (postValidatorExecutionHooks, postSelectorExecutionHooks);
}

function _execUserOpValidation(
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/IModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ interface IModularAccount {
/// @param uninstallData Optional data to be decoded and used by the module to clear module data for the
/// account.
/// @param hookUninstallData Optional data to be used by hooks for cleanup. If any are provided, the array must
/// be of a length equal to existing pre validation hooks plus permission hooks. Hooks are indexed by
/// pre validation hook order first, then permission hooks.
/// be of a length equal to existing pre validation hooks plus execution hooks. Hooks are indexed by
/// pre validation hook order first, then execution hooks.
function uninstallValidation(
ModuleEntity validationFunction,
bytes calldata uninstallData,
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/IModularAccountView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ struct ValidationDataView {
bool isUserOpValidation;
// The pre validation hooks for this validation function.
ModuleEntity[] preValidationHooks;
// Permission hooks for this validation function.
HookConfig[] permissionHooks;
// Execution hooks to run with this validation function.
HookConfig[] executionHooks;
// The set of selectors that may be validated by this validation function.
bytes4[] selectors;
}
Expand Down
2 changes: 1 addition & 1 deletion test/account/ModularAccountView.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ contract ModularAccountViewTest is CustomValidationTestBase {
)
);

assertEq(data.permissionHooks.length, 0);
assertEq(data.executionHooks.length, 0);
assertEq(selectors.length, 1);
assertEq(selectors[0], comprehensiveModule.foo.selector);
}
Expand Down
2 changes: 1 addition & 1 deletion test/account/ReplaceModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ contract UpgradeModuleTest is AccountTestBase {
}

function test_upgradeModuleValidationFunction() public {
// Setup new validaiton with pre validation and permission hooks
// Setup new validaiton with pre validation and execution hooks associated with a validator
SingleSignerValidationModule validation1 = new SingleSignerValidationModule();
SingleSignerValidationModule validation2 = new SingleSignerValidationModule();
uint32 validationEntityId1 = 10;
Expand Down
4 changes: 2 additions & 2 deletions test/mocks/modules/DirectCallModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ contract DirectCallModule is BaseModule, IExecutionHookModule {
override
returns (bytes memory)
{
require(sender == address(this), "mock direct call pre permission hook failed");
require(sender == address(this), "mock direct call pre execution hook failed");
preHookRan = true;
return abi.encode(keccak256(hex"04546b"));
}

function postExecutionHook(uint32, bytes calldata preExecHookData) external override {
require(
abi.decode(preExecHookData, (bytes32)) == keccak256(hex"04546b"),
"mock direct call post permission hook failed"
"mock direct call post execution hook failed"
);
postHookRan = true;
}
Expand Down
Loading