Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: reproduce bug for duplicate lagrange_first commitment #6577

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "barretenberg/common/ref_vector.hpp"
#include "barretenberg/common/zip_view.hpp"
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp"
#include "barretenberg/transcript/transcript.hpp"

namespace bb {
Expand Down Expand Up @@ -489,7 +490,12 @@ template <typename PCS> class ZeroMorphVerifier_ {

// Compute batch mul to get the result
if constexpr (Curve::is_stdlib_type) {
return Commitment::batch_mul(commitments, scalars);
// If Ultra and using biggroup, handle edge cases in batch_mul
if constexpr (IsUltraBuilder<typename Curve::Builder> && stdlib::IsBigGroup<Commitment>) {
return Commitment::batch_mul(commitments, scalars, /*max_num_bits=*/0, /*with_edgecases=*/true);
} else {
return Commitment::batch_mul(commitments, scalars);
}
} else {
return batch_mul_native(commitments, scalars);
}
Expand Down Expand Up @@ -604,7 +610,12 @@ template <typename PCS> class ZeroMorphVerifier_ {
}

if constexpr (Curve::is_stdlib_type) {
return Commitment::batch_mul(commitments, scalars);
// If Ultra and using biggroup, handle edge cases in batch_mul
if constexpr (IsUltraBuilder<typename Curve::Builder> && stdlib::IsBigGroup<Commitment>) {
return Commitment::batch_mul(commitments, scalars, /*max_num_bits=*/0, /*with_edgecases=*/true);
} else {
return Commitment::batch_mul(commitments, scalars);
}
} else {
return batch_mul_native(commitments, scalars);
}
Expand Down Expand Up @@ -706,7 +717,12 @@ template <typename PCS> class ZeroMorphVerifier_ {
auto builder = z_challenge.get_context();
std::vector<FF> scalars = { FF(builder, 1), z_challenge };
std::vector<Commitment> points = { C_zeta_x, C_Z_x };
C_zeta_Z = Commitment::batch_mul(points, scalars);
// If Ultra and using biggroup, handle edge cases in batch_mul
if constexpr (IsUltraBuilder<typename Curve::Builder> && stdlib::IsBigGroup<Commitment>) {
C_zeta_Z = Commitment::batch_mul(points, scalars, /*max_num_bits=*/0, /*with_edgecases=*/true);
} else {
C_zeta_Z = Commitment::batch_mul(points, scalars);
}
} else {
C_zeta_Z = C_zeta_x + C_Z_x * z_challenge;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::ve
commitments.calldata = transcript->template receive_from_prover<Commitment>(commitment_labels.calldata);
commitments.calldata_read_counts =
transcript->template receive_from_prover<Commitment>(commitment_labels.calldata_read_counts);
commitments.calldata_read_tags =
transcript->template receive_from_prover<Commitment>(commitment_labels.calldata_read_tags);
commitments.return_data = transcript->template receive_from_prover<Commitment>(commitment_labels.return_data);
commitments.return_data_read_counts =
transcript->template receive_from_prover<Commitment>(commitment_labels.return_data_read_counts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace bb::stdlib {
template <class Builder, class Fq, class Fr, class NativeGroup> class element {
public:
using bool_ct = stdlib::bool_t<Builder>;
using biggroup_tag = element; // Facilitates a constexpr check IsBigGroup

struct secp256k1_wnaf {
std::vector<field_t<Builder>> wnaf;
Expand Down Expand Up @@ -937,6 +938,16 @@ template <class Builder, class Fq, class Fr, class NativeGroup> class element {
typename std::conditional<HasPlookup<Builder>, batch_lookup_table_plookup<>, batch_lookup_table_base>::type;
};

// template <typename T>
// concept IsBigGroup = requires {
// typename std::enable_if<
// std::is_same_v<T, element<typename T::Builder, typename T::Fq, typename T::Fr, typename
// T::NativeGroup>>>::type;
// };

template <typename T>
concept IsBigGroup = std::is_same_v<typename T::biggroup_tag, T>;

template <typename C, typename Fq, typename Fr, typename G>
inline std::ostream& operator<<(std::ostream& os, element<C, Fq, Fr, G> const& v)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ class MegaFlavor {
// 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 = 58;
static constexpr size_t NUM_ALL_ENTITIES = 59;
// 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 = 17;
static constexpr size_t NUM_WITNESS_ENTITIES = 18;
// Total number of folded polynomials, which is just all polynomials except the shifts
static constexpr size_t NUM_FOLDED_ENTITIES = NUM_PRECOMPUTED_ENTITIES + NUM_WITNESS_ENTITIES;

Expand Down Expand Up @@ -188,6 +188,7 @@ class MegaFlavor {
ecc_op_wire_4, // column 10
calldata, // column 11
calldata_read_counts, // column 12
calldata_read_tags, // column 12
calldata_inverses, // column 13
return_data, // column 14
return_data_read_counts, // column 15
Expand Down Expand Up @@ -664,6 +665,7 @@ class MegaFlavor {
ecc_op_wire_4 = "ECC_OP_WIRE_4";
calldata = "CALLDATA";
calldata_read_counts = "CALLDATA_READ_COUNTS";
calldata_read_tags = "CALLDATA_READ_TAGS";
calldata_inverses = "CALLDATA_INVERSES";
return_data = "RETURN_DATA";
return_data_read_counts = "RETURN_DATA_READ_COUNTS";
Expand Down Expand Up @@ -756,6 +758,7 @@ class MegaFlavor {
this->ecc_op_wire_4 = commitments.ecc_op_wire_4;
this->calldata = commitments.calldata;
this->calldata_read_counts = commitments.calldata_read_counts;
this->calldata_read_tags = commitments.calldata_read_tags;
this->calldata_inverses = commitments.calldata_inverses;
this->return_data = commitments.return_data;
this->return_data_read_counts = commitments.return_data_read_counts;
Expand Down Expand Up @@ -786,6 +789,7 @@ class MegaFlavor {
Commitment ecc_op_wire_4_comm;
Commitment calldata_comm;
Commitment calldata_read_counts_comm;
Commitment calldata_read_tags_comm;
Commitment calldata_inverses_comm;
Commitment return_data_comm;
Commitment return_data_read_counts_comm;
Expand Down Expand Up @@ -842,6 +846,7 @@ class MegaFlavor {
ecc_op_wire_4_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_read_counts_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_read_tags_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
calldata_inverses_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
return_data_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
return_data_read_counts_comm = deserialize_from_buffer<Commitment>(proof_data, num_frs_read);
Expand Down Expand Up @@ -883,6 +888,7 @@ class MegaFlavor {
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(calldata_read_tags_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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ void ProverInstance_<Flavor>::construct_databus_polynomials(Circuit& circuit)
{
auto& public_calldata = proving_key.polynomials.calldata;
auto& calldata_read_counts = proving_key.polynomials.calldata_read_counts;
auto& calldata_read_tags = proving_key.polynomials.calldata_read_tags;
auto& public_return_data = proving_key.polynomials.return_data;
auto& return_data_read_counts = proving_key.polynomials.return_data_read_counts;

Expand All @@ -59,6 +60,12 @@ void ProverInstance_<Flavor>::construct_databus_polynomials(Circuit& circuit)
return_data_read_counts[idx] = return_data.get_read_count(idx);
}

// DEBUG: The issue seems to be related to have two identical commitments. If we swet calldata_read_tags[0] = 1,
// then its commitment is identical to [L_1]. If we set calldata_read_tags[0] = 2, there doesn't seem to be any
// problem.
calldata_read_tags[0] = 1; // This causes failure in Mega recursion with Ultra arith
// calldata_read_tags[0] = 2; // This causes the tests to pass

auto& databus_id = proving_key.polynomials.databus_id;
// Compute a simple identity polynomial for use in the databus lookup argument
for (size_t i = 0; i < databus_id.size(); ++i) {
Expand Down
3 changes: 3 additions & 0 deletions barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,12 @@ template <IsUltraFlavor Flavor> void OinkProver<Flavor>::execute_wire_commitment
// Commit to DataBus columns and corresponding read counts
witness_commitments.calldata = commitment_key->commit(proving_key.polynomials.calldata);
witness_commitments.calldata_read_counts = commitment_key->commit(proving_key.polynomials.calldata_read_counts);
witness_commitments.calldata_read_tags = commitment_key->commit(proving_key.polynomials.calldata_read_tags);
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);
transcript->send_to_verifier(domain_separator + commitment_labels.calldata_read_tags,
witness_commitments.calldata_read_tags);
witness_commitments.return_data = commitment_key->commit(proving_key.polynomials.return_data);
witness_commitments.return_data_read_counts =
commitment_key->commit(proving_key.polynomials.return_data_read_counts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ template <IsUltraFlavor Flavor> void OinkVerifier<Flavor>::execute_wire_commitme
transcript->template receive_from_prover<Commitment>(domain_separator + comm_labels.calldata);
witness_comms.calldata_read_counts =
transcript->template receive_from_prover<Commitment>(domain_separator + comm_labels.calldata_read_counts);
witness_comms.calldata_read_tags =
transcript->template receive_from_prover<Commitment>(domain_separator + comm_labels.calldata_read_tags);
witness_comms.return_data =
transcript->template receive_from_prover<Commitment>(domain_separator + comm_labels.return_data);
witness_comms.return_data_read_counts = transcript->template receive_from_prover<Commitment>(
Expand Down
Loading