From bee40e155365dccfc24d83c9f49c990604a9e55b Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Fri, 26 Aug 2022 01:59:40 +0300 Subject: [PATCH 1/6] Actually fix major sync detection --- client/network/common/src/sync.rs | 2 ++ client/network/src/service.rs | 13 +++++++------ client/network/sync/src/lib.rs | 22 +++++++++++----------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index 2ee8f8c51814b..a7c22ce76e73a 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -67,6 +67,8 @@ pub struct SyncStatus { pub state: SyncState, /// Target sync block number. pub best_seen_block: Option>, + /// Are we actively catching up with the chain? + pub is_major_syncing: bool, /// Number of peers participating in syncing. pub num_peers: u32, /// Number of blocks queued for import diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 162b9c6a30491..1656d38179a9b 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -71,7 +71,7 @@ use sc_network_common::{ NotificationSender as NotificationSenderT, NotificationSenderError, NotificationSenderReady as NotificationSenderReadyT, Signature, SigningError, }, - sync::{SyncState, SyncStatus}, + sync::SyncStatus, }; use sc_peerset::PeersetHandle; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; @@ -1938,11 +1938,12 @@ where *this.external_addresses.lock() = external_addresses; } - let is_major_syncing = - match this.network_service.behaviour_mut().user_protocol_mut().sync_state().state { - SyncState::Idle => false, - SyncState::Downloading => true, - }; + let is_major_syncing = this + .network_service + .behaviour_mut() + .user_protocol_mut() + .sync_state() + .is_major_syncing; this.tx_handler_controller.set_gossip_enabled(!is_major_syncing); diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index aae5f4de353fe..5c0d207b240f3 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -389,8 +389,10 @@ where /// Returns the current sync status. fn status(&self) -> SyncStatus { - let best_seen = self.best_seen(); - let sync_state = if let Some(n) = best_seen { + let median_seen = self.median_seen(); + let best_seen_block = + median_seen.and_then(|median| (median > self.best_queued_number).then_some(median)); + let sync_state = if let Some(n) = median_seen { // A chain is classified as downloading if the provided best block is // more than `MAJOR_SYNC_BLOCKS` behind the best block. let best_block = self.client.info().best_number; @@ -414,9 +416,12 @@ where _ => None, }; + let is_major_syncing = matches!(sync_state, SyncState::Downloading); + SyncStatus { state: sync_state, - best_seen_block: best_seen, + best_seen_block, + is_major_syncing, num_peers: self.peers.len() as u32, queued_blocks: self.queue_blocks.len() as u32, state_sync: self.state_sync.as_ref().map(|s| s.progress()), @@ -1719,8 +1724,8 @@ where Ok(sync) } - /// Returns the best seen block number if we don't have that block yet, `None` otherwise. - fn best_seen(&self) -> Option> { + /// Returns the median seen block number. + fn median_seen(&self) -> Option> { let mut best_seens = self.peers.values().map(|p| p.best_number).collect::>(); if best_seens.is_empty() { @@ -1729,12 +1734,7 @@ where let middle = best_seens.len() / 2; // Not the "perfect median" when we have an even number of peers. - let median = *best_seens.select_nth_unstable(middle).1; - if median > self.best_queued_number { - Some(median) - } else { - None - } + Some(*best_seens.select_nth_unstable(middle).1) } } From 912eb7a8abb96eae3687191f46ef1fb00c6ee67b Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Mon, 29 Aug 2022 16:22:22 +0300 Subject: [PATCH 2/6] Introduce `SyncState::Importing` state --- client/informant/src/display.rs | 3 ++- client/network/common/src/sync.rs | 2 ++ client/network/sync/src/lib.rs | 14 ++++++++++---- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/client/informant/src/display.rs b/client/informant/src/display.rs index 0441011b0ec42..6895e7f2fa392 100644 --- a/client/informant/src/display.rs +++ b/client/informant/src/display.rs @@ -126,8 +126,9 @@ impl InformantDisplay { (SyncState::Idle, _, _, _) => ("💤", "Idle".into(), "".into()), (SyncState::Downloading, None, _, _) => ("⚙️ ", format!("Preparing{}", speed), "".into()), - (SyncState::Downloading, Some(n), None, _) => + (SyncState::Downloading, Some(n), _, _) => ("⚙️ ", format!("Syncing{}", speed), format!(", target=#{}", n)), + (SyncState::Importing, _, _, _) => ("⚙️ ", format!("Importing{}", speed), "".into()), }; if self.format.enable_color { diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index a7c22ce76e73a..99c6433793b36 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -49,6 +49,8 @@ pub enum SyncState { Idle, /// Actively catching up with the chain. Downloading, + /// All blocks are downloaded and are being imported. + Importing, } /// Reported state download progress. diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 5c0d207b240f3..8860ff4357c8c 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -392,12 +392,18 @@ where let median_seen = self.median_seen(); let best_seen_block = median_seen.and_then(|median| (median > self.best_queued_number).then_some(median)); - let sync_state = if let Some(n) = median_seen { + let sync_state = if let Some(n) = best_seen_block.or(median_seen) { // A chain is classified as downloading if the provided best block is - // more than `MAJOR_SYNC_BLOCKS` behind the best block. + // more than `MAJOR_SYNC_BLOCKS` behind the best block or as importing + // if the same can be said about queued blocks. let best_block = self.client.info().best_number; if n > best_block && n - best_block > MAJOR_SYNC_BLOCKS.into() { - SyncState::Downloading + // If target is not queued, we're downloading, otherwise importing. + if n > self.best_queued_number { + SyncState::Downloading + } else { + SyncState::Importing + } } else { SyncState::Idle } @@ -416,7 +422,7 @@ where _ => None, }; - let is_major_syncing = matches!(sync_state, SyncState::Downloading); + let is_major_syncing = !matches!(sync_state, SyncState::Idle); SyncStatus { state: sync_state, From 03df7d59bcc013d5e57a5c0350072890e118c587 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 6 Sep 2022 13:17:34 +0300 Subject: [PATCH 3/6] Add target to SyncState enum variants and add `is_major_syncing` method on it --- client/informant/src/display.rs | 64 +++++++++++++--------------- client/network/common/src/service.rs | 2 +- client/network/common/src/sync.rs | 17 +++++--- client/network/src/service.rs | 3 +- client/network/sync/src/lib.rs | 19 ++++----- 5 files changed, 51 insertions(+), 54 deletions(-) diff --git a/client/informant/src/display.rs b/client/informant/src/display.rs index 6895e7f2fa392..3d585a9985134 100644 --- a/client/informant/src/display.rs +++ b/client/informant/src/display.rs @@ -93,43 +93,37 @@ impl InformantDisplay { (diff_bytes_inbound, diff_bytes_outbound) }; - let (level, status, target) = match ( - net_status.sync_state, - net_status.best_seen_block, - net_status.state_sync, - net_status.warp_sync, - ) { - ( - _, - _, - _, - Some(WarpSyncProgress { phase: WarpSyncPhase::DownloadingBlocks(n), .. }), - ) => ("⏩", "Block history".into(), format!(", #{}", n)), - (_, _, _, Some(warp)) => ( - "⏩", - "Warping".into(), - format!( - ", {}, {:.2} Mib", - warp.phase, - (warp.total_bytes as f32) / (1024f32 * 1024f32) + let (level, status, target) = + match (net_status.sync_state, net_status.state_sync, net_status.warp_sync) { + ( + _, + _, + Some(WarpSyncProgress { phase: WarpSyncPhase::DownloadingBlocks(n), .. }), + ) => ("⏩", "Block history".into(), format!(", #{}", n)), + (_, _, Some(warp)) => ( + "⏩", + "Warping".into(), + format!( + ", {}, {:.2} Mib", + warp.phase, + (warp.total_bytes as f32) / (1024f32 * 1024f32) + ), ), - ), - (_, _, Some(state), _) => ( - "⚙️ ", - "Downloading state".into(), - format!( - ", {}%, {:.2} Mib", - state.percentage, - (state.size as f32) / (1024f32 * 1024f32) + (_, Some(state), _) => ( + "⚙️ ", + "Downloading state".into(), + format!( + ", {}%, {:.2} Mib", + state.percentage, + (state.size as f32) / (1024f32 * 1024f32) + ), ), - ), - (SyncState::Idle, _, _, _) => ("💤", "Idle".into(), "".into()), - (SyncState::Downloading, None, _, _) => - ("⚙️ ", format!("Preparing{}", speed), "".into()), - (SyncState::Downloading, Some(n), _, _) => - ("⚙️ ", format!("Syncing{}", speed), format!(", target=#{}", n)), - (SyncState::Importing, _, _, _) => ("⚙️ ", format!("Importing{}", speed), "".into()), - }; + (SyncState::Idle, _, _) => ("💤", "Idle".into(), "".into()), + (SyncState::Downloading { target }, _, _) => + ("⚙️ ", format!("Syncing{}", speed), format!(", target=#{target}")), + (SyncState::Importing { target }, _, _) => + ("⚙️ ", format!("Preparing{}", speed), format!(", target=#{target}")), + }; if self.format.enable_color { info!( diff --git a/client/network/common/src/service.rs b/client/network/common/src/service.rs index 0fff32b12d66c..281bc6bea0152 100644 --- a/client/network/common/src/service.rs +++ b/client/network/common/src/service.rs @@ -97,7 +97,7 @@ where #[derive(Clone)] pub struct NetworkStatus { /// Current global sync state. - pub sync_state: SyncState, + pub sync_state: SyncState>, /// Target sync block number. pub best_seen_block: Option>, /// Number of peers participating in syncing. diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index 99c6433793b36..df12705985870 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -44,13 +44,20 @@ pub struct PeerInfo { /// Reported sync state. #[derive(Clone, Eq, PartialEq, Debug)] -pub enum SyncState { +pub enum SyncState { /// Initial sync is complete, keep-up sync is active. Idle, /// Actively catching up with the chain. - Downloading, + Downloading { target: BlockNumber }, /// All blocks are downloaded and are being imported. - Importing, + Importing { target: BlockNumber }, +} + +impl SyncState { + /// Are we actively catching up with the chain? + pub fn is_major_syncing(&self) -> bool { + !matches!(self, SyncState::Idle) + } } /// Reported state download progress. @@ -66,11 +73,9 @@ pub struct StateDownloadProgress { #[derive(Clone)] pub struct SyncStatus { /// Current global sync state. - pub state: SyncState, + pub state: SyncState>, /// Target sync block number. pub best_seen_block: Option>, - /// Are we actively catching up with the chain? - pub is_major_syncing: bool, /// Number of peers participating in syncing. pub num_peers: u32, /// Number of blocks queued for import diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 1656d38179a9b..75077d18bec0b 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -1943,7 +1943,8 @@ where .behaviour_mut() .user_protocol_mut() .sync_state() - .is_major_syncing; + .state + .is_major_syncing(); this.tx_handler_controller.set_gossip_enabled(!is_major_syncing); diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 8860ff4357c8c..a686500eb44dd 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -392,17 +392,17 @@ where let median_seen = self.median_seen(); let best_seen_block = median_seen.and_then(|median| (median > self.best_queued_number).then_some(median)); - let sync_state = if let Some(n) = best_seen_block.or(median_seen) { + let sync_state = if let Some(target) = median_seen { // A chain is classified as downloading if the provided best block is // more than `MAJOR_SYNC_BLOCKS` behind the best block or as importing // if the same can be said about queued blocks. let best_block = self.client.info().best_number; - if n > best_block && n - best_block > MAJOR_SYNC_BLOCKS.into() { + if target > best_block && target - best_block > MAJOR_SYNC_BLOCKS.into() { // If target is not queued, we're downloading, otherwise importing. - if n > self.best_queued_number { - SyncState::Downloading + if target > self.best_queued_number { + SyncState::Downloading { target } } else { - SyncState::Importing + SyncState::Importing { target } } } else { SyncState::Idle @@ -422,12 +422,9 @@ where _ => None, }; - let is_major_syncing = !matches!(sync_state, SyncState::Idle); - SyncStatus { state: sync_state, best_seen_block, - is_major_syncing, num_peers: self.peers.len() as u32, queued_blocks: self.queue_blocks.len() as u32, state_sync: self.state_sync.as_ref().map(|s| s.progress()), @@ -681,7 +678,7 @@ where trace!(target: "sync", "Too many blocks in the queue."); return Box::new(std::iter::empty()) } - let major_sync = self.status().state == SyncState::Downloading; + let is_major_syncing = self.status().state.is_major_syncing(); let attrs = self.required_block_attributes(); let blocks = &mut self.blocks; let fork_targets = &mut self.fork_targets; @@ -691,7 +688,7 @@ where let client = &self.client; let queue = &self.queue_blocks; let allowed_requests = self.allowed_requests.take(); - let max_parallel = if major_sync { 1 } else { self.max_parallel_downloads }; + let max_parallel = if is_major_syncing { 1 } else { self.max_parallel_downloads }; let gap_sync = &mut self.gap_sync; let iter = self.peers.iter_mut().filter_map(move |(id, peer)| { if !peer.state.is_available() || !allowed_requests.contains(id) { @@ -1782,7 +1779,7 @@ where ); } - let origin = if !gap && self.status().state != SyncState::Downloading { + let origin = if !gap && !self.status().state.is_major_syncing() { BlockOrigin::NetworkBroadcast } else { BlockOrigin::NetworkInitialSync From bb8abd458c939881c049f69d59f3acba47c97c5c Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 6 Sep 2022 13:32:18 +0300 Subject: [PATCH 4/6] Remove unnecessary duplicated `best_seen_block` from `SyncState` struct --- client/network/common/src/sync.rs | 2 -- client/network/src/protocol.rs | 8 ++++++-- client/network/sync/src/lib.rs | 3 --- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index df12705985870..19c1d7c35920e 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -74,8 +74,6 @@ pub struct StateDownloadProgress { pub struct SyncStatus { /// Current global sync state. pub state: SyncState>, - /// Target sync block number. - pub best_seen_block: Option>, /// Number of peers participating in syncing. pub num_peers: u32, /// Number of blocks queued for import diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 351e7d207ad1e..101ea7c34f450 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -52,7 +52,7 @@ use sc_network_common::{ warp::{EncodedProof, WarpProofRequest}, BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, OpaqueBlockRequest, OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PollBlockAnnounceValidation, - SyncStatus, + SyncState, SyncStatus, }, }; use sp_arithmetic::traits::SaturatedConversion; @@ -501,7 +501,11 @@ where /// Target sync block number. pub fn best_seen_block(&self) -> Option> { - self.chain_sync.status().best_seen_block + match self.chain_sync.status().state { + SyncState::Idle => None, + SyncState::Downloading { target } => Some(target), + SyncState::Importing { .. } => None, + } } /// Number of peers participating in syncing. diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index a686500eb44dd..49de6fd9268e1 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -390,8 +390,6 @@ where /// Returns the current sync status. fn status(&self) -> SyncStatus { let median_seen = self.median_seen(); - let best_seen_block = - median_seen.and_then(|median| (median > self.best_queued_number).then_some(median)); let sync_state = if let Some(target) = median_seen { // A chain is classified as downloading if the provided best block is // more than `MAJOR_SYNC_BLOCKS` behind the best block or as importing @@ -424,7 +422,6 @@ where SyncStatus { state: sync_state, - best_seen_block, num_peers: self.peers.len() as u32, queued_blocks: self.queue_blocks.len() as u32, state_sync: self.state_sync.as_ref().map(|s| s.progress()), From a45fd528a84aee13b78de3817b661788c9838719 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 18 Oct 2022 11:35:20 +0300 Subject: [PATCH 5/6] Revert "Remove unnecessary duplicated `best_seen_block` from `SyncState` struct" This reverts commit bb8abd458c939881c049f69d59f3acba47c97c5c. --- client/network/common/src/sync.rs | 2 ++ client/network/src/protocol.rs | 8 ++------ client/network/sync/src/lib.rs | 3 +++ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index 19c1d7c35920e..df12705985870 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -74,6 +74,8 @@ pub struct StateDownloadProgress { pub struct SyncStatus { /// Current global sync state. pub state: SyncState>, + /// Target sync block number. + pub best_seen_block: Option>, /// Number of peers participating in syncing. pub num_peers: u32, /// Number of blocks queued for import diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index a56fccf846c13..9bcf363cd0692 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -53,7 +53,7 @@ use sc_network_common::{ warp::{EncodedProof, WarpProofRequest}, BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, OpaqueBlockRequest, OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PollBlockAnnounceValidation, - SyncState, SyncStatus, + SyncStatus, }, }; use sp_arithmetic::traits::SaturatedConversion; @@ -494,11 +494,7 @@ where /// Target sync block number. pub fn best_seen_block(&self) -> Option> { - match self.chain_sync.status().state { - SyncState::Idle => None, - SyncState::Downloading { target } => Some(target), - SyncState::Importing { .. } => None, - } + self.chain_sync.status().best_seen_block } /// Number of peers participating in syncing. diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 49de6fd9268e1..a686500eb44dd 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -390,6 +390,8 @@ where /// Returns the current sync status. fn status(&self) -> SyncStatus { let median_seen = self.median_seen(); + let best_seen_block = + median_seen.and_then(|median| (median > self.best_queued_number).then_some(median)); let sync_state = if let Some(target) = median_seen { // A chain is classified as downloading if the provided best block is // more than `MAJOR_SYNC_BLOCKS` behind the best block or as importing @@ -422,6 +424,7 @@ where SyncStatus { state: sync_state, + best_seen_block, num_peers: self.peers.len() as u32, queued_blocks: self.queue_blocks.len() as u32, state_sync: self.state_sync.as_ref().map(|s| s.progress()), From 1e4e550dfa3f7c3851a86a9133989d27f8167e30 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 18 Oct 2022 11:44:59 +0300 Subject: [PATCH 6/6] Add missing `websocket` feature to `libp2p` --- client/network/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/network/Cargo.toml b/client/network/Cargo.toml index 9c2833ec00908..81b684af433d2 100644 --- a/client/network/Cargo.toml +++ b/client/network/Cargo.toml @@ -26,7 +26,7 @@ fnv = "1.0.6" futures = "0.3.21" futures-timer = "3.0.2" ip_network = "0.4.1" -libp2p = { version = "0.49.0", features = ["async-std", "dns", "identify", "kad", "mdns-async-io", "mplex", "noise", "ping", "tcp", "yamux"] } +libp2p = { version = "0.49.0", features = ["async-std", "dns", "identify", "kad", "mdns-async-io", "mplex", "noise", "ping", "tcp", "yamux", "websocket"] } linked_hash_set = "0.1.3" linked-hash-map = "0.5.4" log = "0.4.17"