From 504bcc0caeb329f1e95b46991772b0bc7775f532 Mon Sep 17 00:00:00 2001 From: Arkadiy Paronyan <arkady.paronyan@gmail.com> Date: Thu, 21 Jun 2018 15:12:49 +0200 Subject: [PATCH] Fixed block download sequence (#223) --- substrate/substrate/network/src/blocks.rs | 23 +++++++++++++++++---- substrate/substrate/network/src/protocol.rs | 1 + substrate/substrate/network/src/sync.rs | 6 ++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/substrate/substrate/network/src/blocks.rs b/substrate/substrate/network/src/blocks.rs index 49b7c18508a..f5b32ad3873 100644 --- a/substrate/substrate/network/src/blocks.rs +++ b/substrate/substrate/network/src/blocks.rs @@ -109,7 +109,7 @@ impl<B: BlockT> BlockCollection<B> where B::Header: HeaderT<Number=u64> { &(Some((start, &BlockRangeState::Downloading { ref len, downloading })), _) if downloading < MAX_PARALLEL_DOWNLOADS => (*start .. *start + *len, downloading), &(Some((start, r)), Some((next_start, _))) if start + r.len() < *next_start => - (*start + r.len() .. cmp::min(*next_start, *start + count), 0), // gap + (*start + r.len() .. cmp::min(*next_start, *start + r.len() + count), 0), // gap &(Some((start, r)), None) => (start + r.len() .. start + r.len() + count, 0), // last range &(None, None) => @@ -123,16 +123,17 @@ impl<B: BlockT> BlockCollection<B> where B::Header: HeaderT<Number=u64> { } } }; - // crop to peers best if range.start > peer_best { trace!(target: "sync", "Out of range for peer {} ({} vs {})", peer_id, range.start, peer_best); return None; } range.end = cmp::min(peer_best + 1, range.end); - self.peer_requests.insert(peer_id, range.start); self.blocks.insert(range.start, BlockRangeState::Downloading{ len: range.end - range.start, downloading: downloading + 1 }); + if range.end <= range.start { + panic!("Empty range {:?}, count={}, peer_best={}, common={}, blocks={:?}", range, count, peer_best, common, self.blocks); + } Some(range) } @@ -189,7 +190,7 @@ impl<B: BlockT> BlockCollection<B> where B::Header: HeaderT<Number=u64> { #[cfg(test)] mod test { - use super::{BlockCollection, BlockData}; + use super::{BlockCollection, BlockData, BlockRangeState}; use message; use runtime_primitives::testing::Block as RawBlock; use primitives::H256; @@ -264,4 +265,18 @@ mod test { assert_eq!(drained[..40], blocks[81..121].iter().map(|b| BlockData { block: b.clone(), origin: 2 }).collect::<Vec<_>>()[..]); assert_eq!(drained[40..], blocks[121..150].iter().map(|b| BlockData { block: b.clone(), origin: 1 }).collect::<Vec<_>>()[..]); } + + #[test] + fn large_gap() { + let mut bc: BlockCollection<Block> = BlockCollection::new(); + bc.blocks.insert(100, BlockRangeState::Downloading { + len: 128, + downloading: 1, + }); + let blocks = generate_blocks(10).into_iter().map(|b| BlockData { block: b, origin: 0 }).collect(); + bc.blocks.insert(114305, BlockRangeState::Complete(blocks)); + + assert_eq!(bc.needed_blocks(0, 128, 10000, 000), Some(1 .. 100)); + assert_eq!(bc.needed_blocks(0, 128, 10000, 600), Some(100 + 128 .. 100 + 128 + 128)); + } } diff --git a/substrate/substrate/network/src/protocol.rs b/substrate/substrate/network/src/protocol.rs index d49b7be1c81..cf8a7ce7bf7 100644 --- a/substrate/substrate/network/src/protocol.rs +++ b/substrate/substrate/network/src/protocol.rs @@ -282,6 +282,7 @@ impl<B: BlockT> Protocol<B> where id: request.id, blocks: blocks, }; + trace!(target: "sync", "Sending BlockResponse with {} blocks", response.blocks.len()); self.send_message(io, peer, GenericMessage::BlockResponse(response)) } diff --git a/substrate/substrate/network/src/sync.rs b/substrate/substrate/network/src/sync.rs index ce7d6438db0..af4c0897f00 100644 --- a/substrate/substrate/network/src/sync.rs +++ b/substrate/substrate/network/src/sync.rs @@ -192,8 +192,10 @@ impl<B: BlockT> ChainSync<B> where trace!(target: "sync", "Got ancestry block #{} ({}) from peer {}", n, block.hash, peer_id); match protocol.chain().block_hash(n) { Ok(Some(block_hash)) if block_hash == block.hash => { - peer.common_hash = block.hash; - peer.common_number = n; + if peer.common_number < n { + peer.common_hash = block.hash; + peer.common_number = n; + } peer.state = PeerSyncState::Available; trace!(target:"sync", "Found common ancestor for peer {}: {} ({})", peer_id, block.hash, n); vec![] -- GitLab