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

feat: fixed number of pub inputs for databus commitment propagation #9336

Merged
merged 14 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -598,8 +598,8 @@ void prove_tube(const std::string& output_path)
// these public inputs by turning proof into witnesses and call
// set_public on each witness
auto num_public_inputs = static_cast<uint32_t>(static_cast<uint256_t>(proof.folding_proof[1]));
num_public_inputs -= bb::AGGREGATION_OBJECT_SIZE; // don't add the agg object
num_public_inputs -= 1 * 8; // TODO(https://github.com/AztecProtocol/barretenberg/issues/1125) Make this dynamic
num_public_inputs -= bb::AGGREGATION_OBJECT_SIZE; // don't add the agg object
num_public_inputs -= bb::PROPAGATED_DATABUS_COMMITMENTS_SIZE; // exclude propagated databus commitments
for (size_t i = 0; i < num_public_inputs; i++) {
auto offset = acir_format::HONK_RECURSION_PUBLIC_INPUT_OFFSET;
builder->add_public_variable(proof.folding_proof[i + offset]);
Expand Down
26 changes: 18 additions & 8 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ void ClientIVC::perform_recursive_verification_and_databus_consistency_checks(
const std::shared_ptr<RecursiveVerificationKey>& vkey,
const QUEUE_TYPE type)
{
// Store the decider vk for the incoming circuit; its data is used in the databus consistency checks below
std::shared_ptr<RecursiveDeciderVerificationKey> decider_vk;

switch (type) {
case QUEUE_TYPE::PG: {
// Construct stdlib verifier accumulator from the native counterpart computed on a previous round
Expand All @@ -66,10 +69,8 @@ void ClientIVC::perform_recursive_verification_and_databus_consistency_checks(
// Extract native verifier accumulator from the stdlib accum for use on the next round
verifier_accumulator = std::make_shared<DeciderVerificationKey>(verifier_accum->get_value());

// Perform databus commitment consistency checks and propagate return data commitments via public inputs
bus_depot.execute(verifier.keys_to_fold[1]->witness_commitments,
verifier.keys_to_fold[1]->public_inputs,
verifier.keys_to_fold[1]->verification_key->databus_propagation_data);
decider_vk = verifier.keys_to_fold[1]; // decider vk for the incoming circuit

break;
}
case QUEUE_TYPE::OINK: {
Expand All @@ -86,14 +87,20 @@ void ClientIVC::perform_recursive_verification_and_databus_consistency_checks(
// Initialize the gate challenges to zero for use in first round of folding
verifier_accumulator->gate_challenges = std::vector<FF>(CONST_PG_LOG_N, 0);

// Perform databus commitment consistency checks and propagate return data commitments via public inputs
bus_depot.execute(verifier_accum->witness_commitments,
verifier_accum->public_inputs,
verifier_accum->verification_key->databus_propagation_data);
decider_vk = verifier_accum; // decider vk for the incoming circuit

break;
}
}

// Set the return data commitment to be propagated on the public inputs of the present kernel and peform consistency
// checks between the calldata commitments and the return data commitments contained within the public inputs
bus_depot.set_return_data_to_be_propagated_and_perform_consistency_checks(
decider_vk->witness_commitments.return_data,
decider_vk->witness_commitments.calldata,
decider_vk->witness_commitments.secondary_calldata,
decider_vk->public_inputs,
decider_vk->verification_key->databus_propagation_data);
}

/**
Expand Down Expand Up @@ -133,6 +140,9 @@ void ClientIVC::complete_kernel_circuit_logic(ClientCircuit& circuit)
}
stdlib_verification_queue.clear();

// Propagate return data commitments via the public inputs for use in databus consistency checks
bus_depot.propagate_return_data_commitments(circuit);

// Perform recursive merge verification for every merge proof in the queue
process_recursive_merge_verification_queue(circuit);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,14 @@ TEST_F(ClientIVCIntegrationTests, BenchmarkCasePrecomputedVKs)

size_t NUM_CIRCUITS = 6;

MockCircuitProducer circuit_producer;

auto precomputed_vks = circuit_producer.precompute_verification_keys(NUM_CIRCUITS, ivc.trace_structure);
// Precompute the verification keys for each circuit in the IVC
std::vector<std::shared_ptr<VerificationKey>> precomputed_vks;
{
MockCircuitProducer circuit_producer;
precomputed_vks = circuit_producer.precompute_verification_keys(NUM_CIRCUITS, ivc.trace_structure);
}

MockCircuitProducer circuit_producer;
// Construct and accumulate a series of mocked private function execution circuits
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
Builder circuit = circuit_producer.create_next_circuit(ivc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ template <class Builder> class DataBusDepot {
using Commitment = typename Curve::Group;
using Fr = typename Curve::ScalarField;
using Fq = typename Curve::BaseField;
using CommitmentNative = typename Curve::AffineElementNative;
using FrNative = typename Curve::ScalarFieldNative;

using RecursiveFlavor = MegaRecursiveFlavor_<Builder>;
using RecursiveDeciderVerificationKeys =
Expand All @@ -77,54 +79,95 @@ template <class Builder> class DataBusDepot {
static constexpr size_t NUM_FR_LIMBS_PER_FQ = Fq::NUM_LIMBS;
static constexpr size_t NUM_FR_LIMBS_PER_COMMITMENT = NUM_FR_LIMBS_PER_FQ * 2;

Commitment app_return_data_commitment;
Commitment kernel_return_data_commitment;
bool app_return_data_commitment_exists = false;
bool kernel_return_data_commitment_exists = false;

/**
* @brief Execute circuit logic to establish proper transfer of databus data between circuits
* @details The databus mechanism establishes the transfer of data between two circuits (i-1 and i) in a third
* circuit (i+1) via commitment equality checks of the form [R_{i-1}] = [C_i], where R and C represent return data
* and calldata, respectively. In practice, circuit (i+1) is given access to [R_{i-1}] via the public inputs of
* \pi_i, and it has access to [C_i] directly from \pi_i. The consistency checks in circuit (i+1) are thus of the
* form \pi_i.public_inputs.[R_{i-1}] = \pi_i.[C_i]. This method peforms the two primary operations required for
* these checks: (1) extract commitments [R] from proofs received as private witnesses and propagate them to the
* next circuit via adding them to the public inputs. (2) Assert equality of commitments.
* form \pi_i.public_inputs.[R_{i-1}] = \pi_i.[C_i]. This method performs these consistency checks. It also prepares
* return data commitments [R] to be propagated via the public inputs of the present circuit.
Copy link
Contributor

Choose a reason for hiding this comment

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

this means the public inputs that will be passed along, supposingly to a next circuit, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, by making something a public input of the circuit A, I'm passing to the circuit that verifies the proof of circuit A since it will be present in the public inputs of the proof pi_A

*
* In Aztec private function execution, this mechanism is used as follows. Kernel circuit K_{i+1} must in general
* perform two databus consistency checks: (1) that the return_data of app circuit A_{i} was secondary calldata to
* K_{i}, and (2) that the return_data of K_{i-1} was calldata to K_{i}.
* @note In Aztec private function execution, this mechanism is used as follows. Kernel circuit K_{i+1} must in
* general perform two databus consistency checks: (1) that the return_data of app circuit A_{i} was secondary
* calldata to K_{i}, and (2) that the return_data of K_{i-1} was calldata to K_{i}.
*
* @param commitments Witness polynomial commitments for an key that has been accumulated
* @param public_inputs The public inputs of that key
* @param propagation_data Data about the presence of databus commitments on the public inputs of the key.
* @param return_data Return data from either an app or a kernel
* @param calldata Calldata corresponding to return data from a previous kernel
* @param secondary_calldata Calldata corresponding to some app return data
* @param public_inputs Public inputs of a kernel proof which contain propagated return data commitments
* @param propagation_data Info about the return data commitments stored in the provided public inputs
*/
void execute(const WitnessCommitments& commitments,
const std::vector<Fr>& public_inputs,
const DatabusPropagationData& propagation_data)
void set_return_data_to_be_propagated_and_perform_consistency_checks(const Commitment& return_data,
const Commitment& calldata,
const Commitment& secondary_calldata,
const std::vector<Fr>& public_inputs,
const DatabusPropagationData& propagation_data)
{
// Flag indicating whether the input data corresponds to a kernel decider proving key (else, an app decider
// proving key). This is used to indicate whether the return data commitment being propagated belongs to a
// kernel or an app so that it can be checked against the appropriate calldata commitment in a subsequent round.
bool is_kernel_data = propagation_data.is_kernel;

// Assert equality between return data commitments propagated via the public inputs and the corresponding
// calldata commitment
if (propagation_data.contains_app_return_data_commitment) { // public inputs contain [R]_app
ASSERT(is_kernel_data); // Only kernels should contain databus commitments in their public inputs
size_t start_idx = propagation_data.app_return_data_public_input_idx;
Commitment app_return_data = reconstruct_commitment_from_public_inputs(public_inputs, start_idx);
// App return data should correspond to the secondary calldata of the subsequent kernel
assert_equality_of_commitments(app_return_data, commitments.secondary_calldata);
// Set the kernel/app return data commitment to be propagated via the public inputs
if (propagation_data.is_kernel) {
kernel_return_data_commitment = return_data;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not something that is a priority now but I wonder whether the *_exists variables shouldn't be bool_t and the kernel_return_data_commitment be set via conditional assigns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note of this to the larger issue for these default commitments

kernel_return_data_commitment_exists = true;
} else {
app_return_data_commitment = return_data;
app_return_data_commitment_exists = true;
}

if (propagation_data.contains_kernel_return_data_commitment) { // pub inputs contain [R]_kernel
ASSERT(is_kernel_data); // Only kernels should contain databus commitments in their public inputs
// If the input data corresponds to a kernel, perform consistency checks between the provided calldata
// commitments and the return data commitments stored in the provided kernel proof public inputs
if (propagation_data.is_kernel) {
// Reconstruct the kernel and app return data commitments stored in the public inputs of the kernel proof
size_t start_idx = propagation_data.kernel_return_data_public_input_idx;
Commitment kernel_return_data = reconstruct_commitment_from_public_inputs(public_inputs, start_idx);
// Previous kernel return data should correspond to the calldata of the subsequent kernel
assert_equality_of_commitments(kernel_return_data, commitments.calldata);
start_idx = propagation_data.app_return_data_public_input_idx;
Commitment app_return_data = reconstruct_commitment_from_public_inputs(public_inputs, start_idx);

// Assert equality between the corresponding calldata and return data commitments
assert_equality_of_commitments(kernel_return_data, calldata);
assert_equality_of_commitments(app_return_data, secondary_calldata);
}
}

// Propagate the return data commitment via the public inputs mechanism
propagate_commitment_via_public_inputs(commitments.return_data, is_kernel_data);
};
/**
* @brief Propagate the existing return data commitments via the public inputs of the provided circuit
* @details For consistent behavior across kernels, every kernel propagates two return data commitments via its
* public inputs. If one of either the app or kernel return data does not exist, it is populated with a default
* value that will satisfy the consistency check on the next cycle. For example, the first kernel has no previous
* kernel to verify and thus neither receives a previous kernel return data commitment nor a calldata input
* corresponding to a previous kernel. The commitment to the "empty" calldata will take a default value and thus we
* set the same value for the missing return data so that the consistency check will be satisfied. See issue
* https://github.com/AztecProtocol/barretenberg/issues/1138 for more discussion.
* @note The ordering of the kernel/app return data commitments within the public inputs is arbitrary but must be
* consistent across all kernels in order for the corresponding conistency check constraints to be consistent.
*
* @param builder
*/
void propagate_return_data_commitments(Builder& builder)
{
// Set default commitment value to be used in the absence of one or the other return_data commitment
CommitmentNative default_commitment_val = CommitmentNative::one() * FrNative(BusVector::DEFAULT_VALUE);
if (kernel_return_data_commitment_exists) {
propagate_commitment_via_public_inputs(kernel_return_data_commitment, /*is_kernel=*/true);
} else {
Commitment default_commitment = Commitment::from_witness(&builder, default_commitment_val);
propagate_commitment_via_public_inputs(default_commitment, /*is_kernel=*/true);
}

if (app_return_data_commitment_exists) {
propagate_commitment_via_public_inputs(app_return_data_commitment, /*is_kernel=*/false);
} else {
Commitment default_commitment = Commitment::from_witness(&builder, default_commitment_val);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we shouldn't use from_witness here since the default_commitment_val is a constant (also above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some logic for converting constants to fixed witnesses as we discussed

propagate_commitment_via_public_inputs(default_commitment, /*is_kernel=*/false);
}
// Reset flags indicating existence of return data commitments
kernel_return_data_commitment_exists = false;
app_return_data_commitment_exists = false;
}

/**
* @brief Set the witness indices for a commitment to public
Expand All @@ -142,10 +185,8 @@ template <class Builder> class DataBusDepot {
// Set flag indicating propagation of return data; save the index at which it will be stored in public inputs
auto start_idx = static_cast<uint32_t>(context->public_inputs.size());
if (is_kernel) {
context->databus_propagation_data.contains_kernel_return_data_commitment = true;
context->databus_propagation_data.kernel_return_data_public_input_idx = start_idx;
} else {
context->databus_propagation_data.contains_app_return_data_commitment = true;
context->databus_propagation_data.app_return_data_public_input_idx = start_idx;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
#pragma once

#include "barretenberg/ecc/curves/bn254/fr.hpp"
#include <cstdint>
namespace bb {

// We assume all kernels have space for two return data commitments on their public inputs
constexpr uint32_t PROPAGATED_DATABUS_COMMITMENTS_SIZE = 16;

/**
* @brief A DataBus column
*
*/
struct BusVector {

// TODO(https://github.com/AztecProtocol/barretenberg/issues/1138): A default value added to every databus column to
Copy link
Contributor

Choose a reason for hiding this comment

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

If we, at some hypothetic point in the future, fix all the remaining issues with point at infinity, would this default value still be needed for the consistency checks?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, the issue describes it clearly

// avoid the point at infinity commitment and to ensure the validity of the databus commitment consistency checks.
static constexpr bb::fr DEFAULT_VALUE = 25;

/**
* @brief Add an element to the data defining this bus column
*
Expand Down Expand Up @@ -70,10 +78,6 @@ enum class BusId { CALLDATA, SECONDARY_CALLDATA, RETURNDATA };
struct DatabusPropagationData {
bool operator==(const DatabusPropagationData&) const = default;

// Flags indicating whether the public inputs contain commitment(s) to app/kernel return data
bool contains_app_return_data_commitment = false;
bool contains_kernel_return_data_commitment = false;

// The start index of the return data commitments (if present) in the public inputs. Note: a start index is all
// that's needed here since the commitents are represented by a fixed number of witnesses and are contiguous in the
// public inputs by construction.
Expand All @@ -85,19 +89,13 @@ struct DatabusPropagationData {

friend std::ostream& operator<<(std::ostream& os, DatabusPropagationData const& data)
{
os << data.contains_app_return_data_commitment << ",\n"
<< data.contains_kernel_return_data_commitment << ",\n"
<< data.app_return_data_public_input_idx << ",\n"
os << data.app_return_data_public_input_idx << ",\n"
<< data.kernel_return_data_public_input_idx << ",\n"
<< data.is_kernel << "\n";
return os;
};

MSGPACK_FIELDS(contains_app_return_data_commitment,
contains_kernel_return_data_commitment,
app_return_data_public_input_idx,
kernel_return_data_public_input_idx,
is_kernel);
MSGPACK_FIELDS(app_return_data_public_input_idx, kernel_return_data_public_input_idx, is_kernel);
};

} // namespace bb
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ TYPED_TEST(FlavorSerializationTests, VerificationKeySerialization)

// Populate some non-zero values in the databus_propagation_data to ensure its being handled
if constexpr (IsMegaBuilder<Builder>) {
original_vkey.databus_propagation_data.contains_app_return_data_commitment = 1;
original_vkey.databus_propagation_data.contains_kernel_return_data_commitment = 1;
original_vkey.databus_propagation_data.app_return_data_public_input_idx = 2;
original_vkey.databus_propagation_data.kernel_return_data_public_input_idx = 4;
original_vkey.databus_propagation_data.is_kernel = 1;
Expand Down
Loading
Loading