zcash_primitives: Replace sapling::redjubjub with redjubjub crate

zcash_primitives/CHANGELOG.md

@@ -45,6 +45,8 @@ and this library adheres to Rust's notion of
   - `circuit::{SpendVerifyingKey, PreparedSpendVerifyingKey}`
   - `circuit::{OutputVerifyingKey, PreparedOutputVerifyingKey}`
   - `constants` module.
+  - `keys::SpendAuthorizingKey`
+  - `keys::SpendValidatingKey`
   - `note_encryption::CompactOutputDescription` (moved from
     `zcash_primitives::transaction::components::sapling`).
   - `note_encryption::SaplingDomain::new`
@@ -100,6 +102,10 @@ and this library adheres to Rust's notion of
 - `zcash_primitives::sapling`:
   - `BatchValidator::validate` now takes the `SpendVerifyingKey` and
     `OutputVerifyingKey` newtypes.
+  - `SaplingVerificationContext::new` now always creates a context with ZIP 216
+    rules enforced, and no longer has a boolean for configuring this.
+  - `SaplingVerificationContext::{check_spend, final_check}` now use the
+    `redjubjub` crate types for `rk`, `spend_auth_sig`, and `binding_sig`.
   - `SaplingVerificationContext::{check_spend, check_output}` now take
     the `PreparedSpendVerifyingKey` and `PreparedOutputVerifyingKey`
     newtypes.
@@ -127,11 +133,23 @@ and this library adheres to Rust's notion of
     - `Error::MissingSignatures`
   - `bundle::Bundle` now has a second generic parameter `V`.
   - `bundle::Bundle::value_balance` now returns `&V` instead of `&Amount`.
+  - `bundle::Authorized::binding_sig` now has type `redjubjub::Signature<Binding>`.
+  - `bundle::Authorized::AuthSig` now has type `redjubjub::Signature<SpendAuth>`.
+  - `bundle::SpendDescription::temporary_zcashd_from_parts` now takes `rk` as
+    `redjubjub::VerificationKey<SpendAuth>` instead of
+    `zcash_primitives::sapling::redjubjub::PublicKey`.
+  - `bundle::SpendDescription::rk` now returns `&redjubjub::VerificationKey<SpendAuth>`.
+  - `bundle::SpendDescriptionV5::into_spend_description` now takes
+    `spend_auth_sig` as `redjubjub::Signature<SpendAuth>` instead of
+    `zcash_primitives::sapling::redjubjub::Signature`.
   - `bundle::testing::arb_bundle` now takes a `value_balance: V` argument.
   - `bundle::MapAuth` trait methods now take `&mut self` instead of `&self`.
   - `circuit::ValueCommitmentOpening::value` is now represented as a `NoteValue`
     instead of as a bare `u64`.
   - `keys::DecodingError` has a new variant `UnsupportedChildIndex`.
+  - `keys::ExpandedSpendingKey.ask` now has type `SpendAuthorizingKey`.
+  - `keys::ProofGenerationKey.ak` now has type `SpendValidatingKey`.
+  - `keys::ViewingKey.ak` now has type `SpendValidatingKey`.
   - `note_encryption`:
     - `SaplingDomain` no longer has a `P: consensus::Parameters` type parameter.
     - The following methods now take a `Zip212Enforcement` argument instead of a
@@ -150,6 +168,11 @@ and this library adheres to Rust's notion of
       - `try_sapling_output_recovery`
   - `util::generate_random_rseed` now takes a `Zip212Enforcement` argument
     instead of a `P: consensus::Parameters` argument and a height.
+  - `value::TrapdoorSum::into_bsk` now returns `redjubjub::SigningKey<Binding>`
+    instead of `zcash_primitives::sapling::redjubjub::PrivateKey`.
+  - `value::CommitmentSum::into_bvk` now returns
+    `redjubjub::VerificationKey<Binding>` instead of
+    `zcash_primitives::sapling::redjubjub::PublicKey`.
 - `zcash_primitives::transaction`:
   - `builder::Builder::{build, build_zfuture}` now take
     `&impl SpendProver, &impl OutputProver` instead of `&impl TxProver`.
@@ -192,6 +215,8 @@ and this library adheres to Rust's notion of
     - `OutputDescriptionV5::read`
   - `note_encryption::SaplingDomain::for_height` (use `SaplingDomain::new`
     instead).
+  - `redjubjub` module (use the `redjubjub` crate instead).
+  - `spend_sig` (use `redjubjub::SigningKey::{randomize, sign}` instead).
 - `zcash_primitives::transaction::components::sapling`:
   - The following types were removed from this module (moved into
     `zcash_primitives::sapling::bundle`):
