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: Recursive verifier updates #3452

Merged
merged 10 commits into from
Dec 4, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ template <typename BuilderType> class GoblinUltraRecursive_ {
using Commitment = typename Curve::Element;
using CommitmentHandle = typename Curve::Element;
using FF = typename Curve::ScalarField;
using NativeVerificationKey = flavor::GoblinUltra::VerificationKey;

// Note(luke): Eventually this may not be needed at all
using VerifierCommitmentKey = pcs::VerifierCommitmentKey<Curve>;
Expand Down Expand Up @@ -324,7 +325,7 @@ template <typename BuilderType> class GoblinUltraRecursive_ {
* @param builder
* @param native_key Native verification key from which to extract the precomputed commitments
*/
VerificationKey(CircuitBuilder* builder, auto native_key)
VerificationKey(CircuitBuilder* builder, std::shared_ptr<NativeVerificationKey> native_key)
: VerificationKey_<PrecomputedEntities<Commitment>>(native_key->circuit_size, native_key->num_public_inputs)
{
this->q_m = Commitment::from_witness(builder, native_key->q_m);
Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/flavor/ultra_recursive.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ template <typename BuilderType> class UltraRecursive_ {
using Commitment = typename Curve::Element;
using CommitmentHandle = typename Curve::Element;
using FF = typename Curve::ScalarField;
using NativeVerificationKey = flavor::Ultra::VerificationKey;

// Note(luke): Eventually this may not be needed at all
using VerifierCommitmentKey = pcs::VerifierCommitmentKey<Curve>;
Expand Down Expand Up @@ -256,7 +257,7 @@ template <typename BuilderType> class UltraRecursive_ {
* @param builder
* @param native_key Native verification key from which to extract the precomputed commitments
*/
VerificationKey(CircuitBuilder* builder, auto native_key)
VerificationKey(CircuitBuilder* builder, std::shared_ptr<NativeVerificationKey> native_key)
: VerificationKey_<PrecomputedEntities<Commitment>>(native_key->circuit_size, native_key->num_public_inputs)
{
this->q_m = Commitment::from_witness(builder, native_key->q_m);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ template <typename FF_> class UltraHonk {
SelectorType& q_elliptic() { return selectors[8]; };
SelectorType& q_aux() { return selectors[9]; };
SelectorType& q_lookup_type() { return selectors[10]; };
SelectorType& q_busread() { return this->selectors[11]; };
SelectorType& q_busread() { return selectors[11]; };
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, can you explain what happened here? I believe the other q_busread changes are related to this,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually an insignificant change. In this context this->selectors and selectors are identical. In some cases the distinction is important but here the this-> was simply unnecessary so I removed it to avoid confusion.


const auto& get() const { return selectors; };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_gates_to_ensure_
this->w_l.emplace_back(public_calldata[read_idx]); // populate with value of calldata at read index
this->w_r.emplace_back(this->add_variable(FF(read_idx))); // populate with read index as witness
calldata_read_counts[read_idx]++; // increment read count at read index
q_busread.emplace_back(1); // read selector on
q_busread().emplace_back(1); // read selector on

// populate all other components with zero
this->w_o.emplace_back(this->zero_idx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
WireVector& ecc_op_wire_3 = std::get<2>(ecc_op_wires);
WireVector& ecc_op_wire_4 = std::get<3>(ecc_op_wires);

SelectorVector& q_busread = this->selectors.q_busread();
SelectorVector& q_busread() { return this->selectors.q_busread(); };

// DataBus call/return data arrays
std::vector<uint32_t> public_calldata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ void UltraCircuitBuilder_<Arithmetization>::add_gates_to_ensure_all_polys_are_no
q_elliptic.emplace_back(1);
q_aux.emplace_back(1);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;

// Some relations depend on wire shifts so we add another gate with
Expand Down Expand Up @@ -140,6 +141,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_add_gate(const add_triple_<FF
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -172,6 +174,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_big_add_gate(const add_quad_<
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -266,6 +269,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_big_mul_gate(const mul_quad_<
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand All @@ -292,6 +296,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_balanced_add_gate(const add_q
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
// Why 3? TODO: return to this
// The purpose of this gate is to do enable lazy 32-bit addition.
Expand Down Expand Up @@ -334,6 +339,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_mul_gate(const mul_triple_<FF
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}
/**
Expand Down Expand Up @@ -363,6 +369,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_bool_gate(const uint32_t vari
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -394,6 +401,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_poly_gate(const poly_triple_<
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -448,6 +456,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_ecc_add_gate(const ecc_add_ga
q_elliptic.emplace_back(1);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}
w_l.emplace_back(in.x2);
Expand All @@ -466,6 +475,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_ecc_add_gate(const ecc_add_ga
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -512,6 +522,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_ecc_dbl_gate(const ecc_dbl_ga
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand All @@ -531,6 +542,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_ecc_dbl_gate(const ecc_dbl_ga
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -561,6 +573,7 @@ void UltraCircuitBuilder_<Arithmetization>::fix_witness(const uint32_t witness_i
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}

Expand Down Expand Up @@ -636,6 +649,7 @@ plookup::ReadData<uint32_t> UltraCircuitBuilder_<Arithmetization>::create_gates_
q_elliptic.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
++this->num_gates;
}
return read_data;
Expand Down Expand Up @@ -945,6 +959,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint(const std::ve
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
}
// dummy gate needed because of sort widget's check of next row
w_l.emplace_back(variable_index[variable_index.size() - 1]);
Expand All @@ -964,6 +979,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint(const std::ve
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
}

// useful to put variables in the witness that aren't already used - e.g. the dummy variables of the range constraint in
Expand Down Expand Up @@ -998,6 +1014,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_dummy_constraints(const std::
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
}
}

Expand Down Expand Up @@ -1029,6 +1046,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint_with_edges(
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
// enforce range check for middle rows
for (size_t i = gate_width; i < variable_index.size() - gate_width; i += gate_width) {

Expand All @@ -1049,6 +1067,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint_with_edges(
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
}
// enforce range checks of last row and ending at end
if (variable_index.size() > gate_width) {
Expand All @@ -1069,6 +1088,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint_with_edges(
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
}

// dummy gate needed because of sort widget's check of next row
Expand All @@ -1090,6 +1110,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint_with_edges(
q_lookup_type.emplace_back(0);
q_aux.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
}

// range constraint a value by decomposing it into limbs whose size should be the default range constraint size
Expand Down Expand Up @@ -1206,6 +1227,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::LIMB_ACCUMULATE_2: {
Expand All @@ -1217,6 +1239,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::NON_NATIVE_FIELD_1: {
Expand All @@ -1228,6 +1251,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::NON_NATIVE_FIELD_2: {
Expand All @@ -1239,6 +1263,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::NON_NATIVE_FIELD_3: {
Expand All @@ -1250,6 +1275,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::ROM_CONSISTENCY_CHECK: {
Expand All @@ -1265,6 +1291,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::RAM_CONSISTENCY_CHECK: {
Expand All @@ -1281,6 +1308,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(1);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::RAM_TIMESTAMP_CHECK: {
Expand All @@ -1294,6 +1322,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::ROM_READ: {
Expand All @@ -1308,6 +1337,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0); // read/write flag stored in q_c
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::RAM_READ: {
Expand All @@ -1322,6 +1352,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0); // read/write flag stored in q_c
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
case AUX_SELECTORS::RAM_WRITE: {
Expand All @@ -1336,6 +1367,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(1); // read/write flag stored in q_c
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
default: {
Expand All @@ -1347,6 +1379,7 @@ void UltraCircuitBuilder_<Arithmetization>::apply_aux_selectors(const AUX_SELECT
q_c.emplace_back(0);
q_arith.emplace_back(0);
selectors.pad_additional();
check_selector_length_consistency();
break;
}
}
Expand Down Expand Up @@ -1869,6 +1902,7 @@ std::array<uint32_t, 5> UltraCircuitBuilder_<Arithmetization>::evaluate_non_nati
q_aux.emplace_back(0);
selectors.pad_additional();
}
check_selector_length_consistency();

this->num_gates += 4;
return std::array<uint32_t, 5>{
Expand Down Expand Up @@ -1991,6 +2025,7 @@ std::array<uint32_t, 5> UltraCircuitBuilder_<Arithmetization>::evaluate_non_nati
q_aux.emplace_back(0);
selectors.pad_additional();
}
check_selector_length_consistency();

this->num_gates += 4;
return std::array<uint32_t, 5>{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,25 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization:
};
~UltraCircuitBuilder_() override = default;

/**
* @brief Debug helper method for ensuring all selectors have the same size
* @details Each gate construction method manually appends values to the selectors. Failing to update one of the
* selectors will lead to an unsatisfiable circuit. This method provides a mechanism for ensuring that each selector
* has been updated as expected. Its logic is only active in debug mode.
*
*/
void check_selector_length_consistency()
{
#if NDEBUG
// do nothing
#else
size_t nominal_size = selectors.get()[0].size();
for (size_t idx = 0; idx < selectors.get().size(); ++idx) {
ledwards2225 marked this conversation as resolved.
Show resolved Hide resolved
ASSERT(selectors.get()[idx].size() == nominal_size);
}
#endif // NDEBUG
}

void finalize_circuit();

void add_gates_to_ensure_all_polys_are_non_zero();
Expand Down
Loading