feat: use new MultiFieldChallenger in static verifier

[zlangley]

Corresponds to openvm-org/stark-backend#311

[github-actions]

Claude finished @zlangley's task in 1m 25s —— View job

---

Code Review: Use new MultiFieldChallenger in static verifier

• Gather context and read changed files
• Review changes and provide feedback

---

This PR replaces the old transcript chip (3-limb split + reduce-to-BabyBear) with a new one mirroring the MultiFieldTranscript from stark-backend. The new design uses base-2^31 packing for observations and base-BabyBear decomposition for sampling, with explicit absorb/sample index tracking for the duplex sponge. Overall the approach is clean and well-structured. Here are my findings:

Soundness

1. Decomposition boundary check looks correct. The decompose_bn254_to_base_baby_bear_digits function properly constrains packed = top_quotient * p^7 + lower and verifies the boundary case when top_quotient is at its maximum. This ensures the decomposition is unique and covers the full BN254 range.

2. sponge_squeeze logic matches standard duplex semantics. The condition absorb_idx != 0 || sample_idx == 0 correctly triggers a permutation when there's pending absorbed data or when all squeeze outputs have been consumed.

3. observe_commit correctly flushes partial observe buffer before direct absorption. This ensures digest words aren't mixed with partially-packed BabyBear values.

Suggestions

4. sample() reverses then pops — consider just indexing from the end (mod.rs:274-279):

   let mut digits = decompose_bn254_to_base_baby_bear_digits(ctx, &self.baby_bear, squeezed);
   digits.reverse();
   self.sample_buf = digits.to_vec();
   self.sample_buf.pop().expect(...)

   The reverse + pop pattern works but is slightly indirect. Since digits is a fixed-size array of length 7, digits is already a [BabyBearWire; 7] — you could avoid the reverse by directly converting to a Vec and consuming from the front (or just keeping the current pattern, since it works fine). This is minor / stylistic only.

5. digits.to_vec() on an array (mod.rs:276): Since digits is [BabyBearWire; NUM_SAMPLES_PER_WORD], calling .to_vec() works but .into() or Vec::from(digits) is slightly more idiomatic. Very minor.

6. Comment p^(k+1) <= q could be more precise (mod.rs:32):

   /// = largest k such that p^(k+1) <= q = 7
   

   The = 7 reads as q = 7 at first glance. Consider rewording to: /// = largest k such that p^(k+1) <= q; evaluates to 7 for clarity.

Questions

7. Doc comment says MultiFieldTranscript<BabyBear, Bn254Scalar, _, 3, 2> (mod.rs:168). Could you confirm these generic params (3, 2) match the actual native transcript configuration? This is the key correctness invariant — the circuit must mirror the native transcript exactly.

8. NUM_OBS_PER_WORD = 8 and NUM_SAMPLES_PER_WORD = 7: These are derived from floor(254/31) = 8 and largest k s.t. p^(k+1) <= q. It might be worth adding a const_assert! or debug assertion that validates these against the actual field moduli, to catch issues if the field parameters ever change. Not blocking.

Test Changes

9. The test updates to use explicit FiatShamirTranscript::<RootConfig>:: disambiguation via helper functions are clean and well-motivated — they ensure the circuit-side transcript matches the trait-based native transcript exactly, including for observe_ext, observe_commit, sample_ext, sample_bits, and check_witness.

