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.

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: update ValidationDataView with ValidationFlags #207

Merged
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@erc6900/reference-implementation",
"version": "0.8.0-rc.6",
"version": "0.8.0-rc.7",
"devDependencies": {
"pnpm": "^8.7.5",
"solhint": "^3.6.2"
Expand Down
14 changes: 7 additions & 7 deletions src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity ^0.8.20;

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

import {HookConfig, ModuleEntity} from "../interfaces/IModularAccount.sol";
import {HookConfig, ModuleEntity, ValidationFlags} from "../interfaces/IModularAccount.sol";

// bytes = keccak256("ERC6900.ReferenceModularAccount.Storage")
bytes32 constant _ACCOUNT_STORAGE_SLOT = 0xc531f081ecdb5a90f38c197521797881a6e5c752a7d451780f325a95f8b91f45;
Expand All @@ -25,12 +25,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.
Expand Down
4 changes: 1 addition & 3 deletions src/account/ModularAccountView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
19 changes: 10 additions & 9 deletions src/account/ModuleManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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]));
Expand Down Expand Up @@ -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());
Expand Down
17 changes: 11 additions & 6 deletions src/account/ReferenceModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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(
Expand Down
7 changes: 4 additions & 3 deletions src/interfaces/IModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this type from ValidationConfigLib into IModularAccount. I think it's slightly more consistent for all our UDVTs (like HookConfig) to be in the interfaces and not the libraries.

// ValidationFlags layout:
// 0b00000___ // unused
// 0b_____A__ // isGlobal
// 0b______B_ // isSignatureValidation
Expand Down
14 changes: 7 additions & 7 deletions src/interfaces/IModularAccountView.sol
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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.
Expand Down
13 changes: 3 additions & 10 deletions src/libraries/ValidationConfigLib.sol
Original file line number Diff line number Diff line change
@@ -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
Expand Down
31 changes: 16 additions & 15 deletions standard/ERCs/erc-6900.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 8 additions & 6 deletions test/account/ModularAccountView.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@ pragma solidity ^0.8.20;

import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";

import {HookConfig, IModularAccount, ValidationFlags} 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 {HookConfig, IModularAccount} from "../../src/interfaces/IModularAccount.sol";
import {ExecutionDataView, ValidationDataView} from "../../src/interfaces/IModularAccountView.sol";
import {ValidationConfigLib} 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);
Expand Down Expand Up @@ -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]),
Expand Down
Loading