fix(aztec-nr): Prevent handshake nullifier griefing and nullifier sequence tests by vezenovm · Pull Request #24258 · AztecProtocol/aztec-packages

vezenovm · 2026-06-23T20:49:39Z

Add a sender-only secret to the sequence nullifier and prevent the recipient from griefing the sender. Red-green TXE test added.

nchamo

Didn't go over the entire PR, but have a question that could imply some big refactor, so we should address that

nchamo

Great work!

Left some comments, nothing too major though

nchamo · 2026-06-24T18:57:28Z

            registry,
            VALIDATE_HANDSHAKE_SELECTOR,
-            [sender.to_field(), recipient.to_field(), secret],
+            [sender.to_field(), recipient.to_field(), secrets.shared, secrets.sender_only],


I was thinking... do we need to include secrets.shared as part of the nullifier? I'm just thinking out loud

I think you are correct that it is not needed for safety. It does feel weird to remove shared when the sequence tuple is keyed by the shared secret everywhere else like for the tag. But yeah I believe it is safe. ~~The nullifier is unique per send and private to everyone but the sender.~~ EDIT: This isn't enough for our needs though. Outlined in the thread below.

But validate_handshake still needs to constrain both fields. We pass only the sender secret below.

Actually I think we do still need to verify the shared secret in the nullifier. For index > 0 if the caller supplied a different shared secret while holding the sender_only secret fixed I would expect a different predecessor nullifier proof. It would only prove the continuity of sender_only, it would not bind the shared secret to its sender chain. e.g., you could extend a tagging sequence using a different valid handshake predecessor when yours is invalid, essentially letting a malicious prover spoof a valid handshake.

You can’t start a fake constrained sequence at index 0, but the predecessor nullifier without shared would let you extend a fake/different sequence with a malicious secret at index 1 using another valid sequence’s predecessor. The malicious shared shared secret will then be used in the constrained log tag, the recipient will scan for the wrong tag and not find the note.

I added a test to protect against this here e690858

We can't remove shared but the secret is already derived from the (sender, recipient). I think we should actually be able to remove sender and recipient from the nullifier. I leave that for a follow-up though, as I need to explore the actual safety. If there is redundancy it doesn't have much of a cost.

…r-secret Resolve standard-contract artifact conflicts by re-pinning: recompiled the four standard contracts against the merged tree and regenerated standard_addresses.nr, standard_contract_data.ts, and pinned-standard-contracts.tar.gz. The three unchanged contracts inherit the base's version-bumped addresses; HandshakeRegistry gets a fresh address reflecting both the version bump and the F-702 source changes.

…ange The initial re-pin compiled the standard contracts while standard_addresses.nr still held the base's handshake registry address, so every contract's sync logic (which bakes STANDARD_HANDSHAKE_REGISTRY_ADDRESS into handshake discovery via fetch_handshake_page) embedded the stale address. That surfaced at runtime as 'Function artifact not found for contract 0x11306cd9... selector 0xe43bd3ff' during contract sync. Iterating pin-standard-build + generate:data to a fixpoint: the new handshake registry address (0x15afc452...) cascades into the addresses of AuthRegistry, MultiCallEntrypoint, and PublicChecks, since they all embed it through their default sync. All four standard-contract addresses now match the artifacts pinned in the tarball.

nchamo

Small question about the tests

nchamo · 2026-06-26T19:52:59Z

    }
