feat!: improve L2ToL1MessageWitness API#21231
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| 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 <noreply@anthropic.com>
…ique-message-identifier-in
…ique-message-identifier-in
…ique-message-identifier-in
Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com>
…ique-message-identifier-in
…ique-message-identifier-in
|
❌ 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.
## Summary PR #21231 changed the `computeL2ToL1MembershipWitness` API signature from `(node, epoch, message)` to `(node, message, txHash)`, but the `example_swap` docs example (added in PR #20233) was written against the old API. This caused the merge-train CI to fail with `TypeError: Cannot read properties of undefined (reading 'siblingPath')` because the function received wrong arguments and returned `undefined`. ## Changes - Removed `RollupContract` import and usage (no longer needed to manually compute epoch) - Updated both `computeL2ToL1MembershipWitness` calls to use new signature: `(node, message, txHash)` - Get epoch numbers from the returned witness (`witness.epochNumber`) instead of computing manually ClaudeBox log: https://claudebox.work/s/5eb93c1235ed3384?run=2 Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com>
BEGIN_COMMIT_OVERRIDE fix: skip oracle version check for pinned protocol contracts (#21349) fix: not reusing tags of partially reverted txs (#20817) feat: move storage_slot from partial commitment to completion hash (#21351) feat: offchain reception (#20893) fix: handle workspace members in needsRecompile crate collection (#21284) fix(aztec-nr): return Option from decode functions and fix event commitment capacity (#21264) fix: handle bad note lengths on compute_note_hash_and_nullifier (#21271) fix: address review feedback from PRs #21284 and #21237 (#21369) fix: claim contract & improve nullif docs (#21234) feat!: auto-enqueue public init nullifier for contracts with public functions (#20775) fix: search for all note nonces instead of just the one for the note index (#21438) fix: set anvilSlotsInAnEpoch in e2e_offchain_payment to prevent finalization race (#21452) fix: complete legacy oracle mappings for all pinned contracts (#21404) fix: correct inverted constrained encryption check in message delivery (#21399) feat!: improve L2ToL1MessageWitness API (#21231) END_COMMIT_OVERRIDE
…21463) ## Summary Backport of #21231 to `v4`. Cherry-pick of merge commit `a8b2277a50` with two conflicts resolved: - `docs/docs-developers/docs/resources/migration_notes.md` — accepted incoming migration notes entries - `yarn-project/archiver/src/store/block_store.ts` — included `getEpochAtSlot` import ### Commit structure 1. **Cherry-pick commit** — raw cherry-pick with conflict markers preserved for review 2. **Conflict resolution commit** — resolved the two conflicts above No additional build fixes were needed — the code compiles cleanly on v4. ClaudeBox log: https://claudebox.work/s/8bd8c8724c80cbb2?run=1
BEGIN_COMMIT_OVERRIDE fix: orchestrator enqueue yield (#21286) chore: default multiplier (#21482) chore: update yarn.lock (#21479) chore: backport #21231 feat!: improve L2ToL1MessageWitness API to v4 (#21463) feat(rpc): add package version to RPC response headers (#21526) feat(ethereum): check VK tree root and protocol contracts hash in rollup compatibility (#21537) feat: add public log filtering by tag (#21561) END_COMMIT_OVERRIDE
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