feat(aztec-nr): add interactive handshake to the handshake registry - #24473
Conversation
nchamo
added
ci-no-fail-fast
| // `interactive_handshake` must obtain the recipient's signature from a `resolveCustomRequest` resolver before | ||
| // storing anything. The TXE test environment configures no such resolver, so the call fails at the resolver hop. | ||
| #[test(should_fail_with = "no resolveCustomRequest hook is configured")] | ||
| unconstrained fn interactive_handshake_requires_a_resolver() { |
There was a problem hiding this comment.
We currently can't test "green" scenarios since we can't mock an oracle inside private context. At the same time, we can't generate a signature from TXE accounts today
I think the best way to test the happy path would be with the e2e automatic test we are building
There was a problem hiding this comment.
Can we not mock regularly using OracleMock::mock("aztec_prv_callPrivateFunction")? Or do we need this to be exposed through TXE (I did not look deeply into this)? Just was looking at the unit tests added in constrained_delivery.nr.
There was a problem hiding this comment.
OracleMock only intercepts oracles fired from the test process itself. The constrained_delivery.nr tests are aztec-nr library unit tests, so the code under test runs in-process and its oracles are mockable. Here interactive_handshake runs inside TXE via env.call_private, so aztec_prv_resolveCustomRequest resolves in the TXE server (which throws without a hook) and never reaches the test-side mock. Happy path needs TXE-side resolver support (a signing resolver using the recipient account's mspk) or the e2e
nchamo
changed the title
vezenovm
left a comment
vezenovm
left a comment
There was a problem hiding this comment.
Overall looks good! Just some nits.
| /// If `recipient` is not a valid curve point. There are no upstream side effects in this call frame to | ||
| /// protect, and a fallback would insert a permanent note recording a handshake with an invalid recipient, | ||
| /// polluting registry state. | ||
| /// If `recipient` is not a valid curve point. |
There was a problem hiding this comment.
nit: I felt the justification for the panic was good info, but I am also good with just cleaning it up.
There was a problem hiding this comment.
oh I see you moved this info to the tests now
| /// Siloing by `self.msg_sender()` means a hostile contract cannot read another app's siloed secrets: whatever | ||
| /// arguments a caller passes, it only ever learns values siloed to its own address. Contracts should still call | ||
| /// [HandshakeRegistry::validate_handshake] when they need a constrained proof that supplied app-siloed secrets | ||
| /// [`HandshakeRegistry::validate_handshake`] when they need a constrained proof that supplied app-siloed secrets |
There was a problem hiding this comment.
nit: I saw we added several back-ticks in our doc links. I don't think they are necessary for linking functionality
There was a problem hiding this comment.
The link only shows on my IDE with the back-ticks. That why I assumed it was needed. Doesn't hurt though
There was a problem hiding this comment.
oh ok, ignore me then. Mine wasn't needing them but also works with the ticks.
| // `interactive_handshake` must obtain the recipient's signature from a `resolveCustomRequest` resolver before | ||
| // storing anything. The TXE test environment configures no such resolver, so the call fails at the resolver hop. | ||
| #[test(should_fail_with = "no resolveCustomRequest hook is configured")] | ||
| unconstrained fn interactive_handshake_requires_a_resolver() { |
There was a problem hiding this comment.
Can we not mock regularly using OracleMock::mock("aztec_prv_callPrivateFunction")? Or do we need this to be exposed through TXE (I did not look deeply into this)? Just was looking at the unit tests added in constrained_delivery.nr.
…24511) ## Summary - Renames the HandshakeRegistry's `get_handshakes` utility to `get_non_interactive_handshakes`: it only returns handshakes discovered from non-interactive announcement logs, so interactive handshakes (#24473) never appear in it, and the name should say which kind it returns. - Sweeps the rename through the aztec-nr selector constant, the registry's internal reader, the PXE default-authorization allowlist, and tests, and documents the getter's privacy properties (any contract can read a scope's discovered handshakes; ephemeral keys don't reveal the shared secret). - Re-pins the standard contracts, since the registry's artifact changes. Stacked on #24483. Fixes F-769 --------- Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
2 participants
Motivation
Non-interactive handshakes are unilateral: the recipient never consents, and the announcement log reveals that the recipient was contacted. Some flows want the opposite trade-off: the recipient explicitly authorizes the handshake, and nothing is announced
The change
HandshakeRegistry::interactive_handshake. It shares secret generation and note storage with the non-interactive flow, but requires the recipient to sign the freshly generated ephemeral key, and emits no announcement log.resolveCustomRequesthook. The response deserializes into a newRecipientSignaturestruct: the recipient's public keys, partial address, message-signing key point, and a schnorr signature.AztecAddress::compute, the signing key is tied to the address-committedmspk_m_hash, and the signature is checked over[chain_id, version, registry, eph_pk.x]under a new domain separator. The signed message never includes the sender, so the recipient authorizes without learning who initiated the handshake.