feat(circuits): hints nullifier transient commitments (#2056)

Solves #837 

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [x] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [x] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).

circuits/cpp/src/aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp

@@ -3,6 +3,7 @@
 #include "private_call_data.hpp"
 #include "../previous_kernel_data.hpp"
 
+#include "aztec3/constants.hpp"
 #include "aztec3/utils/types/circuit_types.hpp"
 #include "aztec3/utils/types/native_types.hpp"
 
@@ -20,10 +21,11 @@ template <typename NCT> struct PrivateKernelInputsOrdering {
 
     PreviousKernelData<NCT> previous_kernel{};
 
-    std::array<fr, MAX_READ_REQUESTS_PER_TX> hint_to_commitments{};
+    std::array<fr, MAX_READ_REQUESTS_PER_TX> read_commitment_hints{};
+    std::array<fr, MAX_NEW_NULLIFIERS_PER_TX> nullifier_commitment_hints{};
 
     // For serialization, update with new fields
-    MSGPACK_FIELDS(previous_kernel, hint_to_commitments);
+    MSGPACK_FIELDS(previous_kernel, read_commitment_hints, nullifier_commitment_hints);
     boolean operator==(PrivateKernelInputsOrdering<NCT> const& other) const
     {
         return msgpack_derived_equals<boolean>(*this, other);
@@ -36,7 +38,8 @@ template <typename NCT> struct PrivateKernelInputsOrdering {
 
         PrivateKernelInputsOrdering<CircuitTypes<Builder>> private_inputs = {
             previous_kernel.to_circuit_type(builder),
-            hint_to_commitments.to_circuit_type(builder),
+            read_commitment_hints.to_circuit_type(builder),
+            nullifier_commitment_hints.to_circuit_type(builder),
         };
 
         return private_inputs;

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp

@@ -4,7 +4,6 @@
 #include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp"
 #include "aztec3/circuits/abis/read_request_membership_witness.hpp"
 #include "aztec3/circuits/apps/test_apps/escrow/deposit.hpp"
-#include "aztec3/circuits/hash.hpp"
 #include "aztec3/circuits/kernel/private/common.hpp"
 #include "aztec3/circuits/kernel/private/init.hpp"
 #include "aztec3/constants.hpp"
@@ -59,8 +58,8 @@ TEST_F(native_private_kernel_tests, native_accumulate_transient_read_requests)
     private_inputs_init.private_call.call_stack_item.public_inputs.read_requests[0] = fr(23);
     private_inputs_init.private_call.read_request_membership_witnesses[0].is_transient = true;
 
-    std::array<fr, MAX_READ_REQUESTS_PER_TX> hint_to_commitments{};
-    hint_to_commitments[0] = fr(1);
+    std::array<fr, MAX_READ_REQUESTS_PER_TX> read_commitment_hints{};
+    read_commitment_hints[0] = fr(1);
 
     DummyBuilder builder = DummyBuilder("native_private_kernel_tests__native_accumulate_transient_read_requests");
     auto public_inputs = native_private_kernel_circuit_initial(builder, private_inputs_init);
@@ -76,7 +75,7 @@ TEST_F(native_private_kernel_tests, native_accumulate_transient_read_requests)
     private_inputs_inner.private_call.call_stack_item.public_inputs.read_requests[0] = fr(12);
     private_inputs_inner.private_call.read_request_membership_witnesses[0].is_transient = true;
 
-    hint_to_commitments[1] = fr(0);
+    read_commitment_hints[1] = fr(0);
 
     // We need to update the previous_kernel's private_call_stack because the current_call_stack_item has changed
     // i.e. we changed the new_commitments and read_requests of the current_call_stack_item's public_inputs
@@ -97,7 +96,7 @@ TEST_F(native_private_kernel_tests, native_accumulate_transient_read_requests)
     auto& previous_kernel = private_inputs_inner.previous_kernel;
     previous_kernel.public_inputs = public_inputs;
 
-    PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, hint_to_commitments };
+    PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, read_commitment_hints };
     auto final_public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs);
 
     ASSERT_FALSE(builder.failed()) << "failure: " << builder.get_first_failure()
@@ -116,8 +115,8 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match)
     private_inputs_init.private_call.call_stack_item.public_inputs.read_requests[0] = fr(23);
     private_inputs_init.private_call.read_request_membership_witnesses[0].is_transient = true;
 
-    std::array<fr, MAX_READ_REQUESTS_PER_TX> hint_to_commitments{};
-    hint_to_commitments[0] = fr(1);
+    std::array<fr, MAX_READ_REQUESTS_PER_TX> read_commitment_hints{};
+    read_commitment_hints[0] = fr(1);
 
     DummyBuilder builder = DummyBuilder("native_private_kernel_tests__native_transient_read_requests_no_match");
     auto public_inputs = native_private_kernel_circuit_initial(builder, private_inputs_init);
