diff --git a/components/zcash_address/src/kind/unified/address.rs b/components/zcash_address/src/kind/unified/address.rs index 00d3c5c540..8942b49720 100644 --- a/components/zcash_address/src/kind/unified/address.rs +++ b/components/zcash_address/src/kind/unified/address.rs @@ -1,4 +1,4 @@ -use zcash_protocol::{PoolType, ShieldedProtocol}; +use zcash_protocol::PoolType; use super::{private::SealedItem, ParseError, Typecode}; @@ -107,9 +107,9 @@ impl Address { /// Returns whether this address has the ability to receive transfers of the given pool type. pub fn has_receiver_of_type(&self, pool_type: PoolType) -> bool { self.0.iter().any(|r| match r { - Receiver::Orchard(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Orchard), - Receiver::Sapling(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Sapling), - Receiver::P2pkh(_) | Receiver::P2sh(_) => pool_type == PoolType::Transparent, + Receiver::Orchard(_) => pool_type == PoolType::ORCHARD, + Receiver::Sapling(_) => pool_type == PoolType::SAPLING, + Receiver::P2pkh(_) | Receiver::P2sh(_) => pool_type == PoolType::TRANSPARENT, Receiver::Unknown { .. } => false, }) } diff --git a/components/zcash_address/src/lib.rs b/components/zcash_address/src/lib.rs index e5db457290..32a3c05f4d 100644 --- a/components/zcash_address/src/lib.rs +++ b/components/zcash_address/src/lib.rs @@ -143,7 +143,7 @@ pub use encoding::ParseError; pub use kind::unified; use kind::unified::Receiver; pub use zcash_protocol::consensus::NetworkType as Network; -use zcash_protocol::{PoolType, ShieldedProtocol}; +use zcash_protocol::PoolType; /// A Zcash address. #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -274,9 +274,9 @@ impl ZcashAddress { use AddressKind::*; match &self.kind { Sprout(_) => false, - Sapling(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Sapling), + Sapling(_) => pool_type == PoolType::SAPLING, Unified(addr) => addr.has_receiver_of_type(pool_type), - P2pkh(_) | P2sh(_) | Tex(_) => pool_type == PoolType::Transparent, + P2pkh(_) | P2sh(_) | Tex(_) => pool_type == PoolType::TRANSPARENT, } } diff --git a/components/zcash_encoding/CHANGELOG.md b/components/zcash_encoding/CHANGELOG.md index da71b50f7c..9ee79287d5 100644 --- a/components/zcash_encoding/CHANGELOG.md +++ b/components/zcash_encoding/CHANGELOG.md @@ -6,6 +6,9 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- `zcash_encoding::CompactSize::serialized_size` +- `zcash_encoding::Vector::serialized_size_of_u8_vec` ## [0.2.0] - 2022-10-19 ### Changed diff --git a/components/zcash_encoding/src/lib.rs b/components/zcash_encoding/src/lib.rs index 8e23f7b68f..69f1c5c63c 100644 --- a/components/zcash_encoding/src/lib.rs +++ b/components/zcash_encoding/src/lib.rs @@ -93,6 +93,16 @@ impl CompactSize { } } } + + /// Returns the number of bytes needed to encode the given size in compact form. + pub fn serialized_size(size: usize) -> usize { + match size { + s if s < 253 => 1, + s if s <= 0xFFFF => 3, + s if s <= 0xFFFFFFFF => 5, + _ => 9, + } + } } /// Namespace for functions that perform encoding of vectors. @@ -171,6 +181,12 @@ impl Vector { CompactSize::write(&mut writer, items.len())?; items.try_for_each(|e| func(&mut writer, e)) } + + /// Returns the serialized size of a vector of `u8` as written by `[Vector::write]`. + pub fn serialized_size_of_u8_vec(vec: &[u8]) -> usize { + let length = vec.len(); + CompactSize::serialized_size(length) + length + } } /// Namespace for functions that perform encoding of array contents. @@ -279,8 +295,11 @@ mod tests { >::Error: Debug, { let mut data = vec![]; - CompactSize::write(&mut data, value.try_into().unwrap()).unwrap(); + let value_usize: usize = value.try_into().unwrap(); + CompactSize::write(&mut data, value_usize).unwrap(); assert_eq!(&data[..], expected); + let serialized_size = CompactSize::serialized_size(value_usize); + assert_eq!(serialized_size, expected.len()); let result: io::Result = CompactSize::read_t(&data[..]); match result { Ok(n) => assert_eq!(n, value), @@ -308,6 +327,8 @@ mod tests { let mut data = vec![]; CompactSize::write(&mut data, value).unwrap(); assert_eq!(&data[..], encoded); + let serialized_size = CompactSize::serialized_size(value); + assert_eq!(serialized_size, encoded.len()); assert!(CompactSize::read(encoded).is_err()); } } @@ -320,6 +341,8 @@ mod tests { let mut data = vec![]; Vector::write(&mut data, &$value, |w, e| w.write_u8(*e)).unwrap(); assert_eq!(&data[..], &$expected[..]); + let serialized_size = Vector::serialized_size_of_u8_vec(&$value); + assert_eq!(serialized_size, $expected.len()); match Vector::read(&data[..], |r| r.read_u8()) { Ok(v) => assert_eq!(v, $value), Err(e) => panic!("Unexpected error: {:?}", e), diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 9462f1694c..234434b5c6 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -10,6 +10,10 @@ and this library adheres to Rust's notion of ### Added - `zcash_client_backend::data_api`: - `chain::BlockCache` trait, behind the `sync` feature flag. +- `zcash_client_backend::fees`: + - `ChangeValue::{transparent, shielded}` + - `sapling::EmptyBundleView` + - `orchard::EmptyBundleView` - `zcash_client_backend::scanning`: - `testing` module - `zcash_client_backend::sync` module, behind the `sync` feature flag. @@ -26,6 +30,16 @@ and this library adheres to Rust's notion of - `zcash_client_backend::proto::proposal::Proposal::{from_standard_proposal, try_into_standard_proposal}` each no longer require a `consensus::Parameters` argument. +- `zcash_client_backend::data_api::fees` + - The return type of `ChangeValue::output_pool`, and the type of the + `output_pool` argument to `ChangeValue::new`, have changed from + `ShieldedProtocol` to `zcash_protocol::PoolType`. + - The return type of `ChangeValue::new` is now optional; it returns `None` + if a memo is given for the transparent pool. Use `ChangeValue::shielded` + to avoid this error case when creating a `ChangeValue` known to be for a + shielded pool. +- `zcash_client_backend::input_selection::GreedyInputSelectorError` has a + new variant `UnsupportedTexAddress`. - `zcash_client_backend::wallet::Recipient` variants have changed. Instead of wrapping protocol-address types, the `Recipient` type now wraps a `zcash_address::ZcashAddress`. This simplifies the process of tracking the @@ -42,7 +56,7 @@ and this library adheres to Rust's notion of ### Added - A new `orchard` feature flag has been added to make it possible to - build client code without `orchard` dependendencies. Additions and + build client code without `orchard` dependencies. Additions and changes related to `Orchard` below are introduced under this feature flag. - `zcash_client_backend::data_api`: @@ -232,7 +246,7 @@ and this library adheres to Rust's notion of ### Changed - Migrated to `zcash_primitives 0.14`, `orchard 0.7`. - Several structs and functions now take an `AccountId` type parameter - parameter in order to decouple the concept of an account identifier from + in order to decouple the concept of an account identifier from the ZIP 32 account index. Many APIs that previously referenced `zcash_primitives::zip32::AccountId` now reference the generic type. Impacted types and functions are: @@ -518,7 +532,7 @@ and this library adheres to Rust's notion of - `chain::CommitmentTreeRoot` - `scanning` A new module containing types required for `suggest_scan_ranges` - `testing::MockWalletDb::new` - - `wallet::input_sellection::Proposal::{min_target_height, min_anchor_height}` + - `wallet::input_selection::Proposal::{min_target_height, min_anchor_height}` - `SAPLING_SHARD_HEIGHT` constant - `zcash_client_backend::proto::compact_formats`: - `impl From<&sapling::SpendDescription> for CompactSaplingSpend` @@ -616,7 +630,7 @@ and this library adheres to Rust's notion of - `wallet::input_selection`: - `Proposal::target_height` (use `Proposal::min_target_height` instead). - `zcash_client_backend::data_api::chain::validate_chain` (logic merged into - `chain::scan_cached_blocks`. + `chain::scan_cached_blocks`). - `zcash_client_backend::data_api::chain::error::{ChainError, Cause}` have been replaced by `zcash_client_backend::scanning::ScanError` - `zcash_client_backend::proto::compact_formats`: @@ -771,8 +785,8 @@ and this library adheres to Rust's notion of - `WalletWrite::get_next_available_address` - `WalletWrite::put_received_transparent_utxo` - `impl From for error::Error` - - `chain::error`: a module containing error types type that that can occur only - in chain validation and sync have been separated out from errors related to + - `chain::error`: a module containing error types that can occur only + in chain validation and sync, separated out from errors related to other wallet operations. - `input_selection`: a module containing types related to the process of selecting inputs to be spent, given a transaction request. @@ -805,7 +819,7 @@ and this library adheres to Rust's notion of likely to be modified and/or moved to a different module in a future release: - `zcash_client_backend::address::UnifiedAddress` - - `zcash_client_backend::keys::{UnifiedSpendingKey`, `UnifiedFullViewingKey`, `Era`, `DecodingError`} + - `zcash_client_backend::keys::{UnifiedSpendingKey, UnifiedFullViewingKey, Era, DecodingError}` - `zcash_client_backend::encoding::AddressCodec` - `zcash_client_backend::encoding::encode_payment_address` - `zcash_client_backend::encoding::encode_transparent_address` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 2721219ae5..a535e50e28 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1514,8 +1514,11 @@ pub trait WalletWrite: WalletRead { received_tx: DecryptedTransaction, ) -> Result<(), Self::Error>; - /// Saves information about a transaction that was constructed and sent by the wallet to the - /// persistent wallet store. + /// Saves information about a transaction constructed by the wallet to the persistent + /// wallet store. + /// + /// The name `store_sent_tx` is somewhat misleading; this must be called *before* the + /// transaction is sent to the network. fn store_sent_tx( &mut self, sent_tx: &SentTransaction, diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 2c10db70ca..562045a2af 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -85,6 +85,8 @@ pub enum Error { /// An error occurred parsing the address from a payment request. Address(ConversionError<&'static str>), + /// The address associated with a record being inserted was not recognized as + /// belonging to the wallet. #[cfg(feature = "transparent-inputs")] AddressNotRecognized(TransparentAddress), } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 69cc88f932..2518998906 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -289,6 +289,13 @@ where ) } +type ErrorT = Error< + ::Error, + ::Error, + InputsErrT, + ::Error, +>; + /// Constructs a transaction or series of transactions that send funds as specified /// by the `request` argument, stores them to the wallet's "sent transactions" data /// store, and returns the [`TxId`] for each transaction constructed. @@ -353,15 +360,7 @@ pub fn spend( request: zip321::TransactionRequest, ovk_policy: OvkPolicy, min_confirmations: NonZeroU32, -) -> Result< - NonEmpty, - Error< - ::Error, - ::Error, - InputsT::Error, - ::Error, - >, -> +) -> Result, ErrorT> where DbT: InputSource, DbT: WalletWrite< @@ -592,15 +591,7 @@ pub fn create_proposed_transactions( usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, proposal: &Proposal, -) -> Result< - NonEmpty, - Error< - ::Error, - ::Error, - InputsErrT, - FeeRuleT::Error, - >, -> +) -> Result, ErrorT> where DbT: WalletWrite + WalletCommitmentTrees, ParamsT: consensus::Parameters + Clone, @@ -645,15 +636,7 @@ fn create_proposed_transaction( min_target_height: BlockHeight, prior_step_results: &[(&proposal::Step, BuildResult)], proposal_step: &proposal::Step, -) -> Result< - BuildResult, - Error< - ::Error, - ::Error, - InputsErrT, - FeeRuleT::Error, - >, -> +) -> Result> where DbT: WalletWrite + WalletCommitmentTrees, ParamsT: consensus::Parameters + Clone, @@ -980,10 +963,7 @@ where memo.clone(), )?; orchard_output_meta.push(( - Recipient::External( - payment.recipient_address().clone(), - PoolType::Shielded(ShieldedProtocol::Orchard), - ), + Recipient::External(payment.recipient_address().clone(), *output_pool), payment.amount(), Some(memo), )); @@ -997,10 +977,7 @@ where memo.clone(), )?; sapling_output_meta.push(( - Recipient::External( - payment.recipient_address().clone(), - PoolType::Shielded(ShieldedProtocol::Sapling), - ), + Recipient::External(payment.recipient_address().clone(), *output_pool), payment.amount(), Some(memo), )); @@ -1044,6 +1021,9 @@ where payment.amount(), )); } + Address::Tex(_) => { + return Err(Error::ProposalNotSupported); + } } } @@ -1051,8 +1031,9 @@ where let memo = change_value .memo() .map_or_else(MemoBytes::empty, |m| m.clone()); - match change_value.output_pool() { - ShieldedProtocol::Sapling => { + let output_pool = change_value.output_pool(); + match output_pool { + PoolType::Shielded(ShieldedProtocol::Sapling) => { builder.add_sapling_output( sapling_internal_ovk(), sapling_dfvk.change_address().1, @@ -1063,17 +1044,15 @@ where Recipient::InternalAccount { receiving_account: account, external_address: None, - note: PoolType::Shielded(ShieldedProtocol::Sapling), + note: output_pool, }, change_value.value(), Some(memo), )) } - ShieldedProtocol::Orchard => { + PoolType::Shielded(ShieldedProtocol::Orchard) => { #[cfg(not(feature = "orchard"))] - return Err(Error::UnsupportedChangeType(PoolType::Shielded( - ShieldedProtocol::Orchard, - ))); + return Err(Error::UnsupportedChangeType(output_pool)); #[cfg(feature = "orchard")] { @@ -1087,13 +1066,16 @@ where Recipient::InternalAccount { receiving_account: account, external_address: None, - note: PoolType::Shielded(ShieldedProtocol::Orchard), + note: output_pool, }, change_value.value(), Some(memo), )) } } + PoolType::Transparent => { + return Err(Error::UnsupportedChangeType(output_pool)); + } } } @@ -1115,7 +1097,7 @@ where let recipient = recipient .map_internal_account_note(|pool| { - assert!(pool == PoolType::Shielded(ShieldedProtocol::Orchard)); + assert!(pool == PoolType::ORCHARD); build_result .transaction() .orchard_bundle() @@ -1145,7 +1127,7 @@ where let recipient = recipient .map_internal_account_note(|pool| { - assert!(pool == PoolType::Shielded(ShieldedProtocol::Sapling)); + assert!(pool == PoolType::SAPLING); build_result .transaction() .sapling_bundle() diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index ebbeef66a4..3632fa56cf 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -222,6 +222,8 @@ pub enum GreedyInputSelectorError { Balance(BalanceError), /// A unified address did not contain a supported receiver. UnsupportedAddress(Box), + /// Support for transparent-source-only (TEX) addresses requires the transparent-inputs feature. + UnsupportedTexAddress, /// An error was encountered in change selection. Change(ChangeError), } @@ -239,6 +241,9 @@ impl fmt::Display for GreedyInputSelectorErro // don't have network parameters here write!(f, "Unified address contains no supported receivers.") } + GreedyInputSelectorError::UnsupportedTexAddress => { + write!(f, "Support for transparent-source-only (TEX) addresses requires the transparent-inputs feature.") + } GreedyInputSelectorError::Change(err) => { write!(f, "An error occurred computing change and fees: {}", err) } @@ -367,32 +372,37 @@ where match recipient_address { Address::Transparent(addr) => { - payment_pools.insert(*idx, PoolType::Transparent); + payment_pools.insert(*idx, PoolType::TRANSPARENT); transparent_outputs.push(TxOut { value: payment.amount(), script_pubkey: addr.script(), }); } + Address::Tex(_) => { + return Err(InputSelectorError::Selection( + GreedyInputSelectorError::UnsupportedTexAddress, + )); + } Address::Sapling(_) => { - payment_pools.insert(*idx, PoolType::Shielded(ShieldedProtocol::Sapling)); + payment_pools.insert(*idx, PoolType::SAPLING); sapling_outputs.push(SaplingPayment(payment.amount())); } Address::Unified(addr) => { #[cfg(feature = "orchard")] if addr.orchard().is_some() { - payment_pools.insert(*idx, PoolType::Shielded(ShieldedProtocol::Orchard)); + payment_pools.insert(*idx, PoolType::ORCHARD); orchard_outputs.push(OrchardPayment(payment.amount())); continue; } if addr.sapling().is_some() { - payment_pools.insert(*idx, PoolType::Shielded(ShieldedProtocol::Sapling)); + payment_pools.insert(*idx, PoolType::SAPLING); sapling_outputs.push(SaplingPayment(payment.amount())); continue; } if let Some(addr) = addr.transparent() { - payment_pools.insert(*idx, PoolType::Transparent); + payment_pools.insert(*idx, PoolType::TRANSPARENT); transparent_outputs.push(TxOut { value: payment.amount(), script_pubkey: addr.script(), @@ -587,17 +597,9 @@ where target_height, &transparent_inputs, &Vec::::new(), - &( - ::sapling::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &sapling::EmptyBundleView, #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &orchard_fees::EmptyBundleView, &self.dust_output_policy, ); @@ -612,17 +614,9 @@ where target_height, &transparent_inputs, &Vec::::new(), - &( - ::sapling::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &sapling::EmptyBundleView, #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &orchard_fees::EmptyBundleView, &self.dust_output_policy, )? } diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 9106a3883a..f4fcfc2312 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -11,8 +11,7 @@ use zcash_primitives::{ fees::{transparent, FeeRule}, }, }; - -use crate::ShieldedProtocol; +use zcash_protocol::{PoolType, ShieldedProtocol}; pub(crate) mod common; pub mod fixed; @@ -25,7 +24,7 @@ pub mod zip317; /// A proposed change amount and output pool. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ChangeValue { - output_pool: ShieldedProtocol, + output_pool: PoolType, value: NonNegativeAmount, memo: Option, } @@ -33,38 +32,52 @@ pub struct ChangeValue { impl ChangeValue { /// Constructs a new change value from its constituent parts. pub fn new( - output_pool: ShieldedProtocol, + output_pool: PoolType, value: NonNegativeAmount, memo: Option, - ) -> Self { - Self { + ) -> Option { + (matches!(output_pool, PoolType::Shielded(_)) || memo.is_none()).then_some(Self { output_pool, value, memo, + }) + } + + /// Constructs a new change value that will be created as a transparent output. + pub fn transparent(value: NonNegativeAmount) -> Self { + Self { + output_pool: PoolType::TRANSPARENT, + value, + memo: None, } } - /// Constructs a new change value that will be created as a Sapling output. - pub fn sapling(value: NonNegativeAmount, memo: Option) -> Self { + /// Constructs a new change value that will be created as a shielded output. + pub fn shielded( + protocol: ShieldedProtocol, + value: NonNegativeAmount, + memo: Option, + ) -> Self { Self { - output_pool: ShieldedProtocol::Sapling, + output_pool: PoolType::Shielded(protocol), value, memo, } } + /// Constructs a new change value that will be created as a Sapling output. + pub fn sapling(value: NonNegativeAmount, memo: Option) -> Self { + Self::shielded(ShieldedProtocol::Sapling, value, memo) + } + /// Constructs a new change value that will be created as an Orchard output. #[cfg(feature = "orchard")] pub fn orchard(value: NonNegativeAmount, memo: Option) -> Self { - Self { - output_pool: ShieldedProtocol::Orchard, - value, - memo, - } + Self::shielded(ShieldedProtocol::Orchard, value, memo) } /// Returns the pool to which the change output should be sent. - pub fn output_pool(&self) -> ShieldedProtocol { + pub fn output_pool(&self) -> PoolType { self.output_pool } @@ -237,20 +250,20 @@ impl From for ChangeError { } } -/// An enumeration of actions to tak when a transaction would potentially create dust -/// outputs (outputs that are likely to be without economic value due to fee rules.) +/// An enumeration of actions to take when a transaction would potentially create dust +/// outputs (outputs that are likely to be without economic value due to fee rules). #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum DustAction { /// Do not allow creation of dust outputs; instead, require that additional inputs be provided. Reject, /// Explicitly allow the creation of dust change amounts greater than the specified value. AllowDustChange, - /// Allow dust amounts to be added to the transaction fee + /// Allow dust amounts to be added to the transaction fee. AddDustToFee, } /// A policy describing how a [`ChangeStrategy`] should treat potentially dust-valued change -/// outputs (outputs that are likely to be without economic value due to fee rules.) +/// outputs (outputs that are likely to be without economic value due to fee rules). #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct DustOutputPolicy { action: DustAction, @@ -262,7 +275,7 @@ impl DustOutputPolicy { /// /// A dust policy created with `None` as the dust threshold will delegate determination /// of the dust threshold to the change strategy that is evaluating the strategy; this - /// recommended, but an explicit value (including zero) may be provided to explicitly + /// is recommended, but an explicit value (including zero) may be provided to explicitly /// override the determination of the change strategy. pub fn new(action: DustAction, dust_threshold: Option) -> Self { Self { @@ -271,7 +284,7 @@ impl DustOutputPolicy { } } - /// Returns the action to take in the event that a dust change amount would be produced + /// Returns the action to take in the event that a dust change amount would be produced. pub fn action(&self) -> DustAction { self.action } diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index a8a791a527..1910ca1951 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -6,8 +6,7 @@ use zcash_primitives::{ fees::{transparent, FeeRule}, }, }; - -use crate::ShieldedProtocol; +use zcash_protocol::ShieldedProtocol; use super::{ sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy, @@ -91,35 +90,34 @@ where }) } -pub(crate) fn single_change_output_policy( +/// Decide which shielded pool change should go to if there is any. +pub(crate) fn single_change_output_policy( _net_flows: &NetFlows, _fallback_change_pool: ShieldedProtocol, -) -> Result<(ShieldedProtocol, usize, usize), ChangeError> -where - E: From + From, -{ +) -> (ShieldedProtocol, usize, usize) { // TODO: implement a less naive strategy for selecting the pool to which change will be sent. - #[cfg(feature = "orchard")] - let (change_pool, sapling_change, orchard_change) = + let change_pool = { + #[cfg(feature = "orchard")] if _net_flows.orchard_in.is_positive() || _net_flows.orchard_out.is_positive() { - // Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs - (ShieldedProtocol::Orchard, 0, 1) + // Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs. + ShieldedProtocol::Orchard } else if _net_flows.sapling_in.is_positive() || _net_flows.sapling_out.is_positive() { // Otherwise, send change to Sapling if we're spending any Sapling inputs or creating any // Sapling outputs, so that we avoid pool-crossing. - (ShieldedProtocol::Sapling, 1, 0) + ShieldedProtocol::Sapling } else { - // This is a fully-transparent transaction, so the caller gets to decide - // where to shield change. - match _fallback_change_pool { - ShieldedProtocol::Orchard => (_fallback_change_pool, 0, 1), - ShieldedProtocol::Sapling => (_fallback_change_pool, 1, 0), - } - }; - #[cfg(not(feature = "orchard"))] - let (change_pool, sapling_change, orchard_change) = (ShieldedProtocol::Sapling, 1, 0); - - Ok((change_pool, sapling_change, orchard_change)) + // The flows are transparent, so there may not be change. If there is, the caller + // gets to decide where to shield it. + _fallback_change_pool + } + #[cfg(not(feature = "orchard"))] + ShieldedProtocol::Sapling + }; + ( + change_pool, + (change_pool == ShieldedProtocol::Sapling).into(), + (change_pool == ShieldedProtocol::Orchard).into(), + ) } #[allow(clippy::too_many_arguments)] @@ -155,7 +153,7 @@ where orchard, )?; let (change_pool, sapling_change, _orchard_change) = - single_change_output_policy::(&net_flows, _fallback_change_pool)?; + single_change_output_policy(&net_flows, _fallback_change_pool); let sapling_input_count = sapling .bundle_type() @@ -184,8 +182,8 @@ where .fee_required( params, target_height, - transparent_inputs, - transparent_outputs, + transparent_inputs.iter().map(|i| i.serialized_size()), + transparent_outputs.iter().map(|i| i.serialized_size()), sapling_input_count, sapling_output_count, orchard_action_count, @@ -220,7 +218,11 @@ where }) } DustAction::AllowDustChange => TransactionBalance::new( - vec![ChangeValue::new(change_pool, proposed_change, change_memo)], + vec![ChangeValue::shielded( + change_pool, + proposed_change, + change_memo, + )], fee_amount, ) .map_err(|_| overflow()), @@ -232,7 +234,11 @@ where } } else { TransactionBalance::new( - vec![ChangeValue::new(change_pool, proposed_change, change_memo)], + vec![ChangeValue::shielded( + change_pool, + proposed_change, + change_memo, + )], fee_amount, ) .map_err(|_| overflow()) diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index a20ab3ef13..ee032ab013 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -83,9 +83,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(test)] mod tests { - #[cfg(feature = "orchard")] - use std::convert::Infallible; - use zcash_primitives::{ consensus::{Network, NetworkUpgrade, Parameters}, transaction::{ @@ -104,6 +101,9 @@ mod tests { ShieldedProtocol, }; + #[cfg(feature = "orchard")] + use crate::fees::orchard as orchard_fees; + #[test] fn change_without_dust() { #[allow(deprecated)] @@ -117,8 +117,8 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), - &Vec::::new(), + &[] as &[TestTransparentInput], + &[] as &[TxOut], &( sapling::builder::BundleType::DEFAULT, &[TestSaplingInput { @@ -130,11 +130,7 @@ mod tests { ))][..], ), #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &[] as &[Infallible], - &[] as &[Infallible], - ), + &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), ); @@ -159,8 +155,8 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), - &Vec::::new(), + &[] as &[TestTransparentInput], + &[] as &[TxOut], &( sapling::builder::BundleType::DEFAULT, &[ @@ -179,11 +175,7 @@ mod tests { ))][..], ), #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &[] as &[Infallible], - &[] as &[Infallible], - ), + &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), ); diff --git a/zcash_client_backend/src/fees/orchard.rs b/zcash_client_backend/src/fees/orchard.rs index ac90a01130..b4a9278519 100644 --- a/zcash_client_backend/src/fees/orchard.rs +++ b/zcash_client_backend/src/fees/orchard.rs @@ -41,6 +41,26 @@ impl<'a, NoteRef, In: InputView, Out: OutputView> BundleView } } +/// A [`BundleView`] for the empty bundle with [`BundleType::DEFAULT`] bundle type. +pub struct EmptyBundleView; + +impl BundleView for EmptyBundleView { + type In = Infallible; + type Out = Infallible; + + fn bundle_type(&self) -> BundleType { + BundleType::DEFAULT + } + + fn inputs(&self) -> &[Self::In] { + &[] + } + + fn outputs(&self) -> &[Self::Out] { + &[] + } +} + /// A trait that provides a minimized view of an Orchard input suitable for use in fee and change /// calculation. pub trait InputView { diff --git a/zcash_client_backend/src/fees/sapling.rs b/zcash_client_backend/src/fees/sapling.rs index fa7ef6157e..9f17f53a61 100644 --- a/zcash_client_backend/src/fees/sapling.rs +++ b/zcash_client_backend/src/fees/sapling.rs @@ -41,6 +41,26 @@ impl<'a, NoteRef, In: InputView, Out: OutputView> BundleView } } +/// A [`BundleView`] for the empty bundle with [`BundleType::DEFAULT`] bundle type. +pub struct EmptyBundleView; + +impl BundleView for EmptyBundleView { + type In = Infallible; + type Out = Infallible; + + fn bundle_type(&self) -> BundleType { + BundleType::DEFAULT + } + + fn inputs(&self) -> &[Self::In] { + &[] + } + + fn outputs(&self) -> &[Self::Out] { + &[] + } +} + /// A trait that provides a minimized view of a Sapling input suitable for use in /// fee and change calculation. pub trait InputView { diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index e3acacd590..750743aee3 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -126,10 +126,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, )?; let (_, sapling_change, orchard_change) = - single_change_output_policy::( - &net_flows, - self.fallback_change_pool, - )?; + single_change_output_policy(&net_flows, self.fallback_change_pool); let s_non_dust = sapling.inputs().len() - sapling_dust.len(); let s_allowed_dust = @@ -238,7 +235,10 @@ mod tests { }; #[cfg(feature = "orchard")] - use crate::data_api::wallet::input_selection::OrchardPayment; + use { + crate::data_api::wallet::input_selection::OrchardPayment, + crate::fees::orchard as orchard_fees, + }; #[test] fn change_without_dust() { @@ -254,8 +254,8 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), - &Vec::::new(), + &[] as &[TestTransparentInput], + &[] as &[TxOut], &( sapling::builder::BundleType::DEFAULT, &[TestSaplingInput { @@ -267,11 +267,7 @@ mod tests { ))][..], ), #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), ); @@ -298,19 +294,19 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), - &Vec::::new(), + &[] as &[TestTransparentInput], + &[] as &[TxOut], &( sapling::builder::BundleType::DEFAULT, &[TestSaplingInput { note_id: 0, value: NonNegativeAmount::const_from_u64(55000), }][..], - &Vec::::new()[..], + &[] as &[Infallible], ), &( orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], + &[] as &[Infallible], &[OrchardPayment::new(NonNegativeAmount::const_from_u64( 30000, ))][..], @@ -340,7 +336,7 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), + &[] as &[TestTransparentInput], &[TxOut { value: NonNegativeAmount::const_from_u64(40000), script_pubkey: Script(vec![]), @@ -351,14 +347,10 @@ mod tests { note_id: 0, value: NonNegativeAmount::const_from_u64(55000), }][..], - &Vec::::new()[..], + &[] as &[Infallible], ), #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), ); @@ -383,8 +375,8 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), - &Vec::::new(), + &[] as &[TestTransparentInput], + &[] as &[TxOut], &( sapling::builder::BundleType::DEFAULT, &[ @@ -402,11 +394,7 @@ mod tests { ))][..], ), #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), ); @@ -431,8 +419,8 @@ mod tests { Network::TestNetwork .activation_height(NetworkUpgrade::Nu5) .unwrap(), - &Vec::::new(), - &Vec::::new(), + &[] as &[TestTransparentInput], + &[] as &[TxOut], &( sapling::builder::BundleType::DEFAULT, &[ @@ -454,11 +442,7 @@ mod tests { ))][..], ), #[cfg(feature = "orchard")] - &( - orchard::builder::BundleType::DEFAULT, - &Vec::::new()[..], - &Vec::::new()[..], - ), + &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), ); diff --git a/zcash_client_backend/src/proposal.rs b/zcash_client_backend/src/proposal.rs index 9fe25934f5..3c0c01215a 100644 --- a/zcash_client_backend/src/proposal.rs +++ b/zcash_client_backend/src/proposal.rs @@ -186,7 +186,7 @@ impl Proposal { for t_out in step.transparent_inputs() { let key = ( - PoolType::Transparent, + PoolType::TRANSPARENT, TxId::from_bytes(*t_out.outpoint().hash()), t_out.outpoint().n(), ); @@ -198,9 +198,9 @@ impl Proposal { for s_out in step.shielded_inputs().iter().flat_map(|i| i.notes().iter()) { let key = ( match &s_out.note() { - Note::Sapling(_) => PoolType::Shielded(ShieldedProtocol::Sapling), + Note::Sapling(_) => PoolType::SAPLING, #[cfg(feature = "orchard")] - Note::Orchard(_) => PoolType::Shielded(ShieldedProtocol::Orchard), + Note::Orchard(_) => PoolType::ORCHARD, }, *s_out.txid(), s_out.output_index().into(), @@ -490,57 +490,37 @@ impl Step { self.is_shielding } - /// Returns whether or not this proposal requires interaction with the specified pool + /// Returns whether or not this proposal requires interaction with the specified pool. pub fn involves(&self, pool_type: PoolType) -> bool { - match pool_type { - PoolType::Transparent => { - self.is_shielding - || !self.transparent_inputs.is_empty() - || self - .payment_pools() - .values() - .any(|pool| matches!(pool, PoolType::Transparent)) - } + let input_in_this_pool = || match pool_type { + PoolType::Transparent => self.is_shielding || !self.transparent_inputs.is_empty(), PoolType::Shielded(ShieldedProtocol::Sapling) => { - let sapling_in = self.shielded_inputs.iter().any(|s_in| { + self.shielded_inputs.iter().any(|s_in| { s_in.notes() .iter() .any(|note| matches!(note.note(), Note::Sapling(_))) - }); - let sapling_out = self - .payment_pools() - .values() - .any(|pool| matches!(pool, PoolType::Shielded(ShieldedProtocol::Sapling))); - let sapling_change = self - .balance - .proposed_change() - .iter() - .any(|c| c.output_pool() == ShieldedProtocol::Sapling); - - sapling_in || sapling_out || sapling_change + }) } + #[cfg(feature = "orchard")] PoolType::Shielded(ShieldedProtocol::Orchard) => { - #[cfg(not(feature = "orchard"))] - let orchard_in = false; - #[cfg(feature = "orchard")] - let orchard_in = self.shielded_inputs.iter().any(|s_in| { + self.shielded_inputs.iter().any(|s_in| { s_in.notes() .iter() .any(|note| matches!(note.note(), Note::Orchard(_))) - }); - let orchard_out = self - .payment_pools() - .values() - .any(|pool| matches!(pool, PoolType::Shielded(ShieldedProtocol::Orchard))); - let orchard_change = self - .balance - .proposed_change() - .iter() - .any(|c| c.output_pool() == ShieldedProtocol::Orchard); - - orchard_in || orchard_out || orchard_change + }) } - } + #[cfg(not(feature = "orchard"))] + PoolType::Shielded(ShieldedProtocol::Orchard) => false, + }; + let output_in_this_pool = || self.payment_pools().values().any(|pool| *pool == pool_type); + let change_in_this_pool = || { + self.balance + .proposed_change() + .iter() + .any(|c| c.output_pool() == pool_type) + }; + + input_in_this_pool() || output_in_this_pool() || change_in_this_pool() } } diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 65d4c483cb..915d1d39e3 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -1,4 +1,4 @@ -//! Generated code for handling light client protobuf structs. +//! This module contains generated code for handling light client protobuf structs. use incrementalmerkletree::frontier::CommitmentTree; use nonempty::NonEmpty; @@ -441,9 +441,9 @@ impl std::error::Error for ProposalDecodingError fn pool_type(pool_id: i32) -> Result> { match proposal::ValuePool::try_from(pool_id) { - Ok(proposal::ValuePool::Transparent) => Ok(PoolType::Transparent), - Ok(proposal::ValuePool::Sapling) => Ok(PoolType::Shielded(ShieldedProtocol::Sapling)), - Ok(proposal::ValuePool::Orchard) => Ok(PoolType::Shielded(ShieldedProtocol::Orchard)), + Ok(proposal::ValuePool::Transparent) => Ok(PoolType::TRANSPARENT), + Ok(proposal::ValuePool::Sapling) => Ok(PoolType::SAPLING), + Ok(proposal::ValuePool::Orchard) => Ok(PoolType::ORCHARD), _ => Err(ProposalDecodingError::ValuePoolNotSupported(pool_id)), } } @@ -675,7 +675,7 @@ impl proposal::Proposal { .ok_or({ ProposalDecodingError::InputNotFound( txid, - PoolType::Transparent, + PoolType::TRANSPARENT, out.index, ) })?, diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 04897387da..c0a10cac4b 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -89,7 +89,7 @@ pub enum SqliteClientError { AccountIdOutOfRange, /// The address associated with a record being inserted was not recognized as - /// belonging to the wallet + /// belonging to the wallet. #[cfg(feature = "transparent-inputs")] AddressNotRecognized(TransparentAddress), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index b171bf4577..092d060d84 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -233,9 +233,7 @@ impl, P: consensus::Parameters> InputSource for .map(|opt| opt.map(|n| n.map_note(Note::Orchard))); #[cfg(not(feature = "orchard"))] - return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded( - ShieldedProtocol::Orchard, - ))); + return Err(SqliteClientError::UnsupportedPoolType(PoolType::ORCHARD)); } } } @@ -1074,7 +1072,7 @@ impl WalletWrite for WalletDb receiver.to_zcash_address(wdb.params.network_type()) ); - Recipient::External(wallet_address, PoolType::Shielded(ShieldedProtocol::Sapling)) + Recipient::External(wallet_address, PoolType::SAPLING) }; wallet::put_sent_output( @@ -1155,7 +1153,7 @@ impl WalletWrite for WalletDb receiver.to_zcash_address(wdb.params.network_type()) ); - Recipient::External(wallet_address, PoolType::Shielded(ShieldedProtocol::Orchard)) + Recipient::External(wallet_address, PoolType::ORCHARD) }; wallet::put_sent_output( @@ -1277,7 +1275,7 @@ impl WalletWrite for WalletDb #[cfg(not(feature = "transparent-inputs"))] let recipient_addr = receiver.to_zcash_address(wdb.params.network_type()); - let recipient = Recipient::External(recipient_addr, PoolType::Transparent); + let recipient = Recipient::External(recipient_addr, PoolType::TRANSPARENT); wallet::put_sent_output( wdb.conn.0, diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index d6161ea2d7..53d2b6c19b 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -1807,7 +1807,9 @@ fn fake_compact_block_spending( ) .0, ), - Address::Transparent(_) => panic!("transparent addresses not supported in compact blocks"), + Address::Transparent(_) | Address::Tex(_) => { + panic!("transparent addresses not supported in compact blocks") + } Address::Unified(ua) => { // This is annoying to implement, because the protocol-aware UA type has no // concept of ZIP 316 preference order. diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 4a47dd24a9..d3c43ac44c 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -63,11 +63,14 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::{ - fees::TransactionBalance, proposal::Step, wallet::WalletTransparentOutput, PoolType, + fees::TransactionBalance, proposal::Step, wallet::WalletTransparentOutput, }, zcash_primitives::transaction::components::{OutPoint, TxOut}, }; +#[cfg(any(feature = "transparent-inputs", feature = "orchard"))] +use zcash_client_backend::PoolType; + pub(crate) type OutputRecoveryError = Error< SqliteClientError, commitment_tree::Error, @@ -386,7 +389,7 @@ pub(crate) fn send_multi_step_proposed_transfer() { let step1 = Step::from_parts( &[step0.clone()], request1, - [(0, PoolType::Transparent)].into_iter().collect(), + [(0, PoolType::TRANSPARENT)].into_iter().collect(), vec![], None, vec![StepOutput::new(0, StepOutputIndex::Payment(0))], @@ -1203,7 +1206,7 @@ pub(crate) fn shield_transparent() { st.scan_cached_blocks(h, 1); let utxo = WalletTransparentOutput::from_parts( - OutPoint::new([1u8; 32], 1), + OutPoint::fake(), TxOut { value: NonNegativeAmount::const_from_u64(10000), script_pubkey: taddr.script(), @@ -1485,7 +1488,10 @@ pub(crate) fn pool_crossing_required( @@ -1661,7 +1670,7 @@ pub(crate) fn fully_funded_send_to_t( diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index b51b52948a..38a46a364a 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -623,9 +623,7 @@ pub(crate) fn get_transparent_receivers( let ua_str: String = row.get(0)?; let di_vec: Vec = row.get(1)?; let mut di: [u8; 11] = di_vec.try_into().map_err(|_| { - SqliteClientError::CorruptedData( - "Diverisifier index is not an 11-byte value".to_owned(), - ) + SqliteClientError::CorruptedData("Diversifier index is not an 11-byte value".to_owned()) })?; di.reverse(); // BE -> LE conversion @@ -665,12 +663,12 @@ pub(crate) fn get_transparent_receivers( } } - if let Some((taddr, child_index)) = get_legacy_transparent_address(params, conn, account)? { + if let Some((taddr, address_index)) = get_legacy_transparent_address(params, conn, account)? { ret.insert( taddr, Some(TransparentAddressMetadata::new( Scope::External.into(), - child_index, + address_index, )), ); } @@ -1418,9 +1416,7 @@ pub(crate) fn get_received_memo( ShieldedProtocol::Orchard => fetch_memo(ORCHARD_TABLES_PREFIX, "action_index")?.flatten(), #[cfg(not(feature = "orchard"))] ShieldedProtocol::Orchard => { - return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded( - ShieldedProtocol::Orchard, - ))) + return Err(SqliteClientError::UnsupportedPoolType(PoolType::ORCHARD)) } }; @@ -2921,7 +2917,7 @@ mod tests { // Create a fake transparent output. let value = NonNegativeAmount::const_from_u64(100000); - let outpoint = OutPoint::new([1u8; 32], 1); + let outpoint = OutPoint::fake(); let txout = TxOut { value, script_pubkey: taddr.script(), @@ -3100,7 +3096,6 @@ mod tests { // Create a fake transparent output. let value = NonNegativeAmount::from_u64(100000).unwrap(); - let outpoint = OutPoint::new([1u8; 32], 1); let txout = TxOut { value, script_pubkey: taddr.script(), @@ -3108,7 +3103,7 @@ mod tests { // Pretend the output was received in the chain tip. let height = st.wallet().chain_height().unwrap().unwrap(); - let utxo = WalletTransparentOutput::from_parts(outpoint, txout, height).unwrap(); + let utxo = WalletTransparentOutput::from_parts(OutPoint::fake(), txout, height).unwrap(); st.wallet_mut() .put_received_transparent_utxo(&utxo) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index dfe62470b5..665c253a57 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -905,7 +905,7 @@ mod tests { wdb.conn.execute( "INSERT INTO sent_notes (tx, output_pool, output_index, from_account, address, value) VALUES (0, ?, 0, ?, ?, 0)", - [pool_code(PoolType::Transparent).to_sql()?, u32::from(account).to_sql()?, taddr.to_sql()?])?; + [pool_code(PoolType::TRANSPARENT).to_sql()?, u32::from(account).to_sql()?, taddr.to_sql()?])?; } Ok(()) diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs index a84f99a05b..b73c7af896 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs @@ -420,7 +420,7 @@ mod tests { BlockHeight::from(3), Some(transparent::Bundle { vin: vec![TxIn { - prevout: OutPoint::new([1u8; 32], 1), + prevout: OutPoint::fake(), script_sig: Script(vec![]), sequence: 0, }], diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs index 45a5b45c45..431341114c 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs @@ -186,12 +186,12 @@ fn get_transparent_receivers( } } - if let Some((taddr, child_index)) = get_legacy_transparent_address(params, conn, account)? { + if let Some((taddr, address_index)) = get_legacy_transparent_address(params, conn, account)? { ret.insert( taddr, Some(TransparentAddressMetadata::new( Scope::External.into(), - child_index, + address_index, )), ); } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs b/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs index 550dc14f43..beb3825971 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs @@ -5,7 +5,7 @@ use std::collections::HashSet; use schemer_rusqlite::RusqliteMigration; use uuid::Uuid; -use zcash_client_backend::{PoolType, ShieldedProtocol}; +use zcash_client_backend::PoolType; use super::full_account_ids; use crate::wallet::{init::WalletMigrationError, pool_code}; @@ -72,8 +72,8 @@ impl RusqliteMigration for Migration { )?; transaction.execute_batch({ - let sapling_pool_code = pool_code(PoolType::Shielded(ShieldedProtocol::Sapling)); - let orchard_pool_code = pool_code(PoolType::Shielded(ShieldedProtocol::Orchard)); + let sapling_pool_code = pool_code(PoolType::SAPLING); + let orchard_pool_code = pool_code(PoolType::ORCHARD); &format!( "CREATE VIEW v_received_notes AS SELECT @@ -109,8 +109,8 @@ impl RusqliteMigration for Migration { })?; transaction.execute_batch({ - let sapling_pool_code = pool_code(PoolType::Shielded(ShieldedProtocol::Sapling)); - let orchard_pool_code = pool_code(PoolType::Shielded(ShieldedProtocol::Orchard)); + let sapling_pool_code = pool_code(PoolType::SAPLING); + let orchard_pool_code = pool_code(PoolType::ORCHARD); &format!( "CREATE VIEW v_received_note_spends AS SELECT @@ -128,7 +128,7 @@ impl RusqliteMigration for Migration { })?; transaction.execute_batch({ - let transparent_pool_code = pool_code(PoolType::Transparent); + let transparent_pool_code = pool_code(PoolType::TRANSPARENT); &format!( "DROP VIEW v_transactions; CREATE VIEW v_transactions AS @@ -257,7 +257,7 @@ impl RusqliteMigration for Migration { })?; transaction.execute_batch({ - let transparent_pool_code = pool_code(PoolType::Transparent); + let transparent_pool_code = pool_code(PoolType::TRANSPARENT); &format!( "DROP VIEW v_tx_outputs; CREATE VIEW v_tx_outputs AS diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs index b4af042352..c8d14f4225 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs @@ -7,9 +7,7 @@ use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; -use zcash_client_backend::{ - address::Address, keys::UnifiedSpendingKey, PoolType, ShieldedProtocol, -}; +use zcash_client_backend::{address::Address, keys::UnifiedSpendingKey, PoolType}; use zcash_keys::keys::UnifiedAddressRequest; use zcash_primitives::{consensus, zip32::AccountId}; @@ -131,7 +129,7 @@ impl RusqliteMigration for Migration

{ }); } } - Address::Transparent(_) => { + Address::Transparent(_) | Address::Tex(_) => { return Err(WalletMigrationError::CorruptedData( "Address field value decoded to a transparent address; should have been Sapling or unified.".to_string())); } @@ -262,10 +260,10 @@ impl RusqliteMigration for Migration

{ )) })?; let output_pool = match decoded_address { - Address::Sapling(_) => { - Ok(pool_code(PoolType::Shielded(ShieldedProtocol::Sapling))) + Address::Sapling(_) => Ok(pool_code(PoolType::SAPLING)), + Address::Transparent(_) | Address::Tex(_) => { + Ok(pool_code(PoolType::TRANSPARENT)) } - Address::Transparent(_) => Ok(pool_code(PoolType::Transparent)), Address::Unified(_) => Err(WalletMigrationError::CorruptedData( "Unified addresses should not yet appear in the sent_notes table." .to_string(), diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs index c94d08a677..8b4aa6d198 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs @@ -6,7 +6,7 @@ use rusqlite::{self, named_params}; use schemer; use schemer_rusqlite::RusqliteMigration; use uuid::Uuid; -use zcash_client_backend::{PoolType, ShieldedProtocol}; +use zcash_client_backend::PoolType; use super::add_transaction_views; use crate::wallet::{init::WalletMigrationError, pool_code}; @@ -44,7 +44,7 @@ impl RusqliteMigration for Migration { SELECT tx, :output_pool, output_index, from_account, from_account, value FROM sent_notes", named_params![ - ":output_pool": &pool_code(PoolType::Shielded(ShieldedProtocol::Sapling)) + ":output_pool": &pool_code(PoolType::SAPLING) ] )?; diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 6987ac04e7..accb894fb5 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -5,11 +5,17 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Notable changes +- `zcash_keys` now supports TEX (transparent-source-only) addresses as specified + in [ZIP 320](https://zips.z.cash/zip-0320). + ### Added - `zcash_keys::address::Address::try_from_zcash_address` - `zcash_keys::address::Receiver` ### Changed +- `zcash_keys::Address` has a new variant `Tex`. +- `zcash_keys::address::Address::has_receiver` has been renamed to `can_receive_as`. - MSRV is now 1.70.0. - `zcash_keys::keys`: - The (unstable) encoding of `UnifiedSpendingKey` has changed. diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index 8d3e0e4609..7f2abfdcb0 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -293,10 +293,22 @@ impl Receiver { /// An address that funds can be sent to. #[derive(Debug, PartialEq, Eq, Clone)] pub enum Address { + /// A Sapling payment address. #[cfg(feature = "sapling")] Sapling(PaymentAddress), + + /// A transparent address corresponding to either a public key hash or a script hash. Transparent(TransparentAddress), + + /// A [ZIP 316] Unified Address. + /// + /// [ZIP 316]: https://zips.z.cash/zip-0316 Unified(UnifiedAddress), + + /// A [ZIP 320] transparent-source-only P2PKH address, or "TEX address". + /// + /// [ZIP 320]: https://zips.z.cash/zip-0320 + Tex([u8; 20]), } #[cfg(feature = "sapling")] @@ -344,6 +356,10 @@ impl TryFromRawAddress for Address { fn try_from_raw_transparent_p2sh(data: [u8; 20]) -> Result> { Ok(TransparentAddress::ScriptHash(data).into()) } + + fn try_from_raw_tex(data: [u8; 20]) -> Result> { + Ok(Address::Tex(data)) + } } impl Address { @@ -379,6 +395,7 @@ impl Address { } }, Address::Unified(ua) => ua.to_address(net), + Address::Tex(data) => ZcashAddress::from_tex(net, *data), } } @@ -387,14 +404,16 @@ impl Address { self.to_zcash_address(params).to_string() } - /// Returns whether or not this [`Address`] can send funds to the specified pool. - pub fn has_receiver(&self, pool_type: PoolType) -> bool { + /// Returns whether or not this [`Address`] can receive funds in the specified pool. + pub fn can_receive_as(&self, pool_type: PoolType) -> bool { match self { #[cfg(feature = "sapling")] Address::Sapling(_) => { matches!(pool_type, PoolType::Shielded(ShieldedProtocol::Sapling)) } - Address::Transparent(_) => matches!(pool_type, PoolType::Transparent), + Address::Transparent(_) | Address::Tex(_) => { + matches!(pool_type, PoolType::Transparent) + } Address::Unified(ua) => match pool_type { PoolType::Transparent => ua.transparent().is_some(), PoolType::Shielded(ShieldedProtocol::Sapling) => { @@ -449,6 +468,7 @@ pub mod testing { arb_payment_address().prop_map(Address::Sapling), arb_transparent_addr().prop_map(Address::Transparent), arb_unified_addr(Network::TestNetwork, request).prop_map(Address::Unified), + proptest::array::uniform20(any::()).prop_map(Address::Tex), ] } @@ -457,6 +477,7 @@ pub mod testing { return prop_oneof![ arb_transparent_addr().prop_map(Address::Transparent), arb_unified_addr(Network::TestNetwork, request).prop_map(Address::Unified), + proptest::array::uniform20(any::()).prop_map(Address::Tex), ]; } } diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 829f808440..9e2d78a6f8 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -11,6 +11,11 @@ and this library adheres to Rust's notion of - `impl From for bip32::ChildNumber` - `impl From for bip32::ChildNumber` - `impl TryFrom for NonHardenedChildIndex` +- `zcash_primitives::legacy::Script::serialized_size` +- `zcash_primitives::transaction::fees::transparent`: + - `InputSize` + - `InputView::serialized_size` + - `OutputView::serialized_size` ### Changed - MSRV is now 1.70.0. @@ -30,6 +35,11 @@ and this library adheres to Rust's notion of - `AccountPubKey::derive_internal_ivk` - `AccountPubKey::deserialize` - `IncomingViewingKey::derive_address` +- `zcash_primitives::transaction::fees::FeeRule::fee_required`: the types + of parameters relating to transparent inputs and outputs have changed. + This method now requires their `tx_in` and `tx_out` serialized sizes + (expressed as iterators of `InputSize` for inputs and `usize` for outputs) + rather than a slice of `InputView` or `OutputView`. ### Removed - The `zcash_primitives::zip339` module, which reexported parts of the API of diff --git a/zcash_primitives/src/legacy.rs b/zcash_primitives/src/legacy.rs index 398d9ab970..defe8e3760 100644 --- a/zcash_primitives/src/legacy.rs +++ b/zcash_primitives/src/legacy.rs @@ -322,6 +322,11 @@ impl Script { Vector::write(&mut writer, &self.0, |w, e| w.write_u8(*e)) } + /// Returns the length of this script as encoded (including the initial CompactSize). + pub fn serialized_size(&self) -> usize { + Vector::serialized_size_of_u8_vec(&self.0) + } + /// Returns the address that this Script contains, if any. pub(crate) fn address(&self) -> Option { if self.0.len() == 25 @@ -374,7 +379,7 @@ impl Shl<&[u8]> for Script { } } -/// A transparent address corresponding to either a public key or a `Script`. +/// A transparent address corresponding to either a public key hash or a script hash. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum TransparentAddress { PublicKeyHash([u8; 20]), diff --git a/zcash_primitives/src/legacy/keys.rs b/zcash_primitives/src/legacy/keys.rs index 3d204ed549..8357762cc1 100644 --- a/zcash_primitives/src/legacy/keys.rs +++ b/zcash_primitives/src/legacy/keys.rs @@ -134,34 +134,34 @@ impl AccountPrivKey { } /// Derives the BIP44 private spending key for the child path - /// `m/44'/'/'//`. + /// `m/44'/'/'//`. pub fn derive_secret_key( &self, scope: TransparentKeyScope, - child_index: NonHardenedChildIndex, + address_index: NonHardenedChildIndex, ) -> Result { self.0 .derive_child(scope.into())? - .derive_child(child_index.into()) + .derive_child(address_index.into()) .map(|k| *k.private_key()) } /// Derives the BIP44 private spending key for the external (incoming payment) child path - /// `m/44'/'/'/0/`. + /// `m/44'/'/'/0/`. pub fn derive_external_secret_key( &self, - child_index: NonHardenedChildIndex, + address_index: NonHardenedChildIndex, ) -> Result { - self.derive_secret_key(zip32::Scope::External.into(), child_index) + self.derive_secret_key(zip32::Scope::External.into(), address_index) } /// Derives the BIP44 private spending key for the internal (change) child path - /// `m/44'/'/'/1/`. + /// `m/44'/'/'/1/`. pub fn derive_internal_secret_key( &self, - child_index: NonHardenedChildIndex, + address_index: NonHardenedChildIndex, ) -> Result { - self.derive_secret_key(zip32::Scope::Internal.into(), child_index) + self.derive_secret_key(zip32::Scope::Internal.into(), address_index) } /// Returns the `AccountPrivKey` serialized using the encoding for a @@ -308,9 +308,9 @@ pub trait IncomingViewingKey: private::SealedChangeLevelKey + std::marker::Sized #[allow(deprecated)] fn derive_address( &self, - child_index: NonHardenedChildIndex, + address_index: NonHardenedChildIndex, ) -> Result { - let child_key = self.extended_pubkey().derive_child(child_index.into())?; + let child_key = self.extended_pubkey().derive_child(address_index.into())?; Ok(pubkey_to_address(child_key.public_key())) } @@ -318,14 +318,14 @@ pub trait IncomingViewingKey: private::SealedChangeLevelKey + std::marker::Sized /// generate a valid transparent address, and returns the resulting /// address and the index at which it was generated. fn default_address(&self) -> (TransparentAddress, NonHardenedChildIndex) { - let mut child_index = NonHardenedChildIndex::ZERO; + let mut address_index = NonHardenedChildIndex::ZERO; loop { - match self.derive_address(child_index) { + match self.derive_address(address_index) { Ok(addr) => { - return (addr, child_index); + return (addr, address_index); } Err(_) => { - child_index = child_index.next().unwrap_or_else(|| { + address_index = address_index.next().unwrap_or_else(|| { panic!("Exhausted child index space attempting to find a default address."); }); } diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index ef352d7e0f..8304c19f80 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -22,7 +22,10 @@ use crate::{ amount::{Amount, BalanceError}, transparent::{self, builder::TransparentBuilder, TxOut}, }, - fees::FeeRule, + fees::{ + transparent::{InputView, OutputView}, + FeeRule, + }, sighash::{signature_hash, SignableInput}, txid::TxIdDigester, Transaction, TransactionData, TxVersion, Unauthorized, @@ -554,8 +557,11 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder< .fee_required( &self.params, self.target_height, - transparent_inputs, - self.transparent_builder.outputs(), + transparent_inputs.iter().map(|i| i.serialized_size()), + self.transparent_builder + .outputs() + .iter() + .map(|i| i.serialized_size()), sapling_spends, self.sapling_builder .as_ref() @@ -597,8 +603,11 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder< .fee_required_zfuture( &self.params, self.target_height, - transparent_inputs, - self.transparent_builder.outputs(), + transparent_inputs.iter().map(|i| i.serialized_size()), + self.transparent_builder + .outputs() + .iter() + .map(|i| i.serialized_size()), sapling_spends, self.sapling_builder .as_ref() diff --git a/zcash_primitives/src/transaction/components/transparent.rs b/zcash_primitives/src/transaction/components/transparent.rs index b7049fb3e9..e2e730d266 100644 --- a/zcash_primitives/src/transaction/components/transparent.rs +++ b/zcash_primitives/src/transaction/components/transparent.rs @@ -96,10 +96,21 @@ pub struct OutPoint { } impl OutPoint { + /// Constructs an `OutPoint` for the output at index `n` in the transaction + /// with txid `hash`. pub fn new(hash: [u8; 32], n: u32) -> Self { OutPoint { hash, n } } + /// Constructs a fake `OutPoint` for use in tests. + #[cfg(any(test, feature = "test-dependencies"))] + pub const fn fake() -> Self { + OutPoint { + hash: [1u8; 32], + n: 1, + } + } + pub fn read(mut reader: R) -> io::Result { let mut hash = [0u8; 32]; reader.read_exact(&mut hash)?; @@ -112,18 +123,20 @@ impl OutPoint { writer.write_u32::(self.n) } - /// Returns `true` if this `OutPoint` is "null" in the Bitcoin sense: it points to the - /// `u32::MAX`th output of the transaction with the all-zeroes txid. + /// Returns `true` if this `OutPoint` is "null" in the Bitcoin sense: it has txid set to + /// all-zeroes and output index set to `u32::MAX`. fn is_null(&self) -> bool { // From `BaseOutPoint::IsNull()` in zcashd: // return (hash.IsNull() && n == (uint32_t) -1); self.hash == [0; 32] && self.n == u32::MAX } + /// Returns the output index of this `OutPoint`. pub fn n(&self) -> u32 { self.n } + /// Returns the txid of the transaction containing this `OutPoint`. pub fn hash(&self) -> &[u8; 32] { &self.hash } diff --git a/zcash_primitives/src/transaction/fees.rs b/zcash_primitives/src/transaction/fees.rs index 0dc82acf2b..ae996de6ce 100644 --- a/zcash_primitives/src/transaction/fees.rs +++ b/zcash_primitives/src/transaction/fees.rs @@ -2,7 +2,7 @@ use crate::{ consensus::{self, BlockHeight}, - transaction::components::amount::NonNegativeAmount, + transaction::{components::amount::NonNegativeAmount, fees::transparent::InputSize}, }; pub mod fixed; @@ -27,8 +27,8 @@ pub trait FeeRule { &self, params: &P, target_height: BlockHeight, - transparent_inputs: &[impl transparent::InputView], - transparent_outputs: &[impl transparent::OutputView], + transparent_input_sizes: impl IntoIterator, + transparent_output_sizes: impl IntoIterator, sapling_input_count: usize, sapling_output_count: usize, orchard_action_count: usize, @@ -49,8 +49,8 @@ pub trait FutureFeeRule: FeeRule { &self, params: &P, target_height: BlockHeight, - transparent_inputs: &[impl transparent::InputView], - transparent_outputs: &[impl transparent::OutputView], + transparent_input_sizes: impl IntoIterator, + transparent_output_sizes: impl IntoIterator, sapling_input_count: usize, sapling_output_count: usize, orchard_action_count: usize, @@ -80,8 +80,8 @@ impl FeeRule for StandardFeeRule { &self, params: &P, target_height: BlockHeight, - transparent_inputs: &[impl transparent::InputView], - transparent_outputs: &[impl transparent::OutputView], + transparent_input_sizes: impl IntoIterator, + transparent_output_sizes: impl IntoIterator, sapling_input_count: usize, sapling_output_count: usize, orchard_action_count: usize, @@ -93,8 +93,8 @@ impl FeeRule for StandardFeeRule { Self::Zip317 => zip317::FeeRule::standard().fee_required( params, target_height, - transparent_inputs, - transparent_outputs, + transparent_input_sizes, + transparent_output_sizes, sapling_input_count, sapling_output_count, orchard_action_count, diff --git a/zcash_primitives/src/transaction/fees/fixed.rs b/zcash_primitives/src/transaction/fees/fixed.rs index 1bb1b38527..ade5a69514 100644 --- a/zcash_primitives/src/transaction/fees/fixed.rs +++ b/zcash_primitives/src/transaction/fees/fixed.rs @@ -52,8 +52,8 @@ impl super::FeeRule for FeeRule { &self, _params: &P, _target_height: BlockHeight, - _transparent_inputs: &[impl transparent::InputView], - _transparent_outputs: &[impl transparent::OutputView], + _transparent_input_sizes: impl IntoIterator, + _transparent_output_sizes: impl IntoIterator, _sapling_input_count: usize, _sapling_output_count: usize, _orchard_action_count: usize, @@ -68,8 +68,8 @@ impl super::FutureFeeRule for FeeRule { &self, _params: &P, _target_height: BlockHeight, - _transparent_inputs: &[impl transparent::InputView], - _transparent_outputs: &[impl transparent::OutputView], + _transparent_input_sizes: impl IntoIterator, + _transparent_output_sizes: impl IntoIterator, _sapling_input_count: usize, _sapling_output_count: usize, _orchard_action_count: usize, diff --git a/zcash_primitives/src/transaction/fees/transparent.rs b/zcash_primitives/src/transaction/fees/transparent.rs index e5c5916040..8beb394b54 100644 --- a/zcash_primitives/src/transaction/fees/transparent.rs +++ b/zcash_primitives/src/transaction/fees/transparent.rs @@ -4,20 +4,47 @@ use std::convert::Infallible; use crate::{ - legacy::Script, - transaction::components::{amount::NonNegativeAmount, transparent::TxOut, OutPoint}, + legacy::{Script, TransparentAddress}, + transaction::{ + components::{amount::NonNegativeAmount, transparent::TxOut, OutPoint}, + fees::zip317::P2PKH_STANDARD_INPUT_SIZE, + }, }; #[cfg(feature = "transparent-inputs")] use crate::transaction::components::transparent::builder::TransparentInputInfo; +/// The size of a transparent input, or the outpoint corresponding to the input +/// if the size of the script required to spend that input is unknown. +pub enum InputSize { + /// The txin size is known. + Known(usize), + /// The size of the script required to spend this input (and therefore the txin size) + /// is unknown. + Unknown(OutPoint), +} + +impl InputSize { + /// An `InputSize` corresponding to the upper bound on the size of a P2PKH input used by ZIP 317. + pub const STANDARD_P2PKH: InputSize = InputSize::Known(P2PKH_STANDARD_INPUT_SIZE); +} + /// This trait provides a minimized view of a transparent input suitable for use in /// fee and change computation. pub trait InputView: std::fmt::Debug { /// The outpoint to which the input refers. fn outpoint(&self) -> &OutPoint; + /// The previous output being spent. fn coin(&self) -> &TxOut; + + /// The size of the transparent script required to spend this input. + fn serialized_size(&self) -> InputSize { + match self.coin().script_pubkey.address() { + Some(TransparentAddress::PublicKeyHash(_)) => InputSize::STANDARD_P2PKH, + _ => InputSize::Unknown(self.outpoint().clone()), + } + } } #[cfg(feature = "transparent-inputs")] @@ -45,8 +72,16 @@ impl InputView for Infallible { pub trait OutputView: std::fmt::Debug { /// Returns the value of the output being created. fn value(&self) -> NonNegativeAmount; + /// Returns the script corresponding to the newly created output. fn script_pubkey(&self) -> &Script; + + /// Returns the serialized size of the txout. + fn serialized_size(&self) -> usize { + // The serialized size of a transparent `TxOut` is the serialized size of an amount + // plus the serialized size of the script pubkey. + 8 + self.script_pubkey().serialized_size() + } } impl OutputView for TxOut { diff --git a/zcash_primitives/src/transaction/fees/zip317.rs b/zcash_primitives/src/transaction/fees/zip317.rs index ee25c8fa38..1d5d7513ac 100644 --- a/zcash_primitives/src/transaction/fees/zip317.rs +++ b/zcash_primitives/src/transaction/fees/zip317.rs @@ -6,7 +6,6 @@ use core::cmp::max; use crate::{ consensus::{self, BlockHeight}, - legacy::TransparentAddress, transaction::{ components::{ amount::{BalanceError, NonNegativeAmount}, @@ -45,8 +44,12 @@ pub const MINIMUM_FEE: NonNegativeAmount = NonNegativeAmount::const_from_u64(10_ /// A [`FeeRule`] implementation that implements the [ZIP 317] fee rule. /// /// This fee rule supports Orchard, Sapling, and (P2PKH only) transparent inputs. -/// Returns an error if a coin containing a non-p2pkh script is provided as an input. -/// This fee rule may slightly overestimate fees in case where the user is attempting to spend more than ~150 transparent inputs. +/// Returns an error if a coin containing a non-P2PKH script is provided as an input. +/// +/// This fee rule may slightly overestimate fees in case where the user is attempting +/// to spend a large number of transparent inputs. This is intentional and is relied +/// on for the correctness of transaction construction algorithms in the +/// `zcash_client_backend` crate. /// /// [`FeeRule`]: crate::transaction::fees::FeeRule /// [ZIP 317]: https//zips.z.cash/zip-0317 @@ -147,27 +150,30 @@ impl super::FeeRule for FeeRule { &self, _params: &P, _target_height: BlockHeight, - transparent_inputs: &[impl transparent::InputView], - transparent_outputs: &[impl transparent::OutputView], + transparent_input_sizes: impl IntoIterator, + transparent_output_sizes: impl IntoIterator, sapling_input_count: usize, sapling_output_count: usize, orchard_action_count: usize, ) -> Result { - let non_p2pkh_inputs: Vec<_> = transparent_inputs - .iter() - .filter_map(|t_in| match t_in.coin().script_pubkey.address() { - Some(TransparentAddress::PublicKeyHash(_)) => None, - _ => Some(t_in.outpoint()), - }) - .cloned() - .collect(); + let mut t_in_total_size: usize = 0; + let mut non_p2pkh_outpoints = vec![]; + for sz in transparent_input_sizes.into_iter() { + match sz { + transparent::InputSize::Known(s) => { + t_in_total_size += s; + } + transparent::InputSize::Unknown(outpoint) => { + non_p2pkh_outpoints.push(outpoint.clone()); + } + } + } - if !non_p2pkh_inputs.is_empty() { - return Err(FeeError::NonP2pkhInputs(non_p2pkh_inputs)); + if !non_p2pkh_outpoints.is_empty() { + return Err(FeeError::NonP2pkhInputs(non_p2pkh_outpoints)); } - let t_in_total_size = transparent_inputs.len() * 150; - let t_out_total_size = transparent_outputs.len() * 34; + let t_out_total_size = transparent_output_sizes.into_iter().sum(); let ceildiv = |num: usize, den: usize| (num + den - 1) / den;