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.

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

refactor(1407): remove forwarding witnesses #1930

Merged
merged 7 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ template <typename NCT> struct CombinedAccumulatedData {
AggregationObject aggregation_object{};

std::array<fr, MAX_READ_REQUESTS_PER_TX> read_requests{};
std::array<ReadRequestMembershipWitness<NCT, PRIVATE_DATA_TREE_HEIGHT>, MAX_READ_REQUESTS_PER_TX>
read_request_membership_witnesses{};

std::array<fr, MAX_NEW_COMMITMENTS_PER_TX> new_commitments{};
std::array<fr, MAX_NEW_NULLIFIERS_PER_TX> new_nullifiers{};
Expand Down Expand Up @@ -62,7 +60,6 @@ template <typename NCT> struct CombinedAccumulatedData {
// for serialization, update with new fields
MSGPACK_FIELDS(aggregation_object,
read_requests,
read_request_membership_witnesses,
new_commitments,
new_nullifiers,
nullified_commitments,
Expand All @@ -79,18 +76,7 @@ template <typename NCT> struct CombinedAccumulatedData {
public_data_reads);
boolean operator==(CombinedAccumulatedData<NCT> const& other) const
{
return aggregation_object == other.aggregation_object && read_requests == other.read_requests &&
read_request_membership_witnesses == other.read_request_membership_witnesses &&
new_commitments == other.new_commitments && new_nullifiers == other.new_nullifiers &&
nullified_commitments == other.nullified_commitments && private_call_stack == other.private_call_stack &&
public_call_stack == other.public_call_stack && new_l2_to_l1_msgs == other.new_l2_to_l1_msgs &&
encrypted_logs_hash == other.encrypted_logs_hash &&
unencrypted_logs_hash == other.unencrypted_logs_hash &&
encrypted_log_preimages_length == other.encrypted_log_preimages_length &&
unencrypted_log_preimages_length == other.unencrypted_log_preimages_length &&
new_contracts == other.new_contracts && optionally_revealed_data == other.optionally_revealed_data &&
public_data_update_requests == other.public_data_update_requests &&
public_data_reads == other.public_data_reads;
return msgpack_derived_equals<boolean>(*this, other);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for applying this refactoring pattern :D

};

template <typename Builder> CombinedAccumulatedData<CircuitTypes<Builder>> to_circuit_type(Builder& builder) const
Expand All @@ -112,7 +98,6 @@ template <typename NCT> struct CombinedAccumulatedData {
},

to_ct(read_requests),
map(read_request_membership_witnesses, to_circuit_type),

to_ct(new_commitments),
to_ct(new_nullifiers),
Expand Down Expand Up @@ -153,7 +138,6 @@ template <typename NCT> struct CombinedAccumulatedData {
},

to_nt(read_requests),
map(read_request_membership_witnesses, to_native_type),

to_nt(new_commitments),
to_nt(new_nullifiers),
Expand Down Expand Up @@ -184,7 +168,6 @@ template <typename NCT> struct CombinedAccumulatedData {
aggregation_object.add_proof_outputs_as_public_inputs();

set_array_public(read_requests);
set_array_public(read_request_membership_witnesses);

set_array_public(new_commitments);
set_array_public(new_nullifiers);
Expand All @@ -211,15 +194,6 @@ template <typename NCT> struct CombinedAccumulatedData {
}
}

template <size_t SIZE>
void set_array_public(std::array<ReadRequestMembershipWitness<NCT, PRIVATE_DATA_TREE_HEIGHT>, SIZE>& arr)
{
static_assert(!(std::is_same<NativeTypes, NCT>::value));
for (auto& e : arr) {
e.set_public();
}
}

template <size_t SIZE> void set_array_public(std::array<OptionallyRevealedData<NCT>, SIZE>& arr)
{
static_assert(!(std::is_same<NativeTypes, NCT>::value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ template <typename NCT> struct PrivateKernelInputsInit {
MSGPACK_FIELDS(tx_request, private_call);
boolean operator==(PrivateKernelInputsInit<NCT> const& other) const
{
return tx_request == other.tx_request && private_call == other.private_call;
return msgpack_derived_equals<boolean>(*this, other);
};

template <typename Builder> PrivateKernelInputsInit<CircuitTypes<Builder>> to_circuit_type(Builder& builder) const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ template <typename NCT> struct PrivateKernelInputsInner {
MSGPACK_FIELDS(previous_kernel, private_call);
boolean operator==(PrivateKernelInputsInner<NCT> const& other) const
{
return previous_kernel == other.previous_kernel && private_call == other.private_call;
return msgpack_derived_equals<boolean>(*this, other);
};

template <typename Builder> PrivateKernelInputsInner<CircuitTypes<Builder>> to_circuit_type(Builder& builder) const
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#pragma once

#include "private_call_data.hpp"
#include "../previous_kernel_data.hpp"

#include "aztec3/utils/types/circuit_types.hpp"
#include "aztec3/utils/types/native_types.hpp"

#include <barretenberg/barretenberg.hpp>

namespace aztec3::circuits::abis::private_kernel {

using aztec3::utils::types::CircuitTypes;
using aztec3::utils::types::NativeTypes;
using std::is_same;

template <typename NCT> struct PrivateKernelInputsOrdering {
using fr = typename NCT::fr;
using boolean = typename NCT::boolean;

PreviousKernelData<NCT> previous_kernel{};

std::array<fr, MAX_READ_REQUESTS_PER_TX> hint_to_commitments{};

// For serialization, update with new fields
MSGPACK_FIELDS(previous_kernel, hint_to_commitments);
boolean operator==(PrivateKernelInputsOrdering<NCT> const& other) const
{
return msgpack_derived_equals<boolean>(*this, other);
};

template <typename Builder>
PrivateKernelInputsOrdering<CircuitTypes<Builder>> to_circuit_type(Builder& builder) const
{
static_assert((std::is_same<NativeTypes, NCT>::value));

PrivateKernelInputsOrdering<CircuitTypes<Builder>> private_inputs = {
previous_kernel.to_circuit_type(builder),
hint_to_commitments.to_circuit_type(builder),
};

return private_inputs;
};
};

} // namespace aztec3::circuits::abis::private_kernel
7 changes: 5 additions & 2 deletions circuits/cpp/src/aztec3/circuits/kernel/private/c_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#include "aztec3/circuits/abis/kernel_circuit_public_inputs.hpp"
#include "aztec3/circuits/abis/previous_kernel_data.hpp"
#include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_inner.hpp"
#include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp"
#include "aztec3/constants.hpp"

#include <barretenberg/barretenberg.hpp>
Expand All @@ -20,6 +22,7 @@ using aztec3::circuits::abis::TxRequest;
using aztec3::circuits::abis::private_kernel::PrivateCallData;
using aztec3::circuits::abis::private_kernel::PrivateKernelInputsInit;
using aztec3::circuits::abis::private_kernel::PrivateKernelInputsInner;
using aztec3::circuits::abis::private_kernel::PrivateKernelInputsOrdering;
using aztec3::circuits::kernel::private_kernel::native_private_kernel_circuit_initial;
using aztec3::circuits::kernel::private_kernel::native_private_kernel_circuit_inner;
using aztec3::circuits::kernel::private_kernel::native_private_kernel_circuit_ordering;
Expand Down Expand Up @@ -123,8 +126,8 @@ WASM_EXPORT uint8_t* private_kernel__sim_inner(uint8_t const* previous_kernel_bu
return builder.alloc_and_serialize_first_failure();
}

CBIND(private_kernel__sim_ordering, [](PreviousKernelData<NT> previous_kernel) {
CBIND(private_kernel__sim_ordering, [](PrivateKernelInputsOrdering<NT> private_inputs) {
DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_ordering");
auto const& public_inputs = native_private_kernel_circuit_ordering(builder, previous_kernel);
auto const& public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs);
return builder.result_or_error(public_inputs);
});
48 changes: 0 additions & 48 deletions circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,6 @@ void common_validate_read_requests(DummyBuilder& builder,
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>,
MAX_READ_REQUESTS_PER_CALL> const& read_request_membership_witnesses)
{
// Arrays read_request and read_request_membership_witnesses must be of the same length. Otherwise,
// we might get into trouble when accumulating them in public_inputs.end
builder.do_assert(array_length(read_requests) == array_length(read_request_membership_witnesses),
format("[private kernel circuit] mismatch array length between read_requests and witnesses - "
"read_requests length: ",
array_length(read_requests),
" witnesses length: ",
array_length(read_request_membership_witnesses)),
CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_WITNESSES_ARRAY_LENGTH_MISMATCH);

// membership witnesses must resolve to the same private data root
// for every request in all kernel iterations
for (size_t rr_idx = 0; rr_idx < aztec3::MAX_READ_REQUESTS_PER_CALL; rr_idx++) {
Expand Down Expand Up @@ -124,39 +114,6 @@ void common_validate_read_requests(DummyBuilder& builder,
}
}


/**
* @brief Ensure that all read requests from previous kernel are transient.
*
* @param builder
* @param read_requests from previous kernel's public inputs
* @param read_request_membership_witnesses from previous kernel's public inputs
*/
void common_validate_previous_kernel_read_requests(
DummyBuilder& builder,
std::array<NT::fr, MAX_READ_REQUESTS_PER_TX> const& read_requests,
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>, MAX_READ_REQUESTS_PER_TX> const&
read_request_membership_witnesses)
{
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& witness = read_request_membership_witnesses[rr_idx];
builder.do_assert(read_request == 0 || witness.is_transient, // rr == 0 means empty
format("Previous kernel's read request[",
rr_idx,
"] is not transient, but kernel should only forward transient reads.",
"\n\tread_request: ",
read_request,
"\n\tleaf_index: ",
witness.leaf_index,
"\n\tis_transient: ",
witness.is_transient,
"\n\thint_to_commitment: ",
witness.hint_to_commitment),
CircuitErrorCode::PRIVATE_KERNEL__UNRESOLVED_NON_TRANSIENT_READ_REQUEST);
}
}

void common_update_end_values(DummyBuilder& builder,
PrivateCallData<NT> const& private_call,
KernelCircuitPublicInputs<NT>& public_inputs)
Expand Down Expand Up @@ -198,11 +155,6 @@ void common_update_end_values(DummyBuilder& builder,
siloed_read_request,
format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING,
"too many transient read requests in one tx"));
array_push(builder,
public_inputs.end.read_request_membership_witnesses,
witness,
format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING,
"too many transient read request membership witnesses in one tx"));
}
}
}
Expand Down
6 changes: 0 additions & 6 deletions circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ void common_validate_read_requests(DummyBuilder& builder,
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>,
MAX_READ_REQUESTS_PER_CALL> const& read_request_membership_witnesses);

void common_validate_previous_kernel_read_requests(
DummyBuilder& builder,
std::array<NT::fr, MAX_READ_REQUESTS_PER_TX> const& read_requests,
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>, MAX_READ_REQUESTS_PER_TX> const&
read_request_membership_witnesses);

void common_update_end_values(DummyBuilder& builder,
PrivateCallData<NT> const& private_call,
KernelCircuitPublicInputs<NT>& public_inputs);
Expand Down
Loading