feat: WIP integrate batched blobs into l1 contracts + ts#14329
feat: WIP integrate batched blobs into l1 contracts + ts#14329MirandaWood merged 141 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
… mw/blob-batching-bls-utils-ts
…nto mw/blob-batching-integration
…to mw/blob-batching-integration-ts-sol
…to mw/blob-batching-integration-ts-sol
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)
…atching-integration
…to mw/blob-batching-integration-ts-sol
…atching-integration
…to mw/blob-batching-integration-ts-sol
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>
| * input[1:] - blob commitments (48 bytes * num blobs in block) | ||
| * @param _blobsInput - The above bytes to verify our input blob commitments match real blobs | ||
| * @param _checkBlob - Whether to skip blob related checks. Hardcoded to true (See RollupCore.sol -> checkBlob), exists only to be overriden in tests. | ||
| */ |
There was a problem hiding this comment.
Please add @return for the different values here to make it a bit simpler to follow.
There was a problem hiding this comment.
Also probably good to add an assumption about there not being any non-aztec blobs in the tx.
There was a problem hiding this comment.
Would it be worth adding a loop from numBlobs to whatever the max is and ensuring the blobHash is 0 for those?
| // to one another. | ||
| mapping(address prover => mapping(Epoch epoch => bool claimed)) proverClaimed; | ||
| RollupConfig config; | ||
| // TODO(#14646): We only ever need to store AZTEC_MAX_EPOCH_DURATION values below => make fixed length and overwrite once we start a new epoch |
There was a problem hiding this comment.
I'm guess the AZTEC_MAX_EPOCH_DURATION is something that is here to keep some flexibility for the circuits to not necessarily follow the actual epoch duration.
However, it is not sufficient to just store for one epoch, as prune can remove more than that, I think that the value you are probably interested in is the submission window.
Regardless, something that could be potentially of interest (not now I would expect) is keeping a number of running hashes in there instead. Essentially allowing you are the time of proof submission to just read one storage variable instead of one for each block.
Separately, why is this value in here, and not part of the BlockLog it seems more convenient if we keep them close (BlockLog) as it is much clearer when doing optimisations what can be grouped. For example storing a hash of the blocklog content and storing that reducing to a single store etc, and just simpler to figure out then.
There was a problem hiding this comment.
as prune can remove more than that, I think that the value you are probably interested in is the submission window.
Ah ok. I didn't realise this. This was the reason why the value is here and not BlockLog - I wanted it to be separate so it would be possible to change later to a fixed length thing and just re use the slots once an epoch is proven. But that was based on a wrong assumption, so I'll add the value to BlockLog for now.
Regardless, something that could be potentially of interest (not now I would expect) is keeping a number of running hashes in there instead. Essentially allowing you are the time of proof submission to just read one storage variable instead of one for each block.
I had assumed this wouldn't work because of pruning, but thinking about it now this would work. We only need the 'current' value of the epoch, then iterate and update that value, unless it's the first block of an epoch where we initialise the value. I'll add an issue for this, thanks!
There was a problem hiding this comment.
Fixed here: 32bf652
(Didn't have time to test yet, may fail)
| blobInputStart += Constants.BLS12_POINT_COMPRESSED_BYTES; | ||
|
|
||
| // TODO(#14646): Use kzg_to_versioned_hash & VERSIONED_HASH_VERSION_KZG | ||
| // Using bytes32 array to force bytes into memory |
There was a problem hiding this comment.
Why you want to force it into memory vs on stack here?
Personally, I think it is slightly simpler to just read as some binary operations on the stack, e.g., as seen below. When seeing anything with arrays and memory, my mind will first be thinking "oh is it updating the length of the array" as that is what would have happened if it was not a fixed size.
bytes32 blobHashCheck = bytes32(
(
uint256(sha256(blobCommitments[i]))
& 0x00FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
) | 0x0100000000000000000000000000000000000000000000000000000000000000
);There was a problem hiding this comment.
Purely because I thought it was a little clearer to see the 'overwriting' happen with mstore but you raise a good point. I will use your suggestion, thanks
## 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>
Finalises integration of batched blobs
mw/blob-batching-integrationadds 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:propose, instead injects the blob commitments, check they correspond to the broadcast blobs, and stores them in theblobCommitmentsHashblobCommitmentsHash(no longer required)submitEpochRootProoffor ALL blobs in the epochblobCommitmentsHashto link the circuit batched blob, real L1 blobs, and the batched blob verified on L1orchestratorsubmitEpochRootProofonfinaliseEpoch()TODOs/Current issues
When using real proofs (e.g.EDIT: solved - must include theyarn-project/prover-client/src/test/bb_prover_full_rollup.test.ts), the root rollup proof is generated correctly but fails verification checks inbbdue to incorrect number of public inputs. Changing the number correctly updates vks and all constants elsewhere, butbbdoes not change.is_infpoint member for now (see below TODO)TheEDIT: temporarily fixed - details in this repro (Bigcurve/Bignum nargo execute failure repro #14381) and noir issue (Prover.tomlfor block-root is not executing. The error manifests in the same way as that in fix: fixassert_split_transformed_value_arraysconditional access index underflow #12540 (but may be different).printlnstatement changes execution result of program noir-lang/noir#8563).is_infflag) 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 featuremw/blob-batching-bls-utils<- BLS12-381 bigcurve and bignum utils (noir) (feat: blob batching methods #13583)mw/blob-batching-bls-utils-ts<- BLS12-381 bigcurve and bignum utils (ts) (feat: blob batching methods (ts) #13606)mw/blob-batching-integration<- Integrate batching into noir protocol circuits (feat: WIP batch blobs and validate in rollup #13817)mw/blob-batching-integration-ts-sol<- Integrate batching into ts and solidity (feat: WIP integrate batched blobs into l1 contracts + ts #14329)