Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zcash_client_sqlite: Add Orchard wallet support #1182

Merged
merged 14 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ and this library adheres to Rust's notion of
constraint on its `<AccountId>` parameter has been strengthened to `Copy`.
- `zcash_client_backend::fees`:
- Arguments to `ChangeStrategy::compute_balance` have changed.
- `ChangeError::DustInputs` now has an `orchard` field behind the `orchard`
feature flag.
- `zcash_client_backend::proto`:
- `ProposalDecodingError` has a new variant `TransparentMemo`.
- `zcash_client_backend::zip321::render::amount_str` now takes a
Expand Down
9 changes: 8 additions & 1 deletion zcash_client_backend/src/data_api/wallet/input_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,15 @@
)
.map_err(InputSelectorError::Proposal);
}
Err(ChangeError::DustInputs { mut sapling, .. }) => {
Err(ChangeError::DustInputs {

Check warning on line 448 in zcash_client_backend/src/data_api/wallet/input_selection.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/wallet/input_selection.rs#L448

Added line #L448 was not covered by tests
mut sapling,
#[cfg(feature = "orchard")]
mut orchard,
..
}) => {
exclude.append(&mut sapling);
#[cfg(feature = "orchard")]
exclude.append(&mut orchard);
}
Err(ChangeError::InsufficientFunds { required, .. }) => {
amount_required = required;
Expand Down
20 changes: 19 additions & 1 deletion zcash_client_backend/src/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@
transparent: Vec<OutPoint>,
/// The identifiers for Sapling inputs having no current economic value
sapling: Vec<NoteRefT>,
/// The identifiers for Orchard inputs having no current economic value
#[cfg(feature = "orchard")]
orchard: Vec<NoteRefT>,
},
/// An error occurred that was specific to the change selection strategy in use.
StrategyError(E),
Expand All @@ -169,9 +172,13 @@
ChangeError::DustInputs {
transparent,
sapling,
#[cfg(feature = "orchard")]
orchard,

Check warning on line 176 in zcash_client_backend/src/fees.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees.rs#L175-L176

Added lines #L175 - L176 were not covered by tests
} => ChangeError::DustInputs {
transparent,
sapling,
#[cfg(feature = "orchard")]
orchard,
},
ChangeError::StrategyError(e) => ChangeError::StrategyError(f(e)),
ChangeError::BundleError(e) => ChangeError::BundleError(e),
Expand All @@ -194,10 +201,21 @@
ChangeError::DustInputs {
transparent,
sapling,
#[cfg(feature = "orchard")]
orchard,

Check warning on line 205 in zcash_client_backend/src/fees.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees.rs#L204-L205

Added lines #L204 - L205 were not covered by tests
} => {
#[cfg(feature = "orchard")]
let orchard_len = orchard.len();

Check warning on line 208 in zcash_client_backend/src/fees.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees.rs#L208

Added line #L208 was not covered by tests
#[cfg(not(feature = "orchard"))]
let orchard_len = 0;

Check warning on line 210 in zcash_client_backend/src/fees.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees.rs#L210

Added line #L210 was not covered by tests

// we can't encode the UA to its string representation because we
// don't have network parameters here
write!(f, "Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.", transparent.len() + sapling.len())
write!(
f,

Check warning on line 215 in zcash_client_backend/src/fees.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees.rs#L214-L215

Added lines #L214 - L215 were not covered by tests
"Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.",
transparent.len() + sapling.len() + orchard_len,

Check warning on line 217 in zcash_client_backend/src/fees.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees.rs#L217

Added line #L217 was not covered by tests
)
}
ChangeError::StrategyError(err) => {
write!(f, "{}", err)
Expand Down
126 changes: 91 additions & 35 deletions zcash_client_backend/src/fees/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,26 @@
#[cfg(feature = "orchard")]
use super::orchard as orchard_fees;

pub(crate) struct NetFlows {
t_in: NonNegativeAmount,
t_out: NonNegativeAmount,
sapling_in: NonNegativeAmount,
sapling_out: NonNegativeAmount,
orchard_in: NonNegativeAmount,
orchard_out: NonNegativeAmount,
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn single_change_output_balance<
P: consensus::Parameters,
NoteRefT: Clone,
F: FeeRule,
E,
>(
params: &P,
fee_rule: &F,
target_height: BlockHeight,
pub(crate) fn calculate_net_flows<NoteRefT: Clone, F: FeeRule, E>(
transparent_inputs: &[impl transparent::InputView],
transparent_outputs: &[impl transparent::OutputView],
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
default_dust_threshold: NonNegativeAmount,
change_memo: Option<MemoBytes>,
_fallback_change_pool: ShieldedProtocol,
) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
) -> Result<NetFlows, ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));

let t_in = transparent_inputs
.iter()
Expand Down Expand Up @@ -85,13 +81,30 @@
#[cfg(not(feature = "orchard"))]
let orchard_out = NonNegativeAmount::ZERO;

Ok(NetFlows {
t_in,
t_out,
sapling_in,
sapling_out,
orchard_in,
orchard_out,
})
}

