feat: add signature validation hooks [2/2]

src/account/UpgradeableModularAccount.sol

@@ -295,8 +295,27 @@ contract UpgradeableModularAccount is
 
     function isValidSignature(bytes32 hash, bytes calldata signature) public view override returns (bytes4) {
         ModuleEntity sigValidation = ModuleEntity.wrap(bytes24(signature));
+        signature = signature[24:];
 
-        return _exec1271Validation(sigValidation, hash, signature[24:]);
+        ModuleEntity[] memory preSignatureValidationHooks =
+            getAccountStorage().validationData[sigValidation].preValidationHooks;
+
+        for (uint256 i = 0; i < preSignatureValidationHooks.length; ++i) {
+            (address hookModule, uint32 hookEntityId) = preSignatureValidationHooks[i].unpack();
+
+            bytes memory currentSignatureSegment;
+
+            (currentSignatureSegment, signature) = signature.advanceSegmentIfAtIndex(uint8(i));
+
+            // If this reverts, bubble up revert reason.
+            IValidationHookModule(hookModule).preSignatureValidationHook(
+                hookEntityId, msg.sender, hash, currentSignatureSegment
+            );
+        }
+
+        signature = signature.getFinalSegment();
+
+        return _exec1271Validation(sigValidation, hash, signature);
     }
 
     /// @notice Gets the entry point for this account

src/interfaces/IValidationHookModule.sol

@@ -32,18 +32,14 @@ interface IValidationHookModule is IModule {
         bytes calldata authorization
     ) external;
 
-    // TODO: support this hook type within the account & in the manifest
-
     /// @notice Run the pre signature validation hook specified by the `entityId`.
     /// @dev To indicate the call should revert, the function MUST revert.
     /// @param entityId An identifier that routes the call to different internal implementations, should there
     /// be more than one.
     /// @param sender The caller address.
     /// @param hash The hash of the message being signed.
     /// @param signature The signature of the message.
-    // function preSignatureValidationHook(uint32 entityId, address sender, bytes32 hash, bytes calldata
-    // signature)
-    //     external
-    //     view
-    //     returns (bytes4);
+    function preSignatureValidationHook(uint32 entityId, address sender, bytes32 hash, bytes calldata signature)
+        external
+        view;
 }

src/modules/NativeTokenLimitModule.sol

@@ -116,6 +116,9 @@ contract NativeTokenLimitModule is BaseModule, IExecutionHookModule, IValidation
         override
     {} // solhint-disable-line no-empty-blocks
 
+    // solhint-disable-next-line no-empty-blocks
+    function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external pure override {}
+
     /// @inheritdoc IModule
     function moduleMetadata() external pure virtual override returns (ModuleMetadata memory) {
         ModuleMetadata memory metadata;

src/modules/permissionhooks/AllowlistModule.sol

@@ -85,6 +85,9 @@ contract AllowlistModule is IValidationHookModule, BaseModule {
         return;
     }
 
+    // solhint-disable-next-line no-empty-blocks
+    function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external pure override {}
+
     function moduleMetadata() external pure override returns (ModuleMetadata memory) {
         ModuleMetadata memory metadata;
         metadata.name = "Allowlist Module";

test/account/PerHookData.t.sol

@@ -355,6 +355,53 @@ contract PerHookDataTest is CustomValidationTestBase {
         );
     }
 
+    function test_pass1271AccessControl() public {
+        string memory message = "Hello, world!";
+
+        bytes32 messageHash = keccak256(abi.encodePacked(message));
+
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, messageHash);
+
+        PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1);
+        preValidationHookData[0] = PreValidationHookData({index: 0, validationData: abi.encodePacked(message)});
+
+        bytes4 result = account1.isValidSignature(
+            messageHash, _encode1271Signature(_signerValidation, preValidationHookData, abi.encodePacked(r, s, v))
+        );
+
+        assertEq(result, bytes4(0x1626ba7e));
+    }
+
+    function test_fail1271AccessControl_badSigData() public {
+        string memory message = "Hello, world!";
+
+        bytes32 messageHash = keccak256(abi.encodePacked(message));
+
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, messageHash);
+
+        PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1);
+        preValidationHookData[0] = PreValidationHookData({
+            index: 0,
+            validationData: abi.encodePacked(address(0x1234123412341234123412341234123412341234))
+        });
+
+        vm.expectRevert("Preimage not provided");
+        account1.isValidSignature(
+            messageHash, _encode1271Signature(_signerValidation, preValidationHookData, abi.encodePacked(r, s, v))
+        );
+    }
+
+    function test_fail1271AccessControl_noSigData() public {
+        string memory message = "Hello, world!";
+
+        bytes32 messageHash = keccak256(abi.encodePacked(message));
+
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, messageHash);
+
+        vm.expectRevert("Preimage not provided");
+        account1.isValidSignature(messageHash, _encode1271Signature(_signerValidation, abi.encodePacked(r, s, v)));
+    }
+
     function _getCounterUserOP() internal view returns (PackedUserOperation memory, bytes32) {
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),

