From f2b0cae3e495e4f7d482e587432ec4e5f2793528 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Sun, 8 Sep 2024 19:01:33 +0200 Subject: [PATCH] Allow Transaction Builder to be called with inconsistent keys. --- masp_primitives/src/lib.rs | 12 + masp_primitives/src/transaction.rs | 22 +- masp_primitives/src/transaction/builder.rs | 21 +- .../src/transaction/components/sapling.rs | 12 +- .../transaction/components/sapling/builder.rs | 49 +++-- .../src/transaction/components/transparent.rs | 7 +- masp_primitives/src/zip32.rs | 4 +- masp_primitives/src/zip32/sapling.rs | 208 +++++++++++++----- 8 files changed, 236 insertions(+), 99 deletions(-) diff --git a/masp_primitives/src/lib.rs b/masp_primitives/src/lib.rs index 07765909..9516920b 100644 --- a/masp_primitives/src/lib.rs +++ b/masp_primitives/src/lib.rs @@ -34,3 +34,15 @@ pub use num_traits; #[cfg(test)] mod test_vectors; + +#[cfg(not(feature = "arbitrary"))] +pub trait MaybeArbitrary<'a> {} + +#[cfg(not(feature = "arbitrary"))] +impl<'a, T> MaybeArbitrary<'a> for T {} + +#[cfg(feature = "arbitrary")] +pub trait MaybeArbitrary<'a>: arbitrary::Arbitrary<'a> {} + +#[cfg(feature = "arbitrary")] +impl<'a, T: for<'b> arbitrary::Arbitrary<'b>> MaybeArbitrary<'a> for T {} diff --git a/masp_primitives/src/transaction.rs b/masp_primitives/src/transaction.rs index 948ca62d..2224fae4 100644 --- a/masp_primitives/src/transaction.rs +++ b/masp_primitives/src/transaction.rs @@ -34,9 +34,11 @@ use self::{ }, txid::{to_txid, BlockTxCommitmentDigester, TxIdDigester}, }; +use crate::MaybeArbitrary; use borsh::schema::add_definition; use borsh::schema::Fields; use borsh::schema::{Declaration, Definition}; +use std::marker::PhantomData; use std::ops::RangeInclusive; #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] @@ -195,23 +197,17 @@ impl BorshSchema for TxVersion { /// Authorization state for a bundle of transaction data. pub trait Authorization { - #[cfg(not(feature = "arbitrary"))] - type TransparentAuth: transparent::Authorization + PartialEq + BorshDeserialize + BorshSerialize; - #[cfg(not(feature = "arbitrary"))] - type SaplingAuth: sapling::Authorization + PartialEq + BorshDeserialize + BorshSerialize; - - #[cfg(feature = "arbitrary")] type TransparentAuth: transparent::Authorization + PartialEq + BorshDeserialize + BorshSerialize - + for<'a> arbitrary::Arbitrary<'a>; - #[cfg(feature = "arbitrary")] + + for<'a> MaybeArbitrary<'a>; + type SaplingAuth: sapling::Authorization + PartialEq + BorshDeserialize + BorshSerialize - + for<'a> arbitrary::Arbitrary<'a>; + + for<'a> MaybeArbitrary<'a>; } #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct Unproven; @@ -224,11 +220,13 @@ impl Authorization for Authorized { type SaplingAuth = sapling::Authorized; } -pub struct Unauthorized; +pub struct Unauthorized(PhantomData); -impl Authorization for Unauthorized { +impl MaybeArbitrary<'a>> + Authorization for Unauthorized +{ type TransparentAuth = transparent::builder::Unauthorized; - type SaplingAuth = sapling::builder::Unauthorized; + type SaplingAuth = sapling::builder::Unauthorized; } /// A MASP transaction. diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index 8f2f033c..72749516 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -30,7 +30,8 @@ use crate::{ txid::TxIdDigester, Transaction, TransactionData, TransparentAddress, TxVersion, Unauthorized, }, - zip32::ExtendedSpendingKey, + zip32::{ExtendedKey, ExtendedSpendingKey}, + MaybeArbitrary, }; #[cfg(feature = "transparent-inputs")] @@ -168,7 +169,11 @@ impl Builder { } } -impl Builder

