Replace guessed+hashed constants with gate-constructed constants#415
Replace guessed+hashed constants with gate-constructed constants#415
Conversation
alon-f
left a comment
There was a problem hiding this comment.
@alon-f partially reviewed 15 files and made 1 comment.
Reviewable status: 10 of 24 files reviewed, 2 unresolved discussions.
crates/circuits/src/finalize_constants.rs line 210 at r3 (raw file):
let mut hi = m31_values.len(); while lo < hi { let mid = lo + (hi - lo) / 2;
use something else
- Register `u = qm31_from_u32s(0, 0, 1, 0)` as the third default constant
(idx 2) in `Context::default()`, alongside zero (idx 0) and one (idx 1)
- Add `Context::u()` accessor returning `Var { idx: 2 }`
- Create `crates/circuit_common/src/finalize_constants.rs` skeleton module
- Register `finalize_constants` in `circuit_common/src/lib.rs`
- Enable indexmap `std` feature in circuits and stark_verifier crates to fix
compilation with indexmap 2.13.0 (which requires explicit hasher when
`default-features = false`)
- Update all existing tests to account for the new u constant at idx 2
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Constants are now derived from u (the QM31 extension element) using arithmetic gates instead of being guessed and Blake-hashed: - Reserve u at address 2 in Context::default() - Build +1 chain for consecutive M31 integers - Power-of-2 base decomposition for larger M31 values (K = (a*base+b)*base+c) - QM31 basis construction (i = u^2 - 2, iu = i*u) for extension field constants - Remove hash_constants from finalize_context (no more Blake hash of constants) - Replace generate_column_indices +1 loop with context.constant() - Update finalize_guessed_vars to skip constants (yield gates come from finalize_constants) The preprocessed trace commits to the circuit structure, implicitly verifying constants without a separate hash commitment. Privacy tests need regenerated fixtures due to changed circuit topology. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The base for constant decomposition can be any value in the chain, not just a power of 2. Using the full chain length as the base gives better coverage and simpler logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Always build the +1 chain to at least 16, providing a reasonable base for decomposition even when few consecutive constants exist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sorting M31 values and scanning is ~10x faster than doing O(N) HashMap lookups with QM31 keys (72us vs 6.8us for 4000 constants). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After sort+dedup, the consecutive run from 0 satisfies m31_values[i] == i. Binary search for the first index where this breaks — O(log N) instead of O(N) scan. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Broadcast QM31 constants where all 4 limbs are equal are now constructed with a single multiply against the (1,1,1,1) vector built as (1+i)+(u+iu), instead of the full 4-mul QM31 decomposition. Also renamed is_m31_broadcast to is_base_field_element (the old name was wrong — it checked (x,0,0,0) not (x,x,x,x)). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Constants come from an IndexMap<QM31, Var> which is already deduplicated by key — no duplicates possible after extraction. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Regenerate PRIVACY_CAIRO_VERIFIER_CONSTS_HASH and PRIVACY_RECURSION_CIRCUIT_CONSTS_HASH after the constants infrastructure change (u is now a default constant). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix import ordering to match codebase convention (external before crate) - Fix stale doc comments referencing power-of-2 base constraint - Consolidate build_plus_one_chain to reuse extend_chain - Remove duplicated finalize_non_constant_guessed_vars test helper, use the real finalize_guessed_vars instead - Fix stale test comment about base value Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The chain is the consecutive +1 sequence of M31 integers. Decomposition intermediates and results are now cached in a separate m31_cache (seeded from the chain) so they can be reused across constants without polluting the chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
u is now marked as an Output gate in finalize_constants. The existing output handling in public_logup_sum and lookup_sum automatically adds a logup use term with u's hardcoded value (0,0,1,0) at address 2. This constrains the prover to use the correct value for u — if they use a different value, the logup won't balance. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When decomposing e.g. 30000 with base=100, the intermediate a*base=300 could shadow a reserved constant 300, causing its Var to be yielded by both the intermediate Mul gate and the final Add gate. Fix: use var_idx_for_m31 to output intermediates to reserved constant Vars when they match, and skip the final Add gate if the result was already produced by an intermediate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The chain is always at least MIN_BASE (16), so base is always > 1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Constants get their yield gates from finalize_constants, not from finalize_guessed_vars. Using new_var instead of guess means they're never added to guessed_vars, removing the need for the constant- skipping check in finalize_guessed_vars. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces verbose context.get/new_var/circuit.push patterns with eval! and ops::eq for readability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The chain is always non-empty (seeded with 0 and 1, extended to at least MIN_BASE=16). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The remaining direct Add/Mul pushes are necessary because they output to specific reserved Var indices — ops::add/mul always create fresh output Vars. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Decomposition now uses a dynamic number of limbs instead of fixed 3: val = (...((limbs[n] * B + limbs[n-1]) * B) + ...) + limbs[0]. With MIN_BASE=256, any M31 value needs at most 4 limbs. This removes the cube root computation from compute_min_chain_length entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The std feature on indexmap was added by mistake — it compiles and passes all tests without it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use position() to find the first gap in the consecutive sequence instead of a manual binary search loop. Restore indexmap std feature — it's needed because IndexMap<K, V> without std requires an explicit hasher type parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ba88fbd to
9f6f0a9
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| build_m31_from_base(context, m31_cache, constant_idxs, base, base_idx, val, out_idx); | ||
| m31_cache.insert(val, out_idx); | ||
| out_idx | ||
| } |
There was a problem hiding this comment.
Duplicate variable allocation in get_or_build_m31_var path
Medium Severity
When get_or_build_m31_var is called for a value not in constant_idxs, var_idx_for_m31 is called once in get_or_build_m31_var (creating fresh var V1 as out_idx), and then called again inside build_m31_from_base during the last loop iteration for the same value (creating another fresh var V2). This results in V2 getting the decomposition gate's yield, followed by an unnecessary copy gate from V2 to V1. Every QM31/broadcast constant whose M31 components are above base and aren't themselves registered constants will produce these duplicate variables and redundant copy gates, needlessly inflating the circuit.
Additional Locations (1)
| [627222561, 1410604646, 1160277334, 1483560693, 1862708576, 1350785689, 1411842836, 2026369701]; | ||
| [2022562963, 1603214417, 2039203382, 1731270385, 331501505, 1117559213, 1393505714, 1996309692]; | ||
|
|
||
| // TODO(constants-infra): Recompute after constants infrastructure change stabilizes. |
There was a problem hiding this comment.
Stale preprocessed root with tests disabled
Medium Severity
PRIVACY_RECURSION_CIRCUIT_PREPROCESSED_ROOT is stale — the other two privacy hash constants were updated for the new gate-constructed constants, but this one was left unchanged. The two tests that would catch this mismatch (test_verify_privacy_with_recursion and test_privacy_recursion_with_preprocessed_context) are #[ignore]d instead of being fixed. Any production code relying on privacy_circuit_preprocessed_root() will use an incorrect value, and there is no active test coverage to prevent regression.


