zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Improve discrimination of proposal errors. 

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Daira-Emma Hopwood 2024-06-27 12:22:30 +01:00
parent 81a2846593
commit f0e5aab692
4 changed files with 70 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
zcash_client_backend/src/data_api/error.rs
View File

@ -37,11 +37,10 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
Proposal(ProposalError),
/// The proposal was structurally valid, but tries to do one of these unsupported things:
/// * spending a prior shielded output or non-ephemeral change output;
/// * leaving an ephemeral output unspent;
/// * spending a prior shielded output;
/// * paying to an output pool for which the corresponding feature is not enabled;
/// * paying to a TEX address if the "transparent-inputs" feature is not enabled;
/// * paying to a TEX address in a transaction that has shielded inputs.
/// * exceeding implementation limits.
ProposalNotSupported,
/// No account could be found corresponding to a provided spending key.
@ -120,12 +119,12 @@ where
Error::Proposal(e) => {
write!(f, "Input selection attempted to construct an invalid proposal: {}", e)
}
Error::ProposalNotSupported => {
write!(
f,
"The proposal was valid, but spending shielded outputs of prior transaction steps is not yet supported."
)
}
Error::ProposalNotSupported => write!(
f,
"The proposal was valid but tried to do something that is not supported \
(spending shielded outputs of prior transaction steps, using a feature \
that is not enabled, or exceeding an implementation limit).",
),
Error::KeyNotRecognized => {
write!(
f,
@ -191,6 +190,12 @@ impl<DE, CE, SE, FE> From<builder::Error<FE>> for Error<DE, CE, SE, FE> {
}
}
impl<DE, CE, SE, FE> From<ProposalError> for Error<DE, CE, SE, FE> {
fn from(e: ProposalError) -> Self {
Error::Proposal(e)
}
}
impl<DE, CE, SE, FE> From<BalanceError> for Error<DE, CE, SE, FE> {
fn from(e: BalanceError) -> Self {
Error::BalanceError(e)

63
zcash_client_backend/src/data_api/wallet.rs
View File

@ -51,7 +51,7 @@ use crate::{
decrypt_transaction,
fees::{self, DustOutputPolicy},
keys::UnifiedSpendingKey,
proposal::{Proposal, Step, StepOutputIndex},
proposal::{Proposal, ProposalError, Step, StepOutputIndex},
wallet::{Note, OvkPolicy, Recipient},
zip321::{self, Payment},
PoolType, ShieldedProtocol,
@ -73,11 +73,7 @@ use zip32::Scope;
#[cfg(feature = "transparent-inputs")]
use {
crate::{
fees::ChangeValue,
proposal::{ProposalError, StepOutput},
wallet::TransparentAddressMetadata,
},
crate::{fees::ChangeValue, proposal::StepOutput, wallet::TransparentAddressMetadata},
core::convert::Infallible,
input_selection::ShieldingSelector,
std::collections::HashMap,
@ -631,15 +627,13 @@ where
// Ephemeral outputs must be referenced exactly once.
#[cfg(feature = "transparent-inputs")]
if unused_transparent_outputs.into_keys().any(|s: StepOutput| {
if let StepOutputIndex::Change(i) = s.output_index() {
// indexing has already been checked
step_results[s.step_index()].0.balance().proposed_change()[i].is_ephemeral()
} else {
false
for so in unused_transparent_outputs.into_keys() {
if let StepOutputIndex::Change(i) = so.output_index() {
// references have already been checked
if step_results[so.step_index()].0.balance().proposed_change()[i].is_ephemeral() {
return Err(ProposalError::EphemeralOutputLeftUnspent(so).into());
}
}
}) {
return Err(Error::ProposalNotSupported);
}
Ok(NonEmpty::from_vec(
@ -674,7 +668,7 @@ where
ParamsT: consensus::Parameters + Clone,
FeeRuleT: FeeRule,
{
#[allow(unused_variables)]
#[cfg(feature = "transparent-inputs")]
let step_index = prior_step_results.len();
// We only support spending transparent payments or transparent ephemeral outputs from a
@ -683,32 +677,27 @@ 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.
#[cfg(feature = "transparent-inputs")]
for input_ref in proposal_step.prior_step_inputs() {
let supported = prior_step_results
let (prior_step, _) = prior_step_results
.get(input_ref.step_index())
.and_then(|(prior_step, _)| match input_ref.output_index() {
StepOutputIndex::Payment(i) => prior_step
.payment_pools()
.get(&i)
.map(|&pool| pool == PoolType::TRANSPARENT),
StepOutputIndex::Change(i) => {
prior_step.balance().proposed_change().get(i).map(|change| {
change.is_ephemeral() && change.output_pool() == PoolType::TRANSPARENT
})
}
})
.ok_or(Error::Proposal(ProposalError::ReferenceError(*input_ref)))?;
.ok_or(ProposalError::ReferenceError(*input_ref))?;
// Return an error on trying to spend a prior shielded output or non-ephemeral change output.
if !supported {
let output_pool = match input_ref.output_index() {
StepOutputIndex::Payment(i) => prior_step.payment_pools().get(&i).cloned(),
StepOutputIndex::Change(i) => match prior_step.balance().proposed_change().get(i) {
Some(change) if !change.is_ephemeral() => {
return Err(ProposalError::SpendsChange(*input_ref).into());
}
other => other.map(|change| change.output_pool()),
},
}
.ok_or(ProposalError::ReferenceError(*input_ref))?;
// Return an error on trying to spend a prior shielded output.
if output_pool != PoolType::TRANSPARENT {
return Err(Error::ProposalNotSupported);
}
}
#[cfg(not(feature = "transparent-inputs"))]
if !proposal_step.prior_step_inputs().is_empty() {
return Err(Error::ProposalNotSupported);
}
let account_id = wallet_db
.get_account_for_ufvk(&usk.to_unified_full_viewing_key())
@ -892,7 +881,7 @@ where
.1
.transaction()
.transparent_bundle()
.ok_or(Error::Proposal(ProposalError::ReferenceError(*input_ref)))?
.ok_or(ProposalError::ReferenceError(*input_ref))?
.vout[outpoint.n() as usize];
add_transparent_input(
@ -1070,7 +1059,7 @@ where
#[cfg(feature = "transparent-inputs")]
Address::Tex(data) => {
if has_shielded_inputs {
return Err(Error::ProposalNotSupported);
return Err(ProposalError::PaysTexFromShielded.into());
}
let to = TransparentAddress::PublicKeyHash(data);
add_transparent_output(&mut builder, &mut transparent_output_meta, to)?;

24
zcash_client_backend/src/proposal.rs
View File

@ -47,6 +47,14 @@ pub enum ProposalError {
/// There was a mismatch between the payments in the proposal's transaction request
/// and the payment pool selection values.
PaymentPoolsMismatch,
/// The proposal tried to spend a change output. Mark the `ChangeValue` as ephemeral if this is intended.
SpendsChange(StepOutput),
/// A proposal step created an ephemeral output that was not spent in any later step.
#[cfg(feature = "transparent-inputs")]
EphemeralOutputLeftUnspent(StepOutput),
/// The proposal included a payment to a TEX address and a spend from a shielded input in the same step.
#[cfg(feature = "transparent-inputs")]
PaysTexFromShielded,
}
impl Display for ProposalError {
@ -90,6 +98,22 @@ impl Display for ProposalError {
f,
"The chosen payment pools did not match the payments of the transaction request."
),
ProposalError::SpendsChange(r) => write!(
f,
"The proposal attempts to spends the change output created at step {:?}.",
r,
),
#[cfg(feature = "transparent-inputs")]
ProposalError::EphemeralOutputLeftUnspent(r) => write!(
f,
"The proposal created an ephemeral output at step {:?} that was not spent in any later step.",
r,
),
#[cfg(feature = "transparent-inputs")]
ProposalError::PaysTexFromShielded => write!(
f,
"The proposal included a payment to a TEX address and a spend from a shielded input in the same step.",
),
}
}
}

8
zcash_client_sqlite/src/testing/pool.rs
View File

@ -482,7 +482,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
#[cfg(feature = "transparent-inputs")]
pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed<T: ShieldedPoolTester>() {
use nonempty::NonEmpty;
use zcash_client_backend::proposal::Proposal;
use zcash_client_backend::proposal::{Proposal, ProposalError, StepOutput, StepOutputIndex};
let mut st = TestBuilder::new()
.with_block_cache()
@ -556,7 +556,11 @@ pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed<T: ShieldedPo
OvkPolicy::Sender,
&frobbed_proposal,
);
assert_matches!(create_proposed_result, Err(Error::ProposalNotSupported));
assert_matches!(
create_proposed_result,
Err(Error::Proposal(ProposalError::EphemeralOutputLeftUnspent(so)))
if so == StepOutput::new(0, StepOutputIndex::Change(1))
);
}
#[allow(deprecated)]