Move snapshot related traits to their proper place (#11012)

* Move snapshot to own crate
Sort out imports

* WIP cargo toml

* Make snapshotting generic over the client
Sort out tests

* Sort out types from blockchain and client

* Sort out sync

* Sort out imports and generics

* Sort out main binary

* Fix sync test-helpers

* Sort out import for secret-store

* Sort out more imports

* Fix easy todos

* cleanup

* Move SnapshotClient and SnapshotWriter to their proper places
Sort out the circular dependency between snapshot and ethcore by moving all snapshot tests to own crate, snapshot-tests

* cleanup

* Cleanup

* fix merge issues

* Update ethcore/snapshot/snapshot-tests/Cargo.toml

Co-Authored-By: Andronik Ordian <write@reusable.software>

* Sort out botched merge

* Ensure snapshot-tests run

* Docs

* Fix grumbles

Cargo.lock

@@ -4156,6 +4156,7 @@ dependencies = [
  "rand_xorshift 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rlp 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "rlp_derive 0.1.0",
+ "snapshot-tests 0.1.0",
  "spec 0.1.0",
  "state-db 0.1.0",
  "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4164,6 +4165,49 @@ dependencies = [
  "triehash-ethereum 0.2.0",
 ]
 
+[[package]]
+name = "snapshot-tests"
+version = "0.1.0"
+dependencies = [
+ "account-db 0.1.0",
+ "account-state 0.1.0",
+ "client-traits 0.1.0",
+ "common-types 0.1.0",
+ "engine 0.1.0",
+ "env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethabi 8.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethabi-contract 8.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethabi-derive 8.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethcore 1.12.0",
+ "ethcore-accounts 0.1.0",
+ "ethcore-blockchain 0.1.0",
+ "ethcore-db 0.1.0",
+ "ethcore-io 1.12.0",
+ "ethereum-types 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethkey 0.3.0",
+ "hash-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "journaldb 0.2.0",
+ "keccak-hash 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "keccak-hasher 0.1.1",
+ "kvdb 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "kvdb-rocksdb 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parity-bytes 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parity-snappy 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "patricia-trie-ethereum 0.1.0",
+ "rand 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rand_xorshift 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rlp 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "snapshot 0.1.0",
+ "spec 0.1.0",
+ "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
+ "trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "trie-standardmap 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "triehash-ethereum 0.2.0",
+]
+
 [[package]]
 name = "socket2"
 version = "0.3.8"

ethcore/client-traits/src/lib.rs

@@ -58,8 +58,6 @@ use trace::{
 use common_types::data_format::DataFormat;
 use vm::{LastHashes, Schedule};
 
-use common_types::snapshot::Progress;
-
 /// State information to be used during client query
 pub enum StateOrBlock {
 	/// State to be used, may be pending
@@ -448,36 +446,6 @@ pub trait DatabaseRestore: Send + Sync {
 	fn restore_db(&self, new_db: &str) -> Result<(), EthcoreError>;
 }
 
-/// Snapshot related functionality
-pub trait SnapshotClient: BlockChainClient + BlockInfo + DatabaseRestore + BlockChainReset {
-	/// Take a snapshot at the given block.
-	/// If the ID given is "latest", this will default to 1000 blocks behind.
-	fn take_snapshot<W: SnapshotWriter + Send>(
-		&self,
-		writer: W,
-		at: BlockId,
-		p: &Progress,
-	) -> Result<(), EthcoreError>;
-}
-
-
-// todo[dvdplm] move this back to snapshot once extracted from ethcore
-/// Something which can write snapshots.
-/// Writing the same chunk multiple times will lead to implementation-defined
-/// behavior, and is not advised.
-pub trait SnapshotWriter {
-	/// Write a compressed state chunk.
-	fn write_state_chunk(&mut self, hash: H256, chunk: &[u8]) -> std::io::Result<()>;
-
-	/// Write a compressed block chunk.
-	fn write_block_chunk(&mut self, hash: H256, chunk: &[u8]) -> std::io::Result<()>;
-
-	/// Complete writing. The manifest's chunk lists must be consistent
-	/// with the chunks written.
-	fn finish(self, manifest: common_types::snapshot::ManifestData) -> std::io::Result<()> where Self: Sized;
-}
-
-
 /// Represents what has to be handled by actor listening to chain events
 pub trait ChainNotify: Send + Sync {
 	/// fires when chain has new blocks.

ethcore/service/src/service.rs

@@ -29,14 +29,14 @@ use blockchain::{BlockChainDB, BlockChainDBHandler};
 use ethcore::client::{Client, ClientConfig};
 use ethcore::miner::Miner;
 use snapshot::service::{Service as SnapshotService, ServiceParams as SnapServiceParams};
-use snapshot::{SnapshotService as _SnapshotService};
+use snapshot::{SnapshotService as _SnapshotService, SnapshotClient};
 use spec::Spec;
 use common_types::{
 	io_message::ClientIoMessage,
 	errors::{EthcoreError, SnapshotError},
 	snapshot::RestorationStatus,
 };
-use client_traits::{ImportBlock, SnapshotClient, Tick};
+use client_traits::{ImportBlock, Tick};
 
 
 use ethcore_private_tx::{self, Importer, Signer};

ethcore/snapshot/Cargo.toml

@@ -52,3 +52,9 @@ lazy_static = { version = "1.3" }
 spec = { path = "../spec" }
 tempdir = "0.3"
 trie-standardmap = "0.15.0"
+# Note[dvdplm]: Ensure the snapshot tests are included in the dependency tree, which in turn means that
+# `cargo test --all` runs the tests.
+snapshot-tests = { path = "./snapshot-tests" }
+
+[features]
+test-helpers = []

ethcore/snapshot/snapshot-tests/Cargo.toml

@@ -0,0 +1,44 @@
+[package]
+name = "snapshot-tests"
+version = "0.1.0"
+authors = ["Parity Technologies <admin@parity.io>"]
+edition = "2018"
+
+[dependencies]
+accounts = { package = "ethcore-accounts", path = "../../../accounts" }
+account-db = { path = "../../account-db" }
+account-state = { path = "../../account-state" }
+blockchain = { package = "ethcore-blockchain", path = "../../blockchain" }
+bytes = { package = "parity-bytes", version = "0.1.0" }
+client-traits = { path = "../../client-traits" }
+common-types = { path = "../../types" }
+engine = { path = "../../engine", features = ["test-helpers"] }
+env_logger = "0.5"
+ethcore = { path = "../..", features = ["test-helpers"] }
+ethcore-db = { path = "../../db" }
+ethcore-io = { path = "../../../util/io" }
+ethereum-types = "0.6.0"
+ethkey = { path = "../../../accounts/ethkey" }
+ethtrie  = { package = "patricia-trie-ethereum", path = "../../../util/patricia-trie-ethereum" }
+hash-db = "0.15.0"
+journaldb = { path = "../../../util/journaldb" }
+keccak-hash = "0.2.0"
+keccak-hasher = { path = "../../../util/keccak-hasher" }
+kvdb = "0.1.0"
+kvdb-rocksdb = { version = "0.1.3" }
+log = "0.4.8"
+parking_lot = "0.8.0"
+rand = "0.6"
+rand_xorshift = "0.1.1"
+rlp = "0.4.2"
+snappy = { package = "parity-snappy", version ="0.1.0" }
+snapshot = { path = "../../snapshot", features = ["test-helpers"] }
+spec = { path = "../../spec" }
+tempdir = "0.3"
+trie-db = "0.15.0"
+trie-standardmap = "0.15.0"
+ethabi = "8.0"
+ethabi-contract = "8.0"
+ethabi-derive = "8.0"
+lazy_static = { version = "1.3" }
+triehash = { package = "triehash-ethereum", version = "0.2",  path = "../../../util/triehash-ethereum" }

ethcore/snapshot/snapshot-tests/src/abridged_block.rs

@@ -0,0 +1,89 @@
+// Copyright 2015-2019 Parity Technologies (UK) Ltd.
+// This file is part of Parity Ethereum.
+
+// Parity Ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Tests for block RLP encoding
+
+use snapshot::test_helpers::AbridgedBlock;
+
+use bytes::Bytes;
+use ethereum_types::{H256, U256, Address};
+use common_types::{
+	transaction::{Action, Transaction},
+	block::Block,
+	view,
+	views::BlockView,
+};
+
+fn encode_block(b: &Block) -> Bytes {
+	b.rlp_bytes()
+}
+
+#[test]
+fn empty_block_abridging() {
+	let b = Block::default();
+	let receipts_root = b.header.receipts_root().clone();
+	let encoded = encode_block(&b);
+
+	let abridged = AbridgedBlock::from_block_view(&view!(BlockView, &encoded));
+	assert_eq!(abridged.to_block(H256::zero(), 0, receipts_root).unwrap(), b);
+}
+
+#[test]
+#[should_panic]
+fn wrong_number() {
+	let b = Block::default();
+	let receipts_root = b.header.receipts_root().clone();
+	let encoded = encode_block(&b);
+
+	let abridged = AbridgedBlock::from_block_view(&view!(BlockView, &encoded));
+	assert_eq!(abridged.to_block(H256::zero(), 2, receipts_root).unwrap(), b);
+}
+
+#[test]
+fn with_transactions() {
+	let mut b = Block::default();
+
+	let t1 = Transaction {
+		action: Action::Create,
+		nonce: U256::from(42),
+		gas_price: U256::from(3000),
+		gas: U256::from(50_000),
+		value: U256::from(1),
+		data: b"Hello!".to_vec()
+	}.fake_sign(Address::from_low_u64_be(0x69));
+
+	let t2 = Transaction {
+		action: Action::Create,
+		nonce: U256::from(88),
+		gas_price: U256::from(12345),
+		gas: U256::from(300000),
+		value: U256::from(1000000000),
+		data: "Eep!".into(),
+	}.fake_sign(Address::from_low_u64_be(0x55));
+
+	b.transactions.push(t1.into());
+	b.transactions.push(t2.into());
+
+	let receipts_root = b.header.receipts_root().clone();
+	b.header.set_transactions_root(triehash::ordered_trie_root(
+		b.transactions.iter().map(::rlp::encode)
+	));
+
+	let encoded = encode_block(&b);
+
+	let abridged = AbridgedBlock::from_block_view(&view!(BlockView, &encoded[..]));
+	assert_eq!(abridged.to_block(H256::zero(), 0, receipts_root).unwrap(), b);
+}

ethcore/snapshot/snapshot-tests/src/account.rs

@@ -0,0 +1,195 @@
+// Copyright 2015-2019 Parity Technologies (UK) Ltd.
+// This file is part of Parity Ethereum.
+
+// Parity Ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Tests for account state encoding and decoding
+
+use std::collections::HashSet;
+
+use account_db::{AccountDB, AccountDBMut};
+use common_types::{
+	basic_account::BasicAccount,
+	snapshot::Progress
+};
+use ethcore::test_helpers::get_temp_state_db;
+use ethereum_types::{H256, Address};
+use hash_db::{HashDB, EMPTY_PREFIX};
+use keccak_hash::{KECCAK_EMPTY, KECCAK_NULL_RLP, keccak};
+use kvdb::DBValue;
+use rlp::Rlp;
+use snapshot::test_helpers::{ACC_EMPTY, to_fat_rlps, from_fat_rlp};
+
+use crate::helpers::fill_storage;
+
+#[test]
+fn encoding_basic() {
+	let mut db = get_temp_state_db();
+	let addr = Address::random();
+
+	let account = BasicAccount {
+		nonce: 50.into(),
+		balance: 123456789.into(),
+		storage_root: KECCAK_NULL_RLP,
+		code_hash: KECCAK_EMPTY,
+		code_version: 0.into(),
+	};
+
+	let thin_rlp = ::rlp::encode(&account);
+	assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
+	let p = Progress::default();
+	let fat_rlps = to_fat_rlps(&keccak(&addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), usize::max_value(), usize::max_value(), &p).unwrap();
+	let fat_rlp = Rlp::new(&fat_rlps[0]).at(1).unwrap();
+	assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, H256::zero()).unwrap().0, account);
+}
+
+#[test]
+fn encoding_version() {
+	let mut db = get_temp_state_db();
+	let addr = Address::random();
+
+	let account = BasicAccount {
+		nonce: 50.into(),
+		balance: 123456789.into(),
+		storage_root: KECCAK_NULL_RLP,
+		code_hash: KECCAK_EMPTY,
+		code_version: 1.into(),
+	};
+
+	let thin_rlp = ::rlp::encode(&account);
+	assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
+	let p = Progress::default();
+	let fat_rlps = to_fat_rlps(&keccak(&addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), usize::max_value(), usize::max_value(), &p).unwrap();
+	let fat_rlp = Rlp::new(&fat_rlps[0]).at(1).unwrap();
+	assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, H256::zero()).unwrap().0, account);
+}
+
+#[test]
+fn encoding_storage() {
+	let mut db = get_temp_state_db();
+	let addr = Address::random();
+
+	let account = {
+		let acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr));
+		let mut root = KECCAK_NULL_RLP;
+		fill_storage(acct_db, &mut root, &mut H256::zero());
+		BasicAccount {
+			nonce: 25.into(),
+			balance: 987654321.into(),
+			storage_root: root,
+			code_hash: KECCAK_EMPTY,
+			code_version: 0.into(),
+		}
+	};
+
+	let thin_rlp = ::rlp::encode(&account);
+	assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
+
+	let p = Progress::default();
+
+	let fat_rlp = to_fat_rlps(&keccak(&addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), usize::max_value(), usize::max_value(), &p).unwrap();
+	let fat_rlp = Rlp::new(&fat_rlp[0]).at(1).unwrap();
+	assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, H256::zero()).unwrap().0, account);
+}
+
+#[test]
+fn encoding_storage_split() {
+	let mut db = get_temp_state_db();
+	let addr = Address::random();
+
+	let account = {
+		let acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr));
+		let mut root = KECCAK_NULL_RLP;
+		fill_storage(acct_db, &mut root, &mut H256::zero());
+		BasicAccount {
+			nonce: 25.into(),
+			balance: 987654321.into(),
+			storage_root: root,
+			code_hash: KECCAK_EMPTY,
+			code_version: 0.into(),
+		}
+	};
+
+	let thin_rlp = ::rlp::encode(&account);
+	assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
+
+	let p = Progress::default();
+	let fat_rlps = to_fat_rlps(&keccak(addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), 500, 1000, &p).unwrap();
+	let mut root = KECCAK_NULL_RLP;
+	let mut restored_account = None;
+	for rlp in fat_rlps {
+		let fat_rlp = Rlp::new(&rlp).at(1).unwrap();
+		restored_account = Some(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, root).unwrap().0);
+		root = restored_account.as_ref().unwrap().storage_root.clone();
+	}
+	assert_eq!(restored_account, Some(account));
+}
+
+#[test]
+fn encoding_code() {
+	let mut db = get_temp_state_db();
+
+	let addr1 = Address::random();
+	let addr2 = Address::random();
+
+	let code_hash = {
+		let mut acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr1));
+		acct_db.insert(EMPTY_PREFIX, b"this is definitely code")
+	};
+
+	{
+		let mut acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr2));
+		acct_db.emplace(code_hash.clone(), EMPTY_PREFIX, DBValue::from_slice(b"this is definitely code"));
+	}
+
+	let account1 = BasicAccount {
+		nonce: 50.into(),
+		balance: 123456789.into(),
+		storage_root: KECCAK_NULL_RLP,
+		code_hash,
+		code_version: 0.into(),
+	};
+
+	let account2 = BasicAccount {
+		nonce: 400.into(),
+		balance: 98765432123456789usize.into(),
+		storage_root: KECCAK_NULL_RLP,
+		code_hash,
+		code_version: 0.into(),
+	};
+
+	let mut used_code = HashSet::new();
+	let p1 = Progress::default();
+	let p2 = Progress::default();
+	let fat_rlp1 = to_fat_rlps(&keccak(&addr1), &account1, &AccountDB::from_hash(db.as_hash_db(), keccak(addr1)), &mut used_code, usize::max_value(), usize::max_value(), &p1).unwrap();
+	let fat_rlp2 = to_fat_rlps(&keccak(&addr2), &account2, &AccountDB::from_hash(db.as_hash_db(), keccak(addr2)), &mut used_code, usize::max_value(), usize::max_value(), &p2).unwrap();
+	assert_eq!(used_code.len(), 1);
+
+	let fat_rlp1 = Rlp::new(&fat_rlp1[0]).at(1).unwrap();
+	let fat_rlp2 = Rlp::new(&fat_rlp2[0]).at(1).unwrap();
+
+	let (acc, maybe_code) = from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr2)), fat_rlp2, H256::zero()).unwrap();
+	assert!(maybe_code.is_none());
+	assert_eq!(acc, account2);
+
+	let (acc, maybe_code) = from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr1)), fat_rlp1, H256::zero()).unwrap();
+	assert_eq!(maybe_code, Some(b"this is definitely code".to_vec()));
+	assert_eq!(acc, account1);
+}
+
+#[test]
+fn encoding_empty_acc() {
+	let mut db = get_temp_state_db();
+	assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(Address::zero())), Rlp::new(&::rlp::NULL_RLP), H256::zero()).unwrap(), (ACC_EMPTY, None));
+}

