add identityManager v3 with modified verifyProof impl

script/Deploy.s.sol

@@ -7,6 +7,7 @@ import "../src/WorldIDRouterImplV1.sol";
 import "../src/WorldIDIdentityManager.sol";
 import "../src/WorldIDIdentityManagerImplV1.sol";
 import "../src/WorldIDIdentityManagerImplV2.sol";
+import "../src/WorldIDIdentityManagerImplV3.sol";
 import "../src/SemaphoreVerifier.sol";
 
 import {Verifier as InsertionB10} from "../src/verifiers/insertion/b10.sol";
@@ -75,13 +76,18 @@ contract Deploy is Script {
 
         WorldIDIdentityManagerImplV1 impl1 = new WorldIDIdentityManagerImplV1();
         WorldIDIdentityManagerImplV2 impl2 = new WorldIDIdentityManagerImplV2();
+        WorldIDIdentityManagerImplV3 impl3 = new WorldIDIdentityManagerImplV3();
 
         WorldIDIdentityManager worldID = new WorldIDIdentityManager(address(impl1), initializeCall);
 
         // Recast to access api
         WorldIDIdentityManagerImplV1 worldIDImplV1 = WorldIDIdentityManagerImplV1(address(worldID));
         worldIDImplV1.upgradeToAndCall(address(impl2), initializeV2Call);
 
+        // Upgrade to V3
+        WorldIDIdentityManagerImplV2 worldIDImplV2 = WorldIDIdentityManagerImplV2(address(worldID));
+        worldIDImplV2.upgradeTo(address(impl3));
+
         vm.stopBroadcast();
 
         return worldID;

src/WorldIDIdentityManagerImplV1.sol

@@ -667,12 +667,11 @@ contract WorldIDIdentityManagerImplV1 is WorldIDImpl, IWorldID {
     /// @dev Note that a double-signaling check is not included here, and should be carried by the
     ///      caller.
     ///
-    /// @param proof The zero-knowledge proof
     /// @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 zero-knowledge proof cannot be verified for the public inputs.
     function verifyProof(
         uint256 root,

src/WorldIDIdentityManagerImplV3.sol

@@ -0,0 +1,107 @@
+pragma solidity ^0.8.21;
+
+import "./WorldIDIdentityManagerImplV2.sol";
+
+/// @title WorldID Identity Manager Implementation Version 3
+/// @author Worldcoin
+/// @notice An implementation of a batch-based identity manager for the WorldID protocol.
+/// @dev The manager is based on the principle of verifying externally-created Zero Knowledge Proofs
+///      to perform the deletions.
+/// @dev This is the implementation delegated to by a proxy.
+contract WorldIDIdentityManagerImplV3 is WorldIDIdentityManagerImplV2 {
+    ///////////////////////////////////////////////////////////////////////////////
+    ///                   A NOTE ON IMPLEMENTATION CONTRACTS                    ///
+    ///////////////////////////////////////////////////////////////////////////////
+
+    // This contract is designed explicitly to operate from behind a proxy contract. As a result,
+    // there are a few important implementation considerations:
+    //
+    // - All updates made after deploying a given version of the implementation should inherit from
+    //   the latest version of the implementation. This contract inherits from its previous implementation
+    //   WorldIDIdentityManagerImplV1. This prevents storage clashes.
+    // - All functions that are less access-restricted than `private` should be marked `virtual` in
+    //   order to enable the fixing of bugs in the existing interface.
+    // - Any function that reads from or modifies state (i.e. is not marked `pure`) must be
+    //   annotated with the `onlyProxy` and `onlyInitialized` modifiers. This ensures that it can
+    //   only be called when it has access to the data in the proxy, otherwise results are likely to
+    //   be nonsensical.
+    // - This contract deals with important data for the WorldID system. Ensure that all newly-added
+    //   functionality is carefully access controlled using `onlyOwner`, or a more granular access
+    //   mechanism.
+    // - Do not assign any contract-level variables at the definition site unless they are
+    //   `constant`.
+    //
+    // Additionally, the following notes apply:
+    //
+    // - Initialisation and ownership management are not protected behind `onlyProxy` intentionally.
+    //   This ensures that the contract can safely be disposed of after it is no longer used.
+    // - Carefully consider what data recovery options are presented as new functionality is added.
+    //   Care must be taken to ensure that a migration plan can exist for cases where upgrades
+    //   cannot recover from an issue or vulnerability.
+
+    ///////////////////////////////////////////////////////////////////////////////
+    ///                    !!!!! DATA: DO NOT REORDER !!!!!                     ///
+    ///////////////////////////////////////////////////////////////////////////////
+
+    // To ensure compatibility between upgrades, it is exceedingly important that no reordering of
+    // these variables takes place. If reordering happens, a storage clash will occur (effectively a
+    // memory safety error).
+
+    ///////////////////////////////////////////////////////////////////////////////
+    ///                    SEMAPHORE PROOF VALIDATION LOGIC                     ///
+    ///////////////////////////////////////////////////////////////////////////////
+
+    /// @notice A verifier for the semaphore protocol that supports compressed proofs and is backwards
+    ///         compatible with the previous implementation. If a proof is compressed, the last 4 uints
+    ///         will be 0. If this condition is met, we will call the semaphore compress proof function
+    ///         instead.
+    /// @dev Note that a double-signaling check is not included here, and should be carried by the
+    ///      caller.
+    ///
+    /// @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 zero-knowledge proof cannot be verified for the public inputs.
+    function verifyProof(
+        uint256 root,
+        uint256 signalHash,
+        uint256 nullifierHash,
+        uint256 externalNullifierHash,
+        uint256[8] calldata proof
+    ) public view virtual override onlyProxy onlyInitialized {
+        // Check the preconditions on the inputs.
+        requireValidRoot(root);
+        uint256[4] memory input = [root, nullifierHash, signalHash, externalNullifierHash];
+        if (proof[4] == 0 && proof[5] == 0 && proof[6] == 0 && proof[7] == 0) {
+            uint256[4] memory compressedProof = [proof[0], proof[1], proof[2], proof[3]];
+            semaphoreVerifier.verifyCompressedProof(compressedProof, input);
+        } else {
+            semaphoreVerifier.verifyProof(proof, input);
+        }
+    }
+
+    /// @notice A verifier for the semaphore protocol that supports compressed proofs
+    /// @dev Note that a double-signaling check is not included here, and should be carried by the
+    ///      caller.
+    ///
+    /// @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 compressedProof The zero-knowledge proof
+    /// @custom:reverts string If the zero-knowledge proof cannot be verified for the public inputs.
+    function verifyCompressedProof(
+        uint256 root,
+        uint256 signalHash,
+        uint256 nullifierHash,
+        uint256 externalNullifierHash,
+        uint256[4] calldata compressedProof
+    ) public view virtual onlyProxy onlyInitialized {
+        // Check the preconditions on the inputs.
+        requireValidRoot(root);
+        uint256[4] memory input = [root, nullifierHash, signalHash, externalNullifierHash];
+        semaphoreVerifier.verifyCompressedProof(compressedProof, input);
+    }
+}

src/interfaces/ISemaphoreVerifier.sol

@@ -15,4 +15,17 @@ interface ISemaphoreVerifier {
     /// @param input the public input field elements in the scalar field Fr.
     /// Elements must be reduced.
     function verifyProof(uint256[8] calldata proof, uint256[4] calldata input) external view;
+
+    /// Verify a Groth16 proof with compressed points.
+    /// @notice Reverts with InvalidProof if the proof is invalid or
+    /// with PublicInputNotInField the public input is not reduced.
+    /// @notice There is no return value. If the function does not revert, the
+    /// proof was succesfully verified.
+    /// @param compressedProof the points (A, B, C) in compressed format
+    /// matching the output of compressProof.
+    /// @param input the public input field elements in the scalar field Fr.
+    /// Elements must be reduced.
+    function verifyCompressedProof(uint256[4] calldata compressedProof, uint256[4] calldata input)
+        external
+        view;
 }

src/test/identity-manager/WorldIDIdentityManagerCalculation.t.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.21;
 
 import {WorldIDIdentityManagerTest} from "./WorldIDIdentityManagerTest.sol";
 
-import {WorldIDIdentityManagerImplV2 as ManagerImpl} from "../../WorldIDIdentityManagerImplV2.sol";
+import {WorldIDIdentityManagerImplV2 as ManagerImplV2} from "../../WorldIDIdentityManagerImplV2.sol";
 import {WorldIDIdentityManagerImplV1 as ManagerImplV1} from "../../WorldIDIdentityManagerImplV1.sol";
 
 /// @title World ID Identity Manager Calculation Tests
@@ -32,7 +32,7 @@ contract WorldIDIdentityManagerCalculation is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.calculateIdentityRegistrationInputHash(
+        managerImplV2.calculateIdentityRegistrationInputHash(
             startIndex, insertionPreRoot, insertionPostRoot, identityCommitments
         );
     }
@@ -42,7 +42,7 @@ contract WorldIDIdentityManagerCalculation is WorldIDIdentityManagerTest {
     function testCalculateIdentityDeletionInputHashFromParametersOnKnownInput() public {
         // Setup
         bytes memory callData = abi.encodeCall(
-            ManagerImpl.calculateIdentityDeletionInputHash,
+            ManagerImplV2.calculateIdentityDeletionInputHash,
             (packedDeletionIndices, deletionPreRoot, deletionPostRoot, deletionBatchSize)
         );
         bytes memory returnData = abi.encode(deletionInputHash);
@@ -57,7 +57,7 @@ contract WorldIDIdentityManagerCalculation is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.calculateIdentityDeletionInputHash(
+        managerImplV2.calculateIdentityDeletionInputHash(
             packedDeletionIndices, deletionPreRoot, deletionPostRoot, deletionBatchSize
         );
     }

src/test/identity-manager/WorldIDIdentityManagerConstruction.t.sol

@@ -5,7 +5,7 @@ import {WorldIDIdentityManagerTest} from "./WorldIDIdentityManagerTest.sol";
 
 import {UUPSUpgradeable} from "contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import {WorldIDIdentityManager as IdentityManager} from "../../WorldIDIdentityManager.sol";
-import {WorldIDIdentityManagerImplV2 as ManagerImpl} from "../../WorldIDIdentityManagerImplV2.sol";
+import {WorldIDIdentityManagerImplV2 as ManagerImplV2} from "../../WorldIDIdentityManagerImplV2.sol";
 import {WorldIDIdentityManagerImplV1 as ManagerImplV1} from "../../WorldIDIdentityManagerImplV1.sol";
 
 /// @title World ID Identity Manager Construction Tests
@@ -45,18 +45,18 @@ contract WorldIDIdentityManagerConstruction is WorldIDIdentityManagerTest {
         );
 
         // Test
-        identityManager = new IdentityManager(address(managerImpl), callData);
+        identityManager = new IdentityManager(address(managerImplV2), callData);
 
         identityManagerAddress = address(identityManager);
 
         // creates Manager Impl V2, which will be used for tests
-        managerImpl = new ManagerImpl();
-        managerImplAddress = address(managerImpl);
+        managerImplV2 = new ManagerImplV2();
+        managerImplV2Address = address(managerImplV2);
 
         bytes memory initCallV2 =
-            abi.encodeCall(ManagerImpl.initializeV2, (defaultDeletionVerifiers));
+            abi.encodeCall(ManagerImplV2.initializeV2, (defaultDeletionVerifiers));
         bytes memory upgradeCall = abi.encodeCall(
-            UUPSUpgradeable.upgradeToAndCall, (address(managerImplAddress), initCallV2)
+            UUPSUpgradeable.upgradeToAndCall, (address(managerImplV2Address), initCallV2)
         );
 
         // Test

src/test/identity-manager/WorldIDIdentityManagerDataQuery.t.sol

@@ -128,7 +128,7 @@ contract WorldIDIdentityManagerDataQuery is WorldIDIdentityManagerTest {
         // Setup
         vm.assume(badRoot != initialRoot);
         bytes memory callData = abi.encodeCall(ManagerImplV1.queryRoot, badRoot);
-        bytes memory returnData = abi.encode(managerImpl.NO_SUCH_ROOT());
+        bytes memory returnData = abi.encode(managerImplV2.NO_SUCH_ROOT());
 
         // Test
         assertCallSucceedsOn(identityManagerAddress, callData, returnData);
@@ -140,7 +140,7 @@ contract WorldIDIdentityManagerDataQuery is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.queryRoot(initialRoot);
+        managerImplV2.queryRoot(initialRoot);
     }
 
     /// @notice Checks that it is possible to get the latest root from the contract.
@@ -167,7 +167,7 @@ contract WorldIDIdentityManagerDataQuery is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.latestRoot();
+        managerImplV2.latestRoot();
     }
 
     /// @notice Checks that it is possible to get the tree depth the contract was initialized with.

src/test/identity-manager/WorldIDIdentityManagerGettersSetters.t.sol

@@ -9,7 +9,7 @@ import {SimpleVerifier, SimpleVerify} from "../mock/SimpleVerifier.sol";
 import {TypeConverter as TC} from "../utils/TypeConverter.sol";
 import {VerifierLookupTable} from "../../data/VerifierLookupTable.sol";
 
-import {WorldIDIdentityManagerImplV2 as ManagerImpl} from "../../WorldIDIdentityManagerImplV2.sol";
+import {WorldIDIdentityManagerImplV2 as ManagerImplV2} from "../../WorldIDIdentityManagerImplV2.sol";
 import {WorldIDIdentityManagerImplV1 as ManagerImplV1} from "../../WorldIDIdentityManagerImplV1.sol";
 
 /// @title World ID Identity Manager Getter and Setter Tests
@@ -20,7 +20,9 @@ import {WorldIDIdentityManagerImplV1 as ManagerImplV1} from "../../WorldIDIdenti
 contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
     /// @notice Taken from WorldIDIdentityManagerImplV1.sol
     event DependencyUpdated(
-        ManagerImpl.Dependency indexed kind, address indexed oldAddress, address indexed newAddress
+        ManagerImplV2.Dependency indexed kind,
+        address indexed oldAddress,
+        address indexed newAddress
     );
     event RootHistoryExpirySet(uint256 indexed oldExpiryTime, uint256 indexed newExpiryTime);
 
@@ -43,7 +45,7 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.getRegisterIdentitiesVerifierLookupTableAddress();
+        managerImplV2.getRegisterIdentitiesVerifierLookupTableAddress();
     }
 
     /// @notice Checks that it is possible to set the lookup table currently being used to verify
@@ -93,15 +95,15 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.setRegisterIdentitiesVerifierLookupTable(insertionVerifiers);
+        managerImplV2.setRegisterIdentitiesVerifierLookupTable(insertionVerifiers);
     }
 
     /// @notice Checks that it is possible to get the address of the contract currently being used
     ///         to verify identity deletion proofs.
     function testCanGetDeleteIdentitiesVerifierLookupTableAddress() public {
         // Setup
         bytes memory callData =
-            abi.encodeCall(ManagerImpl.getDeleteIdentitiesVerifierLookupTableAddress, ());
+            abi.encodeCall(ManagerImplV2.getDeleteIdentitiesVerifierLookupTableAddress, ());
         bytes memory expectedReturn = abi.encode(address(defaultDeletionVerifiers));
 
         // Test
@@ -115,7 +117,7 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.getDeleteIdentitiesVerifierLookupTableAddress();
+        managerImplV2.getDeleteIdentitiesVerifierLookupTableAddress();
     }
 
     /// @notice Checks that it is possible to set the lookup table currently being used to verify
@@ -124,10 +126,11 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         // Setup
         (, VerifierLookupTable deletionVerifiers,) = makeVerifierLookupTables(TC.makeDynArray([40]));
         address newVerifiersAddress = address(deletionVerifiers);
-        bytes memory callData =
-            abi.encodeCall(ManagerImpl.setDeleteIdentitiesVerifierLookupTable, (deletionVerifiers));
+        bytes memory callData = abi.encodeCall(
+            ManagerImplV2.setDeleteIdentitiesVerifierLookupTable, (deletionVerifiers)
+        );
         bytes memory checkCallData =
-            abi.encodeCall(ManagerImpl.getDeleteIdentitiesVerifierLookupTableAddress, ());
+            abi.encodeCall(ManagerImplV2.getDeleteIdentitiesVerifierLookupTableAddress, ());
         bytes memory expectedReturn = abi.encode(newVerifiersAddress);
         vm.expectEmit(true, false, true, true);
         emit DependencyUpdated(
@@ -162,7 +165,7 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.setDeleteIdentitiesVerifierLookupTable(deletionVerifiers);
+        managerImplV2.setDeleteIdentitiesVerifierLookupTable(deletionVerifiers);
     }
 
     /// @notice Ensures that we can get the address of the semaphore verifier.
@@ -181,7 +184,7 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.getSemaphoreVerifierAddress();
+        managerImplV2.getSemaphoreVerifierAddress();
     }
 
     /// @notice Checks that it is possible to set the contract currently being used to verify
@@ -224,7 +227,7 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.setSemaphoreVerifier(newVerifier);
+        managerImplV2.setSemaphoreVerifier(newVerifier);
     }
 
     /// @notice Ensures that it's possible to get the root history expiry time.
@@ -243,7 +246,7 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.getRootHistoryExpiry();
+        managerImplV2.getRootHistoryExpiry();
     }
 
     /// @notice Ensures that it is possible to set the root history expiry time.
@@ -290,6 +293,6 @@ contract WorldIDIdentityManagerGettersSetters is WorldIDIdentityManagerTest {
         vm.expectRevert("Function must be called through delegatecall");
 
         // Test
-        managerImpl.setRootHistoryExpiry(newExpiry);
+        managerImplV2.setRootHistoryExpiry(newExpiry);
     }
 }