Apply whitelisting and disabling for submitZKPResponseV2 (#318)

* Refactor some Verifier methods

* Add unit tests

* Refactor unit tests

* Refactor unit tests

* Small refactor unit test

* Fix tests

* Bump versions

contracts/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@iden3/contracts",
   "description": "Smart Contract library for Solidity",
-  "version": "2.5.4",
+  "version": "2.5.5",
   "files": [
     "**/*.sol",
     "/build/contracts/*.json",

contracts/verifiers/RequestDisableable.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.27;
 
 import {ZKPVerifierBase} from "./ZKPVerifierBase.sol";
 import {ICircuitValidator} from "../interfaces/ICircuitValidator.sol";
+import {IZKPVerifier} from "../interfaces/IZKPVerifier.sol";
 
 contract RequestDisableable is ZKPVerifierBase {
     /// @custom:storage-location erc7201:iden3.storage.RequestDisableable
@@ -26,22 +27,6 @@ contract RequestDisableable is ZKPVerifierBase {
         _;
     }
 
-    /// @dev Submits a ZKP response and updates proof status
-    /// @param requestId The ID of the ZKP request
-    /// @param inputs The input data for the proof
-    /// @param a The first component of the proof
-    /// @param b The second component of the proof
-    /// @param c The third component of the proof
-    function submitZKPResponse(
-        uint64 requestId,
-        uint256[] memory inputs,
-        uint256[2] memory a,
-        uint256[2][2] memory b,
-        uint256[2] memory c
-    ) public virtual override onlyEnabledRequest(requestId) {
-        super.submitZKPResponse(requestId, inputs, a, b, c);
-    }
-
     /// @dev Verifies a ZKP response without updating any proof status
     /// @param requestId The ID of the ZKP request
     /// @param inputs The public inputs for the proof
@@ -82,4 +67,17 @@ contract RequestDisableable is ZKPVerifierBase {
     function _enableZKPRequest(uint64 requestId) internal checkRequestExistence(requestId, true) {
         _getRequestDisableStorage()._requestDisabling[requestId] = false;
     }
+
+    function _getRequestIfCanBeVerified(
+        uint64 requestId
+    )
+        internal
+        view
+        virtual
+        override
+        onlyEnabledRequest(requestId)
+        returns (IZKPVerifier.ZKPRequest storage)
+    {
+        return super._getRequestIfCanBeVerified(requestId);
+    }
 }

contracts/verifiers/UniversalVerifier.sol

@@ -21,7 +21,7 @@ contract UniversalVerifier is
     /**
      * @dev Version of contract
      */
-    string public constant VERSION = "1.1.3";
+    string public constant VERSION = "1.1.4";
 
     /// @dev Event emitted upon submitting a ZKP request
     event ZKPResponseSubmitted(uint64 indexed requestId, address indexed caller);
@@ -113,7 +113,7 @@ contract UniversalVerifier is
         uint256[2] memory a,
         uint256[2][2] memory b,
         uint256[2] memory c
-    ) public override(RequestDisableable, ValidatorWhitelist, ZKPVerifierBase) {
+    ) public override {
         super.submitZKPResponse(requestId, inputs, a, b, c);
         emit ZKPResponseSubmitted(requestId, _msgSender());
     }
@@ -207,4 +207,16 @@ contract UniversalVerifier is
     function removeValidatorFromWhitelist(ICircuitValidator validator) public onlyOwner {
         _removeValidatorFromWhitelist(validator);
     }
+
+    function _getRequestIfCanBeVerified(
+        uint64 requestId
+    )
+        internal
+        view
+        override(RequestDisableable, ValidatorWhitelist, ZKPVerifierBase)
+        onlyEnabledRequest(requestId)
+        returns (IZKPVerifier.ZKPRequest storage)
+    {
+        return super._getRequestIfCanBeVerified(requestId);
+    }
 }

contracts/verifiers/ValidatorWhitelist.sol

@@ -42,24 +42,6 @@ contract ValidatorWhitelist is ZKPVerifierBase {
         super.setZKPRequest(requestId, request);
     }
 
-    /// @dev Submits a ZKP response and updates proof status
-    /// @param requestId The ID of the ZKP request
-    /// @param inputs The input data for the proof
-    /// @param a The first component of the proof
-    /// @param b The second component of the proof
-    /// @param c The third component of the proof
-    function submitZKPResponse(
-        uint64 requestId,
-        uint256[] memory inputs,
-        uint256[2] memory a,
-        uint256[2][2] memory b,
-        uint256[2] memory c
-    ) public virtual override {
-        ICircuitValidator validator = getZKPRequest(requestId).validator;
-        require(isWhitelistedValidator(validator), "Validator is not whitelisted");
-        super.submitZKPResponse(requestId, inputs, a, b, c);
-    }
-
     /// @dev Verifies a ZKP response without updating any proof status
     /// @param requestId The ID of the ZKP request
     /// @param inputs The public inputs for the proof
@@ -101,4 +83,17 @@ contract ValidatorWhitelist is ZKPVerifierBase {
     function _removeValidatorFromWhitelist(ICircuitValidator validator) internal {
         _getValidatorWhitelistStorage()._validatorWhitelist[validator] = false;
     }
+
+    function _getRequestIfCanBeVerified(
+        uint64 requestId
+    )
+        internal
+        view
+        virtual
+        override
+        onlyWhitelistedValidator(getZKPRequest(requestId).validator)
+        returns (IZKPVerifier.ZKPRequest storage)
+    {
+        return super._getRequestIfCanBeVerified(requestId);
+    }
 }

contracts/verifiers/ZKPVerifierBase.sol

@@ -98,11 +98,11 @@ abstract contract ZKPVerifierBase is IZKPVerifier, ContextUpgradeable {
         uint256[2] memory a,
         uint256[2][2] memory b,
         uint256[2] memory c
-    ) public virtual checkRequestExistence(requestId, true) {
+    ) public virtual {
         address sender = _msgSender();
         ZKPVerifierStorage storage $ = _getZKPVerifierStorage();
 
-        IZKPVerifier.ZKPRequest memory request = $._requests[requestId];
+        IZKPVerifier.ZKPRequest storage request = _getRequestIfCanBeVerified(requestId);
         ICircuitValidator.KeyToInputIndex[] memory keyToInpIdxs = request.validator.verify(
             inputs,
             a,
@@ -131,8 +131,9 @@ abstract contract ZKPVerifierBase is IZKPVerifier, ContextUpgradeable {
 
             address sender = _msgSender();
 
-            // TODO some internal method and storage location to save gas?
-            IZKPVerifier.ZKPRequest memory request = getZKPRequest(response.requestId);
+            IZKPVerifier.ZKPRequest storage request = _getRequestIfCanBeVerified(
+                response.requestId
+            );
             ICircuitValidator.Signal[] memory signals = request.validator.verifyV2(
                 response.zkProof,
                 request.data,
@@ -312,4 +313,16 @@ abstract contract ZKPVerifierBase is IZKPVerifier, ContextUpgradeable {
         ZKPVerifierStorage storage s = _getZKPVerifierStorage();
         s._requests[requestId] = request;
     }
+
+    function _getRequestIfCanBeVerified(
+        uint64 requestId
+    )
+        internal
+        view
+        virtual
+        checkRequestExistence(requestId, true)
+        returns (IZKPVerifier.ZKPRequest storage)
+    {
+        return _getZKPVerifierStorage()._requests[requestId];
+    }
 }

test/verifier/embedded-zkp-verifier.test.ts

@@ -10,7 +10,7 @@ import { CircuitId } from "@0xpolygonid/js-sdk";
 import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers";
 
 describe("Embedded ZKP Verifier", function () {
-  let verifier: any, sig: any;
+  let verifier: any, validator: any;
   let owner: Signer;
 
   const query = {
@@ -28,6 +28,9 @@ describe("Embedded ZKP Verifier", function () {
     claimPathNotExists: 0,
   };
 
+  const { inputs, pi_a, pi_b, pi_c } = prepareInputs(proofJson);
+  const metadatas = "0x";
+
   async function deployContractsFixture() {
     const deployHelper = await DeployHelper.initialize(null, true);
     [owner] = await ethers.getSigners();
@@ -42,15 +45,17 @@ describe("Embedded ZKP Verifier", function () {
       await verifierLib.getAddress(),
     );
 
-    const stub = await deployHelper.deployValidatorStub();
-    sig = stub;
+    validator = await deployHelper.deployValidatorStub();
   }
 
-  async function checkStorageFields(verifier: any, requestId: number) {
-    const fieldsToCheck = ["userID", "issuerID"];
-    for (const field of fieldsToCheck) {
-      const value = await verifier.getProofStorageField(await owner.getAddress(), requestId, field);
-      expect(value).to.be.greaterThan(0n);
+  async function checkStorageFields(verifier: any, requestId: number, storageFields: any[]) {
+    for (const field of storageFields) {
+      const value = await verifier.getProofStorageField(
+        await owner.getAddress(),
+        requestId,
+        field.name,
+      );
+      expect(value).to.be.equal(field.value);
     }
   }
 
@@ -61,15 +66,24 @@ describe("Embedded ZKP Verifier", function () {
   it("test submit response", async () => {
     await verifier.setZKPRequest(0, {
       metadata: "metadata",
-      validator: await sig.getAddress(),
+      validator: await validator.getAddress(),
       data: packValidatorParams(query),
     });
 
-    const { inputs, pi_a, pi_b, pi_c } = prepareInputs(proofJson);
-
     const tx = await verifier.submitZKPResponse(0, inputs, pi_a, pi_b, pi_c);
     const txRes = await tx.wait();
-    await checkStorageFields(verifier, 0);
+    const storageFields = [
+      {
+        name: "userID",
+        value: inputs[1],
+      },
+      {
+        name: "issuerID",
+        value: inputs[2],
+      },
+    ];
+
+    await checkStorageFields(verifier, 0, storageFields);
     const receipt = await ethers.provider.getTransactionReceipt(txRes.hash);
 
     // 2 events are emitted
@@ -85,7 +99,7 @@ describe("Embedded ZKP Verifier", function () {
     );
     expect(eventBeforeProofSubmit[0]).to.equal(0);
     expect(eventBeforeProofSubmit[1]).to.deep.equal(inputs.map((x) => BigInt(x)));
-    expect(eventBeforeProofSubmit[2]).to.equal(await sig.getAddress());
+    expect(eventBeforeProofSubmit[2]).to.equal(await validator.getAddress());
 
     const interfaceEventAfterProofSubmit = new ethers.Interface([
       "event AfterProofSubmit(uint64 requestId, uint256[] inputs, address validator)",
@@ -97,7 +111,7 @@ describe("Embedded ZKP Verifier", function () {
     );
     expect(eventAfterProofSubmit[0]).to.equal(0);
     expect(eventAfterProofSubmit[1]).to.deep.equal(inputs.map((x) => BigInt(x)));
-    expect(eventAfterProofSubmit[2]).to.equal(await sig.getAddress());
+    expect(eventAfterProofSubmit[2]).to.equal(await validator.getAddress());
 
     const ownerAddress = await owner.getAddress();
     const requestID = 0;
@@ -138,12 +152,10 @@ describe("Embedded ZKP Verifier", function () {
 
     await verifier.setZKPRequest(0, {
       metadata: "metadata",
-      validator: await sig.getAddress(),
+      validator: await validator.getAddress(),
       data: packValidatorParams(query),
     });
 
-    const { inputs, pi_a, pi_b, pi_c } = prepareInputs(proofJson);
-
     const zkProof = packZKProof(inputs, pi_a, pi_b, pi_c);
     const [signer] = await ethers.getSigners();
 
@@ -154,8 +166,6 @@ describe("Embedded ZKP Verifier", function () {
       ),
     );
 
-    const metadatas = "0x";
-
     const tx = await verifier.submitZKPResponseV2(
       [
         {
@@ -168,7 +178,18 @@ describe("Embedded ZKP Verifier", function () {
     );
 
     const txRes = await tx.wait();
-    await checkStorageFields(verifier, 0);
+
+    const storageFields = [
+      {
+        name: "userID",
+        value: 1n,
+      },
+      {
+        name: "issuerID",
+        value: 2n,
+      },
+    ];
+    await checkStorageFields(verifier, 0, storageFields);
 
     const receipt = await ethers.provider.getTransactionReceipt(txRes.hash);
 
@@ -219,7 +240,7 @@ describe("Embedded ZKP Verifier", function () {
     for (let i = 0; i < requestsCount; i++) {
       await verifier.setZKPRequest(i, {
         metadata: "metadataN" + i,
-        validator: await sig.getAddress(),
+        validator: await validator.getAddress(),
         data: "0x00",
       });
       const reqeustIdExists = await verifier.requestIdExists(i);