Skip to content

Commit

Permalink
chore: simplify handling of pub inputs block (#11747)
Browse files Browse the repository at this point in the history
Simplifies handling of public inputs by populating `pub_inputs` block in
`finalize_circuit()` which ensures that all blocks are complete prior to
any prover work. Also improves some of the log info related to the
overflow mechanism.

closes: AztecProtocol/barretenberg#1220
  • Loading branch information
ledwards2225 authored Feb 8, 2025
1 parent 48b6ed6 commit 4a8136c
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,7 @@ void fill_trace(State& state, TraceSettings settings)
fill_lookup_block(builder);

{
// finalize doesn't populate public inputs block, so copy to verify that the block is being filled well.
// otherwise the pk construction will overflow the block
// alternative: add to finalize or add a flag to check whether PIs have already been populated
auto builder_copy = builder;
builder_copy.finalize_circuit(/* ensure_nonzero */ false);
DeciderProvingKey::Trace::populate_public_inputs_block(builder_copy);

for (const auto [label, block] : zip_view(builder_copy.blocks.get_labels(), builder_copy.blocks.get())) {
for (const auto [label, block] : zip_view(builder.blocks.get_labels(), builder.blocks.get())) {
bool overfilled = block.size() >= block.get_fixed_size();
if (overfilled) {
vinfo(label, " overfilled");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change)
// The below part detects any changes in the join-split circuit
constexpr size_t DYADIC_CIRCUIT_SIZE = 1 << 16;

constexpr uint256_t CIRCUIT_HASH("0x48687216f00a81d2a0f64f0a10cce056fce2ad13c47f8329229eb3712d3f7566");
constexpr uint256_t CIRCUIT_HASH("0x103eeb052dd7c2f8ebf531bdfadb7d3cee0ab95371289f6d507beaa24b210fba");

const uint256_t circuit_hash = circuit.hash_circuit();
// circuit is finalized now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,16 @@ class MegaExecutionTraceBlocks : public MegaTraceBlockData<MegaTraceBlock> {
info("");
}

// Get cumulative size of all blocks
size_t get_total_content_size()
{
size_t total_size(0);
for (const auto& block : this->get()) {
total_size += block.size();
}
return total_size;
}

size_t get_structured_dyadic_size() const
{
size_t total_size = 1; // start at 1 because the 0th row is unused for selectors for Honk
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ class UltraExecutionTraceBlocks : public UltraTraceBlockData<UltraTraceBlock> {
info("overflow :\t", this->overflow.size());
}

// Get cumulative size of all blocks
size_t get_total_content_size()
{
size_t total_size(0);
for (const auto& block : this->get()) {
total_size += block.size();
}
return total_size;
}

size_t get_structured_dyadic_size()
{
size_t total_size = 1; // start at 1 because the 0th row is unused for selectors for Honk
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ void UltraCircuitBuilder_<ExecutionTrace>::finalize_circuit(const bool ensure_no
process_RAM_arrays();
process_range_lists();
#endif
populate_public_inputs_block();
circuit_finalized = true;
} else {
// Gates added after first call to finalize will not be processed since finalization is only performed once
Expand Down Expand Up @@ -1794,6 +1795,24 @@ std::array<uint32_t, 2> UltraCircuitBuilder_<ExecutionTrace>::evaluate_non_nativ
return std::array<uint32_t, 2>{ lo_1_idx, hi_3_idx };
}

/**
* @brief Copy the public input idx data into the public inputs trace block
* @note
*/
template <typename ExecutionTrace> void UltraCircuitBuilder_<ExecutionTrace>::populate_public_inputs_block()
{
PROFILE_THIS_NAME("populate_public_inputs_block");

// Update the public inputs block
for (const auto& idx : this->public_inputs) {
// first two wires get a copy of the public inputs
blocks.pub_inputs.populate_wires(idx, idx, this->zero_idx, this->zero_idx);
for (auto& selector : this->blocks.pub_inputs.selectors) {
selector.emplace_back(0);
}
}
}

/**
* @brief Called in `compute_proving_key` when finalizing circuit.
* Iterates over the cached_non_native_field_multiplication objects,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename ExecutionTrace_:

std::vector<fr> ipa_proof;

void populate_public_inputs_block();

void process_non_native_field_multiplications();
UltraCircuitBuilder_(const size_t size_hint = 0)
: CircuitBuilderBase<FF>(size_hint)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,6 @@
#include "barretenberg/stdlib_circuit_builders/ultra_zk_flavor.hpp"
namespace bb {

template <class Flavor> void TraceToPolynomials<Flavor>::populate_public_inputs_block(Builder& builder)
{
PROFILE_THIS_NAME("populate_public_inputs_block");

// Update the public inputs block
for (const auto& idx : builder.public_inputs) {
for (size_t wire_idx = 0; wire_idx < NUM_WIRES; ++wire_idx) {
if (wire_idx < 2) { // first two wires get a copy of the public inputs
builder.blocks.pub_inputs.wires[wire_idx].emplace_back(idx);
} else { // the remaining wires get zeros
builder.blocks.pub_inputs.wires[wire_idx].emplace_back(builder.zero_idx);
}
}
for (auto& selector : builder.blocks.pub_inputs.selectors) {
selector.emplace_back(0);
}
}
}

template <class Flavor>
void TraceToPolynomials<Flavor>::populate(Builder& builder,
typename Flavor::ProvingKey& proving_key,
Expand Down Expand Up @@ -89,11 +70,6 @@ typename TraceToPolynomials<Flavor>::TraceData TraceToPolynomials<Flavor>::const

PROFILE_THIS_NAME("construct_trace_data");

if constexpr (IsPlonkFlavor<Flavor>) {
// Complete the public inputs execution trace block from builder.public_inputs
populate_public_inputs_block(builder);
}

TraceData trace_data{ builder, proving_key };

uint32_t offset = Flavor::has_zero_row ? 1 : 0; // Offset at which to place each block in the trace polynomials
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,6 @@ template <class Flavor> class TraceToPolynomials {
*/
static void populate(Builder& builder, ProvingKey&, bool is_structured = false);

/**
* @brief Populate the public inputs block
* @details The first two wires are a copy of the public inputs and the other wires and all selectors are zero
*
* @param circuit
*/
static void populate_public_inputs_block(Builder& builder);

private:
/**
* @brief Add the memory records indicating which rows correspond to RAM/ROM reads/writes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ template <IsUltraFlavor Flavor> size_t DeciderProvingKey_<Flavor>::compute_dyadi
const size_t min_size_due_to_lookups = circuit.get_tables_size();

// minimum size of execution trace due to everything else
size_t min_size_of_execution_trace = circuit.public_inputs.size() + circuit.num_gates;
if constexpr (IsMegaFlavor<Flavor>) {
min_size_of_execution_trace += circuit.blocks.ecc_op.size();
}
size_t min_size_of_execution_trace = circuit.blocks.get_total_content_size();

// The number of gates is the maximum required by the lookup argument or everything else, plus an optional zero row
// to allow for shifts.
Expand Down Expand Up @@ -237,6 +234,7 @@ void DeciderProvingKey_<Flavor>::construct_databus_polynomials(Circuit& circuit)
*/
template <IsUltraFlavor Flavor>
void DeciderProvingKey_<Flavor>::move_structured_trace_overflow_to_overflow_block(Circuit& circuit)
requires IsMegaFlavor<Flavor>
{
auto& blocks = circuit.blocks;
auto& overflow_block = circuit.blocks.overflow;
Expand All @@ -252,9 +250,19 @@ void DeciderProvingKey_<Flavor>::move_structured_trace_overflow_to_overflow_bloc
uint32_t fixed_block_size = block.get_fixed_size();
if (block_size > fixed_block_size && block != overflow_block) {
// Disallow overflow in blocks that are not expected to be used by App circuits
ASSERT(!block.is_pub_inputs);
if constexpr (IsMegaFlavor<Flavor>) {
ASSERT(block != blocks.ecc_op);
if (block.is_pub_inputs) {
info("WARNING: Number of public inputs (",
block_size,
") cannot exceed capacity specified in structured trace: ",
fixed_block_size);
ASSERT(false);
}
if (block == blocks.ecc_op) {
info("WARNING: Number of ecc op gates (",
block_size,
") cannot exceed capacity specified in structured trace: ",
fixed_block_size);
ASSERT(false);
}

// Set has_overflow to true if at least one block exceeds its capacity
Expand All @@ -264,7 +272,6 @@ void DeciderProvingKey_<Flavor>::move_structured_trace_overflow_to_overflow_bloc
// the block containing RAM/ROM gates overflows, the indices of the corresponding gates in the memory
// records need to be updated to reflect their new position in the overflow block
if (block.has_ram_rom) {

uint32_t overflow_cur_idx = overflow_block.trace_offset + static_cast<uint32_t>(overflow_block.size());
overflow_cur_idx -= block.trace_offset; // we'll add block.trace_offset to everything later
uint32_t offset = overflow_cur_idx + 1; // +1 accounts for duplication of final gate
Expand Down Expand Up @@ -315,12 +322,7 @@ void DeciderProvingKey_<Flavor>::move_structured_trace_overflow_to_overflow_bloc

// Set the fixed size of the overflow block to its current size
if (overflow_block.size() > overflow_block.get_fixed_size()) {
info("WARNING: Structured trace overflowed beyond the prescribed fixed overflow size. This is valid in the "
"context of one-off VK/proof generation but not in the IVC setting. \nPrescribed overflow size: ",
overflow_block.get_fixed_size(),
". \nActual overflow size: ",
overflow_block.size(),
"\n");
info("WARNING: Structured trace overflow mechanism in use. Performance may be degraded!");
overflow_block.fixed_size = static_cast<uint32_t>(overflow_block.size());
blocks.summarize();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,7 @@ template <IsUltraFlavor Flavor> class DeciderProvingKey_ {
". Log dyadic circuit size: ",
numeric::get_msb(dyadic_circuit_size),
".");

// Complete the public inputs execution trace block from circuit.public_inputs
Trace::populate_public_inputs_block(circuit);
circuit.blocks.compute_offsets(is_structured);
circuit.blocks.compute_offsets(is_structured); // compute offset of each block within the trace

// Find index of last non-trivial wire value in the trace
for (auto& block : circuit.blocks.get()) {
Expand Down Expand Up @@ -227,7 +224,8 @@ template <IsUltraFlavor Flavor> class DeciderProvingKey_ {
void construct_databus_polynomials(Circuit&)
requires HasDataBus<Flavor>;

static void move_structured_trace_overflow_to_overflow_block(Circuit& circuit);
static void move_structured_trace_overflow_to_overflow_block(Circuit& circuit)
requires IsMegaFlavor<Flavor>;
};

} // namespace bb

1 comment on commit 4a8136c

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 4a8136c Previous: 8c1d477 Ratio
wasmClientIVCBench/Full/6 81062.11368 ms/iter 72476.85666300001 ms/iter 1.12
commit(t) 2758758681 ns/iter 2617905823 ns/iter 1.05

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.