Merge pull request #1185 from nerdcash/nonhardenedchildindex

Declare and use NonHardenedChildIndex for transparent

zcash_client_backend/CHANGELOG.md

@@ -52,6 +52,7 @@ and this library adheres to Rust's notion of
   - `wallet::input_selection::ShieldingSelector` has been
     factored out from the `InputSelector` trait to separate out transparent
     functionality and move it behind the `transparent-inputs` feature flag.
+  - `TransparentAddressMetadata` (which replaces `zcash_keys::address::AddressMetadata`).
 - `zcash_client_backend::fees::{standard, sapling}`
 - `zcash_client_backend::fees::ChangeValue::new`
 - `zcash_client_backend::wallet`:
@@ -114,6 +115,8 @@ and this library adheres to Rust's notion of
       been removed from `error::Error`.
     - A new variant `UnsupportedPoolType` has been added.
     - A new variant `NoSupportedReceivers` has been added.
+    - A new variant `NoSpendingKey` has been added.
+    - Variant `ChildIndexOutOfRange` has been removed.
   - `wallet::shield_transparent_funds` no longer takes a `memo` argument;
     instead, memos to be associated with the shielded outputs should be
     specified in the construction of the value of the `input_selector`
@@ -161,6 +164,9 @@ and this library adheres to Rust's notion of
       `get_unspent_transparent_outputs` have been removed; use
       `data_api::InputSource` instead.
     - Added `get_account_ids`.
+    - `get_transparent_receivers` now returns
+      `zcash_client_backend::data_api::TransparentAddressMetadata` instead of
+      `zcash_keys::address::AddressMetadata`.
   - `wallet::{propose_shielding, shield_transparent_funds}` now takes their
     `min_confirmations` arguments as `u32` rather than a `NonZeroU32` to permit
     implmentations to enable zero-conf shielding.

zcash_client_backend/src/data_api.rs

