Skip to content
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.

Sign up for GitHub

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

feat: [v0.8-develop] HookConfig install parameter & internal structure 4/N #109

Merged
merged 6 commits into from
Jul 24, 2024
Merged

feat: [v0.8-develop] HookConfig install parameter & internal structure 4/N #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae4e818
add hookconfig to install
adamegyed Jul 22, 2024
186f1ce
Update internal structures to use hookConfig
adamegyed Jul 22, 2024
c8b31e5
update comment
adamegyed Jul 23, 2024
05e41f2
merge fix
adamegyed Jul 23, 2024
f36594c
Consistent naming
adamegyed Jul 24, 2024
261ea25
review fixes
adamegyed Jul 24, 2024
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
2 changes: 1 addition & 1 deletion foundry.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ runs = 5000
depth = 32

[profile.deep.fuzz]
runs = 10000
runs = 100000

[profile.deep.invariant]
runs = 5000
Expand Down
3 changes: 1 addition & 2 deletions src/account/AccountFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ contract AccountFactory is Ownable {
ValidationConfigLib.pack(SINGLE_SIGNER_VALIDATION, entityId, true, true),
new bytes4[](0),
pluginInstallData,
"",
""
new bytes[](0)
);
}

Expand Down
22 changes: 16 additions & 6 deletions src/account/AccountLoupe.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeab
import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import {HookConfigLib} from "../helpers/HookConfigLib.sol";
import {ExecutionHook, IAccountLoupe} from "../interfaces/IAccountLoupe.sol";
import {IModuleManager, ModuleEntity} from "../interfaces/IModuleManager.sol";
import {HookConfig, IModuleManager, ModuleEntity} from "../interfaces/IModuleManager.sol";
import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
import {getAccountStorage, toExecutionHook, toSelector} from "./AccountStorage.sol";
import {getAccountStorage, toHookConfig, toSelector} from "./AccountStorage.sol";

abstract contract AccountLoupe is IAccountLoupe {
using EnumerableSet for EnumerableSet.Bytes32Set;
using EnumerableMap for EnumerableMap.AddressToUintMap;
using HookConfigLib for HookConfig;

/// @inheritdoc IAccountLoupe
function getExecutionFunctionHandler(bytes4 selector) external view override returns (address module) {
Expand Down Expand Up @@ -56,8 +58,12 @@ abstract contract AccountLoupe is IAccountLoupe {

for (uint256 i = 0; i < executionHooksLength; ++i) {
bytes32 key = hooks.at(i);
ExecutionHook memory execHook = execHooks[i];
(execHook.hookFunction, execHook.isPreHook, execHook.isPostHook) = toExecutionHook(key);
HookConfig hookConfig = toHookConfig(key);
execHooks[i] = ExecutionHook({
hookFunction: hookConfig.moduleEntity(),
isPreHook: hookConfig.hasPreHook(),
isPostHook: hookConfig.hasPostHook()
});
}
}

Expand All @@ -74,8 +80,12 @@ abstract contract AccountLoupe is IAccountLoupe {
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);
HookConfig hookConfig = toHookConfig(key);
permissionHooks[i] = ExecutionHook({
hookFunction: hookConfig.moduleEntity(),
isPreHook: hookConfig.hasPreHook(),
isPostHook: hookConfig.hasPostHook()
});
}
}

Expand Down
18 changes: 5 additions & 13 deletions src/account/AccountStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ pragma solidity ^0.8.25;

import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import {ExecutionHook} from "../interfaces/IAccountLoupe.sol";
import {ModuleEntity} from "../interfaces/IModuleManager.sol";
import {HookConfig, ModuleEntity} from "../interfaces/IModuleManager.sol";

