From bdc7e7d88f4970ff6fd43a0bc33c4288dbcbf55b Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Tue, 15 Oct 2024 14:19:50 -0400 Subject: [PATCH] fix: rename validationData/executionData to validationStorage/executionStorage --- src/account/AccountStorage.sol | 8 +-- src/account/ModularAccountView.sol | 30 +++++----- src/account/ModuleManagerInternals.sol | 75 +++++++++++++------------ src/account/ReferenceModularAccount.sol | 32 +++++------ 4 files changed, 73 insertions(+), 72 deletions(-) diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 12e8f10e..5662aad0 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -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; @@ -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. @@ -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; } diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol index fc73dd3d..eee83e77 100644 --- a/src/account/ModularAccountView.sol +++ b/src/account/ModularAccountView.sol @@ -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; @@ -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)); } } } @@ -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) { diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index b454b5b2..6b5e9ad2 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -19,8 +19,8 @@ import {ValidationConfigLib} from "../libraries/ValidationConfigLib.sol"; import { AccountStorage, - ExecutionData, - ValidationData, + ExecutionStorage, + ValidationStorage, getAccountStorage, toModuleEntity, toSetValue @@ -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); } @@ -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 { @@ -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, @@ -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, @@ -224,8 +224,8 @@ 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) { @@ -233,10 +233,10 @@ abstract contract ModuleManagerInternals is IModularAccount { 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(); } @@ -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()); @@ -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); @@ -280,33 +280,34 @@ 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); @@ -314,10 +315,10 @@ abstract contract ModuleManagerInternals is IModularAccount { } // 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); diff --git a/src/account/ReferenceModularAccount.sol b/src/account/ReferenceModularAccount.sol index 88e18a7a..f89d0b1d 100644 --- a/src/account/ReferenceModularAccount.sol +++ b/src/account/ReferenceModularAccount.sol @@ -106,7 +106,7 @@ contract ReferenceModularAccount is /// @dev We route calls to execution functions based on incoming msg.sig /// @dev If there's no module associated with this function selector, revert fallback(bytes calldata) external payable returns (bytes memory) { - address execModule = getAccountStorage().executionData[msg.sig].module; + address execModule = getAccountStorage().executionStorage[msg.sig].module; if (execModule == address(0)) { revert UnrecognizedFunction(msg.sig); } @@ -140,7 +140,7 @@ contract ReferenceModularAccount is ModuleEntity userOpValidationFunction = ModuleEntity.wrap(bytes24(userOp.signature[:24])); PostExecToRun[] memory postValidatorExecHooks = - _doPreHooks(getAccountStorage().validationData[userOpValidationFunction].executionHooks, msg.data); + _doPreHooks(getAccountStorage().validationStorage[userOpValidationFunction].executionHooks, msg.data); (bool success, bytes memory result) = address(this).call(userOp.callData[4:]); @@ -204,7 +204,7 @@ contract ReferenceModularAccount is // If runtime validation passes, run exec hooks associated with the validator PostExecToRun[] memory postValidatorExecHooks = - _doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].executionHooks, data); + _doPreHooks(getAccountStorage().validationStorage[runtimeValidationFunction].executionHooks, data); // Execute the call (bool success, bytes memory returnData) = address(this).call(data); @@ -312,7 +312,7 @@ contract ReferenceModularAccount is signature = signature[24:]; HookConfig[] memory preSignatureValidationHooks = - getAccountStorage().validationData[sigValidation].validationHooks; + getAccountStorage().validationStorage[sigValidation].validationHooks; for (uint256 i = 0; i < preSignatureValidationHooks.length; ++i) { (address hookModule, uint32 hookEntityId) = preSignatureValidationHooks[i].moduleEntity().unpack(); @@ -365,7 +365,7 @@ contract ReferenceModularAccount is // 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].executionHooks.length() > 0 + getAccountStorage().validationStorage[userOpValidationFunction].executionHooks.length() > 0 && bytes4(userOp.callData[:4]) != this.executeUserOp.selector ) { revert RequireUserOperationContext(); @@ -385,7 +385,7 @@ contract ReferenceModularAccount is // Do preUserOpValidation hooks HookConfig[] memory preUserOpValidationHooks = - getAccountStorage().validationData[userOpValidationFunction].validationHooks; + getAccountStorage().validationStorage[userOpValidationFunction].validationHooks; for (uint256 i = 0; i < preUserOpValidationHooks.length; ++i) { (userOp.signature, signature) = signature.advanceSegmentIfAtIndex(uint8(i)); @@ -425,7 +425,7 @@ contract ReferenceModularAccount is ) internal { // run all preRuntimeValidation hooks HookConfig[] memory preRuntimeValidationHooks = - getAccountStorage().validationData[runtimeValidationFunction].validationHooks; + getAccountStorage().validationStorage[runtimeValidationFunction].validationHooks; for (uint256 i = 0; i < preRuntimeValidationHooks.length; ++i) { bytes memory currentAuthSegment; @@ -559,7 +559,7 @@ contract ReferenceModularAccount is // and the selector isn't public. if ( msg.sender != address(_ENTRY_POINT) && msg.sender != address(this) - && !_storage.executionData[msg.sig].skipRuntimeValidation + && !_storage.executionStorage[msg.sig].skipRuntimeValidation ) { ModuleEntity directCallValidationKey = ModuleEntityLib.pack(msg.sender, DIRECT_CALL_VALIDATION_ENTITYID); @@ -570,7 +570,7 @@ contract ReferenceModularAccount is // Validation hooks HookConfig[] memory preRuntimeValidationHooks = - _storage.validationData[directCallValidationKey].validationHooks; + _storage.validationStorage[directCallValidationKey].validationHooks; uint256 hookLen = preRuntimeValidationHooks.length; for (uint256 i = 0; i < hookLen; ++i) { @@ -579,12 +579,12 @@ contract ReferenceModularAccount is // Execution hooks associated with the validator postValidatorExecutionHooks = - _doPreHooks(_storage.validationData[directCallValidationKey].executionHooks, msg.data); + _doPreHooks(_storage.validationStorage[directCallValidationKey].executionHooks, msg.data); } // Exec hooks associated with the selector PostExecToRun[] memory postSelectorExecutionHooks = - _doPreHooks(_storage.executionData[msg.sig].executionHooks, msg.data); + _doPreHooks(_storage.executionStorage[msg.sig].executionHooks, msg.data); return (postValidatorExecutionHooks, postSelectorExecutionHooks); } @@ -598,7 +598,7 @@ contract ReferenceModularAccount is (address module, uint32 entityId) = userOpValidationFunction.unpack(); - if (!_storage.validationData[userOpValidationFunction].isUserOpValidation) { + if (!_storage.validationStorage[userOpValidationFunction].isUserOpValidation) { revert UserOpValidationInvalid(module, entityId); } @@ -633,7 +633,7 @@ contract ReferenceModularAccount is AccountStorage storage _storage = getAccountStorage(); (address module, uint32 entityId) = sigValidation.unpack(); - if (!_storage.validationData[sigValidation].isSignatureValidation) { + if (!_storage.validationStorage[sigValidation].isSignatureValidation) { revert SignatureValidationInvalid(module, entityId); } @@ -656,11 +656,11 @@ contract ReferenceModularAccount is return true; } - return getAccountStorage().executionData[selector].allowGlobalValidation; + return getAccountStorage().executionStorage[selector].allowGlobalValidation; } function _isValidationGlobal(ModuleEntity validationFunction) internal view virtual returns (bool) { - return getAccountStorage().validationData[validationFunction].isGlobal; + return getAccountStorage().validationStorage[validationFunction].isGlobal; } function _checkIfValidationAppliesCallData( @@ -754,6 +754,6 @@ contract ReferenceModularAccount is view returns (bool) { - return getAccountStorage().validationData[validationFunction].selectors.contains(toSetValue(selector)); + return getAccountStorage().validationStorage[validationFunction].selectors.contains(toSetValue(selector)); } }