Clean up the `sapling::Note` API

zcash_client_backend/src/data_api/wallet.rs

@@ -4,7 +4,7 @@ use zcash_primitives::{
     consensus::{self, NetworkUpgrade},
     memo::MemoBytes,
     merkle_tree::MerklePath,
-    sapling::{self, prover::TxProver as SaplingProver},
+    sapling::{self, prover::TxProver as SaplingProver, Node},
     transaction::{
         builder::Builder,
         components::amount::{Amount, BalanceError},
@@ -610,11 +610,11 @@ fn select_key_for_note<N>(
 
     let expected_root = selected.witness.root();
     external_note
-        .filter(|n| expected_root == merkle_path.root(n.commitment()))
+        .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu())))
         .map(|n| (n, extsk.clone(), merkle_path.clone()))
         .or_else(|| {
             internal_note
-                .filter(|n| expected_root == merkle_path.root(n.commitment()))
+                .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu())))
                 .map(|n| (n, extsk.derive_internal(), merkle_path))
         })
 }

zcash_extensions/src/transparent/demo.rs

@@ -488,7 +488,7 @@ mod tests {
         extensions::transparent::{self as tze, Extension, FromPayload, ToPayload},
         legacy::TransparentAddress,
         merkle_tree::{CommitmentTree, IncrementalWitness},
-        sapling::Rseed,
+        sapling::{Node, Rseed},
         transaction::{
             builder::Builder,
             components::{
@@ -815,7 +815,7 @@ mod tests {
         let extsk = ExtendedSpendingKey::master(&[]);
         let to = extsk.default_address().1;
         let note1 = to.create_note(101000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)));
-        let cm1 = note1.commitment();
+        let cm1 = Node::from_cmu(&note1.cmu());
         let mut tree = CommitmentTree::empty();
         // fake that the note appears in some previous
         // shielded output

zcash_primitives/CHANGELOG.md

@@ -58,6 +58,9 @@ and this library adheres to Rust's notion of
 ### Removed
 - `zcash_primitives::sapling`:
   - The fields of `Note` are now private (use the new getter methods instead).
+  - `Note::uncommitted` (use `Node::empty_leaf` instead).
+  - `Note::derive_esk` (use `SaplingDomain::derive_esk` instead).
+  - `Note::commitment` (use `Node::from_cmu` instead).
   - `PaymentAddress::g_d`
   - `NoteValue` (use `zcash_primitives::sapling::value::NoteValue` instead).
   - `ValueCommitment` (use `zcash_primitives::sapling::value::ValueCommitment`

zcash_primitives/src/sapling/note.rs

@@ -1,7 +1,7 @@
 use group::{ff::Field, GroupEncoding};
 use rand_core::{CryptoRng, RngCore};
 
-use super::{value::NoteValue, Node, Nullifier, NullifierDerivingKey, PaymentAddress};
+use super::{value::NoteValue, Nullifier, NullifierDerivingKey, PaymentAddress};
 use crate::keys::prf_expand;
 
 mod commitment;
@@ -90,12 +90,6 @@ impl Note {
         &self.rseed
     }
 
-    pub fn uncommitted() -> bls12_381::Scalar {
-        // The smallest u-coordinate that is not on the curve
-        // is one.
-        bls12_381::Scalar::one()
-    }
-
     /// Computes the note commitment, returning the full point.
     fn cm_full_point(&self) -> NoteCommitment {
         NoteCommitment::derive(
@@ -117,10 +111,15 @@ impl Note {
         self.cm_full_point().into()
     }
 
+    /// Defined in [Zcash Protocol Spec § 4.7.2: Sending Notes (Sapling)][saplingsend].
+    ///
+    /// [saplingsend]: https://zips.z.cash/protocol/protocol.pdf#saplingsend
     pub fn rcm(&self) -> jubjub::Fr {
         self.rseed.rcm().0
     }
 
+    /// Derives `esk` from the internal `Rseed` value, or generates a random value if this
+    /// note was created with a v1 (i.e. pre-ZIP 212) note plaintext.
     pub fn generate_or_derive_esk<R: RngCore + CryptoRng>(&self, rng: &mut R) -> jubjub::Fr {
         self.generate_or_derive_esk_internal(rng)
     }
@@ -133,7 +132,7 @@ impl Note {
     }
 
     /// Returns the derived `esk` if this note was created after ZIP 212 activated.
-    pub fn derive_esk(&self) -> Option<jubjub::Fr> {
+    pub(crate) fn derive_esk(&self) -> Option<jubjub::Fr> {
         match self.rseed {
             Rseed::BeforeZip212(_) => None,
             Rseed::AfterZip212(rseed) => Some(jubjub::Fr::from_bytes_wide(
@@ -141,14 +140,6 @@ impl Note {
             )),
         }
     }
-
-    /// Returns [`self.cmu`] in the correct representation for inclusion in the Sapling
-    /// note commitment tree.
-    pub fn commitment(&self) -> Node {
-        Node {
-            repr: self.cmu().to_bytes(),
-        }
-    }
 }
 
 #[cfg(any(test, feature = "test-dependencies"))]

zcash_primitives/src/sapling/tree.rs

@@ -5,7 +5,7 @@ use lazy_static::lazy_static;
 use std::io::{self, Read, Write};
 
 use super::{
-    note::{ExtractedNoteCommitment, Note},
+    note::ExtractedNoteCommitment,
     pedersen_hash::{pedersen_hash, Personalization},
 };
 use crate::merkle_tree::{HashSer, Hashable};
@@ -14,6 +14,7 @@ pub const SAPLING_COMMITMENT_TREE_DEPTH: usize = 32;
 pub const SAPLING_COMMITMENT_TREE_DEPTH_U8: u8 = 32;
 
 lazy_static! {
+    static ref UNCOMMITTED_SAPLING: bls12_381::Scalar = bls12_381::Scalar::one();
     static ref EMPTY_ROOTS: Vec<Node> = {
         let mut v = vec![Node::blank()];
         for d in 0..SAPLING_COMMITMENT_TREE_DEPTH {
@@ -88,7 +89,7 @@ impl Node {
 impl incrementalmerkletree::Hashable for Node {
     fn empty_leaf() -> Self {
         Node {
-            repr: Note::uncommitted().to_repr(),
+            repr: UNCOMMITTED_SAPLING.to_repr(),
         }
     }
 

zcash_primitives/src/transaction/builder.rs

@@ -552,7 +552,7 @@ mod tests {
         legacy::TransparentAddress,
         memo::MemoBytes,
         merkle_tree::{CommitmentTree, IncrementalWitness},
-        sapling::Rseed,
+        sapling::{Node, Rseed},
         transaction::components::{
             amount::{Amount, DEFAULT_FEE},
             sapling::builder::{self as build_s},
@@ -671,7 +671,7 @@ mod tests {
         let mut rng = OsRng;
 
         let note1 = to.create_note(50000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)));
-        let cmu1 = note1.commitment();
+        let cmu1 = Node::from_cmu(&note1.cmu());
         let mut tree = CommitmentTree::empty();
         tree.append(cmu1).unwrap();
         let witness1 = IncrementalWitness::from_tree(&tree);
@@ -779,7 +779,7 @@ mod tests {
         }
 
         let note1 = to.create_note(50999, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)));
-        let cmu1 = note1.commitment();
+        let cmu1 = Node::from_cmu(&note1.cmu());
         let mut tree = CommitmentTree::empty();
         tree.append(cmu1).unwrap();
         let mut witness1 = IncrementalWitness::from_tree(&tree);
@@ -817,7 +817,7 @@ mod tests {
         }
 
         let note2 = to.create_note(1, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)));
-        let cmu2 = note2.commitment();
+        let cmu2 = Node::from_cmu(&note2.cmu());
         tree.append(cmu2).unwrap();
         witness1.append(cmu2).unwrap();
         let witness2 = IncrementalWitness::from_tree(&tree);

zcash_primitives/src/transaction/components/sapling/builder.rs

@@ -290,7 +290,7 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
         merkle_path: MerklePath<Node>,
     ) -> Result<(), Error> {
         // Consistency check: all anchors must equal the first one
-        let node = note.commitment();
+        let node = Node::from_cmu(&note.cmu());
         if let Some(anchor) = self.anchor {
             let path_root: bls12_381::Scalar = merkle_path.root(node).into();
             if path_root != anchor {