From 06ba399d80c26756526a2c5ca7d9c6c604e49ec5 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 10 Jan 2023 00:08:20 +0000 Subject: [PATCH] Clean up the `sapling::Note` API --- zcash_client_backend/src/data_api/wallet.rs | 6 ++--- zcash_extensions/src/transparent/demo.rs | 4 ++-- zcash_primitives/CHANGELOG.md | 3 +++ zcash_primitives/src/sapling/note.rs | 23 ++++++------------- zcash_primitives/src/sapling/tree.rs | 5 ++-- zcash_primitives/src/transaction/builder.rs | 8 +++---- .../transaction/components/sapling/builder.rs | 2 +- 7 files changed, 23 insertions(+), 28 deletions(-) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 970409a0f..3dcf28122 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/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( 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)) }) } diff --git a/zcash_extensions/src/transparent/demo.rs b/zcash_extensions/src/transparent/demo.rs index e6c2cd9aa..a8d3a7781 100644 --- a/zcash_extensions/src/transparent/demo.rs +++ b/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(¬e1.cmu()); let mut tree = CommitmentTree::empty(); // fake that the note appears in some previous // shielded output diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 93624ef48..e74035ca6 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/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` diff --git a/zcash_primitives/src/sapling/note.rs b/zcash_primitives/src/sapling/note.rs index e58b19629..578f5dbdc 100644 --- a/zcash_primitives/src/sapling/note.rs +++ b/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(&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 { + pub(crate) fn derive_esk(&self) -> Option { 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"))] diff --git a/zcash_primitives/src/sapling/tree.rs b/zcash_primitives/src/sapling/tree.rs index d22bec370..976f5709d 100644 --- a/zcash_primitives/src/sapling/tree.rs +++ b/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 = { 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(), } } diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 274dba783..d1930ece5 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/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(¬e1.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(¬e1.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(¬e2.cmu()); tree.append(cmu2).unwrap(); witness1.append(cmu2).unwrap(); let witness2 = IncrementalWitness::from_tree(&tree); diff --git a/zcash_primitives/src/transaction/components/sapling/builder.rs b/zcash_primitives/src/transaction/components/sapling/builder.rs index 9eb45ac90..60d8450c2 100644 --- a/zcash_primitives/src/transaction/components/sapling/builder.rs +++ b/zcash_primitives/src/transaction/components/sapling/builder.rs @@ -290,7 +290,7 @@ impl SaplingBuilder

{ merkle_path: MerklePath, ) -> Result<(), Error> { // Consistency check: all anchors must equal the first one - let node = note.commitment(); + let node = Node::from_cmu(¬e.cmu()); if let Some(anchor) = self.anchor { let path_root: bls12_381::Scalar = merkle_path.root(node).into(); if path_root != anchor {