feat: merge-train/barretenberg #19541
Merged
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
Chonk debugging info were wrong for reset and tail kernel because the verifier accumulator hash was not being updated. Fixed the problem and improved debugging info by printing the hash only when it is updated.
Collaborator
Author
Flakey Tests🤖 says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
### 🧾 Audit Context
Minor, cosmetic changes to the non-native field and decomposition
relations. No logic changes, just a few variable name changes and
comments for clarity.
### 🛠️ Changes Made
tldr: Old pattern → New pattern
- `*_low_limbs*` → `*_limb_0*` (or `*_1_limb_0*` for z variables)
- `*_low_limbs_*_shift` → `*_limb_1*` (or `*_2_limb_0*` for z variables)
- `*_high_limbs*` → `*_limb_2*` (or `*_1_limb_1*` for z variables)
- `*_high_limbs_*_shift` → `*_limb_3*` (or `*_2_limb_1*` for z
variables)
- `*_shift` (accumulator) → `prev_*` (accumulator only)
- `relation_wide_limbs` → `relation_wide_limbs_lo`
- `relation_wide_limbs_shift` → `relation_wide_limbs_hi`
Variable name changes in the non-native relation:
| Old name | New name | Description |
|------------------|------------------|-------------|
| `v_0_{0-4}` | `v_{0-4}` | v limbs (batching challenge) |
| `v_1_{0-4}` | `v_sqr_{0-4}` | v² limbs |
| `v_2_{0-4}` | `v_cube_{0-4}` | v³ limbs |
| `v_3_{0-4}` | `v_quad_{0-4}` | v⁴ limbs |
| `p_x_low_limbs` | `p_x_limb_0` | P.x limb 0 |
| `p_x_low_limbs_shift` | `p_x_limb_1` | P.x limb 1 |
| `p_x_high_limbs` | `p_x_limb_2` | P.x limb 2 |
| `p_x_high_limbs_shift` | `p_x_limb_3` | P.x limb 3 |
| `p_y_low_limbs` | `p_y_limb_0` | P.y limb 0 |
| `p_y_low_limbs_shift` | `p_y_limb_1` | P.y limb 1 |
| `p_y_high_limbs` | `p_y_limb_2` | P.y limb 2 |
| `p_y_high_limbs_shift` | `p_y_limb_3` | P.y limb 3 |
| `z_low_limbs` | `z_1_limb_0` | z₁ limb 0 |
| `z_high_limbs` | `z_1_limb_1` | z₁ limb 1 |
| `z_low_limbs_shift` | `z_2_limb_0` | z₂ limb 0 |
| `z_high_limbs_shift` | `z_2_limb_1` | z₂ limb 1 |
| `quotient_low_binary_limbs` | `quotient_binary_limbs_0` | Quotient
limb 0 |
| `quotient_high_binary_limbs` | `quotient_binary_limbs_1` | Quotient
limb 1 |
| `quotient_low_binary_limbs_shift` | `quotient_binary_limbs_2` |
Quotient limb 2 |
| `quotient_high_binary_limbs_shift` | `quotient_binary_limbs_3` |
Quotient limb 3 |
| `accumulators_binary_limbs_{0-3}_shift` |
`prev_accumulators_binary_limbs_{0-3}` | Previous accumulator |
| `relation_wide_limbs` | `relation_wide_limbs_lo` | Lower wide limb |
| `relation_wide_limbs_shift` | `relation_wide_limbs_hi` | Higher wide
limb |
Variable name changes in the decomposition relation:
| Old Name Pattern | New Name Pattern | Description |
|------------------|------------------|-------------|
| `accumulator_low_limbs_range_constraint_{0-4}` |
`accumulator_limb_0_range_constraint_{0-4}` | Accumulator A₀ microlimbs
|
| `accumulator_low_limbs_range_constraint_{0-4}_shift` |
`accumulator_limb_1_range_constraint_{0-4}` | Accumulator A₁ microlimbs
|
| `accumulator_high_limbs_range_constraint_{0-4}` |
`accumulator_limb_2_range_constraint_{0-4}` | Accumulator A₂ microlimbs
|
| `accumulator_high_limbs_range_constraint_{0-3}_shift` |
`accumulator_limb_3_range_constraint_{0-3}` | Accumulator A₃ microlimbs
|
| `p_x_low_limbs*` | `p_x_limb_0*` | P.x limb 0 and microlimbs |
| `p_x_low_limbs_*_shift` | `p_x_limb_1_*` | P.x limb 1 and microlimbs |
| `p_x_high_limbs*` | `p_x_limb_2*` | P.x limb 2 and microlimbs |
| `p_x_high_limbs_*_shift` | `p_x_limb_3_*` | P.x limb 3 and microlimbs
|
| `p_y_low_limbs*` | `p_y_limb_0*` | P.y limb 0 and microlimbs |
| `p_y_low_limbs_*_shift` | `p_y_limb_1_*` | P.y limb 1 and microlimbs |
| `p_y_high_limbs*` | `p_y_limb_2*` | P.y limb 2 and microlimbs |
| `p_y_high_limbs_*_shift` | `p_y_limb_3_*` | P.y limb 3 and microlimbs
|
| `z_low_limbs*` | `z_1_limb_0*` | z₁ limb 0 and microlimbs |
| `z_low_limbs_*_shift` | `z_2_limb_0_*` | z₂ limb 0 and microlimbs |
| `z_high_limbs*` | `z_1_limb_1*` | z₁ limb 1 and microlimbs |
| `z_high_limbs_*_shift` | `z_2_limb_1_*` | z₂ limb 1 and microlimbs |
| `quotient_low_binary_limbs*` | `quotient_limb_0*` | Quotient q₀ and
microlimbs |
| `quotient_low_binary_limbs_*_shift` | `quotient_limb_2_*` | Quotient
q₂ and microlimbs |
| `quotient_high_binary_limbs*` | `quotient_limb_1*` | Quotient q₁ and
microlimbs |
| `quotient_high_binary_limbs_*_shift` | `quotient_limb_3_*` | Quotient
q₃ and microlimbs |
| `relation_wide_limbs*` | `relation_wide_limbs_lo*` | Lower wide limb
and microlimbs |
| `relation_wide_limbs_*_shift` | `relation_wide_limbs_hi_*` | Higher
wide limb and microlimbs |
Other changes:
- Changed `static auto` → `static constexpr auto` for shift constants
- Added `clang-format off/on` directives for complex expressions
- Enhanced inline documentation with mathematical notation (e.g., `Pₓ =
(Pₓ,₃ || Pₓ,₂ || Pₓ,₁ || Pₓ,₀)`)
- Improved sub-relation comments with detailed explanations
- Added section headers for variable groups
### ✅ Checklist
- [x] Audited all methods of the relevant module/class
- [x] Audited the interface of the module/class with other (relevant)
components
- [x] Documented existing functionality and any changes made (as per
Doxygen requirements)
- [x] Resolved and/or closed all issues/TODOs pertaining to the audited
files
- [x] Confirmed and documented any security or other issues found (if
applicable)
- [x] Verified that tests cover all critical paths (and added tests if
necessary)
- [ ] Updated audit tracking for the files audited (check the start of
each file you audited)
### 📌 Notes for Reviewers
No logic changes
Adds some asserts in derive_generators to protect against undefined behavior and adds generator_data.hpp to pedersen hash scope for additional scrutiny
## Summary
Multiple fixes for `ci-barretenberg-full` mode:
1. **Link vm2_stub in barretenberg-rs** - Add static library version of
vm2_stub and always link it instead of conditionally linking vm2
2. **Propagate AVM env var** - Pass `AVM` environment variable to
isolated docker containers so tests use the correct `bb` binary
3. **Denoise barretenberg-rs** - Add `denoise` to build and test
commands for cleaner CI output
## Problem
`ci-barretenberg-full` mode was failing with multiple issues:
1. barretenberg-rs build failed with:
```
error: could not find native static library `vm2`, perhaps an -L flag is
missing?
```
This happened because `ci-barretenberg-full` builds with `AVM=0`, so
libvm2.a is not produced.
2. acir_tests failed with:
```
barretenberg/cpp/build/bin/bb-avm: No such file or directory
```
The `AVM` environment variable wasn't being passed to isolated docker
containers, causing `find-bb` to incorrectly select `bb-avm` instead of
`bb`.
## Solution
1. Create a static library `libvm2_stub.a` from the existing vm2_stub
OBJECT library and always link it in barretenberg-rs (the stub throws at
runtime if AVM recursion is attempted)
2. Propagate `AVM` env var to isolated docker containers in
`ci3/docker_isolate`
3. Use `denoise` for barretenberg-rs build and test commands
## Caching
Verified that `libvm2_stub.a` will be included in the cache upload since
it's built to `build/lib/` and the native build cache uploads
`build/{bin,lib}`.
---------
Co-authored-by: johnathan79717 <511655+johnathan79717@users.noreply.github.com>
…9475) In the constructor of a cycle_scalar from a bigfield element, limb1 is split across the lo and hi slices of the scalar. If the max value of limb1 is such that the limb fits entirely in the lo slice, calling `no_wrap_split_at` would result in an assertion failure (i.e. `BB_ASSERT(lsb_index < num_bits);`). This PR introduces handling for this edge case by checking this condition first then setting the hi slice contribution of limb1 to 0 if needed.
Tying up loose ends for Ultra/Mega builders audit: - Review gate_data.hpp and plookup_tables/dummy.hpp - Ensure all relation block comments are at class level (rather than on accumulate) for consistency - Minor improvements in some relation docs (e.g. remove ascii art from NNF)
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
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: Improve Chonk debug info (#19538)
chore: translator non-native and decomp relations audit (#19081)
chore: add safety to derive_generators and tweak pedersen scope (#19525)
fix: ci-barretenberg-full mode fixes (#19466)
test: use WASM backend for bbjs-test acir tests (#19529)
fix: use absolute path in run_test.sh for CI
fix: use env_objects in bb-external library
fix: completeness issue in cycle scalar constructor from bigfield (#19475)
chore: review a few minor files for ultra/mega audit (#19513)
END_COMMIT_OVERRIDE