Address comments from code review.
This commit is contained in:
parent
8c78d7f2a0
commit
050a124cb6
|
@ -156,8 +156,7 @@ and this library adheres to Rust's notion of
|
|||
- It returns a `NonEmpty<TxId>` 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<TxId>` instead of a
|
||||
single `TxId`.
|
||||
`NonEmpty<TxId>` instead of a single `TxId`.
|
||||
- `wallet::spend` returns its result as a `NonEmpty<TxId>` 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<Note>`
|
||||
- `SentTransactionOutput::recipient` now returns a `Recipient<Note>`
|
||||
- `OvkPolicy::Custom` now wraps a bare `[u8; 32]` instead of a Sapling `OutgoingViewingKey`.
|
||||
- `SentTransactionOutput::from_parts` now takes a `Recipient<Note>`.
|
||||
- `SentTransactionOutput::recipient` now returns a `Recipient<Note>`.
|
||||
- `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<usize, Payment>` instead of `&[Payment]` so that parameter
|
||||
|
|
|
@ -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<OutPoint>,
|
||||
}
|
||||
|
||||
/// 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<Note>,
|
||||
|
|
|
@ -67,7 +67,11 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
|
|||
/// 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::<String>()
|
||||
),
|
||||
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),
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -73,7 +73,7 @@ pub enum Recipient<N> {
|
|||
}
|
||||
|
||||
impl<N> Recipient<N> {
|
||||
pub fn map<B, F: FnOnce(N) -> B>(self, f: F) -> Recipient<B> {
|
||||
pub fn map_internal_account<B, F: FnOnce(N) -> B>(self, f: F) -> Recipient<B> {
|
||||
match self {
|
||||
Recipient::Transparent(t) => Recipient::Transparent(t),
|
||||
Recipient::Sapling(s) => Recipient::Sapling(s),
|
||||
|
@ -84,7 +84,7 @@ impl<N> Recipient<N> {
|
|||
}
|
||||
|
||||
impl<N> Recipient<Option<N>> {
|
||||
pub fn transpose(self) -> Option<Recipient<N>> {
|
||||
pub fn internal_account_transpose_option(self) -> Option<Recipient<N>> {
|
||||
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")]
|
||||
|
|
|
@ -445,6 +445,7 @@ impl<Cache> TestState<Cache> {
|
|||
ovk_policy: OvkPolicy,
|
||||
min_confirmations: NonZeroU32,
|
||||
change_memo: Option<MemoBytes>,
|
||||
fallback_change_pool: ShieldedProtocol,
|
||||
) -> Result<
|
||||
NonEmpty<TxId>,
|
||||
data_api::error::Error<
|
||||
|
@ -468,7 +469,7 @@ impl<Cache> TestState<Cache> {
|
|||
ovk_policy,
|
||||
min_confirmations,
|
||||
change_memo,
|
||||
ShieldedProtocol::Sapling,
|
||||
fallback_change_pool,
|
||||
)
|
||||
}
|
||||
|
||||
|
@ -551,6 +552,7 @@ impl<Cache> TestState<Cache> {
|
|||
amount: NonNegativeAmount,
|
||||
memo: Option<MemoBytes>,
|
||||
change_memo: Option<MemoBytes>,
|
||||
fallback_change_pool: ShieldedProtocol,
|
||||
) -> Result<
|
||||
Proposal<StandardFeeRule, ReceivedNoteId>,
|
||||
data_api::error::Error<
|
||||
|
@ -571,7 +573,7 @@ impl<Cache> TestState<Cache> {
|
|||
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<rusqlite::Connection, Network>,
|
||||
standard::SingleOutputChangeStrategy,
|
||||
> {
|
||||
let change_memo = change_memo.map(|m| MemoBytes::from(m.parse::<Memo>().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())
|
||||
}
|
||||
|
||||
|
|
|
@ -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(_)
|
||||
);
|
||||
|
|
|
@ -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`
|
||||
|
|
|
@ -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<Typecode> {
|
||||
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.
|
||||
|
|
Loading…
Reference in New Issue