refactor(aztec-nr): keep handshake secrets internal to TagSecretSource

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 {

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,

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),
+        );
+    }
 }