feat: 1 blob -> 3 blobs #10014
feat: 1 blob -> 3 blobs #10014
Conversation
There was a problem hiding this comment.
DELIGHTFUL! Thanks so much for this.
I've only properly checked the smart contracts and circuits. I've added some comments.
It's quite elegant, really. There's no extra complex stuff to handle info that spans multiple blobs, because it's all shoved into one continuous sponge.
l1-contracts/src/core/Rollup.sol
Outdated
|
|
||
| // protocol_contract_tree_root | ||
| publicInputs[feesEnd + 1] = protocolContractTreeRoot; | ||
| publicInputs[offset++] = protocolContractTreeRoot; |
There was a problem hiding this comment.
I have a feeling this might break. I faintly recall that in Solidity, offset++ will return the old value of offset, and then increment, which (comparing against what was previously here: feesEnd + 1) is not what we want, here.
Either ++offset, or (because I think some people don't like ++ operators because of their bad readability) increment it on a separate line with offset += 1;
There was a problem hiding this comment.
Whoops - I just forgot to add ++ to the previous use of offset. I'm a bit confused why all the verification tests I ran passed then, so I'll take a look.
There was a problem hiding this comment.
Ohhh, good point. If you discover why they're passing, we should add a test to catch whatever's been missed!
l1-contracts/src/core/Rollup.sol
Outdated
|
|
||
| // prover_id: id of current epoch's prover | ||
| publicInputs[feesEnd + 2] = _args[6]; | ||
| publicInputs[offset++] = _args[6]; |
| { | ||
| bytes32 calcBlobPublicInputsHash = sha256( | ||
| abi.encodePacked( | ||
| _blobPublicInputsAndAggregationObject[blobOffset:blobOffset + 112 * blobsInBlock] |
There was a problem hiding this comment.
Magic number 112 should eventually be a constant (although I appreciate the code you're making changes to already included lots of magic numbers that should also be constants, so maybe make it a todo). Maybe also a comment to say: 112 = |z| + |y| + |commitment|, where commitment encoded as 31 + 17 bytes.
noir-projects/noir-protocol-circuits/crates/blob/src/blob_public_inputs.nr
Show resolved
Hide resolved
| blob_hash: Field, | ||
| blob_commitments: [[Field; 2]; BLOBS_PER_BLOCK], | ||
| // Flat sha256 hash of the EVM blob hashes, can be injected here as the contract check its validity vs the blob_public_inputs below | ||
| // NB: to fit it into a field, we truncate to 31 bytes |
There was a problem hiding this comment.
Todo: We should have a think about what happens if Ethereum updates blobs, to introduce a new "version" of the versioned hash. Suppose we end up in a world with more than one value for a "first byte" is possible for a versioned-blob-hash. And suppose Aztec can't update at the exact same pace as Ethereum. Are there any attacks that can be done, if we're omitting this first byte?
There was a problem hiding this comment.
From only a quick think, I believe the answer is no. Simply because our repo-coded version = 0x01, so if the version updates, ours will be out of sync, and anything going on-chain will fail. Only the circuits deal with the truncated version of the hash, and they 'don't care' what the version is, just to have a number to hash into the header.
Since this 'version' number is one constant on the EVM, i don't think they will ever allow two versions to be running at once
This comment was marked as off-topic.
This comment was marked as off-topic.
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
This comment was marked as off-topic.
This comment was marked as off-topic.
Made a separate PR just because there are a lot of changes.
(pls ignore the commits from merging with master!)