ZIP-225/244 #1: Minor refactoring and preparatory updates.

components/zcash_note_encryption/src/lib.rs

@@ -445,7 +445,6 @@ pub fn try_compact_note_decryption<D: Domain, Output: ShieldedOutput<D>>(
 ///
 /// Implements part of section 4.19.3 of the
 /// [Zcash Protocol Specification](https://zips.z.cash/protocol/nu5.pdf#decryptovk)
-/// For decryption using a Full Viewing Key see [`try_sapling_output_recovery`].
 pub fn try_output_recovery_with_ock<D: Domain, Output: ShieldedOutput<D>>(
     domain: &D,
     ock: &OutgoingCipherKey,

zcash_client_backend/Cargo.toml

@@ -23,7 +23,7 @@ hex = "0.4"
 jubjub = "0.6"
 nom = "6.1"
 percent-encoding = "2.1.0"
-proptest = { version = "0.10.1", optional = true }
+proptest = { version = "1.0.0", optional = true }
 protobuf = "2.20"
 rand_core = "0.6"
 subtle = "2.2.3"

zcash_client_backend/src/data_api.rs

@@ -412,14 +412,14 @@ pub mod testing {
             &mut self,
             _received_tx: &ReceivedTransaction,
         ) -> Result<Self::TxRef, Self::Error> {
-            Ok(TxId([0u8; 32]))
+            Ok(TxId::from_bytes([0u8; 32]))
         }
 
         fn store_sent_tx(
             &mut self,
             _sent_tx: &SentTransaction,
         ) -> Result<Self::TxRef, Self::Error> {
-            Ok(TxId([0u8; 32]))
+            Ok(TxId::from_bytes([0u8; 32]))
         }
 
         fn rewind_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> {

zcash_client_backend/src/data_api/error.rs

@@ -24,6 +24,9 @@ pub enum ChainInvalid {
 
 #[derive(Debug)]
 pub enum Error<NoteId> {
+    /// The amount specified exceeds the allowed range.
+    InvalidAmount,
+
     /// Unable to create a new spend because the wallet balance is not sufficient.
     InsufficientBalance(Amount, Amount),
 
@@ -72,6 +75,10 @@ impl ChainInvalid {
 impl<N: fmt::Display> fmt::Display for Error<N> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match &self {
+            Error::InvalidAmount => write!(
+                f,
+                "The value lies outside the valid range of Zcash amounts."
+            ),
             Error::InsufficientBalance(have, need) => write!(
                 f,
                 "Insufficient balance (have {}, need {} including fee)",

zcash_client_backend/src/data_api/wallet.rs

@@ -183,11 +183,15 @@ where
         .get_target_and_anchor_heights()
         .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?;
 
-    let target_value = value + DEFAULT_FEE;
+    let target_value = (value + DEFAULT_FEE).ok_or_else(|| E::from(Error::InvalidAmount))?;
     let spendable_notes = wallet_db.select_spendable_notes(account, target_value, anchor_height)?;
 
     // Confirm we were able to select sufficient value
-    let selected_value = spendable_notes.iter().map(|n| n.note_value).sum();
+    let selected_value = spendable_notes
+        .iter()
+        .map(|n| n.note_value)
+        .sum::<Option<_>>()
+        .ok_or_else(|| E::from(Error::InvalidAmount))?;
     if selected_value < target_value {
         return Err(E::from(Error::InsufficientBalance(
             selected_value,

zcash_client_backend/src/proto.rs

@@ -8,7 +8,10 @@ use zcash_primitives::{
     block::{BlockHash, BlockHeader},
     consensus::BlockHeight,
     sapling::Nullifier,
-    transaction::components::sapling::{CompactOutputDescription, OutputDescription},
+    transaction::{
+        components::sapling::{CompactOutputDescription, OutputDescription},
+        TxId,
+    },
 };
 
 use zcash_note_encryption::COMPACT_NOTE_SIZE;
@@ -74,6 +77,15 @@ impl compact_formats::CompactBlock {
     }
 }
 
+impl compact_formats::CompactTx {
+    /// Returns the transaction Id
+    pub fn txid(&self) -> TxId {
+        let mut hash = [0u8; 32];
+        hash.copy_from_slice(&self.hash);
+        TxId::from_bytes(hash)
+    }
+}
+
 impl compact_formats::CompactOutput {
     /// Returns the note commitment for this output.
     ///

zcash_client_backend/src/welding_rig.rs

@@ -12,7 +12,7 @@ use zcash_primitives::{
         note_encryption::{try_sapling_compact_note_decryption, SaplingDomain},
         Node, Note, Nullifier, PaymentAddress, SaplingIvk,
     },
-    transaction::{components::sapling::CompactOutputDescription, TxId},
+    transaction::components::sapling::CompactOutputDescription,
     zip32::ExtendedFullViewingKey,
 };
 
@@ -32,7 +32,8 @@ use crate::wallet::{AccountId, WalletShieldedOutput, WalletShieldedSpend, Wallet
 fn scan_output<P: consensus::Parameters, K: ScanningKey>(
     params: &P,
     height: BlockHeight,
-    (index, output): (usize, CompactOutput),
+    index: usize,
+    output: CompactOutput,
     vks: &[(&AccountId, &K)],
     spent_from_accounts: &HashSet<AccountId>,
     tree: &mut CommitmentTree<Node>,
@@ -198,6 +199,8 @@ pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
     let block_height = block.height();
 
     for tx in block.vtx.into_iter() {
+        let txid = tx.txid();
+        let index = tx.index as usize;
         let num_spends = tx.spends.len();
         let num_outputs = tx.outputs.len();
 
@@ -249,7 +252,7 @@ pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
                 })
                 .collect();
 
-            for to_scan in tx.outputs.into_iter().enumerate() {
+            for (idx, c_out) in tx.outputs.into_iter().enumerate() {
                 // Grab mutable references to new witnesses from previous outputs
                 // in this transaction so that we can update them. Scoped so we
                 // don't hold mutable references to shielded_outputs for too long.
@@ -261,7 +264,8 @@ pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
                 if let Some(output) = scan_output(
                     params,
                     block_height,
-                    to_scan,
+                    idx,
+                    c_out,
                     vks,
                     &spent_from_accounts,
                     tree,
@@ -275,11 +279,9 @@ pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
         }
 
         if !(shielded_spends.is_empty() && shielded_outputs.is_empty()) {
-            let mut txid = TxId([0u8; 32]);
-            txid.0.copy_from_slice(&tx.hash);
             wtxs.push(WalletTx {
                 txid,
-                index: tx.index as usize,
+                index,
                 num_spends,
                 num_outputs,
                 shielded_spends,

zcash_client_sqlite/src/chain.rs

@@ -358,13 +358,19 @@ mod tests {
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
 
         // Account balance should reflect both received notes
-        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value + value2);
+        assert_eq!(
+            get_balance(&db_data, AccountId(0)).unwrap(),
+            (value + value2).unwrap()
+        );
 
         // "Rewind" to height of last scanned block
         rewind_to_height(&db_data, sapling_activation_height() + 1).unwrap();
 
         // Account balance should be unaltered
-        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value + value2);
+        assert_eq!(
+            get_balance(&db_data, AccountId(0)).unwrap(),
+            (value + value2).unwrap()
+        );
 
         // Rewind so that one block is dropped
         rewind_to_height(&db_data, sapling_activation_height()).unwrap();
@@ -376,7 +382,10 @@ mod tests {
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
 
         // Account balance should again reflect both received notes
-        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value + value2);
+        assert_eq!(
+            get_balance(&db_data, AccountId(0)).unwrap(),
+            (value + value2).unwrap()
+        );
     }
 
     #[test]
@@ -485,7 +494,10 @@ mod tests {
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
 
         // Account balance should reflect both received notes
-        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value + value2);
+        assert_eq!(
+            get_balance(&db_data, AccountId(0)).unwrap(),
+            (value + value2).unwrap()
+        );
     }
 
     #[test]
@@ -543,6 +555,9 @@ mod tests {
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
 
         // Account balance should equal the change
-        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value - value2);
+        assert_eq!(
+            get_balance(&db_data, AccountId(0)).unwrap(),
+            (value - value2).unwrap()
+        );
     }
 }

zcash_client_sqlite/src/lib.rs

@@ -701,7 +701,7 @@ mod tests {
             let note = Note {
                 g_d: change_addr.diversifier().g_d().unwrap(),
                 pk_d: *change_addr.pk_d(),
-                value: (in_value - value).into(),
+                value: (in_value - value).unwrap().into(),
                 rseed,
             };
             let encryptor = sapling_note_encryption::<_, Network>(

zcash_client_sqlite/src/wallet.rs

@@ -389,7 +389,7 @@ pub fn block_height_extrema<P>(
 ///
 /// let data_file = NamedTempFile::new().unwrap();
 /// let db = WalletDb::for_path(data_file, Network::TestNetwork).unwrap();
-/// let height = get_tx_height(&db, TxId([0u8; 32]));
+/// let height = get_tx_height(&db, TxId::from_bytes([0u8; 32]));
 /// ```
 pub fn get_tx_height<P>(
     wdb: &WalletDb<P>,
@@ -398,7 +398,7 @@ pub fn get_tx_height<P>(
     wdb.conn
         .query_row(
             "SELECT block FROM transactions WHERE txid = ?",
-            &[txid.0.to_vec()],
+            &[txid.as_ref().to_vec()],
             |row| row.get(0).map(u32::into),
         )
         .optional()
@@ -616,7 +616,7 @@ pub fn put_tx_meta<'a, P, N>(
     tx: &WalletTx<N>,
     height: BlockHeight,
 ) -> Result<i64, SqliteClientError> {
-    let txid = tx.txid.0.to_vec();
+    let txid = tx.txid.as_ref().to_vec();
     if stmts
         .stmt_update_tx_meta
         .execute(params![u32::from(height), (tx.index as i64), txid])?
@@ -643,7 +643,7 @@ pub fn put_tx_data<'a, P>(
     tx: &Transaction,
     created_at: Option<time::OffsetDateTime>,
 ) -> Result<i64, SqliteClientError> {
-    let txid = tx.txid().0.to_vec();
+    let txid = tx.txid().as_ref().to_vec();
 
     let mut raw_tx = vec![];
     tx.write(&mut raw_tx)?;

zcash_client_sqlite/src/wallet/transact.rs

@@ -356,7 +356,10 @@ mod tests {
 
         // Verified balance does not include the second note
         let (_, anchor_height2) = (&db_data).get_target_and_anchor_heights().unwrap().unwrap();
-        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value + value);
+        assert_eq!(
+            get_balance(&db_data, AccountId(0)).unwrap(),
+            (value + value).unwrap()
+        );
         assert_eq!(
             get_balance_at(&db_data, AccountId(0), anchor_height2).unwrap(),
             value

zcash_extensions/src/transparent/demo.rs

@@ -625,7 +625,7 @@ mod tests {
         //
 
         let in_b = TzeIn {
-            prevout: TzeOutPoint::new(tx_a.txid().0, 0),
+            prevout: TzeOutPoint::new(tx_a.txid(), 0),
             witness: tze::Witness::from(0, &Witness::open(preimage_1)),
         };
         let out_b = TzeOut {
@@ -642,7 +642,7 @@ mod tests {
         //
 
         let in_c = TzeIn {
-            prevout: TzeOutPoint::new(tx_b.txid().0, 0),
+            prevout: TzeOutPoint::new(tx_b.txid(), 0),
             witness: tze::Witness::from(0, &Witness::close(preimage_2)),
         };
 
@@ -737,10 +737,10 @@ mod tests {
             extension_id: 0,
         };
         let prevout_a = (
-            TzeOutPoint::new(tx_a.txid().0, 0),
+            TzeOutPoint::new(tx_a.txid(), 0),
             tx_a.tze_outputs[0].clone(),
         );
-        let value_xfr = value - DEFAULT_FEE;
+        let value_xfr = (value - DEFAULT_FEE).unwrap();
         db_b.demo_transfer_to_close(prevout_a, value_xfr, preimage_1, h2)
             .map_err(|e| format!("transfer failure: {:?}", e))
             .unwrap();
@@ -759,7 +759,7 @@ mod tests {
             extension_id: 0,
         };
         let prevout_b = (
-            TzeOutPoint::new(tx_a.txid().0, 0),
+            TzeOutPoint::new(tx_a.txid(), 0),
             tx_b.tze_outputs[0].clone(),
         );
         db_c.demo_close(prevout_b, preimage_2)
@@ -769,7 +769,7 @@ mod tests {
         builder_c
             .add_transparent_output(
                 &TransparentAddress::PublicKey([0; 20]),
-                value_xfr - DEFAULT_FEE,
+                (value_xfr - DEFAULT_FEE).unwrap(),
             )
             .unwrap();
 

zcash_primitives/CHANGELOG.md

@@ -10,7 +10,12 @@ and this library adheres to Rust's notion of
 - `zcash_primitives::transaction::Builder::with_progress_notifier`, for setting
   a notification channel on which transaction build progress updates will be
   sent.
-
+- `zcash_primitives::transaction::Txid::{read, write, from_bytes}`
+- `zcash_primitives::sapling::NoteValue` a typesafe wrapper for Sapling note values.
+- `zcash_primitives::consensus::BranchId::{height_range, height_bounds}` functions 
+  to provide range values for branch active heights.
+- `zcash_primitives::consensus::NetworkUpgrade::Nu5` value representing the Nu5 upgrade.
+- `zcash_primitives::consensus::BranchId::Nu5` value representing the Nu5 consensus branch.
 ### Changed
 - MSRV is now 1.51.0.
 - The following modules and helpers have been moved into
@@ -24,6 +29,13 @@ and this library adheres to Rust's notion of
   - `zcash_primitives::util::{hash_to_scalar, generate_random_rseed}`
 - Renamed `zcash_primitives::transaction::components::JSDescription` to
   `JsDescription` (matching Rust naming conventions).
+- `zcash_primitives::transaction::TxId` contents is now private.
+- Renamed `zcash_primitives::transaction::components::tze::hash` to
+  `zcash_primitives::transaction::components::tze::txid`
+- `zcash_primitives::transaction::components::tze::TzeOutPoint` constructor
+  now taxes a TxId rather than a raw byte array.
+- `zcash_primitives::transaction::components::Amount` addition, subtraction,
+  and summation now return `Option` rather than panicing on overflow.
 
 ## [0.5.0] - 2021-03-26
 ### Added

zcash_primitives/Cargo.toml

@@ -31,7 +31,9 @@ hex = "0.4"
 jubjub = "0.6"
 lazy_static = "1"
 log = "0.4"
-proptest = { version = "0.10.1", optional = true }
+nonempty = "0.6"
+orchard = { git = "https://github.com/zcash/orchard", branch = "main" }
+proptest = { version = "1.0.0", optional = true }
 rand = "0.8"
 rand_core = "0.6"
 ripemd160 = { version = "0.9", optional = true }
@@ -43,11 +45,16 @@ zcash_note_encryption = { version = "0.0", path = "../components/zcash_note_encr
 # Temporary workaround for https://github.com/myrrlyn/funty/issues/3
 funty = "=1.1.0"
 
+[dependencies.pasta_curves]
+git = "https://github.com/zcash/pasta_curves.git"
+rev = "b55a6960dfafd7f767e2820ddf1adaa499322f98"
+
 [dev-dependencies]
 criterion = "0.3"
 hex-literal = "0.3"
-proptest = "0.10.1"
+proptest = "1.0.0"
 rand_xorshift = "0.3"
+orchard = { git = "https://github.com/zcash/orchard", branch = "main", features = ["test-dependencies"] }
 
 [features]
 transparent-inputs = ["ripemd160", "secp256k1"]