@@ -133,7 +132,7 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match)
     private_inputs_inner.private_call.call_stack_item.public_inputs.read_requests[0] = fr(12);
     private_inputs_inner.private_call.read_request_membership_witnesses[0].is_transient = true;
 
-    hint_to_commitments[1] = fr(0);  // There is not correct possible value.
+    read_commitment_hints[1] = fr(0);  // There is not correct possible value.
 
     // We need to update the previous_kernel's private_call_stack because the current_call_stack_item has changed
     // i.e. we changed the new_commitments and read_requests of the current_call_stack_item's public_inputs
@@ -154,7 +153,7 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match)
     auto& previous_kernel = private_inputs_inner.previous_kernel;
     previous_kernel.public_inputs = public_inputs;
 
-    PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, hint_to_commitments };
+    PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, read_commitment_hints };
     auto final_public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs);
 
     ASSERT_TRUE(builder.failed());
@@ -176,7 +175,7 @@ TEST_F(native_private_kernel_tests, native_empty_nullified_commitment_respected)
 
     private_inputs_inner.private_call.call_stack_item.public_inputs.nullified_commitments[0] =
         fr(EMPTY_NULLIFIED_COMMITMENT);
-    private_inputs_inner.private_call.call_stack_item.public_inputs.nullified_commitments[1] = fr(23);
+    private_inputs_inner.private_call.call_stack_item.public_inputs.nullified_commitments[1] = fr(33);
 
     // update the private call stack contents to reflect the above changes which affect the item hash
     private_inputs_inner.previous_kernel.public_inputs.end.private_call_stack[0] =
@@ -203,7 +202,9 @@ TEST_F(native_private_kernel_tests, native_empty_nullified_commitment_respected)
     auto& previous_kernel = private_inputs_inner.previous_kernel;
     previous_kernel.public_inputs = public_inputs;
 
-    PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel };
+    PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel,
+                                                    .nullifier_commitment_hints =
+                                                        std::array<fr, MAX_NEW_NULLIFIERS_PER_TX>{ 0, 1 } };
 
     auto final_public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs);
 

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp

