finalize_constants: Feed all M31 limbs into find_max_consecutive

[ilyalesokhin-starkware]

What

Implements the long-standing TODO in finalize_constants_with_min_base:

// TODO(dan): Consider adding the limbs from all cm31s and qm31s into find_max_consecutive.

The decomposition base is now sized from the consecutive prefix [0, N] of all required M31 values — the keys of m31_constants plus the M31 limbs of every CM31/QM31 constant — rather than only the pure (x, 0, 0, 0) constants.

Each such limb is eventually built (via the +1 chain or build_m31_from_base), so a limb that extends the prefix is now produced by the chain for free instead of via base decomposition.

How

• The limb set is collected in the existing population pass into a HashSet<u32> — no extra iteration over the constants.
• find_max_consecutive now takes a &HashSet<u32> and walks up from 0 in O(M), replacing the previous O(M log M) sort+dedup. (Dropped the now-unused itertools import.)

Why it's safe

This change only affects the chosen base. The rest of the algorithm is correct for any base >= 2 (every limb is < base and covered by the +1 chain), so this is a gate-count optimization, not a soundness change. Adding values to the set is monotonic — the base can only grow or stay the same.

Tests

• test_find_max_consecutive — gap-at-1, gap-after-zero, full run, large-values-past-gap.
• test_find_max_consecutive_without_zero_panics — the zero precondition.
• test_qm31_limbs_extend_plus_one_chain — constant (2,3,4,5) with min_base=2; its limbs grow the base to 5 so the +1 chain runs 2..=5, verified via the circuit topology and carried values.

All circuits tests (73) pass; circuit-common compiles; clippy + rustfmt clean.

Note for reviewers

This changes the base chosen for real circuits (finalize_context uses min_base=256), so a constant whose limbs form a consecutive run past 256 will enlarge the +1 chain and shift trace/proof sizes. Worth a full cargo test --release if there are proof/trace-size fixtures I didn't run.

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Only the decomposition base and gate topology change (sound for any base ≥ 2), but production min_base=256 circuits can grow the +1 chain and shift trace/proof sizes when limb sets extend past 256.

Overview
finalize_constants now sizes the +1 chain base from every M31 limb needed by the circuit (pure M31 constants and all four limbs of each QM31 constant), not only (x,0,0,0) keys. Limbs that continue a consecutive run above min_base extend the base so those values are built by the chain instead of base decomposition—a gate-count tweak, not a constraint change.

find_max_consecutive is rewritten to take a HashSet<u32> and min_base: it returns the largest n ≥ min_base with every value in (min_base, n] present in the set (O(M) walk vs the old sorted scan from 0 on m31_constants). min_base / chain bounds use u32; itertools is dropped.

New tests cover the helper’s gap/min_base semantics and end-to-end cases where QM31 limbs lengthen the chain or leave limbs past a gap on decomposition.

^{Reviewed by Cursor Bugbot for commit ae754f4. Bugbot is set up for automated code reviews on this repo. Configure here.}

[reviewable-StarkWare]

This change is 

[ilyalesokhin-starkware]

replaced by https://github.com/starkware-libs/proving/pull/12

[dancarmoz]

@dancarmoz made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).

---

crates/circuits/src/finalize_constants_test.rs line 219 at r1 (raw file):

    let mut context = TraceContext::default();
    // A single QM31 constant whose limbs (2, 3, 4, 5) fill the consecutive prefix above
    // `min_base = 2`. No *pure* M31 constant requires 3, 4 or 5, but because the limbs are fed into

I think this works because 0 and 1 are added as constants globally, so they fill the gap up 2 (and then the limbs 2,3,4,5 fill the rest up to 5). So this has nothing to with min_base = 2. Right?
If this code had min_base = 3 and the constant was (3, 4, 5, 6), then I believe the base would have been 3, not 6, because find_max_consecutive would have detected a gap at 2, so it would return 1, and then the max would be 3.
If the preferred logic in this case would be to go up to 6 (which I think makes sense), maybe we should just feed min_base into find_max_consecutive and have the loop start from it instead of from 0?

[ilyalesokhin-starkware]

@ilyalesokhin-starkware made 1 comment and resolved 1 discussion.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on dancarmoz).

---

crates/circuits/src/finalize_constants_test.rs line 219 at r1 (raw file):

Previously, dancarmoz (Dan Carmon) wrote…

I think this works because 0 and 1 are added as constants globally, so they fill the gap up 2 (and then the limbs 2,3,4,5 fill the rest up to 5). So this has nothing to with min_base = 2. Right?
If this code had min_base = 3 and the constant was (3, 4, 5, 6), then I believe the base would have been 3, not 6, because find_max_consecutive would have detected a gap at 2, so it would return 1, and then the max would be 3.
If the preferred logic in this case would be to go up to 6 (which I think makes sense), maybe we should just feed min_base into find_max_consecutive and have the loop start from it instead of from 0?

done

[dancarmoz]

@dancarmoz reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on ilyalesokhin-starkware).

---

-- commits line 13 at r3:
I don't think it's critical but the commit message does not accurately document the new changes? i.e. the function now walks up not from 0 but from the min_base argument.

---

crates/circuits/src/finalize_constants.rs line 150 at r3 (raw file):

    // M31 limbs are all < 2^31, so `n + 1` never overflows a u32.
    let mut n = min_base;
    while m31_values.contains(&(n as u32 + 1)) {

Might be nicer if n is a u32 variable and converted to usize at the end (or maybe even later) rather than in every iteration?

[ilyalesokhin-starkware]

• finalize_constants: Feed all M31 limbs into find_max_consecutive #604 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[ilyalesokhin-starkware]

crates/circuits/src/finalize_constants.rs line 150 at r3 (raw file):

Previously, dancarmoz (Dan Carmon) wrote…

Might be nicer if n is a u32 variable and converted to usize at the end (or maybe even later) rather than in every iteration?

changed the base type to u32.

[ilyalesokhin-starkware]

@ilyalesokhin-starkware made 1 comment and resolved 2 discussions.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on dancarmoz).

---

-- commits line 13 at r3:

Previously, dancarmoz (Dan Carmon) wrote…

I don't think it's critical but the commit message does not accurately document the new changes? i.e. the function now walks up not from 0 but from the min_base argument.

done