Merged
Conversation
Clean up of tube-related files and functions following the move of the tube to Noir (see #16400 and #16713) Closes AztecProtocol/barretenberg#1523
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Cleaning up and restricting the interface to cycle_scalar. The main purpose of this thread of work is to make it easier to identify when certain constraints need to be applied (e.g. primality) and to make applying them less error prone: - Add method `field_t::split_at_unrestricted()` (better name TBD!) to implement hi/lo splitting logic previously housed in `cycle_scalar` - Remove unused constructors `cycle_scalar(const field_t&)` and `cycle_scalar(const ScalarField&)` - remove option to `skip_primality_test` in method `create_from_bn254_scalar()`
### 🧾 Audit Context `stdlib::field_conversion` provides the serialization/deserialization methods for `stdlib::Transcript`. ### 🛠️ Changes Made - Simplified the logic by making it more uniform and getting rid of redundant arguments - Enabled `on_curve` checks for Ultra deserialization of group elements (BN254 and Grumpkin) - Removed redundant range constraints in `fr --> bigfield` conversion and optimized the constraint ensuring that `fr = lo + hi * shift`. Note that limbs were constrained to be (136, 118) bits, although such range constraints are enforced by `bigfield(low, hi)` constructor. - Optimized `point_at_infinity` check for Grumpkin points. - Expanded docs and made them Doxygen-friendly - Added tests and [an issue](AztecProtocol/barretenberg#1541) that will be resolved in a follow-up - The vks and circuit sizes have changed due to enabled checks and the small optimizations described above ### 📌 Notes for Reviewers Agreed with @suyash67 that existing point-at-infinity issue AztecProtocol/barretenberg#1527 should be handled by ensuring the consistent representation of points at infinity in all groups. Added a test that will fail once the issue is resolved. --------- Co-authored-by: ledwards2225 <l.edwards.d@gmail.com>
ledwards2225
requested review from
IlyasRidhuan,
fcarreiro and
jeanmon
as code owners
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
Summary of the fixes based on the audit of the `bigfield` class by Veridise (on commit [480f49d](https://github.com/AztecProtocol/aztec-packages/tree/480f49dad314ea4e753ff1e5180992a16926d2b3)): #### [V-AZT-VUL-001: Missing zero-check for division by constant]() Added an assertion that triggers when the divisor is a constant zero and the flag `check_for_zero` is set to true. Also added a test that validates this behaviour. Response: Fixed in bbf2f73 #### [V-AZT-VUL-002: Incomplete ‘assert_is_not_equal‘ assumes random distribution]() The function `assert_is_not_equal` is no longer exposed as a public function (via noir). In fact, none of the non-native field methods (and group operations) are exposed to developers via noir. Noir instead uses the natively written [noir-bignum](https://github.com/noir-lang/noir-bignum) library. (The `bigint_constraint.cpp` file that contains the interface for non-native field methods between noir and barretenberg is yet to be removed from the repository, this will be done in an upcoming PR.) Response: Acknowledged #### [V-AZT-VUL-003: borrow_lo rounds down]() Updated the computation of `borrow_lo_value` to use ceiling instead of floor: ```cpp uint256_t borrow_lo_value = (max_remainders_lo + ((uint256_t(1) << (2 * NUM_LIMB_BITS)) - 1)) >> (2 * NUM_LIMB_BITS); ``` Response: Fixed in c1bdd88 #### [V-AZT-VUL-004: Unsafe default for `msub_div()`]() Changed the default value of `enable_divisor_nz_check` to `true` and explicitly pass it as `false` in its usage in the `biggroup` class. In a few instances, passing this parameter as `false` can lead to incorrect handling of the point at infinity, added TODOs at such places. These instances will be analysed separately to avoid any unexpected behaviour. Response: Fixed in 25e820c, usage of `msub_div` will be analysed separately #### [V-AZT-VUL-005: Limbs with a maximum value of 0 trigger compilation failure]() In the function `decompose_into_default_range` we have an assertion: ```cpp ASSERT(num_bits > 0) ``` because number of bits (of a circuit variable) being 0 does not really make sense. The only case when `carry_hi_msb` or `carry_lo_msb` can be 0 is when: ```cpp template <typename Builder, typename T> void bigfield<Builder, T>::unsafe_evaluate_multiply_add(const bigfield& input_left, const bigfield& input_to_mul, const std::vector<bigfield>& to_add, const bigfield& input_quotient, const std::vector<bigfield>& input_remainders) ``` is called with `input_to_mul` as constant 0 and/or `input_quotient` as constant 0. In the usage of the function `unsafe_evaluate_multiply_add` (or `unsafe_evaluate_multiple_multiply_add`) the `input_quotient` is always a circuit witness. This ensures that the `carry_lo_msb > 0` and `carry_hi_msb > 0` (added assertions to verify this empirically). Response: We think it does not make sense to support `num_bits = 0` in range constraint functions, but open to discussion on this if there's any way `carry_lo_msb` or `carry_hi_msb` can actually be 0 in practice. #### [V-AZT-VUL-006: Off-by-one comparison of maximum_unreduced_limb_value]() Changed the definition of `get_maximum_unreduced_limb_value()` to $2^Q - 1$ (from $2^Q$). Response: Fixed in 3da35ba #### [V-AZT-VUL-007: Maintainability Improvements]() 1. Removed unused constants: - `bigfield.prime_basis_maximum_limb` - `bigfield.shift_right_1` - `bigfield.shift_right_2` 2. Renamed `negative_prime_modulus_mod_binary_basis` to `negative_prime_modulus_mod_native_basis` and use that while checking the prime limb consistency (in `evaluate_polynomial_identity`) 3. Used `NUM_LIMBS` instead of hard-coded 4. 4. Fixed comments (using $L$ instead of $t$) 5. Changed the definition of `get_maximum_unreduced_value()` to a form $(2^N - 1)$ and use the comparison operators correctly 6. Fixed `static constexpr uint64_t MAX_UNREDUCED_LIMB_BITS = NUM_LIMB_BITS + MAX_ADDITION_LOG` 7. Got rid of variable `neg_prime` 8. Used `is_constant()` instead of iterating over each limb to check constant-ness in `operator+` and `operator-`. 9. Removed duplicate/redundant assertion: `ASSERT(input_left.size() == input_right.size() && input_left.size() < 1024);` 10. Fixed misc comments Response: Fixed in e2024df #### [V-AZT-VUL-008: Incorrect term used during calculation on bound of upper limb]() Corrected the calculation of the resultant maximum bound of the upper and lower limbs: $$D_{\textsf{hi}} < 2^{2Q + L + \textcolor{red}{3}},$$ $$D_{\textsf{lo}} < 2^{2Q + L + \textcolor{red}{2}}.$$ Response: Fixed in f41813e #### [V-AZT-VUL-009: Optimization Improvements]() 1. In the function `add_two`, we could add another `if` condition when two of the three inputs are constant and reduced by the prime. But the current implementation should handle that implicitly on calling `add_two` on each limb. Note that no additional gates would be added since each `add_two` call on limbs would just update additive constants on the witness. 2. Agree with the observation but we think its fine to have redundant reductions in constant case as that would cause negligible impact on prover performance. 3. It is a good suggestion to use a tighter bound for range constraint on the quotient when reduction is not necessary. However, while creating the quotient witness using the constructor: ```cpp template <typename Builder, typename T> bigfield<Builder, T> bigfield<Builder, T>::create_from_u512_as_witness(Builder* ctx, const uint512_t& value, const bool can_overflow, const size_t maximum_bitlength) ``` when `can_overflow` is set to `false`, `maximum_bitlength` must be greater than $3L$. This assertion breaks if we use tighter range constraint on the quotient. This needs to be investigated further.
alexghr
pushed a commit
that referenced
this pull request
Nov 5, 2025
# v2.0.3..v2.1.0-rc.1 Notes ## Significant L1 Changes ### 1. **Rollup Contract Interface Changes** - **`propose()` function signature changed**: Now requires an additional `_attestationsAndSignersSignature` parameter - **`validateHeaderWithAttestations()` function signature changed**: Also requires the new signature parameter - This affects any code that directly calls these functions on the rollup contract ### 2. **New Required Configuration Parameters** Several new configuration parameters are now required for deployment: - `localEjectionThreshold`: Stricter ejection threshold local to specific rollup (default: 196,000 tokens) - `slashingDisableDuration`: How long slashing can be disabled in seconds (default: 5 days) ### 3. **GSE Contract Changes** - **New function**: `setProofOfPossessionGasLimit()` \- allows governance to adjust gas limits for BLS proof validation - **Gas-limited proof validation**: Proof of possession validation now has configurable gas limits (default: 200,000 gas) ### 4. **Validator Queue Management Changes** - **`flushEntryQueue()` behavior changed**: Now has an overload accepting a `_toAdd` parameter to limit validator additions - **New validator flush accounting**: System now tracks available validator flushes per epoch Significant Non-Breaking Changes -------------------------------- ### 1. **Enhanced Slashing Controls** - **Temporary slashing disable**: Vetoers can now temporarily disable slashing for the configured duration - **New function**: `setSlashingEnabled(bool)` for controlling slashing state ### 2. **Improved Validator Selection** - **Configurable lag period**: Validator sampling now uses configurable epoch lag instead of fixed 2-epoch delay - **Better bootstrapping**: Enhanced validator set bootstrapping with improved flush size calculations ### 3. **Updated Default Values** - **Coin issuer rate**: Updated to `25,000,000,000 tokens / year` (approximately 793 tokens per second) - **Local ejection threshold**: Set to 196,000 tokens (stricter than global 50,000 threshold) ## Significant Node Changes ### Fixes - Rollback world state on failed block sync – Prevents bad state persistence by rolling back uncommitted data if block sync fails. [(#17158)](github.com//pull/17158) - Early rejection of duplicate nullifiers – Detects and rejects transactions with duplicate nullifiers before inclusion. [(#17157)](github.com//pull/17157) - Watcher pruning fix – Watcher now re-executes only blocks from the relevant pruned epoch, avoiding cross-epoch slashing issues. [(#17145)](github.com//pull/17145) - Improved proposal validation – Fully validates proposal headers (including archive root derivation) and blocks attempts to reuse existing block numbers. [(#17144)](github.com//pull/17144) - L1 to L2 message sync reliability – Waits for rollup to reach the inbox block before marking L1→L2 messages as synced; adds helpers to track message readiness. [(#17132)](github.com//pull/17132) - Slashing round recovery – Executes pending slashing rounds skipped during the first executable round; adds slashExecuteRoundsLookBack to control re-check depth. [(#17125)](github.com//pull/17125) - Broker restart on rollup change – Ensures broker restarts when rollup chain changes to stay synchronized. [(#17120)](github.com//pull/17120) - Remote signer readiness check – Verifies that a remote signer is available before use. [(#17119)](github.com//pull/17119) - Orchestrator and agent retry improvements – Makes connections to the broker more robust under transient failures. [(#17117)](github.com//pull/17117) - Telemetry cleanup – Fixes incorrect or spammy telemetry warnings. [(#17155)](github.com//pull/17155) ### Features - Network configuration support – Introduces centralized configuration for network parameters. [(#17113)](github.com//pull/17113) ## Full Changelog You can generate this yourself with `./scripts/commits v2.0.3..v2.1.0-rc.1 1000 -m -g`. #### Fixes - fix: use archiveAt(0) instead of getBlock to get genesis archive tree - backport v2 ([#17447](#17447)) — spypsy, 5 days ago - fix: add keystoreDirectory option to sequencer ([#17265](#17265)) — spypsy, 13 days ago - fix: testnet archival node - v2 ([#17142](#17142)) — Aztec Bot, 3 weeks ago #### Chores - chore: bump minor version — Mitch, 4 days ago — [dbc243f](dbc243f) - chore: backport dependabot deps ([#17463](#17463)) — Aztec Bot, 5 days ago - chore: Backport slack alerts ([#17460](#17460)) — PhilWindle, 5 days ago - chore(backport-to-v2): chore: New salt for staging-ignition (#17453) ([#17453](#17453)) — Aztec Bot, 5 days ago - chore(backport-to-v2): fix: improve libp2p connection limits for network discovery (#17425) ([#17425](#17425)) — Aztec Bot, 5 days ago - chore(backport-to-v2): feat: add flushing rewarder (#17335) ([#17335](#17335)) — Aztec Bot, 6 days ago - chore(backport-to-v2): feat: add date gated relayer (#17323) ([#17323](#17323)) — Aztec Bot, 6 days ago - chore(backport-to-v2): feat: support using existing ERC20 token for fee and staking (#17413) ([#17413](#17413)) — Aztec Bot, 6 days ago - chore: Delete contract addresses from chain l2 config ([#17430](#17430)) — PhilWindle, 6 days ago - chore: More updated staging public config ([#17364](#17364)) — PhilWindle, 7 days ago - chore(backport-to-V2): L1 backports ([#17365](#17365)) — Lasse Herskind, 7 days ago - chore: Ensure DB map sizes are configured for networks ([#17383](#17383)) — PhilWindle, 7 days ago - chore: Backport of fixes into v2 ([#17206](#17206)) — PhilWindle, 8 days ago - chore: update zkpassport version ([#17339](#17339)) — saleel, 8 days ago - chore: Backport of workflow fix ([#17333](#17333)) — PhilWindle, 11 days ago - chore: Streamline staging deployments ([#17328](#17328)) — PhilWindle, 11 days ago - chore(backport-to-v2): fix: avm gracefully handles shifts (shl) with huge bit sizes (#17171) ([#17171](#17171)) — Aztec Bot, 12 days ago - chore(backport-to-v2): chore: remove unconstrained generics from trait impls (#17075) ([#17075](#17075)) — Aztec Bot, 12 days ago - chore: Backport deployment refactor ([#17280](#17280)) — PhilWindle, 12 days ago - chore(backport-to-v2): fix(docs): Update Counter contract tutorial imports and remove unnecessary sections (#17241) ([#17241](#17241)) — Aztec Bot, 13 days ago - chore: remove ACCEPT_DISABLED_AVM_VK_TREE_ROOT ([#17238](#17238)) — Alex Gherghisan, 13 days ago - chore: remove bad rollup-version default ([#17223](#17223)) — Alex Gherghisan, 2 weeks ago - chore(docs): node docs to v2 ([#17205](#17205)) — esau, 2 weeks ago - chore(backport-to-v2): chore(avm)!: Fix a misleading log in recursive verifier related to public input (#17184) ([#17184](#17184)) — Aztec Bot, 2 weeks ago - chore: Backport of ignition fix attempt 2 ([#17201](#17201)) — PhilWindle, 2 weeks ago - chore: turn on testnet compat test ([#17195](#17195)) — Alex Gherghisan, 2 weeks ago - chore: Backport fix to staging-ignition to v2 ([#17159](#17159)) — PhilWindle, 3 weeks ago - chore: kubectl ([#17140](#17140)) — Alex Gherghisan, 3 weeks ago #### Other - backport dependabots p2 ([#17488](#17488)) — mralj, 4 days ago --------- Co-authored-by: AztecBot <tech@aztecprotocol.com>
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
6 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.
BEGIN_COMMIT_OVERRIDE
chore: Tube-related clean up (#17064)
chore: cycle group cleanup #5 (#17026)
chore:
field_conversionclean-up/int audit (#16898)fix: bigfield veridise audit fixes (#16842)
END_COMMIT_OVERRIDE