Merge pull request #1043 from zcash/zip32-refactor

Refactor ZIP 32 code

zcash_client_backend/src/data_api/wallet.rs

@@ -9,6 +9,7 @@ use zcash_primitives::{
         self,
         note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey},
         prover::{OutputProver, SpendProver},
+        zip32::{DiversifiableFullViewingKey, ExtendedSpendingKey},
         Node,
     },
     transaction::{
@@ -17,7 +18,7 @@ use zcash_primitives::{
         fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
         Transaction, TxId,
     },
-    zip32::{sapling::DiversifiableFullViewingKey, sapling::ExtendedSpendingKey, AccountId, Scope},
+    zip32::{AccountId, Scope},
 };
 
 use crate::{

zcash_client_backend/src/keys.rs

@@ -28,7 +28,7 @@ use {
 };
 
 pub mod sapling {
-    pub use zcash_primitives::zip32::sapling::{
+    pub use zcash_primitives::sapling::zip32::{
         DiversifiableFullViewingKey, ExtendedFullViewingKey, ExtendedSpendingKey,
     };
     use zcash_primitives::zip32::{AccountId, ChildIndex};
@@ -62,9 +62,9 @@ pub mod sapling {
         ExtendedSpendingKey::from_path(
             &ExtendedSpendingKey::master(seed),
             &[
-                ChildIndex::Hardened(32),
+                ChildIndex::hardened(32),
-                ChildIndex::Hardened(coin_type),
+                ChildIndex::hardened(coin_type),
-                ChildIndex::Hardened(account.into()),
+                account.into(),
             ],
         )
     }

zcash_client_backend/src/scanning.rs

@@ -13,9 +13,10 @@ use zcash_primitives::{
     sapling::{
         self,
         note_encryption::{CompactOutputDescription, PreparedIncomingViewingKey, SaplingDomain},
+        zip32::DiversifiableFullViewingKey,
         SaplingIvk,
     },
-    zip32::{sapling::DiversifiableFullViewingKey, AccountId, Scope},
+    zip32::{AccountId, Scope},
 };
 
 use crate::data_api::{BlockMetadata, ScannedBlock, ShieldedProtocol};

zcash_client_sqlite/src/testing.rs

@@ -49,6 +49,7 @@ use zcash_primitives::{
         note_encryption::{sapling_note_encryption, SaplingDomain},
         util::generate_random_rseed,
         value::NoteValue,
+        zip32::DiversifiableFullViewingKey,
         Note, Nullifier, PaymentAddress,
     },
     transaction::{
@@ -56,7 +57,7 @@ use zcash_primitives::{
         fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
         Transaction, TxId,
     },
-    zip32::{sapling::DiversifiableFullViewingKey, DiversifierIndex},
+    zip32::DiversifierIndex,
 };
 
 use crate::{

zcash_client_sqlite/src/wallet/init.rs

@@ -176,8 +176,9 @@ mod tests {
 
     use zcash_primitives::{
         consensus::{self, BlockHeight, BranchId, Network, NetworkUpgrade, Parameters},
+        sapling::zip32::ExtendedFullViewingKey,
         transaction::{TransactionData, TxVersion},
-        zip32::{sapling::ExtendedFullViewingKey, AccountId},
+        zip32::AccountId,
     };
 
     use crate::{testing::TestBuilder, wallet::scanning::priority_code, WalletDb};

zcash_client_sqlite/src/wallet/sapling.rs

@@ -429,6 +429,7 @@ pub(crate) mod tests {
         sapling::{
             note_encryption::try_sapling_output_recovery,
             prover::{OutputProver, SpendProver},
+            zip32::ExtendedSpendingKey,
             Node, Note, PaymentAddress,
         },
         transaction::{
@@ -438,7 +439,7 @@ pub(crate) mod tests {
             },
             Transaction,
         },
-        zip32::{sapling::ExtendedSpendingKey, Scope},
+        zip32::Scope,
     };
 
     use zcash_client_backend::{

zcash_primitives/CHANGELOG.md

@@ -54,6 +54,7 @@ and this library adheres to Rust's notion of
     - `ValueCommitTrapdoor::from_bytes`
     - `impl Sub<TrapdoorSum> for TrapdoorSum`
     - `impl Sub<CommitmentSum> for CommitmentSum`
+  - `zip32` module (moved from `zcash_primitives::zip32::sapling`).
   - `impl Debug for keys::{ExpandedSpendingKey, ProofGenerationKey}`
 - `zcash_primitives::transaction`:
   - `builder::get_fee`
@@ -71,6 +72,12 @@ and this library adheres to Rust's notion of
     - `GRACE_ACTIONS`
     - `P2PKH_STANDARD_INPUT_SIZE`
     - `P2PKH_STANDARD_OUTPUT_SIZE`
+- `zcash_primitives::zip32`:
+  - `ChildIndex::hardened`
+  - `ChildIndex::index`
+  - `ChainCode::new`
+  - `ChainCode::as_bytes`
+  - `impl From<AccountId> for ChildIndex`
 - Test helpers, behind the `test-dependencies` feature flag:
   - `zcash_primitives::sapling::prover::mock::{MockSpendProver, MockOutputProver}`
 - Additions related to `zcash_primitive::components::amount::Amount`
@@ -120,6 +127,7 @@ and this library adheres to Rust's notion of
   - `bundle::MapAuth` trait methods now take `&mut self` instead of `&self`.
   - `circuit::ValueCommitmentOpening::value` is now represented as a `NoteValue`
     instead of as a bare `u64`.
+  - `keys::DecodingError` has a new variant `UnsupportedChildIndex`.
   - `note_encryption`:
     - `SaplingDomain` no longer has a `P: consensus::Parameters` type parameter.
     - The following methods now take a `Zip212Enforcement` argument instead of a
@@ -154,6 +162,9 @@ and this library adheres to Rust's notion of
     - `fees::fixed::FeeRule::fixed_fee`
     - `fees::zip317::FeeRule::marginal_fee`
     - `sighash::TransparentAuthorizingContext::input_amounts`
+- `zcash_primitives::zip32`:
+  - `ChildIndex` has been changed from an enum to an opaque struct, and no
+    longer supports non-hardened indices.
 
 ### Removed
 - `zcash_primitives::constants`:
@@ -187,6 +198,11 @@ and this library adheres to Rust's notion of
   - `Bundle::<Unauthorized>::apply_signatures` (use
     `Bundle::<InProgress<Proven, Unsigned>>::apply_signatures` instead).
 - `impl From<zcash_primitive::components::transaction::Amount> for u64`
+- `zcash_primitives::zip32`:
+  - `sapling` module (moved from `zcash_primitives::sapling::zip32`).
+  - `ChildIndex::Hardened` (use `ChildIndex::hardened` instead).
+  - `ChildIndex::NonHardened`
+  - `sapling::ExtendedFullViewingKey::derive_child`
 
 ## [0.13.0] - 2023-09-25
 ### Added

zcash_primitives/src/sapling.rs

@@ -17,6 +17,7 @@ mod tree;
 pub mod util;
 pub mod value;
 mod verifier;
+pub mod zip32;
 
 use group::GroupEncoding;
 use rand_core::{CryptoRng, RngCore};

zcash_primitives/src/sapling/builder.rs

@@ -865,17 +865,15 @@ pub mod testing {
     use proptest::prelude::*;
     use rand::{rngs::StdRng, SeedableRng};
 
-    use crate::{
+    use crate::sapling::{
-        sapling::{
+        bundle::{Authorized, Bundle},
-            bundle::{Authorized, Bundle},
+        note_encryption::Zip212Enforcement,
-            note_encryption::Zip212Enforcement,
+        prover::mock::{MockOutputProver, MockSpendProver},
-            prover::mock::{MockOutputProver, MockSpendProver},
+        redjubjub::PrivateKey,
-            redjubjub::PrivateKey,
+        testing::{arb_node, arb_note},
-            testing::{arb_node, arb_note},
+        value::testing::arb_positive_note_value,
-            value::testing::arb_positive_note_value,
+        zip32::testing::arb_extended_spending_key,
-            Diversifier,
+        Diversifier,
-        },
-        zip32::sapling::testing::arb_extended_spending_key,
     };
     use incrementalmerkletree::{
         frontier::testing::arb_commitment_tree, witness::IncrementalWitness,

zcash_primitives/src/sapling/keys.rs

@@ -33,6 +33,9 @@ pub enum DecodingError {
     InvalidAsk,
     /// Could not decode the `nsk` bytes to a jubjub field element.
     InvalidNsk,
+    /// An extended spending key had an unsupported child index: either a non-hardened
+    /// index, or a non-zero index at depth 0.
+    UnsupportedChildIndex,
 }
 
 /// An outgoing viewing key
@@ -101,7 +104,9 @@ impl ExpandedSpendingKey {
             DecodingError::InvalidNsk => {
                 io::Error::new(io::ErrorKind::InvalidData, "nsk not in field")
             }
-            DecodingError::LengthInvalid { .. } => unreachable!(),
+            DecodingError::LengthInvalid { .. } | DecodingError::UnsupportedChildIndex => {
+                unreachable!()
+            }
         })
     }
 

zcash_primitives/src/zip32.rs

@@ -5,12 +5,10 @@
 use memuse::{self, DynamicUsage};
 use subtle::{Choice, ConditionallySelectable};
 
-use crate::sapling::{Diversifier, NullifierDerivingKey, PaymentAddress, ViewingKey};
 pub mod fingerprint;
-pub mod sapling;
 
 #[deprecated(note = "Please use the types exported from the `zip32::sapling` module instead.")]
-pub use sapling::{
+pub use crate::sapling::zip32::{
     sapling_address, sapling_default_address, sapling_derive_internal_fvk, sapling_find_address,
     DiversifiableFullViewingKey, ExtendedFullViewingKey, ExtendedSpendingKey,
     ZIP32_SAPLING_FVFP_PERSONALIZATION, ZIP32_SAPLING_INT_PERSONALIZATION,
@@ -35,6 +33,13 @@ impl From<AccountId> for u32 {
     }
 }
 
+impl From<AccountId> for ChildIndex {
+    fn from(id: AccountId) -> Self {
+        // Account IDs are always hardened in derivation paths.
+        ChildIndex::hardened(id.0)
+    }
+}
+
 impl ConditionallySelectable for AccountId {
     fn conditional_select(a0: &Self, a1: &Self, c: Choice) -> Self {
         AccountId(u32::conditional_select(&a0.0, &a1.0, c))
@@ -43,41 +48,54 @@ impl ConditionallySelectable for AccountId {
 
 // ZIP 32 structures
 
-/// A child index for a derived key
+/// A child index for a derived key.
+///
+/// Only hardened derivation is supported.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum ChildIndex {
+pub struct ChildIndex(u32);
-    NonHardened(u32),
-    Hardened(u32), // Hardened(n) == n + (1 << 31) == n' in path notation
-}
 
 impl ChildIndex {
-    pub fn from_index(i: u32) -> Self {
+    /// Parses the given ZIP 32 child index.
-        match i {
+    ///
-            n if n >= (1 << 31) => ChildIndex::Hardened(n - (1 << 31)),
+    /// Returns `None` if the hardened bit is not set.
-            n => ChildIndex::NonHardened(n),
+    pub fn from_index(i: u32) -> Option<Self> {
+        if i >= (1 << 31) {
+            Some(ChildIndex(i))
+        } else {
+            None
         }
     }
 
-    fn master() -> Self {
+    /// Constructs a hardened `ChildIndex` from the given value.
-        ChildIndex::from_index(0)
+    ///
+    /// # Panics
+    ///
+    /// Panics if `value >= (1 << 31)`.
+    pub const fn hardened(value: u32) -> Self {
+        assert!(value < (1 << 31));
+        Self(value + (1 << 31))
     }
 
-    fn value(&self) -> u32 {
+    /// Returns the index as a 32-bit integer, including the hardened bit.
-        match *self {
+    pub fn index(&self) -> u32 {
-            ChildIndex::Hardened(i) => i + (1 << 31),
+        self.0
-            ChildIndex::NonHardened(i) => i,
-        }
     }
 }
 
-/// A BIP-32 chain code
+/// A value that is needed, in addition to a spending key, in order to derive descendant
+/// keys and addresses of that key.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct ChainCode([u8; 32]);
 
 impl ChainCode {
-    /// Returns byte representation of the chain code, as required for
+    /// Constructs a `ChainCode` from the given array.
+    pub fn new(c: [u8; 32]) -> Self {
+        Self(c)
+    }
+
+    /// Returns the byte representation of the chain code, as required for
     /// [ZIP 32](https://zips.z.cash/zip-0032) encoding.
-    fn as_bytes(&self) -> &[u8; 32] {
+    pub fn as_bytes(&self) -> &[u8; 32] {
         &self.0
     }
 }