From dbf7ea05fa9c02325c87227a48ad941befbdbc48 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 23 Dec 2024 06:40:40 +0000 Subject: [PATCH 01/37] added sync status test --- libtonode-tests/tests/sync.rs | 50 +++++++++++++++++++++++++++++++++-- zingo-sync/src/sync.rs | 18 +++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/libtonode-tests/tests/sync.rs b/libtonode-tests/tests/sync.rs index 0bf6b79cc..88e713160 100644 --- a/libtonode-tests/tests/sync.rs +++ b/libtonode-tests/tests/sync.rs @@ -3,7 +3,7 @@ use tempfile::TempDir; use testvectors::seeds::HOSPITAL_MUSEUM_SEED; use zingo_netutils::GrpcConnector; -use zingo_sync::sync::sync; +use zingo_sync::sync::{self, sync}; use zingolib::{ config::{construct_lightwalletd_uri, load_clientconfig, DEFAULT_LIGHTWALLETD_SERVER}, get_base_address_macro, @@ -12,7 +12,7 @@ use zingolib::{ wallet::WalletBase, }; -#[ignore = "too slow, and flakey"] +#[ignore = "temporary mainnet test for sync development"] #[tokio::test] async fn sync_mainnet_test() { rustls::crypto::ring::default_provider() @@ -51,6 +51,52 @@ async fn sync_mainnet_test() { dbg!(&wallet.sync_state); } +#[ignore = "mainnet test for large chain"] +#[tokio::test] +async fn sync_status() { + rustls::crypto::ring::default_provider() + .install_default() + .expect("Ring to work as a default"); + tracing_subscriber::fmt().init(); + + let uri = construct_lightwalletd_uri(Some(DEFAULT_LIGHTWALLETD_SERVER.to_string())); + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path().to_path_buf(); + let config = load_clientconfig( + uri.clone(), + Some(temp_path), + zingolib::config::ChainType::Mainnet, + true, + ) + .unwrap(); + let lightclient = LightClient::create_from_wallet_base_async( + WalletBase::from_string(HOSPITAL_MUSEUM_SEED.to_string()), + &config, + 2_700_000, + true, + ) + .await + .unwrap(); + + let client = GrpcConnector::new(uri).get_client().await.unwrap(); + + let wallet = lightclient.wallet.clone(); + let sync_handle = tokio::spawn(async move { + sync(client, &config.chain, wallet).await.unwrap(); + }); + + let wallet = lightclient.wallet.clone(); + tokio::spawn(async move { + loop { + let wallet = wallet.clone(); + sync::sync_status(wallet).await.unwrap(); + } + }); + + sync_handle.await.unwrap(); +} + +// temporary test for sync development #[tokio::test] async fn sync_test() { tracing_subscriber::fmt().init(); diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index c4ae1f432..2bfbb4a18 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -172,6 +172,24 @@ where Ok(()) } +/// TODO +pub async fn sync_status(wallet: Arc>) -> Result<(), SyncError> +where + W: SyncWallet, +{ + let scan_ranges = wallet + .lock() + .await + .get_sync_state() + .unwrap() + .scan_ranges() + .clone(); + + dbg!(scan_ranges); + + Ok(()) +} + /// Returns true if sync is complete. /// /// Sync is complete when: From 83be516cbf611a9a45a3f84815a895a4f388f3cf Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 23 Dec 2024 12:33:53 +0000 Subject: [PATCH 02/37] added sync status with test --- libtonode-tests/tests/sync.rs | 6 ++- zingo-sync/src/client.rs | 2 +- zingo-sync/src/client/fetch.rs | 12 ++--- zingo-sync/src/primitives.rs | 25 ++++++++-- zingo-sync/src/scan/task.rs | 6 +-- zingo-sync/src/sync.rs | 61 +++++++++++++++++------ zingolib/src/wallet/disk/testing/tests.rs | 2 +- 7 files changed, 81 insertions(+), 33 deletions(-) diff --git a/libtonode-tests/tests/sync.rs b/libtonode-tests/tests/sync.rs index 88e713160..02368da69 100644 --- a/libtonode-tests/tests/sync.rs +++ b/libtonode-tests/tests/sync.rs @@ -1,5 +1,7 @@ #![cfg(feature = "sync")] +use std::time::Duration; + use tempfile::TempDir; use testvectors::seeds::HOSPITAL_MUSEUM_SEED; use zingo_netutils::GrpcConnector; @@ -89,7 +91,9 @@ async fn sync_status() { tokio::spawn(async move { loop { let wallet = wallet.clone(); - sync::sync_status(wallet).await.unwrap(); + let sync_status = sync::sync_status(wallet).await; + dbg!(sync_status); + tokio::time::sleep(Duration::from_secs(1)).await; } }); diff --git a/zingo-sync/src/client.rs b/zingo-sync/src/client.rs index 04e22980b..a6baf2f65 100644 --- a/zingo-sync/src/client.rs +++ b/zingo-sync/src/client.rs @@ -158,7 +158,7 @@ pub async fn get_transparent_address_transactions( pub async fn get_mempool_transaction_stream( client: &mut CompactTxStreamerClient, ) -> Result, ()> { - tracing::info!("Fetching mempool stream"); + tracing::debug!("Fetching mempool stream"); let mempool_stream = fetch::get_mempool_stream(client).await.unwrap(); Ok(mempool_stream) diff --git a/zingo-sync/src/client/fetch.rs b/zingo-sync/src/client/fetch.rs index 3b660f5e9..1362e0b04 100644 --- a/zingo-sync/src/client/fetch.rs +++ b/zingo-sync/src/client/fetch.rs @@ -103,29 +103,29 @@ async fn fetch_from_server( ) -> Result<(), ()> { match fetch_request { FetchRequest::ChainTip(sender) => { - tracing::info!("Fetching chain tip."); + tracing::debug!("Fetching chain tip."); let block_id = get_latest_block(client).await.unwrap(); sender.send(block_id).unwrap(); } FetchRequest::CompactBlockRange(sender, block_range) => { - tracing::info!("Fetching compact blocks. {:?}", &block_range); + tracing::debug!("Fetching compact blocks. {:?}", &block_range); let compact_blocks = get_block_range(client, block_range).await.unwrap(); sender.send(compact_blocks).unwrap(); } FetchRequest::TreeState(sender, block_height) => { - tracing::info!("Fetching tree state. {:?}", &block_height); + tracing::debug!("Fetching tree state. {:?}", &block_height); let tree_state = get_tree_state(client, block_height).await.unwrap(); sender.send(tree_state).unwrap(); } FetchRequest::Transaction(sender, txid) => { - tracing::info!("Fetching transaction. {:?}", txid); + tracing::debug!("Fetching transaction. {:?}", txid); let transaction = get_transaction(client, consensus_parameters, txid) .await .unwrap(); sender.send(transaction).unwrap(); } FetchRequest::UtxoMetadata(sender, (addresses, start_height)) => { - tracing::info!( + tracing::debug!( "Fetching unspent transparent output metadata from {:?} for addresses:\n{:?}", &start_height, &addresses @@ -136,7 +136,7 @@ async fn fetch_from_server( sender.send(utxo_metadata).unwrap(); } FetchRequest::TransparentAddressTxs(sender, (address, block_range)) => { - tracing::info!( + tracing::debug!( "Fetching raw transactions in block range {:?} for address {:?}", &block_range, &address diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index aab95173b..112913f0d 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -26,7 +26,7 @@ use crate::{ pub type Locator = (BlockHeight, TxId); /// Encapsulates the current state of sync -#[derive(Debug, Getters, MutGetters)] +#[derive(Debug, Clone, Getters, MutGetters, CopyGetters, Setters)] #[getset(get = "pub", get_mut = "pub")] pub struct SyncState { /// A vec of block ranges with scan priorities from wallet birthday to chain tip. @@ -34,6 +34,16 @@ pub struct SyncState { scan_ranges: Vec, /// Locators for relevent transactions to the wallet. locators: BTreeSet, + /// Fully scanned wallet height at start of sync. + /// Reset when sync starts. + #[getset(skip)] + #[getset(get_copy = "pub", set = "pub")] + sync_start_height: BlockHeight, + /// Total number of blocks to scan this session + /// Reset when sync starts. + #[getset(skip)] + #[getset(get_copy = "pub", set = "pub")] + total_blocks_to_scan: u32, } impl SyncState { @@ -42,6 +52,8 @@ impl SyncState { SyncState { scan_ranges: Vec::new(), locators: BTreeSet::new(), + sync_start_height: 0.into(), + total_blocks_to_scan: 0, } } @@ -70,10 +82,13 @@ impl SyncState { } } -impl Default for SyncState { - fn default() -> Self { - Self::new() - } +/// A snapshot of the current state of sync. Useful for displaying the status of sync to a user / consumer. +#[derive(Debug, Clone, Getters)] +pub struct SyncStatus { + pub scan_ranges: Vec, + pub scanned_blocks: u32, + pub unscanned_blocks: u32, + pub percentage_blocks_complete: f32, } /// Output ID for a given pool type diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index ef3cecfdb..e4d6f8866 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -88,7 +88,7 @@ where /// /// When the worker is running it will wait for a scan task. pub(crate) fn spawn_worker(&mut self) { - tracing::info!("Spawning worker {}", self.unique_id); + tracing::debug!("Spawning worker {}", self.unique_id); let mut worker = ScanWorker::new( self.unique_id, self.consensus_parameters.clone(), @@ -286,7 +286,7 @@ where } fn add_scan_task(&self, scan_task: ScanTask) -> Result<(), ()> { - tracing::info!("Adding scan task to worker {}:\n{:#?}", self.id, &scan_task); + tracing::debug!("Adding scan task to worker {}:\n{:#?}", self.id, &scan_task); self.scan_task_sender .clone() .unwrap() @@ -301,7 +301,7 @@ where /// /// This should always be called in the context of the scanner as it must be also be removed from the worker pool. async fn shutdown(&mut self) -> Result<(), JoinError> { - tracing::info!("Shutting down worker {}", self.id); + tracing::debug!("Shutting down worker {}", self.id); if let Some(sender) = self.scan_task_sender.take() { drop(sender); } diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 2bfbb4a18..a6d1262b3 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -8,7 +8,7 @@ use std::time::Duration; use crate::client::{self, FetchRequest}; use crate::error::SyncError; use crate::keys::transparent::TransparentAddressId; -use crate::primitives::{NullifierMap, OutPointMap}; +use crate::primitives::{NullifierMap, OutPointMap, SyncState, SyncStatus}; use crate::scan::error::{ContinuityError, ScanError}; use crate::scan::task::Scanner; use crate::scan::transactions::scan_transaction; @@ -65,7 +65,7 @@ where }); let mut wallet_lock = wallet.lock().await; - let wallet_height = state::get_wallet_height(consensus_parameters, &*wallet_lock).unwrap(); + let mut wallet_height = state::get_wallet_height(consensus_parameters, &*wallet_lock).unwrap(); let chain_height = client::get_chain_height(fetch_request_sender.clone()) .await .unwrap(); @@ -77,6 +77,7 @@ where ); } truncate_wallet_data(&mut *wallet_lock, chain_height).unwrap(); + wallet_height = chain_height; } let ufvks = wallet_lock.get_unified_full_viewing_keys().unwrap(); @@ -105,6 +106,20 @@ where ) .await .unwrap(); + + let sync_state = wallet_lock.get_sync_state_mut().unwrap(); + let total_blocks_to_scan = sync_state + .scan_ranges() + .iter() + .filter(|scan_range| scan_range.priority() != ScanPriority::Scanned) + .map(|scan_range| scan_range.block_range()) + .fold(0, |acc, block_range| { + acc + u32::from(block_range.end - block_range.start) + }); + sync_state.set_total_blocks_to_scan(total_blocks_to_scan); + let sync_start_height = sync_state.fully_scanned_height() + 1; + sync_state.set_sync_start_height(sync_start_height); + drop(wallet_lock); // create channel for receiving scan results and launch scanner @@ -163,6 +178,8 @@ where } } + // TODO: clear locators + drop(wallet_lock); drop(scanner); drop(fetch_request_sender); @@ -172,22 +189,34 @@ where Ok(()) } -/// TODO -pub async fn sync_status(wallet: Arc>) -> Result<(), SyncError> +/// Obtains the mutex guard to the wallet and creates a [`crate::primitives::SyncStatus`] from the wallet's current +/// [`crate::primitives::SyncState`]. +/// +/// Designed to be called during the sync process with minimal interruption. +pub async fn sync_status(wallet: Arc>) -> SyncStatus where W: SyncWallet, { - let scan_ranges = wallet - .lock() - .await - .get_sync_state() - .unwrap() - .scan_ranges() - .clone(); + let sync_state: SyncState = wallet.lock().await.get_sync_state().unwrap().clone(); - dbg!(scan_ranges); - - Ok(()) + let unscanned_blocks = sync_state + .scan_ranges() + .iter() + .filter(|scan_range| scan_range.priority() != ScanPriority::Scanned) + .map(|scan_range| scan_range.block_range()) + .fold(0, |acc, block_range| { + acc + u32::from(block_range.end - block_range.start) + }); + let scanned_blocks = sync_state.total_blocks_to_scan() - unscanned_blocks; + let percentage_blocks_complete = + (scanned_blocks as f32 / sync_state.total_blocks_to_scan() as f32) * 100.0; + + SyncStatus { + scan_ranges: sync_state.scan_ranges().clone(), + unscanned_blocks, + scanned_blocks, + percentage_blocks_complete, + } } /// Returns true if sync is complete. @@ -242,7 +271,7 @@ where ScanPriority::Scanned, ) .unwrap(); - tracing::info!("Scan results processed."); + tracing::debug!("Scan results processed."); } Err(ScanError::ContinuityError(ContinuityError::HashDiscontinuity { height, .. })) => { tracing::info!("Re-org detected."); @@ -289,7 +318,7 @@ async fn process_mempool_transaction( ) .unwrap(); - tracing::info!( + tracing::debug!( "mempool received txid {} at height {}", transaction.txid(), block_height diff --git a/zingolib/src/wallet/disk/testing/tests.rs b/zingolib/src/wallet/disk/testing/tests.rs index 1fb3dd950..28699dd90 100644 --- a/zingolib/src/wallet/disk/testing/tests.rs +++ b/zingolib/src/wallet/disk/testing/tests.rs @@ -303,9 +303,9 @@ async fn reload_wallet_from_buffer() { #[tokio::test] #[cfg(feature = "sync")] async fn reload_wallet_from_buffer() { - use crate::testvectors::seeds::CHIMNEY_BETTER_SEED; use crate::wallet::WalletBase; use crate::wallet::WalletCapability; + use testvectors::seeds::CHIMNEY_BETTER_SEED; let mid_wallet = NetworkSeedVersion::Testnet(TestnetSeedVersion::ChimneyBetter(ChimneyBetterVersion::V28)) From 25ece4de4f7815eb4d150c618daeadd3c0791f3d Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 24 Dec 2024 08:00:40 +0000 Subject: [PATCH 03/37] set new grpc method to debug level logging --- zingo-sync/src/client/fetch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zingo-sync/src/client/fetch.rs b/zingo-sync/src/client/fetch.rs index 9271b60d5..e7d6b64ba 100644 --- a/zingo-sync/src/client/fetch.rs +++ b/zingo-sync/src/client/fetch.rs @@ -113,7 +113,7 @@ async fn fetch_from_server( sender.send(compact_blocks).unwrap(); } FetchRequest::GetSubtreeRoots(sender, start_index, shielded_protocol, max_entries) => { - tracing::info!( + tracing::debug!( "Fetching subtree roots. start index: {}. shielded protocol: {}", start_index, shielded_protocol From d4e2a9c506393523fed10b6dbe02ff1092913ea2 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 26 Dec 2024 05:25:23 +0000 Subject: [PATCH 04/37] move forming of vec of subtree roots earlier to use for batching --- zingo-sync/src/client.rs | 11 ++-- zingo-sync/src/sync.rs | 110 +++++++++++++------------------------- zingo-sync/src/witness.rs | 45 +++++++++++----- 3 files changed, 78 insertions(+), 88 deletions(-) diff --git a/zingo-sync/src/client.rs b/zingo-sync/src/client.rs index 30d083198..ce94ac0af 100644 --- a/zingo-sync/src/client.rs +++ b/zingo-sync/src/client.rs @@ -93,7 +93,7 @@ pub async fn get_subtree_roots( start_index: u32, shielded_protocol: i32, max_entries: u32, -) -> Result, ()> { +) -> Result, ()> { let (reply_sender, reply_receiver) = oneshot::channel(); fetch_request_sender .send(FetchRequest::GetSubtreeRoots( @@ -103,8 +103,13 @@ pub async fn get_subtree_roots( max_entries, )) .unwrap(); - let shards = reply_receiver.await.unwrap(); - Ok(shards) + let mut subtree_root_stream = reply_receiver.await.unwrap(); + let mut subtree_roots = Vec::new(); + while let Some(subtree_root) = subtree_root_stream.message().await.unwrap() { + subtree_roots.push(subtree_root); + } + + Ok(subtree_roots) } /// Gets the frontiers for a specified block height. diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index a0c67fe79..830a2fa0c 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -16,9 +16,9 @@ use crate::scan::{DecryptedNoteData, ScanResults}; use crate::traits::{ SyncBlocks, SyncNullifiers, SyncOutPoints, SyncShardTrees, SyncTransactions, SyncWallet, }; +use crate::witness; -use futures::StreamExt; -use shardtree::{store::ShardStore, LocatedPrunableTree, RetentionFlags}; +use shardtree::store::ShardStore; use state::set_initial_state; use zcash_client_backend::proto::service::RawTransaction; use zcash_client_backend::{ @@ -215,77 +215,6 @@ where } } -async fn update_subtree_roots( - fetch_request_sender: mpsc::UnboundedSender, - wallet: &mut W, -) where - W: SyncWallet + SyncBlocks + SyncTransactions + SyncNullifiers + SyncOutPoints + SyncShardTrees, -{ - let sapling_start_index = wallet - .get_shard_trees() - .unwrap() - .sapling() - .store() - .get_shard_roots() - .unwrap() - .len() as u32; - let orchard_start_index = wallet - .get_shard_trees() - .unwrap() - .orchard() - .store() - .get_shard_roots() - .unwrap() - .len() as u32; - let (sapling_shards, orchard_shards) = futures::join!( - client::get_subtree_roots(fetch_request_sender.clone(), sapling_start_index, 0, 0), - client::get_subtree_roots(fetch_request_sender, orchard_start_index, 1, 0) - ); - - let trees = wallet.get_shard_trees_mut().unwrap(); - - let (sapling_tree, orchard_tree) = trees.both_mut(); - futures::join!( - update_tree_with_shards(sapling_shards, sapling_tree), - update_tree_with_shards(orchard_shards, orchard_tree) - ); -} - -async fn update_tree_with_shards( - shards: Result, ()>, - tree: &mut shardtree::ShardTree, -) where - S: ShardStore< - H: incrementalmerkletree::Hashable + Clone + PartialEq + crate::traits::FromBytes<32>, - CheckpointId: Clone + Ord + std::fmt::Debug, - Error = std::convert::Infallible, - >, -{ - let shards = shards - .unwrap() - .collect::>() - .await - .into_iter() - .collect::, _>>() - .unwrap(); - shards - .into_iter() - .enumerate() - .for_each(|(index, tree_root)| { - let node = >::from_bytes( - tree_root.root_hash.try_into().unwrap(), - ); - let shard = LocatedPrunableTree::with_root_value( - incrementalmerkletree::Address::from_parts( - incrementalmerkletree::Level::new(SHARD_HEIGHT), - index as u64, - ), - (node, RetentionFlags::EPHEMERAL), - ); - tree.store_mut().put_shard(shard).unwrap(); - }); -} - /// Returns true if sync is complete. /// /// Sync is complete when: @@ -552,6 +481,41 @@ where Ok(()) } +async fn update_subtree_roots( + fetch_request_sender: mpsc::UnboundedSender, + wallet: &mut W, +) where + W: SyncShardTrees, +{ + let sapling_start_index = wallet + .get_shard_trees() + .unwrap() + .sapling() + .store() + .get_shard_roots() + .unwrap() + .len() as u32; + let orchard_start_index = wallet + .get_shard_trees() + .unwrap() + .orchard() + .store() + .get_shard_roots() + .unwrap() + .len() as u32; + let (sapling_subtree_roots, orchard_subtree_roots) = futures::join!( + client::get_subtree_roots(fetch_request_sender.clone(), sapling_start_index, 0, 0), + client::get_subtree_roots(fetch_request_sender, orchard_start_index, 1, 0) + ); + + let sapling_subtree_roots = sapling_subtree_roots.unwrap(); + let orchard_subtree_roots = orchard_subtree_roots.unwrap(); + + let shard_trees = wallet.get_shard_trees_mut().unwrap(); + witness::add_subtree_roots(sapling_subtree_roots, shard_trees.sapling_mut()); + witness::add_subtree_roots(orchard_subtree_roots, shard_trees.orchard_mut()); +} + /// Sets up mempool stream. /// /// If there is some raw transaction, send to be scanned. diff --git a/zingo-sync/src/witness.rs b/zingo-sync/src/witness.rs index 8958c9277..72387c327 100644 --- a/zingo-sync/src/witness.rs +++ b/zingo-sync/src/witness.rs @@ -5,7 +5,11 @@ use std::collections::BTreeMap; use getset::{Getters, MutGetters}; use incrementalmerkletree::{Position, Retention}; use orchard::tree::MerkleHashOrchard; -use shardtree::{store::memory::MemoryShardStore, LocatedPrunableTree, ShardTree}; +use shardtree::{ + store::{memory::MemoryShardStore, ShardStore}, + LocatedPrunableTree, ShardTree, +}; +use zcash_client_backend::proto::service::SubtreeRoot; use zcash_primitives::consensus::BlockHeight; const NOTE_COMMITMENT_TREE_DEPTH: u8 = 32; @@ -34,17 +38,6 @@ impl ShardTrees { orchard: ShardTree::new(MemoryShardStore::empty(), MAX_CHECKPOINTS), } } - - // This lets us 'split the borrow' to operate - // on both trees concurrently. - pub(crate) fn both_mut( - &mut self, - ) -> ( - &mut ShardTree, - &mut ShardTree, - ) { - (&mut self.sapling, &mut self.orchard) - } } impl Default for ShardTrees { @@ -127,3 +120,31 @@ where Ok(located_tree_data) } + +pub(crate) fn add_subtree_roots( + subtree_roots: Vec, + shard_tree: &mut shardtree::ShardTree, +) where + S: ShardStore< + H: incrementalmerkletree::Hashable + Clone + PartialEq + crate::traits::FromBytes<32>, + CheckpointId: Clone + Ord + std::fmt::Debug, + Error = std::convert::Infallible, + >, +{ + subtree_roots + .into_iter() + .enumerate() + .for_each(|(index, tree_root)| { + let node = >::from_bytes( + tree_root.root_hash.try_into().unwrap(), + ); + let shard = LocatedPrunableTree::with_root_value( + incrementalmerkletree::Address::from_parts( + incrementalmerkletree::Level::new(SHARD_HEIGHT), + index as u64, + ), + (node, shardtree::RetentionFlags::EPHEMERAL), + ); + shard_tree.store_mut().put_shard(shard).unwrap(); + }); +} From 52c80ac2a3e70d525a4e235cc6fcdc7716c3e63d Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 26 Dec 2024 08:00:28 +0000 Subject: [PATCH 05/37] improve comments --- zingo-sync/src/primitives.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index 112913f0d..687e496cb 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -34,7 +34,7 @@ pub struct SyncState { scan_ranges: Vec, /// Locators for relevent transactions to the wallet. locators: BTreeSet, - /// Fully scanned wallet height at start of sync. + /// The block above the fully scanned wallet height at start of sync. /// Reset when sync starts. #[getset(skip)] #[getset(get_copy = "pub", set = "pub")] @@ -82,7 +82,9 @@ impl SyncState { } } -/// A snapshot of the current state of sync. Useful for displaying the status of sync to a user / consumer. +/// A snapshot of the current state of sync. +/// +/// Useful for displaying the status of sync to a user / consumer. #[derive(Debug, Clone, Getters)] pub struct SyncStatus { pub scan_ranges: Vec, From f72e996cdf1fe5130c7184ebb601be37c172712c Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 26 Dec 2024 08:02:55 +0000 Subject: [PATCH 06/37] fix clippy --- zingo-sync/src/primitives.rs | 6 ++++++ zingo-sync/src/sync.rs | 2 +- zingo-sync/src/sync/state.rs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index 52be0a11f..eff776c49 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -82,6 +82,12 @@ impl SyncState { } } +impl Default for SyncState { + fn default() -> Self { + Self::new() + } +} + /// A snapshot of the current state of sync. Useful for displaying the status of sync to a user / consumer. #[derive(Debug, Clone, Getters)] pub struct SyncStatus { diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index fa98891d2..0ed23ac8f 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -201,7 +201,7 @@ where .filter(|scan_range| scan_range.priority() != ScanPriority::Scanned) .map(|scan_range| scan_range.block_range()) .fold(0, |acc, block_range| { - acc + u32::from(block_range.end - block_range.start) + acc + (block_range.end - block_range.start) }); let scanned_blocks = sync_state.total_blocks_to_scan() - unscanned_blocks; let percentage_blocks_complete = diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index a799b4d74..d29eac6fb 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -436,7 +436,7 @@ pub(super) fn set_initial_state(sync_state: &mut SyncState) { .filter(|scan_range| scan_range.priority() != ScanPriority::Scanned) .map(|scan_range| scan_range.block_range()) .fold(0, |acc, block_range| { - acc + u32::from(block_range.end - block_range.start) + acc + (block_range.end - block_range.start) }); sync_state.set_total_blocks_to_scan(total_blocks_to_scan); let sync_start_height = sync_state.fully_scanned_height() + 1; From a0281c606715a74640bb5b17345084396994237e Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Fri, 27 Dec 2024 07:27:11 +0000 Subject: [PATCH 07/37] implemented batch by shards --- libtonode-tests/tests/sync.rs | 4 +- zingo-sync/src/primitives.rs | 42 ++++++- zingo-sync/src/sync.rs | 27 ++++- zingo-sync/src/sync/state.rs | 204 ++++++++++++++++++++++++++-------- 4 files changed, 221 insertions(+), 56 deletions(-) diff --git a/libtonode-tests/tests/sync.rs b/libtonode-tests/tests/sync.rs index 738f0ae65..aab6278bb 100644 --- a/libtonode-tests/tests/sync.rs +++ b/libtonode-tests/tests/sync.rs @@ -74,7 +74,7 @@ async fn sync_status() { let lightclient = LightClient::create_from_wallet_base_async( WalletBase::from_string(HOSPITAL_MUSEUM_SEED.to_string()), &config, - 2_700_000, + 2_670_000, true, ) .await @@ -105,7 +105,7 @@ async fn sync_status() { async fn sync_test() { tracing_subscriber::fmt().init(); - let (_regtest_manager, _cph, faucet, mut recipient, _txid) = + let (_regtest_manager, _cph, faucet, recipient, _txid) = scenarios::faucet_funded_recipient_default(5_000_000).await; from_inputs::quick_send( &faucet, diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index 4b9a8a831..fdbf90951 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -1,6 +1,9 @@ //! Module for primitive structs associated with the sync engine -use std::collections::{BTreeMap, BTreeSet}; +use std::{ + collections::{BTreeMap, BTreeSet}, + ops::Range, +}; use getset::{CopyGetters, Getters, MutGetters, Setters}; @@ -32,6 +35,16 @@ pub struct SyncState { /// A vec of block ranges with scan priorities from wallet birthday to chain tip. /// In block height order with no overlaps or gaps. scan_ranges: Vec, + /// The block ranges that contain all sapling outputs of complete sapling shards. + /// + /// There is an edge case where a range may include two (or more) shards. However, this only occurs when the lower + /// shards are already scanned so the trimming will be handled by [`crate::sync::state::determine_block_range`]. + sapling_shard_ranges: Vec>, + /// The block ranges that contain all orchard outputs of complete orchard shards. + /// + /// There is an edge case where a range may include two (or more) shards. However, this only occurs when the lower + /// shards are already scanned so the trimming will be handled by [`crate::sync::state::determine_block_range`]. + orchard_shard_ranges: Vec>, /// Locators for relevant transactions to the wallet. locators: BTreeSet, /// The block above the fully scanned wallet height at start of sync. @@ -51,6 +64,8 @@ impl SyncState { pub fn new() -> Self { SyncState { scan_ranges: Vec::new(), + sapling_shard_ranges: Vec::new(), + orchard_shard_ranges: Vec::new(), locators: BTreeSet::new(), sync_start_height: 0.into(), total_blocks_to_scan: 0, @@ -65,6 +80,8 @@ impl SyncState { } /// Returns the block height at which all blocks equal to and below this height are scanned. + /// + /// Will panic if called before scan ranges are updated for the first time. pub fn fully_scanned_height(&self) -> BlockHeight { if let Some(scan_range) = self .scan_ranges() @@ -80,6 +97,29 @@ impl SyncState { .end } } + + /// Returns the wallet birthday. + /// + /// Will panic if called before scan ranges are updated for the first time. + pub fn wallet_birthday(&self) -> BlockHeight { + self.scan_ranges() + .first() + .expect("scan ranges always non-empty") + .block_range() + .start + } + + /// Returns the last known chain height to the wallet. + /// + /// Will panic if called before scan ranges are updated for the first time. + pub fn wallet_height(&self) -> BlockHeight { + self.scan_ranges() + .last() + .expect("scan ranges always non-empty") + .block_range() + .end + - 1 + } } impl Default for SyncState { diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index aa9c1dc32..8caa59edd 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -21,6 +21,7 @@ use crate::witness; use shardtree::store::ShardStore; use state::set_initial_state; use zcash_client_backend::proto::service::RawTransaction; +use zcash_client_backend::ShieldedProtocol; use zcash_client_backend::{ data_api::scanning::{ScanPriority, ScanRange}, proto::service::compact_tx_streamer_client::CompactTxStreamerClient, @@ -37,8 +38,6 @@ pub(crate) mod state; pub(crate) mod transparent; // TODO: move parameters to config module -// TODO; replace fixed batches with orchard shard ranges (block ranges containing all note commitments to an orchard shard or fragment of a shard) -const BATCH_SIZE: u32 = 5_000; const VERIFY_BLOCK_RANGE_SIZE: u32 = 10; const MAX_VERIFICATION_WINDOW: u32 = 100; // TODO: fail if re-org goes beyond this window @@ -115,7 +114,12 @@ where set_initial_state(wallet_guard.get_sync_state_mut().unwrap()); - update_subtree_roots(fetch_request_sender.clone(), &mut *wallet_guard).await; + update_subtree_roots( + consensus_parameters, + fetch_request_sender.clone(), + &mut *wallet_guard, + ) + .await; drop(wallet_guard); @@ -482,10 +486,11 @@ where } async fn update_subtree_roots( + consensus_parameters: &impl consensus::Parameters, fetch_request_sender: mpsc::UnboundedSender, wallet: &mut W, ) where - W: SyncShardTrees, + W: SyncWallet + SyncShardTrees, { let sapling_start_index = wallet .get_shard_trees() @@ -511,6 +516,20 @@ async fn update_subtree_roots( let sapling_subtree_roots = sapling_subtree_roots.unwrap(); let orchard_subtree_roots = orchard_subtree_roots.unwrap(); + let sync_state = wallet.get_sync_state_mut().unwrap(); + state::add_shard_ranges( + consensus_parameters, + ShieldedProtocol::Sapling, + sync_state, + &sapling_subtree_roots, + ); + state::add_shard_ranges( + consensus_parameters, + ShieldedProtocol::Orchard, + sync_state, + &orchard_subtree_roots, + ); + let shard_trees = wallet.get_shard_trees_mut().unwrap(); witness::add_subtree_roots(sapling_subtree_roots, shard_trees.sapling_mut()); witness::add_subtree_roots(orchard_subtree_roots, shard_trees.orchard_mut()); diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index d29eac6fb..434fdad33 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -2,7 +2,11 @@ use std::{cmp, collections::HashMap, ops::Range}; -use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange}; +use zcash_client_backend::{ + data_api::scanning::{ScanPriority, ScanRange}, + proto::service::SubtreeRoot, + ShieldedProtocol, +}; use zcash_primitives::{ consensus::{self, BlockHeight}, transaction::TxId, @@ -15,7 +19,7 @@ use crate::{ traits::{SyncBlocks, SyncWallet}, }; -use super::{BATCH_SIZE, VERIFY_BLOCK_RANGE_SIZE}; +use super::VERIFY_BLOCK_RANGE_SIZE; /// Used to determine which end of the scan range is verified. pub(super) enum VerifyEnd { @@ -159,8 +163,11 @@ pub(super) fn set_verify_scan_range( }, }; - let split_ranges = - split_out_scan_range(scan_range, &block_range_to_verify, ScanPriority::Verify); + let split_ranges = split_out_scan_range( + scan_range.clone(), + block_range_to_verify, + ScanPriority::Verify, + ); let scan_range_to_verify = match verify_end { VerifyEnd::VerifyHighest => split_ranges @@ -180,7 +187,7 @@ pub(super) fn set_verify_scan_range( scan_range_to_verify } -/// Punches in the shard block ranges surrounding each locator with `ScanPriority::FoundNote` (TODO). +/// Punches in the shard block ranges surrounding each locator with `ScanPriority::FoundNote`. fn set_found_note_scan_range(sync_state: &mut SyncState) -> Result<(), ()> { let locator_heights: Vec = sync_state .locators() @@ -189,7 +196,8 @@ fn set_found_note_scan_range(sync_state: &mut SyncState) -> Result<(), ()> { .collect(); locator_heights.into_iter().for_each(|block_height| { // TODO: add protocol info to locators to determine whether the orchard or sapling shard should be scanned - let block_range = determine_block_range(block_height); + let block_range = + determine_block_range(sync_state, block_height, ShieldedProtocol::Orchard); punch_scan_priority(sync_state, &block_range, ScanPriority::FoundNote).unwrap(); }); @@ -198,14 +206,21 @@ fn set_found_note_scan_range(sync_state: &mut SyncState) -> Result<(), ()> { /// Punches in the chain tip block range with `ScanPriority::ChainTip`. /// -/// Determines the chain tip block range by finding the lowest height of the latest completed shard for each shielded -/// protocol (TODO). +/// Determines the chain tip block range by finding the lowest start height of the latest incomplete shard for each +/// shielded protocol. fn set_chain_tip_scan_range( sync_state: &mut SyncState, chain_height: BlockHeight, ) -> Result<(), ()> { - // TODO: when batching by shards, determine the lowest height that covers both orchard and sapling incomplete shards - let chain_tip = determine_block_range(chain_height); + let sapling_incomplete_shard = + determine_block_range(sync_state, chain_height, ShieldedProtocol::Sapling); + let orchard_incomplete_shard = + determine_block_range(sync_state, chain_height, ShieldedProtocol::Orchard); + let chain_tip = if sapling_incomplete_shard.start < orchard_incomplete_shard.start { + sapling_incomplete_shard + } else { + orchard_incomplete_shard + }; punch_scan_priority(sync_state, &chain_tip, ScanPriority::ChainTip).unwrap(); @@ -277,7 +292,7 @@ fn punch_scan_priority( // split out the scan ranges in reverse order to maintain the correct index for lower scan ranges for (index, scan_range) in overlapping_scan_ranges.into_iter().rev() { - let split_ranges = split_out_scan_range(&scan_range, block_range, scan_priority); + let split_ranges = split_out_scan_range(scan_range, block_range.clone(), scan_priority); sync_state .scan_ranges_mut() .splice(index..=index, split_ranges); @@ -286,23 +301,56 @@ fn punch_scan_priority( Ok(()) } -/// Determines which range of blocks should be scanned for a given `block_height` -fn determine_block_range(block_height: BlockHeight) -> Range { - let start = block_height - (u32::from(block_height) % BATCH_SIZE); // TODO: will be replaced with first block of associated orchard shard - let end = start + BATCH_SIZE; // TODO: will be replaced with last block of associated orchard shard - Range { start, end } +/// Determines the block range which contains all the outputs for the shard of a given `shielded_protocol` surrounding +/// the specified `block_height`. +/// +/// If no shard range exists for the given `block_height`, return the range of the incomplete shard at the chain tip. +fn determine_block_range( + sync_state: &SyncState, + block_height: BlockHeight, + shielded_protocol: ShieldedProtocol, +) -> Range { + let shard_ranges = match shielded_protocol { + ShieldedProtocol::Sapling => sync_state.sapling_shard_ranges(), + ShieldedProtocol::Orchard => sync_state.sapling_shard_ranges(), + }; + + shard_ranges + .iter() + .find(|range| range.contains(&block_height)) + .map_or_else( + || { + let start = if let Some(range) = shard_ranges.last() { + range.end - 1 + } else { + sync_state.wallet_birthday() + }; + let end = sync_state.wallet_height() + 1; + + let range = Range { start, end }; + + if !range.contains(&block_height) { + panic!( + "block height should always be within the incomplete shard at chain tip when no complete shard range is found!" + ); + } + + range + }, + |range| range.clone(), + ) } -/// Takes a scan range and splits it at `block_range.start` and `block_range.end`, returning a vec of scan ranges where -/// the scan range with the specified `block_range` has the given `scan_priority`. +/// Takes a `scan_range` and splits it at `block_range.start` and `block_range.end`, returning a vec of scan ranges where +/// the scan range contained within the specified `block_range` has the given `scan_priority`. /// /// If `block_range` goes beyond the bounds of `scan_range.block_range()` no splitting will occur at the upper and/or -/// lower bound but the priority will still be updated +/// lower bound but the priority will still be updated. /// -/// Panics if no blocks in `block_range` are contained within `scan_range.block_range()` +/// Panics if no blocks in `block_range` are contained within `scan_range.block_range()`. fn split_out_scan_range( - scan_range: &ScanRange, - block_range: &Range, + scan_range: ScanRange, + block_range: Range, scan_priority: ScanPriority, ) -> Vec { let mut split_ranges = Vec::new(); @@ -351,50 +399,48 @@ fn split_out_scan_range( fn select_scan_range(sync_state: &mut SyncState) -> Option { let scan_ranges = sync_state.scan_ranges_mut(); - let mut scan_ranges_priority_sorted: Vec<&ScanRange> = scan_ranges.iter().collect(); - scan_ranges_priority_sorted.sort_by(|a, b| b.block_range().start.cmp(&a.block_range().start)); - scan_ranges_priority_sorted.sort_by_key(|scan_range| scan_range.priority()); - let highest_priority_scan_range = scan_ranges_priority_sorted + // scan ranges are sorted from lowest to highest priority + // scan ranges with the same priority are sorted in reverse block height order + // the highest priority scan range is the last in the list, the highest priority with lowest starting block height + let mut scan_ranges_priority_sorted: Vec<(usize, ScanRange)> = + scan_ranges.iter().cloned().enumerate().collect(); + scan_ranges_priority_sorted + .sort_by(|(_, a), (_, b)| b.block_range().start.cmp(&a.block_range().start)); + scan_ranges_priority_sorted.sort_by_key(|(_, scan_range)| scan_range.priority()); + let (index, highest_priority_scan_range) = scan_ranges_priority_sorted .pop() - .expect("scan ranges should be non-empty after setup") - .clone(); + .expect("scan ranges should be non-empty after pre-scan initialisation"); if highest_priority_scan_range.priority() == ScanPriority::Scanned || highest_priority_scan_range.priority() == ScanPriority::Ignored { return None; } - let (index, selected_scan_range) = scan_ranges - .iter_mut() - .enumerate() - .find(|(_, scan_range)| { - scan_range.block_range() == highest_priority_scan_range.block_range() - }) - .expect("scan range should exist"); - - // TODO: batch by shards - let batch_block_range = Range { - start: selected_scan_range.block_range().start, - end: selected_scan_range.block_range().start + BATCH_SIZE, - }; + let selected_priority = highest_priority_scan_range.priority(); + let shard_range = determine_block_range( + sync_state, + highest_priority_scan_range.block_range().start, + ShieldedProtocol::Orchard, + ); let split_ranges = split_out_scan_range( - selected_scan_range, - &batch_block_range, + highest_priority_scan_range, + shard_range, ScanPriority::Ignored, ); - - let trimmed_block_range = split_ranges + let selected_block_range = split_ranges .first() - .expect("vec should always be non-empty") + .expect("split ranges should always be non-empty") .block_range() .clone(); - scan_ranges.splice(index..=index, split_ranges); + sync_state + .scan_ranges_mut() + .splice(index..=index, split_ranges); // TODO: when this library has its own version of ScanRange this can be simplified and more readable Some(ScanRange::from_parts( - trimmed_block_range, - highest_priority_scan_range.priority(), + selected_block_range, + selected_priority, )) } @@ -442,3 +488,63 @@ pub(super) fn set_initial_state(sync_state: &mut SyncState) { let sync_start_height = sync_state.fully_scanned_height() + 1; sync_state.set_sync_start_height(sync_start_height); } + +/// Creates block ranges that contain all outputs for the shards associated with `subtree_roots` and adds the to +/// `sync_state`. +/// +/// The network upgrade activation height for the `shielded_protocol` is the first shard start height for the case +/// where shard ranges in `sync_state` are empty. +pub(super) fn add_shard_ranges( + consensus_parameters: &impl consensus::Parameters, + shielded_protocol: ShieldedProtocol, + sync_state: &mut SyncState, + subtree_roots: &[SubtreeRoot], +) { + let network_upgrade_activation_height = match shielded_protocol { + ShieldedProtocol::Sapling => consensus_parameters + .activation_height(consensus::NetworkUpgrade::Sapling) + .expect("activation height should exist for this network upgrade!"), + ShieldedProtocol::Orchard => consensus_parameters + .activation_height(consensus::NetworkUpgrade::Nu5) + .expect("activation height should exist for this network upgrade!"), + }; + + let shard_ranges = match shielded_protocol { + ShieldedProtocol::Sapling => sync_state.sapling_shard_ranges_mut(), + ShieldedProtocol::Orchard => sync_state.orchard_shard_ranges_mut(), + }; + + let highest_subtree_completing_height = if let Some(shard_range) = shard_ranges.last() { + shard_range.end - 1 + } else { + network_upgrade_activation_height + }; + + subtree_roots + .iter() + .map(|subtree_root| { + BlockHeight::from_u32( + subtree_root + .completing_block_height + .try_into() + .expect("overflow should never occur"), + ) + }) + .fold( + highest_subtree_completing_height, + |previous_subtree_completing_height, subtree_completing_height| { + shard_ranges.push(Range { + start: previous_subtree_completing_height, + end: subtree_completing_height + 1, + }); + + tracing::debug!( + "{:?} subtree root height: {}", + shielded_protocol, + subtree_completing_height + ); + + subtree_completing_height + }, + ); +} From c23b61770c3a52892f55fc4351bf74753020906f Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Sat, 28 Dec 2024 07:16:33 +0000 Subject: [PATCH 08/37] fixed punch scan priority --- zingo-sync/src/sync.rs | 16 +++++++------ zingo-sync/src/sync/state.rs | 46 ++++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 8caa59edd..e43921024 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -38,6 +38,8 @@ pub(crate) mod state; pub(crate) mod transparent; // TODO: move parameters to config module +// TODO; replace fixed batches with variable batches with fixed memory size +const BATCH_SIZE: u32 = 5_000; const VERIFY_BLOCK_RANGE_SIZE: u32 = 10; const MAX_VERIFICATION_WINDOW: u32 = 100; // TODO: fail if re-org goes beyond this window @@ -104,6 +106,13 @@ where ) .await; + update_subtree_roots( + consensus_parameters, + fetch_request_sender.clone(), + &mut *wallet_guard, + ) + .await; + state::update_scan_ranges( wallet_height, chain_height, @@ -114,13 +123,6 @@ where set_initial_state(wallet_guard.get_sync_state_mut().unwrap()); - update_subtree_roots( - consensus_parameters, - fetch_request_sender.clone(), - &mut *wallet_guard, - ) - .await; - drop(wallet_guard); // create channel for receiving scan results and launch scanner diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 434fdad33..f996b7bee 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -19,7 +19,7 @@ use crate::{ traits::{SyncBlocks, SyncWallet}, }; -use super::VERIFY_BLOCK_RANGE_SIZE; +use super::{BATCH_SIZE, VERIFY_BLOCK_RANGE_SIZE}; /// Used to determine which end of the scan range is verified. pub(super) enum VerifyEnd { @@ -198,7 +198,7 @@ fn set_found_note_scan_range(sync_state: &mut SyncState) -> Result<(), ()> { // TODO: add protocol info to locators to determine whether the orchard or sapling shard should be scanned let block_range = determine_block_range(sync_state, block_height, ShieldedProtocol::Orchard); - punch_scan_priority(sync_state, &block_range, ScanPriority::FoundNote).unwrap(); + punch_scan_priority(sync_state, block_range, ScanPriority::FoundNote).unwrap(); }); Ok(()) @@ -216,13 +216,14 @@ fn set_chain_tip_scan_range( determine_block_range(sync_state, chain_height, ShieldedProtocol::Sapling); let orchard_incomplete_shard = determine_block_range(sync_state, chain_height, ShieldedProtocol::Orchard); + let chain_tip = if sapling_incomplete_shard.start < orchard_incomplete_shard.start { sapling_incomplete_shard } else { orchard_incomplete_shard }; - punch_scan_priority(sync_state, &chain_tip, ScanPriority::ChainTip).unwrap(); + punch_scan_priority(sync_state, chain_tip, ScanPriority::ChainTip).unwrap(); Ok(()) } @@ -250,21 +251,22 @@ pub(super) fn set_scan_priority( Ok(()) } -/// Punches in a `scan_priority` for given `block_range`. +/// Punches in a `scan_priority` for a given `block_range`. /// /// This function will set all scan ranges in `sync_state` with block range boundaries contained by `block_range` to /// the given `scan_priority`. -/// Any scan ranges with `Ignored` (Scanning) or `Scanned` priority or with higher (or equal) priority than -/// `scan_priority` will be ignored. /// If any scan ranges in `sync_state` are found to overlap with the given `block_range`, they will be split at the /// boundary and the new scan ranges contained by `block_range` will be set to `scan_priority`. +/// Any scan ranges that fully contain the `block_range` will be split out with the given `scan_priority`. +/// Any scan ranges with `Ignored` (Scanning) or `Scanned` priority or with higher (or equal) priority than +/// `scan_priority` will be ignored. fn punch_scan_priority( sync_state: &mut SyncState, - block_range: &Range, + block_range: Range, scan_priority: ScanPriority, ) -> Result<(), ()> { - let mut fully_contained_scan_ranges = Vec::new(); - let mut overlapping_scan_ranges = Vec::new(); + let mut scan_ranges_contained_by_block_range = Vec::new(); + let mut scan_ranges_for_splitting = Vec::new(); for (index, scan_range) in sync_state.scan_ranges().iter().enumerate() { if scan_range.priority() == ScanPriority::Scanned @@ -277,21 +279,23 @@ fn punch_scan_priority( match ( block_range.contains(&scan_range.block_range().start), block_range.contains(&scan_range.block_range().end), + scan_range.block_range().contains(&block_range.start), ) { - (true, true) => fully_contained_scan_ranges.push(scan_range.clone()), - (true, false) | (false, true) => { - overlapping_scan_ranges.push((index, scan_range.clone())) + (true, true, _) => scan_ranges_contained_by_block_range.push(scan_range.clone()), + (true, false, _) | (false, true, _) => { + scan_ranges_for_splitting.push((index, scan_range.clone())) } - (false, false) => (), + (false, false, true) => scan_ranges_for_splitting.push((index, scan_range.clone())), + (false, false, false) => {} } } - for scan_range in fully_contained_scan_ranges { + for scan_range in scan_ranges_contained_by_block_range { set_scan_priority(sync_state, scan_range.block_range(), scan_priority).unwrap(); } // split out the scan ranges in reverse order to maintain the correct index for lower scan ranges - for (index, scan_range) in overlapping_scan_ranges.into_iter().rev() { + for (index, scan_range) in scan_ranges_for_splitting.into_iter().rev() { let split_ranges = split_out_scan_range(scan_range, block_range.clone(), scan_priority); sync_state .scan_ranges_mut() @@ -417,14 +421,14 @@ fn select_scan_range(sync_state: &mut SyncState) -> Option { } let selected_priority = highest_priority_scan_range.priority(); - let shard_range = determine_block_range( - sync_state, - highest_priority_scan_range.block_range().start, - ShieldedProtocol::Orchard, - ); + // TODO: fixed memory batching + let batch_block_range = Range { + start: highest_priority_scan_range.block_range().start, + end: highest_priority_scan_range.block_range().start + BATCH_SIZE, + }; let split_ranges = split_out_scan_range( highest_priority_scan_range, - shard_range, + batch_block_range, ScanPriority::Ignored, ); let selected_block_range = split_ranges From 628f9b41a90a5f110d878af3a6dfbabcb6cc6c38 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Sat, 28 Dec 2024 07:26:04 +0000 Subject: [PATCH 09/37] fix doc warnings --- zingo-sync/src/primitives.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index fdbf90951..aa0281d5b 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -38,12 +38,12 @@ pub struct SyncState { /// The block ranges that contain all sapling outputs of complete sapling shards. /// /// There is an edge case where a range may include two (or more) shards. However, this only occurs when the lower - /// shards are already scanned so the trimming will be handled by [`crate::sync::state::determine_block_range`]. + /// shards are already scanned so will cause no issues when punching in the higher scan priorites. sapling_shard_ranges: Vec>, /// The block ranges that contain all orchard outputs of complete orchard shards. /// /// There is an edge case where a range may include two (or more) shards. However, this only occurs when the lower - /// shards are already scanned so the trimming will be handled by [`crate::sync::state::determine_block_range`]. + /// shards are already scanned so will cause no issues when punching in the higher scan priorites. orchard_shard_ranges: Vec>, /// Locators for relevant transactions to the wallet. locators: BTreeSet, From 4a44cdf42125cb41e55a5ce31ebc3a52a66f8932 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 30 Dec 2024 02:56:59 +0000 Subject: [PATCH 10/37] fix whitespace --- zingo-sync/src/sync/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index f996b7bee..fd9ba8b47 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -497,7 +497,7 @@ pub(super) fn set_initial_state(sync_state: &mut SyncState) { /// `sync_state`. /// /// The network upgrade activation height for the `shielded_protocol` is the first shard start height for the case -/// where shard ranges in `sync_state` are empty. +/// where shard ranges in `sync_state` are empty. pub(super) fn add_shard_ranges( consensus_parameters: &impl consensus::Parameters, shielded_protocol: ShieldedProtocol, From 0d20cf12f99d51f8a18882d3614ddf9360978fb0 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 30 Dec 2024 05:39:06 +0000 Subject: [PATCH 11/37] implement logic for scanning sapling shard ranges on spend detection --- libtonode-tests/tests/sync.rs | 3 +- zingo-sync/src/primitives.rs | 25 ++++-------- zingo-sync/src/scan/transactions.rs | 7 ++-- zingo-sync/src/sync.rs | 3 +- zingo-sync/src/sync/spend.rs | 22 ++++++++++- zingo-sync/src/sync/state.rs | 59 ++++++++++++++--------------- 6 files changed, 63 insertions(+), 56 deletions(-) diff --git a/libtonode-tests/tests/sync.rs b/libtonode-tests/tests/sync.rs index aab6278bb..7fba16f88 100644 --- a/libtonode-tests/tests/sync.rs +++ b/libtonode-tests/tests/sync.rs @@ -74,7 +74,8 @@ async fn sync_status() { let lightclient = LightClient::create_from_wallet_base_async( WalletBase::from_string(HOSPITAL_MUSEUM_SEED.to_string()), &config, - 2_670_000, + 2_750_000, + // 2_670_000, true, ) .await diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index aa0281d5b..4fb48fc59 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -24,8 +24,8 @@ use crate::{ utils, }; -/// Block height and txid of relevant transactions that have yet to be scanned. These may be added due to spend -/// detections or transparent output discovery. +/// Block height and txid of relevant transactions that have yet to be scanned. These may be added due transparent +/// output/spend discovery or for targetted rescan. pub type Locator = (BlockHeight, TxId); /// Encapsulates the current state of sync @@ -98,27 +98,18 @@ impl SyncState { } } - /// Returns the wallet birthday. - /// - /// Will panic if called before scan ranges are updated for the first time. - pub fn wallet_birthday(&self) -> BlockHeight { + /// Returns the wallet birthday or `None` if `self.scan_ranges` is empty. + pub fn wallet_birthday(&self) -> Option { self.scan_ranges() .first() - .expect("scan ranges always non-empty") - .block_range() - .start + .map(|range| range.block_range().start) } - /// Returns the last known chain height to the wallet. - /// - /// Will panic if called before scan ranges are updated for the first time. - pub fn wallet_height(&self) -> BlockHeight { + /// Returns the last known chain height to the wallet or `None` if `self.scan_ranges` is empty. + pub fn wallet_height(&self) -> Option { self.scan_ranges() .last() - .expect("scan ranges always non-empty") - .block_range() - .end - - 1 + .map(|range| range.block_range().end - 1) } } diff --git a/zingo-sync/src/scan/transactions.rs b/zingo-sync/src/scan/transactions.rs index 4770aa029..09f6df422 100644 --- a/zingo-sync/src/scan/transactions.rs +++ b/zingo-sync/src/scan/transactions.rs @@ -519,9 +519,6 @@ fn collect_outpoints( fetch_request_sender: mpsc::UnboundedSender, consensus_parameters: &P, @@ -545,10 +542,12 @@ where } spending_txids.insert(txid); + // TODO: fetch block from server if not in wallet so wallet blocks added to wallet while scanning out of order + // don't need to be held in memory wallet_blocks.insert( block_height, wallet.get_wallet_block(block_height).expect( - "wallet block should be in the wallet due to the context of this functions purpose", + "wallet block should be in the wallet as nullifiers are mapped during scanning", ), ); } diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index e43921024..acc63a1e9 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -19,7 +19,6 @@ use crate::traits::{ use crate::witness; use shardtree::store::ShardStore; -use state::set_initial_state; use zcash_client_backend::proto::service::RawTransaction; use zcash_client_backend::ShieldedProtocol; use zcash_client_backend::{ @@ -121,7 +120,7 @@ where .await .unwrap(); - set_initial_state(wallet_guard.get_sync_state_mut().unwrap()); + state::set_initial_state(wallet_guard.get_sync_state_mut().unwrap()); drop(wallet_guard); diff --git a/zingo-sync/src/sync/spend.rs b/zingo-sync/src/sync/spend.rs index debc4572b..0fc6b23b8 100644 --- a/zingo-sync/src/sync/spend.rs +++ b/zingo-sync/src/sync/spend.rs @@ -3,6 +3,7 @@ use std::collections::{BTreeMap, HashMap}; use tokio::sync::mpsc; +use zcash_client_backend::ShieldedProtocol; use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::{ consensus::{self, BlockHeight}, @@ -17,11 +18,14 @@ use crate::{ traits::{SyncBlocks, SyncNullifiers, SyncOutPoints, SyncTransactions}, }; +use super::state; + /// Helper function for handling spend detection and the spend status of notes. /// -/// Locates any nullifiers of notes in the wallet's transactions which match a nullifier in the wallet's nullifier map. +/// Detects if any derived nullifiers of notes in the wallet's transactions match a nullifier in the wallet's nullifier map. /// If a spend is detected, the nullifier is removed from the nullifier map and added to the map of spend locators. -/// The spend locators are then used to fetch and scan the transactions with detected spends. +/// The spend locators are used to set the surrounding shard block ranges to be prioritised for scanning and then to +/// fetch and scan the transactions with detected spends in the case that they evaded trial decryption. /// Finally, all notes that were detected as spent are updated with the located spending transaction. pub(super) async fn update_shielded_spends( consensus_parameters: &P, @@ -42,6 +46,20 @@ where orchard_derived_nullifiers, ); + let sync_state = wallet.get_sync_state_mut().unwrap(); + state::set_found_note_scan_range( + sync_state, + ShieldedProtocol::Sapling, + sapling_spend_locators.values().cloned(), + ) + .unwrap(); + state::set_found_note_scan_range( + sync_state, + ShieldedProtocol::Orchard, + orchard_spend_locators.values().cloned(), + ) + .unwrap(); + // in the edge case where a spending transaction received no change, scan the transactions that evaded trial decryption scan_located_transactions( fetch_request_sender, diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index fd9ba8b47..5df4c369c 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -38,21 +38,20 @@ where P: consensus::Parameters, W: SyncWallet, { - let wallet_height = - if let Some(highest_range) = wallet.get_sync_state().unwrap().scan_ranges().last() { - highest_range.block_range().end - 1 - } else { - let wallet_birthday = wallet.get_birthday().unwrap(); - let sapling_activation_height = consensus_parameters - .activation_height(consensus::NetworkUpgrade::Sapling) - .expect("sapling activation height should always return Some"); - - let highest = match wallet_birthday.cmp(&sapling_activation_height) { - cmp::Ordering::Greater | cmp::Ordering::Equal => wallet_birthday, - cmp::Ordering::Less => sapling_activation_height, - }; - highest - 1 + let wallet_height = if let Some(height) = wallet.get_sync_state().unwrap().wallet_height() { + height + } else { + let wallet_birthday = wallet.get_birthday().unwrap(); + let sapling_activation_height = consensus_parameters + .activation_height(consensus::NetworkUpgrade::Sapling) + .expect("sapling activation height should always return Some"); + + let highest = match wallet_birthday.cmp(&sapling_activation_height) { + cmp::Ordering::Greater | cmp::Ordering::Equal => wallet_birthday, + cmp::Ordering::Less => sapling_activation_height, }; + highest - 1 + }; Ok(wallet_height) } @@ -80,7 +79,8 @@ pub(super) async fn update_scan_ranges( ) -> Result<(), ()> { reset_scan_ranges(sync_state)?; create_scan_range(wallet_height, chain_height, sync_state).await?; - set_found_note_scan_range(sync_state)?; + let locators = sync_state.locators().clone(); + set_found_note_scan_range(sync_state, ShieldedProtocol::Orchard, locators.into_iter())?; set_chain_tip_scan_range(sync_state, chain_height)?; // TODO: add logic to merge scan ranges @@ -187,19 +187,18 @@ pub(super) fn set_verify_scan_range( scan_range_to_verify } -/// Punches in the shard block ranges surrounding each locator with `ScanPriority::FoundNote`. -fn set_found_note_scan_range(sync_state: &mut SyncState) -> Result<(), ()> { - let locator_heights: Vec = sync_state - .locators() - .iter() - .map(|(block_height, _)| *block_height) - .collect(); - locator_heights.into_iter().for_each(|block_height| { - // TODO: add protocol info to locators to determine whether the orchard or sapling shard should be scanned - let block_range = - determine_block_range(sync_state, block_height, ShieldedProtocol::Orchard); - punch_scan_priority(sync_state, block_range, ScanPriority::FoundNote).unwrap(); - }); +/// Punches in the `shielded_protocol` shard block ranges surrounding each locator with `ScanPriority::FoundNote`. +pub(super) fn set_found_note_scan_range>( + sync_state: &mut SyncState, + shielded_protocol: ShieldedProtocol, + locators: L, +) -> Result<(), ()> { + locators + .map(|(block_height, _)| block_height) + .for_each(|block_height| { + let block_range = determine_block_range(sync_state, block_height, shielded_protocol); + punch_scan_priority(sync_state, block_range, ScanPriority::FoundNote).unwrap(); + }); Ok(()) } @@ -327,9 +326,9 @@ fn determine_block_range( let start = if let Some(range) = shard_ranges.last() { range.end - 1 } else { - sync_state.wallet_birthday() + sync_state.wallet_birthday().expect("scan range should not be empty") }; - let end = sync_state.wallet_height() + 1; + let end = sync_state.wallet_height().expect("scan range should not be empty") + 1; let range = Range { start, end }; From f74ba3a4036660fd1a579f687022f1097e1de074 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 30 Dec 2024 11:34:17 +0000 Subject: [PATCH 12/37] set found note shard priorities --- zingo-sync/src/scan.rs | 18 ++++---- zingo-sync/src/scan/compact_blocks.rs | 15 +++---- zingo-sync/src/scan/task.rs | 6 +-- zingo-sync/src/scan/transactions.rs | 19 +++++---- zingo-sync/src/sync.rs | 11 ++++- zingo-sync/src/sync/spend.rs | 4 +- zingo-sync/src/sync/state.rs | 60 ++++++++++++++++++++++----- 7 files changed, 92 insertions(+), 41 deletions(-) diff --git a/zingo-sync/src/scan.rs b/zingo-sync/src/scan.rs index 1a3c8e4b4..8dd09a855 100644 --- a/zingo-sync/src/scan.rs +++ b/zingo-sync/src/scan.rs @@ -1,6 +1,6 @@ use std::{ cmp, - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet, HashMap}, }; use orchard::tree::MerkleHashOrchard; @@ -115,7 +115,7 @@ impl InitialScanData { struct ScanData { nullifiers: NullifierMap, wallet_blocks: BTreeMap, - relevant_txids: HashSet, + decrypted_locators: BTreeSet, decrypted_note_data: DecryptedNoteData, witness_data: WitnessData, } @@ -144,14 +144,18 @@ impl DecryptedNoteData { } /// Scans a given range and returns all data relevant to the specified keys. +/// /// `previous_wallet_block` is the wallet block with height [scan_range.start - 1]. +/// `locators` are the block height and txid of transactions in the `scan_range` that are known to be relevant to the +/// wallet and are appended to during scanning if trial decryption succeeds. If there are no known relevant transctions +/// then `locators` will start empty. pub(crate) async fn scan

( fetch_request_sender: mpsc::UnboundedSender, parameters: &P, ufvks: &HashMap, scan_range: ScanRange, previous_wallet_block: Option, - locators: Vec, + mut locators: BTreeSet, transparent_addresses: HashMap, ) -> Result where @@ -191,21 +195,19 @@ where let ScanData { nullifiers, wallet_blocks, - mut relevant_txids, + mut decrypted_locators, decrypted_note_data, witness_data, } = scan_data; - locators.into_iter().map(|(_, txid)| txid).for_each(|txid| { - relevant_txids.insert(txid); - }); + locators.append(&mut decrypted_locators); let mut outpoints = OutPointMap::new(); let wallet_transactions = scan_transactions( fetch_request_sender, parameters, ufvks, - relevant_txids, + locators, decrypted_note_data, &wallet_blocks, &mut outpoints, diff --git a/zingo-sync/src/scan/compact_blocks.rs b/zingo-sync/src/scan/compact_blocks.rs index 2d724c394..b1dd11893 100644 --- a/zingo-sync/src/scan/compact_blocks.rs +++ b/zingo-sync/src/scan/compact_blocks.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use incrementalmerkletree::{Marking, Position, Retention}; use orchard::{note_encryption::CompactAction, tree::MerkleHashOrchard}; @@ -11,7 +11,6 @@ use zcash_note_encryption::Domain; use zcash_primitives::{ block::BlockHash, consensus::{BlockHeight, Parameters}, - transaction::TxId, zip32::AccountId, }; @@ -31,7 +30,7 @@ use super::{ mod runners; // TODO: move parameters to config module -const TRIAL_DECRYPT_TASK_SIZE: usize = 500; +const TRIAL_DECRYPT_TASK_SIZE: usize = 1_000; pub(crate) fn scan_compact_blocks

( compact_blocks: Vec, @@ -49,7 +48,7 @@ where let mut wallet_blocks: BTreeMap = BTreeMap::new(); let mut nullifiers = NullifierMap::new(); - let mut relevant_txids: HashSet = HashSet::new(); + let mut decrypted_locators = BTreeSet::new(); let mut decrypted_note_data = DecryptedNoteData::new(); let mut witness_data = WitnessData::new( Position::from(u64::from(initial_scan_data.sapling_initial_tree_size)), @@ -58,6 +57,8 @@ where let mut sapling_tree_size = initial_scan_data.sapling_initial_tree_size; let mut orchard_tree_size = initial_scan_data.orchard_initial_tree_size; for block in &compact_blocks { + let block_height = + BlockHeight::from_u32(block.height.try_into().expect("should never overflow")); let mut transactions = block.vtx.iter().peekable(); while let Some(transaction) = transactions.next() { // collect trial decryption results by transaction @@ -72,10 +73,10 @@ where // the edge case of transactions that this capability created but did not receive change // or create outgoing data is handled when the nullifiers are added and linked incoming_sapling_outputs.iter().for_each(|(output_id, _)| { - relevant_txids.insert(output_id.txid()); + decrypted_locators.insert((block_height, output_id.txid())); }); incoming_orchard_outputs.iter().for_each(|(output_id, _)| { - relevant_txids.insert(output_id.txid()); + decrypted_locators.insert((block_height, output_id.txid())); }); collect_nullifiers(&mut nullifiers, block.height(), transaction).unwrap(); @@ -136,7 +137,7 @@ where Ok(ScanData { nullifiers, wallet_blocks, - relevant_txids, + decrypted_locators, decrypted_note_data, witness_data, }) diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index e4d6f8866..120c808ab 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashMap, + collections::{BTreeSet, HashMap}, sync::{ atomic::{self, AtomicBool}, Arc, @@ -318,7 +318,7 @@ where pub(crate) struct ScanTask { scan_range: ScanRange, previous_wallet_block: Option, - locators: Vec, + locators: BTreeSet, transparent_addresses: HashMap, } @@ -326,7 +326,7 @@ impl ScanTask { pub(crate) fn from_parts( scan_range: ScanRange, previous_wallet_block: Option, - locators: Vec, + locators: BTreeSet, transparent_addresses: HashMap, ) -> Self { Self { diff --git a/zingo-sync/src/scan/transactions.rs b/zingo-sync/src/scan/transactions.rs index 09f6df422..1f4c2dcf2 100644 --- a/zingo-sync/src/scan/transactions.rs +++ b/zingo-sync/src/scan/transactions.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use incrementalmerkletree::Position; use orchard::{ @@ -71,15 +71,15 @@ pub(crate) async fn scan_transactions( fetch_request_sender: mpsc::UnboundedSender, consensus_parameters: &P, ufvks: &HashMap, - relevant_txids: HashSet, + locators: BTreeSet, decrypted_note_data: DecryptedNoteData, wallet_blocks: &BTreeMap, outpoint_map: &mut OutPointMap, transparent_addresses: HashMap, ) -> Result, ()> { - let mut wallet_transactions = HashMap::with_capacity(relevant_txids.len()); + let mut wallet_transactions = HashMap::with_capacity(locators.len()); - for txid in relevant_txids { + for (_, txid) in locators { let (transaction, block_height) = client::get_transaction_and_block_height(fetch_request_sender.clone(), txid) .await @@ -533,15 +533,18 @@ where { let wallet_transactions = wallet.get_wallet_transactions().unwrap(); let wallet_txids = wallet_transactions.keys().copied().collect::>(); - let mut spending_txids = HashSet::new(); + let mut spending_locators = BTreeSet::new(); let mut wallet_blocks = BTreeMap::new(); - for (block_height, txid) in locators { + for locator in locators { + let block_height = locator.0; + let txid = locator.1; + // skip if transaction already exists in the wallet if wallet_txids.contains(&txid) { continue; } - spending_txids.insert(txid); + spending_locators.insert(locator); // TODO: fetch block from server if not in wallet so wallet blocks added to wallet while scanning out of order // don't need to be held in memory wallet_blocks.insert( @@ -557,7 +560,7 @@ where fetch_request_sender, consensus_parameters, ufvks, - spending_txids, + spending_locators, DecryptedNoteData::new(), &wallet_blocks, &mut outpoint_map, diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index acc63a1e9..f9d79c398 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -8,7 +8,7 @@ use std::time::Duration; use crate::client::{self, FetchRequest}; use crate::error::SyncError; use crate::keys::transparent::TransparentAddressId; -use crate::primitives::{NullifierMap, OutPointMap, SyncState, SyncStatus}; +use crate::primitives::{NullifierMap, OutPointMap, SyncState, SyncStatus, WalletTransaction}; use crate::scan::error::{ContinuityError, ScanError}; use crate::scan::task::Scanner; use crate::scan::transactions::scan_transaction; @@ -36,9 +36,10 @@ pub(crate) mod spend; pub(crate) mod state; pub(crate) mod transparent; +// TODO: a single block height can be associated with two shards of a single protocol! // TODO: move parameters to config module // TODO; replace fixed batches with variable batches with fixed memory size -const BATCH_SIZE: u32 = 5_000; +const BATCH_SIZE: u32 = 10_000; const VERIFY_BLOCK_RANGE_SIZE: u32 = 10; const MAX_VERIFICATION_WINDOW: u32 = 100; // TODO: fail if re-org goes beyond this window @@ -429,6 +430,12 @@ where orchard_located_trees, } = scan_results; + let sync_state = wallet.get_sync_state_mut().unwrap(); + for transaction in wallet_transactions.values() { + state::update_found_note_shard_priority(sync_state, ShieldedProtocol::Sapling, transaction); + state::update_found_note_shard_priority(sync_state, ShieldedProtocol::Orchard, transaction); + } + wallet.append_wallet_blocks(wallet_blocks).unwrap(); wallet .extend_wallet_transactions(wallet_transactions) diff --git a/zingo-sync/src/sync/spend.rs b/zingo-sync/src/sync/spend.rs index 0fc6b23b8..44925b559 100644 --- a/zingo-sync/src/sync/spend.rs +++ b/zingo-sync/src/sync/spend.rs @@ -47,13 +47,13 @@ where ); let sync_state = wallet.get_sync_state_mut().unwrap(); - state::set_found_note_scan_range( + state::set_found_note_scan_ranges( sync_state, ShieldedProtocol::Sapling, sapling_spend_locators.values().cloned(), ) .unwrap(); - state::set_found_note_scan_range( + state::set_found_note_scan_ranges( sync_state, ShieldedProtocol::Orchard, orchard_spend_locators.values().cloned(), diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 5df4c369c..1e56a808c 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -1,6 +1,10 @@ //! Module for reading and updating the fields of [`crate::primitives::SyncState`] which tracks the wallet's state of sync. -use std::{cmp, collections::HashMap, ops::Range}; +use std::{ + cmp, + collections::{BTreeSet, HashMap}, + ops::Range, +}; use zcash_client_backend::{ data_api::scanning::{ScanPriority, ScanRange}, @@ -14,7 +18,7 @@ use zcash_primitives::{ use crate::{ keys::transparent::TransparentAddressId, - primitives::{Locator, SyncState}, + primitives::{Locator, SyncState, WalletTransaction}, scan::task::ScanTask, traits::{SyncBlocks, SyncWallet}, }; @@ -58,7 +62,7 @@ where /// Returns the locators for a given `block_range` from the wallet's [`crate::primitives::SyncState`] // TODO: unit test high priority -fn find_locators(sync_state: &SyncState, block_range: &Range) -> Vec { +fn find_locators(sync_state: &SyncState, block_range: &Range) -> BTreeSet { sync_state .locators() .range( @@ -80,7 +84,7 @@ pub(super) async fn update_scan_ranges( reset_scan_ranges(sync_state)?; create_scan_range(wallet_height, chain_height, sync_state).await?; let locators = sync_state.locators().clone(); - set_found_note_scan_range(sync_state, ShieldedProtocol::Orchard, locators.into_iter())?; + set_found_note_scan_ranges(sync_state, ShieldedProtocol::Orchard, locators.into_iter())?; set_chain_tip_scan_range(sync_state, chain_height)?; // TODO: add logic to merge scan ranges @@ -188,17 +192,27 @@ pub(super) fn set_verify_scan_range( } /// Punches in the `shielded_protocol` shard block ranges surrounding each locator with `ScanPriority::FoundNote`. -pub(super) fn set_found_note_scan_range>( +pub(super) fn set_found_note_scan_ranges>( sync_state: &mut SyncState, shielded_protocol: ShieldedProtocol, locators: L, ) -> Result<(), ()> { - locators - .map(|(block_height, _)| block_height) - .for_each(|block_height| { - let block_range = determine_block_range(sync_state, block_height, shielded_protocol); - punch_scan_priority(sync_state, block_range, ScanPriority::FoundNote).unwrap(); - }); + for locator in locators { + set_found_note_scan_range(sync_state, shielded_protocol, locator)?; + } + + Ok(()) +} + +/// Punches in the `shielded_protocol` shard block range surrounding the `locator` with `ScanPriority::FoundNote`. +pub(super) fn set_found_note_scan_range( + sync_state: &mut SyncState, + shielded_protocol: ShieldedProtocol, + locator: Locator, +) -> Result<(), ()> { + let block_height = locator.0; + let block_range = determine_block_range(sync_state, block_height, shielded_protocol); + punch_scan_priority(sync_state, block_range, ScanPriority::FoundNote).unwrap(); Ok(()) } @@ -551,3 +565,27 @@ pub(super) fn add_shard_ranges( }, ); } + +/// Updates the `shielded_protocol` shard range to `FoundNote` scan priority if the `wallet_transaction` contains +/// a note from the corresponding `shielded_protocol`. +pub(super) fn update_found_note_shard_priority<'a>( + sync_state: &mut SyncState, + shielded_protocol: ShieldedProtocol, + wallet_transaction: &'a WalletTransaction, +) { + let found_note = match shielded_protocol { + ShieldedProtocol::Sapling => !wallet_transaction.sapling_notes().is_empty(), + ShieldedProtocol::Orchard => !wallet_transaction.orchard_notes().is_empty(), + }; + if found_note { + set_found_note_scan_range( + sync_state, + shielded_protocol, + ( + wallet_transaction.confirmation_status().get_height(), + wallet_transaction.txid(), + ), + ) + .unwrap(); + } +} From 2c4c522714a0929cb12eaa2526814a4d66ba5072 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 30 Dec 2024 12:00:09 +0000 Subject: [PATCH 13/37] add logic for shard boundaries --- zingo-sync/src/sync.rs | 11 ++++-- zingo-sync/src/sync/state.rs | 67 ++++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index f9d79c398..3cb3ca7dd 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -8,7 +8,7 @@ use std::time::Duration; use crate::client::{self, FetchRequest}; use crate::error::SyncError; use crate::keys::transparent::TransparentAddressId; -use crate::primitives::{NullifierMap, OutPointMap, SyncState, SyncStatus, WalletTransaction}; +use crate::primitives::{NullifierMap, OutPointMap, SyncState, SyncStatus}; use crate::scan::error::{ContinuityError, ScanError}; use crate::scan::task::Scanner; use crate::scan::transactions::scan_transaction; @@ -36,7 +36,6 @@ pub(crate) mod spend; pub(crate) mod state; pub(crate) mod transparent; -// TODO: a single block height can be associated with two shards of a single protocol! // TODO: move parameters to config module // TODO; replace fixed batches with variable batches with fixed memory size const BATCH_SIZE: u32 = 10_000; @@ -113,6 +112,14 @@ where ) .await; + tracing::info!( + "SHARD RANGES: {:#?}", + wallet_guard + .get_sync_state() + .unwrap() + .orchard_shard_ranges() + ); + state::update_scan_ranges( wallet_height, chain_height, diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 1e56a808c..6aa6cfa40 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -318,10 +318,11 @@ fn punch_scan_priority( Ok(()) } -/// Determines the block range which contains all the outputs for the shard of a given `shielded_protocol` surrounding +/// Determines the block range which contains all the note commitments for the shard of a given `shielded_protocol` surrounding /// the specified `block_height`. /// /// If no shard range exists for the given `block_height`, return the range of the incomplete shard at the chain tip. +/// If `block_height` contains note commitments from multiple shards, return the block range of all of those shards combined. fn determine_block_range( sync_state: &SyncState, block_height: BlockHeight, @@ -329,33 +330,49 @@ fn determine_block_range( ) -> Range { let shard_ranges = match shielded_protocol { ShieldedProtocol::Sapling => sync_state.sapling_shard_ranges(), - ShieldedProtocol::Orchard => sync_state.sapling_shard_ranges(), + ShieldedProtocol::Orchard => sync_state.orchard_shard_ranges(), }; - shard_ranges + let target_ranges = shard_ranges .iter() - .find(|range| range.contains(&block_height)) - .map_or_else( - || { - let start = if let Some(range) = shard_ranges.last() { - range.end - 1 - } else { - sync_state.wallet_birthday().expect("scan range should not be empty") - }; - let end = sync_state.wallet_height().expect("scan range should not be empty") + 1; - - let range = Range { start, end }; - - if !range.contains(&block_height) { - panic!( - "block height should always be within the incomplete shard at chain tip when no complete shard range is found!" - ); - } - - range - }, - |range| range.clone(), - ) + .filter(|range| range.contains(&block_height)) + .cloned() + .collect::>(); + + if target_ranges.is_empty() { + let start = if let Some(range) = shard_ranges.last() { + range.end - 1 + } else { + sync_state + .wallet_birthday() + .expect("scan range should not be empty") + }; + let end = sync_state + .wallet_height() + .expect("scan range should not be empty") + + 1; + + let range = Range { start, end }; + + if !range.contains(&block_height) { + panic!( + "block height should always be within the incomplete shard at chain tip when no complete shard range is found!" + ); + } + + range + } else { + Range { + start: target_ranges + .first() + .expect("should not be empty in this closure") + .start, + end: target_ranges + .last() + .expect("should not be empty in this closure") + .end, + } + } } /// Takes a `scan_range` and splits it at `block_range.start` and `block_range.end`, returning a vec of scan ranges where From 072430ca95d1477948fbc03ae9f1239d3e8ad40b Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 30 Dec 2024 12:01:54 +0000 Subject: [PATCH 14/37] fix clippy --- zingo-sync/src/sync/state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 6aa6cfa40..1486d3a34 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -585,10 +585,10 @@ pub(super) fn add_shard_ranges( /// Updates the `shielded_protocol` shard range to `FoundNote` scan priority if the `wallet_transaction` contains /// a note from the corresponding `shielded_protocol`. -pub(super) fn update_found_note_shard_priority<'a>( +pub(super) fn update_found_note_shard_priority( sync_state: &mut SyncState, shielded_protocol: ShieldedProtocol, - wallet_transaction: &'a WalletTransaction, + wallet_transaction: &WalletTransaction, ) { let found_note = match shielded_protocol { ShieldedProtocol::Sapling => !wallet_transaction.sapling_notes().is_empty(), From 28c7c181346f2173d41c3ba1392232d14554794d Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Mon, 30 Dec 2024 12:12:43 +0000 Subject: [PATCH 15/37] remove debug --- zingo-sync/src/sync.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 3cb3ca7dd..60ed542c6 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -112,14 +112,6 @@ where ) .await; - tracing::info!( - "SHARD RANGES: {:#?}", - wallet_guard - .get_sync_state() - .unwrap() - .orchard_shard_ranges() - ); - state::update_scan_ranges( wallet_height, chain_height, From fec01edf3584630fc6dca19c1d038dc88f42880a Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 06:47:32 +0000 Subject: [PATCH 16/37] fix reorg bug and add end seam block for improved continuity checks --- zingo-sync/src/primitives.rs | 21 ++++++++++++++ zingo-sync/src/scan.rs | 19 ++++++++----- zingo-sync/src/scan/compact_blocks.rs | 41 +++++++++++++++++++++------ zingo-sync/src/scan/task.rs | 14 +++++---- zingo-sync/src/sync/state.rs | 14 +++++---- 5 files changed, 84 insertions(+), 25 deletions(-) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index 4fb48fc59..207d611c2 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -98,7 +98,28 @@ impl SyncState { } } + /// Returns the highest block height that has been scanned. + /// + /// If no scan ranges have been scanned, returns the block below the wallet birthday. + /// Will panic if called before scan ranges are updated for the first time. + pub fn highest_scanned_height(&self) -> BlockHeight { + if let Some(last_scanned_range) = self + .scan_ranges() + .iter() + .filter(|scan_range| scan_range.priority() == ScanPriority::Scanned) + .last() + { + last_scanned_range.block_range().end - 1 + } else { + self.wallet_birthday() + .expect("scan ranges always non-empty") + - 1 + } + } + /// Returns the wallet birthday or `None` if `self.scan_ranges` is empty. + /// + /// If the wallet birthday is below the sapling activation height, returns the sapling activation height instead. pub fn wallet_birthday(&self) -> Option { self.scan_ranges() .first() diff --git a/zingo-sync/src/scan.rs b/zingo-sync/src/scan.rs index 8dd09a855..12c8313c8 100644 --- a/zingo-sync/src/scan.rs +++ b/zingo-sync/src/scan.rs @@ -32,7 +32,8 @@ pub(crate) mod task; pub(crate) mod transactions; struct InitialScanData { - previous_block: Option, + start_seam_block: Option, + end_seam_block: Option, sapling_initial_tree_size: u32, orchard_initial_tree_size: u32, } @@ -42,7 +43,8 @@ impl InitialScanData { fetch_request_sender: mpsc::UnboundedSender, consensus_parameters: &P, first_block: &CompactBlock, - previous_wallet_block: Option, + start_seam_block: Option, + end_seam_block: Option, ) -> Result where P: Parameters + Sync + Send + 'static, @@ -51,7 +53,7 @@ impl InitialScanData { // otherwise, from first block if available // otherwise, fetches frontiers from server let (sapling_initial_tree_size, orchard_initial_tree_size) = - if let Some(prev) = &previous_wallet_block { + if let Some(prev) = &start_seam_block { ( prev.sapling_commitment_tree_size(), prev.orchard_commitment_tree_size(), @@ -105,7 +107,8 @@ impl InitialScanData { }; Ok(InitialScanData { - previous_block: previous_wallet_block, + start_seam_block, + end_seam_block, sapling_initial_tree_size, orchard_initial_tree_size, }) @@ -145,7 +148,7 @@ impl DecryptedNoteData { /// Scans a given range and returns all data relevant to the specified keys. /// -/// `previous_wallet_block` is the wallet block with height [scan_range.start - 1]. +/// `start_seam_block` and `end_seam_block` are the blocks adjacent to the `scan_range` for verification of continuity. /// `locators` are the block height and txid of transactions in the `scan_range` that are known to be relevant to the /// wallet and are appended to during scanning if trial decryption succeeds. If there are no known relevant transctions /// then `locators` will start empty. @@ -154,7 +157,8 @@ pub(crate) async fn scan

( parameters: &P, ufvks: &HashMap, scan_range: ScanRange, - previous_wallet_block: Option, + start_seam_block: Option, + end_seam_block: Option, mut locators: BTreeSet, transparent_addresses: HashMap, ) -> Result @@ -174,7 +178,8 @@ where compact_blocks .first() .expect("compacts blocks should not be empty"), - previous_wallet_block, + start_seam_block, + end_seam_block, ) .await .unwrap(); diff --git a/zingo-sync/src/scan/compact_blocks.rs b/zingo-sync/src/scan/compact_blocks.rs index b1dd11893..568bfa220 100644 --- a/zingo-sync/src/scan/compact_blocks.rs +++ b/zingo-sync/src/scan/compact_blocks.rs @@ -41,7 +41,11 @@ pub(crate) fn scan_compact_blocks

( where P: Parameters + Sync + Send + 'static, { - check_continuity(&compact_blocks, initial_scan_data.previous_block.as_ref())?; + check_continuity( + &compact_blocks, + initial_scan_data.start_seam_block.as_ref(), + initial_scan_data.end_seam_block.as_ref(), + )?; let scanning_keys = ScanningKeys::from_account_ufvks(ufvks.clone()); let mut runners = trial_decrypt(parameters, &scanning_keys, &compact_blocks).unwrap(); @@ -160,19 +164,21 @@ where Ok(runners) } -// checks height and hash continuity of a batch of compact blocks. -// takes the last wallet compact block of the adjacent lower scan range, if available. -// TODO: remove option and revisit scanner flow to use the last block of previously scanned batch to check continuity +/// Checks height and hash continuity of a batch of compact blocks. +/// +/// If available, also checks continuity with the blocks adjacent to the `compact_blocks` forming the start and end +/// seams of the scan ranges. fn check_continuity( compact_blocks: &[CompactBlock], - previous_compact_block: Option<&WalletBlock>, + start_seam_block: Option<&WalletBlock>, + end_seam_block: Option<&WalletBlock>, ) -> Result<(), ContinuityError> { let mut prev_height: Option = None; let mut prev_hash: Option = None; - if let Some(prev) = previous_compact_block { - prev_height = Some(prev.block_height()); - prev_hash = Some(prev.block_hash()); + if let Some(start_seam_block) = start_seam_block { + prev_height = Some(start_seam_block.block_height()); + prev_hash = Some(start_seam_block.block_hash()); } for block in compact_blocks { @@ -199,6 +205,25 @@ fn check_continuity( prev_hash = Some(block.hash()); } + if let Some(end_seam_block) = end_seam_block { + let prev_height = prev_height.expect("compact blocks should not be empty"); + if end_seam_block.block_height() != prev_height + 1 { + return Err(ContinuityError::HeightDiscontinuity { + height: end_seam_block.block_height(), + previous_block_height: prev_height, + }); + } + + let prev_hash = prev_hash.expect("compact blocks should not be empty"); + if end_seam_block.prev_hash() != prev_hash { + return Err(ContinuityError::HashDiscontinuity { + height: end_seam_block.block_height(), + prev_hash: end_seam_block.prev_hash(), + previous_block_hash: prev_hash, + }); + } + } + Ok(()) } diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index 120c808ab..26e3362f0 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -139,7 +139,7 @@ where /// Updates the scanner. /// - /// If verification is still in progress, do not create scan tasks. + /// If verification is still in progress, only create scan tasks with `Verify` scan priority. /// If there is an idle worker, create a new scan task and add to worker. /// If there are no more range available to scan, shutdown the idle workers. pub(crate) async fn update(&mut self, wallet: &mut W, shutdown_mempool: Arc) @@ -261,7 +261,8 @@ where &consensus_parameters, &ufvks, scan_task.scan_range.clone(), - scan_task.previous_wallet_block, + scan_task.start_seam_block, + scan_task.end_seam_block, scan_task.locators, scan_task.transparent_addresses, ) @@ -317,7 +318,8 @@ where #[derive(Debug)] pub(crate) struct ScanTask { scan_range: ScanRange, - previous_wallet_block: Option, + start_seam_block: Option, + end_seam_block: Option, locators: BTreeSet, transparent_addresses: HashMap, } @@ -325,13 +327,15 @@ pub(crate) struct ScanTask { impl ScanTask { pub(crate) fn from_parts( scan_range: ScanRange, - previous_wallet_block: Option, + start_seam_block: Option, + end_seam_block: Option, locators: BTreeSet, transparent_addresses: HashMap, ) -> Self { Self { scan_range, - previous_wallet_block, + start_seam_block, + end_seam_block, locators, transparent_addresses, } diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 1486d3a34..64cff5e1a 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -87,6 +87,11 @@ pub(super) async fn update_scan_ranges( set_found_note_scan_ranges(sync_state, ShieldedProtocol::Orchard, locators.into_iter())?; set_chain_tip_scan_range(sync_state, chain_height)?; + let verification_height = sync_state.highest_scanned_height() + 1; + if verification_height <= chain_height { + set_verify_scan_range(sync_state, verification_height, VerifyEnd::VerifyLowest); + } + // TODO: add logic to merge scan ranges Ok(()) @@ -116,8 +121,6 @@ async fn create_scan_range( panic!("scan ranges should never be empty after updating"); } - set_verify_scan_range(sync_state, wallet_height + 1, VerifyEnd::VerifyLowest); - Ok(()) } @@ -484,10 +487,10 @@ where W: SyncWallet + SyncBlocks, { if let Some(scan_range) = select_scan_range(wallet.get_sync_state_mut().unwrap()) { - // TODO: disallow scanning without previous wallet block - let previous_wallet_block = wallet + let start_seam_block = wallet .get_wallet_block(scan_range.block_range().start - 1) .ok(); + let end_seam_block = wallet.get_wallet_block(scan_range.block_range().end).ok(); let locators = find_locators(wallet.get_sync_state().unwrap(), scan_range.block_range()); let transparent_addresses: HashMap = wallet @@ -499,7 +502,8 @@ where Ok(Some(ScanTask::from_parts( scan_range, - previous_wallet_block, + start_seam_block, + end_seam_block, locators, transparent_addresses, ))) From 627769a60817e327d027712fc6fee20f6c00866d Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 10:44:15 +0000 Subject: [PATCH 17/37] fix clippy and combine located tree builds into one spawn blocking --- zingo-sync/src/scan.rs | 17 ++++++++--------- zingo-sync/src/sync.rs | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/zingo-sync/src/scan.rs b/zingo-sync/src/scan.rs index 12c8313c8..c9d827c15 100644 --- a/zingo-sync/src/scan.rs +++ b/zingo-sync/src/scan.rs @@ -152,6 +152,7 @@ impl DecryptedNoteData { /// `locators` are the block height and txid of transactions in the `scan_range` that are known to be relevant to the /// wallet and are appended to during scanning if trial decryption succeeds. If there are no known relevant transctions /// then `locators` will start empty. +#[allow(clippy::too_many_arguments)] pub(crate) async fn scan

( fetch_request_sender: mpsc::UnboundedSender, parameters: &P, @@ -228,15 +229,13 @@ where orchard_leaves_and_retentions, } = witness_data; - let sapling_located_trees = tokio::task::spawn_blocking(move || { - witness::build_located_trees(sapling_initial_position, sapling_leaves_and_retentions) - .unwrap() - }) - .await - .unwrap(); - let orchard_located_trees = tokio::task::spawn_blocking(move || { - witness::build_located_trees(orchard_initial_position, orchard_leaves_and_retentions) - .unwrap() + let (sapling_located_trees, orchard_located_trees) = tokio::task::spawn_blocking(move || { + ( + witness::build_located_trees(sapling_initial_position, sapling_leaves_and_retentions) + .unwrap(), + witness::build_located_trees(orchard_initial_position, orchard_leaves_and_retentions) + .unwrap(), + ) }) .await .unwrap(); diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 60ed542c6..bc6fc496d 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -52,7 +52,7 @@ where P: consensus::Parameters + Sync + Send + 'static, W: SyncWallet + SyncBlocks + SyncTransactions + SyncNullifiers + SyncOutPoints + SyncShardTrees, { - tracing::info!("Syncing wallet..."); + tracing::info!("Starting sync..."); // create channel for sending fetch requests and launch fetcher task let (fetch_request_sender, fetch_request_receiver) = mpsc::unbounded_channel(); From feb5d2abe9bb83dc161f061e7712d10af1c4689d Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 11:20:28 +0000 Subject: [PATCH 18/37] revisit wallet data cleanup --- zingo-sync/src/scan/transactions.rs | 13 +++++++++---- zingo-sync/src/sync.rs | 14 ++------------ zingo-sync/src/sync/spend.rs | 4 ++-- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/zingo-sync/src/scan/transactions.rs b/zingo-sync/src/scan/transactions.rs index 1f4c2dcf2..7175bd366 100644 --- a/zingo-sync/src/scan/transactions.rs +++ b/zingo-sync/src/scan/transactions.rs @@ -518,8 +518,15 @@ fn collect_outpoints( +/// For each locator, fetch the spending transaction and then scan and append to the wallet transactions. +/// +/// This is only intended to be used for transactions that do not contain any incoming notes and therefore evaded +/// trial decryption. +/// For targetted rescan of transactions by locator, locators should be added to the wallet using the `TODO` API and +/// the `FoundNote` priorities will be automatically set for scan prioritisation. Transactions with incoming notes +/// are required to be scanned in the context of a scan task to correctly derive the nullifiers and positions for +/// spending. +pub(crate) async fn scan_spending_transactions( fetch_request_sender: mpsc::UnboundedSender, consensus_parameters: &P, wallet: &mut W, @@ -545,8 +552,6 @@ where } spending_locators.insert(locator); - // TODO: fetch block from server if not in wallet so wallet blocks added to wallet while scanning out of order - // don't need to be held in memory wallet_blocks.insert( block_height, wallet.get_wallet_block(block_height).expect( diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index bc6fc496d..7702ca008 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -444,12 +444,10 @@ where wallet .update_shard_trees(sapling_located_trees, orchard_located_trees) .unwrap(); - // TODO: add trait to save wallet data to persistence for in-memory wallets Ok(()) } -// TODO: replace this function with a filter on the data added to wallet fn remove_irrelevant_data(wallet: &mut W, scan_range: &ScanRange) -> Result<(), ()> where W: SyncWallet + SyncBlocks + SyncNullifiers + SyncTransactions, @@ -458,15 +456,7 @@ where return Ok(()); } - let wallet_height = wallet - .get_sync_state() - .unwrap() - .scan_ranges() - .last() - .expect("wallet should always have scan ranges after sync has started") - .block_range() - .end; - + let highest_scanned_height = wallet.get_sync_state().unwrap().highest_scanned_height(); let wallet_transaction_heights = wallet .get_wallet_transactions() .unwrap() @@ -475,7 +465,7 @@ where .collect::>(); wallet.get_wallet_blocks_mut().unwrap().retain(|height, _| { *height >= scan_range.block_range().end - 1 - || *height >= wallet_height - 100 + || *height >= highest_scanned_height - MAX_VERIFICATION_WINDOW || wallet_transaction_heights.contains(height) }); wallet diff --git a/zingo-sync/src/sync/spend.rs b/zingo-sync/src/sync/spend.rs index 44925b559..4391044c8 100644 --- a/zingo-sync/src/sync/spend.rs +++ b/zingo-sync/src/sync/spend.rs @@ -14,7 +14,7 @@ use zip32::AccountId; use crate::{ client::FetchRequest, primitives::{Locator, NullifierMap, OutPointMap, OutputId, WalletTransaction}, - scan::transactions::scan_located_transactions, + scan::transactions::scan_spending_transactions, traits::{SyncBlocks, SyncNullifiers, SyncOutPoints, SyncTransactions}, }; @@ -61,7 +61,7 @@ where .unwrap(); // in the edge case where a spending transaction received no change, scan the transactions that evaded trial decryption - scan_located_transactions( + scan_spending_transactions( fetch_request_sender, consensus_parameters, wallet, From 6d4daee330b4ccf1e90c67cbdb729d530818dfd8 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 11:28:58 +0000 Subject: [PATCH 19/37] clear locators --- zingo-sync/src/sync.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 7702ca008..29af90378 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -179,8 +179,6 @@ where } } - // TODO: clear locators - drop(wallet_guard); drop(scanner); drop(fetch_request_sender); @@ -478,6 +476,11 @@ where .unwrap() .orchard_mut() .retain(|_, (height, _)| *height >= scan_range.block_range().end); + wallet + .get_sync_state_mut() + .unwrap() + .locators_mut() + .retain(|(height, _)| *height >= scan_range.block_range().end); Ok(()) } From 0a21d09ff0f012a3097bf84312d8887263caf7e9 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 11:47:07 +0000 Subject: [PATCH 20/37] fix bug in --- libtonode-tests/tests/sync.rs | 2 ++ zingo-sync/src/sync.rs | 20 +++++++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/libtonode-tests/tests/sync.rs b/libtonode-tests/tests/sync.rs index 7fba16f88..f1f706721 100644 --- a/libtonode-tests/tests/sync.rs +++ b/libtonode-tests/tests/sync.rs @@ -99,6 +99,8 @@ async fn sync_status() { }); sync_handle.await.unwrap(); + + dbg!(&lightclient.wallet.lock().await.wallet_blocks); } // temporary test for sync development diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 29af90378..8ab3d2734 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -263,13 +263,13 @@ where ) .await .unwrap(); - remove_irrelevant_data(wallet, &scan_range).unwrap(); state::set_scan_priority( wallet.get_sync_state_mut().unwrap(), scan_range.block_range(), ScanPriority::Scanned, ) .unwrap(); + remove_irrelevant_data(wallet).unwrap(); tracing::debug!("Scan results processed."); } Err(ScanError::ContinuityError(ContinuityError::HashDiscontinuity { height, .. })) => { @@ -446,15 +446,13 @@ where Ok(()) } -fn remove_irrelevant_data(wallet: &mut W, scan_range: &ScanRange) -> Result<(), ()> +fn remove_irrelevant_data(wallet: &mut W) -> Result<(), ()> where W: SyncWallet + SyncBlocks + SyncNullifiers + SyncTransactions, { - if scan_range.priority() != ScanPriority::Historic { - return Ok(()); - } - - let highest_scanned_height = wallet.get_sync_state().unwrap().highest_scanned_height(); + let sync_state = wallet.get_sync_state().unwrap(); + let fully_scanned_height = sync_state.fully_scanned_height(); + let highest_scanned_height = sync_state.highest_scanned_height(); let wallet_transaction_heights = wallet .get_wallet_transactions() .unwrap() @@ -462,7 +460,7 @@ where .filter_map(|tx| tx.confirmation_status().get_confirmed_height()) .collect::>(); wallet.get_wallet_blocks_mut().unwrap().retain(|height, _| { - *height >= scan_range.block_range().end - 1 + *height >= fully_scanned_height - 1 || *height >= highest_scanned_height - MAX_VERIFICATION_WINDOW || wallet_transaction_heights.contains(height) }); @@ -470,17 +468,17 @@ where .get_nullifiers_mut() .unwrap() .sapling_mut() - .retain(|_, (height, _)| *height >= scan_range.block_range().end); + .retain(|_, (height, _)| *height > fully_scanned_height); wallet .get_nullifiers_mut() .unwrap() .orchard_mut() - .retain(|_, (height, _)| *height >= scan_range.block_range().end); + .retain(|_, (height, _)| *height > fully_scanned_height); wallet .get_sync_state_mut() .unwrap() .locators_mut() - .retain(|(height, _)| *height >= scan_range.block_range().end); + .retain(|(height, _)| *height > fully_scanned_height); Ok(()) } From 6175ba09d847ba31acbbacd9fc69f2561288cb07 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 11:59:10 +0000 Subject: [PATCH 21/37] add max re-org window --- zingo-sync/src/sync.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 8ab3d2734..257587124 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -138,6 +138,11 @@ where // TODO: implement an option for continuous scanning where it doesnt exit when complete let mut wallet_guard = wallet.lock().await; + let initial_verification_height = wallet_guard + .get_sync_state() + .unwrap() + .highest_scanned_height() + + 1; let mut interval = tokio::time::interval(Duration::from_millis(30)); loop { tokio::select! { @@ -149,6 +154,7 @@ where &ufvks, scan_range, scan_results, + initial_verification_height, ) .await .unwrap(); @@ -246,6 +252,7 @@ async fn process_scan_results( ufvks: &HashMap, scan_range: ScanRange, scan_results: Result, + initial_verification_height: BlockHeight, ) -> Result<(), SyncError> where P: consensus::Parameters, @@ -289,6 +296,15 @@ where state::VerifyEnd::VerifyHighest, ); truncate_wallet_data(wallet, scan_range_to_verify.block_range().start - 1).unwrap(); + + if initial_verification_height - scan_range_to_verify.block_range().start + > MAX_VERIFICATION_WINDOW + { + panic!( + "sync failed. re-org of larger than {} blocks detected", + MAX_VERIFICATION_WINDOW + ); + } } else { scan_results?; } From 03bbbc8b6c7e6689f82a1559d4e6429467330c08 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 31 Dec 2024 12:02:08 +0000 Subject: [PATCH 22/37] remove todo --- zingo-sync/src/sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 257587124..2dc67161a 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -40,7 +40,7 @@ pub(crate) mod transparent; // TODO; replace fixed batches with variable batches with fixed memory size const BATCH_SIZE: u32 = 10_000; const VERIFY_BLOCK_RANGE_SIZE: u32 = 10; -const MAX_VERIFICATION_WINDOW: u32 = 100; // TODO: fail if re-org goes beyond this window +const MAX_VERIFICATION_WINDOW: u32 = 100; /// Syncs a wallet to the latest state of the blockchain pub async fn sync( From 0d371596e56bd195acfbc76d295adfd42b5ddd24 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 2 Jan 2025 02:16:33 +0000 Subject: [PATCH 23/37] get block range returns a stream --- libtonode-tests/tests/sync.rs | 2 -- zingo-sync/src/client.rs | 11 +++++++---- zingo-sync/src/client/fetch.rs | 17 +++++------------ zingo-sync/src/scan/task.rs | 19 +++++++++++++++++-- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/libtonode-tests/tests/sync.rs b/libtonode-tests/tests/sync.rs index f1f706721..7fba16f88 100644 --- a/libtonode-tests/tests/sync.rs +++ b/libtonode-tests/tests/sync.rs @@ -99,8 +99,6 @@ async fn sync_status() { }); sync_handle.await.unwrap(); - - dbg!(&lightclient.wallet.lock().await.wallet_blocks); } // temporary test for sync development diff --git a/zingo-sync/src/client.rs b/zingo-sync/src/client.rs index ce94ac0af..5acc2282e 100644 --- a/zingo-sync/src/client.rs +++ b/zingo-sync/src/client.rs @@ -29,7 +29,10 @@ pub enum FetchRequest { /// Gets the height of the blockchain from the server. ChainTip(oneshot::Sender), /// Gets the specified range of compact blocks from the server (end exclusive). - CompactBlockRange(oneshot::Sender>, Range), + CompactBlockRange( + oneshot::Sender>, + Range, + ), /// Gets the tree states for a specified block height. TreeState(oneshot::Sender, BlockHeight), /// Get a full transaction by txid. @@ -74,14 +77,14 @@ pub async fn get_chain_height( pub async fn get_compact_block_range( fetch_request_sender: UnboundedSender, block_range: Range, -) -> Result, ()> { +) -> Result, ()> { let (reply_sender, reply_receiver) = oneshot::channel(); fetch_request_sender .send(FetchRequest::CompactBlockRange(reply_sender, block_range)) .unwrap(); - let compact_blocks = reply_receiver.await.unwrap(); + let block_stream = reply_receiver.await.unwrap(); - Ok(compact_blocks) + Ok(block_stream) } /// Gets the stream of shards (subtree roots) diff --git a/zingo-sync/src/client/fetch.rs b/zingo-sync/src/client/fetch.rs index 0b8339227..4cb364e2d 100644 --- a/zingo-sync/src/client/fetch.rs +++ b/zingo-sync/src/client/fetch.rs @@ -109,8 +109,8 @@ async fn fetch_from_server( } FetchRequest::CompactBlockRange(sender, block_range) => { tracing::debug!("Fetching compact blocks. {:?}", &block_range); - let compact_blocks = get_block_range(client, block_range).await.unwrap(); - sender.send(compact_blocks).unwrap(); + let block_stream = get_block_range(client, block_range).await.unwrap(); + sender.send(block_stream).unwrap(); } FetchRequest::GetSubtreeRoots(sender, start_index, shielded_protocol, max_entries) => { tracing::debug!( @@ -172,10 +172,7 @@ async fn get_latest_block( async fn get_block_range( client: &mut CompactTxStreamerClient, block_range: Range, -) -> Result, ()> { - let mut compact_blocks: Vec = - Vec::with_capacity(u64::from(block_range.end - block_range.start) as usize); - +) -> Result, ()> { let request = tonic::Request::new(BlockRange { start: Some(BlockId { height: u64::from(block_range.start), @@ -186,13 +183,9 @@ async fn get_block_range( hash: vec![], }), }); - let mut block_stream = client.get_block_range(request).await.unwrap().into_inner(); - - while let Some(compact_block) = block_stream.message().await.unwrap() { - compact_blocks.push(compact_block); - } + let block_stream = client.get_block_range(request).await.unwrap().into_inner(); - Ok(compact_blocks) + Ok(block_stream) } async fn get_subtree_roots( diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index 26e3362f0..c3c45df1e 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -11,7 +11,10 @@ use tokio::{ task::{JoinError, JoinHandle}, }; -use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange}; +use zcash_client_backend::{ + data_api::scanning::{ScanPriority, ScanRange}, + proto::compact_formats::CompactBlock, +}; use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::{ consensus::{self}, @@ -19,7 +22,7 @@ use zcash_primitives::{ }; use crate::{ - client::FetchRequest, + client::{self, FetchRequest}, keys::transparent::TransparentAddressId, primitives::{Locator, WalletBlock}, sync, @@ -29,6 +32,7 @@ use crate::{ use super::{error::ScanError, scan, ScanResults}; const MAX_WORKER_POOLSIZE: usize = 2; +const MAX_BATCH_OUTPUTS: usize = 16384; // 2^14 pub(crate) enum ScannerState { Verification, @@ -256,6 +260,17 @@ where let handle = tokio::spawn(async move { while let Some(scan_task) = scan_task_receiver.recv().await { + let mut block_stream = client::get_compact_block_range( + fetch_request_sender.clone(), + scan_task.scan_range.block_range().clone(), + ) + .await + .unwrap(); + let mut compact_blocks: Vec = Vec::new(); + while let Some(compact_block) = block_stream.message().await.unwrap() { + compact_blocks.push(compact_block); + } + let scan_results = scan( fetch_request_sender.clone(), &consensus_parameters, From 0b6f68bd5eeb69c672c767b5ad981219ae5ed846 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Fri, 3 Jan 2025 07:14:14 +0000 Subject: [PATCH 24/37] implemented batcher --- zingo-sync/src/scan.rs | 38 ++++-- zingo-sync/src/scan/task.rs | 251 ++++++++++++++++++++++++++++-------- zingo-sync/src/sync.rs | 2 +- 3 files changed, 224 insertions(+), 67 deletions(-) diff --git a/zingo-sync/src/scan.rs b/zingo-sync/src/scan.rs index c9d827c15..0bd419b25 100644 --- a/zingo-sync/src/scan.rs +++ b/zingo-sync/src/scan.rs @@ -4,10 +4,11 @@ use std::{ }; use orchard::tree::MerkleHashOrchard; +use task::ScanTask; use tokio::sync::mpsc; use incrementalmerkletree::Position; -use zcash_client_backend::{data_api::scanning::ScanRange, proto::compact_formats::CompactBlock}; +use zcash_client_backend::proto::compact_formats::CompactBlock; use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::{ consensus::{BlockHeight, NetworkUpgrade, Parameters}, @@ -17,7 +18,6 @@ use zcash_primitives::{ use crate::{ client::{self, FetchRequest}, - keys::transparent::TransparentAddressId, primitives::{Locator, NullifierMap, OutPointMap, OutputId, WalletBlock, WalletTransaction}, witness::{self, LocatedTreeData, WitnessData}, }; @@ -157,21 +157,33 @@ pub(crate) async fn scan

( fetch_request_sender: mpsc::UnboundedSender, parameters: &P, ufvks: &HashMap, - scan_range: ScanRange, - start_seam_block: Option, - end_seam_block: Option, - mut locators: BTreeSet, - transparent_addresses: HashMap, + scan_task: ScanTask, ) -> Result where P: Parameters + Sync + Send + 'static, { - let compact_blocks = client::get_compact_block_range( - fetch_request_sender.clone(), - scan_range.block_range().clone(), - ) - .await - .unwrap(); + let ScanTask { + compact_blocks, + scan_range, + start_seam_block, + end_seam_block, + mut locators, + transparent_addresses, + } = scan_task; + + if compact_blocks + .first() + .expect("compacts blocks should not be empty") + .height + != scan_range.block_range().start.into() + || compact_blocks + .last() + .expect("compacts blocks should not be empty") + .height + != (scan_range.block_range().end - 1).into() + { + panic!("compact blocks do not match scan range!") + } let initial_scan_data = InitialScanData::new( fetch_request_sender.clone(), diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index c3c45df1e..5d91c3655 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -7,7 +7,7 @@ use std::{ }; use tokio::{ - sync::mpsc, + sync::mpsc::{self, error::TryRecvError}, task::{JoinError, JoinHandle}, }; @@ -52,6 +52,7 @@ impl ScannerState { pub(crate) struct Scanner

{ state: ScannerState, + batcher: Option, workers: Vec>, unique_id: usize, scan_results_sender: mpsc::UnboundedSender<(ScanRange, Result)>, @@ -75,19 +76,42 @@ where Self { state: ScannerState::Verification, + batcher: None, workers, unique_id: 0, - consensus_parameters, scan_results_sender, fetch_request_sender, + consensus_parameters, ufvks, } } + pub(crate) fn launch(&mut self) { + self.spawn_batcher(); + self.spawn_workers(); + } + pub(crate) fn worker_poolsize(&self) -> usize { self.workers.len() } + /// Spawns the batcher. + /// + /// When the batcher is running it will wait for a scan task. + pub(crate) fn spawn_batcher(&mut self) { + tracing::debug!("Spawning batcher"); + let mut batcher = Batcher::new(self.fetch_request_sender.clone()); + batcher.run().unwrap(); + self.batcher = Some(batcher); + } + + async fn shutdown_batcher(&mut self) -> Result<(), JoinError> { + let mut batcher = self.batcher.take().expect("batcher should exist!"); + batcher.shutdown().await?; + + Ok(()) + } + /// Spawns a worker. /// /// When the worker is running it will wait for a scan task. @@ -96,7 +120,6 @@ where let mut worker = ScanWorker::new( self.unique_id, self.consensus_parameters.clone(), - None, self.scan_results_sender.clone(), self.fetch_request_sender.clone(), self.ufvks.clone(), @@ -144,14 +167,20 @@ where /// Updates the scanner. /// /// If verification is still in progress, only create scan tasks with `Verify` scan priority. - /// If there is an idle worker, create a new scan task and add to worker. - /// If there are no more range available to scan, shutdown the idle workers. + /// If there are no batches ready and the batcher is idle, + /// If there are no more range available to scan, shutdown the batcher, idle workers and mempool. pub(crate) async fn update(&mut self, wallet: &mut W, shutdown_mempool: Arc) where W: SyncWallet + SyncBlocks, { match self.state { ScannerState::Verification => { + self.batcher + .as_mut() + .expect("batcher should be running") + .update_batch_store(); + self.update_workers(); + let sync_state = wallet.get_sync_state().unwrap(); if !sync_state .scan_ranges() @@ -173,24 +202,16 @@ where } // scan ranges with `Verify` priority - if let Some(worker) = self.idle_worker() { - let scan_task = sync::state::create_scan_task(wallet) - .unwrap() - .expect("scan range with `Verify` priority must exist!"); - - assert_eq!(scan_task.scan_range.priority(), ScanPriority::Verify); - worker.add_scan_task(scan_task).unwrap(); - } + self.update_batcher(wallet); } ScannerState::Scan => { - // create scan tasks until all ranges are scanned or currently scanning - if let Some(worker) = self.idle_worker() { - if let Some(scan_task) = sync::state::create_scan_task(wallet).unwrap() { - worker.add_scan_task(scan_task).unwrap(); - } else if wallet.get_sync_state().unwrap().scan_complete() { - self.state.scan_completed(); - } - } + // create scan tasks for batching and scanning until all ranges are scanned + self.batcher + .as_mut() + .expect("batcher should be running") + .update_batch_store(); + self.update_workers(); + self.update_batcher(wallet); } ScannerState::Shutdown => { // shutdown mempool @@ -198,10 +219,13 @@ where // shutdown idle workers while let Some(worker) = self.idle_worker() { - self.shutdown_worker(worker.id) - .await - .expect("worker should be in worker pool"); + self.shutdown_worker(worker.id).await.unwrap(); } + + // shutdown batcher + self.shutdown_batcher() + .await + .expect("batcher should not fail!"); } } @@ -209,6 +233,142 @@ where panic!("worker pool should not be empty with unscanned ranges!") } } + + fn update_workers(&mut self) { + let batcher = self.batcher.as_ref().expect("batcher should be running"); + if batcher.batch.is_some() { + if let Some(worker) = self.idle_worker() { + let batch = batcher + .batch + .clone() + .expect("batch should exist in this closure"); + worker.add_scan_task(batch); + self.batcher + .as_mut() + .expect("batcher should be running") + .batch = None; + } + } + } + + fn update_batcher(&mut self, wallet: &mut W) + where + W: SyncWallet + SyncBlocks, + { + let batcher = self.batcher.as_ref().expect("batcher should be running"); + if !batcher.is_batching() { + if let Some(scan_task) = sync::state::create_scan_task(wallet).unwrap() { + batcher.add_scan_task(scan_task); + } else if wallet.get_sync_state().unwrap().scan_complete() { + self.state.scan_completed(); + } + } + } +} + +struct Batcher { + handle: Option>, + is_batching: Arc, + batch: Option, + scan_task_sender: Option>, + batch_receiver: Option>, + fetch_request_sender: mpsc::UnboundedSender, +} + +impl Batcher { + fn new(fetch_request_sender: mpsc::UnboundedSender) -> Self { + Self { + handle: None, + is_batching: Arc::new(AtomicBool::new(false)), + batch: None, + scan_task_sender: None, + batch_receiver: None, + fetch_request_sender, + } + } + + /// Runs the batcher in a new tokio task. + /// + /// Waits for a scan task and then fetches compact blocks to form fixed output batches. The scan task is split if + /// needed and the compact blocks are added to each scan task and sent to the scan workers for scanning. + fn run(&mut self) -> Result<(), ()> { + let (scan_task_sender, mut scan_task_receiver) = mpsc::channel::(1); + let (batch_sender, batch_receiver) = mpsc::channel::(1); + + let is_batching = self.is_batching.clone(); + let fetch_request_sender = self.fetch_request_sender.clone(); + + let handle = tokio::spawn(async move { + while let Some(mut scan_task) = scan_task_receiver.recv().await { + let mut block_stream = client::get_compact_block_range( + fetch_request_sender.clone(), + scan_task.scan_range.block_range().clone(), + ) + .await + .unwrap(); + while let Some(compact_block) = block_stream.message().await.unwrap() { + scan_task.compact_blocks.push(compact_block); + } + + batch_sender + .send(scan_task) + .await + .expect("receiver should never be dropped before sender!"); + + is_batching.store(false, atomic::Ordering::Release); + } + }); + + self.handle = Some(handle); + self.scan_task_sender = Some(scan_task_sender); + self.batch_receiver = Some(batch_receiver); + + Ok(()) + } + + fn is_batching(&self) -> bool { + self.is_batching.load(atomic::Ordering::Acquire) + } + + fn add_scan_task(&self, scan_task: ScanTask) { + tracing::debug!("Adding scan task to batcher:\n{:#?}", &scan_task); + self.scan_task_sender + .clone() + .expect("batcher should be running") + .try_send(scan_task) + .expect("batcher should never be sent multiple tasks at one time"); + self.is_batching.store(true, atomic::Ordering::Release); + } + + fn update_batch_store(&mut self) { + let batch_receiver = self + .batch_receiver + .as_mut() + .expect("batcher should be running"); + if self.batch.is_none() && !batch_receiver.is_empty() { + self.batch = Some( + batch_receiver + .try_recv() + .expect("channel should be non-empty!"), + ); + } + } + + /// Shuts down batcher by dropping the sender to the batcher task and awaiting the handle. + /// + /// This should always be called in the context of the scanner as it must be also be taken from the Scanner struct. + async fn shutdown(&mut self) -> Result<(), JoinError> { + tracing::debug!("Shutting down batcher"); + if let Some(sender) = self.scan_task_sender.take() { + drop(sender); + } + let handle = self + .handle + .take() + .expect("batcher should always have a handle to take!"); + + handle.await + } } struct ScanWorker

{ @@ -229,7 +389,6 @@ where fn new( id: usize, consensus_parameters: P, - scan_task_sender: Option>, scan_results_sender: mpsc::UnboundedSender<(ScanRange, Result)>, fetch_request_sender: mpsc::UnboundedSender, ufvks: HashMap, @@ -239,7 +398,7 @@ where handle: None, is_scanning: Arc::new(AtomicBool::new(false)), consensus_parameters, - scan_task_sender, + scan_task_sender: None, scan_results_sender, fetch_request_sender, ufvks, @@ -260,31 +419,17 @@ where let handle = tokio::spawn(async move { while let Some(scan_task) = scan_task_receiver.recv().await { - let mut block_stream = client::get_compact_block_range( - fetch_request_sender.clone(), - scan_task.scan_range.block_range().clone(), - ) - .await - .unwrap(); - let mut compact_blocks: Vec = Vec::new(); - while let Some(compact_block) = block_stream.message().await.unwrap() { - compact_blocks.push(compact_block); - } - + let scan_range = scan_task.scan_range.clone(); let scan_results = scan( fetch_request_sender.clone(), &consensus_parameters, &ufvks, - scan_task.scan_range.clone(), - scan_task.start_seam_block, - scan_task.end_seam_block, - scan_task.locators, - scan_task.transparent_addresses, + scan_task, ) .await; scan_results_sender - .send((scan_task.scan_range, scan_results)) + .send((scan_range, scan_results)) .expect("receiver should never be dropped before sender!"); is_scanning.store(false, atomic::Ordering::Release); @@ -301,16 +446,14 @@ where self.is_scanning.load(atomic::Ordering::Acquire) } - fn add_scan_task(&self, scan_task: ScanTask) -> Result<(), ()> { + fn add_scan_task(&self, scan_task: ScanTask) { tracing::debug!("Adding scan task to worker {}:\n{:#?}", self.id, &scan_task); self.scan_task_sender .clone() - .unwrap() + .expect("worker should be running") .try_send(scan_task) .expect("worker should never be sent multiple tasks at one time"); self.is_scanning.store(true, atomic::Ordering::Release); - - Ok(()) } /// Shuts down worker by dropping the sender to the worker task and awaiting the handle. @@ -330,13 +473,14 @@ where } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct ScanTask { - scan_range: ScanRange, - start_seam_block: Option, - end_seam_block: Option, - locators: BTreeSet, - transparent_addresses: HashMap, + pub(crate) compact_blocks: Vec, + pub(crate) scan_range: ScanRange, + pub(crate) start_seam_block: Option, + pub(crate) end_seam_block: Option, + pub(crate) locators: BTreeSet, + pub(crate) transparent_addresses: HashMap, } impl ScanTask { @@ -348,6 +492,7 @@ impl ScanTask { transparent_addresses: HashMap, ) -> Self { Self { + compact_blocks: Vec::new(), scan_range, start_seam_block, end_seam_block, diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 2dc67161a..1947aa539 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -132,7 +132,7 @@ where fetch_request_sender.clone(), ufvks.clone(), ); - scanner.spawn_workers(); + scanner.launch(); // TODO: invalidate any pending transactions after eviction height (40 below best chain height?) // TODO: implement an option for continuous scanning where it doesnt exit when complete From 182eb2f3de05770b18d80bfadb20a999fc786bb3 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 7 Jan 2025 02:28:59 +0000 Subject: [PATCH 25/37] added fixed output batching --- zingo-sync/src/scan/task.rs | 96 +++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index 5d91c3655..8898538d7 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -17,7 +17,8 @@ use zcash_client_backend::{ }; use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::{ - consensus::{self}, + consensus::{self, BlockHeight}, + transaction::TxId, zip32::AccountId, }; @@ -166,9 +167,12 @@ where /// Updates the scanner. /// - /// If verification is still in progress, only create scan tasks with `Verify` scan priority. - /// If there are no batches ready and the batcher is idle, - /// If there are no more range available to scan, shutdown the batcher, idle workers and mempool. + /// Creates a new scan task and sends to batcher if it's idle. + /// The batcher will stream compact blocks into the scan task, splitting the scan task when the maximum number of + /// outputs is reached. When a scan task is ready is it stored in the batcher ready to be taken by an idle scan + /// worker for scanning. + /// If verification is still in progress, only scan tasks with `Verify` scan priority are created. + /// If there are no more ranges available to scan, the batcher, idle workers and mempool are shutdown. pub(crate) async fn update(&mut self, wallet: &mut W, shutdown_mempool: Arc) where W: SyncWallet + SyncBlocks, @@ -299,7 +303,16 @@ impl Batcher { let fetch_request_sender = self.fetch_request_sender.clone(); let handle = tokio::spawn(async move { + // save seam blocks between scan tasks for linear scanning continuuity checks + // during non-linear scanning the wallet blocks from the scanned ranges will already be saved in the wallet + let mut first_block: Option = None; + let mut last_block: Option = None; + // TODO: finish seam block logic + while let Some(mut scan_task) = scan_task_receiver.recv().await { + let mut sapling_output_count = 0; + let mut orchard_output_count = 0; + let mut block_stream = client::get_compact_block_range( fetch_request_sender.clone(), scan_task.scan_range.block_range().clone(), @@ -307,6 +320,45 @@ impl Batcher { .await .unwrap(); while let Some(compact_block) = block_stream.message().await.unwrap() { + sapling_output_count += compact_block + .vtx + .iter() + .fold(0, |acc, transaction| acc + transaction.outputs.len()); + orchard_output_count += compact_block + .vtx + .iter() + .fold(0, |acc, transaction| acc + transaction.actions.len()); + + if sapling_output_count + orchard_output_count > MAX_BATCH_OUTPUTS { + let new_batch_first_block = WalletBlock::from_parts( + compact_block.height(), + compact_block.hash(), + compact_block.prev_hash(), + 0, + Vec::new(), + 0, + 0, + ); + + let (full_batch, new_batch) = scan_task + .clone() + .split( + new_batch_first_block.block_height(), + Some(new_batch_first_block), + None, + ) + .unwrap(); + + batch_sender + .send(full_batch) + .await + .expect("receiver should never be dropped before sender!"); + + scan_task = new_batch; + sapling_output_count = 0; + orchard_output_count = 0; + } + scan_task.compact_blocks.push(compact_block); } @@ -500,4 +552,40 @@ impl ScanTask { transparent_addresses, } } + + fn split( + self, + block_height: BlockHeight, + lower_task_end_seam_block: Option, + upper_task_start_seam_block: Option, + ) -> Result<(Self, Self), ()> { + if block_height > self.scan_range.block_range().start + && block_height < self.scan_range.block_range().end + { + panic!("block height should be within scan tasks block range!"); + } + + let mut lower_task_locators = self.locators; + let upper_task_locators = + lower_task_locators.split_off(&(block_height, TxId::from_bytes([0; 32]))); + + Ok(( + ScanTask { + compact_blocks: self.compact_blocks, + scan_range: self.scan_range.truncate_end(block_height).unwrap(), + start_seam_block: self.start_seam_block, + end_seam_block: lower_task_end_seam_block, + locators: lower_task_locators, + transparent_addresses: self.transparent_addresses.clone(), + }, + ScanTask { + compact_blocks: Vec::new(), + scan_range: self.scan_range.truncate_start(block_height).unwrap(), + start_seam_block: upper_task_start_seam_block, + end_seam_block: self.end_seam_block, + locators: upper_task_locators, + transparent_addresses: self.transparent_addresses, + }, + )) + } } From ac4d7433551efcf0c0dc4565f22c958ececf09be Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 7 Jan 2025 02:58:08 +0000 Subject: [PATCH 26/37] removed sync feature from libtonode --- libtonode-tests/Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libtonode-tests/Cargo.toml b/libtonode-tests/Cargo.toml index 8308f020d..3b8a9bf40 100644 --- a/libtonode-tests/Cargo.toml +++ b/libtonode-tests/Cargo.toml @@ -6,14 +6,13 @@ edition = "2021" [features] chain_generic_tests = [] ci = ["zingolib/ci"] -sync = ["dep:zingo-sync"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] zingolib = { path = "../zingolib", features = [ "deprecations", "test-elevation" ] } zingo-status = { path = "../zingo-status" } zingo-netutils = { path = "../zingo-netutils" } -zingo-sync = { path = "../zingo-sync", optional = true } +zingo-sync = { path = "../zingo-sync" } testvectors = { path = "../testvectors" } bip0039.workspace = true From 69fe10fd9f09e4f159ab0cf817e7063d670eda8f Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 7 Jan 2025 05:02:28 +0000 Subject: [PATCH 27/37] start work on setting scanned ranges correctly in post-scan processing --- zingo-sync/src/scan/task.rs | 6 +- zingo-sync/src/sync.rs | 6 +- zingo-sync/src/sync/state.rs | 106 +++++++++++++++++++++++------------ 3 files changed, 75 insertions(+), 43 deletions(-) diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index 8898538d7..a50e19fe7 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -7,7 +7,7 @@ use std::{ }; use tokio::{ - sync::mpsc::{self, error::TryRecvError}, + sync::mpsc, task::{JoinError, JoinHandle}, }; @@ -559,8 +559,8 @@ impl ScanTask { lower_task_end_seam_block: Option, upper_task_start_seam_block: Option, ) -> Result<(Self, Self), ()> { - if block_height > self.scan_range.block_range().start - && block_height < self.scan_range.block_range().end + if block_height < self.scan_range.block_range().start + && block_height > self.scan_range.block_range().end - 1 { panic!("block height should be within scan tasks block range!"); } diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 1947aa539..c5f8a9fb9 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -37,8 +37,6 @@ pub(crate) mod state; pub(crate) mod transparent; // TODO: move parameters to config module -// TODO; replace fixed batches with variable batches with fixed memory size -const BATCH_SIZE: u32 = 10_000; const VERIFY_BLOCK_RANGE_SIZE: u32 = 10; const MAX_VERIFICATION_WINDOW: u32 = 100; @@ -270,9 +268,9 @@ where ) .await .unwrap(); - state::set_scan_priority( + state::punch_scan_priority( wallet.get_sync_state_mut().unwrap(), - scan_range.block_range(), + scan_range.block_range().clone(), ScanPriority::Scanned, ) .unwrap(); diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 64cff5e1a..b31f3f2aa 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -23,7 +23,7 @@ use crate::{ traits::{SyncBlocks, SyncWallet}, }; -use super::{BATCH_SIZE, VERIFY_BLOCK_RANGE_SIZE}; +use super::VERIFY_BLOCK_RANGE_SIZE; /// Used to determine which end of the scan range is verified. pub(super) enum VerifyEnd { @@ -194,6 +194,30 @@ pub(super) fn set_verify_scan_range( scan_range_to_verify } +/// Punches in the chain tip block range with `ScanPriority::ChainTip`. +/// +/// Determines the chain tip block range by finding the lowest start height of the latest incomplete shard for each +/// shielded protocol. +fn set_chain_tip_scan_range( + sync_state: &mut SyncState, + chain_height: BlockHeight, +) -> Result<(), ()> { + let sapling_incomplete_shard = + determine_block_range(sync_state, chain_height, ShieldedProtocol::Sapling); + let orchard_incomplete_shard = + determine_block_range(sync_state, chain_height, ShieldedProtocol::Orchard); + + let chain_tip = if sapling_incomplete_shard.start < orchard_incomplete_shard.start { + sapling_incomplete_shard + } else { + orchard_incomplete_shard + }; + + punch_scan_priority(sync_state, chain_tip, ScanPriority::ChainTip).unwrap(); + + Ok(()) +} + /// Punches in the `shielded_protocol` shard block ranges surrounding each locator with `ScanPriority::FoundNote`. pub(super) fn set_found_note_scan_ranges>( sync_state: &mut SyncState, @@ -220,27 +244,19 @@ pub(super) fn set_found_note_scan_range( Ok(()) } -/// Punches in the chain tip block range with `ScanPriority::ChainTip`. -/// -/// Determines the chain tip block range by finding the lowest start height of the latest incomplete shard for each -/// shielded protocol. -fn set_chain_tip_scan_range( +pub(super) fn set_scanned_scan_range( sync_state: &mut SyncState, - chain_height: BlockHeight, + scanned_range: Range, ) -> Result<(), ()> { - let sapling_incomplete_shard = - determine_block_range(sync_state, chain_height, ShieldedProtocol::Sapling); - let orchard_incomplete_shard = - determine_block_range(sync_state, chain_height, ShieldedProtocol::Orchard); + let scan_ranges = sync_state.scan_ranges_mut(); - let chain_tip = if sapling_incomplete_shard.start < orchard_incomplete_shard.start { - sapling_incomplete_shard - } else { - orchard_incomplete_shard + let Some((index, scan_range)) = scan_ranges.iter().enumerate().find(|(_, scan_range)| { + scan_range.block_range().contains(&scanned_range.start) + && scan_range.block_range().contains(&scanned_range.end) + }) else { + panic!("scan range containing scanned range should exist!"); }; - punch_scan_priority(sync_state, chain_tip, ScanPriority::ChainTip).unwrap(); - Ok(()) } @@ -276,7 +292,7 @@ pub(super) fn set_scan_priority( /// Any scan ranges that fully contain the `block_range` will be split out with the given `scan_priority`. /// Any scan ranges with `Ignored` (Scanning) or `Scanned` priority or with higher (or equal) priority than /// `scan_priority` will be ignored. -fn punch_scan_priority( +pub(crate) fn punch_scan_priority( sync_state: &mut SyncState, block_range: Range, scan_priority: ScanPriority, @@ -454,25 +470,43 @@ fn select_scan_range(sync_state: &mut SyncState) -> Option { } let selected_priority = highest_priority_scan_range.priority(); - // TODO: fixed memory batching - let batch_block_range = Range { - start: highest_priority_scan_range.block_range().start, - end: highest_priority_scan_range.block_range().start + BATCH_SIZE, - }; - let split_ranges = split_out_scan_range( - highest_priority_scan_range, - batch_block_range, - ScanPriority::Ignored, - ); - let selected_block_range = split_ranges - .first() - .expect("split ranges should always be non-empty") - .block_range() - .clone(); - sync_state - .scan_ranges_mut() - .splice(index..=index, split_ranges); + // historic scan ranges can be larger than a shard block range so must be split out. + // otherwise, just set the scan priority of selected range to `Ignored` (scanning) in sync state. + let selected_block_range = if selected_priority == ScanPriority::Historic { + let shard_block_range = determine_block_range( + &sync_state, + highest_priority_scan_range.block_range().start, + ShieldedProtocol::Orchard, + ); + let split_ranges = split_out_scan_range( + highest_priority_scan_range, + shard_block_range, + ScanPriority::Ignored, + ); + let selected_block_range = split_ranges + .first() + .expect("split ranges should always be non-empty") + .block_range() + .clone(); + sync_state + .scan_ranges_mut() + .splice(index..=index, split_ranges); + + selected_block_range + } else { + let selected_scan_range = sync_state + .scan_ranges_mut() + .get_mut(index) + .expect("scan range should exist due to previous logic"); + + *selected_scan_range = ScanRange::from_parts( + highest_priority_scan_range.block_range().clone(), + ScanPriority::Ignored, + ); + + selected_scan_range.block_range().clone() + }; // TODO: when this library has its own version of ScanRange this can be simplified and more readable Some(ScanRange::from_parts( From 24aa381c555495a72284e6e199ffe612b2994de3 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 7 Jan 2025 13:49:03 +0000 Subject: [PATCH 28/37] improve scan task split --- zingo-sync/src/scan/task.rs | 81 ++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index a50e19fe7..18d3bc9f2 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -171,8 +171,8 @@ where /// The batcher will stream compact blocks into the scan task, splitting the scan task when the maximum number of /// outputs is reached. When a scan task is ready is it stored in the batcher ready to be taken by an idle scan /// worker for scanning. - /// If verification is still in progress, only scan tasks with `Verify` scan priority are created. - /// If there are no more ranges available to scan, the batcher, idle workers and mempool are shutdown. + /// When verification is still in progress, only scan tasks with `Verify` scan priority are created. + /// When all ranges are scanned, the batcher, idle workers and mempool are shutdown. pub(crate) async fn update(&mut self, wallet: &mut W, shutdown_mempool: Arc) where W: SyncWallet + SyncBlocks, @@ -209,7 +209,6 @@ where self.update_batcher(wallet); } ScannerState::Scan => { - // create scan tasks for batching and scanning until all ranges are scanned self.batcher .as_mut() .expect("batcher should be running") @@ -218,15 +217,10 @@ where self.update_batcher(wallet); } ScannerState::Shutdown => { - // shutdown mempool shutdown_mempool.store(true, atomic::Ordering::Release); - - // shutdown idle workers while let Some(worker) = self.idle_worker() { self.shutdown_worker(worker.id).await.unwrap(); } - - // shutdown batcher self.shutdown_batcher() .await .expect("batcher should not fail!"); @@ -330,23 +324,14 @@ impl Batcher { .fold(0, |acc, transaction| acc + transaction.actions.len()); if sapling_output_count + orchard_output_count > MAX_BATCH_OUTPUTS { - let new_batch_first_block = WalletBlock::from_parts( - compact_block.height(), - compact_block.hash(), - compact_block.prev_hash(), - 0, - Vec::new(), - 0, - 0, - ); - let (full_batch, new_batch) = scan_task .clone() - .split( - new_batch_first_block.block_height(), - Some(new_batch_first_block), - None, - ) + .split(BlockHeight::from_u32( + compact_block + .height + .try_into() + .expect("should never overflow"), + )) .unwrap(); batch_sender @@ -553,35 +538,65 @@ impl ScanTask { } } - fn split( - self, - block_height: BlockHeight, - lower_task_end_seam_block: Option, - upper_task_start_seam_block: Option, - ) -> Result<(Self, Self), ()> { + /// Splits a scan task into two at `block_height`. + /// + /// Panics if `block_height` is not contained in the scan task's block range. + fn split(self, block_height: BlockHeight) -> Result<(Self, Self), ()> { if block_height < self.scan_range.block_range().start && block_height > self.scan_range.block_range().end - 1 { panic!("block height should be within scan tasks block range!"); } + let mut lower_compact_blocks = self.compact_blocks; + let upper_compact_blocks = if let Some(index) = lower_compact_blocks + .iter() + .position(|block| block.height == block_height.into()) + { + lower_compact_blocks.split_off(index) + } else { + Vec::new() + }; + let mut lower_task_locators = self.locators; let upper_task_locators = lower_task_locators.split_off(&(block_height, TxId::from_bytes([0; 32]))); + let lower_task_last_block = lower_compact_blocks.last().map(|block| { + WalletBlock::from_parts( + block.height(), + block.hash(), + block.prev_hash(), + 0, + Vec::new(), + 0, + 0, + ) + }); + let upper_task_first_block = upper_compact_blocks.first().map(|block| { + WalletBlock::from_parts( + block.height(), + block.hash(), + block.prev_hash(), + 0, + Vec::new(), + 0, + 0, + ) + }); Ok(( ScanTask { - compact_blocks: self.compact_blocks, + compact_blocks: lower_compact_blocks, scan_range: self.scan_range.truncate_end(block_height).unwrap(), start_seam_block: self.start_seam_block, - end_seam_block: lower_task_end_seam_block, + end_seam_block: upper_task_first_block, locators: lower_task_locators, transparent_addresses: self.transparent_addresses.clone(), }, ScanTask { - compact_blocks: Vec::new(), + compact_blocks: upper_compact_blocks, scan_range: self.scan_range.truncate_start(block_height).unwrap(), - start_seam_block: upper_task_start_seam_block, + start_seam_block: lower_task_last_block, end_seam_block: self.end_seam_block, locators: upper_task_locators, transparent_addresses: self.transparent_addresses, From 59dfbefc3b4b24e62d54fc69248c9efc70f12b00 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 7 Jan 2025 15:22:19 +0000 Subject: [PATCH 29/37] complete batcher with linear scanning continuity checks --- zingo-sync/src/scan/compact_blocks.rs | 5 +- zingo-sync/src/scan/task.rs | 84 ++++++++++++++++++++++----- zingo-sync/src/sync.rs | 3 +- zingo-sync/src/sync/state.rs | 23 +++++--- 4 files changed, 89 insertions(+), 26 deletions(-) diff --git a/zingo-sync/src/scan/compact_blocks.rs b/zingo-sync/src/scan/compact_blocks.rs index 568bfa220..b322d6ecd 100644 --- a/zingo-sync/src/scan/compact_blocks.rs +++ b/zingo-sync/src/scan/compact_blocks.rs @@ -30,7 +30,7 @@ use super::{ mod runners; // TODO: move parameters to config module -const TRIAL_DECRYPT_TASK_SIZE: usize = 1_000; +const TRIAL_DECRYPT_TASK_SIZE: usize = 1_024; // 2^10 pub(crate) fn scan_compact_blocks

( compact_blocks: Vec, @@ -61,8 +61,7 @@ where let mut sapling_tree_size = initial_scan_data.sapling_initial_tree_size; let mut orchard_tree_size = initial_scan_data.orchard_initial_tree_size; for block in &compact_blocks { - let block_height = - BlockHeight::from_u32(block.height.try_into().expect("should never overflow")); + let block_height = block.height(); let mut transactions = block.vtx.iter().peekable(); while let Some(transaction) = transactions.next() { // collect trial decryption results by transaction diff --git a/zingo-sync/src/scan/task.rs b/zingo-sync/src/scan/task.rs index 18d3bc9f2..016eb902f 100644 --- a/zingo-sync/src/scan/task.rs +++ b/zingo-sync/src/scan/task.rs @@ -33,7 +33,7 @@ use crate::{ use super::{error::ScanError, scan, ScanResults}; const MAX_WORKER_POOLSIZE: usize = 2; -const MAX_BATCH_OUTPUTS: usize = 16384; // 2^14 +const MAX_BATCH_OUTPUTS: usize = 8_192; // 2^13 pub(crate) enum ScannerState { Verification, @@ -299,13 +299,13 @@ impl Batcher { let handle = tokio::spawn(async move { // save seam blocks between scan tasks for linear scanning continuuity checks // during non-linear scanning the wallet blocks from the scanned ranges will already be saved in the wallet - let mut first_block: Option = None; - let mut last_block: Option = None; - // TODO: finish seam block logic + let mut previous_task_first_block: Option = None; + let mut previous_task_last_block: Option = None; while let Some(mut scan_task) = scan_task_receiver.recv().await { let mut sapling_output_count = 0; let mut orchard_output_count = 0; + let mut first_batch = true; let mut block_stream = client::get_compact_block_range( fetch_request_sender.clone(), @@ -314,6 +314,53 @@ impl Batcher { .await .unwrap(); while let Some(compact_block) = block_stream.message().await.unwrap() { + if let Some(block) = previous_task_last_block.as_ref() { + if scan_task.start_seam_block.is_none() + && scan_task.scan_range.block_range().start == block.block_height() + 1 + { + scan_task.start_seam_block = previous_task_last_block.clone(); + } + } + if let Some(block) = previous_task_first_block.as_ref() { + if scan_task.end_seam_block.is_none() + && scan_task.scan_range.block_range().end == block.block_height() + { + scan_task.end_seam_block = previous_task_first_block.clone(); + } + } + + if first_batch { + // TODO: check conditions where chain metadata is none + let chain_metadata = compact_block + .chain_metadata + .expect("chain metadata should always exist"); + previous_task_first_block = Some(WalletBlock::from_parts( + compact_block.height(), + compact_block.hash(), + compact_block.prev_hash(), + compact_block.time, + compact_block.vtx.iter().map(|tx| tx.txid()).collect(), + chain_metadata.sapling_commitment_tree_size, + chain_metadata.orchard_commitment_tree_size, + )); + first_batch = false; + } + if compact_block.height() == scan_task.scan_range.block_range().end - 1 { + // TODO: check conditions where chain metadata is none + let chain_metadata = compact_block + .chain_metadata + .expect("chain metadata should always exist"); + previous_task_last_block = Some(WalletBlock::from_parts( + compact_block.height(), + compact_block.hash(), + compact_block.prev_hash(), + compact_block.time, + compact_block.vtx.iter().map(|tx| tx.txid()).collect(), + chain_metadata.sapling_commitment_tree_size, + chain_metadata.orchard_commitment_tree_size, + )); + } + sapling_output_count += compact_block .vtx .iter() @@ -322,7 +369,6 @@ impl Batcher { .vtx .iter() .fold(0, |acc, transaction| acc + transaction.actions.len()); - if sapling_output_count + orchard_output_count > MAX_BATCH_OUTPUTS { let (full_batch, new_batch) = scan_task .clone() @@ -551,7 +597,7 @@ impl ScanTask { let mut lower_compact_blocks = self.compact_blocks; let upper_compact_blocks = if let Some(index) = lower_compact_blocks .iter() - .position(|block| block.height == block_height.into()) + .position(|block| block.height() == block_height) { lower_compact_blocks.split_off(index) } else { @@ -563,25 +609,35 @@ impl ScanTask { lower_task_locators.split_off(&(block_height, TxId::from_bytes([0; 32]))); let lower_task_last_block = lower_compact_blocks.last().map(|block| { + // TODO: check conditions where chain metadata is none + let chain_metadata = block + .chain_metadata + .expect("chain metadata should always exist"); + WalletBlock::from_parts( block.height(), block.hash(), block.prev_hash(), - 0, - Vec::new(), - 0, - 0, + block.time, + block.vtx.iter().map(|tx| tx.txid()).collect(), + chain_metadata.sapling_commitment_tree_size, + chain_metadata.orchard_commitment_tree_size, ) }); let upper_task_first_block = upper_compact_blocks.first().map(|block| { + // TODO: check conditions where chain metadata is none + let chain_metadata = block + .chain_metadata + .expect("chain metadata should always exist"); + WalletBlock::from_parts( block.height(), block.hash(), block.prev_hash(), - 0, - Vec::new(), - 0, - 0, + block.time, + block.vtx.iter().map(|tx| tx.txid()).collect(), + chain_metadata.sapling_commitment_tree_size, + chain_metadata.orchard_commitment_tree_size, ) }); Ok(( diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index c5f8a9fb9..ff6bfa387 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -268,10 +268,9 @@ where ) .await .unwrap(); - state::punch_scan_priority( + state::set_scanned_scan_range( wallet.get_sync_state_mut().unwrap(), scan_range.block_range().clone(), - ScanPriority::Scanned, ) .unwrap(); remove_irrelevant_data(wallet).unwrap(); diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index b31f3f2aa..c9833bf74 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -252,11 +252,20 @@ pub(super) fn set_scanned_scan_range( let Some((index, scan_range)) = scan_ranges.iter().enumerate().find(|(_, scan_range)| { scan_range.block_range().contains(&scanned_range.start) - && scan_range.block_range().contains(&scanned_range.end) + && scan_range.block_range().contains(&(scanned_range.end - 1)) }) else { panic!("scan range containing scanned range should exist!"); }; + let split_ranges = split_out_scan_range( + scan_range.clone(), + scanned_range.clone(), + ScanPriority::Scanned, + ); + sync_state + .scan_ranges_mut() + .splice(index..=index, split_ranges); + Ok(()) } @@ -292,7 +301,7 @@ pub(super) fn set_scan_priority( /// Any scan ranges that fully contain the `block_range` will be split out with the given `scan_priority`. /// Any scan ranges with `Ignored` (Scanning) or `Scanned` priority or with higher (or equal) priority than /// `scan_priority` will be ignored. -pub(crate) fn punch_scan_priority( +fn punch_scan_priority( sync_state: &mut SyncState, block_range: Range, scan_priority: ScanPriority, @@ -310,7 +319,7 @@ pub(crate) fn punch_scan_priority( match ( block_range.contains(&scan_range.block_range().start), - block_range.contains(&scan_range.block_range().end), + block_range.contains(&(scan_range.block_range().end - 1)), scan_range.block_range().contains(&block_range.start), ) { (true, true, _) => scan_ranges_contained_by_block_range.push(scan_range.clone()), @@ -410,28 +419,28 @@ fn split_out_scan_range( if let Some((lower_range, higher_range)) = scan_range.split_at(block_range.start) { split_ranges.push(lower_range); if let Some((middle_range, higher_range)) = higher_range.split_at(block_range.end) { - // [scan_range] is split at the upper and lower bound of [block_range] + // `scan_range` is split at the upper and lower bound of `block_range` split_ranges.push(ScanRange::from_parts( middle_range.block_range().clone(), scan_priority, )); split_ranges.push(higher_range); } else { - // [scan_range] is split only at the lower bound of [block_range] + // `scan_range` is split only at the lower bound of `block_range` split_ranges.push(ScanRange::from_parts( higher_range.block_range().clone(), scan_priority, )); } } else if let Some((lower_range, higher_range)) = scan_range.split_at(block_range.end) { - // [scan_range] is split only at the upper bound of [block_range] + // `scan_range` is split only at the upper bound of `block_range` split_ranges.push(ScanRange::from_parts( lower_range.block_range().clone(), scan_priority, )); split_ranges.push(higher_range); } else { - // [scan_range] is not split as it is fully contained within [block_range] + // `scan_range` is not split as it is fully contained within `block_range` // only scan priority is updated assert!(scan_range.block_range().start >= block_range.start); assert!(scan_range.block_range().end <= block_range.end); From 7b9ddc44036e663d0b2522c758edd484b1c8ce12 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Tue, 7 Jan 2025 15:24:05 +0000 Subject: [PATCH 30/37] fix clippy warnings --- zingo-sync/src/sync/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index c9833bf74..71f05fc1a 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -484,7 +484,7 @@ fn select_scan_range(sync_state: &mut SyncState) -> Option { // otherwise, just set the scan priority of selected range to `Ignored` (scanning) in sync state. let selected_block_range = if selected_priority == ScanPriority::Historic { let shard_block_range = determine_block_range( - &sync_state, + sync_state, highest_priority_scan_range.block_range().start, ShieldedProtocol::Orchard, ); From 790f417db22d5773b227f3d7e73c19597920246e Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Wed, 15 Jan 2025 10:15:25 +0000 Subject: [PATCH 31/37] added outputs to initial sync state --- zingo-sync/src/primitives.rs | 67 +++++++++--- zingo-sync/src/scan.rs | 113 ++++++++++--------- zingo-sync/src/scan/compact_blocks.rs | 31 +++--- zingo-sync/src/sync.rs | 16 ++- zingo-sync/src/sync/state.rs | 151 +++++++++++++++++++++++++- 5 files changed, 283 insertions(+), 95 deletions(-) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index eff776c49..749906d2b 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -25,6 +25,40 @@ use crate::{ /// detections or transparent output discovery. pub type Locator = (BlockHeight, TxId); +/// Initial sync state. +/// +/// All fields will be reset when a new sync session starts. +#[derive(Debug, Clone, CopyGetters, Setters)] +#[getset(get_copy = "pub", set = "pub")] +pub struct InitialSyncState { + /// One block above the fully scanned wallet height at start of sync. + sync_start_height: BlockHeight, + /// Sapling tree size of fully scanned wallet height at start of sync. + sync_start_sapling_tree_size: u32, + /// Orchard tree size of fully scanned wallet height at start of sync. + sync_start_orchard_tree_size: u32, + /// Total number of blocks to scan. + total_blocks_to_scan: u32, + /// Total number of sapling outputs to scan. + total_sapling_outputs_to_scan: u32, + /// Total number of orchard outputs to scan. + total_orchard_outputs_to_scan: u32, +} + +impl InitialSyncState { + /// Create new InitialSyncState + pub fn new() -> Self { + InitialSyncState { + sync_start_height: 0.into(), + sync_start_sapling_tree_size: 0, + sync_start_orchard_tree_size: 0, + total_blocks_to_scan: 0, + total_sapling_outputs_to_scan: 0, + total_orchard_outputs_to_scan: 0, + } + } +} + /// Encapsulates the current state of sync #[derive(Debug, Clone, Getters, MutGetters, CopyGetters, Setters)] #[getset(get = "pub", get_mut = "pub")] @@ -34,16 +68,8 @@ pub struct SyncState { scan_ranges: Vec, /// Locators for relevant transactions to the wallet. locators: BTreeSet, - /// Fully scanned wallet height at start of sync. - /// Reset when sync starts. - #[getset(skip)] - #[getset(get_copy = "pub", set = "pub")] - sync_start_height: BlockHeight, - /// Total number of blocks to scan this session - /// Reset when sync starts. - #[getset(skip)] - #[getset(get_copy = "pub", set = "pub")] - total_blocks_to_scan: u32, + /// Initial sync state. + initial_sync_state: InitialSyncState, } impl SyncState { @@ -52,8 +78,7 @@ impl SyncState { SyncState { scan_ranges: Vec::new(), locators: BTreeSet::new(), - sync_start_height: 0.into(), - total_blocks_to_scan: 0, + initial_sync_state: InitialSyncState::new(), } } @@ -171,8 +196,10 @@ pub struct WalletBlock { time: u32, #[getset(skip)] txids: Vec, - sapling_commitment_tree_size: u32, - orchard_commitment_tree_size: u32, + sapling_initial_tree_size: u32, + orchard_initial_tree_size: u32, + sapling_final_tree_size: u32, + orchard_final_tree_size: u32, } impl WalletBlock { @@ -182,8 +209,10 @@ impl WalletBlock { prev_hash: BlockHash, time: u32, txids: Vec, - sapling_commitment_tree_size: u32, - orchard_commitment_tree_size: u32, + sapling_starting_tree_size: u32, + orchard_starting_tree_size: u32, + sapling_final_tree_size: u32, + orchard_final_tree_size: u32, ) -> Self { Self { block_height, @@ -191,8 +220,10 @@ impl WalletBlock { prev_hash, time, txids, - sapling_commitment_tree_size, - orchard_commitment_tree_size, + sapling_initial_tree_size: sapling_starting_tree_size, + orchard_initial_tree_size: orchard_starting_tree_size, + sapling_final_tree_size, + orchard_final_tree_size, } } diff --git a/zingo-sync/src/scan.rs b/zingo-sync/src/scan.rs index 1a3c8e4b4..37115d0ab 100644 --- a/zingo-sync/src/scan.rs +++ b/zingo-sync/src/scan.rs @@ -50,59 +50,68 @@ impl InitialScanData { // gets initial tree size from previous block if available // otherwise, from first block if available // otherwise, fetches frontiers from server - let (sapling_initial_tree_size, orchard_initial_tree_size) = - if let Some(prev) = &previous_wallet_block { - ( - prev.sapling_commitment_tree_size(), - prev.orchard_commitment_tree_size(), - ) - } else if let Some(chain_metadata) = &first_block.chain_metadata { - // calculate initial tree size by subtracting number of outputs in block from the blocks final tree size - let sapling_output_count: u32 = first_block - .vtx - .iter() - .map(|tx| tx.outputs.len()) - .sum::() - .try_into() - .expect("Sapling output count cannot exceed a u32"); - let orchard_output_count: u32 = first_block - .vtx - .iter() - .map(|tx| tx.actions.len()) - .sum::() - .try_into() - .expect("Sapling output count cannot exceed a u32"); - - ( - chain_metadata - .sapling_commitment_tree_size - .checked_sub(sapling_output_count) - .unwrap(), - chain_metadata - .orchard_commitment_tree_size - .checked_sub(orchard_output_count) - .unwrap(), - ) - } else { - let sapling_activation_height = consensus_parameters - .activation_height(NetworkUpgrade::Sapling) - .expect("should have some sapling activation height"); - - match first_block.height().cmp(&sapling_activation_height) { - cmp::Ordering::Greater => { - let frontiers = - client::get_frontiers(fetch_request_sender, first_block.height() - 1) - .await - .unwrap(); - ( - frontiers.final_sapling_tree().tree_size() as u32, - frontiers.final_orchard_tree().tree_size() as u32, - ) - } - cmp::Ordering::Equal => (0, 0), - cmp::Ordering::Less => panic!("pre-sapling not supported!"), + let (sapling_initial_tree_size, orchard_initial_tree_size) = if let Some(prev) = + &previous_wallet_block + { + ( + prev.sapling_final_tree_size(), + prev.orchard_final_tree_size(), + ) + } else if let Some(chain_metadata) = &first_block.chain_metadata { + // calculate initial tree size by subtracting number of outputs in block from the blocks final tree size + let sapling_output_count: u32 = first_block + .vtx + .iter() + .map(|tx| tx.outputs.len()) + .sum::() + .try_into() + .expect("Sapling output count cannot exceed a u32"); + let orchard_output_count: u32 = first_block + .vtx + .iter() + .map(|tx| tx.actions.len()) + .sum::() + .try_into() + .expect("Sapling output count cannot exceed a u32"); + + ( + chain_metadata + .sapling_commitment_tree_size + .checked_sub(sapling_output_count) + .unwrap(), + chain_metadata + .orchard_commitment_tree_size + .checked_sub(orchard_output_count) + .unwrap(), + ) + } else { + let sapling_activation_height = consensus_parameters + .activation_height(NetworkUpgrade::Sapling) + .expect("should have some sapling activation height"); + + match first_block.height().cmp(&sapling_activation_height) { + cmp::Ordering::Greater => { + let frontiers = + client::get_frontiers(fetch_request_sender, first_block.height() - 1) + .await + .unwrap(); + ( + frontiers + .final_sapling_tree() + .tree_size() + .try_into() + .expect("should not be more than 2^32 note commitments in the tree!"), + frontiers + .final_orchard_tree() + .tree_size() + .try_into() + .expect("should not be more than 2^32 note commitments in the tree!"), + ) } - }; + cmp::Ordering::Equal => (0, 0), + cmp::Ordering::Less => panic!("pre-sapling not supported!"), + } + }; Ok(InitialScanData { previous_block: previous_wallet_block, diff --git a/zingo-sync/src/scan/compact_blocks.rs b/zingo-sync/src/scan/compact_blocks.rs index 2d724c394..144aee17e 100644 --- a/zingo-sync/src/scan/compact_blocks.rs +++ b/zingo-sync/src/scan/compact_blocks.rs @@ -55,9 +55,14 @@ where Position::from(u64::from(initial_scan_data.sapling_initial_tree_size)), Position::from(u64::from(initial_scan_data.orchard_initial_tree_size)), ); - let mut sapling_tree_size = initial_scan_data.sapling_initial_tree_size; - let mut orchard_tree_size = initial_scan_data.orchard_initial_tree_size; + let mut sapling_initial_tree_size; + let mut orchard_initial_tree_size; + let mut sapling_final_tree_size = initial_scan_data.sapling_initial_tree_size; + let mut orchard_final_tree_size = initial_scan_data.orchard_initial_tree_size; for block in &compact_blocks { + sapling_initial_tree_size = sapling_final_tree_size; + orchard_initial_tree_size = orchard_final_tree_size; + let mut transactions = block.vtx.iter().peekable(); while let Some(transaction) = transactions.next() { // collect trial decryption results by transaction @@ -100,21 +105,21 @@ where ); calculate_nullifiers_and_positions( - sapling_tree_size, + sapling_final_tree_size, scanning_keys.sapling(), &incoming_sapling_outputs, &mut decrypted_note_data.sapling_nullifiers_and_positions, ); calculate_nullifiers_and_positions( - orchard_tree_size, + orchard_final_tree_size, scanning_keys.orchard(), &incoming_orchard_outputs, &mut decrypted_note_data.orchard_nullifiers_and_positions, ); - sapling_tree_size += u32::try_from(transaction.outputs.len()) + sapling_final_tree_size += u32::try_from(transaction.outputs.len()) .expect("should not be more than 2^32 outputs in a transaction"); - orchard_tree_size += u32::try_from(transaction.actions.len()) + orchard_final_tree_size += u32::try_from(transaction.actions.len()) .expect("should not be more than 2^32 outputs in a transaction"); } @@ -124,8 +129,10 @@ where block.prev_hash(), block.time, block.vtx.iter().map(|tx| tx.txid()).collect(), - sapling_tree_size, - orchard_tree_size, + sapling_initial_tree_size, + orchard_initial_tree_size, + sapling_final_tree_size, + orchard_final_tree_size, ); check_tree_size(block, &wallet_block).unwrap(); @@ -203,14 +210,10 @@ fn check_continuity( fn check_tree_size(compact_block: &CompactBlock, wallet_block: &WalletBlock) -> Result<(), ()> { if let Some(chain_metadata) = &compact_block.chain_metadata { - if chain_metadata.sapling_commitment_tree_size - != wallet_block.sapling_commitment_tree_size() - { + if chain_metadata.sapling_commitment_tree_size != wallet_block.sapling_final_tree_size() { panic!("sapling tree size is incorrect!") } - if chain_metadata.orchard_commitment_tree_size - != wallet_block.orchard_commitment_tree_size() - { + if chain_metadata.orchard_commitment_tree_size != wallet_block.orchard_final_tree_size() { panic!("orchard tree size is incorrect!") } } diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 0ed23ac8f..2d1442885 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -19,7 +19,6 @@ use crate::traits::{ use futures::StreamExt; use shardtree::{store::ShardStore, LocatedPrunableTree, RetentionFlags}; -use state::set_initial_state; use zcash_client_backend::proto::service::RawTransaction; use zcash_client_backend::{ data_api::scanning::{ScanPriority, ScanRange}, @@ -113,7 +112,13 @@ where .await .unwrap(); - set_initial_state(wallet_guard.get_sync_state_mut().unwrap()); + state::set_initial_state( + consensus_parameters, + fetch_request_sender.clone(), + &mut *wallet_guard, + chain_height, + ) + .await; update_subtree_roots(fetch_request_sender.clone(), &mut *wallet_guard).await; @@ -203,9 +208,10 @@ where .fold(0, |acc, block_range| { acc + (block_range.end - block_range.start) }); - let scanned_blocks = sync_state.total_blocks_to_scan() - unscanned_blocks; - let percentage_blocks_complete = - (scanned_blocks as f32 / sync_state.total_blocks_to_scan() as f32) * 100.0; + let scanned_blocks = sync_state.initial_sync_state().total_blocks_to_scan() - unscanned_blocks; + let percentage_blocks_complete = (scanned_blocks as f32 + / sync_state.initial_sync_state().total_blocks_to_scan() as f32) + * 100.0; SyncStatus { scan_ranges: sync_state.scan_ranges().clone(), diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index d29eac6fb..93385b95f 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -2,13 +2,15 @@ use std::{cmp, collections::HashMap, ops::Range}; +use tokio::sync::mpsc; use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange}; use zcash_primitives::{ - consensus::{self, BlockHeight}, + consensus::{self, BlockHeight, NetworkUpgrade}, transaction::TxId, }; use crate::{ + client::{self, FetchRequest}, keys::transparent::TransparentAddressId, primitives::{Locator, SyncState}, scan::task::ScanTask, @@ -428,8 +430,114 @@ where } } -/// Sets the `total_blocks_to_scan` and `sync_start_height` fields at the start of the sync process -pub(super) fn set_initial_state(sync_state: &mut SyncState) { +/// Sets the `initial_sync_state` field at the start of the sync session +pub(super) async fn set_initial_state( + consensus_parameters: &impl consensus::Parameters, + fetch_request_sender: mpsc::UnboundedSender, + wallet: &mut W, + chain_height: BlockHeight, +) where + W: SyncWallet + SyncBlocks, +{ + struct ScannedRangeTreeBoundaries { + sapling_initial_tree_size: u32, + orchard_initial_tree_size: u32, + sapling_final_tree_size: u32, + orchard_final_tree_size: u32, + } + + /// Gets `block_height` final tree sizes from wallet block if it exists, otherwise from frontiers fetched from server. + /// + /// Only used in context of setting initial sync state as can also use the compact blocks to calculate tree sizes + /// during scanning. + async fn final_tree_sizes( + consensus_parameters: &impl consensus::Parameters, + fetch_request_sender: mpsc::UnboundedSender, + wallet: &mut W, + block_height: BlockHeight, + ) -> (u32, u32) + where + W: SyncWallet + SyncBlocks, + { + if let Ok(block) = wallet.get_wallet_block(block_height) { + ( + block.sapling_final_tree_size(), + block.orchard_final_tree_size(), + ) + } else { + // TODO: move this whole block into `client::get_frontiers` + let sapling_activation_height = consensus_parameters + .activation_height(NetworkUpgrade::Sapling) + .expect("should have some sapling activation height"); + + match block_height.cmp(&sapling_activation_height) { + cmp::Ordering::Greater => { + let frontiers = + client::get_frontiers(fetch_request_sender.clone(), block_height) + .await + .unwrap(); + ( + frontiers + .final_sapling_tree() + .tree_size() + .try_into() + .expect("should not be more than 2^32 note commitments in the tree!"), + frontiers + .final_orchard_tree() + .tree_size() + .try_into() + .expect("should not be more than 2^32 note commitments in the tree!"), + ) + } + cmp::Ordering::Equal => (0, 0), + cmp::Ordering::Less => panic!("pre-sapling not supported!"), + } + } + } + + /// Gets the initial and final tree sizes of a `scanned_range`. + /// + /// Panics if `scanned_range` boundary wallet blocks are not found in the wallet. + fn scanned_range_tree_boundaries( + wallet: &mut W, + scanned_range: Range, + ) -> ScannedRangeTreeBoundaries + where + W: SyncWallet + SyncBlocks, + { + let start_block = wallet + .get_wallet_block(scanned_range.start) + .expect("scanned range boundary blocks should be retained in the wallet"); + let end_block = wallet + .get_wallet_block(scanned_range.end - 1) + .expect("scanned range boundary blocks should be retained in the wallet"); + + ScannedRangeTreeBoundaries { + sapling_initial_tree_size: start_block.sapling_initial_tree_size(), + orchard_initial_tree_size: start_block.orchard_initial_tree_size(), + sapling_final_tree_size: end_block.sapling_final_tree_size(), + orchard_final_tree_size: end_block.orchard_final_tree_size(), + } + } + + let fully_scanned_height = wallet.get_sync_state().unwrap().fully_scanned_height(); + + let (sync_start_sapling_tree_size, sync_start_orchard_tree_size) = final_tree_sizes( + consensus_parameters, + fetch_request_sender.clone(), + wallet, + fully_scanned_height, + ) + .await; + let (chain_tip_sapling_tree_size, chain_tip_orchard_tree_size) = final_tree_sizes( + consensus_parameters, + fetch_request_sender.clone(), + wallet, + chain_height, + ) + .await; + + let sync_state = wallet.get_sync_state().unwrap(); let total_blocks_to_scan = sync_state .scan_ranges() .iter() @@ -438,7 +546,38 @@ pub(super) fn set_initial_state(sync_state: &mut SyncState) { .fold(0, |acc, block_range| { acc + (block_range.end - block_range.start) }); - sync_state.set_total_blocks_to_scan(total_blocks_to_scan); - let sync_start_height = sync_state.fully_scanned_height() + 1; - sync_state.set_sync_start_height(sync_start_height); + + let scanned_block_ranges = sync_state + .scan_ranges() + .iter() + .filter(|scan_range| { + scan_range.priority() == ScanPriority::Scanned + && scan_range.block_range().start > fully_scanned_height + }) + .map(|scan_range| scan_range.block_range().clone()) + .collect::>(); + let (scanned_sapling_outputs, scanned_orchard_outputs) = scanned_block_ranges + .iter() + .map(|block_range| scanned_range_tree_boundaries(wallet, block_range.clone())) + .fold((0, 0), |acc, tree_sizes| { + ( + acc.0 + (tree_sizes.sapling_final_tree_size - tree_sizes.sapling_initial_tree_size), + acc.1 + (tree_sizes.orchard_final_tree_size - tree_sizes.orchard_initial_tree_size), + ) + }); + let total_sapling_outputs_to_scan = + chain_tip_sapling_tree_size - sync_start_sapling_tree_size - scanned_sapling_outputs; + let total_orchard_outputs_to_scan = + chain_tip_orchard_tree_size - sync_start_orchard_tree_size - scanned_orchard_outputs; + + let initial_sync_state = wallet + .get_sync_state_mut() + .unwrap() + .initial_sync_state_mut(); + initial_sync_state.set_sync_start_height(fully_scanned_height + 1); + initial_sync_state.set_sync_start_sapling_tree_size(sync_start_sapling_tree_size); + initial_sync_state.set_sync_start_orchard_tree_size(sync_start_orchard_tree_size); + initial_sync_state.set_total_blocks_to_scan(total_blocks_to_scan); + initial_sync_state.set_total_sapling_outputs_to_scan(total_sapling_outputs_to_scan); + initial_sync_state.set_total_orchard_outputs_to_scan(total_orchard_outputs_to_scan); } From b8b2aca3b3dfe74fa5443f871594d3bd87f261e0 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 16 Jan 2025 05:34:15 +0000 Subject: [PATCH 32/37] updated sync status to include outputs --- zingo-sync/src/primitives.rs | 53 +++--- zingo-sync/src/scan.rs | 4 +- zingo-sync/src/scan/compact_blocks.rs | 20 ++- zingo-sync/src/sync.rs | 37 +++- zingo-sync/src/sync/state.rs | 239 ++++++++++++++------------ 5 files changed, 209 insertions(+), 144 deletions(-) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index 749906d2b..db00bd67a 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -31,12 +31,10 @@ pub type Locator = (BlockHeight, TxId); #[derive(Debug, Clone, CopyGetters, Setters)] #[getset(get_copy = "pub", set = "pub")] pub struct InitialSyncState { - /// One block above the fully scanned wallet height at start of sync. + /// One block above the fully scanned wallet height at start of sync session. sync_start_height: BlockHeight, - /// Sapling tree size of fully scanned wallet height at start of sync. - sync_start_sapling_tree_size: u32, - /// Orchard tree size of fully scanned wallet height at start of sync. - sync_start_orchard_tree_size: u32, + /// The tree sizes of the fully scanned height and chain tip at start of sync session. + sync_tree_boundaries: TreeBoundaries, /// Total number of blocks to scan. total_blocks_to_scan: u32, /// Total number of sapling outputs to scan. @@ -50,8 +48,12 @@ impl InitialSyncState { pub fn new() -> Self { InitialSyncState { sync_start_height: 0.into(), - sync_start_sapling_tree_size: 0, - sync_start_orchard_tree_size: 0, + sync_tree_boundaries: TreeBoundaries { + sapling_initial_tree_size: 0, + sapling_final_tree_size: 0, + orchard_initial_tree_size: 0, + orchard_final_tree_size: 0, + }, total_blocks_to_scan: 0, total_sapling_outputs_to_scan: 0, total_orchard_outputs_to_scan: 0, @@ -59,6 +61,11 @@ impl InitialSyncState { } } +impl Default for InitialSyncState { + fn default() -> Self { + Self::new() + } +} /// Encapsulates the current state of sync #[derive(Debug, Clone, Getters, MutGetters, CopyGetters, Setters)] #[getset(get = "pub", get_mut = "pub")] @@ -113,13 +120,28 @@ impl Default for SyncState { } } +#[derive(Debug, Clone, Copy)] +pub struct TreeBoundaries { + pub sapling_initial_tree_size: u32, + pub sapling_final_tree_size: u32, + pub orchard_initial_tree_size: u32, + pub orchard_final_tree_size: u32, +} + /// A snapshot of the current state of sync. Useful for displaying the status of sync to a user / consumer. +/// +/// `percentage_outputs_scanned` is a much more accurate indicator of sync completion than `percentage_blocks_scanned`. #[derive(Debug, Clone, Getters)] pub struct SyncStatus { pub scan_ranges: Vec, pub scanned_blocks: u32, pub unscanned_blocks: u32, - pub percentage_blocks_complete: f32, + pub percentage_blocks_scanned: f32, + pub scanned_sapling_outputs: u32, + pub unscanned_sapling_outputs: u32, + pub scanned_orchard_outputs: u32, + pub unscanned_orchard_outputs: u32, + pub percentage_outputs_scanned: f32, } /// Output ID for a given pool type @@ -196,10 +218,7 @@ pub struct WalletBlock { time: u32, #[getset(skip)] txids: Vec, - sapling_initial_tree_size: u32, - orchard_initial_tree_size: u32, - sapling_final_tree_size: u32, - orchard_final_tree_size: u32, + tree_boundaries: TreeBoundaries, } impl WalletBlock { @@ -209,10 +228,7 @@ impl WalletBlock { prev_hash: BlockHash, time: u32, txids: Vec, - sapling_starting_tree_size: u32, - orchard_starting_tree_size: u32, - sapling_final_tree_size: u32, - orchard_final_tree_size: u32, + tree_boundaries: TreeBoundaries, ) -> Self { Self { block_height, @@ -220,10 +236,7 @@ impl WalletBlock { prev_hash, time, txids, - sapling_initial_tree_size: sapling_starting_tree_size, - orchard_initial_tree_size: orchard_starting_tree_size, - sapling_final_tree_size, - orchard_final_tree_size, + tree_boundaries, } } diff --git a/zingo-sync/src/scan.rs b/zingo-sync/src/scan.rs index 37115d0ab..0abf75e07 100644 --- a/zingo-sync/src/scan.rs +++ b/zingo-sync/src/scan.rs @@ -54,8 +54,8 @@ impl InitialScanData { &previous_wallet_block { ( - prev.sapling_final_tree_size(), - prev.orchard_final_tree_size(), + prev.tree_boundaries().sapling_final_tree_size, + prev.tree_boundaries().orchard_final_tree_size, ) } else if let Some(chain_metadata) = &first_block.chain_metadata { // calculate initial tree size by subtracting number of outputs in block from the blocks final tree size diff --git a/zingo-sync/src/scan/compact_blocks.rs b/zingo-sync/src/scan/compact_blocks.rs index 144aee17e..fcd2497ff 100644 --- a/zingo-sync/src/scan/compact_blocks.rs +++ b/zingo-sync/src/scan/compact_blocks.rs @@ -17,7 +17,7 @@ use zcash_primitives::{ use crate::{ keys::{KeyId, ScanningKeyOps, ScanningKeys}, - primitives::{NullifierMap, OutputId, WalletBlock}, + primitives::{NullifierMap, OutputId, TreeBoundaries, WalletBlock}, witness::WitnessData, }; @@ -129,10 +129,12 @@ where block.prev_hash(), block.time, block.vtx.iter().map(|tx| tx.txid()).collect(), - sapling_initial_tree_size, - orchard_initial_tree_size, - sapling_final_tree_size, - orchard_final_tree_size, + TreeBoundaries { + sapling_initial_tree_size, + sapling_final_tree_size, + orchard_initial_tree_size, + orchard_final_tree_size, + }, ); check_tree_size(block, &wallet_block).unwrap(); @@ -210,10 +212,14 @@ fn check_continuity( fn check_tree_size(compact_block: &CompactBlock, wallet_block: &WalletBlock) -> Result<(), ()> { if let Some(chain_metadata) = &compact_block.chain_metadata { - if chain_metadata.sapling_commitment_tree_size != wallet_block.sapling_final_tree_size() { + if chain_metadata.sapling_commitment_tree_size + != wallet_block.tree_boundaries().sapling_final_tree_size + { panic!("sapling tree size is incorrect!") } - if chain_metadata.orchard_commitment_tree_size != wallet_block.orchard_final_tree_size() { + if chain_metadata.orchard_commitment_tree_size + != wallet_block.tree_boundaries().orchard_final_tree_size + { panic!("orchard tree size is incorrect!") } } diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 2d1442885..f90c00994 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -8,7 +8,7 @@ use std::time::Duration; use crate::client::{self, FetchRequest}; use crate::error::SyncError; use crate::keys::transparent::TransparentAddressId; -use crate::primitives::{NullifierMap, OutPointMap, SyncState, SyncStatus}; +use crate::primitives::{NullifierMap, OutPointMap, SyncStatus}; use crate::scan::error::{ContinuityError, ScanError}; use crate::scan::task::Scanner; use crate::scan::transactions::scan_transaction; @@ -196,9 +196,10 @@ where /// Designed to be called during the sync process with minimal interruption. pub async fn sync_status(wallet: Arc>) -> SyncStatus where - W: SyncWallet, + W: SyncWallet + SyncBlocks, { - let sync_state: SyncState = wallet.lock().await.get_sync_state().unwrap().clone(); + let wallet_guard = wallet.lock().await; + let sync_state = wallet_guard.get_sync_state().unwrap().clone(); let unscanned_blocks = sync_state .scan_ranges() @@ -209,15 +210,39 @@ where acc + (block_range.end - block_range.start) }); let scanned_blocks = sync_state.initial_sync_state().total_blocks_to_scan() - unscanned_blocks; - let percentage_blocks_complete = (scanned_blocks as f32 + let percentage_blocks_scanned = (scanned_blocks as f32 / sync_state.initial_sync_state().total_blocks_to_scan() as f32) * 100.0; + let (unscanned_sapling_outputs, unscanned_orchard_outputs) = + state::calculate_unscanned_outputs(&*wallet_guard); + let scanned_sapling_outputs = sync_state + .initial_sync_state() + .total_sapling_outputs_to_scan() + - unscanned_sapling_outputs; + let scanned_orchard_outputs = sync_state + .initial_sync_state() + .total_orchard_outputs_to_scan() + - unscanned_orchard_outputs; + let percentage_outputs_scanned = ((scanned_sapling_outputs + scanned_orchard_outputs) as f32 + / (sync_state + .initial_sync_state() + .total_sapling_outputs_to_scan() + + sync_state + .initial_sync_state() + .total_orchard_outputs_to_scan()) as f32) + * 100.0; + SyncStatus { scan_ranges: sync_state.scan_ranges().clone(), - unscanned_blocks, scanned_blocks, - percentage_blocks_complete, + unscanned_blocks, + percentage_blocks_scanned, + scanned_sapling_outputs, + unscanned_sapling_outputs, + scanned_orchard_outputs, + unscanned_orchard_outputs, + percentage_outputs_scanned, } } diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 93385b95f..d78ec8e3a 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -12,7 +12,7 @@ use zcash_primitives::{ use crate::{ client::{self, FetchRequest}, keys::transparent::TransparentAddressId, - primitives::{Locator, SyncState}, + primitives::{Locator, SyncState, TreeBoundaries}, scan::task::ScanTask, traits::{SyncBlocks, SyncWallet}, }; @@ -439,89 +439,7 @@ pub(super) async fn set_initial_state( ) where W: SyncWallet + SyncBlocks, { - struct ScannedRangeTreeBoundaries { - sapling_initial_tree_size: u32, - orchard_initial_tree_size: u32, - sapling_final_tree_size: u32, - orchard_final_tree_size: u32, - } - - /// Gets `block_height` final tree sizes from wallet block if it exists, otherwise from frontiers fetched from server. - /// - /// Only used in context of setting initial sync state as can also use the compact blocks to calculate tree sizes - /// during scanning. - async fn final_tree_sizes( - consensus_parameters: &impl consensus::Parameters, - fetch_request_sender: mpsc::UnboundedSender, - wallet: &mut W, - block_height: BlockHeight, - ) -> (u32, u32) - where - W: SyncWallet + SyncBlocks, - { - if let Ok(block) = wallet.get_wallet_block(block_height) { - ( - block.sapling_final_tree_size(), - block.orchard_final_tree_size(), - ) - } else { - // TODO: move this whole block into `client::get_frontiers` - let sapling_activation_height = consensus_parameters - .activation_height(NetworkUpgrade::Sapling) - .expect("should have some sapling activation height"); - - match block_height.cmp(&sapling_activation_height) { - cmp::Ordering::Greater => { - let frontiers = - client::get_frontiers(fetch_request_sender.clone(), block_height) - .await - .unwrap(); - ( - frontiers - .final_sapling_tree() - .tree_size() - .try_into() - .expect("should not be more than 2^32 note commitments in the tree!"), - frontiers - .final_orchard_tree() - .tree_size() - .try_into() - .expect("should not be more than 2^32 note commitments in the tree!"), - ) - } - cmp::Ordering::Equal => (0, 0), - cmp::Ordering::Less => panic!("pre-sapling not supported!"), - } - } - } - - /// Gets the initial and final tree sizes of a `scanned_range`. - /// - /// Panics if `scanned_range` boundary wallet blocks are not found in the wallet. - fn scanned_range_tree_boundaries( - wallet: &mut W, - scanned_range: Range, - ) -> ScannedRangeTreeBoundaries - where - W: SyncWallet + SyncBlocks, - { - let start_block = wallet - .get_wallet_block(scanned_range.start) - .expect("scanned range boundary blocks should be retained in the wallet"); - let end_block = wallet - .get_wallet_block(scanned_range.end - 1) - .expect("scanned range boundary blocks should be retained in the wallet"); - - ScannedRangeTreeBoundaries { - sapling_initial_tree_size: start_block.sapling_initial_tree_size(), - orchard_initial_tree_size: start_block.orchard_initial_tree_size(), - sapling_final_tree_size: end_block.sapling_final_tree_size(), - orchard_final_tree_size: end_block.orchard_final_tree_size(), - } - } - let fully_scanned_height = wallet.get_sync_state().unwrap().fully_scanned_height(); - let (sync_start_sapling_tree_size, sync_start_orchard_tree_size) = final_tree_sizes( consensus_parameters, fetch_request_sender.clone(), @@ -537,7 +455,22 @@ pub(super) async fn set_initial_state( ) .await; - let sync_state = wallet.get_sync_state().unwrap(); + let initial_sync_state = wallet + .get_sync_state_mut() + .unwrap() + .initial_sync_state_mut(); + initial_sync_state.set_sync_start_height(fully_scanned_height + 1); + initial_sync_state.set_sync_tree_boundaries(TreeBoundaries { + sapling_initial_tree_size: sync_start_sapling_tree_size, + sapling_final_tree_size: chain_tip_sapling_tree_size, + orchard_initial_tree_size: sync_start_orchard_tree_size, + orchard_final_tree_size: chain_tip_orchard_tree_size, + }); + + let (total_sapling_outputs_to_scan, total_orchard_outputs_to_scan) = + calculate_unscanned_outputs(wallet); + + let sync_state = wallet.get_sync_state_mut().unwrap(); let total_blocks_to_scan = sync_state .scan_ranges() .iter() @@ -547,37 +480,125 @@ pub(super) async fn set_initial_state( acc + (block_range.end - block_range.start) }); - let scanned_block_ranges = sync_state + let initial_sync_state = sync_state.initial_sync_state_mut(); + initial_sync_state.set_total_blocks_to_scan(total_blocks_to_scan); + initial_sync_state.set_total_sapling_outputs_to_scan(total_sapling_outputs_to_scan); + initial_sync_state.set_total_orchard_outputs_to_scan(total_orchard_outputs_to_scan); +} + +pub(super) fn calculate_unscanned_outputs(wallet: &W) -> (u32, u32) +where + W: SyncWallet + SyncBlocks, +{ + let sync_state = wallet.get_sync_state().unwrap(); + let sync_start_height = sync_state.initial_sync_state().sync_start_height(); + + let nonlinear_scanned_block_ranges = sync_state .scan_ranges() .iter() .filter(|scan_range| { scan_range.priority() == ScanPriority::Scanned - && scan_range.block_range().start > fully_scanned_height + && scan_range.block_range().start >= sync_start_height }) .map(|scan_range| scan_range.block_range().clone()) .collect::>(); - let (scanned_sapling_outputs, scanned_orchard_outputs) = scanned_block_ranges - .iter() - .map(|block_range| scanned_range_tree_boundaries(wallet, block_range.clone())) - .fold((0, 0), |acc, tree_sizes| { - ( - acc.0 + (tree_sizes.sapling_final_tree_size - tree_sizes.sapling_initial_tree_size), - acc.1 + (tree_sizes.orchard_final_tree_size - tree_sizes.orchard_initial_tree_size), - ) - }); - let total_sapling_outputs_to_scan = - chain_tip_sapling_tree_size - sync_start_sapling_tree_size - scanned_sapling_outputs; - let total_orchard_outputs_to_scan = - chain_tip_orchard_tree_size - sync_start_orchard_tree_size - scanned_orchard_outputs; + let (nonlinear_scanned_sapling_outputs, nonlinear_scanned_orchard_outputs) = + nonlinear_scanned_block_ranges + .iter() + .map(|block_range| scanned_range_tree_boundaries(wallet, block_range.clone())) + .fold((0, 0), |acc, tree_boundaries| { + ( + acc.0 + + (tree_boundaries.sapling_final_tree_size + - tree_boundaries.sapling_initial_tree_size), + acc.1 + + (tree_boundaries.orchard_final_tree_size + - tree_boundaries.orchard_initial_tree_size), + ) + }); + + let initial_sync_state = wallet.get_sync_state().unwrap().initial_sync_state(); + let unscanned_sapling_outputs = initial_sync_state + .sync_tree_boundaries() + .sapling_final_tree_size + - initial_sync_state + .sync_tree_boundaries() + .sapling_initial_tree_size + - nonlinear_scanned_sapling_outputs; + let unscanned_orchard_outputs = initial_sync_state + .sync_tree_boundaries() + .orchard_final_tree_size + - initial_sync_state + .sync_tree_boundaries() + .orchard_initial_tree_size + - nonlinear_scanned_orchard_outputs; + + (unscanned_sapling_outputs, unscanned_orchard_outputs) +} - let initial_sync_state = wallet - .get_sync_state_mut() - .unwrap() - .initial_sync_state_mut(); - initial_sync_state.set_sync_start_height(fully_scanned_height + 1); - initial_sync_state.set_sync_start_sapling_tree_size(sync_start_sapling_tree_size); - initial_sync_state.set_sync_start_orchard_tree_size(sync_start_orchard_tree_size); - initial_sync_state.set_total_blocks_to_scan(total_blocks_to_scan); - initial_sync_state.set_total_sapling_outputs_to_scan(total_sapling_outputs_to_scan); - initial_sync_state.set_total_orchard_outputs_to_scan(total_orchard_outputs_to_scan); +/// Gets `block_height` final tree sizes from wallet block if it exists, otherwise from frontiers fetched from server. +async fn final_tree_sizes( + consensus_parameters: &impl consensus::Parameters, + fetch_request_sender: mpsc::UnboundedSender, + wallet: &mut W, + block_height: BlockHeight, +) -> (u32, u32) +where + W: SyncBlocks, +{ + if let Ok(block) = wallet.get_wallet_block(block_height) { + ( + block.tree_boundaries().sapling_final_tree_size, + block.tree_boundaries().orchard_final_tree_size, + ) + } else { + // TODO: move this whole block into `client::get_frontiers` + let sapling_activation_height = consensus_parameters + .activation_height(NetworkUpgrade::Sapling) + .expect("should have some sapling activation height"); + + match block_height.cmp(&sapling_activation_height) { + cmp::Ordering::Greater => { + let frontiers = client::get_frontiers(fetch_request_sender.clone(), block_height) + .await + .unwrap(); + ( + frontiers + .final_sapling_tree() + .tree_size() + .try_into() + .expect("should not be more than 2^32 note commitments in the tree!"), + frontiers + .final_orchard_tree() + .tree_size() + .try_into() + .expect("should not be more than 2^32 note commitments in the tree!"), + ) + } + cmp::Ordering::Equal => (0, 0), + cmp::Ordering::Less => panic!("pre-sapling not supported!"), + } + } +} + +/// Gets the initial and final tree sizes of a `scanned_range`. +/// +/// Panics if `scanned_range` boundary wallet blocks are not found in the wallet. +fn scanned_range_tree_boundaries(wallet: &W, scanned_range: Range) -> TreeBoundaries +where + W: SyncBlocks, +{ + let start_block = wallet + .get_wallet_block(scanned_range.start) + .expect("scanned range boundary blocks should be retained in the wallet"); + let end_block = wallet + .get_wallet_block(scanned_range.end - 1) + .expect("scanned range boundary blocks should be retained in the wallet"); + + TreeBoundaries { + sapling_initial_tree_size: start_block.tree_boundaries().sapling_initial_tree_size, + sapling_final_tree_size: end_block.tree_boundaries().sapling_final_tree_size, + orchard_initial_tree_size: start_block.tree_boundaries().orchard_initial_tree_size, + orchard_final_tree_size: end_block.tree_boundaries().orchard_final_tree_size, + } } From a18ec666d2fe1c09b50283e0ea75db3bc8009a07 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 16 Jan 2025 05:36:35 +0000 Subject: [PATCH 33/37] format --- zingo-sync/src/primitives.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zingo-sync/src/primitives.rs b/zingo-sync/src/primitives.rs index db00bd67a..4f584f967 100644 --- a/zingo-sync/src/primitives.rs +++ b/zingo-sync/src/primitives.rs @@ -66,6 +66,7 @@ impl Default for InitialSyncState { Self::new() } } + /// Encapsulates the current state of sync #[derive(Debug, Clone, Getters, MutGetters, CopyGetters, Setters)] #[getset(get = "pub", get_mut = "pub")] From 23f4442a1a3d24a9d3899736e01e0a1b4b42996a Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 16 Jan 2025 08:31:39 +0000 Subject: [PATCH 34/37] retain all scanned ranges boundary blocks in the wallet --- zingo-sync/src/sync.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index b0ae46aa3..984e07c95 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -502,15 +502,34 @@ where let sync_state = wallet.get_sync_state().unwrap(); let fully_scanned_height = sync_state.fully_scanned_height(); let highest_scanned_height = sync_state.highest_scanned_height(); + let sync_start_height = sync_state.initial_sync_state().sync_start_height(); + + let scanned_block_range_boundaries = sync_state + .scan_ranges() + .iter() + .filter(|scan_range| { + scan_range.priority() == ScanPriority::Scanned + && scan_range.block_range().start >= sync_start_height + }) + .flat_map(|scan_range| { + vec![ + scan_range.block_range().start, + scan_range.block_range().end - 1, + ] + }) + .collect::>(); + let wallet_transaction_heights = wallet .get_wallet_transactions() .unwrap() .values() .filter_map(|tx| tx.confirmation_status().get_confirmed_height()) .collect::>(); + wallet.get_wallet_blocks_mut().unwrap().retain(|height, _| { - *height >= fully_scanned_height - 1 + *height >= sync_start_height - 1 || *height >= highest_scanned_height - MAX_VERIFICATION_WINDOW + || scanned_block_range_boundaries.contains(height) || wallet_transaction_heights.contains(height) }); wallet From 45c95e6366e485a89b462176df0959c4c6d895e7 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Thu, 16 Jan 2025 08:52:38 +0000 Subject: [PATCH 35/37] fix overflow bugs --- zingo-sync/src/sync.rs | 9 ++++++--- zingo-sync/src/sync/state.rs | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index f90c00994..69f5f4ada 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -209,7 +209,10 @@ where .fold(0, |acc, block_range| { acc + (block_range.end - block_range.start) }); - let scanned_blocks = sync_state.initial_sync_state().total_blocks_to_scan() - unscanned_blocks; + let scanned_blocks = sync_state + .initial_sync_state() + .total_blocks_to_scan() + .saturating_sub(unscanned_blocks); let percentage_blocks_scanned = (scanned_blocks as f32 / sync_state.initial_sync_state().total_blocks_to_scan() as f32) * 100.0; @@ -219,11 +222,11 @@ where let scanned_sapling_outputs = sync_state .initial_sync_state() .total_sapling_outputs_to_scan() - - unscanned_sapling_outputs; + .saturating_sub(unscanned_sapling_outputs); let scanned_orchard_outputs = sync_state .initial_sync_state() .total_orchard_outputs_to_scan() - - unscanned_orchard_outputs; + .saturating_sub(unscanned_orchard_outputs); let percentage_outputs_scanned = ((scanned_sapling_outputs + scanned_orchard_outputs) as f32 / (sync_state .initial_sync_state() diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index d78ec8e3a..783c0e4e1 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -557,7 +557,7 @@ where .activation_height(NetworkUpgrade::Sapling) .expect("should have some sapling activation height"); - match block_height.cmp(&sapling_activation_height) { + match block_height.cmp(&(sapling_activation_height - 1)) { cmp::Ordering::Greater => { let frontiers = client::get_frontiers(fetch_request_sender.clone(), block_height) .await From 20c0eb41a0f4731a5a8661148430b55fdd56debd Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Fri, 17 Jan 2025 03:25:30 +0000 Subject: [PATCH 36/37] fix case where block height is lower than activation height when determining shard range --- zingo-sync/src/sync.rs | 23 +++++++++--- zingo-sync/src/sync/spend.rs | 2 ++ zingo-sync/src/sync/state.rs | 70 +++++++++++++++++++++++++++++++----- 3 files changed, 82 insertions(+), 13 deletions(-) diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 75a53e2bc..193f31490 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -114,6 +114,7 @@ where .await; state::update_scan_ranges( + consensus_parameters, wallet_height, chain_height, wallet_guard.get_sync_state_mut().unwrap(), @@ -291,7 +292,7 @@ where { match scan_results { Ok(results) => { - update_wallet_data(wallet, results).unwrap(); + update_wallet_data(consensus_parameters, wallet, results).unwrap(); spend::update_transparent_spends(wallet).unwrap(); spend::update_shielded_spends( consensus_parameters, @@ -452,7 +453,11 @@ where } /// Updates the wallet with data from `scan_results` -fn update_wallet_data(wallet: &mut W, scan_results: ScanResults) -> Result<(), ()> +fn update_wallet_data( + consensus_parameters: &impl consensus::Parameters, + wallet: &mut W, + scan_results: ScanResults, +) -> Result<(), ()> where W: SyncBlocks + SyncTransactions + SyncNullifiers + SyncOutPoints + SyncShardTrees, { @@ -467,8 +472,18 @@ where let sync_state = wallet.get_sync_state_mut().unwrap(); for transaction in wallet_transactions.values() { - state::update_found_note_shard_priority(sync_state, ShieldedProtocol::Sapling, transaction); - state::update_found_note_shard_priority(sync_state, ShieldedProtocol::Orchard, transaction); + state::update_found_note_shard_priority( + consensus_parameters, + sync_state, + ShieldedProtocol::Sapling, + transaction, + ); + state::update_found_note_shard_priority( + consensus_parameters, + sync_state, + ShieldedProtocol::Orchard, + transaction, + ); } wallet.append_wallet_blocks(wallet_blocks).unwrap(); diff --git a/zingo-sync/src/sync/spend.rs b/zingo-sync/src/sync/spend.rs index 44925b559..acdb1d638 100644 --- a/zingo-sync/src/sync/spend.rs +++ b/zingo-sync/src/sync/spend.rs @@ -48,12 +48,14 @@ where let sync_state = wallet.get_sync_state_mut().unwrap(); state::set_found_note_scan_ranges( + consensus_parameters, sync_state, ShieldedProtocol::Sapling, sapling_spend_locators.values().cloned(), ) .unwrap(); state::set_found_note_scan_ranges( + consensus_parameters, sync_state, ShieldedProtocol::Orchard, orchard_spend_locators.values().cloned(), diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index bde4946ae..85fc470eb 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -80,6 +80,7 @@ fn find_locators(sync_state: &SyncState, block_range: &Range) -> BT /// Update scan ranges for scanning pub(super) async fn update_scan_ranges( + consensus_parameters: &impl consensus::Parameters, wallet_height: BlockHeight, chain_height: BlockHeight, sync_state: &mut SyncState, @@ -87,8 +88,13 @@ pub(super) async fn update_scan_ranges( reset_scan_ranges(sync_state)?; create_scan_range(wallet_height, chain_height, sync_state).await?; let locators = sync_state.locators().clone(); - set_found_note_scan_ranges(sync_state, ShieldedProtocol::Orchard, locators.into_iter())?; - set_chain_tip_scan_range(sync_state, chain_height)?; + set_found_note_scan_ranges( + consensus_parameters, + sync_state, + ShieldedProtocol::Orchard, + locators.into_iter(), + )?; + set_chain_tip_scan_range(consensus_parameters, sync_state, chain_height)?; // TODO: add logic to merge scan ranges @@ -196,12 +202,13 @@ pub(super) fn set_verify_scan_range( /// Punches in the `shielded_protocol` shard block ranges surrounding each locator with `ScanPriority::FoundNote`. pub(super) fn set_found_note_scan_ranges>( + consensus_parameters: &impl consensus::Parameters, sync_state: &mut SyncState, shielded_protocol: ShieldedProtocol, locators: L, ) -> Result<(), ()> { for locator in locators { - set_found_note_scan_range(sync_state, shielded_protocol, locator)?; + set_found_note_scan_range(consensus_parameters, sync_state, shielded_protocol, locator)?; } Ok(()) @@ -209,12 +216,18 @@ pub(super) fn set_found_note_scan_ranges>( /// Punches in the `shielded_protocol` shard block range surrounding the `locator` with `ScanPriority::FoundNote`. pub(super) fn set_found_note_scan_range( + consensus_parameters: &impl consensus::Parameters, sync_state: &mut SyncState, shielded_protocol: ShieldedProtocol, locator: Locator, ) -> Result<(), ()> { let block_height = locator.0; - let block_range = determine_block_range(sync_state, block_height, shielded_protocol); + let block_range = determine_block_range( + consensus_parameters, + sync_state, + block_height, + shielded_protocol, + ); punch_scan_priority(sync_state, block_range, ScanPriority::FoundNote).unwrap(); Ok(()) @@ -225,13 +238,22 @@ pub(super) fn set_found_note_scan_range( /// Determines the chain tip block range by finding the lowest start height of the latest incomplete shard for each /// shielded protocol. fn set_chain_tip_scan_range( + consensus_parameters: &impl consensus::Parameters, sync_state: &mut SyncState, chain_height: BlockHeight, ) -> Result<(), ()> { - let sapling_incomplete_shard = - determine_block_range(sync_state, chain_height, ShieldedProtocol::Sapling); - let orchard_incomplete_shard = - determine_block_range(sync_state, chain_height, ShieldedProtocol::Orchard); + let sapling_incomplete_shard = determine_block_range( + consensus_parameters, + sync_state, + chain_height, + ShieldedProtocol::Sapling, + ); + let orchard_incomplete_shard = determine_block_range( + consensus_parameters, + sync_state, + chain_height, + ShieldedProtocol::Orchard, + ); let chain_tip = if sapling_incomplete_shard.start < orchard_incomplete_shard.start { sapling_incomplete_shard @@ -327,10 +349,38 @@ fn punch_scan_priority( /// If no shard range exists for the given `block_height`, return the range of the incomplete shard at the chain tip. /// If `block_height` contains note commitments from multiple shards, return the block range of all of those shards combined. fn determine_block_range( + consensus_parameters: &impl consensus::Parameters, sync_state: &SyncState, block_height: BlockHeight, - shielded_protocol: ShieldedProtocol, + mut shielded_protocol: ShieldedProtocol, ) -> Range { + loop { + match shielded_protocol { + ShieldedProtocol::Sapling => { + if block_height + < consensus_parameters + .activation_height(consensus::NetworkUpgrade::Sapling) + .expect("network activation height should be set") + { + panic!("pre-sapling not supported"); + } else { + break; + } + } + ShieldedProtocol::Orchard => { + if block_height + < consensus_parameters + .activation_height(consensus::NetworkUpgrade::Nu5) + .expect("network activation height should be set") + { + shielded_protocol = ShieldedProtocol::Sapling; + } else { + break; + } + } + } + } + let shard_ranges = match shielded_protocol { ShieldedProtocol::Sapling => sync_state.sapling_shard_ranges(), ShieldedProtocol::Orchard => sync_state.orchard_shard_ranges(), @@ -747,6 +797,7 @@ pub(super) fn add_shard_ranges( /// Updates the `shielded_protocol` shard range to `FoundNote` scan priority if the `wallet_transaction` contains /// a note from the corresponding `shielded_protocol`. pub(super) fn update_found_note_shard_priority( + consensus_parameters: &impl consensus::Parameters, sync_state: &mut SyncState, shielded_protocol: ShieldedProtocol, wallet_transaction: &WalletTransaction, @@ -757,6 +808,7 @@ pub(super) fn update_found_note_shard_priority( }; if found_note { set_found_note_scan_range( + consensus_parameters, sync_state, shielded_protocol, ( From 93394884adf3dcc4394f7e51c9afff01b0df0b12 Mon Sep 17 00:00:00 2001 From: Oscar Pepper Date: Fri, 17 Jan 2025 04:54:28 +0000 Subject: [PATCH 37/37] small cleanup --- zingo-sync/src/client/fetch.rs | 6 ++++-- zingo-sync/src/sync.rs | 3 +-- zingo-sync/src/sync/state.rs | 5 ----- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/zingo-sync/src/client/fetch.rs b/zingo-sync/src/client/fetch.rs index 4cb364e2d..61d9bdc08 100644 --- a/zingo-sync/src/client/fetch.rs +++ b/zingo-sync/src/client/fetch.rs @@ -169,6 +169,7 @@ async fn get_latest_block( Ok(client.get_latest_block(request).await.unwrap().into_inner()) } + async fn get_block_range( client: &mut CompactTxStreamerClient, block_range: Range, @@ -183,9 +184,8 @@ async fn get_block_range( hash: vec![], }), }); - let block_stream = client.get_block_range(request).await.unwrap().into_inner(); - Ok(block_stream) + Ok(client.get_block_range(request).await.unwrap().into_inner()) } async fn get_subtree_roots( @@ -199,12 +199,14 @@ async fn get_subtree_roots( shielded_protocol, max_entries, }; + Ok(client .get_subtree_roots(request) .await .unwrap() .into_inner()) } + async fn get_tree_state( client: &mut CompactTxStreamerClient, block_height: BlockHeight, diff --git a/zingo-sync/src/sync.rs b/zingo-sync/src/sync.rs index 50eab338b..3cdfe5078 100644 --- a/zingo-sync/src/sync.rs +++ b/zingo-sync/src/sync.rs @@ -37,13 +37,12 @@ pub(crate) mod spend; pub(crate) mod state; pub(crate) mod transparent; -// TODO: move parameters to config module const VERIFY_BLOCK_RANGE_SIZE: u32 = 10; const MAX_VERIFICATION_WINDOW: u32 = 100; /// Syncs a wallet to the latest state of the blockchain pub async fn sync( - client: CompactTxStreamerClient, // TODO: change underlying service for generic + client: CompactTxStreamerClient, consensus_parameters: &P, wallet: Arc>, ) -> Result<(), SyncError> diff --git a/zingo-sync/src/sync/state.rs b/zingo-sync/src/sync/state.rs index 9a5ad09c8..5b31c4c4c 100644 --- a/zingo-sync/src/sync/state.rs +++ b/zingo-sync/src/sync/state.rs @@ -64,7 +64,6 @@ where } /// Returns the locators for a given `block_range` from the wallet's [`crate::primitives::SyncState`] -// TODO: unit test high priority fn find_locators(sync_state: &SyncState, block_range: &Range) -> BTreeSet { sync_state .locators() @@ -76,8 +75,6 @@ fn find_locators(sync_state: &SyncState, block_range: &Range) -> BT .collect() } -// TODO: remove locators after range is scanned - /// Update scan ranges for scanning pub(super) async fn update_scan_ranges( consensus_parameters: &impl consensus::Parameters, @@ -148,8 +145,6 @@ fn reset_scan_ranges(sync_state: &mut SyncState) -> Result<(), ()> { set_scan_priority(sync_state, scan_range.block_range(), ScanPriority::Verify).unwrap(); } - // TODO: determine OpenAdjacent priority ranges from the end block of previous ChainTip ranges - Ok(()) }