Skip to content

Commit

Permalink
feat: Simplify offsets and sizing using new block structure (#5404)
Browse files Browse the repository at this point in the history
Fixes two small areas of brittleness using the newly introduced block
and execution trace sorting work.

1) We no longer need the variable num_ecc_op_gates since this is
equivalent to blocks.ecc_op.size() which is a safer representation of
the number of goblin op gates.
2) We can compute pub_inputs_offset once and for all at the trace
generation stage and avoid recomputing it elsewhere.
  • Loading branch information
ledwards2225 authored Mar 23, 2024
1 parent 034fbf0 commit efa0842
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ TEST(GoblinUltraCircuitBuilder, GoblinSimple)
EXPECT_EQ(accumulator, g1::affine_point_at_infinity);

// Check number of ecc op "gates"/rows = 3 ops * 2 rows per op = 6
EXPECT_EQ(builder.num_ecc_op_gates, 6);
EXPECT_EQ(builder.blocks.ecc_op.size(), 6);

// Check that the expected op codes have been correctly recorded in the 1st op wire
EXPECT_EQ(builder.blocks.ecc_op.w_l()[0], EccOpCode::ADD_ACCUM);
Expand Down Expand Up @@ -150,7 +150,7 @@ TEST(GoblinUltraCircuitBuilder, GoblinEccOpQueueUltraOps)
// Check that the ultra ops recorded in the EccOpQueue match the ops recorded in the wires
auto ultra_ops = builder.op_queue->get_aggregate_transcript();
for (size_t i = 1; i < 4; ++i) {
for (size_t j = 0; j < builder.num_ecc_op_gates; ++j) {
for (size_t j = 0; j < builder.blocks.ecc_op.size(); ++j) {
auto op_wire_val = builder.variables[builder.blocks.ecc_op.wires[i][j]];
auto ultra_op_val = ultra_ops[i][j];
ASSERT_EQ(op_wire_val, ultra_op_val);
Expand Down
2 changes: 0 additions & 2 deletions barretenberg/cpp/src/barretenberg/flavor/goblin_ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,6 @@ class GoblinUltraFlavor {
std::vector<uint32_t> memory_write_records;
std::array<Polynomial, 4> sorted_polynomials;

size_t num_ecc_op_gates; // needed to determine public input offset

auto get_to_be_shifted()
{
return RefArray{ this->table_1, this->table_2, this->table_3, this->table_4, this->w_l, this->w_r,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ template <typename FF, size_t NUM_WIRES, size_t NUM_SELECTORS> class ExecutionTr

Wires wires; // vectors of indices into a witness variables array
Selectors selectors;
bool has_ram_rom = false; // does the block contain RAM/ROM gates
bool has_ram_rom = false; // does the block contain RAM/ROM gates
bool is_pub_inputs = false; // is this the public inputs block

bool operator==(const ExecutionTraceBlock& other) const = default;

Expand Down Expand Up @@ -156,7 +157,11 @@ template <typename FF_> class UltraArith {
UltraTraceBlock aux;
UltraTraceBlock lookup;

TraceBlocks() { aux.has_ram_rom = true; }
TraceBlocks()
{
aux.has_ram_rom = true;
pub_inputs.is_pub_inputs = true;
}

auto get() { return RefArray{ pub_inputs, arithmetic, delta_range, elliptic, aux, lookup }; }

Expand Down Expand Up @@ -264,7 +269,11 @@ template <typename FF_> class UltraHonkArith {
UltraHonkTraceBlock poseidon_external;
UltraHonkTraceBlock poseidon_internal;

TraceBlocks() { aux.has_ram_rom = true; }
TraceBlocks()
{
aux.has_ram_rom = true;
pub_inputs.is_pub_inputs = true;
}

auto get()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::decompose_ecc_operands(uint32_t op_
*
* @param in Variables array indices corresponding to operation inputs
* @note We dont explicitly set values for the selectors here since their values are fully determined by
* num_ecc_op_gates. E.g. in the composer we can reconstruct q_ecc_op as the indicator on the first num_ecc_op_gates
* indices. All other selectors are simply 0 on this domain.
* the number of ecc op gates. E.g. in the composer we can reconstruct q_ecc_op as the indicator over the range of ecc
* op gates. All other selectors are simply 0 on this domain.
*/
template <typename FF> void GoblinUltraCircuitBuilder_<FF>::populate_ecc_op_wires(const ecc_op_tuple& in)
{
Expand All @@ -204,8 +204,6 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::populate_ecc_op_wire
for (auto& selector : this->blocks.ecc_op.selectors) {
selector.emplace_back(0);
}

num_ecc_op_gates += 2;
};

template <typename FF> void GoblinUltraCircuitBuilder_<FF>::set_goblin_ecc_op_code_constant_variables()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
static constexpr size_t DEFAULT_NON_NATIVE_FIELD_LIMB_BITS =
UltraCircuitBuilder_<UltraHonkArith<FF>>::DEFAULT_NON_NATIVE_FIELD_LIMB_BITS;

size_t num_ecc_op_gates = 0; // number of ecc op "gates" (rows); these are placed at the start of the circuit

// Stores record of ecc operations and performs corresponding native operations internally
std::shared_ptr<ECCOpQueue> op_queue;

Expand Down Expand Up @@ -100,7 +98,8 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
size_t get_num_gates() const override
{
auto num_ultra_gates = UltraCircuitBuilder_<UltraHonkArith<FF>>::get_num_gates();
return num_ultra_gates + num_ecc_op_gates;
auto num_goblin_ecc_op_gates = this->blocks.ecc_op.size();
return num_ultra_gates + num_goblin_ecc_op_gates;
}

/**x
Expand All @@ -116,11 +115,12 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
size_t nnfcount = 0;
UltraCircuitBuilder_<UltraHonkArith<FF>>::get_num_gates_split_into_components(
count, rangecount, romcount, ramcount, nnfcount);
auto num_goblin_ecc_op_gates = this->blocks.ecc_op.size();

size_t total = count + romcount + ramcount + rangecount + num_ecc_op_gates;
size_t total = count + romcount + ramcount + rangecount + num_goblin_ecc_op_gates;
std::cout << "gates = " << total << " (arith " << count << ", rom " << romcount << ", ram " << ramcount
<< ", range " << rangecount << ", non native field gates " << nnfcount << ", goblin ecc op gates "
<< num_ecc_op_gates << "), pubinp = " << this->public_inputs.size() << std::endl;
<< num_goblin_ecc_op_gates << "), pubinp = " << this->public_inputs.size() << std::endl;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,20 +147,16 @@ PermutationMapping<Flavor::NUM_WIRES, generalized> compute_permutation_mapping(
cycle_index++;
}

// Add information about public inputs to the computation
// Add information about public inputs so that the cycles can be altered later; See the construction of the
// permutation polynomials for details.
const auto num_public_inputs = static_cast<uint32_t>(circuit_constructor.public_inputs.size());

// TODO(https://github.com/AztecProtocol/barretenberg/issues/862): this is brittle. depends on when PI are placed.
// how can we make this more robust?
// The public inputs are placed at the top of the execution trace, potentially offset by a zero row.
const size_t num_zero_rows = Flavor::has_zero_row ? 1 : 0;
size_t pub_input_offset = num_zero_rows;
// If Goblin, PI are further offset by number of ecc op gates
if constexpr (IsGoblinFlavor<Flavor>) {
pub_input_offset += circuit_constructor.num_ecc_op_gates;
size_t pub_inputs_offset = 0;
if constexpr (IsHonkFlavor<Flavor>) {
pub_inputs_offset = proving_key->pub_inputs_offset;
}
for (size_t i = 0; i < num_public_inputs; ++i) {
size_t idx = i + pub_input_offset;
size_t idx = i + pub_inputs_offset;
mapping.sigmas[0][idx].row_index = static_cast<uint32_t>(idx);
mapping.sigmas[0][idx].column_index = 0;
mapping.sigmas[0][idx].is_public_input = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ void ExecutionTrace_<Flavor>::add_wires_and_selectors_to_proving_key(
for (auto [pkey_selector, trace_selector] : zip_view(proving_key->get_selectors(), trace_data.selectors)) {
pkey_selector = trace_selector.share();
}
proving_key->pub_inputs_offset = trace_data.pub_inputs_offset;
} else if constexpr (IsPlonkFlavor<Flavor>) {
for (size_t idx = 0; idx < trace_data.wires.size(); ++idx) {
std::string wire_tag = "w_" + std::to_string(idx + 1) + "_lagrange";
Expand Down Expand Up @@ -106,6 +107,10 @@ typename ExecutionTrace_<Flavor>::TraceData ExecutionTrace_<Flavor>::construct_t
if (block.has_ram_rom) {
trace_data.ram_rom_offset = offset;
}
// Store offset of public inputs block for use in the pub input mechanism of the permutation argument
if (block.is_pub_inputs) {
trace_data.pub_inputs_offset = offset;
}

offset += block_size;
}
Expand Down Expand Up @@ -144,14 +149,13 @@ void ExecutionTrace_<Flavor>::add_ecc_op_wires_to_proving_key(
// Copy the ecc op data from the conventional wires into the op wires over the range of ecc op gates
const size_t op_wire_offset = Flavor::has_zero_row ? 1 : 0;
for (auto [ecc_op_wire, wire] : zip_view(op_wire_polynomials, proving_key->get_wires())) {
for (size_t i = 0; i < builder.num_ecc_op_gates; ++i) {
for (size_t i = 0; i < builder.blocks.ecc_op.size(); ++i) {
size_t idx = i + op_wire_offset;
ecc_op_wire[idx] = wire[idx];
ecc_op_selector[idx] = 1; // construct the selector as the indicator on the ecc op block
}
}

proving_key->num_ecc_op_gates = builder.num_ecc_op_gates;
proving_key->ecc_op_wire_1 = op_wire_polynomials[0].share();
proving_key->ecc_op_wire_2 = op_wire_polynomials[1].share();
proving_key->ecc_op_wire_3 = op_wire_polynomials[2].share();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ template <class Flavor> class ExecutionTrace_ {
std::array<Polynomial, Builder::Arithmetization::NUM_SELECTORS> selectors;
// A vector of sets (vectors) of addresses into the wire polynomials whose values are copy constrained
std::vector<CyclicPermutation> copy_cycles;
// The starting index in the trace of the block containing RAM/RAM read/write gates
uint32_t ram_rom_offset = 0;
uint32_t ram_rom_offset = 0; // offset of the RAM/ROM block in the execution trace
uint32_t pub_inputs_offset = 0; // offset of the public inputs block in the execution trace

TraceData(size_t dyadic_circuit_size, Builder& builder)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ template <class Flavor> size_t ProverInstance_<Flavor>::compute_dyadic_size(Circ
// minumum size of execution trace due to everything else
size_t min_size_of_execution_trace = circuit.public_inputs.size() + circuit.num_gates;
if constexpr (IsGoblinFlavor<Flavor>) {
min_size_of_execution_trace += circuit.num_ecc_op_gates;
min_size_of_execution_trace += circuit.blocks.ecc_op.size();
}

// The number of gates is the maxmimum required by the lookup argument or everything else, plus an optional zero row
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,6 @@ template <class Flavor> class ProverInstance_ {

std::span<FF> public_wires_source = proving_key->w_r;

// Determine public input offsets in the circuit relative to the 0th index for Ultra flavors
proving_key->pub_inputs_offset = Flavor::has_zero_row ? 1 : 0;
if constexpr (IsGoblinFlavor<Flavor>) {
proving_key->pub_inputs_offset += proving_key->num_ecc_op_gates;
}
// Construct the public inputs array
for (size_t i = 0; i < proving_key->num_public_inputs; ++i) {
size_t idx = i + proving_key->pub_inputs_offset;
Expand Down

0 comments on commit efa0842

Please sign in to comment.