-
Notifications
You must be signed in to change notification settings - Fork 265
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
Changes from 8 commits
ac4c255
4952e0a
1210630
b060611
84f4b9c
9439169
b2f31d1
2ef815d
e48c0da
7a21b56
db16470
abf3031
9dfc4a5
d5c1dda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = | ||
|
@@ -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. | ||
* | ||
* 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not something that is a priority now but I wonder whether the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we shouldn't use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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; | ||
} | ||
|
||
|
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
* | ||
|
@@ -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. | ||
|
@@ -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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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