diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7c5b19843..b29f63126 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -75,6 +75,9 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail not enabled). - `zcash_client_backend::input_selection::GreedyInputSelectorError` has a new variant `UnsupportedTexAddress`. +- `zcash_client_backend::proposal::ProposalError` has new variants + `SpendsChange`, `EphemeralOutputLeftUnspent`, and `PaysTexFromShielded`. + (the last two are conditional on the "transparent-inputs" feature). - `zcash_client_backend::proto::ProposalDecodingError` has a new variant `InvalidEphemeralRecipient`. - `zcash_client_backend::wallet::Recipient` variants have changed. Instead of diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 1edc2b0ff..bb3b2d589 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -922,7 +922,7 @@ pub trait WalletRead { } /// Returns the metadata associated with a given transparent receiver in an account - /// controlled by this wallet. + /// controlled by this wallet, if available. /// /// This is equivalent to (but may be implemented more efficiently than): /// ```compile_fail @@ -956,13 +956,19 @@ pub trait WalletRead { Ok(None) } - /// Returns the set of reserved ephemeral transparent addresses associated with the - /// given account controlled by this wallet. + /// Returns a set of ephemeral transparent addresses associated with the given + /// account controlled by this wallet, along with their metadata. /// - /// The set contains all ephemeral transparent receivers that are known to have - /// been derived under this account. Wallets should scan the chain for UTXOs sent to - /// these receivers, but do not need to do so regularly. Under expected usage, outputs - /// would only be detected with these receivers in the following situations: + /// If `for_detection` is false, the set only includes addresses reserved by + /// `reserve_next_n_ephemeral_addresses`. If `for_detection` is true, it includes + /// those addresses and also the ones that will be reserved next, for an additional + /// `GAP_LIMIT` indices (up to and including the maximum index given by + /// `NonHardenedChildIndex::from_index(i32::MAX as u32)`). + /// + /// Wallets should scan the chain for UTXOs sent to the ephemeral transparent + /// receivers obtained with `for_detection` set to `true`, but do not need to do + /// so regularly. Under expected usage, outputs would only be detected with these + /// receivers in the following situations: /// /// - This wallet created a payment to a ZIP 320 (TEX) address, but the second /// transaction (that spent the output sent to the ephemeral address) did not get @@ -1542,6 +1548,8 @@ pub trait WalletWrite: WalletRead { /// funds have been received by the currently-available account (in order to enable automated /// account recovery). /// + /// # Panics + /// /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. /// /// [ZIP 316]: https://zips.z.cash/zip-0316 @@ -1634,7 +1642,9 @@ pub trait WalletWrite: WalletRead { /// the given number of addresses, or if the account identifier does not correspond /// to a known account. /// - /// Precondition: `n < 0x80000000` + /// # Panics + /// + /// Panics if the precondition `n < 0x80000000` does not hold. /// /// [BIP 44]: https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#user-content-Address_gap_limit #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 439e8a2b1..2712608a9 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -680,6 +680,7 @@ where // TODO: Maybe support spending prior shielded outputs at some point? Doing so would require // a higher-level approach in the wallet that waits for transactions with shielded outputs to // be mined and only then attempts to perform the next step. + #[allow(clippy::never_loop)] for input_ref in proposal_step.prior_step_inputs() { let (prior_step, _) = prior_step_results .get(input_ref.step_index()) @@ -1251,13 +1252,18 @@ where #[allow(unused_variables)] let transparent_outputs = transparent_output_meta.into_iter().enumerate().map( - |(n, (recipient, ephemeral_address, value, step_output_index))| { + |(n, (recipient, address, value, step_output_index))| { + // This assumes that transparent outputs are pushed onto `transparent_output_meta` + // with the same indices they have in the transaction's transparent outputs. + // We do not reorder transparent outputs; there is no reason to do so because it + // would not usefully improve privacy. let outpoint = OutPoint::new(txid, n as u32); + let recipient = recipient.map_ephemeral_transparent_outpoint(|()| outpoint.clone()); #[cfg(feature = "transparent-inputs")] unused_transparent_outputs.insert( StepOutput::new(step_index, step_output_index), - (ephemeral_address, outpoint), + (address, outpoint), ); SentTransactionOutput::from_parts(n, recipient, value, None) }, diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index d0b3eebca..560d8b4e6 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -521,10 +521,17 @@ pub(crate) fn check_for_uneconomic_inputs( // Return the number of allowed dust inputs from each pool. let allowed_dust = |(t_change, s_change, o_change): &(usize, usize, usize)| { - // What is the maximum number of dust inputs in each pool, out of the ones we - // actually have, that can be economically spent along with the non-dust inputs? - // Get an initial estimate by calculating the number of dust inputs in each pool - // that will be allowed regardless of padding or grace. + // Here we assume a "ZIP 317-like" fee model in which the existence of an output + // to a given pool implies that a corresponding input from that pool can be + // provided without increasing the fee. (This is also likely to be true for + // future fee models if we do not want to penalize use of Orchard relative to + // other pools.) + // + // Under that assumption, we want to calculate the maximum number of dust inputs + // from each pool, out of the ones we actually have, that can be economically + // spent along with the non-dust inputs. Get an initial estimate by calculating + // the number of dust inputs in each pool that will be allowed regardless of + // padding or grace. let t_allowed = min( t_dust.len(), @@ -545,10 +552,12 @@ pub(crate) fn check_for_uneconomic_inputs( #[cfg(feature = "orchard")] let o_req_inputs = o_non_dust + o_allowed; - // This calculates the hypothetical number of actions with given extra inputs. - // The padding rules are subtle (they also depend on `bundle_required` for example). - // To reliably tell whether we can add an extra input of a given type, we will need - // to call them both with without that input; if the number of actions does not + // This calculates the hypothetical number of actions with given extra inputs, + // for ZIP 317 and the padding rules in effect. The padding rules for each + // pool are subtle (they also depend on `bundle_required` for example), so we + // must actually call them rather than try to predict their effect. To tell + // whether we can freely add an extra input from a given pool, we need to call + // them both with and without that input; if the number of actions does not // increase, then the input is free to add. let hypothetical_actions = |t_extra, s_extra, _o_extra| { let s_spend_count = sapling diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 878320614..1bc4dae3f 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -647,6 +647,8 @@ pub struct TransparentAddressMetadata { #[cfg(feature = "transparent-inputs")] impl TransparentAddressMetadata { + /// Returns a `TransparentAddressMetadata` in the given scope for the + /// given address index. pub fn new(scope: TransparentKeyScope, address_index: NonHardenedChildIndex) -> Self { Self { scope, diff --git a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs index b79532eb1..445275c76 100644 --- a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs +++ b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs @@ -25,23 +25,10 @@ use crate::{error::SqliteClientError, wallet::get_account, AccountId, SqlTransac /// of them to be mined. This is the same as the gap limit in Bitcoin. pub(crate) const GAP_LIMIT: i32 = 20; -// The custom scope used for derivation of ephemeral addresses. -// -// This must match the constant used in -// `zcash_primitives::legacy::keys::AccountPubKey::derive_ephemeral_ivk`. -// -// TODO: consider moving this to `zcash_primitives::legacy::keys`, or else -// provide a way to derive `ivk`s for custom scopes in general there, so that -// the constant isn't duplicated. -const EPHEMERAL_SCOPE: TransparentKeyScope = match TransparentKeyScope::custom(2) { - Some(s) => s, - None => unreachable!(), -}; - // Returns `TransparentAddressMetadata` in the ephemeral scope for the // given address index. pub(crate) fn metadata(address_index: NonHardenedChildIndex) -> TransparentAddressMetadata { - TransparentAddressMetadata::new(EPHEMERAL_SCOPE, address_index) + TransparentAddressMetadata::new(TransparentKeyScope::EPHEMERAL, address_index) } /// Returns the last reserved ephemeral address index in the given account, @@ -61,9 +48,7 @@ pub(crate) fn last_reserved_index( ) .optional()? { - Some(i) if i < 0 => Err(SqliteClientError::CorruptedData( - "negative index".to_owned(), - )), + Some(i) if i < 0 => unreachable!("violates constraint address_index_in_range"), Some(i) => Ok(i), None => Ok(-1), } @@ -92,12 +77,10 @@ pub(crate) fn last_safe_index( ) .optional()? { - Some(i) if i < 0 => Err(SqliteClientError::CorruptedData( - "negative index".to_owned(), - )), - Some(i) => Ok(i), - None => Ok(-1), - }?; + Some(i) if i < 0 => unreachable!("violates constraint address_index_in_range"), + Some(i) => i, + None => -1, + }; Ok(u32::try_from(last_mined_index.saturating_add(GAP_LIMIT)).unwrap()) } @@ -105,7 +88,9 @@ pub(crate) fn last_safe_index( /// and is of length up to `n`. The range is truncated if necessary to end at /// the maximum valid address index, `i32::MAX`. /// -/// Precondition: `i >= -1 and n > 0` +/// # Panics +/// +/// Panics if the precondition `i >= -1 and n > 0` does not hold. pub(crate) fn range_after(i: i32, n: i32) -> RangeInclusive { assert!(i >= -1); assert!(n > 0); @@ -128,8 +113,12 @@ pub(crate) fn get_ephemeral_ivk( .derive_ephemeral_ivk()?) } -/// Returns a vector with all ephemeral transparent addresses potentially belonging to this wallet. -/// If `for_detection` is true, this includes addresses for an additional GAP_LIMIT indices. +/// Returns a mapping of ephemeral transparent addresses potentially belonging +/// to this wallet to their metadata. +/// +/// If `for_detection` is false, the result only includes reserved addresses. +/// If `for_detection` is true, it includes addresses for an additional +/// `GAP_LIMIT` indices, up to the limit. pub(crate) fn get_reserved_ephemeral_addresses( conn: &rusqlite::Connection, params: &P, @@ -172,7 +161,9 @@ pub(crate) fn get_reserved_ephemeral_addresses( /// Returns a vector with the next `n` previously unreserved ephemeral addresses for /// the given account. /// -/// Precondition: `n < 0x80000000` +/// # Panics +/// +/// Panics if the precondition `n < 0x80000000` does not hold. /// /// # Errors /// diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 6773d516e..ddd5850c7 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -14,6 +14,7 @@ and this library adheres to Rust's notion of - `EphemeralIvk` - `AccountPubKey::derive_ephemeral_ivk` - `TransparentKeyScope::custom` is now `const`. + - `TransparentKeyScope::{EXTERNAL, INTERNAL, EPHEMERAL}` - `zcash_primitives::legacy::Script::serialized_size` - `zcash_primitives::transaction::fees::transparent`: - `InputSize` diff --git a/zcash_primitives/src/legacy/keys.rs b/zcash_primitives/src/legacy/keys.rs index e969cd733..60131671f 100644 --- a/zcash_primitives/src/legacy/keys.rs +++ b/zcash_primitives/src/legacy/keys.rs @@ -21,6 +21,10 @@ use super::TransparentAddress; pub struct TransparentKeyScope(u32); impl TransparentKeyScope { + /// Returns an arbitrary custom `TransparentKeyScope`. This should be used + /// with care: funds associated with keys derived under a custom scope may + /// not be recoverable if the wallet seed is restored in another wallet. + /// It is usually preferable to use standardized key scopes. pub const fn custom(i: u32) -> Option { if i < (1 << 31) { Some(TransparentKeyScope(i)) @@ -28,13 +32,24 @@ impl TransparentKeyScope { None } } + + /// The scope used to derive keys for external transparent addresses, + /// intended to be used to send funds to this wallet. + pub const EXTERNAL: Self = TransparentKeyScope(0); + + /// The scope used to derive keys for internal wallet operations, e.g. + /// change or UTXO management. + pub const INTERNAL: Self = TransparentKeyScope(1); + + /// The scope used to derive keys for ephemeral transparent addresses. + pub const EPHEMERAL: Self = TransparentKeyScope(2); } impl From for TransparentKeyScope { fn from(value: zip32::Scope) -> Self { match value { - zip32::Scope::External => TransparentKeyScope(0), - zip32::Scope::Internal => TransparentKeyScope(1), + zip32::Scope::External => TransparentKeyScope::EXTERNAL, + zip32::Scope::Internal => TransparentKeyScope::INTERNAL, } } }