diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index d536d7c23..d95fe1b95 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -156,8 +156,7 @@ and this library adheres to Rust's notion of - It returns a `NonEmpty` instead of a single `TxId` value. - `wallet::create_spend_to_address` now takes additional `change_memo` and `fallback_change_pool` arguments. It also returns its result as a - `NonEmpty` instead of a - single `TxId`. + `NonEmpty` instead of a single `TxId`. - `wallet::spend` returns its result as a `NonEmpty` instead of a single `TxId`. - The error type of `wallet::create_spend_to_address` has been changed to use @@ -237,9 +236,10 @@ and this library adheres to Rust's notion of are provided for each previously public field. - `Recipient` is now polymorphic in the type of the payload for wallet-internal recipients. This simplifies the handling of wallet-internal outputs. - - `SentTransactionOutput::from_parts` now takes a `Recipient` - - `SentTransactionOutput::recipient` now returns a `Recipient` - - `OvkPolicy::Custom` now wraps a bare `[u8; 32]` instead of a Sapling `OutgoingViewingKey`. + - `SentTransactionOutput::from_parts` now takes a `Recipient`. + - `SentTransactionOutput::recipient` now returns a `Recipient`. + - `OvkPolicy::Custom` is now a structured variant that can contain independent + Sapling and Orchard `OutgoingViewingKey`s. - `zcash_client_backend::scanning::ScanError` has a new variant, `TreeSizeInvalid`. - `zcash_client_backend::zip321::TransactionRequest::payments` now returns a `BTreeMap` instead of `&[Payment]` so that parameter diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index c18a9e11f..16e0f757f 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -8,7 +8,6 @@ use std::{ }; use incrementalmerkletree::{frontier::Frontier, Retention}; -use sapling; use secrecy::SecretVec; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use zcash_primitives::{ @@ -815,7 +814,9 @@ pub struct SentTransaction<'a> { pub utxos_spent: Vec, } -/// A type that represents an output (either Sapling or transparent) that was sent by the wallet. +/// An output of a transaction generated by the wallet. +/// +/// This type is capable of representing both shielded and transparent outputs. pub struct SentTransactionOutput { output_index: usize, recipient: Recipient, diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 928d87a35..abafa6bed 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -67,7 +67,11 @@ pub enum Error { /// It is forbidden to provide a memo when constructing a transparent output. MemoForbidden, - /// Attempted to create a send change to an unsupported pool. + /// Attempted to send change to an unsupported pool. + /// + /// This is indicative of a programming error; execution of a transaction proposal that + /// presumes support for the specified pool was performed using an application that does not + /// provide such support. UnsupportedChangeType(PoolType), /// Attempted to create a spend to an unsupported Unified Address receiver @@ -142,7 +146,11 @@ where Error::Builder(e) => write!(f, "An error occurred building the transaction: {}", e), Error::MemoForbidden => write!(f, "It is not possible to send a memo to a transparent address."), Error::UnsupportedChangeType(t) => write!(f, "Attempted to send change to an unsupported pool type: {}", t), - Error::NoSupportedReceivers(_) => write!(f, "A recipient's unified address does not contain any receivers to which the wallet can send funds."), + Error::NoSupportedReceivers(ua) => write!( + f, + "A recipient's unified address does not contain any receivers to which the wallet can send funds; required {}", + ua.receiver_types().iter().enumerate().map(|(i, tc)| format!("{}{:?}", if i > 0 { ", " } else { "" }, tc)).collect::() + ), 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), diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 357cc8584..18f36b91f 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -846,9 +846,9 @@ where let orchard_fvk: orchard::keys::FullViewingKey = usk.orchard().into(); #[cfg(feature = "orchard")] - let orchard_external_ovk = match ovk_policy { + let orchard_external_ovk = match &ovk_policy { OvkPolicy::Sender => Some(orchard_fvk.to_ovk(orchard::keys::Scope::External)), - OvkPolicy::Custom(ovk) => Some(orchard::keys::OutgoingViewingKey::from(ovk)), + OvkPolicy::Custom { orchard, .. } => Some(orchard.clone()), OvkPolicy::Discard => None, }; @@ -870,9 +870,9 @@ where let sapling_dfvk = usk.sapling().to_diversifiable_full_viewing_key(); // Apply the outgoing viewing key policy. - let sapling_external_ovk = match ovk_policy { + let sapling_external_ovk = match &ovk_policy { OvkPolicy::Sender => Some(sapling_dfvk.to_ovk(Scope::External)), - OvkPolicy::Custom(ovk) => Some(sapling::keys::OutgoingViewingKey(ovk)), + OvkPolicy::Custom { sapling, .. } => Some(*sapling), OvkPolicy::Discard => None, }; @@ -1040,7 +1040,7 @@ where .expect("An action should exist in the transaction for each Orchard output."); let recipient = recipient - .map(|pool| { + .map_internal_account(|pool| { assert!(pool == PoolType::Shielded(ShieldedProtocol::Orchard)); build_result .transaction() @@ -1051,7 +1051,7 @@ where .map(|(note, _, _)| Note::Orchard(note)) }) }) - .transpose() + .internal_account_transpose_option() .expect("Wallet-internal outputs must be decryptable with the wallet's IVK"); SentTransactionOutput::from_parts(output_index, recipient, value, memo) @@ -1070,7 +1070,7 @@ where .expect("An output should exist in the transaction for each Sapling payment."); let recipient = recipient - .map(|pool| { + .map_internal_account(|pool| { assert!(pool == PoolType::Shielded(ShieldedProtocol::Sapling)); build_result .transaction() @@ -1087,7 +1087,7 @@ where .map(|(note, _, _)| Note::Sapling(note)) }) }) - .transpose() + .internal_account_transpose_option() .expect("Wallet-internal outputs must be decryptable with the wallet's IVK"); SentTransactionOutput::from_parts(output_index, recipient, value, memo) diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 30def87ee..1e7ca75a2 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -73,7 +73,7 @@ pub enum Recipient { } impl Recipient { - pub fn map B>(self, f: F) -> Recipient { + pub fn map_internal_account B>(self, f: F) -> Recipient { match self { Recipient::Transparent(t) => Recipient::Transparent(t), Recipient::Sapling(s) => Recipient::Sapling(s), @@ -84,7 +84,7 @@ impl Recipient { } impl Recipient> { - pub fn transpose(self) -> Option> { + pub fn internal_account_transpose_option(self) -> Option> { match self { Recipient::Transparent(t) => Some(Recipient::Transparent(t)), Recipient::Sapling(s) => Some(Recipient::Sapling(s)), @@ -415,13 +415,30 @@ pub enum OvkPolicy { /// /// Transaction outputs will be decryptable by the recipients, and whoever controls /// the provided outgoing viewing key. - Custom([u8; 32]), - + Custom { + sapling: sapling::keys::OutgoingViewingKey, + #[cfg(feature = "orchard")] + orchard: orchard::keys::OutgoingViewingKey, + }, /// Use no outgoing viewing key. Transaction outputs will be decryptable by their /// recipients, but not by the sender. Discard, } +impl OvkPolicy { + /// Construct an [`OvkPolicy::Custom`] value from an arbitrary 32-byte key. + /// + /// Transactions constructed under this OVK policy will have their outputs + /// recoverable using this key irrespective of the output pool. + pub fn custom_from_common_bytes(key: &[u8; 32]) -> Self { + OvkPolicy::Custom { + sapling: sapling::keys::OutgoingViewingKey(*key), + #[cfg(feature = "orchard")] + orchard: orchard::keys::OutgoingViewingKey::from(*key), + } + } +} + /// Metadata related to the ZIP 32 derivation of a transparent address. #[derive(Debug, Clone, PartialEq, Eq)] #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 84330a4de..43e1c8f54 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -445,6 +445,7 @@ impl TestState { ovk_policy: OvkPolicy, min_confirmations: NonZeroU32, change_memo: Option, + fallback_change_pool: ShieldedProtocol, ) -> Result< NonEmpty, data_api::error::Error< @@ -468,7 +469,7 @@ impl TestState { ovk_policy, min_confirmations, change_memo, - ShieldedProtocol::Sapling, + fallback_change_pool, ) } @@ -551,6 +552,7 @@ impl TestState { amount: NonNegativeAmount, memo: Option, change_memo: Option, + fallback_change_pool: ShieldedProtocol, ) -> Result< Proposal, data_api::error::Error< @@ -571,7 +573,7 @@ impl TestState { amount, memo, change_memo, - ShieldedProtocol::Sapling, + fallback_change_pool, ); if let Ok(proposal) = &result { @@ -1058,13 +1060,14 @@ impl TestCache for FsBlockCache { pub(crate) fn input_selector( fee_rule: StandardFeeRule, change_memo: Option<&str>, + fallback_change_pool: ShieldedProtocol, ) -> GreedyInputSelector< WalletDb, standard::SingleOutputChangeStrategy, > { let change_memo = change_memo.map(|m| MemoBytes::from(m.parse::().unwrap())); let change_strategy = - standard::SingleOutputChangeStrategy::new(fee_rule, change_memo, ShieldedProtocol::Sapling); + standard::SingleOutputChangeStrategy::new(fee_rule, change_memo, fallback_change_pool); GreedyInputSelector::new(change_strategy, DustOutputPolicy::default()) } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index c6cf86a15..7061e62e8 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -859,7 +859,8 @@ pub(crate) mod tests { None, OvkPolicy::Sender, NonZeroU32::new(1).unwrap(), - None + None, + ShieldedProtocol::Sapling ), Err(data_api::error::Error::KeyNotRecognized) ); @@ -888,7 +889,8 @@ pub(crate) mod tests { &to, NonNegativeAmount::const_from_u64(1), None, - None + None, + ShieldedProtocol::Sapling ), Err(data_api::error::Error::ScanRequired) ); @@ -955,7 +957,8 @@ pub(crate) mod tests { &to, NonNegativeAmount::const_from_u64(70000), None, - None + None, + ShieldedProtocol::Sapling ), Err(data_api::error::Error::InsufficientFunds { available, @@ -984,7 +987,8 @@ pub(crate) mod tests { &to, NonNegativeAmount::const_from_u64(70000), None, - None + None, + ShieldedProtocol::Sapling ), Err(data_api::error::Error::InsufficientFunds { available, @@ -1019,6 +1023,7 @@ pub(crate) mod tests { amount_sent, None, None, + ShieldedProtocol::Sapling, ) .unwrap(); @@ -1076,6 +1081,7 @@ pub(crate) mod tests { NonNegativeAmount::const_from_u64(15000), None, None, + ShieldedProtocol::Sapling, ) .unwrap(); @@ -1094,7 +1100,8 @@ pub(crate) mod tests { &to, NonNegativeAmount::const_from_u64(2000), None, - None + None, + ShieldedProtocol::Sapling ), Err(data_api::error::Error::InsufficientFunds { available, @@ -1123,7 +1130,8 @@ pub(crate) mod tests { &to, NonNegativeAmount::const_from_u64(2000), None, - None + None, + ShieldedProtocol::Sapling ), Err(data_api::error::Error::InsufficientFunds { available, @@ -1156,6 +1164,7 @@ pub(crate) mod tests { amount_sent2, None, None, + ShieldedProtocol::Sapling, ) .unwrap(); @@ -1223,6 +1232,7 @@ pub(crate) mod tests { NonNegativeAmount::const_from_u64(15000), None, None, + ShieldedProtocol::Sapling, )?; // Executing the proposal should succeed @@ -1325,6 +1335,7 @@ pub(crate) mod tests { NonNegativeAmount::const_from_u64(50000), None, None, + ShieldedProtocol::Sapling, ) .unwrap(); @@ -1387,6 +1398,7 @@ pub(crate) mod tests { NonNegativeAmount::const_from_u64(50000), None, None, + ShieldedProtocol::Sapling, ) .unwrap(); @@ -1555,7 +1567,8 @@ pub(crate) mod tests { assert_eq!(st.get_total_balance(account), total); assert_eq!(st.get_spendable_balance(account, 1), total); - let input_selector = input_selector(StandardFeeRule::Zip317, None); + let input_selector = + input_selector(StandardFeeRule::Zip317, None, ShieldedProtocol::Sapling); // This first request will fail due to insufficient non-dust funds let req = TransactionRequest::new(vec![Payment { @@ -1828,7 +1841,8 @@ pub(crate) mod tests { None, OvkPolicy::Sender, NonZeroU32::new(5).unwrap(), - None + None, + ShieldedProtocol::Sapling ), Ok(_) ); diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 56fab2845..fd33276e0 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -14,7 +14,8 @@ The entries below are relative to the `zcash_client_backend` crate as of - `address` - `encoding` - `keys` -- `zcash_keys::address::UnifiedAddress::{unknown, has_orchard, has_sapling, has_transparent}`: +- `zcash_keys::address::UnifiedAddress::{unknown, has_orchard, has_sapling, + has_transparent, receiver_types}`: - `zcash_keys::keys`: - `AddressGenerationError` - `UnifiedAddressRequest` diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index 7ba589f82..cac82d705 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -1,10 +1,8 @@ //! Structs for handling supported address types. -use std::convert::TryFrom; - use sapling::PaymentAddress; use zcash_address::{ - unified::{self, Container, Encoding}, + unified::{self, Container, Encoding, Typecode}, ConversionError, Network, ToAddress, TryFromRawAddress, ZcashAddress, }; use zcash_primitives::{consensus, legacy::TransparentAddress}; @@ -188,6 +186,21 @@ impl UnifiedAddress { self.to_address(params.address_network().expect("Unrecognized network")) .to_string() } + + /// Returns the set of receiver typecodes. + pub fn receiver_types(&self) -> Vec { + let result = std::iter::empty(); + #[cfg(feature = "orchard")] + let result = result.chain(self.orchard.map(|_| Typecode::Orchard)); + let result = result.chain(self.sapling.map(|_| Typecode::Sapling)); + let result = result.chain(self.transparent.map(|_| Typecode::P2pkh)); + let result = result.chain( + self.unknown() + .iter() + .map(|(typecode, _)| Typecode::Unknown(*typecode)), + ); + result.collect() + } } /// An address that funds can be sent to.