-
Notifications
You must be signed in to change notification settings - Fork 593
feat: merge-train/barretenberg #20440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
AztecBot
wants to merge
15
commits into
next
Choose a base branch
from
merge-train/barretenberg
base: next
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+1,061
−2,179
Conversation
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
One commit per issue. --------- Co-authored-by: notnotraju <raju@aztec-labs.com>
Resolves or addresses issues 45, 48, 49, 51, 52 noted in Sherlock external audit of cycle_group. Notes: - Issue 52: Addressed via adding missing range constraints on scalar of constant point at infinity in `batch_mul`, as suggested - Issue 48: Addressed via setting `sign_coefficient` directly in `create_ecc_add_gate` via boolean input `is_addition` - Issue 45: Addressed with comments only - Issue 49: No update in this PR as this issue was addressed independently in the main repo prior to the start of this audit
## Summary - Adds optional instrumentation (gated behind `BB_POLY_STATS` env var) that reports per-polynomial value-size distribution after ProverInstance construction - Helps assess feasibility of variable-length encoding for memory reduction (re: AztecProtocol/barretenberg#1623) ## Usage ```bash BB_POLY_STATS=1 ./build/bin/chonk_bench --benchmark_filter=Full/2 ``` Outputs a table like: ``` === Polynomial Memory Analysis === Polynomial | AllocSize | Zeros | <=4B | <=8B | <=16B | <=32B | Mem(MB) | Compr(MB) sigma_1 | 336637 | 0 | 336629 | 0 | 0 | 8 | 10.27 | 1.28 w_l | 336637 | 45462 | 165039 | 13556 | 76637 | 35943 | 10.27 | 3.00 ... TOTAL | 8125052 | 3192926 | 4004383 | 148196 | 318296 | 461251 | 247.96 | 35.34 Total actual memory: 247.96 MB Total compressed memory: 35.34 MB Potential savings: 85.7% ``` ## Initial findings (2^19 mock app circuit) - **85.7% theoretical savings** with ideal variable-length encoding - **Sigma/ID polys** (8 polys, ~82 MB): 100% values fit in 4 bytes — `uint32_t` gives 8x compression - **Wire polys** (w_l–w_4, ~41 MB): ~50% fit in 4B, ~20% need full 32B - **Selector polys** (~96 MB): ~80% zeros, rest mostly small — already partially optimized via mega-selectors - **z_perm, lookup_inverses** (~11 MB): 100% zeros at construction time (filled later) ## Test plan - [x] Builds with `cmake --build build --target chonk_bench` - [x] `BB_POLY_STATS=1 ./build/bin/chonk_bench` produces correct per-polynomial report - [x] No output when `BB_POLY_STATS` is unset (zero overhead in normal operation)
Makes OriginTag checking more stringent in the following sense: - Previous check: values from different rounds are combined using _any_ challenge - Updated check: values from different rounds are combined using a challenge whose round provenance is at least the max of the submitted value round provenances This has many false positives since we safely break this pattern when values are otherwise constrained by the protocol itself (the canonical example being evaluations of already committed polynomials). These cases are handled by explicitly updating the values in question with the tag of the challenge that binds them (e.g. the evaluation challenge in the aforementioned poly eval case). Note: Also adds a standalone suite for basic OriginTag functionality and separates OriginTag testing for stdlib primitives into standalone tests in their respective suites.
## Summary - Remove 24 dead `c_bind.hpp`/`c_bind.cpp` files that exported individual `extern "C"` WASM functions (the old binding architecture) - All functionality is now exclusively accessed through the unified `bbapi()` msgpack entry point by TypeScript (bb.js), Rust (barretenberg-rs), and the native CLI - Remove 3 now-empty object library references from `CMakeLists.txt` (`crypto_blake3s_objects`, `crypto_ecdsa_objects`, `crypto_schnorr_objects`) - Remove stale `#include` of deleted headers from 2 test files ## Binary size reduction | Binary | Before | After | Reduction | |--------|--------|-------|-----------| | `barretenberg.wasm` | 10,355,061 | 9,839,961 | **-515,100 (5.0%)** | | `barretenberg.wasm.gz` | 2,840,344 | 2,712,449 | **-127,895 (4.5%)** | | `libbarretenberg.a` | 74,680,286 | 72,745,528 | **-1,934,758 (2.6%)** | ## Test plan - [x] Native build (`cmake --preset clang20`) succeeds - [x] WASM build (`cmake --preset wasm`) succeeds - [ ] CI passes (no callers of removed functions exist)
…strain EC ops (#20349) Before, the slope conditions on a given "row" of the MSM table were accumulated into a single accumulator. We split this up to four individual accumulators. This applies to `add`, `double`, and `skew`. --------- Co-authored-by: notnotraju <raju@aztec-labs.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
BEGIN_COMMIT_OVERRIDE
chore: changes after external memory audit (#20118)
fix: cycle_group external audit 1 (#20058)
feat: add BB_POLY_STATS for prover polynomial memory analysis (#20421)
chore: origin tags 0 (#20084)
refactor: remove dead old-style c_bind files (#20422)
fix: [ECCVM] split up
addanddoubleconstraints to correctly constrain EC ops (#20349)END_COMMIT_OVERRIDE