Remove Commitment Uniqueness Checks

contracts/src/ProtocolAdapter.sol

@@ -200,13 +200,7 @@ contract ProtocolAdapter is IProtocolAdapter, ReentrancyGuardTransient, Commitme
                     }
 
                     {
-                        // Check that the commitment does not already exist
-                        tags.checkCommitmentNonExistence(cm);
-
-                        // Check that the commitment does not exist in the commitment accumulator
-                        _checkCommitmentNonExistence(cm);
-
-                        // Add the nullifier to the list of tags
+                        // Add the commitment to the list of tags
                         tags[resCounter++] = cm;
                         actionTreeTags[(2 * j) + 1] = cm;
                     }
@@ -260,10 +254,13 @@ contract ProtocolAdapter is IProtocolAdapter, ReentrancyGuardTransient, Commitme
 
                 for (uint256 j = 0; j < nResources; ++j) {
                     Logic.VerifierInput calldata input = action.logicVerifierInputs[j];
-
-                    _verifyLogicProof(
-                        input, actionTreeRoot, TagLookup.isFoundInEvenOrOddPosition(actionTreeTags, input.tag, true)
-                    );
+                    if (actionTreeTags.isNullifierContained({nullifier: input.tag})) {
+                        // Check the logic proof for nullified resource
+                        _verifyLogicProof(input, actionTreeRoot, true);
+                    } else {
+                        // Else the tag is a created resource
+                        _verifyLogicProof(input, actionTreeRoot, false);
+                    }
                 }
             }
         }

contracts/src/libs/TagLookup.sol

