[beta] Backports (#8785)

* Fix light sync with initial validator-set contract (#8528) * Fix #8468 * Use U256::max_value() instead * Fix again * Also change initial transaction gas * Resumable warp-sync / Seed downloaded snapshots (#8544) * Start dividing sync chain : first supplier method * WIP - updated chain sync supplier * Finish refactoring the Chain Sync Supplier * Create Chain Sync Requester * Add Propagator for Chain Sync * Add the Chain Sync Handler * Move tests from mod -> handler * Move tests to propagator * Refactor SyncRequester arguments * Refactoring peer fork header handler * Fix wrong highest block number in snapshot sync * Small refactor... * Resume warp-sync downloaded chunks * Add comments * Refactoring the previous chunks import * Fix tests * Address PR grumbles * Fix not seeding current snapshot * Address PR Grumbles * Address PR grumble * Retry failed CI job * Update SnapshotService readiness check Fix restoration locking issue for previous chunks restoration * Fix tests * Fix tests * Fix test * Early abort importing previous chunks * PR Grumbles * Update Gitlab CI config * SyncState back to Waiting when Manifest peers disconnect * Move fix * Better fix * Revert GitLab CI changes * Fix Warning * Refactor resuming snapshots * Fix string construction * Revert "Refactor resuming snapshots" This reverts commit 75fd4b553a38e4a49dc5d6a878c70e830ff382eb. * Update informant log * Fix string construction * Refactor resuming snapshots * Fix informant * PR Grumbles * Update informant message : show chunks done * PR Grumbles * Fix * Fix Warning * PR Grumbles * Don't open Browser post-install on Mac (#8641) Since we start parity with the UI disabled per default now, opening the browser post installation will show an annoying error message, confusing the user. This patch removes opening the browser to prevent that annoyance. fixes #8194 * Fix not downloading old blocks (#8642) * Fix PoW blockchains sealing notifications in chain_new_blocks (#8656) * Shutdown the Snapshot Service early (#8658) * Shutdown the Snapshot Service when shutting down the runner * Rename `service` to `client_service` * Fix tests * Fix cli signer (#8682) * Update ethereum-types so `{:#x}` applies 0x prefix * Set the request index to that of the current request (#8683) * Set the request index to that of the current request When setting up the chain of (two) requests to look up a block by hash, the second need to refer to the first. This fixes an issue where the back ref was set to the subsequent request, not the current one. When the requests are executed we loop through them in order and ensure the requests that should produce headers all match up. We do this by index so they better be right. In other words: off by one. * network-devp2p: handle UselessPeer disconnect (#8686) * Fix local transactions policy. (#8691) * CI: Fixes for Android Pipeline (#8745) * ci: Remove check for shared libraries in gitlab script * ci: allow android arm build to fail * Custom Error Messages on ENFILE and EMFILE IO Errors (#8744) * Custom Error Messages on ENFILE and EMFILE IO Errors Add custom mapping of ENFILE and EMFILE IO Errors (Failure because of missing system resource) right when chaining ioError into ::util::Network::Error to improve Error Messages given to user Note: Adds libc as a dependency to util/network * Use assert-matches for more readable tests * Fix Wording and consistency * ethcore-sync: fix connection to peers behind chain fork block (#8710)
2018-06-04 21:29:20 +01:00
parent 69d6d8239f
commit 7bf16cce1b
30 changed files with 706 additions and 265 deletions
--- a/ethcore/sync/src/blocks.rs
+++ b/ethcore/sync/src/blocks.rs
@@ -266,7 +266,7 @@ impl BlockCollection {
 		}
 	}

-	/// Get a valid chain of blocks ordered in descending order and ready for importing into blockchain.
+	/// Get a valid chain of blocks ordered in ascending order and ready for importing into blockchain.
 	pub fn drain(&mut self) -> Vec<BlockAndReceipts> {
 		if self.blocks.is_empty() || self.head.is_none() {
 			return Vec::new();
--- a/ethcore/sync/src/chain/handler.rs
+++ b/ethcore/sync/src/chain/handler.rs
@@ -100,14 +100,27 @@ impl SyncHandler {
 	}

 	/// Called by peer when it is disconnecting
-	pub fn on_peer_aborting(sync: &mut ChainSync, io: &mut SyncIo, peer: PeerId) {
-		trace!(target: "sync", "== Disconnecting {}: {}", peer, io.peer_info(peer));
-		sync.handshaking_peers.remove(&peer);
-		if sync.peers.contains_key(&peer) {
-			debug!(target: "sync", "Disconnected {}", peer);
-			sync.clear_peer_download(peer);
-			sync.peers.remove(&peer);
-			sync.active_peers.remove(&peer);
+	pub fn on_peer_aborting(sync: &mut ChainSync, io: &mut SyncIo, peer_id: PeerId) {
+		trace!(target: "sync", "== Disconnecting {}: {}", peer_id, io.peer_info(peer_id));
+		sync.handshaking_peers.remove(&peer_id);
+		if sync.peers.contains_key(&peer_id) {
+			debug!(target: "sync", "Disconnected {}", peer_id);
+			sync.clear_peer_download(peer_id);
+			sync.peers.remove(&peer_id);
+			sync.active_peers.remove(&peer_id);
+
+			if sync.state == SyncState::SnapshotManifest {
+				// Check if we are asking other peers for
+				// the snapshot manifest as well.
+				// If not, return to initial state
+				let still_asking_manifest = sync.peers.iter()
+					.filter(|&(id, p)| sync.active_peers.contains(id) && p.asking == PeerAsking::SnapshotManifest)
+					.next().is_none();
+
+				if still_asking_manifest {
+					sync.state = ChainSync::get_init_state(sync.warp_sync, io.chain());
+				}
+			}
 			sync.continue_sync(io);
 		}
 	}
@@ -319,10 +332,6 @@ impl SyncHandler {
 		Ok(())
 	}

-	fn on_peer_confirmed(sync: &mut ChainSync, io: &mut SyncIo, peer_id: PeerId) {
-		sync.sync_peer(io, peer_id, false);
-	}
-
 	fn on_peer_fork_header(sync: &mut ChainSync, io: &mut SyncIo, peer_id: PeerId, r: &Rlp) -> Result<(), PacketDecodeError> {
 		{
 			let peer = sync.peers.get_mut(&peer_id).expect("Is only called when peer is present in peers");
@@ -332,24 +341,27 @@ impl SyncHandler {

 			if item_count == 0 || item_count != 1 {
 				trace!(target: "sync", "{}: Chain is too short to confirm the block", peer_id);
-				io.disable_peer(peer_id);
-				return Ok(());
-			}
+				peer.confirmation = ForkConfirmation::TooShort;

-			let header = r.at(0)?.as_raw();
-			if keccak(&header) != fork_hash {
-				trace!(target: "sync", "{}: Fork mismatch", peer_id);
-				io.disable_peer(peer_id);
-				return Ok(());
-			}
+			} else {
+				let header = r.at(0)?.as_raw();
+				if keccak(&header) != fork_hash {
+					trace!(target: "sync", "{}: Fork mismatch", peer_id);
+					io.disable_peer(peer_id);
+					return Ok(());
+				}

-			trace!(target: "sync", "{}: Confirmed peer", peer_id);
-			peer.confirmation = ForkConfirmation::Confirmed;
-			if !io.chain_overlay().read().contains_key(&fork_number) {
-				io.chain_overlay().write().insert(fork_number, header.to_vec());
+				trace!(target: "sync", "{}: Confirmed peer", peer_id);
+				peer.confirmation = ForkConfirmation::Confirmed;
+
+				if !io.chain_overlay().read().contains_key(&fork_number) {
+					trace!(target: "sync", "Inserting (fork) block {} header", fork_number);
+					io.chain_overlay().write().insert(fork_number, header.to_vec());
+				}
 			}
 		}
-		SyncHandler::on_peer_confirmed(sync, io, peer_id);
+
+		sync.sync_peer(io, peer_id, false);
 		return Ok(());
 	}

@@ -560,6 +572,10 @@ impl SyncHandler {
 				sync.continue_sync(io);
 				return Ok(());
 			},
+			RestorationStatus::Initializing  { .. } => {
+				trace!(target: "warp", "{}: Snapshot restoration is initializing", peer_id);
+				return Ok(());
+			}
 			RestorationStatus::Ongoing { .. } => {
 				trace!(target: "sync", "{}: Snapshot restoration is ongoing", peer_id);
 			},
@@ -659,11 +675,18 @@ impl SyncHandler {
 		// Let the current sync round complete first.
 		sync.active_peers.insert(peer_id.clone());
 		debug!(target: "sync", "Connected {}:{}", peer_id, io.peer_info(peer_id));
-		if let Some((fork_block, _)) = sync.fork_block {
-			SyncRequester::request_fork_header(sync, io, peer_id, fork_block);
-		} else {
-			SyncHandler::on_peer_confirmed(sync, io, peer_id);
+
+		match sync.fork_block {
+			Some((fork_block, _)) => {
+				SyncRequester::request_fork_header(sync, io, peer_id, fork_block);
+			},
+			_ => {
+				// when there's no `fork_block` defined we initialize the peer with
+				// `confirmation: ForkConfirmation::Confirmed`.
+				sync.sync_peer(io, peer_id, false);
+			}
 		}
+
 		Ok(())
 	}

--- a/ethcore/sync/src/chain/mod.rs
+++ b/ethcore/sync/src/chain/mod.rs
@@ -245,9 +245,12 @@ pub struct SyncStatus {
 impl SyncStatus {
 	/// Indicates if snapshot download is in progress
 	pub fn is_snapshot_syncing(&self) -> bool {
-		self.state == SyncState::SnapshotManifest
-			|| self.state == SyncState::SnapshotData
-			|| self.state == SyncState::SnapshotWaiting
+		match self.state {
+			SyncState::SnapshotManifest |
+				SyncState::SnapshotData |
+				SyncState::SnapshotWaiting => true,
+			_ => false,
+		}
 	}

 	/// Returns max no of peers to display in informants
@@ -291,6 +294,8 @@ pub enum BlockSet {
 pub enum ForkConfirmation {
 	/// Fork block confirmation pending.
 	Unconfirmed,
+	/// Peer's chain is too short to confirm the fork.
+	TooShort,
 	/// Fork is confirmed.
 	Confirmed,
 }
@@ -643,7 +648,7 @@ impl ChainSync {
 		}
 	}

-	/// Resume  downloading
+	/// Resume downloading
 	fn continue_sync(&mut self, io: &mut SyncIo) {
 		// Collect active peers that can sync
 		let confirmed_peers: Vec<(PeerId, u8)> = self.peers.iter().filter_map(|(peer_id, peer)|
@@ -751,26 +756,45 @@ impl ChainSync {
 						}
 					}

-					if let Some(request) = self.old_blocks.as_mut().and_then(|d| d.request_blocks(io, num_active_peers)) {
-						SyncRequester::request_blocks(self, io, peer_id, request, BlockSet::OldBlocks);
-						return;
+					// Only ask for old blocks if the peer has a higher difficulty
+					if force || higher_difficulty {
+						if let Some(request) = self.old_blocks.as_mut().and_then(|d| d.request_blocks(io, num_active_peers)) {
+							SyncRequester::request_blocks(self, io, peer_id, request, BlockSet::OldBlocks);
+							return;
+						}
+					} else {
+						trace!(target: "sync", "peer {} is not suitable for asking old blocks", peer_id);
+						self.deactivate_peer(io, peer_id);
 					}
 				},
 				SyncState::SnapshotData => {
-					if let RestorationStatus::Ongoing { state_chunks_done, block_chunks_done, .. } = io.snapshot_service().status() {
-						if self.snapshot.done_chunks() - (state_chunks_done + block_chunks_done) as usize > MAX_SNAPSHOT_CHUNKS_DOWNLOAD_AHEAD {
-							trace!(target: "sync", "Snapshot queue full, pausing sync");
-							self.state = SyncState::SnapshotWaiting;
+					match io.snapshot_service().status() {
+						RestorationStatus::Ongoing { state_chunks_done, block_chunks_done, .. } => {
+							// Initialize the snapshot if not already done
+							self.snapshot.initialize(io.snapshot_service());
+							if self.snapshot.done_chunks() - (state_chunks_done + block_chunks_done) as usize > MAX_SNAPSHOT_CHUNKS_DOWNLOAD_AHEAD {
+								trace!(target: "sync", "Snapshot queue full, pausing sync");
+								self.state = SyncState::SnapshotWaiting;
+								return;
+							}
+						},
+						RestorationStatus::Initializing { .. } => {
+							trace!(target: "warp", "Snapshot is stil initializing.");
 							return;
-						}
+						},
+						_ => {
+							return;
+						},
 					}
+
 					if peer_snapshot_hash.is_some() && peer_snapshot_hash == self.snapshot.snapshot_hash() {
 						self.clear_peer_download(peer_id);
 						SyncRequester::request_snapshot_data(self, io, peer_id);
 					}
 				},
 				SyncState::SnapshotManifest | //already downloading from other peer
-					SyncState::Waiting | SyncState::SnapshotWaiting => ()
+					SyncState::Waiting |
+					SyncState::SnapshotWaiting => ()
 			}
 		} else {
 			trace!(target: "sync", "Skipping peer {}, force={}, td={:?}, our td={}, state={:?}", peer_id, force, peer_difficulty, syncing_difficulty, self.state);
@@ -861,10 +885,7 @@ impl ChainSync {
 		packet.append(&chain.best_block_hash);
 		packet.append(&chain.genesis_hash);
 		if warp_protocol {
-			let manifest = match self.old_blocks.is_some() {
-				true => None,
-				false => io.snapshot_service().manifest(),
-			};
+			let manifest = io.snapshot_service().manifest();
 			let block_number = manifest.as_ref().map_or(0, |m| m.block_number);
 			let manifest_hash = manifest.map_or(H256::new(), |m| keccak(m.into_rlp()));
 			packet.append(&manifest_hash);
@@ -908,29 +929,36 @@ impl ChainSync {
 	}

 	fn check_resume(&mut self, io: &mut SyncIo) {
-		if self.state == SyncState::Waiting && !io.chain().queue_info().is_full() {
-			self.state = SyncState::Blocks;
-			self.continue_sync(io);
-		} else if self.state == SyncState::SnapshotWaiting {
-			match io.snapshot_service().status() {
-				RestorationStatus::Inactive => {
-					trace!(target:"sync", "Snapshot restoration is complete");
-					self.restart(io);
-				},
-				RestorationStatus::Ongoing { state_chunks_done, block_chunks_done, .. } => {
-					if !self.snapshot.is_complete() && self.snapshot.done_chunks() - (state_chunks_done + block_chunks_done) as usize <= MAX_SNAPSHOT_CHUNKS_DOWNLOAD_AHEAD {
-						trace!(target:"sync", "Resuming snapshot sync");
-						self.state = SyncState::SnapshotData;
+		match self.state {
+			SyncState::Waiting if !io.chain().queue_info().is_full() => {
+				self.state = SyncState::Blocks;
+				self.continue_sync(io);
+			},
+			SyncState::SnapshotWaiting => {
+				match io.snapshot_service().status() {
+					RestorationStatus::Inactive => {
+						trace!(target:"sync", "Snapshot restoration is complete");
+						self.restart(io);
+					},
+					RestorationStatus::Initializing { .. } => {
+						trace!(target:"sync", "Snapshot restoration is initializing");
+					},
+					RestorationStatus::Ongoing { state_chunks_done, block_chunks_done, .. } => {
+						if !self.snapshot.is_complete() && self.snapshot.done_chunks() - (state_chunks_done + block_chunks_done) as usize <= MAX_SNAPSHOT_CHUNKS_DOWNLOAD_AHEAD {
+							trace!(target:"sync", "Resuming snapshot sync");
+							self.state = SyncState::SnapshotData;
+							self.continue_sync(io);
+						}
+					},
+					RestorationStatus::Failed => {
+						trace!(target: "sync", "Snapshot restoration aborted");
+						self.state = SyncState::WaitingPeers;
+						self.snapshot.clear();
 						self.continue_sync(io);
-					}
-				},
-				RestorationStatus::Failed => {
-					trace!(target: "sync", "Snapshot restoration aborted");
-					self.state = SyncState::WaitingPeers;
-					self.snapshot.clear();
-					self.continue_sync(io);
-				},
-			}
+					},
+				}
+			},
+			_ => (),
 		}
 	}

--- a/ethcore/sync/src/chain/supplier.rs
+++ b/ethcore/sync/src/chain/supplier.rs
@@ -120,8 +120,9 @@ impl SyncSupplier {
 				None => return Ok(Some((BLOCK_HEADERS_PACKET, RlpStream::new_list(0)))) //no such header, return nothing
 			}
 		} else {
-			trace!(target: "sync", "{} -> GetBlockHeaders (number: {}, max: {}, skip: {}, reverse:{})", peer_id, r.val_at::<BlockNumber>(0)?, max_headers, skip, reverse);
-			r.val_at(0)?
+			let number = r.val_at::<BlockNumber>(0)?;
+			trace!(target: "sync", "{} -> GetBlockHeaders (number: {}, max: {}, skip: {}, reverse:{})", peer_id, number, max_headers, skip, reverse);
+			number
 		};

 		let mut number = if reverse {
@@ -135,7 +136,10 @@ impl SyncSupplier {
 		let inc = (skip + 1) as BlockNumber;
 		let overlay = io.chain_overlay().read();

-		while number <= last && count < max_count {
+		// We are checking the `overlay` as well since it's where the ForkBlock
+		// header is cached : so peers can confirm we are on the right fork,
+		// even if we are not synced until the fork block
+		while (number <= last || overlay.contains_key(&number)) && count < max_count {
 			if let Some(hdr) = overlay.get(&number) {
 				trace!(target: "sync", "{}: Returning cached fork header", peer_id);
 				data.extend_from_slice(hdr);
@@ -152,8 +156,7 @@ impl SyncSupplier {
 					break;
 				}
 				number -= inc;
-			}
-			else {
+			} else {
 				number += inc;
 			}
 		}
@@ -237,20 +240,20 @@ impl SyncSupplier {
 	/// Respond to GetSnapshotManifest request
 	fn return_snapshot_manifest(io: &SyncIo, r: &Rlp, peer_id: PeerId) -> RlpResponseResult {
 		let count = r.item_count().unwrap_or(0);
-		trace!(target: "sync", "{} -> GetSnapshotManifest", peer_id);
+		trace!(target: "warp", "{} -> GetSnapshotManifest", peer_id);
 		if count != 0 {
-			debug!(target: "sync", "Invalid GetSnapshotManifest request, ignoring.");
+			debug!(target: "warp", "Invalid GetSnapshotManifest request, ignoring.");
 			return Ok(None);
 		}
 		let rlp = match io.snapshot_service().manifest() {
 			Some(manifest) => {
-				trace!(target: "sync", "{} <- SnapshotManifest", peer_id);
+				trace!(target: "warp", "{} <- SnapshotManifest", peer_id);
 				let mut rlp = RlpStream::new_list(1);
 				rlp.append_raw(&manifest.into_rlp(), 1);
 				rlp
 			},
 			None => {
-				trace!(target: "sync", "{}: No manifest to return", peer_id);
+				trace!(target: "warp", "{}: No snapshot manifest to return", peer_id);
 				RlpStream::new_list(0)
 			}
 		};
@@ -260,15 +263,16 @@ impl SyncSupplier {
 	/// Respond to GetSnapshotData request
 	fn return_snapshot_data(io: &SyncIo, r: &Rlp, peer_id: PeerId) -> RlpResponseResult {
 		let hash: H256 = r.val_at(0)?;
-		trace!(target: "sync", "{} -> GetSnapshotData {:?}", peer_id, hash);
+		trace!(target: "warp", "{} -> GetSnapshotData {:?}", peer_id, hash);
 		let rlp = match io.snapshot_service().chunk(hash) {
 			Some(data) => {
 				let mut rlp = RlpStream::new_list(1);
-				trace!(target: "sync", "{} <- SnapshotData", peer_id);
+				trace!(target: "warp", "{} <- SnapshotData", peer_id);
 				rlp.append(&data);
 				rlp
 			},
 			None => {
+				trace!(target: "warp", "{}: No snapshot data to return", peer_id);
 				RlpStream::new_list(0)
 			}
 		};
--- a/ethcore/sync/src/snapshot.rs
+++ b/ethcore/sync/src/snapshot.rs
@@ -14,10 +14,13 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.

-use hash::keccak;
+use ethcore::snapshot::{ManifestData, SnapshotService};
 use ethereum_types::H256;
+use hash::keccak;
+use rand::{thread_rng, Rng};
+
 use std::collections::HashSet;
-use ethcore::snapshot::ManifestData;
+use std::iter::FromIterator;

 #[derive(PartialEq, Eq, Debug)]
 pub enum ChunkType {
@@ -32,6 +35,7 @@ pub struct Snapshot {
 	completed_chunks: HashSet<H256>,
 	snapshot_hash: Option<H256>,
 	bad_hashes: HashSet<H256>,
+	initialized: bool,
 }

 impl Snapshot {
@@ -44,9 +48,29 @@ impl Snapshot {
 			completed_chunks: HashSet::new(),
 			snapshot_hash: None,
 			bad_hashes: HashSet::new(),
+			initialized: false,
 		}
 	}

+	/// Sync the Snapshot completed chunks with the Snapshot Service
+	pub fn initialize(&mut self, snapshot_service: &SnapshotService) {
+		if self.initialized {
+			return;
+		}
+
+		if let Some(completed_chunks) = snapshot_service.completed_chunks() {
+			self.completed_chunks = HashSet::from_iter(completed_chunks);
+		}
+
+		trace!(
+			target: "snapshot",
+			"Snapshot is now initialized with {} completed chunks.",
+			self.completed_chunks.len(),
+		);
+
+		self.initialized = true;
+	}
+
 	/// Clear everything.
 	pub fn clear(&mut self) {
 		self.pending_state_chunks.clear();
@@ -54,6 +78,7 @@ impl Snapshot {
 		self.downloading_chunks.clear();
 		self.completed_chunks.clear();
 		self.snapshot_hash = None;
+		self.initialized = false;
 	}

 	/// Check if currently downloading a snapshot.
@@ -89,18 +114,35 @@ impl Snapshot {
 		Err(())
 	}

-	/// Find a chunk to download
+	/// Find a random chunk to download
 	pub fn needed_chunk(&mut self) -> Option<H256> {
-		// check state chunks first
-		let chunk = self.pending_state_chunks.iter()
-			.chain(self.pending_block_chunks.iter())
-			.find(|&h| !self.downloading_chunks.contains(h) && !self.completed_chunks.contains(h))
-			.cloned();
+		// Find all random chunks: first blocks, then state
+		let needed_chunks = {
+			let chunk_filter = |h| !self.downloading_chunks.contains(h) && !self.completed_chunks.contains(h);
+
+			let needed_block_chunks = self.pending_block_chunks.iter()
+				.filter(|&h| chunk_filter(h))
+				.map(|h| *h)
+				.collect::<Vec<H256>>();
+
+			// If no block chunks to download, get the state chunks
+			if needed_block_chunks.len() == 0 {
+				self.pending_state_chunks.iter()
+					.filter(|&h| chunk_filter(h))
+					.map(|h| *h)
+					.collect::<Vec<H256>>()
+			} else {
+				needed_block_chunks
+			}
+		};
+
+		// Get a random chunk
+		let chunk = thread_rng().choose(&needed_chunks);

 		if let Some(hash) = chunk {
 			self.downloading_chunks.insert(hash.clone());
 		}
-		chunk
+		chunk.map(|h| *h)
 	}

 	pub fn clear_chunk_download(&mut self, hash: &H256) {
@@ -185,8 +227,15 @@ mod test {

 		let requested: Vec<H256> = (0..40).map(|_| snapshot.needed_chunk().unwrap()).collect();
 		assert!(snapshot.needed_chunk().is_none());
-		assert_eq!(&requested[0..20], &manifest.state_hashes[..]);
-		assert_eq!(&requested[20..40], &manifest.block_hashes[..]);
+
+		let requested_all_block_chunks = manifest.block_hashes.iter()
+			.all(|h| requested.iter().any(|rh| rh == h));
+		assert!(requested_all_block_chunks);
+
+		let requested_all_state_chunks = manifest.state_hashes.iter()
+			.all(|h| requested.iter().any(|rh| rh == h));
+		assert!(requested_all_state_chunks);
+
 		assert_eq!(snapshot.downloading_chunks.len(), 40);

 		assert_eq!(snapshot.validate_chunk(&state_chunks[4]), Ok(ChunkType::State(manifest.state_hashes[4].clone())));
--- a/ethcore/sync/src/tests/snapshot.rs
+++ b/ethcore/sync/src/tests/snapshot.rs
@@ -80,6 +80,10 @@ impl SnapshotService for TestSnapshotService {
 		Some((1, 2))
 	}

+	fn completed_chunks(&self) -> Option<Vec<H256>> {
+		Some(vec![])
+	}
+
 	fn chunk(&self, hash: H256) -> Option<Bytes> {
 		self.chunks.get(&hash).cloned()
 	}
@@ -129,6 +133,10 @@ impl SnapshotService for TestSnapshotService {
 			self.block_restoration_chunks.lock().insert(hash, chunk);
 		}
 	}
+
+	fn shutdown(&self) {
+		self.abort_restore();
+	}
 }

 #[test]