ethcore/snapshot/snapshot-tests/src/helpers.rs

@@ -20,30 +20,36 @@
 use std::sync::Arc;
 use std::sync::atomic::AtomicBool;
 
-use keccak_hash::{KECCAK_NULL_RLP};
 use account_db::AccountDBMut;
-use common_types::basic_account::BasicAccount;
+use account_state;
 use blockchain::{BlockChain, BlockChainDB};
-use ethcore::client::Client;
+use client_traits::ChainInfo;
-use client_traits::{ChainInfo, SnapshotClient};
+use common_types::{
+	ids::BlockId,
+	basic_account::BasicAccount,
+	errors::EthcoreError
+};
 use engine::Engine;
-use crate::{
+use ethcore::client::Client;
+use ethereum_types::H256;
+use ethtrie::{SecTrieDBMut, TrieDB, TrieDBMut};
+use hash_db::HashDB;
+use journaldb;
+use keccak_hash::{KECCAK_NULL_RLP};
+use keccak_hasher::KeccakHasher;
+use kvdb::DBValue;
+use log::trace;
+use rand::Rng;
+use rlp;
+use snapshot::{
+	SnapshotClient,
 	StateRebuilder,
 	io::{SnapshotReader, PackedWriter, PackedReader},
+	chunker,
 };
-
 use tempdir::TempDir;
-use rand::Rng;
-use log::trace;
-use kvdb::DBValue;
-use ethereum_types::H256;
-use hash_db::HashDB;
-use keccak_hasher::KeccakHasher;
-use journaldb;
 use trie_db::{TrieMut, Trie};
-use ethtrie::{SecTrieDBMut, TrieDB, TrieDBMut};
 use trie_standardmap::{Alphabet, StandardMap, ValueMode};
-use common_types::errors::EthcoreError;
 
 // the proportion of accounts we will alter each tick.
 const ACCOUNT_CHURN: f32 = 0.01;
@@ -80,10 +86,10 @@ impl StateProducer {
 
 		// sweep once to alter storage tries.
 		for &mut (ref mut address_hash, ref mut account_data) in &mut accounts_to_modify {
-			let mut account: BasicAccount = ::rlp::decode(&*account_data).expect("error decoding basic account");
+			let mut account: BasicAccount = rlp::decode(&*account_data).expect("error decoding basic account");
 			let acct_db = AccountDBMut::from_hash(db, *address_hash);
 			fill_storage(acct_db, &mut account.storage_root, &mut self.storage_seed);
-			*account_data = DBValue::from_vec(::rlp::encode(&account));
+			*account_data = DBValue::from_vec(rlp::encode(&account));
 		}
 
 		// sweep again to alter account trie.
@@ -136,8 +142,6 @@ pub fn fill_storage(mut db: AccountDBMut, root: &mut H256, seed: &mut H256) {
 /// Take a snapshot from the given client into a temporary file.
 /// Return a snapshot reader for it.
 pub fn snap(client: &Client) -> (Box<dyn SnapshotReader>, TempDir) {
-	use common_types::ids::BlockId;
-
 	let tempdir = TempDir::new("").unwrap();
 	let path = tempdir.path().join("file");
 	let writer = PackedWriter::new(&path).unwrap();
@@ -160,7 +164,7 @@ pub fn restore(
 	genesis: &[u8],
 ) -> Result<(), EthcoreError> {
 	let flag = AtomicBool::new(true);
-	let chunker = crate::chunker(engine.snapshot_mode()).expect("the engine used here supports snapshots");
+	let chunker = chunker(engine.snapshot_mode()).expect("the engine used here supports snapshots");
 	let manifest = reader.manifest();
 
 	let mut state = StateRebuilder::new(db.key_value().clone(), journaldb::Algorithm::Archive);

ethcore/snapshot/snapshot-tests/src/io.rs

@@ -0,0 +1,109 @@
+// Copyright 2015-2019 Parity Technologies (UK) Ltd.
+// This file is part of Parity Ethereum.
+
+// Parity Ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Tests for snapshot i/o.
+
+use tempdir::TempDir;
+use keccak_hash::keccak;
+
+use common_types::snapshot::ManifestData;
+use snapshot::io::{
+	SnapshotWriter,SnapshotReader,
+	PackedWriter, PackedReader, LooseWriter, LooseReader,
+	SNAPSHOT_VERSION,
+};
+
+const STATE_CHUNKS: &'static [&'static [u8]] = &[b"dog", b"cat", b"hello world", b"hi", b"notarealchunk"];
+const BLOCK_CHUNKS: &'static [&'static [u8]] = &[b"hello!", b"goodbye!", b"abcdefg", b"hijklmnop", b"qrstuvwxy", b"and", b"z"];
+
+#[test]
+fn packed_write_and_read() {
+	let tempdir = TempDir::new("").unwrap();
+	let path = tempdir.path().join("packed");
+	let mut writer = PackedWriter::new(&path).unwrap();
+
+	let mut state_hashes = Vec::new();
+	let mut block_hashes = Vec::new();
+
+	for chunk in STATE_CHUNKS {
+		let hash = keccak(&chunk);
+		state_hashes.push(hash.clone());
+		writer.write_state_chunk(hash, chunk).unwrap();
+	}
+
+	for chunk in BLOCK_CHUNKS {
+		let hash = keccak(&chunk);
+		block_hashes.push(hash.clone());
+		writer.write_block_chunk(keccak(&chunk), chunk).unwrap();
+	}
+
+	let manifest = ManifestData {
+		version: SNAPSHOT_VERSION,
+		state_hashes,
+		block_hashes,
+		state_root: keccak(b"notarealroot"),
+		block_number: 12345678987654321,
+		block_hash: keccak(b"notarealblock"),
+	};
+
+	writer.finish(manifest.clone()).unwrap();
+
+	let reader = PackedReader::new(&path).unwrap().unwrap();
+	assert_eq!(reader.manifest(), &manifest);
+
+	for hash in manifest.state_hashes.iter().chain(&manifest.block_hashes) {
+		reader.chunk(hash.clone()).unwrap();
+	}
+}
+
+#[test]
+fn loose_write_and_read() {
+	let tempdir = TempDir::new("").unwrap();
+	let mut writer = LooseWriter::new(tempdir.path().into()).unwrap();
+
+	let mut state_hashes = Vec::new();
+	let mut block_hashes = Vec::new();
+
+	for chunk in STATE_CHUNKS {
+		let hash = keccak(&chunk);
+		state_hashes.push(hash.clone());
+		writer.write_state_chunk(hash, chunk).unwrap();
+	}
+
+	for chunk in BLOCK_CHUNKS {
+		let hash = keccak(&chunk);
+		block_hashes.push(hash.clone());
+		writer.write_block_chunk(keccak(&chunk), chunk).unwrap();
+	}
+
+	let manifest = ManifestData {
+		version: SNAPSHOT_VERSION,
+		state_hashes,
+		block_hashes,
+		state_root: keccak(b"notarealroot"),
+		block_number: 12345678987654321,
+		block_hash: keccak(b"notarealblock)"),
+	};
+
+	writer.finish(manifest.clone()).unwrap();
+
+	let reader = LooseReader::new(tempdir.path().into()).unwrap();
+	assert_eq!(reader.manifest(), &manifest);
+
+	for hash in manifest.state_hashes.iter().chain(&manifest.block_hashes) {
+		reader.chunk(hash.clone()).unwrap();
+	}
+}

ethcore/snapshot/snapshot-tests/src/lib.rs

@@ -16,17 +16,22 @@
 
 //! Snapshot tests.
 
+#![cfg(test)]
+
+mod abridged_block;
+mod account;
+mod io;
 mod proof_of_work;
 mod proof_of_authority;
 mod state;
 mod service;
+mod watcher;
 
-pub mod helpers;
+mod helpers;
-
-use common_types::snapshot::ManifestData;
 
 #[test]
 fn manifest_rlp() {
+    use common_types::snapshot::ManifestData;
     let manifest = ManifestData {
         version: 2,
         block_hashes: Vec::new(),

ethcore/snapshot/snapshot-tests/src/proof_of_authority.rs

@@ -21,25 +21,25 @@ use std::sync::Arc;
 use std::str::FromStr;
 
 use accounts::AccountProvider;
-use ethcore::client::Client;
 use client_traits::{BlockChainClient, ChainInfo};
-use ethkey::Secret;
-use crate::tests::helpers as snapshot_helpers;
-use spec::Spec;
-use ethcore::test_helpers::generate_dummy_client_with_spec;
 use common_types::transaction::{Transaction, Action, SignedTransaction};
-use tempdir::TempDir;
+use ethabi_contract::use_contract;
-use log::trace;
-use ethereum_types::Address;
 use ethcore::{
-	test_helpers,
+	client::Client,
+	test_helpers::{self, generate_dummy_client_with_spec},
 	miner::{self, MinerService},
 };
+use ethereum_types::Address;
+use ethkey::Secret;
 use keccak_hash::keccak;
-use ethabi_contract::use_contract;
 use lazy_static::lazy_static;
+use log::trace;
+use spec::Spec;
+use tempdir::TempDir;
 
-use_contract!(test_validator_set, "../res/contracts/test_validator_set.json");
+use crate::helpers as snapshot_helpers;
+
+use_contract!(test_validator_set, "../../res/contracts/test_validator_set.json");
 
 const PASS: &'static str = "";
 const TRANSITION_BLOCK_1: usize = 2; // block at which the contract becomes activated.

ethcore/snapshot/snapshot-tests/src/proof_of_work.rs

@@ -19,18 +19,16 @@
 use std::sync::atomic::AtomicBool;
 use tempdir::TempDir;
 use common_types::{
-	errors::EthcoreError as Error,
+	errors::{EthcoreError as Error, SnapshotError},
 	engines::ForkChoice,
 	snapshot::{Progress, ManifestData},
 };
-
 use blockchain::generator::{BlockGenerator, BlockBuilder};
 use blockchain::{BlockChain, ExtrasInsert};
-use client_traits::SnapshotWriter;
+use snapshot::{
-use crate::{
 	chunk_secondary,
-	Error as SnapshotError, SnapshotComponents,
+	SnapshotComponents,
-	io::{PackedReader, PackedWriter, SnapshotReader},
+	io::{PackedReader, PackedWriter, SnapshotReader, SnapshotWriter},
 	PowSnapshot,
 };
 use parking_lot::Mutex;

ethcore/snapshot/snapshot-tests/src/service.rs

@@ -22,17 +22,18 @@ use std::sync::Arc;
 use tempdir::TempDir;
 use blockchain::BlockProvider;
 use ethcore::client::{Client, ClientConfig};
-use client_traits::{BlockInfo, ImportBlock, SnapshotWriter};
+use client_traits::{BlockInfo, ImportBlock};
 use common_types::{
+	io_message::ClientIoMessage,
 	ids::BlockId,
 	snapshot::Progress,
 	verification::Unverified,
 	snapshot::{ManifestData, RestorationStatus},
 };
-use crate::{
+use snapshot::{
 	chunk_state, chunk_secondary, SnapshotService,
-	io::{PackedReader, PackedWriter, SnapshotReader},
+	io::{PackedReader, PackedWriter, SnapshotReader, SnapshotWriter},
-	service::{Service, ServiceParams},
+	service::{Service, ServiceParams, Guard, Restoration, RestorationParams},
 	PowSnapshot,
 };
 use spec;
@@ -42,8 +43,97 @@ use ethcore::{
 };
 
 use parking_lot::Mutex;
-use ethcore_io::IoChannel;
+use ethcore_io::{IoChannel, IoService};
 use kvdb_rocksdb::DatabaseConfig;
+use journaldb::Algorithm;
+
+#[test]
+fn sends_async_messages() {
+	let gas_prices = vec![1.into(), 2.into(), 3.into(), 999.into()];
+	let client = generate_dummy_client_with_spec_and_data(spec::new_null, 400, 5, &gas_prices);
+	let service = IoService::<ClientIoMessage<Client>>::start().unwrap();
+	let spec = spec::new_test();
+
+	let tempdir = TempDir::new("").unwrap();
+	let dir = tempdir.path().join("snapshot");
+
+	let snapshot_params = ServiceParams {
+		engine: spec.engine.clone(),
+		genesis_block: spec.genesis_block(),
+		restoration_db_handler: restoration_db_handler(Default::default()),
+		pruning: Algorithm::Archive,
+		channel: service.channel(),
+		snapshot_root: dir,
+		client,
+	};
+
+	let service = Service::new(snapshot_params).unwrap();
+
+	assert!(service.manifest().is_none());
+	assert!(service.chunk(Default::default()).is_none());
+	assert_eq!(service.status(), RestorationStatus::Inactive);
+
+	let manifest = ManifestData {
+		version: 2,
+		state_hashes: vec![],
+		block_hashes: vec![],
+		state_root: Default::default(),
+		block_number: 0,
+		block_hash: Default::default(),
+	};
+
+	service.begin_restore(manifest);
+	service.abort_restore();
+	service.restore_state_chunk(Default::default(), vec![]);
+	service.restore_block_chunk(Default::default(), vec![]);
+}
+
+#[test]
+fn cannot_finish_with_invalid_chunks() {
+	use ethereum_types::H256;
+	use kvdb_rocksdb::DatabaseConfig;
+
+	let spec = spec::new_test();
+	let tempdir = TempDir::new("").unwrap();
+
+	let state_hashes: Vec<_> = (0..5).map(|_| H256::random()).collect();
+	let block_hashes: Vec<_> = (0..5).map(|_| H256::random()).collect();
+	let db_config = DatabaseConfig::with_columns(ethcore_db::NUM_COLUMNS);
+	let gb = spec.genesis_block();
+	let flag = ::std::sync::atomic::AtomicBool::new(true);
+
+	let engine = &*spec.engine.clone();
+	let params = RestorationParams::new(
+		ManifestData {
+			version: 2,
+			state_hashes: state_hashes.clone(),
+			block_hashes: block_hashes.clone(),
+			state_root: H256::zero(),
+			block_number: 100000,
+			block_hash: H256::zero(),
+		},
+		Algorithm::Archive,
+		restoration_db_handler(db_config).open(&tempdir.path().to_owned()).unwrap(),
+		None,
+		&gb,
+		Guard::benign(),
+		engine,
+	);
+
+	let mut restoration = Restoration::new(params).unwrap();
+	let definitely_bad_chunk = [1, 2, 3, 4, 5];
+
+	for hash in state_hashes {
+		assert!(restoration.feed_state(hash, &definitely_bad_chunk, &flag).is_err());
+		assert!(!restoration.is_done());
+	}
+
+	for hash in block_hashes {
+		assert!(restoration.feed_blocks(hash, &definitely_bad_chunk, &*spec.engine, &flag).is_err());
+		assert!(!restoration.is_done());
+	}
+}
+
 
 #[test]
 fn restored_is_equivalent() {
@@ -280,7 +370,7 @@ fn keep_ancient_blocks() {
 
 #[test]
 fn recover_aborted_recovery() {
-	let _ = ::env_logger::try_init();
+	let _ = env_logger::try_init();
 
 	const NUM_BLOCKS: u32 = 400;
 	let gas_prices = vec![1.into(), 2.into(), 3.into(), 999.into()];

ethcore/snapshot/snapshot-tests/src/state.rs

@@ -18,19 +18,17 @@
 
 use std::sync::Arc;
 use std::sync::atomic::AtomicBool;
-use keccak_hash::{KECCAK_NULL_RLP, keccak};
 
+use keccak_hash::{KECCAK_NULL_RLP, keccak};
 use common_types::{
 	basic_account::BasicAccount,
-	errors::EthcoreError as Error,
+	errors::{EthcoreError as Error, SnapshotError},
-	snapshot::ManifestData,
+	snapshot::{ManifestData, Progress},
 };
-use client_traits::SnapshotWriter;
+use snapshot::{
-use crate::{
+	test_helpers::to_fat_rlps,
-	account,
+	chunk_state, StateRebuilder, SNAPSHOT_SUBPARTS,
-	{chunk_state, Error as SnapshotError, Progress, StateRebuilder, SNAPSHOT_SUBPARTS},
+	io::{PackedReader, PackedWriter, SnapshotReader, SnapshotWriter},
-	io::{PackedReader, PackedWriter, SnapshotReader},
-	tests::helpers::StateProducer,
 };
 use rand::SeedableRng;
 use rand_xorshift::XorShiftRng;
@@ -40,6 +38,8 @@ use kvdb_rocksdb::{Database, DatabaseConfig};
 use parking_lot::Mutex;
 use tempdir::TempDir;
 
+use crate::helpers::StateProducer;
+
 const RNG_SEED: [u8; 16] = [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16];
 
 #[test]
@@ -134,7 +134,7 @@ fn get_code_from_prev_chunk() {
 		let mut db = journaldb::new_memory_db();
 		AccountDBMut::from_hash(&mut db, hash).insert(EMPTY_PREFIX, &code[..]);
 		let p = Progress::default();
-		let fat_rlp = account::to_fat_rlps(&hash, &acc, &AccountDB::from_hash(&db, hash), &mut used_code, usize::max_value(), usize::max_value(), &p).unwrap();
+		let fat_rlp = to_fat_rlps(&hash, &acc, &AccountDB::from_hash(&db, hash), &mut used_code, usize::max_value(), usize::max_value(), &p).unwrap();
 		let mut stream = RlpStream::new_list(1);
 		stream.append_raw(&fat_rlp[0], 1);
 		stream.out()

ethcore/snapshot/snapshot-tests/src/watcher.rs

@@ -0,0 +1,93 @@
+// Copyright 2015-2019 Parity Technologies (UK) Ltd.
+// This file is part of Parity Ethereum.
+
+// Parity Ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Tests for block RLP encoding
+
+use std::collections::HashMap;
+use std::time::Duration;
+
+use client_traits::ChainNotify;
+use common_types::chain_notify::{NewBlocks, ChainRoute};
+use ethereum_types::{H256, U256, BigEndianHash};
+
+use snapshot::{
+	Broadcast,
+	Oracle,
+	test_helpers::Watcher,
+};
+
+struct TestOracle(HashMap<H256, u64>);
+
+impl Oracle for TestOracle {
+	fn to_number(&self, hash: H256) -> Option<u64> {
+		self.0.get(&hash).cloned()
+	}
+
+	fn is_major_importing(&self) -> bool { false }
+}
+
+struct TestBroadcast(Option<u64>);
+impl Broadcast for TestBroadcast {
+	fn take_at(&self, num: Option<u64>) {
+		if num != self.0 {
+			panic!("Watcher broadcast wrong number. Expected {:?}, found {:?}", self.0, num);
+		}
+	}
+}
+
+// helper harness for tests which expect a notification.
+fn harness(numbers: Vec<u64>, period: u64, history: u64, expected: Option<u64>) {
+	const DURATION_ZERO: Duration = Duration::from_millis(0);
+
+	let hashes: Vec<_> = numbers.clone().into_iter().map(|x| BigEndianHash::from_uint(&U256::from(x))).collect();
+	let map = hashes.clone().into_iter().zip(numbers).collect();
+	let watcher = Watcher::new_test(
+		Box::new(TestOracle(map)),
+		Box::new(TestBroadcast(expected)),
+		period,
+		history,
+	);
+
+	watcher.new_blocks(NewBlocks::new(
+		hashes,
+		vec![],
+		ChainRoute::default(),
+		vec![],
+		vec![],
+		DURATION_ZERO,
+		false
+	));
+}
+
+#[test]
+fn should_not_fire() {
+	harness(vec![0], 5, 0, None);
+}
+
+#[test]
+fn fires_once_for_two() {
+	harness(vec![14, 15], 10, 5, Some(10));
+}
+
+#[test]
+fn finds_highest() {
+	harness(vec![15, 25], 10, 5, Some(20));
+}
+
+#[test]
+fn doesnt_fire_before_history() {
+	harness(vec![10, 11], 10, 5, None);
+}

ethcore/snapshot/src/account.rs

@@ -35,7 +35,7 @@ use rlp::{RlpStream, Rlp};
 use trie_db::{Trie, TrieMut};
 
 // An empty account -- these were replaced with RLP null data for a space optimization in v1.
-const ACC_EMPTY: BasicAccount = BasicAccount {
+pub const ACC_EMPTY: BasicAccount = BasicAccount {
 	nonce: U256([0, 0, 0, 0]),
 	balance: U256([0, 0, 0, 0]),
 	storage_root: KECCAK_NULL_RLP,
@@ -244,183 +244,3 @@ pub fn from_fat_rlp(
 
 	Ok((acc, new_code))
 }
-
-#[cfg(test)]
-mod tests {
-	use account_db::{AccountDB, AccountDBMut};
-	use common_types::basic_account::BasicAccount;
-	use ethcore::test_helpers::get_temp_state_db;
-	use crate::tests::helpers::fill_storage;
-	use common_types::snapshot::Progress;
-
-	use keccak_hash::{KECCAK_EMPTY, KECCAK_NULL_RLP, keccak};
-	use ethereum_types::{H256, Address};
-	use hash_db::{HashDB, EMPTY_PREFIX};
-	use kvdb::DBValue;
-	use rlp::Rlp;
-
-	use std::collections::HashSet;
-
-	use super::{ACC_EMPTY, to_fat_rlps, from_fat_rlp};
-
-	#[test]
-	fn encoding_basic() {
-		let mut db = get_temp_state_db();
-		let addr = Address::random();
-
-		let account = BasicAccount {
-			nonce: 50.into(),
-			balance: 123456789.into(),
-			storage_root: KECCAK_NULL_RLP,
-			code_hash: KECCAK_EMPTY,
-			code_version: 0.into(),
-		};
-
-		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
-		let p = Progress::default();
-		let fat_rlps = to_fat_rlps(&keccak(&addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), usize::max_value(), usize::max_value(), &p).unwrap();
-		let fat_rlp = Rlp::new(&fat_rlps[0]).at(1).unwrap();
-		assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, H256::zero()).unwrap().0, account);
-	}
-
-	#[test]
-	fn encoding_version() {
-		let mut db = get_temp_state_db();
-		let addr = Address::random();
-
-		let account = BasicAccount {
-			nonce: 50.into(),
-			balance: 123456789.into(),
-			storage_root: KECCAK_NULL_RLP,
-			code_hash: KECCAK_EMPTY,
-			code_version: 1.into(),
-		};
-
-		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
-		let p = Progress::default();
-		let fat_rlps = to_fat_rlps(&keccak(&addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), usize::max_value(), usize::max_value(), &p).unwrap();
-		let fat_rlp = Rlp::new(&fat_rlps[0]).at(1).unwrap();
-		assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, H256::zero()).unwrap().0, account);
-	}
-
-	#[test]
-	fn encoding_storage() {
-		let mut db = get_temp_state_db();
-		let addr = Address::random();
-
-		let account = {
-			let acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr));
-			let mut root = KECCAK_NULL_RLP;
-			fill_storage(acct_db, &mut root, &mut H256::zero());
-			BasicAccount {
-				nonce: 25.into(),
-				balance: 987654321.into(),
-				storage_root: root,
-				code_hash: KECCAK_EMPTY,
-				code_version: 0.into(),
-			}
-		};
-
-		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
-
-		let p = Progress::default();
-
-		let fat_rlp = to_fat_rlps(&keccak(&addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), usize::max_value(), usize::max_value(), &p).unwrap();
-		let fat_rlp = Rlp::new(&fat_rlp[0]).at(1).unwrap();
-		assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, H256::zero()).unwrap().0, account);
-	}
-
-	#[test]
-	fn encoding_storage_split() {
-		let mut db = get_temp_state_db();
-		let addr = Address::random();
-
-		let account = {
-			let acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr));
-			let mut root = KECCAK_NULL_RLP;
-			fill_storage(acct_db, &mut root, &mut H256::zero());
-			BasicAccount {
-				nonce: 25.into(),
-				balance: 987654321.into(),
-				storage_root: root,
-				code_hash: KECCAK_EMPTY,
-				code_version: 0.into(),
-			}
-		};
-
-		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
-
-		let p = Progress::default();
-		let fat_rlps = to_fat_rlps(&keccak(addr), &account, &AccountDB::from_hash(db.as_hash_db(), keccak(addr)), &mut Default::default(), 500, 1000, &p).unwrap();
-		let mut root = KECCAK_NULL_RLP;
-		let mut restored_account = None;
-		for rlp in fat_rlps {
-			let fat_rlp = Rlp::new(&rlp).at(1).unwrap();
-			restored_account = Some(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr)), fat_rlp, root).unwrap().0);
-			root = restored_account.as_ref().unwrap().storage_root.clone();
-		}
-		assert_eq!(restored_account, Some(account));
-	}
-
-	#[test]
-	fn encoding_code() {
-		let mut db = get_temp_state_db();
-
-		let addr1 = Address::random();
-		let addr2 = Address::random();
-
-		let code_hash = {
-			let mut acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr1));
-			acct_db.insert(EMPTY_PREFIX, b"this is definitely code")
-		};
-
-		{
-			let mut acct_db = AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr2));
-			acct_db.emplace(code_hash.clone(), EMPTY_PREFIX, DBValue::from_slice(b"this is definitely code"));
-		}
-
-		let account1 = BasicAccount {
-			nonce: 50.into(),
-			balance: 123456789.into(),
-			storage_root: KECCAK_NULL_RLP,
-			code_hash,
-			code_version: 0.into(),
-		};
-
-		let account2 = BasicAccount {
-			nonce: 400.into(),
-			balance: 98765432123456789usize.into(),
-			storage_root: KECCAK_NULL_RLP,
-			code_hash,
-			code_version: 0.into(),
-		};
-
-		let mut used_code = HashSet::new();
-		let p1 = Progress::default();
-		let p2 = Progress::default();
-		let fat_rlp1 = to_fat_rlps(&keccak(&addr1), &account1, &AccountDB::from_hash(db.as_hash_db(), keccak(addr1)), &mut used_code, usize::max_value(), usize::max_value(), &p1).unwrap();
-		let fat_rlp2 = to_fat_rlps(&keccak(&addr2), &account2, &AccountDB::from_hash(db.as_hash_db(), keccak(addr2)), &mut used_code, usize::max_value(), usize::max_value(), &p2).unwrap();
-		assert_eq!(used_code.len(), 1);
-
-		let fat_rlp1 = Rlp::new(&fat_rlp1[0]).at(1).unwrap();
-		let fat_rlp2 = Rlp::new(&fat_rlp2[0]).at(1).unwrap();
-
-		let (acc, maybe_code) = from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr2)), fat_rlp2, H256::zero()).unwrap();
-		assert!(maybe_code.is_none());
-		assert_eq!(acc, account2);
-
-		let (acc, maybe_code) = from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(addr1)), fat_rlp1, H256::zero()).unwrap();
-		assert_eq!(maybe_code, Some(b"this is definitely code".to_vec()));
-		assert_eq!(acc, account1);
-	}
-
-	#[test]
-	fn encoding_empty_acc() {
-		let mut db = get_temp_state_db();
-		assert_eq!(from_fat_rlp(&mut AccountDBMut::from_hash(db.as_hash_db_mut(), keccak(Address::zero())), Rlp::new(&::rlp::NULL_RLP), H256::zero()).unwrap(), (ACC_EMPTY, None));
-	}
-}

