Merge pull request #867 from nuttycom/feature/pre_dag_sync-require_scan_range

zcash_client_backend: Make scan range bounds required in `data_api::chain::scan_cached_blocks`

zcash_client_backend/CHANGELOG.md

@@ -11,14 +11,15 @@ and this library adheres to Rust's notion of
 - `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}`
 - `impl Debug` for `zcash_client_backend::{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}`
 - `zcash_client_backend::data_api`:
+  - `BlockMetadata`
+  - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers`
+  - `ScannedBlock`
+  - `ShieldedProtocol`
   - `WalletRead::{block_metadata, block_fully_scanned, suggest_scan_ranges}`
   - `WalletWrite::put_block`
   - `WalletCommitmentTrees`
-  - `testing::MockWalletDb::new`
-  - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers`
-  - `BlockMetadata`
-  - `ScannedBlock`
   - `chain::CommitmentTreeRoot`
+  - `testing::MockWalletDb::new`
   - `wallet::input_sellection::Proposal::{min_target_height, min_anchor_height}`:
 - `zcash_client_backend::wallet::WalletSaplingOutput::note_commitment_tree_position`
 - `zcash_client_backend::scanning::ScanError`
@@ -35,7 +36,8 @@ and this library adheres to Rust's notion of
     and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`.
   - `WalletRead::get_target_and_anchor_heights` now takes its argument as a `NonZeroU32`
   - `chain::scan_cached_blocks` now takes a `from_height` argument that
-    permits the caller to control the starting position of the scan range.
+    permits the caller to control the starting position of the scan range
+    In addition, the `limit` parameter is now required.
   - A new `CommitmentTree` variant has been added to `data_api::error::Error`
   - `data_api::wallet::{create_spend_to_address, create_proposed_transaction,
     shield_transparent_funds}` all now require that `WalletCommitmentTrees` be
@@ -49,6 +51,8 @@ and this library adheres to Rust's notion of
     now take their respective `min_confirmations` arguments as `NonZeroU32`
   - A new `Scan` variant has been added to `data_api::chain::error::Error`.
   - A new `SyncRequired` variant has been added to `data_api::wallet::input_selection::InputSelectorError`.
+  - The variants of the `PoolType` enum have changed; the `PoolType::Sapling` variant has been 
+    removed in favor of a `PoolType::Shielded` variant that wraps a `ShieldedProtocol` value.
 - `zcash_client_backend::wallet`:
   - `SpendableNote` has been renamed to `ReceivedSaplingNote`.
   - Arguments to `WalletSaplingOutput::from_parts` have changed.

zcash_client_backend/src/data_api.rs

@@ -372,14 +372,21 @@ pub struct SentTransaction<'a> {
     pub utxos_spent: Vec<OutPoint>,
 }
 
+/// A shielded transfer protocol supported by the wallet.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum ShieldedProtocol {
+    /// The Sapling protocol
+    Sapling,
+    // TODO: Orchard
+}
+
 /// A value pool to which the wallet supports sending transaction outputs.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum PoolType {
     /// The transparent value pool
     Transparent,
-    /// The Sapling value pool
+    /// A shielded value pool.
-    Sapling,
+    Shielded(ShieldedProtocol),
-    // TODO: Orchard
 }
 
 /// A type that represents the recipient of a transaction output; a recipient address (and, for
@@ -487,7 +494,6 @@ pub trait WalletWrite: WalletRead {
     /// Updates the state of the wallet database by persisting the provided block information,
     /// along with the note commitments that were detected when scanning the block for transactions
     /// pertaining to this wallet.
-    #[allow(clippy::type_complexity)]
     fn put_block(
         &mut self,
         block: ScannedBlock<sapling::Nullifier>,

zcash_client_backend/src/data_api/chain.rs

@@ -45,12 +45,13 @@
 //! // At this point, the cache and scanned data are locally consistent (though not
 //! // necessarily consistent with the latest chain tip - this would be discovered the
 //! // next time this codepath is executed after new blocks are received).
-//! scan_cached_blocks(&network, &block_source, &mut db_data, None, None)
+//! scan_cached_blocks(&network, &block_source, &mut db_data, BlockHeight::from(0), 10)
 //! # }
 //! # }
 //! ```
 
 use zcash_primitives::{
+    block::BlockHash,
     consensus::{self, BlockHeight},
     sapling::{self, note_encryption::PreparedIncomingViewingKey},
     zip32::Scope,
@@ -60,9 +61,11 @@ use crate::{
     data_api::{NullifierQuery, WalletWrite},
     proto::compact_formats::CompactBlock,
     scan::BatchRunner,
-    scanning::{add_block_to_runner, scan_block_with_runner},
+    scanning::{add_block_to_runner, check_continuity, scan_block_with_runner},
 };
 
+use super::BlockMetadata;
+
 pub mod error;
 use error::Error;
 
@@ -141,8 +144,8 @@ pub fn scan_cached_blocks<ParamsT, DbT, BlockSourceT>(
     params: &ParamsT,
     block_source: &BlockSourceT,
     data_db: &mut DbT,
-    from_height: Option<BlockHeight>,
+    from_height: BlockHeight,
-    limit: Option<u32>,
+    limit: u32,
 ) -> Result<(), Error<DbT::Error, BlockSourceT::Error>>
 where
     ParamsT: consensus::Parameters + Send + 'static,
@@ -178,34 +181,66 @@ where
             .map(|(tag, ivk)| (tag, PreparedIncomingViewingKey::new(&ivk))),
     );
 