@@ -217,6 +242,12 @@ and this library adheres to Rust's notion of
   - `ChildIndex::NonHardened`
   - `sapling::ExtendedFullViewingKey::derive_child`
 
+### Fixed
+- `zcash_primitives::keys::ExpandedSpendingKey::from_spending_key` now panics if the
+  spending key expands to `ask = 0`. This has a negligible probability of occurring.
+- `zcash_primitives::zip32::ExtendedSpendingKey::derive_child` now panics if the
+  child key has `ask = 0`. This has a negligible probability of occurring.
+
 ## [0.13.0] - 2023-09-25
 ### Added
 - `zcash_primitives::consensus::BlockHeight::saturating_sub`

zcash_primitives/src/sapling.rs

@@ -11,21 +11,13 @@ pub mod note;
 pub mod note_encryption;
 pub mod pedersen_hash;
 pub mod prover;
-pub mod redjubjub;
 mod spec;
 mod tree;
 pub mod util;
 pub mod value;
 mod verifier;
 pub mod zip32;
 
-use group::GroupEncoding;
-use rand_core::{CryptoRng, RngCore};
-
-use constants::SPENDING_KEY_GENERATOR;
-
-use self::redjubjub::{PrivateKey, PublicKey, Signature};
-
 pub use address::PaymentAddress;
 pub use bundle::Bundle;
 pub use keys::{Diversifier, NullifierDerivingKey, ProofGenerationKey, SaplingIvk, ViewingKey};
@@ -35,64 +27,13 @@ pub use tree::{
 };
 pub use verifier::{BatchValidator, SaplingVerificationContext};
 
-/// Create the spendAuthSig for a Sapling SpendDescription.
-pub fn spend_sig<R: RngCore + CryptoRng>(
-    ask: PrivateKey,
-    ar: jubjub::Fr,
-    sighash: &[u8; 32],
-    rng: &mut R,
-) -> Signature {
-    spend_sig_internal(&ask, ar, sighash, rng)
-}
-
-pub(crate) fn spend_sig_internal<R: RngCore>(
-    ask: &PrivateKey,
-    ar: jubjub::Fr,
-    sighash: &[u8; 32],
-    rng: &mut R,
-) -> Signature {
-    // We compute `rsk`...
-    let rsk = ask.randomize(ar);
-
-    // We compute `rk` from there (needed for key prefixing)
-    let rk = PublicKey::from_private(&rsk, SPENDING_KEY_GENERATOR);
-
-    // Compute the signature's message for rk/spend_auth_sig
-    let mut data_to_be_signed = [0u8; 64];
-    data_to_be_signed[0..32].copy_from_slice(&rk.0.to_bytes());
-    data_to_be_signed[32..64].copy_from_slice(&sighash[..]);
-
-    // Do the signing
-    rsk.sign(&data_to_be_signed, rng, SPENDING_KEY_GENERATOR)
-}
-
-/// Verifies a spendAuthSig.
-///
-/// This only exists because the RedJubjub implementation inside `zcash_primitives` does
-/// not implement key prefixing (which was added in response to a Sapling audit). This
-/// will be fixed by migrating to the redjubjub crate.
-pub(crate) fn verify_spend_sig(
-    ak: &PublicKey,
-    alpha: jubjub::Fr,
-    sighash: &[u8; 32],
-    sig: &Signature,
-) -> bool {
-    // We compute `rk` (needed for key prefixing)
-    let rk = ak.randomize(alpha, SPENDING_KEY_GENERATOR);
-
-    // Compute the signature's message for rk/spend_auth_sig
-    let mut data_to_be_signed = [0u8; 64];
-    data_to_be_signed[0..32].copy_from_slice(&rk.0.to_bytes());
-    data_to_be_signed[32..64].copy_from_slice(&sighash[..]);
-
-    // Do the verifying
-    rk.verify(&data_to_be_signed, sig, SPENDING_KEY_GENERATOR)
-}
-
 #[cfg(any(test, feature = "test-dependencies"))]
 pub mod testing {
     pub use super::{
         address::testing::arb_payment_address, keys::testing::arb_incoming_viewing_key,
         note::testing::arb_note, tree::testing::arb_node,
     };
 }
+
+#[cfg(test)]
+mod test_vectors;

zcash_primitives/src/sapling/builder.rs