test/account/UpgradeableModularAccount.t.sol

@@ -397,7 +397,7 @@ contract UpgradeableModularAccountTest is AccountTestBase {
 
         // singleSignerValidationModule.ownerOf(address(account1));
 
-        bytes memory signature = abi.encodePacked(_signerValidation, r, s, v);
+        bytes memory signature = _encode1271Signature(_signerValidation, abi.encodePacked(r, s, v));
 
         bytes4 validationResult = IERC1271(address(account1)).isValidSignature(message, signature);
 

test/mocks/modules/ComprehensiveModule.sol

@@ -104,6 +104,10 @@ contract ComprehensiveModule is
         revert NotImplemented();
     }
 
+    function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external pure override {
+        return;
+    }
+
     function validateSignature(address, uint32 entityId, address, bytes32, bytes calldata)
         external
         pure

test/mocks/modules/MockAccessControlHookModule.sol

@@ -72,5 +72,17 @@ contract MockAccessControlHookModule is IValidationHookModule, BaseModule {
         revert NotImplemented();
     }
 
+    function preSignatureValidationHook(uint32, address, bytes32 hash, bytes calldata signature)
+        external
+        pure
+        override
+    {
+        // Simulates some signature checking by requiring a preimage of the hash.
+
+        require(keccak256(signature) == hash, "Preimage not provided");
+
+        return;
+    }
+
     function moduleMetadata() external pure override returns (ModuleMetadata memory) {}
 }

test/mocks/modules/ValidationModuleMocks.sol

@@ -64,6 +64,8 @@ abstract contract MockBaseUserOpValidationModule is
         revert NotImplemented();
     }
 
+    function preSignatureValidationHook(uint32, address, bytes32, bytes calldata) external pure override {}
+
     function validateSignature(address, uint32, address, bytes32, bytes calldata)
         external
         pure

test/utils/AccountTestBase.sol

@@ -198,6 +198,21 @@ abstract contract AccountTestBase is OptimizedTest {
         return bytes32(uint256((g1 << 128) + uint128(g2)));
     }
 
+    // helper function to encode a 1271 signature, according to the per-hook and per-validation data format.
+    function _encode1271Signature(
+        ModuleEntity validationFunction,
+        PreValidationHookData[] memory preValidationHookData,
+        bytes memory validationData
+    ) internal pure returns (bytes memory) {
+        bytes memory sig = abi.encodePacked(validationFunction);
+
+        sig = abi.encodePacked(sig, _packPreHookDatas(preValidationHookData));
+
+        sig = abi.encodePacked(sig, _packValidationResWithIndex(255, validationData));
+
+        return sig;
+    }
+
     // helper function to encode a signature, according to the per-hook and per-validation data format.
     function _encodeSignature(
         ModuleEntity validationFunction,
@@ -207,17 +222,8 @@ abstract contract AccountTestBase is OptimizedTest {
     ) internal pure returns (bytes memory) {
         bytes memory sig = abi.encodePacked(validationFunction, globalOrNot);
 
-        for (uint256 i = 0; i < preValidationHookData.length; ++i) {
-            sig = abi.encodePacked(
-                sig,
-                _packValidationResWithIndex(
-                    preValidationHookData[i].index, preValidationHookData[i].validationData
-                )
-            );
-        }
+        sig = abi.encodePacked(sig, _packPreHookDatas(preValidationHookData));
 
-        // Index of the actual validation data is the length of the preValidationHooksRetrieved - aka
-        // one-past-the-end
         sig = abi.encodePacked(sig, _packValidationResWithIndex(255, validationData));
 
         return sig;
@@ -233,6 +239,36 @@ abstract contract AccountTestBase is OptimizedTest {
         return _encodeSignature(validationFunction, globalOrNot, emptyPreValidationHookData, validationData);
     }
 
+    // overload for the case where there are no pre-validation hooks
+    function _encode1271Signature(ModuleEntity validationFunction, bytes memory validationData)
+        internal
+        pure
+        returns (bytes memory)
+    {
+        PreValidationHookData[] memory emptyPreValidationHookData = new PreValidationHookData[](0);
+        return _encode1271Signature(validationFunction, emptyPreValidationHookData, validationData);
+    }
+
+    // helper function to pack pre-validation hook datas, according to the sparse calldata segment spec.
+    function _packPreHookDatas(PreValidationHookData[] memory preValidationHookData)
+        internal
+        pure
+        returns (bytes memory)
+    {
+        bytes memory res = "";
+
+        for (uint256 i = 0; i < preValidationHookData.length; ++i) {
+            res = abi.encodePacked(
+                res,
+                _packValidationResWithIndex(
+                    preValidationHookData[i].index, preValidationHookData[i].validationData
+                )
+            );
+        }
+
+        return res;
+    }
+
     // helper function to pack validation data with an index, according to the sparse calldata segment spec.
     function _packValidationResWithIndex(uint8 index, bytes memory validationData)
         internal