-    // Start at either the provided height, or where we synced up to previously.
+    let mut prior_block_metadata = if from_height > BlockHeight::from(0) {
-    let (scan_from, mut prior_block_metadata) = match from_height {
+        data_db
-        Some(h) => {
+            .block_metadata(from_height - 1)
-            // if we are provided with a starting height, obtain the metadata for the previous
+            .map_err(Error::Wallet)?
-            // block (if any is available)
-            (
-                Some(h),
-                if h > BlockHeight::from(0) {
-                    data_db.block_metadata(h - 1).map_err(Error::Wallet)?
     } else {
         None
-                },
-            )
-        }
-        None => {
-            let last_scanned = data_db.block_fully_scanned().map_err(Error::Wallet)?;
-            last_scanned.map_or_else(|| (None, None), |m| (Some(m.block_height + 1), Some(m)))
-        }
     };
 
-    block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| {
+    let mut continuity_check_metadata = prior_block_metadata;
+    block_source.with_blocks::<_, DbT::Error>(
+        Some(from_height),
+        Some(limit),
+        |block: CompactBlock| {
+            // check block continuity
+            if let Some(scan_error) = check_continuity(&block, continuity_check_metadata.as_ref()) {
+                return Err(Error::Scan(scan_error));
+            }
+
+            if from_height == BlockHeight::from(0) {
+                // We can always derive a valid `continuity_check_metadata` for the
+                // genesis block, even if the block source doesn't have
+                // `sapling_commitment_tree_size`. So briefly set it to a dummy value that
+                // ensures the `map` below produces the correct genesis block value.
+                assert!(continuity_check_metadata.is_none());
+                continuity_check_metadata = Some(BlockMetadata::from_parts(
+                    BlockHeight::from(0),
+                    BlockHash([0; 32]),
+                    0,
+                ));
+            }
+            continuity_check_metadata = continuity_check_metadata.as_ref().map(|m| {
+                BlockMetadata::from_parts(
+                    block.height(),
+                    block.hash(),
+                    block
+                        .chain_metadata
+                        .as_ref()
+                        .map(|m| m.sapling_commitment_tree_size)
+                        .unwrap_or_else(|| {
+                            m.sapling_tree_size()
+                                + u32::try_from(
+                                    block.vtx.iter().map(|tx| tx.outputs.len()).sum::<usize>(),
+                                )
+                                .unwrap()
+                        }),
+                )
+            });
+
             add_block_to_runner(params, block, &mut batch_runner);
+
             Ok(())
-    })?;
+        },
+    )?;
 
     batch_runner.flush();
 