@@ -3,9 +3,10 @@
 use core::fmt;
 use std::{marker::PhantomData, sync::mpsc::Sender};
 
-use ff::Field;
+use group::ff::Field;
 use rand::{seq::SliceRandom, RngCore};
 use rand_core::CryptoRng;
+use redjubjub::{Binding, SpendAuth};
 
 use crate::{
     sapling::{
@@ -14,17 +15,13 @@
             Authorization, Authorized, Bundle, GrothProofBytes, MapAuth, OutputDescription,
             SpendDescription,
         },
-        constants::{SPENDING_KEY_GENERATOR, VALUE_COMMITMENT_RANDOMNESS_GENERATOR},
-        keys::OutgoingViewingKey,
+        keys::{OutgoingViewingKey, SpendAuthorizingKey, SpendValidatingKey},
         note_encryption::{sapling_note_encryption, Zip212Enforcement},
         prover::{OutputProver, SpendProver},
-        redjubjub::{PrivateKey, PublicKey, Signature},
-        spend_sig_internal,
         util::generate_random_rseed_internal,
         value::{
             CommitmentSum, NoteValue, TrapdoorSum, ValueCommitTrapdoor, ValueCommitment, ValueSum,
         },
-        verify_spend_sig,
         zip32::ExtendedSpendingKey,
         Diversifier, MerklePath, Node, Note, PaymentAddress, ProofGenerationKey, SaplingIvk,
     },
@@ -117,10 +114,10 @@
         // Construct the value commitment.
         let cv = ValueCommitment::derive(self.note.value(), self.rcv.clone());
 
-        let ak = PublicKey(self.proof_generation_key.ak.into());
+        let ak = self.proof_generation_key.ak.clone();
 
         // This is the result of the re-randomization, we compute it for the caller
-        let rk = ak.randomize(self.alpha, SPENDING_KEY_GENERATOR);
+        let rk = ak.randomize(&self.alpha);
 
         let nullifier = self.note.nf(
             &self.proof_generation_key.to_viewing_key().nk,
@@ -507,10 +504,7 @@
             (spends - outputs)
                 .into_bvk(i64::try_from(self.value_balance).map_err(|_| Error::InvalidAmount)?)
         };
-        assert_eq!(
-            PublicKey::from_private(&bsk, VALUE_COMMITMENT_RANDOMNESS_GENERATOR).0,
-            bvk.0,
-        );
+        assert_eq!(redjubjub::VerificationKey::from(&bsk), bvk);
 
         let bundle = if shielded_spends.is_empty() && shielded_outputs.is_empty() {
             None
@@ -676,16 +670,9 @@
 }
 
 /// Marker for an unauthorized bundle with no signatures.
+#[derive(Clone)]
 pub struct Unsigned {
-    bsk: PrivateKey,
-}
-
-impl Clone for Unsigned {
-    fn clone(&self) -> Self {
-        Self {
-            bsk: PrivateKey(self.bsk.0),
-        }
-    }
+    bsk: redjubjub::SigningKey<Binding>,
 }
 
 impl fmt::Debug for Unsigned {
@@ -703,15 +690,15 @@
 pub struct SigningParts {
     /// The spend validating key for this spend description. Used to match spend
     /// authorizing keys to spend descriptions they can create signatures for.
-    ak: PublicKey,
+    ak: SpendValidatingKey,
     /// The randomization needed to derive the actual signing key for this note.
     alpha: jubjub::Scalar,
 }
 
 /// Marker for a partially-authorized bundle, in the process of being signed.
 #[derive(Clone, Debug)]
 pub struct PartiallyAuthorized {
-    binding_signature: Signature,
+    binding_signature: redjubjub::Signature<Binding>,
     sighash: [u8; 32],
 }
 
@@ -720,16 +707,18 @@
 }
 
 /// A heisen[`Signature`] for a particular [`SpendDescription`].
