From c19b1836794773f29daf32840737acd493968a2d Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 28 Mar 2024 02:31:14 +0000 Subject: [PATCH 01/21] clean up relation a bit --- .../relations/databus_lookup_relation.hpp | 42 +++++++------------ 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index eb79bb21efe..9c0fdea25f3 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -50,6 +50,7 @@ template class DatabusLookupRelationImpl { * @return auto& */ template static auto& get_inverse_polynomial(AllEntities& in) { return in.lookup_inverses; } + /** * @brief Compute the Accumulator whose values indicate whether the inverse is computed or not * @details This is needed for efficiency since we don't need to compute the inverse unless the log derivative @@ -60,25 +61,21 @@ template class DatabusLookupRelationImpl { static Accumulator compute_inverse_exists(const AllEntities& in) { using View = typename Accumulator::View; - // TODO(luke): row_has_read should really be a boolean object thats equal to 1 when counts > 0 and 0 otherwise. - // This current structure will lead to failure if call_data_read_counts > 1. - const auto row_has_write = View(in.q_busread); - const auto row_has_read = View(in.calldata_read_counts); - - return row_has_write + row_has_read - (row_has_write * row_has_read); + // WORKTODO(luke): row_has_read should really be a boolean object thats equal to 1 when counts > 0 and 0 + // otherwise. This current structure will lead to failure if call_data_read_counts > 1. + const auto is_read_gate = View(in.q_busread); + const auto is_read_data = View(in.calldata_read_counts); - return Accumulator(View(in.q_busread) + View(in.calldata_read_counts)); + return is_read_gate + is_read_data - (is_read_gate * is_read_data); } template static Accumulator lookup_read_counts(const AllEntities& in) { using View = typename Accumulator::View; + ASSERT(index == 0); - if constexpr (index == 0) { - return Accumulator(View(in.calldata_read_counts)); - } - return Accumulator(1); + return Accumulator(View(in.calldata_read_counts)); } /** @@ -90,11 +87,9 @@ template class DatabusLookupRelationImpl { { using View = typename Accumulator::View; + ASSERT(read_index == 0); - if constexpr (read_index == 0) { - return Accumulator(View(in.q_busread)); - } - return Accumulator(1); + return Accumulator(View(in.q_busread)); } /** @@ -104,6 +99,7 @@ template class DatabusLookupRelationImpl { template static Accumulator compute_write_term_predicate(const AllEntities& /*unused*/) { + ASSERT(write_index == 0); return Accumulator(1); } @@ -117,7 +113,7 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; using ParameterView = GetParameterView; - static_assert(write_index < WRITE_TERMS); + ASSERT(write_index == 0); const auto& calldata = View(in.calldata); const auto& id = View(in.databus_id); @@ -126,11 +122,7 @@ template class DatabusLookupRelationImpl { const auto& beta = ParameterView(params.beta); // Construct b_i + idx_i*\beta + \gamma - if constexpr (write_index == 0) { - return calldata + gamma + id * beta; // degree 1 - } - - return Accumulator(1); + return calldata + gamma + id * beta; // degree 1 } /** @@ -143,7 +135,7 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; using ParameterView = GetParameterView; - static_assert(read_index < READ_TERMS); + ASSERT(read_index == 0); // Bus value stored in w_1, index into bus column stored in w_2 const auto& w_1 = View(in.w_l); @@ -153,11 +145,7 @@ template class DatabusLookupRelationImpl { const auto& beta = ParameterView(params.beta); // Construct value + index*\beta + \gamma - if constexpr (read_index == 0) { - return w_1 + gamma + w_2 * beta; - } - - return Accumulator(1); + return w_1 + gamma + w_2 * beta; } /** From 10c8a277605b98b71ae97faea09486ffdb477e65 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 28 Mar 2024 02:31:52 +0000 Subject: [PATCH 02/21] generalize via BusVector and DataBus --- .../goblin_ultra_circuit_builder.cpp | 11 +++-- .../goblin_ultra_circuit_builder.hpp | 46 +++++++++++++++---- .../sumcheck/instance/prover_instance.cpp | 6 +-- 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 0267a5be4fa..4c5f81c346d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -222,21 +222,22 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co * @param read_idx_witness_idx Variable index of the read index * @return uint32_t Variable index of the result of the read */ -template uint32_t GoblinUltraCircuitBuilder_::read_calldata(const uint32_t& read_idx_witness_idx) +template +uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, const uint32_t& read_idx_witness_idx) { // Get the raw index into the calldata const uint32_t read_idx = static_cast(uint256_t(this->get_variable(read_idx_witness_idx))); // Ensure that the read index is valid - ASSERT(read_idx < public_calldata.size()); + ASSERT(read_idx < bus_vector.size()); // Create a variable corresponding to the result of the read. Note that we do not in general connect reads from // calldata via copy constraints (i.e. we create a unique variable for the result of each read) - FF calldata_value = this->get_variable(public_calldata[read_idx]); - uint32_t value_witness_idx = this->add_variable(calldata_value); + FF value = this->get_variable(bus_vector[read_idx]); + uint32_t value_witness_idx = this->add_variable(value); create_calldata_read_gate({ read_idx_witness_idx, value_witness_idx }); - calldata_read_counts[read_idx]++; + bus_vector.read_counts[read_idx]++; return value_witness_idx; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index d0f77eee38a..7f0a5dc8172 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -10,6 +10,27 @@ using namespace bb; template class GoblinUltraCircuitBuilder_ : public UltraCircuitBuilder_> { public: + struct BusVector { + std::vector data; // variable indices corresponding to data in this bus vector + std::vector read_counts; // count of reads at each index into data + + std::vector> gate_data; // pairs of {read_idx_witness_idx, value_witness_idx} + + void append(const uint32_t& idx) + { + data.emplace_back(idx); + read_counts.resize(data.size()); + } + + size_t size() { return data.size(); } + + uint32_t operator[](size_t idx) { return data[idx]; } + }; + + struct DataBus { + BusVector calldata; + }; + using Arithmetization = UltraHonkArith; static constexpr std::string_view NAME_STRING = "GoblinUltraArithmetization"; @@ -26,10 +47,8 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui uint32_t mul_accum_op_idx; uint32_t equality_op_idx; - // DataBus call/return data arrays - std::vector public_calldata; - std::vector calldata_read_counts; - std::vector public_return_data; + // Container for public calldata/returndata + DataBus databus; // Functions for adding ECC op queue "gates" ecc_op_tuple queue_ecc_add_accum(const g1::affine_element& point); @@ -128,16 +147,23 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui * * @param in Value to be added to calldata. * */ - uint32_t add_public_calldata(const FF& in) + uint32_t add_public_calldata(const FF& in) { return add_to_bus_vector(databus.calldata, in); } + + /** + * @brief Read from calldata and create a corresponding databus read gate + * + * @param read_idx Witness index for the calldata read index + * @return uint32_t Witness index for the result of the read + */ + uint32_t read_calldata(const uint32_t& read_idx) { return read_bus_vector(databus.calldata, read_idx); }; + + uint32_t add_to_bus_vector(BusVector& bus_vector, const FF& in) { const uint32_t index = this->add_variable(in); - public_calldata.emplace_back(index); - // Note: this is a bit inefficent to do every time but for safety these need to be coupled - calldata_read_counts.resize(public_calldata.size()); + bus_vector.append(index); return index; } - - uint32_t read_calldata(const uint32_t& read_idx_witness_idx); + uint32_t read_bus_vector(BusVector& bus_vector, const uint32_t& read_idx_witness_idx); void create_poseidon2_external_gate(const poseidon2_external_gate_& in); void create_poseidon2_internal_gate(const poseidon2_internal_gate_& in); diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp index 9900d20b9de..74edeee961e 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp @@ -46,9 +46,9 @@ void ProverInstance_::construct_databus_polynomials(Circuit& circuit) Polynomial databus_id{ dyadic_circuit_size }; // Note: We do not utilize a zero row for databus columns - for (size_t idx = 0; idx < circuit.public_calldata.size(); ++idx) { - public_calldata[idx] = circuit.get_variable(circuit.public_calldata[idx]); - calldata_read_counts[idx] = circuit.calldata_read_counts[idx]; + for (size_t idx = 0; idx < circuit.databus.calldata.size(); ++idx) { + public_calldata[idx] = circuit.get_variable(circuit.databus.calldata[idx]); + calldata_read_counts[idx] = circuit.databus.calldata.read_counts[idx]; } // Compute a simple identity polynomial for use in the databus lookup argument From c19d36ebced51759612d5f38da2dff2f930f73dd Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 28 Mar 2024 15:42:08 +0000 Subject: [PATCH 03/21] generic gate --- .../goblin_ultra_circuit_builder.cpp | 17 +++++++++++++++-- .../goblin_ultra_circuit_builder.hpp | 3 +-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 4c5f81c346d..9b9d7689048 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -248,8 +248,7 @@ uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, * @tparam FF * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value */ -template -void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_lookup_gate_& in) +template void GoblinUltraCircuitBuilder_::create_databus_read_gate(const databus_lookup_gate_& in) { auto& block = this->blocks.busread; block.populate_wires(in.value, in.index, this->zero_idx, this->zero_idx); @@ -274,6 +273,20 @@ void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_loo ++this->num_gates; } +/** + * @brief Create a calldata lookup/read gate + * + * @tparam FF + * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value + */ +template +void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_lookup_gate_& in) +{ + create_databus_read_gate(in); + auto& block = this->blocks.busread; + block.q_1()[block.size() - 1] = 1; +} + /** * @brief Poseidon2 external round gate, activates the q_poseidon2_external selector and relation */ diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index 7f0a5dc8172..18b06393206 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -14,8 +14,6 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui std::vector data; // variable indices corresponding to data in this bus vector std::vector read_counts; // count of reads at each index into data - std::vector> gate_data; // pairs of {read_idx_witness_idx, value_witness_idx} - void append(const uint32_t& idx) { data.emplace_back(idx); @@ -59,6 +57,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui void populate_ecc_op_wires(const ecc_op_tuple& in); ecc_op_tuple decompose_ecc_operands(uint32_t op, const g1::affine_element& point, const FF& scalar = FF::zero()); void set_goblin_ecc_op_code_constant_variables(); + void create_databus_read_gate(const databus_lookup_gate_& in); void create_calldata_read_gate(const databus_lookup_gate_& in); public: From 69dbfaa47fc3747bc4fe7d1dd9f3105da8cff37d Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 28 Mar 2024 16:59:50 +0000 Subject: [PATCH 04/21] calldata selected by q_busread q_1 --- .../relations/databus_lookup_relation.hpp | 15 +++++++++++---- .../goblin_ultra_circuit_builder.cpp | 1 + 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 9c0fdea25f3..615b4b7d02d 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -39,7 +39,8 @@ template class DatabusLookupRelationImpl { */ template static bool operation_exists_at_row(const AllValues& row) { - return (row.q_busread == 1 || row.calldata_read_counts > 0); + bool is_read_gate = row.q_busread == 1 && row.q_l == 1; + return (is_read_gate || row.calldata_read_counts > 0); } /** @@ -63,7 +64,9 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; // WORKTODO(luke): row_has_read should really be a boolean object thats equal to 1 when counts > 0 and 0 // otherwise. This current structure will lead to failure if call_data_read_counts > 1. - const auto is_read_gate = View(in.q_busread); + auto q_busread = View(in.q_busread); + auto q_1 = View(in.q_l); + const auto is_read_gate = q_busread * q_1; const auto is_read_data = View(in.calldata_read_counts); return is_read_gate + is_read_data - (is_read_gate * is_read_data); @@ -86,10 +89,14 @@ template class DatabusLookupRelationImpl { static Accumulator compute_read_term_predicate([[maybe_unused]] const AllEntities& in) { - using View = typename Accumulator::View; ASSERT(read_index == 0); + using View = typename Accumulator::View; + auto q_busread = View(in.q_busread); + auto q_1 = View(in.q_l); + + auto result = q_busread * q_1; - return Accumulator(View(in.q_busread)); + return result; } /** diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 9b9d7689048..0371de78b24 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -282,6 +282,7 @@ template void GoblinUltraCircuitBuilder_::create_databus_read_ template void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_lookup_gate_& in) { + // Create generic read gate then set q_1 = 1 to specify a calldata read create_databus_read_gate(in); auto& block = this->blocks.busread; block.q_1()[block.size() - 1] = 1; From b3c847a6a9ae399aae2171d52bd0d1029cc5a0f8 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 28 Mar 2024 21:23:17 +0000 Subject: [PATCH 05/21] simplify databus relation and make it self contained --- .../relations/databus_lookup_relation.hpp | 77 ++++++++++++------- .../goblin_ultra_flavor.hpp | 4 +- 2 files changed, 52 insertions(+), 29 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 615b4b7d02d..c45f32330dc 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -72,12 +72,9 @@ template class DatabusLookupRelationImpl { return is_read_gate + is_read_data - (is_read_gate * is_read_data); } - template - static Accumulator lookup_read_counts(const AllEntities& in) + template static Accumulator get_read_counts(const AllEntities& in) { using View = typename Accumulator::View; - ASSERT(index == 0); - return Accumulator(View(in.calldata_read_counts)); } @@ -85,11 +82,10 @@ template class DatabusLookupRelationImpl { * @brief Compute scalar for read term in log derivative lookup argument * */ - template - static Accumulator compute_read_term_predicate([[maybe_unused]] const AllEntities& in) + template + static Accumulator get_read_selector([[maybe_unused]] const AllEntities& in) { - ASSERT(read_index == 0); using View = typename Accumulator::View; auto q_busread = View(in.q_busread); auto q_1 = View(in.q_l); @@ -99,29 +95,16 @@ template class DatabusLookupRelationImpl { return result; } - /** - * @brief Compute scalar for write term in log derivative lookup argument - * - */ - template - static Accumulator compute_write_term_predicate(const AllEntities& /*unused*/) - { - ASSERT(write_index == 0); - return Accumulator(1); - } - /** * @brief Compute write term denominator in log derivative lookup argument * */ - template + template static Accumulator compute_write_term(const AllEntities& in, const Parameters& params) { using View = typename Accumulator::View; using ParameterView = GetParameterView; - ASSERT(write_index == 0); - const auto& calldata = View(in.calldata); const auto& id = View(in.databus_id); @@ -136,14 +119,12 @@ template class DatabusLookupRelationImpl { * @brief Compute read term denominator in log derivative lookup argument * */ - template + template static Accumulator compute_read_term(const AllEntities& in, const Parameters& params) { using View = typename Accumulator::View; using ParameterView = GetParameterView; - ASSERT(read_index == 0); - // Bus value stored in w_1, index into bus column stored in w_2 const auto& w_1 = View(in.w_l); const auto& w_2 = View(in.w_r); @@ -155,6 +136,50 @@ template class DatabusLookupRelationImpl { return w_1 + gamma + w_2 * beta; } + template + static void compute_logderivative_inverse(Polynomials& polynomials, + auto& relation_parameters, + const size_t circuit_size) + { + auto& inverse_polynomial = get_inverse_polynomial(polynomials); + for (size_t i = 0; i < circuit_size; ++i) { + auto row = polynomials.get_row(i); + // We only compute the inverse if this row contains a read gate or data that has been read + if (operation_exists_at_row(row)) { + inverse_polynomial[i] = + compute_read_term(row, relation_parameters) * compute_write_term(row, relation_parameters); + } + } + // WORKTODO: turn this note from Zac into a genuine TODO + // todo might be inverting zero in field bleh bleh + FF::batch_invert(inverse_polynomial); + }; + + template + static void accumulate_subrelation_contributions(ContainerOverSubrelations& accumulator, + const AllEntities& in, + const Parameters& params, + const FF& scaling_factor) + { + using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>; + using View = typename Accumulator::View; + + const auto lookup_inverses = View(get_inverse_polynomial(in)); + const auto read_term = compute_read_term(in, params); + const auto write_term = compute_write_term(in, params); + const auto inverse_exists = compute_inverse_exists(in); + const auto write_inverse = lookup_inverses * read_term; + const auto read_inverse = lookup_inverses * write_term; + + // Establish the correctness of the polynomial of inverses I. Note: lookup_inverses is computed so that the + // value is 0 if !inverse_exists + std::get<0>(accumulator) += (read_term * write_term * lookup_inverses - inverse_exists) * scaling_factor; + + // Establish the validity of the read. + std::get<1>(accumulator) += + get_read_selector(in) * read_inverse - (get_read_counts(in) * write_inverse); + } + /** * @brief Accumulate the contribution from two surelations for the log derivative databus lookup argument * @details See logderivative_library.hpp for details of the generic log-derivative lookup argument @@ -170,9 +195,7 @@ template class DatabusLookupRelationImpl { const Parameters& params, const FF& scaling_factor) { - - accumulate_logderivative_lookup_subrelation_contributions>( - accumulator, in, params, scaling_factor); + accumulate_subrelation_contributions(accumulator, in, params, scaling_factor); } }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp index 4eb513ae5a5..d5bf249d6ba 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp @@ -358,8 +358,8 @@ class GoblinUltraFlavor { void compute_logderivative_inverse(const RelationParameters& relation_parameters) { auto prover_polynomials = ProverPolynomials(*this); - // Compute permutation and lookup grand product polynomials - bb::compute_logderivative_inverse( + // Compute inverses polynomial used in log derivative relations + DatabusLookupRelation::compute_logderivative_inverse( prover_polynomials, relation_parameters, this->circuit_size); this->lookup_inverses = prover_polynomials.lookup_inverses; } From 5add60a07a09a1cf08ef4c0e0b86c2f1f43b6160 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 29 Mar 2024 16:15:05 +0000 Subject: [PATCH 06/21] cleanup and comments --- .../relations/databus_lookup_relation.hpp | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index c45f32330dc..00ecf5a2815 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -18,6 +18,8 @@ template class DatabusLookupRelationImpl { // 1 + polynomial degree of this relation static constexpr size_t LENGTH = READ_TERMS + WRITE_TERMS + 3; + // Note: The first subrelation actually has length = LENGTH-1 but taking advantage of this would require additional + // computation that would nullify the benefits. static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ LENGTH, // inverse polynomial correctness subrelation LENGTH // log-derivative lookup argument subrelation @@ -161,28 +163,29 @@ template class DatabusLookupRelationImpl { const Parameters& params, const FF& scaling_factor) { - using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>; + using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>; using View = typename Accumulator::View; - const auto lookup_inverses = View(get_inverse_polynomial(in)); - const auto read_term = compute_read_term(in, params); - const auto write_term = compute_write_term(in, params); - const auto inverse_exists = compute_inverse_exists(in); - const auto write_inverse = lookup_inverses * read_term; - const auto read_inverse = lookup_inverses * write_term; + const auto lookup_inverses = View(get_inverse_polynomial(in)); // Degree 1 + const auto read_term = View(compute_read_term(in, params)); // Degree 1 + const auto write_term = compute_write_term(in, params); // Degree 1 + const auto inverse_exists = compute_inverse_exists(in); // Degree 1 + const auto read_counts = get_read_counts(in); // Degree 1 + const auto read_selector = get_read_selector(in); // Degree 2 + const auto write_inverse = lookup_inverses * read_term; // Degree 2 + const auto read_inverse = lookup_inverses * write_term; // Degree 2 // Establish the correctness of the polynomial of inverses I. Note: lookup_inverses is computed so that the - // value is 0 if !inverse_exists + // value is 0 if !inverse_exists. Degree 3 std::get<0>(accumulator) += (read_term * write_term * lookup_inverses - inverse_exists) * scaling_factor; - // Establish the validity of the read. - std::get<1>(accumulator) += - get_read_selector(in) * read_inverse - (get_read_counts(in) * write_inverse); + // Establish validity of the read. Note: no scaling factor here since this subrelation is linearly dependent + std::get<1>(accumulator) += read_selector * read_inverse - read_counts * write_inverse; // Degree 4 } /** * @brief Accumulate the contribution from two surelations for the log derivative databus lookup argument - * @details See logderivative_library.hpp for details of the generic log-derivative lookup argument + * @details Each databus column requires two two subrelations * * @param accumulator transformed to `evals + C(in(X)...)*scaling_factor` * @param in an std::array containing the fully extended Accumulator edges. From a58a8d1b761ff70b8749af7822d53a06c384ed3e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 29 Mar 2024 17:10:30 +0000 Subject: [PATCH 07/21] introduce bus index --- .../relations/databus_lookup_relation.hpp | 140 ++++++++++++------ .../goblin_ultra_flavor.hpp | 2 +- 2 files changed, 95 insertions(+), 47 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 00ecf5a2815..0dfff321794 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -17,6 +17,7 @@ template class DatabusLookupRelationImpl { static constexpr size_t WRITE_TERMS = 1; // 1 + polynomial degree of this relation static constexpr size_t LENGTH = READ_TERMS + WRITE_TERMS + 3; + static constexpr size_t NUM_BUS_COLUMNS = 1; // Note: The first subrelation actually has length = LENGTH-1 but taking advantage of this would require additional // computation that would nullify the benefits. @@ -39,10 +40,14 @@ template class DatabusLookupRelationImpl { * @return true * @return false */ - template static bool operation_exists_at_row(const AllValues& row) + template static bool operation_exists_at_row(const AllValues& row) { - bool is_read_gate = row.q_busread == 1 && row.q_l == 1; - return (is_read_gate || row.calldata_read_counts > 0); + ASSERT(bus_idx == 0); + if constexpr (bus_idx == 0) { + bool is_read_gate = row.q_busread == 1 && row.q_l == 1; + return (is_read_gate || row.calldata_read_counts > 0); + } + return false; } /** @@ -52,7 +57,13 @@ template class DatabusLookupRelationImpl { * @param in * @return auto& */ - template static auto& get_inverse_polynomial(AllEntities& in) { return in.lookup_inverses; } + template static auto& get_inverse_polynomial(AllEntities& in) + { + ASSERT(bus_idx == 0); + if constexpr (bus_idx == 0) { + return in.lookup_inverses; + } + } /** * @brief Compute the Accumulator whose values indicate whether the inverse is computed or not @@ -60,65 +71,79 @@ template class DatabusLookupRelationImpl { * lookup relation is active at a given row. * */ - template + template static Accumulator compute_inverse_exists(const AllEntities& in) { using View = typename Accumulator::View; // WORKTODO(luke): row_has_read should really be a boolean object thats equal to 1 when counts > 0 and 0 // otherwise. This current structure will lead to failure if call_data_read_counts > 1. - auto q_busread = View(in.q_busread); - auto q_1 = View(in.q_l); - const auto is_read_gate = q_busread * q_1; - const auto is_read_data = View(in.calldata_read_counts); - - return is_read_gate + is_read_data - (is_read_gate * is_read_data); + ASSERT(bus_idx == 0); + if constexpr (bus_idx == 0) { + auto q_busread = View(in.q_busread); + auto q_1 = View(in.q_l); + const auto is_read_gate = q_busread * q_1; + const auto is_read_data = View(in.calldata_read_counts); + + return is_read_gate + is_read_data - (is_read_gate * is_read_data); + } } - template static Accumulator get_read_counts(const AllEntities& in) + template + static Accumulator get_read_counts(const AllEntities& in) { using View = typename Accumulator::View; - return Accumulator(View(in.calldata_read_counts)); + ASSERT(bus_idx == 0); + if constexpr (bus_idx == 0) { + return Accumulator(View(in.calldata_read_counts)); + } } /** * @brief Compute scalar for read term in log derivative lookup argument * */ - template + template static Accumulator get_read_selector([[maybe_unused]] const AllEntities& in) { using View = typename Accumulator::View; - auto q_busread = View(in.q_busread); - auto q_1 = View(in.q_l); + ASSERT(bus_idx == 0); + if constexpr (bus_idx == 0) { + auto q_busread = View(in.q_busread); + auto q_1 = View(in.q_l); - auto result = q_busread * q_1; + auto result = q_busread * q_1; - return result; + return result; + } } /** * @brief Compute write term denominator in log derivative lookup argument * */ - template + template static Accumulator compute_write_term(const AllEntities& in, const Parameters& params) { using View = typename Accumulator::View; using ParameterView = GetParameterView; - const auto& calldata = View(in.calldata); - const auto& id = View(in.databus_id); + ASSERT(bus_idx == 0); + if constexpr (bus_idx == 0) { + const auto& calldata = View(in.calldata); + const auto& id = View(in.databus_id); - const auto& gamma = ParameterView(params.gamma); - const auto& beta = ParameterView(params.beta); + const auto& gamma = ParameterView(params.gamma); + const auto& beta = ParameterView(params.beta); - // Construct b_i + idx_i*\beta + \gamma - return calldata + gamma + id * beta; // degree 1 + // Construct b_i + idx_i*\beta + \gamma + return calldata + gamma + id * beta; // degree 1 + } } /** * @brief Compute read term denominator in log derivative lookup argument + * @note No bus_idx required here since inputs to a read are of the same form regardless the bus column * */ template @@ -138,18 +163,18 @@ template class DatabusLookupRelationImpl { return w_1 + gamma + w_2 * beta; } - template + template static void compute_logderivative_inverse(Polynomials& polynomials, auto& relation_parameters, const size_t circuit_size) { - auto& inverse_polynomial = get_inverse_polynomial(polynomials); + auto& inverse_polynomial = get_inverse_polynomial(polynomials); for (size_t i = 0; i < circuit_size; ++i) { auto row = polynomials.get_row(i); // We only compute the inverse if this row contains a read gate or data that has been read - if (operation_exists_at_row(row)) { - inverse_polynomial[i] = - compute_read_term(row, relation_parameters) * compute_write_term(row, relation_parameters); + if (operation_exists_at_row(row)) { + inverse_polynomial[i] = compute_read_term(row, relation_parameters) * + compute_write_term(row, relation_parameters); } } // WORKTODO: turn this note from Zac into a genuine TODO @@ -157,35 +182,55 @@ template class DatabusLookupRelationImpl { FF::batch_invert(inverse_polynomial); }; - template + /** + * @brief Accumulate the subrelation contributions for reads from a single databus column + * @details Two subrelations are required per bus column, one to establish correctness of the precomputed inverses + * and one to establish the validity of the read. + * + * @param accumulator + * @param in + * @param params + * @param scaling_factor + */ + template static void accumulate_subrelation_contributions(ContainerOverSubrelations& accumulator, const AllEntities& in, const Parameters& params, const FF& scaling_factor) { - using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>; + using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>; using View = typename Accumulator::View; - const auto lookup_inverses = View(get_inverse_polynomial(in)); // Degree 1 - const auto read_term = View(compute_read_term(in, params)); // Degree 1 - const auto write_term = compute_write_term(in, params); // Degree 1 - const auto inverse_exists = compute_inverse_exists(in); // Degree 1 - const auto read_counts = get_read_counts(in); // Degree 1 - const auto read_selector = get_read_selector(in); // Degree 2 - const auto write_inverse = lookup_inverses * read_term; // Degree 2 - const auto read_inverse = lookup_inverses * write_term; // Degree 2 + const auto lookup_inverses = View(get_inverse_polynomial(in)); // Degree 1 + const auto read_term = View(compute_read_term(in, params)); // Degree 1 + const auto write_term = compute_write_term(in, params); // Degree 1 + const auto inverse_exists = compute_inverse_exists(in); // Degree 1 + const auto read_counts = get_read_counts(in); // Degree 1 + const auto read_selector = get_read_selector(in); // Degree 2 + const auto write_inverse = lookup_inverses * read_term; // Degree 2 + const auto read_inverse = lookup_inverses * write_term; // Degree 2 + + // Determine which subrelations to update based on which bus column is being read + constexpr size_t subrel_idx_1 = 2 * bus_idx; + constexpr size_t subrel_idx_2 = 2 * bus_idx + 1; // Establish the correctness of the polynomial of inverses I. Note: lookup_inverses is computed so that the // value is 0 if !inverse_exists. Degree 3 - std::get<0>(accumulator) += (read_term * write_term * lookup_inverses - inverse_exists) * scaling_factor; + std::get(accumulator) += + (read_term * write_term * lookup_inverses - inverse_exists) * scaling_factor; - // Establish validity of the read. Note: no scaling factor here since this subrelation is linearly dependent - std::get<1>(accumulator) += read_selector * read_inverse - read_counts * write_inverse; // Degree 4 + // Establish validity of the read. Note: no scaling factor here since this constraint is enforced across the + // entire trace, not on a per-row basis + std::get(accumulator) += read_selector * read_inverse - read_counts * write_inverse; // Degree 4 } /** - * @brief Accumulate the contribution from two surelations for the log derivative databus lookup argument - * @details Each databus column requires two two subrelations + * @brief Accumulate the log derivative databus lookup argument subrelation contributions for each databus column + * @details Each databus column requires two subrelations * * @param accumulator transformed to `evals + C(in(X)...)*scaling_factor` * @param in an std::array containing the fully extended Accumulator edges. @@ -198,7 +243,10 @@ template class DatabusLookupRelationImpl { const Parameters& params, const FF& scaling_factor) { - accumulate_subrelation_contributions(accumulator, in, params, scaling_factor); + // Accumulate the subrelations for column of the databus + bb::constexpr_for<0, NUM_BUS_COLUMNS, 1>([&]() { + accumulate_subrelation_contributions(accumulator, in, params, scaling_factor); + }); } }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp index d5bf249d6ba..654da77590f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp @@ -359,7 +359,7 @@ class GoblinUltraFlavor { { auto prover_polynomials = ProverPolynomials(*this); // Compute inverses polynomial used in log derivative relations - DatabusLookupRelation::compute_logderivative_inverse( + DatabusLookupRelation::compute_logderivative_inverse( prover_polynomials, relation_parameters, this->circuit_size); this->lookup_inverses = prover_polynomials.lookup_inverses; } From 83a112dccd9af4550f0bcf2ed68ba65929c7f24b Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 29 Mar 2024 18:09:37 +0000 Subject: [PATCH 08/21] remove incorrect View --- .../cpp/src/barretenberg/relations/databus_lookup_relation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 0dfff321794..142571c7851 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -206,7 +206,7 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; const auto lookup_inverses = View(get_inverse_polynomial(in)); // Degree 1 - const auto read_term = View(compute_read_term(in, params)); // Degree 1 + const auto read_term = compute_read_term(in, params); // Degree 1 const auto write_term = compute_write_term(in, params); // Degree 1 const auto inverse_exists = compute_inverse_exists(in); // Degree 1 const auto read_counts = get_read_counts(in); // Degree 1 From 897b0b8c42a12d79a78d2df163e7f50e7908ade6 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 29 Mar 2024 20:09:35 +0000 Subject: [PATCH 09/21] cleanup and comment --- .../relations/databus_lookup_relation.hpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 142571c7851..376c78ea487 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -75,13 +75,12 @@ template class DatabusLookupRelationImpl { static Accumulator compute_inverse_exists(const AllEntities& in) { using View = typename Accumulator::View; - // WORKTODO(luke): row_has_read should really be a boolean object thats equal to 1 when counts > 0 and 0 - // otherwise. This current structure will lead to failure if call_data_read_counts > 1. ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { auto q_busread = View(in.q_busread); auto q_1 = View(in.q_l); const auto is_read_gate = q_busread * q_1; + // Note: read_counts is constructed such that read_count_i <= 1. const auto is_read_data = View(in.calldata_read_counts); return is_read_gate + is_read_data - (is_read_gate * is_read_data); @@ -89,17 +88,18 @@ template class DatabusLookupRelationImpl { } template - static Accumulator get_read_counts(const AllEntities& in) + static Accumulator::View get_read_counts(const AllEntities& in) { using View = typename Accumulator::View; ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { - return Accumulator(View(in.calldata_read_counts)); + return View(in.calldata_read_counts); } } /** * @brief Compute scalar for read term in log derivative lookup argument + * @details The selector indicating read from bus column j is given by q_busread * q_j, j = 1,2,3 * */ template @@ -163,12 +163,20 @@ template class DatabusLookupRelationImpl { return w_1 + gamma + w_2 * beta; } + /** + * @brief Construct the polynomial I whose components are the inverse of the product of the read and write terms + * @details If the denominators of log derivative lookup relation are read_term and write_term, then I_i = + * (read_term_i*write_term_i)^{-1}. + * @note Importantly, I_i = 0 for rows i at which there is no read or write. + * + */ template static void compute_logderivative_inverse(Polynomials& polynomials, auto& relation_parameters, const size_t circuit_size) { auto& inverse_polynomial = get_inverse_polynomial(polynomials); + // Compute the product of the read and write terms for each row for (size_t i = 0; i < circuit_size; ++i) { auto row = polynomials.get_row(i); // We only compute the inverse if this row contains a read gate or data that has been read @@ -177,8 +185,7 @@ template class DatabusLookupRelationImpl { compute_write_term(row, relation_parameters); } } - // WORKTODO: turn this note from Zac into a genuine TODO - // todo might be inverting zero in field bleh bleh + // Compute inverse polynomial I by inverting the product at each row FF::batch_invert(inverse_polynomial); }; From b59e085996d1e4a713c5a5e10e245e7f094e3bc1 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sun, 31 Mar 2024 23:04:11 +0000 Subject: [PATCH 10/21] busvector members are private --- .../goblin_ultra_circuit_builder.cpp | 2 +- .../goblin_ultra_circuit_builder.hpp | 26 ++++++++++++++++--- .../sumcheck/instance/prover_instance.cpp | 2 +- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 6070201e736..615229242d9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -241,7 +241,7 @@ uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, uint32_t value_witness_idx = this->add_variable(value); create_calldata_read_gate({ read_idx_witness_idx, value_witness_idx }); - bus_vector.read_counts[read_idx]++; + bus_vector.increment_count(read_idx); return value_witness_idx; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index 18b06393206..e3a94808dd5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -11,8 +11,6 @@ using namespace bb; template class GoblinUltraCircuitBuilder_ : public UltraCircuitBuilder_> { public: struct BusVector { - std::vector data; // variable indices corresponding to data in this bus vector - std::vector read_counts; // count of reads at each index into data void append(const uint32_t& idx) { @@ -20,9 +18,29 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui read_counts.resize(data.size()); } - size_t size() { return data.size(); } + size_t size() const { return data.size(); } - uint32_t operator[](size_t idx) { return data[idx]; } + const uint32_t& operator[](size_t idx) const + { + ASSERT(idx < size()); + return data[idx]; + } + + const uint32_t& get_read_count(size_t idx) const + { + ASSERT(idx < read_counts.size()); + return read_counts[idx]; + } + + void increment_count(size_t idx) + { + ASSERT(idx < read_counts.size()); + read_counts[idx]++; + } + + private: + std::vector read_counts; // count of reads at each index into data + std::vector data; // variable indices corresponding to data in this bus vector }; struct DataBus { diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp index 74edeee961e..817ee4f5e4d 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp @@ -48,7 +48,7 @@ void ProverInstance_::construct_databus_polynomials(Circuit& circuit) // Note: We do not utilize a zero row for databus columns for (size_t idx = 0; idx < circuit.databus.calldata.size(); ++idx) { public_calldata[idx] = circuit.get_variable(circuit.databus.calldata[idx]); - calldata_read_counts[idx] = circuit.databus.calldata.read_counts[idx]; + calldata_read_counts[idx] = circuit.databus.calldata.get_read_count(idx); } // Compute a simple identity polynomial for use in the databus lookup argument From ccbe3a460d70061c14e3adf3452f96ce6aaaad7b Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sun, 31 Mar 2024 23:42:31 +0000 Subject: [PATCH 11/21] databus in new file and basic builder methods for return data --- .../stdlib_circuit_builders/databus.hpp | 60 +++++++++++++++++++ .../goblin_ultra_circuit_builder.cpp | 25 ++++++-- .../goblin_ultra_circuit_builder.hpp | 56 ++++++----------- 3 files changed, 97 insertions(+), 44 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp new file mode 100644 index 00000000000..8606bfc5c77 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp @@ -0,0 +1,60 @@ +#pragma once + +#include +namespace bb { + +using namespace bb; + +/** + * @brief A DataBus column + * + */ +struct BusVector { + + /** + * @brief Add an element to the data defining this bus column + * + * @param idx Index of the element in the variables vector of a builder + */ + void append(const uint32_t& idx) + { + data.emplace_back(idx); + read_counts.resize(data.size()); // WORKTODO: emplace_back(0)? + } + + size_t size() const { return data.size(); } + + const uint32_t& operator[](size_t idx) const + { + ASSERT(idx < size()); + return data[idx]; + } + + const uint32_t& get_read_count(size_t idx) const + { + ASSERT(idx < read_counts.size()); + return read_counts[idx]; + } + + void increment_count(size_t idx) + { + ASSERT(idx < read_counts.size()); + read_counts[idx]++; + } + + private: + std::vector read_counts; // count of reads at each index into data + std::vector data; // variable indices corresponding to data in this bus vector +}; + +/** + * @brief The DataBus; facilitates storage of public circuit input/output + * @details WORKTODO: explain basic purpose and ideas + * + */ +struct DataBus { + BusVector calldata; + BusVector return_data; +}; + +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 615229242d9..874f63d2bec 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -219,8 +219,8 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co } /** - * @brief Read from calldata - * @details Creates a calldata lookup gate based on the read data + * @brief Read from a databus column + * @details Creates a databus lookup gate based on the input index and read result * * @tparam FF * @param read_idx_witness_idx Variable index of the read index @@ -247,10 +247,10 @@ uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, } /** - * @brief Create a calldata lookup/read gate + * @brief Create a databus lookup/read gate * * @tparam FF - * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value + * @param databus_lookup_gate_ witness indices corresponding to: read index, result value */ template void GoblinUltraCircuitBuilder_::create_databus_read_gate(const databus_lookup_gate_& in) { @@ -278,7 +278,7 @@ template void GoblinUltraCircuitBuilder_::create_databus_read_ } /** - * @brief Create a calldata lookup/read gate + * @brief Create a databus calldata lookup/read gate * * @tparam FF * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value @@ -292,6 +292,21 @@ void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_loo block.q_1()[block.size() - 1] = 1; } +/** + * @brief Create a databus return data lookup/read gate + * + * @tparam FF + * @param databus_lookup_gate_ witness indices corresponding to: read index, result value + */ +template +void GoblinUltraCircuitBuilder_::create_return_data_read_gate(const databus_lookup_gate_& in) +{ + // Create generic read gate then set q_1 = 1 to specify a calldata read + create_databus_read_gate(in); + auto& block = this->blocks.busread; + block.q_2()[block.size() - 1] = 1; +} + /** * @brief Poseidon2 external round gate, activates the q_poseidon2_external selector and relation */ diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index e3a94808dd5..5261a298313 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -2,6 +2,7 @@ #include "barretenberg/execution_trace/execution_trace.hpp" #include "barretenberg/plonk_honk_shared/arithmetization/arithmetization.hpp" #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" +#include "databus.hpp" #include "ultra_circuit_builder.hpp" namespace bb { @@ -10,43 +11,6 @@ using namespace bb; template class GoblinUltraCircuitBuilder_ : public UltraCircuitBuilder_> { public: - struct BusVector { - - void append(const uint32_t& idx) - { - data.emplace_back(idx); - read_counts.resize(data.size()); - } - - size_t size() const { return data.size(); } - - const uint32_t& operator[](size_t idx) const - { - ASSERT(idx < size()); - return data[idx]; - } - - const uint32_t& get_read_count(size_t idx) const - { - ASSERT(idx < read_counts.size()); - return read_counts[idx]; - } - - void increment_count(size_t idx) - { - ASSERT(idx < read_counts.size()); - read_counts[idx]++; - } - - private: - std::vector read_counts; // count of reads at each index into data - std::vector data; // variable indices corresponding to data in this bus vector - }; - - struct DataBus { - BusVector calldata; - }; - using Arithmetization = UltraHonkArith; static constexpr std::string_view NAME_STRING = "GoblinUltraArithmetization"; @@ -75,8 +39,10 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui void populate_ecc_op_wires(const ecc_op_tuple& in); ecc_op_tuple decompose_ecc_operands(uint32_t op, const g1::affine_element& point, const FF& scalar = FF::zero()); void set_goblin_ecc_op_code_constant_variables(); + uint32_t read_bus_vector(BusVector& bus_vector, const uint32_t& read_idx_witness_idx); void create_databus_read_gate(const databus_lookup_gate_& in); void create_calldata_read_gate(const databus_lookup_gate_& in); + void create_return_data_read_gate(const databus_lookup_gate_& in); public: GoblinUltraCircuitBuilder_(const size_t size_hint = 0, @@ -162,10 +128,15 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui /** * @brief Add a witness variable to the public calldata. * - * @param in Value to be added to calldata. * */ uint32_t add_public_calldata(const FF& in) { return add_to_bus_vector(databus.calldata, in); } + /** + * @brief Add a witness variable to the public return_data. + * + * */ + uint32_t add_public_return_data(const FF& in) { return add_to_bus_vector(databus.return_data, in); } + /** * @brief Read from calldata and create a corresponding databus read gate * @@ -174,13 +145,20 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui */ uint32_t read_calldata(const uint32_t& read_idx) { return read_bus_vector(databus.calldata, read_idx); }; + /** + * @brief Read from return_data and create a corresponding databus read gate + * + * @param read_idx Witness index for the return_data read index + * @return uint32_t Witness index for the result of the read + */ + uint32_t read_return_data(const uint32_t& read_idx) { return read_bus_vector(databus.return_data, read_idx); }; + uint32_t add_to_bus_vector(BusVector& bus_vector, const FF& in) { const uint32_t index = this->add_variable(in); bus_vector.append(index); return index; } - uint32_t read_bus_vector(BusVector& bus_vector, const uint32_t& read_idx_witness_idx); void create_poseidon2_external_gate(const poseidon2_external_gate_& in); void create_poseidon2_internal_gate(const poseidon2_internal_gate_& in); From f98b583c2074001f5cb0d2dacf01162ca9baf252 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 1 Apr 2024 17:34:00 +0000 Subject: [PATCH 12/21] fully added return data; tests seem to be passing --- .../relations/databus_lookup_relation.hpp | 80 +++++++++++++------ .../protogalaxy_recursive_verifier.cpp | 6 +- .../verifier/ultra_recursive_verifier.cpp | 9 ++- .../stdlib_circuit_builders/databus.hpp | 2 +- .../goblin_ultra_circuit_builder.cpp | 14 +++- .../goblin_ultra_circuit_builder.hpp | 18 ++++- .../goblin_ultra_flavor.hpp | 59 +++++++++----- .../sumcheck/instance/prover_instance.cpp | 10 ++- .../goblin_ultra_transcript.test.cpp | 7 +- .../barretenberg/ultra_honk/oink_prover.cpp | 17 +++- .../barretenberg/ultra_honk/oink_verifier.cpp | 12 ++- .../ultra_honk/relation_correctness.test.cpp | 5 +- 12 files changed, 175 insertions(+), 64 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 376c78ea487..01e3ef79462 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -13,22 +13,26 @@ namespace bb { template class DatabusLookupRelationImpl { public: using FF = FF_; - static constexpr size_t READ_TERMS = 1; - static constexpr size_t WRITE_TERMS = 1; + static constexpr size_t NUM_BUS_COLUMNS = 2; + static constexpr size_t READ_TERMS = 1; // per bus column + static constexpr size_t WRITE_TERMS = 1; // per bus column // 1 + polynomial degree of this relation static constexpr size_t LENGTH = READ_TERMS + WRITE_TERMS + 3; - static constexpr size_t NUM_BUS_COLUMNS = 1; // Note: The first subrelation actually has length = LENGTH-1 but taking advantage of this would require additional // computation that would nullify the benefits. - static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ + static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ + LENGTH, // inverse polynomial correctness subrelation + LENGTH, // log-derivative lookup argument subrelation LENGTH, // inverse polynomial correctness subrelation LENGTH // log-derivative lookup argument subrelation }; - // The second subrelation is "linearly dependant" in the sense that it establishes the value of a sum across the + // The second subrelation is "linearly dependent" in the sense that it establishes the value of a sum across the // entire execution trace rather than a per-row identity. - static constexpr std::array SUBRELATION_LINEARLY_INDEPENDENT = { true, false }; + static constexpr std::array SUBRELATION_LINEARLY_INDEPENDENT = { + true, false, true, false + }; /** * @brief Determine whether the inverse I needs to be computed at a given row @@ -42,11 +46,14 @@ template class DatabusLookupRelationImpl { */ template static bool operation_exists_at_row(const AllValues& row) { - ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { bool is_read_gate = row.q_busread == 1 && row.q_l == 1; return (is_read_gate || row.calldata_read_counts > 0); } + if constexpr (bus_idx == 1) { + bool is_read_gate = row.q_busread == 1 && row.q_r == 1; + return (is_read_gate || row.return_data_read_counts > 0); + } return false; } @@ -59,9 +66,11 @@ template class DatabusLookupRelationImpl { */ template static auto& get_inverse_polynomial(AllEntities& in) { - ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { - return in.lookup_inverses; + return in.calldata_inverses; + } + if constexpr (bus_idx == 1) { + return in.return_data_inverses; } } @@ -75,7 +84,6 @@ template class DatabusLookupRelationImpl { static Accumulator compute_inverse_exists(const AllEntities& in) { using View = typename Accumulator::View; - ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { auto q_busread = View(in.q_busread); auto q_1 = View(in.q_l); @@ -83,6 +91,15 @@ template class DatabusLookupRelationImpl { // Note: read_counts is constructed such that read_count_i <= 1. const auto is_read_data = View(in.calldata_read_counts); + return is_read_gate + is_read_data - (is_read_gate * is_read_data); + } + if constexpr (bus_idx == 1) { + auto q_busread = View(in.q_busread); + auto q_2 = View(in.q_r); + const auto is_read_gate = q_busread * q_2; + // Note: read_counts is constructed such that read_count_i <= 1. + const auto is_read_data = View(in.return_data_read_counts); + return is_read_gate + is_read_data - (is_read_gate * is_read_data); } } @@ -91,10 +108,12 @@ template class DatabusLookupRelationImpl { static Accumulator::View get_read_counts(const AllEntities& in) { using View = typename Accumulator::View; - ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { return View(in.calldata_read_counts); } + if constexpr (bus_idx == 1) { + return View(in.return_data_read_counts); + } } /** @@ -103,17 +122,24 @@ template class DatabusLookupRelationImpl { * */ template - static Accumulator get_read_selector([[maybe_unused]] const AllEntities& in) + static Accumulator get_read_selector(const AllEntities& in) { using View = typename Accumulator::View; - ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { auto q_busread = View(in.q_busread); auto q_1 = View(in.q_l); auto result = q_busread * q_1; + return result; + } + if constexpr (bus_idx == 1) { + auto q_busread = View(in.q_busread); + auto q_2 = View(in.q_r); + + auto result = q_busread * q_2; + return result; } } @@ -128,7 +154,6 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; using ParameterView = GetParameterView; - ASSERT(bus_idx == 0); if constexpr (bus_idx == 0) { const auto& calldata = View(in.calldata); const auto& id = View(in.databus_id); @@ -139,6 +164,16 @@ template class DatabusLookupRelationImpl { // Construct b_i + idx_i*\beta + \gamma return calldata + gamma + id * beta; // degree 1 } + if constexpr (bus_idx == 1) { + const auto& return_data = View(in.return_data); + const auto& id = View(in.databus_id); + + const auto& gamma = ParameterView(params.gamma); + const auto& beta = ParameterView(params.beta); + + // Construct b_i + idx_i*\beta + \gamma + return return_data + gamma + id * beta; // degree 1 + } } /** @@ -212,23 +247,22 @@ template class DatabusLookupRelationImpl { using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>; using View = typename Accumulator::View; - const auto lookup_inverses = View(get_inverse_polynomial(in)); // Degree 1 + const auto inverses = View(get_inverse_polynomial(in)); // Degree 1 const auto read_term = compute_read_term(in, params); // Degree 1 const auto write_term = compute_write_term(in, params); // Degree 1 const auto inverse_exists = compute_inverse_exists(in); // Degree 1 const auto read_counts = get_read_counts(in); // Degree 1 const auto read_selector = get_read_selector(in); // Degree 2 - const auto write_inverse = lookup_inverses * read_term; // Degree 2 - const auto read_inverse = lookup_inverses * write_term; // Degree 2 + const auto write_inverse = inverses * read_term; // Degree 2 + const auto read_inverse = inverses * write_term; // Degree 2 - // Determine which subrelations to update based on which bus column is being read + // Determine which pair of subrelations to update based on which bus column is being read constexpr size_t subrel_idx_1 = 2 * bus_idx; constexpr size_t subrel_idx_2 = 2 * bus_idx + 1; - // Establish the correctness of the polynomial of inverses I. Note: lookup_inverses is computed so that the - // value is 0 if !inverse_exists. Degree 3 - std::get(accumulator) += - (read_term * write_term * lookup_inverses - inverse_exists) * scaling_factor; + // Establish the correctness of the polynomial of inverses I. Note: inverses is computed so that the value is 0 + // if !inverse_exists. Degree 3 + std::get(accumulator) += (read_term * write_term * inverses - inverse_exists) * scaling_factor; // Establish validity of the read. Note: no scaling factor here since this constraint is enforced across the // entire trace, not on a per-row basis @@ -250,7 +284,7 @@ template class DatabusLookupRelationImpl { const Parameters& params, const FF& scaling_factor) { - // Accumulate the subrelations for column of the databus + // Accumulate the subrelation contributions for each column of the databus bb::constexpr_for<0, NUM_BUS_COLUMNS, 1>([&]() { accumulate_subrelation_contributions(accumulator, in, params, scaling_factor); }); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp index f1b50360f94..e9b0169a5fb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp @@ -62,8 +62,10 @@ void ProtoGalaxyRecursiveVerifier_::receive_and_finalise_inst // If Goblin (i.e. using DataBus) receive commitments to log-deriv inverses polynomial if constexpr (IsGoblinFlavor) { - witness_commitments.lookup_inverses = transcript->template receive_from_prover( - domain_separator + "_" + commitment_labels.lookup_inverses); + witness_commitments.calldata_inverses = transcript->template receive_from_prover( + domain_separator + "_" + commitment_labels.calldata_inverses); + witness_commitments.return_data_inverses = transcript->template receive_from_prover( + domain_separator + "_" + commitment_labels.return_data_inverses); } witness_commitments.z_perm = diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/ultra_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/ultra_recursive_verifier.cpp index cf97878c6d7..36e0a4b943c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/ultra_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/ultra_recursive_verifier.cpp @@ -68,6 +68,9 @@ std::array UltraRecursiveVerifier_::ve commitments.calldata = transcript->template receive_from_prover(commitment_labels.calldata); commitments.calldata_read_counts = transcript->template receive_from_prover(commitment_labels.calldata_read_counts); + commitments.return_data = transcript->template receive_from_prover(commitment_labels.return_data); + commitments.return_data_read_counts = + transcript->template receive_from_prover(commitment_labels.return_data_read_counts); } // Get challenge for sorted list batching and wire four memory records @@ -85,8 +88,10 @@ std::array UltraRecursiveVerifier_::ve // If Goblin (i.e. using DataBus) receive commitments to log-deriv inverses polynomial if constexpr (IsGoblinFlavor) { - commitments.lookup_inverses = - transcript->template receive_from_prover(commitment_labels.lookup_inverses); + commitments.calldata_inverses = + transcript->template receive_from_prover(commitment_labels.calldata_inverses); + commitments.return_data_inverses = + transcript->template receive_from_prover(commitment_labels.return_data_inverses); } const FF public_input_delta = compute_public_input_delta( diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp index 8606bfc5c77..fc0057bdcbf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp @@ -36,7 +36,7 @@ struct BusVector { return read_counts[idx]; } - void increment_count(size_t idx) + void increment_read_count(size_t idx) { ASSERT(idx < read_counts.size()); read_counts[idx]++; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 874f63d2bec..7c929dc2562 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -37,6 +37,13 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ auto read_idx = this->add_variable(raw_read_idx); read_calldata(read_idx); + // Create an arbitrary return data read gate + add_public_return_data(FF(19)); // ensure there is at least one entry in return data + add_public_return_data(FF(17)); // ensure there is at least one entry in return data + raw_read_idx = 1; // read first entry in return data + read_idx = this->add_variable(raw_read_idx); + read_return_data(read_idx); + // mock a poseidon external gate, with all zeros as input this->blocks.poseidon_external.populate_wires(this->zero_idx, this->zero_idx, this->zero_idx, this->zero_idx); this->blocks.poseidon_external.q_m().emplace_back(0); @@ -229,19 +236,18 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co template uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, const uint32_t& read_idx_witness_idx) { - // Get the raw index into the calldata + // Get the raw index into the databus column const uint32_t read_idx = static_cast(uint256_t(this->get_variable(read_idx_witness_idx))); // Ensure that the read index is valid ASSERT(read_idx < bus_vector.size()); // Create a variable corresponding to the result of the read. Note that we do not in general connect reads from - // calldata via copy constraints (i.e. we create a unique variable for the result of each read) + // databus via copy constraints (i.e. we create a unique variable for the result of each read) FF value = this->get_variable(bus_vector[read_idx]); uint32_t value_witness_idx = this->add_variable(value); - create_calldata_read_gate({ read_idx_witness_idx, value_witness_idx }); - bus_vector.increment_count(read_idx); + bus_vector.increment_read_count(read_idx); return value_witness_idx; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index 5261a298313..ae5f04d6f43 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -140,18 +140,28 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui /** * @brief Read from calldata and create a corresponding databus read gate * - * @param read_idx Witness index for the calldata read index + * @param read_idx_witness_idx Witness index for the calldata read index * @return uint32_t Witness index for the result of the read */ - uint32_t read_calldata(const uint32_t& read_idx) { return read_bus_vector(databus.calldata, read_idx); }; + uint32_t read_calldata(const uint32_t& read_idx_witness_idx) + { + uint32_t value_witness_idx = read_bus_vector(databus.calldata, read_idx_witness_idx); + create_calldata_read_gate({ read_idx_witness_idx, value_witness_idx }); + return value_witness_idx; + }; /** * @brief Read from return_data and create a corresponding databus read gate * - * @param read_idx Witness index for the return_data read index + * @param read_idx_witness_idx Witness index for the return_data read index * @return uint32_t Witness index for the result of the read */ - uint32_t read_return_data(const uint32_t& read_idx) { return read_bus_vector(databus.return_data, read_idx); }; + uint32_t read_return_data(const uint32_t& read_idx_witness_idx) + { + uint32_t value_witness_idx = read_bus_vector(databus.return_data, read_idx_witness_idx); + create_return_data_read_gate({ read_idx_witness_idx, value_witness_idx }); + return value_witness_idx; + }; uint32_t add_to_bus_vector(BusVector& bus_vector, const FF& in) { diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp index 654da77590f..9525095fada 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp @@ -40,12 +40,12 @@ class GoblinUltraFlavor { // 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`. // Note: this number does not include the individual sorted list polynomials. - static constexpr size_t NUM_ALL_ENTITIES = 55; + static constexpr size_t NUM_ALL_ENTITIES = 58; // 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 = 14; + static constexpr size_t NUM_WITNESS_ENTITIES = 17; // 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; @@ -173,16 +173,19 @@ class GoblinUltraFlavor { template class DerivedEntities { public: DEFINE_FLAVOR_MEMBERS(DataType, - sorted_accum, // column 4 - z_perm, // column 5 - z_lookup, // column 6 - ecc_op_wire_1, // column 7 - ecc_op_wire_2, // column 8 - ecc_op_wire_3, // column 9 - ecc_op_wire_4, // column 10 - calldata, // column 11 - calldata_read_counts, // column 12 - lookup_inverses); // column 13 + sorted_accum, // column 4 + z_perm, // column 5 + z_lookup, // column 6 + ecc_op_wire_1, // column 7 + ecc_op_wire_2, // column 8 + ecc_op_wire_3, // column 9 + ecc_op_wire_4, // column 10 + calldata, // column 11 + calldata_read_counts, // column 12 + calldata_inverses, // column 13 + return_data, // column 14 + return_data_read_counts, // column 15 + return_data_inverses); // column 16 }; /** @@ -359,9 +362,14 @@ class GoblinUltraFlavor { { auto prover_polynomials = ProverPolynomials(*this); // Compute inverses polynomial used in log derivative relations + // WORKTODO: make this a loop? DatabusLookupRelation::compute_logderivative_inverse( prover_polynomials, relation_parameters, this->circuit_size); - this->lookup_inverses = prover_polynomials.lookup_inverses; + this->calldata_inverses = prover_polynomials.calldata_inverses; + + DatabusLookupRelation::compute_logderivative_inverse( + prover_polynomials, relation_parameters, this->circuit_size); + this->return_data_inverses = prover_polynomials.return_data_inverses; } /** @@ -510,7 +518,10 @@ class GoblinUltraFlavor { ecc_op_wire_4 = "ECC_OP_WIRE_4"; calldata = "CALLDATA"; calldata_read_counts = "CALLDATA_READ_COUNTS"; - lookup_inverses = "LOOKUP_INVERSES"; + calldata_inverses = "CALLDATA_INVERSES"; + return_data = "RETURN_DATA"; + return_data_read_counts = "RETURN_DATA_READ_COUNTS"; + return_data_inverses = "RETURN_DATA_INVERSES"; q_c = "Q_C"; q_l = "Q_L"; @@ -599,7 +610,10 @@ class GoblinUltraFlavor { this->ecc_op_wire_4 = commitments.ecc_op_wire_4; this->calldata = commitments.calldata; this->calldata_read_counts = commitments.calldata_read_counts; - this->lookup_inverses = commitments.lookup_inverses; + this->calldata_inverses = commitments.calldata_inverses; + this->return_data = commitments.return_data; + this->return_data_read_counts = commitments.return_data_read_counts; + this->return_data_inverses = commitments.return_data_inverses; } } }; @@ -626,7 +640,10 @@ class GoblinUltraFlavor { Commitment ecc_op_wire_4_comm; Commitment calldata_comm; Commitment calldata_read_counts_comm; - Commitment lookup_inverses_comm; + Commitment calldata_inverses_comm; + Commitment return_data_comm; + Commitment return_data_read_counts_comm; + Commitment return_data_inverses_comm; Commitment sorted_accum_comm; Commitment w_4_comm; Commitment z_perm_comm; @@ -679,7 +696,10 @@ class GoblinUltraFlavor { ecc_op_wire_4_comm = deserialize_from_buffer(proof_data, num_frs_read); calldata_comm = deserialize_from_buffer(proof_data, num_frs_read); calldata_read_counts_comm = deserialize_from_buffer(proof_data, num_frs_read); - lookup_inverses_comm = deserialize_from_buffer(proof_data, num_frs_read); + calldata_inverses_comm = deserialize_from_buffer(proof_data, num_frs_read); + return_data_comm = deserialize_from_buffer(proof_data, num_frs_read); + return_data_read_counts_comm = deserialize_from_buffer(proof_data, num_frs_read); + return_data_inverses_comm = deserialize_from_buffer(proof_data, num_frs_read); sorted_accum_comm = deserialize_from_buffer(proof_data, num_frs_read); w_4_comm = deserialize_from_buffer(proof_data, num_frs_read); z_perm_comm = deserialize_from_buffer(proof_data, num_frs_read); @@ -717,7 +737,10 @@ class GoblinUltraFlavor { serialize_to_buffer(ecc_op_wire_4_comm, proof_data); serialize_to_buffer(calldata_comm, proof_data); serialize_to_buffer(calldata_read_counts_comm, proof_data); - serialize_to_buffer(lookup_inverses_comm, proof_data); + serialize_to_buffer(calldata_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_inverses_comm, proof_data); serialize_to_buffer(sorted_accum_comm, proof_data); serialize_to_buffer(w_4_comm, proof_data); serialize_to_buffer(z_perm_comm, proof_data); diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp index 817ee4f5e4d..84f1078034c 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp @@ -43,14 +43,20 @@ void ProverInstance_::construct_databus_polynomials(Circuit& circuit) { Polynomial public_calldata{ dyadic_circuit_size }; Polynomial calldata_read_counts{ dyadic_circuit_size }; - Polynomial databus_id{ dyadic_circuit_size }; + Polynomial public_return_data{ dyadic_circuit_size }; + Polynomial return_data_read_counts{ dyadic_circuit_size }; // Note: We do not utilize a zero row for databus columns for (size_t idx = 0; idx < circuit.databus.calldata.size(); ++idx) { public_calldata[idx] = circuit.get_variable(circuit.databus.calldata[idx]); calldata_read_counts[idx] = circuit.databus.calldata.get_read_count(idx); } + for (size_t idx = 0; idx < circuit.databus.return_data.size(); ++idx) { + public_return_data[idx] = circuit.get_variable(circuit.databus.return_data[idx]); + return_data_read_counts[idx] = circuit.databus.return_data.get_read_count(idx); + } + Polynomial databus_id{ dyadic_circuit_size }; // Compute a simple identity polynomial for use in the databus lookup argument for (size_t i = 0; i < databus_id.size(); ++i) { databus_id[i] = i; @@ -58,6 +64,8 @@ void ProverInstance_::construct_databus_polynomials(Circuit& circuit) proving_key->calldata = public_calldata.share(); proving_key->calldata_read_counts = calldata_read_counts.share(); + proving_key->return_data = public_return_data.share(); + proving_key->return_data_read_counts = return_data_read_counts.share(); proving_key->databus_id = databus_id.share(); } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp index 64d10ee0f25..909d22efb31 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp @@ -60,6 +60,8 @@ class GoblinUltraTranscriptTests : public ::testing::Test { manifest_expected.add_entry(round, "ECC_OP_WIRE_4", frs_per_G); manifest_expected.add_entry(round, "CALLDATA", frs_per_G); manifest_expected.add_entry(round, "CALLDATA_READ_COUNTS", frs_per_G); + manifest_expected.add_entry(round, "RETURN_DATA", frs_per_G); + manifest_expected.add_entry(round, "RETURN_DATA_READ_COUNTS", frs_per_G); manifest_expected.add_challenge(round, "eta", "eta_two", "eta_three"); round++; @@ -68,7 +70,8 @@ class GoblinUltraTranscriptTests : public ::testing::Test { manifest_expected.add_challenge(round, "beta", "gamma"); round++; - manifest_expected.add_entry(round, "LOOKUP_INVERSES", frs_per_G); + manifest_expected.add_entry(round, "CALLDATA_INVERSES", frs_per_G); + manifest_expected.add_entry(round, "RETURN_DATA_INVERSES", frs_per_G); manifest_expected.add_entry(round, "Z_PERM", frs_per_G); manifest_expected.add_entry(round, "Z_LOOKUP", frs_per_G); @@ -186,6 +189,8 @@ TEST_F(GoblinUltraTranscriptTests, VerifierManifestConsistency) // Check consistency between the manifests generated by the prover and verifier auto prover_manifest = prover.transcript->get_manifest(); auto verifier_manifest = verifier.transcript->get_manifest(); + // prover_manifest.print(); + // verifier_manifest.print(); // Note: a manifest can be printed using manifest.print() for (size_t round = 0; round < prover_manifest.size(); ++round) { diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp index b5a5047443b..6cd7dd4a321 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp @@ -82,12 +82,18 @@ template void OinkProver::execute_wire_commitment for (size_t idx = 0; idx < Flavor::NUM_WIRES; ++idx) { transcript->send_to_verifier(domain_separator + labels[idx], op_wire_comms[idx]); } - // Commit to DataBus columns + + // Commit to DataBus columns and corresponding read counts witness_commitments.calldata = commitment_key->commit(proving_key->calldata); witness_commitments.calldata_read_counts = commitment_key->commit(proving_key->calldata_read_counts); transcript->send_to_verifier(domain_separator + commitment_labels.calldata, witness_commitments.calldata); transcript->send_to_verifier(domain_separator + commitment_labels.calldata_read_counts, witness_commitments.calldata_read_counts); + witness_commitments.return_data = commitment_key->commit(proving_key->return_data); + witness_commitments.return_data_read_counts = commitment_key->commit(proving_key->return_data_read_counts); + transcript->send_to_verifier(domain_separator + commitment_labels.return_data, witness_commitments.return_data); + transcript->send_to_verifier(domain_separator + commitment_labels.return_data_read_counts, + witness_commitments.return_data_read_counts); } } @@ -127,9 +133,12 @@ template void OinkProver::execute_log_derivative_ if constexpr (IsGoblinFlavor) { // Compute and commit to the logderivative inverse used in DataBus proving_key->compute_logderivative_inverse(relation_parameters); - witness_commitments.lookup_inverses = commitment_key->commit(proving_key->lookup_inverses); - transcript->send_to_verifier(domain_separator + commitment_labels.lookup_inverses, - witness_commitments.lookup_inverses); + witness_commitments.calldata_inverses = commitment_key->commit(proving_key->calldata_inverses); + witness_commitments.return_data_inverses = commitment_key->commit(proving_key->return_data_inverses); + transcript->send_to_verifier(domain_separator + commitment_labels.calldata_inverses, + witness_commitments.calldata_inverses); + transcript->send_to_verifier(domain_separator + commitment_labels.return_data_inverses, + witness_commitments.return_data_inverses); } } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.cpp index 47c21e9bb42..af011b13ce6 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.cpp @@ -72,6 +72,10 @@ template void OinkVerifier::execute_wire_commitme transcript->template receive_from_prover(domain_separator + comm_labels.calldata); witness_comms.calldata_read_counts = transcript->template receive_from_prover(domain_separator + comm_labels.calldata_read_counts); + witness_comms.return_data = + transcript->template receive_from_prover(domain_separator + comm_labels.return_data); + witness_comms.return_data_read_counts = transcript->template receive_from_prover( + domain_separator + comm_labels.return_data_read_counts); } } @@ -103,10 +107,12 @@ template void OinkVerifier::execute_log_derivativ auto [beta, gamma] = transcript->template get_challenges(domain_separator + "beta", domain_separator + "gamma"); relation_parameters.beta = beta; relation_parameters.gamma = gamma; - // 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) { - witness_comms.lookup_inverses = - transcript->template receive_from_prover(domain_separator + comm_labels.lookup_inverses); + witness_comms.calldata_inverses = + transcript->template receive_from_prover(domain_separator + comm_labels.calldata_inverses); + witness_comms.return_data_inverses = + transcript->template receive_from_prover(domain_separator + comm_labels.return_data_inverses); } } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/relation_correctness.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/relation_correctness.test.cpp index 7c21616cd32..739079d4ac4 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/relation_correctness.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/relation_correctness.test.cpp @@ -347,7 +347,10 @@ TEST_F(UltraRelationCorrectnessTests, GoblinUltra) ensure_non_zero(proving_key->calldata); ensure_non_zero(proving_key->calldata_read_counts); - ensure_non_zero(proving_key->lookup_inverses); + ensure_non_zero(proving_key->calldata_inverses); + ensure_non_zero(proving_key->return_data); + ensure_non_zero(proving_key->return_data_read_counts); + ensure_non_zero(proving_key->return_data_inverses); // Construct the round for applying sumcheck relations and results for storing computed results using Relations = typename Flavor::Relations; From 7b264bff939b8ba2982457f2b23a95a77d37c03b Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 1 Apr 2024 18:02:35 +0000 Subject: [PATCH 13/21] update PG recursive verifier --- .../verifier/protogalaxy_recursive_verifier.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp index e9b0169a5fb..2e2d2454610 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp @@ -47,6 +47,10 @@ void ProtoGalaxyRecursiveVerifier_::receive_and_finalise_inst transcript->template receive_from_prover(domain_separator + "_" + labels.calldata); witness_commitments.calldata_read_counts = transcript->template receive_from_prover(domain_separator + "_" + labels.calldata_read_counts); + witness_commitments.return_data = + transcript->template receive_from_prover(domain_separator + "_" + labels.return_data); + witness_commitments.return_data_read_counts = transcript->template receive_from_prover( + domain_separator + "_" + labels.return_data_read_counts); } // Get challenge for sorted list batching and wire four memory records commitment From c5008aa22aedc61068406cc5e29cd7dbbf21fd61 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 1 Apr 2024 22:04:42 +0000 Subject: [PATCH 14/21] more tests and an assert on read counts --- .../goblin_ultra_circuit_builder.cpp | 9 +- .../ultra_honk/databus_composer.test.cpp | 134 ++++++++++++++---- 2 files changed, 114 insertions(+), 29 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 7c929dc2562..a6d8390e72c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -33,14 +33,13 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ // Create an arbitrary calldata read gate add_public_calldata(FF(25)); // ensure there is at least one entry in calldata - uint32_t raw_read_idx = 0; // read first entry in calldata + auto raw_read_idx = static_cast(databus.calldata.size()) - 1; // read data that was just added auto read_idx = this->add_variable(raw_read_idx); read_calldata(read_idx); // Create an arbitrary return data read gate - add_public_return_data(FF(19)); // ensure there is at least one entry in return data add_public_return_data(FF(17)); // ensure there is at least one entry in return data - raw_read_idx = 1; // read first entry in return data + raw_read_idx = static_cast(databus.return_data.size()) - 1; // read data that was just added read_idx = this->add_variable(raw_read_idx); read_return_data(read_idx); @@ -239,8 +238,8 @@ uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, // Get the raw index into the databus column const uint32_t read_idx = static_cast(uint256_t(this->get_variable(read_idx_witness_idx))); - // Ensure that the read index is valid - ASSERT(read_idx < bus_vector.size()); + ASSERT(read_idx < bus_vector.size()); // Ensure that the read index is valid + ASSERT(bus_vector.get_read_count(read_idx) < 1); // Reading more than once at the same index is not supported // Create a variable corresponding to the result of the read. Note that we do not in general connect reads from // databus via copy constraints (i.e. we create a unique variable for the result of each read) diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp index b7729c1099f..1606dd6f057 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -23,23 +23,34 @@ class DataBusComposerTests : public ::testing::Test { using Curve = curve::BN254; using FF = Curve::ScalarField; + + static bool construct_and_verify_proof(GoblinUltraCircuitBuilder& builder) + { + GoblinUltraProver prover{ builder }; + auto verification_key = std::make_shared(prover.instance->proving_key); + GoblinUltraVerifier verifier{ verification_key }; + auto proof = prover.construct_proof(); + return verifier.verify_proof(proof); + } + + static GoblinUltraCircuitBuilder construct_test_builder() + { + auto op_queue = std::make_shared(); + auto builder = GoblinUltraCircuitBuilder{ op_queue }; + GoblinMockCircuits::construct_simple_circuit(builder); + return builder; + } }; /** * @brief Test proof construction/verification for a circuit with calldata lookup gates * gates - * @note We simulate op queue interactions with a previous circuit so the actual circuit under test utilizes an op queue - * with non-empty 'previous' data. This avoid complications with zero-commitments etc. * */ TEST_F(DataBusComposerTests, CallDataRead) { - auto op_queue = std::make_shared(); - - auto builder = GoblinUltraCircuitBuilder{ op_queue }; - - // Add some ecc op gates and arithmetic gates - GoblinMockCircuits::construct_simple_circuit(builder); + // Construct a circuit and add some ecc op gates and arithmetic gates + auto builder = construct_test_builder(); // Add some values to calldata std::vector calldata_values = { 7, 10, 3, 12, 1 }; @@ -47,32 +58,107 @@ TEST_F(DataBusComposerTests, CallDataRead) builder.add_public_calldata(val); } - // Define some raw indices at which to read calldata (these will be ASSERTed to be valid) + // Define some raw indices at which to read calldata std::vector read_indices = { 1, 4 }; - // Create some calldata read gates. (Normally we'd use the result of the read. Example of that is below) + // Create some calldata read gates and store the variable indices of the result for later + std::vector result_witness_indices; for (uint32_t& read_idx : read_indices) { // Create a variable corresponding to the index at which we want to read into calldata uint32_t read_idx_witness_idx = builder.add_variable(read_idx); - builder.read_calldata(read_idx_witness_idx); + auto value_witness_idx = builder.read_calldata(read_idx_witness_idx); + result_witness_indices.emplace_back(value_witness_idx); + } + + // Generally, we'll want to use the result of a read in some other operation. As an example, we construct a gate + // that shows the sum of the two values just read is equal to the expected sum. + FF expected_sum = 0; + for (uint32_t& read_idx : read_indices) { + expected_sum += calldata_values[read_idx]; + } + builder.create_add_gate( + { result_witness_indices[0], result_witness_indices[1], builder.zero_idx, 1, 1, 0, -expected_sum }); + + // Construct and verify Honk proof + bool result = construct_and_verify_proof(builder); + EXPECT_TRUE(result); +} + +/** + * @brief Test proof construction/verification for a circuit with return data lookup gates + * gates + * + */ +TEST_F(DataBusComposerTests, ReturnDataRead) +{ + // Construct a circuit and add some ecc op gates and arithmetic gates + auto builder = construct_test_builder(); + + // Add some values to return_data + std::vector return_data_values = { 7, 10, 3, 12, 1 }; + for (auto& val : return_data_values) { + builder.add_public_return_data(val); + } + + // Define some raw indices at which to read return_data + std::vector read_indices = { 1, 4 }; + + // Create some return_data read gates and store the variable indices of the result for later + std::vector result_witness_indices; + for (uint32_t& read_idx : read_indices) { + // Create a variable corresponding to the index at which we want to read into return_data + uint32_t read_idx_witness_idx = builder.add_variable(read_idx); + + auto value_witness_idx = builder.read_return_data(read_idx_witness_idx); + result_witness_indices.emplace_back(value_witness_idx); + } + + // Generally, we'll want to use the result of a read in some other operation. As an example, we construct a gate + // that shows the sum of the two values just read is equal to the expected sum. + FF expected_sum = 0; + for (uint32_t& read_idx : read_indices) { + expected_sum += return_data_values[read_idx]; } + builder.create_add_gate( + { result_witness_indices[0], result_witness_indices[1], builder.zero_idx, 1, 1, 0, -expected_sum }); + + // Construct and verify Honk proof + bool result = construct_and_verify_proof(builder); + EXPECT_TRUE(result); +} - // In general we'll want to use the result of a calldata read in another operation. Here's an example using - // an add gate to show that the result of the read is as expected: +/** + * @brief Test reads from calldata and return data in the same circuit + * + */ +TEST_F(DataBusComposerTests, CallDataAndReturnData) +{ + // Construct a circuit and add some ecc op gates and arithmetic gates + auto builder = construct_test_builder(); + + // Add some values to calldata + std::vector calldata_values = { 5, 27, 11 }; + for (auto& val : calldata_values) { + builder.add_public_calldata(val); + } + + // Add some values to return_data + std::vector return_data_values = { 7, 10 }; + for (auto& val : return_data_values) { + builder.add_public_return_data(val); + } + + // Make some aribitrary reads from calldata and return data uint32_t read_idx = 2; - FF expected_result = calldata_values[read_idx]; uint32_t read_idx_witness_idx = builder.add_variable(read_idx); - uint32_t result_witness_idx = builder.read_calldata(read_idx_witness_idx); - builder.create_add_gate({ result_witness_idx, builder.zero_idx, builder.zero_idx, 1, 0, 0, -expected_result }); + builder.read_calldata(read_idx_witness_idx); + + read_idx = 0; + read_idx_witness_idx = builder.add_variable(read_idx); + builder.read_return_data(read_idx_witness_idx); // Construct and verify Honk proof - auto instance = std::make_shared>(builder); - // For debugging, use "instance_inspector::print_databus_info(instance)" - GoblinUltraProver prover(instance); - auto verification_key = std::make_shared(instance->proving_key); - GoblinUltraVerifier verifier(verification_key); - auto proof = prover.construct_proof(); - bool verified = verifier.verify_proof(proof); - EXPECT_TRUE(verified); + bool result = construct_and_verify_proof(builder); + EXPECT_TRUE(result); } From b3272417d7a36adf1e3289daabf685444882f3b6 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 1 Apr 2024 23:42:52 +0000 Subject: [PATCH 15/21] remove some duplication in bus relation --- .../relations/databus_lookup_relation.hpp | 61 +++++++------------ 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 01e3ef79462..32a2b373d1d 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -35,7 +35,7 @@ template class DatabusLookupRelationImpl { }; /** - * @brief Determine whether the inverse I needs to be computed at a given row + * @brief Determine whether the inverse I needs to be computed at a given row for a given bus column * @details The value of the inverse polynomial I(X) only needs to be computed when the databus lookup gate is * "active". Otherwise it is set to 0. This method allows for determination of when the inverse should be computed. * @@ -46,19 +46,19 @@ template class DatabusLookupRelationImpl { */ template static bool operation_exists_at_row(const AllValues& row) { + auto read_selector = get_read_selector(row); + if constexpr (bus_idx == 0) { - bool is_read_gate = row.q_busread == 1 && row.q_l == 1; - return (is_read_gate || row.calldata_read_counts > 0); + return (read_selector == 1 || row.calldata_read_counts > 0); } if constexpr (bus_idx == 1) { - bool is_read_gate = row.q_busread == 1 && row.q_r == 1; - return (is_read_gate || row.return_data_read_counts > 0); + return (read_selector == 1 || row.return_data_read_counts > 0); } return false; } /** - * @brief Get the lookup inverse polynomial + * @brief Get the lookup inverse polynomial for the indicated bus column * * @tparam AllEntities * @param in @@ -78,28 +78,22 @@ template class DatabusLookupRelationImpl { * @brief Compute the Accumulator whose values indicate whether the inverse is computed or not * @details This is needed for efficiency since we don't need to compute the inverse unless the log derivative * lookup relation is active at a given row. + * @note read_counts is constructed such that read_count_i <= 1 and is thus treated as boolean. * */ template static Accumulator compute_inverse_exists(const AllEntities& in) { using View = typename Accumulator::View; + + const auto is_read_gate = get_read_selector(in); + if constexpr (bus_idx == 0) { - auto q_busread = View(in.q_busread); - auto q_1 = View(in.q_l); - const auto is_read_gate = q_busread * q_1; - // Note: read_counts is constructed such that read_count_i <= 1. const auto is_read_data = View(in.calldata_read_counts); - return is_read_gate + is_read_data - (is_read_gate * is_read_data); } if constexpr (bus_idx == 1) { - auto q_busread = View(in.q_busread); - auto q_2 = View(in.q_r); - const auto is_read_gate = q_busread * q_2; - // Note: read_counts is constructed such that read_count_i <= 1. const auto is_read_data = View(in.return_data_read_counts); - return is_read_gate + is_read_data - (is_read_gate * is_read_data); } } @@ -123,24 +117,18 @@ template class DatabusLookupRelationImpl { */ template static Accumulator get_read_selector(const AllEntities& in) - { using View = typename Accumulator::View; - if constexpr (bus_idx == 0) { - auto q_busread = View(in.q_busread); - auto q_1 = View(in.q_l); - auto result = q_busread * q_1; + auto q_busread = View(in.q_busread); - return result; + if constexpr (bus_idx == 0) { + auto q_1 = View(in.q_l); + return q_busread * q_1; } if constexpr (bus_idx == 1) { - auto q_busread = View(in.q_busread); auto q_2 = View(in.q_r); - - auto result = q_busread * q_2; - - return result; + return q_busread * q_2; } } @@ -154,24 +142,17 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; using ParameterView = GetParameterView; + const auto& gamma = ParameterView(params.gamma); + const auto& beta = ParameterView(params.beta); + const auto& id = View(in.databus_id); + + // Construct b_i + idx_i*\beta + \gamma if constexpr (bus_idx == 0) { const auto& calldata = View(in.calldata); - const auto& id = View(in.databus_id); - - const auto& gamma = ParameterView(params.gamma); - const auto& beta = ParameterView(params.beta); - - // Construct b_i + idx_i*\beta + \gamma return calldata + gamma + id * beta; // degree 1 } if constexpr (bus_idx == 1) { const auto& return_data = View(in.return_data); - const auto& id = View(in.databus_id); - - const auto& gamma = ParameterView(params.gamma); - const auto& beta = ParameterView(params.beta); - - // Construct b_i + idx_i*\beta + \gamma return return_data + gamma + id * beta; // degree 1 } } @@ -220,7 +201,7 @@ template class DatabusLookupRelationImpl { compute_write_term(row, relation_parameters); } } - // Compute inverse polynomial I by inverting the product at each row + // Compute inverse polynomial I in place by inverting the product at each row FF::batch_invert(inverse_polynomial); }; From adcba13ebfd05e168653b051bb30af8d5611bb9b Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Apr 2024 18:02:56 +0000 Subject: [PATCH 16/21] refactor relation with BusData --- .../relations/databus_lookup_relation.hpp | 113 ++++++------------ 1 file changed, 39 insertions(+), 74 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 32a2b373d1d..dc4cb010239 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -3,7 +3,6 @@ #include #include "barretenberg/common/constexpr_utils.hpp" -#include "barretenberg/honk/proof_system/logderivative_library.hpp" #include "barretenberg/polynomials/polynomial.hpp" #include "barretenberg/polynomials/univariate.hpp" #include "barretenberg/relations/relation_types.hpp" @@ -28,12 +27,32 @@ template class DatabusLookupRelationImpl { LENGTH // log-derivative lookup argument subrelation }; - // The second subrelation is "linearly dependent" in the sense that it establishes the value of a sum across the + // The lookup subrelations are "linearly dependent" in the sense that they establishe the value of a sum across the // entire execution trace rather than a per-row identity. static constexpr std::array SUBRELATION_LINEARLY_INDEPENDENT = { true, false, true, false }; + template struct BusData; + + // Specialization for bus_idx 0 + template struct BusData { + static auto& values(const AllEntities& in) { return in.calldata; } + static auto& selector(const AllEntities& in) { return in.q_l; } + static auto& inverses(AllEntities& in) { return in.calldata_inverses; } + static auto& inverses(const AllEntities& in) { return in.calldata_inverses; } // const version + static auto& read_counts(const AllEntities& in) { return in.calldata_read_counts; } + }; + + // Specialization for bus_idx 1 + template struct BusData { + static auto& values(const AllEntities& in) { return in.return_data; } + static auto& selector(const AllEntities& in) { return in.q_r; } + 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; } + }; + /** * @brief Determine whether the inverse I needs to be computed at a given row for a given bus column * @details The value of the inverse polynomial I(X) only needs to be computed when the databus lookup gate is @@ -47,31 +66,8 @@ template class DatabusLookupRelationImpl { template static bool operation_exists_at_row(const AllValues& row) { auto read_selector = get_read_selector(row); - - if constexpr (bus_idx == 0) { - return (read_selector == 1 || row.calldata_read_counts > 0); - } - if constexpr (bus_idx == 1) { - return (read_selector == 1 || row.return_data_read_counts > 0); - } - return false; - } - - /** - * @brief Get the lookup inverse polynomial for the indicated bus column - * - * @tparam AllEntities - * @param in - * @return auto& - */ - template static auto& get_inverse_polynomial(AllEntities& in) - { - if constexpr (bus_idx == 0) { - return in.calldata_inverses; - } - if constexpr (bus_idx == 1) { - return in.return_data_inverses; - } + auto read_counts = BusData::read_counts(row); + return (read_selector == 1 || read_counts > 0); } /** @@ -87,27 +83,9 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; const auto is_read_gate = get_read_selector(in); + const auto read_counts = View(BusData::read_counts(in)); - if constexpr (bus_idx == 0) { - const auto is_read_data = View(in.calldata_read_counts); - return is_read_gate + is_read_data - (is_read_gate * is_read_data); - } - if constexpr (bus_idx == 1) { - const auto is_read_data = View(in.return_data_read_counts); - return is_read_gate + is_read_data - (is_read_gate * is_read_data); - } - } - - template - static Accumulator::View get_read_counts(const AllEntities& in) - { - using View = typename Accumulator::View; - if constexpr (bus_idx == 0) { - return View(in.calldata_read_counts); - } - if constexpr (bus_idx == 1) { - return View(in.return_data_read_counts); - } + return is_read_gate + read_counts - (is_read_gate * read_counts); } /** @@ -121,15 +99,9 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; auto q_busread = View(in.q_busread); + auto column_selector = View(BusData::selector(in)); - if constexpr (bus_idx == 0) { - auto q_1 = View(in.q_l); - return q_busread * q_1; - } - if constexpr (bus_idx == 1) { - auto q_2 = View(in.q_r); - return q_busread * q_2; - } + return q_busread * column_selector; } /** @@ -142,19 +114,13 @@ template class DatabusLookupRelationImpl { using View = typename Accumulator::View; using ParameterView = GetParameterView; + const auto& id = View(in.databus_id); + const auto& value = View(BusData::values(in)); const auto& gamma = ParameterView(params.gamma); const auto& beta = ParameterView(params.beta); - const auto& id = View(in.databus_id); // Construct b_i + idx_i*\beta + \gamma - if constexpr (bus_idx == 0) { - const auto& calldata = View(in.calldata); - return calldata + gamma + id * beta; // degree 1 - } - if constexpr (bus_idx == 1) { - const auto& return_data = View(in.return_data); - return return_data + gamma + id * beta; // degree 1 - } + return value + gamma + id * beta; // degree 1 } /** @@ -171,7 +137,6 @@ template class DatabusLookupRelationImpl { // Bus value stored in w_1, index into bus column stored in w_2 const auto& w_1 = View(in.w_l); const auto& w_2 = View(in.w_r); - const auto& gamma = ParameterView(params.gamma); const auto& beta = ParameterView(params.beta); @@ -191,7 +156,7 @@ template class DatabusLookupRelationImpl { auto& relation_parameters, const size_t circuit_size) { - auto& inverse_polynomial = get_inverse_polynomial(polynomials); + auto& inverse_polynomial = BusData::inverses(polynomials); // Compute the product of the read and write terms for each row for (size_t i = 0; i < circuit_size; ++i) { auto row = polynomials.get_row(i); @@ -228,14 +193,14 @@ template class DatabusLookupRelationImpl { using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>; using View = typename Accumulator::View; - const auto inverses = View(get_inverse_polynomial(in)); // Degree 1 - const auto read_term = compute_read_term(in, params); // Degree 1 - const auto write_term = compute_write_term(in, params); // Degree 1 - const auto inverse_exists = compute_inverse_exists(in); // Degree 1 - const auto read_counts = get_read_counts(in); // Degree 1 - const auto read_selector = get_read_selector(in); // Degree 2 - const auto write_inverse = inverses * read_term; // Degree 2 - const auto read_inverse = inverses * write_term; // Degree 2 + const auto inverses = View(BusData::inverses(in)); // Degree 1 + const auto read_counts = View(BusData::read_counts(in)); // Degree 1 + const auto read_term = compute_read_term(in, params); // Degree 1 + const auto write_term = compute_write_term(in, params); // Degree 1 + const auto inverse_exists = compute_inverse_exists(in); // Degree 1 + const auto read_selector = get_read_selector(in); // Degree 2 + const auto write_inverse = inverses * read_term; // Degree 2 + const auto read_inverse = inverses * write_term; // Degree 2 // Determine which pair of subrelations to update based on which bus column is being read constexpr size_t subrel_idx_1 = 2 * bus_idx; From c3d290b4d292fb004888aed54d149319611f32ec Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Apr 2024 20:36:07 +0000 Subject: [PATCH 17/21] cleanup and commenting --- .../relations/databus_lookup_relation.hpp | 14 ++++++-------- .../stdlib_circuit_builders/databus.hpp | 8 ++++++-- .../goblin_ultra_circuit_builder.cpp | 2 +- .../goblin_ultra_circuit_builder.hpp | 17 ++++++++--------- .../goblin_ultra_flavor.hpp | 7 ++++--- ...tabus_composer.test.cpp => databus.test.cpp} | 10 ++++++---- .../ultra_honk/goblin_ultra_transcript.test.cpp | 2 -- 7 files changed, 31 insertions(+), 29 deletions(-) rename barretenberg/cpp/src/barretenberg/ultra_honk/{databus_composer.test.cpp => databus.test.cpp} (95%) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index dc4cb010239..424d2b8c990 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -18,8 +18,7 @@ template class DatabusLookupRelationImpl { // 1 + polynomial degree of this relation static constexpr size_t LENGTH = READ_TERMS + WRITE_TERMS + 3; - // Note: The first subrelation actually has length = LENGTH-1 but taking advantage of this would require additional - // computation that would nullify the benefits. + // Note: Inverse correctness subrelations are actually LENGTH-1; taking advantage would require additional work static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ LENGTH, // inverse polynomial correctness subrelation LENGTH, // log-derivative lookup argument subrelation @@ -27,15 +26,16 @@ template class DatabusLookupRelationImpl { LENGTH // log-derivative lookup argument subrelation }; - // The lookup subrelations are "linearly dependent" in the sense that they establishe the value of a sum across the + // 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 SUBRELATION_LINEARLY_INDEPENDENT = { true, false, true, false }; + // Interface for easy access of databus components by column (bus_idx) template struct BusData; - // Specialization for bus_idx 0 + // Specialization for calldata (bus_idx = 0) template struct BusData { static auto& values(const AllEntities& in) { return in.calldata; } static auto& selector(const AllEntities& in) { return in.q_l; } @@ -44,7 +44,7 @@ template class DatabusLookupRelationImpl { static auto& read_counts(const AllEntities& in) { return in.calldata_read_counts; } }; - // Specialization for bus_idx 1 + // Specialization for return data (bus_idx = 1) template struct BusData { static auto& values(const AllEntities& in) { return in.return_data; } static auto& selector(const AllEntities& in) { return in.q_r; } @@ -60,8 +60,6 @@ template class DatabusLookupRelationImpl { * * @tparam AllValues * @param row - * @return true - * @return false */ template static bool operation_exists_at_row(const AllValues& row) { @@ -119,7 +117,7 @@ template class DatabusLookupRelationImpl { const auto& gamma = ParameterView(params.gamma); const auto& beta = ParameterView(params.beta); - // Construct b_i + idx_i*\beta + \gamma + // Construct value_i + idx_i*\beta + \gamma return value + gamma + id * beta; // degree 1 } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp index fc0057bdcbf..caba698e6ab 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp @@ -19,7 +19,7 @@ struct BusVector { void append(const uint32_t& idx) { data.emplace_back(idx); - read_counts.resize(data.size()); // WORKTODO: emplace_back(0)? + read_counts.emplace_back(0); } size_t size() const { return data.size(); } @@ -49,7 +49,11 @@ struct BusVector { /** * @brief The DataBus; facilitates storage of public circuit input/output - * @details WORKTODO: explain basic purpose and ideas + * @details The DataBus is designed to facilitate efficient transfer of large amounts of public data between circuits. + * It is expected that only a small subset of the data being passed needs to be used in any single circuit, thus we + * provide a read mechanism (implemented through a Builder) that results in prover work proportional to only the data + * that is used. (The prover must still commit to all data in each bus vector but we do not need to hash all data + * in-circuit as we would with public inputs). * */ struct DataBus { diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index a6d8390e72c..d013730b170 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -306,7 +306,7 @@ void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_loo template void GoblinUltraCircuitBuilder_::create_return_data_read_gate(const databus_lookup_gate_& in) { - // Create generic read gate then set q_1 = 1 to specify a calldata read + // Create generic read gate then set q_2 = 1 to specify a return data read create_databus_read_gate(in); auto& block = this->blocks.busread; block.q_2()[block.size() - 1] = 1; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index ae5f04d6f43..0cb7520570e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -43,6 +43,12 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui void create_databus_read_gate(const databus_lookup_gate_& in); void create_calldata_read_gate(const databus_lookup_gate_& in); void create_return_data_read_gate(const databus_lookup_gate_& in); + uint32_t append_to_bus_vector(BusVector& bus_vector, const FF& in) + { + const uint32_t index = this->add_variable(in); + bus_vector.append(index); + return index; + } public: GoblinUltraCircuitBuilder_(const size_t size_hint = 0, @@ -129,13 +135,13 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui * @brief Add a witness variable to the public calldata. * * */ - uint32_t add_public_calldata(const FF& in) { return add_to_bus_vector(databus.calldata, in); } + uint32_t add_public_calldata(const FF& in) { return append_to_bus_vector(databus.calldata, in); } /** * @brief Add a witness variable to the public return_data. * * */ - uint32_t add_public_return_data(const FF& in) { return add_to_bus_vector(databus.return_data, in); } + uint32_t add_public_return_data(const FF& in) { return append_to_bus_vector(databus.return_data, in); } /** * @brief Read from calldata and create a corresponding databus read gate @@ -163,13 +169,6 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui return value_witness_idx; }; - uint32_t add_to_bus_vector(BusVector& bus_vector, const FF& in) - { - const uint32_t index = this->add_variable(in); - bus_vector.append(index); - return index; - } - void create_poseidon2_external_gate(const poseidon2_external_gate_& in); void create_poseidon2_internal_gate(const poseidon2_internal_gate_& in); }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp index ae94b80fb52..677749016b5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_flavor.hpp @@ -352,7 +352,7 @@ class GoblinUltraFlavor { } /** - * @brief Compute the inverse polynomial used in the log derivative lookup argument + * @brief Compute the inverse polynomial used in the databus log derivative lookup argument * * @tparam Flavor * @param beta @@ -361,12 +361,13 @@ class GoblinUltraFlavor { void compute_logderivative_inverse(const RelationParameters& relation_parameters) { auto prover_polynomials = ProverPolynomials(*this); - // Compute inverses polynomial used in log derivative relations - // WORKTODO: make this a loop? + + // Compute inverses for calldata reads DatabusLookupRelation::compute_logderivative_inverse( prover_polynomials, relation_parameters, this->circuit_size); this->calldata_inverses = prover_polynomials.calldata_inverses; + // Compute inverses for return data reads DatabusLookupRelation::compute_logderivative_inverse( prover_polynomials, relation_parameters, this->circuit_size); this->return_data_inverses = prover_polynomials.return_data_inverses; diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus.test.cpp similarity index 95% rename from barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp rename to barretenberg/cpp/src/barretenberg/ultra_honk/databus.test.cpp index 1606dd6f057..46c418fe1fa 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus.test.cpp @@ -17,13 +17,14 @@ namespace { auto& engine = numeric::get_debug_randomness(); } -class DataBusComposerTests : public ::testing::Test { +class DataBusTests : public ::testing::Test { protected: static void SetUpTestSuite() { bb::srs::init_crs_factory("../srs_db/ignition"); } using Curve = curve::BN254; using FF = Curve::ScalarField; + // Construct and verify a GUH proof for a given circuit static bool construct_and_verify_proof(GoblinUltraCircuitBuilder& builder) { GoblinUltraProver prover{ builder }; @@ -33,6 +34,7 @@ class DataBusComposerTests : public ::testing::Test { return verifier.verify_proof(proof); } + // Construct a Goblin Ultra circuit with some arbitrary sample gates static GoblinUltraCircuitBuilder construct_test_builder() { auto op_queue = std::make_shared(); @@ -47,7 +49,7 @@ class DataBusComposerTests : public ::testing::Test { * gates * */ -TEST_F(DataBusComposerTests, CallDataRead) +TEST_F(DataBusTests, CallDataRead) { // Construct a circuit and add some ecc op gates and arithmetic gates auto builder = construct_test_builder(); @@ -90,7 +92,7 @@ TEST_F(DataBusComposerTests, CallDataRead) * gates * */ -TEST_F(DataBusComposerTests, ReturnDataRead) +TEST_F(DataBusTests, ReturnDataRead) { // Construct a circuit and add some ecc op gates and arithmetic gates auto builder = construct_test_builder(); @@ -132,7 +134,7 @@ TEST_F(DataBusComposerTests, ReturnDataRead) * @brief Test reads from calldata and return data in the same circuit * */ -TEST_F(DataBusComposerTests, CallDataAndReturnData) +TEST_F(DataBusTests, CallDataAndReturnData) { // Construct a circuit and add some ecc op gates and arithmetic gates auto builder = construct_test_builder(); diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp index 71a7e2dba1b..0bccfa8084f 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_transcript.test.cpp @@ -189,8 +189,6 @@ TEST_F(GoblinUltraTranscriptTests, VerifierManifestConsistency) // Check consistency between the manifests generated by the prover and verifier auto prover_manifest = prover.transcript->get_manifest(); auto verifier_manifest = verifier.transcript->get_manifest(); - // prover_manifest.print(); - // verifier_manifest.print(); // Note: a manifest can be printed using manifest.print() for (size_t round = 0; round < prover_manifest.size(); ++round) { From 8e5decf24a8cd0423e0807436f4f252fe7d4d3ea Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Apr 2024 21:28:46 +0000 Subject: [PATCH 18/21] big comment for databus relation --- .../relations/databus_lookup_relation.hpp | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 424d2b8c990..7f7a2f0f215 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -9,14 +9,41 @@ namespace bb { +/** + * @brief Log-derivative lookup argument relation for estabishing DataBus reads + * @details Each column of the databus can be thought of as a table from which we can look up values. The log-derivative + * lookup argument seeks to prove lookups from a column by establishing the following sum: + * + * \sum_{i=0}^{n-1} q_{logderiv_lookup}_i * (1 / write_term_i) + read_count_i * (1 / read_term_i) = 0 + * + * where the read and write terms are both of the form value_i + idx_i*\beta + \gamma. For the write term, the (idx, + * value) pair comes from the "table" (bus column), and for the read term the (idx, value) pair comes from wires 1 and 2 + * which should contain a valid entry in the table. + * + * In practice, we must rephrase this expression in terms of polynomials, one of which is a polynomial I containing + * (indirectly) the rational functions in the above expression: I_i = 1/[(read_term_i) * (write_term_i)]. This leads to + * two subrelations. The first demonstrates that the inverse polynomial I is correctly formed. The second is the primary + * lookup identity, where the rational functions are replaced by the use of the inverse polynomial I. These two + * subrelations can be expressed as follows: + * + * (1) I_i * (read_term_i) * (write_term_i) - 1 = 0 + * + * (2) \sum_{i=0}^{n-1} [q_{logderiv_lookup} * I_i * write_term_i + read_count_i * I_i * read_term_i] = 0 + * + * Each column of the DataBus requires its own pair of subrelations. The column being read is selected via a unique + * product, i.e. a lookup from bus column j is selected via q_busread * q_j (j = 1,2,...). + * + * Note: that the latter subrelation is "linearly dependent" in the sense that it establishes that a sum across all + * rows of the exectution trace is zero, rather than that some expression holds independently at each row. Accordingly, + * this subrelation is not multiplied by a scaling factor at each accumulation step. + * + * + */ template class DatabusLookupRelationImpl { public: using FF = FF_; - static constexpr size_t NUM_BUS_COLUMNS = 2; - static constexpr size_t READ_TERMS = 1; // per bus column - static constexpr size_t WRITE_TERMS = 1; // per bus column - // 1 + polynomial degree of this relation - static constexpr size_t LENGTH = READ_TERMS + WRITE_TERMS + 3; + static constexpr size_t LENGTH = 5; // 1 + polynomial degree of this relation + static constexpr size_t NUM_BUS_COLUMNS = 2; // calldata, return data // Note: Inverse correctness subrelations are actually LENGTH-1; taking advantage would require additional work static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ From 3d410bdde6df04aaf017762df0888f8cf3aa380a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Apr 2024 21:30:24 +0000 Subject: [PATCH 19/21] woops --- .../cpp/src/barretenberg/relations/databus_lookup_relation.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 7f7a2f0f215..8301d532004 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -37,7 +37,6 @@ namespace bb { * rows of the exectution trace is zero, rather than that some expression holds independently at each row. Accordingly, * this subrelation is not multiplied by a scaling factor at each accumulation step. * - * */ template class DatabusLookupRelationImpl { public: From 7132823ad230215c57455d1b9111ef14beaed6eb Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Apr 2024 21:30:47 +0000 Subject: [PATCH 20/21] typo --- .../cpp/src/barretenberg/relations/databus_lookup_relation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 8301d532004..f82f316491b 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -10,7 +10,7 @@ namespace bb { /** - * @brief Log-derivative lookup argument relation for estabishing DataBus reads + * @brief Log-derivative lookup argument relation for establishing DataBus reads * @details Each column of the databus can be thought of as a table from which we can look up values. The log-derivative * lookup argument seeks to prove lookups from a column by establishing the following sum: * From a64d57a74cf90506af8c5e19be6a5ede72c99c7c Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Apr 2024 21:13:17 +0000 Subject: [PATCH 21/21] comments in response to review --- .../relations/databus_lookup_relation.hpp | 11 ++++++++--- .../barretenberg/stdlib_circuit_builders/databus.hpp | 4 ++-- .../goblin_ultra_circuit_builder.cpp | 5 +++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index f82f316491b..d919738a928 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -16,9 +16,14 @@ namespace bb { * * \sum_{i=0}^{n-1} q_{logderiv_lookup}_i * (1 / write_term_i) + read_count_i * (1 / read_term_i) = 0 * - * where the read and write terms are both of the form value_i + idx_i*\beta + \gamma. For the write term, the (idx, - * value) pair comes from the "table" (bus column), and for the read term the (idx, value) pair comes from wires 1 and 2 - * which should contain a valid entry in the table. + * where the read and write terms are both of the form value_i + idx_i*\beta + \gamma. This expression is motivated by + * taking the derivative of the log of a more conventional grand product style set equivalence argument (see e.g. + * https://eprint.iacr.org/2022/1530.pdf for details). For the write term, the (idx, value) pair comes from the "table" + * (bus column), and for the read term the (idx, value) pair comes from wires 1 and 2 which should contain a valid entry + * in the table. (Note: the meaning of "read" here is clear: the inputs are an (index, value) pair that we want to read + * from the table. Here "write" refers to data that is present in the "table", i.e. the bus column. There is no gate + * associated with a write, the data is simply populated in the corresponding column and committed to when constructing + * a proof). * * In practice, we must rephrase this expression in terms of polynomials, one of which is a polynomial I containing * (indirectly) the rational functions in the above expression: I_i = 1/[(read_term_i) * (write_term_i)]. This leads to diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp index caba698e6ab..b5fc9a7dcc4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp @@ -57,8 +57,8 @@ struct BusVector { * */ struct DataBus { - BusVector calldata; - BusVector return_data; + BusVector calldata; // the public input to the circuit + BusVector return_data; // the public output of the circuit }; } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index d013730b170..75d8f2e71c7 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -238,8 +238,9 @@ uint32_t GoblinUltraCircuitBuilder_::read_bus_vector(BusVector& bus_vector, // Get the raw index into the databus column const uint32_t read_idx = static_cast(uint256_t(this->get_variable(read_idx_witness_idx))); - ASSERT(read_idx < bus_vector.size()); // Ensure that the read index is valid - ASSERT(bus_vector.get_read_count(read_idx) < 1); // Reading more than once at the same index is not supported + ASSERT(read_idx < bus_vector.size()); // Ensure that the read index is valid + // NOTE(https://github.com/AztecProtocol/barretenberg/issues/937): Multiple reads at same index is not supported. + ASSERT(bus_vector.get_read_count(read_idx) < 1); // Create a variable corresponding to the result of the read. Note that we do not in general connect reads from // databus via copy constraints (i.e. we create a unique variable for the result of each read)