pub(crate) fn single_change_output_policy<NoteRefT: Clone, F: FeeRule, E>(
_net_flows: &NetFlows,
_fallback_change_pool: ShieldedProtocol,
) -> Result<(ShieldedProtocol, usize, usize), ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
// 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) =
if orchard_in.is_positive() || orchard_out.is_positive() {
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)
} else if sapling_in.is_positive() || sapling_out.is_positive() {
} 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)
Expand All @@ -104,42 +117,85 @@
}
};
#[cfg(not(feature = "orchard"))]
let (change_pool, sapling_change) = (ShieldedProtocol::Sapling, 1);
let (change_pool, sapling_change, orchard_change) = (ShieldedProtocol::Sapling, 1, 0);

Ok((change_pool, sapling_change, orchard_change))
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn single_change_output_balance<
P: consensus::Parameters,
NoteRefT: Clone,
F: FeeRule,
E,
>(
params: &P,
fee_rule: &F,
target_height: BlockHeight,
transparent_inputs: &[impl transparent::InputView],
transparent_outputs: &[impl transparent::OutputView],
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
default_dust_threshold: NonNegativeAmount,
change_memo: Option<MemoBytes>,
_fallback_change_pool: ShieldedProtocol,
) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));

let net_flows = calculate_net_flows::<NoteRefT, F, E>(
transparent_inputs,
transparent_outputs,
sapling,
#[cfg(feature = "orchard")]
orchard,
)?;
let (change_pool, sapling_change, _orchard_change) =
single_change_output_policy::<NoteRefT, F, E>(&net_flows, _fallback_change_pool)?;

let sapling_input_count = sapling
.bundle_type()
.num_spends(sapling.inputs().len())
.map_err(ChangeError::BundleError)?;
let sapling_output_count = sapling
.bundle_type()
.num_outputs(
sapling.inputs().len(),
sapling.outputs().len() + sapling_change,
)
.map_err(ChangeError::BundleError)?;

#[cfg(feature = "orchard")]
let orchard_num_actions = orchard
let orchard_action_count = orchard

Check warning on line 173 in zcash_client_backend/src/fees/common.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees/common.rs#L173

Added line #L173 was not covered by tests
.bundle_type()
.num_actions(
orchard.inputs().len(),
orchard.outputs().len() + orchard_change,
orchard.outputs().len() + _orchard_change,

Check warning on line 177 in zcash_client_backend/src/fees/common.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/fees/common.rs#L177

Added line #L177 was not covered by tests
)
.map_err(ChangeError::BundleError)?;
#[cfg(not(feature = "orchard"))]
let orchard_num_actions = 0;
let orchard_action_count = 0;

let fee_amount = fee_rule
.fee_required(
params,
target_height,
transparent_inputs,
transparent_outputs,
sapling
.bundle_type()
.num_spends(sapling.inputs().len())
.map_err(ChangeError::BundleError)?,
sapling
.bundle_type()
.num_outputs(
sapling.inputs().len(),
sapling.outputs().len() + sapling_change,
)
.map_err(ChangeError::BundleError)?,
orchard_num_actions,
sapling_input_count,
sapling_output_count,
orchard_action_count,
)
.map_err(|fee_error| ChangeError::StrategyError(E::from(fee_error)))?;

let total_in = (t_in + sapling_in + orchard_in).ok_or_else(overflow)?;
let total_out = (t_out + sapling_out + orchard_out + fee_amount).ok_or_else(overflow)?;
let total_in =
(net_flows.t_in + net_flows.sapling_in + net_flows.orchard_in).ok_or_else(overflow)?;
let total_out = (net_flows.t_out + net_flows.sapling_out + net_flows.orchard_out + fee_amount)
.ok_or_else(overflow)?;

