From c4da498cf4baf5a6a174eeae19d4343dc77be593 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 26 Aug 2022 15:30:41 -0600 Subject: [PATCH] Shield funds to the internal Sapling key for a specified account. This updates `shield_transparent_funds` to look up the internal (change) address for the account specified, and use that as the destination for shielding transparent funds. Fixed #614 --- zcash_client_backend/src/data_api/error.rs | 13 ++++++ zcash_client_backend/src/data_api/wallet.rs | 52 ++++++++++++--------- zcash_primitives/CHANGELOG.md | 3 ++ zcash_primitives/src/sapling/keys.rs | 7 +++ 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 132e3bb02..c48568267 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -2,6 +2,7 @@ use std::error; use std::fmt; +use zcash_address::unified::Typecode; use zcash_primitives::{ consensus::BlockHeight, sapling::Node, @@ -34,6 +35,12 @@ pub enum Error { /// Chain validation detected an error in the block at the specified block height. InvalidChain(BlockHeight, ChainInvalid), + /// A provided extsk is not associated with the specified account. + AccountNotFound(AccountId), + + /// No key of the given type was associated with the specified account. + KeyNotFound(AccountId, Typecode), + /// A provided extsk is not associated with the specified account. InvalidExtSk(AccountId), @@ -91,6 +98,12 @@ impl fmt::Display for Error { Error::InvalidChain(upper_bound, cause) => { write!(f, "Invalid chain (upper bound: {}): {:?}", u32::from(*upper_bound), cause) } + Error::AccountNotFound(account) => { + write!(f, "Unrecognized account {}", u32::from(*account)) + } + Error::KeyNotFound(account, typecode) => { + write!(f, "No {:?} key was available for account {}", typecode, u32::from(*account)) + } Error::InvalidExtSk(account) => { write!(f, "Incorrect ExtendedSpendingKey for account {}", u32::from(*account)) } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 13a4f1b96..9d62ffb1a 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -12,8 +12,11 @@ use zcash_primitives::{ }; #[cfg(feature = "transparent-inputs")] -use zcash_primitives::{ - keys::OutgoingViewingKey, legacy::keys as transparent, legacy::keys::IncomingViewingKey, +use { + zcash_address::unified::Typecode, + zcash_primitives::{ + keys::OutgoingViewingKey, legacy::keys as transparent, legacy::keys::IncomingViewingKey, + }, }; use crate::{ @@ -418,11 +421,10 @@ where /// * `prover`: The TxProver to use in constructing the shielded transaction. /// * `sk`: The secp256k1 secret key that will be used to detect and spend transparent /// UTXOs. -/// * `extfvk`: The extended full viewing key that will be used to produce the -/// Sapling address to which funds will be sent. -/// * `account`: The ZIP32 account identifier associated with the the extended -/// full viewing key. This procedure will return an error if this does not correctly -/// correspond to `extfvk`. +/// * `account`: The ZIP32 account identifier for the account to which funds will +/// be shielded. Funds will be shielded to the internal (change) address associated with the +/// Sapling key corresponding to this account, or if a Sapling key is not available for this +/// account this function will return an error. /// * `memo`: A memo to be included in the output to the (internal) recipient. /// This can be used to take notes about auto-shielding operations internal /// to the wallet that the wallet can use to improve how it represents those @@ -437,7 +439,6 @@ pub fn shield_transparent_funds( params: &P, prover: impl TxProver, sk: &transparent::AccountPrivKey, - extfvk: &ExtendedFullViewingKey, account: AccountId, memo: &MemoBytes, min_confirmations: u32, @@ -448,11 +449,20 @@ where R: Copy + Debug, D: WalletWrite + WalletWriteTransparent, { - // Check that the ExtendedSpendingKey we have been given corresponds to the - // ExtendedFullViewingKey for the account we are spending from. - if !wallet_db.is_valid_account_extfvk(account, extfvk)? { - return Err(E::from(Error::InvalidExtSk(account))); - } + // Obtain the UFVK for the specified account & use its internal change address + // as the destination for shielded funds. + let shielding_address = wallet_db + .get_unified_full_viewing_keys() + .and_then(|ufvks| { + ufvks + .get(&account) + .ok_or_else(|| E::from(Error::AccountNotFound(account))) + .and_then(|ufvk| { + ufvk.sapling() + .map(|dfvk| dfvk.change_address().1) + .ok_or_else(|| E::from(Error::KeyNotFound(account, Typecode::Sapling))) + }) + })?; let (latest_scanned_height, latest_anchor) = wallet_db .get_target_and_anchor_heights(min_confirmations) @@ -461,11 +471,6 @@ where let account_pubkey = sk.to_account_pubkey(); let ovk = OutgoingViewingKey(account_pubkey.internal_ovk().as_bytes()); - // derive own shielded address from the provided extended full viewing key - // TODO: this should become the internal change address derived from - // the wallet's UFVK - let z_address = extfvk.default_address().1; - // derive the t-address for the extpubkey at the minimum valid child index let (taddr, child_index) = account_pubkey .derive_external_ivk() @@ -497,11 +502,16 @@ where } // there are no sapling notes so we set the change manually - builder.send_change_to(ovk, z_address.clone()); + builder.send_change_to(ovk, shielding_address.clone()); // add the sapling output to shield the funds builder - .add_sapling_output(Some(ovk), z_address.clone(), amount_to_shield, memo.clone()) + .add_sapling_output( + Some(ovk), + shielding_address.clone(), + amount_to_shield, + memo.clone(), + ) .map_err(Error::Builder)?; let (tx, tx_metadata) = builder.build(&prover).map_err(Error::Builder)?; @@ -515,7 +525,7 @@ where account, outputs: vec![SentTransactionOutput { output_index, - recipient_address: &RecipientAddress::Shielded(z_address), + recipient_address: &RecipientAddress::Shielded(shielding_address), value: amount_to_shield, memo: Some(memo.clone()), }], diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 9c28a7a16..cbc3ef70f 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -108,6 +108,9 @@ and this library adheres to Rust's notion of refactored to become a member function of a new `DiversifiableFullViewingKey` type, which represents the ability to derive IVKs, OVKs, and addresses, but not child viewing keys. +- `zcash_primitives::sapling::keys::DiversifiableFullViewingKey::change_address` + has been added as a convenience method for obtaining the change address + at the default diversifier. - A new module `zcash_primitives::legacy::keys` has been added under the `transparent-inputs` feature flag to support types related to supporting transparent components of unified addresses and derivation of OVKs for diff --git a/zcash_primitives/src/sapling/keys.rs b/zcash_primitives/src/sapling/keys.rs index cd2fc592f..93df13d13 100644 --- a/zcash_primitives/src/sapling/keys.rs +++ b/zcash_primitives/src/sapling/keys.rs @@ -292,6 +292,13 @@ impl DiversifiableFullViewingKey { zip32::sapling_default_address(&self.fvk, &self.dk) } + /// Returns the internal address corresponding to the smallest valid diversifier index, + /// along with that index. + pub fn change_address(&self) -> (zip32::DiversifierIndex, PaymentAddress) { + let internal_dfvk = self.derive_internal(); + zip32::sapling_default_address(&internal_dfvk.fvk, &internal_dfvk.dk) + } + /// Attempts to decrypt the given address's diversifier with this full viewing key. /// /// This method extracts the diversifier from the given address and decrypts it as a