@@ -9,41 +9,28 @@ library TagLookup {
     error NullifierDuplicated(bytes32 nullifier);
     error CommitmentDuplicated(bytes32 commitment);
 
-    /// @notice Returns whether an array contains a tag or not in even or odd positions.
+    /// @notice Returns whether an array of tags contains a nullifier or not.
     /// @param tags The tags array to check.
-    /// @param tag The tag to check.
-    /// @param even Whether even or odd positions should be checked.
-    /// @return found Whether the element was found in the array or not.
-    function isFoundInEvenOrOddPosition(bytes32[] memory tags, bytes32 tag, bool even)
-        internal
-        pure
-        returns (bool found)
-    {
+    /// @param nullifier The nullifier to check.
+    /// @return isContained Whether the element is contained in the array or not.
+    /// @dev This assumes that nullifiers and commitments are found in even and odd positions, respectively.
+    function isNullifierContained(bytes32[] memory tags, bytes32 nullifier) internal pure returns (bool isContained) {
         uint256 len = tags.length;
 
-        for (uint256 i = even ? 0 : 1; i < len; i += 2) {
-            if (tags[i] == tag) {
-                return found = true;
+        for (uint256 i = 0; i < len; i += 2) {
+            if (tags[i] == nullifier) {
+                return isContained = true;
             }
         }
-        return found = false;
+        return isContained = false;
     }
 
     /// @notice Checks if a nullifier is non-existent in an array of tags and reverts if it not.
     /// @param tags The tags array to check.
     /// @param nullifier The nullifier to check non-existence for.
     function checkNullifierNonExistence(bytes32[] memory tags, bytes32 nullifier) internal pure {
-        if (isFoundInEvenOrOddPosition({tags: tags, tag: nullifier, even: true})) {
+        if (isNullifierContained(tags, nullifier)) {
             revert NullifierDuplicated(nullifier);
         }
     }
-
-    /// @notice Checks if a commitment is non-existent in an array of tags and reverts if it not.
-    /// @param tags The tags array to check.
-    /// @param commitment The commitment to check non-existence for.
-    function checkCommitmentNonExistence(bytes32[] memory tags, bytes32 commitment) internal pure {
-        if (isFoundInEvenOrOddPosition({tags: tags, tag: commitment, even: false})) {
-            revert CommitmentDuplicated(commitment);
-        }
-    }
 }

contracts/test/ProtocolAdapter.t.sol

@@ -83,21 +83,6 @@ contract ProtocolAdapterTest is Test {
         _pa.verify(txn);
     }
 
-    function test_verify_reverts_on_action_with_duplicated_commitments() public {
-        Action[] memory actions = new Action[](1);
-        actions[0] = _actionWithDuplicatedComplianceUnit();
-
-        bytes32 duplicatedCommitment = actions[0].complianceVerifierInputs[0].instance.created.commitment;
-        actions[0].complianceVerifierInputs[1].instance.consumed.nullifier = keccak256("Not a duplicate");
-
-        Transaction memory txn = Transaction({actions: actions, deltaProof: ""});
-
-        vm.expectRevert(
-            abi.encodeWithSelector(TagLookup.CommitmentDuplicated.selector, duplicatedCommitment), address(_pa)
-        );
-        _pa.verify(txn);
-    }
-
     function test_verify_empty_tx() public view {
         Transaction memory txn = Transaction({actions: new Action[](0), deltaProof: ""});
         _pa.verify(txn);

contracts/test/ProtocolAdapterMock.t.sol

@@ -13,7 +13,6 @@ import {MerkleTree} from "../src/libs/MerkleTree.sol";
 import {TagLookup} from "../src/libs/TagLookup.sol";
 import {ProtocolAdapter} from "../src/ProtocolAdapter.sol";
 import {Logic} from "../src/proving/Logic.sol";
-import {CommitmentAccumulator} from "../src/state/CommitmentAccumulator.sol";
 import {NullifierSet} from "../src/state/NullifierSet.sol";
 import {ForwarderCalldata, Transaction, Resource} from "../src/Types.sol";
 
@@ -277,48 +276,17 @@ contract ProtocolAdapterMockTest is Test {
         _mockPa.verify(tx2);
     }
 
-    function test_verify_reverts_on_pre_existing_commitment() public {
-        TxGen.ActionConfig[] memory configs = TxGen.generateActionConfigs({nActions: 1, nCUs: 1});
-
-        (Transaction memory tx1, bytes32 updatedNonce) =
-            _mockVerifier.transaction({nonce: 0, configs: configs, commitmentTreeDepth: _TEST_COMMITMENT_TREE_DEPTH});
-        bytes32 preExistingCm = tx1.actions[0].complianceVerifierInputs[0].instance.created.commitment;
-        _mockPa.execute(tx1);
-
-        (Transaction memory tx2,) = _mockVerifier.transaction({
-            nonce: updatedNonce,
-            configs: configs,
-            commitmentTreeDepth: _TEST_COMMITMENT_TREE_DEPTH
-        });
-        tx2.actions[0].complianceVerifierInputs[0].instance.created.commitment = preExistingCm;
-        vm.expectRevert(
-            abi.encodeWithSelector(CommitmentAccumulator.PreExistingCommitment.selector, preExistingCm),
-            address(_mockPa)
-        );
-        _mockPa.verify(tx2);
-    }
-
     function test_verify_reverts_on_duplicated_nullifier() public {
         TxGen.ActionConfig[] memory configs = TxGen.generateActionConfigs({nActions: 1, nCUs: 2});
 
         (Transaction memory txn,) =
             _mockVerifier.transaction({nonce: 0, configs: configs, commitmentTreeDepth: _TEST_COMMITMENT_TREE_DEPTH});
         bytes32 duplicatedNf = txn.actions[0].complianceVerifierInputs[0].instance.consumed.nullifier;
-        txn.actions[0].complianceVerifierInputs[1].instance.consumed.nullifier = duplicatedNf;
-
-        vm.expectRevert(abi.encodeWithSelector(TagLookup.NullifierDuplicated.selector, duplicatedNf), address(_mockPa));
-        _mockPa.verify(txn);
-    }
-
-    function test_verify_reverts_on_duplicated_commitment() public {
-        TxGen.ActionConfig[] memory configs = TxGen.generateActionConfigs({nActions: 1, nCUs: 2});
-
-        (Transaction memory txn,) =
-            _mockVerifier.transaction({nonce: 0, configs: configs, commitmentTreeDepth: _TEST_COMMITMENT_TREE_DEPTH});
         bytes32 duplicatedCm = txn.actions[0].complianceVerifierInputs[0].instance.created.commitment;
+        txn.actions[0].complianceVerifierInputs[1].instance.consumed.nullifier = duplicatedNf;
         txn.actions[0].complianceVerifierInputs[1].instance.created.commitment = duplicatedCm;
 
-        vm.expectRevert(abi.encodeWithSelector(TagLookup.CommitmentDuplicated.selector, duplicatedCm), address(_mockPa));
+        vm.expectRevert(abi.encodeWithSelector(TagLookup.NullifierDuplicated.selector, duplicatedNf), address(_mockPa));
         _mockPa.verify(txn);
     }
 

contracts/test/TagLookup.t.sol

@@ -24,44 +24,18 @@ contract TagLookupTest is Test {
         _tags.checkNullifierNonExistence(_tags[0]);
     }
 
-    function test_checkCommitmentNonExistence_reverts_if_commitment_exists() public {
-        vm.expectRevert(abi.encodeWithSelector(TagLookup.CommitmentDuplicated.selector, _tags[1]), address(this));
-        _tags.checkCommitmentNonExistence(_tags[1]);
-    }
-
     function test_checkNullifierNonExistence_passes_if_the_nullifier_does_not_exist() public view {
         _tags.checkNullifierNonExistence(_tags[1]);
     }
 
-    function test_checkCommitmentNonExistence_passes_if_the_commitment_does_not_exist() public view {
-        _tags.checkCommitmentNonExistence(_tags[0]);
-    }
-
-    function test_isFoundInEvenOrOddPosition_returns_true_for_existent_tag_in_even_positions() public view {
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[0], even: true}), true);
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[2], even: true}), true);
-    }
-
-    function test_isFoundInEvenOrOddPosition_returns_false_for_non_existent_tag_in_even_positions() public view {
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[1], even: true}), false);
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[3], even: true}), false);
-    }
-
-    function test_isFoundInEvenOrOddPosition_returns_true_for_existent_tag_in_odd_positions() public view {
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[1], even: false}), true);
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[3], even: false}), true);
-    }
-
-    function test_isFoundInEvenOrOddPosition_returns_false_for_non_existent_tag_in_odd_positions() public view {
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[0], even: false}), false);
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: _tags[2], even: false}), false);
+    function test_isNullifierContained_returns_true_for_existent_nullifier() public view {
+        assertEq(_tags.isNullifierContained({nullifier: _tags[0]}), true);
+        assertEq(_tags.isNullifierContained({nullifier: _tags[2]}), true);
     }
 
-    function test_isFoundInEvenOrOddPosition_returns_false_if_a_tag_is_non_existent_in_both_even_or_odd_positions()
-        public
-        view
-    {
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: bytes32(type(uint256).max), even: true}), false);
-        assertEq(_tags.isFoundInEvenOrOddPosition({tag: bytes32(type(uint256).max), even: false}), false);
+    function test_isNullifierContained_returns_false_for_non_existent_nullifier() public view {
+        assertEq(_tags.isNullifierContained({nullifier: _tags[1]}), false);
+        assertEq(_tags.isNullifierContained({nullifier: _tags[3]}), false);
+        assertEq(_tags.isNullifierContained({nullifier: bytes32(type(uint256).max)}), false);
     }
 }