zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

zcash_client_sqlite: Fix scan range matching with incomplete shard 

The `v_sapling_shard_scan_ranges` view pairs every scan range with every
shard range, such that each row shows an overlapping pair.

For the complete shards, this is an overlap check between two ranges,
which the previous query was performing correctly (if verbosely).

For the last incomplete shard, we have a half-open range that needs to
be handled separately. The previous query only handled the case where a
scan range was contained within the last shard, and did not handle the
case where the scan range contained the last shard.

This led to a puzzling bug, where `WalletDb::get_wallet_summary` was
sometimes treating any note received within the last shard as part of
the wallet's pending balance. If the wallet's scan queue contained a
range that encompassed the last incomplete shard, the bug in the
`v_sapling_shard_scan_ranges` view meant that it omitted any mention of
the last shard, which translated into these notes being considered
unmined when joining `sapling_received_notes` against the sub-view
`v_sapling_shards_scan_state`.

The bug was made harder to diagnose due to the previous commit's bug
that was causing scan ranges to not be correctly merged; this resulted
in smaller scan ranges that were more likely to be contained within the
last shard, making it visible in `v_sapling_shard_scan_ranges` and
enabling notes to be detected as mined.

The fixed view uses a simpler query that enables us to handle complete
and incomplete shards together.

Time spent investigating and fixing: 4.5 hours

Co-authored-by: Kris Nuttycombe <kris@nutty.land>
This commit is contained in:
Jack Grigg 2023-09-08 04:04:10 +00:00
parent 457c9d26dd
commit 504efcfab7
2 changed files with 14 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
zcash_client_sqlite/src/wallet/init.rs
View File

@ -383,13 +383,13 @@ mod tests {
FROM sapling_tree_shards shard
LEFT OUTER JOIN sapling_tree_shards prev_shard
ON shard.shard_index = prev_shard.shard_index + 1
INNER JOIN scan_queue ON
(scan_queue.block_range_start >= subtree_start_height AND shard.subtree_end_height IS NULL) OR
(scan_queue.block_range_start BETWEEN subtree_start_height AND shard.subtree_end_height) OR
((scan_queue.block_range_end - 1) BETWEEN subtree_start_height AND shard.subtree_end_height) OR
-- Join with scan ranges that overlap with the subtree's involved blocks.
INNER JOIN scan_queue ON (
subtree_start_height < scan_queue.block_range_end AND
(
scan_queue.block_range_start <= prev_shard.subtree_end_height
AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height
scan_queue.block_range_start <= shard.subtree_end_height OR
shard.subtree_end_height IS NULL
)
)",
u32::from(st.network().activation_height(NetworkUpgrade::Sapling).unwrap()),
),

12
zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs
View File

@ -52,13 +52,13 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
FROM sapling_tree_shards shard
LEFT OUTER JOIN sapling_tree_shards prev_shard
ON shard.shard_index = prev_shard.shard_index + 1
INNER JOIN scan_queue ON
(scan_queue.block_range_start >= subtree_start_height AND shard.subtree_end_height IS NULL) OR
(scan_queue.block_range_start BETWEEN subtree_start_height AND shard.subtree_end_height) OR
((scan_queue.block_range_end - 1) BETWEEN subtree_start_height AND shard.subtree_end_height) OR
-- Join with scan ranges that overlap with the subtree's involved blocks.
INNER JOIN scan_queue ON (
subtree_start_height < scan_queue.block_range_end AND
(
scan_queue.block_range_start <= prev_shard.subtree_end_height
AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height
scan_queue.block_range_start <= shard.subtree_end_height OR
shard.subtree_end_height IS NULL
)
)",
SAPLING_SHARD_HEIGHT,
SAPLING_SHARD_HEIGHT,