From 1a2407a72814ee62a689af8a7c67e99eeaf7310a Mon Sep 17 00:00:00 2001 From: yushihang Date: Sun, 12 Jan 2025 09:51:16 +0800 Subject: [PATCH 1/4] add zero address checking --- contracts/cross-chain/CrossChainProofValidator.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/cross-chain/CrossChainProofValidator.sol b/contracts/cross-chain/CrossChainProofValidator.sol index 09f271f2..5e0829e5 100644 --- a/contracts/cross-chain/CrossChainProofValidator.sol +++ b/contracts/cross-chain/CrossChainProofValidator.sol @@ -40,6 +40,7 @@ contract CrossChainProofValidator is Ownable, EIP712, ICrossChainProofValidator string memory signatureVersion, address oracleSigningAddress ) EIP712(domainName, signatureVersion) Ownable(msg.sender) { + require(oracleSigningAddress != address(0), "Oracle signing address should not be zero"); bytes32 hashedName = keccak256(bytes(domainName)); bytes32 hashedVersion = keccak256(bytes(signatureVersion)); uint256 chainId = 0; @@ -63,6 +64,7 @@ contract CrossChainProofValidator is Ownable, EIP712, ICrossChainProofValidator * @param oracleSigningAddress The new oracle signing address **/ function setOracleSigningAddress(address oracleSigningAddress) public onlyOwner { + require(oracleSigningAddress != address(0), "Oracle signing address should not be zero"); _oracleSigningAddress = oracleSigningAddress; } From 8f629949869b82b364142a2ccf6b5481cbf551d5 Mon Sep 17 00:00:00 2001 From: yushihang Date: Fri, 31 Jan 2025 17:04:03 +0800 Subject: [PATCH 2/4] https://github.com/iden3/contracts/pull/327 create internal method instead _checkOracleSigningAddress and use custom error error OracleSigningAddressShouldNotBeZero() in it? --- contracts/cross-chain/CrossChainProofValidator.sol | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/contracts/cross-chain/CrossChainProofValidator.sol b/contracts/cross-chain/CrossChainProofValidator.sol index 5e0829e5..163c3ce8 100644 --- a/contracts/cross-chain/CrossChainProofValidator.sol +++ b/contracts/cross-chain/CrossChainProofValidator.sol @@ -13,6 +13,8 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; contract CrossChainProofValidator is Ownable, EIP712, ICrossChainProofValidator { using ECDSA for bytes32; + error OracleSigningAddressShouldNotBeZero(); + bytes32 public constant TYPE_HASH = keccak256( "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" @@ -35,12 +37,18 @@ contract CrossChainProofValidator is Ownable, EIP712, ICrossChainProofValidator address private _oracleSigningAddress; + function _checkOracleSigningAddress(address oracleSigningAddress) internal pure { + if (oracleSigningAddress == address(0)) { + revert OracleSigningAddressShouldNotBeZero(); + } + } + constructor( string memory domainName, string memory signatureVersion, address oracleSigningAddress ) EIP712(domainName, signatureVersion) Ownable(msg.sender) { - require(oracleSigningAddress != address(0), "Oracle signing address should not be zero"); + _checkOracleSigningAddress(oracleSigningAddress); bytes32 hashedName = keccak256(bytes(domainName)); bytes32 hashedVersion = keccak256(bytes(signatureVersion)); uint256 chainId = 0; @@ -64,7 +72,7 @@ contract CrossChainProofValidator is Ownable, EIP712, ICrossChainProofValidator * @param oracleSigningAddress The new oracle signing address **/ function setOracleSigningAddress(address oracleSigningAddress) public onlyOwner { - require(oracleSigningAddress != address(0), "Oracle signing address should not be zero"); + _checkOracleSigningAddress(oracleSigningAddress); _oracleSigningAddress = oracleSigningAddress; } From dd3bc975f34a5dc983b41c85cff500e8b1989f3a Mon Sep 17 00:00:00 2001 From: yushihang Date: Mon, 3 Feb 2025 17:49:53 +0800 Subject: [PATCH 3/4] chore: bump version to 2.5.7 and fix test assertions - Bump package version from 2.5.6 to 2.5.7 - Update test assertions in cross-chain-proof-validator.test.ts to properly handle custom Solidity errors using revertedWithCustomError Technical changes: - Update package.json version - Replace revertedWith with revertedWithCustomError in test cases --- contracts/package.json | 2 +- .../cross-chain-proof-validator.test.ts | 47 ++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/contracts/package.json b/contracts/package.json index 42b94bfa..dfbf225c 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,7 +1,7 @@ { "name": "@iden3/contracts", "description": "Smart Contract library for Solidity", - "version": "2.5.6", + "version": "2.5.7", "files": [ "**/*.sol", "/build/contracts/*.json", diff --git a/test/cross-chain/cross-chain-proof-validator.test.ts b/test/cross-chain/cross-chain-proof-validator.test.ts index 7f66f0ba..b3ec34ea 100644 --- a/test/cross-chain/cross-chain-proof-validator.test.ts +++ b/test/cross-chain/cross-chain-proof-validator.test.ts @@ -6,7 +6,7 @@ import { } from "../utils/packData"; import { expect } from "chai"; import { DeployHelper } from "../../helpers/DeployHelper"; -import { Contract } from "ethers"; +import { Contract, ZeroAddress } from "ethers"; import { ethers } from "hardhat"; import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers"; @@ -180,3 +180,48 @@ describe("State Cross Chain", function () { ); }); }); + +describe("Oracle Signing Address Validation", function () { + let CrossChainProofValidatorFactory; + let crossChainProofValidator; + let otherAccount; + const oracleSigningAddress = "0x1234567890123456789012345678901234567890"; + + before(async function () { + [, otherAccount] = await ethers.getSigners(); + CrossChainProofValidatorFactory = await ethers.getContractFactory("CrossChainProofValidator"); + }); + + it("should deploy with correct parameters", async function () { + crossChainProofValidator = await CrossChainProofValidatorFactory.deploy( + "StateInfo", + "1", + oracleSigningAddress, + ); + await crossChainProofValidator.waitForDeployment(); + expect(await crossChainProofValidator.getOracleSigningAddress()).to.equal(oracleSigningAddress); + }); + + it("should revert when deploying with zero oracle signing address", async function () { + await expect( + CrossChainProofValidatorFactory.deploy("TestDomain", "1", ZeroAddress), + ).to.be.revertedWithCustomError( + CrossChainProofValidatorFactory, + "OracleSigningAddressShouldNotBeZero", + ); + }); + + it("should set a new oracle signing address", async function () { + await crossChainProofValidator.setOracleSigningAddress(otherAccount.address); + expect(await crossChainProofValidator.getOracleSigningAddress()).to.equal(otherAccount.address); + }); + + it("should revert when setting oracle signing address to zero", async function () { + await expect( + crossChainProofValidator.setOracleSigningAddress(ZeroAddress), + ).to.be.revertedWithCustomError( + crossChainProofValidator, + "OracleSigningAddressShouldNotBeZero", + ); + }); +}); From 827ffe48d1c787e6823ced524c9638ff5bb23596 Mon Sep 17 00:00:00 2001 From: yushihang Date: Mon, 3 Feb 2025 18:05:37 +0800 Subject: [PATCH 4/4] add logger for contract deployment result --- .../cross-chain-proof-validator.test.ts | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/cross-chain/cross-chain-proof-validator.test.ts b/test/cross-chain/cross-chain-proof-validator.test.ts index b3ec34ea..e6df7592 100644 --- a/test/cross-chain/cross-chain-proof-validator.test.ts +++ b/test/cross-chain/cross-chain-proof-validator.test.ts @@ -1,3 +1,4 @@ +import { Logger } from "../../helpers/helperUtils"; import { GlobalStateMessage, IdentityStateMessage, @@ -186,10 +187,20 @@ describe("Oracle Signing Address Validation", function () { let crossChainProofValidator; let otherAccount; const oracleSigningAddress = "0x1234567890123456789012345678901234567890"; + const contractName = "CrossChainProofValidator"; before(async function () { - [, otherAccount] = await ethers.getSigners(); - CrossChainProofValidatorFactory = await ethers.getContractFactory("CrossChainProofValidator"); + [otherAccount] = await ethers.getSigners(); + CrossChainProofValidatorFactory = await ethers.getContractFactory(contractName); + }); + + it("should revert when deploying with zero oracle signing address", async function () { + await expect( + CrossChainProofValidatorFactory.deploy("StateInfo", "1", ZeroAddress), + ).to.be.revertedWithCustomError( + CrossChainProofValidatorFactory, + "OracleSigningAddressShouldNotBeZero", + ); }); it("should deploy with correct parameters", async function () { @@ -199,18 +210,10 @@ describe("Oracle Signing Address Validation", function () { oracleSigningAddress, ); await crossChainProofValidator.waitForDeployment(); + Logger.success(`${contractName} deployed to: ${await crossChainProofValidator.getAddress()}`); expect(await crossChainProofValidator.getOracleSigningAddress()).to.equal(oracleSigningAddress); }); - it("should revert when deploying with zero oracle signing address", async function () { - await expect( - CrossChainProofValidatorFactory.deploy("TestDomain", "1", ZeroAddress), - ).to.be.revertedWithCustomError( - CrossChainProofValidatorFactory, - "OracleSigningAddressShouldNotBeZero", - ); - }); - it("should set a new oracle signing address", async function () { await crossChainProofValidator.setOracleSigningAddress(otherAccount.address); expect(await crossChainProofValidator.getOracleSigningAddress()).to.equal(otherAccount.address);