-    block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| {
+    block_source.with_blocks::<_, DbT::Error>(
+        Some(from_height),
+        Some(limit),
+        |block: CompactBlock| {
             let scanned_block = scan_block_with_runner(
                 params,
                 block,
@@ -232,7 +267,8 @@ where
             prior_block_metadata = Some(*scanned_block.metadata());
             data_db.put_block(scanned_block).map_err(Error::Wallet)?;
             Ok(())
-    })?;
+        },
+    )?;
 
     Ok(())
 }

zcash_client_backend/src/data_api/wallet.rs

@@ -37,6 +37,8 @@ use crate::{
 pub mod input_selection;
 use input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector};
 
+use super::ShieldedProtocol;
+
 #[cfg(feature = "transparent-inputs")]
 use {
     crate::wallet::WalletTransparentOutput,
@@ -550,7 +552,7 @@ where
                     payment.memo.clone().unwrap_or_else(MemoBytes::empty),
                 )?;
                 sapling_output_meta.push((
-                    Recipient::Unified(ua.clone(), PoolType::Sapling),
+                    Recipient::Unified(ua.clone(), PoolType::Shielded(ShieldedProtocol::Sapling)),
                     payment.amount,
                     payment.memo.clone(),
                 ));
@@ -589,7 +591,10 @@ where
                     MemoBytes::empty(),
                 )?;
                 sapling_output_meta.push((
-                    Recipient::InternalAccount(account, PoolType::Sapling),
+                    Recipient::InternalAccount(
+                        account,
+                        PoolType::Shielded(ShieldedProtocol::Sapling),
+                    ),
                     *amount,
                     change_memo.clone(),
                 ))
@@ -610,8 +615,11 @@ where
                     .output_index(i)
                     .expect("An output should exist in the transaction for each shielded payment.");
 
-                let received_as =
+                let received_as = if let Recipient::InternalAccount(
-                    if let Recipient::InternalAccount(account, PoolType::Sapling) = recipient {
+                    account,
+                    PoolType::Shielded(ShieldedProtocol::Sapling),
+                ) = recipient
+                {
                     tx.sapling_bundle().and_then(|bundle| {
                         try_sapling_note_decryption(
                             params,

zcash_client_backend/src/scanning.rs

@@ -21,7 +21,7 @@ use zcash_primitives::{
     zip32::{sapling::DiversifiableFullViewingKey, AccountId, Scope},
 };
 
-use crate::data_api::{BlockMetadata, ScannedBlock};
+use crate::data_api::{BlockMetadata, ScannedBlock, ShieldedProtocol};
 use crate::{
     proto::compact_formats::CompactBlock,
     scan::{Batch, BatchRunner, Tasks},
@@ -116,17 +116,29 @@ pub enum ScanError {
     /// the current chain tip.
     PrevHashMismatch { at_height: BlockHeight },
 
-    /// The block height field of the proposed new chain tip is not equal to the height of the
+    /// The block height field of the proposed new block is not equal to the height of the previous
-    /// previous chain tip + 1. This variant stores a copy of the incorrect height value for
+    /// block + 1.
-    /// reporting purposes.
     BlockHeightDiscontinuity {
-        previous_tip: BlockHeight,
+        prev_height: BlockHeight,
         new_height: BlockHeight,
     },
 
-    /// The size of the Sapling note commitment tree was not provided as part of a [`CompactBlock`]
+    /// The note commitment tree size for the given protocol at the proposed new block is not equal
-    /// being scanned, making it impossible to construct the nullifier for a detected note.
+    /// to the size at the previous block plus the count of this block's outputs.
-    SaplingTreeSizeUnknown { at_height: BlockHeight },
+    TreeSizeMismatch {
+        protocol: ShieldedProtocol,
+        at_height: BlockHeight,
+        given: u32,
+        computed: u32,
+    },
+
+    /// The size of the note commitment tree for the given protocol was not provided as part of a
+    /// [`CompactBlock`] being scanned, making it impossible to construct the nullifier for a
+    /// detected note.
+    TreeSizeUnknown {
+        protocol: ShieldedProtocol,
+        at_height: BlockHeight,
+    },
 }
 
 impl fmt::Display for ScanError {
@@ -137,11 +149,14 @@ impl fmt::Display for ScanError {
                 "The parent hash of proposed block does not correspond to the block hash at height {}.",
                 at_height
             ),
-            ScanError::BlockHeightDiscontinuity { previous_tip, new_height } => {
+            ScanError::BlockHeightDiscontinuity { prev_height, new_height } => {
-                write!(f, "Block height discontinuity at height {}; next height is : {}", previous_tip, new_height)
+                write!(f, "Block height discontinuity at height {}; next height is : {}", prev_height, new_height)
             }
-            ScanError::SaplingTreeSizeUnknown { at_height } => {
+            ScanError::TreeSizeMismatch { protocol, at_height, given, computed } => {
-                write!(f, "Unable to determine Sapling note commitment tree size at height {}", at_height)
+                write!(f, "The {:?} note commitment tree size provided by a compact block did not match the expected size at height {}; given {}, expected {}", protocol, at_height, given, computed)
+            }
+            ScanError::TreeSizeUnknown { protocol, at_height } => {
+                write!(f, "Unable to determine {:?} note commitment tree size at height {}", protocol, at_height)
             }
         }
     }
@@ -224,6 +239,46 @@ pub(crate) fn add_block_to_runner<P, S, T>(
     }
 }
 
+pub(crate) fn check_continuity(
+    block: &CompactBlock,
+    prior_block_metadata: Option<&BlockMetadata>,
+) -> Option<ScanError> {
+    if let Some(prev) = prior_block_metadata {
+        if block.height() != prev.block_height() + 1 {
+            return Some(ScanError::BlockHeightDiscontinuity {
+                prev_height: prev.block_height(),
+                new_height: block.height(),
+            });
+        }
+
+        if block.prev_hash() != prev.block_hash() {
+            return Some(ScanError::PrevHashMismatch {
+                at_height: block.height(),
+            });
+        }
+
+        if let Some(given) = block
+            .chain_metadata
+            .as_ref()
+            .map(|m| m.sapling_commitment_tree_size)
+        {
+            let computed = prev.sapling_tree_size()
+                + u32::try_from(block.vtx.iter().map(|tx| tx.outputs.len()).sum::<usize>())
+                    .unwrap();
+            if given != computed {
+                return Some(ScanError::TreeSizeMismatch {
+                    protocol: ShieldedProtocol::Sapling,
+                    at_height: block.height(),
+                    given,
+                    computed,
+                });
+            }
+        }
+    }
+
+    None
+}
+
 #[tracing::instrument(skip_all, fields(height = block.height))]
 pub(crate) fn scan_block_with_runner<
     P: consensus::Parameters + Send + 'static,
@@ -237,26 +292,13 @@ pub(crate) fn scan_block_with_runner<
     prior_block_metadata: Option<&BlockMetadata>,
     mut batch_runner: Option<&mut TaggedBatchRunner<P, K::Scope, T>>,
 ) -> Result<ScannedBlock<K::Nf>, ScanError> {
-    let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
+    if let Some(scan_error) = check_continuity(&block, prior_block_metadata) {
-    let mut sapling_note_commitments: Vec<(sapling::Node, Retention<BlockHeight>)> = vec![];
+        return Err(scan_error);
+    }
+
     let cur_height = block.height();
     let cur_hash = block.hash();
 
-    if let Some(prev) = prior_block_metadata {
-        if cur_height != prev.block_height() + 1 {
-            return Err(ScanError::BlockHeightDiscontinuity {
-                previous_tip: prev.block_height(),
-                new_height: cur_height,
-            });
-        }
-
-        if block.prev_hash() != prev.block_hash() {
-            return Err(ScanError::PrevHashMismatch {
-                at_height: cur_height,
-            });
-        }
-    }
-
     // It's possible to make progress without a Sapling tree position if we don't have any Sapling
     // notes in the block, since we only use the position for constructing nullifiers for our own
     // received notes. Thus, we allow it to be optional here, and only produce an error if we try
@@ -281,11 +323,14 @@ pub(crate) fn scan_block_with_runner<
             }
         })
         .or_else(|| prior_block_metadata.map(|m| m.sapling_tree_size()))
-        .ok_or(ScanError::SaplingTreeSizeUnknown {
+        .ok_or(ScanError::TreeSizeUnknown {
+            protocol: ShieldedProtocol::Sapling,
             at_height: cur_height,
         })?;
 
     let compact_block_tx_count = block.vtx.len();
+    let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
+    let mut sapling_note_commitments: Vec<(sapling::Node, Retention<BlockHeight>)> = vec![];
     for (tx_idx, tx) in block.vtx.into_iter().enumerate() {
         let txid = tx.txid();
 

zcash_client_sqlite/src/chain.rs

@@ -330,7 +330,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb);
 
         // Scan the cache
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         // Create a second fake CompactBlock sending more value to the address
         let (cb2, _) = fake_compact_block(
@@ -344,7 +351,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb2);
 
         // Scan the cache again
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 1,
+            1,
+        )
+        .unwrap();
     }
 
     #[test]
@@ -381,7 +395,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb2);
 
         // Scan the cache
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            2,
+        )
+        .unwrap();
 
         // Create more fake CompactBlocks that don't connect to the scanned ones
         let (cb3, _) = fake_compact_block(
@@ -405,7 +426,13 @@ mod tests {
 
         // Data+cache chain should be invalid at the data/cache boundary
         assert_matches!(
-            scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None),
+            scan_cached_blocks(
+                &tests::network(),
+                &db_cache,
+                &mut db_data,
+                sapling_activation_height() + 2,
+                2
+            ),
             Err(_) // FIXME: check error result more closely
         );
     }
@@ -444,7 +471,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb2);
 
         // Scan the cache
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            2,
+        )
+        .unwrap();
 
         // Create more fake CompactBlocks that contain a reorg
         let (cb3, _) = fake_compact_block(
@@ -468,7 +502,13 @@ mod tests {
 
         // Data+cache chain should be invalid inside the cache
         assert_matches!(
-            scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None),
+            scan_cached_blocks(
+                &tests::network(),
+                &db_cache,
+                &mut db_data,
+                sapling_activation_height() + 2,
+                2
+            ),
             Err(_) // FIXME: check error result more closely
         );
     }
