From 97e5e9f64c758025fa8e7b77146f28c8b5504d4c Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 12 Mar 2024 10:12:42 -0600 Subject: [PATCH] Apply suggestions from code review Co-authored-by: str4d --- zcash_client_sqlite/src/wallet/common.rs | 4 +--- zcash_client_sqlite/src/wallet/orchard.rs | 11 +++++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs index 31f9b23ef7..1a47fc404a 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 a4b1f83b9f..f9820048b7 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,