Bigcurve/Bignum nargo execute failure repro#14381
Closed
MirandaWood wants to merge 63 commits intomw/blob-batchingfrom
Closed
Bigcurve/Bignum nargo execute failure repro#14381MirandaWood wants to merge 63 commits intomw/blob-batchingfrom
MirandaWood wants to merge 63 commits intomw/blob-batchingfrom
Conversation
This reverts commit 68be71e.
…atching-bls-utils
… mw/blob-batching-bls-utils-ts
…atching-bls-utils
… mw/blob-batching-bls-utils-ts
… mw/blob-batching-bls-utils-ts
…atching-bls-utils
… mw/blob-batching-bls-utils-ts
…nto mw/blob-batching-integration
… mw/blob-batching-bls-utils-ts
…atching-bls-utils
… mw/blob-batching-bls-utils-ts
…nto mw/blob-batching-integration
…to mw/blob-batching-integration-ts-sol
5 tasks
TomAFrench
force-pushed
the
mw/bigcurve-bignum-fail-repro
branch
from
0737236 to
9027c6a
Compare
MirandaWood
added a commit
that referenced
this pull request
Jun 4, 2025
## Finalises integration of batched blobs `mw/blob-batching-integration` adds batching to the rollup .nr circuits only (=> will not run in the repo). This PR brings those changes downstream to the typescript and L1 contracts. Main changes: - L1 Contracts: - No longer calls the point evaluation precompile on `propose`, instead injects the blob commitments, check they correspond to the broadcast blobs, and stores them in the `blobCommitmentsHash` - Does not store any blob public inputs apart from the `blobCommitmentsHash` (no longer required) - Calls the point evaluation precompile once on `submitEpochRootProof` for ALL blobs in the epoch - Uses the same precompile inputs as pubic inputs to the root proof verification alonge with the `blobCommitmentsHash` to link the circuit batched blob, real L1 blobs, and the batched blob verified on L1 - Refactors mock blob oracle - Injects the final blob challenges used on each blob into all block building methods in `orchestrator` - Accumulates blobs in ts when building blocks and uses as inputs to each rollup circuit - Returns the blob inputs required for `submitEpochRootProof` on `finaliseEpoch()` - Updates nr structs in ts plus fixtures and tests ## TODOs/Current issues - ~When using real proofs (e.g. `yarn-project/prover-client/src/test/bb_prover_full_rollup.test.ts`), the root rollup proof is generated correctly but fails verification checks in `bb` due to incorrect number of public inputs. Changing the number correctly updates vks and all constants elsewhere, but `bb` does not change.~ EDIT: solved - must include the `is_inf` point member for now (see below TODO) - ~The `Prover.toml` for block-root is not executing. The error manifests in the same way as that in #12540 (but may be different).~ EDIT: temporarily fixed - details in this repro (#14381) and noir issue (noir-lang/noir#8563). - BLS points in noir take up 9 fields (4 for each coordinate as a limbed bignum, 1 for the `is_inf` flag) but can be compressed to only 2. For recursive verification in block root and above, would it be worth the gates to compress these? It depends whether the gate cost of compression is more/less than gate cost of recursively verifying 7 more public inputs. ## PR Stack - [ ] `mw/blob-batching` <- main feature - [ ] ^ `mw/blob-batching-bls-utils` <- BLS12-381 bigcurve and bignum utils (noir) (#13583) - [ ] ^ `mw/blob-batching-bls-utils-ts` <- BLS12-381 bigcurve and bignum utils (ts) (#13606) - [ ] ^ `mw/blob-batching-integration` <- Integrate batching into noir protocol circuits (#13817) - [x] ^ `mw/blob-batching-integration-ts-sol` <- Integrate batching into ts and solidity (#14329) --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Base automatically changed from
mw/blob-batching-integration-ts-sol
to
mw/blob-batching
June 4, 2025 15:22
Contributor
Author
|
Closing as noir-lang/noir#8563 has been resolved by noir-lang/noir#8701 |
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
2 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.
Not sure why, but couldn't make a smaller repro (maybe related to many inputs in the witness?) of a failure I was seeing in
nargo executewith correct inputs.Repro:
./bootstrap.shin/noir-projects/noir-protocol-circuitsshould suffice, have removed the disallow warnings clause for visibility errors)noir-projects/noir-protocol-circuits/crates../../../noir/noir-repo/target/release/nargo execute --package rollup_block_rootTo get it to pass, change:
back to:
in
noir-projects/noir-protocol-circuits/crates/blob/Nargo.tomland re bootstrap. Themw/bumpbranch just addsprintlnsin bigcurve above the failing call: noir-lang/noir_bigcurve@tf/branch-for-aztec...mw/bumpEDIT: converted the
printlnstoassert_eqinmw/bumpto avoid witness parsing issues in aztec-packages.