From bbcdbf4d4c0d67162cb703ae8e8045c54d7af358 Mon Sep 17 00:00:00 2001 From: Nico Chamo Date: Fri, 3 Jul 2026 13:09:14 -0300 Subject: [PATCH] refactor(aztec-nr): keep handshake secrets internal to TagSecretSource --- .../aztec/src/messages/delivery/tag.nr | 13 +-- .../src/messages/delivery/tag_derivation.nr | 2 +- .../messages/delivery/tag_secret_source.nr | 109 ++++++++++-------- 3 files changed, 69 insertions(+), 55 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/delivery/tag.nr b/noir-projects/aztec-nr/aztec/src/messages/delivery/tag.nr index 1a22bc46d4e0..241fc929418e 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/delivery/tag.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/delivery/tag.nr @@ -4,7 +4,6 @@ use crate::context::PrivateContext; use crate::messages::delivery::{ - constrained_delivery::emit_sequence_nullifier, OnchainDeliveryMode, tag_derivation::{existing_handshake_secrets_or_else, TagDerivation}, tag_secret_source::TagSecretSource, @@ -55,25 +54,23 @@ fn default_tag_secret_source( } fn tag_with( - source: TagSecretSource, + mut source: TagSecretSource, context: &mut PrivateContext, mode: OnchainDeliveryMode, sender: AztecAddress, recipient: AztecAddress, ) -> Field { - let secrets = source.obtain_secrets(context, sender, recipient); + let secret = source.resolve_tag_secret(context, sender, recipient); // Safety: the index is untrusted. Constrained delivery constrains it below before emitting the tag; unconstrained // discovery tolerates gaps, so a wrong index only yields an undiscoverable tag. - let index = unsafe { get_next_tagging_index(secrets.shared, mode) }; + let index = unsafe { get_next_tagging_index(secret, mode) }; if mode == OnchainDeliveryMode::onchain_constrained() { - source.constrain_secrets(context, sender, recipient, secrets, index); - emit_sequence_nullifier(context, sender, recipient, secrets, index); + source.constrain_tag_secret(context, sender, recipient, index); } - // The discovery tag stays derived from the shared secret only, so the recipient still finds every message. - tag_from_secret_and_index(secrets.shared, index, mode) + tag_from_secret_and_index(secret, index, mode) } fn tag_domain_separator(mode: OnchainDeliveryMode) -> u32 { diff --git a/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_derivation.nr b/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_derivation.nr index c8b46f495c2a..b0ae3cdc50cb 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_derivation.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_derivation.nr @@ -41,7 +41,7 @@ impl TagDerivation { /// Resolves this choice into the [`TagSecretSource`] backing the message tag. pub(crate) fn into_tag_secret_source(self, sender: AztecAddress, recipient: AztecAddress) -> TagSecretSource { if self.kind == NON_INTERACTIVE_HANDSHAKE { - // A fresh handshake is created in a constrained manner in `obtain_secrets`. + // A fresh handshake is created in a constrained manner in `TagSecretSource::resolve_tag_secret`. existing_handshake_secrets_or_else( sender, recipient, diff --git a/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_secret_source.nr b/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_secret_source.nr index e880dda6f011..7f5adc86c0d5 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_secret_source.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/delivery/tag_secret_source.nr @@ -1,6 +1,6 @@ use crate::context::PrivateContext; use crate::messages::delivery::{ - constrained_delivery::constrain_preexisting_handshake_secrets, + constrained_delivery::{constrain_preexisting_handshake_secrets, emit_sequence_nullifier}, handshake::{AppSiloedHandshakeSecrets, create_non_interactive_handshake}, }; use crate::protocol::address::AztecAddress; @@ -10,68 +10,65 @@ global EXISTING_HANDSHAKE: u8 = 1; global NEW_NON_INTERACTIVE_HANDSHAKE: u8 = 2; global UNCONSTRAINED_SECRET: u8 = 3; -/// How a message's tagging secrets are sourced and, for constrained delivery, constrained. +/// How a message's tag secret is sourced and, for constrained delivery, constrained. +#[derive(Eq)] pub(crate) struct TagSecretSource { kind: u8, - secrets: AppSiloedHandshakeSecrets, -} - -// Hand-written rather than `#[derive(Eq)]`: the derive resolves each field's `Eq` at expansion time and can't see -// `AppSiloedHandshakeSecrets`'s own derived `Eq` (a cross-module derive dependency), whereas this direct comparison -// resolves it normally. -impl Eq for TagSecretSource { - fn eq(self, other: Self) -> bool { - (self.kind == other.kind) & (self.secrets == other.secrets) - } + /// The secret the message tag derives from. Every source resolves to exactly one. + tag_secret: Field, + /// The handshake sender-only secret, folded into the constrained-delivery sequence nullifier. Only meaningful + /// for handshake-backed sources. + sender_only: Field, } impl TagSecretSource { /// Reuses the secrets of a handshake already registered for the pair (mode-agnostic). pub(crate) fn existing_handshake(secrets: AppSiloedHandshakeSecrets) -> Self { - Self { kind: EXISTING_HANDSHAKE, secrets } + Self { kind: EXISTING_HANDSHAKE, tag_secret: secrets.shared, sender_only: secrets.sender_only } } /// Establishes a fresh non-interactive handshake, publishing information about the recipient onchain. pub(crate) fn new_non_interactive_handshake() -> Self { - Self { kind: NEW_NON_INTERACTIVE_HANDSHAKE, secrets: AppSiloedHandshakeSecrets { shared: 0, sender_only: 0 } } + Self { kind: NEW_NON_INTERACTIVE_HANDSHAKE, tag_secret: 0, sender_only: 0 } } - /// A ready-to-use unconstrained secret the wallet resolved. Sound only for unconstrained delivery, which derives - /// the discovery tag from the shared secret alone and never folds in a sender-only secret. + /// A ready-to-use unconstrained secret the wallet resolved. Sound only for unconstrained delivery, which never + /// anchors a sequence, so no sender-only secret is involved. pub(crate) fn unconstrained_secret(secret: Field) -> Self { - Self { kind: UNCONSTRAINED_SECRET, secrets: AppSiloedHandshakeSecrets { shared: secret, sender_only: 0 } } + Self { kind: UNCONSTRAINED_SECRET, tag_secret: secret, sender_only: 0 } } - /// Returns the app-siloed handshake secrets, performing any handshake creation the source requires. - pub(crate) fn obtain_secrets( - self, + /// Returns the tag secret backing the message tag, performing any handshake creation the source requires. + pub(crate) fn resolve_tag_secret( + &mut self, context: &mut PrivateContext, sender: AztecAddress, recipient: AztecAddress, - ) -> AppSiloedHandshakeSecrets { + ) -> Field { if self.kind == NEW_NON_INTERACTIVE_HANDSHAKE { - create_non_interactive_handshake( + let secrets = create_non_interactive_handshake( context, STANDARD_HANDSHAKE_REGISTRY_ADDRESS, sender, recipient, - ) - } else { - // An existing or unconstrained secret is already resolved; only a fresh handshake must be created. - self.secrets + ); + self.tag_secret = secrets.shared; + self.sender_only = secrets.sender_only; } + self.tag_secret } - /// Constrains `(secrets, index)` for constrained delivery, each source proving its secrets its own way. A source - /// that cannot back constrained delivery rejects it here. Only called for constrained delivery. - pub(crate) fn constrain_secrets( + /// Constrains `(tag secret, index)` for constrained delivery and emits the sequence nullifier, each source + /// proving its secrets its own way. A source that cannot back constrained delivery rejects it here. Only called + /// for constrained delivery, after [`Self::resolve_tag_secret`]. + pub(crate) fn constrain_tag_secret( self, context: &mut PrivateContext, sender: AztecAddress, recipient: AztecAddress, - secrets: AppSiloedHandshakeSecrets, index: u32, ) { + let secrets = AppSiloedHandshakeSecrets { shared: self.tag_secret, sender_only: self.sender_only }; if self.kind == EXISTING_HANDSHAKE { constrain_preexisting_handshake_secrets( context, @@ -89,11 +86,14 @@ impl TagSecretSource { "an unconstrained tagging secret cannot back constrained delivery", ); } + emit_sequence_nullifier(context, sender, recipient, secrets, index); } } mod test { + use crate::context::PrivateContext; use crate::hash::hash_args; + use crate::messages::delivery::constrained_delivery::compute_constrained_msg_nullifier; use crate::messages::delivery::handshake::AppSiloedHandshakeSecrets; use crate::oracle::execution_cache; use crate::protocol::{address::AztecAddress, traits::{FromField, Serialize}}; @@ -105,27 +105,33 @@ mod test { global RECIPIENT: AztecAddress = AztecAddress::from_field(8); #[test] - unconstrained fn existing_handshake_obtains_its_secrets() { + unconstrained fn existing_handshake_resolves_and_constrains_its_secrets() { let env = TestEnvironment::new(); let secrets = AppSiloedHandshakeSecrets { shared: 42, sender_only: 7 }; + let index: u32 = 3; + env.private_context(|context| { - assert_eq( - TagSecretSource::existing_handshake(secrets).obtain_secrets(context, SENDER, RECIPIENT), - secrets, - ); + let _ = OracleMock::mock("aztec_prv_isNullifierPending").returns(false).times(1); + + let mut source = TagSecretSource::existing_handshake(secrets); + assert_eq(source.resolve_tag_secret(context, SENDER, RECIPIENT), secrets.shared); + + source.constrain_tag_secret(context, SENDER, RECIPIENT, index); + assert_emitted_sequence_nullifier(context, secrets, index); }); } #[test] - unconstrained fn unconstrained_secret_obtains_its_secret() { + unconstrained fn unconstrained_secret_resolves_its_secret() { let env = TestEnvironment::new(); env.private_context(|context| { - assert_eq(TagSecretSource::unconstrained_secret(42).obtain_secrets(context, SENDER, RECIPIENT).shared, 42); + let mut source = TagSecretSource::unconstrained_secret(42); + assert_eq(source.resolve_tag_secret(context, SENDER, RECIPIENT), 42); }); } #[test] - unconstrained fn new_non_interactive_handshake_obtains_the_bootstrapped_secrets() { + unconstrained fn new_non_interactive_handshake_resolves_and_constrains_the_bootstrapped_secrets() { let env = TestEnvironment::new(); let secrets = AppSiloedHandshakeSecrets { shared: 7, sender_only: 9 }; @@ -139,10 +145,11 @@ mod test { .returns((child_call_end_counter, returns_hash)) .times(1); - assert_eq( - TagSecretSource::new_non_interactive_handshake().obtain_secrets(context, SENDER, RECIPIENT), - secrets, - ); + let mut source = TagSecretSource::new_non_interactive_handshake(); + assert_eq(source.resolve_tag_secret(context, SENDER, RECIPIENT), secrets.shared); + + source.constrain_tag_secret(context, SENDER, RECIPIENT, 0); + assert_emitted_sequence_nullifier(context, secrets, 0); }); } @@ -151,11 +158,10 @@ mod test { unconstrained fn fresh_handshake_secret_must_start_at_index_zero() { let env = TestEnvironment::new(); env.private_context(|context| { - TagSecretSource::new_non_interactive_handshake().constrain_secrets( + TagSecretSource::new_non_interactive_handshake().constrain_tag_secret( context, AztecAddress::zero(), AztecAddress::zero(), - AppSiloedHandshakeSecrets { shared: 0, sender_only: 0 }, 1, ); }); @@ -165,13 +171,24 @@ mod test { unconstrained fn unconstrained_secret_cannot_back_constrained_delivery() { let env = TestEnvironment::new(); env.private_context(|context| { - TagSecretSource::unconstrained_secret(7).constrain_secrets( + TagSecretSource::unconstrained_secret(7).constrain_tag_secret( context, AztecAddress::zero(), AztecAddress::zero(), - AppSiloedHandshakeSecrets { shared: 7, sender_only: 0 }, 0, ); }); } + + unconstrained fn assert_emitted_sequence_nullifier( + context: &mut PrivateContext, + secrets: AppSiloedHandshakeSecrets, + index: u32, + ) { + assert_eq(context.nullifiers.len(), 1); + assert_eq( + context.nullifiers.get(0).inner.value, + compute_constrained_msg_nullifier(SENDER, RECIPIENT, secrets, index), + ); + } }