From 2ae4d87cbfdce3e87e3177d5834fdf5fc8b50852 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Fri, 19 May 2023 16:29:55 +0000
Subject: [PATCH] zcash_primitives: Remove redundant checks from Sapling note
 decryption

- The consistency check between `esk` and `ephemeral_key` is checked
  inside `zcash_note_encryption::try_output_recovery_with_ock`.
- The `diversifier` validity check is necessarily performed when
  deriving `pk_d` for the `ivk` pathway, so we keep it there for the
  `ovk` pathway as well, and drop the check from the `PaymentAddress`
  internal constructor.
---
 zcash_primitives/src/sapling/address.rs       | 11 +++++++++++
 .../src/sapling/note_encryption.rs            | 19 +++++++++----------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/zcash_primitives/src/sapling/address.rs b/zcash_primitives/src/sapling/address.rs
index 55a457a06..4ee547e53 100644
--- a/zcash_primitives/src/sapling/address.rs
+++ b/zcash_primitives/src/sapling/address.rs
@@ -36,6 +36,17 @@ impl PaymentAddress {
         // Check that the diversifier is valid
         diversifier.g_d()?;
 
+        Self::from_parts_unchecked(diversifier, pk_d)
+    }
+
+    /// Constructs a PaymentAddress from a diversifier and a Jubjub point.
+    ///
+    /// Returns None if `pk_d` is the identity. The caller must check that `diversifier`
+    /// is valid for Sapling.
+    pub(crate) fn from_parts_unchecked(
+        diversifier: Diversifier,
+        pk_d: DiversifiedTransmissionKey,
+    ) -> Option<Self> {
         if pk_d.is_identity() {
             None
         } else {
diff --git a/zcash_primitives/src/sapling/note_encryption.rs b/zcash_primitives/src/sapling/note_encryption.rs
index d3fb41d79..1bb81af6f 100644
--- a/zcash_primitives/src/sapling/note_encryption.rs
+++ b/zcash_primitives/src/sapling/note_encryption.rs
@@ -64,10 +64,12 @@ pub fn prf_ock(
     )
 }
 
+/// `get_pk_d` must check that the diversifier contained within the note plaintext is a
+/// valid Sapling diversifier.
 fn sapling_parse_note_plaintext_without_memo<F, P: consensus::Parameters>(
     domain: &SaplingDomain<P>,
     plaintext: &[u8],
-    get_validated_pk_d: F,
+    get_pk_d: F,
 ) -> Option<(Note, PaymentAddress)>
 where
     F: FnOnce(&Diversifier) -> Option<DiversifiedTransmissionKey>,
@@ -91,9 +93,10 @@ where
         Rseed::AfterZip212(r)
     };
 
-    let pk_d = get_validated_pk_d(&diversifier)?;
+    let pk_d = get_pk_d(&diversifier)?;
 
-    let to = PaymentAddress::from_parts(diversifier, pk_d)?;
+    // `diversifier` was checked by `get_pk_d`.
+    let to = PaymentAddress::from_parts_unchecked(diversifier, pk_d)?;
     let note = to.create_note(value.into(), rseed);
     Some((note, to))
 }
@@ -254,16 +257,12 @@ impl<P: consensus::Parameters> Domain for SaplingDomain<P> {
     fn parse_note_plaintext_without_memo_ovk(
         &self,
         pk_d: &Self::DiversifiedTransmissionKey,
-        esk: &Self::EphemeralSecretKey,
-        ephemeral_key: &EphemeralKeyBytes,
+        _esk: &Self::EphemeralSecretKey,
+        _ephemeral_key: &EphemeralKeyBytes,
         plaintext: &NotePlaintextBytes,
     ) -> Option<(Self::Note, Self::Recipient)> {
         sapling_parse_note_plaintext_without_memo(self, &plaintext.0, |diversifier| {
-            if esk.derive_public(diversifier.g_d()?.into()).to_bytes().0 == ephemeral_key.0 {
-                Some(*pk_d)
-            } else {
-                None
-            }
+            diversifier.g_d().map(|_| *pk_d)
         })
     }