@@ -516,7 +556,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb2);
 
         // Scan the cache
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            2,
+        )
+        .unwrap();
 
         // Account balance should reflect both received notes
         assert_eq!(
@@ -551,7 +598,14 @@ mod tests {
         );
 
         // Scan the cache again
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            2,
+        )
+        .unwrap();
 
         // Account balance should again reflect both received notes
         assert_eq!(
@@ -586,7 +640,14 @@ mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb1);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
         assert_eq!(
             get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
             value
@@ -617,8 +678,8 @@ mod tests {
                 &tests::network(),
                 &db_cache,
                 &mut db_data,
-                Some(sapling_activation_height() + 2),
+                sapling_activation_height() + 2,
-                None
+                1
             ),
             Ok(_)
         );
@@ -629,8 +690,8 @@ mod tests {
             &tests::network(),
             &db_cache,
             &mut db_data,
-            Some(sapling_activation_height() + 1),
+            sapling_activation_height() + 1,
-            Some(1),
+            1,
         )
         .unwrap();
         assert_eq!(
@@ -699,7 +760,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb);
 
         // Scan the cache
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         // Account balance should reflect the received note
         assert_eq!(
@@ -720,7 +788,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb2);
 
         // Scan the cache again
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 1,
+            1,
+        )
+        .unwrap();
 
         // Account balance should reflect both received notes
         assert_eq!(
@@ -761,7 +836,14 @@ mod tests {
         insert_into_cache(&db_cache, &cb);
 
         // Scan the cache
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         // Account balance should reflect the received note
         assert_eq!(
@@ -787,7 +869,14 @@ mod tests {
         );
 
         // Scan the cache again
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 1,
+            1,
+        )
+        .unwrap();
 
         // Account balance should equal the change
         assert_eq!(

zcash_client_sqlite/src/lib.rs

@@ -59,7 +59,8 @@ use zcash_client_backend::{
         self,
         chain::{BlockSource, CommitmentTreeRoot},
         BlockMetadata, DecryptedTransaction, NullifierQuery, PoolType, Recipient, ScannedBlock,
-        SentTransaction, WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT,
+        SentTransaction, ShieldedProtocol, WalletCommitmentTrees, WalletRead, WalletWrite,
+        SAPLING_SHARD_HEIGHT,
     },
     keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::compact_formats::CompactBlock,
@@ -459,7 +460,10 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                         let recipient = if output.transfer_type == TransferType::Outgoing {
                             Recipient::Sapling(output.note.recipient())
                         } else {
-                            Recipient::InternalAccount(output.account, PoolType::Sapling)
+                            Recipient::InternalAccount(
+                                output.account,
+                                PoolType::Shielded(ShieldedProtocol::Sapling)
+                            )
                         };
 
                         wallet::put_sent_output(

zcash_client_sqlite/src/wallet.rs

@@ -65,9 +65,10 @@
 //! - `memo` the shielded memo associated with the output, if any.
 
 use rusqlite::{self, named_params, OptionalExtension, ToSql};
+use std::collections::HashMap;
 use std::convert::TryFrom;
-use std::io::Cursor;
+use std::io::{self, Cursor};
-use std::{collections::HashMap, io};
+use zcash_client_backend::data_api::ShieldedProtocol;
 
 use zcash_primitives::{
     block::BlockHash,
@@ -116,7 +117,7 @@ pub(crate) fn pool_code(pool_type: PoolType) -> i64 {
     // implementation detail.
     match pool_type {
         PoolType::Transparent => 0i64,
-        PoolType::Sapling => 2i64,
+        PoolType::Shielded(ShieldedProtocol::Sapling) => 2i64,
     }
 }
 
@@ -1119,7 +1120,11 @@ fn recipient_params<P: consensus::Parameters>(
 ) -> (Option<String>, Option<u32>, PoolType) {
     match to {
         Recipient::Transparent(addr) => (Some(addr.encode(params)), None, PoolType::Transparent),
-        Recipient::Sapling(addr) => (Some(addr.encode(params)), None, PoolType::Sapling),
+        Recipient::Sapling(addr) => (
+            Some(addr.encode(params)),
+            None,
+            PoolType::Shielded(ShieldedProtocol::Sapling),
+        ),
         Recipient::Unified(addr, pool) => (Some(addr.encode(params)), None, *pool),
         Recipient::InternalAccount(id, pool) => (None, Some(u32::from(*id)), *pool),
     }

zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs

@@ -8,7 +8,9 @@ use secrecy::{ExposeSecret, SecretVec};
 use uuid::Uuid;
 
 use zcash_client_backend::{
-    address::RecipientAddress, data_api::PoolType, keys::UnifiedSpendingKey,
+    address::RecipientAddress,
+    data_api::{PoolType, ShieldedProtocol},
+    keys::UnifiedSpendingKey,
 };
 use zcash_primitives::{consensus, zip32::AccountId};
 
@@ -229,7 +231,9 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                         ))
                     })?;
                 let output_pool = match decoded_address {
-                    RecipientAddress::Shielded(_) => Ok(pool_code(PoolType::Sapling)),
+                    RecipientAddress::Shielded(_) => {
+                        Ok(pool_code(PoolType::Shielded(ShieldedProtocol::Sapling)))
+                    }
                     RecipientAddress::Transparent(_) => Ok(pool_code(PoolType::Transparent)),
                     RecipientAddress::Unified(_) => Err(WalletMigrationError::CorruptedData(
                         "Unified addresses should not yet appear in the sent_notes table."

zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs

@@ -6,9 +6,10 @@ use rusqlite::{self, named_params};
 use schemer;
 use schemer_rusqlite::RusqliteMigration;
 use uuid::Uuid;
+use zcash_client_backend::data_api::{PoolType, ShieldedProtocol};
 
 use super::add_transaction_views;
-use crate::wallet::{init::WalletMigrationError, pool_code, PoolType};
+use crate::wallet::{init::WalletMigrationError, pool_code};
 
 pub(super) const MIGRATION_ID: Uuid = Uuid::from_fields(
     0x2aa4d24f,
@@ -48,7 +49,7 @@ impl RusqliteMigration for Migration {
              SELECT tx, :output_pool, output_index, from_account, from_account, value
              FROM sent_notes",
              named_params![
-                ":output_pool": &pool_code(PoolType::Sapling)
+                ":output_pool": &pool_code(PoolType::Shielded(ShieldedProtocol::Sapling))
              ]
         )?;
 

zcash_client_sqlite/src/wallet/sapling.rs

@@ -571,7 +571,14 @@ pub(crate) mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         // Verified balance matches total balance
         let (_, anchor_height) = db_data
@@ -598,7 +605,14 @@ pub(crate) mod tests {
         )
         .0;
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 1,
+            1,
+        )
+        .unwrap();
 
         // Verified balance does not include the second note
         let (_, anchor_height2) = db_data
@@ -651,7 +665,14 @@ pub(crate) mod tests {
             .0;
             insert_into_cache(&db_cache, &cb);
         }
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 2,
+            8,
+        )
+        .unwrap();
 
         // Second spend still fails
         assert_matches!(
@@ -681,11 +702,18 @@ pub(crate) mod tests {
             &dfvk,
             AddressType::DefaultExternal,
             value,
-            11,
+            10,
         )
         .0;
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 10,
+            1,
+        )
+        .unwrap();
 
         // Second spend should now succeed
         assert_matches!(
@@ -730,7 +758,14 @@ pub(crate) mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
         assert_eq!(
             get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
             value
@@ -788,7 +823,14 @@ pub(crate) mod tests {
             .0;
             insert_into_cache(&db_cache, &cb);
         }
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 1,
+            41,
+        )
+        .unwrap();
 
         // Second spend still fails
         assert_matches!(
@@ -821,7 +863,14 @@ pub(crate) mod tests {
         )
         .0;
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 42,
+            1,
+        )
+        .unwrap();
 
         // Second spend should now succeed
         create_spend_to_address(
@@ -865,7 +914,14 @@ pub(crate) mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
         assert_eq!(
             get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
             value
@@ -938,7 +994,14 @@ pub(crate) mod tests {
             .0;
             insert_into_cache(&db_cache, &cb);
         }
-        scan_cached_blocks(&network, &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &network,
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height() + 1,
+            42,
+        )
+        .unwrap();
 
         // Send the funds again, discarding history.
         // Neither transaction output is decryptable by the sender.
@@ -971,7 +1034,14 @@ pub(crate) mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         // Verified balance matches total balance
         let (_, anchor_height) = db_data
@@ -1030,7 +1100,14 @@ pub(crate) mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         // Verified balance matches total balance
         let (_, anchor_height) = db_data
@@ -1103,7 +1180,14 @@ pub(crate) mod tests {
             insert_into_cache(&db_cache, &cb);
         }
 
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            11,
+        )
+        .unwrap();
 
         // Verified balance matches total balance
         let total = Amount::from_u64(60000).unwrap();
@@ -1225,7 +1309,14 @@ pub(crate) mod tests {
             0,
         );
         insert_into_cache(&db_cache, &cb);
-        scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
+        scan_cached_blocks(
+            &tests::network(),
+            &db_cache,
+            &mut db_data,
+            sapling_activation_height(),
+            1,
+        )
+        .unwrap();
 
         assert_matches!(
             shield_transparent_funds(