From 3bfbddaa941174c9101b03e60fe3daa5878acd42 Mon Sep 17 00:00:00 2001 From: Oleksandr Brezhniev Date: Tue, 29 Sep 2026 17:58:02 +0100 Subject: [PATCH] Use abi.encode for multiRequestId hash multiRequestId was keccak256(abi.encodePacked(requestIds, groupIds, sender)). With two dynamic uint256[] arrays the packed encoding is ambiguous, so e.g. ([1,2],[3]) and ([1],[2,3]) produced the same id (slither encode-packed-collision). Switch to abi.encode. Breaking for multiRequest creators: ids computed with the old formula now revert with MultiRequestIdNotValid. Existing multiRequests are unaffected, since the id is only checked in setMultiRequest. - UniversalVerifier VERSION 3.0.0 -> 3.1.0 - Tests use the local calculateMultiRequestId helper instead of js-sdk's - Add tests for legacy id rejection and split collision Co-Authored-By: Claude Opus 5.5 --- contracts/lib/VerifierLib.sol | 2 +- contracts/verifiers/UniversalVerifier.sol | 2 +- helpers/constants.ts | 2 +- .../integration-verifier.test.ts | 3 ++- test/utils/id-calculation-utils.ts | 5 +++- test/verifier/universal-verifier.test.ts | 3 ++- test/verifier/verifier.test.ts | 27 ++++++++++++++++++- 7 files changed, 37 insertions(+), 7 deletions(-) diff --git a/contracts/lib/VerifierLib.sol b/contracts/lib/VerifierLib.sol index 27ffd8684..02d36b319 100644 --- a/contracts/lib/VerifierLib.sol +++ b/contracts/lib/VerifierLib.sol @@ -749,7 +749,7 @@ library VerifierLib { address sender ) public checkMultiRequestExistence(self, multiRequest.multiRequestId, false) { uint256 expectedMultiRequestId = uint256( - keccak256(abi.encodePacked(multiRequest.requestIds, multiRequest.groupIds, sender)) + keccak256(abi.encode(multiRequest.requestIds, multiRequest.groupIds, sender)) ); if (expectedMultiRequestId != multiRequest.multiRequestId) { revert MultiRequestIdNotValid(expectedMultiRequestId, multiRequest.multiRequestId); diff --git a/contracts/verifiers/UniversalVerifier.sol b/contracts/verifiers/UniversalVerifier.sol index e7455fa26..ae285d6e5 100644 --- a/contracts/verifiers/UniversalVerifier.sol +++ b/contracts/verifiers/UniversalVerifier.sol @@ -23,7 +23,7 @@ contract UniversalVerifier is /** * @dev Version of contract */ - string public constant VERSION = "3.0.0"; + string public constant VERSION = "3.1.0"; /** * @dev Event emitted upon submitting a request diff --git a/helpers/constants.ts b/helpers/constants.ts index 06079b940..247fa480c 100644 --- a/helpers/constants.ts +++ b/helpers/constants.ts @@ -169,7 +169,7 @@ export const contractsInfo = Object.freeze({ }, UNIVERSAL_VERIFIER: { name: "UniversalVerifier", - version: "3.0.0", + version: "3.1.0", unifiedAddress: "0x2B0D3f664A5EbbfBD76E6cbc2cA9A504a68d2F4F", create2Calldata: ethers.hexlify(ethers.toUtf8Bytes("iden3.create2.UniversalVerifier.v2")), verificationOpts: { diff --git a/test/integration-tests/integration-verifier.test.ts b/test/integration-tests/integration-verifier.test.ts index 2ccbb4769..1df103e89 100644 --- a/test/integration-tests/integration-verifier.test.ts +++ b/test/integration-tests/integration-verifier.test.ts @@ -11,7 +11,8 @@ import { packLinkedMultiQueryValidatorParams, packV3ValidatorParams, } from "../utils/validator-pack-utils"; -import { calculateGroupId, calculateMultiRequestId, CircuitId } from "@0xpolygonid/js-sdk"; +import { calculateGroupId, CircuitId } from "@0xpolygonid/js-sdk"; +import { calculateMultiRequestId } from "../utils/id-calculation-utils"; import { calculateQueryHashV3 } from "../utils/query-hash-utils"; import { chainIdInfoMap, contractsInfo, TEN_YEARS } from "../../helpers/constants"; import CredentialAtomicQueryV3ValidatorModule from "../../ignition/modules/deployEverythingBasicStrategy/credentialAtomicQueryV3Validator"; diff --git a/test/utils/id-calculation-utils.ts b/test/utils/id-calculation-utils.ts index 2dc1c932e..5c2c4fe03 100644 --- a/test/utils/id-calculation-utils.ts +++ b/test/utils/id-calculation-utils.ts @@ -27,7 +27,10 @@ export function calculateMultiRequestId( ): bigint { return BigInt( ethers.keccak256( - ethers.solidityPacked(["uint256[]", "uint256[]", "address"], [requestIds, groupIds, sender]), + ethers.AbiCoder.defaultAbiCoder().encode( + ["uint256[]", "uint256[]", "address"], + [requestIds, groupIds, sender], + ), ), ); } diff --git a/test/verifier/universal-verifier.test.ts b/test/verifier/universal-verifier.test.ts index b1db1e9a4..8b63263af 100644 --- a/test/verifier/universal-verifier.test.ts +++ b/test/verifier/universal-verifier.test.ts @@ -1,7 +1,8 @@ import { expect } from "chai"; import { packValidatorParams } from "../utils/validator-pack-utils"; import { AbiCoder, Block } from "ethers"; -import { byteEncoder, calculateMultiRequestId, CircuitId } from "@0xpolygonid/js-sdk"; +import { byteEncoder, CircuitId } from "@0xpolygonid/js-sdk"; +import { calculateMultiRequestId } from "../utils/id-calculation-utils"; import { chainIdInfoMap, contractsInfo } from "../../helpers/constants"; import { beforeEach } from "mocha"; import { network } from "hardhat"; diff --git a/test/verifier/verifier.test.ts b/test/verifier/verifier.test.ts index 01d874f9c..b00d783c5 100644 --- a/test/verifier/verifier.test.ts +++ b/test/verifier/verifier.test.ts @@ -5,7 +5,8 @@ import { network } from "hardhat"; import { getChainId } from "../../helpers/helperUtils"; import StateModule from "../../ignition/modules/deployEverythingBasicStrategy/state"; import { Groth16VerifierStubModule } from "../../ignition/modules/deployEverythingBasicStrategy/testHelpers"; -import { calculateGroupId, calculateMultiRequestId, calculateRequestId } from "@0xpolygonid/js-sdk"; +import { calculateGroupId, calculateRequestId } from "@0xpolygonid/js-sdk"; +import { calculateMultiRequestId } from "../utils/id-calculation-utils"; const { ethers, ignition } = await network.connect(); @@ -592,6 +593,30 @@ describe("Verifier tests", function () { .withArgs(multiRequest.multiRequestId, 1); }); + it("setMultiRequest: should reject legacy abi.encodePacked multi request id", async function () { + const legacyMultiRequestId = BigInt( + ethers.keccak256( + ethers.solidityPacked( + ["uint256[]", "uint256[]", "address"], + [multiRequest.requestIds, multiRequest.groupIds, signerAddress], + ), + ), + ); + expect(legacyMultiRequestId).to.not.equal(multiRequest.multiRequestId); + await expect( + verifier.setMultiRequest({ ...multiRequest, multiRequestId: legacyMultiRequestId }), + ) + .to.be.revertedWithCustomError(verifierLib, "MultiRequestIdNotValid") + .withArgs(multiRequest.multiRequestId, legacyMultiRequestId); + }); + + it("calculateMultiRequestId: different requestIds/groupIds splits give different ids", async function () { + // With abi.encodePacked both splits packed to the same bytes and collided. + expect(calculateMultiRequestId([1n, 2n], [3n], signerAddress)).to.not.equal( + calculateMultiRequestId([1n], [2n, 3n], signerAddress), + ); + }); + it("setMultiRequest: requestIds and groupIds should exist", async function () { const multiRequest2 = { multiRequestId: calculateMultiRequestId([2n], [], signerAddress),