@@ -14,7 +14,7 @@ use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
 use zcash_primitives::{
     block::BlockHash,
     consensus::BlockHeight,
-    legacy::TransparentAddress,
+    legacy::{NonHardenedChildIndex, TransparentAddress},
     memo::{Memo, MemoBytes},
     transaction::{
         components::amount::{Amount, BalanceError, NonNegativeAmount},
@@ -24,7 +24,7 @@ use zcash_primitives::{
 };
 
 use crate::{
-    address::{AddressMetadata, UnifiedAddress},
+    address::UnifiedAddress,
     decrypt::DecryptedOutput,
     keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::service::TreeState,
@@ -56,6 +56,30 @@ pub enum NullifierQuery {
     All,
 }
 
+/// Describes the derivation of a transparent address.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct TransparentAddressMetadata {
+    scope: zip32::Scope,
+    address_index: NonHardenedChildIndex,
+}
+
+impl TransparentAddressMetadata {
+    pub fn new(scope: zip32::Scope, address_index: NonHardenedChildIndex) -> Self {
+        Self {
+            scope,
+            address_index,
+        }
+    }
+
+    pub fn scope(&self) -> zip32::Scope {
+        self.scope
+    }
+
+    pub fn address_index(&self) -> NonHardenedChildIndex {
+        self.address_index
+    }
+}
+
 /// Balance information for a value within a single pool in an account.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct Balance {
@@ -560,7 +584,7 @@ pub trait WalletRead {
     fn get_transparent_receivers(
         &self,
         account: AccountId,
-    ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error>;
+    ) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error>;
 
     /// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance,
     /// for each address associated with a nonzero balance.
@@ -1116,7 +1140,7 @@ pub mod testing {
     };
 
     use crate::{
-        address::{AddressMetadata, UnifiedAddress},
+        address::UnifiedAddress,
         keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
         wallet::{Note, NoteId, ReceivedNote, WalletTransparentOutput},
         ShieldedProtocol,
@@ -1125,7 +1149,8 @@ pub mod testing {
     use super::{
         chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata,
         DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SentTransaction,
-        WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
+        TransparentAddressMetadata, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite,
+        SAPLING_SHARD_HEIGHT,
     };
 
     pub struct MockWalletDb {
@@ -1284,7 +1309,8 @@ pub mod testing {
         fn get_transparent_receivers(
             &self,
             _account: AccountId,
-        ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
+        ) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error>
+        {
             Ok(HashMap::new())
         }
 

zcash_client_backend/src/data_api/error.rs

@@ -17,7 +17,7 @@ use crate::data_api::wallet::input_selection::InputSelectorError;
 use crate::PoolType;
 
 #[cfg(feature = "transparent-inputs")]
-use zcash_primitives::{legacy::TransparentAddress, zip32::DiversifierIndex};
+use zcash_primitives::legacy::TransparentAddress;
 
 use crate::wallet::NoteId;
 
@@ -64,15 +64,18 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
     /// Attempted to create a spend to an unsupported Unified Address receiver
     NoSupportedReceivers(Vec<u32>),
 
+    /// A proposed transaction cannot be built because it requires spending an input
+    /// for which no spending key is available.
+    ///
+    /// The argument is the address of the note or UTXO being spent.
+    NoSpendingKey(String),
+
     /// A note being spent does not correspond to either the internal or external
     /// full viewing key for an account.
     NoteMismatch(NoteId),
 
     #[cfg(feature = "transparent-inputs")]
     AddressNotRecognized(TransparentAddress),
-
-    #[cfg(feature = "transparent-inputs")]
-    ChildIndexOutOfRange(DiversifierIndex),
 }
 
 impl<DE, CE, SE, FE> fmt::Display for Error<DE, CE, SE, FE>
@@ -122,20 +125,13 @@ where
             Error::MemoForbidden => write!(f, "It is not possible to send a memo to a transparent address."),
             Error::UnsupportedPoolType(t) => write!(f, "Attempted to send to an unsupported pool: {}", t),
             Error::NoSupportedReceivers(t) => write!(f, "Unified address contained only unsupported receiver types: {:?}", &t[..]),
+            Error::NoSpendingKey(addr) => write!(f, "No spending key available for address: {}", addr),
             Error::NoteMismatch(n) => write!(f, "A note being spent ({:?}) does not correspond to either the internal or external full viewing key for the provided spending key.", n),
 
             #[cfg(feature = "transparent-inputs")]
             Error::AddressNotRecognized(_) => {
                 write!(f, "The specified transparent address was not recognized as belonging to the wallet.")
             }
-            #[cfg(feature = "transparent-inputs")]
-            Error::ChildIndexOutOfRange(i) => {
-                write!(
-                    f,
-                    "The diversifier index {:?} is out of range for transparent addresses.",
-                    i
-                )
-            }
         }
     }
 }

zcash_client_backend/src/data_api/wallet.rs

@@ -5,6 +5,7 @@ use sapling::{
     note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey},
     prover::{OutputProver, SpendProver},
 };
+use zcash_keys::encoding::AddressCodec;
 use zcash_primitives::{
     consensus::{self, NetworkUpgrade},
     memo::MemoBytes,
@@ -639,17 +640,17 @@ where
         for utxo in proposal.transparent_inputs() {
             utxos.push(utxo.clone());
 
-            let diversifier_index = known_addrs
+            let address_metadata = known_addrs
                 .get(utxo.recipient_address())
                 .ok_or_else(|| Error::AddressNotRecognized(*utxo.recipient_address()))?
-                .diversifier_index();
-
-            let child_index = u32::try_from(*diversifier_index)
-                .map_err(|_| Error::ChildIndexOutOfRange(*diversifier_index))?;
+                .clone()
+                .ok_or_else(|| {
+                    Error::NoSpendingKey(utxo.recipient_address().encode(params))
+                })?;
 
             let secret_key = usk
                 .transparent()
-                .derive_external_secret_key(child_index)
+                .derive_external_secret_key(address_metadata.address_index())
                 .unwrap();
 
             builder.add_transparent_input(

zcash_client_sqlite/src/lib.rs

@@ -58,14 +58,14 @@ use zcash_primitives::{
 };
 
 use zcash_client_backend::{
-    address::{AddressMetadata, UnifiedAddress},
+    address::UnifiedAddress,
     data_api::{
         self,
         chain::{BlockSource, CommitmentTreeRoot},
         scanning::{ScanPriority, ScanRange},
         AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery,
-        ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary,
-        WalletWrite, SAPLING_SHARD_HEIGHT,
+        ScannedBlock, SentTransaction, TransparentAddressMetadata, WalletCommitmentTrees,
+        WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
     },
     keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::compact_formats::CompactBlock,
@@ -346,7 +346,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
     fn get_transparent_receivers(
         &self,
         _account: AccountId,
-    ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
+    ) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error> {
         #[cfg(feature = "transparent-inputs")]
         return wallet::get_transparent_receivers(self.conn.borrow(), &self.params, _account);
 

zcash_client_sqlite/src/wallet.rs

@@ -112,9 +112,9 @@ use {
     crate::UtxoId,
     rusqlite::Row,
     std::collections::BTreeSet,
-    zcash_client_backend::{address::AddressMetadata, wallet::WalletTransparentOutput},
+    zcash_client_backend::{data_api::TransparentAddressMetadata, wallet::WalletTransparentOutput},
     zcash_primitives::{
-        legacy::{keys::IncomingViewingKey, Script, TransparentAddress},
+        legacy::{keys::IncomingViewingKey, NonHardenedChildIndex, Script, TransparentAddress},
         transaction::components::{OutPoint, TxOut},
     },
 };
@@ -349,8 +349,8 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
     conn: &rusqlite::Connection,
     params: &P,
     account: AccountId,
-) -> Result<HashMap<TransparentAddress, AddressMetadata>, SqliteClientError> {
-    let mut ret = HashMap::new();
+) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, SqliteClientError> {
+    let mut ret: HashMap<TransparentAddress, Option<TransparentAddressMetadata>> = HashMap::new();
 
     // Get all UAs derived
     let mut ua_query = conn
@@ -360,12 +360,12 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
     while let Some(row) = rows.next()? {
         let ua_str: String = row.get(0)?;
         let di_vec: Vec<u8> = row.get(1)?;
-        let mut di_be: [u8; 11] = di_vec.try_into().map_err(|_| {
+        let mut di: [u8; 11] = di_vec.try_into().map_err(|_| {
             SqliteClientError::CorruptedData(
                 "Diverisifier index is not an 11-byte value".to_owned(),
             )
         })?;
-        di_be.reverse();
+        di.reverse(); // BE -> LE conversion
 
         let ua = Address::decode(params, &ua_str)
             .ok_or_else(|| {
@@ -380,16 +380,34 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
             })?;
 
         if let Some(taddr) = ua.transparent() {
+            let index = NonHardenedChildIndex::from_index(
+                DiversifierIndex::from(di).try_into().map_err(|_| {
+                    SqliteClientError::CorruptedData(
+                        "Unable to get diversifier for transparent address.".to_string(),
+                    )
+                })?,
+            )
+            .ok_or_else(|| {
+                SqliteClientError::CorruptedData(
+                    "Unexpected hardened index for transparent address.".to_string(),
+                )
+            })?;
+
             ret.insert(
                 *taddr,
-                AddressMetadata::new(account, DiversifierIndex::from(di_be)),
+                Some(TransparentAddressMetadata::new(Scope::External, index)),
             );
         }
     }
 
-    if let Some((taddr, diversifier_index)) = get_legacy_transparent_address(params, conn, account)?
-    {
-        ret.insert(taddr, AddressMetadata::new(account, diversifier_index));
+    if let Some((taddr, child_index)) = get_legacy_transparent_address(params, conn, account)? {
+        ret.insert(
+            taddr,
+            Some(TransparentAddressMetadata::new(
+                Scope::External,
+                child_index,
+            )),
+        );
     }
 
     Ok(ret)
@@ -400,7 +418,7 @@ pub(crate) fn get_legacy_transparent_address<P: consensus::Parameters>(
     params: &P,
     conn: &rusqlite::Connection,
     account: AccountId,
-) -> Result<Option<(TransparentAddress, DiversifierIndex)>, SqliteClientError> {
+) -> Result<Option<(TransparentAddress, NonHardenedChildIndex)>, SqliteClientError> {
     // Get the UFVK for the account.
     let ufvk_str: Option<String> = conn
         .query_row(
@@ -418,10 +436,7 @@ pub(crate) fn get_legacy_transparent_address<P: consensus::Parameters>(
         ufvk.transparent()
             .map(|tfvk| {
                 tfvk.derive_external_ivk()
-                    .map(|tivk| {
-                        let (taddr, child_index) = tivk.default_address();
-                        (taddr, DiversifierIndex::from(child_index))
-                    })
+                    .map(|tivk| tivk.default_address())
                     .map_err(SqliteClientError::HdwalletError)
             })
             .transpose()

zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs

@@ -396,7 +396,9 @@ mod tests {
     #[cfg(feature = "transparent-inputs")]
     fn migrate_from_wm2() {
         use zcash_client_backend::keys::UnifiedAddressRequest;
-        use zcash_primitives::transaction::components::amount::NonNegativeAmount;
+        use zcash_primitives::{
+            legacy::NonHardenedChildIndex, transaction::components::amount::NonNegativeAmount,
+        };
 
         use crate::UA_TRANSPARENT;
 
@@ -450,7 +452,7 @@ mod tests {
             .and_then(|k| {
                 k.derive_external_ivk()
                     .ok()
-                    .map(|k| k.derive_address(0).unwrap())
+                    .map(|k| k.derive_address(NonHardenedChildIndex::ZERO).unwrap())
             })
             .map(|a| a.encode(&network));
 

zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs

@@ -292,13 +292,13 @@ mod tests {
     use zcash_primitives::{
         block::BlockHash,
         consensus::{BlockHeight, Network, NetworkUpgrade, Parameters},
-        legacy::keys::IncomingViewingKey,
+        legacy::{keys::IncomingViewingKey, NonHardenedChildIndex},
         memo::MemoBytes,
         transaction::{
             builder::{BuildConfig, BuildResult, Builder},
-            components::amount::NonNegativeAmount,
+            components::{amount::NonNegativeAmount, transparent},
+            fees::fixed,
         },
-        transaction::{components::transparent, fees::fixed},
         zip32::{AccountId, Scope},
     };
     use zcash_proofs::prover::LocalTxProver;
@@ -354,7 +354,9 @@ mod tests {
         );
         builder
             .add_transparent_input(
-                usk0.transparent().derive_external_secret_key(0).unwrap(),
+                usk0.transparent()
+                    .derive_external_secret_key(NonHardenedChildIndex::ZERO)
+                    .unwrap(),
                 transparent::OutPoint::new([1; 32], 0),
                 transparent::TxOut {
                     value: NonNegativeAmount::const_from_u64(EXTERNAL_VALUE + INTERNAL_VALUE),

zcash_keys/CHANGELOG.md

@@ -44,3 +44,7 @@ The entries below are relative to the `zcash_client_backend` crate as of
     `transparent-inputs` feature is enabled.
   - `UnifiedFullViewingKey::new` no longer takes an Orchard full viewing key
     argument unless the `orchard` feature is enabled.
+
+### Removed
+- `zcash_keys::address::AddressMetadata` has been moved to 
+  `zcash_client_backend::data_api::TransparentAddressMetadata` and fields changed.