feat: upstream oxide aztec-nr changes#22979
Conversation
| pub fn encode_private_note_message<Note>( | ||
| /// Shared encoder used by [`encode_private_note_message`] and by custom message handlers that use the same format as | ||
| /// standard private note message but perform additional validation logic. | ||
| pub fn encode_private_note_message_with_msg_type_id<Note>( |
There was a problem hiding this comment.
This is admittedly a bit messy and I am not sure it will survive long-term but I exposed this because in oxide there we have a custom message handler that triggers the standard note message processing pipeline while performing additional logic.
This means that there we have a message encoded exactly the same way as PRIVATE_NOTE_MSG_TYPE_ID message but it travels here via the custom message handler.
Maybe with time we will want to create some standard for a custom message handler that triggers default aztec-nr message handlers but it's too soon for that so think this "messyness" is justified for now.
There was a problem hiding this comment.
This makes sense 👍. The only thing that I find a bit strange now is that this source file lives under messages/logs.
I don't see the connection with logs, this all seems to be purely about putting notes inside messages
There was a problem hiding this comment.
This being under logs is a pure tech debt from times where the separation between log and messages didn't really exist.
I remember Nico was doing some restructurings like a year ago.
@nventuro was this just forgotten?
benesjan
added
claudebox
benesjan
requested review from
mverzilli
and removed request for
LeilaWang and
Thunkar
| if (txHash.hash.isZero()) { | ||
| throw new Error('Invalid tx hash passed into aztec_utl_getTxEffect oracle handler'); | ||
| } | ||
|
|
||
| const txEffect = await this.aztecNode.getTxEffect(txHash); | ||
| if (!txEffect || txEffect.l2BlockNumber > this.anchorBlockHeader.getBlockNumber()) { | ||
| return null; | ||
| } | ||
|
|
||
| return txEffect.data; |
There was a problem hiding this comment.
I don't have context on oxide, so maybe dumb questions, but:
- how often will this oracle be invoked?
- how often will this oracle be invoked trying to read the same tx effect?
considering we've been trying to get a bit more economical about rpc's, should we consider caching right from the get go? (probably easier to do now than waiting for Grego to get grumpy and having to try to detect these throughout the codebase :P)
There was a problem hiding this comment.
The oracle is only invoked here and it's triggered only when a note is being delivered. So I would say it's fine as I would not expect users to have that many incoming notes (zk.money is basically a payments app).
Anyway, summoning @Thunkar to give him a chance to contribute some grumpiness to this discussion.
Update: Fixed the link
There was a problem hiding this comment.
I think the link is broken. This is 100% going to become an issue for zkmoney, but we can defer optimizations provided this doesn't affect our regular sync flow.
Caching txeffects could be problematic, since we can only be sure of "dropped" status (pending can change at any moment and success can be rekt by a reorg)
There was a problem hiding this comment.
Caching txeffects could be problematic, since we can only be sure of "dropped" status
Oxide doesn't use the tx status in any way but somehow reflecting that in the API is probably not feasible.
It sucks that the message processing is serial here as this gets triggered in a loop by sync_state.
(so indeed might turn out to be a problem eventually for people receiving a lot of payments)
mverzilli
left a comment
mverzilli
left a comment
There was a problem hiding this comment.
Nice! Thanks for the in-PR comments, they really provided context that made reviewing more straightforward
…s oracle - Replace `aztec_utl_isBlockInArchive` with `aztec_utl_getBlockHashMembershipWitnessV2`, which returns `Option<MembershipWitness<ARCHIVE_HEIGHT>>`. Callers wanting existence-only can use `.is_some()`. - Expose `get_maybe_block_hash_membership_witness` as a Noir wrapper around the V2 oracle. - Refactor `get_block_hash_membership_witness` to call the V2 oracle and `.expect()` the Option, surfacing a richer error message that includes the block hash and anchor block hash. - Update TXE and PXE handlers accordingly. The legacy V1 TS handlers are kept for now (TODO links to F-651 for cleanup). - Add `mod test` covering both V1 and V2 wrappers (success path and unknown-hash path). - Flag `get_note_hash_membership_witness` for follow-up tests (F-652). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…acle (#23179) ## Summary - Replaces `aztec_utl_isBlockInArchive` with `aztec_utl_getBlockHashMembershipWitnessV2`, which returns `Option<MembershipWitness<ARCHIVE_HEIGHT>>` instead of a bool. Callers wanting existence-only can use `.is_some()`. - Exposes a new `get_maybe_block_hash_membership_witness` Noir wrapper for callers that want to handle absence. - Refactors `get_block_hash_membership_witness` to call the V2 oracle and `.expect()` on the Option, producing a richer panic message that includes the block hash and anchor block hash. - TXE and PXE handlers updated accordingly. The legacy V1 TS handlers are kept for now and marked with a TODO linking [F-651](https://linear.app/aztec-labs/issue/F-651/replace-aztec-utl-getblockhashmembershipwitness-with-aztec-utl) for follow-up cleanup once Noir-side callers (currently still pointing at the V1 oracle through `get_block_hash_membership_witness`) are migrated. - Adds direct in-tree Noir tests for both `get_block_hash_membership_witness` and `get_maybe_block_hash_membership_witness` (success path and unknown-hash path). - Flags `get_note_hash_membership_witness` with a TODO pointing at [F-652](https://linear.app/aztec-labs/issue/F-652/implement-noir-tests-of-aztec-utl-getnotehashmembershipwitness) for analogous test coverage. Motivation: discussion at #22979 (comment) — `aztec_utl_isBlockInArchive` only existed because the witness oracle panicked on miss; the Option-returning V2 collapses that surface and lets callers handle absence in Noir. ## Test plan - [ ] `nargo test` in `noir-projects/aztec-nr/aztec` — new tests in `oracle::get_membership_witness::test` pass (note: blocked locally by a pre-existing TXE bootstrap issue affecting all `private_context` tests; expect CI to run them cleanly). - [ ] `yarn build` in `yarn-project` — PXE and TXE packages build cleanly. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This PR accumulates backport commits throughout the day and will be auto-merged overnight. Latest backport: #22979 - feat: upstream oxide aztec-nr changes 🤖 This PR is managed automatically by the backport workflow.
Forward port of #22979 (merged into `backport-to-v4-next-staging`) onto `merge-train/fairies`. --------- Co-authored-by: sirasistant <sirasistant@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 participants
Does the following changes to Aztec.nr to make https://github.com/aztec-labs-eng/oxide possible:
aztec_utl_getTxEffectoracle - needed by oxide's custom message handler that implements custom TEE-related validation logic,aztec_utl_isBlockInArchiveoracle - once again needed to verify that the TEE output is legit,PRIVATE_NOTE_MSG_TYPE_ID) but it layers the TEE-related validation on top. This means that most of the functionality in Aztec.nr related to PRIVATE_NOTE_MSG_TYPE_ID` was valuable there as well.Resolves GK-563.
Originally implemented by @sirasistant in
arv/oxide_infra_depsto make https://github.com/aztec-labs-eng/oxide possible. I just polished some stuff and implemented tests.