From 0e0e8afb474aabaafca1cb0da8b158671fe53fdf Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Mon, 30 Dec 2024 12:11:39 -0500 Subject: [PATCH 1/6] update WorldID interface --- contracts/src/PBHEntryPointImplV1.sol | 10 ++--- contracts/src/interfaces/IPBHEntryPoint.sol | 4 +- contracts/src/interfaces/IWorldID.sol | 38 +++++++++++++++++++ .../test/PBHEntryPointConstruction.t.sol | 4 +- contracts/test/Setup.sol | 4 +- contracts/test/mocks/MockWorldIDGroups.sol | 4 +- 6 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 contracts/src/interfaces/IWorldID.sol diff --git a/contracts/src/PBHEntryPointImplV1.sol b/contracts/src/PBHEntryPointImplV1.sol index 4db74d53..9757a504 100644 --- a/contracts/src/PBHEntryPointImplV1.sol +++ b/contracts/src/PBHEntryPointImplV1.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.28; -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; +import {IWorldID} from "./interfaces/IWorldID.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {IPBHEntryPoint} from "./interfaces/IPBHEntryPoint.sol"; import {IMulticall3} from "./interfaces/IMulticall3.sol"; @@ -67,7 +67,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { ////////////////////////////////////////////////////////////////////////////// event PBHEntryPointImplInitialized( - IWorldIDGroups indexed worldId, IEntryPoint indexed entryPoint, uint8 indexed numPbhPerMonth, address multicall3 + IWorldID indexed worldId, IEntryPoint indexed entryPoint, uint8 indexed numPbhPerMonth, address multicall3 ); /// @notice Emitted once for each successful PBH verification. @@ -99,7 +99,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { uint256 internal constant _GROUP_ID = 1; /// @dev The World ID instance that will be used for verifying proofs - IWorldIDGroups public worldId; + IWorldID public worldId; /// @dev The EntryPoint where Aggregated PBH Bundles will be proxied to. IEntryPoint public entryPoint; @@ -141,7 +141,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @param _numPbhPerMonth The number of allowed PBH transactions per month. /// /// @custom:reverts string If called more than once at the same initialisation number. - function initialize(IWorldIDGroups _worldId, IEntryPoint _entryPoint, uint8 _numPbhPerMonth, address _multicall3) + function initialize(IWorldID _worldId, IEntryPoint _entryPoint, uint8 _numPbhPerMonth, address _multicall3) external reinitializer(1) { @@ -271,7 +271,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @notice Sets the World ID instance that will be used for verifying proofs. /// @param _worldId The World ID instance that will be used for verifying proofs. function setWorldId(address _worldId) external virtual onlyOwner onlyProxy onlyInitialized { - worldId = IWorldIDGroups(_worldId); + worldId = IWorldID(_worldId); emit WorldIdSet(_worldId); } } diff --git a/contracts/src/interfaces/IPBHEntryPoint.sol b/contracts/src/interfaces/IPBHEntryPoint.sol index 5f30b032..934f8046 100644 --- a/contracts/src/interfaces/IPBHEntryPoint.sol +++ b/contracts/src/interfaces/IPBHEntryPoint.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.28; -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; +import {IWorldID} from "./IWorldID.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {IMulticall3} from "./IMulticall3.sol"; @@ -25,7 +25,7 @@ interface IPBHEntryPoint { function pbhMulticall(IMulticall3.Call3[] calldata calls, PBHPayload calldata pbhPayload) external; - function initialize(IWorldIDGroups worldId, IEntryPoint entryPoint, uint8 _numPbhPerMonth, address _multicall3) + function initialize(IWorldID worldId, IEntryPoint entryPoint, uint8 _numPbhPerMonth, address _multicall3) external; function validateSignaturesCallback(bytes32 hashedOps) external view; diff --git a/contracts/src/interfaces/IWorldID.sol b/contracts/src/interfaces/IWorldID.sol new file mode 100644 index 00000000..f10453ad --- /dev/null +++ b/contracts/src/interfaces/IWorldID.sol @@ -0,0 +1,38 @@ +//SPDX-License-Identifier: MIT +pragma solidity ^0.8.21; + +interface IWorldID { + /////////////////////////////////////////////////////////////////////////////// + /// ERRORS /// + /////////////////////////////////////////////////////////////////////////////// + + /// @notice Thrown when attempting to validate a root that has expired. + error ExpiredRoot(); + + /// @notice Thrown when attempting to validate a root that has yet to be added to the root + /// history. + error NonExistentRoot(); + + /// @notice Verifies a WorldID zero knowledge proof. + /// @dev Note that a double-signaling check is not included here, and should be carried by the + /// caller. + /// @dev It is highly recommended that the implementation is restricted to `view` if possible. + /// + /// @param groupId The group identifier for the group to verify a proof for. + /// @param root The of the Merkle tree + /// @param signalHash A keccak256 hash of the Semaphore signal + /// @param nullifierHash The nullifier hash + /// @param externalNullifierHash A keccak256 hash of the external nullifier + /// @param proof The zero-knowledge proof + /// + /// @custom:reverts string If the `proof` is invalid. + /// @custom:reverts NoSuchGroup If the provided `groupId` references a group that does not exist. + function verifyProof( + uint256 root, + uint256 groupId, + uint256 signalHash, + uint256 nullifierHash, + uint256 externalNullifierHash, + uint256[8] calldata proof + ) external view; +} diff --git a/contracts/test/PBHEntryPointConstruction.t.sol b/contracts/test/PBHEntryPointConstruction.t.sol index 1a6f885d..540af0af 100644 --- a/contracts/test/PBHEntryPointConstruction.t.sol +++ b/contracts/test/PBHEntryPointConstruction.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.21; import {Setup} from "./Setup.sol"; -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; +import {IWorldID} from "../src/interfaces/IWorldID.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {PBHEntryPointImplV1} from "../src/PBHEntryPointImplV1.sol"; import {IPBHEntryPoint} from "../src/interfaces/IPBHEntryPoint.sol"; @@ -28,7 +28,7 @@ contract PBHEntryPointConstruction is Setup { } /// @notice Tests that it is possible to properly construct and initialise a router. - function testCanConstructRouterWithDelegate(IWorldIDGroups dummy, IEntryPoint entryPoint) public { + function testCanConstructRouterWithDelegate(IWorldID dummy, IEntryPoint entryPoint) public { // Setup vm.expectEmit(true, true, true, true); emit Initialized(1); diff --git a/contracts/test/Setup.sol b/contracts/test/Setup.sol index 3639ed38..0ff412fc 100644 --- a/contracts/test/Setup.sol +++ b/contracts/test/Setup.sol @@ -8,7 +8,7 @@ import {IPBHEntryPoint} from "../src/interfaces/IPBHEntryPoint.sol"; import {PBHSignatureAggregator} from "../src/PBHSignatureAggregator.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {IAggregator} from "@account-abstraction/contracts/interfaces/IAggregator.sol"; -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; +import {IWorldID} from "../src/interfaces/IWorldID.sol"; import {IAccount} from "@account-abstraction/contracts/interfaces/IAccount.sol"; import {MockAccount} from "./mocks/MockAccount.sol"; import {PBHEntryPointImplV1} from "../src/PBHEntryPointImplV1.sol"; @@ -74,7 +74,7 @@ contract Setup is Test { /// /// @param initialGroupAddress The initial group's identity manager. /// @param initialEntryPoint The initial entry point. - function deployPBHEntryPoint(IWorldIDGroups initialGroupAddress, IEntryPoint initialEntryPoint) public { + function deployPBHEntryPoint(IWorldID initialGroupAddress, IEntryPoint initialEntryPoint) public { pbhEntryPointImpl = address(new PBHEntryPointImplV1()); bytes memory initCallData = diff --git a/contracts/test/mocks/MockWorldIDGroups.sol b/contracts/test/mocks/MockWorldIDGroups.sol index 52b52e3a..5daa82aa 100644 --- a/contracts/test/mocks/MockWorldIDGroups.sol +++ b/contracts/test/mocks/MockWorldIDGroups.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; +import {IWorldID} from "../../src/interfaces/IWorldID.sol"; -contract MockWorldIDGroups is IWorldIDGroups { +contract MockWorldIDGroups is IWorldID { bool public verifyProofSuccess = true; event VerifyProofCalled( From ba30b9960e0609b4607dd733623ac59212fe1011 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Mon, 30 Dec 2024 12:34:25 -0500 Subject: [PATCH 2/6] wip --- contracts/src/PBHEntryPointImplV1.sol | 134 +++++--- .../test/PBHEntryPointConstruction.t.sol | 18 +- contracts/test/PBHEntryPointImplV1.t.sol | 140 ++++++++ .../PBHEntryPointOwnershipManagement.t.sol | 110 +++++-- contracts/test/PBHExternalNullifier.t.sol | 151 +++++++-- contracts/test/PBHSignatureAggregator.t.sol | 309 ++++++++++++++---- contracts/test/PBHVerifier.t.sol | 71 ++-- contracts/test/{Setup.sol => TestSetup.sol} | 66 +++- contracts/test/TestUtils.sol | 32 +- 9 files changed, 804 insertions(+), 227 deletions(-) create mode 100644 contracts/test/PBHEntryPointImplV1.t.sol rename contracts/test/{Setup.sol => TestSetup.sol} (79%) diff --git a/contracts/src/PBHEntryPointImplV1.sol b/contracts/src/PBHEntryPointImplV1.sol index 9757a504..87eb44c0 100644 --- a/contracts/src/PBHEntryPointImplV1.sol +++ b/contracts/src/PBHEntryPointImplV1.sol @@ -67,7 +67,10 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { ////////////////////////////////////////////////////////////////////////////// event PBHEntryPointImplInitialized( - IWorldID indexed worldId, IEntryPoint indexed entryPoint, uint8 indexed numPbhPerMonth, address multicall3 + IWorldID indexed worldId, + IEntryPoint indexed entryPoint, + uint8 indexed numPbhPerMonth, + address multicall3 ); /// @notice Emitted once for each successful PBH verification. @@ -141,10 +144,12 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @param _numPbhPerMonth The number of allowed PBH transactions per month. /// /// @custom:reverts string If called more than once at the same initialisation number. - function initialize(IWorldID _worldId, IEntryPoint _entryPoint, uint8 _numPbhPerMonth, address _multicall3) - external - reinitializer(1) - { + function initialize( + IWorldID _worldId, + IEntryPoint _entryPoint, + uint8 _numPbhPerMonth, + address _multicall3 + ) external reinitializer(1) { // First, ensure that all of the parent contracts are initialised. __delegateInit(); @@ -155,7 +160,12 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { // Say that the contract is initialized. __setInitialized(); - emit PBHEntryPointImplInitialized(_worldId, _entryPoint, _numPbhPerMonth, _multicall3); + emit PBHEntryPointImplInitialized( + _worldId, + _entryPoint, + _numPbhPerMonth, + _multicall3 + ); } /// @notice Responsible for initialising all of the supertypes of this contract. @@ -172,6 +182,37 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// Functions /// ////////////////////////////////////////////////////////////////////////////// + /// @param pbhPayload The PBH payload containing the proof data. + function verifyPbh( + uint256 signalHash, + PBHPayload memory pbhPayload + ) public view virtual onlyInitialized onlyProxy { + // First, we make sure this nullifier has not been used before. + if (nullifierHashes[pbhPayload.nullifierHash]) { + revert InvalidNullifier(); + } + + // Verify the external nullifier + PBHExternalNullifier.verify( + pbhPayload.pbhExternalNullifier, + numPbhPerMonth + ); + + // If worldId address is set, proceed with on chain verification, + // otherwise assume verification has been done off chain by the builder. + if (address(worldId) != address(0)) { + // We now verify the provided proof is valid and the user is verified by World ID + worldId.verifyProof( + pbhPayload.root, + _GROUP_ID, + signalHash, + pbhPayload.nullifierHash, + pbhPayload.pbhExternalNullifier, + pbhPayload.proof + ); + } + } + /// Execute a batch of PackedUserOperation with Aggregators /// @param opsPerAggregator - The operations to execute, grouped by aggregator (or address(0) for no-aggregator accounts). /// @param beneficiary - The address to receive the fees. @@ -180,20 +221,31 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { address payable beneficiary ) external virtual onlyProxy onlyInitialized nonReentrant { for (uint256 i = 0; i < opsPerAggregator.length; ++i) { - bytes32 hashedOps = keccak256(abi.encode(opsPerAggregator[i].userOps)); + bytes32 hashedOps = keccak256( + abi.encode(opsPerAggregator[i].userOps) + ); assembly ("memory-safe") { - if gt(tload(hashedOps), 0) { revert(0, 0) } + if gt(tload(hashedOps), 0) { + revert(0, 0) + } tstore(hashedOps, hashedOps) } - PBHPayload[] memory pbhPayloads = abi.decode(opsPerAggregator[i].signature, (PBHPayload[])); + PBHPayload[] memory pbhPayloads = abi.decode( + opsPerAggregator[i].signature, + (PBHPayload[]) + ); for (uint256 j = 0; j < pbhPayloads.length; ++j) { address sender = opsPerAggregator[i].userOps[j].sender; // We now generate the signal hash from the sender, nonce, and calldata - uint256 signalHash = abi.encodePacked( - sender, opsPerAggregator[i].userOps[j].nonce, opsPerAggregator[i].userOps[j].callData - ).hashToField(); + uint256 signalHash = abi + .encodePacked( + sender, + opsPerAggregator[i].userOps[j].nonce, + opsPerAggregator[i].userOps[j].callData + ) + .hashToField(); verifyPbh(signalHash, pbhPayloads[j]); nullifierHashes[pbhPayloads[j].nullifierHash] = true; @@ -206,19 +258,20 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @notice Validates the hashed operations is the same as the hash transiently stored. /// @param hashedOps The hashed operations to validate. - function validateSignaturesCallback(bytes32 hashedOps) external view virtual onlyProxy onlyInitialized { + function validateSignaturesCallback( + bytes32 hashedOps + ) external view virtual onlyProxy onlyInitialized { assembly ("memory-safe") { - if iszero(eq(tload(hashedOps), hashedOps)) { revert(0, 0) } + if iszero(eq(tload(hashedOps), hashedOps)) { + revert(0, 0) + } } } - function pbhMulticall(IMulticall3.Call3[] calldata calls, PBHPayload calldata pbhPayload) - external - virtual - onlyProxy - onlyInitialized - nonReentrant - { + function pbhMulticall( + IMulticall3.Call3[] calldata calls, + PBHPayload calldata pbhPayload + ) external virtual onlyProxy onlyInitialized nonReentrant { uint256 signalHash = abi.encode(msg.sender, calls).hashToField(); verifyPbh(signalHash, pbhPayload); @@ -229,40 +282,11 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { emit PBH(msg.sender, pbhPayload); } - /// @param pbhPayload The PBH payload containing the proof data. - function verifyPbh(uint256 signalHash, PBHPayload memory pbhPayload) - public - view - virtual - onlyInitialized - onlyProxy - { - // First, we make sure this nullifier has not been used before. - if (nullifierHashes[pbhPayload.nullifierHash]) { - revert InvalidNullifier(); - } - - // Verify the external nullifier - PBHExternalNullifier.verify(pbhPayload.pbhExternalNullifier, numPbhPerMonth); - - // If worldId address is set, proceed with on chain verification, - // otherwise assume verification has been done off chain by the builder. - if (address(worldId) != address(0)) { - // We now verify the provided proof is valid and the user is verified by World ID - worldId.verifyProof( - pbhPayload.root, - _GROUP_ID, - signalHash, - pbhPayload.nullifierHash, - pbhPayload.pbhExternalNullifier, - pbhPayload.proof - ); - } - } - /// @notice Sets the number of PBH transactions allowed per month. /// @param _numPbhPerMonth The number of allowed PBH transactions per month. - function setNumPbhPerMonth(uint8 _numPbhPerMonth) external virtual onlyOwner onlyProxy onlyInitialized { + function setNumPbhPerMonth( + uint8 _numPbhPerMonth + ) external virtual onlyOwner onlyProxy onlyInitialized { numPbhPerMonth = _numPbhPerMonth; emit NumPbhPerMonthSet(_numPbhPerMonth); } @@ -270,7 +294,9 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @dev If the World ID address is set to 0, then it is assumed that verification will take place off chain. /// @notice Sets the World ID instance that will be used for verifying proofs. /// @param _worldId The World ID instance that will be used for verifying proofs. - function setWorldId(address _worldId) external virtual onlyOwner onlyProxy onlyInitialized { + function setWorldId( + address _worldId + ) external virtual onlyOwner onlyProxy onlyInitialized { worldId = IWorldID(_worldId); emit WorldIdSet(_worldId); } diff --git a/contracts/test/PBHEntryPointConstruction.t.sol b/contracts/test/PBHEntryPointConstruction.t.sol index 540af0af..be98d4c4 100644 --- a/contracts/test/PBHEntryPointConstruction.t.sol +++ b/contracts/test/PBHEntryPointConstruction.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.21; -import {Setup} from "./Setup.sol"; +import {TestSetup} from "./TestSetup.sol"; import {IWorldID} from "../src/interfaces/IWorldID.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {PBHEntryPointImplV1} from "../src/PBHEntryPointImplV1.sol"; @@ -13,7 +13,7 @@ import {PBHEntryPoint} from "../src/PBHEntryPoint.sol"; /// @author Worldcoin /// @dev This test suite tests both the proxy and the functionality of the underlying implementation /// so as to test everything in the context of how it will be deployed. -contract PBHEntryPointConstruction is Setup { +contract PBHEntryPointConstruction is TestSetup { /// @notice Taken from Initializable.sol event Initialized(uint8 version); @@ -28,15 +28,23 @@ contract PBHEntryPointConstruction is Setup { } /// @notice Tests that it is possible to properly construct and initialise a router. - function testCanConstructRouterWithDelegate(IWorldID dummy, IEntryPoint entryPoint) public { + function testCanConstructRouterWithDelegate( + IWorldID dummy, + IEntryPoint entryPoint + ) public { // Setup vm.expectEmit(true, true, true, true); emit Initialized(1); pbhEntryPointImpl = address(new PBHEntryPointImplV1()); - bytes memory callData = abi.encodeCall(IPBHEntryPoint.initialize, (dummy, entryPoint, 30, this.MULTICALL3())); + bytes memory callData = abi.encodeCall( + IPBHEntryPoint.initialize, + (dummy, entryPoint, 30, this.MULTICALL3()) + ); // Test - pbhEntryPoint = IPBHEntryPoint(address(new PBHEntryPoint(address(pbhEntryPointImpl), callData))); + pbhEntryPoint = IPBHEntryPoint( + address(new PBHEntryPoint(address(pbhEntryPointImpl), callData)) + ); } } diff --git a/contracts/test/PBHEntryPointImplV1.t.sol b/contracts/test/PBHEntryPointImplV1.t.sol new file mode 100644 index 00000000..a0cce5ce --- /dev/null +++ b/contracts/test/PBHEntryPointImplV1.t.sol @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.21; + +import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; +import {MockWorldIDGroups} from "./mocks/MockWorldIDGroups.sol"; +import {CheckInitialized} from "@world-id-contracts/utils/CheckInitialized.sol"; +import {WorldIDImpl} from "@world-id-contracts/abstract/WorldIDImpl.sol"; +import {ByteHasher} from "@helpers/ByteHasher.sol"; +import {IPBHEntryPoint} from "../src/interfaces/IPBHEntryPoint.sol"; +import {PBHEntryPointImplV1} from "../src/PBHEntryPointImplV1.sol"; + +import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; +import "@helpers/PBHExternalNullifier.sol"; +import {TestSetup} from "./TestSetup.sol"; + +/// @title PBHVerifer Verify Tests +/// @notice Contains tests for the pbhVerifier +/// @author Worldcoin +contract PBHEntryPointImplV1Test is TestSetup { + using ByteHasher for bytes; + + event PBH(address indexed sender, IPBHEntryPoint.PBHPayload payload); + event NumPbhPerMonthSet(uint8 indexed numPbhPerMonth); + event WorldIdSet(address indexed worldId); + + /// @notice Test payload for the PBHVerifier + IPBHEntryPoint.PBHPayload public testPayload = + IPBHEntryPoint.PBHPayload({ + root: 1, + pbhExternalNullifier: getValidPBHExternalNullifier(), + nullifierHash: 1, + proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] + }); + + uint256 internal nonce = 1; + address internal sender = address(0x123); + bytes internal testCallData = hex"deadbeef"; + + // TODO: move this to test utils + function getValidPBHExternalNullifier() public view returns (uint256) { + uint8 month = uint8( + BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) + ); + uint16 year = uint16( + BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) + ); + return + PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 0, + month, + year + ); + } + + // TODO: + function test_verifyPbh() public { + uint256 signalHash = abi + .encodePacked(sender, nonce, testCallData) + .hashToField(); + + pbhEntryPoint.verifyPbh(signalHash, testPayload); + + // TODO: update to use mock work id + // Expect revert when proof verification fails + MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(false); + vm.expectRevert("Proof verification failed"); + pbhEntryPoint.verifyPbh(signalHash, testPayload); + + // Now expect success + MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(true); + pbhEntryPoint.verifyPbh(signalHash, testPayload); + } + + // TODO: + function test_verifyPbh_RevertIfInvalidNullifier() public {} + + /// @notice Test that setNumPBHPerMonth works as expected + function testSetNumPBHPerMonth() public { + uint256 signalHash = abi + .encodePacked(sender, nonce, testCallData) + .hashToField(); + + MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(true); + uint8 month = uint8( + BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) + ); + uint16 year = uint16( + BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) + ); + + // Value starts at 30, make sure 30 reverts. + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 30, + month, + year + ); + + testPayload.nullifierHash = 0; + vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); + pbhEntryPoint.verifyPbh(signalHash, testPayload); + + // Increase numPbhPerMonth from non owner, expect revert + vm.prank(address(123)); + vm.expectRevert("Ownable: caller is not the owner"); + pbhEntryPoint.setNumPbhPerMonth(40); + + // Increase numPbhPerMonth from owner + vm.prank(thisAddress); + vm.expectEmit(true, false, false, false); + emit NumPbhPerMonthSet(40); + pbhEntryPoint.setNumPbhPerMonth(40); + + // Try again, it should work + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 30, + month, + year + ); + testPayload.nullifierHash = 1; + pbhEntryPoint.verifyPbh(signalHash, testPayload); + } + + function testSetWorldId() public { + vm.expectEmit(true, false, false, false); + emit WorldIdSet(address(0x123)); + pbhEntryPoint.setWorldId(address(0x123)); + } + + function test_FailSetWorldId_NotOwner(address naughty) public { + if (naughty == thisAddress) { + return; + } + vm.prank(naughty); + vm.expectRevert("Ownable: caller is not the owner"); + pbhEntryPoint.setWorldId(address(0x123)); + } +} diff --git a/contracts/test/PBHEntryPointOwnershipManagement.t.sol b/contracts/test/PBHEntryPointOwnershipManagement.t.sol index 183225c6..2fca7699 100644 --- a/contracts/test/PBHEntryPointOwnershipManagement.t.sol +++ b/contracts/test/PBHEntryPointOwnershipManagement.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.21; -import {Setup} from "./Setup.sol"; +import {TestSetup} from "./TestSetup.sol"; import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; import {CheckInitialized} from "@world-id-contracts/utils/CheckInitialized.sol"; @@ -15,7 +15,7 @@ import {WorldIDImpl} from "@world-id-contracts/abstract/WorldIDImpl.sol"; /// @author Worldcoin /// @dev This test suite tests both the proxy and the functionality of the underlying implementation /// so as to test everything in the context of how it will be deployed. -contract PBHVerifierRouting is Setup { +contract PBHVerifierRouting is TestSetup { address internal pbhEntryPointAddress; function setUp() public override { @@ -24,7 +24,10 @@ contract PBHVerifierRouting is Setup { } /// @notice Taken from OwnableUpgradable.sol - event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); + event OwnershipTransferred( + address indexed previousOwner, + address indexed newOwner + ); /// @notice Checks that it is possible to get the owner, and that the owner is correctly /// initialised. @@ -41,32 +44,75 @@ contract PBHVerifierRouting is Setup { function testTransferOwner(address newOwner) public { // Setup vm.assume(newOwner != nullAddress); - bytes memory transferCallData = abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (newOwner)); - bytes memory ownerCallData = abi.encodeCall(OwnableUpgradeable.owner, ()); - bytes memory pendingOwnerCallData = abi.encodeCall(Ownable2StepUpgradeable.pendingOwner, ()); - bytes memory acceptOwnerCallData = abi.encodeCall(Ownable2StepUpgradeable.acceptOwnership, ()); + bytes memory transferCallData = abi.encodeCall( + Ownable2StepUpgradeable.transferOwnership, + (newOwner) + ); + bytes memory ownerCallData = abi.encodeCall( + OwnableUpgradeable.owner, + () + ); + bytes memory pendingOwnerCallData = abi.encodeCall( + Ownable2StepUpgradeable.pendingOwner, + () + ); + bytes memory acceptOwnerCallData = abi.encodeCall( + Ownable2StepUpgradeable.acceptOwnership, + () + ); // Test - assertCallSucceedsOn(pbhEntryPointAddress, transferCallData, new bytes(0x0)); - assertCallSucceedsOn(pbhEntryPointAddress, pendingOwnerCallData, abi.encode(newOwner)); - assertCallSucceedsOn(pbhEntryPointAddress, ownerCallData, abi.encode(thisAddress)); + assertCallSucceedsOn( + pbhEntryPointAddress, + transferCallData, + new bytes(0x0) + ); + assertCallSucceedsOn( + pbhEntryPointAddress, + pendingOwnerCallData, + abi.encode(newOwner) + ); + assertCallSucceedsOn( + pbhEntryPointAddress, + ownerCallData, + abi.encode(thisAddress) + ); vm.expectEmit(true, true, true, true); emit OwnershipTransferred(thisAddress, newOwner); vm.prank(newOwner); - assertCallSucceedsOn(pbhEntryPointAddress, acceptOwnerCallData, new bytes(0x0)); - assertCallSucceedsOn(pbhEntryPointAddress, ownerCallData, abi.encode(newOwner)); + assertCallSucceedsOn( + pbhEntryPointAddress, + acceptOwnerCallData, + new bytes(0x0) + ); + assertCallSucceedsOn( + pbhEntryPointAddress, + ownerCallData, + abi.encode(newOwner) + ); } /// @notice Tests that only the pending owner can accept the ownership transfer. - function testCannotAcceptOwnershipAsNonPendingOwner(address newOwner, address notNewOwner) public { + function testCannotAcceptOwnershipAsNonPendingOwner( + address newOwner, + address notNewOwner + ) public { // Setup vm.assume(newOwner != nullAddress); vm.assume(notNewOwner != newOwner); - bytes memory callData = abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (newOwner)); - bytes memory acceptCallData = abi.encodeCall(Ownable2StepUpgradeable.acceptOwnership, ()); - bytes memory expectedError = encodeStringRevert("Ownable2Step: caller is not the new owner"); + bytes memory callData = abi.encodeCall( + Ownable2StepUpgradeable.transferOwnership, + (newOwner) + ); + bytes memory acceptCallData = abi.encodeCall( + Ownable2StepUpgradeable.acceptOwnership, + () + ); + bytes memory expectedError = encodeStringRevert( + "Ownable2Step: caller is not the new owner" + ); assertCallSucceedsOn(pbhEntryPointAddress, callData); vm.prank(notNewOwner); @@ -75,11 +121,19 @@ contract PBHVerifierRouting is Setup { } /// @notice Ensures that it is impossible to transfer ownership without being the owner. - function testCannotTransferOwnerIfNotOwner(address naughty, address newOwner) public { + function testCannotTransferOwnerIfNotOwner( + address naughty, + address newOwner + ) public { // Setup vm.assume(naughty != thisAddress && newOwner != nullAddress); - bytes memory callData = abi.encodeCall(OwnableUpgradeable.transferOwnership, (newOwner)); - bytes memory expectedReturn = encodeStringRevert("Ownable: caller is not the owner"); + bytes memory callData = abi.encodeCall( + OwnableUpgradeable.transferOwnership, + (newOwner) + ); + bytes memory expectedReturn = encodeStringRevert( + "Ownable: caller is not the owner" + ); vm.prank(naughty); // Test @@ -89,8 +143,13 @@ contract PBHVerifierRouting is Setup { /// @notice Tests that it is impossible to renounce ownership, even as the owner. function testCannotRenounceOwnershipAsOwner() public { // Setup - bytes memory renounceData = abi.encodeCall(OwnableUpgradeable.renounceOwnership, ()); - bytes memory errorData = abi.encodeWithSelector(WorldIDImpl.CannotRenounceOwnership.selector); + bytes memory renounceData = abi.encodeCall( + OwnableUpgradeable.renounceOwnership, + () + ); + bytes memory errorData = abi.encodeWithSelector( + WorldIDImpl.CannotRenounceOwnership.selector + ); // Test assertCallFailsOn(pbhEntryPointAddress, renounceData, errorData); @@ -100,8 +159,13 @@ contract PBHVerifierRouting is Setup { function testCannotRenounceOwnershipIfNotOwner(address naughty) public { // Setup vm.assume(naughty != thisAddress && naughty != nullAddress); - bytes memory callData = abi.encodeCall(OwnableUpgradeable.renounceOwnership, ()); - bytes memory returnData = encodeStringRevert("Ownable: caller is not the owner"); + bytes memory callData = abi.encodeCall( + OwnableUpgradeable.renounceOwnership, + () + ); + bytes memory returnData = encodeStringRevert( + "Ownable: caller is not the owner" + ); vm.prank(naughty); // Test diff --git a/contracts/test/PBHExternalNullifier.t.sol b/contracts/test/PBHExternalNullifier.t.sol index 7aa504de..76dd23a1 100644 --- a/contracts/test/PBHExternalNullifier.t.sol +++ b/contracts/test/PBHExternalNullifier.t.sol @@ -5,7 +5,9 @@ import "forge-std/Test.sol"; import "@helpers/PBHExternalNullifier.sol"; import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; -contract PBHExternalNullifierLibTest is Test { +contract PBHExternalNullifierTest is Test { + // TODO: dont hard code these, use fuzzing + uint8 constant VALID_VERSION = PBHExternalNullifier.V1; uint8 constant VALID_PBH_NONCE = 5; uint8 constant VALID_MONTH = 12; @@ -18,69 +20,132 @@ contract PBHExternalNullifierLibTest is Test { uint8 month = VALID_MONTH; uint16 year = VALID_YEAR; - uint256 encoded = PBHExternalNullifier.encode(version, pbhNonce, month, year); - (uint8 decodedVersion, uint8 decodedNonce, uint8 decodedMonth, uint16 decodedYear) = - PBHExternalNullifier.decode(encoded); - - assertEq(decodedVersion, version, "Decoded version should match the original"); - assertEq(decodedNonce, pbhNonce, "Decoded nonce should match the original"); - assertEq(decodedMonth, month, "Decoded month should match the original"); + uint256 encoded = PBHExternalNullifier.encode( + version, + pbhNonce, + month, + year + ); + ( + uint8 decodedVersion, + uint8 decodedNonce, + uint8 decodedMonth, + uint16 decodedYear + ) = PBHExternalNullifier.decode(encoded); + + assertEq( + decodedVersion, + version, + "Decoded version should match the original" + ); + assertEq( + decodedNonce, + pbhNonce, + "Decoded nonce should match the original" + ); + assertEq( + decodedMonth, + month, + "Decoded month should match the original" + ); assertEq(decodedYear, year, "Decoded year should match the original"); } function testEncodeInvalidMonth() public { uint8 invalidMonth = 13; - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); - PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, invalidMonth, VALID_YEAR); + vm.expectRevert( + PBHExternalNullifier.InvalidExternalNullifierMonth.selector + ); + PBHExternalNullifier.encode( + VALID_VERSION, + VALID_PBH_NONCE, + invalidMonth, + VALID_YEAR + ); } function testVerifyValidExternalNullifier() public { // Mock the current date to match VALID_YEAR and VALID_MONTH - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( + VALID_YEAR, + VALID_MONTH, + 1, + 0, + 0, + 0 + ); vm.warp(timestamp); - uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + uint256 encoded = PBHExternalNullifier.encode( + VALID_VERSION, + VALID_PBH_NONCE, + VALID_MONTH, + VALID_YEAR + ); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } function testVerifyInvalidYear() public { - uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( - 2023, // Mock the year to 2023 - VALID_MONTH, - 1, - 0, - 0, - 0 - ); + uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary + .timestampFromDateTime( + 2023, // Mock the year to 2023 + VALID_MONTH, + 1, + 0, + 0, + 0 + ); vm.warp(currentTimestamp); - uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + uint256 encoded = PBHExternalNullifier.encode( + VALID_VERSION, + VALID_PBH_NONCE, + VALID_MONTH, + VALID_YEAR + ); - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierYear.selector); + vm.expectRevert( + PBHExternalNullifier.InvalidExternalNullifierYear.selector + ); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } function testVerifyInvalidMonth() public { - uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( - VALID_YEAR, - 11, // Mock the month to November - 1, - 0, - 0, - 0 - ); + uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary + .timestampFromDateTime( + VALID_YEAR, + 11, // Mock the month to November + 1, + 0, + 0, + 0 + ); vm.warp(currentTimestamp); - uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + uint256 encoded = PBHExternalNullifier.encode( + VALID_VERSION, + VALID_PBH_NONCE, + VALID_MONTH, + VALID_YEAR + ); - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); + vm.expectRevert( + PBHExternalNullifier.InvalidExternalNullifierMonth.selector + ); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } function testVerifyInvalidPbhNonce() public { - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( + VALID_YEAR, + VALID_MONTH, + 1, + 0, + 0, + 0 + ); vm.warp(timestamp); uint256 encoded = PBHExternalNullifier.encode( @@ -95,12 +160,26 @@ contract PBHExternalNullifierLibTest is Test { } function testVerifyInvalidVersion() public { - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( + VALID_YEAR, + VALID_MONTH, + 1, + 0, + 0, + 0 + ); vm.warp(timestamp); - uint256 encoded = PBHExternalNullifier.encode(2, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + uint256 encoded = PBHExternalNullifier.encode( + 2, + VALID_PBH_NONCE, + VALID_MONTH, + VALID_YEAR + ); - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierVersion.selector); + vm.expectRevert( + PBHExternalNullifier.InvalidExternalNullifierVersion.selector + ); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } } diff --git a/contracts/test/PBHSignatureAggregator.t.sol b/contracts/test/PBHSignatureAggregator.t.sol index 648869b9..11724336 100644 --- a/contracts/test/PBHSignatureAggregator.t.sol +++ b/contracts/test/PBHSignatureAggregator.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.21; -import {Setup} from "./Setup.sol"; +import {TestSetup} from "./TestSetup.sol"; import {console} from "@forge-std/console.sol"; import {TestUtils} from "./TestUtils.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; @@ -11,7 +11,7 @@ import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; import "../src/helpers/PBHExternalNullifier.sol"; import {PBHSignatureAggregator} from "../src/PBHSignatureAggregator.sol"; -contract PBHSignatureAggregatorTest is TestUtils, Setup { +contract PBHSignatureAggregatorTest is TestUtils, TestSetup { function setUp() public override { super.setUp(); } @@ -20,7 +20,12 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { uint256 timestamp = block.timestamp; uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(timestamp)); uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(timestamp)); - uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 0, month, year); + uint256 encoded = PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 0, + month, + year + ); worldIDGroups.setVerifyProofSuccess(true); IPBHEntryPoint.PBHPayload memory proof0 = IPBHEntryPoint.PBHPayload({ @@ -41,17 +46,29 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { proofs[0] = abi.encode(proof0); proofs[1] = abi.encode(proof1); - PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, 1); - bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); + PackedUserOperation[] memory uoTestFixture = createUOTestData( + address(safe), + proofs, + 1 + ); + bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures( + uoTestFixture + ); - IEntryPoint.UserOpsPerAggregator[] memory userOpsPerAggregator = new IEntryPoint.UserOpsPerAggregator[](1); + IEntryPoint.UserOpsPerAggregator[] + memory userOpsPerAggregator = new IEntryPoint.UserOpsPerAggregator[]( + 1 + ); userOpsPerAggregator[0] = IEntryPoint.UserOpsPerAggregator({ aggregator: pbhAggregator, userOps: uoTestFixture, signature: aggregatedSignature }); - pbhEntryPoint.handleAggregatedOps(userOpsPerAggregator, payable(address(this))); + pbhEntryPoint.handleAggregatedOps( + userOpsPerAggregator, + payable(address(this)) + ); } function testAggregateSignatures( @@ -77,37 +94,121 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { bytes[] memory proofs = new bytes[](2); proofs[0] = abi.encode(proof); proofs[1] = abi.encode(proof); - PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, 1); - bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); - IPBHEntryPoint.PBHPayload[] memory decodedProofs = - abi.decode(aggregatedSignature, (IPBHEntryPoint.PBHPayload[])); + PackedUserOperation[] memory uoTestFixture = createUOTestData( + address(safe), + proofs, + 1 + ); + bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures( + uoTestFixture + ); + IPBHEntryPoint.PBHPayload[] memory decodedProofs = abi.decode( + aggregatedSignature, + (IPBHEntryPoint.PBHPayload[]) + ); assertEq(decodedProofs.length, 2, "Decoded proof length should be 1"); assertEq(decodedProofs[0].root, proof.root, "Root should match"); assertEq( - decodedProofs[0].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" - ); - assertEq(decodedProofs[0].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); - assertEq(decodedProofs[0].proof[0], proof.proof[0], "Proof should match"); - assertEq(decodedProofs[0].proof[1], proof.proof[1], "Proof should match"); - assertEq(decodedProofs[0].proof[2], proof.proof[2], "Proof should match"); - assertEq(decodedProofs[0].proof[3], proof.proof[3], "Proof should match"); - assertEq(decodedProofs[0].proof[4], proof.proof[4], "Proof should match"); - assertEq(decodedProofs[0].proof[5], proof.proof[5], "Proof should match"); - assertEq(decodedProofs[0].proof[6], proof.proof[6], "Proof should match"); - assertEq(decodedProofs[0].proof[7], proof.proof[7], "Proof should match"); + decodedProofs[0].pbhExternalNullifier, + proof.pbhExternalNullifier, + "PBH External Nullifier should match" + ); + assertEq( + decodedProofs[0].nullifierHash, + proof.nullifierHash, + "Nullifier Hash should match" + ); + assertEq( + decodedProofs[0].proof[0], + proof.proof[0], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[1], + proof.proof[1], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[2], + proof.proof[2], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[3], + proof.proof[3], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[4], + proof.proof[4], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[5], + proof.proof[5], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[6], + proof.proof[6], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[7], + proof.proof[7], + "Proof should match" + ); assertEq(decodedProofs[1].root, proof.root, "Root should match"); assertEq( - decodedProofs[1].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" - ); - assertEq(decodedProofs[1].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); - assertEq(decodedProofs[1].proof[0], proof.proof[0], "Proof should match"); - assertEq(decodedProofs[1].proof[1], proof.proof[1], "Proof should match"); - assertEq(decodedProofs[1].proof[2], proof.proof[2], "Proof should match"); - assertEq(decodedProofs[1].proof[3], proof.proof[3], "Proof should match"); - assertEq(decodedProofs[1].proof[4], proof.proof[4], "Proof should match"); - assertEq(decodedProofs[1].proof[5], proof.proof[5], "Proof should match"); - assertEq(decodedProofs[1].proof[6], proof.proof[6], "Proof should match"); - assertEq(decodedProofs[1].proof[7], proof.proof[7], "Proof should match"); + decodedProofs[1].pbhExternalNullifier, + proof.pbhExternalNullifier, + "PBH External Nullifier should match" + ); + assertEq( + decodedProofs[1].nullifierHash, + proof.nullifierHash, + "Nullifier Hash should match" + ); + assertEq( + decodedProofs[1].proof[0], + proof.proof[0], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[1], + proof.proof[1], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[2], + proof.proof[2], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[3], + proof.proof[3], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[4], + proof.proof[4], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[5], + proof.proof[5], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[6], + proof.proof[6], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[7], + proof.proof[7], + "Proof should match" + ); } function testAggregateSignatures_VariableThreshold( @@ -135,37 +236,121 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { bytes[] memory proofs = new bytes[](2); proofs[0] = abi.encode(proof); proofs[1] = abi.encode(proof); - PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, threshold); - bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); - IPBHEntryPoint.PBHPayload[] memory decodedProofs = - abi.decode(aggregatedSignature, (IPBHEntryPoint.PBHPayload[])); + PackedUserOperation[] memory uoTestFixture = createUOTestData( + address(safe), + proofs, + threshold + ); + bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures( + uoTestFixture + ); + IPBHEntryPoint.PBHPayload[] memory decodedProofs = abi.decode( + aggregatedSignature, + (IPBHEntryPoint.PBHPayload[]) + ); assertEq(decodedProofs.length, 2, "Decoded proof length should be 1"); assertEq(decodedProofs[0].root, proof.root, "Root should match"); assertEq( - decodedProofs[0].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" - ); - assertEq(decodedProofs[0].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); - assertEq(decodedProofs[0].proof[0], proof.proof[0], "Proof should match"); - assertEq(decodedProofs[0].proof[1], proof.proof[1], "Proof should match"); - assertEq(decodedProofs[0].proof[2], proof.proof[2], "Proof should match"); - assertEq(decodedProofs[0].proof[3], proof.proof[3], "Proof should match"); - assertEq(decodedProofs[0].proof[4], proof.proof[4], "Proof should match"); - assertEq(decodedProofs[0].proof[5], proof.proof[5], "Proof should match"); - assertEq(decodedProofs[0].proof[6], proof.proof[6], "Proof should match"); - assertEq(decodedProofs[0].proof[7], proof.proof[7], "Proof should match"); + decodedProofs[0].pbhExternalNullifier, + proof.pbhExternalNullifier, + "PBH External Nullifier should match" + ); + assertEq( + decodedProofs[0].nullifierHash, + proof.nullifierHash, + "Nullifier Hash should match" + ); + assertEq( + decodedProofs[0].proof[0], + proof.proof[0], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[1], + proof.proof[1], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[2], + proof.proof[2], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[3], + proof.proof[3], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[4], + proof.proof[4], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[5], + proof.proof[5], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[6], + proof.proof[6], + "Proof should match" + ); + assertEq( + decodedProofs[0].proof[7], + proof.proof[7], + "Proof should match" + ); assertEq(decodedProofs[1].root, proof.root, "Root should match"); assertEq( - decodedProofs[1].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" - ); - assertEq(decodedProofs[1].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); - assertEq(decodedProofs[1].proof[0], proof.proof[0], "Proof should match"); - assertEq(decodedProofs[1].proof[1], proof.proof[1], "Proof should match"); - assertEq(decodedProofs[1].proof[2], proof.proof[2], "Proof should match"); - assertEq(decodedProofs[1].proof[3], proof.proof[3], "Proof should match"); - assertEq(decodedProofs[1].proof[4], proof.proof[4], "Proof should match"); - assertEq(decodedProofs[1].proof[5], proof.proof[5], "Proof should match"); - assertEq(decodedProofs[1].proof[6], proof.proof[6], "Proof should match"); - assertEq(decodedProofs[1].proof[7], proof.proof[7], "Proof should match"); + decodedProofs[1].pbhExternalNullifier, + proof.pbhExternalNullifier, + "PBH External Nullifier should match" + ); + assertEq( + decodedProofs[1].nullifierHash, + proof.nullifierHash, + "Nullifier Hash should match" + ); + assertEq( + decodedProofs[1].proof[0], + proof.proof[0], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[1], + proof.proof[1], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[2], + proof.proof[2], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[3], + proof.proof[3], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[4], + proof.proof[4], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[5], + proof.proof[5], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[6], + proof.proof[6], + "Proof should match" + ); + assertEq( + decodedProofs[1].proof[7], + proof.proof[7], + "Proof should match" + ); } function testFailAggregateSignatures_InvalidSignatureLength() public { @@ -179,7 +364,11 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { bytes[] memory proofs = new bytes[](2); proofs[0] = abi.encode(proof); proofs[1] = abi.encode(proof); - PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, 1); + PackedUserOperation[] memory uoTestFixture = createUOTestData( + address(safe), + proofs, + 1 + ); uoTestFixture[0].signature = new bytes(12); vm.expectRevert(PBHSignatureAggregator.InvalidSignatureLength.selector); pbhAggregator.aggregateSignatures(uoTestFixture); diff --git a/contracts/test/PBHVerifier.t.sol b/contracts/test/PBHVerifier.t.sol index a94b3515..c481f117 100644 --- a/contracts/test/PBHVerifier.t.sol +++ b/contracts/test/PBHVerifier.t.sol @@ -11,12 +11,12 @@ import {PBHEntryPointImplV1} from "../src/PBHEntryPointImplV1.sol"; import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; import "@helpers/PBHExternalNullifier.sol"; -import {Setup} from "./Setup.sol"; +import {TestSetup} from "./TestSetup.sol"; /// @title PBHVerifer Verify Tests /// @notice Contains tests for the pbhVerifier /// @author Worldcoin -contract PBHVerifierTest is Setup { +contract PBHVerifierTest is TestSetup { using ByteHasher for bytes; event PBH(address indexed sender, IPBHEntryPoint.PBHPayload payload); @@ -24,29 +24,43 @@ contract PBHVerifierTest is Setup { event WorldIdSet(address indexed worldId); /// @notice Test payload for the PBHVerifier - IPBHEntryPoint.PBHPayload public testPayload = IPBHEntryPoint.PBHPayload({ - root: 1, - pbhExternalNullifier: getValidPBHExternalNullifier(), - nullifierHash: 1, - proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] - }); + IPBHEntryPoint.PBHPayload public testPayload = + IPBHEntryPoint.PBHPayload({ + root: 1, + pbhExternalNullifier: getValidPBHExternalNullifier(), + nullifierHash: 1, + proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] + }); uint256 internal nonce = 1; address internal sender = address(0x123); bytes internal testCallData = hex"deadbeef"; function getValidPBHExternalNullifier() public view returns (uint256) { - uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); - uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); - return PBHExternalNullifier.encode(PBHExternalNullifier.V1, 0, month, year); + uint8 month = uint8( + BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) + ); + uint16 year = uint16( + BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) + ); + return + PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 0, + month, + year + ); } - /// @notice Test that a valid proof is verified correctly. - function testverifyPbhSuccess() public { - uint256 signalHash = abi.encodePacked(sender, nonce, testCallData).hashToField(); + // TODO: + function test_verifyPbh() public { + uint256 signalHash = abi + .encodePacked(sender, nonce, testCallData) + .hashToField(); pbhEntryPoint.verifyPbh(signalHash, testPayload); + // TODO: update to use mock work id // Expect revert when proof verification fails MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(false); vm.expectRevert("Proof verification failed"); @@ -57,16 +71,30 @@ contract PBHVerifierTest is Setup { pbhEntryPoint.verifyPbh(signalHash, testPayload); } + // TODO: + function test_verifyPbh_RevertIfInvalidNullifier() public {} + /// @notice Test that setNumPBHPerMonth works as expected function testSetNumPBHPerMonth() public { - uint256 signalHash = abi.encodePacked(sender, nonce, testCallData).hashToField(); + uint256 signalHash = abi + .encodePacked(sender, nonce, testCallData) + .hashToField(); MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(true); - uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); - uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); + uint8 month = uint8( + BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) + ); + uint16 year = uint16( + BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) + ); // Value starts at 30, make sure 30 reverts. - testPayload.pbhExternalNullifier = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 30, month, year); + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 30, + month, + year + ); testPayload.nullifierHash = 0; vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); @@ -84,7 +112,12 @@ contract PBHVerifierTest is Setup { pbhEntryPoint.setNumPbhPerMonth(40); // Try again, it should work - testPayload.pbhExternalNullifier = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 30, month, year); + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( + PBHExternalNullifier.V1, + 30, + month, + year + ); testPayload.nullifierHash = 1; pbhEntryPoint.verifyPbh(signalHash, testPayload); } diff --git a/contracts/test/Setup.sol b/contracts/test/TestSetup.sol similarity index 79% rename from contracts/test/Setup.sol rename to contracts/test/TestSetup.sol index 0ff412fc..4d63ec29 100644 --- a/contracts/test/Setup.sol +++ b/contracts/test/TestSetup.sol @@ -18,13 +18,14 @@ import {PBHEntryPoint} from "../src/PBHEntryPoint.sol"; /// @author Worldcoin /// @dev This test suite tests both the proxy and the functionality of the underlying implementation /// so as to test everything in the context of how it will be deployed. -contract Setup is Test { +contract TestSetup is Test { /////////////////////////////////////////////////////////////////////////////// /// TEST DATA /// /////////////////////////////////////////////////////////////////////////////// /// @notice The 4337 Entry Point on Ethereum Mainnet. - IEntryPoint internal entryPoint = IEntryPoint(address(0x0000000071727De22E5E9d8BAf0edAc6f37da032)); + IEntryPoint internal entryPoint = + IEntryPoint(address(0x0000000071727De22E5E9d8BAf0edAc6f37da032)); /// @notice The PBHEntryPoint contract. IPBHEntryPoint public pbhEntryPoint; /// @notice The PBHSignatureAggregator contract. @@ -37,7 +38,8 @@ contract Setup is Test { address public pbhEntryPointImpl; address public immutable thisAddress = address(this); address public constant nullAddress = address(0); - address public constant MULTICALL3 = 0xcA11bde05977b3631167028862bE2a173976CA11; + address public constant MULTICALL3 = + 0xcA11bde05977b3631167028862bE2a173976CA11; /////////////////////////////////////////////////////////////////////////////// /// TEST ORCHESTRATION /// /////////////////////////////////////////////////////////////////////////////// @@ -74,14 +76,26 @@ contract Setup is Test { /// /// @param initialGroupAddress The initial group's identity manager. /// @param initialEntryPoint The initial entry point. - function deployPBHEntryPoint(IWorldID initialGroupAddress, IEntryPoint initialEntryPoint) public { + function deployPBHEntryPoint( + IWorldID initialGroupAddress, + IEntryPoint initialEntryPoint + ) public { pbhEntryPointImpl = address(new PBHEntryPointImplV1()); - bytes memory initCallData = - abi.encodeCall(PBHEntryPointImplV1.initialize, (initialGroupAddress, initialEntryPoint, 30, MULTICALL3)); + bytes memory initCallData = abi.encodeCall( + PBHEntryPointImplV1.initialize, + (initialGroupAddress, initialEntryPoint, 30, MULTICALL3) + ); vm.expectEmit(true, true, true, true); - emit PBHEntryPointImplV1.PBHEntryPointImplInitialized(initialGroupAddress, initialEntryPoint, 30, MULTICALL3); - pbhEntryPoint = IPBHEntryPoint(address(new PBHEntryPoint(pbhEntryPointImpl, initCallData))); + emit PBHEntryPointImplV1.PBHEntryPointImplInitialized( + initialGroupAddress, + initialEntryPoint, + 30, + MULTICALL3 + ); + pbhEntryPoint = IPBHEntryPoint( + address(new PBHEntryPoint(pbhEntryPointImpl, initCallData)) + ); } /// @notice Initializes a new PBHSignatureAggregator. @@ -92,7 +106,10 @@ contract Setup is Test { /// @notice Initializes a new safe account. /// @dev It is constructed in the globals. - function deploySafeAccount(address _pbhSignatureAggregator, uint256 threshold) public { + function deploySafeAccount( + address _pbhSignatureAggregator, + uint256 threshold + ) public { safe = new MockAccount(_pbhSignatureAggregator, threshold); } @@ -106,15 +123,22 @@ contract Setup is Test { /// @dev It is constructed in the globals. function makeUninitPBHEntryPoint() public { pbhEntryPointImpl = address(new PBHEntryPointImplV1()); - pbhEntryPoint = IPBHEntryPoint(address(new PBHEntryPoint(pbhEntryPointImpl, new bytes(0x0)))); + pbhEntryPoint = IPBHEntryPoint( + address(new PBHEntryPoint(pbhEntryPointImpl, new bytes(0x0))) + ); } + // TODO: remove these + /// @notice Asserts that making the external call using `callData` on `target` succeeds. /// /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. - function assertCallSucceedsOn(address target, bytes memory callData) public { - (bool status,) = target.call(callData); + function assertCallSucceedsOn( + address target, + bytes memory callData + ) public { + (bool status, ) = target.call(callData); assert(status); } @@ -123,7 +147,11 @@ contract Setup is Test { /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. /// @param expectedReturnData The expected return data from the function. - function assertCallSucceedsOn(address target, bytes memory callData, bytes memory expectedReturnData) public { + function assertCallSucceedsOn( + address target, + bytes memory callData, + bytes memory expectedReturnData + ) public { (bool status, bytes memory returnData) = target.call(callData); assert(status); assertEq(expectedReturnData, returnData); @@ -134,7 +162,7 @@ contract Setup is Test { /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. function assertCallFailsOn(address target, bytes memory callData) public { - (bool status,) = target.call(callData); + (bool status, ) = target.call(callData); assert(!status); } @@ -143,7 +171,11 @@ contract Setup is Test { /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. /// @param expectedReturnData The expected return data from the function. - function assertCallFailsOn(address target, bytes memory callData, bytes memory expectedReturnData) public { + function assertCallFailsOn( + address target, + bytes memory callData, + bytes memory expectedReturnData + ) public { (bool status, bytes memory returnData) = target.call(callData); assert(!status); assertEq(expectedReturnData, returnData); @@ -155,7 +187,9 @@ contract Setup is Test { /// @param reason The string reason for the revert. /// /// @return data The ABI encoding of the revert. - function encodeStringRevert(string memory reason) public pure returns (bytes memory data) { + function encodeStringRevert( + string memory reason + ) public pure returns (bytes memory data) { return abi.encodeWithSignature("Error(string)", reason); } } diff --git a/contracts/test/TestUtils.sol b/contracts/test/TestUtils.sol index 499da634..1cfec1eb 100644 --- a/contracts/test/TestUtils.sol +++ b/contracts/test/TestUtils.sol @@ -1,31 +1,35 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +import "@solady/LibBytes.sol"; +import "@forge-std/console.sol"; import "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {IAggregator} from "@account-abstraction/contracts/interfaces/IAggregator.sol"; -import "@forge-std/console.sol"; -import "@solady/LibBytes.sol"; contract TestUtils { - function encodeSignature(bytes memory proofData, uint256 signatureThreshold) - public - pure - returns (bytes memory res) - { + function encodeSignature( + bytes memory proofData, + uint256 signatureThreshold + ) public pure returns (bytes memory res) { bytes memory sigBuffer = new bytes(65 * signatureThreshold + 12); res = LibBytes.concat(sigBuffer, proofData); } /// @notice Create a test data for UserOperations. - function createUOTestData(address sender, bytes[] memory proofs, uint256 signatureThreshold) - public - pure - returns (PackedUserOperation[] memory) - { - PackedUserOperation[] memory uOps = new PackedUserOperation[](proofs.length); + function createUOTestData( + address sender, + bytes[] memory proofs, + uint256 signatureThreshold + ) public pure returns (PackedUserOperation[] memory) { + PackedUserOperation[] memory uOps = new PackedUserOperation[]( + proofs.length + ); for (uint256 i = 0; i < proofs.length; i++) { - bytes memory signature = encodeSignature(proofs[i], signatureThreshold); + bytes memory signature = encodeSignature( + proofs[i], + signatureThreshold + ); PackedUserOperation memory uo = PackedUserOperation({ sender: sender, nonce: i, From c25fde7df57610f4486a875fce210b7b1f5e0874 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Mon, 30 Dec 2024 13:02:51 -0500 Subject: [PATCH 3/6] update pbh ext nullifier tests --- contracts/src/PBHEntryPointImplV1.sol | 88 ++--- .../src/helpers/PBHExternalNullifier.sol | 4 + .../test/PBHEntryPointConstruction.t.sol | 14 +- contracts/test/PBHEntryPointImplV1.t.sol | 59 +--- .../PBHEntryPointOwnershipManagement.t.sol | 106 ++---- contracts/test/PBHExternalNullifier.t.sol | 182 ++++------- contracts/test/PBHSignatureAggregator.t.sol | 305 ++++-------------- contracts/test/PBHVerifier.t.sol | 59 +--- contracts/test/TestSetup.sol | 62 +--- contracts/test/TestUtils.sol | 28 +- 10 files changed, 236 insertions(+), 671 deletions(-) diff --git a/contracts/src/PBHEntryPointImplV1.sol b/contracts/src/PBHEntryPointImplV1.sol index 87eb44c0..f05df4dd 100644 --- a/contracts/src/PBHEntryPointImplV1.sol +++ b/contracts/src/PBHEntryPointImplV1.sol @@ -67,10 +67,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { ////////////////////////////////////////////////////////////////////////////// event PBHEntryPointImplInitialized( - IWorldID indexed worldId, - IEntryPoint indexed entryPoint, - uint8 indexed numPbhPerMonth, - address multicall3 + IWorldID indexed worldId, IEntryPoint indexed entryPoint, uint8 indexed numPbhPerMonth, address multicall3 ); /// @notice Emitted once for each successful PBH verification. @@ -144,12 +141,10 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @param _numPbhPerMonth The number of allowed PBH transactions per month. /// /// @custom:reverts string If called more than once at the same initialisation number. - function initialize( - IWorldID _worldId, - IEntryPoint _entryPoint, - uint8 _numPbhPerMonth, - address _multicall3 - ) external reinitializer(1) { + function initialize(IWorldID _worldId, IEntryPoint _entryPoint, uint8 _numPbhPerMonth, address _multicall3) + external + reinitializer(1) + { // First, ensure that all of the parent contracts are initialised. __delegateInit(); @@ -160,12 +155,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { // Say that the contract is initialized. __setInitialized(); - emit PBHEntryPointImplInitialized( - _worldId, - _entryPoint, - _numPbhPerMonth, - _multicall3 - ); + emit PBHEntryPointImplInitialized(_worldId, _entryPoint, _numPbhPerMonth, _multicall3); } /// @notice Responsible for initialising all of the supertypes of this contract. @@ -183,20 +173,20 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { ////////////////////////////////////////////////////////////////////////////// /// @param pbhPayload The PBH payload containing the proof data. - function verifyPbh( - uint256 signalHash, - PBHPayload memory pbhPayload - ) public view virtual onlyInitialized onlyProxy { + function verifyPbh(uint256 signalHash, PBHPayload memory pbhPayload) + public + view + virtual + onlyInitialized + onlyProxy + { // First, we make sure this nullifier has not been used before. if (nullifierHashes[pbhPayload.nullifierHash]) { revert InvalidNullifier(); } // Verify the external nullifier - PBHExternalNullifier.verify( - pbhPayload.pbhExternalNullifier, - numPbhPerMonth - ); + PBHExternalNullifier.verify(pbhPayload.pbhExternalNullifier, numPbhPerMonth); // If worldId address is set, proceed with on chain verification, // otherwise assume verification has been done off chain by the builder. @@ -221,31 +211,20 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { address payable beneficiary ) external virtual onlyProxy onlyInitialized nonReentrant { for (uint256 i = 0; i < opsPerAggregator.length; ++i) { - bytes32 hashedOps = keccak256( - abi.encode(opsPerAggregator[i].userOps) - ); + bytes32 hashedOps = keccak256(abi.encode(opsPerAggregator[i].userOps)); assembly ("memory-safe") { - if gt(tload(hashedOps), 0) { - revert(0, 0) - } + if gt(tload(hashedOps), 0) { revert(0, 0) } tstore(hashedOps, hashedOps) } - PBHPayload[] memory pbhPayloads = abi.decode( - opsPerAggregator[i].signature, - (PBHPayload[]) - ); + PBHPayload[] memory pbhPayloads = abi.decode(opsPerAggregator[i].signature, (PBHPayload[])); for (uint256 j = 0; j < pbhPayloads.length; ++j) { address sender = opsPerAggregator[i].userOps[j].sender; // We now generate the signal hash from the sender, nonce, and calldata - uint256 signalHash = abi - .encodePacked( - sender, - opsPerAggregator[i].userOps[j].nonce, - opsPerAggregator[i].userOps[j].callData - ) - .hashToField(); + uint256 signalHash = abi.encodePacked( + sender, opsPerAggregator[i].userOps[j].nonce, opsPerAggregator[i].userOps[j].callData + ).hashToField(); verifyPbh(signalHash, pbhPayloads[j]); nullifierHashes[pbhPayloads[j].nullifierHash] = true; @@ -258,20 +237,19 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @notice Validates the hashed operations is the same as the hash transiently stored. /// @param hashedOps The hashed operations to validate. - function validateSignaturesCallback( - bytes32 hashedOps - ) external view virtual onlyProxy onlyInitialized { + function validateSignaturesCallback(bytes32 hashedOps) external view virtual onlyProxy onlyInitialized { assembly ("memory-safe") { - if iszero(eq(tload(hashedOps), hashedOps)) { - revert(0, 0) - } + if iszero(eq(tload(hashedOps), hashedOps)) { revert(0, 0) } } } - function pbhMulticall( - IMulticall3.Call3[] calldata calls, - PBHPayload calldata pbhPayload - ) external virtual onlyProxy onlyInitialized nonReentrant { + function pbhMulticall(IMulticall3.Call3[] calldata calls, PBHPayload calldata pbhPayload) + external + virtual + onlyProxy + onlyInitialized + nonReentrant + { uint256 signalHash = abi.encode(msg.sender, calls).hashToField(); verifyPbh(signalHash, pbhPayload); @@ -284,9 +262,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @notice Sets the number of PBH transactions allowed per month. /// @param _numPbhPerMonth The number of allowed PBH transactions per month. - function setNumPbhPerMonth( - uint8 _numPbhPerMonth - ) external virtual onlyOwner onlyProxy onlyInitialized { + function setNumPbhPerMonth(uint8 _numPbhPerMonth) external virtual onlyOwner onlyProxy onlyInitialized { numPbhPerMonth = _numPbhPerMonth; emit NumPbhPerMonthSet(_numPbhPerMonth); } @@ -294,9 +270,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @dev If the World ID address is set to 0, then it is assumed that verification will take place off chain. /// @notice Sets the World ID instance that will be used for verifying proofs. /// @param _worldId The World ID instance that will be used for verifying proofs. - function setWorldId( - address _worldId - ) external virtual onlyOwner onlyProxy onlyInitialized { + function setWorldId(address _worldId) external virtual onlyOwner onlyProxy onlyInitialized { worldId = IWorldID(_worldId); emit WorldIdSet(_worldId); } diff --git a/contracts/src/helpers/PBHExternalNullifier.sol b/contracts/src/helpers/PBHExternalNullifier.sol index 8eeb86df..ca949eca 100644 --- a/contracts/src/helpers/PBHExternalNullifier.sol +++ b/contracts/src/helpers/PBHExternalNullifier.sol @@ -13,6 +13,8 @@ import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; /// - Bits 16-31: Month /// - Bits 8-15: Nonce /// - Bits 0-7: Version + +//TODO: move this to a lib dir library PBHExternalNullifier { /// @notice Thrown when the provided external nullifier doesn't /// contain the correct leading zeros @@ -76,6 +78,8 @@ library PBHExternalNullifier { require(version == V1, InvalidExternalNullifierVersion()); require(year == BokkyPooBahsDateTimeLibrary.getYear(block.timestamp), InvalidExternalNullifierYear()); require(month == BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp), InvalidExternalNullifierMonth()); + + //TODO: this should be <=? require(pbhNonce < numPbhPerMonth, InvalidPbhNonce()); } } diff --git a/contracts/test/PBHEntryPointConstruction.t.sol b/contracts/test/PBHEntryPointConstruction.t.sol index be98d4c4..79ad1978 100644 --- a/contracts/test/PBHEntryPointConstruction.t.sol +++ b/contracts/test/PBHEntryPointConstruction.t.sol @@ -28,23 +28,15 @@ contract PBHEntryPointConstruction is TestSetup { } /// @notice Tests that it is possible to properly construct and initialise a router. - function testCanConstructRouterWithDelegate( - IWorldID dummy, - IEntryPoint entryPoint - ) public { + function testCanConstructRouterWithDelegate(IWorldID dummy, IEntryPoint entryPoint) public { // Setup vm.expectEmit(true, true, true, true); emit Initialized(1); pbhEntryPointImpl = address(new PBHEntryPointImplV1()); - bytes memory callData = abi.encodeCall( - IPBHEntryPoint.initialize, - (dummy, entryPoint, 30, this.MULTICALL3()) - ); + bytes memory callData = abi.encodeCall(IPBHEntryPoint.initialize, (dummy, entryPoint, 30, this.MULTICALL3())); // Test - pbhEntryPoint = IPBHEntryPoint( - address(new PBHEntryPoint(address(pbhEntryPointImpl), callData)) - ); + pbhEntryPoint = IPBHEntryPoint(address(new PBHEntryPoint(address(pbhEntryPointImpl), callData))); } } diff --git a/contracts/test/PBHEntryPointImplV1.t.sol b/contracts/test/PBHEntryPointImplV1.t.sol index a0cce5ce..74dc3a92 100644 --- a/contracts/test/PBHEntryPointImplV1.t.sol +++ b/contracts/test/PBHEntryPointImplV1.t.sol @@ -24,13 +24,12 @@ contract PBHEntryPointImplV1Test is TestSetup { event WorldIdSet(address indexed worldId); /// @notice Test payload for the PBHVerifier - IPBHEntryPoint.PBHPayload public testPayload = - IPBHEntryPoint.PBHPayload({ - root: 1, - pbhExternalNullifier: getValidPBHExternalNullifier(), - nullifierHash: 1, - proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] - }); + IPBHEntryPoint.PBHPayload public testPayload = IPBHEntryPoint.PBHPayload({ + root: 1, + pbhExternalNullifier: getValidPBHExternalNullifier(), + nullifierHash: 1, + proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] + }); uint256 internal nonce = 1; address internal sender = address(0x123); @@ -38,26 +37,14 @@ contract PBHEntryPointImplV1Test is TestSetup { // TODO: move this to test utils function getValidPBHExternalNullifier() public view returns (uint256) { - uint8 month = uint8( - BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) - ); - uint16 year = uint16( - BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) - ); - return - PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 0, - month, - year - ); + uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); + uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); + return PBHExternalNullifier.encode(PBHExternalNullifier.V1, 0, month, year); } // TODO: function test_verifyPbh() public { - uint256 signalHash = abi - .encodePacked(sender, nonce, testCallData) - .hashToField(); + uint256 signalHash = abi.encodePacked(sender, nonce, testCallData).hashToField(); pbhEntryPoint.verifyPbh(signalHash, testPayload); @@ -77,25 +64,14 @@ contract PBHEntryPointImplV1Test is TestSetup { /// @notice Test that setNumPBHPerMonth works as expected function testSetNumPBHPerMonth() public { - uint256 signalHash = abi - .encodePacked(sender, nonce, testCallData) - .hashToField(); + uint256 signalHash = abi.encodePacked(sender, nonce, testCallData).hashToField(); MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(true); - uint8 month = uint8( - BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) - ); - uint16 year = uint16( - BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) - ); + uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); + uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); // Value starts at 30, make sure 30 reverts. - testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 30, - month, - year - ); + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 30, month, year); testPayload.nullifierHash = 0; vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); @@ -113,12 +89,7 @@ contract PBHEntryPointImplV1Test is TestSetup { pbhEntryPoint.setNumPbhPerMonth(40); // Try again, it should work - testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 30, - month, - year - ); + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 30, month, year); testPayload.nullifierHash = 1; pbhEntryPoint.verifyPbh(signalHash, testPayload); } diff --git a/contracts/test/PBHEntryPointOwnershipManagement.t.sol b/contracts/test/PBHEntryPointOwnershipManagement.t.sol index 2fca7699..7765a6dd 100644 --- a/contracts/test/PBHEntryPointOwnershipManagement.t.sol +++ b/contracts/test/PBHEntryPointOwnershipManagement.t.sol @@ -24,10 +24,7 @@ contract PBHVerifierRouting is TestSetup { } /// @notice Taken from OwnableUpgradable.sol - event OwnershipTransferred( - address indexed previousOwner, - address indexed newOwner - ); + event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); /// @notice Checks that it is possible to get the owner, and that the owner is correctly /// initialised. @@ -44,75 +41,32 @@ contract PBHVerifierRouting is TestSetup { function testTransferOwner(address newOwner) public { // Setup vm.assume(newOwner != nullAddress); - bytes memory transferCallData = abi.encodeCall( - Ownable2StepUpgradeable.transferOwnership, - (newOwner) - ); - bytes memory ownerCallData = abi.encodeCall( - OwnableUpgradeable.owner, - () - ); - bytes memory pendingOwnerCallData = abi.encodeCall( - Ownable2StepUpgradeable.pendingOwner, - () - ); - bytes memory acceptOwnerCallData = abi.encodeCall( - Ownable2StepUpgradeable.acceptOwnership, - () - ); + bytes memory transferCallData = abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (newOwner)); + bytes memory ownerCallData = abi.encodeCall(OwnableUpgradeable.owner, ()); + bytes memory pendingOwnerCallData = abi.encodeCall(Ownable2StepUpgradeable.pendingOwner, ()); + bytes memory acceptOwnerCallData = abi.encodeCall(Ownable2StepUpgradeable.acceptOwnership, ()); // Test - assertCallSucceedsOn( - pbhEntryPointAddress, - transferCallData, - new bytes(0x0) - ); - assertCallSucceedsOn( - pbhEntryPointAddress, - pendingOwnerCallData, - abi.encode(newOwner) - ); - assertCallSucceedsOn( - pbhEntryPointAddress, - ownerCallData, - abi.encode(thisAddress) - ); + assertCallSucceedsOn(pbhEntryPointAddress, transferCallData, new bytes(0x0)); + assertCallSucceedsOn(pbhEntryPointAddress, pendingOwnerCallData, abi.encode(newOwner)); + assertCallSucceedsOn(pbhEntryPointAddress, ownerCallData, abi.encode(thisAddress)); vm.expectEmit(true, true, true, true); emit OwnershipTransferred(thisAddress, newOwner); vm.prank(newOwner); - assertCallSucceedsOn( - pbhEntryPointAddress, - acceptOwnerCallData, - new bytes(0x0) - ); - assertCallSucceedsOn( - pbhEntryPointAddress, - ownerCallData, - abi.encode(newOwner) - ); + assertCallSucceedsOn(pbhEntryPointAddress, acceptOwnerCallData, new bytes(0x0)); + assertCallSucceedsOn(pbhEntryPointAddress, ownerCallData, abi.encode(newOwner)); } /// @notice Tests that only the pending owner can accept the ownership transfer. - function testCannotAcceptOwnershipAsNonPendingOwner( - address newOwner, - address notNewOwner - ) public { + function testCannotAcceptOwnershipAsNonPendingOwner(address newOwner, address notNewOwner) public { // Setup vm.assume(newOwner != nullAddress); vm.assume(notNewOwner != newOwner); - bytes memory callData = abi.encodeCall( - Ownable2StepUpgradeable.transferOwnership, - (newOwner) - ); - bytes memory acceptCallData = abi.encodeCall( - Ownable2StepUpgradeable.acceptOwnership, - () - ); - bytes memory expectedError = encodeStringRevert( - "Ownable2Step: caller is not the new owner" - ); + bytes memory callData = abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (newOwner)); + bytes memory acceptCallData = abi.encodeCall(Ownable2StepUpgradeable.acceptOwnership, ()); + bytes memory expectedError = encodeStringRevert("Ownable2Step: caller is not the new owner"); assertCallSucceedsOn(pbhEntryPointAddress, callData); vm.prank(notNewOwner); @@ -121,19 +75,11 @@ contract PBHVerifierRouting is TestSetup { } /// @notice Ensures that it is impossible to transfer ownership without being the owner. - function testCannotTransferOwnerIfNotOwner( - address naughty, - address newOwner - ) public { + function testCannotTransferOwnerIfNotOwner(address naughty, address newOwner) public { // Setup vm.assume(naughty != thisAddress && newOwner != nullAddress); - bytes memory callData = abi.encodeCall( - OwnableUpgradeable.transferOwnership, - (newOwner) - ); - bytes memory expectedReturn = encodeStringRevert( - "Ownable: caller is not the owner" - ); + bytes memory callData = abi.encodeCall(OwnableUpgradeable.transferOwnership, (newOwner)); + bytes memory expectedReturn = encodeStringRevert("Ownable: caller is not the owner"); vm.prank(naughty); // Test @@ -143,13 +89,8 @@ contract PBHVerifierRouting is TestSetup { /// @notice Tests that it is impossible to renounce ownership, even as the owner. function testCannotRenounceOwnershipAsOwner() public { // Setup - bytes memory renounceData = abi.encodeCall( - OwnableUpgradeable.renounceOwnership, - () - ); - bytes memory errorData = abi.encodeWithSelector( - WorldIDImpl.CannotRenounceOwnership.selector - ); + bytes memory renounceData = abi.encodeCall(OwnableUpgradeable.renounceOwnership, ()); + bytes memory errorData = abi.encodeWithSelector(WorldIDImpl.CannotRenounceOwnership.selector); // Test assertCallFailsOn(pbhEntryPointAddress, renounceData, errorData); @@ -159,13 +100,8 @@ contract PBHVerifierRouting is TestSetup { function testCannotRenounceOwnershipIfNotOwner(address naughty) public { // Setup vm.assume(naughty != thisAddress && naughty != nullAddress); - bytes memory callData = abi.encodeCall( - OwnableUpgradeable.renounceOwnership, - () - ); - bytes memory returnData = encodeStringRevert( - "Ownable: caller is not the owner" - ); + bytes memory callData = abi.encodeCall(OwnableUpgradeable.renounceOwnership, ()); + bytes memory returnData = encodeStringRevert("Ownable: caller is not the owner"); vm.prank(naughty); // Test diff --git a/contracts/test/PBHExternalNullifier.t.sol b/contracts/test/PBHExternalNullifier.t.sol index 76dd23a1..23b62666 100644 --- a/contracts/test/PBHExternalNullifier.t.sol +++ b/contracts/test/PBHExternalNullifier.t.sol @@ -14,138 +14,94 @@ contract PBHExternalNullifierTest is Test { uint16 constant VALID_YEAR = 2024; uint8 constant MAX_PBH_PER_MONTH = 10; - function testEncodeDecodeValidInput() public { - uint8 version = VALID_VERSION; - uint8 pbhNonce = VALID_PBH_NONCE; - uint8 month = VALID_MONTH; - uint16 year = VALID_YEAR; + function testFuzz_encode(uint8 pbhNonce, uint8 month, uint16 year) public pure { + vm.assume(month <= 12); + PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); + } - uint256 encoded = PBHExternalNullifier.encode( - version, - pbhNonce, - month, - year - ); - ( - uint8 decodedVersion, - uint8 decodedNonce, - uint8 decodedMonth, - uint16 decodedYear - ) = PBHExternalNullifier.decode(encoded); - - assertEq( - decodedVersion, - version, - "Decoded version should match the original" - ); - assertEq( - decodedNonce, - pbhNonce, - "Decoded nonce should match the original" - ); - assertEq( - decodedMonth, - month, - "Decoded month should match the original" - ); - assertEq(decodedYear, year, "Decoded year should match the original"); + function testFuzz_encode_RevertIf_InvalidMonth(uint8 pbhNonce, uint8 month, uint16 year) public { + vm.assume(month > 12); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); + PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); } - function testEncodeInvalidMonth() public { - uint8 invalidMonth = 13; + function testFuzz_decode(uint8 pbhNonce, uint8 month, uint16 year) public { + vm.assume(month <= 12); + uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); - vm.expectRevert( - PBHExternalNullifier.InvalidExternalNullifierMonth.selector - ); - PBHExternalNullifier.encode( - VALID_VERSION, - VALID_PBH_NONCE, - invalidMonth, - VALID_YEAR - ); + (uint8 decodedVersion, uint8 decodedNonce, uint8 decodedMonth, uint16 decodedYear) = + PBHExternalNullifier.decode(encoded); + + assertEq(decodedVersion, PBHExternalNullifier.V1); + assertEq(decodedNonce, pbhNonce); + assertEq(decodedMonth, month); + assertEq(decodedYear, year); } + // TODO: + function testFuzz_verify() public {} + + // TODO: + function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros() public {} + + // TODO: + function testFuzz_verify_RevertIf_InvalidExternalNullifierVersion() public {} + + // TODO: + function testFuzz_verify_RevertIf_InvalidExternalNullifierYear() public {} + + // TODO: + function testFuzz_verify_RevertIf_InvalidExternalNullifierMonth() public {} + + // TODO: + function testFuzz_verify_RevertIf_InvalidPbhNonce() public {} + function testVerifyValidExternalNullifier() public { // Mock the current date to match VALID_YEAR and VALID_MONTH - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( - VALID_YEAR, - VALID_MONTH, - 1, - 0, - 0, - 0 - ); + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); vm.warp(timestamp); - uint256 encoded = PBHExternalNullifier.encode( - VALID_VERSION, - VALID_PBH_NONCE, - VALID_MONTH, - VALID_YEAR - ); + uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } function testVerifyInvalidYear() public { - uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary - .timestampFromDateTime( - 2023, // Mock the year to 2023 - VALID_MONTH, - 1, - 0, - 0, - 0 - ); - vm.warp(currentTimestamp); - - uint256 encoded = PBHExternalNullifier.encode( - VALID_VERSION, - VALID_PBH_NONCE, + uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( + 2023, // Mock the year to 2023 VALID_MONTH, - VALID_YEAR - ); - - vm.expectRevert( - PBHExternalNullifier.InvalidExternalNullifierYear.selector + 1, + 0, + 0, + 0 ); - PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); - } - - function testVerifyInvalidMonth() public { - uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary - .timestampFromDateTime( - VALID_YEAR, - 11, // Mock the month to November - 1, - 0, - 0, - 0 - ); vm.warp(currentTimestamp); - uint256 encoded = PBHExternalNullifier.encode( - VALID_VERSION, - VALID_PBH_NONCE, - VALID_MONTH, - VALID_YEAR - ); + uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); - vm.expectRevert( - PBHExternalNullifier.InvalidExternalNullifierMonth.selector - ); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierYear.selector); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } - function testVerifyInvalidPbhNonce() public { - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( + function testVerifyInvalidMonth() public { + uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( VALID_YEAR, - VALID_MONTH, + 11, // Mock the month to November 1, 0, 0, 0 ); + vm.warp(currentTimestamp); + + uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); + PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); + } + + function testVerifyInvalidPbhNonce() public { + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); vm.warp(timestamp); uint256 encoded = PBHExternalNullifier.encode( @@ -160,26 +116,12 @@ contract PBHExternalNullifierTest is Test { } function testVerifyInvalidVersion() public { - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( - VALID_YEAR, - VALID_MONTH, - 1, - 0, - 0, - 0 - ); + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); vm.warp(timestamp); - uint256 encoded = PBHExternalNullifier.encode( - 2, - VALID_PBH_NONCE, - VALID_MONTH, - VALID_YEAR - ); + uint256 encoded = PBHExternalNullifier.encode(2, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); - vm.expectRevert( - PBHExternalNullifier.InvalidExternalNullifierVersion.selector - ); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierVersion.selector); PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); } } diff --git a/contracts/test/PBHSignatureAggregator.t.sol b/contracts/test/PBHSignatureAggregator.t.sol index 11724336..1e836122 100644 --- a/contracts/test/PBHSignatureAggregator.t.sol +++ b/contracts/test/PBHSignatureAggregator.t.sol @@ -20,12 +20,7 @@ contract PBHSignatureAggregatorTest is TestUtils, TestSetup { uint256 timestamp = block.timestamp; uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(timestamp)); uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(timestamp)); - uint256 encoded = PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 0, - month, - year - ); + uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 0, month, year); worldIDGroups.setVerifyProofSuccess(true); IPBHEntryPoint.PBHPayload memory proof0 = IPBHEntryPoint.PBHPayload({ @@ -46,29 +41,17 @@ contract PBHSignatureAggregatorTest is TestUtils, TestSetup { proofs[0] = abi.encode(proof0); proofs[1] = abi.encode(proof1); - PackedUserOperation[] memory uoTestFixture = createUOTestData( - address(safe), - proofs, - 1 - ); - bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures( - uoTestFixture - ); + PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, 1); + bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); - IEntryPoint.UserOpsPerAggregator[] - memory userOpsPerAggregator = new IEntryPoint.UserOpsPerAggregator[]( - 1 - ); + IEntryPoint.UserOpsPerAggregator[] memory userOpsPerAggregator = new IEntryPoint.UserOpsPerAggregator[](1); userOpsPerAggregator[0] = IEntryPoint.UserOpsPerAggregator({ aggregator: pbhAggregator, userOps: uoTestFixture, signature: aggregatedSignature }); - pbhEntryPoint.handleAggregatedOps( - userOpsPerAggregator, - payable(address(this)) - ); + pbhEntryPoint.handleAggregatedOps(userOpsPerAggregator, payable(address(this))); } function testAggregateSignatures( @@ -94,121 +77,37 @@ contract PBHSignatureAggregatorTest is TestUtils, TestSetup { bytes[] memory proofs = new bytes[](2); proofs[0] = abi.encode(proof); proofs[1] = abi.encode(proof); - PackedUserOperation[] memory uoTestFixture = createUOTestData( - address(safe), - proofs, - 1 - ); - bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures( - uoTestFixture - ); - IPBHEntryPoint.PBHPayload[] memory decodedProofs = abi.decode( - aggregatedSignature, - (IPBHEntryPoint.PBHPayload[]) - ); + PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, 1); + bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); + IPBHEntryPoint.PBHPayload[] memory decodedProofs = + abi.decode(aggregatedSignature, (IPBHEntryPoint.PBHPayload[])); assertEq(decodedProofs.length, 2, "Decoded proof length should be 1"); assertEq(decodedProofs[0].root, proof.root, "Root should match"); assertEq( - decodedProofs[0].pbhExternalNullifier, - proof.pbhExternalNullifier, - "PBH External Nullifier should match" - ); - assertEq( - decodedProofs[0].nullifierHash, - proof.nullifierHash, - "Nullifier Hash should match" - ); - assertEq( - decodedProofs[0].proof[0], - proof.proof[0], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[1], - proof.proof[1], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[2], - proof.proof[2], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[3], - proof.proof[3], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[4], - proof.proof[4], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[5], - proof.proof[5], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[6], - proof.proof[6], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[7], - proof.proof[7], - "Proof should match" - ); + decodedProofs[0].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" + ); + assertEq(decodedProofs[0].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); + assertEq(decodedProofs[0].proof[0], proof.proof[0], "Proof should match"); + assertEq(decodedProofs[0].proof[1], proof.proof[1], "Proof should match"); + assertEq(decodedProofs[0].proof[2], proof.proof[2], "Proof should match"); + assertEq(decodedProofs[0].proof[3], proof.proof[3], "Proof should match"); + assertEq(decodedProofs[0].proof[4], proof.proof[4], "Proof should match"); + assertEq(decodedProofs[0].proof[5], proof.proof[5], "Proof should match"); + assertEq(decodedProofs[0].proof[6], proof.proof[6], "Proof should match"); + assertEq(decodedProofs[0].proof[7], proof.proof[7], "Proof should match"); assertEq(decodedProofs[1].root, proof.root, "Root should match"); assertEq( - decodedProofs[1].pbhExternalNullifier, - proof.pbhExternalNullifier, - "PBH External Nullifier should match" - ); - assertEq( - decodedProofs[1].nullifierHash, - proof.nullifierHash, - "Nullifier Hash should match" - ); - assertEq( - decodedProofs[1].proof[0], - proof.proof[0], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[1], - proof.proof[1], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[2], - proof.proof[2], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[3], - proof.proof[3], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[4], - proof.proof[4], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[5], - proof.proof[5], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[6], - proof.proof[6], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[7], - proof.proof[7], - "Proof should match" - ); + decodedProofs[1].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" + ); + assertEq(decodedProofs[1].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); + assertEq(decodedProofs[1].proof[0], proof.proof[0], "Proof should match"); + assertEq(decodedProofs[1].proof[1], proof.proof[1], "Proof should match"); + assertEq(decodedProofs[1].proof[2], proof.proof[2], "Proof should match"); + assertEq(decodedProofs[1].proof[3], proof.proof[3], "Proof should match"); + assertEq(decodedProofs[1].proof[4], proof.proof[4], "Proof should match"); + assertEq(decodedProofs[1].proof[5], proof.proof[5], "Proof should match"); + assertEq(decodedProofs[1].proof[6], proof.proof[6], "Proof should match"); + assertEq(decodedProofs[1].proof[7], proof.proof[7], "Proof should match"); } function testAggregateSignatures_VariableThreshold( @@ -236,121 +135,37 @@ contract PBHSignatureAggregatorTest is TestUtils, TestSetup { bytes[] memory proofs = new bytes[](2); proofs[0] = abi.encode(proof); proofs[1] = abi.encode(proof); - PackedUserOperation[] memory uoTestFixture = createUOTestData( - address(safe), - proofs, - threshold - ); - bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures( - uoTestFixture - ); - IPBHEntryPoint.PBHPayload[] memory decodedProofs = abi.decode( - aggregatedSignature, - (IPBHEntryPoint.PBHPayload[]) - ); + PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, threshold); + bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); + IPBHEntryPoint.PBHPayload[] memory decodedProofs = + abi.decode(aggregatedSignature, (IPBHEntryPoint.PBHPayload[])); assertEq(decodedProofs.length, 2, "Decoded proof length should be 1"); assertEq(decodedProofs[0].root, proof.root, "Root should match"); assertEq( - decodedProofs[0].pbhExternalNullifier, - proof.pbhExternalNullifier, - "PBH External Nullifier should match" - ); - assertEq( - decodedProofs[0].nullifierHash, - proof.nullifierHash, - "Nullifier Hash should match" - ); - assertEq( - decodedProofs[0].proof[0], - proof.proof[0], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[1], - proof.proof[1], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[2], - proof.proof[2], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[3], - proof.proof[3], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[4], - proof.proof[4], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[5], - proof.proof[5], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[6], - proof.proof[6], - "Proof should match" - ); - assertEq( - decodedProofs[0].proof[7], - proof.proof[7], - "Proof should match" - ); + decodedProofs[0].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" + ); + assertEq(decodedProofs[0].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); + assertEq(decodedProofs[0].proof[0], proof.proof[0], "Proof should match"); + assertEq(decodedProofs[0].proof[1], proof.proof[1], "Proof should match"); + assertEq(decodedProofs[0].proof[2], proof.proof[2], "Proof should match"); + assertEq(decodedProofs[0].proof[3], proof.proof[3], "Proof should match"); + assertEq(decodedProofs[0].proof[4], proof.proof[4], "Proof should match"); + assertEq(decodedProofs[0].proof[5], proof.proof[5], "Proof should match"); + assertEq(decodedProofs[0].proof[6], proof.proof[6], "Proof should match"); + assertEq(decodedProofs[0].proof[7], proof.proof[7], "Proof should match"); assertEq(decodedProofs[1].root, proof.root, "Root should match"); assertEq( - decodedProofs[1].pbhExternalNullifier, - proof.pbhExternalNullifier, - "PBH External Nullifier should match" - ); - assertEq( - decodedProofs[1].nullifierHash, - proof.nullifierHash, - "Nullifier Hash should match" - ); - assertEq( - decodedProofs[1].proof[0], - proof.proof[0], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[1], - proof.proof[1], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[2], - proof.proof[2], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[3], - proof.proof[3], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[4], - proof.proof[4], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[5], - proof.proof[5], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[6], - proof.proof[6], - "Proof should match" - ); - assertEq( - decodedProofs[1].proof[7], - proof.proof[7], - "Proof should match" - ); + decodedProofs[1].pbhExternalNullifier, proof.pbhExternalNullifier, "PBH External Nullifier should match" + ); + assertEq(decodedProofs[1].nullifierHash, proof.nullifierHash, "Nullifier Hash should match"); + assertEq(decodedProofs[1].proof[0], proof.proof[0], "Proof should match"); + assertEq(decodedProofs[1].proof[1], proof.proof[1], "Proof should match"); + assertEq(decodedProofs[1].proof[2], proof.proof[2], "Proof should match"); + assertEq(decodedProofs[1].proof[3], proof.proof[3], "Proof should match"); + assertEq(decodedProofs[1].proof[4], proof.proof[4], "Proof should match"); + assertEq(decodedProofs[1].proof[5], proof.proof[5], "Proof should match"); + assertEq(decodedProofs[1].proof[6], proof.proof[6], "Proof should match"); + assertEq(decodedProofs[1].proof[7], proof.proof[7], "Proof should match"); } function testFailAggregateSignatures_InvalidSignatureLength() public { @@ -364,11 +179,7 @@ contract PBHSignatureAggregatorTest is TestUtils, TestSetup { bytes[] memory proofs = new bytes[](2); proofs[0] = abi.encode(proof); proofs[1] = abi.encode(proof); - PackedUserOperation[] memory uoTestFixture = createUOTestData( - address(safe), - proofs, - 1 - ); + PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs, 1); uoTestFixture[0].signature = new bytes(12); vm.expectRevert(PBHSignatureAggregator.InvalidSignatureLength.selector); pbhAggregator.aggregateSignatures(uoTestFixture); diff --git a/contracts/test/PBHVerifier.t.sol b/contracts/test/PBHVerifier.t.sol index c481f117..e4f5fe1d 100644 --- a/contracts/test/PBHVerifier.t.sol +++ b/contracts/test/PBHVerifier.t.sol @@ -24,39 +24,26 @@ contract PBHVerifierTest is TestSetup { event WorldIdSet(address indexed worldId); /// @notice Test payload for the PBHVerifier - IPBHEntryPoint.PBHPayload public testPayload = - IPBHEntryPoint.PBHPayload({ - root: 1, - pbhExternalNullifier: getValidPBHExternalNullifier(), - nullifierHash: 1, - proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] - }); + IPBHEntryPoint.PBHPayload public testPayload = IPBHEntryPoint.PBHPayload({ + root: 1, + pbhExternalNullifier: getValidPBHExternalNullifier(), + nullifierHash: 1, + proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] + }); uint256 internal nonce = 1; address internal sender = address(0x123); bytes internal testCallData = hex"deadbeef"; function getValidPBHExternalNullifier() public view returns (uint256) { - uint8 month = uint8( - BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) - ); - uint16 year = uint16( - BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) - ); - return - PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 0, - month, - year - ); + uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); + uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); + return PBHExternalNullifier.encode(PBHExternalNullifier.V1, 0, month, year); } // TODO: function test_verifyPbh() public { - uint256 signalHash = abi - .encodePacked(sender, nonce, testCallData) - .hashToField(); + uint256 signalHash = abi.encodePacked(sender, nonce, testCallData).hashToField(); pbhEntryPoint.verifyPbh(signalHash, testPayload); @@ -76,25 +63,14 @@ contract PBHVerifierTest is TestSetup { /// @notice Test that setNumPBHPerMonth works as expected function testSetNumPBHPerMonth() public { - uint256 signalHash = abi - .encodePacked(sender, nonce, testCallData) - .hashToField(); + uint256 signalHash = abi.encodePacked(sender, nonce, testCallData).hashToField(); MockWorldIDGroups(address(worldIDGroups)).setVerifyProofSuccess(true); - uint8 month = uint8( - BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp) - ); - uint16 year = uint16( - BokkyPooBahsDateTimeLibrary.getYear(block.timestamp) - ); + uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); + uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); // Value starts at 30, make sure 30 reverts. - testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 30, - month, - year - ); + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 30, month, year); testPayload.nullifierHash = 0; vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); @@ -112,12 +88,7 @@ contract PBHVerifierTest is TestSetup { pbhEntryPoint.setNumPbhPerMonth(40); // Try again, it should work - testPayload.pbhExternalNullifier = PBHExternalNullifier.encode( - PBHExternalNullifier.V1, - 30, - month, - year - ); + testPayload.pbhExternalNullifier = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 30, month, year); testPayload.nullifierHash = 1; pbhEntryPoint.verifyPbh(signalHash, testPayload); } diff --git a/contracts/test/TestSetup.sol b/contracts/test/TestSetup.sol index 4d63ec29..bd1a6371 100644 --- a/contracts/test/TestSetup.sol +++ b/contracts/test/TestSetup.sol @@ -24,8 +24,7 @@ contract TestSetup is Test { /////////////////////////////////////////////////////////////////////////////// /// @notice The 4337 Entry Point on Ethereum Mainnet. - IEntryPoint internal entryPoint = - IEntryPoint(address(0x0000000071727De22E5E9d8BAf0edAc6f37da032)); + IEntryPoint internal entryPoint = IEntryPoint(address(0x0000000071727De22E5E9d8BAf0edAc6f37da032)); /// @notice The PBHEntryPoint contract. IPBHEntryPoint public pbhEntryPoint; /// @notice The PBHSignatureAggregator contract. @@ -38,8 +37,7 @@ contract TestSetup is Test { address public pbhEntryPointImpl; address public immutable thisAddress = address(this); address public constant nullAddress = address(0); - address public constant MULTICALL3 = - 0xcA11bde05977b3631167028862bE2a173976CA11; + address public constant MULTICALL3 = 0xcA11bde05977b3631167028862bE2a173976CA11; /////////////////////////////////////////////////////////////////////////////// /// TEST ORCHESTRATION /// /////////////////////////////////////////////////////////////////////////////// @@ -76,26 +74,14 @@ contract TestSetup is Test { /// /// @param initialGroupAddress The initial group's identity manager. /// @param initialEntryPoint The initial entry point. - function deployPBHEntryPoint( - IWorldID initialGroupAddress, - IEntryPoint initialEntryPoint - ) public { + function deployPBHEntryPoint(IWorldID initialGroupAddress, IEntryPoint initialEntryPoint) public { pbhEntryPointImpl = address(new PBHEntryPointImplV1()); - bytes memory initCallData = abi.encodeCall( - PBHEntryPointImplV1.initialize, - (initialGroupAddress, initialEntryPoint, 30, MULTICALL3) - ); + bytes memory initCallData = + abi.encodeCall(PBHEntryPointImplV1.initialize, (initialGroupAddress, initialEntryPoint, 30, MULTICALL3)); vm.expectEmit(true, true, true, true); - emit PBHEntryPointImplV1.PBHEntryPointImplInitialized( - initialGroupAddress, - initialEntryPoint, - 30, - MULTICALL3 - ); - pbhEntryPoint = IPBHEntryPoint( - address(new PBHEntryPoint(pbhEntryPointImpl, initCallData)) - ); + emit PBHEntryPointImplV1.PBHEntryPointImplInitialized(initialGroupAddress, initialEntryPoint, 30, MULTICALL3); + pbhEntryPoint = IPBHEntryPoint(address(new PBHEntryPoint(pbhEntryPointImpl, initCallData))); } /// @notice Initializes a new PBHSignatureAggregator. @@ -106,10 +92,7 @@ contract TestSetup is Test { /// @notice Initializes a new safe account. /// @dev It is constructed in the globals. - function deploySafeAccount( - address _pbhSignatureAggregator, - uint256 threshold - ) public { + function deploySafeAccount(address _pbhSignatureAggregator, uint256 threshold) public { safe = new MockAccount(_pbhSignatureAggregator, threshold); } @@ -123,9 +106,7 @@ contract TestSetup is Test { /// @dev It is constructed in the globals. function makeUninitPBHEntryPoint() public { pbhEntryPointImpl = address(new PBHEntryPointImplV1()); - pbhEntryPoint = IPBHEntryPoint( - address(new PBHEntryPoint(pbhEntryPointImpl, new bytes(0x0))) - ); + pbhEntryPoint = IPBHEntryPoint(address(new PBHEntryPoint(pbhEntryPointImpl, new bytes(0x0)))); } // TODO: remove these @@ -134,11 +115,8 @@ contract TestSetup is Test { /// /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. - function assertCallSucceedsOn( - address target, - bytes memory callData - ) public { - (bool status, ) = target.call(callData); + function assertCallSucceedsOn(address target, bytes memory callData) public { + (bool status,) = target.call(callData); assert(status); } @@ -147,11 +125,7 @@ contract TestSetup is Test { /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. /// @param expectedReturnData The expected return data from the function. - function assertCallSucceedsOn( - address target, - bytes memory callData, - bytes memory expectedReturnData - ) public { + function assertCallSucceedsOn(address target, bytes memory callData, bytes memory expectedReturnData) public { (bool status, bytes memory returnData) = target.call(callData); assert(status); assertEq(expectedReturnData, returnData); @@ -162,7 +136,7 @@ contract TestSetup is Test { /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. function assertCallFailsOn(address target, bytes memory callData) public { - (bool status, ) = target.call(callData); + (bool status,) = target.call(callData); assert(!status); } @@ -171,11 +145,7 @@ contract TestSetup is Test { /// @param target The target at which to make the call. /// @param callData The ABI-encoded call to a function. /// @param expectedReturnData The expected return data from the function. - function assertCallFailsOn( - address target, - bytes memory callData, - bytes memory expectedReturnData - ) public { + function assertCallFailsOn(address target, bytes memory callData, bytes memory expectedReturnData) public { (bool status, bytes memory returnData) = target.call(callData); assert(!status); assertEq(expectedReturnData, returnData); @@ -187,9 +157,7 @@ contract TestSetup is Test { /// @param reason The string reason for the revert. /// /// @return data The ABI encoding of the revert. - function encodeStringRevert( - string memory reason - ) public pure returns (bytes memory data) { + function encodeStringRevert(string memory reason) public pure returns (bytes memory data) { return abi.encodeWithSignature("Error(string)", reason); } } diff --git a/contracts/test/TestUtils.sol b/contracts/test/TestUtils.sol index 1cfec1eb..f3d9d144 100644 --- a/contracts/test/TestUtils.sol +++ b/contracts/test/TestUtils.sol @@ -8,28 +8,24 @@ import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint import {IAggregator} from "@account-abstraction/contracts/interfaces/IAggregator.sol"; contract TestUtils { - function encodeSignature( - bytes memory proofData, - uint256 signatureThreshold - ) public pure returns (bytes memory res) { + function encodeSignature(bytes memory proofData, uint256 signatureThreshold) + public + pure + returns (bytes memory res) + { bytes memory sigBuffer = new bytes(65 * signatureThreshold + 12); res = LibBytes.concat(sigBuffer, proofData); } /// @notice Create a test data for UserOperations. - function createUOTestData( - address sender, - bytes[] memory proofs, - uint256 signatureThreshold - ) public pure returns (PackedUserOperation[] memory) { - PackedUserOperation[] memory uOps = new PackedUserOperation[]( - proofs.length - ); + function createUOTestData(address sender, bytes[] memory proofs, uint256 signatureThreshold) + public + pure + returns (PackedUserOperation[] memory) + { + PackedUserOperation[] memory uOps = new PackedUserOperation[](proofs.length); for (uint256 i = 0; i < proofs.length; i++) { - bytes memory signature = encodeSignature( - proofs[i], - signatureThreshold - ); + bytes memory signature = encodeSignature(proofs[i], signatureThreshold); PackedUserOperation memory uo = PackedUserOperation({ sender: sender, nonce: i, From 7f9ff052bb4651209094bcdc6e8e231549f21ff7 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Mon, 30 Dec 2024 13:46:50 -0500 Subject: [PATCH 4/6] update external nullfier tests --- contracts/src/PBHEntryPointImplV1.sol | 1 + .../src/helpers/PBHExternalNullifier.sol | 5 + contracts/test/PBHExternalNullifier.t.sol | 156 +++++++++--------- 3 files changed, 84 insertions(+), 78 deletions(-) diff --git a/contracts/src/PBHEntryPointImplV1.sol b/contracts/src/PBHEntryPointImplV1.sol index f05df4dd..089a2a53 100644 --- a/contracts/src/PBHEntryPointImplV1.sol +++ b/contracts/src/PBHEntryPointImplV1.sol @@ -263,6 +263,7 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl, ReentrancyGuard { /// @notice Sets the number of PBH transactions allowed per month. /// @param _numPbhPerMonth The number of allowed PBH transactions per month. function setNumPbhPerMonth(uint8 _numPbhPerMonth) external virtual onlyOwner onlyProxy onlyInitialized { + // TODO: require(_numPbhPerMonth > 0, "PBHEntryPointImplV1: numPbhPerMonth must be greater than 0"); numPbhPerMonth = _numPbhPerMonth; emit NumPbhPerMonthSet(_numPbhPerMonth); } diff --git a/contracts/src/helpers/PBHExternalNullifier.sol b/contracts/src/helpers/PBHExternalNullifier.sol index ca949eca..79b779da 100644 --- a/contracts/src/helpers/PBHExternalNullifier.sol +++ b/contracts/src/helpers/PBHExternalNullifier.sol @@ -15,6 +15,7 @@ import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; /// - Bits 0-7: Version //TODO: move this to a lib dir +// TODO: library PBHExternalNullifier { /// @notice Thrown when the provided external nullifier doesn't /// contain the correct leading zeros @@ -48,6 +49,8 @@ library PBHExternalNullifier { return (uint256(year) << 24) | (uint256(month) << 16) | (uint256(pbhNonce) << 8) | uint256(version); } + // TODO: should we provide an encodeV1 helper function? + /// @notice Decodes an encoded PBHExternalNullifier into its constituent components. /// @param externalNullifier The encoded external nullifier to decode. /// @return version The 8-bit version extracted from the external nullifier. @@ -65,6 +68,8 @@ library PBHExternalNullifier { version = uint8(externalNullifier & 0xFF); } + // TODO: revisit, maybe move this function or update the PBH Entrypoint to update the ext nullifier lib for forward compatibility + /// @notice Verifies the validity of a PBHExternalNullifier by checking its components. /// @param externalNullifier The external nullifier to verify. /// @param numPbhPerMonth The maximum allowed value for the `pbhNonce` in the nullifier. diff --git a/contracts/test/PBHExternalNullifier.t.sol b/contracts/test/PBHExternalNullifier.t.sol index 23b62666..4b20503a 100644 --- a/contracts/test/PBHExternalNullifier.t.sol +++ b/contracts/test/PBHExternalNullifier.t.sol @@ -6,27 +6,19 @@ import "@helpers/PBHExternalNullifier.sol"; import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; contract PBHExternalNullifierTest is Test { - // TODO: dont hard code these, use fuzzing - - uint8 constant VALID_VERSION = PBHExternalNullifier.V1; - uint8 constant VALID_PBH_NONCE = 5; - uint8 constant VALID_MONTH = 12; - uint16 constant VALID_YEAR = 2024; - uint8 constant MAX_PBH_PER_MONTH = 10; - function testFuzz_encode(uint8 pbhNonce, uint8 month, uint16 year) public pure { - vm.assume(month <= 12); + vm.assume(month > 0 && month <= 12); PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); } function testFuzz_encode_RevertIf_InvalidMonth(uint8 pbhNonce, uint8 month, uint16 year) public { - vm.assume(month > 12); + vm.assume(month == 0 || month > 12); vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); } function testFuzz_decode(uint8 pbhNonce, uint8 month, uint16 year) public { - vm.assume(month <= 12); + vm.assume(month > 0 && month <= 12); uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); (uint8 decodedVersion, uint8 decodedNonce, uint8 decodedMonth, uint16 decodedYear) = @@ -38,90 +30,98 @@ contract PBHExternalNullifierTest is Test { assertEq(decodedYear, year); } - // TODO: - function testFuzz_verify() public {} - - // TODO: - function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros() public {} - - // TODO: - function testFuzz_verify_RevertIf_InvalidExternalNullifierVersion() public {} + function testFuzz_verify(uint8 pbhNonce, uint8 month, uint16 year, uint8 maxPbh) public { + vm.assume(month > 0 && month <= 12); + vm.assume(year >= 2023); + vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); - // TODO: - function testFuzz_verify_RevertIf_InvalidExternalNullifierYear() public {} + // Warp to timestamp + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month, 1); + vm.warp(timestamp); - // TODO: - function testFuzz_verify_RevertIf_InvalidExternalNullifierMonth() public {} + uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); + PBHExternalNullifier.verify(encoded, maxPbh); + } // TODO: - function testFuzz_verify_RevertIf_InvalidPbhNonce() public {} - - function testVerifyValidExternalNullifier() public { - // Mock the current date to match VALID_YEAR and VALID_MONTH - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); + function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros( + uint8 pbhNonce, + uint8 month, + uint16 year, + uint8 maxPbh + ) public {} + + function testFuzz_verify_RevertIf_InvalidExternalNullifierVersion( + uint8 pbhVersion, + uint8 pbhNonce, + uint8 month, + uint16 year, + uint8 maxPbh + ) public { + vm.assume(pbhVersion != PBHExternalNullifier.V1); + vm.assume(month > 0 && month <= 12); + vm.assume(year >= 2023); + vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); + + // Warp to timestamp + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month, 1); vm.warp(timestamp); - uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); - - PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); + uint256 encoded = PBHExternalNullifier.encode(pbhVersion, pbhNonce, month, year); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierVersion.selector); + PBHExternalNullifier.verify(encoded, maxPbh); } - function testVerifyInvalidYear() public { - uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( - 2023, // Mock the year to 2023 - VALID_MONTH, - 1, - 0, - 0, - 0 - ); - vm.warp(currentTimestamp); - - uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + function testFuzz_verify_RevertIf_InvalidExternalNullifierYear( + uint8 pbhNonce, + uint8 month, + uint16 year, + uint8 maxPbh + ) public { + vm.assume(month > 0 && month <= 12); + vm.assume(year >= 2023 && year < type(uint16).max); + vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); + + // Warp to timestamp + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year + 1, month, 1); + vm.warp(timestamp); + uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierYear.selector); - PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); - } - - function testVerifyInvalidMonth() public { - uint256 currentTimestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime( - VALID_YEAR, - 11, // Mock the month to November - 1, - 0, - 0, - 0 - ); - vm.warp(currentTimestamp); - - uint256 encoded = PBHExternalNullifier.encode(VALID_VERSION, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); - - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); - PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); + PBHExternalNullifier.verify(encoded, maxPbh); } - function testVerifyInvalidPbhNonce() public { - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); + function testFuzz_verify_RevertIf_InvalidExternalNullifierMonth( + uint8 pbhNonce, + uint8 month, + uint16 year, + uint8 maxPbh + ) public { + vm.assume(month > 0 && month <= 11); + vm.assume(year >= 2023 && year < type(uint16).max); + vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); + + // Warp to timestamp + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month + 1, 1); vm.warp(timestamp); - uint256 encoded = PBHExternalNullifier.encode( - VALID_VERSION, - MAX_PBH_PER_MONTH + 1, // Invalid nonce exceeding max - VALID_MONTH, - VALID_YEAR - ); - - vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); - PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); + uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierYear.selector); + PBHExternalNullifier.verify(encoded, maxPbh); } - function testVerifyInvalidVersion() public { - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDateTime(VALID_YEAR, VALID_MONTH, 1, 0, 0, 0); - vm.warp(timestamp); + function testFuzz_verify_RevertIf_InvalidPbhNonce(uint8 pbhNonce, uint8 month, uint16 year, uint8 maxPbh) public { + vm.assume(month > 0 && month <= 12); + vm.assume(year >= 2023 && year < type(uint16).max); + vm.assume(maxPbh > 0); + vm.assume(pbhNonce > maxPbh); - uint256 encoded = PBHExternalNullifier.encode(2, VALID_PBH_NONCE, VALID_MONTH, VALID_YEAR); + // Warp to timestamp + uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month, 1); + vm.warp(timestamp); - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierVersion.selector); - PBHExternalNullifier.verify(encoded, MAX_PBH_PER_MONTH); + uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); + vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); + PBHExternalNullifier.verify(encoded, maxPbh); } } From 3ffb26096ebe475e4778bebb23662e2291c6647e Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Mon, 30 Dec 2024 13:58:19 -0500 Subject: [PATCH 5/6] fix tests --- .../src/helpers/PBHExternalNullifier.sol | 4 +- contracts/test/PBHExternalNullifier.t.sol | 61 ++++++------------- 2 files changed, 19 insertions(+), 46 deletions(-) diff --git a/contracts/src/helpers/PBHExternalNullifier.sol b/contracts/src/helpers/PBHExternalNullifier.sol index 79b779da..f39b27c4 100644 --- a/contracts/src/helpers/PBHExternalNullifier.sol +++ b/contracts/src/helpers/PBHExternalNullifier.sol @@ -83,8 +83,6 @@ library PBHExternalNullifier { require(version == V1, InvalidExternalNullifierVersion()); require(year == BokkyPooBahsDateTimeLibrary.getYear(block.timestamp), InvalidExternalNullifierYear()); require(month == BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp), InvalidExternalNullifierMonth()); - - //TODO: this should be <=? - require(pbhNonce < numPbhPerMonth, InvalidPbhNonce()); + require(pbhNonce <= numPbhPerMonth, InvalidPbhNonce()); } } diff --git a/contracts/test/PBHExternalNullifier.t.sol b/contracts/test/PBHExternalNullifier.t.sol index 4b20503a..995fe90e 100644 --- a/contracts/test/PBHExternalNullifier.t.sol +++ b/contracts/test/PBHExternalNullifier.t.sol @@ -44,81 +44,56 @@ contract PBHExternalNullifierTest is Test { } // TODO: - function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros( - uint8 pbhNonce, - uint8 month, - uint16 year, - uint8 maxPbh - ) public {} - - function testFuzz_verify_RevertIf_InvalidExternalNullifierVersion( - uint8 pbhVersion, - uint8 pbhNonce, - uint8 month, - uint16 year, - uint8 maxPbh - ) public { + function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros() public {} + + function testFuzz_verify_RevertIf_InvalidExternalNullifierVersion(uint8 pbhVersion) public { vm.assume(pbhVersion != PBHExternalNullifier.V1); - vm.assume(month > 0 && month <= 12); - vm.assume(year >= 2023); - vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); - // Warp to timestamp - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month, 1); - vm.warp(timestamp); + uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); + uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); + uint8 pbhNonce = 0; + uint8 maxPbh = 30; uint256 encoded = PBHExternalNullifier.encode(pbhVersion, pbhNonce, month, year); vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierVersion.selector); PBHExternalNullifier.verify(encoded, maxPbh); } - function testFuzz_verify_RevertIf_InvalidExternalNullifierYear( - uint8 pbhNonce, - uint8 month, - uint16 year, - uint8 maxPbh - ) public { + function testFuzz_verify_RevertIf_InvalidExternalNullifierYear(uint8 month, uint16 year) public { vm.assume(month > 0 && month <= 12); vm.assume(year >= 2023 && year < type(uint16).max); - vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); // Warp to timestamp uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year + 1, month, 1); vm.warp(timestamp); + uint8 pbhNonce = 0; + uint8 maxPbh = 30; uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierYear.selector); PBHExternalNullifier.verify(encoded, maxPbh); } - function testFuzz_verify_RevertIf_InvalidExternalNullifierMonth( - uint8 pbhNonce, - uint8 month, - uint16 year, - uint8 maxPbh - ) public { + function testFuzz_verify_RevertIf_InvalidExternalNullifierMonth(uint8 month, uint16 year) public { vm.assume(month > 0 && month <= 11); vm.assume(year >= 2023 && year < type(uint16).max); - vm.assume(maxPbh > 0 && pbhNonce <= maxPbh); // Warp to timestamp uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month + 1, 1); vm.warp(timestamp); + uint8 pbhNonce = 0; + uint8 maxPbh = 30; uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); - vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierYear.selector); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierMonth.selector); PBHExternalNullifier.verify(encoded, maxPbh); } - function testFuzz_verify_RevertIf_InvalidPbhNonce(uint8 pbhNonce, uint8 month, uint16 year, uint8 maxPbh) public { - vm.assume(month > 0 && month <= 12); - vm.assume(year >= 2023 && year < type(uint16).max); - vm.assume(maxPbh > 0); - vm.assume(pbhNonce > maxPbh); + function testFuzz_verify_RevertIf_InvalidPbhNonce(uint8 pbhNonce, uint8 maxPbh) public { + vm.assume(maxPbh > 0 && pbhNonce > maxPbh); - // Warp to timestamp - uint256 timestamp = BokkyPooBahsDateTimeLibrary.timestampFromDate(year, month, 1); - vm.warp(timestamp); + uint8 month = uint8(BokkyPooBahsDateTimeLibrary.getMonth(block.timestamp)); + uint16 year = uint16(BokkyPooBahsDateTimeLibrary.getYear(block.timestamp)); uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, pbhNonce, month, year); vm.expectRevert(PBHExternalNullifier.InvalidPbhNonce.selector); From 5ec753ef0dd99619b42f321d8652997a3ba800dd Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Mon, 30 Dec 2024 14:28:17 -0500 Subject: [PATCH 6/6] update invalid nullifier leading zeros test --- contracts/src/helpers/PBHExternalNullifier.sol | 1 - contracts/test/PBHExternalNullifier.t.sol | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/contracts/src/helpers/PBHExternalNullifier.sol b/contracts/src/helpers/PBHExternalNullifier.sol index f39b27c4..5a1d942e 100644 --- a/contracts/src/helpers/PBHExternalNullifier.sol +++ b/contracts/src/helpers/PBHExternalNullifier.sol @@ -15,7 +15,6 @@ import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; /// - Bits 0-7: Version //TODO: move this to a lib dir -// TODO: library PBHExternalNullifier { /// @notice Thrown when the provided external nullifier doesn't /// contain the correct leading zeros diff --git a/contracts/test/PBHExternalNullifier.t.sol b/contracts/test/PBHExternalNullifier.t.sol index 995fe90e..b257df80 100644 --- a/contracts/test/PBHExternalNullifier.t.sol +++ b/contracts/test/PBHExternalNullifier.t.sol @@ -43,8 +43,11 @@ contract PBHExternalNullifierTest is Test { PBHExternalNullifier.verify(encoded, maxPbh); } - // TODO: - function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros() public {} + function testFuzz_verify_RevertIf_InvalidNullifierLeadingZeros(uint256 encoded) public { + vm.assume(encoded > type(uint40).max); + vm.expectRevert(PBHExternalNullifier.InvalidExternalNullifierLeadingZeros.selector); + PBHExternalNullifier.verify(encoded, 30); + } function testFuzz_verify_RevertIf_InvalidExternalNullifierVersion(uint8 pbhVersion) public { vm.assume(pbhVersion != PBHExternalNullifier.V1);