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

[Improvement] A more forward-compatible ValidationDataView struct #53

Open
jaypaik opened this issue Nov 13, 2024 · 1 comment · May be fixed by erc6900/reference-implementation#207
Open

Comments

@jaypaik
Copy link
Contributor

jaypaik commented Nov 13, 2024

We currently define ValidationDataView, which is returned by IModularAccountView.getValidationView as:

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;
    // The validation hooks for this validation function.
    HookConfig[] validationHooks;
    // Execution hooks to run with this validation function.
    HookConfig[] executionHooks;
    // The set of selectors that may be validated by this validation function.
    bytes4[] selectors;
}

However, our user-defined value type (UDVT) ValidationConfig is defined as:

/// @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
type ValidationConfig is bytes25;

Using named booleans in ValidationDataView makes things slightly more friendly for consumers, but since we already return the UDVT HookConfig in the same struct, it might make sense to return a packed bytes1 validationFlags instead, which is more forward compatible incase we want to add additional flags in the future (e.g., isRuntimeValidation).

Proposal:

struct ValidationDataView {
    // Validation flags layout:
    // 0b00000___ // unused
    // 0b_____A__ // isGlobal
    // 0b______B_ // isSignatureValidation
    // 0b_______C // isUserOpValidation
    bytes1 validationFlags;
    // The validation hooks for this validation function.
    HookConfig[] validationHooks;
    // Execution hooks to run with this validation function.
    HookConfig[] executionHooks;
    // The set of selectors that may be validated by this validation function.
    bytes4[] selectors;
}

Curious to hear others' thoughts.

@huaweigu
Copy link

That makes sense to me. It will require some minor adjustments, but we can make it work. Do we need an unpack function to extract everything for client-side integration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants