From b5bbbb6f5a57693ba69527743e3b05537f378307 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Tue, 9 Mar 2021 13:15:27 -0700
Subject: [PATCH 1/6] Factor out nullifier update from database actions in
 scan_cached_blocks

---
 zcash_client_backend/src/data_api.rs        | 37 ++++++++++++++++-
 zcash_client_backend/src/data_api/chain.rs  | 45 +++++++++++----------
 zcash_client_backend/src/data_api/wallet.rs |  2 +-
 zcash_client_backend/src/wallet.rs          |  1 +
 zcash_client_backend/src/welding_rig.rs     | 25 +++++++-----
 zcash_client_sqlite/src/lib.rs              |  3 +-
 zcash_client_sqlite/src/wallet.rs           |  3 +-
 zcash_primitives/src/merkle_tree.rs         | 15 ++-----
 zcash_primitives/src/primitives.rs          |  2 +-
 9 files changed, 81 insertions(+), 52 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index 544510f38..5412855ac 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/zcash_client_backend/src/data_api.rs
@@ -177,6 +177,33 @@ pub trait WalletRead {
     ) -> Result<Vec<SpendableNote>, Self::Error>;
 }
 
+// /// The subset of information that is relevant to this wallet that has been
+// /// decrypted and extracted from a [CompactBlock].
+// pub struct PrunedBlock {
+//     block_height: BlockHeight,
+//     block_hash: BlockHash,
+//     block_time: u32,
+//     commitment_tree: &CommitmentTree<Node>,
+//     transactions: &Vec<ChainTx>,
+// }
+//
+// pub struct BlockInserted {
+//     witnesses: Vec<(Self::NoteRef, IncrementalWitness<Node>)>
+// }
+//
+//
+//
+//
+// pub trait WalletWrite2: WalletRead {
+//     fn insert_pruned_block(
+//         &mut self,
+//         block: &PrunedBlock
+//     ) -> Result<BlockInserted, Self::Error>;
+//
+//
+//
+// }
+
 /// This trait encapsulates the write capabilities required to update stored
 /// wallet data.
 pub trait WalletWrite: WalletRead {
@@ -243,7 +270,6 @@ pub trait WalletWrite: WalletRead {
     fn put_received_note<T: ShieldedOutput>(
         &mut self,
         output: &T,
-        nf: &Option<Nullifier>,
         tx_ref: Self::TxRef,
     ) -> Result<Self::NoteRef, Self::Error>;
 
@@ -317,6 +343,7 @@ pub trait ShieldedOutput {
     fn note(&self) -> &Note;
     fn memo(&self) -> Option<&Memo>;
     fn is_change(&self) -> Option<bool>;
+    fn nullifier(&self) -> Option<Nullifier>;
 }
 
 impl ShieldedOutput for WalletShieldedOutput {
@@ -338,6 +365,10 @@ impl ShieldedOutput for WalletShieldedOutput {
     fn is_change(&self) -> Option<bool> {
         Some(self.is_change)
     }
+
+    fn nullifier(&self) -> Option<Nullifier> {
+        self.nf.clone()
+    }
 }
 
 impl ShieldedOutput for DecryptedOutput {
@@ -359,6 +390,9 @@ impl ShieldedOutput for DecryptedOutput {
     fn is_change(&self) -> Option<bool> {
         None
     }
+    fn nullifier(&self) -> Option<Nullifier> {
+        None
+    }
 }
 
 #[cfg(feature = "test-dependencies")]
@@ -537,7 +571,6 @@ pub mod testing {
         fn put_received_note<T: ShieldedOutput>(
             &mut self,
             _output: &T,
-            _nf: &Option<Nullifier>,
             _tx_ref: Self::TxRef,
         ) -> Result<Self::NoteRef, Self::Error> {
             Ok(0u32)
diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs
index a57648a4c..984374076 100644
--- a/zcash_client_backend/src/data_api/chain.rs
+++ b/zcash_client_backend/src/data_api/chain.rs
@@ -95,6 +95,7 @@ use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight, NetworkUpgrade},
     merkle_tree::CommitmentTree,
+    primitives::Nullifier,
 };
 
 use crate::{
@@ -261,9 +262,9 @@ where
 
     // Fetch the ExtendedFullViewingKeys we are tracking
     let extfvks = data.get_extended_full_viewing_keys()?;
-    let ivks: Vec<_> = extfvks
+    let vks: Vec<_> = extfvks
         .iter()
-        .map(|(a, extfvk)| (*a, extfvk.fvk.vk.ivk()))
+        .map(|(a, extfvk)| (*a, &extfvk.fvk.vk))
         .collect();
 
     // Get the most recent CommitmentTree
@@ -296,7 +297,7 @@ where
             scan_block(
                 params,
                 block,
-                &ivks,
+                &vks,
                 &nullifiers,
                 &mut tree,
                 &mut witness_refs[..],
@@ -332,7 +333,7 @@ where
             // Insert the block into the database.
             up.insert_block(current_height, block_hash, block_time, &tree)?;
 
-            for tx in txs {
+            for tx in &txs {
                 let tx_row = up.put_tx_meta(&tx, current_height)?;
 
                 // Mark notes as spent and remove them from the scanning cache
@@ -340,24 +341,11 @@ where
                     up.mark_spent(tx_row, &spend.nf)?;
                 }
 
-                // remove spent nullifiers from the nullifier set
-                nullifiers
-                    .retain(|(_, nf)| !tx.shielded_spends.iter().any(|spend| &spend.nf == nf));
+                for output in &tx.shielded_outputs {
+                    let received_note_id = up.put_received_note(output, tx_row)?;
 
-                for output in tx.shielded_outputs {
-                    if let Some(extfvk) = &extfvks.get(&output.account) {
-                        let nf = output
-                            .note
-                            .nf(&extfvk.fvk.vk, output.witness.position() as u64);
-
-                        let received_note_id = up.put_received_note(&output, &Some(nf), tx_row)?;
-
-                        // Save witness for note.
-                        witnesses.push((received_note_id, output.witness));
-
-                        // Cache nullifier for note (to detect subsequent spends in this scan).
-                        nullifiers.push((output.account, nf));
-                    }
+                    // Save witness for note.
+                    witnesses.push((received_note_id, output.witness.clone()));
                 }
             }
 
@@ -373,7 +361,20 @@ where
             up.update_expired_notes(last_height)?;
 
             Ok(())
-        })
+        })?;
+
+        let spent_nf: Vec<Nullifier> = txs
+            .iter()
+            .flat_map(|tx| tx.shielded_spends.iter().map(|spend| spend.nf))
+            .collect();
+        nullifiers.retain(|(_, nf)| !spent_nf.contains(nf));
+        nullifiers.extend(txs.iter().flat_map(|tx| {
+            tx.shielded_outputs
+                .iter()
+                .flat_map(|out| out.nf.map(|nf| (out.account, nf)))
+        }));
+
+        Ok(())
     })?;
 
     Ok(())
diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs
index 60b466efb..1cc5789a7 100644
--- a/zcash_client_backend/src/data_api/wallet.rs
+++ b/zcash_client_backend/src/data_api/wallet.rs
@@ -59,7 +59,7 @@ where
                 if output.outgoing {
                     up.put_sent_note(&output, tx_ref)?;
                 } else {
-                    up.put_received_note(&output, &None, tx_ref)?;
+                    up.put_received_note(&output, tx_ref)?;
                 }
             }
 
diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs
index 83487866c..399fe85bd 100644
--- a/zcash_client_backend/src/wallet.rs
+++ b/zcash_client_backend/src/wallet.rs
@@ -60,6 +60,7 @@ pub struct WalletShieldedOutput {
     pub to: PaymentAddress,
     pub is_change: bool,
     pub witness: IncrementalWitness<Node>,
+    pub nf: Option<Nullifier>,
 }
 
 pub struct SpendableNote {
diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs
index 738af5ace..7bb103bdf 100644
--- a/zcash_client_backend/src/welding_rig.rs
+++ b/zcash_client_backend/src/welding_rig.rs
@@ -7,7 +7,7 @@ use zcash_primitives::{
     consensus::{self, BlockHeight},
     merkle_tree::{CommitmentTree, IncrementalWitness},
     note_encryption::try_sapling_compact_note_decryption,
-    primitives::{Nullifier, SaplingIvk},
+    primitives::{Nullifier, ViewingKey},
     sapling::Node,
     transaction::TxId,
 };
@@ -29,7 +29,7 @@ fn scan_output<P: consensus::Parameters>(
     params: &P,
     height: BlockHeight,
     (index, output): (usize, CompactOutput),
-    ivks: &[(AccountId, SaplingIvk)],
+    vks: &[(AccountId, &ViewingKey)],
     spent_from_accounts: &HashSet<AccountId>,
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
@@ -53,7 +53,8 @@ fn scan_output<P: consensus::Parameters>(
     }
     tree.append(node).unwrap();
 
-    for (account, ivk) in ivks.iter() {
+    for (account, vk) in vks.iter() {
+        let ivk = vk.ivk();
         let (note, to) =
             match try_sapling_compact_note_decryption(params, height, &ivk, &epk, &cmu, &ct) {
                 Some(ret) => ret,
@@ -68,6 +69,9 @@ fn scan_output<P: consensus::Parameters>(
         // - Notes sent from one account to itself.
         let is_change = spent_from_accounts.contains(&account);
 
+        let witness = IncrementalWitness::from_tree(tree);
+        let nf = note.nf(&vk, witness.position() as u64);
+
         return Some(WalletShieldedOutput {
             index,
             cmu,
@@ -76,16 +80,17 @@ fn scan_output<P: consensus::Parameters>(
             note,
             to,
             is_change,
-            witness: IncrementalWitness::from_tree(tree),
+            witness,
+            nf: Some(nf),
         });
     }
     None
 }
 
-/// Scans a [`CompactBlock`] with a set of [`ExtendedFullViewingKey`]s.
+/// Scans a [`CompactBlock`] with a set of [`ViewingKey`]s.
 ///
 /// Returns a vector of [`WalletTx`]s belonging to any of the given
-/// [`ExtendedFullViewingKey`]s, and the corresponding new [`IncrementalWitness`]es.
+/// [`ViewingKey`]s.
 ///
 /// The given [`CommitmentTree`] and existing [`IncrementalWitness`]es are
 /// incremented appropriately.
@@ -94,7 +99,7 @@ fn scan_output<P: consensus::Parameters>(
 pub fn scan_block<P: consensus::Parameters>(
     params: &P,
     block: CompactBlock,
-    ivks: &[(AccountId, SaplingIvk)],
+    vks: &[(AccountId, &ViewingKey)],
     nullifiers: &[(AccountId, Nullifier)],
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
@@ -167,7 +172,7 @@ pub fn scan_block<P: consensus::Parameters>(
                     params,
                     block_height,
                     to_scan,
-                    ivks,
+                    vks,
                     &spent_from_accounts,
                     tree,
                     existing_witnesses,
@@ -334,7 +339,7 @@ mod tests {
         let txs = scan_block(
             &Network::TestNetwork,
             cb,
-            &[(AccountId(0), extfvk.fvk.vk.ivk())],
+            &[(AccountId(0), &extfvk.fvk.vk)],
             &[],
             &mut tree,
             &mut [],
@@ -373,7 +378,7 @@ mod tests {
         let txs = scan_block(
             &Network::TestNetwork,
             cb,
-            &[(AccountId(0), extfvk.fvk.vk.ivk())],
+            &[(AccountId(0), &extfvk.fvk.vk)],
             &[],
             &mut tree,
             &mut [],
diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs
index b1a722c01..91cb83dbe 100644
--- a/zcash_client_sqlite/src/lib.rs
+++ b/zcash_client_sqlite/src/lib.rs
@@ -427,10 +427,9 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
     fn put_received_note<T: ShieldedOutput>(
         &mut self,
         output: &T,
-        nf_opt: &Option<Nullifier>,
         tx_ref: Self::TxRef,
     ) -> Result<Self::NoteRef, Self::Error> {
-        wallet::put_received_note(self, output, nf_opt, tx_ref)
+        wallet::put_received_note(self, output, tx_ref)
     }
 
     fn insert_witness(
diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs
index 014448146..934171de0 100644
--- a/zcash_client_sqlite/src/wallet.rs
+++ b/zcash_client_sqlite/src/wallet.rs
@@ -534,7 +534,6 @@ pub fn mark_spent<'a, P>(
 pub fn put_received_note<'a, P, T: ShieldedOutput>(
     stmts: &mut DataConnStmtCache<'a, P>,
     output: &T,
-    nf_opt: &Option<Nullifier>,
     tx_ref: i64,
 ) -> Result<NoteId, SqliteClientError> {
     let rcm = output.note().rcm().to_repr();
@@ -546,7 +545,7 @@ pub fn put_received_note<'a, P, T: ShieldedOutput>(
     let is_change = output.is_change();
     let tx = tx_ref;
     let output_index = output.index() as i64;
-    let nf_bytes = nf_opt.map(|nf| nf.0.to_vec());
+    let nf_bytes = output.nullifier().map(|nf| nf.0.to_vec());
 
     let sql_args: &[(&str, &dyn ToSql)] = &[
         (&":account", &account),
diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs
index 6258d2299..f01a218d8 100644
--- a/zcash_primitives/src/merkle_tree.rs
+++ b/zcash_primitives/src/merkle_tree.rs
@@ -3,7 +3,6 @@
 use byteorder::{LittleEndian, ReadBytesExt};
 use std::collections::VecDeque;
 use std::io::{self, Read, Write};
-use std::iter;
 
 use crate::sapling::SAPLING_COMMITMENT_TREE_DEPTH;
 use crate::serialize::{Optional, Vector};
@@ -274,17 +273,9 @@ impl<Node: Hashable> IncrementalWitness<Node> {
             .as_ref()
             .map(|c| c.root_inner(self.cursor_depth, PathFiller::empty()));
 
-        let queue = if let Some(node) = cursor_root {
-            self.filled
-                .iter()
-                .cloned()
-                .chain(iter::once(node))
-                .collect()
-        } else {
-            self.filled.iter().cloned().collect()
-        };
-
-        PathFiller { queue }
+        PathFiller {
+            queue: self.filled.iter().cloned().chain(cursor_root).collect(),
+        }
     }
 
     /// Finds the next "depth" of an unfilled subtree.
diff --git a/zcash_primitives/src/primitives.rs b/zcash_primitives/src/primitives.rs
index 6f67d10d9..2fd78253d 100644
--- a/zcash_primitives/src/primitives.rs
+++ b/zcash_primitives/src/primitives.rs
@@ -48,7 +48,7 @@ impl ProofGenerationKey {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct ViewingKey {
     pub ak: jubjub::SubgroupPoint,
     pub nk: jubjub::SubgroupPoint,

From a74cc8b231143155c4bf5d212319d3c9be8d07be Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Tue, 9 Mar 2021 18:10:44 -0700
Subject: [PATCH 2/6] Remove sqlite-specific data organization from data access
 api.

---
 zcash_client_backend/src/data_api.rs        | 217 ++++++--------------
 zcash_client_backend/src/data_api/chain.rs  |  58 ++----
 zcash_client_backend/src/data_api/wallet.rs |  49 ++---
 zcash_client_sqlite/src/lib.rs              | 171 ++++++++-------
 zcash_client_sqlite/src/wallet.rs           |   6 +-
 5 files changed, 197 insertions(+), 304 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index 5412855ac..4a7a59606 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/zcash_client_backend/src/data_api.rs
@@ -177,32 +177,30 @@ pub trait WalletRead {
     ) -> Result<Vec<SpendableNote>, Self::Error>;
 }
 
-// /// The subset of information that is relevant to this wallet that has been
-// /// decrypted and extracted from a [CompactBlock].
-// pub struct PrunedBlock {
-//     block_height: BlockHeight,
-//     block_hash: BlockHash,
-//     block_time: u32,
-//     commitment_tree: &CommitmentTree<Node>,
-//     transactions: &Vec<ChainTx>,
-// }
-//
-// pub struct BlockInserted {
-//     witnesses: Vec<(Self::NoteRef, IncrementalWitness<Node>)>
-// }
-//
-//
-//
-//
-// pub trait WalletWrite2: WalletRead {
-//     fn insert_pruned_block(
-//         &mut self,
-//         block: &PrunedBlock
-//     ) -> Result<BlockInserted, Self::Error>;
-//
-//
-//
-// }
+/// The subset of information that is relevant to this wallet that has been
+/// decrypted and extracted from a [CompactBlock].
+pub struct PrunedBlock<'a> {
+    pub block_height: BlockHeight,
+    pub block_hash: BlockHash,
+    pub block_time: u32,
+    pub commitment_tree: &'a CommitmentTree<Node>,
+    pub transactions: &'a Vec<WalletTx>,
+}
+
+pub struct ReceivedTransaction<'a> {
+    pub tx: &'a Transaction,
+    pub outputs: &'a Vec<DecryptedOutput>,
+}
+
+pub struct SentTransaction<'a> {
+    pub tx: &'a Transaction,
+    pub created: time::OffsetDateTime,
+    pub output_index: usize,
+    pub account: AccountId,
+    pub recipient_address: &'a RecipientAddress,
+    pub value: Amount,
+    pub memo: Option<Memo>,
+}
 
 /// This trait encapsulates the write capabilities required to update stored
 /// wallet data.
@@ -216,14 +214,18 @@ pub trait WalletWrite: WalletRead {
     where
         F: FnOnce(&mut Self) -> Result<A, Self::Error>;
 
-    /// Add the data for a block to the data store.
-    fn insert_block(
+    fn insert_pruned_block(
         &mut self,
-        block_height: BlockHeight,
-        block_hash: BlockHash,
-        block_time: u32,
-        commitment_tree: &CommitmentTree<Node>,
-    ) -> Result<(), Self::Error>;
+        block: &PrunedBlock,
+        updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
+    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error>;
+
+    fn store_received_tx(
+        &mut self,
+        received_tx: &ReceivedTransaction,
+    ) -> Result<Self::TxRef, Self::Error>;
+
+    fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<Self::TxRef, Self::Error>;
 
     /// Rewinds the wallet database to the specified height.
     ///
@@ -240,47 +242,9 @@ pub trait WalletWrite: WalletRead {
     /// There may be restrictions on how far it is possible to rewind.
     fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error>;
 
-    /// Add wallet-relevant metadata for a specific transaction to the data
-    /// store.
-    fn put_tx_meta(
-        &mut self,
-        tx: &WalletTx,
-        height: BlockHeight,
-    ) -> Result<Self::TxRef, Self::Error>;
-
-    /// Add a full transaction contents to the data store.
-    fn put_tx_data(
-        &mut self,
-        tx: &Transaction,
-        created_at: Option<time::OffsetDateTime>,
-    ) -> Result<Self::TxRef, Self::Error>;
-
     /// Mark the specified transaction as spent and record the nullifier.
     fn mark_spent(&mut self, tx_ref: Self::TxRef, nf: &Nullifier) -> Result<(), Self::Error>;
 
-    /// Record a note as having been received, along with its nullifier and the transaction
-    /// within which the note was created.
-    ///
-    /// Implementations of this method must be exclusively additive with respect to stored
-    /// data; passing `None` for the nullifier should not be interpreted as deleting nullifier
-    /// information from the underlying store.
-    ///
-    /// Implementations of this method must ensure that attempting to record the same note
-    /// with a different nullifier to that already stored will return an error.
-    fn put_received_note<T: ShieldedOutput>(
-        &mut self,
-        output: &T,
-        tx_ref: Self::TxRef,
-    ) -> Result<Self::NoteRef, Self::Error>;
-
-    /// Add the incremental witness for the specified note to the database.
-    fn insert_witness(
-        &mut self,
-        note_id: Self::NoteRef,
-        witness: &IncrementalWitness<Node>,
-        height: BlockHeight,
-    ) -> Result<(), Self::Error>;
-
     /// Remove all incremental witness data before the specified block height.
     //  TODO: this is a backend-specific optimization that probably shouldn't be part of
     //  the public API
@@ -292,26 +256,6 @@ pub trait WalletWrite: WalletRead {
     //  TODO: this is a backend-specific optimization that probably shouldn't be part of
     //  the public API
     fn update_expired_notes(&mut self, from_height: BlockHeight) -> Result<(), Self::Error>;
-
-    /// Add the decrypted contents of a sent note to the database if it does not exist;
-    /// otherwise, update the note. This is useful in the case of a wallet restore where
-    /// the send of the note is being discovered via trial decryption.
-    fn put_sent_note(
-        &mut self,
-        output: &DecryptedOutput,
-        tx_ref: Self::TxRef,
-    ) -> Result<(), Self::Error>;
-
-    /// Add the decrypted contents of a sent note to the database.
-    fn insert_sent_note(
-        &mut self,
-        tx_ref: Self::TxRef,
-        output_index: usize,
-        account: AccountId,
-        to: &RecipientAddress,
-        value: Amount,
-        memo: Option<Memo>,
-    ) -> Result<(), Self::Error>;
 }
 
 /// This trait provides sequential access to raw blockchain data via a callback-oriented
@@ -403,21 +347,21 @@ pub mod testing {
         block::BlockHash,
         consensus::BlockHeight,
         merkle_tree::{CommitmentTree, IncrementalWitness},
-        note_encryption::Memo,
         primitives::{Nullifier, PaymentAddress},
         sapling::Node,
-        transaction::{components::Amount, Transaction, TxId},
+        transaction::{components::Amount, TxId},
         zip32::ExtendedFullViewingKey,
     };
 
     use crate::{
-        address::RecipientAddress,
-        decrypt::DecryptedOutput,
         proto::compact_formats::CompactBlock,
-        wallet::{AccountId, SpendableNote, WalletTx},
+        wallet::{AccountId, SpendableNote},
     };
 
-    use super::{error::Error, BlockSource, ShieldedOutput, WalletRead, WalletWrite};
+    use super::{
+        error::Error, BlockSource, PrunedBlock, ReceivedTransaction, SentTransaction, WalletRead,
+        WalletWrite,
+    };
 
     pub struct MockBlockSource {}
 
@@ -534,57 +478,36 @@ pub mod testing {
             f(self)
         }
 
-        fn insert_block(
+        fn insert_pruned_block(
             &mut self,
-            _block_height: BlockHeight,
-            _block_hash: BlockHash,
-            _block_time: u32,
-            _commitment_tree: &CommitmentTree<Node>,
-        ) -> Result<(), Self::Error> {
-            Ok(())
+            _block: &PrunedBlock,
+            _updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
+        ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+            Ok(vec![])
+        }
+
+        fn store_received_tx(
+            &mut self,
+            _received_tx: &ReceivedTransaction,
+        ) -> Result<Self::TxRef, Self::Error> {
+            Ok(TxId([0u8; 32]))
+        }
+
+        fn store_sent_tx(
+            &mut self,
+            _sent_tx: &SentTransaction,
+        ) -> Result<Self::TxRef, Self::Error> {
+            Ok(TxId([0u8; 32]))
         }
 
         fn rewind_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> {
             Ok(())
         }
 
-        fn put_tx_meta(
-            &mut self,
-            _tx: &WalletTx,
-            _height: BlockHeight,
-        ) -> Result<Self::TxRef, Self::Error> {
-            Ok(TxId([0u8; 32]))
-        }
-
-        fn put_tx_data(
-            &mut self,
-            _tx: &Transaction,
-            _created_at: Option<time::OffsetDateTime>,
-        ) -> Result<Self::TxRef, Self::Error> {
-            Ok(TxId([0u8; 32]))
-        }
-
         fn mark_spent(&mut self, _tx_ref: Self::TxRef, _nf: &Nullifier) -> Result<(), Self::Error> {
             Ok(())
         }
 
-        fn put_received_note<T: ShieldedOutput>(
-            &mut self,
-            _output: &T,
-            _tx_ref: Self::TxRef,
-        ) -> Result<Self::NoteRef, Self::Error> {
-            Ok(0u32)
-        }
-
-        fn insert_witness(
-            &mut self,
-            _note_id: Self::NoteRef,
-            _witness: &IncrementalWitness<Node>,
-            _height: BlockHeight,
-        ) -> Result<(), Self::Error> {
-            Ok(())
-        }
-
         fn prune_witnesses(&mut self, _from_height: BlockHeight) -> Result<(), Self::Error> {
             Ok(())
         }
@@ -592,25 +515,5 @@ pub mod testing {
         fn update_expired_notes(&mut self, _from_height: BlockHeight) -> Result<(), Self::Error> {
             Ok(())
         }
-
-        fn put_sent_note(
-            &mut self,
-            _output: &DecryptedOutput,
-            _tx_ref: Self::TxRef,
-        ) -> Result<(), Self::Error> {
-            Ok(())
-        }
-
-        fn insert_sent_note(
-            &mut self,
-            _tx_ref: Self::TxRef,
-            _output_index: usize,
-            _account: AccountId,
-            _to: &RecipientAddress,
-            _value: Amount,
-            _memo: Option<Memo>,
-        ) -> Result<(), Self::Error> {
-            Ok(())
-        }
     }
 }
diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs
index 984374076..e6b554a5d 100644
--- a/zcash_client_backend/src/data_api/chain.rs
+++ b/zcash_client_backend/src/data_api/chain.rs
@@ -101,7 +101,7 @@ use zcash_primitives::{
 use crate::{
     data_api::{
         error::{ChainInvalid, Error},
-        BlockSource, WalletWrite,
+        BlockSource, PrunedBlock, WalletWrite,
     },
     proto::compact_formats::CompactBlock,
     wallet::WalletTx,
@@ -280,13 +280,13 @@ where
 
     cache.with_blocks(last_height, limit, |block: CompactBlock| {
         let current_height = block.height();
+
         // Scanned blocks MUST be height-sequential.
         if current_height != (last_height + 1) {
             return Err(
                 ChainInvalid::block_height_discontinuity(last_height + 1, current_height).into(),
             );
         }
-        last_height = current_height;
 
         let block_hash = BlockHash::from_slice(&block.hash);
         let block_time = block.time;
@@ -310,7 +310,7 @@ where
             let cur_root = tree.root();
             for row in &witnesses {
                 if row.1.root() != cur_root {
-                    return Err(Error::InvalidWitnessAnchor(row.0, last_height).into());
+                    return Err(Error::InvalidWitnessAnchor(row.0, current_height).into());
                 }
             }
             for tx in &txs {
@@ -319,7 +319,7 @@ where
                         return Err(Error::InvalidNewWitnessAnchor(
                             output.index,
                             tx.txid,
-                            last_height,
+                            current_height,
                             output.witness.root(),
                         )
                         .into());
@@ -328,40 +328,22 @@ where
             }
         }
 
-        // database updates for each block are transactional
-        data.transactionally(|up| {
-            // Insert the block into the database.
-            up.insert_block(current_height, block_hash, block_time, &tree)?;
+        let new_witnesses = data.insert_pruned_block(
+            &(PrunedBlock {
+                block_height: current_height,
+                block_hash,
+                block_time,
+                commitment_tree: &tree,
+                transactions: &txs,
+            }),
+            &witnesses,
+        )?;
 
-            for tx in &txs {
-                let tx_row = up.put_tx_meta(&tx, current_height)?;
+        // Prune the stored witnesses (we only expect rollbacks of at most 100 blocks).
+        data.prune_witnesses(current_height - 100)?;
 
-                // Mark notes as spent and remove them from the scanning cache
-                for spend in &tx.shielded_spends {
-                    up.mark_spent(tx_row, &spend.nf)?;
-                }
-
-                for output in &tx.shielded_outputs {
-                    let received_note_id = up.put_received_note(output, tx_row)?;
-
-                    // Save witness for note.
-                    witnesses.push((received_note_id, output.witness.clone()));
-                }
-            }
-
-            // Insert current witnesses into the database.
-            for (received_note_id, witness) in witnesses.iter() {
-                up.insert_witness(*received_note_id, witness, last_height)?;
-            }
-
-            // Prune the stored witnesses (we only expect rollbacks of at most 100 blocks).
-            up.prune_witnesses(last_height - 100)?;
-
-            // Update now-expired transactions that didn't get mined.
-            up.update_expired_notes(last_height)?;
-
-            Ok(())
-        })?;
+        // Update now-expired transactions that didn't get mined.
+        data.update_expired_notes(current_height)?;
 
         let spent_nf: Vec<Nullifier> = txs
             .iter()
@@ -374,6 +356,10 @@ where
                 .flat_map(|out| out.nf.map(|nf| (out.account, nf)))
         }));
 
+        witnesses.extend(new_witnesses);
+
+        last_height = current_height;
+
         Ok(())
     })?;
 
diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs
index 1cc5789a7..558c04ee4 100644
--- a/zcash_client_backend/src/data_api/wallet.rs
+++ b/zcash_client_backend/src/data_api/wallet.rs
@@ -15,7 +15,7 @@ use zcash_primitives::{
 
 use crate::{
     address::RecipientAddress,
-    data_api::{error::Error, WalletWrite},
+    data_api::{error::Error, ReceivedTransaction, SentTransaction, WalletWrite},
     decrypt_transaction,
     wallet::{AccountId, OvkPolicy},
 };
@@ -51,20 +51,12 @@ where
     if outputs.is_empty() {
         Ok(())
     } else {
-        // Update the database atomically, to ensure the result is internally consistent.
-        data.transactionally(|up| {
-            let tx_ref = up.put_tx_data(tx, None)?;
+        data.store_received_tx(&ReceivedTransaction {
+            tx,
+            outputs: &outputs,
+        })?;
 
-            for output in outputs {
-                if output.outgoing {
-                    up.put_sent_note(&output, tx_ref)?;
-                } else {
-                    up.put_received_note(&output, tx_ref)?;
-                }
-            }
-
-            Ok(())
-        })
+        Ok(())
     }
 }
 
@@ -243,26 +235,13 @@ where
         None => panic!("Output 0 should exist in the transaction"),
     };
 
-    // Update the database atomically, to ensure the result is internally consistent.
-    wallet_db.transactionally(|up| {
-        let created = time::OffsetDateTime::now_utc();
-        let tx_ref = up.put_tx_data(&tx, Some(created))?;
-
-        // Mark notes as spent.
-        //
-        // This locks the notes so they aren't selected again by a subsequent call to
-        // create_spend_to_address() before this transaction has been mined (at which point the notes
-        // get re-marked as spent).
-        //
-        // Assumes that create_spend_to_address() will never be called in parallel, which is a
-        // reasonable assumption for a light client such as a mobile phone.
-        for spend in &tx.shielded_spends {
-            up.mark_spent(tx_ref, &spend.nullifier)?;
-        }
-
-        up.insert_sent_note(tx_ref, output_index as usize, account, to, value, memo)?;
-
-        // Return the row number of the transaction, so the caller can fetch it for sending.
-        Ok(tx_ref)
+    wallet_db.store_sent_tx(&SentTransaction {
+        tx: &tx,
+        created: time::OffsetDateTime::now_utc(),
+        output_index: output_index as usize,
+        account,
+        recipient_address: to,
+        value,
+        memo,
     })
 }
diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs
index 91cb83dbe..0ffa13f4b 100644
--- a/zcash_client_sqlite/src/lib.rs
+++ b/zcash_client_sqlite/src/lib.rs
@@ -34,20 +34,19 @@ use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight},
     merkle_tree::{CommitmentTree, IncrementalWitness},
-    note_encryption::Memo,
     primitives::{Nullifier, PaymentAddress},
     sapling::Node,
-    transaction::{components::Amount, Transaction, TxId},
+    transaction::{components::Amount, TxId},
     zip32::ExtendedFullViewingKey,
 };
 
 use zcash_client_backend::{
-    address::RecipientAddress,
-    data_api::{BlockSource, ShieldedOutput, WalletRead, WalletWrite},
+    data_api::{
+        BlockSource, PrunedBlock, ReceivedTransaction, SentTransaction, WalletRead, WalletWrite,
+    },
     encoding::encode_payment_address,
     proto::compact_formats::CompactBlock,
-    wallet::{AccountId, SpendableNote, WalletTx},
-    DecryptedOutput,
+    wallet::{AccountId, SpendableNote},
 };
 
 use crate::error::SqliteClientError;
@@ -387,64 +386,110 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
         }
     }
 
-    fn insert_block(
+    fn insert_pruned_block(
         &mut self,
-        block_height: BlockHeight,
-        block_hash: BlockHash,
-        block_time: u32,
-        commitment_tree: &CommitmentTree<Node>,
-    ) -> Result<(), Self::Error> {
-        wallet::insert_block(self, block_height, block_hash, block_time, commitment_tree)
+        block: &PrunedBlock,
+        updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
+    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+        // database updates for each block are transactional
+        self.transactionally(|up| {
+            // Insert the block into the database.
+            wallet::insert_block(
+                up,
+                block.block_height,
+                block.block_hash,
+                block.block_time,
+                &block.commitment_tree,
+            )?;
+
+            let mut new_witnesses = vec![];
+            for tx in block.transactions {
+                let tx_row = wallet::put_tx_meta(up, &tx, block.block_height)?;
+
+                // Mark notes as spent and remove them from the scanning cache
+                for spend in &tx.shielded_spends {
+                    wallet::mark_spent(up, tx_row, &spend.nf)?;
+                }
+
+                for output in &tx.shielded_outputs {
+                    let received_note_id = wallet::put_received_note(up, output, tx_row)?;
+
+                    // Save witness for note.
+                    new_witnesses.push((received_note_id, output.witness.clone()));
+                }
+            }
+
+            // Insert current new_witnesses into the database.
+            for (received_note_id, witness) in updated_witnesses.iter().chain(new_witnesses.iter())
+            {
+                if let NoteId::ReceivedNoteId(rnid) = *received_note_id {
+                    wallet::insert_witness(up, rnid, witness, block.block_height)?;
+                }
+            }
+
+            Ok(new_witnesses)
+        })
+    }
+
+    fn store_received_tx(
+        &mut self,
+        received_tx: &ReceivedTransaction,
+    ) -> Result<Self::TxRef, Self::Error> {
+        self.transactionally(|up| {
+            let tx_ref = wallet::put_tx_data(up, received_tx.tx, None)?;
+
+            for output in received_tx.outputs {
+                if output.outgoing {
+                    wallet::put_sent_note(up, output, tx_ref)?;
+                } else {
+                    wallet::put_received_note(up, output, tx_ref)?;
+                }
+            }
+
+            Ok(tx_ref)
+        })
+    }
+
+    fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<Self::TxRef, Self::Error> {
+        // Update the database atomically, to ensure the result is internally consistent.
+        self.transactionally(|up| {
+            let tx_ref = wallet::put_tx_data(up, &sent_tx.tx, Some(sent_tx.created))?;
+
+            // Mark notes as spent.
+            //
+            // This locks the notes so they aren't selected again by a subsequent call to
+            // create_spend_to_address() before this transaction has been mined (at which point the notes
+            // get re-marked as spent).
+            //
+            // Assumes that create_spend_to_address() will never be called in parallel, which is a
+            // reasonable assumption for a light client such as a mobile phone.
+            for spend in &sent_tx.tx.shielded_spends {
+                wallet::mark_spent(up, tx_ref, &spend.nullifier)?;
+            }
+
+            wallet::insert_sent_note(
+                up,
+                tx_ref,
+                sent_tx.output_index,
+                sent_tx.account,
+                sent_tx.recipient_address,
+                sent_tx.value,
+                &sent_tx.memo,
+            )?;
+
+            // Return the row number of the transaction, so the caller can fetch it for sending.
+            Ok(tx_ref)
+        })
     }
 
     fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error> {
         wallet::rewind_to_height(self.wallet_db, block_height)
     }
 
-    fn put_tx_meta(
-        &mut self,
-        tx: &WalletTx,
-        height: BlockHeight,
-    ) -> Result<Self::TxRef, Self::Error> {
-        wallet::put_tx_meta(self, tx, height)
-    }
-
-    fn put_tx_data(
-        &mut self,
-        tx: &Transaction,
-        created_at: Option<time::OffsetDateTime>,
-    ) -> Result<Self::TxRef, Self::Error> {
-        wallet::put_tx_data(self, tx, created_at)
-    }
-
     fn mark_spent(&mut self, tx_ref: Self::TxRef, nf: &Nullifier) -> Result<(), Self::Error> {
         wallet::mark_spent(self, tx_ref, nf)
     }
 
-    // Assumptions:
-    // - A transaction will not contain more than 2^63 shielded outputs.
-    // - A note value will never exceed 2^63 zatoshis.
-    fn put_received_note<T: ShieldedOutput>(
-        &mut self,
-        output: &T,
-        tx_ref: Self::TxRef,
-    ) -> Result<Self::NoteRef, Self::Error> {
-        wallet::put_received_note(self, output, tx_ref)
-    }
-
-    fn insert_witness(
-        &mut self,
-        note_id: Self::NoteRef,
-        witness: &IncrementalWitness<Node>,
-        height: BlockHeight,
-    ) -> Result<(), Self::Error> {
-        if let NoteId::ReceivedNoteId(rnid) = note_id {
-            wallet::insert_witness(self, rnid, witness, height)
-        } else {
-            Err(SqliteClientError::InvalidNoteId)
-        }
-    }
-
     fn prune_witnesses(&mut self, below_height: BlockHeight) -> Result<(), Self::Error> {
         wallet::prune_witnesses(self, below_height)
     }
@@ -452,26 +497,6 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
     fn update_expired_notes(&mut self, height: BlockHeight) -> Result<(), Self::Error> {
         wallet::update_expired_notes(self, height)
     }
-
-    fn put_sent_note(
-        &mut self,
-        output: &DecryptedOutput,
-        tx_ref: Self::TxRef,
-    ) -> Result<(), Self::Error> {
-        wallet::put_sent_note(self, output, tx_ref)
-    }
-
-    fn insert_sent_note(
-        &mut self,
-        tx_ref: Self::TxRef,
-        output_index: usize,
-        account: AccountId,
-        to: &RecipientAddress,
-        value: Amount,
-        memo: Option<Memo>,
-    ) -> Result<(), Self::Error> {
-        wallet::insert_sent_note(self, tx_ref, output_index, account, to, value, memo)
-    }
 }
 
 pub struct BlockDB(Connection);
diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs
index 934171de0..459f88c3b 100644
--- a/zcash_client_sqlite/src/wallet.rs
+++ b/zcash_client_sqlite/src/wallet.rs
@@ -644,7 +644,7 @@ pub fn put_sent_note<'a, P: consensus::Parameters>(
             &RecipientAddress::Shielded(output.to.clone()),
             Amount::from_u64(output.note.value)
                 .map_err(|_| SqliteClientError::CorruptedData("Note value invalid.".to_string()))?,
-            Some(output.memo.clone()),
+            &Some(output.memo.clone()),
         )?
     }
 
@@ -658,7 +658,7 @@ pub fn insert_sent_note<'a, P: consensus::Parameters>(
     account: AccountId,
     to: &RecipientAddress,
     value: Amount,
-    memo: Option<Memo>,
+    memo: &Option<Memo>,
 ) -> Result<(), SqliteClientError> {
     let to_str = to.encode(&stmts.wallet_db.params);
     let ivalue: i64 = value.into();
@@ -668,7 +668,7 @@ pub fn insert_sent_note<'a, P: consensus::Parameters>(
         account.0,
         to_str,
         ivalue,
-        memo.map(|m| m.as_bytes().to_vec()),
+        memo.as_ref().map(|m| m.as_bytes().to_vec()),
     ])?;
 
     Ok(())

From 16289750e80efe4bb25ed8c5faf2defd546e0df4 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Tue, 9 Mar 2021 18:18:24 -0700
Subject: [PATCH 3/6] Remove WalletWrite::transactionally

---
 zcash_client_backend/src/data_api.rs       | 43 ----------------------
 zcash_client_backend/src/data_api/chain.rs |  6 ---
 zcash_client_sqlite/src/lib.rs             | 26 ++++++-------
 3 files changed, 11 insertions(+), 64 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index 4a7a59606..c96aa413f 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/zcash_client_backend/src/data_api.rs
@@ -205,15 +205,6 @@ pub struct SentTransaction<'a> {
 /// This trait encapsulates the write capabilities required to update stored
 /// wallet data.
 pub trait WalletWrite: WalletRead {
-    /// Perform one or more write operations of this trait transactionally.
-    /// Implementations of this method must ensure that all mutations to the
-    /// state of the data store made by the provided closure must be performed
-    /// atomically and modifications to state must be automatically rolled back
-    /// if the provided closure returns an error.
-    fn transactionally<F, A>(&mut self, f: F) -> Result<A, Self::Error>
-    where
-        F: FnOnce(&mut Self) -> Result<A, Self::Error>;
-
     fn insert_pruned_block(
         &mut self,
         block: &PrunedBlock,
@@ -241,21 +232,6 @@ pub trait WalletWrite: WalletRead {
     ///
     /// There may be restrictions on how far it is possible to rewind.
     fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error>;
-
-    /// Mark the specified transaction as spent and record the nullifier.
-    fn mark_spent(&mut self, tx_ref: Self::TxRef, nf: &Nullifier) -> Result<(), Self::Error>;
-
-    /// Remove all incremental witness data before the specified block height.
-    //  TODO: this is a backend-specific optimization that probably shouldn't be part of
-    //  the public API
-    fn prune_witnesses(&mut self, from_height: BlockHeight) -> Result<(), Self::Error>;
-
-    /// Remove the spent marker from any received notes that had been spent in a
-    /// transaction constructed by the wallet, but which transaction had not been mined
-    /// by the specified block height.
-    //  TODO: this is a backend-specific optimization that probably shouldn't be part of
-    //  the public API
-    fn update_expired_notes(&mut self, from_height: BlockHeight) -> Result<(), Self::Error>;
 }
 
 /// This trait provides sequential access to raw blockchain data via a callback-oriented
@@ -471,13 +447,6 @@ pub mod testing {
     }
 
     impl WalletWrite for MockWalletDB {
-        fn transactionally<F, A>(&mut self, f: F) -> Result<A, Self::Error>
-        where
-            F: FnOnce(&mut Self) -> Result<A, Self::Error>,
-        {
-            f(self)
-        }
-
         fn insert_pruned_block(
             &mut self,
             _block: &PrunedBlock,
@@ -503,17 +472,5 @@ pub mod testing {
         fn rewind_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> {
             Ok(())
         }
-
-        fn mark_spent(&mut self, _tx_ref: Self::TxRef, _nf: &Nullifier) -> Result<(), Self::Error> {
-            Ok(())
-        }
-
-        fn prune_witnesses(&mut self, _from_height: BlockHeight) -> Result<(), Self::Error> {
-            Ok(())
-        }
-
-        fn update_expired_notes(&mut self, _from_height: BlockHeight) -> Result<(), Self::Error> {
-            Ok(())
-        }
     }
 }
diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs
index e6b554a5d..c8986487a 100644
--- a/zcash_client_backend/src/data_api/chain.rs
+++ b/zcash_client_backend/src/data_api/chain.rs
@@ -339,12 +339,6 @@ where
             &witnesses,
         )?;
 
-        // Prune the stored witnesses (we only expect rollbacks of at most 100 blocks).
-        data.prune_witnesses(current_height - 100)?;
-
-        // Update now-expired transactions that didn't get mined.
-        data.update_expired_notes(current_height)?;
-
         let spent_nf: Vec<Nullifier> = txs
             .iter()
             .flat_map(|tx| tx.shielded_spends.iter().map(|spend| spend.nf))
diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs
index 0ffa13f4b..a33632193 100644
--- a/zcash_client_sqlite/src/lib.rs
+++ b/zcash_client_sqlite/src/lib.rs
@@ -359,10 +359,10 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
     }
 }
 
-impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
-    fn transactionally<F, A>(&mut self, f: F) -> Result<A, Self::Error>
+impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
+    fn transactionally<F, A>(&mut self, f: F) -> Result<A, SqliteClientError>
     where
-        F: FnOnce(&mut Self) -> Result<A, Self::Error>,
+        F: FnOnce(&mut Self) -> Result<A, SqliteClientError>,
     {
         self.wallet_db.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?;
         match f(self) {
@@ -385,7 +385,9 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
             }
         }
     }
+}
 
+impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
     fn insert_pruned_block(
         &mut self,
         block: &PrunedBlock,
@@ -427,6 +429,12 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
                 }
             }
 
+            // Prune the stored witnesses (we only expect rollbacks of at most 100 blocks).
+            wallet::prune_witnesses(up, block.block_height - 100)?;
+
+            // Update now-expired transactions that didn't get mined.
+            wallet::update_expired_notes(up, block.block_height)?;
+
             Ok(new_witnesses)
         })
     }
@@ -485,18 +493,6 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
     fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error> {
         wallet::rewind_to_height(self.wallet_db, block_height)
     }
-
-    fn mark_spent(&mut self, tx_ref: Self::TxRef, nf: &Nullifier) -> Result<(), Self::Error> {
-        wallet::mark_spent(self, tx_ref, nf)
-    }
-
-    fn prune_witnesses(&mut self, below_height: BlockHeight) -> Result<(), Self::Error> {
-        wallet::prune_witnesses(self, below_height)
-    }
-
-    fn update_expired_notes(&mut self, height: BlockHeight) -> Result<(), Self::Error> {
-        wallet::update_expired_notes(self, height)
-    }
 }
 
 pub struct BlockDB(Connection);

From 0e022f22830a7bcdced8c0469a165e9624e7f89c Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Tue, 9 Mar 2021 20:55:44 -0700
Subject: [PATCH 4/6] Allow block scanning using either IVKs or FVKs.

---
 zcash_client_backend/src/data_api.rs       | 72 ++--------------
 zcash_client_backend/src/data_api/chain.rs | 11 ++-
 zcash_client_backend/src/wallet.rs         |  8 +-
 zcash_client_backend/src/welding_rig.rs    | 96 ++++++++++++++++------
 zcash_client_sqlite/src/lib.rs             |  1 +
 zcash_client_sqlite/src/wallet.rs          | 74 +++++++++++++++--
 6 files changed, 157 insertions(+), 105 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index c96aa413f..1f6125412 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/zcash_client_backend/src/data_api.rs
