Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(892): add hints for matching transient read requests with correspondi… #1995

Merged
merged 2 commits into from
Sep 5, 2023
Merged

feat(892): add hints for matching transient read requests with correspondi… #1995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae947ec
892 - add hints for matching transient read requests with correspondi…
jeanmon Sep 5, 2023
742437d
892 - fix C++ tests
jeanmon Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<fr, MAX_READ_REQUESTS_PER_TX> 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);
Expand All @@ -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
Expand Down Expand Up @@ -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<fr, MAX_READ_REQUESTS_PER_TX> 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);
Expand All @@ -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
Expand Down
13 changes: 4 additions & 9 deletions circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "aztec3/utils/circuit_errors.hpp"
#include "aztec3/utils/dummy_circuit_builder.hpp"

#include <cstddef>
#include <barretenberg/numeric/uint256/uint256.hpp>

namespace {
using NT = aztec3::utils::types::NativeTypes;
Expand All @@ -34,9 +34,6 @@ void initialise_end_values(PreviousKernelData<NT> 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<NT::fr, MAX_READ_REQUESTS_PER_TX> const& read_requests,
std::array<NT::fr, MAX_READ_REQUESTS_PER_TX> const& hint_to_commitments,
Expand All @@ -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<size_t>(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) {
suyash67 marked this conversation as resolved.
Show resolved Hide resolved
match_pos = read_request == new_commitments[hint_pos] ? hint_pos : match_pos;
}

// Transient reads MUST match a pending commitment
suyash67 marked this conversation as resolved.
Show resolved Hide resolved
builder.do_assert(
match_pos != MAX_NEW_COMMITMENTS_PER_TX,
format("read_request at position [",
Expand Down
20 changes: 14 additions & 6 deletions ...ts/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_one_read_request_to
std::array<fr, MAX_NEW_COMMITMENTS_PER_TX> siloed_commitments{};
std::array<fr, MAX_NEW_COMMITMENTS_PER_TX> unique_siloed_commitments{};
std::array<fr, MAX_READ_REQUESTS_PER_TX> read_requests{};
std::array<fr, MAX_READ_REQUESTS_PER_TX> hints{};

std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>, MAX_READ_REQUESTS_PER_TX>
read_request_membership_witnesses{};

Expand All @@ -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<NT>(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;
Expand All @@ -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<NT> private_inputs{ previous_kernel, std::array<fr, MAX_READ_REQUESTS_PER_TX>{} };
PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, hints };

DummyBuilder builder =
DummyBuilder("native_private_kernel_ordering_tests__native_matching_one_read_request_to_commitment_works");
Expand All @@ -77,6 +80,8 @@ TEST_F(native_private_kernel_ordering_tests, native_matching_some_read_requests_
std::array<fr, MAX_NEW_COMMITMENTS_PER_TX> siloed_commitments{};
std::array<fr, MAX_NEW_COMMITMENTS_PER_TX> unique_siloed_commitments{};
std::array<fr, MAX_READ_REQUESTS_PER_TX> read_requests{};
std::array<fr, MAX_READ_REQUESTS_PER_TX> hints{};

std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>, MAX_READ_REQUESTS_PER_TX>
read_request_membership_witnesses{};

Expand All @@ -96,14 +101,16 @@ 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;

previous_kernel.public_inputs.end.new_nullifiers = new_nullifiers;
previous_kernel.public_inputs.end.new_commitments = siloed_commitments;
previous_kernel.public_inputs.end.read_requests = read_requests;

PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, std::array<fr, MAX_READ_REQUESTS_PER_TX>{} };
PrivateKernelInputsOrdering<NT> private_inputs{ previous_kernel, hints };

DummyBuilder builder =
DummyBuilder("native_private_kernel_ordering_tests__native_matching_some_read_requests_to_commitments_works");
Expand All @@ -123,13 +130,14 @@ TEST_F(native_private_kernel_ordering_tests, native_read_request_unknown_fails)

std::array<fr, MAX_NEW_COMMITMENTS_PER_TX> siloed_commitments{};
std::array<fr, MAX_READ_REQUESTS_PER_TX> read_requests{};
std::array<fr, MAX_READ_REQUESTS_PER_TX> hints{};
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>, 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
Expand All @@ -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<NT> private_inputs{ previous_kernel, std::array<fr, MAX_READ_REQUESTS_PER_TX>{} };
PrivateKernelInputsOrdering<NT> 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);
Expand Down
29 changes: 25 additions & 4 deletions yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -169,6 +167,10 @@ export class KernelProver {
assertLength<Fr, typeof VK_TREE_HEIGHT>(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);

Expand Down Expand Up @@ -239,4 +241,23 @@ export class KernelProver {
commitment: newCommitments[i],
}));
}

private getReadRequestHints(
readRequests: Tuple<Fr, typeof MAX_READ_REQUESTS_PER_TX>,
commitments: Tuple<Fr, typeof MAX_NEW_COMMITMENTS_PER_TX>,
): Tuple<Fr, typeof MAX_READ_REQUESTS_PER_TX> {
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;
}
}