Skip to content

Commit

Permalink
fix: rename validationData/executionData to validationStorage/executi…
Browse files Browse the repository at this point in the history 
…onStorage (#199)
  • Loading branch information
@jaypaik
jaypaik authored Oct 15, 2024
1 parent 55c9244 commit c75f885
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 72 deletions.
8 changes: 4 additions & 4 deletions src/account/AccountStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {HookConfig, ModuleEntity} from "../interfaces/IModularAccount.sol";
bytes32 constant _ACCOUNT_STORAGE_SLOT = 0xc531f081ecdb5a90f38c197521797881a6e5c752a7d451780f325a95f8b91f45;

// Represents data associated with a specifc function selector.
struct ExecutionData {
struct ExecutionStorage {
// The module that implements this execution function.
// If this is a native function, the address must remain address(0).
address module;
Expand All @@ -24,7 +24,7 @@ struct ExecutionData {
EnumerableSet.Bytes32Set executionHooks;
}

struct ValidationData {
struct ValidationStorage {
// Whether or not this validation can be used as a global validation function.
bool isGlobal;
// Whether or not this validation is allowed to validate ERC-1271 signatures.
Expand All @@ -44,8 +44,8 @@ struct AccountStorage {
uint8 initialized;
bool initializing;
// Execution functions and their associated functions
mapping(bytes4 selector => ExecutionData) executionData;
mapping(ModuleEntity validationFunction => ValidationData) validationData;
mapping(bytes4 selector => ExecutionStorage) executionStorage;
mapping(ModuleEntity validationFunction => ValidationStorage) validationStorage;
// For ERC165 introspection
mapping(bytes4 => uint256) supportedIfaces;
}
Expand Down
30 changes: 15 additions & 15 deletions src/account/ModularAccountView.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet
import {HookConfig, IModularAccount, ModuleEntity} from "../interfaces/IModularAccount.sol";
import {ExecutionDataView, IModularAccountView, ValidationDataView} from "../interfaces/IModularAccountView.sol";
import {HookConfigLib} from "../libraries/HookConfigLib.sol";
import {ExecutionData, ValidationData, getAccountStorage, toHookConfig} from "./AccountStorage.sol";
import {ExecutionStorage, ValidationStorage, getAccountStorage, toHookConfig} from "./AccountStorage.sol";

abstract contract ModularAccountView is IModularAccountView {
using EnumerableSet for EnumerableSet.Bytes32Set;
Expand All @@ -26,15 +26,15 @@ abstract contract ModularAccountView is IModularAccountView {
data.module = address(this);
data.allowGlobalValidation = true;
} else {
ExecutionData storage executionData = getAccountStorage().executionData[selector];
data.module = executionData.module;
data.skipRuntimeValidation = executionData.skipRuntimeValidation;
data.allowGlobalValidation = executionData.allowGlobalValidation;
ExecutionStorage storage executionStorage = getAccountStorage().executionStorage[selector];
data.module = executionStorage.module;
data.skipRuntimeValidation = executionStorage.skipRuntimeValidation;
data.allowGlobalValidation = executionStorage.allowGlobalValidation;

uint256 executionHooksLen = executionData.executionHooks.length();
uint256 executionHooksLen = executionStorage.executionHooks.length();
data.executionHooks = new HookConfig[](executionHooksLen);
for (uint256 i = 0; i < executionHooksLen; ++i) {
data.executionHooks[i] = toHookConfig(executionData.executionHooks.at(i));
data.executionHooks[i] = toHookConfig(executionStorage.executionHooks.at(i));
}
}
}
Expand All @@ -46,19 +46,19 @@ abstract contract ModularAccountView is IModularAccountView {
override
returns (ValidationDataView memory data)
{
ValidationData storage validationData = getAccountStorage().validationData[validationFunction];
data.isGlobal = validationData.isGlobal;
data.isSignatureValidation = validationData.isSignatureValidation;
data.isUserOpValidation = validationData.isUserOpValidation;
data.validationHooks = validationData.validationHooks;
ValidationStorage storage validationStorage = getAccountStorage().validationStorage[validationFunction];
data.isGlobal = validationStorage.isGlobal;
data.isSignatureValidation = validationStorage.isSignatureValidation;
data.isUserOpValidation = validationStorage.isUserOpValidation;
data.validationHooks = validationStorage.validationHooks;

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

bytes32[] memory selectors = validationData.selectors.values();
bytes32[] memory selectors = validationStorage.selectors.values();
uint256 selectorsLen = selectors.length;
data.selectors = new bytes4[](selectorsLen);
for (uint256 j = 0; j < selectorsLen; ++j) {
Expand Down
75 changes: 38 additions & 37 deletions src/account/ModuleManagerInternals.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {ValidationConfigLib} from "../libraries/ValidationConfigLib.sol";

import {
AccountStorage,
ExecutionData,
ValidationData,
ExecutionStorage,
ValidationStorage,
getAccountStorage,
toModuleEntity,
toSetValue
Expand Down Expand Up @@ -53,9 +53,9 @@ abstract contract ModuleManagerInternals is IModularAccount {
bool allowGlobalValidation,
address module
) internal {
ExecutionData storage _executionData = getAccountStorage().executionData[selector];
ExecutionStorage storage _executionStorage = getAccountStorage().executionStorage[selector];

if (_executionData.module != address(0)) {
if (_executionStorage.module != address(0)) {
revert ExecutionFunctionAlreadySet(selector);
}

Expand All @@ -79,25 +79,25 @@ abstract contract ModuleManagerInternals is IModularAccount {
revert Erc4337FunctionNotAllowed(selector);
}

_executionData.module = module;
_executionData.skipRuntimeValidation = skipRuntimeValidation;
_executionData.allowGlobalValidation = allowGlobalValidation;
_executionStorage.module = module;
_executionStorage.skipRuntimeValidation = skipRuntimeValidation;
_executionStorage.allowGlobalValidation = allowGlobalValidation;
}

function _removeExecutionFunction(bytes4 selector) internal {
ExecutionData storage _executionData = getAccountStorage().executionData[selector];
ExecutionStorage storage _executionStorage = getAccountStorage().executionStorage[selector];

_executionData.module = address(0);
_executionData.skipRuntimeValidation = false;
_executionData.allowGlobalValidation = false;
_executionStorage.module = address(0);
_executionStorage.skipRuntimeValidation = false;
_executionStorage.allowGlobalValidation = false;
}

function _removeValidationFunction(ModuleEntity validationFunction) internal {
ValidationData storage _validationData = getAccountStorage().validationData[validationFunction];
ValidationStorage storage _validationStorage = getAccountStorage().validationStorage[validationFunction];

_validationData.isGlobal = false;
_validationData.isSignatureValidation = false;
_validationData.isUserOpValidation = false;
_validationStorage.isGlobal = false;
_validationStorage.isSignatureValidation = false;
_validationStorage.isUserOpValidation = false;
}

function _addExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal {
Expand Down Expand Up @@ -134,7 +134,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
EnumerableSet.Bytes32Set storage execHooks =
_storage.executionData[mh.executionSelector].executionHooks;
_storage.executionStorage[mh.executionSelector].executionHooks;
HookConfig hookConfig = HookConfigLib.packExecHook({
_module: module,
_entityId: mh.entityId,
Expand Down Expand Up @@ -165,7 +165,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
EnumerableSet.Bytes32Set storage execHooks =
_storage.executionData[mh.executionSelector].executionHooks;
_storage.executionStorage[mh.executionSelector].executionHooks;
HookConfig hookConfig = HookConfigLib.packExecHook({
_module: module,
_entityId: mh.entityId,
Expand Down Expand Up @@ -224,19 +224,19 @@ abstract contract ModuleManagerInternals is IModularAccount {
bytes calldata installData,
bytes[] calldata hooks
) internal {
ValidationData storage _validationData =
getAccountStorage().validationData[validationConfig.moduleEntity()];
ValidationStorage storage _validationStorage =
getAccountStorage().validationStorage[validationConfig.moduleEntity()];
ModuleEntity moduleEntity = validationConfig.moduleEntity();

for (uint256 i = 0; i < hooks.length; ++i) {
HookConfig hookConfig = HookConfig.wrap(bytes25(hooks[i][:25]));
bytes calldata hookData = hooks[i][25:];

if (hookConfig.isValidationHook()) {
_validationData.validationHooks.push(hookConfig);
_validationStorage.validationHooks.push(hookConfig);

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

Expand All @@ -245,21 +245,21 @@ abstract contract ModuleManagerInternals is IModularAccount {
continue;
}
// Hook is an execution hook
_addExecHooks(_validationData.executionHooks, hookConfig);
_addExecHooks(_validationStorage.executionHooks, hookConfig);

_onInstall(hookConfig.module(), hookData, type(IExecutionHookModule).interfaceId);
}

for (uint256 i = 0; i < selectors.length; ++i) {
bytes4 selector = selectors[i];
if (!_validationData.selectors.add(toSetValue(selector))) {
if (!_validationStorage.selectors.add(toSetValue(selector))) {
revert ValidationAlreadySet(selector, moduleEntity);
}
}

_validationData.isGlobal = validationConfig.isGlobal();
_validationData.isSignatureValidation = validationConfig.isSignatureValidation();
_validationData.isUserOpValidation = validationConfig.isUserOpValidation();
_validationStorage.isGlobal = validationConfig.isGlobal();
_validationStorage.isSignatureValidation = validationConfig.isSignatureValidation();
_validationStorage.isUserOpValidation = validationConfig.isUserOpValidation();

_onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId);
emit ValidationInstalled(validationConfig.module(), validationConfig.entityId());
Expand All @@ -270,7 +270,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
bytes calldata uninstallData,
bytes[] calldata hookUninstallDatas
) internal {
ValidationData storage _validationData = getAccountStorage().validationData[validationFunction];
ValidationStorage storage _validationStorage = getAccountStorage().validationStorage[validationFunction];
bool onUninstallSuccess = true;

_removeValidationFunction(validationFunction);
Expand All @@ -280,44 +280,45 @@ abstract contract ModuleManagerInternals is IModularAccount {
// If any uninstall data is provided, assert it is of the correct length.
if (
hookUninstallDatas.length
!= _validationData.validationHooks.length + _validationData.executionHooks.length()
!= _validationStorage.validationHooks.length + _validationStorage.executionHooks.length()
) {
revert ArrayLengthMismatch();
}

// Hook uninstall data is provided in the order of pre validation hooks, then execution hooks.
uint256 hookIndex = 0;
for (uint256 i = 0; i < _validationData.validationHooks.length; ++i) {
for (uint256 i = 0; i < _validationStorage.validationHooks.length; ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) = ModuleEntityLib.unpack(_validationData.validationHooks[i].moduleEntity());
(address hookModule,) =
ModuleEntityLib.unpack(_validationStorage.validationHooks[i].moduleEntity());
onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData);
hookIndex++;
}

for (uint256 i = 0; i < _validationData.executionHooks.length(); ++i) {
for (uint256 i = 0; i < _validationStorage.executionHooks.length(); ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) =
ModuleEntityLib.unpack(toModuleEntity(_validationData.executionHooks.at(i)));
ModuleEntityLib.unpack(toModuleEntity(_validationStorage.executionHooks.at(i)));
onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData);
hookIndex++;
}
}

// Clear all stored hooks
delete _validationData.validationHooks;
delete _validationStorage.validationHooks;

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

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

(address module, uint32 entityId) = ModuleEntityLib.unpack(validationFunction);
Expand Down
Loading

0 comments on commit c75f885

Please sign in to comment.