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

Apply suggestions from code review 

Co-authored-by: str4d <thestr4d@gmail.com>
This commit is contained in:
Kris Nuttycombe 2024-02-15 19:44:46 -07:00 committed by Kris Nuttycombe
parent 6aabe60d21
commit 74b487e4c9
7 changed files with 96 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
Cargo.lock generated
View File

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

5
Cargo.toml
View File

@ -49,7 +49,7 @@ bitvec = "1"
blake2s_simd = "1"
bls12_381 = "0.8"
jubjub = "0.10"
sapling = { package = "sapling-crypto", version = "0.1" }
sapling = { package = "sapling-crypto", version = "0.1.1" }
# - Orchard
nonempty = "0.7"
@ -115,6 +115,3 @@ 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" }

6
zcash_client_backend/CHANGELOG.md
View File

@ -35,6 +35,7 @@ and this library adheres to Rust's notion of
## [0.11.0-pre-release] Unreleased
### Added
- `zcash_client_backend::PoolType::is_receiver`
- `zcash_client_backend::data_api`:
- `InputSource`
- `ScannedBlock::{into_commitments, sapling}`
@ -206,6 +207,11 @@ and this library adheres to Rust's notion of
it possible to represent multi-step transactions, such as a deshielding
transaction followed by a zero-conf transfer as required by ZIP 320. Individual
transaction proposals are now represented by the `proposal::Step` type.
- `ProposalError` has new variants:
- `ReferenceError`
- `StepDoubleSpend`
- `ChainDoubleSpend`
- `PaymentPoolsMismatch`
- `zcash_client_backend::fees`:
- `ChangeStrategy::compute_balance` arguments have changed.

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

