[#108] Add tests for signature validator, minor fix in Registry

5afe · Oct 19, 2023 · eec7ede · eec7ede
1 parent 20c2bb8
commit eec7ede
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 4 deletions.
diff --git a/contracts/SafeProtocolRegistry.sol b/contracts/SafeProtocolRegistry.sol
@@ -89,7 +89,7 @@ contract SafeProtocolRegistry is ISafeProtocolRegistry, Ownable2Step {
             moduleTypes & MODULE_TYPE_SIGNATURE_VALIDATOR == MODULE_TYPE_SIGNATURE_VALIDATOR &&
             !IERC165(module).supportsInterface(type(ISafeProtocol712SignatureValidator).interfaceId)
         ) {
-            revert ModuleDoesNotSupportExpectedInterfaceId(module, type(ISafeProtocolFunctionHandler).interfaceId);
+            revert ModuleDoesNotSupportExpectedInterfaceId(module, type(ISafeProtocol712SignatureValidator).interfaceId);
         }
 
         if (

diff --git a/test/SafeProtocolRegistry.spec.ts b/test/SafeProtocolRegistry.spec.ts
@@ -2,7 +2,13 @@ import hre, { ethers, deployments } from "hardhat";
 import { expect } from "chai";
 import { AddressZero } from "@ethersproject/constants";
 import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers";
-import { MODULE_TYPE_PLUGIN, MODULE_TYPE_HOOKS, MODULE_TYPE_FUNCTION_HANDLER } from "../src/utils/constants";
+import {
+    MODULE_TYPE_PLUGIN,
+    MODULE_TYPE_HOOKS,
+    MODULE_TYPE_FUNCTION_HANDLER,
+    MODULE_TYPE_SIGNATURE_VALIDATOR,
+    MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS,
+} from "../src/utils/constants";
 import { getHooksWithPassingChecks, getHooksWithFailingCallToSupportsInterfaceMethod } from "./utils/mockHooksBuilder";
 import { getPluginWithFailingCallToSupportsInterfaceMethod } from "./utils/mockPluginBuilder";
 import { getFunctionHandlerWithFailingCallToSupportsInterfaceMethod } from "./utils/mockFunctionHandlerBuilder";
@@ -39,11 +45,20 @@ describe("SafeProtocolRegistry", async () => {
 
         await safeProtocolRegistry
             .connect(owner)
-            .addModule(mockModule, MODULE_TYPE_PLUGIN + MODULE_TYPE_FUNCTION_HANDLER + MODULE_TYPE_HOOKS);
+            .addModule(
+                mockModule,
+                MODULE_TYPE_PLUGIN +
+                    MODULE_TYPE_FUNCTION_HANDLER +
+                    MODULE_TYPE_HOOKS +
+                    MODULE_TYPE_SIGNATURE_VALIDATOR +
+                    MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS,
+            );
+
         const [listedAt, flaggedAt] = await safeProtocolRegistry.check.staticCall(
             mockModule.target,
             numberToBytes32(MODULE_TYPE_FUNCTION_HANDLER),
         );
+
         expect(listedAt).to.be.greaterThan(0);
         expect(flaggedAt).to.be.equal(0);
 
@@ -54,6 +69,20 @@ describe("SafeProtocolRegistry", async () => {
         const [listedAt3, flaggedAt3] = await safeProtocolRegistry.check.staticCall(mockModule.target, numberToBytes32(MODULE_TYPE_HOOKS));
         expect(listedAt3).to.be.greaterThan(0);
         expect(flaggedAt3).to.be.equal(0);
+
+        const [listedAt4, flaggedAt4] = await safeProtocolRegistry.check.staticCall(
+            mockModule.target,
+            numberToBytes32(MODULE_TYPE_SIGNATURE_VALIDATOR),
+        );
+        expect(listedAt4).to.be.greaterThan(0);
+        expect(flaggedAt4).to.be.equal(0);
+
+        const [listedAt5, flaggedAt5] = await safeProtocolRegistry.check.staticCall(
+            mockModule.target,
+            numberToBytes32(MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS),
+        );
+        expect(listedAt5).to.be.greaterThan(0);
+        expect(flaggedAt5).to.be.equal(0);
     });
 
     it("Should not allow adding a module with invalid moduleTypes", async () => {
@@ -166,4 +195,24 @@ describe("SafeProtocolRegistry", async () => {
             .to.be.revertedWithCustomError(safeProtocolRegistry, "ModuleDoesNotSupportExpectedInterfaceId")
             .withArgs(mockFunctionHandlerAddress, "0xf601ad15");
     });
+
+    it("Should revert when signature validator hooks not supporting expected interfaceId", async () => {
+        const { safeProtocolRegistry } = await setupTests();
+        const mockContract = await (await hre.ethers.getContractFactory("MockContract")).deploy();
+        await mockContract.givenMethodReturnBool("0x01ffc9a7", false);
+
+        await expect(safeProtocolRegistry.connect(owner).addModule(mockContract.target, MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS))
+            .to.be.revertedWithCustomError(safeProtocolRegistry, "ModuleDoesNotSupportExpectedInterfaceId")
+            .withArgs(mockContract.target, "0xd340d5af");
+    });
+
+    it("Should revert when signature validator not supporting expected interfaceId", async () => {
+        const { safeProtocolRegistry } = await setupTests();
+        const mockContract = await (await hre.ethers.getContractFactory("MockContract")).deploy();
+        await mockContract.givenMethodReturnBool("0x01ffc9a7", false);
+
+        await expect(safeProtocolRegistry.connect(owner).addModule(mockContract.target, MODULE_TYPE_SIGNATURE_VALIDATOR))
+            .to.be.revertedWithCustomError(safeProtocolRegistry, "ModuleDoesNotSupportExpectedInterfaceId")
+            .withArgs(mockContract.target, "0x38c8d4e6");
+    });
 });
diff --git a/test/SignatureValidatorManager.spec.ts b/test/SignatureValidatorManager.spec.ts
@@ -10,7 +10,7 @@ import {
 import { expect } from "chai";
 import { getMockSignatureValidationHooks } from "./utils/mockValidationHooksBuilder";
 
-describe.only("SignatureValidatorManager", () => {
+describe("SignatureValidatorManager", () => {
     let deployer: SignerWithAddress, owner: SignerWithAddress;
     const SIGNATURE_SELECTOR = hre.ethers.keccak256(toUtf8Bytes("Account712Signature(bytes32,bytes32,bytes)")).slice(0, 10); // 0xb5c726cb
 
@@ -30,6 +30,31 @@ describe.only("SignatureValidatorManager", () => {
         return { safeProtocolSignatureValidatorManager, safeProtocolManager, safeProtocolRegistry };
     });
 
+    it("should revert when enabling a signature validator not implementing ISafeProtocol712SignatureValidator interface", async () => {
+        const { safeProtocolSignatureValidatorManager, safeProtocolManager, safeProtocolRegistry } = await setupTests();
+        const account = await hre.ethers.deployContract("TestExecutor", [safeProtocolManager.target], { signer: deployer });
+
+        // set up mock contract as a signature validator
+        const mockContract = await hre.ethers.deployContract("MockContract", { signer: deployer });
+
+        await mockContract.givenMethodReturnBool("0x01ffc9a7", true);
+
+        await safeProtocolRegistry.connect(owner).addModule(mockContract.target, MODULE_TYPE_SIGNATURE_VALIDATOR);
+
+        await mockContract.givenMethodReturnBool("0x01ffc9a7", false);
+
+        const domainSeparator = hre.ethers.randomBytes(32);
+
+        const dataSetValidator = safeProtocolSignatureValidatorManager.interface.encodeFunctionData("setSignatureValidator", [
+            domainSeparator,
+            mockContract.target,
+        ]);
+
+        await expect(account.executeCallViaMock(safeProtocolSignatureValidatorManager.target, 0, dataSetValidator, MaxUint256))
+            .to.be.revertedWithCustomError(safeProtocolSignatureValidatorManager, "ContractDoesNotImplementValidInterfaceId")
+            .withArgs(mockContract.target);
+    });
+
     it("should allow to remove signature validator", async () => {
         const { safeProtocolSignatureValidatorManager, safeProtocolManager, safeProtocolRegistry } = await setupTests();
         const account = await hre.ethers.deployContract("TestExecutor", [safeProtocolManager.target], { signer: deployer });
@@ -63,6 +88,28 @@ describe.only("SignatureValidatorManager", () => {
         expect(await safeProtocolSignatureValidatorManager.signatureValidators(account.target, domainSeparator)).to.be.equal(ZeroAddress);
     });
 
+    it("should revert when enabling a signature validator hooks not implementing ISafeProtocolSignatureValidatorHooks interface", async () => {
+        const { safeProtocolSignatureValidatorManager, safeProtocolManager, safeProtocolRegistry } = await setupTests();
+        const account = await hre.ethers.deployContract("TestExecutor", [safeProtocolManager.target], { signer: deployer });
+
+        // set up mock contract as a signature validator
+        const mockContract = await hre.ethers.deployContract("MockContract", { signer: deployer });
+
+        await mockContract.givenMethodReturnBool("0x01ffc9a7", true);
+
+        await safeProtocolRegistry.connect(owner).addModule(mockContract.target, MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS);
+
+        await mockContract.givenMethodReturnBool("0x01ffc9a7", false);
+
+        const dataSetValidatorHooks = safeProtocolSignatureValidatorManager.interface.encodeFunctionData("setSignatureValidatorHooks", [
+            mockContract.target,
+        ]);
+
+        await expect(account.executeCallViaMock(safeProtocolSignatureValidatorManager.target, 0, dataSetValidatorHooks, MaxUint256))
+            .to.be.revertedWithCustomError(safeProtocolSignatureValidatorManager, "ContractDoesNotImplementValidInterfaceId")
+            .withArgs(mockContract.target);
+    });
+
     it("should allow to remove signature validator hooks", async () => {
         const { safeProtocolSignatureValidatorManager, safeProtocolManager, safeProtocolRegistry } = await setupTests();
         const account = await hre.ethers.deployContract("TestExecutor", [safeProtocolManager.target], { signer: deployer });
@@ -309,4 +356,9 @@ describe.only("SignatureValidatorManager", () => {
             "0x000000000000000000000000000000000000000000000000000000001626ba7e",
         ]);
     });
+
+    it("call metadataProvider() for increasing coverage", async () => {
+        const { safeProtocolSignatureValidatorManager } = await setupTests();
+        expect(await safeProtocolSignatureValidatorManager.metadataProvider());
+    });
 });