diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 21f90ef80c..ef20ac7608 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -54,6 +54,13 @@ and this library adheres to Rust's notion of - `KeyError` - `AddressCodec` implementations for `sapling::PaymentAddress` and `UnifiedAddress` +- `zcash_client_backend::fees` + - `ChangeError` + - `ChangeStrategy` + - `ChangeValue` + - `TransactionBalance` + - `BasicFixedFeeChangeStrategy` - a `ChangeStrategy` implementation that + reproduces current wallet change behavior - New experimental APIs that should be considered unstable, and are likely to be modified and/or moved to a different module in a future release: @@ -105,6 +112,8 @@ and this library adheres to Rust's notion of - `data_api::ReceivedTransaction` has been renamed to `DecryptedTransaction`, and its `outputs` field has been renamed to `sapling_outputs`. - `data_api::error::Error` has the following additional cases: + - `Error::BalanceError` in the case of amount addition overflow + or subtraction underflow. - `Error::MemoForbidden` to report the condition where a memo was specified to be sent to a transparent recipient. - `Error::TransparentInputsNotSupported` to represent the condition diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 55913fb3ee..d4640147a8 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -6,7 +6,11 @@ use zcash_address::unified::Typecode; use zcash_primitives::{ consensus::BlockHeight, sapling::Node, - transaction::{builder, components::amount::Amount, TxId}, + transaction::{ + builder, + components::amount::{Amount, BalanceError}, + TxId, + }, zip32::AccountId, }; @@ -33,8 +37,8 @@ pub enum Error { /// No account with the given identifier was found in the wallet. AccountNotFound(AccountId), - /// The amount specified exceeds the allowed range. - InvalidAmount, + /// Zcash amount computation encountered an overflow or underflow. + BalanceError(BalanceError), /// Unable to create a new spend because the wallet balance is not sufficient. /// The first argument is the amount available, the second is the amount needed @@ -113,9 +117,9 @@ impl fmt::Display for Error { Error::AccountNotFound(account) => { write!(f, "Wallet does not contain account {}", u32::from(*account)) } - Error::InvalidAmount => write!( + Error::BalanceError(e) => write!( f, - "The value lies outside the valid range of Zcash amounts." + "The value lies outside the valid range of Zcash amounts: {:?}.", e ), Error::InsufficientBalance(have, need) => write!( f, diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 24113cef2b..e765c985c6 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -5,9 +5,10 @@ use zcash_primitives::{ sapling::prover::TxProver, transaction::{ builder::Builder, - components::{amount::DEFAULT_FEE, Amount}, + components::amount::{Amount, BalanceError, DEFAULT_FEE}, Transaction, }, + zip32::Scope, }; use crate::{ @@ -17,6 +18,7 @@ use crate::{ SentTransactionOutput, WalletWrite, }, decrypt_transaction, + fees::{BasicFixedFeeChangeStrategy, ChangeError, ChangeStrategy, ChangeValue}, keys::UnifiedSpendingKey, wallet::OvkPolicy, zip321::{Payment, TransactionRequest}, @@ -92,19 +94,18 @@ where /// * `wallet_db`: A read/write reference to the wallet database /// * `params`: Consensus parameters /// * `prover`: The TxProver to use in constructing the shielded transaction. -/// * `account`: The ZIP32 account identifier associated with the extended spending -/// key that controls the funds to be used in creating this transaction. This -/// procedure will return an error if this does not correctly correspond to `extsk`. -/// * `extsk`: The extended spending key that controls the funds that will be spent -/// in the resulting transaction. -/// * `amount`: The amount to send. +/// * `usk`: The unified spending key that controls the funds that will be spent +/// in the resulting transaction. This procedure will return an error if the +/// USK does not correspond to an account known to the wallet. /// * `to`: The address to which `amount` will be paid. +/// * `amount`: The amount to send. /// * `memo`: A memo to be included in the output to the recipient. /// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that /// can allow the sender to view the resulting notes on the blockchain. /// * `min_confirmations`: The minimum number of confirmations that a previously /// received note must have in the blockchain in order to be considered for being /// spent. A value of 10 confirmations is recommended. +/// /// # Examples /// /// ``` @@ -236,10 +237,8 @@ where /// * `params`: Consensus parameters /// * `prover`: The TxProver to use in constructing the shielded transaction. /// * `usk`: The unified spending key that controls the funds that will be spent -/// in the resulting transaction. -/// * `account`: The ZIP32 account identifier associated with the extended spending -/// key that controls the funds to be used in creating this transaction. This -/// procedure will return an error if this does not correctly correspond to `extsk`. +/// in the resulting transaction. This procedure will return an error if the +/// USK does not correspond to an account known to the wallet. /// * `request`: The ZIP-321 payment request specifying the recipients and amounts /// for the transaction. /// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that @@ -267,17 +266,17 @@ where .get_account_for_ufvk(&usk.to_unified_full_viewing_key())? .ok_or(Error::KeyNotRecognized)?; - let extfvk = usk.sapling().to_extended_full_viewing_key(); + let dfvk = usk.sapling().to_diversifiable_full_viewing_key(); // Apply the outgoing viewing key policy. let ovk = match ovk_policy { - OvkPolicy::Sender => Some(extfvk.fvk.ovk), + OvkPolicy::Sender => Some(dfvk.fvk().ovk), OvkPolicy::Custom(ovk) => Some(ovk), OvkPolicy::Discard => None, }; // Target the next block, assuming we are up-to-date. - let (height, anchor_height) = wallet_db + let (target_height, anchor_height) = wallet_db .get_target_and_anchor_heights(min_confirmations) .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?; @@ -286,8 +285,9 @@ where .iter() .map(|p| p.amount) .sum::>() - .ok_or_else(|| E::from(Error::InvalidAmount))?; - let target_value = (value + DEFAULT_FEE).ok_or_else(|| E::from(Error::InvalidAmount))?; + .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; + let target_value = (value + DEFAULT_FEE) + .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; let spendable_notes = wallet_db.select_spendable_sapling_notes(account, target_value, anchor_height)?; @@ -296,7 +296,7 @@ where .iter() .map(|n| n.note_value) .sum::>() - .ok_or_else(|| E::from(Error::InvalidAmount))?; + .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; if selected_value < target_value { return Err(E::from(Error::InsufficientBalance( selected_value, @@ -305,10 +305,10 @@ where } // Create the transaction - let mut builder = Builder::new_with_fee(params.clone(), height, DEFAULT_FEE); + let mut builder = Builder::new(params.clone(), target_height); for selected in spendable_notes { - let from = extfvk - .fvk + let from = dfvk + .fvk() .vk .to_payment_address(selected.diversifier) .unwrap(); //DiversifyHash would have to unexpectedly return the zero point for this to be None @@ -361,7 +361,42 @@ where }? } - let (tx, tx_metadata) = builder.build(&prover).map_err(Error::Builder)?; + let fee_strategy = BasicFixedFeeChangeStrategy::new(DEFAULT_FEE); + let balance = fee_strategy + .compute_balance( + params, + target_height, + builder.transparent_inputs(), + builder.transparent_outputs(), + builder.sapling_inputs(), + builder.sapling_outputs(), + ) + .map_err(|e| match e { + ChangeError::InsufficientFunds { + available, + required, + } => Error::InsufficientBalance(available, required), + ChangeError::StrategyError(e) => Error::BalanceError(e), + })?; + + for change_value in balance.proposed_change() { + match change_value { + ChangeValue::Sapling(amount) => { + builder + .add_sapling_output( + Some(dfvk.to_ovk(Scope::Internal)), + dfvk.change_address().1, + *amount, + MemoBytes::empty(), + ) + .map_err(Error::Builder)?; + } + } + } + + let (tx, tx_metadata) = builder + .build(&prover, &fee_strategy.fee_rule()) + .map_err(Error::Builder)?; let sent_outputs = request.payments().iter().enumerate().map(|(i, payment)| { let (output_index, recipient) = match &payment.recipient_address { @@ -405,7 +440,7 @@ where created: time::OffsetDateTime::now_utc(), account, outputs: sent_outputs, - fee_amount: DEFAULT_FEE, + fee_amount: balance.fee_required(), #[cfg(feature = "transparent-inputs")] utxos_spent: vec![], }) @@ -423,12 +458,12 @@ where /// * `wallet_db`: A read/write reference to the wallet database /// * `params`: Consensus parameters /// * `prover`: The TxProver to use in constructing the shielded transaction. -/// * `sk`: The secp256k1 secret key that will be used to detect and spend transparent -/// UTXOs. -/// * `account`: The ZIP32 account identifier for the account to which funds will -/// be shielded. Funds will be shielded to the internal (change) address associated with the -/// most preferred shielded receiver corresponding to this account, or if no shielded -/// receiver can be used for this account, this function will return an error. +/// * `usk`: The unified spending key that will be used to detect and spend transparent UTXOs, +/// and that will provide the shielded address to which funds will be sent. Funds will be +/// shielded to the internal (change) address associated with the most preferred shielded +/// receiver corresponding to this account, or if no shielded receiver can be used for this +/// account, this function will return an error. This procedure will return an error if the +/// USK does not correspond to an account known to the wallet. /// * `memo`: A memo to be included in the output to the (internal) recipient. /// This can be used to take notes about auto-shielding operations internal /// to the wallet that the wallet can use to improve how it represents those @@ -462,7 +497,7 @@ where .to_diversifiable_full_viewing_key() .change_address() .1; - let (latest_scanned_height, latest_anchor) = wallet_db + let (target_height, latest_anchor) = wallet_db .get_target_and_anchor_heights(min_confirmations) .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?; @@ -476,21 +511,15 @@ where utxos.append(&mut outputs); } - let total_amount = utxos + let _total_amount = utxos .iter() .map(|utxo| utxo.txout().value) .sum::>() - .ok_or_else(|| E::from(Error::InvalidAmount))?; - - let fee = DEFAULT_FEE; - if fee >= total_amount { - return Err(E::from(Error::InsufficientBalance(total_amount, fee))); - } - - let amount_to_shield = (total_amount - fee).ok_or_else(|| E::from(Error::InvalidAmount))?; + .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; let addr_metadata = wallet_db.get_transparent_receivers(account)?; - let mut builder = Builder::new_with_fee(params.clone(), latest_scanned_height, fee); + let mut builder = Builder::new(params.clone(), target_height); + for utxo in &utxos { let diversifier_index = addr_metadata .get(utxo.recipient_address()) @@ -510,15 +539,44 @@ where .map_err(Error::Builder)?; } - // there are no sapling notes so we set the change manually - builder.send_change_to(ovk, shielding_address.clone()); + // Compute the balance of the transaction. We have only added inputs, so the total change + // amount required will be the total of the UTXOs minus fees. + let fee_strategy = BasicFixedFeeChangeStrategy::new(DEFAULT_FEE); + let balance = fee_strategy + .compute_balance( + params, + target_height, + builder.transparent_inputs(), + builder.transparent_outputs(), + builder.sapling_inputs(), + builder.sapling_outputs(), + ) + .map_err(|e| match e { + ChangeError::InsufficientFunds { + available, + required, + } => Error::InsufficientBalance(available, required), + ChangeError::StrategyError(e) => Error::BalanceError(e), + })?; + + let fee = balance.fee_required(); + let mut total_out = Amount::zero(); + for change_value in balance.proposed_change() { + total_out = (total_out + change_value.value()) + .ok_or(Error::BalanceError(BalanceError::Overflow))?; + match change_value { + ChangeValue::Sapling(amount) => { + builder + .add_sapling_output(Some(ovk), shielding_address.clone(), *amount, memo.clone()) + .map_err(Error::Builder)?; + } + } + } - // add the sapling output to shield the funds - builder - .add_sapling_output(Some(ovk), shielding_address, amount_to_shield, memo.clone()) + // The transaction build process will check that the inputs and outputs balance + let (tx, tx_metadata) = builder + .build(&prover, &fee_strategy.fee_rule()) .map_err(Error::Builder)?; - - let (tx, tx_metadata) = builder.build(&prover).map_err(Error::Builder)?; let output_index = tx_metadata.output_index(0).expect( "No sapling note was created in autoshielding transaction. This is a programming error.", ); @@ -529,8 +587,8 @@ where account, outputs: vec![SentTransactionOutput { output_index, - value: amount_to_shield, recipient: Recipient::InternalAccount(account, PoolType::Sapling), + value: total_out, memo: Some(memo.clone()), }], fee_amount: fee, diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs new file mode 100644 index 0000000000..9b13d2e247 --- /dev/null +++ b/zcash_client_backend/src/fees.rs @@ -0,0 +1,163 @@ +use zcash_primitives::{ + consensus::{self, BlockHeight}, + transaction::{ + components::{ + amount::{Amount, BalanceError}, + sapling::fees as sapling, + transparent::fees as transparent, + }, + fees::{FeeRule, FixedFeeRule}, + }, +}; + +/// A proposed change amount and output pool. +pub enum ChangeValue { + Sapling(Amount), +} + +impl ChangeValue { + pub fn value(&self) -> Amount { + match self { + ChangeValue::Sapling(value) => *value, + } + } +} + +/// The amount of change and fees required to make a transaction's inputs and +/// outputs balance under a specific fee rule, as computed by a particular +/// [`ChangeStrategy`] that is aware of that rule. +pub struct TransactionBalance { + proposed_change: Vec, + fee_required: Amount, +} + +impl TransactionBalance { + /// Constructs a new balance from its constituent parts. + pub fn new(proposed_change: Vec, fee_required: Amount) -> Self { + TransactionBalance { + proposed_change, + fee_required, + } + } + + /// The change values proposed by the [`ChangeStrategy`] that computed this balance. + pub fn proposed_change(&self) -> &[ChangeValue] { + &self.proposed_change + } + + /// Returns the fee computed for the transaction, assuming that the suggested + /// change outputs are added to the transaction. + pub fn fee_required(&self) -> Amount { + self.fee_required + } +} + +/// Errors that can occur in computing suggested change and/or fees. +pub enum ChangeError { + InsufficientFunds { available: Amount, required: Amount }, + StrategyError(E), +} + +/// A trait that represents the ability to compute the suggested change and fees that must be paid +/// by a transaction having a specified set of inputs and outputs. +pub trait ChangeStrategy { + type FeeRule: FeeRule; + type Error; + + /// Returns the fee rule that this change strategy will respect when performing + /// balance computations. + fn fee_rule(&self) -> Self::FeeRule; + + /// Computes the totals of inputs, suggested change amounts, and fees given the + /// provided inputs and outputs being used to construct a transaction. + /// + /// The fee computed as part of this operation should take into account the prospective + /// change outputs recommended by this operation. If insufficient funds are available to + /// supply the requested outputs and required fees, implementations should return + /// [`ChangeError::InsufficientFunds`]. + fn compute_balance( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + ) -> Result>; +} + +/// A change strategy that uses a fixed fee amount and proposes change as a single output +/// to the most current supported pool. +pub struct BasicFixedFeeChangeStrategy { + fixed_fee: Amount, +} + +impl BasicFixedFeeChangeStrategy { + // Constructs a new [`BasicFixedFeeChangeStrategy`] with the specified fixed fee + // amount. + pub fn new(fixed_fee: Amount) -> Self { + Self { fixed_fee } + } +} + +impl ChangeStrategy for BasicFixedFeeChangeStrategy { + type FeeRule = FixedFeeRule; + type Error = BalanceError; + + fn fee_rule(&self) -> Self::FeeRule { + FixedFeeRule::new(self.fixed_fee) + } + + fn compute_balance( + &self, + _params: &P, + _target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + ) -> Result> { + let overflow = || ChangeError::StrategyError(BalanceError::Overflow); + let underflow = || ChangeError::StrategyError(BalanceError::Underflow); + + let t_in = transparent_inputs + .iter() + .map(|t_in| t_in.coin().value) + .sum::>() + .ok_or_else(overflow)?; + let t_out = transparent_outputs + .iter() + .map(|t_out| t_out.value()) + .sum::>() + .ok_or_else(overflow)?; + let sapling_in = sapling_inputs + .iter() + .map(|s_in| s_in.value()) + .sum::>() + .ok_or_else(overflow)?; + let sapling_out = sapling_outputs + .iter() + .map(|s_out| s_out.value()) + .sum::>() + .ok_or_else(overflow)?; + + let total_in = (t_in + sapling_in).ok_or_else(overflow)?; + let total_out = [t_out, sapling_out, self.fixed_fee] + .iter() + .sum::>() + .ok_or_else(overflow)?; + + let proposed_change = (total_in - total_out).ok_or_else(underflow)?; + if proposed_change < Amount::zero() { + Err(ChangeError::InsufficientFunds { + available: total_in, + required: total_out, + }) + } else { + Ok(TransactionBalance::new( + vec![ChangeValue::Sapling(proposed_change)], + self.fixed_fee, + )) + } + } +} diff --git a/zcash_client_backend/src/lib.rs b/zcash_client_backend/src/lib.rs index 1eb5163c79..931cf99a80 100644 --- a/zcash_client_backend/src/lib.rs +++ b/zcash_client_backend/src/lib.rs @@ -12,6 +12,7 @@ pub mod address; pub mod data_api; mod decrypt; pub mod encoding; +pub mod fees; pub mod keys; pub mod proto; pub mod scan; diff --git a/zcash_extensions/src/transparent/demo.rs b/zcash_extensions/src/transparent/demo.rs index c43b2c31d0..79e0aafbd1 100644 --- a/zcash_extensions/src/transparent/demo.rs +++ b/zcash_extensions/src/transparent/demo.rs @@ -495,6 +495,7 @@ mod tests { amount::{Amount, DEFAULT_FEE}, tze::{Authorized, Bundle, OutPoint, TzeIn, TzeOut}, }, + fees::FixedFeeRule, Transaction, TransactionData, TxVersion, }, zip32::ExtendedSpendingKey, @@ -808,12 +809,13 @@ mod tests { // let mut rng = OsRng; + let fee_rule = FixedFeeRule::new(DEFAULT_FEE); // create some inputs to spend let extsk = ExtendedSpendingKey::master(&[]); let to = extsk.default_address().1; let note1 = to - .create_note(110000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng))) + .create_note(101000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng))) .unwrap(); let cm1 = Node::new(note1.cmu().to_repr()); let mut tree = CommitmentTree::empty(); @@ -835,7 +837,7 @@ mod tests { .unwrap(); let (tx_a, _) = builder_a .txn_builder - .build(&prover) + .build_zfuture(&prover, &fee_rule) .map_err(|e| format!("build failure: {:?}", e)) .unwrap(); let tze_a = tx_a.tze_bundle().unwrap(); @@ -853,7 +855,7 @@ mod tests { .unwrap(); let (tx_b, _) = builder_b .txn_builder - .build(&prover) + .build_zfuture(&prover, &fee_rule) .map_err(|e| format!("build failure: {:?}", e)) .unwrap(); let tze_b = tx_b.tze_bundle().unwrap(); @@ -878,7 +880,7 @@ mod tests { let (tx_c, _) = builder_c .txn_builder - .build(&prover) + .build_zfuture(&prover, &fee_rule) .map_err(|e| format!("build failure: {:?}", e)) .unwrap(); let tze_c = tx_c.tze_bundle().unwrap(); diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index e5fbd6bb94..1f6593f10b 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -10,6 +10,46 @@ and this library adheres to Rust's notion of ### Added - Added in `zcash_primitives::zip32` - An implementation of `TryFrom` for `u32` +- Added to `zcash_primitives::transaction::builder` + - `Error::InsufficientFunds` + - `Error::ChangeRequired` + - `Builder` state accessor methods: + - `Builder::params()` + - `Builder::target_height()` + - `Builder::transparent_inputs()` + - `Builder::transparent_outputs()` + - `Builder::sapling_inputs()` + - `Builder::sapling_outputs()` +- `zcash_primitives::transaction::fees` a new module containing abstractions + and types related to fee calculations. + - `FeeRule` a trait that describes how to compute the fee required for a + transaction given inputs and outputs to the transaction. +- Added to `zcash_primitives::transaction::components::sapling::builder` + - `SaplingBuilder::{inputs, outputs}`: accessors for Sapling builder state. +- `zcash_primitives::transaction::components::sapling::fees` +- Added to `zcash_primitives::transaction::components::transparent::builder` + - `TransparentBuilder::{inputs, outputs}`: accessors for transparent builder state. +- `zcash_primitives::transaction::components::transparent::fees` + +### Changed +- `zcash_primitives::transaction::builder::Builder::build` now takes a `FeeRule` + argument which is used to compute the fee for the transaction as part of the + build process. +- `zcash_primitives::transaction::builder::Builder::{new, new_with_rng}` no + longer fixes the fee for transactions to 0.00001 ZEC; the builder instead + computes the fee using a `FeeRule` implementation at build time. + +### Removed +- Removed from `zcash_primitives::transaction::builder::Builder` + - `Builder::{new_with_fee, new_with_rng_and_fee`} (use `Builder::{new, new_with_rng}` + instead along with a `FeeRule` implementation passed to `Builder::build`.) + - `Builder::send_change_to` has been removed. Change outputs must be added to the + builder by the caller, just like any other output. +- Removed from `zcash_primitives::transaction::builder::Error` + - `Error::ChangeIsNegative` + - `Error::NoChangeAddress` +- `zcash_primitives::transaction::components::sapling::builder::SaplingBuilder::get_candidate_change_address` + has been removed; change outputs must now be added by the caller. ## [0.8.1] - 2022-10-19 ### Added @@ -29,9 +69,9 @@ and this library adheres to Rust's notion of - `ChainCode::as_bytes` - `DiversifierIndex::{as_bytes}` - Implementations of `From` and `From` for `DiversifierIndex` -- `zcash_primitives::zip32::sapling` has been added and now contains +- `zcash_primitives::zip32::sapling` has been added and now contains all of the Sapling zip32 key types that were previously located in - `zcash_primitives::zip32` directly. The base `zip32` module reexports + `zcash_primitives::zip32` directly. The base `zip32` module reexports the moved types for backwards compatibility. - `DiversifierKey::{from_bytes, as_bytes}` - `ExtendedSpendingKey::{from_bytes, to_bytes}` diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 54c3095c49..ccaf7c1cab 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -1,5 +1,7 @@ //! Structs for building transactions. +use std::cmp::Ordering; +use std::convert::Infallible; use std::error; use std::fmt; use std::sync::mpsc::Sender; @@ -18,13 +20,14 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::{Amount, DEFAULT_FEE}, + amount::Amount, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, }, transparent::{self, builder::TransparentBuilder}, }, + fees::FeeRule, sighash::{signature_hash, SignableInput}, txid::TxIdDigester, Transaction, TransactionData, TxVersion, Unauthorized, @@ -38,24 +41,33 @@ use crate::transaction::components::transparent::TxOut; #[cfg(feature = "zfuture")] use crate::{ extensions::transparent::{ExtensionTxBuilder, ToPayload}, - transaction::components::{ - tze::builder::TzeBuilder, - tze::{self, TzeOut}, + transaction::{ + components::{ + tze::builder::TzeBuilder, + tze::{self, TzeOut}, + }, + fees::FutureFeeRule, }, }; -#[cfg(any(test, feature = "test-dependencies"))] -use crate::sapling::prover::mock::MockTxProver; - const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; +/// Errors that can occur during transaction construction. #[derive(Debug, PartialEq, Eq)] pub enum Error { - ChangeIsNegative(Amount), + /// Insufficient funds were provided to the transaction builder; the given + /// additional amount is required in order to construct the transaction. + InsufficientFunds(Amount), + /// The transaction has inputs in excess of outputs and fees; the user must + /// add a change output. + ChangeRequired(Amount), + /// An overflow or underflow occurred when computing value balances InvalidAmount, - NoChangeAddress, + /// An error occurred in constructing the transparent parts of a transaction. TransparentBuild(transparent::builder::Error), + /// An error occurred in constructing the Sapling parts of a transaction. SaplingBuild(sapling::builder::Error), + /// An error occurred in constructing the TZE parts of a transaction. #[cfg(feature = "zfuture")] TzeBuild(tze::builder::Error), } @@ -63,11 +75,17 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Error::ChangeIsNegative(amount) => { - write!(f, "Change is negative ({:?} zatoshis)", amount) - } - Error::InvalidAmount => write!(f, "Invalid amount"), - Error::NoChangeAddress => write!(f, "No change address specified or discoverable"), + Error::InsufficientFunds(amount) => write!( + f, + "Insufficient funds for transaction construction; need an additional {:?} zatoshis", + amount + ), + Error::ChangeRequired(amount) => write!( + f, + "The transaction requires an additional change output of {:?} zatoshis", + amount + ), + Error::InvalidAmount => write!(f, "Invalid amount (overflow or underflow)"), Error::TransparentBuild(err) => err.fmt(f), Error::SaplingBuild(err) => err.fmt(f), #[cfg(feature = "zfuture")] @@ -78,6 +96,12 @@ impl fmt::Display for Error { impl error::Error for Error {} +impl From for Error { + fn from(_: Infallible) -> Error { + unreachable!() + } +} + /// Reports on the progress made by the builder towards building a transaction. pub struct Progress { /// The number of steps completed. @@ -107,20 +131,14 @@ impl Progress { } } -enum ChangeAddress { - SaplingChangeAddress(OutgoingViewingKey, PaymentAddress), -} - /// Generates a [`Transaction`] from its inputs and outputs. pub struct Builder<'a, P, R> { params: P, rng: R, target_height: BlockHeight, expiry_height: BlockHeight, - fee: Amount, transparent_builder: TransparentBuilder, sapling_builder: SaplingBuilder

, - change_address: Option, #[cfg(feature = "zfuture")] tze_builder: TzeBuilder<'a, TransactionData>, #[cfg(not(feature = "zfuture"))] @@ -128,6 +146,42 @@ pub struct Builder<'a, P, R> { progress_notifier: Option>, } +impl<'a, P, R> Builder<'a, P, R> { + /// Returns the network parameters that the builder has been configured for. + pub fn params(&self) -> &P { + &self.params + } + + /// Returns the target height of the transaction under construction. + pub fn target_height(&self) -> BlockHeight { + self.target_height + } + + /// Returns the set of transparent inputs currently committed to be consumed + /// by the transaction. + pub fn transparent_inputs(&self) -> &[impl transparent::fees::InputView] { + self.transparent_builder.inputs() + } + + /// Returns the set of transparent outputs currently set to be produced by + /// the transaction. + pub fn transparent_outputs(&self) -> &[impl transparent::fees::OutputView] { + self.transparent_builder.outputs() + } + + /// Returns the set of Sapling inputs currently committed to be consumed + /// by the transaction. + pub fn sapling_inputs(&self) -> &[impl sapling::fees::InputView] { + self.sapling_builder.inputs() + } + + /// Returns the set of Sapling outputs currently set to be produced by + /// the transaction. + pub fn sapling_outputs(&self) -> &[impl sapling::fees::OutputView] { + self.sapling_builder.outputs() + } +} + impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng> { /// Creates a new `Builder` targeted for inclusion in the block with the given height, /// using default values for general transaction fields and the default OS random. @@ -136,23 +190,9 @@ impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng> { /// /// The expiry height will be set to the given height plus the default transaction /// expiry delta (20 blocks). - /// - /// The fee will be set to the default fee (0.0001 ZEC). pub fn new(params: P, target_height: BlockHeight) -> Self { Builder::new_with_rng(params, target_height, OsRng) } - - /// Creates a new `Builder` targeted for inclusion in the block with the given height, using - /// the specified fee, and otherwise default values for general transaction fields and the - /// default OS random. - /// - /// # Default values - /// - /// The expiry height will be set to the given height plus the default transaction - /// expiry delta (20 blocks). - pub fn new_with_fee(params: P, target_height: BlockHeight, fee: Amount) -> Self { - Builder::new_with_rng_and_fee(params, OsRng, target_height, fee) - } } impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> { @@ -163,27 +203,8 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> { /// /// The expiry height will be set to the given height plus the default transaction /// expiry delta (20 blocks). - /// - /// The fee will be set to the default fee (0.0001 ZEC). pub fn new_with_rng(params: P, target_height: BlockHeight, rng: R) -> Builder<'a, P, R> { - Self::new_internal(params, rng, target_height, DEFAULT_FEE) - } - - /// Creates a new `Builder` targeted for inclusion in the block with the given height, and - /// randomness source, using the specified fee, and otherwise default values for general - /// transaction fields and the default OS random. - /// - /// # Default values - /// - /// The expiry height will be set to the given height plus the default transaction - /// expiry delta (20 blocks). - pub fn new_with_rng_and_fee( - params: P, - rng: R, - target_height: BlockHeight, - fee: Amount, - ) -> Builder<'a, P, R> { - Self::new_internal(params, rng, target_height, fee) + Self::new_internal(params, rng, target_height) } } @@ -192,21 +213,14 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { /// /// WARNING: THIS MUST REMAIN PRIVATE AS IT ALLOWS CONSTRUCTION /// OF BUILDERS WITH NON-CryptoRng RNGs - fn new_internal( - params: P, - rng: R, - target_height: BlockHeight, - fee: Amount, - ) -> Builder<'a, P, R> { + fn new_internal(params: P, rng: R, target_height: BlockHeight) -> Builder<'a, P, R> { Builder { params: params.clone(), rng, target_height, expiry_height: target_height + DEFAULT_TX_EXPIRY_DELTA, - fee, transparent_builder: TransparentBuilder::empty(), sapling_builder: SaplingBuilder::new(params, target_height), - change_address: None, #[cfg(feature = "zfuture")] tze_builder: TzeBuilder::empty(), #[cfg(not(feature = "zfuture"))] @@ -269,14 +283,6 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { .map_err(Error::TransparentBuild) } - /// Sets the Sapling address to which any change will be sent. - /// - /// By default, change is sent to the Sapling address corresponding to the first note - /// being spent (i.e. the first call to [`Builder::add_sapling_spend`]). - pub fn send_change_to(&mut self, ovk: OutgoingViewingKey, to: PaymentAddress) { - self.change_address = Some(ChangeAddress::SaplingChangeAddress(ovk, to)) - } - /// Sets the notifier channel, where progress of building the transaction is sent. /// /// An update is sent after every Spend or Output is computed, and the `u32` sent @@ -310,9 +316,55 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { /// /// Upon success, returns a tuple containing the final transaction, and the /// [`SaplingMetadata`] generated during the build process. - pub fn build( - mut self, + pub fn build( + self, + prover: &impl TxProver, + fee_rule: &FR, + ) -> Result<(Transaction, SaplingMetadata), Error> + where + Error: From, + { + let fee = fee_rule.fee_required( + &self.params, + self.target_height, + self.transparent_builder.inputs(), + self.transparent_builder.outputs(), + self.sapling_builder.inputs(), + self.sapling_builder.outputs(), + )?; + self.build_internal(prover, fee) + } + + /// Builds a transaction from the configured spends and outputs. + /// + /// Upon success, returns a tuple containing the final transaction, and the + /// [`SaplingMetadata`] generated during the build process. + #[cfg(feature = "zfuture")] + pub fn build_zfuture( + self, + prover: &impl TxProver, + fee_rule: &FR, + ) -> Result<(Transaction, SaplingMetadata), Error> + where + Error: From, + { + let fee = fee_rule.fee_required_zfuture( + &self.params, + self.target_height, + self.transparent_builder.inputs(), + self.transparent_builder.outputs(), + self.sapling_builder.inputs(), + self.sapling_builder.outputs(), + self.tze_builder.inputs(), + self.tze_builder.outputs(), + )?; + self.build_internal(prover, fee) + } + + fn build_internal( + self, prover: &impl TxProver, + fee: Amount, ) -> Result<(Transaction, SaplingMetadata), Error> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); @@ -323,33 +375,18 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { // Consistency checks // - // Valid change - let change = (self.value_balance()? - self.fee).ok_or(Error::InvalidAmount)?; + // After fees are accounted for, the value balance of the transaction must be zero. + let balance_after_fees = (self.value_balance()? - fee).ok_or(Error::InvalidAmount)?; - if change.is_negative() { - return Err(Error::ChangeIsNegative(change)); - } - - // - // Change output - // - - if change.is_positive() { - // Send change to the specified change address. If no change address - // was set, send change to the first Sapling address given as input. - match self.change_address.take() { - Some(ChangeAddress::SaplingChangeAddress(ovk, addr)) => { - self.add_sapling_output(Some(ovk), addr, change, MemoBytes::empty())?; - } - None => { - let (ovk, addr) = self - .sapling_builder - .get_candidate_change_address() - .ok_or(Error::NoChangeAddress)?; - self.add_sapling_output(Some(ovk), addr, change, MemoBytes::empty())?; - } + match balance_after_fees.cmp(&Amount::zero()) { + Ordering::Less => { + return Err(Error::InsufficientFunds(-balance_after_fees)); } - } + Ordering::Greater => { + return Err(Error::ChangeRequired(balance_after_fees)); + } + Ordering::Equal => (), + }; let transparent_bundle = self.transparent_builder.build(); @@ -475,24 +512,33 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> ExtensionTxBuilder<'a } #[cfg(any(test, feature = "test-dependencies"))] -impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { - /// Creates a new `Builder` targeted for inclusion in the block with the given height - /// and randomness source, using default values for general transaction fields. - /// - /// # Default values - /// - /// The expiry height will be set to the given height plus the default transaction - /// expiry delta (20 blocks). - /// - /// The fee will be set to the default fee (0.0001 ZEC). - /// - /// WARNING: DO NOT USE IN PRODUCTION - pub fn test_only_new_with_rng(params: P, height: BlockHeight, rng: R) -> Builder<'a, P, R> { - Self::new_internal(params, rng, height, DEFAULT_FEE) - } +mod testing { + use rand::RngCore; - pub fn mock_build(self) -> Result<(Transaction, SaplingMetadata), Error> { - self.build(&MockTxProver) + use super::{Builder, Error, SaplingMetadata}; + use crate::{ + consensus::{self, BlockHeight}, + sapling::prover::mock::MockTxProver, + transaction::{components::amount::DEFAULT_FEE, fees::FixedFeeRule, Transaction}, + }; + + impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { + /// Creates a new `Builder` targeted for inclusion in the block with the given height + /// and randomness source, using default values for general transaction fields. + /// + /// # Default values + /// + /// The expiry height will be set to the given height plus the default transaction + /// expiry delta (20 blocks). + /// + /// WARNING: DO NOT USE IN PRODUCTION + pub fn test_only_new_with_rng(params: P, height: BlockHeight, rng: R) -> Builder<'a, P, R> { + Self::new_internal(params, rng, height) + } + + pub fn mock_build(self) -> Result<(Transaction, SaplingMetadata), Error> { + self.build(&MockTxProver, &FixedFeeRule::new(DEFAULT_FEE)) + } } } @@ -506,7 +552,7 @@ mod tests { legacy::TransparentAddress, memo::MemoBytes, merkle_tree::{CommitmentTree, IncrementalWitness}, - sapling::{prover::mock::MockTxProver, Node, Rseed}, + sapling::{Node, Rseed}, transaction::components::{ amount::{Amount, DEFAULT_FEE}, sapling::builder::{self as build_s}, @@ -515,14 +561,25 @@ mod tests { zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}, }; - use super::{Builder, Error, SaplingBuilder, DEFAULT_TX_EXPIRY_DELTA}; + use super::{Builder, Error}; #[cfg(feature = "zfuture")] + #[cfg(feature = "transparent-inputs")] use super::TzeBuilder; #[cfg(not(feature = "zfuture"))] use std::marker::PhantomData; + #[cfg(feature = "transparent-inputs")] + use crate::{ + legacy::keys::{AccountPrivKey, IncomingViewingKey}, + transaction::{ + builder::{SaplingBuilder, DEFAULT_TX_EXPIRY_DELTA}, + OutPoint, TxOut, + }, + zip32::AccountId, + }; + #[test] fn fails_on_negative_output() { let extsk = ExtendedSpendingKey::master(&[]); @@ -546,7 +603,10 @@ mod tests { ); } + // This test only works with the transparent_inputs feature because we have to + // be able to create a tx with a valid balance, without using Sapling inputs. #[test] + #[cfg(feature = "transparent-inputs")] fn binding_sig_absent_if_no_shielded_spend_or_output() { use crate::consensus::NetworkUpgrade; use crate::transaction::builder::{self, TransparentBuilder}; @@ -561,10 +621,8 @@ mod tests { rng: OsRng, target_height: sapling_activation_height, expiry_height: sapling_activation_height + DEFAULT_TX_EXPIRY_DELTA, - fee: Amount::zero(), transparent_builder: TransparentBuilder::empty(), sapling_builder: SaplingBuilder::new(TEST_NETWORK, sapling_activation_height), - change_address: None, #[cfg(feature = "zfuture")] tze_builder: TzeBuilder::empty(), #[cfg(not(feature = "zfuture"))] @@ -572,12 +630,34 @@ mod tests { progress_notifier: None, }; + let tsk = AccountPrivKey::from_seed(&TEST_NETWORK, &[0u8; 32], AccountId::from(0)).unwrap(); + let prev_coin = TxOut { + value: Amount::from_u64(50000).unwrap(), + script_pubkey: tsk + .to_account_pubkey() + .derive_external_ivk() + .unwrap() + .derive_address(0) + .unwrap() + .script(), + }; + builder + .add_transparent_input( + tsk.derive_external_secret_key(0).unwrap(), + OutPoint::new([0u8; 32], 1), + prev_coin, + ) + .unwrap(); + // Create a tx with only t output. No binding_sig should be present builder - .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount::zero()) + .add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_u64(49000).unwrap(), + ) .unwrap(); - let (tx, _) = builder.build(&MockTxProver).unwrap(); + let (tx, _) = builder.mock_build().unwrap(); // No binding signature, because only t input and outputs assert!(tx.sapling_bundle.is_none()); } @@ -609,13 +689,16 @@ mod tests { .unwrap(); builder - .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount::zero()) + .add_transparent_output( + &TransparentAddress::PublicKey([0; 20]), + Amount::from_u64(49000).unwrap(), + ) .unwrap(); // Expect a binding signature error, because our inputs aren't valid, but this shows // that a binding signature was attempted assert_eq!( - builder.build(&MockTxProver), + builder.mock_build(), Err(Error::SaplingBuild(build_s::Error::BindingSig)) ); } @@ -650,10 +733,8 @@ mod tests { { let builder = Builder::new(TEST_NETWORK, tx_height); assert_eq!( - builder.build(&MockTxProver), - Err(Error::ChangeIsNegative( - (Amount::zero() - DEFAULT_FEE).unwrap() - )) + builder.mock_build(), + Err(Error::InsufficientFunds(DEFAULT_FEE)) ); } @@ -674,9 +755,9 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(&MockTxProver), - Err(Error::ChangeIsNegative( - (Amount::from_i64(-50000).unwrap() - DEFAULT_FEE).unwrap() + builder.mock_build(), + Err(Error::InsufficientFunds( + (Amount::from_i64(50000).unwrap() + DEFAULT_FEE).unwrap() )) ); } @@ -692,9 +773,9 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(&MockTxProver), - Err(Error::ChangeIsNegative( - (Amount::from_i64(-50000).unwrap() - DEFAULT_FEE).unwrap() + builder.mock_build(), + Err(Error::InsufficientFunds( + (Amount::from_i64(50000).unwrap() + DEFAULT_FEE).unwrap() )) ); } @@ -734,8 +815,8 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(&MockTxProver), - Err(Error::ChangeIsNegative(Amount::from_i64(-1).unwrap())) + builder.mock_build(), + Err(Error::InsufficientFunds(Amount::from_i64(1).unwrap())) ); } @@ -780,7 +861,7 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(&MockTxProver), + builder.mock_build(), Err(Error::SaplingBuild(build_s::Error::BindingSig)) ) } diff --git a/zcash_primitives/src/transaction/components/sapling.rs b/zcash_primitives/src/transaction/components/sapling.rs index 2fb4576509..4848a0f975 100644 --- a/zcash_primitives/src/transaction/components/sapling.rs +++ b/zcash_primitives/src/transaction/components/sapling.rs @@ -24,6 +24,7 @@ use super::{amount::Amount, GROTH_PROOF_SIZE}; pub type GrothProofBytes = [u8; GROTH_PROOF_SIZE]; pub mod builder; +pub mod fees; pub trait Authorization: Debug { type Proof: Clone + Debug; diff --git a/zcash_primitives/src/transaction/components/sapling/builder.rs b/zcash_primitives/src/transaction/components/sapling/builder.rs index 3fcfca8b5f..358999439a 100644 --- a/zcash_primitives/src/transaction/components/sapling/builder.rs +++ b/zcash_primitives/src/transaction/components/sapling/builder.rs @@ -25,7 +25,7 @@ use crate::{ components::{ amount::Amount, sapling::{ - Authorization, Authorized, Bundle, GrothProofBytes, OutputDescription, + fees, Authorization, Authorized, Bundle, GrothProofBytes, OutputDescription, SpendDescription, }, }, @@ -69,8 +69,18 @@ pub struct SpendDescriptionInfo { merkle_path: MerklePath, } +impl fees::InputView for SpendDescriptionInfo { + fn value(&self) -> Amount { + // An existing note to be spent must have a valid + // amount value. + Amount::from_u64(self.note.value).unwrap() + } +} + +/// A struct containing the information required in order to construct a +/// Sapling output to a transaction. #[derive(Clone)] -struct SaplingOutput { +struct SaplingOutputInfo { /// `None` represents the `ovk = ⊥` case. ovk: Option, to: PaymentAddress, @@ -78,7 +88,7 @@ struct SaplingOutput { memo: MemoBytes, } -impl SaplingOutput { +impl SaplingOutputInfo { fn new_internal( params: &P, rng: &mut R, @@ -102,7 +112,7 @@ impl SaplingOutput { rseed, }; - Ok(SaplingOutput { + Ok(SaplingOutputInfo { ovk, to, note, @@ -150,6 +160,12 @@ impl SaplingOutput { } } +impl fees::OutputView for SaplingOutputInfo { + fn value(&self) -> Amount { + Amount::from_u64(self.note.value).expect("Note values should be checked at construction.") + } +} + /// Metadata about a transaction created by a [`SaplingBuilder`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SaplingMetadata { @@ -194,7 +210,7 @@ pub struct SaplingBuilder

{ target_height: BlockHeight, value_balance: Amount, spends: Vec, - outputs: Vec, + outputs: Vec, } #[derive(Clone)] @@ -213,7 +229,7 @@ impl Authorization for Unauthorized { type AuthSig = SpendDescriptionInfo; } -impl SaplingBuilder

{ +impl

SaplingBuilder

{ pub fn new(params: P, target_height: BlockHeight) -> Self { SaplingBuilder { params, @@ -225,11 +241,24 @@ impl SaplingBuilder

{ } } + /// Returns the list of Sapling inputs that will be consumed by the transaction being + /// constructed. + pub fn inputs(&self) -> &[impl fees::InputView] { + &self.spends + } + + /// Returns the Sapling outputs that will be produced by the transaction being constructed + pub fn outputs(&self) -> &[impl fees::OutputView] { + &self.outputs + } + /// Returns the net value represented by the spends and outputs added to this builder. pub fn value_balance(&self) -> Amount { self.value_balance } +} +impl SaplingBuilder

{ /// Adds a Sapling note to be spent in this transaction. /// /// Returns an error if the given Merkle path does not have the same anchor as the @@ -278,7 +307,7 @@ impl SaplingBuilder

{ value: Amount, memo: MemoBytes, ) -> Result<(), Error> { - let output = SaplingOutput::new_internal( + let output = SaplingOutputInfo::new_internal( &self.params, &mut rng, self.target_height, @@ -295,15 +324,6 @@ impl SaplingBuilder

{ Ok(()) } - /// Send change to the specified change address. If no change address - /// was set, send change to the first Sapling address given as input. - pub fn get_candidate_change_address(&self) -> Option<(OutgoingViewingKey, PaymentAddress)> { - self.spends.first().and_then(|spend| { - PaymentAddress::from_parts(spend.diversifier, spend.note.pk_d) - .map(|addr| (spend.extsk.expsk.ovk, addr)) - }) - } - pub fn build( self, prover: &Pr, diff --git a/zcash_primitives/src/transaction/components/sapling/fees.rs b/zcash_primitives/src/transaction/components/sapling/fees.rs new file mode 100644 index 0000000000..a1ad1659d9 --- /dev/null +++ b/zcash_primitives/src/transaction/components/sapling/fees.rs @@ -0,0 +1,18 @@ +//! Types related to computation of fees and change related to the Sapling components +//! of a transaction. + +use crate::transaction::components::amount::Amount; + +/// A trait that provides a minimized view of a Sapling input suitable for use in +/// fee and change calculation. +pub trait InputView { + /// The value of the input being spent. + fn value(&self) -> Amount; +} + +/// A trait that provides a minimized view of a Sapling output suitable for use in +/// fee and change calculation. +pub trait OutputView { + /// The value of the output being produced. + fn value(&self) -> Amount; +} diff --git a/zcash_primitives/src/transaction/components/transparent.rs b/zcash_primitives/src/transaction/components/transparent.rs index 39e8e0f113..ed11082593 100644 --- a/zcash_primitives/src/transaction/components/transparent.rs +++ b/zcash_primitives/src/transaction/components/transparent.rs @@ -10,6 +10,7 @@ use crate::legacy::{Script, TransparentAddress}; use super::amount::{Amount, BalanceError}; pub mod builder; +pub mod fees; pub trait Authorization: Debug { type ScriptSig: Debug + Clone + PartialEq; diff --git a/zcash_primitives/src/transaction/components/transparent/builder.rs b/zcash_primitives/src/transaction/components/transparent/builder.rs index 63371ddcda..5d2780ad68 100644 --- a/zcash_primitives/src/transaction/components/transparent/builder.rs +++ b/zcash_primitives/src/transaction/components/transparent/builder.rs @@ -7,9 +7,10 @@ use crate::{ transaction::{ components::{ amount::Amount, - transparent::{self, Authorization, Authorized, Bundle, TxIn, TxOut}, + transparent::{self, fees, Authorization, Authorized, Bundle, TxIn, TxOut}, }, sighash::TransparentAuthorizingContext, + OutPoint, }, }; @@ -17,7 +18,6 @@ use crate::{ use { crate::transaction::{ self as tx, - components::OutPoint, sighash::{signature_hash, SignableInput, SIGHASH_ALL}, TransactionData, TxDigests, }, @@ -40,6 +40,21 @@ impl fmt::Display for Error { } } +/// An uninhabited type that allows the type of [`TransparentBuilder::inputs`] +/// to resolve when the transparent-inputs feature is not turned on. +#[cfg(not(feature = "transparent-inputs"))] +enum InvalidTransparentInput {} + +#[cfg(not(feature = "transparent-inputs"))] +impl fees::InputView for InvalidTransparentInput { + fn outpoint(&self) -> &OutPoint { + panic!("transparent-inputs feature flag is not enabled."); + } + fn coin(&self) -> &TxOut { + panic!("transparent-inputs feature flag is not enabled."); + } +} + #[cfg(feature = "transparent-inputs")] #[derive(Debug, Clone)] struct TransparentInputInfo { @@ -49,6 +64,17 @@ struct TransparentInputInfo { coin: TxOut, } +#[cfg(feature = "transparent-inputs")] +impl fees::InputView for TransparentInputInfo { + fn outpoint(&self) -> &OutPoint { + &self.utxo + } + + fn coin(&self) -> &TxOut { + &self.coin + } +} + pub struct TransparentBuilder { #[cfg(feature = "transparent-inputs")] secp: secp256k1::Secp256k1, @@ -70,6 +96,7 @@ impl Authorization for Unauthorized { } impl TransparentBuilder { + /// Constructs a new TransparentBuilder pub fn empty() -> Self { TransparentBuilder { #[cfg(feature = "transparent-inputs")] @@ -80,6 +107,25 @@ impl TransparentBuilder { } } + /// Returns the list of transparent inputs that will be consumed by the transaction being + /// constructed. + pub fn inputs(&self) -> &[impl fees::InputView] { + #[cfg(feature = "transparent-inputs")] + return &self.inputs; + + #[cfg(not(feature = "transparent-inputs"))] + { + let invalid: &[InvalidTransparentInput] = &[]; + return invalid; + } + } + + /// Returns the transparent outputs that will be produced by the transaction being constructed. + pub fn outputs(&self) -> &[impl fees::OutputView] { + &self.vout + } + + /// Adds a coin (the output of a previous transaction) to be spent to the transaction. #[cfg(feature = "transparent-inputs")] pub fn add_input( &mut self, diff --git a/zcash_primitives/src/transaction/components/transparent/fees.rs b/zcash_primitives/src/transaction/components/transparent/fees.rs new file mode 100644 index 0000000000..1e63e16c33 --- /dev/null +++ b/zcash_primitives/src/transaction/components/transparent/fees.rs @@ -0,0 +1,36 @@ +//! Types related to computation of fees and change related to the transparent components +//! of a transaction. + +use super::TxOut; +use crate::{ + legacy::Script, + transaction::{components::amount::Amount, OutPoint}, +}; + +/// This trait provides a minimized view of a transparent input suitable for use in +/// fee and change computation. +pub trait InputView { + /// The outpoint to which the input refers. + fn outpoint(&self) -> &OutPoint; + /// The previous output being spent. + fn coin(&self) -> &TxOut; +} + +/// This trait provides a minimized view of a transparent output suitable for use in +/// fee and change computation. +pub trait OutputView { + /// Returns the value of the output being created. + fn value(&self) -> Amount; + /// Returns the script corresponding to the newly created output. + fn script_pubkey(&self) -> &Script; +} + +impl OutputView for TxOut { + fn value(&self) -> Amount { + self.value + } + + fn script_pubkey(&self) -> &Script { + &self.script_pubkey + } +} diff --git a/zcash_primitives/src/transaction/components/tze.rs b/zcash_primitives/src/transaction/components/tze.rs index aa3858865a..e766d2f9f5 100644 --- a/zcash_primitives/src/transaction/components/tze.rs +++ b/zcash_primitives/src/transaction/components/tze.rs @@ -12,6 +12,7 @@ use super::amount::Amount; use crate::{extensions::transparent as tze, transaction::TxId}; pub mod builder; +pub mod fees; fn to_io_error(_: std::num::TryFromIntError) -> io::Error { io::Error::new(io::ErrorKind::InvalidData, "value out of range") diff --git a/zcash_primitives/src/transaction/components/tze/builder.rs b/zcash_primitives/src/transaction/components/tze/builder.rs index 27f69bf9ea..8555a4870d 100644 --- a/zcash_primitives/src/transaction/components/tze/builder.rs +++ b/zcash_primitives/src/transaction/components/tze/builder.rs @@ -9,7 +9,7 @@ use crate::{ self as tx, components::{ amount::Amount, - tze::{Authorization, Authorized, Bundle, OutPoint, TzeIn, TzeOut}, + tze::{fees, Authorization, Authorized, Bundle, OutPoint, TzeIn, TzeOut}, }, }, }; @@ -32,13 +32,27 @@ impl fmt::Display for Error { #[allow(clippy::type_complexity)] pub struct TzeSigner<'a, BuildCtx> { - prevout: TzeOut, builder: Box Result<(u32, Vec), Error> + 'a>, } +#[derive(Clone)] +struct TzeBuildInput { + tzein: TzeIn<()>, + coin: TzeOut, +} + +impl fees::InputView for TzeBuildInput { + fn outpoint(&self) -> &OutPoint { + &self.tzein.prevout + } + fn coin(&self) -> &TzeOut { + &self.coin + } +} + pub struct TzeBuilder<'a, BuildCtx> { signers: Vec>, - vin: Vec>, + vin: Vec, vout: Vec, } @@ -58,18 +72,28 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> { } } + pub fn inputs(&self) -> &[impl fees::InputView] { + &self.vin + } + + pub fn outputs(&self) -> &[impl fees::OutputView] { + &self.vout + } + pub fn add_input( &mut self, extension_id: u32, mode: u32, - (outpoint, prevout): (OutPoint, TzeOut), + (outpoint, coin): (OutPoint, TzeOut), witness_builder: WBuilder, ) where WBuilder: 'a + FnOnce(&BuildCtx) -> Result, { - self.vin.push(TzeIn::new(outpoint, extension_id, mode)); + self.vin.push(TzeBuildInput { + tzein: TzeIn::new(outpoint, extension_id, mode), + coin, + }); self.signers.push(TzeSigner { - prevout, builder: Box::new(move |ctx| witness_builder(ctx).map(|x| x.to_payload())), }); } @@ -98,9 +122,9 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> { } pub fn value_balance(&self) -> Option { - self.signers + self.vin .iter() - .map(|s| s.prevout.value) + .map(|tzi| tzi.coin.value) .sum::>()? - self .vout @@ -115,7 +139,7 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> { } else { ( Some(Bundle { - vin: self.vin.clone(), + vin: self.vin.iter().map(|vin| vin.tzein.clone()).collect(), vout: self.vout.clone(), authorization: Unauthorized, }), diff --git a/zcash_primitives/src/transaction/components/tze/fees.rs b/zcash_primitives/src/transaction/components/tze/fees.rs new file mode 100644 index 0000000000..1a72d6cc32 --- /dev/null +++ b/zcash_primitives/src/transaction/components/tze/fees.rs @@ -0,0 +1,37 @@ +//! Abstractions and types related to fee calculations for TZE components of a transaction. + +use crate::{ + extensions::transparent::{self as tze}, + transaction::components::{ + amount::Amount, + tze::{OutPoint, TzeOut}, + }, +}; + +/// This trait provides a minimized view of a TZE input suitable for use in +/// fee computation. +pub trait InputView { + /// The outpoint to which the input refers. + fn outpoint(&self) -> &OutPoint; + /// The previous output being consumed. + fn coin(&self) -> &TzeOut; +} + +/// This trait provides a minimized view of a TZE output suitable for use in +/// fee computation. +pub trait OutputView { + /// The value of the newly created output + fn value(&self) -> Amount; + /// The precondition that must be satisfied in order to spend this output. + fn precondition(&self) -> &tze::Precondition; +} + +impl OutputView for TzeOut { + fn value(&self) -> Amount { + self.value + } + + fn precondition(&self) -> &tze::Precondition { + &self.precondition + } +} diff --git a/zcash_primitives/src/transaction/fees.rs b/zcash_primitives/src/transaction/fees.rs new file mode 100644 index 0000000000..706ea9984d --- /dev/null +++ b/zcash_primitives/src/transaction/fees.rs @@ -0,0 +1,101 @@ +//! Abstractions and types related to fee calculations. + +use crate::{ + consensus::{self, BlockHeight}, + transaction::components::{ + amount::Amount, sapling::fees as sapling, transparent::fees as transparent, + }, +}; + +#[cfg(feature = "zfuture")] +use crate::transaction::components::tze::fees as tze; + +/// A trait that represents the ability to compute the fees that must be paid +/// by a transaction having a specified set of inputs and outputs. +pub trait FeeRule { + type Error; + + /// Computes the total fee required for a transaction given the provided inputs and outputs. + /// + /// Implementations of this method should compute the fee amount given exactly the inputs and + /// outputs specified, and should NOT compute speculative fees given any additional change + /// outputs that may need to be created in order for inputs and outputs to balance. + fn fee_required( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + ) -> Result; +} + +/// A trait that represents the ability to compute the fees that must be paid by a transaction +/// having a specified set of inputs and outputs, for use when experimenting with the TZE feature. +#[cfg(feature = "zfuture")] +pub trait FutureFeeRule: FeeRule { + /// Computes the total fee required for a transaction given the provided inputs and outputs. + /// + /// Implementations of this method should compute the fee amount given exactly the inputs and + /// outputs specified, and should NOT compute speculative fees given any additional change + /// outputs that may need to be created in order for inputs and outputs to balance. + #[allow(clippy::too_many_arguments)] + fn fee_required_zfuture( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + tze_inputs: &[impl tze::InputView], + tze_outputs: &[impl tze::OutputView], + ) -> Result; +} + +/// A fee rule that always returns a fixed fee, irrespective of the structure of +/// the transaction being constructed. +pub struct FixedFeeRule { + fixed_fee: Amount, +} + +impl FixedFeeRule { + /// Creates a new fixed fee rule with the specified fixed fee. + pub fn new(fixed_fee: Amount) -> Self { + Self { fixed_fee } + } +} + +impl FeeRule for FixedFeeRule { + type Error = std::convert::Infallible; + + fn fee_required( + &self, + _params: &P, + _target_height: BlockHeight, + _transparent_inputs: &[impl transparent::InputView], + _transparent_outputs: &[impl transparent::OutputView], + _sapling_inputs: &[impl sapling::InputView], + _sapling_outputs: &[impl sapling::OutputView], + ) -> Result { + Ok(self.fixed_fee) + } +} + +#[cfg(feature = "zfuture")] +impl FutureFeeRule for FixedFeeRule { + fn fee_required_zfuture( + &self, + _params: &P, + _target_height: BlockHeight, + _transparent_inputs: &[impl transparent::InputView], + _transparent_outputs: &[impl transparent::OutputView], + _sapling_inputs: &[impl sapling::InputView], + _sapling_outputs: &[impl sapling::OutputView], + _tze_inputs: &[impl tze::InputView], + _tze_outputs: &[impl tze::OutputView], + ) -> Result { + Ok(self.fixed_fee) + } +} diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index 0a6d03e598..105e4981c1 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -1,6 +1,7 @@ //! Structs and methods for handling Zcash transactions. pub mod builder; pub mod components; +pub mod fees; pub mod sighash; pub mod sighash_v4; pub mod sighash_v5;