fix: handle bad note lengths on compute_note_hash_and_nullifier by nchamo · Pull Request #21271 · AztecProtocol/aztec-packages

nchamo · 2026-03-09T19:57:28Z

Summary

We are now being safe on _compute_note_hash_and_nullifier by returning Option::none() instead of panicking when the length doesn't match the expected value

Fixes F-362

… mismatch

nchamo · 2026-03-09T20:02:38Z

noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr

-///         let note = MyNoteType::unpack(aztec::utils::array::subarray(packed_note.storage(), 0));
+///             let note_hash = note.compute_note_hash(owner, storage_slot, randomness);
+///             let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification(
+///                 HintedNote {


Updated the comment since now storage_slot goes inside HintedNote

nchamo · 2026-03-09T20:02:52Z

noir-projects/aztec-nr/aztec/src/macros/aztec.nr

@@ -298,7 +314,7 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -
            ///
            /// This function is automatically injected by the `#[aztec]` macro.
            #[contract_library_method]
-            unconstrained fn _compute_note_hash_and_nullifier(
+            pub(crate) unconstrained fn _compute_note_hash_and_nullifier(


Made pub(crate) so we could test it

Given that the tests are already in a contract crate this can be avoided by just having a simple wrapper function in the test contract that is exposed and just calls this.

Would say that is preferrable.

Agree, that would be much better 🫡

benesjan

@nchamo are you aware that we would still error out during actually note processing in case of a mismatch here?

Given that I expect the change there to not be big think it makes sense to tackle that as part of this PR as well even though the original issue does not explicitly mention it.

benesjan · 2026-03-10T03:32:32Z

noir-projects/aztec-nr/aztec/src/macros/aztec.nr

@@ -298,7 +314,7 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -
            ///
            /// This function is automatically injected by the `#[aztec]` macro.
            #[contract_library_method]
-            unconstrained fn _compute_note_hash_and_nullifier(
+            pub(crate) unconstrained fn _compute_note_hash_and_nullifier(


Given that the tests are already in a contract crate this can be avoided by just having a simple wrapper function in the test contract that is exposed and just calls this.

Would say that is preferrable.

benesjan · 2026-03-10T03:37:52Z

...ects/noir-contracts/contracts/test/test_contract/src/test/compute_note_hash_and_nullifier.nr

@@ -0,0 +1,48 @@
+use crate::{Test, test_note::TestNote};


Test contract is an old pile of mess so ideally would not add to that and just create a new test contract for this.

nchamo · 2026-03-10T10:34:02Z

@benesjan , I'm aware we still need to handle that scenario, but we already have F-344 for that. I just wanted to make each PR as small as possible, specially if we already had another task for it

…test packages to workspace

…lifier

benesjan

LGTM

benesjan · 2026-03-11T02:30:20Z

...ontracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test.nr

+
+    // TestNote::compute_nullifier_unconstrained returns a hardcoded Some(2) so we can verify the
+    // macro-generated function propagates the inner nullifier correctly.
+    assert_eq(note_hash_and_nullifier.inner_nullifier.unwrap(), 2);


Even though you have it explained here anyway think it would be better to define a TEST_NOTE_NULLIFIER in test_note.nr and use it here. Then you can easily navigate with LSP to where the value originates.

benesjan · 2026-03-11T02:31:18Z

...ontracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test.nr

+#[test]
+unconstrained fn returns_none_for_bad_note_length() {
+    // TestNote has Packable N=1, but we provide 2 fields
+    let packed_note = BoundedVec::from_array([42, 99]);


Would also add a test case for empty BVec.

AztecBot · 2026-03-11T13:37:23Z

❌ Failed to cherry-pick to v4-next due to conflicts. (🤖) View backport run.

…llifier (#21271) Cherry-pick of 20d6c29 with conflicts preserved for review.

…llifier (#21271) (with conflicts)

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

…llifier (#21271) (with conflicts)

…nd_nullifier (#21364)

BEGIN_COMMIT_OVERRIDE fix(aztec-nr): return Option from decode functions and fix event commitment capacity (backport #21264) (#21360) fix: backport #21271 — handle bad note lengths on compute_note_hash_and_nullifier (#21364) fix: not reusing tags of partially reverted txs (#20817) chore: revert accidental backport of #20817 (#21583) feat: Implement commit all and revert all for world state checkpoints (#21532) cherry-pick: fix: dependabot alerts (#21531) fix: dependabot alerts (backport #21531 to v4) (#21592) fix: backport #21443 — Don't update state if we failed to execute sufficient transactions (v4) (#21610) chore: Fix msgpack serialisation (#21612) END_COMMIT_OVERRIDE --------- Co-authored-by: Jan Beneš <janbenes1234@gmail.com> Co-authored-by: PhilWindle <60546371+PhilWindle@users.noreply.github.com> Co-authored-by: Phil Windle <philip.windle@gmail.com> Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: ludamad <adam.domurad@gmail.com>

BEGIN_COMMIT_OVERRIDE fix(aztec-nr): return Option from decode functions and fix event commitment capacity (backport #21264) (#21360) fix: backport #21271 — handle bad note lengths on compute_note_hash_and_nullifier (#21364) fix: not reusing tags of partially reverted txs (#20817) chore: revert accidental backport of #20817 (#21583) feat: Implement commit all and revert all for world state checkpoints (#21532) cherry-pick: fix: dependabot alerts (#21531) fix: dependabot alerts (backport #21531 to v4) (#21592) fix: backport #21443 — Don't update state if we failed to execute sufficient transactions (v4) (#21610) chore: Fix msgpack serialisation (#21612) fix(p2p): fall back to maxTxsPerCheckpoint for per-block tx validation (#21605) chore: merge v4 into backport-to-v4-staging (#21618) fix(revert): avm sim uses event loop again (#21138) (#21630) fix(e2e): remove historic/finalized block checks from epochs_multiple test (#21642) fix: clamp finalized block to oldest available in world-state (#21643) fix: skip handleChainFinalized when block is behind oldest available (#21656) chore: demote finalized block skip log to trace (#21661) fix: off-by-1 in getBlockHashMembershipWitness archive snapshot (backport #21648) (#21663) fix: capture txs not available error reason in proposal handler (#21670) chore: add L1 inclusion time to stg public (#21665) END_COMMIT_OVERRIDE --------- Co-authored-by: Jan Beneš <janbenes1234@gmail.com> Co-authored-by: PhilWindle <60546371+PhilWindle@users.noreply.github.com> Co-authored-by: Phil Windle <philip.windle@gmail.com> Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: ludamad <adam.domurad@gmail.com> Co-authored-by: Alex Gherghisan <alexghr@users.noreply.github.com>

fix: return Option::none() instead of panicking on packed note length…

351ae91

… mismatch

nchamo requested a review from nventuro as a code owner March 9, 2026 19:57

nchamo self-assigned this Mar 9, 2026

chore: nargo fmt

c0b1461

nchamo commented Mar 9, 2026

View reviewed changes

nchamo requested a review from benesjan March 9, 2026 20:48

benesjan reviewed Mar 10, 2026

View reviewed changes

nchamo added the backport-later label Mar 10, 2026

nchamo added 6 commits March 10, 2026 11:48

build: new test contract for compute_note_hash_and_nullifier and add …

4677c6b

…test packages to workspace

test: verify inner nullifier propagation in compute_note_hash_and_nul…

a5259d7

…lifier

refactor: remove unnecessary dummy storage from test contract

f6cfecd

refactor: move tests into contract package for CI compatibility

5736e66

fix: remove unused TestNote import in contract module

64636c5

chore: improve comments in test note and warning message

3932efa

nchamo requested a review from benesjan March 10, 2026 16:03

docs: fix inaccuracies in ComputeNoteHashAndNullifier doc comment

b894c47

ludamad removed the backport-later label Mar 11, 2026

benesjan approved these changes Mar 11, 2026

View reviewed changes

address pr review

834b523

nchamo added the backport-to-v4-next label Mar 11, 2026

nchamo enabled auto-merge (squash) March 11, 2026 13:19

nchamo merged commit 20d6c29 into merge-train/fairies Mar 11, 2026
16 of 20 checks passed

nchamo deleted the fix/handle-bad-note-lengths-compute-note-hash-and-nullifier branch March 11, 2026 13:35

AztecBot mentioned this pull request Mar 11, 2026

feat: merge-train/fairies #21353

Merged

AztecBot pushed a commit that referenced this pull request Mar 11, 2026

cherry-pick: fix: handle bad note lengths on compute_note_hash_and_nu…

bda3e1f

…llifier (#21271) Cherry-pick of 20d6c29 with conflicts preserved for review.

AztecBot mentioned this pull request Mar 11, 2026

fix: handle bad note lengths on compute_note_hash_and_nullifier (backport #21271) #21362

Closed

1 task

AztecBot pushed a commit that referenced this pull request Mar 11, 2026

cherry-pick: fix: handle bad note lengths on compute_note_hash_and_nu…

c277b22

…llifier (#21271) Cherry-pick of 20d6c29 with conflicts preserved for review.

AztecBot mentioned this pull request Mar 11, 2026

fix: handle bad note lengths on compute_note_hash_and_nullifier (backport #21271) #21363

Closed

1 task

AztecBot pushed a commit that referenced this pull request Mar 11, 2026

cherry-pick: fix: handle bad note lengths on compute_note_hash_and_nu…

50ed71b

…llifier (#21271) (with conflicts)

AztecBot pushed a commit that referenced this pull request Mar 11, 2026

cherry-pick: fix: handle bad note lengths on compute_note_hash_and_nu…

00a74a1

…llifier (#21271) (with conflicts)

AztecBot mentioned this pull request Mar 11, 2026

fix: backport #21271 — handle bad note lengths on compute_note_hash_and_nullifier #21364

Merged

AztecBot pushed a commit that referenced this pull request Mar 14, 2026

cherry-pick: fix: handle bad note lengths on compute_note_hash_and_nu…

a892c8a

…llifier (#21271) (with conflicts)

nchamo pushed a commit that referenced this pull request Mar 14, 2026

fix: backport #21271 — handle bad note lengths on compute_note_hash_a…

48c65bb

…nd_nullifier (#21364)

AztecBot mentioned this pull request Mar 16, 2026

chore: Accumulated backports to v4 #21582

Merged

This was referenced Mar 14, 2026

chore(v4): release 4.1.1 #20979

Open

chore: Accumulated backports to v4 #21651

Merged

AztecBot mentioned this pull request Mar 17, 2026

chore: Accumulated backports to v4 #21694

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: handle bad note lengths on compute_note_hash_and_nullifier#21271

fix: handle bad note lengths on compute_note_hash_and_nullifier#21271
nchamo merged 10 commits intomerge-train/fairiesfrom
fix/handle-bad-note-lengths-compute-note-hash-and-nullifier

nchamo commented Mar 9, 2026 •

edited

Loading

Uh oh!

nchamo Mar 9, 2026

Uh oh!

nchamo Mar 9, 2026

Uh oh!

benesjan Mar 10, 2026

Uh oh!

nchamo Mar 10, 2026

Uh oh!

benesjan left a comment

Uh oh!

benesjan Mar 10, 2026

Uh oh!

benesjan Mar 10, 2026

Uh oh!

nchamo commented Mar 10, 2026

Uh oh!

benesjan left a comment

Uh oh!

benesjan Mar 11, 2026

Uh oh!

benesjan Mar 11, 2026

Uh oh!

Uh oh!

AztecBot commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nchamo commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benesjan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nchamo commented Mar 10, 2026

Uh oh!

benesjan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AztecBot commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nchamo commented Mar 9, 2026 •

edited

Loading