let proposed_change = (total_in - total_out).ok_or(ChangeError::InsufficientFunds {
available: total_in,
Expand Down
74 changes: 63 additions & 11 deletions zcash_client_backend/src/fees/zip317.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use zcash_primitives::{
use crate::ShieldedProtocol;

use super::{
common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
DustOutputPolicy, TransactionBalance,
common::{calculate_net_flows, single_change_output_balance, single_change_output_policy},
sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance,
};

#[cfg(feature = "orchard")]
Expand Down Expand Up @@ -93,39 +93,86 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
})
.collect();

#[cfg(feature = "orchard")]
let mut orchard_dust: Vec<NoteRefT> = orchard
.inputs()
.iter()
.filter_map(|i| {
if orchard_fees::InputView::<NoteRefT>::value(i) < self.fee_rule.marginal_fee() {
Some(orchard_fees::InputView::<NoteRefT>::note_id(i).clone())
} else {
None
}
})
.collect();
#[cfg(not(feature = "orchard"))]
let mut orchard_dust: Vec<NoteRefT> = vec![];

// Depending on the shape of the transaction, we may be able to spend up to
// `grace_actions - 1` dust inputs. If we don't have any dust inputs though,
// we don't need to worry about any of that.
if !(transparent_dust.is_empty() && sapling_dust.is_empty()) {
if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty()) {
let t_non_dust = transparent_inputs.len() - transparent_dust.len();
let t_allowed_dust = transparent_outputs.len().saturating_sub(t_non_dust);

// We add one to the sapling outputs for the (single) change output Note that this
// means that wallet-internal shielding transactions are an opportunity to spend a dust
// note.
// We add one to either the Sapling or Orchard outputs for the (single)
// change output. Note that this means that wallet-internal shielding
// transactions are an opportunity to spend a dust note.
let net_flows = calculate_net_flows::<NoteRefT, Self::FeeRule, Self::Error>(
transparent_inputs,
transparent_outputs,
sapling,
#[cfg(feature = "orchard")]
orchard,
)?;
let (_, sapling_change, orchard_change) =
single_change_output_policy::<NoteRefT, Self::FeeRule, Self::Error>(
&net_flows,
self.fallback_change_pool,
)?;

let s_non_dust = sapling.inputs().len() - sapling_dust.len();
let s_allowed_dust = (sapling.outputs().len() + 1).saturating_sub(s_non_dust);
let s_allowed_dust =
(sapling.outputs().len() + sapling_change).saturating_sub(s_non_dust);

#[cfg(feature = "orchard")]
let (orchard_inputs_len, orchard_outputs_len) =
(orchard.inputs().len(), orchard.outputs().len());
#[cfg(not(feature = "orchard"))]
let (orchard_inputs_len, orchard_outputs_len) = (0, 0);

let o_non_dust = orchard_inputs_len - orchard_dust.len();
let o_allowed_dust = (orchard_outputs_len + orchard_change).saturating_sub(o_non_dust);

let available_grace_inputs = self
.fee_rule
.grace_actions()
.saturating_sub(t_non_dust)
.saturating_sub(s_non_dust);
.saturating_sub(s_non_dust)
.saturating_sub(o_non_dust);

let mut t_disallowed_dust = transparent_dust.len().saturating_sub(t_allowed_dust);
let mut s_disallowed_dust = sapling_dust.len().saturating_sub(s_allowed_dust);
let mut o_disallowed_dust = orchard_dust.len().saturating_sub(o_allowed_dust);

if available_grace_inputs > 0 {
// If we have available grace inputs, allocate them first to transparent dust
// and then to Sapling dust. The caller has provided inputs that it is willing
// to spend, so we don't need to consider privacy effects at this layer.
// and then to Sapling dust followed by Orchard dust. The caller has provided
// inputs that it is willing to spend, so we don't need to consider privacy
// effects at this layer.
let t_grace_dust = available_grace_inputs.saturating_sub(t_disallowed_dust);
t_disallowed_dust = t_disallowed_dust.saturating_sub(t_grace_dust);

let s_grace_dust = available_grace_inputs
.saturating_sub(t_grace_dust)
.saturating_sub(s_disallowed_dust);
s_disallowed_dust = s_disallowed_dust.saturating_sub(s_grace_dust);

let o_grace_dust = available_grace_inputs
.saturating_sub(t_grace_dust)
.saturating_sub(s_grace_dust)
.saturating_sub(o_disallowed_dust);
o_disallowed_dust = o_disallowed_dust.saturating_sub(o_grace_dust);
}

// Truncate the lists of inputs to be disregarded in input selection to just the
Expand All @@ -135,11 +182,16 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
transparent_dust.truncate(t_disallowed_dust);
sapling_dust.reverse();
sapling_dust.truncate(s_disallowed_dust);
orchard_dust.reverse();
orchard_dust.truncate(o_disallowed_dust);

if !(transparent_dust.is_empty() && sapling_dust.is_empty()) {
if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty())
{
return Err(ChangeError::DustInputs {
transparent: transparent_dust,
sapling: sapling_dust,
#[cfg(feature = "orchard")]
orchard: orchard_dust,
});
}
}
Expand Down
Loading
Loading