{ +impl< + P: consensus::Parameters, + K: ExtendedKey + std::fmt::Debug + Clone + PartialEq + for<'a> MaybeArbitrary<'a>, + > Builder +{ /// Creates a new `Builder` targeted for inclusion in the block with the given height, /// using default values for general transaction fields and the default OS random. /// @@ -181,12 +186,16 @@ impl Builder

{ } } -impl Builder

{ +impl< + P: consensus::Parameters, + K: ExtendedKey + std::fmt::Debug + Clone + PartialEq + for<'a> MaybeArbitrary<'a>, + > Builder +{ /// Common utility function for builder construction. /// /// WARNING: THIS MUST REMAIN PRIVATE AS IT ALLOWS CONSTRUCTION /// OF BUILDERS WITH NON-CryptoRng RNGs - fn new_internal(params: P, target_height: BlockHeight) -> Builder

{ + fn new_internal(params: P, target_height: BlockHeight) -> Builder { Builder { params: params.clone(), target_height, @@ -203,7 +212,7 @@ impl Builder

{ /// paths for previous Sapling notes. pub fn add_sapling_spend( &mut self, - extsk: ExtendedSpendingKey, + extsk: K, diversifier: Diversifier, note: Note, merkle_path: MerklePath, @@ -347,7 +356,7 @@ impl Builder

{ ) .map_err(Error::SaplingBuild)?; - let unauthed_tx: TransactionData = TransactionData { + let unauthed_tx: TransactionData> = TransactionData { version, consensus_branch_id: BranchId::for_height(&self.params, self.target_height), lock_time: 0, diff --git a/masp_primitives/src/transaction/components/sapling.rs b/masp_primitives/src/transaction/components/sapling.rs index 5db45334..6fcc7bf8 100644 --- a/masp_primitives/src/transaction/components/sapling.rs +++ b/masp_primitives/src/transaction/components/sapling.rs @@ -25,6 +25,7 @@ use crate::{ redjubjub::{self, PublicKey, Signature}, Nullifier, }, + MaybeArbitrary, }; use super::{amount::I128Sum, GROTH_PROOF_SIZE}; @@ -35,15 +36,8 @@ pub mod builder; pub mod fees; pub trait Authorization: Debug { - #[cfg(not(feature = "arbitrary"))] - type Proof: Clone + Debug + PartialEq + Hash; - #[cfg(not(feature = "arbitrary"))] - type AuthSig: Clone + Debug + PartialEq; - - #[cfg(feature = "arbitrary")] - type Proof: Clone + Debug + PartialEq + Hash + for<'a> arbitrary::Arbitrary<'a>; - #[cfg(feature = "arbitrary")] - type AuthSig: Clone + Debug + PartialEq + for<'a> arbitrary::Arbitrary<'a>; + type Proof: Clone + Debug + PartialEq + Hash + for<'a> MaybeArbitrary<'a>; + type AuthSig: Clone + Debug + PartialEq + for<'a> MaybeArbitrary<'a>; } #[derive(Debug, Copy, Clone, PartialEq, Eq)] diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index e4b59a3c..9c304012 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -8,6 +8,7 @@ use ff::PrimeField; use group::GroupEncoding; use rand::{seq::SliceRandom, CryptoRng, RngCore}; +use crate::MaybeArbitrary; use crate::{ asset_type::AssetType, consensus::{self, BlockHeight}, @@ -33,7 +34,7 @@ use crate::{ }, }, }, - zip32::ExtendedSpendingKey, + zip32::{ExtendedKey, ExtendedSpendingKey}, }; use borsh::schema::add_definition; use borsh::schema::Declaration; @@ -41,7 +42,9 @@ use borsh::schema::Definition; use borsh::schema::Fields; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use std::collections::BTreeMap; +use std::fmt::Debug; use std::io::Write; +use std::marker::PhantomData; /// A subset of the parameters necessary to build a transaction pub trait BuildParams { @@ -777,19 +780,22 @@ impl BorshDeserialize for SaplingBui #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] #[derive(Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] -pub struct Unauthorized { +pub struct Unauthorized { tx_metadata: SaplingMetadata, + phantom: PhantomData, } -impl std::fmt::Debug for Unauthorized { +impl std::fmt::Debug for Unauthorized { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { write!(f, "Unauthorized") } } -impl Authorization for Unauthorized { +impl MaybeArbitrary<'a>> Authorization + for Unauthorized +{ type Proof = GrothProofBytes; - type AuthSig = SpendDescriptionInfo; + type AuthSig = SpendDescriptionInfo; } impl SaplingBuilder { @@ -826,14 +832,18 @@ impl SaplingBuilder { } } -impl SaplingBuilder

{ +impl< + P: consensus::Parameters, + K: ExtendedKey + Debug + Clone + PartialEq + for<'a> MaybeArbitrary<'a>, + > SaplingBuilder +{ /// Adds a Sapling note to be spent in this transaction. /// /// Returns an error if the given Merkle path does not have the same anchor as the /// paths for previous Sapling notes. pub fn add_spend( &mut self, - extsk: ExtendedSpendingKey, + extsk: K, diversifier: Diversifier, note: Note, merkle_path: MerklePath, @@ -922,7 +932,7 @@ impl SaplingBuilder

{ bparams: &mut impl BuildParams, target_height: BlockHeight, progress_notifier: Option<&Sender>, - ) -> Result>, Error> { + ) -> Result>>, Error> { // Record initial positions of spends and outputs let value_balance = self.value_balance(); let params = self.params; @@ -961,7 +971,8 @@ impl SaplingBuilder

{ let mut progress = 0u32; // Create Sapling SpendDescriptions - let shielded_spends: Vec> = if !indexed_spends.is_empty() { + let shielded_spends: Vec>> = if !indexed_spends.is_empty() + { let anchor = self .spend_anchor .expect("MASP Spend anchor must be set if MASP spends are present."); @@ -970,7 +981,10 @@ impl SaplingBuilder

{ .into_iter() .enumerate() .map(|(i, (pos, spend))| { - let proof_generation_key = spend.extsk.expsk.proof_generation_key(); + let proof_generation_key = spend + .extsk + .to_proof_generation_key() + .expect("Proof generation key must be known for each MASP spend."); let nullifier = spend.note.nf( &proof_generation_key.to_viewing_key().nk, @@ -1172,7 +1186,10 @@ impl SaplingBuilder

{ shielded_converts, shielded_outputs, value_balance, - authorization: Unauthorized { tx_metadata }, + authorization: Unauthorized { + tx_metadata, + phantom: PhantomData, + }, }) }; @@ -1180,7 +1197,9 @@ impl SaplingBuilder

{ } } -impl SpendDescription { +impl MaybeArbitrary<'a>> + SpendDescription> +{ pub fn apply_signature(&self, spend_auth_sig: Signature) -> SpendDescription { SpendDescription { cv: self.cv, @@ -1193,7 +1212,9 @@ impl SpendDescription { } } -impl Bundle { +impl MaybeArbitrary<'a>> + Bundle> +{ pub fn apply_signatures( self, prover: &Pr, @@ -1214,7 +1235,7 @@ impl Bundle { .enumerate() .map(|(i, spend)| { spend.apply_signature(spend_sig_internal( - PrivateKey(spend.spend_auth_sig.extsk.expsk.ask), + PrivateKey(spend.spend_auth_sig.extsk.to_spending_key().expect("Spend authorization key must be known for each MASP spend.").expsk.ask), bparams.spend_alpha(i), sighash_bytes, rng, diff --git a/masp_primitives/src/transaction/components/transparent.rs b/masp_primitives/src/transaction/components/transparent.rs index 8d92092e..ad83e0c6 100644 --- a/masp_primitives/src/transaction/components/transparent.rs +++ b/masp_primitives/src/transaction/components/transparent.rs @@ -6,6 +6,7 @@ use std::io::{self, Read, Write}; use crate::asset_type::AssetType; use crate::transaction::TransparentAddress; +use crate::MaybeArbitrary; use borsh::schema::add_definition; use borsh::schema::Declaration; use borsh::schema::Definition; @@ -18,11 +19,7 @@ pub mod builder; pub mod fees; pub trait Authorization: fmt::Debug { - #[cfg(not(feature = "arbitrary"))] - type TransparentSig: fmt::Debug + Clone + PartialEq; - - #[cfg(feature = "arbitrary")] - type TransparentSig: fmt::Debug + Clone + PartialEq + for<'a> arbitrary::Arbitrary<'a>; + type TransparentSig: fmt::Debug + Clone + PartialEq + for<'a> MaybeArbitrary<'a>; } #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] diff --git a/masp_primitives/src/zip32.rs b/masp_primitives/src/zip32.rs index 870acb75..cb95f533 100644 --- a/masp_primitives/src/zip32.rs +++ b/masp_primitives/src/zip32.rs @@ -19,8 +19,8 @@ use borsh::{BorshDeserialize, BorshSerialize}; #[deprecated(note = "Please use the types exported from the `zip32::sapling` module instead.")] pub use sapling::{ sapling_address, sapling_default_address, sapling_derive_internal_fvk, sapling_find_address, - DiversifiableFullViewingKey, ExtendedFullViewingKey, ExtendedSpendingKey, - ZIP32_SAPLING_FVFP_PERSONALIZATION, ZIP32_SAPLING_INT_PERSONALIZATION, + DiversifiableFullViewingKey, ExtendedFullViewingKey, ExtendedKey, ExtendedSpendingKey, + PseudoExtendedKey, ZIP32_SAPLING_FVFP_PERSONALIZATION, ZIP32_SAPLING_INT_PERSONALIZATION, ZIP32_SAPLING_MASTER_PERSONALIZATION, }; use std::io::{Read, Write}; diff --git a/masp_primitives/src/zip32/sapling.rs b/masp_primitives/src/zip32/sapling.rs index a9b5241b..01549cff 100644 --- a/masp_primitives/src/zip32/sapling.rs +++ b/masp_primitives/src/zip32/sapling.rs @@ -12,7 +12,7 @@ use crate::{ constants::{PROOF_GENERATION_KEY_GENERATOR, SPENDING_KEY_GENERATOR}, keys::{prf_expand, prf_expand_vec}, sapling::keys::{DecodingError, ExpandedSpendingKey, FullViewingKey, OutgoingViewingKey}, - sapling::{ProofGenerationKey, SaplingIvk}, + sapling::{redjubjub::PrivateKey, ProofGenerationKey, SaplingIvk}, }; use aes::Aes256; use blake2b_simd::Params as Blake2bParams; @@ -28,6 +28,7 @@ use std::collections::BTreeMap; use std::{ cmp::Ordering, convert::TryInto, + hash::{Hash, Hasher}, io::{self, Error, ErrorKind, Read, Write}, ops::AddAssign, str::FromStr, @@ -920,71 +921,176 @@ impl Ord for ExtendedSpendingKey { } } -/// An extended full viewing key bundled with partial authorizations -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] -#[derive(Clone, PartialEq, Eq, Hash, Copy, Debug)] -pub struct PseudoExtendedSpendingKey { - /// Extended key that stores the currently known authorizations - xsk: ExtendedSpendingKey, - /// Viewing key for which this key provides authorizations - xfvk: ExtendedFullViewingKey, +/// Represents the collection of keys that comprise extended spending keys, +/// extended full viewing keys, and proof generation keys. I.e. ask, ak, nsk, +/// nk, ovk, dk, and chain_code. Mathematical relations between these keys are +/// not assumed; i.e. it's not necessarily the case that +/// nk = PROOF_GENERATION_KEY_GENERATOR * nsk or +/// ak = PROOF_GENERATION_KEY_GENERATOR * ask. +pub trait ExtendedKey { + /// Group this collection of keys into an extended full viewing key + fn to_viewing_key(&self) -> ExtendedFullViewingKey; + + /// Group this collection of keys into a proof generation key. Use + /// to_viewing_key when the key nk is required since there's no mathematical + /// relation between nk and nsk in this collection. + fn to_proof_generation_key(&self) -> Option; + + /// Group this collection of keys into an extended spending key. Use + /// to_viewing_key when the keys nk and ak are required since there's no + /// mathematical relation between nk and nsk nor ak and ask in this + /// collection. + fn to_spending_key(&self) -> Option; } -impl PseudoExtendedSpendingKey { - /// Construct a pseudo extended spending key from an extended spending key +/// Represent an extended full viewing key as a collection of keys. +impl ExtendedKey for ExtendedFullViewingKey { + /// Return this key + fn to_viewing_key(&self) -> ExtendedFullViewingKey { + *self + } + + /// Return None since there is insufficient data to construct proof + /// generation key + fn to_proof_generation_key(&self) -> Option { + None + } + + /// Return None since there is insufficient data to construct spending key #[allow(deprecated)] - pub fn from_spending_key(xsk: ExtendedSpendingKey) -> Self { - Self { - xfvk: xsk.to_extended_full_viewing_key(), - xsk, - } + fn to_spending_key(&self) -> Option { + None } - /// Construct a pseudo extended spending key from an extended full viewing key - pub fn from_viewing_key(xfvk: ExtendedFullViewingKey) -> Self { - let xsk = ExtendedSpendingKey { - depth: xfvk.depth, - parent_fvk_tag: xfvk.parent_fvk_tag, - child_index: xfvk.child_index, - chain_code: xfvk.chain_code, - dk: xfvk.dk, - expsk: ExpandedSpendingKey { - ask: jubjub::Fr::default(), - nsk: jubjub::Fr::default(), - ovk: xfvk.fvk.ovk, - }, - }; - Self { xsk, xfvk } +} + +/// Represents an extended spending key as a collection of keys. +impl ExtendedKey for ExtendedSpendingKey { + /// Returns the Sapling derivation of an extended full viewing key from this + /// key + fn to_viewing_key(&self) -> ExtendedFullViewingKey { + self.into() + } + + /// Returns the Sapling derivation of a proof generation key from this key + fn to_proof_generation_key(&self) -> Option { + Some(self.expsk.proof_generation_key()) } - /// Return the extended spending key that generates the contained view key + + /// Returns this key #[allow(deprecated)] - pub fn to_spending_key(&self) -> Option { - if self.xsk.to_extended_full_viewing_key() == self.xfvk { - Some(self.xsk) - } else { - None - } + fn to_spending_key(&self) -> Option { + Some(*self) } - /// Return the viewing key for which this key provides authorizations - pub fn to_viewing_key(&self) -> ExtendedFullViewingKey { - self.xfvk +} + +/// An extended full viewing key bundled with partial authorizations +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] +#[derive(Clone, PartialEq, Eq, Copy, Debug)] +pub struct PseudoExtendedKey { + /// Viewing key for which this key provides authorizations + xfvk: ExtendedFullViewingKey, + /// Spend authorizing key + ask: Option, + /// Proof authorizing key + nsk: Option, +} + +impl Hash for PseudoExtendedKey { + fn hash(&self, state: &mut H) { + self.ask.map(|ask| ask.to_bytes()).hash(state); + self.nsk.map(|nsk| nsk.to_bytes()).hash(state); + self.xfvk.hash(state); } - /// Augment this spending key with proof generation data - pub fn augment(&mut self, pgk: ProofGenerationKey) -> Result<(), ()> { +} + +impl PseudoExtendedKey { + /// Augment this spending key with proof generation data. Fails if the proof + /// generation key is inconsistent with this key. + pub fn augment_proof_generation_key(&mut self, pgk: ProofGenerationKey) -> Result<(), ()> { let nk = NullifierDerivingKey(PROOF_GENERATION_KEY_GENERATOR * pgk.nsk); - if nk == self.xfvk.fvk.vk.nk { - self.xsk.expsk.nsk = pgk.nsk; + if nk == self.xfvk.fvk.vk.nk && pgk.ak == self.xfvk.fvk.vk.ak { + self.nsk = Some(pgk.nsk); Ok(()) } else { Err(()) } } - /// Return a key whose only guarantee is correct proof generation - pub fn partial_spending_key(&self) -> Option { - let nk = NullifierDerivingKey(PROOF_GENERATION_KEY_GENERATOR * self.xsk.expsk.nsk); - if nk == self.xfvk.fvk.vk.nk { - Some(self.xsk) + + /// Augment this this extended key with spend authorization data. Fails if + /// spend authorizing key is inconsistent with this key. + pub fn augment_spend_authorizing_key(&mut self, ask: PrivateKey) -> Result<(), ()> { + let ak = SPENDING_KEY_GENERATOR * ask.0; + if ak == self.xfvk.fvk.vk.ak { + self.ask = Some(ask.0); + Ok(()) } else { - None + Err(()) + } + } + + /// Augment this this extended key with spend authorization data without + /// applying consistency checks + pub fn augment_spend_authorizing_key_unchecked(&mut self, ask: PrivateKey) { + self.ask = Some(ask.0); + } +} + +impl ExtendedKey for PseudoExtendedKey { + /// Returns the extended full viewing key contained in this object + fn to_viewing_key(&self) -> ExtendedFullViewingKey { + self.xfvk + } + + /// Bundle this object into a proof generation key if a proof authorization + /// key has been augmented to this object. + fn to_proof_generation_key(&self) -> Option { + self.nsk.map(|nsk| ProofGenerationKey { + nsk, + ak: self.xfvk.fvk.vk.ak, + }) + } + + /// Bundle this object into an extended spending key if a spend + /// authorization key has been augmented to this object. + #[allow(deprecated)] + fn to_spending_key(&self) -> Option { + self.ask + .zip(self.nsk) + .map(|(ask, nsk)| ExtendedSpendingKey { + depth: self.xfvk.depth, + parent_fvk_tag: self.xfvk.parent_fvk_tag, + child_index: self.xfvk.child_index, + chain_code: self.xfvk.chain_code, + dk: self.xfvk.dk, + expsk: ExpandedSpendingKey { + ask, + nsk, + ovk: self.xfvk.fvk.ovk, + }, + }) + } +} + +impl From for PseudoExtendedKey { + /// Construct a pseudo extended spending key from an extended spending key + #[allow(deprecated)] + fn from(xsk: ExtendedSpendingKey) -> Self { + Self { + xfvk: xsk.to_extended_full_viewing_key(), + ask: Some(xsk.expsk.ask), + nsk: Some(xsk.expsk.nsk), + } + } +} + +impl From for PseudoExtendedKey { + /// Construct a pseudo extended spending key from an extended full viewing key + #[allow(deprecated)] + fn from(xfvk: ExtendedFullViewingKey) -> Self { + Self { + ask: None, + nsk: None, + xfvk, } } }