// bytes = keccak256("ERC6900.UpgradeableModularAccount.Storage")
bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c401499164f368baef687948c315d9073e40;
Expand Down Expand Up @@ -70,19 +69,12 @@ function toModuleEntity(bytes32 setValue) pure returns (ModuleEntity) {
// 0x________________________________________________AA____________________ is pre hook
// 0x__________________________________________________BB__________________ is post hook

function toSetValue(ExecutionHook memory executionHook) pure returns (bytes32) {
return bytes32(ModuleEntity.unwrap(executionHook.hookFunction))
| bytes32(executionHook.isPreHook ? uint256(1) << 56 : 0)
| bytes32(executionHook.isPostHook ? uint256(1) << 48 : 0);
function toSetValue(HookConfig hookConfig) pure returns (bytes32) {
return bytes32(HookConfig.unwrap(hookConfig));
}

function toExecutionHook(bytes32 setValue)
pure
returns (ModuleEntity hookFunction, bool isPreHook, bool isPostHook)
{
hookFunction = ModuleEntity.wrap(bytes24(setValue));
isPreHook = (uint256(setValue) >> 56) & 0xFF == 1;
isPostHook = (uint256(setValue) >> 48) & 0xFF == 1;
function toHookConfig(bytes32 setValue) pure returns (HookConfig) {
return HookConfig.wrap(bytes26(setValue));
}

function toSetValue(bytes4 selector) pure returns (bytes32) {
Expand Down
198 changes: 95 additions & 103 deletions src/account/ModuleManagerInternals.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,35 @@ import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165C

import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import {DIRECT_CALL_VALIDATION_ENTITYID, MAX_PRE_VALIDATION_HOOKS} from "../helpers/Constants.sol";
import {MAX_PRE_VALIDATION_HOOKS} from "../helpers/Constants.sol";
import {HookConfigLib} from "../helpers/HookConfigLib.sol";
import {KnownSelectors} from "../helpers/KnownSelectors.sol";
import {ModuleEntityLib} from "../helpers/ModuleEntityLib.sol";
import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol";
import {ExecutionHook} from "../interfaces/IAccountLoupe.sol";
import {IModule, ManifestExecutionHook, ModuleManifest} from "../interfaces/IModule.sol";
import {IModuleManager, ModuleEntity, ValidationConfig} from "../interfaces/IModuleManager.sol";
import {AccountStorage, SelectorData, ValidationData, getAccountStorage, toSetValue} from "./AccountStorage.sol";
import {HookConfig, IModuleManager, ModuleEntity, ValidationConfig} from "../interfaces/IModuleManager.sol";
import {
AccountStorage,
SelectorData,
ValidationData,
getAccountStorage,
toModuleEntity,
toSetValue
} from "./AccountStorage.sol";

abstract contract ModuleManagerInternals is IModuleManager {
using EnumerableSet for EnumerableSet.Bytes32Set;
using ModuleEntityLib for ModuleEntity;
using ValidationConfigLib for ValidationConfig;
using HookConfigLib for HookConfig;

error ArrayLengthMismatch();
error Erc4337FunctionNotAllowed(bytes4 selector);
error ExecutionFunctionAlreadySet(bytes4 selector);
error IModuleFunctionNotAllowed(bytes4 selector);
error NativeFunctionNotAllowed(bytes4 selector);
error NullModule();
error PermissionAlreadySet(ModuleEntity validationFunction, ExecutionHook hook);
error PermissionAlreadySet(ModuleEntity validationFunction, HookConfig hookConfig);
error ModuleInstallCallbackFailed(address module, bytes revertReason);
error ModuleInterfaceNotSupported(address module);
error ModuleNotInstalled(address module);
Expand Down Expand Up @@ -108,30 +116,12 @@ abstract contract ModuleManagerInternals is IModuleManager {
}
}

function _addExecHooks(
EnumerableSet.Bytes32Set storage hooks,
ModuleEntity hookFunction,
bool isPreExecHook,
bool isPostExecHook
) internal {
hooks.add(
toSetValue(
ExecutionHook({hookFunction: hookFunction, isPreHook: isPreExecHook, isPostHook: isPostExecHook})
)
);
function _addExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
hooks.add(toSetValue(hookConfig));
}

function _removeExecHooks(
EnumerableSet.Bytes32Set storage hooks,
ModuleEntity hookFunction,
bool isPreExecHook,
bool isPostExecHook
) internal {
hooks.remove(
toSetValue(
ExecutionHook({hookFunction: hookFunction, isPreHook: isPreExecHook, isPostHook: isPostExecHook})
)
);
function _removeExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
hooks.remove(toSetValue(hookConfig));
}

function _installModule(address module, ModuleManifest calldata manifest, bytes memory moduleInstallData)
Expand Down Expand Up @@ -162,8 +152,13 @@ abstract contract ModuleManagerInternals is IModuleManager {
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks;
ModuleEntity hookFunction = ModuleEntityLib.pack(module, mh.entityId);
_addExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook);
HookConfig hookConfig = HookConfigLib.packExecHook({
_module: module,
_entityId: mh.entityId,
_hasPre: mh.isPreHook,
_hasPost: mh.isPostHook
});
_addExecHooks(execHooks, hookConfig);
}

length = manifest.interfaceIds.length;
Expand Down Expand Up @@ -191,9 +186,14 @@ abstract contract ModuleManagerInternals is IModuleManager {
uint256 length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
ModuleEntity hookFunction = ModuleEntityLib.pack(module, mh.entityId);
EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks;
_removeExecHooks(execHooks, hookFunction, mh.isPreHook, mh.isPostHook);
HookConfig hookConfig = HookConfigLib.packExecHook({
_module: module,
_entityId: mh.entityId,
_hasPre: mh.isPreHook,
_hasPost: mh.isPostHook
});
_removeExecHooks(execHooks, hookConfig);
}

length = manifest.executionFunctions.length;
Expand All @@ -218,53 +218,44 @@ abstract contract ModuleManagerInternals is IModuleManager {
emit ModuleUninstalled(module, onUninstallSuccess);
}

function _onInstall(address module, bytes calldata data) internal {
if (data.length > 0) {
IModule(module).onInstall(data);
}
}

function _onUninstall(address module, bytes calldata data) internal {
if (data.length > 0) {
IModule(module).onUninstall(data);
}
}

function _installValidation(
ValidationConfig validationConfig,
bytes4[] memory selectors,
bytes4[] calldata selectors,
bytes calldata installData,
bytes memory preValidationHooks,
bytes memory permissionHooks
bytes[] calldata hooks
) internal {
ValidationData storage _validationData =
getAccountStorage().validationData[validationConfig.moduleEntity()];

if (preValidationHooks.length > 0) {
(ModuleEntity[] memory preValidationFunctions, bytes[] memory initDatas) =
abi.decode(preValidationHooks, (ModuleEntity[], bytes[]));

for (uint256 i = 0; i < preValidationFunctions.length; ++i) {
ModuleEntity preValidationFunction = preValidationFunctions[i];
for (uint256 i = 0; i < hooks.length; ++i) {
HookConfig hookConfig = HookConfig.wrap(bytes26(hooks[i][:26]));
bytes calldata hookData = hooks[i][26:];

_validationData.preValidationHooks.push(preValidationFunction);
if (hookConfig.isValidationHook()) {
_validationData.preValidationHooks.push(hookConfig.moduleEntity());

if (initDatas[i].length > 0) {
(address preValidationModule,) = ModuleEntityLib.unpack(preValidationFunction);
IModule(preValidationModule).onInstall(initDatas[i]);
// Avoid collision between reserved index and actual indices
if (_validationData.preValidationHooks.length > MAX_PRE_VALIDATION_HOOKS) {
revert PreValidationHookLimitExceeded();
}
} // Hook is an execution hook
else if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) {
revert PermissionAlreadySet(validationConfig.moduleEntity(), hookConfig);
}

// Avoid collision between reserved index and actual indices
if (_validationData.preValidationHooks.length > MAX_PRE_VALIDATION_HOOKS) {
revert PreValidationHookLimitExceeded();
}
}

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 (!_validationData.permissionHooks.add(toSetValue(permissionFunction))) {
revert PermissionAlreadySet(validationConfig.moduleEntity(), permissionFunction);
}

if (initDatas[i].length > 0) {
(address executionModule,) = ModuleEntityLib.unpack(permissionFunction.hookFunction);
IModule(executionModule).onInstall(initDatas[i]);
}
}
_onInstall(hookConfig.module(), hookData);
}

for (uint256 i = 0; i < selectors.length; ++i) {
Expand All @@ -274,66 +265,67 @@ abstract contract ModuleManagerInternals is IModuleManager {
}
}

if (validationConfig.entityId() != DIRECT_CALL_VALIDATION_ENTITYID) {
// Only allow global validations and signature validations if they're not direct-call validations.
_validationData.isGlobal = validationConfig.isGlobal();
_validationData.isSignatureValidation = validationConfig.isSignatureValidation();

_validationData.isGlobal = validationConfig.isGlobal();
_validationData.isSignatureValidation = validationConfig.isSignatureValidation();
if (installData.length > 0) {
IModule(validationConfig.module()).onInstall(installData);
}
}
_onInstall(validationConfig.module(), installData);
}

function _uninstallValidation(
ModuleEntity validationFunction,
bytes calldata uninstallData,
bytes calldata preValidationHookUninstallData,
bytes calldata permissionHookUninstallData
bytes[] calldata hookUninstallDatas
) internal {
ValidationData storage _validationData = getAccountStorage().validationData[validationFunction];

_removeValidationFunction(validationFunction);

{
bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[]));
// Send `onUninstall` to hooks
if (hookUninstallDatas.length > 0) {
// If any uninstall data is provided, assert it is of the correct length.
if (
hookUninstallDatas.length
!= _validationData.preValidationHooks.length + _validationData.permissionHooks.length()
) {
revert ArrayLengthMismatch();
}

// Clear pre validation hooks
ModuleEntity[] storage preValidationHooks = _validationData.preValidationHooks;
for (uint256 i = 0; i < preValidationHooks.length; ++i) {
ModuleEntity preValidationFunction = preValidationHooks[i];
if (preValidationHookUninstallDatas[0].length > 0) {
(address preValidationModule,) = ModuleEntityLib.unpack(preValidationFunction);
IModule(preValidationModule).onUninstall(preValidationHookUninstallDatas[0]);
}
// Hook uninstall data is provided in the order of pre-validation hooks, then permission hooks.
uint256 hookIndex = 0;
for (uint256 i = 0; i < _validationData.preValidationHooks.length; ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) = ModuleEntityLib.unpack(_validationData.preValidationHooks[i]);
_onUninstall(hookModule, hookData);
hookIndex++;
}
delete _validationData.preValidationHooks;
}

{
bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[]));

// Clear permission hooks
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);
address permissionHookModule = address(uint160(bytes20(permissionHook)));
IModule(permissionHookModule).onUninstall(permissionHookUninstallDatas[i]);
for (uint256 i = 0; i < _validationData.permissionHooks.length(); ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) =
ModuleEntityLib.unpack(toModuleEntity(_validationData.permissionHooks.at(i)));
_onUninstall(hookModule, hookData);
hookIndex++;
}
}

// 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);
}

// Clear selectors
uint256 selectorLen = _validationData.selectors.length();
for (uint256 i = 0; i < selectorLen; ++i) {
bytes32 selectorSetValue = _validationData.selectors.at(0);
_validationData.selectors.remove(selectorSetValue);
}

if (uninstallData.length > 0) {
(address module,) = ModuleEntityLib.unpack(validationFunction);
IModule(module).onUninstall(uninstallData);
}
(address module,) = ModuleEntityLib.unpack(validationFunction);
_onUninstall(module, uninstallData);
}
}
Loading
Loading