From 0955bb7b0903b12c4f041096d51a1dbb48f6359d Mon Sep 17 00:00:00 2001 From: Jean M <132435771+jeanmon@users.noreply.github.com> Date: Tue, 5 Sep 2023 11:39:18 +0200 Subject: [PATCH] =?UTF-8?q?feat(892):=20add=20hints=20for=20matching=20tra?= =?UTF-8?q?nsient=20read=20requests=20with=20correspondi=E2=80=A6=20(#1995?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add hints for matching transient read requests with corresponding commitments. Resolves #892 # 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). --- .../aztec3/circuits/kernel/private/common.cpp | 1 - .../native_private_kernel_circuit.test.cpp | 8 ++--- ...native_private_kernel_circuit_ordering.cpp | 13 +++------ ...e_private_kernel_circuit_ordering.test.cpp | 20 +++++++++---- .../src/kernel_prover/kernel_prover.ts | 29 ++++++++++++++++--- 5 files changed, 47 insertions(+), 24 deletions(-) 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.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 78831b63cef..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 @@ -10,7 +10,7 @@ #include "aztec3/utils/circuit_errors.hpp" #include "aztec3/utils/dummy_circuit_builder.hpp" -#include +#include namespace { using NT = aztec3::utils::types::NativeTypes; @@ -34,9 +34,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 +43,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/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); 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; + } }