Skip to content

Commit

Permalink
zcash_primitives: Replace sapling::redjubjub with redjubjub crate
Browse files Browse the repository at this point in the history
As a side-effect, we remove the ability to verify individual
transactions with pre-ZIP 216 rules (which we already removed from
`zcashd` consensus nodes in zcash/zcash#6000 and zcash/zcash#6399, as
all pre-ZIP 216 transactions on mainnet are also valid under ZIP 216).
  • Loading branch information
str4d committed Nov 30, 2023
1 parent 764127f commit de1ed21
Show file tree
Hide file tree
Showing 15 changed files with 150 additions and 621 deletions.
20 changes: 20 additions & 0 deletions zcash_primitives/CHANGELOG.md
Expand Up @@ -100,6 +100,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.
Expand Down Expand Up @@ -127,6 +131,15 @@ 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`
Expand All @@ -150,6 +163,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`.
Expand Down Expand Up @@ -192,6 +210,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`):
Expand Down
62 changes: 0 additions & 62 deletions zcash_primitives/src/sapling.rs
Expand Up @@ -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};
Expand All @@ -35,60 +27,6 @@ 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::{
Expand Down
77 changes: 36 additions & 41 deletions zcash_primitives/src/sapling/builder.rs
Expand Up @@ -3,9 +3,10 @@
use core::fmt;
use std::{marker::PhantomData, sync::mpsc::Sender};

use ff::Field;
use group::{ff::Field, GroupEncoding};
use rand::{seq::SliceRandom, RngCore};
use rand_core::CryptoRng;
use redjubjub::{Binding, SpendAuth};

use crate::{
sapling::{
Expand All @@ -14,17 +15,13 @@ use crate::{
Authorization, Authorized, Bundle, GrothProofBytes, MapAuth, OutputDescription,
SpendDescription,
},
constants::{SPENDING_KEY_GENERATOR, VALUE_COMMITMENT_RANDOMNESS_GENERATOR},
keys::OutgoingViewingKey,
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,
},
Expand Down Expand Up @@ -117,10 +114,11 @@ impl SpendDescriptionInfo {
// 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 = redjubjub::VerificationKey::try_from(self.proof_generation_key.ak.to_bytes())
.expect("valid points are valid verification keys");

// 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,
Expand Down Expand Up @@ -507,10 +505,7 @@ impl SaplingBuilder {
(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
Expand Down Expand Up @@ -676,16 +671,9 @@ impl<S: InProgressSignatures, V> Bundle<InProgress<Unproven, S>, V> {
}

/// 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 {
Expand All @@ -703,15 +691,15 @@ impl InProgressSignatures for Unsigned {
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: redjubjub::VerificationKey<SpendAuth>,
/// 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],
}

Expand All @@ -720,16 +708,18 @@ impl InProgressSignatures for PartiallyAuthorized {
}

/// 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),
Expand All @@ -752,11 +742,7 @@ impl<P: InProgressProofs, V> Bundle<InProgress<P, Unsigned>, V> {
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(),
Expand All @@ -774,7 +760,7 @@ impl<V> Bundle<InProgress<Proven, Unsigned>, V> {
self,
mut rng: R,
sighash: [u8; 32],
signing_keys: &[PrivateKey],
signing_keys: &[redjubjub::SigningKey<SpendAuth>],
) -> Result<Bundle<Authorized, V>, Error> {
signing_keys
.iter()
Expand All @@ -786,43 +772,52 @@ impl<V> Bundle<InProgress<Proven, Unsigned>, V> {
}

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: &redjubjub::SigningKey<SpendAuth>,
) -> Self {
let expected_ak = redjubjub::VerificationKey::from(ask);
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 => {
let rsk = ask.randomize(&parts.alpha);
MaybeSigned::Signature(rsk.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(

Check warning on line 804 in zcash_primitives/src/sapling/builder.rs

View check run for this annotation

Codecov / codecov/patch

zcash_primitives/src/sapling/builder.rs#L804

Added line #L804 was not covered by tests
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> {

Check warning on line 811 in zcash_primitives/src/sapling/builder.rs

View check run for this annotation

Codecov / codecov/patch

zcash_primitives/src/sapling/builder.rs#L811

Added line #L811 was not covered by tests
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() {

Check warning on line 820 in zcash_primitives/src/sapling/builder.rs

View check run for this annotation

Codecov / codecov/patch

zcash_primitives/src/sapling/builder.rs#L819-L820

Added lines #L819 - L820 were not covered by tests
signature_valid_for += 1;
MaybeSigned::Signature(*signature)
} else {
Expand Down Expand Up @@ -872,7 +867,6 @@ pub mod testing {
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,
Expand Down Expand Up @@ -929,7 +923,8 @@ pub mod testing {
.apply_signatures(
&mut rng,
fake_sighash_bytes,
&[PrivateKey(extsk.expsk.ask)],
&[redjubjub::SigningKey::try_from(extsk.expsk.ask.to_bytes())
.expect("valid scalars are valid signing keys")],

Check warning on line 927 in zcash_primitives/src/sapling/builder.rs

View check run for this annotation

Codecov / codecov/patch

zcash_primitives/src/sapling/builder.rs#L926-L927

Added lines #L926 - L927 were not covered by tests
)
.unwrap()
},
Expand Down

0 comments on commit de1ed21

Please sign in to comment.