Use `NonZeroU32` for all `min_confirmations` values.

This commit is contained in:
Kris Nuttycombe 2023-06-30 12:37:41 -06:00
parent 8625e9a777
commit e225a54d2e
7 changed files with 78 additions and 61 deletions

View File

@ -30,6 +30,7 @@ and this library adheres to Rust's notion of
wallet states where the full note plaintext is not available.
- `WalletRead::get_nullifiers` has been renamed to `WalletRead::get_sapling_nullifiers`
and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`.
- `WalletRead::get_target_and_anchor_heights` now takes its argument as a `NonZeroU32`
- `chain::scan_cached_blocks` now takes a `from_height` argument that
permits the caller to control the starting position of the scan range.
- A new `CommitmentTree` variant has been added to `data_api::error::Error`
@ -38,6 +39,11 @@ and this library adheres to Rust's notion of
implemented for the type passed to them for the `wallet_db` parameter.
- `data_api::wallet::create_proposed_transaction` now takes an additional
`min_confirmations` argument.
- `data_api::wallet::{spend, create_spend_to_address, shield_transparent_funds,
propose_transfer, propose_shielding, create_proposed_transaction}` now take their
respective `min_confirmations` arguments as `NonZeroU32`
- `data_api::wallet::input_selection::InputSelector::{propose_transaction, propose_shielding}`
now take their respective `min_confirmations` arguments as `NonZeroU32`
- A new `Sync` variant has been added to `data_api::chain::error::Error`.
- A new `SyncRequired` variant has been added to `data_api::wallet::input_selection::InputSelectorError`.
- `zcash_client_backend::wallet`:

View File

@ -2,6 +2,7 @@
use std::collections::HashMap;
use std::fmt::Debug;
use std::num::NonZeroU32;
use std::{cmp, ops::Range};
use incrementalmerkletree::Retention;
@ -97,7 +98,7 @@ pub trait WalletRead {
/// This will return `Ok(None)` if no block data is present in the database.
fn get_target_and_anchor_heights(
&self,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<Option<(BlockHeight, BlockHeight)>, Self::Error> {
self.block_height_extrema().map(|heights| {
heights.map(|(min_height, max_height)| {
@ -106,7 +107,7 @@ pub trait WalletRead {
// Select an anchor min_confirmations back from the target block,
// unless that would be before the earliest block we have.
let anchor_height = BlockHeight::from(cmp::max(
u32::from(target_height).saturating_sub(min_confirmations),
u32::from(target_height).saturating_sub(min_confirmations.into()),
u32::from(min_height),
));

View File

@ -1,4 +1,4 @@
use std::convert::Infallible;
use std::{convert::Infallible, num::NonZeroU32};
use std::fmt::Debug;
use shardtree::{ShardStore, ShardTree, ShardTreeError};
@ -122,10 +122,6 @@ where
/// spent. A value of 10 confirmations is recommended and 0-conf transactions are
/// not supported.
///
/// # Panics
///
/// Panics if `min_confirmations == 0`; 0-conf transactions are not supported.
///
/// # Examples
///
/// ```
@ -203,7 +199,7 @@ pub fn create_spend_to_address<DbT, ParamsT>(
amount: Amount,
memo: Option<MemoBytes>,
ovk_policy: OvkPolicy,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<
DbT::TxRef,
Error<
@ -292,7 +288,8 @@ where
/// 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.
/// spent. A value of 10 confirmations is recommended and 0-conf transactions are
/// not supported.
///
/// [`sapling::TxProver`]: zcash_primitives::sapling::prover::TxProver
#[allow(clippy::too_many_arguments)]
@ -305,7 +302,7 @@ pub fn spend<DbT, ParamsT, InputsT>(
usk: &UnifiedSpendingKey,
request: zip321::TransactionRequest,
ovk_policy: OvkPolicy,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<
DbT::TxRef,
Error<
@ -323,10 +320,6 @@ where
ParamsT: consensus::Parameters + Clone,
InputsT: InputSelector<DataSource = DbT>,
{
assert!(
min_confirmations > 0,
"zero-conf transactions are not supported"
);
let account = wallet_db
.get_account_for_ufvk(&usk.to_unified_full_viewing_key())
.map_err(Error::DataSource)?
@ -364,7 +357,7 @@ pub fn propose_transfer<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
spend_from_account: AccountId,
input_selector: &InputsT,
request: zip321::TransactionRequest,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<
Proposal<InputsT::FeeRule, DbT::NoteRef>,
Error<
@ -381,10 +374,6 @@ where
ParamsT: consensus::Parameters + Clone,
InputsT: InputSelector<DataSource = DbT>,
{
assert!(
min_confirmations > 0,
"zero-conf transactions are not supported"
);
input_selector
.propose_transaction(
params,
@ -405,7 +394,7 @@ pub fn propose_shielding<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
input_selector: &InputsT,
shielding_threshold: NonNegativeAmount,
from_addrs: &[TransparentAddress],
min_confirmations: u32,
min_confirmations: NonZeroU32
) -> Result<
Proposal<InputsT::FeeRule, DbT::NoteRef>,
Error<
@ -422,10 +411,6 @@ where
DbT::NoteRef: Copy + Eq + Ord,
InputsT: InputSelector<DataSource = DbT>,
{
assert!(
min_confirmations > 0,
"zero-conf transactions are not supported"
);
input_selector
.propose_shielding(
params,
@ -451,7 +436,7 @@ pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
usk: &UnifiedSpendingKey,
ovk_policy: OvkPolicy,
proposal: Proposal<FeeRuleT, DbT::NoteRef>,
min_confirmations: u32,
min_confirmations: NonZeroU32,
change_memo: Option<MemoBytes>,
) -> Result<
DbT::TxRef,
@ -470,10 +455,6 @@ where
ParamsT: consensus::Parameters + Clone,
FeeRuleT: FeeRule,
{
assert!(
min_confirmations > 0,
"zero-conf transactions are not supported"
);
let account = wallet_db
.get_account_for_ufvk(&usk.to_unified_full_viewing_key())
.map_err(Error::DataSource)?
@ -516,8 +497,7 @@ where
selected,
usk.sapling(),
&dfvk,
usize::try_from(min_confirmations - 1)
.expect("min_confirmations should never be anywhere close to usize::MAX"),
usize::try_from(u32::from(min_confirmations) - 1).unwrap()
)?
.ok_or(Error::NoteMismatch(selected.note_id))?;
@ -711,8 +691,9 @@ where
/// to the wallet that the wallet can use to improve how it represents those
/// shielding transactions to the user.
/// * `min_confirmations`: The minimum number of confirmations that a previously
/// received UTXO must have in the blockchain in order to be considered for being
/// spent.
/// received note must have in the blockchain in order to be considered for being
/// spent. A value of 10 confirmations is recommended and 0-conf transactions are
/// not supported.
///
/// [`sapling::TxProver`]: zcash_primitives::sapling::prover::TxProver
#[cfg(feature = "transparent-inputs")]
@ -727,7 +708,7 @@ pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
usk: &UnifiedSpendingKey,
from_addrs: &[TransparentAddress],
memo: &MemoBytes,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<
DbT::TxRef,
Error<

View File

@ -2,6 +2,7 @@
use core::marker::PhantomData;
use std::fmt;
use std::num::NonZeroU32;
use std::{collections::BTreeSet, fmt::Debug};
use zcash_primitives::{
@ -180,7 +181,7 @@ pub trait InputSelector {
wallet_db: &Self::DataSource,
account: AccountId,
transaction_request: TransactionRequest,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<
Proposal<Self::FeeRule, <<Self as InputSelector>::DataSource as WalletRead>::NoteRef>,
InputSelectorError<<<Self as InputSelector>::DataSource as WalletRead>::Error, Self::Error>,
@ -204,7 +205,7 @@ pub trait InputSelector {
wallet_db: &Self::DataSource,
shielding_threshold: NonNegativeAmount,
source_addrs: &[TransparentAddress],
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<
Proposal<Self::FeeRule, <<Self as InputSelector>::DataSource as WalletRead>::NoteRef>,
InputSelectorError<<<Self as InputSelector>::DataSource as WalletRead>::Error, Self::Error>,
@ -324,7 +325,7 @@ where
wallet_db: &Self::DataSource,
account: AccountId,
transaction_request: TransactionRequest,
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<Proposal<Self::FeeRule, DbT::NoteRef>, InputSelectorError<DbT::Error, Self::Error>>
where
ParamsT: consensus::Parameters,
@ -442,7 +443,7 @@ where
wallet_db: &Self::DataSource,
shielding_threshold: NonNegativeAmount,
source_addrs: &[TransparentAddress],
min_confirmations: u32,
min_confirmations: NonZeroU32,
) -> Result<Proposal<Self::FeeRule, DbT::NoteRef>, InputSelectorError<DbT::Error, Self::Error>>
where
ParamsT: consensus::Parameters,

View File

@ -265,6 +265,8 @@ where
#[cfg(test)]
#[allow(deprecated)]
mod tests {
use std::num::NonZeroU32;
use secrecy::Secret;
use tempfile::NamedTempFile;
@ -681,7 +683,7 @@ mod tests {
&usk,
req,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Ok(_)
);

View File

@ -544,6 +544,8 @@ pub(crate) fn block_height_extrema(
pub(crate) fn fully_scanned_height(
conn: &rusqlite::Connection,
) -> Result<Option<(BlockHeight, CommitmentTreeMeta)>, SqliteClientError> {
// FIXME: this will need to be rewritten once out-of-order scan range suggestion
// is implemented.
let res_opt = conn
.query_row(
"SELECT height, sapling_commitment_tree_size, sapling_tree
@ -1155,6 +1157,8 @@ pub(crate) fn put_sent_output<P: consensus::Parameters>(
#[cfg(test)]
mod tests {
use std::num::NonZeroU32;
use secrecy::Secret;
use tempfile::NamedTempFile;
@ -1197,7 +1201,12 @@ mod tests {
);
// We can't get an anchor height, as we have not scanned any blocks.
assert_eq!(db_data.get_target_and_anchor_heights(10).unwrap(), None);
assert_eq!(
db_data
.get_target_and_anchor_heights(NonZeroU32::new(10).unwrap())
.unwrap(),
None
);
// An invalid account has zero balance
assert_matches!(

View File

@ -367,6 +367,8 @@ pub(crate) fn put_received_note<T: ReceivedSaplingOutput>(
#[cfg(test)]
#[allow(deprecated)]
pub(crate) mod tests {
use std::num::NonZeroU32;
use rusqlite::Connection;
use secrecy::Secret;
use tempfile::NamedTempFile;
@ -461,7 +463,7 @@ pub(crate) mod tests {
Amount::from_u64(1).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Err(data_api::error::Error::KeyNotRecognized)
);
@ -490,7 +492,7 @@ pub(crate) mod tests {
Amount::from_u64(1).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Err(data_api::error::Error::ScanRequired)
);
@ -533,7 +535,7 @@ pub(crate) mod tests {
Amount::from_u64(1).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Err(data_api::error::Error::InsufficientFunds {
available,
@ -572,7 +574,10 @@ pub(crate) mod tests {
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
// Verified balance matches total balance
let (_, anchor_height) = db_data.get_target_and_anchor_heights(10).unwrap().unwrap();
let (_, anchor_height) = db_data
.get_target_and_anchor_heights(NonZeroU32::new(10).unwrap())
.unwrap()
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
value
@ -595,7 +600,10 @@ pub(crate) mod tests {
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
// Verified balance does not include the second note
let (_, anchor_height2) = db_data.get_target_and_anchor_heights(10).unwrap().unwrap();
let (_, anchor_height2) = db_data
.get_target_and_anchor_heights(NonZeroU32::new(10).unwrap())
.unwrap()
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
(value + value).unwrap()
@ -618,7 +626,7 @@ pub(crate) mod tests {
Amount::from_u64(70000).unwrap(),
None,
OvkPolicy::Sender,
10,
NonZeroU32::new(10).unwrap(),
),
Err(data_api::error::Error::InsufficientFunds {
available,
@ -654,7 +662,7 @@ pub(crate) mod tests {
Amount::from_u64(70000).unwrap(),
None,
OvkPolicy::Sender,
10,
NonZeroU32::new(10).unwrap(),
),
Err(data_api::error::Error::InsufficientFunds {
available,
@ -687,7 +695,7 @@ pub(crate) mod tests {
Amount::from_u64(70000).unwrap(),
None,
OvkPolicy::Sender,
10,
NonZeroU32::new(10).unwrap(),
),
Ok(_)
);
@ -738,7 +746,7 @@ pub(crate) mod tests {
Amount::from_u64(15000).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Ok(_)
);
@ -754,7 +762,7 @@ pub(crate) mod tests {
Amount::from_u64(2000).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Err(data_api::error::Error::InsufficientFunds {
available,
@ -789,7 +797,7 @@ pub(crate) mod tests {
Amount::from_u64(2000).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Err(data_api::error::Error::InsufficientFunds {
available,
@ -820,7 +828,7 @@ pub(crate) mod tests {
Amount::from_u64(2000).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
)
.unwrap();
}
@ -872,7 +880,7 @@ pub(crate) mod tests {
Amount::from_u64(15000).unwrap(),
None,
ovk_policy,
1,
NonZeroU32::new(1).unwrap(),
)
.unwrap();
@ -960,7 +968,10 @@ pub(crate) mod tests {
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
// Verified balance matches total balance
let (_, anchor_height) = db_data.get_target_and_anchor_heights(1).unwrap().unwrap();
let (_, anchor_height) = db_data
.get_target_and_anchor_heights(NonZeroU32::new(1).unwrap())
.unwrap()
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
value
@ -981,7 +992,7 @@ pub(crate) mod tests {
Amount::from_u64(50000).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Ok(_)
);
@ -1016,7 +1027,10 @@ pub(crate) mod tests {
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
// Verified balance matches total balance
let (_, anchor_height) = db_data.get_target_and_anchor_heights(10).unwrap().unwrap();
let (_, anchor_height) = db_data
.get_target_and_anchor_heights(NonZeroU32::new(10).unwrap())
.unwrap()
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
value
@ -1037,7 +1051,7 @@ pub(crate) mod tests {
Amount::from_u64(50000).unwrap(),
None,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Ok(_)
);
@ -1086,7 +1100,10 @@ pub(crate) mod tests {
// Verified balance matches total balance
let total = Amount::from_u64(60000).unwrap();
let (_, anchor_height) = db_data.get_target_and_anchor_heights(1).unwrap().unwrap();
let (_, anchor_height) = db_data
.get_target_and_anchor_heights(NonZeroU32::new(1).unwrap())
.unwrap()
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
total
@ -1121,7 +1138,7 @@ pub(crate) mod tests {
&usk,
req,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Err(Error::InsufficientFunds { available, required })
if available == Amount::from_u64(51000).unwrap()
@ -1149,7 +1166,7 @@ pub(crate) mod tests {
&usk,
req,
OvkPolicy::Sender,
1,
NonZeroU32::new(1).unwrap(),
),
Ok(_)
);
@ -1213,7 +1230,7 @@ pub(crate) mod tests {
&usk,
&[*taddr],
&MemoBytes::empty(),
1
NonZeroU32::new(1).unwrap()
),
Ok(_)
);