Summary
u(QM31 extension element) via arithmetic gates instead of being guessed and Blake-hasheduis constrained via public logup sum (Output gate),oneviau*one=u+1chain for consecutive M31 integers, base decomposition for larger values, broadcast(x,x,x,x)optimization, QM31 basis combination for general extension field constantshash_constantsfromfinalize_contextentirelygenerate_column_indicesnow usescontext.constant()instead of inline+1loopTest plan
cargo test --release)scripts/clippy.sh)scripts/rust_fmt.sh --check)finalize_constants.rs🤖 Generated with Claude Code
Note
High Risk
Changes how circuit constants are constrained (now via gate construction rooted at
u), affecting all verifier/prover circuits and their committed topology. Any mistake could silently weaken soundness or break recursive verification due to mismatched preprocessed roots/expected hashes.Overview
Replaces constant handling across the circuit stack: constants are no longer
guessed and Blake-hashed during finalization, and are instead gate-constructed via a newcircuits::finalize_constants::finalize_constantspass (derivingzero, constrainingu/one, building+1chains, base decomposition for large M31 values, broadcast(x,x,x,x)optimization, and general QM31 composition).Circuit builders/verifiers (Cairo and generic
circuit_air) now callfinalize_constantsbeforefinalize_guessed_vars, andcircuit_common::finalize_contextdrops the prior constants/output hashing logic and only pads components. This shifts default constant layout to includeuat index 2, updates many tests/expected indices/stats, regenerates privacy/fibonacci constant hashes, and temporarily disables privacy recursion tests pending an updatedPRIVACY_RECURSION_CIRCUIT_PREPROCESSED_ROOT.Written by Cursor Bugbot for commit 9f6f0a9. This will update automatically on new commits. Configure here.