Skip to content

Conversation

@jaypaik
Copy link
Collaborator

@jaypaik jaypaik commented Sep 30, 2024

For consistency, updating

struct ValidationDataView {
    ...
    // The pre validation hooks for this validation function.
    ModuleEntity[] preValidationHooks;
    // Execution hooks to run with this validation function.
    HookConfig[] executionHooks;
    ...
}

to:

struct ValidationDataView {
    ...
    // The validation hooks for this validation function.
    HookConfig[] validationHooks;
    // Execution hooks to run with this validation function.
    HookConfig[] executionHooks;
    ...
}

Validation hooks are defined in the spec as always running before a validation function, so we can make this simplification here. However, I see a slight edge in keeping preUserOpValidationHook, preRuntimeValidationHook, and preSignatureValidationHook prefixed with "pre" in the rare event that postUserOpValidationHook etc. are introduced in the future. This would allow for backward compatibility for v0.8 validation hook modules and the above proposed change also allows us to keep the ValidationDataView interface the same.

I've opted to also update the storage in the reference account to store HookConfig[] instead of ModuleEntity[] for simplicity.

@jaypaik jaypaik requested a review from a team September 30, 2024 23:12
@jaypaik jaypaik force-pushed the 09-30-feat_consistency_in_hook_UDVTs_for_ValidationDataView branch from 3cfbc7e to afd714e Compare October 1, 2024 04:40
Copy link
Contributor

@adamegyed adamegyed left a comment

Choose a reason for hiding this comment

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

Looks good, 1 small comment

@jaypaik jaypaik force-pushed the 09-30-fix_add_missing_isUserOpValidation_to_ValidationDataView_in_spec branch from 42c537d to d2e8540 Compare October 1, 2024 20:19
Base automatically changed from 09-30-fix_add_missing_isUserOpValidation_to_ValidationDataView_in_spec to develop October 1, 2024 20:42
@jaypaik jaypaik force-pushed the 09-30-feat_consistency_in_hook_UDVTs_for_ValidationDataView branch from afd714e to d06cfbc Compare October 1, 2024 20:43
@octane-security-app
Copy link

octane-security-app bot commented Oct 1, 2024

Summary by Octane

New Contracts

No new contracts were added in this PR.

Updated Contracts

  • AccountStorage.sol: Replaced preValidationHooks with validationHooks in the validation function configuration.
  • ModularAccountView.sol: Changed preValidationHooks to validationHooks to generalize and consolidate hook configurations.
  • ModuleManagerInternals.sol: Consolidated pre-validation hooks into generalized validation hooks for better modularity and maintenance.
  • ReferenceModularAccount.sol: Updated the smart contract to utilize HookConfig for validation hooks, replacing ModuleEntity references with moduleEntity() method calls.
  • IModularAccountView.sol: Pre-validation hooks merged into a single validationHooks structure.

🔗 Commit Hash: fd241dc

@octane-security-app

This comment was marked as resolved.

@jaypaik jaypaik merged commit 8c09ff9 into develop Oct 1, 2024
4 checks passed
@jaypaik jaypaik deleted the 09-30-feat_consistency_in_hook_UDVTs_for_ValidationDataView branch October 1, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants