From a21095fee3bde582db1051f2f3aeeefc79932644 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 14:41:10 -0300 Subject: [PATCH 1/8] Reject a malformed out-of-domain block instead of panicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `verify_batched`'s fold seed indexes both out-of-domain blocks at the `width`/`height` the proof advertises, but `ood_blocks_well_formed` — the guard that pins those dimensions to the AIR and calls `dimensions_consistent()` — did not run until 92 lines later. A proof whose advertised dimensions disagree with its data length therefore panicked in `Table::get_row`'s unchecked slice rather than being rejected. That is the exact gap the guard's own doc comment says it exists to close, and the ordinary verifier keeps the ordering by running it inside the round-1 loop. Hoist the three shape checks into the pre-pass that already validates each table's domain, before any of them is read. The division by `trace_length` is safe there: the same loop rejects zero first. No transcript byte moves — the checks touch no transcript. `prover::batched_verifier::replay` had the same pre-guard read through `Table::columns`; it is test-only, but a patch that fixed only the STARK half would leave a reader thinking the family was covered. Direction is robustness, not soundness: nothing wrong is accepted, the verifier aborts instead of returning false. It is unreachable from bytes today because `BatchedProof` has no derives — which is also why it is the cheapest moment to pay for it, since serializing that format is the point. Also drop `Replay::iotas`, documented as the per-group query indices and always empty, and say plainly that `replay` is a test oracle: it takes the prover's word on the precomputed root and never checks `fold_order` is a permutation, both of which `verify` does. `a_tampered_batched_proof_is_rejected` grows five arms: a group's FRI layer root (the one commitment batching relocated from per-table to per-group), a main root, a composition root, an out-of-domain value, and a block whose advertised width lies. The last one panics at `table.rs:362` without this change and is rejected with it. --- crypto/stark/src/batched_verifier.rs | 49 +++++++++++++++++---------- prover/src/batched_verifier.rs | 43 +++++++++++++++-------- prover/src/tests/batched_fri_tests.rs | 47 +++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 32 deletions(-) diff --git a/crypto/stark/src/batched_verifier.rs b/crypto/stark/src/batched_verifier.rs index 8d47f103d..4b19b3f88 100644 --- a/crypto/stark/src/batched_verifier.rs +++ b/crypto/stark/src/batched_verifier.rs @@ -36,6 +36,10 @@ struct GroupReplay { iotas: Vec, acc: Vec>, acc_sym: Vec>, + /// The first table of the group, whose AIR supplied the domain. Kept from + /// the scan that already found it rather than looked up again: the second + /// scan is what forced an `expect` into verifier code. + member: usize, } /// Verify the rounds after round 1 of every table and each group's FRI. @@ -90,12 +94,33 @@ where let num_queries = first_air.options().fri_number_of_queries; let grinding_factor = first_air.context().proof_options.grinding_factor; - // Every table's domain is its group's. - for (idx, (table, &g)) in tables.iter().zip(group_of).enumerate() { + // Every table's domain is its group's, and every block and opening has the + // shape its AIR declares. + // + // Both run before the fold seed below reads a single out-of-domain value. + // That seed indexes the blocks at the dimensions the *proof* advertises, so + // a proof whose advertised dimensions disagree with its data length has to + // be rejected here rather than panic there — the rule `ood_blocks_well_formed` + // documents, and the one the ordinary verifier keeps by running it in round 1. + for (idx, ((air, table), &g)) in airs.iter().zip(tables).zip(group_of).enumerate() { if table.trace_length == 0 || table.trace_length != groups[g].trace_rows { error!("batched: table {idx} does not live on its group's domain"); return false; } + let view = StarkProofView::Owned(table); + // `trace_length` is non-zero by the check above, so the division is safe. + if table.composition_poly_parts_ood_evaluation.len() + != air.composition_poly_degree_bound(table.trace_length) / table.trace_length + || !V::::ood_blocks_well_formed(*air, view) + || !V::::trace_opening_widths_well_formed( + *air, + view, + num_queries, + ) + { + error!("batched: table {idx}'s blocks or openings are malformed"); + return false; + } } // The fold coefficients, from the seed, in the prover's order. @@ -182,6 +207,7 @@ where iotas, acc: vec![FieldElement::zero(); num_queries], acc_sym: vec![FieldElement::zero(); num_queries], + member, }); } @@ -199,19 +225,9 @@ where if let Some(ref bpi) = table.bus_public_inputs { fork.append_field_element(&bpi.table_contribution); } + // Shapes were pinned to the AIR in the pre-pass above, before the fold + // seed read any of this table's blocks. let domain = new_verifier_domain(*air, table.trace_length); - if table.composition_poly_parts_ood_evaluation.len() - != air.composition_poly_degree_bound(table.trace_length) / table.trace_length - || !V::::ood_blocks_well_formed(*air, view) - || !V::::trace_opening_widths_well_formed( - *air, - view, - num_queries, - ) - { - error!("batched: table {idx}'s blocks or openings are malformed"); - return false; - } let layout = V::::ood_layout(*air); let RoundsChallenges { z, @@ -291,10 +307,7 @@ where // Each group's FRI, from the folded first layer down to the final polynomial. for (g, (group, replay)) in groups.iter().zip(replays.iter()).enumerate() { - let member = group_of - .iter() - .position(|&h| h == g) - .expect("checked above"); + let member = replay.member; let fri = group.fri; let synthetic = StarkProof:: { trace_length: group.trace_rows, diff --git a/prover/src/batched_verifier.rs b/prover/src/batched_verifier.rs index 7c9155e4e..4e4fb429a 100644 --- a/prover/src/batched_verifier.rs +++ b/prover/src/batched_verifier.rs @@ -30,14 +30,21 @@ pub struct Replay { pub logup: Vec>, /// Per table, in AIR order: its fold coefficient. pub coefficients: Vec>, - /// Per group, in the order the prover ran them: the query indices. - pub iotas: Vec>, } /// Replay the transcript the prover walked, from the proof. /// /// `elf_bytes` and the statement come from outside the proof on purpose: a /// proof that could choose its own statement would prove nothing. +/// +/// # This is a test oracle, not a verifier +/// +/// It derives the challenges and stops. [`verify`] does not call it — it walks +/// the same prefix itself and is the stricter of the two: it checks each +/// preprocessed root against the AIR's own constant where this takes the +/// prover's word, and it rejects a `fold_order` that is not a permutation. +/// Promoting this function to a verification path without closing both gaps +/// would be a soundness hole; it exists so a test can compare challenges. pub fn replay( proof: &BatchedProof, elf_bytes: &[u8], @@ -73,6 +80,25 @@ pub fn replay( // The fold coefficients, in the order the prover folded — which the proof // carries because a table's coefficient depends on every table before it. + if proof.fold_order.len() != proof.tables.len() { + return Err(Error::Prover(format!( + "batched verify: {} tables folded of {}", + proof.fold_order.len(), + proof.tables.len() + ))); + } + // Shapes before the seed reads a value, for the same reason + // `verify_batched` pins them before its own fold loop: `Table::columns` + // indexes at the advertised dimensions. + for (idx, t) in proof.tables.iter().enumerate() { + for block in [&t.trace_ood, &t.trace_ood_next] { + if !block.dimensions_consistent() { + return Err(Error::Prover(format!( + "batched verify: table {idx}'s out-of-domain block is malformed" + ))); + } + } + } let mut seed = transcript.clone(); let mut coefficients = vec![FieldElement::::zero(); proof.tables.len()]; for &idx in proof.fold_order.iter() { @@ -97,19 +123,8 @@ pub fn replay( } coefficients[idx] = seed.sample_field_element(); } - if proof.fold_order.len() != proof.tables.len() { - return Err(Error::Prover(format!( - "batched verify: {} tables folded of {}", - proof.fold_order.len(), - proof.tables.len() - ))); - } - Ok(Replay { - logup, - coefficients, - iotas: Vec::new(), - }) + Ok(Replay { logup, coefficients }) } /// Verify a batched proof of `elf_bytes`. diff --git a/prover/src/tests/batched_fri_tests.rs b/prover/src/tests/batched_fri_tests.rs index c7501c341..ce8253efb 100644 --- a/prover/src/tests/batched_fri_tests.rs +++ b/prover/src/tests/batched_fri_tests.rs @@ -641,4 +641,51 @@ fn a_tampered_batched_proof_is_rejected() { rejected(&proof, "a layout with one more CPU chunk"); proof.table_counts.cpu -= 1; accepted(&proof); + + // A group's FRI layer commitment. This is the one thing batching actually + // relocated — per table before, per group now — so it is the commitment most + // worth pinning. It is bound only indirectly: the seed absorbs each layer + // root, so changing one moves the group's query indices. + // + // Groups at or below the terminal size commit no layers at all, so pick one + // that has some. Asserting that such a group exists keeps this from becoming + // a tamper that silently does nothing if the fixture's shape changes. + let with_layers = proof + .groups + .iter() + .position(|(_, fri)| !fri.layer_roots.is_empty()) + .expect("the fixture must produce at least one group that commits FRI layers"); + proof.groups[with_layers].1.layer_roots[0][0] ^= 1; + rejected(&proof, "a group's FRI layer root"); + proof.groups[with_layers].1.layer_roots[0][0] ^= 1; + accepted(&proof); + + // A table's round-1 main commitment. + proof.tables[0].main_root[0] ^= 1; + rejected(&proof, "a main trace root"); + proof.tables[0].main_root[0] ^= 1; + accepted(&proof); + + // A table's composition commitment, which the fold seed absorbs directly. + proof.tables[0].composition_poly_root[0] ^= 1; + rejected(&proof, "a composition root"); + proof.tables[0].composition_poly_root[0] ^= 1; + accepted(&proof); + + // An out-of-domain value, the other thing the fold seed binds. + let orig = proof.tables[0].trace_ood.get(0, 0).clone(); + proof.tables[0].trace_ood.set(0, 0, &orig + &one); + rejected(&proof, "an out-of-domain trace value"); + proof.tables[0].trace_ood.set(0, 0, orig); + accepted(&proof); + + // A block whose advertised dimensions disagree with its data length. The + // verifier must REJECT this, not panic: the fold seed indexes both + // out-of-domain blocks at the dimensions the proof declares, so the shape + // has to be pinned to the AIR before that read rather than after it. + let orig_width = proof.tables[0].trace_ood.width; + proof.tables[0].trace_ood.width += 1; + rejected(&proof, "an out-of-domain block with a lying width"); + proof.tables[0].trace_ood.width = orig_width; + accepted(&proof); } From 996337a47ad23379846f017077b8c644ba7a9fb6 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 14:44:53 -0300 Subject: [PATCH 2/8] Reattach eight doc comments to the items they describe Each of these inserted a new item between an existing doc comment and the item it documented, with no blank line, so rustdoc merged the two blocks and the original item lost its docs: prover.rs table_parallelism -> MainRoots prover.rs plain -> known_roots verifier.rs replay_rounds_* -> replay_rounds_2_and_3 decode.rs update_multiplicities -> add_multiplicities trace_builder.rs cpu32_chip_op -> WalkLeftover trace_builder.rs build_initial_image-> runtime_page_ranges trace_builder.rs touched_memory_cells -> op_count trace_builder.rs collect_epoch -> walk_and_emit_chunks Three of the adopted sentences were actively wrong about their new owner: `known_roots` was labelled "for a plain (non-preprocessed) table" when it takes `precomputed: Option` and serves both; `add_multiplicities` was described in terms of a `lookups` parameter it does not have; and `WalkLeftover`'s public rustdoc opened by describing an ALU dispatch helper. `replay_rounds_2_and_3` is renamed `replay_rounds_2_to_4`: its body still carries an explicit `Round 4` section sampling gamma and the DEEP coefficients, so the orphaned sentence ("rounds 2, 3 and 4") was the accurate one and the new name was not. `bitwise_histogram` carried two stacked doc blocks, the first saying LT, MUL, DVRM, SHIFT, the accelerators and PAGE "are not here yet" and the second, directly below, saying the histogram is complete. The first is left over from an earlier state; `finalize` folds all of them in. --- crypto/stark/src/batched_verifier.rs | 2 +- crypto/stark/src/prover.rs | 48 ++++++++++++++++------------ crypto/stark/src/verifier.rs | 11 ++++--- prover/src/tables/decode.rs | 6 ++-- prover/src/tables/trace_builder.rs | 33 +++++++++---------- 5 files changed, 52 insertions(+), 48 deletions(-) diff --git a/crypto/stark/src/batched_verifier.rs b/crypto/stark/src/batched_verifier.rs index 4b19b3f88..363c8fceb 100644 --- a/crypto/stark/src/batched_verifier.rs +++ b/crypto/stark/src/batched_verifier.rs @@ -235,7 +235,7 @@ where transition_coeffs, trace_term_coeffs, gammas, - } = V::::replay_rounds_2_and_3( + } = V::::replay_rounds_2_to_4( *air, view, &public_inputs[idx], diff --git a/crypto/stark/src/prover.rs b/crypto/stark/src/prover.rs index 13ec7a51a..443bb99ce 100644 --- a/crypto/stark/src/prover.rs +++ b/crypto/stark/src/prover.rs @@ -131,10 +131,12 @@ impl TableCommit where FieldElement: AsBytes, { - /// Build a `TableCommit` for a plain (non-preprocessed) table. /// Roots without a tree, for a pass that will not open against it. The /// tree is the expensive half; a caller that already knows the roots and /// only needs them to travel should not pay for one. + /// + /// Serves preprocessed and plain tables alike — `precomputed` is `Some` + /// exactly when the AIR is preprocessed. fn known_roots(root: Commitment, precomputed: Option) -> Self { Self { tree: Arc::new(BatchedMerkleTree::from_root(root)), @@ -146,6 +148,7 @@ where } } + /// Build a `TableCommit` for a plain (non-preprocessed) table. fn plain(#[allow(unused_mut)] mut tree: BatchedMerkleTree, root: Commitment) -> Self { let leaves_dropped = Self::retire_leaves(&mut tree); Self { @@ -646,26 +649,6 @@ fn host_cores() -> usize { .unwrap_or(4) } -/// Number of tables `multi_prove` proves concurrently, out of `num_airs` of -/// them. -/// -/// Defaults: **every table** under `cuda`, `num_cores / 3` on CPU builds -/// (benchmarked optimal on both M3 Pro and EPYC 9454P — every table there is -/// pure host work, so `k` genuinely competes for cores). Both arms are -/// overridden by the `TABLE_PARALLELISM` env var, and the result is clamped to -/// `1..=num_airs`. Without the `parallel` feature this is 1 and the env var is -/// ignored. -/// -/// # Why the `cuda` arm has no core term -/// -/// Measured over 881 runs on two RTX 5090 boxes (sweep record linked from -/// PR #911): the work `k` divides is device- and workload-bound — invariant to -/// host core count over an 8× range — so `available_parallelism()` is the -/// wrong quantity to scale `k` by. `k` is not a thread count; it counts -/// concurrent drivers whose per-table work all runs on the one global rayon -/// pool. Worst case against the best measured `k`: `num_airs` +1.6 % (inside -/// noise), the old `cores*2/3` +13.0 %. Bounding concurrency is memory -/// admission's job (`VramGate`), not this count's. /// A table's Round 1 roots, in the order Fiat-Shamir absorbs them. /// /// A plain table contributes one root. A preprocessed one contributes two: its @@ -826,6 +809,29 @@ const RETIRE_OVERRIDE_OFF: u8 = 1; const RETIRE_OVERRIDE_ON: u8 = 2; static RETIRE_LDE_OVERRIDE: AtomicU8 = AtomicU8::new(RETIRE_OVERRIDE_UNSET); +/// Number of tables `multi_prove` proves concurrently, out of `num_airs` of +/// them. +/// +/// Defaults: **every table** under `cuda`, `num_cores / 3` on CPU builds +/// (benchmarked optimal on both M3 Pro and EPYC 9454P — every table there is +/// pure host work, so `k` genuinely competes for cores). Both arms are +/// overridden by the `TABLE_PARALLELISM` env var, and the result is clamped to +/// `1..=num_airs`. Without the `parallel` feature this is 1 and the env var is +/// ignored. +/// +/// Not to be confused with `prover::pass::table_parallelism`, which sizes the +/// prove-and-retire walk's batches and reads `A1_TABLE_PARALLELISM`. +/// +/// # Why the `cuda` arm has no core term +/// +/// Measured over 881 runs on two RTX 5090 boxes (sweep record linked from +/// PR #911): the work `k` divides is device- and workload-bound — invariant to +/// host core count over an 8× range — so `available_parallelism()` is the +/// wrong quantity to scale `k` by. `k` is not a thread count; it counts +/// concurrent drivers whose per-table work all runs on the one global rayon +/// pool. Worst case against the best measured `k`: `num_airs` +1.6 % (inside +/// noise), the old `cores*2/3` +13.0 %. Bounding concurrency is memory +/// admission's job (`VramGate`), not this count's. pub fn table_parallelism(num_airs: usize) -> usize { #[cfg(feature = "parallel")] { diff --git a/crypto/stark/src/verifier.rs b/crypto/stark/src/verifier.rs index 7600060c5..94694119c 100644 --- a/crypto/stark/src/verifier.rs +++ b/crypto/stark/src/verifier.rs @@ -1455,12 +1455,13 @@ pub trait IsStarkVerifier< } /// Replays rounds 2, 3 and 4 of the protocol for a given proof, assuming round 1 has - /// already been replayed and the RAP challenges are known. - /// Rounds 2 and 3 of a table's transcript: the constraint coefficients, the - /// out-of-domain point and the DEEP coefficients. The per-table proof goes + /// already been replayed and the RAP challenges are known: the constraint + /// coefficients, the out-of-domain point, and round 4's DEEP coefficients. + /// + /// Stops where the two proof formats part company. The per-table proof goes /// on to its own FRI from here; the batched proof samples its fold /// coefficient instead. - fn replay_rounds_2_and_3( + fn replay_rounds_2_to_4( air: &dyn AIR, proof: StarkProofView<'_, Field, FieldExtension, PI>, public_inputs: &PI, @@ -1591,7 +1592,7 @@ pub trait IsStarkVerifier< transition_coeffs, trace_term_coeffs, gammas, - } = Self::replay_rounds_2_and_3( + } = Self::replay_rounds_2_to_4( air, proof, public_inputs, diff --git a/prover/src/tables/decode.rs b/prover/src/tables/decode.rs index c92537f07..df197de27 100644 --- a/prover/src/tables/decode.rs +++ b/prover/src/tables/decode.rs @@ -172,9 +172,6 @@ pub fn generate_decode_trace( (trace, pc_to_row) } -/// Updates multiplicities in the DECODE trace table. -/// -/// For each PC in `lookups`, increments the MU column in the corresponding row. /// Add `count` lookups of `pc` at once. /// /// The per-lookup form needs one entry per executed cycle, which a prover that @@ -196,6 +193,9 @@ pub fn add_multiplicities( } } +/// Updates multiplicities in the DECODE trace table. +/// +/// For each PC in `lookups`, increments the MU column in the corresponding row. pub fn update_multiplicities( trace: &mut TraceTable, pc_to_row: &PcToRow, diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index 96edfe646..1c7d777dc 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -1249,8 +1249,6 @@ fn collect_cpu32_bitwise(c: &cpu32::Cpu32Operation) -> Vec { ops } -/// The ALU-chip op a word ALU instruction dispatches (SHIFT/MUL/DVRM). ADDW/SUBW -/// are the CPU32 ADD/SUB fast-path (no external chip), returning `None`. /// What a Commit-phase walk still holds when the execution ends. /// /// The chunks it closed are gone — committed and dropped as they filled. This @@ -1439,9 +1437,6 @@ impl WalkLeftover { /// whose rows are the lookup space — only its multiplicity columns depend /// on the run — so it is built once, at the end, and never chunked. /// - /// Only the three sources a walk can retire are folded so far. The rest — - /// LT, MUL, DVRM, SHIFT, the accelerators, PAGE — feed BITWISE too and are - /// not here yet. /// Every BITWISE lookup the run owes, from the retired chunks and from the /// tail — `finalize` folded both in, so this is complete whatever has been /// drained since. PAGE's own lookups are added by `build_pages`. @@ -1636,6 +1631,8 @@ pub struct AccumulatedTables { pub const CPU32_APPENDS_TO: [TableKind; 3] = [TableKind::Shift, TableKind::Mul, TableKind::Dvrm]; #[allow(clippy::type_complexity)] +/// The ALU-chip op a word ALU instruction dispatches (SHIFT/MUL/DVRM). ADDW/SUBW +/// are the CPU32 ADD/SUB fast-path (no external chip), returning `None`. fn cpu32_chip_op( c: &cpu32::Cpu32Operation, shift_ops: &mut Vec, @@ -2499,9 +2496,6 @@ fn private_input_bytes(private_input: &[u8]) -> Vec { .collect() } -/// Build the initial-memory image (byte address -> value) from the ELF segments -/// and the private-input region. Single source of "what memory starts as", read -/// by both `MemoryState` seeding and PAGE/bitwise init. /// Run-length encode the runtime (non-ELF) page bases into `(base, count)`. /// /// Zero-init pages are the runtime ones, so `init_values == None` identifies @@ -2541,6 +2535,9 @@ pub(crate) fn runtime_page_ranges( ranges } +/// Build the initial-memory image (byte address -> value) from the ELF segments +/// and the private-input region. Single source of "what memory starts as", read +/// by both `MemoryState` seeding and PAGE/bitwise init. pub(crate) fn build_initial_image(elf: &Elf, private_input: &[u8]) -> HashMap { let mut image: HashMap = HashMap::new(); for segment in &elf.data { @@ -3221,16 +3218,16 @@ pub struct CollectedEpoch { } impl CollectedEpoch { - /// The epoch's touched memory cells (sorted by address): the exact values - /// `build_traces` later stores in `Traces::touched_memory_cells` (both are - /// [`touched_cells_from_memory_state`] over the same immutable - /// `memory_state`), available before any table is built. /// Ops collected for `kind`. Mirrors [`CollectedOps::buffered`] so a walk's /// output and a finished run's can be compared on the same footing. pub fn op_count(&self, kind: TableKind) -> usize { self.ops.buffered(kind) } + /// The epoch's touched memory cells (sorted by address): the exact values + /// `build_traces` later stores in `Traces::touched_memory_cells` (both are + /// [`touched_cells_from_memory_state`] over the same immutable + /// `memory_state`), available before any table is built. pub fn touched_memory_cells(&self) -> local_to_global::EpochTouches { touched_cells_from_memory_state(&self.memory_state) } @@ -5490,12 +5487,6 @@ impl Traces { ) } - /// The sequential-critical half of an epoch's trace build: log collection - /// and op routing (Phases 1-2), which read the pre-epoch memory image and - /// produce the epoch's memory/register end state. This must run in epoch - /// order (the image advances between epochs); the table generation that - /// consumes the result ([`Self::build_from_collected`]) is epoch-local and - /// can run on another thread. /// Walk the execution and hand each chunked table's chunk to `on_chunk` as /// soon as it is full, dropping its ops right after. /// @@ -5739,6 +5730,12 @@ impl Traces { }) } + /// The sequential-critical half of an epoch's trace build: log collection + /// and op routing (Phases 1-2), which read the pre-epoch memory image and + /// produce the epoch's memory/register end state. This must run in epoch + /// order (the image advances between epochs); the table generation that + /// consumes the result ([`Self::build_from_collected`]) is epoch-local and + /// can run on another thread. pub fn collect_epoch( artifacts: &DecodeArtifacts, initial_image: &I, From ff458c38c1992ab4ba46c98e1b03e9dae594ca34 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 14:47:11 -0300 Subject: [PATCH 3/8] Say what the walk actually holds, and drop a stale table count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two claims about residency were wrong in the same way. `pass.rs`'s header said what the walk holds is one table plus the residents "no matter how long the run is", and the design doc listed LT among the tables handed over "as soon as a chunk fills". LT is not: it, MUL, DVRM and SHIFT are deliberately absent from `CHUNKED_KINDS` because later derivations keep appending to them, so their chunk boundaries are not knowable until the run ends — `walk_and_emit_chunks`'s own doc comment says so two paragraphs below the sentence that contradicted it. Their op lists, the `retired_*` rows a closing chunk converts its ops into, and the walk's BITWISE lookups are all held whole, so that term is O(cycles). It is a small term — compact routed intermediates against trace rows, a low single-digit percentage of the measured peak — and closing it would move LT's chunk boundaries and cost the byte-identical-roots property that makes the per-table variant a drop-in. So this changes no code: it makes the documents say what the code does, and lists the gap in §10 with the observation that BITWISE's half is the cheap one, since a histogram is commutative and could be folded per segment without moving any root. Also in the design doc: - the `A1_TABLE_PARALLELISM` row quoted a sweep ("flat between 12 and 24; 24 costs 6 GB") that does not match the one recorded on `pass::table_parallelism` (no k=12 or k=24 rows; the step is 16 -> 32 for 5.0 GB); - `A1_INFLIGHT` and `LAMBDA_STREAM_LDE` were missing from a table that claims to list every knob, and the second changes this approach's own memory profile; - `--features hash-metrics` is from #987, which is not on this branch, so the verify-hash column cannot be reproduced here — say so rather than give a build command that fails; - "the spec's Open optimization ... was not kept" described code that ships: what was dropped is holding whole Merkle trees between passes, while leaf-dropping is `drop_leaves`/`retire_leaves` behind `LAMBDA_STREAM_LDE`; - §6's header omitted the blowup and the knob settings the numbers were taken at. And five comments said the ethrex block has 227 tables where the doc says 245. Rather than guess which run is stale, they now say "once per table" and the like: none of them needed the number. --- bin/cli/src/main.rs | 2 +- crypto/stark/src/prover.rs | 4 +-- docs/prove_and_retire_design.md | 55 ++++++++++++++++++++++++------ prover/src/challenge_phase.rs | 2 +- prover/src/logup_phase.rs | 2 +- prover/src/pass.rs | 22 ++++++++++-- prover/src/tables/trace_builder.rs | 8 +++-- 7 files changed, 74 insertions(+), 21 deletions(-) diff --git a/bin/cli/src/main.rs b/bin/cli/src/main.rs index 69d51cd93..85a31b0e2 100644 --- a/bin/cli/src/main.rs +++ b/bin/cli/src/main.rs @@ -1258,7 +1258,7 @@ fn report_batched_size(proof: &prover::logup_phase::BatchedProof, tables: usize, /// Where the time went, summed per span label. /// -/// The prover's own spans are per table and there are 227 of them, so the raw +/// The prover's own spans are per table and there are hundreds of them, so the raw /// timeline is unreadable; what answers "where is the time" is the total per /// label. Sums exceed wall time, because tables run concurrently — the ratios /// between labels are the point, not the absolute figures. diff --git a/crypto/stark/src/prover.rs b/crypto/stark/src/prover.rs index 443bb99ce..12d480c15 100644 --- a/crypto/stark/src/prover.rs +++ b/crypto/stark/src/prover.rs @@ -2108,7 +2108,7 @@ pub trait IsStarkProver< /// /// The codeword is kept rather than the LDEs it came from. That is the /// whole reason this split is affordable: on the ethrex block the trace and - /// composition LDEs of all 227 tables are tens of gigabytes, while their + /// composition LDEs of every table are tens of gigabytes, while their /// DEEP codewords together are about 6.5 GB — one extension element per row /// instead of every column. Holding them is what saves walking the /// execution again just to recompute them once the coefficient is known. @@ -2268,7 +2268,7 @@ pub trait IsStarkProver< /// The accumulator is the batch polynomial the spec describes. A member is /// added and dropped, so what is held is one codeword per distinct domain /// rather than one per table — which on the ethrex block is 13 instead of - /// 227, and about a gigabyte instead of eight and a half. + /// one per table, and about a gigabyte instead of eight and a half. fn accumulate( acc: &mut Vec>, coefficient: &FieldElement, diff --git a/docs/prove_and_retire_design.md b/docs/prove_and_retire_design.md index 162d87add..0b2b111a3 100644 --- a/docs/prove_and_retire_design.md +++ b/docs/prove_and_retire_design.md @@ -31,11 +31,27 @@ challenges, an accumulated codeword. ## 2. The passes "Walk" means re-executing the program and rebuilding the tables chunk by chunk -(`prover/src/pass.rs`, `trace_builder::walk_and_emit_chunks`). Chunked tables -(CPU, MEMW, LOAD, LT, …) are handed to the pass's visitor as soon as a chunk -fills; the tables that cannot be chunked — the preprocessed ones, the -accumulators (KECCAK, ECSM, …), REGISTER, HALT, one PAGE per page — are the -*residents*, handed over when the walk ends. +(`prover/src/pass.rs`, `trace_builder::walk_and_emit_chunks`). Tables fall into three +groups, not two. Most chunked tables (CPU, MEMW, MEMW_A, MEMW_R, LOAD, CPU32, +BRANCH, EQ, BYTEWISE, STORE — the list is `CHUNKED_KINDS`) are handed to the +pass's visitor as soon as a chunk fills. Four more — **LT, MUL, DVRM and +SHIFT** — are chunked tables in the finished proof but are *not* handed over +during the walk: later derivations keep appending to them (MEMW and HINT feed +LT, DVRM feeds LT and MUL, CPU32 feeds SHIFT, MUL and DVRM), and the finished +run concatenates each source whole, so closing them early would cut their +chunks somewhere the monolithic build does not — which would move their roots +and cost the byte-identical property of §3. Their op lists are held to the end +and chunked in `pass::finish`. The tables that cannot be chunked at all — the +preprocessed ones, the accumulators (KECCAK, ECSM, …), REGISTER, HALT, one PAGE +per page — are the *residents*, handed over when the walk ends. + +So what the walk holds is one chunk per table of the first group, plus the +residents, plus the op lists of the second group and of BITWISE. The last term +grows with the execution rather than with `k`: on the ethrex block it is a low +single-digit percentage of the peak, but a workload that sends many wide memory +accesses down the general MEMW path (up to eight LT ops each, against one for +the aligned path) grows it considerably faster. §10 lists it as the known +asymptotic gap. | pass | what it does | keeps | drops | |---|---|---|---| @@ -103,14 +119,18 @@ Each run prints the pass timings, `Trace build (prove-and-retire): N tables, T s | knob | default | what it does | |---|---|---| -| `A1_TABLE_PARALLELISM` | cores / 6, at most 16 | tables in flight per batch. Measured flat between 12 and 24; 24 costs 6 GB more | +| `A1_TABLE_PARALLELISM` | cores / 6, at most 16 | tables in flight per batch. The sweep recorded on `pass::table_parallelism` is flat from 1 to 16 (21550 MB) and steps at 32 (26640 MB, +5.0 GB) for 1% of time | | `A1_PIPELINE=0` | pipelined | run each batch inline instead of on the consumer thread (diagnostic) | | `A1_KEEP_COMPOSITION=1` | off | keep the composition parts for the Open pass (§3) | +| `A1_INFLIGHT` | 0 | full batches allowed to wait in the channel beyond the one being processed | +| `LAMBDA_STREAM_LDE` | off | `1`/`true` retires each table's main LDE after the Round 1 commit and rebuilds it on demand, and frees the leaf half of every committed Merkle tree; `auto` decides from the peak-RAM estimate (needs `disk-spill`). Off by default, and it changes this approach's memory profile too — §6's numbers were taken with it off | | `_RJEM_MALLOC_CONF` | — | jemalloc options for experiments; the CLI's own setting is §6 | Verification on its own: `cli verify ` for a per-table proof -written with `--output`. With `--features hash-metrics` (#987) every verify -path prints the keccak hashes it did. +written with `--output`. The `hash-metrics` feature that prints a verify's +keccak count comes from #987, which is **not on this branch** — the verify-hash +column in §6 was measured on a tree that has it, and cannot be reproduced here +until this branch is rebased onto it. ## 5. What verifies, and with what @@ -137,7 +157,11 @@ Still to be reviewed by someone who did not write it: the soundness of the fold coefficient (sampled per table from the shared seed after absorbing that table's round-3 data) and of the absorption order. -## 6. Numbers (ethrex block 25368371, 30.5 M cycles, 96 cores) +## 6. Numbers (ethrex block 25368371, 30.5 M cycles, 96 cores, blowup 2) + +Taken with `LAMBDA_STREAM_LDE` off and `A1_KEEP_COMPOSITION` off except where +the row names it. `trace-build` hard-codes blowup 2 and has no `--blowup` +flag, so the batched path cannot yet be measured at the blowup-4 posture. | | peak heap | prove | vs monolithic | proof | verify | verify hashes | |---|---|---|---|---|---|---| @@ -175,9 +199,12 @@ processed one at a time after each walk (−10% once batched). - One batch polynomial **per height**, not one in total: the fold squares the coset offset each layer, so codewords of different lengths do not line up without a mixed-height commitment (#951's direction). -- The spec's Open optimization (keep the Merkle internal nodes, drop the leaves) +- Holding each table's whole Merkle tree from the fold pass to the Open pass measured +9 GB for −4 s and was not kept; keeping the composition parts - (`A1_KEEP_COMPOSITION`) is the trade that pays. + (`A1_KEEP_COMPOSITION`) is the trade that pays. The spec's Open optimization + proper — keep the internal nodes, drop the leaves — *is* implemented + (`MerkleTree::drop_leaves`, `get_proof_by_pos_with_leaf_sibling`, + `TableCommit::retire_leaves`) and ships behind `LAMBDA_STREAM_LDE`. - The per-table variant is not in the spec; it is what makes the approach a drop-in. Distribution across workers is not attempted; the pipelined batch worker is the seam for it. @@ -225,3 +252,9 @@ of it in the monolithic build and check the walk has each. - Serializing `BatchedProof` to disk (`--output` covers the per-table proof). - Distributing retirement batches across workers. - An independent soundness review of the batched fold (§5). +- Bounding the op lists the walk holds whole (§2): LT, MUL, DVRM and SHIFT + are excluded from `CHUNKED_KINDS` because their chunk boundaries are not + knowable until the run ends, so residency is O(cycles) rather than O(chunk) + for that term. BITWISE's lookup list is the cheaper half of the same problem + and has no ordering constraint — a histogram is commutative, so it could be + folded per segment without moving any root. diff --git a/prover/src/challenge_phase.rs b/prover/src/challenge_phase.rs index d95cafa8a..8d7b67ce7 100644 --- a/prover/src/challenge_phase.rs +++ b/prover/src/challenge_phase.rs @@ -46,7 +46,7 @@ pub struct Challenge { /// (DECODE from the ELF, one per ELF data page, ...) are the bulk of this pass. pub(crate) airs: crate::VmAirs, /// The transcript right after the sampling, which every later pass forks - /// per table. Kept rather than rebuilt: re-absorbing 227 roots to get back + /// per table. Kept rather than rebuilt: re-absorbing every root to get back /// to this state is both slower and a second place for the order to be /// wrong. pub transcript: DefaultTranscript, diff --git a/prover/src/logup_phase.rs b/prover/src/logup_phase.rs index ac78659b5..f60f8de21 100644 --- a/prover/src/logup_phase.rs +++ b/prover/src/logup_phase.rs @@ -902,7 +902,7 @@ pub fn run_open( /// The split is the whole point. A table keeps what only it can answer for — /// its roots, its out-of-domain values, its openings — and a group carries the /// FRI those tables share. That is the 57.9% of a per-table proof that stops -/// being paid 227 times. +/// being paid once per table. pub struct BatchedProof { /// Per table, in AIR order. pub tables: Vec, diff --git a/prover/src/pass.rs b/prover/src/pass.rs index e1b14c924..fb379fee4 100644 --- a/prover/src/pass.rs +++ b/prover/src/pass.rs @@ -9,7 +9,21 @@ //! //! Trading re-execution for memory is the whole bargain: a pass never keeps a //! chunk it has dealt with, so what it holds is one table plus the tables that -//! cannot be retired, no matter how long the run is. +//! cannot be retired. +//! +//! That bound is not O(1) in the length of the run, and the difference is worth +//! being precise about. Four chunked tables — LT, MUL, DVRM and SHIFT — are +//! deliberately absent from [`crate::tables::trace_builder::CHUNKED_KINDS`] +//! because later derivations keep appending to them (MEMW and HINT feed LT, +//! DVRM feeds LT and MUL, CPU32 feeds all three), so their chunk boundaries are +//! not knowable until the run ends. Their op lists, and the `retired_*` buffers +//! a closing MEMW/MEMW_A/CPU32 chunk converts its rows into, are held whole and +//! chunked only in [`finish`]. So is the walk's BITWISE lookup list. Those are +//! compact routed intermediates — tens of bytes per op against the hundreds a +//! trace row costs — and on the measured mainnet block they are a low single +//! digit percentage of the peak, but they grow with the execution, and a +//! workload that sends many wide memory accesses down the general MEMW path +//! (which emits up to eight LT ops each) grows them faster than this one does. use stark::proof::options::ProofOptions; @@ -61,8 +75,10 @@ pub trait Visitor { /// /// Up to 16 the peak does not move at all, because it is set at the end of the /// run by the tables that cannot be retired — BITWISE, DECODE, the pages — and -/// a batch of chunks is small beside them. At 32 the batch itself becomes the -/// peak (it moves to halfway through the run) and buys 1% of time for 5 GB. +/// by the op lists the walk holds whole (LT, MUL, DVRM, SHIFT and the walk's +/// BITWISE lookups; see this module's header), against which a batch of chunks +/// is small. At 32 the batch itself becomes the peak (it moves to halfway +/// through the run) and buys 1% of time for 5 GB. /// /// So the knee is 16 on that machine, and the bound is memory rather than /// cores: a smaller run has a smaller resident set for a batch to hide behind. diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index 1c7d777dc..1fae2a568 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -5493,8 +5493,12 @@ impl Traces { /// This is Approach 1's Commit phase seen from the producer side: the spec /// has the prover commit tables "once the memory pressure becomes too /// large" and drop them, which it can only do if the tables arrive while - /// the execution is still being walked. Buffers here never exceed one - /// chunk per table, so what the walk holds does not grow with the run. + /// the execution is still being walked. The buffers of the kinds listed in + /// [`CHUNKED_KINDS`] never exceed one chunk each; the rest — the four the + /// paragraph below names, the `retired_*` rows a closing chunk converts its + /// ops into, the accumulators and the walk's own BITWISE lookups — are held + /// whole and grow with the run. See `crate::pass`'s header for the size of + /// that term. /// /// The chunks come out exactly as `ops.chunks(max_rows)` would cut them and /// each is built by the same generator, so a consumer sees byte-identical From 62777a376f2f881f2aa25bd882b3aa8531a6989f Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 14:49:07 -0300 Subject: [PATCH 4/8] Stop the CLI changing the allocator for every command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things, all outside the prove-and-retire path. `keep_large_buffers_warm()` ran as the first statement of `main()`, so every subcommand — `prove`, `verify`, `execute`, `--help` — allocated 16 MiB, disabled dirty decay on the oversize arena for the life of the process, and left a 10-second purge thread behind. Disabling decay retains RSS that `auto_storage::available_ram_bytes()` does not model, and it is the sort of change that quietly moves every memory number taken with this binary. Call it from the prove-and-retire path, which is the one that allocates and drops trace-sized buffers in a loop. Both of its mallctl failure paths returned silently, and `env_logger::init()` ran on the next line, so nothing could have been logged even if it had tried. A run where the knob did not land was indistinguishable from one where it did. They now warn. The doc comment also records why `opt.narenas` is the right index — jemalloc 5 reserves the slot after the automatic arenas for the oversize arena (`arena_init_huge`), whose threshold defaults to the same 8 MiB the comment names — since a count used as an index invites a second look. `tikv-jemalloc-ctl` had become a hard dependency carrying `features = ["stats"]`, and `jemalloc-stats` an empty feature. That propagates to `tikv-jemalloc-sys/stats` and so to `--enable-stats`, which puts counters on the malloc fast path of every CLI build, including ones measuring baselines. `keep_large_buffers_warm` needs only `raw`/`mallctl`, so the dependency stays and `stats` goes back behind `jemalloc-stats`, which is what the heap tracker is gated on anyway. Finally, `--output` with any stage but `logup` walked the whole execution, returned no proof, wrote no file and exited 0 — and with `--through batched` it also forced a verification the user had not asked for, because `--output` is OR'd into the `verify` argument. It now fails before the walk with a message naming the stage. --- bin/cli/Cargo.toml | 8 ++++++-- bin/cli/src/main.rs | 48 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/bin/cli/Cargo.toml b/bin/cli/Cargo.toml index 816b34e9c..6c87c555f 100644 --- a/bin/cli/Cargo.toml +++ b/bin/cli/Cargo.toml @@ -14,12 +14,16 @@ clap = { version = "4.3.10", features = ["derive"] } rkyv = { version = "0.8.10", default-features = false, features = ["alloc", "bytecheck", "aligned", "pointer_width_64"] } tempfile = "3" tikv-jemallocator = "0.6" -tikv-jemalloc-ctl = { version = "0.6", features = ["stats"] } +# No `stats` by default: that feature propagates to tikv-jemalloc-sys and builds +# jemalloc with `--enable-stats`, i.e. counters on the malloc fast path, for every +# binary. `keep_large_buffers_warm` needs only `raw`/`mallctl`; the heap tracker is +# what needs the counters, and it is behind `jemalloc-stats`. +tikv-jemalloc-ctl = { version = "0.6" } tikv-jemalloc-sys = "0.6" env_logger = "0.11" [features] -jemalloc-stats = [] +jemalloc-stats = ["tikv-jemalloc-ctl/stats"] disk-spill = ["prover/disk-spill"] instruments = ["prover/instruments", "stark/instruments"] # GPU profiling build (Nsight): CUDA prover + instruments spans + NVTX ranges. diff --git a/bin/cli/src/main.rs b/bin/cli/src/main.rs index 85a31b0e2..64b3a5cdf 100644 --- a/bin/cli/src/main.rs +++ b/bin/cli/src/main.rs @@ -18,8 +18,18 @@ static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; /// Disable the arena's decay and purge it on our own clock instead: hot buffers are /// reused across threads, cold ones still go back to the OS. /// +/// The index is `opt.narenas`: jemalloc 5 reserves the slot immediately after the +/// automatic arenas for the oversize arena (`arena_init_huge`, `huge_arena_ind = +/// narenas_total_get()`), and `opt.oversize_threshold` defaults to the same 8 MiB. +/// /// Linux only: elsewhere jemalloc is built without background threads and the decay /// mallctl traps. +/// +/// Called only from the paths that allocate trace-sized buffers, not from `main`: +/// it disables an arena's decay for the life of the process and leaves a purge +/// thread behind, which is not something `cli verify` or `--help` should pay, and +/// it would otherwise change the allocator under every baseline measured with +/// this binary. fn keep_large_buffers_warm() { #[cfg(target_os = "linux")] { @@ -35,13 +45,24 @@ fn keep_large_buffers_warm() { // SAFETY: `opt.narenas` is `unsigned`, the decay knob is `ssize_t`, and `purge` // takes no value. unsafe { - let Ok(huge_arena) = raw::read::(b"opt.narenas\0") else { - return; + let huge_arena = match raw::read::(b"opt.narenas\0") { + Ok(n) => n, + Err(e) => { + // Not fatal, but the run is then indistinguishable from one + // where the knob worked — which is exactly what makes a + // memory measurement unreadable. Say so. + log::warn!("keep_large_buffers_warm: cannot read opt.narenas ({e}); \ + the oversize arena keeps jemalloc's default decay"); + return; + } }; let decay = format!("arena.{huge_arena}.dirty_decay_ms\0"); - if raw::write(decay.as_bytes(), -1i64).is_err() { + if let Err(e) = raw::write(decay.as_bytes(), -1i64) { + log::warn!("keep_large_buffers_warm: cannot disable decay on arena \ + {huge_arena} ({e}); the oversize arena keeps jemalloc's default"); return; } + log::debug!("keep_large_buffers_warm: decay disabled on arena {huge_arena}"); let purge = CString::new(format!("arena.{huge_arena}.purge")).unwrap(); std::thread::spawn(move || { loop { @@ -356,7 +377,6 @@ enum Stage { } fn main() -> ExitCode { - keep_large_buffers_warm(); env_logger::init(); let cli = Cli::parse(); @@ -1338,6 +1358,23 @@ fn cmd_trace_build( verify: bool, output: Option, ) -> ExitCode { + // Only the per-table proof is serializable today, so `--output` with any + // other stage would walk the whole execution and then write nothing. Say so + // before spending the walk rather than exiting 0 in silence. + if output.is_some() && prove_and_retire && through != Stage::Logup { + eprintln!( + "--output writes the per-table proof, which only `--through logup` assembles; \ + `--through {}` has no serializable proof yet (see docs/prove_and_retire_design.md \u{00A7}10).", + match through { + Stage::Walk => "walk", + Stage::Commit => "commit", + Stage::Challenge => "challenge", + Stage::Batched => "batched", + Stage::Logup => unreachable!(), + } + ); + return ExitCode::FAILURE; + } let elf_data = match std::fs::read(&elf_path) { Ok(data) => data, Err(e) => { @@ -1373,6 +1410,9 @@ fn cmd_trace_build( } }; let outcome = if prove_and_retire { + // The walk allocates and drops one trace-sized buffer after another, + // which is the pattern this works around. + keep_large_buffers_warm(); run_approach_1( &elf, &elf_data, From 54c81ae8727675ed164ad60139ec5250a805e372 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 14:56:16 -0300 Subject: [PATCH 5/8] Make four test assertions able to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `retire_lde_proof_is_byte_identical` compared a proof against itself under `cuda`: there `retire_leaves` returns `None` unconditionally and `retire_main_lde` is compiled out, so both arms take the resident path. That configuration is not hypothetical — `make test-prover-cuda` runs this suite on the merge queue. It is now `#[cfg(not(feature = "cuda"))]`, and each arm asserts `streaming_retire_lde()` actually returned what it set, so the test fails rather than passes if the flag ever stops taking effect. Its `ENV_LOCK` was a function-local `static` that nothing else could name, and libtest calls each `#[test]` once, so it could never be contended — it guarded nothing, and the SAFETY comment above the `set_var` ("single-threaded section guarded by ENV_LOCK") was false on both clauses. Replaced with what is actually true: this is the only writer in the binary, every reader goes through `std::env`, which serialises readers against writers on its own lock, so the exposure is other tests observing the flag under a plain `cargo test` — their coverage, not memory safety. `cargo nextest`, which CI runs, forks per test. The note names the real fix (its own integration binary, as `prover/tests/gpu_force_downgrade.rs` already does) without doing it here. `checkpoint_tests`' `assert!(full.len() > 100_000)` followed an `assert_eq!(full.len(), N_ADDI + 1)` with `N_ADDI = 100_005` — a tautology. The property it was reaching for is already checked by the `logs.len() < full.len()` assertion further down. `chunk_shape_matches_the_built_chunk` gave ops to LT only, so for the other thirteen kinds both sides collapsed to the 4-row padding floor and only the column width was pinned. The row half was covered, but by one kind — so a divergence in a single generator's padding would be missed. It now also populates MUL (dedup, like LT) and SHIFT (plain, 20 ops over a limit of 8, so its chunks are 8/8/4 and sit above the floor), asserts each fixture exercises what it is there for, and counts populated chunks so the loop cannot silently go back to comparing constants. `prover/src/tests/mod.rs` declared `batched_fri_tests` and `challenge_phase_tests` without the `#[cfg(test)]` every other entry carries; the parent `mod tests` is ungated, so those two were the only ones compiled into a non-test build of the library. --- .../src/tests/prove_verify_roundtrip_tests.rs | 30 +++++++++--- executor/src/tests/checkpoint_tests.rs | 4 -- prover/src/tests/mod.rs | 6 ++- prover/src/tests/trace_builder_tests.rs | 47 +++++++++++++++++++ 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/crypto/stark/src/tests/prove_verify_roundtrip_tests.rs b/crypto/stark/src/tests/prove_verify_roundtrip_tests.rs index 4ef3f96be..42ab4f74c 100644 --- a/crypto/stark/src/tests/prove_verify_roundtrip_tests.rs +++ b/crypto/stark/src/tests/prove_verify_roundtrip_tests.rs @@ -253,13 +253,20 @@ fn create_mul_air( /// row-major coset LDE inside the table's fused chain — never its contents. /// A mismatch means the rebuild diverged from what was committed. /// -/// The env var is process-global, so the two proving runs are serialized under -/// a mutex and the prior value is restored. +/// Skipped under `cuda`: there `retire_leaves` returns `None` unconditionally +/// and `retire_main_lde` is compiled out, so both arms would take the resident +/// path and the assertion would compare a proof against itself. +/// +/// The env var is process-global. `cargo nextest`, which is what CI runs, gives +/// each test its own process; a plain `cargo test` does not, so under `make +/// test` the other proving tests in this binary may observe the flag while this +/// one holds it. That costs those tests the mode they meant to exercise, not +/// memory safety: `std::env`'s own `RwLock` serialises every Rust-side reader +/// against the write. The proper fix is this test's own integration binary, the +/// way `prover/tests/gpu_force_downgrade.rs` does it. #[test] +#[cfg(not(feature = "cuda"))] fn retire_lde_proof_is_byte_identical() { - use std::sync::Mutex; - static ENV_LOCK: Mutex<()> = Mutex::new(()); - fn prove_once() -> Vec { let add_column = vec![ FE::one(), @@ -333,14 +340,23 @@ fn retire_lde_proof_is_byte_identical() { serde_cbor::to_vec(&proofs).expect("serialize proofs") } - let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); let prev = std::env::var("LAMBDA_STREAM_LDE").ok(); - // SAFETY: single-threaded section guarded by ENV_LOCK; restored below. + // SAFETY: the only writer in this binary, and every reader of it goes + // through `std::env`, which serialises reads against writes on its own + // lock. Restored below. unsafe { std::env::set_var("LAMBDA_STREAM_LDE", "0") }; + assert!( + !crate::prover::streaming_retire_lde(), + "the flag did not take: this test would compare a proof against itself" + ); let resident = prove_once(); unsafe { std::env::set_var("LAMBDA_STREAM_LDE", "1") }; + assert!( + crate::prover::streaming_retire_lde(), + "the flag did not take: this test would compare a proof against itself" + ); let retired = prove_once(); match prev { diff --git a/executor/src/tests/checkpoint_tests.rs b/executor/src/tests/checkpoint_tests.rs index e0a6f519e..3cac51b85 100644 --- a/executor/src/tests/checkpoint_tests.rs +++ b/executor/src/tests/checkpoint_tests.rs @@ -37,10 +37,6 @@ fn snapshot_resume_produces_identical_logs() { let full = Executor::new(&elf, vec![]).unwrap().run().unwrap().logs; assert_eq!(full.len(), N_ADDI + 1, "every instruction should log once"); - assert!( - full.len() > 100_000, - "must span more than one resume() chunk" - ); // One chunk, then snapshot: the cut lands mid-execution. let mut exec = Executor::new(&elf, vec![]).unwrap(); diff --git a/prover/src/tests/mod.rs b/prover/src/tests/mod.rs index dcd180464..045d31a7d 100644 --- a/prover/src/tests/mod.rs +++ b/prover/src/tests/mod.rs @@ -1,6 +1,7 @@ #[cfg(all(test, feature = "disk-spill"))] pub mod auto_storage_tests; -mod batched_fri_tests; +#[cfg(test)] +pub mod batched_fri_tests; #[cfg(test)] pub mod bitwise_bus_tests; #[cfg(test)] @@ -11,7 +12,8 @@ pub mod branch_bus_tests; pub mod branch_constraints_tests; #[cfg(test)] pub mod bytewise_tests; -mod challenge_phase_tests; +#[cfg(test)] +pub mod challenge_phase_tests; #[cfg(test)] pub mod commit_tests; #[cfg(test)] diff --git a/prover/src/tests/trace_builder_tests.rs b/prover/src/tests/trace_builder_tests.rs index c9742f203..da68f7676 100644 --- a/prover/src/tests/trace_builder_tests.rs +++ b/prover/src/tests/trace_builder_tests.rs @@ -1277,13 +1277,32 @@ fn chunk_shape_matches_the_built_chunk() { let lt_ops: Vec<_> = (0..8u64) .map(|i| crate::tables::lt::LtOperation::new(i % 3, i % 3 + 1, false)) .collect(); + // SHIFT is a plain kind (no dedup): 20 ops over a limit of 8 give chunks of + // 8/8/4, i.e. row counts of 8/8/4. Sizes above 4 matter — an empty op list + // pads to 4, so a fixture whose chunks all land on 4 compares the padding + // floor against itself and would pass even if the row rule were wrong. + let shift_ops: Vec<_> = (0..20u64) + .map(|i| crate::tables::shift::ShiftOperation::new(i, i % 5, false, false, false)) + .collect(); + // MUL deduplicates like LT, and its rows are keyed on the op with the lo/hi + // flag folded into the multiplicity: 12 entries, 3 distinct. + let mul_ops: Vec<_> = (0..6u64) + .flat_map(|i| { + let op = crate::tables::mul::MulOperation::new(i % 3, false, i % 3 + 1, false); + [(op.clone(), false), (op, true)] + }) + .collect(); let routed = CollectedOps { lt_ops, + shift_ops, + mul_ops, ..Default::default() }; let max_rows = crate::tables::MaxRowsConfig { lt: 16, + shift: 8, + mul: 16, ..Default::default() }; @@ -1292,7 +1311,24 @@ fn chunk_shape_matches_the_built_chunk() { 4, "the LT fixture must exercise deduplication (8 ops, 3 distinct)" ); + assert_eq!( + routed.chunk_shape(TableKind::Mul, 0, &max_rows).0, + 4, + "the MUL fixture must exercise deduplication (12 entries, 3 distinct)" + ); + assert_eq!( + routed.num_chunks(TableKind::Shift, &max_rows), + 3, + "the SHIFT fixture must split into several chunks, or the plain path is \ + only ever checked on one" + ); + assert_eq!( + routed.chunk_shape(TableKind::Shift, 0, &max_rows).0, + 8, + "the SHIFT fixture must produce chunks wider than the 4-row padding floor" + ); + let mut populated = 0usize; for kind in [ TableKind::Cpu, TableKind::Memw, @@ -1316,8 +1352,19 @@ fn chunk_shape_matches_the_built_chunk() { (built.num_rows(), built.num_main_columns), "{kind:?} chunk {chunk}: declared shape differs from the built one" ); + if routed.buffered(kind) > 0 { + populated += 1; + } } } + // A kind with no ops pads to the same 4 rows on both sides, so it pins the + // column width and nothing else. Without at least a few populated kinds this + // whole loop is a constant compared against itself. + assert!( + populated >= 3, + "the fixture must give several kinds real ops, or the row half of the \ + comparison is vacuous (populated chunks: {populated})" + ); } /// Collecting an execution chunk by chunk must produce exactly what collecting From e05ca72c3ede5a2b401e18cb7e86f31a97d3fece Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 15:02:11 -0300 Subject: [PATCH 6/8] Satisfy the lint gate `cargo fmt --all`, plus a `clone()` on a `Copy` field that the new out-of-domain tamper arm introduced. --- bin/cli/src/main.rs | 12 ++++++++---- crypto/crypto/src/merkle_tree/merkle.rs | 9 +++++++-- crypto/stark/src/prover.rs | 5 ----- prover/src/batched_verifier.rs | 5 ++++- prover/src/logup_phase.rs | 10 +++------- prover/src/tests/batched_fri_tests.rs | 3 +-- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/bin/cli/src/main.rs b/bin/cli/src/main.rs index 64b3a5cdf..d24acccda 100644 --- a/bin/cli/src/main.rs +++ b/bin/cli/src/main.rs @@ -51,15 +51,19 @@ fn keep_large_buffers_warm() { // Not fatal, but the run is then indistinguishable from one // where the knob worked — which is exactly what makes a // memory measurement unreadable. Say so. - log::warn!("keep_large_buffers_warm: cannot read opt.narenas ({e}); \ - the oversize arena keeps jemalloc's default decay"); + log::warn!( + "keep_large_buffers_warm: cannot read opt.narenas ({e}); \ + the oversize arena keeps jemalloc's default decay" + ); return; } }; let decay = format!("arena.{huge_arena}.dirty_decay_ms\0"); if let Err(e) = raw::write(decay.as_bytes(), -1i64) { - log::warn!("keep_large_buffers_warm: cannot disable decay on arena \ - {huge_arena} ({e}); the oversize arena keeps jemalloc's default"); + log::warn!( + "keep_large_buffers_warm: cannot disable decay on arena \ + {huge_arena} ({e}); the oversize arena keeps jemalloc's default" + ); return; } log::debug!("keep_large_buffers_warm: decay disabled on arena {huge_arena}"); diff --git a/crypto/crypto/src/merkle_tree/merkle.rs b/crypto/crypto/src/merkle_tree/merkle.rs index 54ef9a3d5..01e489b0a 100644 --- a/crypto/crypto/src/merkle_tree/merkle.rs +++ b/crypto/crypto/src/merkle_tree/merkle.rs @@ -330,7 +330,7 @@ where leaves_len: usize, sibling_leaf: B::Node, ) -> Option> { - if leaves_len <= 1 || pos >= leaves_len { + if leaves_len <= 1 || pos >= leaves_len || self.is_root_only() { return None; } let mut merkle_path = Vec::with_capacity(leaves_len.trailing_zeros() as usize); @@ -338,7 +338,12 @@ where let mut node = parent_index(pos + leaves_len - 1); while node != ROOT { - merkle_path.push(self.nodes.get(sibling_index(node))?.clone()); + // `node_get`, not `self.nodes` directly: every other read in this + // file goes through it for the disk-spill mmap indirection. The two + // are mutually exclusive today — `drop_leaves` refuses an mmap-backed + // tree — but a direct read would silently yield `None` here if that + // ever stopped holding, and the prover's opening path unwraps this. + merkle_path.push(self.node_get(sibling_index(node))?.clone()); node = parent_index(node); } self.create_proof(merkle_path) diff --git a/crypto/stark/src/prover.rs b/crypto/stark/src/prover.rs index 12d480c15..00c1e5a44 100644 --- a/crypto/stark/src/prover.rs +++ b/crypto/stark/src/prover.rs @@ -691,10 +691,6 @@ pub struct TableDeep { pub trace_rows: usize, /// The DEEP composition codeword, `lde_size` long. pub deep: Vec>, - /// Where the table sits in the AIR order, carried so the batch can say - /// which group each table ended up in once the codewords are sorted by - /// domain rather than by position. - pub air_index: usize, /// What the batch's coefficient is drawn from: this table's public round-3 /// data, in the order a transcript absorbs it. /// @@ -2186,7 +2182,6 @@ pub trait IsStarkProver< lde_size: domain.interpolation_domain_size * domain.blowup_factor, trace_rows: domain.interpolation_domain_size, deep, - air_index: usize::MAX, bus_contribution: round_1_result .bus_public_inputs .as_ref() diff --git a/prover/src/batched_verifier.rs b/prover/src/batched_verifier.rs index 4e4fb429a..2c44bdc1d 100644 --- a/prover/src/batched_verifier.rs +++ b/prover/src/batched_verifier.rs @@ -124,7 +124,10 @@ pub fn replay( coefficients[idx] = seed.sample_field_element(); } - Ok(Replay { logup, coefficients }) + Ok(Replay { + logup, + coefficients, + }) } /// Verify a batched proof of `elf_bytes`. diff --git a/prover/src/logup_phase.rs b/prover/src/logup_phase.rs index f60f8de21..57279a157 100644 --- a/prover/src/logup_phase.rs +++ b/prover/src/logup_phase.rs @@ -626,13 +626,9 @@ pub fn run_batched( // One FRI per accumulator, and the mapping from table to group. let mut group_of = vec![usize::MAX; n]; let sizes: Vec = acc.keys().copied().collect(); - for (&idx, _) in tables.iter().map(|(i, t)| (i, t)) { - let rows = tables - .iter() - .find(|(i, _)| *i == idx) - .map(|(_, t)| t.trace_rows) - .expect("table just listed"); - let lde = rows * proof_options.blowup_factor as usize; + for (idx, table) in tables.iter() { + let lde = table.trace_rows * proof_options.blowup_factor as usize; + let idx = *idx; group_of[idx] = sizes.iter().position(|s| *s == lde).ok_or_else(|| { Error::Prover(format!( "batched phase: table {idx} has no group of size {lde}" diff --git a/prover/src/tests/batched_fri_tests.rs b/prover/src/tests/batched_fri_tests.rs index ce8253efb..8d083f88b 100644 --- a/prover/src/tests/batched_fri_tests.rs +++ b/prover/src/tests/batched_fri_tests.rs @@ -146,7 +146,6 @@ fn alpha_moves_when_any_table_moves() { lde_size: 8, trace_rows: 4, deep: Vec::new(), - air_index: seed as usize, bus_contribution: Some(FieldElement::::from(seed)), main_roots: stark::prover::MainRoots { precomputed: None, @@ -673,7 +672,7 @@ fn a_tampered_batched_proof_is_rejected() { accepted(&proof); // An out-of-domain value, the other thing the fold seed binds. - let orig = proof.tables[0].trace_ood.get(0, 0).clone(); + let orig = *proof.tables[0].trace_ood.get(0, 0); proof.tables[0].trace_ood.set(0, 0, &orig + &one); rejected(&proof, "an out-of-domain trace value"); proof.tables[0].trace_ood.set(0, 0, orig); From daeef583af403d43bd9489cb93a8a1e1e47f9ee6 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 15:27:20 -0300 Subject: [PATCH 7/8] Declare the `log` dependency the CLI actually uses `keep_large_buffers_warm`'s warnings are inside `#[cfg(target_os = "linux")]`, so a macOS build never compiles them and my local lint runs said nothing. CI, on Linux, did: `use of unresolved module or unlinked crate log`. `log` was reaching `bin/cli` only as a transitive dependency of `env_logger`, which is not a dependency you may name. Declared, with a note on the file that its only user is Linux-gated. --- Cargo.lock | 1 + bin/cli/Cargo.toml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 4cbef8dbe..3c289ff08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -282,6 +282,7 @@ dependencies = [ "env_logger", "executor", "lambda-vm-prover", + "log", "rkyv", "serde_cbor", "stark", diff --git a/bin/cli/Cargo.toml b/bin/cli/Cargo.toml index 6c87c555f..5047b2edf 100644 --- a/bin/cli/Cargo.toml +++ b/bin/cli/Cargo.toml @@ -21,6 +21,9 @@ tikv-jemallocator = "0.6" tikv-jemalloc-ctl = { version = "0.6" } tikv-jemalloc-sys = "0.6" env_logger = "0.11" +# Used by `keep_large_buffers_warm`, whose body is Linux-only — so a macOS build +# will not catch its absence. +log = "0.4" [features] jemalloc-stats = ["tikv-jemalloc-ctl/stats"] From abd54faade63aa7668f5506f21e12bf6f305f998 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Fri, 18 Sep 2026 15:27:20 -0300 Subject: [PATCH 8/8] Build the prover without `parallel` again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make compile-recursion-elfs` compiles `lambda-vm-prover` for the RISC-V guest, where `parallel` is off and there is no rayon. The three new phase modules `use rayon::prelude::*` unconditionally and call `into_par_iter`/`par_iter`, so the recursion guest stopped building: 27 errors, 8 unresolved-`rayon` and 9 missing-method, plus three `E0505`s in `trace_builder`. This is on #994's branch as it stands, not introduced by this PR — the same `cargo check -p lambda-vm-prover --no-default-features` fails identically at `f800e4b0`. It went unnoticed because no CI run has ever touched that branch; this PR is the first, which is how it surfaced. The four `make lint` arms do not catch it either: the workspace-level `--no-default-features` arm still resolves `parallel` through another member's feature unification. Gated with the idiom already used in `trace_builder.rs` — a `#[cfg]` pair around the iterator source, serial arm `into_iter`/`iter`. Where the closure was long enough that duplicating it would be worse than the problem, it is hoisted to a named binding first and both arms map over that, so the body appears once. No behaviour change on any path that runs today: the serial arms exist to compile for the guest, which links the crate for its verifier and never executes these phases. The `E0505`s were the serial arm of the BITWISE collector loop iterating `&collectors` where the parallel arm moves it into `units`, so the closures' borrows of the op lists outlived the point where `CollectedOps` moves those lists. Consumed by value, matching the parallel arm. Verified: `make compile-recursion-elfs` succeeds, all four `make lint` arms and `cargo fmt --check` pass, and the prove-and-retire tests are unchanged at 13/13. --- prover/src/challenge_phase.rs | 35 ++++--- prover/src/commit_phase.rs | 22 ++-- prover/src/logup_phase.rs | 157 +++++++++++++++++------------ prover/src/tables/trace_builder.rs | 5 +- 4 files changed, 131 insertions(+), 88 deletions(-) diff --git a/prover/src/challenge_phase.rs b/prover/src/challenge_phase.rs index 8d7b67ce7..a99ec8568 100644 --- a/prover/src/challenge_phase.rs +++ b/prover/src/challenge_phase.rs @@ -17,6 +17,7 @@ use std::collections::HashMap; +#[cfg(feature = "parallel")] use rayon::prelude::*; use crypto::fiat_shamir::default_transcript::DefaultTranscript; @@ -159,12 +160,17 @@ fn assemble_roots( (&airs.register, &remaining.register, "REGISTER"), ]; // Small tables, many of them: one commit at a time leaves most cores idle. - roots.extend( - fixed - .par_iter() - .map(|(air, trace, name)| commit_resident(air, trace, name)) - .collect::, _>>()?, - ); + #[cfg(feature = "parallel")] + let fixed_roots = fixed + .par_iter() + .map(|(air, trace, name)| commit_resident(air, trace, name)) + .collect::, _>>()?; + #[cfg(not(feature = "parallel"))] + let fixed_roots = fixed + .iter() + .map(|(air, trace, name)| commit_resident(air, trace, name)) + .collect::, _>>()?; + roots.extend(fixed_roots); if airs.include_halt { roots.push(commit_resident(&airs.halt, &remaining.halt, "HALT")?); } @@ -184,12 +190,17 @@ fn assemble_roots( // PAGE is built from the ELF image rather than from an op list, so // it is never retired and is committed here with the rest. let pages: Vec<_> = page_airs.by_ref().collect(); - roots.extend( - pages - .par_iter() - .map(|(air, trace)| commit_resident(air, trace, "PAGE")) - .collect::, _>>()?, - ); + #[cfg(feature = "parallel")] + let page_roots = pages + .par_iter() + .map(|(air, trace)| commit_resident(air, trace, "PAGE")) + .collect::, _>>()?; + #[cfg(not(feature = "parallel"))] + let page_roots = pages + .iter() + .map(|(air, trace)| commit_resident(air, trace, "PAGE")) + .collect::, _>>()?; + roots.extend(page_roots); continue; }; for chunk in 0..count_for(table_counts, kind) { diff --git a/prover/src/commit_phase.rs b/prover/src/commit_phase.rs index d73e539b1..c07062190 100644 --- a/prover/src/commit_phase.rs +++ b/prover/src/commit_phase.rs @@ -100,18 +100,20 @@ fn commit_batch( roots: &std::sync::Mutex>, items: Vec, ) -> Result<(), Error> { + #[cfg(feature = "parallel")] use rayon::prelude::*; type P = stark::prover::Prover; - let done: Result, Error> = items - .into_par_iter() - .map(|(kind, chunk, trace)| { -

>::commit_table_root(airs.get(kind).as_ref(), &trace) - .map(|root| (kind, chunk, root)) - .ok_or_else(|| { - Error::Prover(format!("commit phase: no commitment for a {kind:?} chunk")) - }) - }) - .collect(); + let commit = |(kind, chunk, trace): Item| -> Result { +

>::commit_table_root(airs.get(kind).as_ref(), &trace) + .map(|root| (kind, chunk, root)) + .ok_or_else(|| { + Error::Prover(format!("commit phase: no commitment for a {kind:?} chunk")) + }) + }; + #[cfg(feature = "parallel")] + let done: Result, Error> = items.into_par_iter().map(commit).collect(); + #[cfg(not(feature = "parallel"))] + let done: Result, Error> = items.into_iter().map(commit).collect(); roots.lock().expect("roots").extend(done?); Ok(()) } diff --git a/prover/src/logup_phase.rs b/prover/src/logup_phase.rs index 57279a157..4ffaf8739 100644 --- a/prover/src/logup_phase.rs +++ b/prover/src/logup_phase.rs @@ -97,28 +97,30 @@ fn rounds_batch( done: &Proofs, items: Vec, ) -> Result<(), Error> { + #[cfg(feature = "parallel")] use rayon::prelude::*; let order = &challenge.order; let n = order.len(); - let built: Result, Error> = items - .into_par_iter() - .map(|(kind, chunk, mut trace)| { - let idx = order.index_of(kind, chunk).ok_or_else(|| { - Error::Prover(format!( - "logup phase: {kind:?} chunk {chunk} is not in the layout the Commit phase produced" - )) - })?; - let mut transcript = fork(&challenge.transcript, idx, n); - let rounds = prove_table( - airs.get(kind).as_ref(), - &mut trace, - &challenge.challenges, - &mut transcript, - ) - .map_err(|e| Error::Prover(format!("logup phase: {kind:?} chunk {chunk}: {e}")))?; - Ok((idx, rounds)) - }) - .collect(); + let run_one = |(kind, chunk, mut trace): Item| { + let idx = order.index_of(kind, chunk).ok_or_else(|| { + Error::Prover(format!( + "logup phase: {kind:?} chunk {chunk} is not in the layout the Commit phase produced" + )) + })?; + let mut transcript = fork(&challenge.transcript, idx, n); + let rounds = prove_table( + airs.get(kind).as_ref(), + &mut trace, + &challenge.challenges, + &mut transcript, + ) + .map_err(|e| Error::Prover(format!("logup phase: {kind:?} chunk {chunk}: {e}")))?; + Ok((idx, rounds)) + }; + #[cfg(feature = "parallel")] + let built: Result, Error> = items.into_par_iter().map(run_one).collect(); + #[cfg(not(feature = "parallel"))] + let built: Result, Error> = items.into_iter().map(run_one).collect(); done.lock().expect("logup results").extend(built?); Ok(()) } @@ -235,6 +237,7 @@ fn assemble( resident: &mut Resident, challenge: &Challenge, ) -> Result>, Error> { + #[cfg(feature = "parallel")] use rayon::prelude::*; let order = &challenge.order; let airs = &challenge.airs; @@ -299,10 +302,16 @@ fn assemble( .ok_or_else(|| Error::Prover(format!("logup phase: page {i} is not in the layout")))?; jobs.push((idx, air, trace)); } + #[cfg(feature = "parallel")] let built: Vec<(usize, StarkProof)> = jobs .into_par_iter() .map(|(idx, air, trace)| build(idx, air, trace).map(|proof| (idx, proof))) .collect::>()?; + #[cfg(not(feature = "parallel"))] + let built: Vec<(usize, StarkProof)> = jobs + .into_iter() + .map(|(idx, air, trace)| build(idx, air, trace).map(|proof| (idx, proof))) + .collect::>()?; for (idx, proof) in built { slots[idx] = Some(proof); } @@ -409,29 +418,31 @@ fn deep_batch( done: &Deeps, items: Vec, ) -> Result<(), Error> { + #[cfg(feature = "parallel")] use rayon::prelude::*; let order = &challenge.order; let n = order.len(); - let built: Result, Error> = items - .into_par_iter() - .map(|(kind, chunk, mut trace)| { - let idx = order.index_of(kind, chunk).ok_or_else(|| { - Error::Prover(format!( - "batched phase: {kind:?} chunk {chunk} is not in the layout" - )) - })?; - let mut transcript = fork(&challenge.transcript, idx, n); - let deep = deep_of( - airs.get(kind).as_ref(), - &mut trace, - &challenge.challenges, - &mut transcript, - challenge.roots.get(idx).cloned(), - ) - .map_err(|e| Error::Prover(format!("batched phase: {kind:?} chunk {chunk}: {e}")))?; - Ok((idx, deep)) - }) - .collect(); + let run_one = |(kind, chunk, mut trace): Item| { + let idx = order.index_of(kind, chunk).ok_or_else(|| { + Error::Prover(format!( + "batched phase: {kind:?} chunk {chunk} is not in the layout" + )) + })?; + let mut transcript = fork(&challenge.transcript, idx, n); + let deep = deep_of( + airs.get(kind).as_ref(), + &mut trace, + &challenge.challenges, + &mut transcript, + challenge.roots.get(idx).cloned(), + ) + .map_err(|e| Error::Prover(format!("batched phase: {kind:?} chunk {chunk}: {e}")))?; + Ok((idx, deep)) + }; + #[cfg(feature = "parallel")] + let built: Result, Error> = items.into_par_iter().map(run_one).collect(); + #[cfg(not(feature = "parallel"))] + let built: Result, Error> = items.into_iter().map(run_one).collect(); // Folded in a fixed order within the batch, so the sequence is a function // of the walk and not of which thread finished first. let mut built = built?; @@ -548,6 +559,7 @@ pub fn run_batched( let airs = &challenge.airs; let n = order.len(); { + #[cfg(feature = "parallel")] use rayon::prelude::*; let mut state = done.lock().expect("fold state"); let build = |idx: usize, @@ -600,10 +612,16 @@ pub fn run_batched( })?; jobs.push((idx, air, trace)); } + #[cfg(feature = "parallel")] let deeps: Vec<(usize, Deep)> = jobs .into_par_iter() .map(|(idx, air, trace)| build(idx, air, trace)) .collect::>()?; + #[cfg(not(feature = "parallel"))] + let deeps: Vec<(usize, Deep)> = jobs + .into_iter() + .map(|(idx, air, trace)| build(idx, air, trace)) + .collect::>()?; for (idx, deep) in deeps { fold_one(&mut state, idx, deep); } @@ -714,30 +732,32 @@ fn open_batch( done: &Opens, items: Vec, ) -> Result<(), Error> { + #[cfg(feature = "parallel")] use rayon::prelude::*; let order = &challenge.order; let n = order.len(); - let built: Result, Error> = items - .into_par_iter() - .map(|(kind, chunk, mut trace)| { - let idx = order.index_of(kind, chunk).ok_or_else(|| { - Error::Prover(format!( - "open pass: {kind:?} chunk {chunk} is not in the layout" - )) - })?; - let mut transcript = fork(&challenge.transcript, idx, n); - let opening = open_of( - airs.get(kind).as_ref(), - &mut trace, - &challenge.challenges, - &mut transcript, - iotas_of(batched, idx)?, - take_kept(batched, idx), - ) - .map_err(|e| Error::Prover(format!("open pass: {kind:?} chunk {chunk}: {e}")))?; - Ok((idx, opening)) - }) - .collect(); + let run_one = |(kind, chunk, mut trace): Item| { + let idx = order.index_of(kind, chunk).ok_or_else(|| { + Error::Prover(format!( + "open pass: {kind:?} chunk {chunk} is not in the layout" + )) + })?; + let mut transcript = fork(&challenge.transcript, idx, n); + let opening = open_of( + airs.get(kind).as_ref(), + &mut trace, + &challenge.challenges, + &mut transcript, + iotas_of(batched, idx)?, + take_kept(batched, idx), + ) + .map_err(|e| Error::Prover(format!("open pass: {kind:?} chunk {chunk}: {e}")))?; + Ok((idx, opening)) + }; + #[cfg(feature = "parallel")] + let built: Result, Error> = items.into_par_iter().map(run_one).collect(); + #[cfg(not(feature = "parallel"))] + let built: Result, Error> = items.into_iter().map(run_one).collect(); done.lock().expect("openings").extend(built?); Ok(()) } @@ -868,12 +888,19 @@ pub fn run_open( jobs.push((idx, air, trace)); } { + #[cfg(feature = "parallel")] use rayon::prelude::*; - opens.extend( - jobs.into_par_iter() - .map(|(idx, air, trace)| build(idx, air, trace)) - .collect::, _>>()?, - ); + #[cfg(feature = "parallel")] + let opened = jobs + .into_par_iter() + .map(|(idx, air, trace)| build(idx, air, trace)) + .collect::, _>>()?; + #[cfg(not(feature = "parallel"))] + let opened = jobs + .into_iter() + .map(|(idx, air, trace)| build(idx, air, trace)) + .collect::, _>>()?; + opens.extend(opened); } opens.sort_by_key(|(idx, _)| *idx); diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index 1fae2a568..5ffe32949 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -4226,7 +4226,10 @@ fn build_traces( { base.add_ops(&bitwise_ops); memw_register::collect_bitwise_from_memw_register(&memw_register_rows, &mut base); - for f in &collectors { + // By value, like the parallel arm's `units.extend(collectors)`: these + // closures borrow the op lists, and the lists are moved into + // `CollectedOps` below, so the collectors have to be dropped here. + for f in collectors { f(&mut base); } }