feat: consistency in hook UDVTs for ValidationDataView (#188)

jaypaik · web-flow · commit 8c09ff9d5afe · 2024-10-01T16:56:03.000-04:00
diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol
@@ -31,8 +31,8 @@ struct ValidationData {
     bool isSignatureValidation;
     // Whether or not this validation is allowed to validate ERC-4337 user operations.
     bool isUserOpValidation;
-    // The pre validation hooks for this validation function.
-    ModuleEntity[] preValidationHooks;
+    // The validation hooks for this validation function.
+    HookConfig[] validationHooks;
     // Execution hooks to run with this validation function.
     EnumerableSet.Bytes32Set executionHooks;
     // The set of selectors that may be validated by this validation function.
diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol
@@ -50,7 +50,7 @@ abstract contract ModularAccountView is IModularAccountView {
         data.isGlobal = validationData.isGlobal;
         data.isSignatureValidation = validationData.isSignatureValidation;
         data.isUserOpValidation = validationData.isUserOpValidation;
-        data.preValidationHooks = validationData.preValidationHooks;
+        data.validationHooks = validationData.validationHooks;
 
         uint256 execHooksLen = validationData.executionHooks.length();
         data.executionHooks = new HookConfig[](execHooksLen);
diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol
@@ -233,10 +233,10 @@ abstract contract ModuleManagerInternals is IModularAccount {
             bytes calldata hookData = hooks[i][25:];
 
             if (hookConfig.isValidationHook()) {
-                _validationData.preValidationHooks.push(hookConfig.moduleEntity());
+                _validationData.validationHooks.push(hookConfig);
 
                 // Avoid collision between reserved index and actual indices
-                if (_validationData.preValidationHooks.length > MAX_PRE_VALIDATION_HOOKS) {
+                if (_validationData.validationHooks.length > MAX_PRE_VALIDATION_HOOKS) {
                     revert PreValidationHookLimitExceeded();
                 }
 
@@ -280,16 +280,16 @@ abstract contract ModuleManagerInternals is IModularAccount {
             // If any uninstall data is provided, assert it is of the correct length.
             if (
                 hookUninstallDatas.length
-                    != _validationData.preValidationHooks.length + _validationData.executionHooks.length()
+                    != _validationData.validationHooks.length + _validationData.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.preValidationHooks.length; ++i) {
+            for (uint256 i = 0; i < _validationData.validationHooks.length; ++i) {
                 bytes calldata hookData = hookUninstallDatas[hookIndex];
-                (address hookModule,) = ModuleEntityLib.unpack(_validationData.preValidationHooks[i]);
+                (address hookModule,) = ModuleEntityLib.unpack(_validationData.validationHooks[i].moduleEntity());
                 onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData);
                 hookIndex++;
             }
@@ -304,7 +304,7 @@ abstract contract ModuleManagerInternals is IModularAccount {
         }
 
         // Clear all stored hooks
-        delete _validationData.preValidationHooks;
+        delete _validationData.validationHooks;
 
         EnumerableSet.Bytes32Set storage executionHooks = _validationData.executionHooks;
         uint256 executionHookLen = executionHooks.length();
diff --git a/src/account/ReferenceModularAccount.sol b/src/account/ReferenceModularAccount.sol
@@ -311,11 +311,11 @@ contract ReferenceModularAccount is
         ModuleEntity sigValidation = ModuleEntity.wrap(bytes24(signature));
         signature = signature[24:];
 
-        ModuleEntity[] memory preSignatureValidationHooks =
-            getAccountStorage().validationData[sigValidation].preValidationHooks;
+        HookConfig[] memory preSignatureValidationHooks =
+            getAccountStorage().validationData[sigValidation].validationHooks;
 
         for (uint256 i = 0; i < preSignatureValidationHooks.length; ++i) {
-            (address hookModule, uint32 hookEntityId) = preSignatureValidationHooks[i].unpack();
+            (address hookModule, uint32 hookEntityId) = preSignatureValidationHooks[i].moduleEntity().unpack();
 
             bytes memory currentSignatureSegment;
 
@@ -384,13 +384,13 @@ contract ReferenceModularAccount is
         uint256 validationRes;
 
         // Do preUserOpValidation hooks
-        ModuleEntity[] memory preUserOpValidationHooks =
-            getAccountStorage().validationData[userOpValidationFunction].preValidationHooks;
+        HookConfig[] memory preUserOpValidationHooks =
+            getAccountStorage().validationData[userOpValidationFunction].validationHooks;
 
         for (uint256 i = 0; i < preUserOpValidationHooks.length; ++i) {
             (userOp.signature, signature) = signature.advanceSegmentIfAtIndex(uint8(i));
 
-            (address module, uint32 entityId) = preUserOpValidationHooks[i].unpack();
+            (address module, uint32 entityId) = preUserOpValidationHooks[i].moduleEntity().unpack();
             uint256 currentValidationRes =
                 IValidationHookModule(module).preUserOpValidationHook(entityId, userOp, userOpHash);
 
@@ -424,15 +424,15 @@ contract ReferenceModularAccount is
         bytes calldata authorizationData
     ) internal {
         // run all preRuntimeValidation hooks
-        ModuleEntity[] memory preRuntimeValidationHooks =
-            getAccountStorage().validationData[runtimeValidationFunction].preValidationHooks;
+        HookConfig[] memory preRuntimeValidationHooks =
+            getAccountStorage().validationData[runtimeValidationFunction].validationHooks;
 
         for (uint256 i = 0; i < preRuntimeValidationHooks.length; ++i) {
             bytes memory currentAuthSegment;
 
             (currentAuthSegment, authorizationData) = authorizationData.advanceSegmentIfAtIndex(uint8(i));
 
-            _doPreRuntimeValidationHook(preRuntimeValidationHooks[i], callData, currentAuthSegment);
+            _doPreRuntimeValidationHook(preRuntimeValidationHooks[i].moduleEntity(), callData, currentAuthSegment);
         }
 
         authorizationData = authorizationData.getFinalSegment();
@@ -569,12 +569,12 @@ contract ReferenceModularAccount is
             // Direct call is allowed, run associated execution & validation hooks
 
             // Validation hooks
-            ModuleEntity[] memory preRuntimeValidationHooks =
-                _storage.validationData[directCallValidationKey].preValidationHooks;
+            HookConfig[] memory preRuntimeValidationHooks =
+                _storage.validationData[directCallValidationKey].validationHooks;
 
             uint256 hookLen = preRuntimeValidationHooks.length;
             for (uint256 i = 0; i < hookLen; ++i) {
-                _doPreRuntimeValidationHook(preRuntimeValidationHooks[i], msg.data, "");
+                _doPreRuntimeValidationHook(preRuntimeValidationHooks[i].moduleEntity(), msg.data, "");
             }
 
             // Execution hooks associated with the validator
diff --git a/src/interfaces/IModularAccountView.sol b/src/interfaces/IModularAccountView.sol
@@ -26,8 +26,8 @@ struct ValidationDataView {
     bool isSignatureValidation;
     // Whether or not this validation function is a user operation validation function.
     bool isUserOpValidation;
-    // The pre validation hooks for this validation function.
-    ModuleEntity[] preValidationHooks;
+    // 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.
diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md
@@ -245,8 +245,8 @@ struct ValidationDataView {
     bool isSignatureValidation;
     // Whether or not this validation function is a user operation validation function.
     bool isUserOpValidation;
-    // The pre validation hooks for this validation function.
-    ModuleEntity[] preValidationHooks;
+    // 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.
diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol
@@ -103,19 +103,19 @@ contract ModularAccountViewTest is CustomValidationTestBase {
         assertTrue(data.isGlobal);
         assertTrue(data.isSignatureValidation);
         assertTrue(data.isUserOpValidation);
-        assertEq(data.preValidationHooks.length, 2);
+        assertEq(data.validationHooks.length, 2);
         assertEq(
-            ModuleEntity.unwrap(data.preValidationHooks[0]),
-            ModuleEntity.unwrap(
-                ModuleEntityLib.pack(
+            HookConfig.unwrap(data.validationHooks[0]),
+            HookConfig.unwrap(
+                HookConfigLib.packValidationHook(
                     address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.PRE_VALIDATION_HOOK_1)
                 )
             )
         );
         assertEq(
-            ModuleEntity.unwrap(data.preValidationHooks[1]),
-            ModuleEntity.unwrap(
-                ModuleEntityLib.pack(
+            HookConfig.unwrap(data.validationHooks[1]),
+            HookConfig.unwrap(
+                HookConfigLib.packValidationHook(
                     address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.PRE_VALIDATION_HOOK_2)
                 )
             )
diff --git a/test/module/NativeTokenLimitModule.t.sol b/test/module/NativeTokenLimitModule.t.sol
@@ -11,7 +11,7 @@ import {ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol";
 import {HookConfigLib} from "../../src/helpers/HookConfigLib.sol";
 import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol";
 import {ExecutionManifest} from "../../src/interfaces/IExecutionModule.sol";
-import {Call, IModularAccount} from "../../src/interfaces/IModularAccount.sol";
+import {Call, HookConfig, IModularAccount} from "../../src/interfaces/IModularAccount.sol";
 import {NativeTokenLimitModule} from "../../src/modules/NativeTokenLimitModule.sol";
 import {MockModule} from "../mocks/MockModule.sol";
 
@@ -35,8 +35,8 @@ contract NativeTokenLimitModuleTest is AccountTestBase {
 
         vm.deal(address(acct), 10 ether);
 
-        ModuleEntity[] memory preValidationHooks = new ModuleEntity[](1);
-        preValidationHooks[0] = ModuleEntityLib.pack(address(module), 0);
+        HookConfig[] memory validationHooks = new HookConfig[](1);
+        validationHooks[0] = HookConfigLib.packValidationHook(address(module), 0);
 
         uint256[] memory spendLimits = new uint256[](1);
         spendLimits[0] = spendLimit;
