From 60e1209112c8bb41ffb7be53814e4fae40e0ae4d Mon Sep 17 00:00:00 2001 From: Jay Paik Date: Fri, 22 Nov 2024 17:29:25 -0500 Subject: [PATCH] feat: update ValidationDataView with ValidationFlags --- src/account/AccountStorage.sol | 13 ++++++----- src/account/ModularAccountView.sol | 4 +--- src/account/ModuleManagerInternals.sol | 19 ++++++++------- src/account/ReferenceModularAccount.sol | 17 +++++++++----- src/interfaces/IModularAccount.sol | 7 +++--- src/interfaces/IModularAccountView.sol | 14 +++++------ src/libraries/ValidationConfigLib.sol | 13 +++-------- standard/ERCs/erc-6900.md | 31 +++++++++++++------------ test/account/ModularAccountView.t.sol | 14 ++++++----- 9 files changed, 67 insertions(+), 65 deletions(-) diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 5662aad0..81aad233 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {HookConfig, ModuleEntity} from "../interfaces/IModularAccount.sol"; +import {ValidationFlags} from "../libraries/ValidationConfigLib.sol"; // bytes = keccak256("ERC6900.ReferenceModularAccount.Storage") bytes32 constant _ACCOUNT_STORAGE_SLOT = 0xc531f081ecdb5a90f38c197521797881a6e5c752a7d451780f325a95f8b91f45; @@ -25,12 +26,12 @@ struct ExecutionStorage { } 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. - bool isSignatureValidation; - // Whether or not this validation is allowed to validate ERC-4337 user operations. - bool isUserOpValidation; + // ValidationFlags layout: + // 0b00000___ // unused + // 0b_____A__ // isGlobal + // 0b______B_ // isSignatureValidation + // 0b_______C // isUserOpValidation + ValidationFlags validationFlags; // The validation hooks for this validation function. HookConfig[] validationHooks; // Execution hooks to run with this validation function. diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol index eee83e77..34eb770e 100644 --- a/src/account/ModularAccountView.sol +++ b/src/account/ModularAccountView.sol @@ -47,9 +47,7 @@ abstract contract ModularAccountView is IModularAccountView { returns (ValidationDataView memory data) { ValidationStorage storage validationStorage = getAccountStorage().validationStorage[validationFunction]; - data.isGlobal = validationStorage.isGlobal; - data.isSignatureValidation = validationStorage.isSignatureValidation; - data.isUserOpValidation = validationStorage.isUserOpValidation; + data.validationFlags = validationStorage.validationFlags; data.validationHooks = validationStorage.validationHooks; uint256 execHooksLen = validationStorage.executionHooks.length(); diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index a82a6764..11a71f87 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -8,7 +8,13 @@ import {collectReturnData} from "../helpers/CollectReturnData.sol"; import {MAX_VALIDATION_ASSOC_HOOKS} from "../helpers/Constants.sol"; import {IExecutionHookModule} from "../interfaces/IExecutionHookModule.sol"; import {ExecutionManifest, ManifestExecutionHook} from "../interfaces/IExecutionModule.sol"; -import {HookConfig, IModularAccount, ModuleEntity, ValidationConfig} from "../interfaces/IModularAccount.sol"; +import { + HookConfig, + IModularAccount, + ModuleEntity, + ValidationConfig, + ValidationFlags +} from "../interfaces/IModularAccount.sol"; import {IModule} from "../interfaces/IModule.sol"; import {IValidationHookModule} from "../interfaces/IValidationHookModule.sol"; import {IValidationModule} from "../interfaces/IValidationModule.sol"; @@ -94,10 +100,7 @@ abstract contract ModuleManagerInternals is IModularAccount { function _removeValidationFunction(ModuleEntity validationFunction) internal { ValidationStorage storage _validationStorage = getAccountStorage().validationStorage[validationFunction]; - - _validationStorage.isGlobal = false; - _validationStorage.isSignatureValidation = false; - _validationStorage.isUserOpValidation = false; + _validationStorage.validationFlags = ValidationFlags.wrap(0); } function _addExecHooks(EnumerableSet.Bytes32Set storage hooks, HookConfig hookConfig) internal { @@ -226,7 +229,7 @@ abstract contract ModuleManagerInternals is IModularAccount { ) internal { ValidationStorage storage _validationStorage = getAccountStorage().validationStorage[validationConfig.moduleEntity()]; - ModuleEntity moduleEntity = validationConfig.moduleEntity(); + (ModuleEntity moduleEntity, ValidationFlags validationFlags) = validationConfig.unpack(); for (uint256 i = 0; i < hooks.length; ++i) { HookConfig hookConfig = HookConfig.wrap(bytes25(hooks[i][:25])); @@ -257,9 +260,7 @@ abstract contract ModuleManagerInternals is IModularAccount { } } - _validationStorage.isGlobal = validationConfig.isGlobal(); - _validationStorage.isSignatureValidation = validationConfig.isSignatureValidation(); - _validationStorage.isUserOpValidation = validationConfig.isUserOpValidation(); + _validationStorage.validationFlags = validationFlags; _onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId); emit ValidationInstalled(validationConfig.module(), validationConfig.entityId()); diff --git a/src/account/ReferenceModularAccount.sol b/src/account/ReferenceModularAccount.sol index 99527049..68857ef6 100644 --- a/src/account/ReferenceModularAccount.sol +++ b/src/account/ReferenceModularAccount.sol @@ -5,7 +5,6 @@ import {BaseAccount} from "@eth-infinitism/account-abstraction/core/BaseAccount. import {IAccountExecute} from "@eth-infinitism/account-abstraction/interfaces/IAccountExecute.sol"; import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; - import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; @@ -16,7 +15,13 @@ import {DIRECT_CALL_VALIDATION_ENTITY_ID} from "../helpers/Constants.sol"; import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationResHelpers.sol"; import {IExecutionHookModule} from "../interfaces/IExecutionHookModule.sol"; import {ExecutionManifest} from "../interfaces/IExecutionModule.sol"; -import {Call, IModularAccount, ModuleEntity, ValidationConfig} from "../interfaces/IModularAccount.sol"; +import { + Call, + IModularAccount, + ModuleEntity, + ValidationConfig, + ValidationFlags +} from "../interfaces/IModularAccount.sol"; import {IValidationHookModule} from "../interfaces/IValidationHookModule.sol"; import {IValidationModule} from "../interfaces/IValidationModule.sol"; import {HookConfig, HookConfigLib} from "../libraries/HookConfigLib.sol"; @@ -43,7 +48,7 @@ contract ReferenceModularAccount is { using EnumerableSet for EnumerableSet.Bytes32Set; using ModuleEntityLib for ModuleEntity; - using ValidationConfigLib for ValidationConfig; + using ValidationConfigLib for ValidationFlags; using HookConfigLib for HookConfig; using SparseCalldataSegmentLib for bytes; @@ -598,7 +603,7 @@ contract ReferenceModularAccount is (address module, uint32 entityId) = userOpValidationFunction.unpack(); - if (!_storage.validationStorage[userOpValidationFunction].isUserOpValidation) { + if (!_storage.validationStorage[userOpValidationFunction].validationFlags.isUserOpValidation()) { revert UserOpValidationInvalid(module, entityId); } @@ -633,7 +638,7 @@ contract ReferenceModularAccount is AccountStorage storage _storage = getAccountStorage(); (address module, uint32 entityId) = sigValidation.unpack(); - if (!_storage.validationStorage[sigValidation].isSignatureValidation) { + if (!_storage.validationStorage[sigValidation].validationFlags.isSignatureValidation()) { revert SignatureValidationInvalid(module, entityId); } @@ -660,7 +665,7 @@ contract ReferenceModularAccount is } function _isValidationGlobal(ModuleEntity validationFunction) internal view virtual returns (bool) { - return getAccountStorage().validationStorage[validationFunction].isGlobal; + return getAccountStorage().validationStorage[validationFunction].validationFlags.isGlobal(); } function _checkIfValidationAppliesCallData( diff --git a/src/interfaces/IModularAccount.sol b/src/interfaces/IModularAccount.sol index 0eedcd4e..7846e5f6 100644 --- a/src/interfaces/IModularAccount.sol +++ b/src/interfaces/IModularAccount.sol @@ -15,10 +15,11 @@ type ValidationConfig is bytes25; // Layout: // 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address // 0x________________________________________BBBBBBBB________________ // Entity ID -// 0x________________________________________________CC______________ // validation flags +// 0x________________________________________________CC______________ // ValidationFlags // 0x__________________________________________________00000000000000 // unused -// -// Validation flags layout: + +type ValidationFlags is uint8; +// ValidationFlags layout: // 0b00000___ // unused // 0b_____A__ // isGlobal // 0b______B_ // isSignatureValidation diff --git a/src/interfaces/IModularAccountView.sol b/src/interfaces/IModularAccountView.sol index 2da36e5c..5825652e 100644 --- a/src/interfaces/IModularAccountView.sol +++ b/src/interfaces/IModularAccountView.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: CC0-1.0 pragma solidity ^0.8.20; -import {HookConfig, ModuleEntity} from "../interfaces/IModularAccount.sol"; +import {HookConfig, ModuleEntity, ValidationFlags} from "../interfaces/IModularAccount.sol"; /// @dev Represents data associated with a specific function selector. struct ExecutionDataView { @@ -20,12 +20,12 @@ struct ExecutionDataView { } struct ValidationDataView { - // Whether or not this validation function can be used as a global validation function. - bool isGlobal; - // Whether or not this validation function is a signature validator. - bool isSignatureValidation; - // Whether or not this validation function is a user operation validation function. - bool isUserOpValidation; + // ValidationFlags layout: + // 0b00000___ // unused + // 0b_____A__ // isGlobal + // 0b______B_ // isSignatureValidation + // 0b_______C // isUserOpValidation + ValidationFlags validationFlags; // The validation hooks for this validation function. HookConfig[] validationHooks; // Execution hooks to run with this validation function. diff --git a/src/libraries/ValidationConfigLib.sol b/src/libraries/ValidationConfigLib.sol index 2c6e2b95..e4842e2a 100644 --- a/src/libraries/ValidationConfigLib.sol +++ b/src/libraries/ValidationConfigLib.sol @@ -1,23 +1,16 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.20; -import {ModuleEntity, ValidationConfig} from "../interfaces/IModularAccount.sol"; - -// Validation flags layout: -// 0b00000___ // unused -// 0b_____A__ // isGlobal -// 0b______B_ // isSignatureValidation -// 0b_______C // isUserOpValidation -type ValidationFlags is uint8; +import {ModuleEntity, ValidationConfig, ValidationFlags} from "../interfaces/IModularAccount.sol"; // Validation config is a packed representation of a validation function and flags for its configuration. // Layout: // 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address // 0x________________________________________BBBBBBBB________________ // Entity ID -// 0x________________________________________________CC______________ // validation flags +// 0x________________________________________________CC______________ // ValidationFlags // 0x__________________________________________________00000000000000 // unused -// Validation flags layout: +// ValidationFlags layout: // 0b00000___ // unused // 0b_____A__ // isGlobal // 0b______B_ // isSignatureValidation diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index 1cbc11d3..6adb20ad 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -97,17 +97,18 @@ type ModuleEntity is bytes24; /// @dev A packed representation of a validation function and its associated flags. /// Consists of the following, left-aligned: -/// Module address: 20 bytes -/// Entity ID: 4 bytes -/// Flags: 1 byte -/// -/// Validation flags layout: -/// 0b00000___ // unused -/// 0b_____A__ // isGlobal -/// 0b______B_ // isSignatureValidation -/// 0b_______C // isUserOpValidation +/// Module address: 20 bytes +/// Entity ID: 4 bytes +/// ValidationFlags: 1 byte type ValidationConfig is bytes25; +// ValidationFlags layout: +// 0b00000___ // unused +// 0b_____A__ // isGlobal +// 0b______B_ // isSignatureValidation +// 0b_______C // isUserOpValidation +type ValidationFlags is uint8; + /// @dev A packed representation of a hook function and its associated flags. /// Consists of the following, left-aligned: /// Module address: 20 bytes @@ -239,12 +240,12 @@ struct ExecutionDataView { } struct ValidationDataView { - // Whether or not this validation function can be used as a global validation function. - bool isGlobal; - // Whether or not this validation function is a signature validator. - bool isSignatureValidation; - // Whether or not this validation function is a user operation validation function. - bool isUserOpValidation; + // ValidationFlags layout: + // 0b00000___ // unused + // 0b_____A__ // isGlobal + // 0b______B_ // isSignatureValidation + // 0b_______C // isUserOpValidation + ValidationFlags validationFlags; // The validation hooks for this validation function. HookConfig[] validationHooks; // Execution hooks to run with this validation function. diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol index 436f439e..12451f84 100644 --- a/test/account/ModularAccountView.t.sol +++ b/test/account/ModularAccountView.t.sol @@ -3,16 +3,18 @@ pragma solidity ^0.8.20; import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; -import {HookConfigLib} from "../../src/libraries/HookConfigLib.sol"; -import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; - import {HookConfig, IModularAccount} from "../../src/interfaces/IModularAccount.sol"; import {ExecutionDataView, ValidationDataView} from "../../src/interfaces/IModularAccountView.sol"; +import {HookConfigLib} from "../../src/libraries/HookConfigLib.sol"; +import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; +import {ValidationConfigLib, ValidationFlags} from "../../src/libraries/ValidationConfigLib.sol"; import {ComprehensiveModule} from "../mocks/modules/ComprehensiveModule.sol"; import {CustomValidationTestBase} from "../utils/CustomValidationTestBase.sol"; contract ModularAccountViewTest is CustomValidationTestBase { + using ValidationConfigLib for ValidationFlags; + ComprehensiveModule public comprehensiveModule; event ReceivedCall(bytes msgData, uint256 msgValue); @@ -100,9 +102,9 @@ contract ModularAccountViewTest is CustomValidationTestBase { ValidationDataView memory data = account1.getValidationData(comprehensiveModuleValidation); bytes4[] memory selectors = data.selectors; - assertTrue(data.isGlobal); - assertTrue(data.isSignatureValidation); - assertTrue(data.isUserOpValidation); + assertTrue(data.validationFlags.isGlobal()); + assertTrue(data.validationFlags.isSignatureValidation()); + assertTrue(data.validationFlags.isUserOpValidation()); assertEq(data.validationHooks.length, 2); assertEq( HookConfig.unwrap(data.validationHooks[0]),