@@ -36,14 +36,14 @@ namespace aztec3::circuits::kernel::private_kernel {
 
 void match_reads_to_commitments(DummyCircuitBuilder& builder,
                                 std::array<NT::fr, MAX_READ_REQUESTS_PER_TX> const& read_requests,
-                                std::array<NT::fr, MAX_READ_REQUESTS_PER_TX> const& hint_to_commitments,
+                                std::array<NT::fr, MAX_READ_REQUESTS_PER_TX> const& read_commitment_hints,
                                 std::array<NT::fr, MAX_NEW_COMMITMENTS_PER_TX> const& new_commitments)
 {
     // match reads to commitments from the previous call(s)
     for (size_t rr_idx = 0; rr_idx < MAX_READ_REQUESTS_PER_TX; rr_idx++) {
         const auto& read_request = read_requests[rr_idx];
-        const auto& hint_to_commitment = hint_to_commitments[rr_idx];
-        const auto hint_pos = static_cast<size_t>(uint64_t(hint_to_commitment));
+        const auto& read_commitment_hint = read_commitment_hints[rr_idx];
+        const auto hint_pos = static_cast<size_t>(uint64_t(read_commitment_hint));
 
         if (read_request != 0) {
             size_t match_pos = MAX_NEW_COMMITMENTS_PER_TX;
@@ -59,7 +59,7 @@ void match_reads_to_commitments(DummyCircuitBuilder& builder,
                        "\n\tread_request: ",
                        read_request,
                        "\n\thint_to_commitment: ",
-                       hint_to_commitment,
+                       read_commitment_hint,
                        "\n\t* the read_request position/index is not expected to match position in app-circuit "
                        "outputs because kernel iterations gradually remove non-transient read_requests as "
                        "membership checks are resolved."),
@@ -92,22 +92,24 @@ void match_nullifiers_to_commitments_and_squash(
     DummyCircuitBuilder& builder,
     std::array<NT::fr, MAX_NEW_NULLIFIERS_PER_TX>& new_nullifiers,
     std::array<NT::fr, MAX_NEW_NULLIFIERS_PER_TX> const& nullified_commitments,
+    std::array<NT::fr, MAX_NEW_NULLIFIERS_PER_TX> const& nullifier_commitment_hints,
     std::array<NT::fr, MAX_NEW_COMMITMENTS_PER_TX>& new_commitments)
 {
-    // match reads to commitments from the previous call(s)
+    // match nullifiers/nullified_commitments to commitments from the previous call(s)
     for (size_t n_idx = 0; n_idx < MAX_NEW_NULLIFIERS_PER_TX; n_idx++) {
+        const auto& nullified_commitment = nullified_commitments[n_idx];
+        const auto& nullifier_commitment_hint = nullifier_commitment_hints[n_idx];
+        const auto hint_pos = static_cast<size_t>(uint64_t(nullifier_commitment_hint));
         // Nullified_commitment of value `EMPTY_NULLIFIED_COMMITMENT` implies non-transient (persistable)
         // nullifier in which case no attempt will be made to match it to a commitment.
         // Non-empty nullified_commitment implies transient nullifier which MUST be matched to a commitment below!
         // 0-valued nullified_commitment is empty and will be ignored
         if (nullified_commitments[n_idx] != NT::fr(0) &&
             nullified_commitments[n_idx] != NT::fr(EMPTY_NULLIFIED_COMMITMENT)) {
             size_t match_pos = MAX_NEW_COMMITMENTS_PER_TX;
-            // TODO(https://github.com/AztecProtocol/aztec-packages/issues/837): inefficient
-            // O(n^2) inner loop will be optimized via matching hints
-            for (size_t c_idx = 0; c_idx < MAX_NEW_COMMITMENTS_PER_TX; c_idx++) {
-                // If there are multiple matches, this picks the last one
-                match_pos = (nullified_commitments[n_idx] == new_commitments[c_idx]) ? c_idx : match_pos;
+
+            if (hint_pos < MAX_NEW_COMMITMENTS_PER_TX) {
+                match_pos = nullified_commitment == new_commitments[hint_pos] ? hint_pos : match_pos;
             }
 
             if (match_pos != MAX_NEW_COMMITMENTS_PER_TX) {
@@ -169,7 +171,7 @@ KernelCircuitPublicInputsFinal<NT> native_private_kernel_circuit_ordering(
     // Remark: The commitments in public_inputs.end have already been siloed by contract address!
     match_reads_to_commitments(builder,
                                private_inputs.previous_kernel.public_inputs.end.read_requests,
-                               private_inputs.hint_to_commitments,
+                               private_inputs.read_commitment_hints,
                                private_inputs.previous_kernel.public_inputs.end.new_commitments);
 
     // Matching nullifiers to pending commitments requires the full list of new commitments accumulated over
@@ -180,6 +182,7 @@ KernelCircuitPublicInputsFinal<NT> native_private_kernel_circuit_ordering(
     match_nullifiers_to_commitments_and_squash(builder,
                                                public_inputs.end.new_nullifiers,
                                                public_inputs.end.nullified_commitments,
+                                               private_inputs.nullifier_commitment_hints,
                                                public_inputs.end.new_commitments);
 
     // tx hash

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp

@@ -222,6 +222,8 @@ TEST_F(native_private_kernel_ordering_tests, native_squash_one_of_one_transient_
     previous_kernel.public_inputs.end.new_nullifiers = new_nullifiers;
     previous_kernel.public_inputs.end.nullified_commitments = nullifier_commitments;
 
+    // Correct nullifier_commitment hint for new_nullifiers[0] == 0 is correct due to the default
+    // initialization of the array.
     PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel };
 
     DummyBuilder builder =
@@ -254,7 +256,9 @@ TEST_F(native_private_kernel_ordering_tests, native_squash_one_of_two_transient_
     previous_kernel.public_inputs.end.new_nullifiers = new_nullifiers;
     previous_kernel.public_inputs.end.nullified_commitments = nullifier_commitments;
 
-    PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel };
+    PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel,
+                                                    .nullifier_commitment_hints =
+                                                        std::array<fr, MAX_NEW_NULLIFIERS_PER_TX>{ 1 } };
 
     DummyBuilder builder =
         DummyBuilder("native_private_kernel_ordering_tests__native_squash_one_of_two_transient_matches_works");
@@ -289,7 +293,9 @@ TEST_F(native_private_kernel_ordering_tests, native_squash_two_of_two_transient_
     previous_kernel.public_inputs.end.new_nullifiers = new_nullifiers;
     previous_kernel.public_inputs.end.nullified_commitments = nullifier_commitments;
 
-    PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel };
+    PrivateKernelInputsOrdering<NT> private_inputs{ .previous_kernel = previous_kernel,
+                                                    .nullifier_commitment_hints =
+                                                        std::array<fr, MAX_NEW_NULLIFIERS_PER_TX>{ 1 } };
 
     DummyBuilder builder =
         DummyBuilder("native_private_kernel_ordering_tests__native_squash_two_of_two_transient_matches_works");

yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts

@@ -4,6 +4,7 @@ import {
   CONTRACT_TREE_HEIGHT,
   Fr,
   MAX_NEW_COMMITMENTS_PER_TX,
+  MAX_NEW_NULLIFIERS_PER_TX,
   MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL,
   MAX_READ_REQUESTS_PER_CALL,
   MAX_READ_REQUESTS_PER_TX,
@@ -22,6 +23,7 @@ import {
   makeEmptyProof,
   makeTuple,
 } from '@aztec/circuits.js';
+import { EMPTY_NULLIFIED_COMMITMENT } from '@aztec/circuits.js';
 import { Tuple, assertLength } from '@aztec/foundation/serialize';
 
 import { KernelProofCreator, ProofCreator, ProofOutput, ProofOutputFinal } from './proof_creator.js';
@@ -171,11 +173,21 @@ export class KernelProver {
       assertLength<Fr, typeof VK_TREE_HEIGHT>(previousVkMembershipWitness.siblingPath, VK_TREE_HEIGHT),
     );
 
-    const hintToCommitments = this.getReadRequestHints(
+    const readCommitmentHints = this.getReadRequestHints(
       output.publicInputs.end.readRequests,
       output.publicInputs.end.newCommitments,
     );
-    const privateInputs = new PrivateKernelInputsOrdering(previousKernelData, hintToCommitments);
+
+    const nullifierCommitmentHints = this.getNullifierHints(
+      output.publicInputs.end.nullifiedCommitments,
+      output.publicInputs.end.newCommitments,
+    );
+
+    const privateInputs = new PrivateKernelInputsOrdering(
+      previousKernelData,
+      readCommitmentHints,
+      nullifierCommitmentHints,
+    );
     const outputFinal = await this.proofCreator.createProofOrdering(privateInputs);
 
     // Only return the notes whose commitment is in the commitments of the final proof.
@@ -246,6 +258,16 @@ export class KernelProver {
     }));
   }
 
+  /**
+   * Performs the matching between an array of read request and an array of commitments. This produces
+   * hints for the private kernel ordering circuit to efficiently match a read request with the corresponding
+   * commitment.
+   *
+   * @param readRequests - The array of read requests.
+   * @param commitments - The array of commitments.
+   * @returns An array of hints where each element is the index of the commitment in commitments array
+   *  corresponding to the read request. In other words we have readRequests[i] == commitments[hints[i]].
+   */
   private getReadRequestHints(
     readRequests: Tuple<Fr, typeof MAX_READ_REQUESTS_PER_TX>,
     commitments: Tuple<Fr, typeof MAX_NEW_COMMITMENTS_PER_TX>,
@@ -264,4 +286,37 @@ export class KernelProver {
     }
     return hints;
   }
+
+  /**
+   *  Performs the matching between an array of nullified commitments and an array of commitments. This produces
+   * hints for the private kernel ordering circuit to efficiently match a nullifier with the corresponding
+   * commitment.
+   *
+   * @param nullifiedCommitments - The array of nullified commitments.
+   * @param commitments - The array of commitments.
+   * @returns An array of hints where each element is the index of the commitment in commitments array
+   *  corresponding to the nullified commitments. In other words we have nullifiedCommitments[i] == commitments[hints[i]].
+   */
+  private getNullifierHints(
+    nullifiedCommitments: Tuple<Fr, typeof MAX_NEW_NULLIFIERS_PER_TX>,
+    commitments: Tuple<Fr, typeof MAX_NEW_COMMITMENTS_PER_TX>,
+  ): Tuple<Fr, typeof MAX_NEW_NULLIFIERS_PER_TX> {
+    const hints = makeTuple(MAX_NEW_NULLIFIERS_PER_TX, Fr.zero);
+    for (let i = 0; i < MAX_NEW_NULLIFIERS_PER_TX; i++) {
+      if (!nullifiedCommitments[i].isZero() && !nullifiedCommitments[i].equals(new Fr(EMPTY_NULLIFIED_COMMITMENT))) {
+        const equalToCommitment = (cmt: Fr) => cmt.equals(nullifiedCommitments[i]);
+        const result = commitments.findIndex(equalToCommitment);
+        if (result == -1) {
+          throw new Error(
+            `The nullified commitment at index ${i} with value ${nullifiedCommitments[
+              i
+            ].toString()} does not match to any commitment.`,
+          );
+        } else {
+          hints[i] = new Fr(result);
+        }
+      }
+    }
+    return hints;
+  }
 }