+
+    #[test]
+    unconstrained fn valid_predecessor_from_one_handshake_cannot_authorize_another_sequence() {


I'm not sure what these 2 tests are asserting. Isn't this like testing that compute_constrained_msg_nullifier(handshake_a) != compute_constrained_msg_nullifier(handshake_b)

I specifically wanted to assert that the nullifier is tied to the shared secret. The reasoning for which is here #24258 (comment). I realize how the test on its own isn't super clear. The danger is on the discovery side (the recipient not finding a note sent by a malicious sender with a fake shared secret) so here it felt best to just test the nullifiers.

I could add comments about the consequences of omitting the shared or the sender_only secrets from the nullifier or instead add an e2e test where we maliciously return a bad secret and show that the recipient fails to discover it.

I understand the intent. But I think that can be clearer from:

let secretsA = handshakeSecret { sender: 0, shared: 1 } let secretsB = handshakeSecret { sender: 0, shared: 2 } // Same as A compute_constrained_msg_nullifier(secretsA) != compute_constrained_msg_nullifier(secretsB)

Right? Having the same sender secret doesn't mean you'll have the same nullifier sequence, and that's enough to explain why we should include the shared key. Specially now that the sender can be set by oracle.

Yeah tbh I had the same thought that this didn't make the consequence clear enough. I will explore if we can do it Noir, but I do think we may need to add an e2e to show the true consequences.

compute_constrained_msg_nullifier(secretsA) != compute_constrained_msg_nullifier(secretsB)

I had similar tests before I think we had decided to remove, but we didn't have a strong case then. I agree this is the same and clearer. Will pin it down there and add a comment on why that logic is pinned.

I added an index version too.

…stry_contract/src/main.nr Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

…iloed_secrets The registry utility returns AppSiloedHandshakeSecrets (both the shared and the sender-only secret), so the plural name is accurate. Renamed across the registry contract, the aztec-nr selector constant and signature (GET_APP_SILOED_SECRETS_SELECTOR), the PXE default-authorized read allowlist, the constrained-delivery test contract wrapper, and the Noir/TS tests. Renaming the utility function changes its selector, which feeds the artifact hash and contract class id, so the HandshakeRegistry address changed. Re-pinned the standard contracts to a fixpoint (the new address cascades into AuthRegistry, MultiCallEntrypoint, and PublicChecks through their default handshake sync), regenerating standard_addresses.nr, standard_contract_data.ts, and pinned-standard-contracts.tar.gz.

…der-secret' into mv/f-702-handshake-nullifier-sender-secret

nchamo

Great work!

BEGIN_COMMIT_OVERRIDE fix(aztec-nr): Prevent handshake nullifier griefing and nullifier sequence tests (#24258) feat(aztec-nr): derive message tag from a resolved secret source (#24312) feat(keys)!: add derivation for message-signing and fallback keys (#24348) feat: make sqlite the path of least resistance backend option for PXE and wallet (#24179) END_COMMIT_OVERRIDE

Resolves #24258 (comment) I forgot to commit this before merging.

vezenovm added the ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure label Jun 23, 2026

vezenovm requested review from LeilaWang and nventuro as code owners June 23, 2026 20:49

vezenovm added 3 commits June 23, 2026 16:54

reduce diff

248e205

diff again

3ad19e4

fix: prevent recipient griefing of onchain handshake delivery

44983f1

vezenovm force-pushed the mv/f-702-handshake-nullifier-sender-secret branch from c622b5f to 44983f1 Compare June 23, 2026 21:03

vezenovm added 3 commits June 23, 2026 17:19

fix test

5e8d8aa

update artifacts

572fcdd

cleanup

81a4ac4

vezenovm requested a review from nchamo June 24, 2026 14:04

comment

b7f803a

nchamo reviewed Jun 24, 2026

View reviewed changes

vezenovm added 3 commits June 24, 2026 11:54

one get_app_siloed_secret

fa403f8

single struct

1ad08ac

clean

1701ad1

vezenovm requested a review from nchamo June 24, 2026 16:56

nchamo reviewed Jun 25, 2026

View reviewed changes

vezenovm added 7 commits June 25, 2026 15:10

single field sender secret

615a682

chore: single secret in nullifier, unify methods, pr review

e6cd7d5

merge conflicts

e2740a9

fix: bind constrained delivery nullifiers to handshake secrets

a46ea24

artifacts

cb914ee

improve nullifier forgery tests

e690858

bring back struct

47cc4bf

vezenovm changed the title ~~fix(aztec-nr): Prevent handshake nullifier griefing~~ fix(aztec-nr): Prevent handshake nullifier griefing and nullifier predecessor sequence tests Jun 26, 2026

vezenovm requested a review from nchamo June 26, 2026 13:27

vezenovm added 2 commits June 26, 2026 10:40

nchamo reviewed Jun 26, 2026

View reviewed changes

vezenovm and others added 4 commits June 26, 2026 15:59

Update noir-projects/noir-contracts/contracts/standard/handshake_regi…

8aafeed

…stry_contract/src/main.nr Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

simplify nullifier pins

890ffec

Merge remote-tracking branch 'origin/mv/f-702-handshake-nullifier-sen…

fc8981e

…der-secret' into mv/f-702-handshake-nullifier-sender-secret

vezenovm requested a review from nchamo June 26, 2026 20:54

vezenovm changed the title ~~fix(aztec-nr): Prevent handshake nullifier griefing and nullifier predecessor sequence tests~~ fix(aztec-nr): Prevent handshake nullifier griefing and nullifier sequence tests Jun 26, 2026

nchamo reviewed Jun 27, 2026

View reviewed changes

nchamo approved these changes Jun 27, 2026

View reviewed changes

vezenovm merged commit 0549d21 into merge-train/fairies-v5 Jun 29, 2026
15 checks passed

vezenovm deleted the mv/f-702-handshake-nullifier-sender-secret branch June 29, 2026 13:56

AztecBot mentioned this pull request Jun 29, 2026

feat: merge-train/fairies-v5 #24365

Merged

vezenovm mentioned this pull request Jun 29, 2026

chore(aztec-nr): add constrained delivery index nullifier test #24366

Merged

vezenovm added a commit that referenced this pull request Jun 30, 2026

chore(aztec-nr): add constrained delivery index nullifier test (#24366)

7710d6e

Resolves #24258 (comment) I forgot to commit this before merging.

nchamo mentioned this pull request Jun 30, 2026

fix(aztec-nr): prevent recipient forging a colliding handshake #24403

Merged

AztecBot mentioned this pull request Jul 21, 2026

chore: port v5-next to next #24825

Open

Uh oh!

Conversation

vezenovm commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nchamo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nchamo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vezenovm Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vezenovm Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vezenovm Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nchamo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nchamo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vezenovm commented Jun 23, 2026 •

edited

Loading

vezenovm Jun 25, 2026 •

edited

Loading

vezenovm Jun 25, 2026 •

edited

Loading

vezenovm Jun 26, 2026 •

edited

Loading