diff --git a/circuits/cpp/src/aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp b/circuits/cpp/src/aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp index 890da885bbe..2cfd6d2b643 100644 --- a/circuits/cpp/src/aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp +++ b/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 struct PrivateKernelInputsOrdering { PreviousKernelData previous_kernel{}; - std::array hint_to_commitments{}; + std::array read_commitment_hints{}; + std::array 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 const& other) const { return msgpack_derived_equals(*this, other); @@ -36,7 +38,8 @@ template struct PrivateKernelInputsOrdering { PrivateKernelInputsOrdering> 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; diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp index ab4bf57e068..f9e0aef3d2f 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp +++ b/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 hint_to_commitments{}; - hint_to_commitments[0] = fr(1); + std::array 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 private_inputs{ previous_kernel, hint_to_commitments }; + PrivateKernelInputsOrdering 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 hint_to_commitments{}; - hint_to_commitments[0] = fr(1); + std::array 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 private_inputs{ previous_kernel, hint_to_commitments }; + PrivateKernelInputsOrdering 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 private_inputs{ .previous_kernel = previous_kernel }; + PrivateKernelInputsOrdering private_inputs{ .previous_kernel = previous_kernel, + .nullifier_commitment_hints = + std::array{ 0, 1 } }; auto final_public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs); diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp index 03300df678f..c1fd7362e45 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp +++ b/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 const& read_requests, - std::array const& hint_to_commitments, + std::array const& read_commitment_hints, std::array 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(uint64_t(hint_to_commitment)); + const auto& read_commitment_hint = read_commitment_hints[rr_idx]; + const auto hint_pos = static_cast(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,10 +92,14 @@ void match_nullifiers_to_commitments_and_squash( DummyCircuitBuilder& builder, std::array& new_nullifiers, std::array const& nullified_commitments, + std::array const& nullifier_commitment_hints, std::array& 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(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! @@ -103,11 +107,9 @@ void match_nullifiers_to_commitments_and_squash( 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 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 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 diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp index e1ef0464373..829366423c8 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp +++ b/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 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 private_inputs{ .previous_kernel = previous_kernel }; + PrivateKernelInputsOrdering private_inputs{ .previous_kernel = previous_kernel, + .nullifier_commitment_hints = + std::array{ 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 private_inputs{ .previous_kernel = previous_kernel }; + PrivateKernelInputsOrdering private_inputs{ .previous_kernel = previous_kernel, + .nullifier_commitment_hints = + std::array{ 1 } }; DummyBuilder builder = DummyBuilder("native_private_kernel_ordering_tests__native_squash_two_of_two_transient_matches_works"); diff --git a/yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts b/yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts index 8699ce23582..d85bf17a0df 100644 --- a/yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts +++ b/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(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, commitments: Tuple, @@ -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, + commitments: Tuple, + ): Tuple { + 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; + } } diff --git a/yarn-project/circuits.js/src/cbind/circuits.gen.ts b/yarn-project/circuits.js/src/cbind/circuits.gen.ts index f74f44d2121..008f942f0c8 100644 --- a/yarn-project/circuits.js/src/cbind/circuits.gen.ts +++ b/yarn-project/circuits.js/src/cbind/circuits.gen.ts @@ -1675,19 +1675,24 @@ export function fromPrivateKernelInputsInner(o: PrivateKernelInputsInner): Msgpa interface MsgpackPrivateKernelInputsOrdering { previous_kernel: MsgpackPreviousKernelData; - hint_to_commitments: Tuple; + read_commitment_hints: Tuple; + nullifier_commitment_hints: Tuple; } export function toPrivateKernelInputsOrdering(o: MsgpackPrivateKernelInputsOrdering): PrivateKernelInputsOrdering { if (o.previous_kernel === undefined) { throw new Error('Expected previous_kernel in PrivateKernelInputsOrdering deserialization'); } - if (o.hint_to_commitments === undefined) { - throw new Error('Expected hint_to_commitments in PrivateKernelInputsOrdering deserialization'); + if (o.read_commitment_hints === undefined) { + throw new Error('Expected read_commitment_hints in PrivateKernelInputsOrdering deserialization'); + } + if (o.nullifier_commitment_hints === undefined) { + throw new Error('Expected nullifier_commitment_hints in PrivateKernelInputsOrdering deserialization'); } return new PrivateKernelInputsOrdering( toPreviousKernelData(o.previous_kernel), - mapTuple(o.hint_to_commitments, (v: Buffer) => Fr.fromBuffer(v)), + mapTuple(o.read_commitment_hints, (v: Buffer) => Fr.fromBuffer(v)), + mapTuple(o.nullifier_commitment_hints, (v: Buffer) => Fr.fromBuffer(v)), ); } @@ -1695,12 +1700,16 @@ export function fromPrivateKernelInputsOrdering(o: PrivateKernelInputsOrdering): if (o.previousKernel === undefined) { throw new Error('Expected previousKernel in PrivateKernelInputsOrdering serialization'); } - if (o.hintToCommitments === undefined) { - throw new Error('Expected hintToCommitments in PrivateKernelInputsOrdering serialization'); + if (o.readCommitmentHints === undefined) { + throw new Error('Expected readCommitmentHints in PrivateKernelInputsOrdering serialization'); + } + if (o.nullifierCommitmentHints === undefined) { + throw new Error('Expected nullifierCommitmentHints in PrivateKernelInputsOrdering serialization'); } return { previous_kernel: fromPreviousKernelData(o.previousKernel), - hint_to_commitments: mapTuple(o.hintToCommitments, (v: Fr) => toBuffer(v)), + read_commitment_hints: mapTuple(o.readCommitmentHints, (v: Fr) => toBuffer(v)), + nullifier_commitment_hints: mapTuple(o.nullifierCommitmentHints, (v: Fr) => toBuffer(v)), }; } diff --git a/yarn-project/circuits.js/src/structs/kernel/private_kernel.ts b/yarn-project/circuits.js/src/structs/kernel/private_kernel.ts index 655e5038592..d75b2bca6d1 100644 --- a/yarn-project/circuits.js/src/structs/kernel/private_kernel.ts +++ b/yarn-project/circuits.js/src/structs/kernel/private_kernel.ts @@ -3,6 +3,7 @@ import { Tuple } from '@aztec/foundation/serialize'; import { CONTRACT_TREE_HEIGHT, FUNCTION_TREE_HEIGHT, + MAX_NEW_NULLIFIERS_PER_TX, MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL, MAX_READ_REQUESTS_PER_CALL, MAX_READ_REQUESTS_PER_TX, @@ -159,7 +160,11 @@ export class PrivateKernelInputsOrdering { /** * Contains hints for the transient read requests to localize corresponding commitments. */ - public hintToCommitments: Tuple, + public readCommitmentHints: Tuple, + /** + * Contains hints for the transient nullifiers to localize corresponding commitments. + */ + public nullifierCommitmentHints: Tuple, ) {} /** @@ -167,6 +172,6 @@ export class PrivateKernelInputsOrdering { * @returns The buffer. */ toBuffer() { - return serializeToBuffer(this.previousKernel, this.hintToCommitments); + return serializeToBuffer(this.previousKernel, this.readCommitmentHints); } }