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: bus updates #7522

Merged
merged 6 commits into from
Jul 22, 2024
Merged

feat: bus updates #7522

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a75ef3c
WiP fisrt pass adding calldata_2
ledwards2225 Jul 18, 2024
0255cb0
fix all the bugs
ledwards2225 Jul 18, 2024
fc42e8b
simplify and add test for new column
ledwards2225 Jul 18, 2024
1a645f6
better generalized tests
ledwards2225 Jul 19, 2024
336e99f
add flavor getter for inverses
ledwards2225 Jul 19, 2024
05cd650
comments and naming in response to review
ledwards2225 Jul 22, 2024
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
9 changes: 7 additions & 2 deletions barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,20 @@ template <typename Builder> bool UltraCircuitChecker::check_databus_read(auto& v

// Determine the type of read based on selector values
bool is_calldata_read = (values.q_l == 1);
bool is_return_data_read = (values.q_r == 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's somehow strange to me in hindsight that we were using q_r rather than q_o for a return value but supposingly it was for optimisations purposes so we dont have yet another column, 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.

Yeah this has nothing to do with the original purpose of the selector, its just a means for not having a new individual selector for each bus column. This is similar to what is done in the aux relation

ASSERT(is_calldata_read || is_return_data_read);
bool is_calldata_2_read = (values.q_r == 1);
bool is_return_data_read = (values.q_o == 1);
ASSERT(is_calldata_read || is_calldata_2_read || is_return_data_read);

// Check that the claimed value is present in the calldata/return data at the corresponding index
FF bus_value;
if (is_calldata_read) {
auto calldata = builder.get_calldata();
bus_value = builder.get_variable(calldata[raw_read_idx]);
}
if (is_calldata_2_read) {
auto calldata_2 = builder.get_calldata_2();
bus_value = builder.get_variable(calldata_2[raw_read_idx]);
}
if (is_return_data_read) {
auto return_data = builder.get_return_data();
bus_value = builder.get_variable(return_data[raw_read_idx]);
Expand Down
34 changes: 26 additions & 8 deletions barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ template <typename FF_> class DatabusLookupRelationImpl {
public:
using FF = FF_;
static constexpr size_t LENGTH = 5; // 1 + polynomial degree of this relation
static constexpr size_t NUM_BUS_COLUMNS = 2; // calldata, return data
static constexpr size_t NUM_BUS_COLUMNS = 3; // calldata, return data

// Note: Inverse correctness subrelations are actually LENGTH-1; taking advantage would require additional work
static constexpr std::array<size_t, NUM_BUS_COLUMNS * 2> SUBRELATION_PARTIAL_LENGTHS{
LENGTH, // inverse polynomial correctness subrelation
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make it explicit why we have duplicates here and that they're not quite duplicates, they operate on different values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

LENGTH, // log-derivative lookup argument subrelation
LENGTH, // inverse polynomial correctness subrelation
LENGTH, // log-derivative lookup argument subrelation
LENGTH, // inverse polynomial correctness subrelation
Expand All @@ -64,6 +66,8 @@ polynomials,
* exceed the subrelation partial degree, which is given by LENGTH - 1 in this case.
*/
static constexpr std::array<size_t, NUM_BUS_COLUMNS * 2> SUBRELATION_WITNESS_DEGREES{
LENGTH - 1, // inverse polynomial correctness subrelation
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

LENGTH - 1, // log-derivative lookup argument subrelation
LENGTH - 1, // inverse polynomial correctness subrelation
LENGTH - 1, // log-derivative lookup argument subrelation
LENGTH - 1, // inverse polynomial correctness subrelation
Expand All @@ -72,14 +76,14 @@ polynomials,

// The lookup subrelations are "linearly dependent" in the sense that they establish the value of a sum across the
// entire execution trace rather than a per-row identity.
static constexpr std::array<bool, NUM_BUS_COLUMNS* 2> SUBRELATION_LINEARLY_INDEPENDENT = {
true, false, true, false
};
static constexpr std::array<bool, NUM_BUS_COLUMNS* 2> SUBRELATION_LINEARLY_INDEPENDENT = { true, false, true,
false, true, false };

template <typename AllEntities> inline static bool skip([[maybe_unused]] const AllEntities& in)
{
// Ensure the input does not contain a read gate or data that is being read
return in.q_busread.is_zero() && in.calldata_read_counts.is_zero() && in.return_data_read_counts.is_zero();
return in.q_busread.is_zero() && in.calldata_read_counts.is_zero() && in.calldata_2_read_counts.is_zero() &&
in.return_data_read_counts.is_zero();
}

// Interface for easy access of databus components by column (bus_idx)
Expand All @@ -95,10 +99,20 @@ polynomials,
static auto& read_tags(const AllEntities& in) { return in.calldata_read_tags; }
};

// Specialization for return data (bus_idx = 1)
// Specialization for calldata_2 (bus_idx = 1)
template <typename AllEntities> struct BusData</*bus_idx=*/1, AllEntities> {
static auto& values(const AllEntities& in) { return in.return_data; }
static auto& values(const AllEntities& in) { return in.calldata_2; }
static auto& selector(const AllEntities& in) { return in.q_r; }
static auto& inverses(AllEntities& in) { return in.calldata_2_inverses; }
static auto& inverses(const AllEntities& in) { return in.calldata_2_inverses; } // const version
static auto& read_counts(const AllEntities& in) { return in.calldata_2_read_counts; }
static auto& read_tags(const AllEntities& in) { return in.calldata_2_read_tags; }
};

// Specialization for return data (bus_idx = 2)
template <typename AllEntities> struct BusData</*bus_idx=*/2, AllEntities> {
static auto& values(const AllEntities& in) { return in.return_data; }
static auto& selector(const AllEntities& in) { return in.q_o; }
static auto& inverses(AllEntities& in) { return in.return_data_inverses; }
static auto& inverses(const AllEntities& in) { return in.return_data_inverses; } // const version
static auto& read_counts(const AllEntities& in) { return in.return_data_read_counts; }
Expand Down Expand Up @@ -217,8 +231,12 @@ polynomials,
is_read = q_busread == 1 && polynomials.q_l[i] == 1;
nonzero_read_count = polynomials.calldata_read_counts[i] > 0;
}
if constexpr (bus_idx == 1) { // return data
if constexpr (bus_idx == 1) { // calldata_2
is_read = q_busread == 1 && polynomials.q_r[i] == 1;
nonzero_read_count = polynomials.calldata_2_read_counts[i] > 0;
}
if constexpr (bus_idx == 2) { // return data
is_read = q_busread == 1 && polynomials.q_o[i] == 1;
nonzero_read_count = polynomials.return_data_read_counts[i] > 0;
}
// We only compute the inverse if this row contains a read gate or data that has been read
Expand Down
8 changes: 4 additions & 4 deletions ...rg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ void ProtoGalaxyRecursiveVerifier_<VerifierInstances>::receive_and_finalise_inst

// If Goblin (i.e. using DataBus) receive commitments to log-deriv inverses polynomial
if constexpr (IsGoblinFlavor<Flavor>) {
witness_commitments.calldata_inverses = transcript->template receive_from_prover<Commitment>(
domain_separator + "_" + commitment_labels.calldata_inverses);
witness_commitments.return_data_inverses = transcript->template receive_from_prover<Commitment>(
domain_separator + "_" + commitment_labels.return_data_inverses);
for (auto [commitment, label] :
zip_view(witness_commitments.get_databus_inverses(), commitment_labels.get_databus_inverses())) {
commitment = transcript->template receive_from_prover<Commitment>(domain_separator + "_" + label);
}
}

witness_commitments.z_perm =
Expand Down
11 changes: 6 additions & 5 deletions ...etenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/ultra_recursive_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,14 @@ std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::ve
commitments.lookup_inverses =
transcript->template receive_from_prover<Commitment>(commitment_labels.lookup_inverses);

// If Goblin (i.e. using DataBus) receive commitments to log-deriv inverses polynomial
// If Goblin (i.e. using DataBus) receive commitments to log-deriv inverses polynomials
if constexpr (IsGoblinFlavor<Flavor>) {
commitments.calldata_inverses =
transcript->template receive_from_prover<Commitment>(commitment_labels.calldata_inverses);
commitments.return_data_inverses =
transcript->template receive_from_prover<Commitment>(commitment_labels.return_data_inverses);
for (auto [commitment, label] :
zip_view(commitments.get_databus_inverses(), commitment_labels.get_databus_inverses())) {
commitment = transcript->template receive_from_prover<Commitment>(label);
}
}

const FF public_input_delta = compute_public_input_delta<Flavor>(
public_inputs, beta, gamma, circuit_size, static_cast<uint32_t>(key->pub_inputs_offset));

Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ template <typename Builder> class databus {
public:
// The columns of the DataBus
bus_vector calldata{ BusId::CALLDATA };
bus_vector calldata_2{ BusId::CALLDATA_2 };
bus_vector return_data{ BusId::RETURNDATA };
};
} // namespace bb::stdlib
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct BusVector {
* in-circuit as we would with public inputs).
*
*/
using DataBus = std::array<BusVector, 2>;
enum class BusId { CALLDATA, RETURNDATA };
using DataBus = std::array<BusVector, 3>;
enum class BusId { CALLDATA, CALLDATA_2, RETURNDATA };

} // namespace bb
17 changes: 15 additions & 2 deletions barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ template <typename FF> void MegaCircuitBuilder_<FF>::add_gates_to_ensure_all_pol
auto read_idx = this->add_variable(raw_read_idx);
read_calldata(read_idx);

// Create an arbitrary calldata_2 read gate
add_public_calldata_2(this->add_variable(25)); // ensure there is at least one entry in calldata_2
raw_read_idx = static_cast<uint32_t>(get_calldata_2().size()) - 1; // read data that was just added
read_idx = this->add_variable(raw_read_idx);
read_calldata_2(read_idx);

// Create an arbitrary return data read gate
add_public_return_data(this->add_variable(17)); // ensure there is at least one entry in return data
raw_read_idx = static_cast<uint32_t>(get_return_data().size()) - 1; // read data that was just added
Expand Down Expand Up @@ -238,17 +244,24 @@ template <typename FF> void MegaCircuitBuilder_<FF>::apply_databus_selectors(con
case BusId::CALLDATA: {
block.q_1().emplace_back(1);
block.q_2().emplace_back(0);
block.q_3().emplace_back(0);
break;
}
case BusId::RETURNDATA: {
case BusId::CALLDATA_2: {
block.q_1().emplace_back(0);
block.q_2().emplace_back(1);
block.q_3().emplace_back(0);
break;
}
case BusId::RETURNDATA: {
block.q_1().emplace_back(0);
block.q_2().emplace_back(0);
block.q_3().emplace_back(1);
break;
}
}
block.q_busread().emplace_back(1);
block.q_m().emplace_back(0);
block.q_3().emplace_back(0);
block.q_c().emplace_back(0);
block.q_delta_range().emplace_back(0);
block.q_arith().emplace_back(0);
Expand Down
18 changes: 18 additions & 0 deletions barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ template <typename FF> class MegaCircuitBuilder_ : public UltraCircuitBuilder_<M
*/
void add_public_calldata(const uint32_t& in) { return append_to_bus_vector(BusId::CALLDATA, in); }

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment why we have two of this function. We could specialise the name for Aztec usecase (our proving system is not that general in the end) and call them calldata_function and calldata_kernel so 1 and 2 are not misused, but prolly there's even better naming options in terms of generality

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comment (this was just an oversight) and updated the naming to calldata and secondary_calldata. This seems much better than calldata_2 without over specifying the usage

* @brief Add a witness variable to the public calldata.
*
*/
void add_public_calldata_2(const uint32_t& in) { return append_to_bus_vector(BusId::CALLDATA_2, in); }

/**
* @brief Add a witness variable to the public return_data.
*
Expand All @@ -190,6 +196,17 @@ template <typename FF> class MegaCircuitBuilder_ : public UltraCircuitBuilder_<M
return read_bus_vector(BusId::CALLDATA, read_idx_witness_idx);
};

/**
* @brief Read from calldata_2 and create a corresponding databus read gate
*
* @param read_idx_witness_idx Witness index for the calldata_2 read index
* @return uint32_t Witness index for the result of the read
*/
uint32_t read_calldata_2(const uint32_t& read_idx_witness_idx)
{
return read_bus_vector(BusId::CALLDATA_2, read_idx_witness_idx);
};

/**
* @brief Read from return_data and create a corresponding databus read gate
*
Expand All @@ -207,6 +224,7 @@ template <typename FF> class MegaCircuitBuilder_ : public UltraCircuitBuilder_<M
}

const BusVector& get_calldata() { return databus[static_cast<size_t>(BusId::CALLDATA)]; }
const BusVector& get_calldata_2() { return databus[static_cast<size_t>(BusId::CALLDATA_2)]; }
const BusVector& get_return_data() { return databus[static_cast<size_t>(BusId::RETURNDATA)]; }

void create_poseidon2_external_gate(const poseidon2_external_gate_<FF>& in);
Expand Down
56 changes: 49 additions & 7 deletions barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_flavor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ class MegaFlavor {
static constexpr size_t NUM_WIRES = CircuitBuilder::NUM_WIRES;
// The number of multivariate polynomials on which a sumcheck prover sumcheck operates (including shifts). We often
// need containers of this size to hold related data, so we choose a name more agnostic than `NUM_POLYNOMIALS`.
static constexpr size_t NUM_ALL_ENTITIES = 59;
static constexpr size_t NUM_ALL_ENTITIES = 63;
// The number of polynomials precomputed to describe a circuit and to aid a prover in constructing a satisfying
// assignment of witnesses. We again choose a neutral name.
static constexpr size_t NUM_PRECOMPUTED_ENTITIES = 30;
// The total number of witness entities not including shifts.
static constexpr size_t NUM_WITNESS_ENTITIES = 20;
static constexpr size_t NUM_WITNESS_ENTITIES = 24;
// Total number of folded polynomials, which is just all polynomials except the shifts
static constexpr size_t NUM_FOLDED_ENTITIES = NUM_PRECOMPUTED_ENTITIES + NUM_WITNESS_ENTITIES;

Expand Down Expand Up @@ -188,10 +188,14 @@ class MegaFlavor {
calldata_read_counts, // column 13
calldata_read_tags, // column 14
calldata_inverses, // column 15
return_data, // column 16
return_data_read_counts, // column 17
return_data_read_tags, // column 18
return_data_inverses); // column 19
calldata_2, // column 16
calldata_2_read_counts, // column 17
calldata_2_read_tags, // column 18
calldata_2_inverses, // column 19
return_data, // column 20
return_data_read_counts, // column 21
return_data_read_tags, // column 22
return_data_inverses); // column 23
};

/**
Expand All @@ -212,9 +216,19 @@ class MegaFlavor {
auto get_databus_entities() // Excludes the derived inverse polynomials
{
return RefArray{ this->calldata, this->calldata_read_counts, this->calldata_read_tags,
this->calldata_2, this->calldata_2_read_counts, this->calldata_2_read_tags,
this->return_data, this->return_data_read_counts, this->return_data_read_tags };
}

auto get_databus_inverses()
{
return RefArray{
this->calldata_inverses,
this->calldata_2_inverses,
this->return_data_inverses,
};
}

MSGPACK_FIELDS(this->w_l,
this->w_r,
this->w_o,
Expand All @@ -231,6 +245,10 @@ class MegaFlavor {
this->calldata_read_counts,
this->calldata_read_tags,
this->calldata_inverses,
this->calldata_2,
this->calldata_2_read_counts,
this->calldata_2_read_tags,
this->calldata_2_inverses,
this->return_data,
this->return_data_read_counts,
this->return_data_read_tags,
Expand Down Expand Up @@ -403,9 +421,13 @@ class MegaFlavor {
DatabusLookupRelation<FF>::compute_logderivative_inverse</*bus_idx=*/0>(
this->polynomials, relation_parameters, this->circuit_size);

// Compute inverses for return data reads
// Compute inverses for calldata_2 reads
DatabusLookupRelation<FF>::compute_logderivative_inverse</*bus_idx=*/1>(
this->polynomials, relation_parameters, this->circuit_size);

// Compute inverses for return data reads
DatabusLookupRelation<FF>::compute_logderivative_inverse</*bus_idx=*/2>(
this->polynomials, relation_parameters, this->circuit_size);
}

/**
Expand Down Expand Up @@ -629,6 +651,10 @@ class MegaFlavor {
calldata_read_counts = "CALLDATA_READ_COUNTS";
calldata_read_tags = "CALLDATA_READ_TAGS";
calldata_inverses = "CALLDATA_INVERSES";
calldata_2 = "CALLDATA_2";
calldata_2_read_counts = "CALLDATA_2_READ_COUNTS";
calldata_2_read_tags = "CALLDATA_2_READ_TAGS";
calldata_2_inverses = "CALLDATA_2_INVERSES";
return_data = "RETURN_DATA";
return_data_read_counts = "RETURN_DATA_READ_COUNTS";
return_data_read_tags = "RETURN_DATA_READ_TAGS";
Expand Down Expand Up @@ -724,6 +750,10 @@ class MegaFlavor {
this->calldata_read_counts = commitments.calldata_read_counts;
this->calldata_read_tags = commitments.calldata_read_tags;
this->calldata_inverses = commitments.calldata_inverses;
this->calldata_2 = commitments.calldata_2;
this->calldata_2_read_counts = commitments.calldata_2_read_counts;
this->calldata_2_read_tags = commitments.calldata_2_read_tags;
this->calldata_2_inverses = commitments.calldata_2_inverses;
this->return_data = commitments.return_data;
this->return_data_read_counts = commitments.return_data_read_counts;
this->return_data_read_tags = commitments.return_data_read_tags;
Expand Down Expand Up @@ -756,6 +786,10 @@ class MegaFlavor {
Commitment calldata_read_counts_comm;
Commitment calldata_read_tags_comm;
Commitment calldata_inverses_comm;
Commitment calldata_2_comm;
Commitment calldata_2_read_counts_comm;
Commitment calldata_2_read_tags_comm;
Commitment calldata_2_inverses_comm;
Commitment return_data_comm;
Commitment return_data_read_counts_comm;
Commitment return_data_read_tags_comm;
Expand Down Expand Up @@ -814,6 +848,10 @@ class MegaFlavor {
calldata_read_counts_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_read_tags_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_inverses_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_2_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_2_read_counts_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_2_read_tags_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_2_inverses_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
return_data_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
return_data_read_counts_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
return_data_read_tags_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
Expand Down Expand Up @@ -857,6 +895,10 @@ class MegaFlavor {
serialize_to_buffer(calldata_read_counts_comm, proof_data);
serialize_to_buffer(calldata_read_tags_comm, proof_data);
serialize_to_buffer(calldata_inverses_comm, proof_data);
serialize_to_buffer(calldata_2_comm, proof_data);
serialize_to_buffer(calldata_2_read_counts_comm, proof_data);
serialize_to_buffer(calldata_2_read_tags_comm, proof_data);
serialize_to_buffer(calldata_2_inverses_comm, proof_data);
serialize_to_buffer(return_data_comm, proof_data);
serialize_to_buffer(return_data_read_counts_comm, proof_data);
serialize_to_buffer(return_data_read_tags_comm, proof_data);
Expand Down
Loading
Loading