ethcore/snapshot/src/block.rs

@@ -30,7 +30,8 @@ use triehash::ordered_trie_root;
 const HEADER_FIELDS: usize = 8;
 const BLOCK_FIELDS: usize = 2;
 
-pub(crate) struct AbridgedBlock {
+/// Convenience type to convert raw RLP to and from blocks.
+pub struct AbridgedBlock {
 	rlp: Bytes,
 }
 
@@ -127,78 +128,3 @@ impl AbridgedBlock {
 		Ok(Block { header, transactions, uncles })
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use super::AbridgedBlock;
-
-	use bytes::Bytes;
-	use ethereum_types::{H256, U256, Address};
-	use common_types::{
-		transaction::{Action, Transaction},
-		block::Block,
-		view,
-		views::BlockView,
-	};
-
-	fn encode_block(b: &Block) -> Bytes {
-		b.rlp_bytes()
-	}
-
-	#[test]
-	fn empty_block_abridging() {
-		let b = Block::default();
-		let receipts_root = b.header.receipts_root().clone();
-		let encoded = encode_block(&b);
-
-		let abridged = AbridgedBlock::from_block_view(&view!(BlockView, &encoded));
-		assert_eq!(abridged.to_block(H256::zero(), 0, receipts_root).unwrap(), b);
-	}
-
-	#[test]
-	#[should_panic]
-	fn wrong_number() {
-		let b = Block::default();
-		let receipts_root = b.header.receipts_root().clone();
-		let encoded = encode_block(&b);
-
-		let abridged = AbridgedBlock::from_block_view(&view!(BlockView, &encoded));
-		assert_eq!(abridged.to_block(H256::zero(), 2, receipts_root).unwrap(), b);
-	}
-
-	#[test]
-	fn with_transactions() {
-		let mut b = Block::default();
-
-		let t1 = Transaction {
-			action: Action::Create,
-			nonce: U256::from(42),
-			gas_price: U256::from(3000),
-			gas: U256::from(50_000),
-			value: U256::from(1),
-			data: b"Hello!".to_vec()
-		}.fake_sign(Address::from_low_u64_be(0x69));
-
-		let t2 = Transaction {
-			action: Action::Create,
-			nonce: U256::from(88),
-			gas_price: U256::from(12345),
-			gas: U256::from(300000),
-			value: U256::from(1000000000),
-			data: "Eep!".into(),
-		}.fake_sign(Address::from_low_u64_be(0x55));
-
-		b.transactions.push(t1.into());
-		b.transactions.push(t2.into());
-
-		let receipts_root = b.header.receipts_root().clone();
-		b.header.set_transactions_root(::triehash::ordered_trie_root(
-			b.transactions.iter().map(::rlp::encode)
-		));
-
-		let encoded = encode_block(&b);
-
-		let abridged = AbridgedBlock::from_block_view(&view!(BlockView, &encoded[..]));
-		assert_eq!(abridged.to_block(H256::zero(), 0, receipts_root).unwrap(), b);
-	}
-}

ethcore/snapshot/src/io.rs

@@ -26,7 +26,6 @@ use std::fs::{self, File};
 use std::path::{Path, PathBuf};
 
 use bytes::Bytes;
-use client_traits::SnapshotWriter;
 use common_types::{
 	errors::{SnapshotError, EthcoreError},
 	snapshot::ManifestData,
@@ -36,7 +35,7 @@ use log::trace;
 use rlp::{RlpStream, Rlp};
 use rlp_derive::*;
 
-const SNAPSHOT_VERSION: u64 = 2;
+pub const SNAPSHOT_VERSION: u64 = 2;
 
 // (hash, len, offset)
 #[derive(RlpEncodable, RlpDecodable)]
@@ -184,6 +183,21 @@ pub trait SnapshotReader {
 	fn chunk(&self, hash: H256) -> io::Result<Bytes>;
 }
 
+/// Something which can write snapshots.
+/// Writing the same chunk multiple times will lead to implementation-defined
+/// behavior, and is not advised.
+pub trait SnapshotWriter {
+	/// Write a compressed state chunk.
+	fn write_state_chunk(&mut self, hash: H256, chunk: &[u8]) -> std::io::Result<()>;
+
+	/// Write a compressed block chunk.
+	fn write_block_chunk(&mut self, hash: H256, chunk: &[u8]) -> std::io::Result<()>;
+
+	/// Complete writing. The manifest's chunk lists must be consistent
+	/// with the chunks written.
+	fn finish(self, manifest: ManifestData) -> std::io::Result<()> where Self: Sized;
+}
+
 /// Packed snapshot reader.
 pub struct PackedReader {
 	file: File,
@@ -317,94 +331,3 @@ impl SnapshotReader for LooseReader {
 		Ok(buf)
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use tempdir::TempDir;
-	use keccak_hash::keccak;
-
-	use common_types::snapshot::ManifestData;
-	use super::{SnapshotWriter, SnapshotReader, PackedWriter, PackedReader, LooseWriter, LooseReader, SNAPSHOT_VERSION};
-
-	const STATE_CHUNKS: &'static [&'static [u8]] = &[b"dog", b"cat", b"hello world", b"hi", b"notarealchunk"];
-	const BLOCK_CHUNKS: &'static [&'static [u8]] = &[b"hello!", b"goodbye!", b"abcdefg", b"hijklmnop", b"qrstuvwxy", b"and", b"z"];
-
-	#[test]
-	fn packed_write_and_read() {
-		let tempdir = TempDir::new("").unwrap();
-		let path = tempdir.path().join("packed");
-		let mut writer = PackedWriter::new(&path).unwrap();
-
-		let mut state_hashes = Vec::new();
-		let mut block_hashes = Vec::new();
-
-		for chunk in STATE_CHUNKS {
-			let hash = keccak(&chunk);
-			state_hashes.push(hash.clone());
-			writer.write_state_chunk(hash, chunk).unwrap();
-		}
-
-		for chunk in BLOCK_CHUNKS {
-			let hash = keccak(&chunk);
-			block_hashes.push(hash.clone());
-			writer.write_block_chunk(keccak(&chunk), chunk).unwrap();
-		}
-
-		let manifest = ManifestData {
-			version: SNAPSHOT_VERSION,
-			state_hashes,
-			block_hashes,
-			state_root: keccak(b"notarealroot"),
-			block_number: 12345678987654321,
-			block_hash: keccak(b"notarealblock"),
-		};
-
-		writer.finish(manifest.clone()).unwrap();
-
-		let reader = PackedReader::new(&path).unwrap().unwrap();
-		assert_eq!(reader.manifest(), &manifest);
-
-		for hash in manifest.state_hashes.iter().chain(&manifest.block_hashes) {
-			reader.chunk(hash.clone()).unwrap();
-		}
-	}
-
-	#[test]
-	fn loose_write_and_read() {
-		let tempdir = TempDir::new("").unwrap();
-		let mut writer = LooseWriter::new(tempdir.path().into()).unwrap();
-
-		let mut state_hashes = Vec::new();
-		let mut block_hashes = Vec::new();
-
-		for chunk in STATE_CHUNKS {
-			let hash = keccak(&chunk);
-			state_hashes.push(hash.clone());
-			writer.write_state_chunk(hash, chunk).unwrap();
-		}
-
-		for chunk in BLOCK_CHUNKS {
-			let hash = keccak(&chunk);
-			block_hashes.push(hash.clone());
-			writer.write_block_chunk(keccak(&chunk), chunk).unwrap();
-		}
-
-		let manifest = ManifestData {
-			version: SNAPSHOT_VERSION,
-			state_hashes: state_hashes,
-			block_hashes: block_hashes,
-			state_root: keccak(b"notarealroot"),
-			block_number: 12345678987654321,
-			block_hash: keccak(b"notarealblock)"),
-		};
-
-		writer.finish(manifest.clone()).unwrap();
-
-		let reader = LooseReader::new(tempdir.path().into()).unwrap();
-		assert_eq!(reader.manifest(), &manifest);
-
-		for hash in manifest.state_hashes.iter().chain(&manifest.block_hashes) {
-			reader.chunk(hash.clone()).unwrap();
-		}
-	}
-}

ethcore/snapshot/src/lib.rs

@@ -31,8 +31,6 @@ use account_state::Account as StateAccount;
 use blockchain::{BlockChain, BlockProvider};
 use bloom_journal::Bloom;
 use bytes::Bytes;
-// todo[dvdplm] put back in snapshots once it's extracted
-use client_traits::SnapshotWriter;
 use common_types::{
 	ids::BlockId,
 	header::Header,
@@ -57,23 +55,29 @@ use state_db::StateDB;
 use trie_db::{Trie, TrieMut};
 
 pub use self::consensus::*;
-pub use self::service::Service;
+pub use self::service::{Service, Guard, Restoration, RestorationParams};
-pub use self::traits::{SnapshotService, SnapshotComponents, Rebuilder};
+pub use self::traits::{Broadcast, Oracle, SnapshotService, SnapshotClient, SnapshotComponents, Rebuilder};
+pub use self::io::SnapshotWriter;
 pub use self::watcher::Watcher;
-pub use common_types::basic_account::BasicAccount;
+use common_types::basic_account::BasicAccount;
 
 pub mod io;
 pub mod service;
 
+#[cfg(feature = "test-helpers" )]
+pub mod test_helpers {
+	pub use super::{
+		account::{ACC_EMPTY, to_fat_rlps, from_fat_rlp},
+		block::AbridgedBlock,
+		watcher::Watcher,
+	};
+}
+
 mod account;
 mod block;
 mod consensus;
-mod watcher;
-
-#[cfg(test)]
-mod tests;
-
 mod traits;
+mod watcher;
 
 // Try to have chunks be around 4MB (before compression)
 const PREFERRED_CHUNK_SIZE: usize = 4 * 1024 * 1024;
@@ -88,7 +92,7 @@ const MIN_SUPPORTED_STATE_CHUNK_VERSION: u64 = 1;
 // current state chunk version.
 const STATE_CHUNK_VERSION: u64 = 2;
 /// number of snapshot subparts, must be a power of 2 in [1; 256]
-const SNAPSHOT_SUBPARTS: usize = 16;
+pub const SNAPSHOT_SUBPARTS: usize = 16;
 /// Maximum number of snapshot subparts (must be a multiple of `SNAPSHOT_SUBPARTS`)
 const MAX_SNAPSHOT_SUBPARTS: usize = 256;
 

ethcore/snapshot/src/service.rs

@@ -32,8 +32,7 @@ use common_types::{
 	ids::BlockId,
 	snapshot::{ManifestData, Progress, RestorationStatus},
 };
-// todo[dvdplm] put SnapshotWriter back in snapshots once extracted
+use client_traits::ChainInfo;
-use client_traits::{ChainInfo, SnapshotClient, SnapshotWriter};
 use engine::Engine;
 use ethereum_types::H256;
 use ethcore_io::IoChannel;
@@ -45,6 +44,8 @@ use parking_lot::{Mutex, RwLock, RwLockReadGuard};
 use snappy;
 use trie_db::TrieError;
 
+use crate::{SnapshotClient, SnapshotWriter};
+
 use super::{
 	StateRebuilder,
 	SnapshotService,
@@ -55,13 +56,13 @@ use super::{
 };
 
 /// Helper for removing directories in case of error.
-struct Guard(bool, PathBuf);
+pub struct Guard(bool, PathBuf);
 
 impl Guard {
 	fn new(path: PathBuf) -> Self { Guard(true, path) }
 
-	#[cfg(test)]
+	#[cfg(any(test, feature = "test-helpers"))]
-	fn benign() -> Self { Guard(false, PathBuf::default()) }
+	pub fn benign() -> Self { Guard(false, PathBuf::default()) }
 
 	fn disarm(mut self) { self.0 = false }
 }
@@ -75,7 +76,7 @@ impl Drop for Guard {
 }
 
 /// State restoration manager.
-struct Restoration {
+pub struct Restoration {
 	manifest: ManifestData,
 	state_chunks_left: HashSet<H256>,
 	block_chunks_left: HashSet<H256>,
@@ -88,7 +89,8 @@ struct Restoration {
 	db: Arc<dyn BlockChainDB>,
 }
 
-struct RestorationParams<'a> {
+/// Params to initialise restoration
+pub struct RestorationParams<'a> {
 	manifest: ManifestData, // manifest to base restoration on.
 	pruning: Algorithm, // pruning algorithm for the database.
 	db: Arc<dyn BlockChainDB>, // database
@@ -98,9 +100,24 @@ struct RestorationParams<'a> {
 	engine: &'a dyn Engine,
 }
 
+#[cfg(any(test, feature = "test-helpers"))]
+impl<'a> RestorationParams<'a> {
+	pub fn new(
+		manifest: ManifestData,
+		pruning: Algorithm,
+		db: Arc<dyn BlockChainDB>,
+		writer: Option<LooseWriter>,
+		genesis: &'a [u8],
+		guard: Guard,
+		engine: &'a dyn Engine,
+	) -> Self {
+		Self { manifest, pruning, db, writer, genesis, guard, engine }
+	}
+}
+
 impl Restoration {
-	// make a new restoration using the given parameters.
+	/// Build a Restoration using the given parameters.
-	fn new(params: RestorationParams) -> Result<Self, Error> {
+	pub fn new(params: RestorationParams) -> Result<Self, Error> {
 		let manifest = params.manifest;
 
 		let state_chunks = manifest.state_hashes.iter().cloned().collect();
@@ -130,8 +147,8 @@ impl Restoration {
 		})
 	}
 
-	// feeds a state chunk, aborts early if `flag` becomes false.
+	/// Feeds a chunk of state data to the Restoration. Aborts early if `flag` becomes false.
-	fn feed_state(&mut self, hash: H256, chunk: &[u8], flag: &AtomicBool) -> Result<(), Error> {
+	pub fn feed_state(&mut self, hash: H256, chunk: &[u8], flag: &AtomicBool) -> Result<(), Error> {
 		if self.state_chunks_left.contains(&hash) {
 			let expected_len = snappy::decompressed_len(chunk)?;
 			if expected_len > MAX_CHUNK_SIZE {
@@ -152,8 +169,8 @@ impl Restoration {
 		Ok(())
 	}
 
-	// feeds a block chunk
+	/// Feeds a chunk of block data to the `Restoration`. Aborts early if `flag` becomes false.
-	fn feed_blocks(&mut self, hash: H256, chunk: &[u8], engine: &dyn Engine, flag: &AtomicBool) -> Result<(), Error> {
+	pub fn feed_blocks(&mut self, hash: H256, chunk: &[u8], engine: &dyn Engine, flag: &AtomicBool) -> Result<(), Error> {
 		if self.block_chunks_left.contains(&hash) {
 			let expected_len = snappy::decompressed_len(chunk)?;
 			if expected_len > MAX_CHUNK_SIZE {
@@ -199,8 +216,8 @@ impl Restoration {
 		Ok(())
 	}
 
-	// is everything done?
+	/// Check if we're done restoring: no more block chunks and no more state chunks to process.
-	fn is_done(&self) -> bool {
+	pub fn is_done(&self) -> bool {
 		self.block_chunks_left.is_empty() && self.state_chunks_left.is_empty()
 	}
 }
@@ -892,105 +909,3 @@ impl<C: Send + Sync> Drop for Service<C> {
 		trace!(target: "shutdown", "Dropping Service - snapshot aborted");
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use ethcore::client::Client;
-	use ethcore_io::IoService;
-	use spec;
-	use journaldb::Algorithm;
-	use crate::SnapshotService;
-	use super::*;
-	use common_types::{
-		io_message::ClientIoMessage,
-		snapshot::{ManifestData, RestorationStatus}
-	};
-	use tempdir::TempDir;
-	use ethcore::test_helpers::{generate_dummy_client_with_spec_and_data, restoration_db_handler};
-
-	#[test]
-	fn sends_async_messages() {
-		let gas_prices = vec![1.into(), 2.into(), 3.into(), 999.into()];
-		let client = generate_dummy_client_with_spec_and_data(spec::new_null, 400, 5, &gas_prices);
-		let service = IoService::<ClientIoMessage<Client>>::start().unwrap();
-		let spec = spec::new_test();
-
-		let tempdir = TempDir::new("").unwrap();
-		let dir = tempdir.path().join("snapshot");
-
-		let snapshot_params = ServiceParams {
-			engine: spec.engine.clone(),
-			genesis_block: spec.genesis_block(),
-			restoration_db_handler: restoration_db_handler(Default::default()),
-			pruning: Algorithm::Archive,
-			channel: service.channel(),
-			snapshot_root: dir,
-			client,
-		};
-
-		let service = Service::new(snapshot_params).unwrap();
-
-		assert!(service.manifest().is_none());
-		assert!(service.chunk(Default::default()).is_none());
-		assert_eq!(service.status(), RestorationStatus::Inactive);
-
-		let manifest = ManifestData {
-			version: 2,
-			state_hashes: vec![],
-			block_hashes: vec![],
-			state_root: Default::default(),
-			block_number: 0,
-			block_hash: Default::default(),
-		};
-
-		service.begin_restore(manifest);
-		service.abort_restore();
-		service.restore_state_chunk(Default::default(), vec![]);
-		service.restore_block_chunk(Default::default(), vec![]);
-	}
-
-	#[test]
-	fn cannot_finish_with_invalid_chunks() {
-		use ethereum_types::H256;
-		use kvdb_rocksdb::DatabaseConfig;
-
-		let spec = spec::new_test();
-		let tempdir = TempDir::new("").unwrap();
-
-		let state_hashes: Vec<_> = (0..5).map(|_| H256::random()).collect();
-		let block_hashes: Vec<_> = (0..5).map(|_| H256::random()).collect();
-		let db_config = DatabaseConfig::with_columns(ethcore_db::NUM_COLUMNS);
-		let gb = spec.genesis_block();
-		let flag = ::std::sync::atomic::AtomicBool::new(true);
-
-		let params = RestorationParams {
-			manifest: ManifestData {
-				version: 2,
-				state_hashes: state_hashes.clone(),
-				block_hashes: block_hashes.clone(),
-				state_root: H256::zero(),
-				block_number: 100000,
-				block_hash: H256::zero(),
-			},
-			pruning: Algorithm::Archive,
-			db: restoration_db_handler(db_config).open(&tempdir.path().to_owned()).unwrap(),
-			writer: None,
-			genesis: &gb,
-			guard: Guard::benign(),
-			engine: &*spec.engine.clone(),
-		};
-
-		let mut restoration = Restoration::new(params).unwrap();
-		let definitely_bad_chunk = [1, 2, 3, 4, 5];
-
-		for hash in state_hashes {
-			assert!(restoration.feed_state(hash, &definitely_bad_chunk, &flag).is_err());
-			assert!(!restoration.is_done());
-		}
-
-		for hash in block_hashes {
-			assert!(restoration.feed_blocks(hash, &definitely_bad_chunk, &*spec.engine, &flag).is_err());
-			assert!(!restoration.is_done());
-		}
-	}
-}

ethcore/snapshot/src/traits.rs

@@ -18,13 +18,16 @@ use std::sync::{Arc, atomic::AtomicBool};
 
 use blockchain::{BlockChain, BlockChainDB};
 use bytes::Bytes;
+use client_traits::{BlockChainClient, BlockInfo, DatabaseRestore, BlockChainReset};
 use common_types::{
+	ids::BlockId,
 	errors::{EthcoreError as Error, SnapshotError},
 	snapshot::{ManifestData, ChunkSink, Progress, RestorationStatus},
 };
 use engine::Engine;
 use ethereum_types::H256;
 
+use crate::io::SnapshotWriter;
 
 /// The interface for a snapshot network service.
 /// This handles:
@@ -129,3 +132,31 @@ pub trait SnapshotComponents: Send {
 	/// Current version number
 	fn current_version(&self) -> u64;
 }
+
+/// Snapshot related functionality
+pub trait SnapshotClient: BlockChainClient + BlockInfo + DatabaseRestore + BlockChainReset {
+	/// Take a snapshot at the given block.
+	/// If the ID given is "latest", this will default to 1000 blocks behind.
+	fn take_snapshot<W: SnapshotWriter + Send>(
+		&self,
+		writer: W,
+		at: BlockId,
+		p: &Progress,
+	) -> Result<(), Error>;
+}
+
+/// Helper trait for broadcasting a block to take a snapshot at.
+pub trait Broadcast: Send + Sync {
+	/// Start a snapshot from the given block number.
+	fn take_at(&self, num: Option<u64>);
+}
+
+
+/// Helper trait for transforming hashes to block numbers and checking if syncing.
+pub trait Oracle: Send + Sync {
+	/// Maps a block hash to a block number
+	fn to_number(&self, hash: H256) -> Option<u64>;
+
+	/// Are we currently syncing?
+	fn is_major_importing(&self) -> bool;
+}

ethcore/snapshot/src/watcher.rs

@@ -29,12 +29,7 @@ use ethcore_io::IoChannel;
 use log::{trace, warn};
 use parking_lot::Mutex;
 
-// helper trait for transforming hashes to numbers and checking if syncing.
+use crate::traits::{Broadcast, Oracle};
-trait Oracle: Send + Sync {
-	fn to_number(&self, hash: H256) -> Option<u64>;
-
-	fn is_major_importing(&self) -> bool;
-}
 
 struct StandardOracle<F> where F: 'static + Send + Sync + Fn() -> bool {
 	client: Arc<dyn BlockInfo>,
@@ -53,11 +48,6 @@ impl<F> Oracle for StandardOracle<F>
 	}
 }
 
-// helper trait for broadcasting a block to take a snapshot at.
-trait Broadcast: Send + Sync {
-	fn take_at(&self, num: Option<u64>);
-}
-
 impl<C: 'static> Broadcast for Mutex<IoChannel<ClientIoMessage<C>>> {
 	fn take_at(&self, num: Option<u64>) {
 		let num = match num {
@@ -65,7 +55,7 @@ impl<C: 'static> Broadcast for Mutex<IoChannel<ClientIoMessage<C>>> {
 			None => return,
 		};
 
-		trace!(target: "snapshot_watcher", "broadcast: {}", num);
+		trace!(target: "snapshot_watcher", "Snapshot requested at block #{}", num);
 
 		if let Err(e) = self.lock().send(ClientIoMessage::TakeSnapshot(num)) {
 			warn!("Snapshot watcher disconnected from IoService: {}", e);
@@ -86,7 +76,13 @@ impl Watcher {
 	/// Create a new `Watcher` which will trigger a snapshot event
 	/// once every `period` blocks, but only after that block is
 	/// `history` blocks old.
-	pub fn new<F, C>(client: Arc<dyn BlockInfo>, sync_status: F, channel: IoChannel<ClientIoMessage<C>>, period: u64, history: u64) -> Self
+	pub fn new<F, C>(
+		client: Arc<dyn BlockInfo>,
+		sync_status: F,
+		channel: IoChannel<ClientIoMessage<C>>,
+		period: u64,
+		history: u64
+	) -> Self
 		where
 			F: 'static + Send + Sync + Fn() -> bool,
 			C: 'static + Send + Sync,
@@ -98,6 +94,12 @@ impl Watcher {
 			history,
 		}
 	}
+
+	#[cfg(any(test, feature = "test-helpers"))]
+	/// Instantiate a `Watcher` using anything that impls `Oracle` and `Broadcast`. Test only.
+	pub fn new_test(oracle: Box<dyn Oracle>, broadcast: Box<dyn Broadcast>, period: u64, history: u64) -> Self {
+		Watcher { oracle, broadcast, period, history }
+	}
 }
 
 impl ChainNotify for Watcher {
@@ -119,82 +121,3 @@ impl ChainNotify for Watcher {
 		}
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use std::collections::HashMap;
-	use std::time::Duration;
-
-	use client_traits::ChainNotify;
-	use common_types::chain_notify::{NewBlocks, ChainRoute};
-
-	use ethereum_types::{H256, U256, BigEndianHash};
-
-	use super::{Broadcast, Oracle, Watcher};
-
-	struct TestOracle(HashMap<H256, u64>);
-
-	impl Oracle for TestOracle {
-		fn to_number(&self, hash: H256) -> Option<u64> {
-			self.0.get(&hash).cloned()
-		}
-
-		fn is_major_importing(&self) -> bool { false }
-	}
-
-	struct TestBroadcast(Option<u64>);
-	impl Broadcast for TestBroadcast {
-		fn take_at(&self, num: Option<u64>) {
-			if num != self.0 {
-				panic!("Watcher broadcast wrong number. Expected {:?}, found {:?}", self.0, num);
-			}
-		}
-	}
-
-	// helper harness for tests which expect a notification.
-	fn harness(numbers: Vec<u64>, period: u64, history: u64, expected: Option<u64>) {
-		const DURATION_ZERO: Duration = Duration::from_millis(0);
-
-		let hashes: Vec<_> = numbers.clone().into_iter().map(|x| BigEndianHash::from_uint(&U256::from(x))).collect();
-		let map = hashes.clone().into_iter().zip(numbers).collect();
-
-		let watcher = Watcher {
-			oracle: Box::new(TestOracle(map)),
-			broadcast: Box::new(TestBroadcast(expected)),
-			period,
-			history,
-		};
-
-		watcher.new_blocks(NewBlocks::new(
-			hashes,
-			vec![],
-			ChainRoute::default(),
-			vec![],
-			vec![],
-			DURATION_ZERO,
-			false
-		));
-	}
-
-	// helper
-
-	#[test]
-	fn should_not_fire() {
-		harness(vec![0], 5, 0, None);
-	}
-
-	#[test]
-	fn fires_once_for_two() {
-		harness(vec![14, 15], 10, 5, Some(10));
-	}
-
-	#[test]
-	fn finds_highest() {
-		harness(vec![15, 25], 10, 5, Some(20));
-	}
-
-	#[test]
-	fn doesnt_fire_before_history() {
-		harness(vec![10, 11], 10, 5, None);
-	}
-}

ethcore/src/client/client.rs

@@ -75,8 +75,6 @@ use client_traits::{
 	Nonce,
 	ProvingBlockChainClient,
 	ScheduleInfo,
-	SnapshotClient,
-	SnapshotWriter,
 	StateClient,
 	StateOrBlock,
 	Tick,
@@ -96,7 +94,7 @@ use machine::{
 	transaction_ext::Transaction,
 };
 use miner::{Miner, MinerService};
-use snapshot;
+use snapshot::{self, SnapshotClient, SnapshotWriter};
 use spec::Spec;
 use state_db::StateDB;
 use trace::{self, Database as TraceDatabase, ImportRequest as TraceImportRequest, LocalizedTrace, TraceDB};

parity/snapshot_cmd.rs

@@ -20,9 +20,8 @@ use std::time::Duration;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-use client_traits::SnapshotClient;
 use hash::keccak;
-use snapshot::{SnapshotConfiguration, SnapshotService as SS};
+use snapshot::{SnapshotConfiguration, SnapshotService as SS, SnapshotClient};
 use snapshot::io::{SnapshotReader, PackedReader, PackedWriter};
 use snapshot::service::Service as SnapshotService;
 use ethcore::client::{Client, DatabaseCompactionProfile, VMType};