+///
+/// [`Signature`]: redjubjub::Signature
 #[derive(Clone, Debug)]
 pub enum MaybeSigned {
     /// The information needed to sign this [`SpendDescription`].
     SigningMetadata(SigningParts),
     /// The signature for this [`SpendDescription`].
-    Signature(Signature),
+    Signature(redjubjub::Signature<SpendAuth>),
 }
 
 impl MaybeSigned {
-    fn finalize(self) -> Result<Signature, Error> {
+    fn finalize(self) -> Result<redjubjub::Signature<SpendAuth>, Error> {
         match self {
             Self::Signature(sig) => Ok(sig),
             _ => Err(Error::MissingSignatures),
@@ -752,11 +741,7 @@
             MaybeSigned::SigningMetadata,
             |auth: InProgress<P, Unsigned>| InProgress {
                 sigs: PartiallyAuthorized {
-                    binding_signature: auth.sigs.bsk.sign(
-                        &sighash,
-                        &mut rng,
-                        VALUE_COMMITMENT_RANDOMNESS_GENERATOR,
-                    ),
+                    binding_signature: auth.sigs.bsk.sign(&mut rng, &sighash),
                     sighash,
                 },
                 _proof_state: PhantomData::default(),
@@ -774,7 +759,7 @@
         self,
         mut rng: R,
         sighash: [u8; 32],
-        signing_keys: &[PrivateKey],
+        signing_keys: &[SpendAuthorizingKey],
     ) -> Result<Bundle<Authorized, V>, Error> {
         signing_keys
             .iter()
@@ -786,43 +771,47 @@
 }
 
 impl<P: InProgressProofs, V> Bundle<InProgress<P, PartiallyAuthorized>, V> {
-    /// Signs this bundle with the given [`PrivateKey`].
+    /// Signs this bundle with the given [`redjubjub::SigningKey`].
     ///
     /// This will apply signatures for all notes controlled by this spending key.
-    pub fn sign<R: RngCore + CryptoRng>(self, mut rng: R, ask: &PrivateKey) -> Self {
-        let expected_ak = PublicKey::from_private(ask, SPENDING_KEY_GENERATOR);
+    pub fn sign<R: RngCore + CryptoRng>(self, mut rng: R, ask: &SpendAuthorizingKey) -> Self {
+        let expected_ak = ask.into();
         let sighash = self.authorization().sigs.sighash;
         self.map_authorization((
             |proof| proof,
             |proof| proof,
             |maybe| match maybe {
-                MaybeSigned::SigningMetadata(parts) if parts.ak.0 == expected_ak.0 => {
-                    MaybeSigned::Signature(spend_sig_internal(ask, parts.alpha, &sighash, &mut rng))
+                MaybeSigned::SigningMetadata(parts) if parts.ak == expected_ak => {
+                    MaybeSigned::Signature(ask.randomize(&parts.alpha).sign(&mut rng, &sighash))
                 }
                 s => s,
             },
             |partial| partial,
         ))
     }
 
-    /// Appends externally computed [`Signature`]s.
+    /// Appends externally computed [`redjubjub::Signature`]s.
     ///
     /// Each signature will be applied to the one input for which it is valid. An error
     /// will be returned if the signature is not valid for any inputs, or if it is valid
     /// for more than one input.
-    pub fn append_signatures(self, signatures: &[Signature]) -> Result<Self, Error> {
+    pub fn append_signatures(
+        self,
+        signatures: &[redjubjub::Signature<SpendAuth>],
+    ) -> Result<Self, Error> {
         signatures.iter().try_fold(self, Self::append_signature)
     }
 
-    fn append_signature(self, signature: &Signature) -> Result<Self, Error> {
+    fn append_signature(self, signature: &redjubjub::Signature<SpendAuth>) -> Result<Self, Error> {
         let sighash = self.authorization().sigs.sighash;
         let mut signature_valid_for = 0usize;
         let bundle = self.map_authorization((
             |proof| proof,
             |proof| proof,
             |maybe| match maybe {
                 MaybeSigned::SigningMetadata(parts) => {
-                    if verify_spend_sig(&parts.ak, parts.alpha, &sighash, signature) {
+                    let rk = parts.ak.randomize(&parts.alpha);
+                    if rk.verify(&sighash, signature).is_ok() {
                         signature_valid_for += 1;
                         MaybeSigned::Signature(*signature)
                     } else {
@@ -872,7 +861,6 @@
         bundle::{Authorized, Bundle},
         note_encryption::Zip212Enforcement,
         prover::mock::{MockOutputProver, MockSpendProver},
-        redjubjub::PrivateKey,
         testing::{arb_node, arb_note},
         value::testing::arb_positive_note_value,
         zip32::testing::arb_extended_spending_key,
@@ -926,11 +914,7 @@
                         bundle.create_proofs(&MockSpendProver, &MockOutputProver, &mut rng, None);
 
                     bundle
-                        .apply_signatures(
-                            &mut rng,
-                            fake_sighash_bytes,
-                            &[PrivateKey(extsk.expsk.ask)],
-                        )
+                        .apply_signatures(&mut rng, fake_sighash_bytes, &[extsk.expsk.ask])
                         .unwrap()
                 },
             )