From 01618038bfa274094d1c3a6dd20b61b6b7891371 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 18 Nov 2018 11:20:59 +0000 Subject: [PATCH 01/19] TxProver trait to abstract over the circuit parameters An implementation using local parameters is provided in the zcash_proofs crate. --- Cargo.lock | 33 +++ zcash_primitives/src/lib.rs | 1 + zcash_primitives/src/prover.rs | 162 +++++++++++++++ .../src/transaction/components.rs | 2 +- zcash_proofs/Cargo.toml | 1 + zcash_proofs/src/lib.rs | 2 + zcash_proofs/src/prover.rs | 190 ++++++++++++++++++ 7 files changed, 390 insertions(+), 1 deletion(-) create mode 100644 zcash_primitives/src/prover.rs create mode 100644 zcash_proofs/src/prover.rs diff --git a/Cargo.lock b/Cargo.lock index fac19e3..a9a4f97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -185,6 +185,15 @@ dependencies = [ "generic-array 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "directories" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "libc 0.2.59 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "fake-simd" version = "0.1.2" @@ -531,6 +540,25 @@ name = "unicode-xid" version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "winapi" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "zcash_client_backend" version = "0.0.0" @@ -569,6 +597,7 @@ dependencies = [ "bellman 0.1.0", "blake2b_simd 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "directories 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", "ff 0.4.0", "pairing 0.14.2", "rand_os 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -600,6 +629,7 @@ dependencies = [ "checksum crypto_api_chachapoly 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "2f9ee35dbace0831b5fe7cb9b43eb029aa14a10f594a115025d4628a2baa63ab" "checksum digest 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)" = "00a49051fef47a72c9623101b19bd71924a45cca838826caae3eaa4d00772603" "checksum digest 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "05f47366984d3ad862010e22c7ce81a7dbcaebbdfb37241a620f8b6596ee135c" +"checksum directories 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "72d337a64190607d4fcca2cb78982c5dd57f4916e19696b48a575fa746b6cb0f" "checksum fake-simd 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed" "checksum fpe 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ce3371c82bfbd984f624cab093f55e7336f5a6e589f8518e1258f54f011b89ad" "checksum futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)" = "1a70b146671de62ec8c8ed572219ca5d594d9b06c0b364d5e67b722fc559b48c" @@ -636,3 +666,6 @@ dependencies = [ "checksum syn 0.14.9 (registry+https://github.com/rust-lang/crates.io-index)" = "261ae9ecaa397c42b960649561949d69311f08eeaea86a65696e6e46517cf741" "checksum typenum 1.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "612d636f949607bdf9b123b4a6f6d966dedf3ff669f7f045890d3a4a73948169" "checksum unicode-xid 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fc72304796d0818e357ead4e000d19c9c174ab23dc11093ac919054d20a6a7fc" +"checksum winapi 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)" = "f10e386af2b13e47c89e7236a7a14a086791a2b88ebad6df9bf42040195cf770" +"checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +"checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" diff --git a/zcash_primitives/src/lib.rs b/zcash_primitives/src/lib.rs index 90d69e6..797b9b6 100644 --- a/zcash_primitives/src/lib.rs +++ b/zcash_primitives/src/lib.rs @@ -20,6 +20,7 @@ pub mod block; pub mod keys; pub mod merkle_tree; pub mod note_encryption; +pub mod prover; pub mod sapling; mod serialize; pub mod transaction; diff --git a/zcash_primitives/src/prover.rs b/zcash_primitives/src/prover.rs new file mode 100644 index 0000000..87a76c7 --- /dev/null +++ b/zcash_primitives/src/prover.rs @@ -0,0 +1,162 @@ +//! Abstractions over the proving system and parameters. + +use pairing::bls12_381::{Bls12, Fr}; +use sapling_crypto::{ + jubjub::{edwards, fs::Fs, Unknown}, + primitives::{Diversifier, PaymentAddress, ProofGenerationKey}, + redjubjub::{PublicKey, Signature}, +}; + +use crate::{ + merkle_tree::CommitmentTreeWitness, sapling::Node, transaction::components::GROTH_PROOF_SIZE, +}; + +/// Interface for creating zero-knowledge proofs for shielded transactions. +pub trait TxProver { + /// Type for persisting any necessary context across multiple Sapling proofs. + type SaplingProvingContext; + + /// Instantiate a new Sapling proving context. + fn new_sapling_proving_context(&self) -> Self::SaplingProvingContext; + + /// Create the value commitment, re-randomized key, and proof for a Sapling + /// [`SpendDescription`], while accumulating its value commitment randomness inside + /// the context for later use. + /// + /// [`SpendDescription`]: crate::transaction::components::SpendDescription + fn spend_proof( + &self, + ctx: &mut Self::SaplingProvingContext, + proof_generation_key: ProofGenerationKey, + diversifier: Diversifier, + rcm: Fs, + ar: Fs, + value: u64, + anchor: Fr, + witness: CommitmentTreeWitness, + ) -> Result< + ( + [u8; GROTH_PROOF_SIZE], + edwards::Point, + PublicKey, + ), + (), + >; + + /// Create the value commitment and proof for a Sapling [`OutputDescription`], + /// while accumulating its value commitment randomness inside the context for later + /// use. + /// + /// [`OutputDescription`]: crate::transaction::components::OutputDescription + fn output_proof( + &self, + ctx: &mut Self::SaplingProvingContext, + esk: Fs, + payment_address: PaymentAddress, + rcm: Fs, + value: u64, + ) -> ([u8; GROTH_PROOF_SIZE], edwards::Point); + + /// Create the `bindingSig` for a Sapling transaction. All calls to + /// [`TxProver::spend_proof`] and [`TxProver::output_proof`] must be completed before + /// calling this function. + fn binding_sig( + &self, + ctx: &mut Self::SaplingProvingContext, + value_balance: i64, + sighash: &[u8; 32], + ) -> Result; +} + +#[cfg(test)] +pub(crate) mod mock { + use ff::Field; + use pairing::bls12_381::{Bls12, Fr}; + use rand_os::OsRng; + use sapling_crypto::{ + jubjub::{edwards, fs::Fs, FixedGenerators, Unknown}, + primitives::{Diversifier, PaymentAddress, ProofGenerationKey, ValueCommitment}, + redjubjub::{PublicKey, Signature}, + }; + + use crate::{ + merkle_tree::CommitmentTreeWitness, sapling::Node, + transaction::components::GROTH_PROOF_SIZE, JUBJUB, + }; + + use super::TxProver; + + pub(crate) struct MockTxProver; + + #[cfg(test)] + impl TxProver for MockTxProver { + type SaplingProvingContext = (); + + fn new_sapling_proving_context(&self) -> Self::SaplingProvingContext {} + + fn spend_proof( + &self, + _ctx: &mut Self::SaplingProvingContext, + proof_generation_key: ProofGenerationKey, + _diversifier: Diversifier, + _rcm: Fs, + ar: Fs, + value: u64, + _anchor: Fr, + _witness: CommitmentTreeWitness, + ) -> Result< + ( + [u8; GROTH_PROOF_SIZE], + edwards::Point, + PublicKey, + ), + (), + > { + let mut rng = OsRng; + + let cv = ValueCommitment:: { + value, + randomness: Fs::random(&mut rng), + } + .cm(&JUBJUB) + .into(); + + let rk = PublicKey::(proof_generation_key.ak.clone().into()).randomize( + ar, + FixedGenerators::SpendingKeyGenerator, + &JUBJUB, + ); + + Ok(([0u8; GROTH_PROOF_SIZE], cv, rk)) + } + + fn output_proof( + &self, + _ctx: &mut Self::SaplingProvingContext, + _esk: Fs, + _payment_address: PaymentAddress, + _rcm: Fs, + value: u64, + ) -> ([u8; GROTH_PROOF_SIZE], edwards::Point) { + let mut rng = OsRng; + + let cv = ValueCommitment:: { + value, + randomness: Fs::random(&mut rng), + } + .cm(&JUBJUB) + .into(); + + ([0u8; GROTH_PROOF_SIZE], cv) + } + + fn binding_sig( + &self, + _ctx: &mut Self::SaplingProvingContext, + _value_balance: i64, + _sighash: &[u8; 32], + ) -> Result { + Err(()) + } + } +} diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index b8153f3..141d0ff 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -11,7 +11,7 @@ use serialize::Vector; use JUBJUB; // π_A + π_B + π_C -const GROTH_PROOF_SIZE: usize = (48 + 96 + 48); +pub const GROTH_PROOF_SIZE: usize = (48 + 96 + 48); // π_A + π_A' + π_B + π_B' + π_C + π_C' + π_K + π_H const PHGR_PROOF_SIZE: usize = (33 + 33 + 65 + 33 + 33 + 33 + 33 + 33); diff --git a/zcash_proofs/Cargo.toml b/zcash_proofs/Cargo.toml index 1eca6fd..9e989fa 100644 --- a/zcash_proofs/Cargo.toml +++ b/zcash_proofs/Cargo.toml @@ -9,6 +9,7 @@ authors = [ bellman = { path = "../bellman" } blake2b_simd = "0.5" byteorder = "1" +directories = "1" ff = { path = "../ff" } pairing = { path = "../pairing" } rand_os = "0.2" diff --git a/zcash_proofs/src/lib.rs b/zcash_proofs/src/lib.rs index dd6975c..26aa2f1 100644 --- a/zcash_proofs/src/lib.rs +++ b/zcash_proofs/src/lib.rs @@ -1,6 +1,7 @@ extern crate bellman; extern crate blake2b_simd; extern crate byteorder; +extern crate directories; extern crate ff; extern crate pairing; extern crate rand_os; @@ -14,6 +15,7 @@ use std::io::{self, BufReader}; use std::path::Path; mod hashreader; +pub mod prover; pub mod sapling; pub fn load_parameters( diff --git a/zcash_proofs/src/prover.rs b/zcash_proofs/src/prover.rs new file mode 100644 index 0000000..005d7eb --- /dev/null +++ b/zcash_proofs/src/prover.rs @@ -0,0 +1,190 @@ +//! Abstractions over the proving system and parameters for ease of use. + +use bellman::groth16::{Parameters, PreparedVerifyingKey}; +use directories::BaseDirs; +use pairing::bls12_381::{Bls12, Fr}; +use sapling_crypto::{ + jubjub::{edwards, fs::Fs, Unknown}, + primitives::{Diversifier, PaymentAddress, ProofGenerationKey}, + redjubjub::{PublicKey, Signature}, +}; +use std::path::Path; +use zcash_primitives::{ + merkle_tree::CommitmentTreeWitness, prover::TxProver, sapling::Node, + transaction::components::GROTH_PROOF_SIZE, JUBJUB, +}; + +use crate::{load_parameters, sapling::SaplingProvingContext}; + +const SAPLING_SPEND_HASH: &str = "8270785a1a0d0bc77196f000ee6d221c9c9894f55307bd9357c3f0105d31ca63991ab91324160d8f53e2bbd3c2633a6eb8bdf5205d822e7f3f73edac51b2b70c"; +const SAPLING_OUTPUT_HASH: &str = "657e3d38dbb5cb5e7dd2970e8b03d69b4787dd907285b5a7f0790dcc8072f60bf593b32cc2d1c030e00ff5ae64bf84c5c3beb84ddc841d48264b4a171744d028"; + +/// An implementation of [`TxProver`] using Sapling Spend and Output parameters from +/// locally-accessible paths. +pub struct LocalTxProver { + spend_params: Parameters, + spend_vk: PreparedVerifyingKey, + output_params: Parameters, +} + +impl LocalTxProver { + /// Creates a `LocalTxProver` using parameters from the given local paths. + /// + /// # Examples + /// + /// ```should_panic + /// use std::path::Path; + /// use zcash_proofs::prover::LocalTxProver; + /// + /// let tx_prover = LocalTxProver::new( + /// Path::new("/path/to/sapling-spend.params"), + /// Path::new("/path/to/sapling-output.params"), + /// ); + /// ``` + /// + /// # Panics + /// + /// This function will panic if the paths do not point to valid parameter files with + /// the expected hashes. + pub fn new(spend_path: &Path, output_path: &Path) -> Self { + let (spend_params, spend_vk, output_params, _, _) = load_parameters( + spend_path, + SAPLING_SPEND_HASH, + output_path, + SAPLING_OUTPUT_HASH, + None, + None, + ); + LocalTxProver { + spend_params, + spend_vk, + output_params, + } + } + + /// Attempts to create a `LocalTxProver` using parameters from the default local + /// location. + /// + /// Returns `None` if any of the parameters cannot be found in the default local + /// location. + /// + /// # Examples + /// + /// ``` + /// use zcash_proofs::prover::LocalTxProver; + /// + /// match LocalTxProver::with_default_location() { + /// Some(tx_prover) => (), + /// None => println!("Please run zcash-fetch-params or fetch-params.sh to download the parameters."), + /// } + /// ``` + /// + /// # Panics + /// + /// This function will panic if the parameters in the default local location do not + /// have the expected hashes. + pub fn with_default_location() -> Option { + let base_dirs = BaseDirs::new()?; + let unix_params_dir = base_dirs.home_dir().join(".zcash-params"); + let win_osx_params_dir = base_dirs.data_dir().join("ZcashParams"); + let (spend_path, output_path) = if unix_params_dir.exists() { + ( + unix_params_dir.join("sapling-spend.params"), + unix_params_dir.join("sapling-output.params"), + ) + } else if win_osx_params_dir.exists() { + ( + win_osx_params_dir.join("sapling-spend.params"), + win_osx_params_dir.join("sapling-output.params"), + ) + } else { + return None; + }; + if !(spend_path.exists() && output_path.exists()) { + return None; + } + + Some(LocalTxProver::new(&spend_path, &output_path)) + } +} + +impl TxProver for LocalTxProver { + type SaplingProvingContext = SaplingProvingContext; + + fn new_sapling_proving_context(&self) -> Self::SaplingProvingContext { + SaplingProvingContext::new() + } + + fn spend_proof( + &self, + ctx: &mut Self::SaplingProvingContext, + proof_generation_key: ProofGenerationKey, + diversifier: Diversifier, + rcm: Fs, + ar: Fs, + value: u64, + anchor: Fr, + witness: CommitmentTreeWitness, + ) -> Result< + ( + [u8; GROTH_PROOF_SIZE], + edwards::Point, + PublicKey, + ), + (), + > { + let (proof, cv, rk) = ctx.spend_proof( + proof_generation_key, + diversifier, + rcm, + ar, + value, + anchor, + witness, + &self.spend_params, + &self.spend_vk, + &JUBJUB, + )?; + + let mut zkproof = [0u8; GROTH_PROOF_SIZE]; + proof + .write(&mut zkproof[..]) + .expect("should be able to serialize a proof"); + + Ok((zkproof, cv, rk)) + } + + fn output_proof( + &self, + ctx: &mut Self::SaplingProvingContext, + esk: Fs, + payment_address: PaymentAddress, + rcm: Fs, + value: u64, + ) -> ([u8; GROTH_PROOF_SIZE], edwards::Point) { + let (proof, cv) = ctx.output_proof( + esk, + payment_address, + rcm, + value, + &self.output_params, + &JUBJUB, + ); + + let mut zkproof = [0u8; GROTH_PROOF_SIZE]; + proof + .write(&mut zkproof[..]) + .expect("should be able to serialize a proof"); + + (zkproof, cv) + } + + fn binding_sig( + &self, + ctx: &mut Self::SaplingProvingContext, + value_balance: i64, + sighash: &[u8; 32], + ) -> Result { + ctx.binding_sig(value_balance, sighash, &JUBJUB) + } +} From 1862354ea6d43ed5619b2dd2dbe29eb8442a25bc Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 20 Nov 2018 13:37:21 +0000 Subject: [PATCH 02/19] Sapling transaction builder --- Cargo.lock | 1 + zcash_primitives/Cargo.toml | 1 + zcash_primitives/src/lib.rs | 1 + zcash_primitives/src/merkle_tree.rs | 2 +- zcash_primitives/src/note_encryption.rs | 2 +- zcash_primitives/src/transaction/builder.rs | 615 ++++++++++++++++++++ zcash_primitives/src/transaction/mod.rs | 1 + 7 files changed, 621 insertions(+), 2 deletions(-) create mode 100644 zcash_primitives/src/transaction/builder.rs diff --git a/Cargo.lock b/Cargo.lock index a9a4f97..fc8644f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -584,6 +584,7 @@ dependencies = [ "hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "pairing 0.14.2", + "rand 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand_core 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand_os 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "sapling-crypto 0.0.1", diff --git a/zcash_primitives/Cargo.toml b/zcash_primitives/Cargo.toml index 56070e4..804f8c6 100644 --- a/zcash_primitives/Cargo.toml +++ b/zcash_primitives/Cargo.toml @@ -15,6 +15,7 @@ fpe = "0.1" hex = "0.3" lazy_static = "1" pairing = { path = "../pairing" } +rand = "0.7" rand_core = "0.5" rand_os = "0.2" sapling-crypto = { path = "../sapling-crypto" } diff --git a/zcash_primitives/src/lib.rs b/zcash_primitives/src/lib.rs index 797b9b6..d6ddcc9 100644 --- a/zcash_primitives/src/lib.rs +++ b/zcash_primitives/src/lib.rs @@ -9,6 +9,7 @@ extern crate ff; extern crate fpe; extern crate hex; extern crate pairing; +extern crate rand; extern crate rand_core; extern crate rand_os; extern crate sapling_crypto; diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs index a692073..aa1596d 100644 --- a/zcash_primitives/src/merkle_tree.rs +++ b/zcash_primitives/src/merkle_tree.rs @@ -420,7 +420,7 @@ impl IncrementalWitness { /// A witness to a path from a position in a particular commitment tree to the root of /// that tree. -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct CommitmentTreeWitness { pub auth_path: Vec>, pub position: u64, diff --git a/zcash_primitives/src/note_encryption.rs b/zcash_primitives/src/note_encryption.rs index 1964361..d0a7250 100644 --- a/zcash_primitives/src/note_encryption.rs +++ b/zcash_primitives/src/note_encryption.rs @@ -135,7 +135,7 @@ impl Memo { } } -fn generate_esk() -> Fs { +pub fn generate_esk() -> Fs { // create random 64 byte buffer let mut rng = OsRng; let mut buffer = [0u8; 64]; diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs new file mode 100644 index 0000000..9f6cccd --- /dev/null +++ b/zcash_primitives/src/transaction/builder.rs @@ -0,0 +1,615 @@ +//! Structs for building transactions. + +use ff::Field; +use pairing::bls12_381::{Bls12, Fr}; +use rand::{rngs::OsRng, seq::SliceRandom, CryptoRng, RngCore}; +use sapling_crypto::{ + jubjub::fs::Fs, + primitives::{Diversifier, Note, PaymentAddress}, + redjubjub::PrivateKey, +}; +use zip32::ExtendedSpendingKey; + +use crate::{ + keys::OutgoingViewingKey, + merkle_tree::{CommitmentTreeWitness, IncrementalWitness}, + note_encryption::{generate_esk, Memo, SaplingNoteEncryption}, + prover::TxProver, + sapling::{spend_sig, Node}, + transaction::{ + components::{Amount, OutputDescription, SpendDescription}, + signature_hash_data, Transaction, TransactionData, SIGHASH_ALL, + }, + JUBJUB, +}; + +const DEFAULT_FEE: Amount = Amount(10000); +const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; + +/// If there are any shielded inputs, always have at least two shielded outputs, padding +/// with dummy outputs if necessary. See https://github.com/zcash/zcash/issues/3615 +const MIN_SHIELDED_OUTPUTS: usize = 2; + +#[derive(Debug)] +pub struct Error(ErrorKind); + +impl Error { + pub fn kind(&self) -> &ErrorKind { + &self.0 + } +} + +#[derive(Debug, PartialEq)] +pub enum ErrorKind { + AnchorMismatch, + BindingSig, + ChangeIsNegative(i64), + InvalidAddress, + InvalidAmount, + InvalidWitness, + NoChangeAddress, + SpendProof, +} + +struct SpendDescriptionInfo { + extsk: ExtendedSpendingKey, + diversifier: Diversifier, + note: Note, + alpha: Fs, + witness: CommitmentTreeWitness, +} + +pub struct SaplingOutput { + ovk: OutgoingViewingKey, + to: PaymentAddress, + note: Note, + memo: Memo, +} + +impl SaplingOutput { + pub fn new( + rng: &mut R, + ovk: OutgoingViewingKey, + to: PaymentAddress, + value: Amount, + memo: Option, + ) -> Result { + let g_d = match to.g_d(&JUBJUB) { + Some(g_d) => g_d, + None => return Err(Error(ErrorKind::InvalidAddress)), + }; + if value.0 < 0 { + return Err(Error(ErrorKind::InvalidAmount)); + } + + let rcm = Fs::random(rng); + + let note = Note { + g_d, + pk_d: to.pk_d.clone(), + value: value.0 as u64, + r: rcm, + }; + + Ok(SaplingOutput { + ovk, + to, + note, + memo: memo.unwrap_or_default(), + }) + } + + pub fn build( + self, + prover: &P, + ctx: &mut P::SaplingProvingContext, + ) -> OutputDescription { + let encryptor = + SaplingNoteEncryption::new(self.ovk, self.note.clone(), self.to.clone(), self.memo); + + let (zkproof, cv) = prover.output_proof( + ctx, + encryptor.esk().clone(), + self.to, + self.note.r, + self.note.value, + ); + + let cmu = self.note.cm(&JUBJUB); + + let enc_ciphertext = encryptor.encrypt_note_plaintext(); + let out_ciphertext = encryptor.encrypt_outgoing_plaintext(&cv, &cmu); + + let ephemeral_key = encryptor.epk().clone().into(); + + OutputDescription { + cv, + cmu, + ephemeral_key, + enc_ciphertext, + out_ciphertext, + zkproof, + } + } +} + +/// Metadata about a transaction created by a [`Builder`]. +pub struct TransactionMetadata { + spend_indices: Vec, + output_indices: Vec, +} + +impl TransactionMetadata { + fn new() -> Self { + TransactionMetadata { + spend_indices: vec![], + output_indices: vec![], + } + } + + /// Returns the index within the transaction of the [`SpendDescription`] corresponding + /// to the `n`-th call to [`Builder::add_sapling_spend`]. + /// + /// Note positions are randomized when building transactions for indistinguishability. + /// This means that the transaction consumer cannot assume that e.g. the first spend + /// they added (via the first call to [`Builder::add_sapling_spend`]) is the first + /// [`SpendDescription`] in the transaction. + pub fn spend_index(&self, n: usize) -> Option { + self.spend_indices.get(n).map(|i| *i) + } + + /// Returns the index within the transaction of the [`OutputDescription`] corresponding + /// to the `n`-th call to [`Builder::add_sapling_output`]. + /// + /// Note positions are randomized when building transactions for indistinguishability. + /// This means that the transaction consumer cannot assume that e.g. the first output + /// they added (via the first call to [`Builder::add_sapling_output`]) is the first + /// [`OutputDescription`] in the transaction. + pub fn output_index(&self, n: usize) -> Option { + self.output_indices.get(n).map(|i| *i) + } +} + +/// Generates a [`Transaction`] from its inputs and outputs. +pub struct Builder { + rng: OsRng, + mtx: TransactionData, + fee: Amount, + anchor: Option, + spends: Vec, + outputs: Vec, + change_address: Option<(OutgoingViewingKey, PaymentAddress)>, +} + +impl Builder { + /// Creates a new `Builder` targeted for inclusion in the block with the given height, + /// using default values for general transaction fields. + /// + /// # Default values + /// + /// The expiry height will be set to the given height plus the default transaction + /// expiry delta (20 blocks). + /// + /// The fee will be set to the default fee (0.0001 ZEC). + pub fn new(height: u32) -> Builder { + let mut mtx = TransactionData::new(); + mtx.expiry_height = height + DEFAULT_TX_EXPIRY_DELTA; + + Builder { + rng: OsRng, + mtx, + fee: DEFAULT_FEE, + anchor: None, + spends: vec![], + outputs: vec![], + change_address: None, + } + } + + /// Adds a Sapling note to be spent in this transaction. + /// + /// Returns an error if the given witness does not have the same anchor as previous + /// witnesses, or has no path. + pub fn add_sapling_spend( + &mut self, + extsk: ExtendedSpendingKey, + diversifier: Diversifier, + note: Note, + witness: IncrementalWitness, + ) -> Result<(), Error> { + // Consistency check: all anchors must equal the first one + if let Some(anchor) = self.anchor { + let witness_root: Fr = witness.root().into(); + if witness_root != anchor { + return Err(Error(ErrorKind::AnchorMismatch)); + } + } else { + self.anchor = Some(witness.root().into()) + } + let witness = witness.path().ok_or(Error(ErrorKind::InvalidWitness))?; + + let alpha = Fs::random(&mut self.rng); + + self.mtx.value_balance.0 += note.value as i64; + + self.spends.push(SpendDescriptionInfo { + extsk, + diversifier, + note, + alpha, + witness, + }); + + Ok(()) + } + + /// Adds a Sapling address to send funds to. + pub fn add_sapling_output( + &mut self, + ovk: OutgoingViewingKey, + to: PaymentAddress, + value: Amount, + memo: Option, + ) -> Result<(), Error> { + let output = SaplingOutput::new(&mut self.rng, ovk, to, value, memo)?; + + self.mtx.value_balance.0 -= value.0; + + self.outputs.push(output); + + Ok(()) + } + + /// Sets the Sapling address to which any change will be sent. + /// + /// By default, change is sent to the Sapling address corresponding to the first note + /// being spent (i.e. the first call to [`Builder::add_sapling_spend`]). + pub fn send_change_to(&mut self, ovk: OutgoingViewingKey, to: PaymentAddress) { + self.change_address = Some((ovk, to)); + } + + /// Builds a transaction from the configured spends and outputs. + /// + /// Upon success, returns a tuple containing the final transaction, and the + /// [`TransactionMetadata`] generated during the build process. + /// + /// `consensus_branch_id` must be valid for the block height that this transaction is + /// targeting. An invalid `consensus_branch_id` will *not* result in an error from + /// this function, and instead will generate a transaction that will be rejected by + /// the network. + pub fn build( + mut self, + consensus_branch_id: u32, + prover: impl TxProver, + ) -> Result<(Transaction, TransactionMetadata), Error> { + let mut tx_metadata = TransactionMetadata::new(); + + // + // Consistency checks + // + + // Valid change + let change = self.mtx.value_balance.0 - self.fee.0; + if change.is_negative() { + return Err(Error(ErrorKind::ChangeIsNegative(change))); + } + + // + // Change output + // + + if change.is_positive() { + // Send change to the specified change address. If no change address + // was set, send change to the first Sapling address given as input. + let change_address = if let Some(change_address) = self.change_address.take() { + change_address + } else if !self.spends.is_empty() { + ( + self.spends[0].extsk.expsk.ovk, + PaymentAddress { + diversifier: self.spends[0].diversifier, + pk_d: self.spends[0].note.pk_d.clone(), + }, + ) + } else { + return Err(Error(ErrorKind::NoChangeAddress)); + }; + + self.add_sapling_output(change_address.0, change_address.1, Amount(change), None)?; + } + + // + // Record initial positions of spends and outputs + // + let mut spends: Vec<_> = self.spends.into_iter().enumerate().collect(); + let mut outputs: Vec<_> = self + .outputs + .into_iter() + .enumerate() + .map(|(i, o)| Some((i, o))) + .collect(); + + // + // Sapling spends and outputs + // + + let mut ctx = prover.new_sapling_proving_context(); + let anchor = self.anchor.expect("anchor was set if spends were added"); + + // Pad Sapling outputs + let orig_outputs_len = outputs.len(); + if !spends.is_empty() { + while outputs.len() < MIN_SHIELDED_OUTPUTS { + outputs.push(None); + } + } + + // Randomize order of inputs and outputs + spends.shuffle(&mut self.rng); + outputs.shuffle(&mut self.rng); + tx_metadata.spend_indices.resize(spends.len(), 0); + tx_metadata.output_indices.resize(orig_outputs_len, 0); + + // Create Sapling SpendDescriptions + for (i, (pos, spend)) in spends.iter().enumerate() { + let proof_generation_key = spend.extsk.expsk.proof_generation_key(&JUBJUB); + + let mut nullifier = [0u8; 32]; + nullifier.copy_from_slice(&spend.note.nf( + &proof_generation_key.into_viewing_key(&JUBJUB), + spend.witness.position, + &JUBJUB, + )); + + let (zkproof, cv, rk) = prover + .spend_proof( + &mut ctx, + proof_generation_key, + spend.diversifier, + spend.note.r, + spend.alpha, + spend.note.value, + anchor, + spend.witness.clone(), + ) + .map_err(|()| Error(ErrorKind::SpendProof))?; + + self.mtx.shielded_spends.push(SpendDescription { + cv, + anchor: anchor, + nullifier, + rk, + zkproof, + spend_auth_sig: None, + }); + + // Record the post-randomized spend location + tx_metadata.spend_indices[*pos] = i; + } + + // Create Sapling OutputDescriptions + for (i, output) in outputs.into_iter().enumerate() { + let output_desc = if let Some((pos, output)) = output { + // Record the post-randomized output location + tx_metadata.output_indices[pos] = i; + + output.build(&prover, &mut ctx) + } else { + // This is a dummy output + let (dummy_to, dummy_note) = { + let (diversifier, g_d) = { + let mut diversifier; + let g_d; + loop { + let mut d = [0; 11]; + self.rng.fill_bytes(&mut d); + diversifier = Diversifier(d); + if let Some(val) = diversifier.g_d::(&JUBJUB) { + g_d = val; + break; + } + } + (diversifier, g_d) + }; + + let pk_d = { + let dummy_ivk = Fs::random(&mut self.rng); + g_d.mul(dummy_ivk, &JUBJUB) + }; + + ( + PaymentAddress { + diversifier, + pk_d: pk_d.clone(), + }, + Note { + g_d, + pk_d, + r: Fs::random(&mut self.rng), + value: 0, + }, + ) + }; + + let esk = generate_esk(); + let epk = dummy_note.g_d.mul(esk, &JUBJUB); + + let (zkproof, cv) = + prover.output_proof(&mut ctx, esk, dummy_to, dummy_note.r, dummy_note.value); + + let cmu = dummy_note.cm(&JUBJUB); + + let mut enc_ciphertext = [0u8; 580]; + let mut out_ciphertext = [0u8; 80]; + self.rng.fill_bytes(&mut enc_ciphertext[..]); + self.rng.fill_bytes(&mut out_ciphertext[..]); + + OutputDescription { + cv, + cmu, + ephemeral_key: epk.into(), + enc_ciphertext, + out_ciphertext, + zkproof, + } + }; + + self.mtx.shielded_outputs.push(output_desc); + } + + // + // Signatures + // + + let mut sighash = [0u8; 32]; + sighash.copy_from_slice(&signature_hash_data( + &self.mtx, + consensus_branch_id, + SIGHASH_ALL, + None, + )); + + // Create Sapling spendAuth and binding signatures + for (i, (_, spend)) in spends.into_iter().enumerate() { + self.mtx.shielded_spends[i].spend_auth_sig = Some(spend_sig( + PrivateKey(spend.extsk.expsk.ask), + spend.alpha, + &sighash, + &JUBJUB, + )); + } + self.mtx.binding_sig = Some( + prover + .binding_sig(&mut ctx, self.mtx.value_balance.0, &sighash) + .map_err(|()| Error(ErrorKind::BindingSig))?, + ); + + Ok(( + self.mtx.freeze().expect("Transaction should be complete"), + tx_metadata, + )) + } +} + +#[cfg(test)] +mod tests { + use ff::{Field, PrimeField}; + use rand::rngs::OsRng; + use sapling_crypto::jubjub::fs::Fs; + + use super::{Builder, ErrorKind}; + use crate::{ + merkle_tree::{CommitmentTree, IncrementalWitness}, + prover::mock::MockTxProver, + sapling::Node, + transaction::components::Amount, + zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}, + JUBJUB, + }; + + #[test] + fn fails_on_negative_output() { + let extsk = ExtendedSpendingKey::master(&[]); + let extfvk = ExtendedFullViewingKey::from(&extsk); + let ovk = extfvk.fvk.ovk; + let to = extfvk.default_address().unwrap().1; + + let mut builder = Builder::new(0); + match builder.add_sapling_output(ovk, to, Amount(-1), None) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), + Ok(_) => panic!("Should have failed"), + } + } + + #[test] + fn fails_on_negative_change() { + let mut rng = OsRng; + + // Just use the master key as the ExtendedSpendingKey for this test + let extsk = ExtendedSpendingKey::master(&[]); + + // Fails with no inputs or outputs + // 0.0001 t-ZEC fee + { + let builder = Builder::new(0); + match builder.build(1, MockTxProver) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-10000)), + Ok(_) => panic!("Should have failed"), + } + } + + let extfvk = ExtendedFullViewingKey::from(&extsk); + let ovk = extfvk.fvk.ovk; + let to = extfvk.default_address().unwrap().1; + + // Fail if there is only a Sapling output + // 0.0005 z-ZEC out, 0.0001 t-ZEC fee + { + let mut builder = Builder::new(0); + builder + .add_sapling_output(ovk.clone(), to.clone(), Amount(50000), None) + .unwrap(); + match builder.build(1, MockTxProver) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-60000)), + Ok(_) => panic!("Should have failed"), + } + } + + let note1 = to + .create_note(59999, Fs::random(&mut rng), &JUBJUB) + .unwrap(); + let cm1 = Node::new(note1.cm(&JUBJUB).into_repr()); + let mut tree = CommitmentTree::new(); + tree.append(cm1).unwrap(); + let mut witness1 = IncrementalWitness::from_tree(&tree); + + // Fail if there is only a Sapling output + // 0.0005 z-ZEC out, 0.0001 t-ZEC fee, 0.00059999 z-ZEC in + { + let mut builder = Builder::new(0); + builder + .add_sapling_spend( + extsk.clone(), + to.diversifier, + note1.clone(), + witness1.clone(), + ) + .unwrap(); + builder + .add_sapling_output(ovk.clone(), to.clone(), Amount(50000), None) + .unwrap(); + match builder.build(1, MockTxProver) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-1)), + Ok(_) => panic!("Should have failed"), + } + } + + let note2 = to.create_note(1, Fs::random(&mut rng), &JUBJUB).unwrap(); + let cm2 = Node::new(note2.cm(&JUBJUB).into_repr()); + tree.append(cm2).unwrap(); + witness1.append(cm2).unwrap(); + let witness2 = IncrementalWitness::from_tree(&tree); + + // Succeeds if there is sufficient input + // 0.0005 z-ZEC out, 0.0001 t-ZEC fee, 0.0006 z-ZEC in + // + // (Still fails because we are using a MockTxProver which doesn't correctly + // compute bindingSig.) + { + let mut builder = Builder::new(0); + builder + .add_sapling_spend(extsk.clone(), to.diversifier, note1, witness1) + .unwrap(); + builder + .add_sapling_spend(extsk, to.diversifier, note2, witness2) + .unwrap(); + builder + .add_sapling_output(ovk, to, Amount(50000), None) + .unwrap(); + match builder.build(1, MockTxProver) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::BindingSig), + Ok(_) => panic!("Should have failed"), + } + } + } +} diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index 35e042a..19f4396 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -8,6 +8,7 @@ use std::ops::Deref; use serialize::Vector; +pub mod builder; pub mod components; mod sighash; From dab3c002b765807c2c5c89063cf880e189705e29 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 24 May 2019 12:30:14 +0100 Subject: [PATCH 03/19] Script opcode and data support Overrides the shift-left operator for pushing opcodes onto the Script, matching the notation used in zcashd. --- zcash_primitives/src/legacy.rs | 105 ++++++++++++++++++ zcash_primitives/src/lib.rs | 1 + .../src/transaction/components.rs | 16 +-- zcash_primitives/src/transaction/sighash.rs | 3 +- zcash_primitives/src/transaction/tests.rs | 7 +- 5 files changed, 111 insertions(+), 21 deletions(-) create mode 100644 zcash_primitives/src/legacy.rs diff --git a/zcash_primitives/src/legacy.rs b/zcash_primitives/src/legacy.rs new file mode 100644 index 0000000..ac22c6b --- /dev/null +++ b/zcash_primitives/src/legacy.rs @@ -0,0 +1,105 @@ +//! Support for legacy transparent addresses and scripts. + +use byteorder::{ReadBytesExt, WriteBytesExt}; +use std::io::{self, Read, Write}; +use std::ops::Shl; + +use crate::serialize::Vector; + +/// Script opcodes. +enum OpCode { + // push value + PushData1 = 0x4c, + PushData2 = 0x4d, + PushData4 = 0x4e, +} + +/// A serialized script, used inside transparent inputs and outputs of a transaction. +#[derive(Debug, Default)] +pub struct Script(pub Vec); + +impl Script { + pub fn read(mut reader: R) -> io::Result { + let script = Vector::read(&mut reader, |r| r.read_u8())?; + Ok(Script(script)) + } + + pub fn write(&self, mut writer: W) -> io::Result<()> { + Vector::write(&mut writer, &self.0, |w, e| w.write_u8(*e)) + } +} + +impl Shl for Script { + type Output = Self; + + fn shl(mut self, rhs: OpCode) -> Self { + self.0.push(rhs as u8); + self + } +} + +impl Shl<&[u8]> for Script { + type Output = Self; + + fn shl(mut self, data: &[u8]) -> Self { + if data.len() < OpCode::PushData1 as usize { + self.0.push(data.len() as u8); + } else if data.len() <= 0xff { + self.0.push(OpCode::PushData1 as u8); + self.0.push(data.len() as u8); + } else if data.len() <= 0xffff { + self.0.push(OpCode::PushData2 as u8); + self.0.extend(&(data.len() as u16).to_le_bytes()); + } else { + self.0.push(OpCode::PushData4 as u8); + self.0.extend(&(data.len() as u32).to_le_bytes()); + } + self.0.extend(data); + self + } +} + +#[cfg(test)] +mod tests { + use super::{OpCode, Script}; + + #[test] + fn script_opcode() { + { + let script = Script::default() << OpCode::PushData1; + assert_eq!(&script.0, &[OpCode::PushData1 as u8]); + } + } + + #[test] + fn script_pushdata() { + { + let script = Script::default() << &[1, 2, 3, 4][..]; + assert_eq!(&script.0, &[4, 1, 2, 3, 4]); + } + + { + let short_data = vec![2; 100]; + let script = Script::default() << &short_data[..]; + assert_eq!(script.0[0], OpCode::PushData1 as u8); + assert_eq!(script.0[1] as usize, 100); + assert_eq!(&script.0[2..], &short_data[..]); + } + + { + let medium_data = vec![7; 1024]; + let script = Script::default() << &medium_data[..]; + assert_eq!(script.0[0], OpCode::PushData2 as u8); + assert_eq!(&script.0[1..3], &[0x00, 0x04][..]); + assert_eq!(&script.0[3..], &medium_data[..]); + } + + { + let long_data = vec![42; 1_000_000]; + let script = Script::default() << &long_data[..]; + assert_eq!(script.0[0], OpCode::PushData4 as u8); + assert_eq!(&script.0[1..5], &[0x40, 0x42, 0x0f, 0x00][..]); + assert_eq!(&script.0[5..], &long_data[..]); + } + } +} diff --git a/zcash_primitives/src/lib.rs b/zcash_primitives/src/lib.rs index d6ddcc9..a95d000 100644 --- a/zcash_primitives/src/lib.rs +++ b/zcash_primitives/src/lib.rs @@ -19,6 +19,7 @@ use sapling_crypto::jubjub::JubjubBls12; pub mod block; pub mod keys; +pub mod legacy; pub mod merkle_tree; pub mod note_encryption; pub mod prover; diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index 141d0ff..d4479b5 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -7,7 +7,7 @@ use sapling_crypto::{ }; use std::io::{self, Read, Write}; -use serialize::Vector; +use legacy::Script; use JUBJUB; // π_A + π_B + π_C @@ -58,20 +58,6 @@ impl Amount { } } -#[derive(Debug)] -pub struct Script(pub Vec); - -impl Script { - pub fn read(mut reader: R) -> io::Result { - let script = Vector::read(&mut reader, |r| r.read_u8())?; - Ok(Script(script)) - } - - pub fn write(&self, mut writer: W) -> io::Result<()> { - Vector::write(&mut writer, &self.0, |w, e| w.write_u8(*e)) - } -} - #[derive(Debug)] pub struct OutPoint { hash: [u8; 32], diff --git a/zcash_primitives/src/transaction/sighash.rs b/zcash_primitives/src/transaction/sighash.rs index 774e7b4..bcc971a 100644 --- a/zcash_primitives/src/transaction/sighash.rs +++ b/zcash_primitives/src/transaction/sighash.rs @@ -3,10 +3,11 @@ use byteorder::{LittleEndian, WriteBytesExt}; use ff::{PrimeField, PrimeFieldRepr}; use super::{ - components::{Amount, Script, TxOut}, + components::{Amount, TxOut}, Transaction, TransactionData, OVERWINTER_VERSION_GROUP_ID, SAPLING_TX_VERSION, SAPLING_VERSION_GROUP_ID, }; +use legacy::Script; const ZCASH_SIGHASH_PERSONALIZATION_PREFIX: &'static [u8; 12] = b"ZcashSigHash"; const ZCASH_PREVOUTS_HASH_PERSONALIZATION: &'static [u8; 16] = b"ZcashPrevoutHash"; diff --git a/zcash_primitives/src/transaction/tests.rs b/zcash_primitives/src/transaction/tests.rs index d9788ff..1ebac3a 100644 --- a/zcash_primitives/src/transaction/tests.rs +++ b/zcash_primitives/src/transaction/tests.rs @@ -6,11 +6,8 @@ use sapling_crypto::{ redjubjub::PrivateKey, }; -use super::{ - components::{Amount, Script}, - sighash::signature_hash, - Transaction, TransactionData, -}; +use super::{components::Amount, sighash::signature_hash, Transaction, TransactionData}; +use legacy::Script; use JUBJUB; #[test] From c26188a0bb5dc6bd850aefa1b350bf0c38cf2d17 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 24 May 2019 13:23:27 +0100 Subject: [PATCH 04/19] TransparentAddress struct for P2PKH and P2SH --- zcash_primitives/src/legacy.rs | 65 +++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/zcash_primitives/src/legacy.rs b/zcash_primitives/src/legacy.rs index ac22c6b..02977ab 100644 --- a/zcash_primitives/src/legacy.rs +++ b/zcash_primitives/src/legacy.rs @@ -12,6 +12,17 @@ enum OpCode { PushData1 = 0x4c, PushData2 = 0x4d, PushData4 = 0x4e, + + // stack ops + Dup = 0x76, + + // bit logic + Equal = 0x87, + EqualVerify = 0x88, + + // crypto + Hash160 = 0xa9, + CheckSig = 0xac, } /// A serialized script, used inside transparent inputs and outputs of a transaction. @@ -59,9 +70,37 @@ impl Shl<&[u8]> for Script { } } +/// A transparent address corresponding to either a public key or a `Script`. +#[derive(Debug, PartialEq)] +pub enum TransparentAddress { + PublicKey([u8; 20]), + Script([u8; 20]), +} + +impl TransparentAddress { + /// Generate the `scriptPubKey` corresponding to this address. + pub fn script(&self) -> Script { + match self { + TransparentAddress::PublicKey(key_id) => { + // P2PKH script + Script::default() + << OpCode::Dup + << OpCode::Hash160 + << &key_id[..] + << OpCode::EqualVerify + << OpCode::CheckSig + } + TransparentAddress::Script(script_id) => { + // P2SH script + Script::default() << OpCode::Hash160 << &script_id[..] << OpCode::Equal + } + } + } +} + #[cfg(test)] mod tests { - use super::{OpCode, Script}; + use super::{OpCode, Script, TransparentAddress}; #[test] fn script_opcode() { @@ -102,4 +141,28 @@ mod tests { assert_eq!(&script.0[5..], &long_data[..]); } } + + #[test] + fn p2pkh() { + let addr = TransparentAddress::PublicKey([4; 20]); + assert_eq!( + &addr.script().0, + &[ + 0x76, 0xa9, 0x14, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, + 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x88, 0xac, + ] + ) + } + + #[test] + fn p2sh() { + let addr = TransparentAddress::Script([7; 20]); + assert_eq!( + &addr.script().0, + &[ + 0xa9, 0x14, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, + 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x87, + ] + ) + } } From a1cd9dfbac68f7ec2f3760a2c27c58afc6fcfad7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 24 May 2019 13:32:55 +0100 Subject: [PATCH 05/19] transaction::Builder::add_transparent_output() --- zcash_primitives/src/transaction/builder.rs | 69 +++++++++++++++++-- .../src/transaction/components.rs | 4 +- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 9f6cccd..2def3a8 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -12,12 +12,13 @@ use zip32::ExtendedSpendingKey; use crate::{ keys::OutgoingViewingKey, + legacy::TransparentAddress, merkle_tree::{CommitmentTreeWitness, IncrementalWitness}, note_encryption::{generate_esk, Memo, SaplingNoteEncryption}, prover::TxProver, sapling::{spend_sig, Node}, transaction::{ - components::{Amount, OutputDescription, SpendDescription}, + components::{Amount, OutputDescription, SpendDescription, TxOut}, signature_hash_data, Transaction, TransactionData, SIGHASH_ALL, }, JUBJUB, @@ -260,6 +261,24 @@ impl Builder { Ok(()) } + /// Adds a transparent address to send funds to. + pub fn add_transparent_output( + &mut self, + to: &TransparentAddress, + value: Amount, + ) -> Result<(), Error> { + if value.0 < 0 { + return Err(Error(ErrorKind::InvalidAmount)); + } + + self.mtx.vout.push(TxOut { + value, + script_pubkey: to.script(), + }); + + Ok(()) + } + /// Sets the Sapling address to which any change will be sent. /// /// By default, change is sent to the Sapling address corresponding to the first note @@ -289,7 +308,14 @@ impl Builder { // // Valid change - let change = self.mtx.value_balance.0 - self.fee.0; + let change = self.mtx.value_balance.0 + - self.fee.0 + - self + .mtx + .vout + .iter() + .map(|output| output.value.0) + .sum::(); if change.is_negative() { return Err(Error(ErrorKind::ChangeIsNegative(change))); } @@ -499,6 +525,7 @@ mod tests { use super::{Builder, ErrorKind}; use crate::{ + legacy::TransparentAddress, merkle_tree::{CommitmentTree, IncrementalWitness}, prover::mock::MockTxProver, sapling::Node, @@ -521,6 +548,15 @@ mod tests { } } + #[test] + fn fails_on_negative_transparent_output() { + let mut builder = Builder::new(0); + match builder.add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(-1)) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), + Ok(_) => panic!("Should have failed"), + } + } + #[test] fn fails_on_negative_change() { let mut rng = OsRng; @@ -555,6 +591,19 @@ mod tests { } } + // Fail if there is only a transparent output + // 0.0005 t-ZEC out, 0.0001 t-ZEC fee + { + let mut builder = Builder::new(0); + builder + .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(50000)) + .unwrap(); + match builder.build(1, MockTxProver) { + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-60000)), + Ok(_) => panic!("Should have failed"), + } + } + let note1 = to .create_note(59999, Fs::random(&mut rng), &JUBJUB) .unwrap(); @@ -563,8 +612,8 @@ mod tests { tree.append(cm1).unwrap(); let mut witness1 = IncrementalWitness::from_tree(&tree); - // Fail if there is only a Sapling output - // 0.0005 z-ZEC out, 0.0001 t-ZEC fee, 0.00059999 z-ZEC in + // Fail if there is insufficient input + // 0.0003 z-ZEC out, 0.0002 t-ZEC out, 0.0001 t-ZEC fee, 0.00059999 z-ZEC in { let mut builder = Builder::new(0); builder @@ -576,7 +625,10 @@ mod tests { ) .unwrap(); builder - .add_sapling_output(ovk.clone(), to.clone(), Amount(50000), None) + .add_sapling_output(ovk.clone(), to.clone(), Amount(30000), None) + .unwrap(); + builder + .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(20000)) .unwrap(); match builder.build(1, MockTxProver) { Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-1)), @@ -591,7 +643,7 @@ mod tests { let witness2 = IncrementalWitness::from_tree(&tree); // Succeeds if there is sufficient input - // 0.0005 z-ZEC out, 0.0001 t-ZEC fee, 0.0006 z-ZEC in + // 0.0003 z-ZEC out, 0.0002 t-ZEC out, 0.0001 t-ZEC fee, 0.0006 z-ZEC in // // (Still fails because we are using a MockTxProver which doesn't correctly // compute bindingSig.) @@ -604,7 +656,10 @@ mod tests { .add_sapling_spend(extsk, to.diversifier, note2, witness2) .unwrap(); builder - .add_sapling_output(ovk, to, Amount(50000), None) + .add_sapling_output(ovk, to, Amount(30000), None) + .unwrap(); + builder + .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(20000)) .unwrap(); match builder.build(1, MockTxProver) { Err(e) => assert_eq!(e.kind(), &ErrorKind::BindingSig), diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index d4479b5..f04c5aa 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -107,8 +107,8 @@ impl TxIn { #[derive(Debug)] pub struct TxOut { - value: Amount, - script_pubkey: Script, + pub value: Amount, + pub script_pubkey: Script, } impl TxOut { From 54ef63baced8160ac3396966b60941ceab1dbb3f Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Jun 2019 17:36:09 +0100 Subject: [PATCH 06/19] Place zcash_proofs::prover::LocalTxProver behind a feature flag This enables zcash_proofs to be compiled to WASM, which the directories crate doesn't support. --- zcash_proofs/Cargo.toml | 6 +++++- zcash_proofs/src/lib.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/zcash_proofs/Cargo.toml b/zcash_proofs/Cargo.toml index 9e989fa..598e013 100644 --- a/zcash_proofs/Cargo.toml +++ b/zcash_proofs/Cargo.toml @@ -9,9 +9,13 @@ authors = [ bellman = { path = "../bellman" } blake2b_simd = "0.5" byteorder = "1" -directories = "1" +directories = { version = "1", optional = true } ff = { path = "../ff" } pairing = { path = "../pairing" } rand_os = "0.2" sapling-crypto = { path = "../sapling-crypto" } zcash_primitives = { path = "../zcash_primitives" } + +[features] +default = ["local-prover"] +local-prover = ["directories"] diff --git a/zcash_proofs/src/lib.rs b/zcash_proofs/src/lib.rs index 26aa2f1..d2dc877 100644 --- a/zcash_proofs/src/lib.rs +++ b/zcash_proofs/src/lib.rs @@ -1,13 +1,15 @@ extern crate bellman; extern crate blake2b_simd; extern crate byteorder; -extern crate directories; extern crate ff; extern crate pairing; extern crate rand_os; extern crate sapling_crypto; extern crate zcash_primitives; +#[cfg(feature = "local-prover")] +extern crate directories; + use bellman::groth16::{prepare_verifying_key, Parameters, PreparedVerifyingKey, VerifyingKey}; use pairing::bls12_381::Bls12; use std::fs::File; @@ -15,9 +17,11 @@ use std::io::{self, BufReader}; use std::path::Path; mod hashreader; -pub mod prover; pub mod sapling; +#[cfg(feature = "local-prover")] +pub mod prover; + pub fn load_parameters( spend_path: &Path, spend_hash: &str, From 4b61120cd0658938ac3cb537c9538c098f1ac4b7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Jun 2019 23:11:06 +0100 Subject: [PATCH 07/19] Pass tx builder RNG to SaplingNoteEncryption --- zcash_primitives/src/note_encryption.rs | 21 ++++++++++----------- zcash_primitives/src/transaction/builder.rs | 16 +++++++++++----- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/zcash_primitives/src/note_encryption.rs b/zcash_primitives/src/note_encryption.rs index d0a7250..42e1398 100644 --- a/zcash_primitives/src/note_encryption.rs +++ b/zcash_primitives/src/note_encryption.rs @@ -5,8 +5,7 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use crypto_api_chachapoly::{ChaCha20Ietf, ChachaPolyIetf}; use ff::{PrimeField, PrimeFieldRepr}; use pairing::bls12_381::{Bls12, Fr}; -use rand_core::RngCore; -use rand_os::OsRng; +use rand_core::{CryptoRng, RngCore}; use sapling_crypto::{ jubjub::{ edwards, @@ -135,9 +134,8 @@ impl Memo { } } -pub fn generate_esk() -> Fs { +pub fn generate_esk(rng: &mut R) -> Fs { // create random 64 byte buffer - let mut rng = OsRng; let mut buffer = [0u8; 64]; rng.fill_bytes(&mut buffer); @@ -247,7 +245,7 @@ fn prf_ock( /// let note = to.create_note(value, rcv, &JUBJUB).unwrap(); /// let cmu = note.cm(&JUBJUB); /// -/// let enc = SaplingNoteEncryption::new(ovk, note, to, Memo::default()); +/// let enc = SaplingNoteEncryption::new(ovk, note, to, Memo::default(), &mut rng); /// let encCiphertext = enc.encrypt_note_plaintext(); /// let outCiphertext = enc.encrypt_outgoing_plaintext(&cv.cm(&JUBJUB).into(), &cmu); /// ``` @@ -262,13 +260,14 @@ pub struct SaplingNoteEncryption { impl SaplingNoteEncryption { /// Creates a new encryption context for the given note. - pub fn new( + pub fn new( ovk: OutgoingViewingKey, note: Note, to: PaymentAddress, memo: Memo, + rng: &mut R, ) -> SaplingNoteEncryption { - let esk = generate_esk(); + let esk = generate_esk(rng); let epk = note.g_d.mul(esk, &JUBJUB); SaplingNoteEncryption { @@ -561,7 +560,7 @@ mod tests { use crypto_api_chachapoly::ChachaPolyIetf; use ff::{Field, PrimeField, PrimeFieldRepr}; use pairing::bls12_381::{Bls12, Fr, FrRepr}; - use rand_core::RngCore; + use rand_core::{CryptoRng, RngCore}; use rand_os::OsRng; use sapling_crypto::{ jubjub::{ @@ -694,7 +693,7 @@ mod tests { assert_eq!(Memo::default().to_utf8(), None); } - fn random_enc_ciphertext( + fn random_enc_ciphertext( mut rng: &mut R, ) -> ( OutgoingViewingKey, @@ -724,7 +723,7 @@ mod tests { let cmu = note.cm(&JUBJUB); let ovk = OutgoingViewingKey([0; 32]); - let ne = SaplingNoteEncryption::new(ovk, note, pa, Memo([0; 512])); + let ne = SaplingNoteEncryption::new(ovk, note, pa, Memo([0; 512]), rng); let epk = ne.epk(); let enc_ciphertext = ne.encrypt_note_plaintext(); let out_ciphertext = ne.encrypt_outgoing_plaintext(&cv, &cmu); @@ -1371,7 +1370,7 @@ mod tests { // Test encryption // - let mut ne = SaplingNoteEncryption::new(ovk, note, to, Memo(tv.memo)); + let mut ne = SaplingNoteEncryption::new(ovk, note, to, Memo(tv.memo), &mut OsRng); // Swap in the ephemeral keypair from the test vectors ne.esk = esk; ne.epk = epk; diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 2def3a8..2ea4d0d 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -100,13 +100,19 @@ impl SaplingOutput { }) } - pub fn build( + pub fn build( self, prover: &P, ctx: &mut P::SaplingProvingContext, + rng: &mut R, ) -> OutputDescription { - let encryptor = - SaplingNoteEncryption::new(self.ovk, self.note.clone(), self.to.clone(), self.memo); + let encryptor = SaplingNoteEncryption::new( + self.ovk, + self.note.clone(), + self.to.clone(), + self.memo, + rng, + ); let (zkproof, cv) = prover.output_proof( ctx, @@ -419,7 +425,7 @@ impl Builder { // Record the post-randomized output location tx_metadata.output_indices[pos] = i; - output.build(&prover, &mut ctx) + output.build(&prover, &mut ctx, &mut self.rng) } else { // This is a dummy output let (dummy_to, dummy_note) = { @@ -457,7 +463,7 @@ impl Builder { ) }; - let esk = generate_esk(); + let esk = generate_esk(&mut self.rng); let epk = dummy_note.g_d.mul(esk, &JUBJUB); let (zkproof, cv) = From 532299d46eaa2802356049b209a40e6289ad2621 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Jun 2019 23:11:51 +0100 Subject: [PATCH 08/19] Allow transaction::Builder RNG to be configured --- zcash_primitives/src/transaction/builder.rs | 27 ++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 2ea4d0d..4edcd20 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -178,8 +178,8 @@ impl TransactionMetadata { } /// Generates a [`Transaction`] from its inputs and outputs. -pub struct Builder { - rng: OsRng, +pub struct Builder { + rng: R, mtx: TransactionData, fee: Amount, anchor: Option, @@ -188,9 +188,9 @@ pub struct Builder { change_address: Option<(OutgoingViewingKey, PaymentAddress)>, } -impl Builder { +impl Builder { /// Creates a new `Builder` targeted for inclusion in the block with the given height, - /// using default values for general transaction fields. + /// using default values for general transaction fields and the default OS random. /// /// # Default values /// @@ -198,12 +198,27 @@ impl Builder { /// expiry delta (20 blocks). /// /// The fee will be set to the default fee (0.0001 ZEC). - pub fn new(height: u32) -> Builder { + pub fn new(height: u32) -> Self { + Builder::new_with_rng(height, OsRng) + } +} + +impl Builder { + /// Creates a new `Builder` targeted for inclusion in the block with the given height + /// and randomness source, using default values for general transaction fields. + /// + /// # Default values + /// + /// The expiry height will be set to the given height plus the default transaction + /// expiry delta (20 blocks). + /// + /// The fee will be set to the default fee (0.0001 ZEC). + pub fn new_with_rng(height: u32, rng: R) -> Builder { let mut mtx = TransactionData::new(); mtx.expiry_height = height + DEFAULT_TX_EXPIRY_DELTA; Builder { - rng: OsRng, + rng, mtx, fee: DEFAULT_FEE, anchor: None, From 17f6bbcc67d6d82704d14fc58069b71db77ff0ad Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Jun 2019 23:12:55 +0100 Subject: [PATCH 09/19] Pass tx builder RNG to spend_sig() --- librustzcash/src/rustzcash.rs | 5 ++++- zcash_primitives/src/sapling.rs | 10 ++++------ zcash_primitives/src/transaction/builder.rs | 1 + 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/librustzcash/src/rustzcash.rs b/librustzcash/src/rustzcash.rs index 1db70ac..4f27fa1 100644 --- a/librustzcash/src/rustzcash.rs +++ b/librustzcash/src/rustzcash.rs @@ -1002,8 +1002,11 @@ pub extern "system" fn librustzcash_sapling_spend_sig( Err(_) => return false, }; + // Initialize secure RNG + let mut rng = OsRng; + // Do the signing - let sig = spend_sig(ask, ar, unsafe { &*sighash }, &JUBJUB); + let sig = spend_sig(ask, ar, unsafe { &*sighash }, &mut rng, &JUBJUB); // Write out the signature sig.write(&mut (unsafe { &mut *result })[..]) diff --git a/zcash_primitives/src/sapling.rs b/zcash_primitives/src/sapling.rs index ad7e309..bd981e6 100644 --- a/zcash_primitives/src/sapling.rs +++ b/zcash_primitives/src/sapling.rs @@ -2,7 +2,7 @@ use ff::{BitIterator, PrimeField, PrimeFieldRepr}; use pairing::bls12_381::{Bls12, Fr, FrRepr}; -use rand_os::OsRng; +use rand_core::{CryptoRng, RngCore}; use sapling_crypto::{ jubjub::{fs::Fs, FixedGenerators, JubjubBls12}, pedersen_hash::{pedersen_hash, Personalization}, @@ -106,15 +106,13 @@ lazy_static! { } /// Create the spendAuthSig for a Sapling SpendDescription. -pub fn spend_sig( +pub fn spend_sig( ask: PrivateKey, ar: Fs, sighash: &[u8; 32], + rng: &mut R, params: &JubjubBls12, ) -> Signature { - // Initialize secure RNG - let mut rng = OsRng; - // We compute `rsk`... let rsk = ask.randomize(ar); @@ -130,7 +128,7 @@ pub fn spend_sig( // Do the signing rsk.sign( &data_to_be_signed, - &mut rng, + rng, FixedGenerators::SpendingKeyGenerator, params, ) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 4edcd20..5c0e643 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -522,6 +522,7 @@ impl Builder { PrivateKey(spend.extsk.expsk.ask), spend.alpha, &sighash, + &mut self.rng, &JUBJUB, )); } From fa50d551c8c7d969157d4b3d42e9cbda85548051 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 25 Jul 2019 18:30:56 +0100 Subject: [PATCH 10/19] Move Amount impl into a submodule --- .../src/transaction/components.rs | 90 +------------------ .../src/transaction/components/amount.rs | 89 ++++++++++++++++++ 2 files changed, 92 insertions(+), 87 deletions(-) create mode 100644 zcash_primitives/src/transaction/components/amount.rs diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index f04c5aa..f72dfc5 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -10,6 +10,9 @@ use std::io::{self, Read, Write}; use legacy::Script; use JUBJUB; +mod amount; +pub use self::amount::Amount; + // π_A + π_B + π_C pub const GROTH_PROOF_SIZE: usize = (48 + 96 + 48); // π_A + π_A' + π_B + π_B' + π_C + π_C' + π_K + π_H @@ -18,46 +21,6 @@ const PHGR_PROOF_SIZE: usize = (33 + 33 + 65 + 33 + 33 + 33 + 33 + 33); const ZC_NUM_JS_INPUTS: usize = 2; const ZC_NUM_JS_OUTPUTS: usize = 2; -const COIN: i64 = 1_0000_0000; -const MAX_MONEY: i64 = 21_000_000 * COIN; - -#[derive(Clone, Copy, Debug, PartialEq)] -pub struct Amount(pub i64); - -impl Amount { - // Read an Amount from a signed 64-bit little-endian integer. - pub fn read_i64(mut reader: R, allow_negative: bool) -> io::Result { - let amount = reader.read_i64::()?; - if 0 <= amount && amount <= MAX_MONEY { - Ok(Amount(amount)) - } else if allow_negative && -MAX_MONEY <= amount && amount < 0 { - Ok(Amount(amount)) - } else { - Err(io::Error::new( - io::ErrorKind::InvalidData, - if allow_negative { - "Amount not in {-MAX_MONEY..MAX_MONEY}" - } else { - "Amount not in {0..MAX_MONEY}" - }, - )) - } - } - - // Read an Amount from an unsigned 64-bit little-endian integer. - pub fn read_u64(mut reader: R) -> io::Result { - let amount = reader.read_u64::()?; - if amount <= MAX_MONEY as u64 { - Ok(Amount(amount as i64)) - } else { - Err(io::Error::new( - io::ErrorKind::InvalidData, - "Amount not in {0..MAX_MONEY}", - )) - } - } -} - #[derive(Debug)] pub struct OutPoint { hash: [u8; 32], @@ -432,50 +395,3 @@ impl JSDescription { writer.write_all(&self.ciphertexts[1]) } } - -#[cfg(test)] -mod tests { - use super::{Amount, MAX_MONEY}; - - #[test] - fn amount_in_range() { - let zero = b"\x00\x00\x00\x00\x00\x00\x00\x00"; - assert_eq!(Amount::read_u64(&zero[..]).unwrap(), Amount(0)); - assert_eq!(Amount::read_i64(&zero[..], false).unwrap(), Amount(0)); - assert_eq!(Amount::read_i64(&zero[..], true).unwrap(), Amount(0)); - - let neg_one = b"\xff\xff\xff\xff\xff\xff\xff\xff"; - assert!(Amount::read_u64(&neg_one[..]).is_err()); - assert!(Amount::read_i64(&neg_one[..], false).is_err()); - assert_eq!(Amount::read_i64(&neg_one[..], true).unwrap(), Amount(-1)); - - let max_money = b"\x00\x40\x07\x5a\xf0\x75\x07\x00"; - assert_eq!(Amount::read_u64(&max_money[..]).unwrap(), Amount(MAX_MONEY)); - assert_eq!( - Amount::read_i64(&max_money[..], false).unwrap(), - Amount(MAX_MONEY) - ); - assert_eq!( - Amount::read_i64(&max_money[..], true).unwrap(), - Amount(MAX_MONEY) - ); - - let max_money_p1 = b"\x01\x40\x07\x5a\xf0\x75\x07\x00"; - assert!(Amount::read_u64(&max_money_p1[..]).is_err()); - assert!(Amount::read_i64(&max_money_p1[..], false).is_err()); - assert!(Amount::read_i64(&max_money_p1[..], true).is_err()); - - let neg_max_money = b"\x00\xc0\xf8\xa5\x0f\x8a\xf8\xff"; - assert!(Amount::read_u64(&neg_max_money[..]).is_err()); - assert!(Amount::read_i64(&neg_max_money[..], false).is_err()); - assert_eq!( - Amount::read_i64(&neg_max_money[..], true).unwrap(), - Amount(-MAX_MONEY) - ); - - let neg_max_money_m1 = b"\xff\xbf\xf8\xa5\x0f\x8a\xf8\xff"; - assert!(Amount::read_u64(&neg_max_money_m1[..]).is_err()); - assert!(Amount::read_i64(&neg_max_money_m1[..], false).is_err()); - assert!(Amount::read_i64(&neg_max_money_m1[..], true).is_err()); - } -} diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs new file mode 100644 index 0000000..593e98c --- /dev/null +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -0,0 +1,89 @@ +use byteorder::{LittleEndian, ReadBytesExt}; +use std::io::{self, Read}; + +const COIN: i64 = 1_0000_0000; +const MAX_MONEY: i64 = 21_000_000 * COIN; + +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Amount(pub i64); + +impl Amount { + // Read an Amount from a signed 64-bit little-endian integer. + pub fn read_i64(mut reader: R, allow_negative: bool) -> io::Result { + let amount = reader.read_i64::()?; + if 0 <= amount && amount <= MAX_MONEY { + Ok(Amount(amount)) + } else if allow_negative && -MAX_MONEY <= amount && amount < 0 { + Ok(Amount(amount)) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidData, + if allow_negative { + "Amount not in {-MAX_MONEY..MAX_MONEY}" + } else { + "Amount not in {0..MAX_MONEY}" + }, + )) + } + } + + // Read an Amount from an unsigned 64-bit little-endian integer. + pub fn read_u64(mut reader: R) -> io::Result { + let amount = reader.read_u64::()?; + if amount <= MAX_MONEY as u64 { + Ok(Amount(amount as i64)) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidData, + "Amount not in {0..MAX_MONEY}", + )) + } + } +} + +#[cfg(test)] +mod tests { + use super::{Amount, MAX_MONEY}; + + #[test] + fn amount_in_range() { + let zero = b"\x00\x00\x00\x00\x00\x00\x00\x00"; + assert_eq!(Amount::read_u64(&zero[..]).unwrap(), Amount(0)); + assert_eq!(Amount::read_i64(&zero[..], false).unwrap(), Amount(0)); + assert_eq!(Amount::read_i64(&zero[..], true).unwrap(), Amount(0)); + + let neg_one = b"\xff\xff\xff\xff\xff\xff\xff\xff"; + assert!(Amount::read_u64(&neg_one[..]).is_err()); + assert!(Amount::read_i64(&neg_one[..], false).is_err()); + assert_eq!(Amount::read_i64(&neg_one[..], true).unwrap(), Amount(-1)); + + let max_money = b"\x00\x40\x07\x5a\xf0\x75\x07\x00"; + assert_eq!(Amount::read_u64(&max_money[..]).unwrap(), Amount(MAX_MONEY)); + assert_eq!( + Amount::read_i64(&max_money[..], false).unwrap(), + Amount(MAX_MONEY) + ); + assert_eq!( + Amount::read_i64(&max_money[..], true).unwrap(), + Amount(MAX_MONEY) + ); + + let max_money_p1 = b"\x01\x40\x07\x5a\xf0\x75\x07\x00"; + assert!(Amount::read_u64(&max_money_p1[..]).is_err()); + assert!(Amount::read_i64(&max_money_p1[..], false).is_err()); + assert!(Amount::read_i64(&max_money_p1[..], true).is_err()); + + let neg_max_money = b"\x00\xc0\xf8\xa5\x0f\x8a\xf8\xff"; + assert!(Amount::read_u64(&neg_max_money[..]).is_err()); + assert!(Amount::read_i64(&neg_max_money[..], false).is_err()); + assert_eq!( + Amount::read_i64(&neg_max_money[..], true).unwrap(), + Amount(-MAX_MONEY) + ); + + let neg_max_money_m1 = b"\xff\xbf\xf8\xa5\x0f\x8a\xf8\xff"; + assert!(Amount::read_u64(&neg_max_money_m1[..]).is_err()); + assert!(Amount::read_i64(&neg_max_money_m1[..], false).is_err()); + assert!(Amount::read_i64(&neg_max_money_m1[..], true).is_err()); + } +} From 0ea4408d46e7e54c7d42854e87a0595b1a338c27 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 25 Jul 2019 20:50:17 +0100 Subject: [PATCH 11/19] Amount::{zero, is_positive, is_negative} --- zcash_primitives/src/transaction/builder.rs | 4 ++-- .../src/transaction/components/amount.rs | 17 +++++++++++++++++ zcash_primitives/src/transaction/mod.rs | 4 ++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 5c0e643..a2ea4af 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -79,7 +79,7 @@ impl SaplingOutput { Some(g_d) => g_d, None => return Err(Error(ErrorKind::InvalidAddress)), }; - if value.0 < 0 { + if value.is_negative() { return Err(Error(ErrorKind::InvalidAmount)); } @@ -288,7 +288,7 @@ impl Builder { to: &TransparentAddress, value: Amount, ) -> Result<(), Error> { - if value.0 < 0 { + if value.is_negative() { return Err(Error(ErrorKind::InvalidAmount)); } diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index 593e98c..eedfe2a 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -8,6 +8,11 @@ const MAX_MONEY: i64 = 21_000_000 * COIN; pub struct Amount(pub i64); impl Amount { + /// Returns a zero-valued Amount. + pub const fn zero() -> Self { + Amount(0) + } + // Read an Amount from a signed 64-bit little-endian integer. pub fn read_i64(mut reader: R, allow_negative: bool) -> io::Result { let amount = reader.read_i64::()?; @@ -39,6 +44,18 @@ impl Amount { )) } } + + /// Returns `true` if `self` is positive and `false` if the number is zero or + /// negative. + pub const fn is_positive(self) -> bool { + self.0.is_positive() + } + + /// Returns `true` if `self` is negative and `false` if the number is zero or + /// positive. + pub const fn is_negative(self) -> bool { + self.0.is_negative() + } } #[cfg(test)] diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index 19f4396..d8024eb 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -112,7 +112,7 @@ impl TransactionData { vout: vec![], lock_time: 0, expiry_height: 0, - value_balance: Amount(0), + value_balance: Amount::zero(), shielded_spends: vec![], shielded_outputs: vec![], joinsplits: vec![], @@ -190,7 +190,7 @@ impl Transaction { let so = Vector::read(&mut reader, OutputDescription::read)?; (vb, ss, so) } else { - (Amount(0), vec![], vec![]) + (Amount::zero(), vec![], vec![]) }; let (joinsplits, joinsplit_pubkey, joinsplit_sig) = if version >= 2 { From ab60b8804ae9d6900e51c61a93ce45309b19d822 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 25 Jul 2019 20:53:42 +0100 Subject: [PATCH 12/19] impl operators for Amount --- zcash_primitives/src/transaction/builder.rs | 24 ++++++------- .../src/transaction/components/amount.rs | 36 +++++++++++++++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index a2ea4af..b7bcb62 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -44,7 +44,7 @@ impl Error { pub enum ErrorKind { AnchorMismatch, BindingSig, - ChangeIsNegative(i64), + ChangeIsNegative(Amount), InvalidAddress, InvalidAmount, InvalidWitness, @@ -252,7 +252,7 @@ impl Builder { let alpha = Fs::random(&mut self.rng); - self.mtx.value_balance.0 += note.value as i64; + self.mtx.value_balance += Amount(note.value as i64); self.spends.push(SpendDescriptionInfo { extsk, @@ -275,7 +275,7 @@ impl Builder { ) -> Result<(), Error> { let output = SaplingOutput::new(&mut self.rng, ovk, to, value, memo)?; - self.mtx.value_balance.0 -= value.0; + self.mtx.value_balance -= value; self.outputs.push(output); @@ -329,14 +329,14 @@ impl Builder { // // Valid change - let change = self.mtx.value_balance.0 - - self.fee.0 + let change = self.mtx.value_balance + - self.fee - self .mtx .vout .iter() - .map(|output| output.value.0) - .sum::(); + .map(|output| output.value) + .sum::(); if change.is_negative() { return Err(Error(ErrorKind::ChangeIsNegative(change))); } @@ -362,7 +362,7 @@ impl Builder { return Err(Error(ErrorKind::NoChangeAddress)); }; - self.add_sapling_output(change_address.0, change_address.1, Amount(change), None)?; + self.add_sapling_output(change_address.0, change_address.1, change, None)?; } // @@ -591,7 +591,7 @@ mod tests { { let builder = Builder::new(0); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-10000)), + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-10000))), Ok(_) => panic!("Should have failed"), } } @@ -608,7 +608,7 @@ mod tests { .add_sapling_output(ovk.clone(), to.clone(), Amount(50000), None) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-60000)), + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-60000))), Ok(_) => panic!("Should have failed"), } } @@ -621,7 +621,7 @@ mod tests { .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(50000)) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-60000)), + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-60000))), Ok(_) => panic!("Should have failed"), } } @@ -653,7 +653,7 @@ mod tests { .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(20000)) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(-1)), + Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-1))), Ok(_) => panic!("Should have failed"), } } diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index eedfe2a..fa9ee06 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -1,5 +1,7 @@ use byteorder::{LittleEndian, ReadBytesExt}; use std::io::{self, Read}; +use std::iter::Sum; +use std::ops::{Add, AddAssign, Sub, SubAssign}; const COIN: i64 = 1_0000_0000; const MAX_MONEY: i64 = 21_000_000 * COIN; @@ -58,6 +60,40 @@ impl Amount { } } +impl Add for Amount { + type Output = Amount; + + fn add(self, rhs: Amount) -> Amount { + Amount(self.0 + rhs.0) + } +} + +impl AddAssign for Amount { + fn add_assign(&mut self, rhs: Amount) { + *self = *self + rhs + } +} + +impl Sub for Amount { + type Output = Amount; + + fn sub(self, rhs: Amount) -> Amount { + Amount(self.0 - rhs.0) + } +} + +impl SubAssign for Amount { + fn sub_assign(&mut self, rhs: Amount) { + *self = *self - rhs + } +} + +impl Sum for Amount { + fn sum>(iter: I) -> Amount { + iter.fold(Amount::zero(), Add::add) + } +} + #[cfg(test)] mod tests { use super::{Amount, MAX_MONEY}; From 59ed258c7f673031cb22524004d72d54991cb38e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 25 Jul 2019 22:37:16 +0100 Subject: [PATCH 13/19] Make Amount opaque, and use it more This helps to ensure type-safety of values that are required to satisfy zatoshi range bounds. --- librustzcash/src/rustzcash.rs | 11 ++ zcash_primitives/src/prover.rs | 14 ++- zcash_primitives/src/transaction/builder.rs | 68 ++++++++--- .../src/transaction/components.rs | 29 +++-- .../src/transaction/components/amount.rs | 115 ++++++++++++------ zcash_primitives/src/transaction/mod.rs | 9 +- zcash_primitives/src/transaction/sighash.rs | 11 +- zcash_primitives/src/transaction/tests.rs | 12 +- zcash_proofs/src/prover.rs | 9 +- zcash_proofs/src/sapling/mod.rs | 5 +- zcash_proofs/src/sapling/prover.rs | 6 +- zcash_proofs/src/sapling/verifier.rs | 3 +- 12 files changed, 203 insertions(+), 89 deletions(-) diff --git a/librustzcash/src/rustzcash.rs b/librustzcash/src/rustzcash.rs index 4f27fa1..0fad706 100644 --- a/librustzcash/src/rustzcash.rs +++ b/librustzcash/src/rustzcash.rs @@ -63,6 +63,7 @@ use zcash_primitives::{ merkle_tree::CommitmentTreeWitness, note_encryption::sapling_ka_agree, sapling::{merkle_hash, spend_sig}, + transaction::components::Amount, zip32, JUBJUB, }; use zcash_proofs::{ @@ -704,6 +705,11 @@ pub extern "system" fn librustzcash_sapling_final_check( binding_sig: *const [c_uchar; 64], sighash_value: *const [c_uchar; 32], ) -> bool { + let value_balance = match Amount::from_i64(value_balance, true) { + Ok(vb) => vb, + Err(()) => return false, + }; + // Deserialize the signature let binding_sig = match Signature::read(&(unsafe { &*binding_sig })[..]) { Ok(sig) => sig, @@ -1022,6 +1028,11 @@ pub extern "system" fn librustzcash_sapling_binding_sig( sighash: *const [c_uchar; 32], result: *mut [c_uchar; 64], ) -> bool { + let value_balance = match Amount::from_i64(value_balance, true) { + Ok(vb) => vb, + Err(()) => return false, + }; + // Sign let sig = match unsafe { &*ctx }.binding_sig(value_balance, unsafe { &*sighash }, &JUBJUB) { Ok(s) => s, diff --git a/zcash_primitives/src/prover.rs b/zcash_primitives/src/prover.rs index 87a76c7..1d40965 100644 --- a/zcash_primitives/src/prover.rs +++ b/zcash_primitives/src/prover.rs @@ -8,7 +8,9 @@ use sapling_crypto::{ }; use crate::{ - merkle_tree::CommitmentTreeWitness, sapling::Node, transaction::components::GROTH_PROOF_SIZE, + merkle_tree::CommitmentTreeWitness, + sapling::Node, + transaction::components::{Amount, GROTH_PROOF_SIZE}, }; /// Interface for creating zero-knowledge proofs for shielded transactions. @@ -63,7 +65,7 @@ pub trait TxProver { fn binding_sig( &self, ctx: &mut Self::SaplingProvingContext, - value_balance: i64, + value_balance: Amount, sighash: &[u8; 32], ) -> Result; } @@ -80,8 +82,10 @@ pub(crate) mod mock { }; use crate::{ - merkle_tree::CommitmentTreeWitness, sapling::Node, - transaction::components::GROTH_PROOF_SIZE, JUBJUB, + merkle_tree::CommitmentTreeWitness, + sapling::Node, + transaction::components::{Amount, GROTH_PROOF_SIZE}, + JUBJUB, }; use super::TxProver; @@ -153,7 +157,7 @@ pub(crate) mod mock { fn binding_sig( &self, _ctx: &mut Self::SaplingProvingContext, - _value_balance: i64, + _value_balance: Amount, _sighash: &[u8; 32], ) -> Result { Err(()) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index b7bcb62..36490e4 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -18,13 +18,12 @@ use crate::{ prover::TxProver, sapling::{spend_sig, Node}, transaction::{ - components::{Amount, OutputDescription, SpendDescription, TxOut}, + components::{amount::DEFAULT_FEE, Amount, OutputDescription, SpendDescription, TxOut}, signature_hash_data, Transaction, TransactionData, SIGHASH_ALL, }, JUBJUB, }; -const DEFAULT_FEE: Amount = Amount(10000); const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; /// If there are any shielded inputs, always have at least two shielded outputs, padding @@ -88,7 +87,7 @@ impl SaplingOutput { let note = Note { g_d, pk_d: to.pk_d.clone(), - value: value.0 as u64, + value: value.into(), r: rcm, }; @@ -252,7 +251,8 @@ impl Builder { let alpha = Fs::random(&mut self.rng); - self.mtx.value_balance += Amount(note.value as i64); + self.mtx.value_balance += + Amount::from_u64(note.value).map_err(|_| Error(ErrorKind::InvalidAmount))?; self.spends.push(SpendDescriptionInfo { extsk, @@ -528,7 +528,7 @@ impl Builder { } self.mtx.binding_sig = Some( prover - .binding_sig(&mut ctx, self.mtx.value_balance.0, &sighash) + .binding_sig(&mut ctx, self.mtx.value_balance, &sighash) .map_err(|()| Error(ErrorKind::BindingSig))?, ); @@ -564,7 +564,7 @@ mod tests { let to = extfvk.default_address().unwrap().1; let mut builder = Builder::new(0); - match builder.add_sapling_output(ovk, to, Amount(-1), None) { + match builder.add_sapling_output(ovk, to, Amount::from_i64(-1, true).unwrap(), None) { Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), Ok(_) => panic!("Should have failed"), } @@ -573,7 +573,10 @@ mod tests { #[test] fn fails_on_negative_transparent_output() { let mut builder = Builder::new(0); - match builder.add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(-1)) { + match builder.add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_i64(-1, true).unwrap(), + ) { Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), Ok(_) => panic!("Should have failed"), } @@ -591,7 +594,10 @@ mod tests { { let builder = Builder::new(0); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-10000))), + Err(e) => assert_eq!( + e.kind(), + &ErrorKind::ChangeIsNegative(Amount::from_i64(-10000, true).unwrap()) + ), Ok(_) => panic!("Should have failed"), } } @@ -605,10 +611,18 @@ mod tests { { let mut builder = Builder::new(0); builder - .add_sapling_output(ovk.clone(), to.clone(), Amount(50000), None) + .add_sapling_output( + ovk.clone(), + to.clone(), + Amount::from_u64(50000).unwrap(), + None, + ) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-60000))), + Err(e) => assert_eq!( + e.kind(), + &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000, true).unwrap()) + ), Ok(_) => panic!("Should have failed"), } } @@ -618,10 +632,16 @@ mod tests { { let mut builder = Builder::new(0); builder - .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(50000)) + .add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_u64(50000).unwrap(), + ) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-60000))), + Err(e) => assert_eq!( + e.kind(), + &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000, true).unwrap()) + ), Ok(_) => panic!("Should have failed"), } } @@ -647,13 +667,24 @@ mod tests { ) .unwrap(); builder - .add_sapling_output(ovk.clone(), to.clone(), Amount(30000), None) + .add_sapling_output( + ovk.clone(), + to.clone(), + Amount::from_u64(30000).unwrap(), + None, + ) .unwrap(); builder - .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(20000)) + .add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_u64(20000).unwrap(), + ) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::ChangeIsNegative(Amount(-1))), + Err(e) => assert_eq!( + e.kind(), + &ErrorKind::ChangeIsNegative(Amount::from_i64(-1, true).unwrap()) + ), Ok(_) => panic!("Should have failed"), } } @@ -678,10 +709,13 @@ mod tests { .add_sapling_spend(extsk, to.diversifier, note2, witness2) .unwrap(); builder - .add_sapling_output(ovk, to, Amount(30000), None) + .add_sapling_output(ovk, to, Amount::from_u64(30000).unwrap(), None) .unwrap(); builder - .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount(20000)) + .add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_u64(20000).unwrap(), + ) .unwrap(); match builder.build(1, MockTxProver) { Err(e) => assert_eq!(e.kind(), &ErrorKind::BindingSig), diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index f72dfc5..c7dcebd 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -10,7 +10,7 @@ use std::io::{self, Read, Write}; use legacy::Script; use JUBJUB; -mod amount; +pub mod amount; pub use self::amount::Amount; // π_A + π_B + π_C @@ -76,7 +76,12 @@ pub struct TxOut { impl TxOut { pub fn read(mut reader: &mut R) -> io::Result { - let value = Amount::read_i64(&mut reader, false)?; + let value = { + let mut tmp = [0; 8]; + reader.read_exact(&mut tmp)?; + Amount::from_i64_le_bytes(tmp, false) + } + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "value out of range"))?; let script_pubkey = Script::read(&mut reader)?; Ok(TxOut { @@ -86,7 +91,7 @@ impl TxOut { } pub fn write(&self, mut writer: W) -> io::Result<()> { - writer.write_i64::(self.value.0)?; + writer.write_all(&self.value.to_i64_le_bytes())?; self.script_pubkey.write(&mut writer) } } @@ -298,10 +303,20 @@ impl std::fmt::Debug for JSDescription { impl JSDescription { pub fn read(mut reader: R, use_groth: bool) -> io::Result { // Consensus rule (§4.3): Canonical encoding is enforced here - let vpub_old = Amount::read_u64(&mut reader)?; + let vpub_old = { + let mut tmp = [0; 8]; + reader.read_exact(&mut tmp)?; + Amount::from_u64_le_bytes(tmp) + } + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "vpub_old out of range"))?; // Consensus rule (§4.3): Canonical encoding is enforced here - let vpub_new = Amount::read_u64(&mut reader)?; + let vpub_new = { + let mut tmp = [0; 8]; + reader.read_exact(&mut tmp)?; + Amount::from_u64_le_bytes(tmp) + } + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "vpub_new out of range"))?; // Consensus rule (§4.3): One of vpub_old and vpub_new being zero is // enforced by CheckTransactionWithoutProofVerification() in zcashd. @@ -374,8 +389,8 @@ impl JSDescription { } pub fn write(&self, mut writer: W) -> io::Result<()> { - writer.write_i64::(self.vpub_old.0)?; - writer.write_i64::(self.vpub_new.0)?; + writer.write_all(&self.vpub_old.to_i64_le_bytes())?; + writer.write_all(&self.vpub_new.to_i64_le_bytes())?; writer.write_all(&self.anchor)?; writer.write_all(&self.nullifiers[0])?; writer.write_all(&self.nullifiers[1])?; diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index fa9ee06..dca7056 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -1,13 +1,13 @@ -use byteorder::{LittleEndian, ReadBytesExt}; -use std::io::{self, Read}; use std::iter::Sum; use std::ops::{Add, AddAssign, Sub, SubAssign}; const COIN: i64 = 1_0000_0000; const MAX_MONEY: i64 = 21_000_000 * COIN; +pub const DEFAULT_FEE: Amount = Amount(10000); + #[derive(Clone, Copy, Debug, PartialEq)] -pub struct Amount(pub i64); +pub struct Amount(i64); impl Amount { /// Returns a zero-valued Amount. @@ -15,38 +15,51 @@ impl Amount { Amount(0) } - // Read an Amount from a signed 64-bit little-endian integer. - pub fn read_i64(mut reader: R, allow_negative: bool) -> io::Result { - let amount = reader.read_i64::()?; + /// Creates an Amount from an i64. + /// + /// Returns an error if the amount is out of range. + pub fn from_i64(amount: i64, allow_negative: bool) -> Result { if 0 <= amount && amount <= MAX_MONEY { Ok(Amount(amount)) } else if allow_negative && -MAX_MONEY <= amount && amount < 0 { Ok(Amount(amount)) } else { - Err(io::Error::new( - io::ErrorKind::InvalidData, - if allow_negative { - "Amount not in {-MAX_MONEY..MAX_MONEY}" - } else { - "Amount not in {0..MAX_MONEY}" - }, - )) + Err(()) } } - // Read an Amount from an unsigned 64-bit little-endian integer. - pub fn read_u64(mut reader: R) -> io::Result { - let amount = reader.read_u64::()?; + /// Creates an Amount from a u64. + /// + /// Returns an error if the amount is out of range. + pub fn from_u64(amount: u64) -> Result { if amount <= MAX_MONEY as u64 { Ok(Amount(amount as i64)) } else { - Err(io::Error::new( - io::ErrorKind::InvalidData, - "Amount not in {0..MAX_MONEY}", - )) + Err(()) } } + /// Reads an Amount from a signed 64-bit little-endian integer. + /// + /// Returns an error if the amount is out of range. + pub fn from_i64_le_bytes(bytes: [u8; 8], allow_negative: bool) -> Result { + let amount = i64::from_le_bytes(bytes); + Amount::from_i64(amount, allow_negative) + } + + /// Reads an Amount from an unsigned 64-bit little-endian integer. + /// + /// Returns an error if the amount is out of range. + pub fn from_u64_le_bytes(bytes: [u8; 8]) -> Result { + let amount = u64::from_le_bytes(bytes); + Amount::from_u64(amount) + } + + /// Returns the Amount encoded as a signed 64-bit little-endian integer. + pub fn to_i64_le_bytes(self) -> [u8; 8] { + self.0.to_le_bytes() + } + /// Returns `true` if `self` is positive and `false` if the number is zero or /// negative. pub const fn is_positive(self) -> bool { @@ -60,6 +73,18 @@ impl Amount { } } +impl From for i64 { + fn from(amount: Amount) -> i64 { + amount.0 + } +} + +impl From for u64 { + fn from(amount: Amount) -> u64 { + amount.0 as u64 + } +} + impl Add for Amount { type Output = Amount; @@ -101,42 +126,54 @@ mod tests { #[test] fn amount_in_range() { let zero = b"\x00\x00\x00\x00\x00\x00\x00\x00"; - assert_eq!(Amount::read_u64(&zero[..]).unwrap(), Amount(0)); - assert_eq!(Amount::read_i64(&zero[..], false).unwrap(), Amount(0)); - assert_eq!(Amount::read_i64(&zero[..], true).unwrap(), Amount(0)); + assert_eq!(Amount::from_u64_le_bytes(zero.clone()).unwrap(), Amount(0)); + assert_eq!( + Amount::from_i64_le_bytes(zero.clone(), false).unwrap(), + Amount(0) + ); + assert_eq!( + Amount::from_i64_le_bytes(zero.clone(), true).unwrap(), + Amount(0) + ); let neg_one = b"\xff\xff\xff\xff\xff\xff\xff\xff"; - assert!(Amount::read_u64(&neg_one[..]).is_err()); - assert!(Amount::read_i64(&neg_one[..], false).is_err()); - assert_eq!(Amount::read_i64(&neg_one[..], true).unwrap(), Amount(-1)); + assert!(Amount::from_u64_le_bytes(neg_one.clone()).is_err()); + assert!(Amount::from_i64_le_bytes(neg_one.clone(), false).is_err()); + assert_eq!( + Amount::from_i64_le_bytes(neg_one.clone(), true).unwrap(), + Amount(-1) + ); let max_money = b"\x00\x40\x07\x5a\xf0\x75\x07\x00"; - assert_eq!(Amount::read_u64(&max_money[..]).unwrap(), Amount(MAX_MONEY)); assert_eq!( - Amount::read_i64(&max_money[..], false).unwrap(), + Amount::from_u64_le_bytes(max_money.clone()).unwrap(), Amount(MAX_MONEY) ); assert_eq!( - Amount::read_i64(&max_money[..], true).unwrap(), + Amount::from_i64_le_bytes(max_money.clone(), false).unwrap(), + Amount(MAX_MONEY) + ); + assert_eq!( + Amount::from_i64_le_bytes(max_money.clone(), true).unwrap(), Amount(MAX_MONEY) ); let max_money_p1 = b"\x01\x40\x07\x5a\xf0\x75\x07\x00"; - assert!(Amount::read_u64(&max_money_p1[..]).is_err()); - assert!(Amount::read_i64(&max_money_p1[..], false).is_err()); - assert!(Amount::read_i64(&max_money_p1[..], true).is_err()); + assert!(Amount::from_u64_le_bytes(max_money_p1.clone()).is_err()); + assert!(Amount::from_i64_le_bytes(max_money_p1.clone(), false).is_err()); + assert!(Amount::from_i64_le_bytes(max_money_p1.clone(), true).is_err()); let neg_max_money = b"\x00\xc0\xf8\xa5\x0f\x8a\xf8\xff"; - assert!(Amount::read_u64(&neg_max_money[..]).is_err()); - assert!(Amount::read_i64(&neg_max_money[..], false).is_err()); + assert!(Amount::from_u64_le_bytes(neg_max_money.clone()).is_err()); + assert!(Amount::from_i64_le_bytes(neg_max_money.clone(), false).is_err()); assert_eq!( - Amount::read_i64(&neg_max_money[..], true).unwrap(), + Amount::from_i64_le_bytes(neg_max_money.clone(), true).unwrap(), Amount(-MAX_MONEY) ); let neg_max_money_m1 = b"\xff\xbf\xf8\xa5\x0f\x8a\xf8\xff"; - assert!(Amount::read_u64(&neg_max_money_m1[..]).is_err()); - assert!(Amount::read_i64(&neg_max_money_m1[..], false).is_err()); - assert!(Amount::read_i64(&neg_max_money_m1[..], true).is_err()); + assert!(Amount::from_u64_le_bytes(neg_max_money_m1.clone()).is_err()); + assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone(), false).is_err()); + assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone(), true).is_err()); } } diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index d8024eb..e5b83e5 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -185,7 +185,12 @@ impl Transaction { }; let (value_balance, shielded_spends, shielded_outputs) = if is_sapling_v4 { - let vb = Amount::read_i64(&mut reader, true)?; + let vb = { + let mut tmp = [0; 8]; + reader.read_exact(&mut tmp)?; + Amount::from_i64_le_bytes(tmp, true) + } + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "valueBalance out of range"))?; let ss = Vector::read(&mut reader, SpendDescription::read)?; let so = Vector::read(&mut reader, OutputDescription::read)?; (vb, ss, so) @@ -262,7 +267,7 @@ impl Transaction { } if is_sapling_v4 { - writer.write_i64::(self.value_balance.0)?; + writer.write_all(&self.value_balance.to_i64_le_bytes())?; Vector::write(&mut writer, &self.shielded_spends, |w, e| e.write(w))?; Vector::write(&mut writer, &self.shielded_outputs, |w, e| e.write(w))?; } diff --git a/zcash_primitives/src/transaction/sighash.rs b/zcash_primitives/src/transaction/sighash.rs index bcc971a..b4e9a69 100644 --- a/zcash_primitives/src/transaction/sighash.rs +++ b/zcash_primitives/src/transaction/sighash.rs @@ -30,13 +30,6 @@ macro_rules! update_u32 { }; } -macro_rules! update_i64 { - ($h:expr, $value:expr, $tmp:expr) => { - (&mut $tmp[..8]).write_i64::($value).unwrap(); - $h.update(&$tmp[..8]); - }; -} - macro_rules! update_hash { ($h:expr, $cond:expr, $value:expr) => { if $cond { @@ -214,7 +207,7 @@ pub fn signature_hash_data( update_u32!(h, tx.lock_time, tmp); update_u32!(h, tx.expiry_height, tmp); if sigversion == SigHashVersion::Sapling { - update_i64!(h, tx.value_balance.0, tmp); + h.update(&tx.value_balance.to_i64_le_bytes()); } update_u32!(h, hash_type, tmp); @@ -222,7 +215,7 @@ pub fn signature_hash_data( let mut data = vec![]; tx.vin[n].prevout.write(&mut data).unwrap(); script_code.write(&mut data).unwrap(); - (&mut data).write_i64::(amount.0).unwrap(); + data.extend_from_slice(&amount.to_i64_le_bytes()); (&mut data) .write_u32::(tx.vin[n].sequence) .unwrap(); diff --git a/zcash_primitives/src/transaction/tests.rs b/zcash_primitives/src/transaction/tests.rs index 1ebac3a..7d84100 100644 --- a/zcash_primitives/src/transaction/tests.rs +++ b/zcash_primitives/src/transaction/tests.rs @@ -1902,7 +1902,11 @@ fn zip_0143() { for tv in test_vectors { let tx = Transaction::read(&tv.tx[..]).unwrap(); let transparent_input = if let Some(n) = tv.transparent_input { - Some((n as usize, Script(tv.script_code), Amount(tv.amount))) + Some(( + n as usize, + Script(tv.script_code), + Amount::from_i64(tv.amount, false).unwrap(), + )) } else { None }; @@ -5393,7 +5397,11 @@ fn zip_0243() { for tv in test_vectors { let tx = Transaction::read(&tv.tx[..]).unwrap(); let transparent_input = if let Some(n) = tv.transparent_input { - Some((n as usize, Script(tv.script_code), Amount(tv.amount))) + Some(( + n as usize, + Script(tv.script_code), + Amount::from_i64(tv.amount, false).unwrap(), + )) } else { None }; diff --git a/zcash_proofs/src/prover.rs b/zcash_proofs/src/prover.rs index 005d7eb..4b5b0f4 100644 --- a/zcash_proofs/src/prover.rs +++ b/zcash_proofs/src/prover.rs @@ -10,8 +10,11 @@ use sapling_crypto::{ }; use std::path::Path; use zcash_primitives::{ - merkle_tree::CommitmentTreeWitness, prover::TxProver, sapling::Node, - transaction::components::GROTH_PROOF_SIZE, JUBJUB, + merkle_tree::CommitmentTreeWitness, + prover::TxProver, + sapling::Node, + transaction::components::{Amount, GROTH_PROOF_SIZE}, + JUBJUB, }; use crate::{load_parameters, sapling::SaplingProvingContext}; @@ -182,7 +185,7 @@ impl TxProver for LocalTxProver { fn binding_sig( &self, ctx: &mut Self::SaplingProvingContext, - value_balance: i64, + value_balance: Amount, sighash: &[u8; 32], ) -> Result { ctx.binding_sig(value_balance, sighash, &JUBJUB) diff --git a/zcash_proofs/src/sapling/mod.rs b/zcash_proofs/src/sapling/mod.rs index 4c37306..98c4ba3 100644 --- a/zcash_proofs/src/sapling/mod.rs +++ b/zcash_proofs/src/sapling/mod.rs @@ -2,6 +2,7 @@ use pairing::bls12_381::Bls12; use sapling_crypto::jubjub::{ edwards, fs::FsRepr, FixedGenerators, JubjubBls12, JubjubParams, Unknown, }; +use zcash_primitives::transaction::components::Amount; mod prover; mod verifier; @@ -11,12 +12,12 @@ pub use self::verifier::SaplingVerificationContext; // This function computes `value` in the exponent of the value commitment base fn compute_value_balance( - value: i64, + value: Amount, params: &JubjubBls12, ) -> Option> { // Compute the absolute value (failing if -i64::MAX is // the value) - let abs = match value.checked_abs() { + let abs = match i64::from(value).checked_abs() { Some(a) => a as u64, None => return None, }; diff --git a/zcash_proofs/src/sapling/prover.rs b/zcash_proofs/src/sapling/prover.rs index fce4d8e..4b5a5f4 100644 --- a/zcash_proofs/src/sapling/prover.rs +++ b/zcash_proofs/src/sapling/prover.rs @@ -13,7 +13,9 @@ use sapling_crypto::{ primitives::{Diversifier, Note, PaymentAddress, ProofGenerationKey, ValueCommitment}, redjubjub::{PrivateKey, PublicKey, Signature}, }; -use zcash_primitives::{merkle_tree::CommitmentTreeWitness, sapling::Node}; +use zcash_primitives::{ + merkle_tree::CommitmentTreeWitness, sapling::Node, transaction::components::Amount, +}; use super::compute_value_balance; @@ -245,7 +247,7 @@ impl SaplingProvingContext { /// and output_proof() must be completed before calling this function. pub fn binding_sig( &self, - value_balance: i64, + value_balance: Amount, sighash: &[u8; 32], params: &JubjubBls12, ) -> Result { diff --git a/zcash_proofs/src/sapling/verifier.rs b/zcash_proofs/src/sapling/verifier.rs index e83c426..47fc9fe 100644 --- a/zcash_proofs/src/sapling/verifier.rs +++ b/zcash_proofs/src/sapling/verifier.rs @@ -6,6 +6,7 @@ use sapling_crypto::{ jubjub::{edwards, FixedGenerators, JubjubBls12, Unknown}, redjubjub::{PublicKey, Signature}, }; +use zcash_primitives::transaction::components::Amount; use super::compute_value_balance; @@ -169,7 +170,7 @@ impl SaplingVerificationContext { /// have been checked before calling this function. pub fn final_check( &self, - value_balance: i64, + value_balance: Amount, sighash_value: &[u8; 32], binding_sig: Signature, params: &JubjubBls12, From 7c07914bfdd9e896602b4582ccbd715743a23d5c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 8 Aug 2019 00:55:23 +0100 Subject: [PATCH 14/19] Separate Amount::{from_i64, from_nonnegative_i64} APIs This is more intuitive than a boolean flag for handling non-negative Amounts stored in i64 values. --- librustzcash/src/rustzcash.rs | 4 +- zcash_primitives/src/transaction/builder.rs | 12 ++--- .../src/transaction/components.rs | 2 +- .../src/transaction/components/amount.rs | 54 ++++++++++++------- zcash_primitives/src/transaction/mod.rs | 2 +- zcash_primitives/src/transaction/tests.rs | 4 +- 6 files changed, 46 insertions(+), 32 deletions(-) diff --git a/librustzcash/src/rustzcash.rs b/librustzcash/src/rustzcash.rs index 0fad706..99c3187 100644 --- a/librustzcash/src/rustzcash.rs +++ b/librustzcash/src/rustzcash.rs @@ -705,7 +705,7 @@ pub extern "system" fn librustzcash_sapling_final_check( binding_sig: *const [c_uchar; 64], sighash_value: *const [c_uchar; 32], ) -> bool { - let value_balance = match Amount::from_i64(value_balance, true) { + let value_balance = match Amount::from_i64(value_balance) { Ok(vb) => vb, Err(()) => return false, }; @@ -1028,7 +1028,7 @@ pub extern "system" fn librustzcash_sapling_binding_sig( sighash: *const [c_uchar; 32], result: *mut [c_uchar; 64], ) -> bool { - let value_balance = match Amount::from_i64(value_balance, true) { + let value_balance = match Amount::from_i64(value_balance) { Ok(vb) => vb, Err(()) => return false, }; diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 36490e4..ba07f34 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -564,7 +564,7 @@ mod tests { let to = extfvk.default_address().unwrap().1; let mut builder = Builder::new(0); - match builder.add_sapling_output(ovk, to, Amount::from_i64(-1, true).unwrap(), None) { + match builder.add_sapling_output(ovk, to, Amount::from_i64(-1).unwrap(), None) { Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), Ok(_) => panic!("Should have failed"), } @@ -575,7 +575,7 @@ mod tests { let mut builder = Builder::new(0); match builder.add_transparent_output( &TransparentAddress::PublicKey([0; 20]), - Amount::from_i64(-1, true).unwrap(), + Amount::from_i64(-1).unwrap(), ) { Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), Ok(_) => panic!("Should have failed"), @@ -596,7 +596,7 @@ mod tests { match builder.build(1, MockTxProver) { Err(e) => assert_eq!( e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-10000, true).unwrap()) + &ErrorKind::ChangeIsNegative(Amount::from_i64(-10000).unwrap()) ), Ok(_) => panic!("Should have failed"), } @@ -621,7 +621,7 @@ mod tests { match builder.build(1, MockTxProver) { Err(e) => assert_eq!( e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000, true).unwrap()) + &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) ), Ok(_) => panic!("Should have failed"), } @@ -640,7 +640,7 @@ mod tests { match builder.build(1, MockTxProver) { Err(e) => assert_eq!( e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000, true).unwrap()) + &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) ), Ok(_) => panic!("Should have failed"), } @@ -683,7 +683,7 @@ mod tests { match builder.build(1, MockTxProver) { Err(e) => assert_eq!( e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-1, true).unwrap()) + &ErrorKind::ChangeIsNegative(Amount::from_i64(-1).unwrap()) ), Ok(_) => panic!("Should have failed"), } diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index c7dcebd..d50b4fb 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -79,7 +79,7 @@ impl TxOut { let value = { let mut tmp = [0; 8]; reader.read_exact(&mut tmp)?; - Amount::from_i64_le_bytes(tmp, false) + Amount::from_nonnegative_i64_le_bytes(tmp) } .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "value out of range"))?; let script_pubkey = Script::read(&mut reader)?; diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index dca7056..d244876 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -18,10 +18,19 @@ impl Amount { /// Creates an Amount from an i64. /// /// Returns an error if the amount is out of range. - pub fn from_i64(amount: i64, allow_negative: bool) -> Result { - if 0 <= amount && amount <= MAX_MONEY { + pub fn from_i64(amount: i64) -> Result { + if -MAX_MONEY <= amount && amount <= MAX_MONEY { Ok(Amount(amount)) - } else if allow_negative && -MAX_MONEY <= amount && amount < 0 { + } else { + Err(()) + } + } + + /// Creates a non-negative Amount from an i64. + /// + /// Returns an error if the amount is out of range. + pub fn from_nonnegative_i64(amount: i64) -> Result { + if 0 <= amount && amount <= MAX_MONEY { Ok(Amount(amount)) } else { Err(()) @@ -42,9 +51,17 @@ impl Amount { /// Reads an Amount from a signed 64-bit little-endian integer. /// /// Returns an error if the amount is out of range. - pub fn from_i64_le_bytes(bytes: [u8; 8], allow_negative: bool) -> Result { + pub fn from_i64_le_bytes(bytes: [u8; 8]) -> Result { let amount = i64::from_le_bytes(bytes); - Amount::from_i64(amount, allow_negative) + Amount::from_i64(amount) + } + + /// Reads a non-negative Amount from a signed 64-bit little-endian integer. + /// + /// Returns an error if the amount is out of range. + pub fn from_nonnegative_i64_le_bytes(bytes: [u8; 8]) -> Result { + let amount = i64::from_le_bytes(bytes); + Amount::from_nonnegative_i64(amount) } /// Reads an Amount from an unsigned 64-bit little-endian integer. @@ -128,19 +145,16 @@ mod tests { let zero = b"\x00\x00\x00\x00\x00\x00\x00\x00"; assert_eq!(Amount::from_u64_le_bytes(zero.clone()).unwrap(), Amount(0)); assert_eq!( - Amount::from_i64_le_bytes(zero.clone(), false).unwrap(), - Amount(0) - ); - assert_eq!( - Amount::from_i64_le_bytes(zero.clone(), true).unwrap(), + Amount::from_nonnegative_i64_le_bytes(zero.clone()).unwrap(), Amount(0) ); + assert_eq!(Amount::from_i64_le_bytes(zero.clone()).unwrap(), Amount(0)); let neg_one = b"\xff\xff\xff\xff\xff\xff\xff\xff"; assert!(Amount::from_u64_le_bytes(neg_one.clone()).is_err()); - assert!(Amount::from_i64_le_bytes(neg_one.clone(), false).is_err()); + assert!(Amount::from_nonnegative_i64_le_bytes(neg_one.clone()).is_err()); assert_eq!( - Amount::from_i64_le_bytes(neg_one.clone(), true).unwrap(), + Amount::from_i64_le_bytes(neg_one.clone()).unwrap(), Amount(-1) ); @@ -150,30 +164,30 @@ mod tests { Amount(MAX_MONEY) ); assert_eq!( - Amount::from_i64_le_bytes(max_money.clone(), false).unwrap(), + Amount::from_nonnegative_i64_le_bytes(max_money.clone()).unwrap(), Amount(MAX_MONEY) ); assert_eq!( - Amount::from_i64_le_bytes(max_money.clone(), true).unwrap(), + Amount::from_i64_le_bytes(max_money.clone()).unwrap(), Amount(MAX_MONEY) ); let max_money_p1 = b"\x01\x40\x07\x5a\xf0\x75\x07\x00"; assert!(Amount::from_u64_le_bytes(max_money_p1.clone()).is_err()); - assert!(Amount::from_i64_le_bytes(max_money_p1.clone(), false).is_err()); - assert!(Amount::from_i64_le_bytes(max_money_p1.clone(), true).is_err()); + assert!(Amount::from_nonnegative_i64_le_bytes(max_money_p1.clone()).is_err()); + assert!(Amount::from_i64_le_bytes(max_money_p1.clone()).is_err()); let neg_max_money = b"\x00\xc0\xf8\xa5\x0f\x8a\xf8\xff"; assert!(Amount::from_u64_le_bytes(neg_max_money.clone()).is_err()); - assert!(Amount::from_i64_le_bytes(neg_max_money.clone(), false).is_err()); + assert!(Amount::from_nonnegative_i64_le_bytes(neg_max_money.clone()).is_err()); assert_eq!( - Amount::from_i64_le_bytes(neg_max_money.clone(), true).unwrap(), + Amount::from_i64_le_bytes(neg_max_money.clone()).unwrap(), Amount(-MAX_MONEY) ); let neg_max_money_m1 = b"\xff\xbf\xf8\xa5\x0f\x8a\xf8\xff"; assert!(Amount::from_u64_le_bytes(neg_max_money_m1.clone()).is_err()); - assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone(), false).is_err()); - assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone(), true).is_err()); + assert!(Amount::from_nonnegative_i64_le_bytes(neg_max_money_m1.clone()).is_err()); + assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone()).is_err()); } } diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index e5b83e5..aac0d35 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -188,7 +188,7 @@ impl Transaction { let vb = { let mut tmp = [0; 8]; reader.read_exact(&mut tmp)?; - Amount::from_i64_le_bytes(tmp, true) + Amount::from_i64_le_bytes(tmp) } .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "valueBalance out of range"))?; let ss = Vector::read(&mut reader, SpendDescription::read)?; diff --git a/zcash_primitives/src/transaction/tests.rs b/zcash_primitives/src/transaction/tests.rs index 7d84100..4cd5d72 100644 --- a/zcash_primitives/src/transaction/tests.rs +++ b/zcash_primitives/src/transaction/tests.rs @@ -1905,7 +1905,7 @@ fn zip_0143() { Some(( n as usize, Script(tv.script_code), - Amount::from_i64(tv.amount, false).unwrap(), + Amount::from_nonnegative_i64(tv.amount).unwrap(), )) } else { None @@ -5400,7 +5400,7 @@ fn zip_0243() { Some(( n as usize, Script(tv.script_code), - Amount::from_i64(tv.amount, false).unwrap(), + Amount::from_nonnegative_i64(tv.amount).unwrap(), )) } else { None From e6663212fff323d2750fdc6cf754da860cad1533 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 8 Aug 2019 09:21:09 +0100 Subject: [PATCH 15/19] Improve documentation for Amount --- .../src/transaction/components/amount.rs | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index d244876..f2d9d0f 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -6,6 +6,17 @@ const MAX_MONEY: i64 = 21_000_000 * COIN; pub const DEFAULT_FEE: Amount = Amount(10000); +/// A type-safe representation of some quantity of Zcash. +/// +/// An Amount can only be constructed from an integer that is within the valid monetary +/// range of `{-MAX_MONEY..MAX_MONEY}` (where `MAX_MONEY` = 21,000,000 × 10⁸ zatoshis). +/// However, this range is not preserved as an invariant internally; it is possible to +/// add two valid Amounts together to obtain an invalid Amount. It is the user's +/// responsibility to handle the result of serializing potentially-invalid Amounts. In +/// particular, a [`Transaction`] containing serialized invalid Amounts will be rejected +/// by the network consensus rules. +/// +/// [`Transaction`]: crate::transaction::Transaction #[derive(Clone, Copy, Debug, PartialEq)] pub struct Amount(i64); @@ -17,7 +28,7 @@ impl Amount { /// Creates an Amount from an i64. /// - /// Returns an error if the amount is out of range. + /// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. pub fn from_i64(amount: i64) -> Result { if -MAX_MONEY <= amount && amount <= MAX_MONEY { Ok(Amount(amount)) @@ -28,7 +39,7 @@ impl Amount { /// Creates a non-negative Amount from an i64. /// - /// Returns an error if the amount is out of range. + /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_nonnegative_i64(amount: i64) -> Result { if 0 <= amount && amount <= MAX_MONEY { Ok(Amount(amount)) @@ -39,7 +50,7 @@ impl Amount { /// Creates an Amount from a u64. /// - /// Returns an error if the amount is out of range. + /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_u64(amount: u64) -> Result { if amount <= MAX_MONEY as u64 { Ok(Amount(amount as i64)) @@ -50,7 +61,7 @@ impl Amount { /// Reads an Amount from a signed 64-bit little-endian integer. /// - /// Returns an error if the amount is out of range. + /// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. pub fn from_i64_le_bytes(bytes: [u8; 8]) -> Result { let amount = i64::from_le_bytes(bytes); Amount::from_i64(amount) @@ -58,7 +69,7 @@ impl Amount { /// Reads a non-negative Amount from a signed 64-bit little-endian integer. /// - /// Returns an error if the amount is out of range. + /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_nonnegative_i64_le_bytes(bytes: [u8; 8]) -> Result { let amount = i64::from_le_bytes(bytes); Amount::from_nonnegative_i64(amount) @@ -66,7 +77,7 @@ impl Amount { /// Reads an Amount from an unsigned 64-bit little-endian integer. /// - /// Returns an error if the amount is out of range. + /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_u64_le_bytes(bytes: [u8; 8]) -> Result { let amount = u64::from_le_bytes(bytes); Amount::from_u64(amount) @@ -77,13 +88,13 @@ impl Amount { self.0.to_le_bytes() } - /// Returns `true` if `self` is positive and `false` if the number is zero or + /// Returns `true` if `self` is positive and `false` if the Amount is zero or /// negative. pub const fn is_positive(self) -> bool { self.0.is_positive() } - /// Returns `true` if `self` is negative and `false` if the number is zero or + /// Returns `true` if `self` is negative and `false` if the Amount is zero or /// positive. pub const fn is_negative(self) -> bool { self.0.is_negative() From 3a73f946c58ba914f5c3f1d468f194e67b23a01d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 13 Aug 2019 15:10:57 +0100 Subject: [PATCH 16/19] Simplify structure of transaction builder errors --- zcash_primitives/src/transaction/builder.rs | 57 ++++++++------------- 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index ba07f34..ff9c35f 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -30,17 +30,8 @@ const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; /// with dummy outputs if necessary. See https://github.com/zcash/zcash/issues/3615 const MIN_SHIELDED_OUTPUTS: usize = 2; -#[derive(Debug)] -pub struct Error(ErrorKind); - -impl Error { - pub fn kind(&self) -> &ErrorKind { - &self.0 - } -} - #[derive(Debug, PartialEq)] -pub enum ErrorKind { +pub enum Error { AnchorMismatch, BindingSig, ChangeIsNegative(Amount), @@ -76,10 +67,10 @@ impl SaplingOutput { ) -> Result { let g_d = match to.g_d(&JUBJUB) { Some(g_d) => g_d, - None => return Err(Error(ErrorKind::InvalidAddress)), + None => return Err(Error::InvalidAddress), }; if value.is_negative() { - return Err(Error(ErrorKind::InvalidAmount)); + return Err(Error::InvalidAmount); } let rcm = Fs::random(rng); @@ -242,17 +233,16 @@ impl Builder { if let Some(anchor) = self.anchor { let witness_root: Fr = witness.root().into(); if witness_root != anchor { - return Err(Error(ErrorKind::AnchorMismatch)); + return Err(Error::AnchorMismatch); } } else { self.anchor = Some(witness.root().into()) } - let witness = witness.path().ok_or(Error(ErrorKind::InvalidWitness))?; + let witness = witness.path().ok_or(Error::InvalidWitness)?; let alpha = Fs::random(&mut self.rng); - self.mtx.value_balance += - Amount::from_u64(note.value).map_err(|_| Error(ErrorKind::InvalidAmount))?; + self.mtx.value_balance += Amount::from_u64(note.value).map_err(|_| Error::InvalidAmount)?; self.spends.push(SpendDescriptionInfo { extsk, @@ -289,7 +279,7 @@ impl Builder { value: Amount, ) -> Result<(), Error> { if value.is_negative() { - return Err(Error(ErrorKind::InvalidAmount)); + return Err(Error::InvalidAmount); } self.mtx.vout.push(TxOut { @@ -338,7 +328,7 @@ impl Builder { .map(|output| output.value) .sum::(); if change.is_negative() { - return Err(Error(ErrorKind::ChangeIsNegative(change))); + return Err(Error::ChangeIsNegative(change)); } // @@ -359,7 +349,7 @@ impl Builder { }, ) } else { - return Err(Error(ErrorKind::NoChangeAddress)); + return Err(Error::NoChangeAddress); }; self.add_sapling_output(change_address.0, change_address.1, change, None)?; @@ -419,7 +409,7 @@ impl Builder { anchor, spend.witness.clone(), ) - .map_err(|()| Error(ErrorKind::SpendProof))?; + .map_err(|()| Error::SpendProof)?; self.mtx.shielded_spends.push(SpendDescription { cv, @@ -529,7 +519,7 @@ impl Builder { self.mtx.binding_sig = Some( prover .binding_sig(&mut ctx, self.mtx.value_balance, &sighash) - .map_err(|()| Error(ErrorKind::BindingSig))?, + .map_err(|()| Error::BindingSig)?, ); Ok(( @@ -545,7 +535,7 @@ mod tests { use rand::rngs::OsRng; use sapling_crypto::jubjub::fs::Fs; - use super::{Builder, ErrorKind}; + use super::{Builder, Error}; use crate::{ legacy::TransparentAddress, merkle_tree::{CommitmentTree, IncrementalWitness}, @@ -565,7 +555,7 @@ mod tests { let mut builder = Builder::new(0); match builder.add_sapling_output(ovk, to, Amount::from_i64(-1).unwrap(), None) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), + Err(e) => assert_eq!(e, Error::InvalidAmount), Ok(_) => panic!("Should have failed"), } } @@ -577,7 +567,7 @@ mod tests { &TransparentAddress::PublicKey([0; 20]), Amount::from_i64(-1).unwrap(), ) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount), + Err(e) => assert_eq!(e, Error::InvalidAmount), Ok(_) => panic!("Should have failed"), } } @@ -595,8 +585,8 @@ mod tests { let builder = Builder::new(0); match builder.build(1, MockTxProver) { Err(e) => assert_eq!( - e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-10000).unwrap()) + e, + Error::ChangeIsNegative(Amount::from_i64(-10000).unwrap()) ), Ok(_) => panic!("Should have failed"), } @@ -620,8 +610,8 @@ mod tests { .unwrap(); match builder.build(1, MockTxProver) { Err(e) => assert_eq!( - e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) + e, + Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) ), Ok(_) => panic!("Should have failed"), } @@ -639,8 +629,8 @@ mod tests { .unwrap(); match builder.build(1, MockTxProver) { Err(e) => assert_eq!( - e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) + e, + Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) ), Ok(_) => panic!("Should have failed"), } @@ -681,10 +671,7 @@ mod tests { ) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!( - e.kind(), - &ErrorKind::ChangeIsNegative(Amount::from_i64(-1).unwrap()) - ), + Err(e) => assert_eq!(e, Error::ChangeIsNegative(Amount::from_i64(-1).unwrap())), Ok(_) => panic!("Should have failed"), } } @@ -718,7 +705,7 @@ mod tests { ) .unwrap(); match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e.kind(), &ErrorKind::BindingSig), + Err(e) => assert_eq!(e, Error::BindingSig), Ok(_) => panic!("Should have failed"), } } From 1760b275a7388bbe1e12e126640c2f520b422e92 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 13 Aug 2019 15:24:08 +0100 Subject: [PATCH 17/19] Simplify transaction builder tests Requires impl PartialEq for Transaction, which is implemented as a TxId comparison (relying on the invariant that Transaction is immutable). --- zcash_primitives/src/transaction/builder.rs | 69 +++++++++------------ zcash_primitives/src/transaction/mod.rs | 6 ++ 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index ff9c35f..a3134c3 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -131,6 +131,7 @@ impl SaplingOutput { } /// Metadata about a transaction created by a [`Builder`]. +#[derive(Debug, PartialEq)] pub struct TransactionMetadata { spend_indices: Vec, output_indices: Vec, @@ -554,22 +555,22 @@ mod tests { let to = extfvk.default_address().unwrap().1; let mut builder = Builder::new(0); - match builder.add_sapling_output(ovk, to, Amount::from_i64(-1).unwrap(), None) { - Err(e) => assert_eq!(e, Error::InvalidAmount), - Ok(_) => panic!("Should have failed"), - } + assert_eq!( + builder.add_sapling_output(ovk, to, Amount::from_i64(-1).unwrap(), None), + Err(Error::InvalidAmount) + ); } #[test] fn fails_on_negative_transparent_output() { let mut builder = Builder::new(0); - match builder.add_transparent_output( - &TransparentAddress::PublicKey([0; 20]), - Amount::from_i64(-1).unwrap(), - ) { - Err(e) => assert_eq!(e, Error::InvalidAmount), - Ok(_) => panic!("Should have failed"), - } + assert_eq!( + builder.add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_i64(-1).unwrap(), + ), + Err(Error::InvalidAmount) + ); } #[test] @@ -583,13 +584,10 @@ mod tests { // 0.0001 t-ZEC fee { let builder = Builder::new(0); - match builder.build(1, MockTxProver) { - Err(e) => assert_eq!( - e, - Error::ChangeIsNegative(Amount::from_i64(-10000).unwrap()) - ), - Ok(_) => panic!("Should have failed"), - } + assert_eq!( + builder.build(1, MockTxProver), + Err(Error::ChangeIsNegative(Amount::from_i64(-10000).unwrap())) + ); } let extfvk = ExtendedFullViewingKey::from(&extsk); @@ -608,13 +606,10 @@ mod tests { None, ) .unwrap(); - match builder.build(1, MockTxProver) { - Err(e) => assert_eq!( - e, - Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) - ), - Ok(_) => panic!("Should have failed"), - } + assert_eq!( + builder.build(1, MockTxProver), + Err(Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap())) + ); } // Fail if there is only a transparent output @@ -627,13 +622,10 @@ mod tests { Amount::from_u64(50000).unwrap(), ) .unwrap(); - match builder.build(1, MockTxProver) { - Err(e) => assert_eq!( - e, - Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap()) - ), - Ok(_) => panic!("Should have failed"), - } + assert_eq!( + builder.build(1, MockTxProver), + Err(Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap())) + ); } let note1 = to @@ -670,10 +662,10 @@ mod tests { Amount::from_u64(20000).unwrap(), ) .unwrap(); - match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e, Error::ChangeIsNegative(Amount::from_i64(-1).unwrap())), - Ok(_) => panic!("Should have failed"), - } + assert_eq!( + builder.build(1, MockTxProver), + Err(Error::ChangeIsNegative(Amount::from_i64(-1).unwrap())) + ); } let note2 = to.create_note(1, Fs::random(&mut rng), &JUBJUB).unwrap(); @@ -704,10 +696,7 @@ mod tests { Amount::from_u64(20000).unwrap(), ) .unwrap(); - match builder.build(1, MockTxProver) { - Err(e) => assert_eq!(e, Error::BindingSig), - Ok(_) => panic!("Should have failed"), - } + assert_eq!(builder.build(1, MockTxProver), Err(Error::BindingSig)) } } } diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index aac0d35..a8ecb17 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -50,6 +50,12 @@ impl Deref for Transaction { } } +impl PartialEq for Transaction { + fn eq(&self, other: &Transaction) -> bool { + self.txid == other.txid + } +} + pub struct TransactionData { pub overwintered: bool, pub version: u32, From a28d94ff2e30897aca6806ff66d0d059f50f706e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 14 Aug 2019 00:16:09 +0100 Subject: [PATCH 18/19] Panic if Amount addition or subtraction overflows --- .../src/transaction/components/amount.rs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index f2d9d0f..8d358fb 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -117,7 +117,7 @@ impl Add for Amount { type Output = Amount; fn add(self, rhs: Amount) -> Amount { - Amount(self.0 + rhs.0) + Amount::from_i64(self.0 + rhs.0).expect("addition should remain in range") } } @@ -131,7 +131,7 @@ impl Sub for Amount { type Output = Amount; fn sub(self, rhs: Amount) -> Amount { - Amount(self.0 - rhs.0) + Amount::from_i64(self.0 - rhs.0).expect("subtraction should remain in range") } } @@ -201,4 +201,32 @@ mod tests { assert!(Amount::from_nonnegative_i64_le_bytes(neg_max_money_m1.clone()).is_err()); assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone()).is_err()); } + + #[test] + #[should_panic] + fn add_panics_on_overflow() { + let v = Amount(MAX_MONEY); + let sum = v + Amount(1); + } + + #[test] + #[should_panic] + fn add_assign_panics_on_overflow() { + let mut a = Amount(MAX_MONEY); + a += Amount(1); + } + + #[test] + #[should_panic] + fn sub_panics_on_underflow() { + let v = Amount(-MAX_MONEY); + let diff = v - Amount(1); + } + + #[test] + #[should_panic] + fn sub_assign_panics_on_underflow() { + let mut a = Amount(-MAX_MONEY); + a -= Amount(1); + } } From 4a6c9ec4259931d14bd6be87d53c12850991ebaf Mon Sep 17 00:00:00 2001 From: str4d Date: Wed, 14 Aug 2019 00:30:18 +0100 Subject: [PATCH 19/19] Comment that we support a minimal set of script opcodes Co-Authored-By: Daira Hopwood --- zcash_primitives/src/legacy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zcash_primitives/src/legacy.rs b/zcash_primitives/src/legacy.rs index 02977ab..d1d7c1a 100644 --- a/zcash_primitives/src/legacy.rs +++ b/zcash_primitives/src/legacy.rs @@ -6,7 +6,7 @@ use std::ops::Shl; use crate::serialize::Vector; -/// Script opcodes. +/// Minimal subset of script opcodes. enum OpCode { // push value PushData1 = 0x4c,