zcash_client_backend: Add validation to the `Proposal::multi_step` constructor.

This commit is contained in:
Kris Nuttycombe 2024-02-15 10:37:58 -07:00
parent dd6711aec1
commit 6aabe60d21
6 changed files with 252 additions and 16 deletions

3
Cargo.lock generated
View File

@ -2105,8 +2105,7 @@ dependencies = [
[[package]]
name = "sapling-crypto"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3f5de898a7cdb7f6d9c8fb888341b6ae6e2aeae88227b7f435f1dda49ecf9e62"
source = "git+https://github.com/zcash/sapling-crypto?rev=346a6507413eb2388b2967c014672cd83199648a#346a6507413eb2388b2967c014672cd83199648a"
dependencies = [
"aes",
"bellman",

View File

@ -115,3 +115,6 @@ zip32 = "0.1"
lto = true
panic = 'abort'
codegen-units = 1
[patch.crates-io]
sapling = { package = "sapling-crypto", git = "https://github.com/zcash/sapling-crypto", rev = "346a6507413eb2388b2967c014672cd83199648a" }

View File

@ -11,7 +11,10 @@ use zcash_primitives::{
memo::MemoBytes,
transaction::{
builder::{BuildConfig, BuildResult, Builder},
components::amount::{Amount, NonNegativeAmount},
components::{
amount::{Amount, NonNegativeAmount},
TxOut,
},
fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
Transaction, TxId,
},
@ -721,9 +724,9 @@ where
.map_err(Error::DataSource)?;
let mut utxos_spent: Vec<OutPoint> = vec![];
let mut add_transparent_input = |addr,
let mut add_transparent_input = |addr: &TransparentAddress,
outpoint: OutPoint,
utxo|
utxo: TxOut|
-> Result<
(),
Error<

View File

@ -101,7 +101,7 @@ pub enum ShieldedProtocol {
}
/// A value pool to which the wallet supports sending transaction outputs.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum PoolType {
/// The transparent value pool
Transparent,

View File

@ -1,20 +1,21 @@
//! Types related to the construction and evaluation of transaction proposals.
use std::{
collections::BTreeMap,
collections::{BTreeMap, BTreeSet},
fmt::{self, Debug, Display},
};
use nonempty::NonEmpty;
use zcash_primitives::{
consensus::BlockHeight, transaction::components::amount::NonNegativeAmount,
consensus::BlockHeight,
transaction::{components::amount::NonNegativeAmount, TxId},
};
use crate::{
fees::TransactionBalance,
wallet::{Note, ReceivedNote, WalletTransparentOutput},
zip321::TransactionRequest,
PoolType,
PoolType, ShieldedProtocol,
};
/// Errors that can occur in construction of a [`Step`].
@ -39,6 +40,10 @@ pub enum ProposalError {
ShieldingInvalid,
/// A reference to the output of a prior step is invalid.
ReferenceError(StepOutput),
/// An attempted double-spend of a prior step output was detected.
StepDoubleSpend(StepOutput),
/// An attempted double-spend of an output belonging to the wallet was detected.
ChainDoubleSpend(PoolType, TxId, u32),
}
impl Display for ProposalError {
@ -65,7 +70,19 @@ impl Display for ProposalError {
f,
"The proposal violates the rules for a shielding transaction."
),
ProposalError::ReferenceError(r) => write!(f, "No prior step output found for {:?}", r),
ProposalError::ReferenceError(r) => {
write!(f, "No prior step output found for reference {:?}", r)
}
ProposalError::StepDoubleSpend(r) => write!(
f,
"The proposal uses the output of step {:?} in more than one place.",
r
),
ProposalError::ChainDoubleSpend(pool, txid, index) => write!(
f,
"The proposal attempts to spend the same output twice: {}, {}, {}",
pool, txid, index
),
}
}
}
@ -131,7 +148,64 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
min_target_height: BlockHeight,
steps: NonEmpty<Step<NoteRef>>,
) -> Result<Self, ProposalError> {
// TODO: actually perform the validation described in the documentation.
let mut consumed_chain_inputs: BTreeSet<(PoolType, TxId, u32)> = BTreeSet::new();
let mut consumed_prior_inputs: BTreeSet<StepOutput> = BTreeSet::new();
for (i, step) in steps.iter().enumerate() {
for prior_ref in step.prior_step_inputs() {
// check that there are no forward references
if prior_ref.step_index() >= i {
return Err(ProposalError::ReferenceError(*prior_ref));
}
// check that the reference is valid
let prior_step = &steps[prior_ref.step_index()];
match prior_ref.output_index() {
StepOutputIndex::Payment(idx) => {
if prior_step.transaction_request().payments().len() <= idx {
return Err(ProposalError::ReferenceError(*prior_ref));
}
}
StepOutputIndex::Change(idx) => {
if prior_step.balance().proposed_change().len() <= idx {
return Err(ProposalError::ReferenceError(*prior_ref));
}
}
}
// check that there are no double-spends
if consumed_prior_inputs.contains(prior_ref) {
return Err(ProposalError::StepDoubleSpend(*prior_ref));
}
consumed_prior_inputs.insert(*prior_ref);
}
for t_out in step.transparent_inputs() {
let key = (
PoolType::Transparent,
TxId::from_bytes(*t_out.outpoint().hash()),
t_out.outpoint().n(),
);
if consumed_chain_inputs.contains(&key) {
return Err(ProposalError::ChainDoubleSpend(key.0, key.1, key.2));
}
consumed_chain_inputs.insert(key);
}
for s_out in step.shielded_inputs().iter().flat_map(|i| i.notes().iter()) {
let key = (
match &s_out.note() {
Note::Sapling(_) => PoolType::Shielded(ShieldedProtocol::Sapling),
#[cfg(feature = "orchard")]
Note::Orchard(_) => PoolType::Shielded(ShieldedProtocol::Orchard),
},
*s_out.txid(),
s_out.output_index().into(),
);
if consumed_chain_inputs.contains(&key) {
return Err(ProposalError::ChainDoubleSpend(key.0, key.1, key.2));
}
consumed_chain_inputs.insert(key);
}
}
Ok(Self {
fee_rule,
@ -214,14 +288,14 @@ impl<FeeRuleT: Debug, NoteRef> Debug for Proposal<FeeRuleT, NoteRef> {
}
/// A reference to either a payment or change output within a step.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum StepOutputIndex {
Payment(usize),
Change(usize),
}
/// A reference to the output of a step in a proposal.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct StepOutput {
step_index: usize,
output_index: StepOutputIndex,

View File

@ -521,8 +521,13 @@ pub(crate) mod tests {
#[cfg(feature = "transparent-inputs")]
use {
zcash_client_backend::wallet::WalletTransparentOutput,
zcash_primitives::transaction::components::{OutPoint, TxOut},
zcash_client_backend::{
fees::TransactionBalance, proposal::Step, wallet::WalletTransparentOutput, PoolType,
},
zcash_primitives::{
legacy::keys::IncomingViewingKey,
transaction::components::{OutPoint, TxOut},
},
};
pub(crate) fn test_prover() -> impl SpendProver + OutputProver {
@ -530,7 +535,7 @@ pub(crate) mod tests {
}
#[test]
fn send_proposed_transfer() {
fn send_single_step_proposed_transfer() {
let mut st = TestBuilder::new()
.with_block_cache()
.with_test_account(AccountBirthday::from_sapling_activation)
@ -677,6 +682,158 @@ pub(crate) mod tests {
);
}
#[test]
#[cfg(feature = "transparent-inputs")]
fn send_multi_step_proposed_transfer() {
use nonempty::NonEmpty;
use zcash_client_backend::proposal::{Proposal, StepOutput, StepOutputIndex};
let mut st = TestBuilder::new()
.with_block_cache()
.with_test_account(AccountBirthday::from_sapling_activation)
.build();
let (account, usk, _) = st.test_account().unwrap();
let dfvk = st.test_account_sapling().unwrap();
// Add funds to the wallet in a single note
let value = NonNegativeAmount::const_from_u64(65000);
let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value);
st.scan_cached_blocks(h, 1);
// Spendable balance matches total balance
assert_eq!(st.get_total_balance(account), value);
assert_eq!(st.get_spendable_balance(account, 1), value);
assert_eq!(
block_max_scanned(&st.wallet().conn, &st.wallet().params)
.unwrap()
.unwrap()
.block_height(),
h
);
// Generate a single-step proposal. Then, instead of executing that proposal,
// we will use its only step as the first step in a multi-step proposal that
// spends the first step's output.
// The first step will deshield to the wallet's default transparent address
let to0 = Address::Transparent(usk.default_transparent_address().0);
let request0 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: to0,
amount: NonNegativeAmount::const_from_u64(50000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
.unwrap();
let fee_rule = StandardFeeRule::Zip317;
let input_selector = GreedyInputSelector::new(
standard::SingleOutputChangeStrategy::new(fee_rule, None),
DustOutputPolicy::default(),
);
let proposal0 = st
.propose_transfer(
account,
&input_selector,
request0,
NonZeroU32::new(1).unwrap(),
)
.unwrap();
let min_target_height = proposal0.min_target_height();
let step0 = &proposal0.steps().head;
assert!(step0.balance().proposed_change().is_empty());
assert_eq!(
step0.balance().fee_required(),
NonNegativeAmount::const_from_u64(15000)
);
// We'll use an internal transparent address that hasn't been added to the wallet
// to simulate an external transparent recipient.
let to1 = Address::Transparent(
usk.transparent()
.to_account_pubkey()
.derive_internal_ivk()
.unwrap()
.default_address()
.0,
);
let request1 = zip321::TransactionRequest::new(vec![Payment {
recipient_address: to1,
amount: NonNegativeAmount::const_from_u64(40000),
memo: None,
label: None,
message: None,
other_params: vec![],
}])
.unwrap();
let step1 = Step::from_parts(
&[step0.clone()],
request1,
[(0, PoolType::Transparent)].into_iter().collect(),
vec![],
None,
vec![StepOutput::new(0, StepOutputIndex::Payment(0))],
TransactionBalance::new(vec![], NonNegativeAmount::const_from_u64(10000)).unwrap(),
false,
)
.unwrap();
let proposal = Proposal::multi_step(
fee_rule,
min_target_height,
NonEmpty::from_vec(vec![step0.clone(), step1]).unwrap(),
)
.unwrap();
let create_proposed_result =
st.create_proposed_transactions::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal);
assert_matches!(&create_proposed_result, Ok(txids) if txids.len() == 2);
let txids = create_proposed_result.unwrap();
// Verify that the stored sent outputs match what we're expecting
let mut stmt_sent = st
.wallet()
.conn
.prepare(
"SELECT value
FROM sent_notes
JOIN transactions ON transactions.id_tx = sent_notes.tx
WHERE transactions.txid = ?",
)
.unwrap();
let confirmed_sent = txids
.iter()
.map(|sent_txid| {
// check that there's a sent output with the correct value corresponding to
stmt_sent
.query(rusqlite::params![sent_txid.as_ref()])
.unwrap()
.mapped(|row| {
let value: u32 = row.get(0)?;
Ok((sent_txid, value))
})
.collect::<Result<Vec<_>, _>>()
.unwrap()
})
.collect::<Vec<_>>();
assert_eq!(
confirmed_sent.get(0).and_then(|v| v.get(0)),
Some(&(&txids[0], 50000))
);
assert_eq!(
confirmed_sent.get(1).and_then(|v| v.get(0)),
Some(&(&txids[1], 40000))
);
}
#[test]
#[allow(deprecated)]
fn create_to_address_fails_on_incorrect_usk() {