@@ -9,7 +9,7 @@ use zcash_primitives::{
     consensus::BlockHeight,
     merkle_tree::{CommitmentTree, IncrementalWitness},
     note_encryption::Memo,
-    primitives::{Note, Nullifier, PaymentAddress},
+    primitives::{Nullifier, PaymentAddress},
     sapling::Node,
     transaction::{components::Amount, Transaction, TxId},
     zip32::ExtendedFullViewingKey,
@@ -20,7 +20,7 @@ use crate::{
     data_api::wallet::ANCHOR_OFFSET,
     decrypt::DecryptedOutput,
     proto::compact_formats::CompactBlock,
-    wallet::{AccountId, SpendableNote, WalletShieldedOutput, WalletTx},
+    wallet::{AccountId, SpendableNote, WalletTx},
 };
 
 pub mod chain;
@@ -184,7 +184,7 @@ pub struct PrunedBlock<'a> {
     pub block_hash: BlockHash,
     pub block_time: u32,
     pub commitment_tree: &'a CommitmentTree<Node>,
-    pub transactions: &'a Vec<WalletTx>,
+    pub transactions: &'a Vec<WalletTx<Nullifier>>,
 }
 
 pub struct ReceivedTransaction<'a> {
@@ -205,6 +205,7 @@ pub struct SentTransaction<'a> {
 /// This trait encapsulates the write capabilities required to update stored
 /// wallet data.
 pub trait WalletWrite: WalletRead {
+    #[allow(clippy::type_complexity)]
     fn insert_pruned_block(
         &mut self,
         block: &PrunedBlock,
@@ -251,70 +252,6 @@ pub trait BlockSource {
         F: FnMut(CompactBlock) -> Result<(), Self::Error>;
 }
 
-/// This trait provides a generalization over shielded output representations
-/// that allows a wallet to avoid coupling to a specific one.
-// TODO: it'd probably be better not to unify the definitions of
-// `WalletShieldedOutput` and `DecryptedOutput` via a compositional
-// approach, if possible.
-pub trait ShieldedOutput {
-    fn index(&self) -> usize;
-    fn account(&self) -> AccountId;
-    fn to(&self) -> &PaymentAddress;
-    fn note(&self) -> &Note;
-    fn memo(&self) -> Option<&Memo>;
-    fn is_change(&self) -> Option<bool>;
-    fn nullifier(&self) -> Option<Nullifier>;
-}
-
-impl ShieldedOutput for WalletShieldedOutput {
-    fn index(&self) -> usize {
-        self.index
-    }
-    fn account(&self) -> AccountId {
-        self.account
-    }
-    fn to(&self) -> &PaymentAddress {
-        &self.to
-    }
-    fn note(&self) -> &Note {
-        &self.note
-    }
-    fn memo(&self) -> Option<&Memo> {
-        None
-    }
-    fn is_change(&self) -> Option<bool> {
-        Some(self.is_change)
-    }
-
-    fn nullifier(&self) -> Option<Nullifier> {
-        self.nf.clone()
-    }
-}
-
-impl ShieldedOutput for DecryptedOutput {
-    fn index(&self) -> usize {
-        self.index
-    }
-    fn account(&self) -> AccountId {
-        self.account
-    }
-    fn to(&self) -> &PaymentAddress {
-        &self.to
-    }
-    fn note(&self) -> &Note {
-        &self.note
-    }
-    fn memo(&self) -> Option<&Memo> {
-        Some(&self.memo)
-    }
-    fn is_change(&self) -> Option<bool> {
-        None
-    }
-    fn nullifier(&self) -> Option<Nullifier> {
-        None
-    }
-}
-
 #[cfg(feature = "test-dependencies")]
 pub mod testing {
     use std::collections::HashMap;
@@ -447,6 +384,7 @@ pub mod testing {
     }
 
     impl WalletWrite for MockWalletDB {
+        #[allow(clippy::type_complexity)]
         fn insert_pruned_block(
             &mut self,
             _block: &PrunedBlock,
diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs
index c8986487a..c033fdbd7 100644
--- a/zcash_client_backend/src/data_api/chain.rs
+++ b/zcash_client_backend/src/data_api/chain.rs
@@ -291,7 +291,7 @@ where
         let block_hash = BlockHash::from_slice(&block.hash);
         let block_time = block.time;
 
-        let txs: Vec<WalletTx> = {
+        let txs: Vec<WalletTx<Nullifier>> = {
             let mut witness_refs: Vec<_> = witnesses.iter_mut().map(|w| &mut w.1).collect();
 
             scan_block(
@@ -344,11 +344,10 @@ where
             .flat_map(|tx| tx.shielded_spends.iter().map(|spend| spend.nf))
             .collect();
         nullifiers.retain(|(_, nf)| !spent_nf.contains(nf));
-        nullifiers.extend(txs.iter().flat_map(|tx| {
-            tx.shielded_outputs
-                .iter()
-                .flat_map(|out| out.nf.map(|nf| (out.account, nf)))
-        }));
+        nullifiers.extend(
+            txs.iter()
+                .flat_map(|tx| tx.shielded_outputs.iter().map(|out| (out.account, out.nf))),
+        );
 
         witnesses.extend(new_witnesses);
 
diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs
index 399fe85bd..d2170d93b 100644
--- a/zcash_client_backend/src/wallet.rs
+++ b/zcash_client_backend/src/wallet.rs
@@ -30,13 +30,13 @@ impl ConditionallySelectable for AccountId {
 /// A subset of a [`Transaction`] relevant to wallets and light clients.
 ///
 /// [`Transaction`]: zcash_primitives::transaction::Transaction
-pub struct WalletTx {
+pub struct WalletTx<N> {
     pub txid: TxId,
     pub index: usize,
     pub num_spends: usize,
     pub num_outputs: usize,
     pub shielded_spends: Vec<WalletShieldedSpend>,
-    pub shielded_outputs: Vec<WalletShieldedOutput>,
+    pub shielded_outputs: Vec<WalletShieldedOutput<N>>,
 }
 
 /// A subset of a [`SpendDescription`] relevant to wallets and light clients.
@@ -51,7 +51,7 @@ pub struct WalletShieldedSpend {
 /// A subset of an [`OutputDescription`] relevant to wallets and light clients.
 ///
 /// [`OutputDescription`]: zcash_primitives::transaction::components::OutputDescription
-pub struct WalletShieldedOutput {
+pub struct WalletShieldedOutput<N> {
     pub index: usize,
     pub cmu: bls12_381::Scalar,
     pub epk: jubjub::ExtendedPoint,
@@ -60,7 +60,7 @@ pub struct WalletShieldedOutput {
     pub to: PaymentAddress,
     pub is_change: bool,
     pub witness: IncrementalWitness<Node>,
-    pub nf: Option<Nullifier>,
+    pub nf: N,
 }
 
 pub struct SpendableNote {
diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs
index 7bb103bdf..fdc39142a 100644
--- a/zcash_client_backend/src/welding_rig.rs
+++ b/zcash_client_backend/src/welding_rig.rs
@@ -7,7 +7,7 @@ use zcash_primitives::{
     consensus::{self, BlockHeight},
     merkle_tree::{CommitmentTree, IncrementalWitness},
     note_encryption::try_sapling_compact_note_decryption,
-    primitives::{Nullifier, ViewingKey},
+    primitives::{Note, Nullifier, PaymentAddress, SaplingIvk, ViewingKey},
     sapling::Node,
     transaction::TxId,
 };
@@ -25,17 +25,17 @@ use crate::wallet::{AccountId, WalletShieldedOutput, WalletShieldedSpend, Wallet
 ///
 /// [`ExtendedFullViewingKey`]: zcash_primitives::zip32::ExtendedFullViewingKey
 #[allow(clippy::too_many_arguments)]
-fn scan_output<P: consensus::Parameters>(
+fn scan_output<P: consensus::Parameters, K: ScanningKey>(
     params: &P,
     height: BlockHeight,
     (index, output): (usize, CompactOutput),
-    vks: &[(AccountId, &ViewingKey)],
+    vks: &[(AccountId, &K)],
     spent_from_accounts: &HashSet<AccountId>,
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
     block_witnesses: &mut [&mut IncrementalWitness<Node>],
     new_witnesses: &mut [&mut IncrementalWitness<Node>],
-) -> Option<WalletShieldedOutput> {
+) -> Option<WalletShieldedOutput<K::Nf>> {
     let cmu = output.cmu().ok()?;
     let epk = output.epk().ok()?;
     let ct = output.ciphertext;
@@ -54,12 +54,10 @@ fn scan_output<P: consensus::Parameters>(
     tree.append(node).unwrap();
 
     for (account, vk) in vks.iter() {
-        let ivk = vk.ivk();
-        let (note, to) =
-            match try_sapling_compact_note_decryption(params, height, &ivk, &epk, &cmu, &ct) {
-                Some(ret) => ret,
-                None => continue,
-            };
+        let (note, to) = match vk.try_decryption(params, height, &epk, &cmu, &ct) {
+            Some(ret) => ret,
+            None => continue,
+        };
 
         // A note is marked as "change" if the account that received it
         // also spent notes in the same transaction. This will catch,
@@ -70,7 +68,7 @@ fn scan_output<P: consensus::Parameters>(
         let is_change = spent_from_accounts.contains(&account);
 
         let witness = IncrementalWitness::from_tree(tree);
-        let nf = note.nf(&vk, witness.position() as u64);
+        let nf = vk.nf(&note, &witness);
 
         return Some(WalletShieldedOutput {
             index,
@@ -81,30 +79,81 @@ fn scan_output<P: consensus::Parameters>(
             to,
             is_change,
             witness,
-            nf: Some(nf),
+            nf,
         });
     }
+
     None
 }
 
-/// Scans a [`CompactBlock`] with a set of [`ViewingKey`]s.
+pub trait ScanningKey {
+    type Nf;
+
+    fn try_decryption<P: consensus::Parameters>(
+        &self,
+        params: &P,
+        height: BlockHeight,
+        epk: &jubjub::ExtendedPoint,
+        cmu: &bls12_381::Scalar,
+        ct: &[u8],
+    ) -> Option<(Note, PaymentAddress)>;
+
+    fn nf(&self, note: &Note, witness: &IncrementalWitness<Node>) -> Self::Nf;
+}
+
+impl ScanningKey for ViewingKey {
+    type Nf = Nullifier;
+
+    fn try_decryption<P: consensus::Parameters>(
+        &self,
+        params: &P,
+        height: BlockHeight,
+        epk: &jubjub::ExtendedPoint,
+        cmu: &bls12_381::Scalar,
+        ct: &[u8],
+    ) -> Option<(Note, PaymentAddress)> {
+        try_sapling_compact_note_decryption(params, height, &self.ivk(), &epk, &cmu, &ct)
+    }
+
+    fn nf(&self, note: &Note, witness: &IncrementalWitness<Node>) -> Self::Nf {
+        note.nf(self, witness.position() as u64)
+    }
+}
+
+impl ScanningKey for SaplingIvk {
+    type Nf = ();
+
+    fn try_decryption<P: consensus::Parameters>(
+        &self,
+        params: &P,
+        height: BlockHeight,
+        epk: &jubjub::ExtendedPoint,
+        cmu: &bls12_381::Scalar,
+        ct: &[u8],
+    ) -> Option<(Note, PaymentAddress)> {
+        try_sapling_compact_note_decryption(params, height, self, &epk, &cmu, &ct)
+    }
+
+    fn nf(&self, _note: &Note, _witness: &IncrementalWitness<Node>) {}
+}
+
+/// Scans a [`CompactBlock`] with a set of [`ScanningKeys`]s.
 ///
 /// Returns a vector of [`WalletTx`]s belonging to any of the given
-/// [`ViewingKey`]s.
+/// [`ScanningKey`]s. If scanning with a full viewing key, the nullifiers
+/// of the resulting [`WalletShieldedOutput`]s will also be computed.
 ///
 /// The given [`CommitmentTree`] and existing [`IncrementalWitness`]es are
 /// incremented appropriately.
-///
-/// [`ExtendedFullViewingKey`]: zcash_primitives::zip32::ExtendedFullViewingKey
-pub fn scan_block<P: consensus::Parameters>(
+pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
     params: &P,
     block: CompactBlock,
-    vks: &[(AccountId, &ViewingKey)],
+    vks: &[(AccountId, &K)],
     nullifiers: &[(AccountId, Nullifier)],
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
-) -> Vec<WalletTx> {
-    let mut wtxs: Vec<WalletTx> = vec![];
+) -> Vec<WalletTx<K::Nf>> {
+    let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
     let block_height = block.height();
 
     for tx in block.vtx.into_iter() {
@@ -145,7 +194,7 @@ pub fn scan_block<P: consensus::Parameters>(
             shielded_spends.iter().map(|spend| spend.account).collect();
 
         // Check for incoming notes while incrementing tree and witnesses
-        let mut shielded_outputs: Vec<WalletShieldedOutput> = vec![];
+        let mut shielded_outputs: Vec<WalletShieldedOutput<K::Nf>> = vec![];
         {
             // Grab mutable references to new witnesses from previous transactions
             // in this block so that we can update them. Scoped so we don't hold
@@ -211,7 +260,7 @@ mod tests {
         constants::SPENDING_KEY_GENERATOR,
         merkle_tree::CommitmentTree,
         note_encryption::{Memo, SaplingNoteEncryption},
-        primitives::{Note, Nullifier},
+        primitives::{Note, Nullifier, SaplingIvk},
         transaction::components::Amount,
         util::generate_random_rseed,
         zip32::{ExtendedFullViewingKey, ExtendedSpendingKey},
@@ -408,12 +457,13 @@ mod tests {
 
         let cb = fake_compact_block(1u32.into(), nf, extfvk, Amount::from_u64(5).unwrap(), false);
         assert_eq!(cb.vtx.len(), 2);
+        let vks: Vec<(AccountId, &SaplingIvk)> = vec![];
 
         let mut tree = CommitmentTree::empty();
         let txs = scan_block(
             &Network::TestNetwork,
             cb,
-            &[],
+            &vks[..],
             &[(account, nf)],
             &mut tree,
             &mut [],
diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs
index a33632193..5d429a601 100644
--- a/zcash_client_sqlite/src/lib.rs
+++ b/zcash_client_sqlite/src/lib.rs
@@ -388,6 +388,7 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
 }
 
 impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
+    #[allow(clippy::type_complexity)]
     fn insert_pruned_block(
         &mut self,
         block: &PrunedBlock,
diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs
index 459f88c3b..bf86ddef0 100644
--- a/zcash_client_sqlite/src/wallet.rs
+++ b/zcash_client_sqlite/src/wallet.rs
@@ -9,7 +9,7 @@ use zcash_primitives::{
     consensus::{self, BlockHeight, NetworkUpgrade},
     merkle_tree::{CommitmentTree, IncrementalWitness},
     note_encryption::Memo,
-    primitives::{Nullifier, PaymentAddress},
+    primitives::{Note, Nullifier, PaymentAddress},
     sapling::Node,
     transaction::{components::Amount, Transaction, TxId},
     zip32::ExtendedFullViewingKey,
@@ -17,12 +17,12 @@ use zcash_primitives::{
 
 use zcash_client_backend::{
     address::RecipientAddress,
-    data_api::{error::Error, ShieldedOutput},
+    data_api::error::Error,
     encoding::{
         decode_extended_full_viewing_key, decode_payment_address, encode_extended_full_viewing_key,
         encode_payment_address,
     },
-    wallet::{AccountId, WalletTx},
+    wallet::{AccountId, WalletShieldedOutput, WalletTx},
     DecryptedOutput,
 };
 
@@ -31,6 +31,70 @@ use crate::{error::SqliteClientError, DataConnStmtCache, NoteId, WalletDB};
 pub mod init;
 pub mod transact;
 
+/// This trait provides a generalization over shielded output representations
+/// that allows a wallet to avoid coupling to a specific one.
+// TODO: it'd probably be better not to unify the definitions of
+// `WalletShieldedOutput` and `DecryptedOutput` via a compositional
+// approach, if possible.
+pub trait ShieldedOutput {
+    fn index(&self) -> usize;
+    fn account(&self) -> AccountId;
+    fn to(&self) -> &PaymentAddress;
+    fn note(&self) -> &Note;
+    fn memo(&self) -> Option<&Memo>;
+    fn is_change(&self) -> Option<bool>;
+    fn nullifier(&self) -> Option<Nullifier>;
+}
+
+impl ShieldedOutput for WalletShieldedOutput<Nullifier> {
+    fn index(&self) -> usize {
+        self.index
+    }
+    fn account(&self) -> AccountId {
+        self.account
+    }
+    fn to(&self) -> &PaymentAddress {
+        &self.to
+    }
+    fn note(&self) -> &Note {
+        &self.note
+    }
+    fn memo(&self) -> Option<&Memo> {
+        None
+    }
+    fn is_change(&self) -> Option<bool> {
+        Some(self.is_change)
+    }
+
+    fn nullifier(&self) -> Option<Nullifier> {
+        Some(self.nf)
+    }
+}
+
+impl ShieldedOutput for DecryptedOutput {
+    fn index(&self) -> usize {
+        self.index
+    }
+    fn account(&self) -> AccountId {
+        self.account
+    }
+    fn to(&self) -> &PaymentAddress {
+        &self.to
+    }
+    fn note(&self) -> &Note {
+        &self.note
+    }
+    fn memo(&self) -> Option<&Memo> {
+        Some(&self.memo)
+    }
+    fn is_change(&self) -> Option<bool> {
+        None
+    }
+    fn nullifier(&self) -> Option<Nullifier> {
+        None
+    }
+}
+
 /// Returns the address for the account.
 ///
 /// # Examples
@@ -458,9 +522,9 @@ pub fn insert_block<'a, P>(
     Ok(())
 }
 
-pub fn put_tx_meta<'a, P>(
+pub fn put_tx_meta<'a, P, N>(
     stmts: &mut DataConnStmtCache<'a, P>,
-    tx: &WalletTx,
+    tx: &WalletTx<N>,
     height: BlockHeight,
 ) -> Result<i64, SqliteClientError> {
     let txid = tx.txid.0.to_vec();

From 5a9b29a75ac8b41fa2631c6650a5db5a9f9ff021 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Thu, 11 Mar 2021 13:01:22 -0700
Subject: [PATCH 5/6] Address comments from code review.

---
 zcash_client_backend/src/data_api.rs       |  4 ++--
 zcash_client_backend/src/data_api/chain.rs | 12 ++++------
 zcash_client_backend/src/welding_rig.rs    | 27 +++++++++++-----------
 zcash_client_sqlite/src/lib.rs             |  4 +++-
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index 1f6125412..f783e086a 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/zcash_client_backend/src/data_api.rs
@@ -206,7 +206,7 @@ pub struct SentTransaction<'a> {
 /// wallet data.
 pub trait WalletWrite: WalletRead {
     #[allow(clippy::type_complexity)]
-    fn insert_pruned_block(
+    fn advance_by_block(
         &mut self,
         block: &PrunedBlock,
         updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
@@ -385,7 +385,7 @@ pub mod testing {
 
     impl WalletWrite for MockWalletDB {
         #[allow(clippy::type_complexity)]
-        fn insert_pruned_block(
+        fn advance_by_block(
             &mut self,
             _block: &PrunedBlock,
             _updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs
index c033fdbd7..56fb1ba92 100644
--- a/zcash_client_backend/src/data_api/chain.rs
+++ b/zcash_client_backend/src/data_api/chain.rs
@@ -96,6 +96,7 @@ use zcash_primitives::{
     consensus::{self, BlockHeight, NetworkUpgrade},
     merkle_tree::CommitmentTree,
     primitives::Nullifier,
+    zip32::ExtendedFullViewingKey,
 };
 
 use crate::{
@@ -104,7 +105,7 @@ use crate::{
         BlockSource, PrunedBlock, WalletWrite,
     },
     proto::compact_formats::CompactBlock,
-    wallet::WalletTx,
+    wallet::{AccountId, WalletTx},
     welding_rig::scan_block,
 };
 
@@ -262,10 +263,7 @@ where
 
     // Fetch the ExtendedFullViewingKeys we are tracking
     let extfvks = data.get_extended_full_viewing_keys()?;
-    let vks: Vec<_> = extfvks
-        .iter()
-        .map(|(a, extfvk)| (*a, &extfvk.fvk.vk))
-        .collect();
+    let extfvks: Vec<(&AccountId, &ExtendedFullViewingKey)> = extfvks.iter().collect();
 
     // Get the most recent CommitmentTree
     let mut tree = data
@@ -297,7 +295,7 @@ where
             scan_block(
                 params,
                 block,
-                &vks,
+                &extfvks,
                 &nullifiers,
                 &mut tree,
                 &mut witness_refs[..],
@@ -328,7 +326,7 @@ where
             }
         }
 
-        let new_witnesses = data.insert_pruned_block(
+        let new_witnesses = data.advance_by_block(
             &(PrunedBlock {
                 block_height: current_height,
                 block_hash,
diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs
index fdc39142a..b0b732b1e 100644
--- a/zcash_client_backend/src/welding_rig.rs
+++ b/zcash_client_backend/src/welding_rig.rs
@@ -7,9 +7,10 @@ use zcash_primitives::{
     consensus::{self, BlockHeight},
     merkle_tree::{CommitmentTree, IncrementalWitness},
     note_encryption::try_sapling_compact_note_decryption,
-    primitives::{Note, Nullifier, PaymentAddress, SaplingIvk, ViewingKey},
+    primitives::{Note, Nullifier, PaymentAddress, SaplingIvk},
     sapling::Node,
     transaction::TxId,
+    zip32::ExtendedFullViewingKey,
 };
 
 use crate::proto::compact_formats::{CompactBlock, CompactOutput};
@@ -22,14 +23,12 @@ use crate::wallet::{AccountId, WalletShieldedOutput, WalletShieldedSpend, Wallet
 ///
 /// The given [`CommitmentTree`] and existing [`IncrementalWitness`]es are incremented
 /// with this output's commitment.
-///
-/// [`ExtendedFullViewingKey`]: zcash_primitives::zip32::ExtendedFullViewingKey
 #[allow(clippy::too_many_arguments)]
 fn scan_output<P: consensus::Parameters, K: ScanningKey>(
     params: &P,
     height: BlockHeight,
     (index, output): (usize, CompactOutput),
-    vks: &[(AccountId, &K)],
+    vks: &[(&AccountId, &K)],
     spent_from_accounts: &HashSet<AccountId>,
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
@@ -74,7 +73,7 @@ fn scan_output<P: consensus::Parameters, K: ScanningKey>(
             index,
             cmu,
             epk,
-            account: *account,
+            account: **account,
             note,
             to,
             is_change,
@@ -86,6 +85,8 @@ fn scan_output<P: consensus::Parameters, K: ScanningKey>(
     None
 }
 
+/// A key that can be used to perform trial decryption and nullifier
+/// computation for a Sapling [`ShieldedOutput`]
 pub trait ScanningKey {
     type Nf;
 
@@ -101,7 +102,7 @@ pub trait ScanningKey {
     fn nf(&self, note: &Note, witness: &IncrementalWitness<Node>) -> Self::Nf;
 }
 
-impl ScanningKey for ViewingKey {
+impl ScanningKey for ExtendedFullViewingKey {
     type Nf = Nullifier;
 
     fn try_decryption<P: consensus::Parameters>(
@@ -112,11 +113,11 @@ impl ScanningKey for ViewingKey {
         cmu: &bls12_381::Scalar,
         ct: &[u8],
     ) -> Option<(Note, PaymentAddress)> {
-        try_sapling_compact_note_decryption(params, height, &self.ivk(), &epk, &cmu, &ct)
+        try_sapling_compact_note_decryption(params, height, &self.fvk.vk.ivk(), &epk, &cmu, &ct)
     }
 
     fn nf(&self, note: &Note, witness: &IncrementalWitness<Node>) -> Self::Nf {
-        note.nf(self, witness.position() as u64)
+        note.nf(&self.fvk.vk, witness.position() as u64)
     }
 }
 
@@ -137,7 +138,7 @@ impl ScanningKey for SaplingIvk {
     fn nf(&self, _note: &Note, _witness: &IncrementalWitness<Node>) {}
 }
 
-/// Scans a [`CompactBlock`] with a set of [`ScanningKeys`]s.
+/// Scans a [`CompactBlock`] with a set of [`ScanningKey`]s.
 ///
 /// Returns a vector of [`WalletTx`]s belonging to any of the given
 /// [`ScanningKey`]s. If scanning with a full viewing key, the nullifiers
@@ -148,7 +149,7 @@ impl ScanningKey for SaplingIvk {
 pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
     params: &P,
     block: CompactBlock,
-    vks: &[(AccountId, &K)],
+    vks: &[(&AccountId, &K)],
     nullifiers: &[(AccountId, Nullifier)],
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
@@ -388,7 +389,7 @@ mod tests {
         let txs = scan_block(
             &Network::TestNetwork,
             cb,
-            &[(AccountId(0), &extfvk.fvk.vk)],
+            &[(&AccountId(0), &extfvk)],
             &[],
             &mut tree,
             &mut [],
@@ -427,7 +428,7 @@ mod tests {
         let txs = scan_block(
             &Network::TestNetwork,
             cb,
-            &[(AccountId(0), &extfvk.fvk.vk)],
+            &[(&AccountId(0), &extfvk)],
             &[],
             &mut tree,
             &mut [],
@@ -457,7 +458,7 @@ mod tests {
 
         let cb = fake_compact_block(1u32.into(), nf, extfvk, Amount::from_u64(5).unwrap(), false);
         assert_eq!(cb.vtx.len(), 2);
-        let vks: Vec<(AccountId, &SaplingIvk)> = vec![];
+        let vks: Vec<(&AccountId, &SaplingIvk)> = vec![];
 
         let mut tree = CommitmentTree::empty();
         let txs = scan_block(
diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs
index 5d429a601..b085babae 100644
--- a/zcash_client_sqlite/src/lib.rs
+++ b/zcash_client_sqlite/src/lib.rs
@@ -389,7 +389,7 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
 
 impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
     #[allow(clippy::type_complexity)]
-    fn insert_pruned_block(
+    fn advance_by_block(
         &mut self,
         block: &PrunedBlock,
         updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
@@ -427,6 +427,8 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
             {
                 if let NoteId::ReceivedNoteId(rnid) = *received_note_id {
                     wallet::insert_witness(up, rnid, witness, block.block_height)?;
+                } else {
+                    return Err(SqliteClientError::InvalidNoteId);
                 }
             }
 

From 2b4951759c25c23fef9aec6e87e287ae27e24339 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Mon, 15 Mar 2021 17:59:23 -0600
Subject: [PATCH 6/6] Fix welding rig intra-doc links.

---
 zcash_client_backend/src/welding_rig.rs | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs
index b0b732b1e..3eb19c85b 100644
--- a/zcash_client_backend/src/welding_rig.rs
+++ b/zcash_client_backend/src/welding_rig.rs
@@ -16,10 +16,10 @@ use zcash_primitives::{
 use crate::proto::compact_formats::{CompactBlock, CompactOutput};
 use crate::wallet::{AccountId, WalletShieldedOutput, WalletShieldedSpend, WalletTx};
 
-/// Scans a [`CompactOutput`] with a set of [`ExtendedFullViewingKey`]s.
+/// Scans a [`CompactOutput`] with a set of [`ScanningKey`]s.
 ///
 /// Returns a [`WalletShieldedOutput`] and corresponding [`IncrementalWitness`] if this
-/// output belongs to any of the given [`ExtendedFullViewingKey`]s.
+/// output belongs to any of the given [`ScanningKey`]s.
 ///
 /// The given [`CommitmentTree`] and existing [`IncrementalWitness`]es are incremented
 /// with this output's commitment.
@@ -86,7 +86,9 @@ fn scan_output<P: consensus::Parameters, K: ScanningKey>(
 }
 
 /// A key that can be used to perform trial decryption and nullifier
-/// computation for a Sapling [`ShieldedOutput`]
+/// computation for a Sapling [`CompactOutput`]
+///
+/// [`CompactOutput`]: crate::proto::compact_formats::CompactOutput
 pub trait ScanningKey {
     type Nf;
 
@@ -146,6 +148,20 @@ impl ScanningKey for SaplingIvk {
 ///
 /// The given [`CommitmentTree`] and existing [`IncrementalWitness`]es are
 /// incremented appropriately.
+///
+/// The implementation of [`ScanningKey`] may either support or omit the computation of
+/// the nullifiers for received notes; the implementation for [`ExtendedFullViewingKey`]
+/// will derive the nullifiers for received notes and return them as part of the resulting
+/// [`WalletShieldedOutput`]s, whereas since the implementation for [`SaplingIvk`] cannot 
+/// do so and it will return the unit value in those outputs instead.
+///
+/// [`ExtendedFullViewingKey`]: zcash_primitives::zip32::ExtendedFullViewingKey
+/// [`SaplingIvk`]: zcash_primitives::SaplingIvk
+/// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock
+/// [`ScanningKey`]: self::ScanningKey
+/// [`CommitmentTree`]: zcash_primitives::merkle_tree::CommitmentTree
+/// [`IncrementalWitness`]: zcash_primitives::merkle_tree::IncrementalWitness
+/// [`WalletShieldedOutput`]: crate::wallet::WalletShieldedOutput
 pub fn scan_block<P: consensus::Parameters, K: ScanningKey>(
     params: &P,
     block: CompactBlock,