refactor(aztec-nr): Option-returning block hash membership witness oracle#23179
Conversation
…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>
| ) -> MembershipWitness<NOTE_HASH_TREE_HEIGHT> {} | ||
|
|
||
| #[oracle(aztec_utl_getBlockHashMembershipWitness)] | ||
| #[oracle(aztec_utl_getBlockHashMembershipWitnessV2)] |
There was a problem hiding this comment.
Note that us not using the aztec_utl_getBlockHashMembershipWitness here means that the TS implementation is now basically a dead code and not tested but given that the code there is almost the same as aztec_utl_getBlockHashMembershipWitnessV2 and given that it's only a few simple lines I am not worried about regression there that would rug people.
| pub unconstrained fn get_maybe_block_hash_membership_witness( | ||
| anchor_block_header: BlockHeader, | ||
| block_hash: Field, | ||
| ) -> Option<MembershipWitness<ARCHIVE_HEIGHT>> { | ||
| let anchor_block_hash = anchor_block_header.hash(); | ||
| is_block_in_archive_oracle(anchor_block_hash, block_hash) | ||
| get_block_hash_membership_witness_oracle(anchor_block_hash, block_hash) | ||
| } |
There was a problem hiding this comment.
Decided to expose this function along with get_note_hash_membership_witness because I think for most of the use-cases people will just want to use get_note_hash_membership_witness. But if they want to handle the none case or just use this to check for existence (like oxide does) or to have a custom error message they can use this one.
| get_block_hash_membership_witness_oracle(anchor_block_hash, block_hash) | ||
| } | ||
|
|
||
| mod test { |
There was a problem hiding this comment.
This PR was a good opportunity to sneak in some tests. Created a separate issue for the get_note_hash_membership_witness function as sneaking tests of that in here is too unrelated.
mverzilli
left a comment
mverzilli
left a comment
There was a problem hiding this comment.
Looks good, just a nit on naming
2 participants
Summary
aztec_utl_isBlockInArchivewithaztec_utl_getBlockHashMembershipWitnessV2, which returnsOption<MembershipWitness<ARCHIVE_HEIGHT>>instead of a bool. Callers wanting existence-only can use.is_some().get_maybe_block_hash_membership_witnessNoir wrapper for callers that want to handle absence.get_block_hash_membership_witnessto call the V2 oracle and.expect()on the Option, producing a richer panic message that includes the block hash and anchor block hash.get_block_hash_membership_witness) are migrated.get_block_hash_membership_witnessandget_maybe_block_hash_membership_witness(success path and unknown-hash path).get_note_hash_membership_witnesswith a TODO pointing at F-652 for analogous test coverage.Motivation: discussion at #22979 (comment) —
aztec_utl_isBlockInArchiveonly 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 testinnoir-projects/aztec-nr/aztec— new tests inoracle::get_membership_witness::testpass (note: blocked locally by a pre-existing TXE bootstrap issue affecting allprivate_contexttests; expect CI to run them cleanly).yarn buildinyarn-project— PXE and TXE packages build cleanly.🤖 Generated with Claude Code