From 021128b10683702f86a547365599c68c7ae0356d Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 7 Mar 2024 20:34:47 -0700 Subject: [PATCH 01/14] zcash_client_sqlite: Add Orchard note selection. --- zcash_client_sqlite/src/lib.rs | 35 ++- zcash_client_sqlite/src/wallet/orchard.rs | 325 +++++++++++++++++++--- 2 files changed, 322 insertions(+), 38 deletions(-) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 7b3ff6194..8c7e50d94 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -69,13 +69,13 @@ use zcash_client_backend::{ }, proto::compact_formats::CompactBlock, wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput}, - DecryptedOutput, ShieldedProtocol, TransferType, + DecryptedOutput, PoolType, ShieldedProtocol, TransferType, }; use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore}; #[cfg(feature = "orchard")] -use zcash_client_backend::{data_api::ORCHARD_SHARD_HEIGHT, PoolType}; +use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; #[cfg(feature = "transparent-inputs")] use { @@ -111,6 +111,7 @@ pub(crate) const PRUNING_DEPTH: u32 = 100; pub(crate) const VERIFY_LOOKAHEAD: u32 = 10; pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling"; + #[cfg(feature = "orchard")] pub(crate) const ORCHARD_TABLES_PREFIX: &str = "orchard"; @@ -208,7 +209,20 @@ impl, P: consensus::Parameters> InputSource for txid, index, ), - ShieldedProtocol::Orchard => Ok(None), + ShieldedProtocol::Orchard => { + #[cfg(feature = "orchard")] + return wallet::orchard::get_spendable_orchard_note( + self.conn.borrow(), + &self.params, + txid, + index, + ); + + #[cfg(not(feature = "orchard"))] + return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded( + ShieldedProtocol::Orchard, + ))); + } } } @@ -220,14 +234,25 @@ impl, P: consensus::Parameters> InputSource for anchor_height: BlockHeight, exclude: &[Self::NoteRef], ) -> Result>, Self::Error> { - wallet::sapling::select_spendable_sapling_notes( + let received_iter = std::iter::empty(); + let received_iter = received_iter.chain(wallet::sapling::select_spendable_sapling_notes( self.conn.borrow(), &self.params, account, target_value, anchor_height, exclude, - ) + )?); + #[cfg(feature = "orchard")] + let received_iter = received_iter.chain(wallet::orchard::select_spendable_orchard_notes( + self.conn.borrow(), + &self.params, + account, + target_value, + anchor_height, + exclude, + )?); + Ok(received_iter.collect()) } #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index f6ac8a312..e14f1ff35 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -1,24 +1,38 @@ +use std::rc::Rc; + use incrementalmerkletree::Position; -use rusqlite::{named_params, params, Connection}; +use orchard::{ + keys::Diversifier, + note::{Note, Nullifier, RandomSeed}, +}; +use rusqlite::{named_params, params, types::Value, Connection, Row}; use zcash_client_backend::{ - data_api::NullifierQuery, wallet::WalletOrchardOutput, DecryptedOutput, TransferType, + data_api::NullifierQuery, + wallet::{ReceivedNote, WalletOrchardOutput}, + DecryptedOutput, ShieldedProtocol, TransferType, +}; +use zcash_keys::keys::UnifiedFullViewingKey; +use zcash_primitives::transaction::TxId; +use zcash_protocol::{ + consensus::{self, BlockHeight}, + memo::MemoBytes, + value::Zatoshis, }; -use zcash_protocol::memo::MemoBytes; use zip32::Scope; -use crate::{error::SqliteClientError, AccountId}; +use crate::{error::SqliteClientError, AccountId, ReceivedNoteId}; -use super::{memo_repr, scope_code}; +use super::{memo_repr, parse_scope, scope_code, wallet_birthday}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedOrchardOutput { fn index(&self) -> usize; fn account_id(&self) -> AccountId; - fn note(&self) -> &orchard::note::Note; + fn note(&self) -> &Note; fn memo(&self) -> Option<&MemoBytes>; fn is_change(&self) -> bool; - fn nullifier(&self) -> Option<&orchard::note::Nullifier>; + fn nullifier(&self) -> Option<&Nullifier>; fn note_commitment_tree_position(&self) -> Option; fn recipient_key_scope(&self) -> Option; } @@ -30,7 +44,7 @@ impl ReceivedOrchardOutput for WalletOrchardOutput { fn account_id(&self) -> AccountId { *WalletOrchardOutput::account_id(self) } - fn note(&self) -> &orchard::note::Note { + fn note(&self) -> &Note { WalletOrchardOutput::note(self) } fn memo(&self) -> Option<&MemoBytes> { @@ -39,7 +53,7 @@ impl ReceivedOrchardOutput for WalletOrchardOutput { fn is_change(&self) -> bool { WalletOrchardOutput::is_change(self) } - fn nullifier(&self) -> Option<&orchard::note::Nullifier> { + fn nullifier(&self) -> Option<&Nullifier> { self.nf() } fn note_commitment_tree_position(&self) -> Option { @@ -50,7 +64,7 @@ impl ReceivedOrchardOutput for WalletOrchardOutput { } } -impl ReceivedOrchardOutput for DecryptedOutput { +impl ReceivedOrchardOutput for DecryptedOutput { fn index(&self) -> usize { self.index() } @@ -66,7 +80,7 @@ impl ReceivedOrchardOutput for DecryptedOutput { fn is_change(&self) -> bool { self.transfer_type() == TransferType::WalletInternal } - fn nullifier(&self) -> Option<&orchard::note::Nullifier> { + fn nullifier(&self) -> Option<&Nullifier> { None } fn note_commitment_tree_position(&self) -> Option { @@ -81,6 +95,249 @@ impl ReceivedOrchardOutput for DecryptedOutput { } } +fn to_spendable_note( + params: &P, + row: &Row, +) -> Result< + Option>, + SqliteClientError, +> { + let note_id = ReceivedNoteId(ShieldedProtocol::Orchard, row.get(0)?); + let txid = row.get::<_, [u8; 32]>(1).map(TxId::from_bytes)?; + let action_index = row.get(2)?; + let diversifier = { + let d: Vec<_> = row.get(3)?; + if d.len() != 11 { + return Err(SqliteClientError::CorruptedData( + "Invalid diversifier length".to_string(), + )); + } + let mut tmp = [0; 11]; + tmp.copy_from_slice(&d); + Diversifier::from_bytes(tmp) + }; + + let note_value: u64 = row.get::<_, i64>(4)?.try_into().map_err(|_e| { + SqliteClientError::CorruptedData("Note values must be nonnegative".to_string()) + })?; + + let rho = { + let rho_bytes: [u8; 32] = row.get(5)?; + Option::from(Nullifier::from_bytes(&rho_bytes)) + .ok_or_else(|| SqliteClientError::CorruptedData("Invalid rho.".to_string())) + }?; + + let rseed = { + let rseed_bytes: [u8; 32] = row.get(6)?; + Option::from(RandomSeed::from_bytes(rseed_bytes, &rho)).ok_or_else(|| { + SqliteClientError::CorruptedData("Invalid Orchard random seed.".to_string()) + }) + }?; + + let note_commitment_tree_position = + Position::from(u64::try_from(row.get::<_, i64>(7)?).map_err(|_| { + SqliteClientError::CorruptedData("Note commitment tree position invalid.".to_string()) + })?); + + let ufvk_str: Option = row.get(8)?; + let scope_code: Option = row.get(9)?; + + ufvk_str + .zip(scope_code) + .map(|(ufvk_str, scope_code)| { + let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str) + .map_err(SqliteClientError::CorruptedData)?; + + let spending_key_scope = parse_scope(scope_code).ok_or_else(|| { + SqliteClientError::CorruptedData(format!("Invalid key scope code {}", scope_code)) + })?; + let recipient = ufvk + .orchard() + .map(|fvk| fvk.to_ivk(spending_key_scope).address(diversifier)) + .ok_or_else(|| { + SqliteClientError::CorruptedData("Diversifier invalid.".to_owned()) + })?; + + let note = Option::from(Note::from_parts( + recipient, + orchard::value::NoteValue::from_raw(note_value), + rho, + rseed, + )) + .ok_or_else(|| SqliteClientError::CorruptedData("Invalid Orchard note.".to_string()))?; + + Ok(ReceivedNote::from_parts( + note_id, + txid, + action_index, + zcash_client_backend::wallet::Note::Orchard(note), + spending_key_scope, + note_commitment_tree_position, + )) + }) + .transpose() +} + +// The `clippy::let_and_return` lint is explicitly allowed here because a bug in Clippy +// (https://github.com/rust-lang/rust-clippy/issues/11308) means it fails to identify that the `result` temporary +// is required in order to resolve the borrows involved in the `query_and_then` call. +#[allow(clippy::let_and_return)] +pub(crate) fn get_spendable_orchard_note( + conn: &Connection, + params: &P, + txid: &TxId, + index: u32, +) -> Result< + Option>, + SqliteClientError, +> { + let result = conn.query_row_and_then( + "SELECT orchard_received_notes.id, txid, action_index, + diversifier, value, rho, rseed, commitment_tree_position, + accounts.ufvk, recipient_key_scope + FROM orchard_received_notes + INNER JOIN accounts on accounts.id = orchard_received_notes.account_id + INNER JOIN transactions ON transactions.id_tx = orchard_received_notes.tx + WHERE txid = :txid + AND action_index = :action_index + AND nf IS NOT NULL + AND spent IS NULL", + named_params![ + ":txid": txid.as_ref(), + ":action_index": index, + ], + |row| to_spendable_note(params, row), + ); + + // `OptionalExtension` doesn't work here because the error type of `Result` is already + // `SqliteClientError` + match result { + Ok(r) => Ok(r), + Err(SqliteClientError::DbError(rusqlite::Error::QueryReturnedNoRows)) => Ok(None), + Err(e) => Err(e), + } +} + +/// Utility method for determining whether we have any spendable notes +/// +/// If the tip shard has unscanned ranges below the anchor height and greater than or equal to +/// the wallet birthday, none of our notes can be spent because we cannot construct witnesses at +/// the provided anchor height. +fn unscanned_tip_exists( + conn: &Connection, + anchor_height: BlockHeight, +) -> Result { + // v_orchard_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so + // we don't need to refer to the birthday in this query. + conn.query_row( + "SELECT EXISTS ( + SELECT 1 FROM v_orchard_shard_unscanned_ranges range + WHERE range.block_range_start <= :anchor_height + AND :anchor_height BETWEEN + range.subtree_start_height + AND IFNULL(range.subtree_end_height, :anchor_height) + )", + named_params![":anchor_height": u32::from(anchor_height),], + |row| row.get::<_, bool>(0), + ) +} + +pub(crate) fn select_spendable_orchard_notes( + conn: &Connection, + params: &P, + account: AccountId, + target_value: Zatoshis, + anchor_height: BlockHeight, + exclude: &[ReceivedNoteId], +) -> Result>, SqliteClientError> +{ + let birthday_height = match wallet_birthday(conn)? { + Some(birthday) => birthday, + None => { + // the wallet birthday can only be unknown if there are no accounts in the wallet; in + // such a case, the wallet has no notes to spend. + return Ok(vec![]); + } + }; + + if unscanned_tip_exists(conn, anchor_height)? { + return Ok(vec![]); + } + + // The goal of this SQL statement is to select the oldest notes until the required + // value has been reached. + // 1) Use a window function to create a view of all notes, ordered from oldest to + // newest, with an additional column containing a running sum: + // - Unspent notes accumulate the values of all unspent notes in that note's + // account, up to itself. + // - Spent notes accumulate the values of all notes in the transaction they were + // spent in, up to itself. + // + // 2) Select all unspent notes in the desired account, along with their running sum. + // + // 3) Select all notes for which the running sum was less than the required value, as + // well as a single note for which the sum was greater than or equal to the + // required value, bringing the sum of all selected notes across the threshold. + // + // 4) Match the selected notes against the witnesses at the desired height. + let mut stmt_select_notes = conn.prepare_cached( + "WITH eligible AS ( + SELECT + orchard_received_notes.id AS id, txid, action_index, + diversifier, value, rho, rseed, commitment_tree_position, + SUM(value) + OVER (PARTITION BY orchard_received_notes.account_id, spent ORDER BY orchard_received_notes.id) AS so_far, + accounts.ufvk as ufvk, recipient_key_scope + FROM orchard_received_notes + INNER JOIN accounts on accounts.id = orchard_received_notes.account_id + INNER JOIN transactions + ON transactions.id_tx = orchard_received_notes.tx + WHERE orchard_received_notes.account_id = :account + AND commitment_tree_position IS NOT NULL + AND spent IS NULL + AND transactions.block <= :anchor_height + AND orchard_received_notes.id NOT IN rarray(:exclude) + AND NOT EXISTS ( + SELECT 1 FROM v_orchard_shard_unscanned_ranges unscanned + -- select all the unscanned ranges involving the shard containing this note + WHERE orchard_received_notes.commitment_tree_position >= unscanned.start_position + AND orchard_received_notes.commitment_tree_position < unscanned.end_position_exclusive + -- exclude unscanned ranges that start above the anchor height (they don't affect spendability) + AND unscanned.block_range_start <= :anchor_height + -- exclude unscanned ranges that end below the wallet birthday + AND unscanned.block_range_end > :wallet_birthday + ) + ) + SELECT id, txid, action_index, + diversifier, value, rho, rseed, commitment_tree_position, + ufvk, recipient_key_scope + FROM eligible WHERE so_far < :target_value + UNION + SELECT id, txid, action_index, + diversifier, value, rho, rseed, commitment_tree_position, + ufvk, recipient_key_scope + FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", + )?; + + let excluded: Vec = exclude.iter().map(|n| Value::from(n.1)).collect(); + let excluded_ptr = Rc::new(excluded); + + let notes = stmt_select_notes.query_and_then( + named_params![ + ":account": account.0, + ":anchor_height": &u32::from(anchor_height), + ":target_value": &u64::from(target_value), + ":exclude": &excluded_ptr, + ":wallet_birthday": u32::from(birthday_height) + ], + |r| to_spendable_note(params, r), + )?; + + notes + .filter_map(|r| r.transpose()) + .collect::>() +} + /// Records the specified shielded output as having been received. /// /// This implementation relies on the facts that: @@ -94,27 +351,23 @@ pub(crate) fn put_received_note( ) -> Result<(), SqliteClientError> { let mut stmt_upsert_received_note = conn.prepare_cached( "INSERT INTO orchard_received_notes - (tx, action_index, account_id, diversifier, value, rseed, memo, nf, - is_change, spent, commitment_tree_position, - recipient_key_scope) + ( + tx, action_index, account_id, + diversifier, value, rho, rseed, memo, nf, + is_change, spent, commitment_tree_position, + recipient_key_scope + ) VALUES ( - :tx, - :action_index, - :account_id, - :diversifier, - :value, - :rseed, - :memo, - :nf, - :is_change, - :spent, - :commitment_tree_position, + :tx, :action_index, :account_id, + :diversifier, :value, :rho, :rseed, :memo, :nf, + :is_change, :spent, :commitment_tree_position, :recipient_key_scope ) ON CONFLICT (tx, action_index) DO UPDATE SET account_id = :account_id, diversifier = :diversifier, value = :value, + rho = :rho, rseed = :rseed, nf = IFNULL(:nf, nf), memo = IFNULL(:memo, memo), @@ -130,10 +383,11 @@ pub(crate) fn put_received_note( let sql_args = named_params![ ":tx": &tx_ref, - ":output_index": i64::try_from(output.index()).expect("output indices are representable as i64"), + ":action_index": i64::try_from(output.index()).expect("output indices are representable as i64"), ":account_id": output.account_id().0, ":diversifier": diversifier.as_array(), ":value": output.note().value().inner(), + ":rho": output.note().rho().to_bytes(), ":rseed": &rseed.as_bytes(), ":nf": output.nullifier().map(|nf| nf.to_bytes()), ":memo": memo_repr(output.memo()), @@ -159,7 +413,7 @@ pub(crate) fn put_received_note( pub(crate) fn get_orchard_nullifiers( conn: &Connection, query: NullifierQuery, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { // Get the nullifiers for the notes we are tracking let mut stmt_fetch_nullifiers = match query { NullifierQuery::Unspent => conn.prepare( @@ -180,10 +434,7 @@ pub(crate) fn get_orchard_nullifiers( let nullifiers = stmt_fetch_nullifiers.query_and_then([], |row| { let account = AccountId(row.get(1)?); let nf_bytes: [u8; 32] = row.get(2)?; - Ok::<_, rusqlite::Error>(( - account, - orchard::note::Nullifier::from_bytes(&nf_bytes).unwrap(), - )) + Ok::<_, rusqlite::Error>((account, Nullifier::from_bytes(&nf_bytes).unwrap())) })?; let res: Vec<_> = nullifiers.collect::>()?; @@ -198,7 +449,7 @@ pub(crate) fn get_orchard_nullifiers( pub(crate) fn mark_orchard_note_spent( conn: &Connection, tx_ref: i64, - nf: &orchard::note::Nullifier, + nf: &Nullifier, ) -> Result { let mut stmt_mark_orchard_note_spent = conn.prepare_cached("UPDATE orchard_received_notes SET spent = ? WHERE nf = ?")?; @@ -233,6 +484,7 @@ pub(crate) mod tests { use zcash_primitives::transaction::Transaction; use zcash_protocol::{consensus::BlockHeight, memo::MemoBytes, ShieldedProtocol}; + use super::select_spendable_orchard_notes; use crate::{ error::SqliteClientError, testing::{ @@ -321,7 +573,14 @@ pub(crate) mod tests { anchor_height: BlockHeight, exclude: &[crate::ReceivedNoteId], ) -> Result>, SqliteClientError> { - todo!() + select_spendable_orchard_notes( + &st.wallet().conn, + &st.wallet().params, + account, + target_value, + anchor_height, + exclude, + ) } fn decrypted_pool_outputs_count( From ac7113c721f2788c4f2d7498654a496925223085 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sat, 9 Mar 2024 14:59:49 -0700 Subject: [PATCH 02/14] zcash_client_sqlite: Get the minimum of Sapling and Orchard anchor heights for the anchor. --- zcash_client_sqlite/src/wallet.rs | 19 ++++++++++++++++++- zcash_client_sqlite/src/wallet/scanning.rs | 15 ++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 8b51e167f..c24c2b76b 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1321,7 +1321,24 @@ pub(crate) fn get_target_and_anchor_heights( min_confirmations, )?; - Ok(sapling_anchor_height.map(|h| (chain_tip_height + 1, h))) + #[cfg(feature = "orchard")] + let orchard_anchor_height = get_max_checkpointed_height( + conn, + ORCHARD_TABLES_PREFIX, + chain_tip_height, + min_confirmations, + )?; + + #[cfg(not(feature = "orchard"))] + let orchard_anchor_height: Option = None; + + let anchor_height = sapling_anchor_height + .zip(orchard_anchor_height) + .map(|(s, o)| std::cmp::min(s, o)) + .or(sapling_anchor_height) + .or(orchard_anchor_height); + + Ok(anchor_height.map(|h| (chain_tip_height + 1, h))) } None => Ok(None), } diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 73de6cae6..310c48fce 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -381,15 +381,12 @@ pub(crate) fn scan_complete( .map(|extended| ScanRange::from_parts(range.end..extended.end, ScanPriority::FoundNote)) .filter(|range| !range.is_empty()); - replace_queue_entries::( - conn, - &query_range, - Some(scanned) - .into_iter() - .chain(extended_before) - .chain(extended_after), - false, - )?; + let replacement = Some(scanned) + .into_iter() + .chain(extended_before) + .chain(extended_after); + + replace_queue_entries::(conn, &query_range, replacement, false)?; Ok(()) } From a1a8af01d3d9b367014f89250e64c2811dc802f8 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 10 Mar 2024 19:51:57 +0000 Subject: [PATCH 03/14] zcash_client_sqlite: Add Orchard support to `get_wallet_summary` --- zcash_client_sqlite/src/wallet.rs | 329 ++++++++++++++++++++++-------- 1 file changed, 248 insertions(+), 81 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index c24c2b76b..ac4c1fdaa 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -723,6 +723,15 @@ pub(crate) trait ScanProgress { fully_scanned_height: BlockHeight, chain_tip_height: BlockHeight, ) -> Result>, SqliteClientError>; + + #[cfg(feature = "orchard")] + fn orchard_scan_progress( + &self, + conn: &rusqlite::Connection, + birthday_height: BlockHeight, + fully_scanned_height: BlockHeight, + chain_tip_height: BlockHeight, + ) -> Result>, SqliteClientError>; } #[derive(Debug)] @@ -804,6 +813,83 @@ impl ScanProgress for SubtreeScanProgress { .flatten()) } } + + #[cfg(feature = "orchard")] + #[tracing::instrument(skip(conn))] + fn orchard_scan_progress( + &self, + conn: &rusqlite::Connection, + birthday_height: BlockHeight, + fully_scanned_height: BlockHeight, + chain_tip_height: BlockHeight, + ) -> Result>, SqliteClientError> { + if fully_scanned_height == chain_tip_height { + // Compute the total blocks scanned since the wallet birthday + conn.query_row( + "SELECT SUM(orchard_action_count) + FROM blocks + WHERE height >= :birthday_height", + named_params![":birthday_height": u32::from(birthday_height)], + |row| { + let scanned = row.get::<_, Option>(0)?; + Ok(scanned.map(|n| Ratio::new(n, n))) + }, + ) + .map_err(SqliteClientError::from) + } else { + let start_height = birthday_height; + // Compute the starting number of notes directly from the blocks table + let start_size = conn.query_row( + "SELECT MAX(orchard_commitment_tree_size) + FROM blocks + WHERE height <= :start_height", + named_params![":start_height": u32::from(start_height)], + |row| row.get::<_, Option>(0), + )?; + + // Compute the total blocks scanned so far above the starting height + let scanned_count = conn.query_row( + "SELECT SUM(orchard_action_count) + FROM blocks + WHERE height > :start_height", + named_params![":start_height": u32::from(start_height)], + |row| row.get::<_, Option>(0), + )?; + + // We don't have complete information on how many actions will exist in the shard at + // the chain tip without having scanned the chain tip block, so we overestimate by + // computing the maximum possible number of notes directly from the shard indices. + // + // TODO: it would be nice to be able to reliably have the size of the commitment tree + // at the chain tip without having to have scanned that block. + Ok(conn + .query_row( + "SELECT MIN(shard_index), MAX(shard_index) + FROM orchard_tree_shards + WHERE subtree_end_height > :start_height + OR subtree_end_height IS NULL", + named_params![":start_height": u32::from(start_height)], + |row| { + let min_tree_size = row + .get::<_, Option>(0)? + .map(|min| min << ORCHARD_SHARD_HEIGHT); + let max_idx = row.get::<_, Option>(1)?; + Ok(start_size + .or(min_tree_size) + .zip(max_idx) + .map(|(min_tree_size, max)| { + let max_tree_size = (max + 1) << ORCHARD_SHARD_HEIGHT; + Ratio::new( + scanned_count.unwrap_or(0), + max_tree_size - min_tree_size, + ) + })) + }, + ) + .optional()? + .flatten()) + } + } } /// Returns the spendable balance for the account at the specified height. @@ -841,27 +927,27 @@ pub(crate) fn get_wallet_summary( chain_tip_height, )?; - // If the shard containing the summary height contains any unscanned ranges that start below or - // including that height, none of our balance is currently spendable. - #[tracing::instrument(skip_all)] - fn is_any_spendable( - conn: &rusqlite::Connection, - summary_height: BlockHeight, - ) -> Result { - conn.query_row( - "SELECT NOT EXISTS( - SELECT 1 FROM v_sapling_shard_unscanned_ranges - WHERE :summary_height - BETWEEN subtree_start_height - AND IFNULL(subtree_end_height, :summary_height) - AND block_range_start <= :summary_height - )", - named_params![":summary_height": u32::from(summary_height)], - |row| row.get::<_, bool>(0), - ) - .map_err(|e| e.into()) - } - let any_spendable = is_any_spendable(tx, summary_height)?; + #[cfg(feature = "orchard")] + let orchard_scan_progress = progress.orchard_scan_progress( + tx, + birthday_height, + fully_scanned_height, + chain_tip_height, + )?; + #[cfg(not(feature = "orchard"))] + let orchard_scan_progress: Option> = None; + + // Treat Sapling and Orchard outputs as having the same cost to scan. + let scan_progress = sapling_scan_progress + .zip(orchard_scan_progress) + .map(|(s, o)| { + Ratio::new( + s.numerator() + o.numerator(), + s.denominator() + o.denominator(), + ) + }) + .or(sapling_scan_progress) + .or(orchard_scan_progress); let mut stmt_accounts = tx.prepare_cached("SELECT id FROM accounts")?; let mut account_balances = stmt_accounts @@ -871,78 +957,159 @@ pub(crate) fn get_wallet_summary( }) .collect::, _>>()?; - let sapling_trace = tracing::info_span!("stmt_select_notes").entered(); - let mut stmt_select_notes = tx.prepare_cached( - "SELECT n.account_id, n.value, n.is_change, scan_state.max_priority, t.block - FROM sapling_received_notes n - JOIN transactions t ON t.id_tx = n.tx - LEFT OUTER JOIN v_sapling_shards_scan_state scan_state - ON n.commitment_tree_position >= scan_state.start_position - AND n.commitment_tree_position < scan_state.end_position_exclusive - WHERE n.spent IS NULL - AND ( - t.expiry_height IS NULL - OR t.block IS NOT NULL - OR t.expiry_height >= :summary_height - )", - )?; + fn count_notes( + tx: &rusqlite::Transaction, + summary_height: BlockHeight, + account_balances: &mut HashMap, + table_prefix: &'static str, + with_pool_balance: F, + ) -> Result<(), SqliteClientError> + where + F: Fn( + &mut AccountBalance, + NonNegativeAmount, + NonNegativeAmount, + NonNegativeAmount, + ) -> Result<(), SqliteClientError>, + { + // If the shard containing the summary height contains any unscanned ranges that start below or + // including that height, none of our balance is currently spendable. + #[tracing::instrument(skip_all)] + fn is_any_spendable( + conn: &rusqlite::Connection, + summary_height: BlockHeight, + table_prefix: &'static str, + ) -> Result { + conn.query_row( + &format!( + "SELECT NOT EXISTS( + SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges + WHERE :summary_height + BETWEEN subtree_start_height + AND IFNULL(subtree_end_height, :summary_height) + AND block_range_start <= :summary_height + )" + ), + named_params![":summary_height": u32::from(summary_height)], + |row| row.get::<_, bool>(0), + ) + .map_err(|e| e.into()) + } - let mut rows = - stmt_select_notes.query(named_params![":summary_height": u32::from(summary_height)])?; - while let Some(row) = rows.next()? { - let account = AccountId(row.get::<_, u32>(0)?); + let any_spendable = is_any_spendable(tx, summary_height, table_prefix)?; + let mut stmt_select_notes = tx.prepare_cached(&format!( + "SELECT n.account_id, n.value, n.is_change, scan_state.max_priority, t.block + FROM {table_prefix}_received_notes n + JOIN transactions t ON t.id_tx = n.tx + LEFT OUTER JOIN v_{table_prefix}_shards_scan_state scan_state + ON n.commitment_tree_position >= scan_state.start_position + AND n.commitment_tree_position < scan_state.end_position_exclusive + WHERE n.spent IS NULL + AND ( + t.expiry_height IS NULL + OR t.block IS NOT NULL + OR t.expiry_height >= :summary_height + )", + ))?; - let value_raw = row.get::<_, i64>(1)?; - let value = NonNegativeAmount::from_nonnegative_i64(value_raw).map_err(|_| { - SqliteClientError::CorruptedData(format!("Negative received note value: {}", value_raw)) - })?; + let mut rows = + stmt_select_notes.query(named_params![":summary_height": u32::from(summary_height)])?; + while let Some(row) = rows.next()? { + let account = AccountId(row.get::<_, u32>(0)?); - let is_change = row.get::<_, bool>(2)?; + let value_raw = row.get::<_, i64>(1)?; + let value = NonNegativeAmount::from_nonnegative_i64(value_raw).map_err(|_| { + SqliteClientError::CorruptedData(format!( + "Negative received note value: {}", + value_raw + )) + })?; - // If `max_priority` is null, this means that the note is not positioned; the note - // will not be spendable, so we assign the scan priority to `ChainTip` as a priority - // that is greater than `Scanned` - let max_priority_raw = row.get::<_, Option>(3)?; - let max_priority = max_priority_raw.map_or_else( - || Ok(ScanPriority::ChainTip), - |raw| { - parse_priority_code(raw).ok_or_else(|| { - SqliteClientError::CorruptedData(format!( - "Priority code {} not recognized.", - raw - )) + let is_change = row.get::<_, bool>(2)?; + + // If `max_priority` is null, this means that the note is not positioned; the note + // will not be spendable, so we assign the scan priority to `ChainTip` as a priority + // that is greater than `Scanned` + let max_priority_raw = row.get::<_, Option>(3)?; + let max_priority = max_priority_raw.map_or_else( + || Ok(ScanPriority::ChainTip), + |raw| { + parse_priority_code(raw).ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "Priority code {} not recognized.", + raw + )) + }) + }, + )?; + + let received_height = row.get::<_, Option>(4)?.map(BlockHeight::from); + + let is_spendable = any_spendable + && received_height.iter().any(|h| h <= &summary_height) + && max_priority <= ScanPriority::Scanned; + + let is_pending_change = + is_change && received_height.iter().all(|h| h > &summary_height); + + let (spendable_value, change_pending_confirmation, value_pending_spendability) = { + let zero = NonNegativeAmount::ZERO; + if is_spendable { + (value, zero, zero) + } else if is_pending_change { + (zero, value, zero) + } else { + (zero, zero, value) + } + }; + + if let Some(balances) = account_balances.get_mut(&account) { + with_pool_balance( + balances, + spendable_value, + change_pending_confirmation, + value_pending_spendability, + )?; + } + } + Ok(()) + } + + #[cfg(feature = "orchard")] + { + let orchard_trace = tracing::info_span!("orchard_balances").entered(); + count_notes( + tx, + summary_height, + &mut account_balances, + ORCHARD_TABLES_PREFIX, + |balances, spendable_value, change_pending_confirmation, value_pending_spendability| { + balances.with_orchard_balance_mut::<_, SqliteClientError>(|bal| { + bal.add_spendable_value(spendable_value)?; + bal.add_pending_change_value(change_pending_confirmation)?; + bal.add_pending_spendable_value(value_pending_spendability)?; + Ok(()) }) }, )?; + drop(orchard_trace); + } - let received_height = row.get::<_, Option>(4)?.map(BlockHeight::from); - - let is_spendable = any_spendable - && received_height.iter().any(|h| h <= &summary_height) - && max_priority <= ScanPriority::Scanned; - - let is_pending_change = is_change && received_height.iter().all(|h| h > &summary_height); - - let (spendable_value, change_pending_confirmation, value_pending_spendability) = { - let zero = NonNegativeAmount::ZERO; - if is_spendable { - (value, zero, zero) - } else if is_pending_change { - (zero, value, zero) - } else { - (zero, zero, value) - } - }; - - if let Some(balances) = account_balances.get_mut(&account) { + let sapling_trace = tracing::info_span!("sapling_balances").entered(); + count_notes( + tx, + summary_height, + &mut account_balances, + SAPLING_TABLES_PREFIX, + |balances, spendable_value, change_pending_confirmation, value_pending_spendability| { balances.with_sapling_balance_mut::<_, SqliteClientError>(|bal| { bal.add_spendable_value(spendable_value)?; bal.add_pending_change_value(change_pending_confirmation)?; bal.add_pending_spendable_value(value_pending_spendability)?; Ok(()) - })?; - } - } + }) + }, + )?; drop(sapling_trace); #[cfg(feature = "transparent-inputs")] @@ -1025,7 +1192,7 @@ pub(crate) fn get_wallet_summary( account_balances, chain_tip_height, fully_scanned_height, - sapling_scan_progress, + scan_progress, next_sapling_subtree_index, #[cfg(feature = "orchard")] next_orchard_subtree_index, From 44f5a55b9212ca7e709ec496345574ddae454528 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 10 Mar 2024 20:07:56 +0000 Subject: [PATCH 04/14] zcash_client_sqlite: Add Orchard support to `get_received_memo` --- zcash_client_sqlite/src/wallet.rs | 39 ++++++++++++++++++------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index ac4c1fdaa..552434dd4 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1206,24 +1206,31 @@ pub(crate) fn get_received_memo( conn: &rusqlite::Connection, note_id: NoteId, ) -> Result, SqliteClientError> { - let memo_bytes: Option> = match note_id.protocol() { - ShieldedProtocol::Sapling => conn - .query_row( - "SELECT memo FROM sapling_received_notes - JOIN transactions ON sapling_received_notes.tx = transactions.id_tx + let fetch_memo = |table_prefix: &'static str, output_col: &'static str| { + conn.query_row( + &format!( + "SELECT memo FROM {table_prefix}_received_notes + JOIN transactions ON {table_prefix}_received_notes.tx = transactions.id_tx WHERE transactions.txid = :txid - AND sapling_received_notes.output_index = :output_index", - named_params![ - ":txid": note_id.txid().as_ref(), - ":output_index": note_id.output_index() - ], - |row| row.get(0), - ) - .optional()? - .flatten(), - _ => { + AND {table_prefix}_received_notes.{output_col} = :output_index" + ), + named_params![ + ":txid": note_id.txid().as_ref(), + ":output_index": note_id.output_index() + ], + |row| row.get(0), + ) + .optional() + }; + + let memo_bytes: Option> = match note_id.protocol() { + ShieldedProtocol::Sapling => fetch_memo(SAPLING_TABLES_PREFIX, "output_index")?.flatten(), + #[cfg(feature = "orchard")] + ShieldedProtocol::Orchard => fetch_memo(ORCHARD_TABLES_PREFIX, "action_index")?.flatten(), + #[cfg(not(feature = "orchard"))] + ShieldedProtocol::Orchard => { return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded( - note_id.protocol(), + ShieldedProtocol::Orchard, ))) } }; From 6086774b9bba0d80828e2a678fb33aea71737f27 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 15:05:33 -0600 Subject: [PATCH 05/14] zcash_client_sqlite: Ensure that we only exclude the correct notes from selection. --- zcash_client_sqlite/src/wallet/orchard.rs | 5 ++++- zcash_client_sqlite/src/wallet/sapling.rs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index e14f1ff35..dbdc80bd8 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -319,7 +319,10 @@ pub(crate) fn select_spendable_orchard_notes( FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", )?; - let excluded: Vec = exclude.iter().map(|n| Value::from(n.1)).collect(); + let excluded: Vec = exclude + .iter() + .filter_map(|n| matches!(n.0, ShieldedProtocol::Orchard).then(|| Value::from(n.1))) + .collect(); let excluded_ptr = Rc::new(excluded); let notes = stmt_select_notes.query_and_then( diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index ed748b48b..2649ff436 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -317,7 +317,10 @@ pub(crate) fn select_spendable_sapling_notes( FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", )?; - let excluded: Vec = exclude.iter().map(|n| Value::from(n.1)).collect(); + let excluded: Vec = exclude + .iter() + .filter_map(|n| matches!(n.0, ShieldedProtocol::Sapling).then(|| Value::from(n.1))) + .collect(); let excluded_ptr = Rc::new(excluded); let notes = stmt_select_notes.query_and_then( From cad174c1d7167e9c2169a0e51bce6e8333a25efd Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 15:07:54 -0600 Subject: [PATCH 06/14] zcash_client_sqlite: Make note selection queries consistent between Sapling and Orchard. --- zcash_client_sqlite/src/wallet/orchard.rs | 8 +++++++- zcash_client_sqlite/src/wallet/sapling.rs | 23 ++++++++++++++++------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index dbdc80bd8..a6ad5048e 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -200,6 +200,8 @@ pub(crate) fn get_spendable_orchard_note( INNER JOIN transactions ON transactions.id_tx = orchard_received_notes.tx WHERE txid = :txid AND action_index = :action_index + AND accounts.ufvk IS NOT NULL + AND recipient_key_scope IS NOT NULL AND nf IS NOT NULL AND spent IS NULL", named_params![ @@ -289,10 +291,14 @@ pub(crate) fn select_spendable_orchard_notes( OVER (PARTITION BY orchard_received_notes.account_id, spent ORDER BY orchard_received_notes.id) AS so_far, accounts.ufvk as ufvk, recipient_key_scope FROM orchard_received_notes - INNER JOIN accounts on accounts.id = orchard_received_notes.account_id + INNER JOIN accounts + ON accounts.id = orchard_received_notes.account_id INNER JOIN transactions ON transactions.id_tx = orchard_received_notes.tx WHERE orchard_received_notes.account_id = :account + AND accounts.ufvk IS NOT NULL + AND recipient_key_scope IS NOT NULL + AND nf IS NOT NULL AND commitment_tree_position IS NOT NULL AND spent IS NULL AND transactions.block <= :anchor_height diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 2649ff436..61d6da996 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -197,12 +197,14 @@ pub(crate) fn get_spendable_sapling_note( diversifier, value, rcm, commitment_tree_position, accounts.ufvk, recipient_key_scope FROM sapling_received_notes - INNER JOIN accounts on accounts.id = sapling_received_notes.account_id + INNER JOIN accounts ON accounts.id = sapling_received_notes.account_id INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx WHERE txid = :txid + AND output_index = :output_index AND accounts.ufvk IS NOT NULL AND recipient_key_scope IS NOT NULL - AND output_index = :output_index + AND nf IS NOT NULL + AND commitment_tree_position IS NOT NULL AND spent IS NULL", named_params![ ":txid": txid.as_ref(), @@ -284,17 +286,20 @@ pub(crate) fn select_spendable_sapling_notes( let mut stmt_select_notes = conn.prepare_cached( "WITH eligible AS ( SELECT - sapling_received_notes.id AS id, txid, output_index, diversifier, value, rcm, commitment_tree_position, + sapling_received_notes.id AS id, txid, output_index, + diversifier, value, rcm, commitment_tree_position, SUM(value) OVER (PARTITION BY sapling_received_notes.account_id, spent ORDER BY sapling_received_notes.id) AS so_far, accounts.ufvk as ufvk, recipient_key_scope FROM sapling_received_notes - INNER JOIN accounts on accounts.id = sapling_received_notes.account_id + INNER JOIN accounts + ON accounts.id = sapling_received_notes.account_id INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx WHERE sapling_received_notes.account_id = :account - AND ufvk IS NOT NULL + AND accounts.ufvk IS NOT NULL AND recipient_key_scope IS NOT NULL + AND nf IS NOT NULL AND commitment_tree_position IS NOT NULL AND spent IS NULL AND transactions.block <= :anchor_height @@ -310,10 +315,14 @@ pub(crate) fn select_spendable_sapling_notes( AND unscanned.block_range_end > :wallet_birthday ) ) - SELECT id, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, recipient_key_scope + SELECT id, txid, output_index, + diversifier, value, rcm, commitment_tree_position, + ufvk, recipient_key_scope FROM eligible WHERE so_far < :target_value UNION - SELECT id, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, recipient_key_scope + SELECT id, txid, output_index, + diversifier, value, rcm, commitment_tree_position, + ufvk, recipient_key_scope FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", )?; From 820b1f9c2d7ca810e4396c05704678a3a7c1c004 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 15:28:06 -0600 Subject: [PATCH 07/14] zcash_client_sqlite: Factor out common note selection code. --- zcash_client_sqlite/src/wallet.rs | 1 + zcash_client_sqlite/src/wallet/common.rs | 218 ++++++++++++++++++++++ zcash_client_sqlite/src/wallet/orchard.rs | 165 ++-------------- zcash_client_sqlite/src/wallet/sapling.rs | 162 ++-------------- 4 files changed, 258 insertions(+), 288 deletions(-) create mode 100644 zcash_client_sqlite/src/wallet/common.rs diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 552434dd4..8ef0f4db0 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -129,6 +129,7 @@ use { }; pub mod commitment_tree; +pub(crate) mod common; pub mod init; #[cfg(feature = "orchard")] pub(crate) mod orchard; diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs new file mode 100644 index 000000000..31f9b23ef --- /dev/null +++ b/zcash_client_sqlite/src/wallet/common.rs @@ -0,0 +1,218 @@ +//! Functions common to Sapling and Orchard support in the wallet. + +use rusqlite::{named_params, types::Value, Connection, Row}; +use std::rc::Rc; + +use zcash_client_backend::{ + wallet::{Note, ReceivedNote}, + ShieldedProtocol, +}; +use zcash_primitives::transaction::{components::amount::NonNegativeAmount, TxId}; +use zcash_protocol::consensus::{self, BlockHeight}; + +use super::wallet_birthday; +use crate::{error::SqliteClientError, AccountId, ReceivedNoteId, SAPLING_TABLES_PREFIX}; + +#[cfg(feature = "orchard")] +use crate::ORCHARD_TABLES_PREFIX; + +fn per_protocol_names(protocol: ShieldedProtocol) -> (&'static str, &'static str, &'static str) { + match protocol { + ShieldedProtocol::Sapling => (SAPLING_TABLES_PREFIX, "output_index", "rcm"), + #[cfg(feature = "orchard")] + ShieldedProtocol::Orchard => (ORCHARD_TABLES_PREFIX, "action_index", "rho, rseed"), + #[cfg(not(feature = "orchard"))] + ShieldedProtocol::Orchard => { + unreachable!("Should never be called unless the `orchard` feature is enabled") + } + } +} + +fn unscanned_tip_exists( + conn: &Connection, + anchor_height: BlockHeight, + table_prefix: &str, +) -> Result { + // v_sapling_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so + // we don't need to refer to the birthday in this query. + conn.query_row( + &format!( + "SELECT EXISTS ( + SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges range + WHERE range.block_range_start <= :anchor_height + AND :anchor_height BETWEEN + range.subtree_start_height + AND IFNULL(range.subtree_end_height, :anchor_height) + )" + ), + named_params![":anchor_height": u32::from(anchor_height),], + |row| row.get::<_, bool>(0), + ) +} + +// The `clippy::let_and_return` lint is explicitly allowed here because a bug in Clippy +// (https://github.com/rust-lang/rust-clippy/issues/11308) means it fails to identify that the `result` temporary +// is required in order to resolve the borrows involved in the `query_and_then` call. +#[allow(clippy::let_and_return)] +pub(crate) fn get_spendable_note( + conn: &Connection, + params: &P, + txid: &TxId, + index: u32, + protocol: ShieldedProtocol, + to_spendable_note: F, +) -> Result>, SqliteClientError> +where + F: Fn(&P, &Row) -> Result>, SqliteClientError>, +{ + let (table_prefix, index_col, note_reconstruction_cols) = per_protocol_names(protocol); + let result = conn.query_row_and_then( + &format!( + "SELECT {table_prefix}_received_notes.id, txid, {index_col}, + diversifier, value, {note_reconstruction_cols}, commitment_tree_position, + accounts.ufvk, recipient_key_scope + FROM {table_prefix}_received_notes + INNER JOIN accounts ON accounts.id = {table_prefix}_received_notes.account_id + INNER JOIN transactions ON transactions.id_tx = {table_prefix}_received_notes.tx + WHERE txid = :txid + AND {index_col} = :output_index + AND accounts.ufvk IS NOT NULL + AND recipient_key_scope IS NOT NULL + AND nf IS NOT NULL + AND commitment_tree_position IS NOT NULL + AND spent IS NULL" + ), + named_params![ + ":txid": txid.as_ref(), + ":output_index": index, + ], + |row| to_spendable_note(params, row), + ); + + // `OptionalExtension` doesn't work here because the error type of `Result` is already + // `SqliteClientError` + match result { + Ok(r) => Ok(r), + Err(SqliteClientError::DbError(rusqlite::Error::QueryReturnedNoRows)) => Ok(None), + Err(e) => Err(e), + } +} + +#[allow(clippy::too_many_arguments)] +pub(crate) fn select_spendable_notes( + conn: &Connection, + params: &P, + account: AccountId, + target_value: NonNegativeAmount, + anchor_height: BlockHeight, + exclude: &[ReceivedNoteId], + protocol: ShieldedProtocol, + to_spendable_note: F, +) -> Result>, SqliteClientError> +where + F: Fn(&P, &Row) -> Result>, SqliteClientError>, +{ + let birthday_height = match wallet_birthday(conn)? { + Some(birthday) => birthday, + None => { + // the wallet birthday can only be unknown if there are no accounts in the wallet; in + // such a case, the wallet has no notes to spend. + return Ok(vec![]); + } + }; + + let (table_prefix, index_col, note_reconstruction_cols) = per_protocol_names(protocol); + if unscanned_tip_exists(conn, anchor_height, table_prefix)? { + return Ok(vec![]); + } + + // The goal of this SQL statement is to select the oldest notes until the required + // value has been reached. + // 1) Use a window function to create a view of all notes, ordered from oldest to + // newest, with an additional column containing a running sum: + // - Unspent notes accumulate the values of all unspent notes in that note's + // account, up to itself. + // - Spent notes accumulate the values of all notes in the transaction they were + // spent in, up to itself. + // + // 2) Select all unspent notes in the desired account, along with their running sum. + // + // 3) Select all notes for which the running sum was less than the required value, as + // well as a single note for which the sum was greater than or equal to the + // required value, bringing the sum of all selected notes across the threshold. + // + // 4) Match the selected notes against the witnesses at the desired height. + let mut stmt_select_notes = conn.prepare_cached( + &format!( + "WITH eligible AS ( + SELECT + {table_prefix}_received_notes.id AS id, txid, {index_col}, + diversifier, value, {note_reconstruction_cols}, commitment_tree_position, + SUM(value) OVER ( + PARTITION BY {table_prefix}_received_notes.account_id, spent + ORDER BY {table_prefix}_received_notes.id + ) AS so_far, + accounts.ufvk as ufvk, recipient_key_scope + FROM {table_prefix}_received_notes + INNER JOIN accounts + ON accounts.id = {table_prefix}_received_notes.account_id + INNER JOIN transactions + ON transactions.id_tx = {table_prefix}_received_notes.tx + WHERE {table_prefix}_received_notes.account_id = :account + AND accounts.ufvk IS NOT NULL + AND recipient_key_scope IS NOT NULL + AND nf IS NOT NULL + AND commitment_tree_position IS NOT NULL + AND spent IS NULL + AND transactions.block <= :anchor_height + AND {table_prefix}_received_notes.id NOT IN rarray(:exclude) + AND NOT EXISTS ( + SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges unscanned + -- select all the unscanned ranges involving the shard containing this note + WHERE {table_prefix}_received_notes.commitment_tree_position >= unscanned.start_position + AND {table_prefix}_received_notes.commitment_tree_position < unscanned.end_position_exclusive + -- exclude unscanned ranges that start above the anchor height (they don't affect spendability) + AND unscanned.block_range_start <= :anchor_height + -- exclude unscanned ranges that end below the wallet birthday + AND unscanned.block_range_end > :wallet_birthday + ) + ) + SELECT id, txid, {index_col}, + diversifier, value, {note_reconstruction_cols}, commitment_tree_position, + ufvk, recipient_key_scope + FROM eligible WHERE so_far < :target_value + UNION + SELECT id, txid, {index_col}, + diversifier, value, {note_reconstruction_cols}, commitment_tree_position, + ufvk, recipient_key_scope + FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", + ) + )?; + + let excluded: Vec = exclude + .iter() + .filter_map(|ReceivedNoteId(p, n)| { + if *p == protocol { + Some(Value::from(*n)) + } else { + None + } + }) + .collect(); + let excluded_ptr = Rc::new(excluded); + + let notes = stmt_select_notes.query_and_then( + named_params![ + ":account": account.0, + ":anchor_height": &u32::from(anchor_height), + ":target_value": &u64::from(target_value), + ":exclude": &excluded_ptr, + ":wallet_birthday": u32::from(birthday_height) + ], + |r| to_spendable_note(params, r), + )?; + + notes + .filter_map(|r| r.transpose()) + .collect::>() +} diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index a6ad5048e..a4b1f83b9 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -1,11 +1,9 @@ -use std::rc::Rc; - use incrementalmerkletree::Position; use orchard::{ keys::Diversifier, note::{Note, Nullifier, RandomSeed}, }; -use rusqlite::{named_params, params, types::Value, Connection, Row}; +use rusqlite::{named_params, params, Connection, Row}; use zcash_client_backend::{ data_api::NullifierQuery, @@ -23,7 +21,7 @@ use zip32::Scope; use crate::{error::SqliteClientError, AccountId, ReceivedNoteId}; -use super::{memo_repr, parse_scope, scope_code, wallet_birthday}; +use super::{memo_repr, parse_scope, scope_code}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedOrchardOutput { @@ -191,56 +189,13 @@ pub(crate) fn get_spendable_orchard_note( Option>, SqliteClientError, > { - let result = conn.query_row_and_then( - "SELECT orchard_received_notes.id, txid, action_index, - diversifier, value, rho, rseed, commitment_tree_position, - accounts.ufvk, recipient_key_scope - FROM orchard_received_notes - INNER JOIN accounts on accounts.id = orchard_received_notes.account_id - INNER JOIN transactions ON transactions.id_tx = orchard_received_notes.tx - WHERE txid = :txid - AND action_index = :action_index - AND accounts.ufvk IS NOT NULL - AND recipient_key_scope IS NOT NULL - AND nf IS NOT NULL - AND spent IS NULL", - named_params![ - ":txid": txid.as_ref(), - ":action_index": index, - ], - |row| to_spendable_note(params, row), - ); - - // `OptionalExtension` doesn't work here because the error type of `Result` is already - // `SqliteClientError` - match result { - Ok(r) => Ok(r), - Err(SqliteClientError::DbError(rusqlite::Error::QueryReturnedNoRows)) => Ok(None), - Err(e) => Err(e), - } -} - -/// Utility method for determining whether we have any spendable notes -/// -/// If the tip shard has unscanned ranges below the anchor height and greater than or equal to -/// the wallet birthday, none of our notes can be spent because we cannot construct witnesses at -/// the provided anchor height. -fn unscanned_tip_exists( - conn: &Connection, - anchor_height: BlockHeight, -) -> Result { - // v_orchard_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so - // we don't need to refer to the birthday in this query. - conn.query_row( - "SELECT EXISTS ( - SELECT 1 FROM v_orchard_shard_unscanned_ranges range - WHERE range.block_range_start <= :anchor_height - AND :anchor_height BETWEEN - range.subtree_start_height - AND IFNULL(range.subtree_end_height, :anchor_height) - )", - named_params![":anchor_height": u32::from(anchor_height),], - |row| row.get::<_, bool>(0), + super::common::get_spendable_note( + conn, + params, + txid, + index, + ShieldedProtocol::Orchard, + to_spendable_note, ) } @@ -253,98 +208,16 @@ pub(crate) fn select_spendable_orchard_notes( exclude: &[ReceivedNoteId], ) -> Result>, SqliteClientError> { - let birthday_height = match wallet_birthday(conn)? { - Some(birthday) => birthday, - None => { - // the wallet birthday can only be unknown if there are no accounts in the wallet; in - // such a case, the wallet has no notes to spend. - return Ok(vec![]); - } - }; - - if unscanned_tip_exists(conn, anchor_height)? { - return Ok(vec![]); - } - - // The goal of this SQL statement is to select the oldest notes until the required - // value has been reached. - // 1) Use a window function to create a view of all notes, ordered from oldest to - // newest, with an additional column containing a running sum: - // - Unspent notes accumulate the values of all unspent notes in that note's - // account, up to itself. - // - Spent notes accumulate the values of all notes in the transaction they were - // spent in, up to itself. - // - // 2) Select all unspent notes in the desired account, along with their running sum. - // - // 3) Select all notes for which the running sum was less than the required value, as - // well as a single note for which the sum was greater than or equal to the - // required value, bringing the sum of all selected notes across the threshold. - // - // 4) Match the selected notes against the witnesses at the desired height. - let mut stmt_select_notes = conn.prepare_cached( - "WITH eligible AS ( - SELECT - orchard_received_notes.id AS id, txid, action_index, - diversifier, value, rho, rseed, commitment_tree_position, - SUM(value) - OVER (PARTITION BY orchard_received_notes.account_id, spent ORDER BY orchard_received_notes.id) AS so_far, - accounts.ufvk as ufvk, recipient_key_scope - FROM orchard_received_notes - INNER JOIN accounts - ON accounts.id = orchard_received_notes.account_id - INNER JOIN transactions - ON transactions.id_tx = orchard_received_notes.tx - WHERE orchard_received_notes.account_id = :account - AND accounts.ufvk IS NOT NULL - AND recipient_key_scope IS NOT NULL - AND nf IS NOT NULL - AND commitment_tree_position IS NOT NULL - AND spent IS NULL - AND transactions.block <= :anchor_height - AND orchard_received_notes.id NOT IN rarray(:exclude) - AND NOT EXISTS ( - SELECT 1 FROM v_orchard_shard_unscanned_ranges unscanned - -- select all the unscanned ranges involving the shard containing this note - WHERE orchard_received_notes.commitment_tree_position >= unscanned.start_position - AND orchard_received_notes.commitment_tree_position < unscanned.end_position_exclusive - -- exclude unscanned ranges that start above the anchor height (they don't affect spendability) - AND unscanned.block_range_start <= :anchor_height - -- exclude unscanned ranges that end below the wallet birthday - AND unscanned.block_range_end > :wallet_birthday - ) - ) - SELECT id, txid, action_index, - diversifier, value, rho, rseed, commitment_tree_position, - ufvk, recipient_key_scope - FROM eligible WHERE so_far < :target_value - UNION - SELECT id, txid, action_index, - diversifier, value, rho, rseed, commitment_tree_position, - ufvk, recipient_key_scope - FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", - )?; - - let excluded: Vec = exclude - .iter() - .filter_map(|n| matches!(n.0, ShieldedProtocol::Orchard).then(|| Value::from(n.1))) - .collect(); - let excluded_ptr = Rc::new(excluded); - - let notes = stmt_select_notes.query_and_then( - named_params![ - ":account": account.0, - ":anchor_height": &u32::from(anchor_height), - ":target_value": &u64::from(target_value), - ":exclude": &excluded_ptr, - ":wallet_birthday": u32::from(birthday_height) - ], - |r| to_spendable_note(params, r), - )?; - - notes - .filter_map(|r| r.transpose()) - .collect::>() + super::common::select_spendable_notes( + conn, + params, + account, + target_value, + anchor_height, + exclude, + ShieldedProtocol::Orchard, + to_spendable_note, + ) } /// Records the specified shielded output as having been received. diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 61d6da996..4801ca69b 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -2,8 +2,7 @@ use group::ff::PrimeField; use incrementalmerkletree::Position; -use rusqlite::{named_params, params, types::Value, Connection, Row}; -use std::rc::Rc; +use rusqlite::{named_params, params, Connection, Row}; use sapling::{self, Diversifier, Nullifier, Rseed}; use zcash_client_backend::{ @@ -21,7 +20,7 @@ use zip32::Scope; use crate::{error::SqliteClientError, AccountId, ReceivedNoteId}; -use super::{memo_repr, parse_scope, scope_code, wallet_birthday}; +use super::{memo_repr, parse_scope, scope_code}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedSaplingOutput { @@ -192,34 +191,14 @@ pub(crate) fn get_spendable_sapling_note( txid: &TxId, index: u32, ) -> Result>, SqliteClientError> { - let result = conn.query_row_and_then( - "SELECT sapling_received_notes.id, txid, output_index, - diversifier, value, rcm, commitment_tree_position, - accounts.ufvk, recipient_key_scope - FROM sapling_received_notes - INNER JOIN accounts ON accounts.id = sapling_received_notes.account_id - INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx - WHERE txid = :txid - AND output_index = :output_index - AND accounts.ufvk IS NOT NULL - AND recipient_key_scope IS NOT NULL - AND nf IS NOT NULL - AND commitment_tree_position IS NOT NULL - AND spent IS NULL", - named_params![ - ":txid": txid.as_ref(), - ":output_index": index, - ], - |row| to_spendable_note(params, row), - ); - - // `OptionalExtension` doesn't work here because the error type of `Result` is already - // `SqliteClientError` - match result { - Ok(r) => Ok(r), - Err(SqliteClientError::DbError(rusqlite::Error::QueryReturnedNoRows)) => Ok(None), - Err(e) => Err(e), - } + super::common::get_spendable_note( + conn, + params, + txid, + index, + ShieldedProtocol::Sapling, + to_spendable_note, + ) } /// Utility method for determining whether we have any spendable notes @@ -227,25 +206,6 @@ pub(crate) fn get_spendable_sapling_note( /// If the tip shard has unscanned ranges below the anchor height and greater than or equal to /// the wallet birthday, none of our notes can be spent because we cannot construct witnesses at /// the provided anchor height. -fn unscanned_tip_exists( - conn: &Connection, - anchor_height: BlockHeight, -) -> Result { - // v_sapling_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so - // we don't need to refer to the birthday in this query. - conn.query_row( - "SELECT EXISTS ( - SELECT 1 FROM v_sapling_shard_unscanned_ranges range - WHERE range.block_range_start <= :anchor_height - AND :anchor_height BETWEEN - range.subtree_start_height - AND IFNULL(range.subtree_end_height, :anchor_height) - )", - named_params![":anchor_height": u32::from(anchor_height),], - |row| row.get::<_, bool>(0), - ) -} - pub(crate) fn select_spendable_sapling_notes( conn: &Connection, params: &P, @@ -254,98 +214,16 @@ pub(crate) fn select_spendable_sapling_notes( anchor_height: BlockHeight, exclude: &[ReceivedNoteId], ) -> Result>, SqliteClientError> { - let birthday_height = match wallet_birthday(conn)? { - Some(birthday) => birthday, - None => { - // the wallet birthday can only be unknown if there are no accounts in the wallet; in - // such a case, the wallet has no notes to spend. - return Ok(vec![]); - } - }; - - if unscanned_tip_exists(conn, anchor_height)? { - return Ok(vec![]); - } - - // The goal of this SQL statement is to select the oldest notes until the required - // value has been reached. - // 1) Use a window function to create a view of all notes, ordered from oldest to - // newest, with an additional column containing a running sum: - // - Unspent notes accumulate the values of all unspent notes in that note's - // account, up to itself. - // - Spent notes accumulate the values of all notes in the transaction they were - // spent in, up to itself. - // - // 2) Select all unspent notes in the desired account, along with their running sum. - // - // 3) Select all notes for which the running sum was less than the required value, as - // well as a single note for which the sum was greater than or equal to the - // required value, bringing the sum of all selected notes across the threshold. - // - // 4) Match the selected notes against the witnesses at the desired height. - let mut stmt_select_notes = conn.prepare_cached( - "WITH eligible AS ( - SELECT - sapling_received_notes.id AS id, txid, output_index, - diversifier, value, rcm, commitment_tree_position, - SUM(value) - OVER (PARTITION BY sapling_received_notes.account_id, spent ORDER BY sapling_received_notes.id) AS so_far, - accounts.ufvk as ufvk, recipient_key_scope - FROM sapling_received_notes - INNER JOIN accounts - ON accounts.id = sapling_received_notes.account_id - INNER JOIN transactions - ON transactions.id_tx = sapling_received_notes.tx - WHERE sapling_received_notes.account_id = :account - AND accounts.ufvk IS NOT NULL - AND recipient_key_scope IS NOT NULL - AND nf IS NOT NULL - AND commitment_tree_position IS NOT NULL - AND spent IS NULL - AND transactions.block <= :anchor_height - AND sapling_received_notes.id NOT IN rarray(:exclude) - AND NOT EXISTS ( - SELECT 1 FROM v_sapling_shard_unscanned_ranges unscanned - -- select all the unscanned ranges involving the shard containing this note - WHERE sapling_received_notes.commitment_tree_position >= unscanned.start_position - AND sapling_received_notes.commitment_tree_position < unscanned.end_position_exclusive - -- exclude unscanned ranges that start above the anchor height (they don't affect spendability) - AND unscanned.block_range_start <= :anchor_height - -- exclude unscanned ranges that end below the wallet birthday - AND unscanned.block_range_end > :wallet_birthday - ) - ) - SELECT id, txid, output_index, - diversifier, value, rcm, commitment_tree_position, - ufvk, recipient_key_scope - FROM eligible WHERE so_far < :target_value - UNION - SELECT id, txid, output_index, - diversifier, value, rcm, commitment_tree_position, - ufvk, recipient_key_scope - FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", - )?; - - let excluded: Vec = exclude - .iter() - .filter_map(|n| matches!(n.0, ShieldedProtocol::Sapling).then(|| Value::from(n.1))) - .collect(); - let excluded_ptr = Rc::new(excluded); - - let notes = stmt_select_notes.query_and_then( - named_params![ - ":account": account.0, - ":anchor_height": &u32::from(anchor_height), - ":target_value": &u64::from(target_value), - ":exclude": &excluded_ptr, - ":wallet_birthday": u32::from(birthday_height) - ], - |r| to_spendable_note(params, r), - )?; - - notes - .filter_map(|r| r.transpose()) - .collect::>() + super::common::select_spendable_notes( + conn, + params, + account, + target_value, + anchor_height, + exclude, + ShieldedProtocol::Sapling, + to_spendable_note, + ) } /// Retrieves the set of nullifiers for "potentially spendable" Sapling notes that the From 6601820a2acfcbd9db944eaecc23a238dea10efc Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 10 Mar 2024 22:14:27 +0000 Subject: [PATCH 08/14] zcash_client_sqlite: Add Orchard support to `truncate_to_height` --- zcash_client_sqlite/src/wallet.rs | 37 ++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 8ef0f4db0..33273d7c4 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1733,7 +1733,7 @@ pub(crate) fn get_max_height_hash( pub(crate) fn get_min_unspent_height( conn: &rusqlite::Connection, ) -> Result, SqliteClientError> { - conn.query_row( + let min_sapling: Option = conn.query_row( "SELECT MIN(tx.block) FROM sapling_received_notes n JOIN transactions tx ON tx.id_tx = n.tx @@ -1743,8 +1743,27 @@ pub(crate) fn get_min_unspent_height( row.get(0) .map(|maybe_height: Option| maybe_height.map(|height| height.into())) }, - ) - .map_err(SqliteClientError::from) + )?; + #[cfg(feature = "orchard")] + let min_orchard: Option = conn.query_row( + "SELECT MIN(tx.block) + FROM orchard_received_notes n + JOIN transactions tx ON tx.id_tx = n.tx + WHERE n.spent IS NULL", + [], + |row| { + row.get(0) + .map(|maybe_height: Option| maybe_height.map(|height| height.into())) + }, + )?; + #[cfg(not(feature = "orchard"))] + let min_orchard = None; + + Ok(min_sapling + .zip(min_orchard) + .map(|(s, o)| s.min(o)) + .or(min_sapling) + .or(min_orchard)) } /// Truncates the database to the given height. @@ -1799,6 +1818,18 @@ pub(crate) fn truncate_to_height( );", [u32::from(block_height)], )?; + #[cfg(feature = "orchard")] + conn.execute( + "DELETE FROM orchard_received_notes + WHERE id IN ( + SELECT rn.id + FROM orchard_received_notes rn + LEFT OUTER JOIN transactions tx + ON tx.id_tx = rn.tx + WHERE tx.block IS NOT NULL AND tx.block > ? + );", + [u32::from(block_height)], + )?; // Do not delete sent notes; this can contain data that is not recoverable // from the chain. Wallets must continue to operate correctly in the From 1028894324cc97f5aa688a12a13bf4154232eed2 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 16:42:14 -0600 Subject: [PATCH 09/14] zcash_client_sqlite: Minor refactoring for improved debuggability & future Sapling flagging. --- zcash_client_backend/src/fees/common.rs | 31 ++++++++++++++----------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 8807a12f7..31c511daa 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -106,8 +106,20 @@ where #[cfg(not(feature = "orchard"))] let (change_pool, sapling_change) = (ShieldedProtocol::Sapling, 1); + 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 .bundle_type() .num_actions( orchard.inputs().len(), @@ -115,7 +127,7 @@ where ) .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( @@ -123,18 +135,9 @@ where 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)))?; From 8e09b78ca12779d0da51319953198dadcbd300cb Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 10 Mar 2024 23:40:06 +0000 Subject: [PATCH 10/14] zcash_client_sqlite: Call `mark_orchard_note_spent` in `WalletDb::store_sent_tx` --- zcash_client_sqlite/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 8c7e50d94..fa162ec69 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -960,6 +960,19 @@ impl WalletWrite for WalletDb )?; } } + if let Some(_bundle) = sent_tx.tx().orchard_bundle() { + #[cfg(feature = "orchard")] + for action in _bundle.actions() { + wallet::orchard::mark_orchard_note_spent( + wdb.conn.0, + tx_ref, + action.nullifier(), + )?; + } + + #[cfg(not(feature = "orchard"))] + panic!("Sent a transaction with Orchard Actions without `orchard` enabled?"); + } #[cfg(feature = "transparent-inputs")] for utxo_outpoint in sent_tx.utxos_spent() { From 5a6057b8fb3227a0ceb98af8f0a38f49e423347c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 11 Mar 2024 00:56:54 +0000 Subject: [PATCH 11/14] zcash_client_backend: Detect Orchard dust in `zip317::SingleOutputChangeStrategy` --- zcash_client_backend/CHANGELOG.md | 2 + .../src/data_api/wallet/input_selection.rs | 9 +- zcash_client_backend/src/fees.rs | 20 +++- zcash_client_backend/src/fees/common.rs | 95 +++++++++++++++---- zcash_client_backend/src/fees/zip317.rs | 74 ++++++++++++--- 5 files changed, 166 insertions(+), 34 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 86cbe6337..327c93d61 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -76,6 +76,8 @@ and this library adheres to Rust's notion of constraint on its `` 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 diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index 1627698ba..2cbef319e 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -445,8 +445,15 @@ where ) .map_err(InputSelectorError::Proposal); } - Err(ChangeError::DustInputs { mut sapling, .. }) => { + Err(ChangeError::DustInputs { + 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; diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 6c512a204..9106a3883 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -149,6 +149,9 @@ pub enum ChangeError { transparent: Vec, /// The identifiers for Sapling inputs having no current economic value sapling: Vec, + /// The identifiers for Orchard inputs having no current economic value + #[cfg(feature = "orchard")] + orchard: Vec, }, /// An error occurred that was specific to the change selection strategy in use. StrategyError(E), @@ -169,9 +172,13 @@ impl ChangeError { ChangeError::DustInputs { transparent, sapling, + #[cfg(feature = "orchard")] + orchard, } => ChangeError::DustInputs { transparent, sapling, + #[cfg(feature = "orchard")] + orchard, }, ChangeError::StrategyError(e) => ChangeError::StrategyError(f(e)), ChangeError::BundleError(e) => ChangeError::BundleError(e), @@ -194,10 +201,21 @@ impl fmt::Display for ChangeError { ChangeError::DustInputs { transparent, sapling, + #[cfg(feature = "orchard")] + orchard, } => { + #[cfg(feature = "orchard")] + let orchard_len = orchard.len(); + #[cfg(not(feature = "orchard"))] + let orchard_len = 0; + // 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, + "Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.", + transparent.len() + sapling.len() + orchard_len, + ) } ChangeError::StrategyError(err) => { write!(f, "{}", err) diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 31c511daa..a8a791a52 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -17,30 +17,26 @@ use super::{ #[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( transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, - dust_output_policy: &DustOutputPolicy, - default_dust_threshold: NonNegativeAmount, - change_memo: Option, - _fallback_change_pool: ShieldedProtocol, -) -> Result> +) -> Result> where E: From + From, { let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow)); - let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow)); let t_in = transparent_inputs .iter() @@ -85,13 +81,30 @@ where #[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( + _net_flows: &NetFlows, + _fallback_change_pool: ShieldedProtocol, +) -> Result<(ShieldedProtocol, usize, usize), ChangeError> +where + E: From + From, +{ // 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) @@ -104,7 +117,45 @@ where } }; #[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, + #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, + dust_output_policy: &DustOutputPolicy, + default_dust_threshold: NonNegativeAmount, + change_memo: Option, + _fallback_change_pool: ShieldedProtocol, +) -> Result> +where + E: From + From, +{ + let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow)); + let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow)); + + let net_flows = calculate_net_flows::( + transparent_inputs, + transparent_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, + )?; + let (change_pool, sapling_change, _orchard_change) = + single_change_output_policy::(&net_flows, _fallback_change_pool)?; let sapling_input_count = sapling .bundle_type() @@ -123,7 +174,7 @@ where .bundle_type() .num_actions( orchard.inputs().len(), - orchard.outputs().len() + orchard_change, + orchard.outputs().len() + _orchard_change, ) .map_err(ChangeError::BundleError)?; #[cfg(not(feature = "orchard"))] @@ -141,8 +192,10 @@ where ) .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, diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index 3b703f67e..ebc2aec86 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -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")] @@ -93,32 +93,73 @@ impl ChangeStrategy for SingleOutputChangeStrategy { }) .collect(); + #[cfg(feature = "orchard")] + let mut orchard_dust: Vec = orchard + .inputs() + .iter() + .filter_map(|i| { + if orchard_fees::InputView::::value(i) < self.fee_rule.marginal_fee() { + Some(orchard_fees::InputView::::note_id(i).clone()) + } else { + None + } + }) + .collect(); + #[cfg(not(feature = "orchard"))] + let mut orchard_dust: Vec = 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::( + transparent_inputs, + transparent_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, + )?; + let (_, sapling_change, orchard_change) = + single_change_output_policy::( + &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); @@ -126,6 +167,12 @@ impl ChangeStrategy for SingleOutputChangeStrategy { .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 @@ -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, }); } } From d68a01a221c005e2373ed809bb77cb591a2019ae Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 11 Mar 2024 19:00:26 +0000 Subject: [PATCH 12/14] Fix typos --- zcash_client_sqlite/src/lib.rs | 2 +- zcash_client_sqlite/src/wallet/scanning.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index fa162ec69..992ac9533 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -744,7 +744,7 @@ impl WalletWrite for WalletDb .map(|res| (res.subtree, res.checkpoints)) .collect::>(); - // Update the Sapling note commitment tree with all newly read note commitments + // Update the Orchard note commitment tree with all newly read note commitments let mut orchard_subtrees = orchard_subtrees.into_iter(); wdb.with_orchard_tree_mut::<_, _, Self::Error>(move |orchard_tree| { for (tree, checkpoints) in &mut orchard_subtrees { diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 310c48fce..814049313 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -442,7 +442,7 @@ pub(crate) fn update_chain_tip( // `ScanRange` uses an exclusive upper bound. let chain_end = new_tip + 1; - // Read the maximum height from each of the the shards tables. The minimum of the two + // Read the maximum height from each of the shards tables. The minimum of the two // gives the start of a height range that covers the last incomplete shard of both the // Sapling and Orchard pools. let sapling_shard_tip = tip_shard_end_height(conn, SAPLING_TABLES_PREFIX)?; From 2eb5061eb10500177a9b97b2475b22d626473302 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 12 Mar 2024 10:01:49 -0600 Subject: [PATCH 13/14] zcash_client_sqlite: Ensure that truncation is applied to the Orchard note commitment tree. --- zcash_client_sqlite/src/wallet.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 33273d7c4..02a158332 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1805,6 +1805,10 @@ pub(crate) fn truncate_to_height( wdb.with_sapling_tree_mut(|tree| { tree.truncate_removing_checkpoint(&block_height).map(|_| ()) })?; + #[cfg(feature = "orchard")] + wdb.with_orchard_tree_mut(|tree| { + tree.truncate_removing_checkpoint(&block_height).map(|_| ()) + })?; // Rewind received notes conn.execute( From 5a2897061ca1fd31966b79666e7139d694e9832a Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 12 Mar 2024 10:12:42 -0600 Subject: [PATCH 14/14] Apply suggestions from code review Co-authored-by: str4d --- zcash_client_sqlite/src/wallet/common.rs | 4 +--- zcash_client_sqlite/src/wallet/orchard.rs | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs index 31f9b23ef..1a47fc404 100644 --- a/zcash_client_sqlite/src/wallet/common.rs +++ b/zcash_client_sqlite/src/wallet/common.rs @@ -31,7 +31,7 @@ fn per_protocol_names(protocol: ShieldedProtocol) -> (&'static str, &'static str fn unscanned_tip_exists( conn: &Connection, anchor_height: BlockHeight, - table_prefix: &str, + table_prefix: &'static str, ) -> Result { // v_sapling_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so // we don't need to refer to the birthday in this query. @@ -140,8 +140,6 @@ where // 3) Select all notes for which the running sum was less than the required value, as // well as a single note for which the sum was greater than or equal to the // required value, bringing the sum of all selected notes across the threshold. - // - // 4) Match the selected notes against the witnesses at the desired height. let mut stmt_select_notes = conn.prepare_cached( &format!( "WITH eligible AS ( diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index a4b1f83b9..e3eacdfe8 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -140,6 +140,13 @@ fn to_spendable_note( let ufvk_str: Option = row.get(8)?; let scope_code: Option = row.get(9)?; + // If we don't have information about the recipient key scope or the ufvk we can't determine + // which spending key to use. This may be because the received note was associated with an + // imported viewing key, so we treat such notes as not spendable. Although this method is + // presently only called using the results of queries where both the ufvk and + // recipient_key_scope columns are checked to be non-null, this is method is written + // defensively to account for the fact that both of these are nullable columns in case it + // is used elsewhere in the future. ufvk_str .zip(scope_code) .map(|(ufvk_str, scope_code)| { @@ -176,10 +183,6 @@ fn to_spendable_note( .transpose() } -// The `clippy::let_and_return` lint is explicitly allowed here because a bug in Clippy -// (https://github.com/rust-lang/rust-clippy/issues/11308) means it fails to identify that the `result` temporary -// is required in order to resolve the borrows involved in the `query_and_then` call. -#[allow(clippy::let_and_return)] pub(crate) fn get_spendable_orchard_note( conn: &Connection, params: &P, @@ -299,7 +302,7 @@ pub(crate) fn get_orchard_nullifiers( // Get the nullifiers for the notes we are tracking let mut stmt_fetch_nullifiers = match query { NullifierQuery::Unspent => conn.prepare( - "SELECT rn.id, rn.account_id, rn.nf + "SELECT rn.account_id, rn.nf FROM orchard_received_notes rn LEFT OUTER JOIN transactions tx ON tx.id_tx = rn.spent @@ -307,15 +310,15 @@ pub(crate) fn get_orchard_nullifiers( AND nf IS NOT NULL", )?, NullifierQuery::All => conn.prepare( - "SELECT rn.id, rn.account_id, rn.nf + "SELECT rn.account_id, rn.nf FROM orchard_received_notes rn WHERE nf IS NOT NULL", )?, }; let nullifiers = stmt_fetch_nullifiers.query_and_then([], |row| { - let account = AccountId(row.get(1)?); - let nf_bytes: [u8; 32] = row.get(2)?; + let account = AccountId(row.get(0)?); + let nf_bytes: [u8; 32] = row.get(1)?; Ok::<_, rusqlite::Error>((account, Nullifier::from_bytes(&nf_bytes).unwrap())) })?;