feat: blob batching methods#13583
Conversation
…atching-bls-utils
|
|
||
| pub struct BatchingBlobCommitment { | ||
| pub point: BLSPoint, | ||
| pub compressed: [u8; 48], // TODO(MW): get Q::num_bytes from somewhere, rather than hardcoded 48 |
There was a problem hiding this comment.
I can't find a way to retrieve num bytes for a specific field as defined in bignum - am I missing something? (I don't love having a hardcoded 48 here)
There was a problem hiding this comment.
I can't see a way either.
The hard-coded values for this bignum/bigcurve are 381, and 4 limbs of u128. Bigcurve doesn't concern itself with compression standards, so 48 doesn't appear anywhere. Perhaps we just create a constant COMPRESSED_BLS12_381_BYTES = 48 in constants.nr?
There was a problem hiding this comment.
Sounds good! I'll add this
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Outdated
Show resolved
Hide resolved
…atching-bls-utils
…atching-bls-utils
…atching-bls-utils
…atching-bls-utils
| pub global BLOCK_ROOT_OR_BLOCK_MERGE_PUBLIC_INPUTS_LENGTH: u32 = APPEND_ONLY_TREE_SNAPSHOT_LENGTH | ||
| + APPEND_ONLY_TREE_SNAPSHOT_LENGTH |
There was a problem hiding this comment.
This is an unrelated fix - constants.in.ts incorrectly parsed 2 \n * APPEND_ONLY_TREE_SNAPSHOT_LENGTH as 2 + ... (+ instead of *), discovered when editing BLOCK_ROOT_OR_BLOCK_MERGE_PUBLIC_INPUTS_LENGTH
There was a problem hiding this comment.
I opened an issue, because that sounds like a scary bug #14550
| let c_i = compress_to_blob_commitment(kzg_commitments_points[i]); | ||
| let (z_i, y_i) = | ||
| evaluate_blob_for_batching(single_blob_fields, c_i, hashed_blobs_fields, challenge_z); | ||
| if !(y_i.is_zero()) & !(single_blob_fields[0] == 0) { |
There was a problem hiding this comment.
(single_blob_fields[0] != 0) might be easier to read?
There was a problem hiding this comment.
Why do we skip the accumulation when the first field is 0? Isn't it possible that the first field can be 0 and followed by non-zero values? Like when the offset falls on the log data that is 0.
There was a problem hiding this comment.
This was copied over from the old blob methods which I think are now outdated (when I started on batching IIRC we did not include 0s at all?) - good catch! I will edit this, I think y_i being 0 may be sufficient to ensure we have an empty blob but I'll think on it
There was a problem hiding this comment.
I'm actually not sure whether y_i being 0 is enough to say the blob is empty - for example it could be possible for a blob to have two items which end up having coefficients in the polynomial which are negatives of each other. I'm not sure it's even possible for a user to actually inject many blob fields since they are mostly hashes, but to be on the safe side I added a check on the number of fields in the sponge.
Unfortunately it is a range check, so relatively expensive! However if it is required, we could in future reduce the number of gates by commandeering the existing check in check_block_blob_sponge's hash that we haven't added more blob elements than we claim. If during that check we pass i * FIELDS_PER_BLOB and are above the number of fields in the blob, we could return a flag to say 'blob i is empty'. But I'd rather avoid adding that if y_i being 0 is sufficient then ending up having to remove it!
(Sorry for the essay!!)
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching.nr
Outdated
Show resolved
Hide resolved
iAmMichaelConnor
left a comment
There was a problem hiding this comment.
Only a couple of small suggestions that would be nice to include in this PR.
Then I'm keeping a list of other minor issues that can be tackled after we've merged the big PRs, here: #14646
| pub global BLOCK_ROOT_OR_BLOCK_MERGE_PUBLIC_INPUTS_LENGTH: u32 = APPEND_ONLY_TREE_SNAPSHOT_LENGTH | ||
| + APPEND_ONLY_TREE_SNAPSHOT_LENGTH |
There was a problem hiding this comment.
I opened an issue, because that sounds like a scary bug #14550
|
|
||
| pub struct BatchingBlobCommitment { | ||
| pub point: BLSPoint, | ||
| pub compressed: [u8; 48], // TODO(MW): get Q::num_bytes from somewhere, rather than hardcoded 48 |
There was a problem hiding this comment.
I can't see a way either.
The hard-coded values for this bignum/bigcurve are 381, and 4 limbs of u128. Bigcurve doesn't concern itself with compression standards, so 48 doesn't appear anywhere. Perhaps we just create a constant COMPRESSED_BLS12_381_BYTES = 48 in constants.nr?
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Outdated
Show resolved
Hide resolved
| gamma: hashed_y_0, | ||
| gamma_pow: final_gamma, |
There was a problem hiding this comment.
Example of naming that would help readability:
| gamma: hashed_y_0, | |
| gamma_pow: final_gamma, | |
| gamma_acc: hashed_y_0, | |
| gamma_pow_acc: gamma, |
There was a problem hiding this comment.
As above, I did have it like this! I do think final_gamma is clearer than gamma_acc and gamma if I'm honest, what do you think?
There was a problem hiding this comment.
My brain's just used to to the original write-up of gamma always meaning the final gamma, and _acc meaning "probably not the final thing". But I can adapt.
noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching_public_inputs.nr
Outdated
Show resolved
Hide resolved
iAmMichaelConnor
left a comment
There was a problem hiding this comment.
Approved, given the latest commit which addresses some comments, and #14646 which tracks other small tasks that can be done afterwards.
Ts only blob batching methods plus tests. Points to the parent methods PR: #13583. TODOs (Marked in files as `TODO(MW)`): - [ ] Remove the large trusted setup file? Not sure if it's required, but it is currently the only way I show in tests that our BLS12 methods match those in c-kzg. - [x] Add nr fixture where we can use `updateInlineTestData` for point compression. Other TODOs must wait until we actually integrate batching, otherwise I will break the repo. NB: The files `bls12_fields.ts` and `bls12_point.ts` and their tests are essentially copies of `./fields.ts` and `./point.ts`. When reviewing please keep that in mind and double check the original file if you see an issue before commenting (@iAmMichaelConnor ;) ). --- ## PR Stack - [ ] `mw/blob-batching` <- main feature - [ ] ^ `mw/blob-batching-bls-utils` <- BLS12-381 bigcurve and bignum utils (noir) (#13583) - [x] ^ `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) - [ ] ^ `mw/blob-batching-integration-ts-sol` <- Integrate batching into ts and solidity (#14329)
WIP TODOs - [ ] Compress BLS12 fq and fr values to fewer native fields to reduce number of public inputs (somewhat blocked by #13608 since that dictates how large bls12fr value gamma is) - [ ] Delete old `blob.nr` files and remove `pub`s w/o batching (will do this later so it's easier to review) - [x] Rework `RootRollupPublicInputs` so it doesn't contain unnecessary values not needed for L1 verification --- ## 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) - [x] ^ `mw/blob-batching-integration` <- Integrate batching into noir protocol circuits (#13817) - [ ] ^ `mw/blob-batching-integration-ts-sol` <- Integrate batching into ts and solidity (#14329) --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
## 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>
## The blobs are back in town. This PR reworks blobs so that instead of calling the point evaluation precompile for each blob (currently up to 3 per block => up to 96 (?) calls per epoch), we call it once per epoch by batching blobs to a single kzg commitment, opening, challenge, and proof. How we can be sure that this one pairing check is equivalent to a check per blob is covered in the maths by @iAmMichaelConnor [here](https://hackmd.io/WUtNusQxS5KAw-af3gxycA?view) 🎉 ## Overview Instead of pushing to a long array of `BlobPublicInputs`, which are then individually checked on L1, we batch each blob together to a single set of `BlobAccumulatorPublicInputs`. The `start` accumulator state is fed into each block root circuit, where the block's blobs are accumulated and the `end` state is set. Each block merge circuit checks that the state follows on correctly and, finally, the root circuit checks that the very `start` state was empty and finalises the last `end` state. This last `end` state makes up the set of inputs for the point evaluation precompile. If the pairing check in that precompile passes, we know that all blobs for all blocks in the epoch are valid and contain only the tx effects validated by the rollup. ### Circuits Key changes: - Integrate BLS12-381 curve operations with `bignum` and `bigcurve` libraries, plus tests. - Rework the `blob` package to batch blobs and store in reworked structs, plus tests. - Rework the rollup circuits from `block_root` above to handle blob accumulation state rather than a list of individual blob inputs, plus (you guessed it) tests. ### Contracts The contracts: - No longer call the point evaluation precompile on `propose`, instead inject the blob commitments, check they correspond to the broadcast blobs, and stores them in the `blobCommitmentsHash`. - Do not store any blob public inputs apart from the `blobCommitmentsHash`. - Call the point evaluation precompile once on `submitEpochRootProof` for ALL blobs in the epoch. - Use the same precompile inputs as pubic inputs to the root proof verification along with the `blobCommitmentsHash` to link the circuit batched blob, real L1 blobs, and the batched blob verified on L1. ### TypeScript Key changes: - Edit all the structs and methods reliant on the circuits/contracts to match the above changes. - Inject the final blob challenges used on each blob into all block building methods in `orchestrator`. - Accumulate blobs in ts when building blocks and use as inputs to each rollup circuit, plus tests. - Return the blob inputs required for `submitEpochRootProof` on `finaliseEpoch()`. ### TODOs/Related Issues - Choose field for hashing challenge: #13608 - Instead of exponentiating `gamma` (expensive!), hash it for each iteration: #13740 - Number of public inputs: 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. - Remove the large trusted setup file from `yarn-project/blob-lib/src/trusted_setup_bit_reversed.json`? Used in testing, but may not be worth keeping (see code comments). - Cleanup old, unused blob stuff in #14637. ## PR Stack - [x] `mw/blob-batching` <- main feature - [x] ^ `mw/blob-batching-bls-utils` <- BLS12-381 bigcurve and bignum utils (noir) (#13583) - [x] ^ `mw/blob-batching-bls-utils-ts` <- BLS12-381 bigcurve and bignum utils (ts) (#13606) - [x] ^ `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) - [ ] ^ `mw/blob-batching-cleanup` <- Remove old blob code --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
## The blobs are back in town. This PR reworks blobs so that instead of calling the point evaluation precompile for each blob (currently up to 3 per block => up to 96 (?) calls per epoch), we call it once per epoch by batching blobs to a single kzg commitment, opening, challenge, and proof. How we can be sure that this one pairing check is equivalent to a check per blob is covered in the maths by @iAmMichaelConnor [here](https://hackmd.io/WUtNusQxS5KAw-af3gxycA?view) 🎉 ## Overview Instead of pushing to a long array of `BlobPublicInputs`, which are then individually checked on L1, we batch each blob together to a single set of `BlobAccumulatorPublicInputs`. The `start` accumulator state is fed into each block root circuit, where the block's blobs are accumulated and the `end` state is set. Each block merge circuit checks that the state follows on correctly and, finally, the root circuit checks that the very `start` state was empty and finalises the last `end` state. This last `end` state makes up the set of inputs for the point evaluation precompile. If the pairing check in that precompile passes, we know that all blobs for all blocks in the epoch are valid and contain only the tx effects validated by the rollup. ### Circuits Key changes: - Integrate BLS12-381 curve operations with `bignum` and `bigcurve` libraries, plus tests. - Rework the `blob` package to batch blobs and store in reworked structs, plus tests. - Rework the rollup circuits from `block_root` above to handle blob accumulation state rather than a list of individual blob inputs, plus (you guessed it) tests. ### Contracts The contracts: - No longer call the point evaluation precompile on `propose`, instead inject the blob commitments, check they correspond to the broadcast blobs, and stores them in the `blobCommitmentsHash`. - Do not store any blob public inputs apart from the `blobCommitmentsHash`. - Call the point evaluation precompile once on `submitEpochRootProof` for ALL blobs in the epoch. - Use the same precompile inputs as pubic inputs to the root proof verification along with the `blobCommitmentsHash` to link the circuit batched blob, real L1 blobs, and the batched blob verified on L1. ### TypeScript Key changes: - Edit all the structs and methods reliant on the circuits/contracts to match the above changes. - Inject the final blob challenges used on each blob into all block building methods in `orchestrator`. - Accumulate blobs in ts when building blocks and use as inputs to each rollup circuit, plus tests. - Return the blob inputs required for `submitEpochRootProof` on `finaliseEpoch()`. ### TODOs/Related Issues - Choose field for hashing challenge: AztecProtocol#13608 - Instead of exponentiating `gamma` (expensive!), hash it for each iteration: AztecProtocol#13740 - Number of public inputs: 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. - Remove the large trusted setup file from `yarn-project/blob-lib/src/trusted_setup_bit_reversed.json`? Used in testing, but may not be worth keeping (see code comments). - Cleanup old, unused blob stuff in AztecProtocol#14637. ## PR Stack - [x] `mw/blob-batching` <- main feature - [x] ^ `mw/blob-batching-bls-utils` <- BLS12-381 bigcurve and bignum utils (noir) (AztecProtocol#13583) - [x] ^ `mw/blob-batching-bls-utils-ts` <- BLS12-381 bigcurve and bignum utils (ts) (AztecProtocol#13606) - [x] ^ `mw/blob-batching-integration` <- Integrate batching into noir protocol circuits (AztecProtocol#13817) - [x] ^ `mw/blob-batching-integration-ts-sol` <- Integrate batching into ts and solidity (AztecProtocol#14329) - [ ] ^ `mw/blob-batching-cleanup` <- Remove old blob code --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Note: merging ts methods into this branch because many nr tests use the same inputs and process as the ts versions. It's useful (to me!) to have them side by side in the same branch.
Hopefully it's not too difficult to review - the new unreviewed files in this PR are .nr code only, ignore all .ts. If that's not useful, I can recommit the ts files to
mw/blob-batching, leaving only the nr ones here.TODOs:
noir-protocol-circuitsto bignum 0.7.0 (blocked by bignum/bigcurve)mw/bumptemporarily resolves thesegamma(the 'challenge' for a random linear combination on BLS12 elts) - feat(blobs): Finalise hash method for challenge gamma (used in blob batching) #13608 (not blocked, but requires some cryptography thinking)finalizeas a separate fn once accumulators are complete (the only thing finalize actually does is hash the finalgamma_accwithz).TODOs which can only be completed once batching is integrated:
pubs all over the placeBlobCommitmenttype and replace entirely with the properties ofBatchingBlobCommitmentPR Stack
mw/blob-batching<- main featuremw/blob-batching-bls-utils<- BLS12-381 bigcurve and bignum utils (noir)mw/blob-batching-bls-utils-ts<- BLS12-381 bigcurve and bignum utils (ts)mw/blob-batching-integration<- Integrate batching into noir protocol circuitsmw/blob-batching-integration-ts-sol<- Integrate batching into ts and solidity