10. The removal of stale comments (// check_witness now returns () and asserts internally) is good cleanup.

Dependency Changes

11. The Cargo.toml points all stark-backend deps to the feat/multi-field-transcript branch. This will need to be updated to the target branch (e.g., develop-v2) before merge, or merged in coordination with stark-backend#311.

Overall: Looks good. The core transcript logic is sound, the decomposition constraints are properly bounded, and the tests verify native-vs-circuit consistency. The main thing to verify is that the generic params in the doc comment match the actual native configuration.

[github-actions]

Claude finished @zlangley's task in 1m 54s —— View job

---

Code Review: Use new MultiFieldChallenger in static verifier

• Gather context and read changed files
• Review transcript implementation changes
• Review test changes
• Review dependency and binary changes
• Provide final review feedback

---

This PR replaces the old transcript chip (3-limb split + reduce-to-BabyBear) with a new one that mirrors MultiFieldTranscript from stark-backend. The new design uses base-2^31 packing for observations and base-BabyBear decomposition for sampling, with explicit absorb/sample index tracking for the duplex sponge. The implementation is clean and well-structured. Here are my detailed findings:

Soundness

Decomposition constraints are correct. decompose_bn254_to_base_baby_bear_digits properly:

• Range-checks each digit < BABY_BEAR_MODULUS
• Range-checks top_quotient <= floor((q-1)/p^7)
• Verifies recomposition: packed == top_quotient * p^7 + sum(digit_i * p^i)
• Handles the boundary case: when top_quotient is at max, constrains that the lower part doesn't exceed the remainder

This ensures a unique, bijective decomposition covering the full BN254 range.

Sponge logic is correct. sponge_absorb overwrites state at absorb_idx and triggers a permutation when the rate is filled. sponge_squeeze triggers a permutation when there's pending absorbed data (absorb_idx != 0) or all squeeze outputs are consumed (sample_idx == 0), matching standard duplex sponge semantics.

observe_commit correctly flushes the partial observe buffer before directly absorbing digest elements. This prevents mixing packed BabyBear values with raw digest words.

Issues

1. New binary and SVG files should likely not be committed — crates/sdk/cache/sdk_onion_commit.bin, sdk_root_proof.bin, sdk_root_vk.bin, and the four SVG profiling files under crates/sdk/profile/ are all new files that don't exist on develop-v2.0.0-beta. The cache bins (~277KB total) are build artifacts that will change with every VK update, and the SVG files (4×491 lines) are profiling outputs. Are these intentionally checked in? If they're meant for CI caching or documentation, that's fine, but they look like they may be local artifacts that got staged accidentally.

Suggestions

2. sample() reverse-then-pop is slightly indirect (mod.rs:274-279): The digits.reverse(); self.sample_buf = digits.into(); self.sample_buf.pop() pattern works correctly but is a roundabout way to consume the first element. Since digits is [BabyBearWire; 7], you could use a VecDeque and pop_front(), or simply keep the array and track an index. Very minor / stylistic.

3. is_big_less_than_safe clones a BigUint on every call (mod.rs:124): bounds.lower_max_plus_one.clone() allocates each time decompose_bn254_to_base_baby_bear_digits is called. Since this bound is static, you could precompute it as an Fr constant in BaseBabyBearDecompBounds if the API supports it. Not blocking — the allocation is small relative to constraint generation cost.

4. Comment precision (mod.rs:32): The current comment = largest k such that p^(k+1) <= q; evaluates to 7 is already much clearer than the previous review noted. Looks good.

Test Coverage

5. Tests are thorough. The main test (transcript_outputs_match_native_interleaved_flow) exercises observe, observe_ext, observe_commit, sample, sample_ext, sample_bits, and check_witness in an interleaved sequence, comparing circuit outputs against the native transcript. The PoW witness grinding ensures the check_witness path is exercised end-to-end. The zero-bits and rejection tests cover edge cases.

6. FiatShamirTranscript:: disambiguation helpers are well-motivated — they ensure the circuit tests call the exact same trait methods as the native verifier, preventing accidental use of inherent methods that might behave differently.

Dependencies

7. Cargo.toml points stark-backend deps to feat/multi-field-transcript branch. This needs to be updated to the target branch before merge, or merged in coordination with stark-backend#311.

Overall: Looks good. The core logic is sound, the decomposition constraints are properly bounded, and the tests provide strong native-vs-circuit consistency checks. The main open question is whether the cache/profile binary files are intentional (issue 1).

[github-actions]

Claude finished @zlangley's task in 2m 20s —— View job

---

Code Review: Use new MultiFieldChallenger in static verifier

• Gather context and read changed files
• Review transcript implementation changes
• Review test changes
• Review dependency changes
• Provide final review feedback

---

This PR replaces the old transcript chip (3-limb split + reduce-to-BabyBear) with one mirroring MultiFieldTranscript from stark-backend. The new design uses base-2^31 packing for observations and base-BabyBear decomposition for sampling, with explicit absorb/sample index tracking for the duplex sponge.

Overall: Looks good. The core logic is sound and well-tested. A few observations below.

Soundness

1. Decomposition constraints are correct. constrain_base_baby_bear_decomposition (mod.rs:100-142) properly:

   • Range-checks each digit < BABY_BEAR_MODULUS and top_quotient <= max
   • Verifies recomposition via inner_product + mul_add
   • Handles the boundary case: when top_quotient is at max, constrains lower < lower_max_plus_one
   • This ensures a unique decomposition covering the full BN254 range

2. Sponge logic matches duplex semantics. sponge_squeeze (mod.rs:250-258) correctly triggers a permutation when there's pending absorbed data (absorb_idx != 0) or all squeeze outputs are consumed (sample_idx == 0). The descending squeeze direction (state[RATE-1] first) is consistent with the documented native DuplexSponge behavior, confirmed by the native-vs-circuit test.

3. observe_commit correctly flushes the partial observe buffer (mod.rs:293-299) before directly absorbing digest elements into the sponge.

Suggestions

4. is_big_less_than_safe clones a BigUint on every call (mod.rs:138):

   let lower_is_valid = range.is_big_less_than_safe(ctx, lower, bounds.lower_max_plus_one.clone());

   Since bounds is &'static, this allocation happens each time decompose_bn254_to_base_baby_bear_digits is called. Negligible relative to constraint generation cost, but could be precomputed as an Fr constant if the API supports it. Not blocking.

5. Minor: NUM_OBS_PER_WORD/NUM_SAMPLES_PER_WORD could benefit from compile-time assertions (mod.rs:28-34). A const_assert! validating these against the actual field moduli would guard against silent breakage if field parameters ever change. Nice-to-have, not blocking.

Tests

6. Tests are thorough. The transcript_outputs_match_native_interleaved_flow test exercises observe, observe_ext, observe_commit, sample, sample_ext, sample_bits, and check_witness in an interleaved sequence, comparing circuit outputs against the native transcript end-to-end including PoW witness grinding. Zero-bits and rejection edge cases are also covered.

7. FiatShamirTranscript:: disambiguation helpers (tests.rs:76-95) are well-motivated — they prevent accidental use of inherent methods that could diverge from the trait-based native verifier behavior.

Dependencies

8. Cargo.toml points stark-backend deps to feat/multi-field-transcript branch. This needs to be updated to the merged target branch before this PR merges, or coordinated with stark-backend#311. Expected, just flagging.

9. Unused import removal in crates/sdk/src/tests.rs — clean and correct.

[github-actions]

Claude finished @zlangley's task in 1m 15s —— View job

---

Code Review: Use new MultiFieldChallenger in static verifier

• Gather context and read changed files
• Review transcript implementation changes
• Review test changes
• Review dependency changes
• Provide final review feedback

---

This PR replaces the old transcript chip (3-limb split + reduce-to-BabyBear) with a new one mirroring MultiFieldTranscript from stark-backend. The new design uses base-2^31 packing for observations and base-BabyBear decomposition for sampling, with explicit absorb/sample index tracking for the duplex sponge.

Overall: Looks good. The core transcript logic is sound, well-structured, and properly tested.

Soundness

1. Decomposition constraints are correct (mod.rs:100-142). constrain_base_baby_bear_decomposition properly range-checks each digit < BABY_BEAR_MODULUS, range-checks top_quotient <= max, verifies recomposition via inner_product + mul_add, and handles the boundary case (when top_quotient is at max, constrains lower < lower_max_plus_one). This ensures a unique decomposition covering the full BN254 range.

2. Sponge logic matches duplex semantics (mod.rs:240-258). sponge_absorb overwrites state at absorb_idx and triggers a permutation when the rate is filled. sponge_squeeze correctly triggers a permutation when there's pending absorbed data (absorb_idx != 0) or all squeeze outputs are consumed (sample_idx == 0). The descending squeeze direction (state[RATE-1] first) is consistent with the native DuplexSponge.

3. observe_commit correctly flushes the partial observe buffer (mod.rs:292-299) before directly absorbing digest elements, preventing mixing of packed BabyBear values with raw digest words.

4. Old NUM_SPLIT_LIMBS is fully removed — no stale references remain in the codebase.

Nit / minor suggestions

5. is_big_less_than_safe clones a BigUint on every call (mod.rs:138):

   let lower_is_valid = range.is_big_less_than_safe(ctx, lower, bounds.lower_max_plus_one.clone());

   Since bounds is &'static, this allocation happens on each call to decompose_bn254_to_base_baby_bear_digits. Negligible relative to constraint generation cost, but could be precomputed as an Fr constant if the API supports it. Not blocking.

6. NUM_OBS_PER_WORD = 8 and NUM_SAMPLES_PER_WORD = 7 (mod.rs:28-34): These are derived from field parameter relationships. A const_assert! validating them against the actual field moduli would guard against silent breakage if the fields ever change. Nice-to-have.

Tests

7. Tests are thorough. transcript_outputs_match_native_interleaved_flow exercises observe, observe_ext, observe_commit, sample, sample_ext, sample_bits, and check_witness in an interleaved sequence, comparing circuit outputs against the native transcript end-to-end including PoW witness grinding. Zero-bits and rejection edge cases are also covered.

8. FiatShamirTranscript:: disambiguation helpers (tests.rs:76-95) are well-motivated — they ensure the circuit tests use the exact same trait methods as the native verifier, preventing accidental divergence via inherent methods.

Dependencies

9. Cargo.toml points stark-backend deps to feat/multi-field-transcript branch. This needs to be updated to the merged target branch before this PR merges, or coordinated with stark-backend#311. Expected, just flagging.

10. Unused import removal in crates/sdk/src/tests.rs (SystemParams, WhirProximityStrategy, log_up_security_params_baby_bear_100_bits) — clean and correct.

[github-actions]

group	app.proof_time_ms	app.cycles	leaf.proof_time_ms
fibonacci	3,753	12,000,265	922
keccak	15,661	1,235,218	2,182
regex	1,408	4,136,694	367
ecrecover	637	122,348	266
pairing	926	1,745,757	284
kitchen_sink	2,370	154,763	415

Note: cells_used metrics omitted because CUDA tracegen does not expose unpadded trace heights.

Commit: 0ef78a0

Benchmark Workflow