Skip to content

feat(aztec-nr): wire handshake secret discovery into contract sync#23938

Merged
nchamo merged 25 commits into
merge-train/fairies-v5from
nchamo/f-588-constrained-delivery-tagged-log-retrieval-in-default-2
Jun 10, 2026
Merged

feat(aztec-nr): wire handshake secret discovery into contract sync#23938
nchamo merged 25 commits into
merge-train/fairies-v5from
nchamo/f-588-constrained-delivery-tagged-log-retrieval-in-default-2

Conversation

@nchamo

@nchamo nchamo commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Moves DiscoveredHandshake, HandshakePage, and MAX_HANDSHAKES_PER_PAGE from the HandshakeRegistry contract into aztec::messages::delivery::handshake so aztec-nr owns the shared types and the contract imports them.
  • Implements get_handshake_secrets which paginates through the HandshakeRegistry's get_handshakes utility function, derives app-siloed tagging secrets via get_shared_secrets, and returns them as ProvidedSecret entries for log discovery during do_sync_state.
  • Auto-authorizes the HandshakeRegistry's get_handshakes call in PXE (gated on both target address and function selector) and preloads its artifact in all PXE entrypoints.
  • Changes get_shared_secrets to accept and return EphemeralArray directly, removing the BoundedVec conversion layer.

Fixes F-588

@nchamo nchamo requested a review from nventuro as a code owner June 8, 2026 16:48
@nchamo nchamo self-assigned this Jun 8, 2026
Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/handshake.nr
@nchamo nchamo 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 8, 2026
@nchamo nchamo requested a review from vezenovm June 9, 2026 11:19
Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/handshake.nr Outdated
Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/handshake.nr
Comment thread noir-projects/aztec-nr/aztec/src/oracle/shared_secret.nr
10: generate_contract_library_method_compute_note_hash
11: generate_contract_library_method_compute_note_hash
at <repo>/noir-projects/aztec-nr/aztec/src/macros/aztec/compute_note_hash_and_nullifier.nr:<line>:<col>
11: generate_note_type_impl

@vezenovm vezenovm Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not for this PR, but looks like this is extra noise that can be removed with <line>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With what?

@vezenovm vezenovm Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh oops I cut it off. With <line>.

EDIT: TIL turns out if you don't put <word here> in a code snippet (e.g., ``) it won't display in the comment.

}
const protocolContractsProvider = new BundledProtocolContractsProvider();
const preloadedContractsProvider = options.preloadedContractsProvider ?? {
getPreloadedContracts: async () => [await getStandardMultiCallEntrypoint()],

@vezenovm vezenovm Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For mutli call entry point I think this was fine but for the handshake registry which is called on every contract we would override the previous preloadedContractsProvider right? Do we allow custom preloaded contracts in PXE? Should we add a unit test for custom preloaded contract with the handshake discovery?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with Grego, decided to do this: c1c5040

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just discussed it again. Will change it up a bit and request your re-review when finished

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the idea will be:

  • If the user specifies a provider, then we will honor it. We won't force-add any contracts, because they might not need it and we want to keep the loading to be as quick as possible
  • If the user doesn't specify a provider, our entry points will add all 3 standard contracts (multicall, auth registry, handshake registry)
  • On PXE, if no provider is set, we will add the ones that aztec-nr needs: auth registry + handshake registry

I'll mark you as reviewer when the CI passes, since I'm fighting against limits


export async function getAuthRegistryArtifact(): Promise<ContractArtifact> {
if (!standardContractArtifact) {
// Cannot assert this import as it's incompatible with bundlers like vite

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the case when this was introduced (2025). Not anymore

@nchamo nchamo requested a review from vezenovm June 10, 2026 18:42
Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/builder.nr
Comment thread yarn-project/pxe/src/pxe.ts
@nchamo nchamo merged commit 539b1d9 into merge-train/fairies-v5 Jun 10, 2026
11 checks passed
@nchamo nchamo deleted the nchamo/f-588-constrained-delivery-tagged-log-retrieval-in-default-2 branch June 10, 2026 19:26
);
// The HandshakeRegistry is called during every contract's sync to discover handshake secrets.
// It is a standard contract that only reads its own state, so it is always authorized.
const isHandshakeRegistryRead =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, do we think this bypass is too broad? The state is owned by the registry but it is scoped to the recipient. Thus a malicious utility can read registry state for the scoped user. Any regular utility call can call HandshakeRegistry.get_handshakes and learn handshake metadata. Do we want to try and add some info as to whether we are executing a contract sync here? Only then bypass the wallet hook.

Otherwise, I think this escapes where we intend for handshake info to live. Handshakes are supposed to be used as discovery input for the contract/PXE and to help find logs for that contract. With this bypass, a contract can call get_handshakes and return the info to the dapp without the wallet explicitly authorizing it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline, we are ok with this being exposed to apps. Just added some extra documentation here #24018

vezenovm added a commit that referenced this pull request Jun 11, 2026
The registry source changes from #23938 shift the standard registry's
artifact hash, class id, and address. Regenerates the standard-contract
data and address pins and repacks the registry entry in
pinned-standard-contracts.tar.gz from the merged build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants