From ae947ec98cc303f1d9aa35960fbbb90803159955 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Tue, 5 Sep 2023 07:01:03 +0000 Subject: [PATCH 1/2] 892 - add hints for matching transient read requests with corresponding commitments --- .../private_kernel_inputs_ordering.hpp | 2 ++ .../aztec3/circuits/kernel/private/common.cpp | 1 - ...native_private_kernel_circuit_ordering.cpp | 14 ++++----- .../src/kernel_prover/kernel_prover.ts | 29 ++++++++++++++++--- 4 files changed, 33 insertions(+), 13 deletions(-) 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..a0f3e0d6af0 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 @@ -8,6 +8,8 @@ #include +#include + namespace aztec3::circuits::abis::private_kernel { using aztec3::utils::types::CircuitTypes; diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp index c88ed739a08..5e2fc9208de 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp @@ -22,7 +22,6 @@ using aztec3::circuits::abis::KernelCircuitPublicInputs; using aztec3::circuits::abis::NewContractData; using aztec3::circuits::abis::ReadRequestMembershipWitness; -using aztec3::utils::array_length; using aztec3::utils::array_push; using aztec3::utils::is_array_empty; using aztec3::utils::push_array_to_array; 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 78831b63cef..eec3e518500 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 @@ -10,7 +10,10 @@ #include "aztec3/utils/circuit_errors.hpp" #include "aztec3/utils/dummy_circuit_builder.hpp" +#include + #include +#include namespace { using NT = aztec3::utils::types::NativeTypes; @@ -34,9 +37,6 @@ void initialise_end_values(PreviousKernelData const& previous_kernel, namespace aztec3::circuits::kernel::private_kernel { -// TODO(https://github.com/AztecProtocol/aztec-packages/issues/892): optimized based on hints -// regarding matching a read request to a commitment -// i.e., we get pairs i,j such that read_requests[i] == new_commitments[j] void match_reads_to_commitments(DummyCircuitBuilder& builder, std::array const& read_requests, std::array const& hint_to_commitments, @@ -46,16 +46,14 @@ void match_reads_to_commitments(DummyCircuitBuilder& builder, 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)); if (read_request != 0) { size_t match_pos = MAX_NEW_COMMITMENTS_PER_TX; - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/892): 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++) { - match_pos = (read_request == new_commitments[c_idx]) ? c_idx : match_pos; + if (hint_pos < MAX_NEW_COMMITMENTS_PER_TX) { + match_pos = read_request == new_commitments[hint_pos] ? hint_pos : match_pos; } - // Transient reads MUST match a pending commitment builder.do_assert( match_pos != MAX_NEW_COMMITMENTS_PER_TX, format("read_request at position [", 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 2573c7f6afc..716538ffb2b 100644 --- a/yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts +++ b/yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts @@ -3,6 +3,7 @@ import { AztecAddress, CONTRACT_TREE_HEIGHT, Fr, + MAX_NEW_COMMITMENTS_PER_TX, MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL, MAX_READ_REQUESTS_PER_CALL, MAX_READ_REQUESTS_PER_TX, @@ -19,7 +20,7 @@ import { makeEmptyProof, makeTuple, } from '@aztec/circuits.js'; -import { assertLength } from '@aztec/foundation/serialize'; +import { Tuple, assertLength } from '@aztec/foundation/serialize'; import { KernelProofCreator, ProofCreator, ProofOutput, ProofOutputFinal } from './proof_creator.js'; import { ProvingDataOracle } from './proving_data_oracle.js'; @@ -85,9 +86,6 @@ export class KernelProver { proof: makeEmptyProof(), }; - //TODO(#892): Dealing with this ticket we will fill the following hint array with the correct hints. - const hintToCommitments = makeTuple(MAX_READ_REQUESTS_PER_TX, Fr.zero); - while (executionStack.length) { const currentExecution = executionStack.pop()!; executionStack.push(...currentExecution.nestedExecutions); @@ -169,6 +167,10 @@ export class KernelProver { assertLength(previousVkMembershipWitness.siblingPath, VK_TREE_HEIGHT), ); + const hintToCommitments = this.getReadRequestHints( + output.publicInputs.end.readRequests, + output.publicInputs.end.newCommitments, + ); const privateInputs = new PrivateKernelInputsOrdering(previousKernelData, hintToCommitments); const outputFinal = await this.proofCreator.createProofOrdering(privateInputs); @@ -239,4 +241,23 @@ export class KernelProver { commitment: newCommitments[i], })); } + + private getReadRequestHints( + readRequests: Tuple, + commitments: Tuple, + ): Tuple { + const hints = makeTuple(MAX_READ_REQUESTS_PER_TX, Fr.zero); + for (let i = 0; i < MAX_READ_REQUESTS_PER_TX && !readRequests[i].isZero(); i++) { + const equalToRR = (cmt: Fr) => cmt.equals(readRequests[i]); + const result = commitments.findIndex(equalToRR); + if (result == -1) { + throw new Error( + `The read request at index ${i} with value ${readRequests[i].toString()} does not match to any commitment.`, + ); + } else { + hints[i] = new Fr(result); + } + } + return hints; + } } From 742437d5e6aec88f951eb4b3e68880a600a89b0e Mon Sep 17 00:00:00 2001 From: jeanmon Date: Tue, 5 Sep 2023 08:21:38 +0000 Subject: [PATCH 2/2] 892 - fix C++ tests --- .../private_kernel_inputs_ordering.hpp | 2 -- .../native_private_kernel_circuit.test.cpp | 8 ++++---- ...native_private_kernel_circuit_ordering.cpp | 3 --- ...e_private_kernel_circuit_ordering.test.cpp | 20 +++++++++++++------ 4 files changed, 18 insertions(+), 15 deletions(-) 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 a0f3e0d6af0..890da885bbe 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 @@ -8,8 +8,6 @@ #include -#include - namespace aztec3::circuits::abis::private_kernel { using aztec3::utils::types::CircuitTypes; 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 14fe520d7dc..ab4bf57e068 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 @@ -60,7 +60,7 @@ TEST_F(native_private_kernel_tests, native_accumulate_transient_read_requests) private_inputs_init.private_call.read_request_membership_witnesses[0].is_transient = true; std::array hint_to_commitments{}; - hint_to_commitments[0] = private_inputs_init.private_call.read_request_membership_witnesses[0].hint_to_commitment; + hint_to_commitments[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 +76,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] = private_inputs_inner.private_call.read_request_membership_witnesses[0].hint_to_commitment; + hint_to_commitments[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 @@ -117,7 +117,7 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match) private_inputs_init.private_call.read_request_membership_witnesses[0].is_transient = true; std::array hint_to_commitments{}; - hint_to_commitments[0] = private_inputs_init.private_call.read_request_membership_witnesses[0].hint_to_commitment; + hint_to_commitments[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 +133,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] = private_inputs_inner.private_call.read_request_membership_witnesses[0].hint_to_commitment; + hint_to_commitments[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 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 eec3e518500..03300df678f 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 @@ -12,9 +12,6 @@ #include -#include -#include - namespace { using NT = aztec3::utils::types::NativeTypes; 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 e0db864ce1d..2d075001fd5 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 @@ -39,6 +39,8 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_one_read_request_to std::array siloed_commitments{}; std::array unique_siloed_commitments{}; std::array read_requests{}; + std::array hints{}; + std::array, MAX_READ_REQUESTS_PER_TX> read_request_membership_witnesses{}; @@ -50,6 +52,7 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_one_read_request_to siloed_commitments[0] == 0 ? 0 : compute_unique_commitment(nonce, siloed_commitments[0]); read_requests[0] = siloed_commitments[0]; + // hints[0] == fr(0) due to the default initialization of hints read_request_membership_witnesses[0].is_transient = true; auto& previous_kernel = private_inputs_inner.previous_kernel; @@ -58,7 +61,7 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_one_read_request_to previous_kernel.public_inputs.end.new_commitments = siloed_commitments; previous_kernel.public_inputs.end.read_requests = read_requests; - PrivateKernelInputsOrdering private_inputs{ previous_kernel, std::array{} }; + PrivateKernelInputsOrdering private_inputs{ previous_kernel, hints }; DummyBuilder builder = DummyBuilder("native_private_kernel_ordering_tests__native_matching_one_read_request_to_commitment_works"); @@ -77,6 +80,8 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_some_read_requests_ std::array siloed_commitments{}; std::array unique_siloed_commitments{}; std::array read_requests{}; + std::array hints{}; + std::array, MAX_READ_REQUESTS_PER_TX> read_request_membership_witnesses{}; @@ -96,6 +101,8 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_some_read_requests_ read_requests[1] = siloed_commitments[3]; read_request_membership_witnesses[0].is_transient = true; read_request_membership_witnesses[1].is_transient = true; + hints[0] = fr(1); + hints[1] = fr(3); auto& previous_kernel = private_inputs_inner.previous_kernel; @@ -103,7 +110,7 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_some_read_requests_ previous_kernel.public_inputs.end.new_commitments = siloed_commitments; previous_kernel.public_inputs.end.read_requests = read_requests; - PrivateKernelInputsOrdering private_inputs{ previous_kernel, std::array{} }; + PrivateKernelInputsOrdering private_inputs{ previous_kernel, hints }; DummyBuilder builder = DummyBuilder("native_private_kernel_ordering_tests__native_matching_some_read_requests_to_commitments_works"); @@ -123,13 +130,14 @@ TEST_F(native_private_kernel_ordering_tests, native_read_request_unknown_fails) std::array siloed_commitments{}; std::array read_requests{}; + std::array hints{}; std::array, MAX_READ_REQUESTS_PER_TX> read_request_membership_witnesses{}; for (size_t c_idx = 0; c_idx < MAX_NEW_COMMITMENTS_PER_TX; c_idx++) { - siloed_commitments[c_idx] = NT::fr::random_element(); // create random commitment - read_requests[c_idx] = siloed_commitments[c_idx]; // create random read requests - // ^ will match each other! + siloed_commitments[c_idx] = NT::fr::random_element(); // create random commitment + read_requests[c_idx] = siloed_commitments[c_idx]; // create random read requests + hints[c_idx] = fr(c_idx); // ^ will match each other! read_request_membership_witnesses[c_idx].is_transient = true; // ordering circuit only allows transient reads } read_requests[3] = NT::fr::random_element(); // force one read request not to match @@ -139,7 +147,7 @@ TEST_F(native_private_kernel_ordering_tests, native_read_request_unknown_fails) previous_kernel.public_inputs.end.new_commitments = siloed_commitments; previous_kernel.public_inputs.end.read_requests = read_requests; - PrivateKernelInputsOrdering private_inputs{ previous_kernel, std::array{} }; + PrivateKernelInputsOrdering private_inputs{ previous_kernel, hints }; DummyBuilder builder = DummyBuilder("native_private_kernel_ordering_tests__native_read_request_unknown_fails"); native_private_kernel_circuit_ordering(builder, private_inputs);