@ -11,10 +11,7 @@ use zcash_primitives::{
memo::MemoBytes,
transaction::{
builder::{BuildConfig, BuildResult, Builder},
components::{
amount::{Amount, NonNegativeAmount},
TxOut,
},
components::amount::{Amount, NonNegativeAmount},
fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
Transaction, TxId,
},
@ -30,6 +27,7 @@ use crate::{
decrypt_transaction,
fees::{self, DustOutputPolicy},
keys::UnifiedSpendingKey,
proposal::ProposalError,
proposal::{self, Proposal},
wallet::{Note, OvkPolicy, Recipient},
zip321::{self, Payment},
@ -45,10 +43,12 @@ use super::InputSource;
#[cfg(feature = "transparent-inputs")]
use {
crate::proposal::ProposalError, input_selection::ShieldingSelector,
sapling::keys::OutgoingViewingKey, std::convert::Infallible,
zcash_keys::encoding::AddressCodec, zcash_primitives::legacy::TransparentAddress,
zcash_primitives::transaction::components::OutPoint,
input_selection::ShieldingSelector,
sapling::keys::OutgoingViewingKey,
std::convert::Infallible,
zcash_keys::encoding::AddressCodec,
zcash_primitives::legacy::TransparentAddress,
zcash_primitives::transaction::components::{OutPoint, TxOut},
};
/// Scans a [`Transaction`] for any information that can be decrypted by the accounts in
@ -522,13 +522,14 @@ where
/// Construct, prove, and sign a transaction or series of transactions using the inputs supplied by
/// the given proposal, and persist it to the wallet database.
///
/// Returns the database identifier for eacy newly constructed transaction, or an error if
/// Returns the database identifier for each newly constructed transaction, or an error if
/// an error occurs in transaction construction, proving, or signing.
///
/// When evaluating multi-step proposals, only transparent outputs of any given step may be spent
/// in later steps; attempting to spend a shielded note (including change) output by an earlier
/// step is not supported, because the ultimate positions of those notes cannot be known and
/// therefore the required spend proofs for such notes cannot be constructed.
/// step is not supported, because the ultimate positions of those notes in the global note
/// commitment tree cannot be known until the transaction that produces those notes is mined,
/// and therefore the required spend proofs for such notes cannot be constructed.
#[allow(clippy::too_many_arguments)]
#[allow(clippy::type_complexity)]
pub fn create_proposed_transactions<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
@ -610,24 +611,33 @@ where
// supported. Maybe support this 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.
if proposal_step.prior_step_inputs().iter().any(|s_ref| {
prior_step_results.len() <= s_ref.step_index()
|| match s_ref.output_index() {
proposal::StepOutputIndex::Payment(i) => prior_step_results[s_ref.step_index()]
for s_ref in proposal_step.prior_step_inputs() {
prior_step_results.get(s_ref.step_index()).map_or_else(
|| {
// Return an error in case the step index doesn't match up with a step
Err(Error::Proposal(ProposalError::ReferenceError(*s_ref)))
},
|step| match s_ref.output_index() {
proposal::StepOutputIndex::Payment(i) => {
let prior_pool = step
.0
.payment_pools()
.get(&i)
.iter()
.all(|pool| matches!(pool, PoolType::Shielded(_))),
.ok_or(Error::Proposal(ProposalError::ReferenceError(*s_ref)))?;
if matches!(prior_pool, PoolType::Shielded(_)) {
Err(Error::ProposalNotSupported)
} else {
Ok(())
}
}
proposal::StepOutputIndex::Change(_) => {
// Only shielded change is supported by zcash_client_backend, so multi-step
// transactions cannot yet spend prior transactions' change outputs.
true
Err(Error::ProposalNotSupported)
}
}
}) {
return Err(Error::ProposalNotSupported);
},
)?;
}
let account = wallet_db

22
zcash_client_backend/src/lib.rs
View File

@ -64,6 +64,7 @@
pub use zcash_keys::address;
pub mod data_api;
mod decrypt;
use zcash_keys::address::Address;
pub use zcash_keys::encoding;
pub mod fees;
pub use zcash_keys::keys;
@ -109,6 +110,27 @@ pub enum PoolType {
Shielded(ShieldedProtocol),
}
impl PoolType {
pub fn is_receiver(&self, addr: &Address) -> bool {
match addr {
Address::Sapling(_) => matches!(self, PoolType::Shielded(ShieldedProtocol::Sapling)),
Address::Transparent(_) => matches!(self, PoolType::Transparent),
Address::Unified(ua) => match self {
PoolType::Transparent => ua.transparent().is_some(),
PoolType::Shielded(ShieldedProtocol::Sapling) => ua.sapling().is_some(),
#[cfg(zcash_unstable = "orchard")]
PoolType::Shielded(ShieldedProtocol::Orchard) => {
#[cfg(feature = "orchard")]
return ua.orchard().is_some();
#[cfg(not(feature = "orchard"))]
return false;
}
},
}
}
}
impl fmt::Display for PoolType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {

37
zcash_client_backend/src/proposal.rs
View File

@ -44,6 +44,9 @@ pub enum ProposalError {
StepDoubleSpend(StepOutput),
/// An attempted double-spend of an output belonging to the wallet was detected.
ChainDoubleSpend(PoolType, TxId, u32),
/// There was a mismatch between the payments in the proposal's transaction request
/// and the payment pool selection values.
PaymentPoolsMismatch,
}
impl Display for ProposalError {
@ -83,6 +86,10 @@ impl Display for ProposalError {
"The proposal attempts to spend the same output twice: {}, {}, {}",
pool, txid, index
),
ProposalError::PaymentPoolsMismatch => write!(
f,
"The chosen payment pools did not match the payments of the transaction request."
),
}
}
}
@ -172,10 +179,9 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
}
}
// check that there are no double-spends
if consumed_prior_inputs.contains(prior_ref) {
if !consumed_prior_inputs.insert(*prior_ref) {
return Err(ProposalError::StepDoubleSpend(*prior_ref));
}
consumed_prior_inputs.insert(*prior_ref);
}
for t_out in step.transparent_inputs() {
@ -184,10 +190,9 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
TxId::from_bytes(*t_out.outpoint().hash()),
t_out.outpoint().n(),
);
if consumed_chain_inputs.contains(&key) {
if !consumed_chain_inputs.insert(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()) {
@ -200,10 +205,9 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
*s_out.txid(),
s_out.output_index().into(),
);
if consumed_chain_inputs.contains(&key) {
if !consumed_chain_inputs.insert(key) {
return Err(ProposalError::ChainDoubleSpend(key.0, key.1, key.2));
}
consumed_chain_inputs.insert(key);
}
}
@ -343,7 +347,11 @@ impl<NoteRef> Step<NoteRef> {
/// Parameters:
/// * `transaction_request`: The ZIP 321 transaction request describing the payments
/// to be made.
/// * `payment_pools`: A map from payment index to pool type.
/// * `payment_pools`: A map from payment index to pool type. The set of payment indices
/// provided here must exactly match the set of payment indices in the [`TransactionRequest`],
/// and the selected pool for an index must correspond to a valid receiver of the
/// address at that index (or the address itself in the case of bare transparent or Sapling
/// addresses).
/// * `transparent_inputs`: The set of previous transparent outputs to be spent.
/// * `shielded_inputs`: The sets of previous shielded outputs to be spent.
/// * `balance`: The change outputs to be added the transaction and the fee to be paid.
@ -360,6 +368,21 @@ impl<NoteRef> Step<NoteRef> {
balance: TransactionBalance,
is_shielding: bool,
) -> Result<Self, ProposalError> {
// Verify that the set of payment pools matches exactly a set of valid payment recipients
if transaction_request.payments().len() != payment_pools.len() {
return Err(ProposalError::PaymentPoolsMismatch);
}
for (idx, pool) in &payment_pools {
if !transaction_request
.payments()
.get(idx)
.iter()
.any(|payment| pool.is_receiver(&payment.recipient_address))
{
return Err(ProposalError::PaymentPoolsMismatch);
}
}
let transparent_input_total = transparent_inputs
.iter()
.map(|out| out.txout().value)

1
zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs
View File

@ -2,7 +2,6 @@
use std::collections::HashSet;
use rusqlite::{self, types::ToSql, OptionalExtension};
use schemer;
use schemer_rusqlite::RusqliteMigration;
use uuid::Uuid;