feat: reduce CRS and polynomial memory for non-ZK proofs#20731
Merged
johnathan79717 merged 6 commits intomerge-train/barretenbergfrom Mar 3, 2026
Merged
feat: reduce CRS and polynomial memory for non-ZK proofs#20731johnathan79717 merged 6 commits intomerge-train/barretenbergfrom
johnathan79717 merged 6 commits intomerge-train/barretenbergfrom
Conversation
Size the CRS at max_end_index (actual data extent) rather than dyadic_size for non-ZK proofs. The Shplonk quotient polynomial round_up_power_2 is now conditional on having non-dyadic claims (Libra/sumcheck), which only exist in the ZK path. Also size PolynomialBatcher's temporary polynomials (batched_unshifted, batched_to_be_shifted, A_0_pos, A_0_neg) at actual_data_size rather than full_batched_size, reducing transient memory allocations. Measured ~16-18 MiB peak RSS savings on Chonk AMM flow (max_end_index=278,429 vs dyadic_size=524,288).
The previous commit accidentally changed get_polynomial_size() from virtual_size() to size(), which breaks relation correctness checks that iterate over the full dyadic circuit size.
johnathan79717
requested review from
ledwards2225
and removed request for
ledwards2225
barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp
Outdated
Show resolved
Hide resolved
nishatkoti
reviewed
Feb 24, 2026
- Remove round_up_power_2 entirely from shplonk: no operations in compute_batched_quotient require dyadic polynomial sizes - Simplify CRS key_size in ultra_prover: remove redundant dyadic_size/2 guard since max_end_index > dyadic_size/2 always holds - Delete unused Polynomial::evaluate(z, target_size) overload
barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp
Outdated
Show resolved
Hide resolved
nishatkoti
approved these changes
Feb 24, 2026
Contributor
nishatkoti
left a comment
nishatkoti
left a comment
There was a problem hiding this comment.
The polynomial part looks good to me.
Move the ZK minimum CRS size guard from ultra_prover into CommitmentKey itself, so callers don't need to special-case ZK. Also remove the now-stale comment in shplonk.
ledwards2225
reviewed
Mar 2, 2026
| // Enforce this as a minimum so callers don't need to special-case ZK. | ||
| static constexpr size_t minimum_crs_size() | ||
| { | ||
| if constexpr (requires { Curve::SUBGROUP_SIZE; }) { |
Contributor
There was a problem hiding this comment.
This is minor but I think this is currently only needed for Grumpkin not BN. Won't matter in practice since outside of tests our circuits are way bigger than these minimums but might be a bit unclear
ledwards2225
approved these changes
Mar 2, 2026
Contributor
ledwards2225
left a comment
ledwards2225
left a comment
There was a problem hiding this comment.
LGTM - one minor comment. Thanks for the cleanup along the way!
The minimum is only relevant for ZK-flavor tiny test circuits. Keep it in construct_proof with a HasZK guard rather than in CommitmentKey where it applies unconditionally to both curves.
johnathan79717
added a commit
that referenced
this pull request
Mar 4, 2026
## Summary - Size the CRS at `max_end_index` (actual data extent) rather than `dyadic_size` for non-ZK proofs - Make Shplonk quotient polynomial `round_up_power_2` conditional on having non-dyadic claims (Libra/sumcheck), which only exist in the ZK path - Size PolynomialBatcher's temporary polynomials (`batched_unshifted`, `batched_to_be_shifted`, `A_0_pos`, `A_0_neg`) at `actual_data_size` rather than `full_batched_size` ## Benchmark results Chonk AMM flow (`ecdsar1+amm_add_liquidity_1_recursions+sponsored_fpc`), `max_end_index=278,429` vs `dyadic_size=524,288`: ``` bb prove --scheme chonk --ivc_inputs_path .../ecdsar1+amm_add_liquidity_1_recursions+sponsored_fpc/ivc-inputs.msgpack -o /tmp/out ``` | | Run 1 (KB) | Run 2 (KB) | Run 3 (KB) | Avg (MiB) | |---|---|---|---|---| | Before | 702,660 | 704,044 | 691,328 | 682.7 | | After | 657,096 | 672,916 | 677,508 | 654.8 | | **Savings** | **45,564** | **31,128** | **13,820** | **~28 MiB (~4.1%)** | ## Test plan - [x] `ultra_honk_bench` at 2^20 passes (no crash) - [x] `ultra_honk_tests`: 260 passed, 5 skipped - [x] `commitment_schemes_tests`: 82 passed, 2 skipped - [ ] CI passes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
max_end_index(actual data extent) rather thandyadic_sizefor non-ZK proofsround_up_power_2conditional on having non-dyadic claims (Libra/sumcheck), which only exist in the ZK pathbatched_unshifted,batched_to_be_shifted,A_0_pos,A_0_neg) atactual_data_sizerather thanfull_batched_sizeBenchmark results
Chonk AMM flow (
ecdsar1+amm_add_liquidity_1_recursions+sponsored_fpc),max_end_index=278,429vsdyadic_size=524,288:Test plan
ultra_honk_benchat 2^20 passes (no crash)ultra_honk_tests: 260 passed, 5 skippedcommitment_schemes_tests: 82 passed, 2 skipped