From 97a3f818ec990e537f338eb6f8707f14328478e8 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Fri, 21 Jul 2023 20:59:24 +0000
Subject: [PATCH 1/2] zcash_client_sqlite: Only import recent frontiers into
 `ShardTree`

We only need to load frontiers into the ShardTree that are close enough
to the wallet's known chain tip to fill `PRUNING_DEPTH` checkpoints, so
that ShardTree's witness generation will be able to correctly handle
anchor depths. Loading frontiers further back than this doesn't add any
useful nodes to the ShardTree (as we don't support rollbacks beyond
`PRUNING_DEPTH`, and we won't be finding notes in earlier blocks), and
hurts performance (as frontier importing has a significant Merkle tree
hashing cost).

Closes zcash/librustzcash#877.
---
 .../init/migrations/shardtree_support.rs      | 57 ++++++++++++-------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
index f8c4a61af..663a1c37d 100644
--- a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
+++ b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
@@ -103,6 +103,8 @@ impl RusqliteMigration for Migration {
             );",
         )?;
 
+        let block_height_extrema = block_height_extrema(transaction)?;
+
         let shard_store =
             SqliteShardStore::<_, sapling::Node, SAPLING_SHARD_HEIGHT>::from_connection(
                 transaction,
@@ -149,25 +151,40 @@ impl RusqliteMigration for Migration {
                 stmt_update_block_sapling_tree_size
                     .execute(params![block_end_tree.size(), block_height])?;
 
-                if let Some(nonempty_frontier) = block_end_tree.to_frontier().value() {
-                    trace!(
-                        height = block_height,
-                        frontier = ?nonempty_frontier,
-                        "Inserting frontier nodes",
-                    );
-                    shard_tree
-                        .insert_frontier_nodes(
-                            nonempty_frontier.clone(),
-                            Retention::Checkpoint {
-                                id: BlockHeight::from(block_height),
-                                is_marked: false,
-                            },
-                        )
-                        .map_err(|e| match e {
-                            ShardTreeError::Query(e) => ShardTreeError::Query(e),
-                            ShardTreeError::Insert(e) => ShardTreeError::Insert(e),
-                            ShardTreeError::Storage(_) => unreachable!(),
-                        })?
+                // We only need to load frontiers into the ShardTree that are close enough
+                // to the wallet's known chain tip to fill `PRUNING_DEPTH` checkpoints, so
+                // that ShardTree's witness generation will be able to correctly handle
+                // anchor depths. Loading frontiers further back than this doesn't add any
+                // useful nodes to the ShardTree (as we don't support rollbacks beyond
+                // `PRUNING_DEPTH`, and we won't be finding notes in earlier blocks), and
+                // hurts performance (as frontier importing has a significant Merkle tree
+                // hashing cost).
+                if let Some((nonempty_frontier, (_, latest_height))) = block_end_tree
+                    .to_frontier()
+                    .value()
+                    .zip(block_height_extrema)
+                {
+                    let block_height = BlockHeight::from(block_height);
+                    if block_height + PRUNING_DEPTH >= latest_height {
+                        trace!(
+                            height = u32::from(block_height),
+                            frontier = ?nonempty_frontier,
+                            "Inserting frontier nodes",
+                        );
+                        shard_tree
+                            .insert_frontier_nodes(
+                                nonempty_frontier.clone(),
+                                Retention::Checkpoint {
+                                    id: block_height,
+                                    is_marked: false,
+                                },
+                            )
+                            .map_err(|e| match e {
+                                ShardTreeError::Query(e) => ShardTreeError::Query(e),
+                                ShardTreeError::Insert(e) => ShardTreeError::Insert(e),
+                                ShardTreeError::Storage(_) => unreachable!(),
+                            })?
+                    }
                 }
             }
         }
@@ -241,7 +258,7 @@ impl RusqliteMigration for Migration {
             );",
         )?;
 
-        if let Some((start, end)) = block_height_extrema(transaction)? {
+        if let Some((start, end)) = block_height_extrema {
             // `ScanRange` uses an exclusive upper bound.
             let chain_end = end + 1;
             insert_queue_entries(

From ed06a37b099609f8870e85b0c4511389e6063326 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Fri, 21 Jul 2023 21:01:03 +0000
Subject: [PATCH 2/2] zcash_client_sqlite: Mark existing blocks scanned in
 `shardtree` migration

The `shardtree` migration is applied to a database state that was
created via linear scanning, so we have complete wallet information for
those blocks.
---
 .../src/wallet/init/migrations/shardtree_support.rs             | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
index 663a1c37d..1abba5f42 100644
--- a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
+++ b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
@@ -265,7 +265,7 @@ impl RusqliteMigration for Migration {
                 transaction,
                 Some(ScanRange::from_parts(
                     start..chain_end,
-                    ScanPriority::Historic,
+                    ScanPriority::Scanned,
                 ))
                 .iter(),
             )?;