feat!: improve L2ToL1MessageWitness API#21231
Conversation
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Refactors `computeL2ToL1MembershipWitness` to take a `txHash` instead of requiring the caller to pass the epoch number. The epoch, checkpoint index, block index, and tx index are now resolved internally via the node. Also adds `epochNumber` to `L2ToL1MembershipWitness` type, adds `getCheckpointsDataForEpoch` to `AztecNode`, and supports explicit `messageIndexInTx` for disambiguating duplicate messages. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
| this.getProvenBlockNumber(), | ||
| this.getCheckpointedL2BlockNumber(), | ||
| this.getFinalizedL2BlockNumber(), | ||
| this.getBlock(blockNumber), |
There was a problem hiding this comment.
This call is expensive, since it deserializes all txs in a block. I'd prefer if we could instead store the slot along with the IndexedTxEffect, so we get it automatically when we do getTxEffect.
Alternatively, we can use getBlockDataFromBlockStorage(this.#blocks.getAsync(blockNumber)) to get just the block header instead of the entire block.
|
|
||
| const [messagesInEpoch, block, txEffect, checkpointsData] = await Promise.all([ | ||
| node.getL2ToL1Messages(epochNumber), | ||
| node.getBlock(blockNumber), |
There was a problem hiding this comment.
Let's get the block header only rather than the whole block
| const [messagesInEpoch, block, txEffect, checkpointsData] = await Promise.all([ | ||
| node.getL2ToL1Messages(epochNumber), | ||
| node.getBlock(blockNumber), | ||
| node.getTxEffect(txHash), | ||
| node.getCheckpointsDataForEpoch(epochNumber), | ||
| ]); |
There was a problem hiding this comment.
It feels like we're getting a ton of data from the node, when we actually need a lot less. I'd prefer if instead we stored the indices needed in the IndexedTxEffect. We are already storing the txIndexInBlock, we could just add the ones for block and checkpoint if needed.
Actually, why don't we just expose a method getL2ToL1MembershipWitness and perform the computeL2ToL1MembershipWitnessFromMessagesInEpoch on the node directly, so we don't need to send whole blocks and checkpoints down the wire? It'd also mimic the API we have for getL1ToL2MessageMembershipWitness.
There was a problem hiding this comment.
I considered that, but this seemed like a more disruptive change since we'd bake in a bunch of new behavior into the node. E.g. my handling of the optional msgIndexInTx feels a bit out of place there.
Fine with taking that approach if you think it's better though. If not, adding stuff to IndexedTxEffect seems like a good plan.
Do you think it'd be possible to tweak the private kernels so L2-to-L1 messages are unique, by hashing them with a counter or something? Not for alpha, of course. We made this change back in the day for L1-to-L2 messages and it made things so much easier. |
There was a problem hiding this comment.
Given the timelines, I think you're right in that it's better to keep the node changes to a minimum for now. Let's just make the small changes pointed out and ship it, and avoid bundling the complexity of computing the merkle tree on the fly on the node, or the database schema changes.
Let's also log an issue to revisit this later. We have several options, not all mutually exclusive:
- Computing the merkle tree on the node if it's cheap enough. We could also precompute it or cache it for future accesses.
- Add the missing indices (blockIndexWithinCheckpoint, etc) to the
IndexedTxEffectso we can rapidly identify the leaf of the tree we need. - Add a missing
indexWithinEpochto theCheckpointData, so we can combine that with the tx'sindexWithinBlockand the block'sindexWithinCheckpointto identify the leaf.
Apologies for the back and forth!
Avoids deserializing all transactions in the block when only the slot number is needed to compute the epoch. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ique-message-identifier-in
…ique-message-identifier-in
…ique-message-identifier-in
Co-authored-by: Santiago Palladino <[email protected]>
…ique-message-identifier-in
…ique-message-identifier-in
nventuro
deleted the
nico/f-388-feature-support-unique-message-identifier-in
branch
|
❌ Failed to cherry-pick to |
Cherry-pick of a8b2277 from next. Conflicts left as-is for review.
Cherry-pick of a8b2277 with conflicts preserved for review.
3 participants
The old API had two issues:
This changes things so that users no longer need to know the epoch, and instead query with
(message, txHash). If more than one message is found in the tx matchingmessage, we throw and request an additional optional parammessageIndexInTx. If this is passed, then we assert that the message at that index indeed matchesmessage.Most users should not need to pass the index, but it is there in case it is needed. It is expected that apps would know the messages they're interested in, and can look at indices from the tx effects.
Doing this requires fetching the epoch as well as checkpoint, block and tx indices from the node. There was no API for checkpoint data, so I exposed
getCheckpointsDataForEpochso that we can find the checkpoint's index in the epoch. I also added the epoch number to the tx receipt.I used Claude heavily here as I don't really know my way around the node and archiver code, but I think the result makes sense.
Closes #20874