diff --git a/substrate/core/network/src/sync.rs b/substrate/core/network/src/sync.rs index 2666c49823bf27f073aa3ece05953cf1a6bfdfd4..a4dc1855ef00aa42073fc319d072d230c9401f13 100644 --- a/substrate/core/network/src/sync.rs +++ b/substrate/core/network/src/sync.rs @@ -27,7 +27,7 @@ use consensus::import_queue::{ImportQueue, IncomingBlock}; use client::error::Error as ClientError; use crate::blocks::BlockCollection; use runtime_primitives::Justification; -use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, As, NumberFor, Zero}; +use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, As, NumberFor, Zero, CheckedSub}; use runtime_primitives::generic::BlockId; use crate::message::{self, generic::Message as GenericMessage}; use crate::config::Roles; @@ -58,9 +58,19 @@ struct PeerSync<B: BlockT> { pub recently_announced: VecDeque<B::Hash>, } +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +enum AncestorSearchState<B: BlockT> { + /// Use exponential backoff to find an ancestor, then switch to binary search. + /// We keep track of the exponent. + ExponentialBackoff(NumberFor<B>), + /// Using binary search to find the best ancestor. + /// We keep track of left and right bounds. + BinarySearch(NumberFor<B>, NumberFor<B>), +} + #[derive(Copy, Clone, Eq, PartialEq, Debug)] enum PeerSyncState<B: BlockT> { - AncestorSearch(NumberFor<B>), + AncestorSearch(NumberFor<B>, AncestorSearchState<B>), Available, DownloadingNew(NumberFor<B>), DownloadingStale(B::Hash), @@ -138,9 +148,9 @@ impl<B: BlockT> PendingJustifications<B> { peer_best_number >= request.1 && !self.previous_requests - .get(&request) - .map(|requests| requests.iter().any(|i| i.0 == peer)) - .unwrap_or(false) + .get(&request) + .map(|requests| requests.iter().any(|i| i.0 == peer)) + .unwrap_or(false) }; if !peer_eligible { @@ -228,9 +238,9 @@ impl<B: BlockT> PendingJustifications<B> { if success { if self.justifications.finalize_root(&request.0).is_none() { warn!(target: "sync", "Imported justification for {:?} {:?} which isn't a root in the tree: {:?}", - request.0, - request.1, - self.justifications.roots().collect::<Vec<_>>(), + request.0, + request.1, + self.justifications.roots().collect::<Vec<_>>(), ); return; }; @@ -419,7 +429,8 @@ impl<B: BlockT> ChainSync<B> { let previous_state = self.state(&previous_best_seen); if let Some(info) = protocol.peer_info(who) { - match (block_status(&*protocol.client(), &self.queue_blocks, info.best_hash), info.best_number) { + let status = block_status(&*protocol.client(), &self.queue_blocks, info.best_hash); + match (status, info.best_number) { (Err(e), _) => { debug!(target:"sync", "Error reading blockchain: {:?}", e); let reason = format!("Error legimimately reading blockchain status: {:?}", e); @@ -453,7 +464,7 @@ impl<B: BlockT> ChainSync<B> { common_number: As::sa(0), best_hash: info.best_hash, best_number: info.best_number, - state: PeerSyncState::AncestorSearch(common_best), + state: PeerSyncState::AncestorSearch(common_best, AncestorSearchState::ExponentialBackoff(As::sa(1))), recently_announced: Default::default(), }); Self::request_ancestry(protocol, who, common_best) @@ -500,6 +511,44 @@ impl<B: BlockT> ChainSync<B> { } } + fn handle_ancestor_search_state( + state: AncestorSearchState<B>, + curr_block_num: NumberFor<B>, + block_hash_match: bool, + ) -> Option<(AncestorSearchState<B>, NumberFor<B>)> { + match state { + AncestorSearchState::ExponentialBackoff(next_distance_to_tip) => { + if block_hash_match && next_distance_to_tip == As::sa(1) { + // We found the ancestor in the first step so there is no need to execute binary search. + return None; + } + if block_hash_match { + let left = curr_block_num; + let right = left + next_distance_to_tip / As::sa(2); + let middle = left + (right - left) / As::sa(2); + Some((AncestorSearchState::BinarySearch(left, right), middle)) + } else { + let next_block_num = curr_block_num.checked_sub(&next_distance_to_tip).unwrap_or(As::sa(0)); + let next_distance_to_tip = next_distance_to_tip * As::sa(2); + Some((AncestorSearchState::ExponentialBackoff(next_distance_to_tip), next_block_num)) + } + }, + AncestorSearchState::BinarySearch(mut left, mut right) => { + if left >= curr_block_num { + return None; + } + if block_hash_match { + left = curr_block_num; + } else { + right = curr_block_num; + } + assert!(right >= left); + let middle = left + (right - left) / As::sa(2); + Some((AncestorSearchState::BinarySearch(left, right), middle)) + }, + } + } + /// Handle new block data. pub(crate) fn on_block_data( &mut self, @@ -514,7 +563,8 @@ impl<B: BlockT> ChainSync<B> { trace!(target: "sync", "Reversing incoming block list"); blocks.reverse(); } - match peer.state { + let peer_state = peer.state.clone(); + match peer_state { PeerSyncState::DownloadingNew(start_block) => { self.blocks.clear_peer_download(who); peer.state = PeerSyncState::Available; @@ -544,43 +594,38 @@ impl<B: BlockT> ChainSync<B> { } }).collect() }, - PeerSyncState::AncestorSearch(n) => { - match blocks.get(0) { - Some(ref block) => { - trace!(target: "sync", "Got ancestry block #{} ({}) from peer {}", n, block.hash, who); - match protocol.client().block_hash(n) { - Ok(Some(block_hash)) if block_hash == block.hash => { - if peer.common_number < n { - peer.common_number = n; - } - peer.state = PeerSyncState::Available; - trace!(target:"sync", "Found common ancestor for peer {}: {} ({})", who, block.hash, n); - vec![] - }, - Ok(our_best) if n > As::sa(0) => { - trace!(target:"sync", "Ancestry block mismatch for peer {}: theirs: {} ({}), ours: {:?}", who, block.hash, n, our_best); - let n = n - As::sa(1); - peer.state = PeerSyncState::AncestorSearch(n); - Self::request_ancestry(protocol, who, n); - return; - }, - Ok(_) => { // genesis mismatch - trace!(target:"sync", "Ancestry search: genesis mismatch for peer {}", who); - protocol.report_peer(who, Severity::Bad("Ancestry search: genesis mismatch for peer".to_string())); - return; - }, - Err(e) => { - let reason = format!("Error answering legitimate blockchain query: {:?}", e); - protocol.report_peer(who, Severity::Useless(reason)); - return; - } - } + PeerSyncState::AncestorSearch(num, state) => { + let block_hash_match = match (blocks.get(0), protocol.client().block_hash(num)) { + (Some(ref block), Ok(maybe_our_block_hash)) => { + trace!(target: "sync", "Got ancestry block #{} ({}) from peer {}", num, block.hash, who); + maybe_our_block_hash.map_or(false, |x| x == block.hash) }, - None => { + (None, _) => { trace!(target:"sync", "Invalid response when searching for ancestor from {}", who); protocol.report_peer(who, Severity::Bad("Invalid response when searching for ancestor".to_string())); return; - } + }, + (_, Err(e)) => { + let reason = format!("Error answering legitimate blockchain query: {:?}", e); + protocol.report_peer(who, Severity::Useless(reason)); + return; + }, + }; + if block_hash_match && peer.common_number < num { + peer.common_number = num; + } + if !block_hash_match && num == As::sa(0) { + trace!(target:"sync", "Ancestry search: genesis mismatch for peer {}", who); + protocol.report_peer(who, Severity::Bad("Ancestry search: genesis mismatch for peer".to_string())); + return; + } + if let Some((next_state, next_block_num)) = Self::handle_ancestor_search_state(state, num, block_hash_match) { + peer.state = PeerSyncState::AncestorSearch(next_block_num, next_state); + Self::request_ancestry(protocol, who, next_block_num); + return; + } else { + peer.state = PeerSyncState::Available; + vec![] } }, PeerSyncState::Available | PeerSyncState::DownloadingJustification(..) => Vec::new(), @@ -745,7 +790,7 @@ impl<B: BlockT> ChainSync<B> { } // Update common blocks for (n, peer) in self.peers.iter_mut() { - if let PeerSyncState::AncestorSearch(_) = peer.state { + if let PeerSyncState::AncestorSearch(_, _) = peer.state { // Abort search. peer.state = PeerSyncState::Available; } @@ -782,7 +827,7 @@ impl<B: BlockT> ChainSync<B> { peer.best_number = number; peer.best_hash = hash; } - if let PeerSyncState::AncestorSearch(_) = peer.state { + if let PeerSyncState::AncestorSearch(_, _) = peer.state { return; } if header.parent_hash() == &self.best_queued_hash || known_parent { diff --git a/substrate/core/network/src/test/sync.rs b/substrate/core/network/src/test/sync.rs index 74cc30ed6424e6b286ec2026f7fb20dbcf54ece6..4a2f3339fba82b9af7ba1523ca4b423b40cd5990 100644 --- a/substrate/core/network/src/test/sync.rs +++ b/substrate/core/network/src/test/sync.rs @@ -24,6 +24,24 @@ use std::thread; use std::time::Duration; use super::*; +fn test_ancestor_search_when_common_is(n: usize) { + let _ = ::env_logger::try_init(); + let mut net = TestNet::new(3); + + net.peer(0).push_blocks(n, false); + net.peer(1).push_blocks(n, false); + net.peer(2).push_blocks(n, false); + + net.peer(0).push_blocks(10, true); + net.peer(1).push_blocks(100, false); + net.peer(2).push_blocks(100, false); + + net.restart_peer(0); + net.sync(); + assert!(net.peer(0).client.backend().as_in_memory().blockchain() + .canon_equals_to(net.peer(1).client.backend().as_in_memory().blockchain())); +} + #[test] fn sync_peers_works() { let _ = ::env_logger::try_init(); @@ -140,6 +158,66 @@ fn sync_from_two_peers_with_ancestry_search_works() { .canon_equals_to(net.peer(1).client.backend().as_in_memory().blockchain())); } +#[test] +fn ancestry_search_works_when_common_is_hundred() { + let _ = ::env_logger::try_init(); + let mut net = TestNet::new(3); + + net.peer(0).push_blocks(100, false); + net.peer(1).push_blocks(100, false); + net.peer(2).push_blocks(100, false); + + net.peer(0).push_blocks(10, true); + net.peer(1).push_blocks(100, false); + net.peer(2).push_blocks(100, false); + + net.restart_peer(0); + net.sync(); + + assert!(net.peer(0).client.backend().as_in_memory().blockchain() + .canon_equals_to(net.peer(1).client.backend().as_in_memory().blockchain())); +} + +#[test] +fn ancestry_search_works_when_backoff_is_one() { + let _ = ::env_logger::try_init(); + let mut net = TestNet::new(3); + + net.peer(0).push_blocks(1, false); + net.peer(1).push_blocks(2, false); + net.peer(2).push_blocks(2, false); + + net.restart_peer(0); + net.sync(); + assert!(net.peer(0).client.backend().as_in_memory().blockchain() + .canon_equals_to(net.peer(1).client.backend().as_in_memory().blockchain())); +} + +#[test] +fn ancestry_search_works_when_ancestor_is_genesis() { + let _ = ::env_logger::try_init(); + let mut net = TestNet::new(3); + + net.peer(0).push_blocks(13, true); + net.peer(1).push_blocks(100, false); + net.peer(2).push_blocks(100, false); + + net.restart_peer(0); + net.sync(); + assert!(net.peer(0).client.backend().as_in_memory().blockchain() + .canon_equals_to(net.peer(1).client.backend().as_in_memory().blockchain())); +} + +#[test] +fn ancestry_search_works_when_common_is_one() { + test_ancestor_search_when_common_is(1); +} + +#[test] +fn ancestry_search_works_when_common_is_two() { + test_ancestor_search_when_common_is(2); +} + #[test] fn sync_long_chain_works() { let mut net = TestNet::new(2);