Networking tests and fixes (#61)

* BlockId in client interface

* Sync fixes and tests

* Updated to latest primitives

* Updated dependencies

* Updated for new (old) primitives

* Network as workspace member

* substrate-network

* Removed obsolete file

* begin_transaction on hash

Cargo.toml

@@ -10,22 +10,21 @@ authors = ["Parity Technologies <admin@parity.io>"]
 [dependencies]
 error-chain = "0.11"
 polkadot-cli = { path = "polkadot/cli" }
-polkadot-network = { path = "polkadot/network" }
 
 [workspace]
 members = [
 	"substrate/client",
 	"substrate/codec",
 	"substrate/environmental",
 	"substrate/executor",
-	"polkadot/network",
 	"polkadot/candidate-agreement",
 	"polkadot/cli",
 	"polkadot/collator",
 	"polkadot/executor",
 	"polkadot/runtime",
 	"polkadot/primitives",
 	"polkadot/validator",
+	"substrate/network",
 	"substrate/primitives",
 	"substrate/rpc",
 	"substrate/rpc-servers",

substrate/client/src/in_mem.rs

@@ -35,6 +35,7 @@ struct PendingBlock {
 	is_best: bool,
 }
 
+#[derive(PartialEq, Eq, Clone)]
 struct Block {
 	header: block::Header,
 	body: Option<block::Body>,
@@ -46,6 +47,7 @@ pub struct BlockImportOperation {
 	pending_state: state_machine::backend::InMemory,
 }
 
+#[derive(Clone)]
 struct BlockchainStorage {
 	blocks: HashMap<HeaderHash, Block>,
 	hashes: HashMap<block::Number, HeaderHash>,
@@ -59,6 +61,14 @@ pub struct Blockchain {
 	storage: RwLock<BlockchainStorage>,
 }
 
+impl Clone for Blockchain {
+	fn clone(&self) -> Blockchain {
+		Blockchain {
+			storage: RwLock::new(self.storage.read().clone()),
+		}
+	}
+}
+
 impl Blockchain {
 	fn id(&self, id: BlockId) -> Option<HeaderHash> {
 		match id {
@@ -96,6 +106,21 @@ impl Blockchain {
 			storage.genesis_hash = hash;
 		}
 	}
+
+	/// Compare this blockchain with another in-mem blockchain
+	pub fn equals_to(&self, other: &Blockchain) -> bool {
+		self.canon_equals_to(other) && self.storage.read().blocks == other.storage.read().blocks
+	}
+
+	/// Compare canonical chain to other canonical chain.
+	pub fn canon_equals_to(&self, other: &Blockchain) -> bool {
+		let this = self.storage.read();
+		let other = other.storage.read();
+			this.hashes == other.hashes
+		    && this.best_hash == other.best_hash
+			&& this.best_number == other.best_number
+			&& this.genesis_hash == other.genesis_hash
+	}
 }
 
 impl blockchain::Backend for Blockchain {
@@ -167,6 +192,39 @@ impl Backend {
 			blockchain: Blockchain::new(),
 		}
 	}
+
+	/// Generate and import a sequence of blocks. A user supplied function is allowed to modify each block header. Useful for testing.
+	pub fn generate_blocks<F>(&self, count: usize, edit_header: F) where F: Fn(&mut block::Header) {
+		use backend::{Backend, BlockImportOperation};
+		let info = blockchain::Backend::info(&self.blockchain).expect("In-memory backend never fails");
+		let mut best_num = info.best_number;
+		let mut best_hash = info.best_hash;
+		let state_root = blockchain::Backend::header(&self.blockchain, BlockId::Hash(best_hash))
+			.expect("In-memory backend never fails")
+			.expect("Best header always exists in the blockchain")
+			.state_root;
+		for _ in 0 .. count {
+			best_num = best_num + 1;
+			let mut header = block::Header {
+				parent_hash: best_hash,
+				number: best_num,
+				state_root: state_root,
+				transaction_root: Default::default(),
+				digest: Default::default(),
+			};
+			edit_header(&mut header);
+
+			let mut tx = self.begin_transaction(BlockId::Hash(best_hash)).expect("In-memory backend does not fail");
+			best_hash = header_hash(&header);
+			tx.import_block(header, None, true).expect("In-memory backend does not fail");
+			self.commit_transaction(tx).expect("In-memory backend does not fail");
+		}
+	}
+
+	/// Generate and import a sequence of blocks. Useful for testing.
+	pub fn push_blocks(&self, count: usize) {
+		self.generate_blocks(count, |_| {})
+	}
 }
 
 impl backend::Backend for Backend {

substrate/client/src/lib.rs

@@ -139,22 +139,27 @@ impl<B, E> Client<B, E> where
 		})
 	}
 
-	fn state_at(&self, hash: &block::HeaderHash) -> error::Result<B::State> {
-		self.backend.state_at(BlockId::Hash(*hash))
+	fn state_at(&self, id: BlockId) -> error::Result<B::State> {
+		self.backend.state_at(id)
 	}
 
-	/// Return single storage entry of contract under given address in state in a block of given hash.
-	pub fn storage(&self, hash: &block::HeaderHash, key: &StorageKey) -> error::Result<StorageData> {
-		Ok(self.state_at(hash)?
+	/// Expose backend reference. To be used in tests only
+	pub fn backend(&self) -> &B {
+		&self.backend
+	}
+
+	/// Return single storage entry of contract under given address in state in a block of given id.
+	pub fn storage(&self, id: &BlockId, key: &StorageKey) -> error::Result<StorageData> {
+		Ok(self.state_at(*id)?
 			.storage(&key.0)
 			.map(|x| StorageData(x.to_vec()))?)
 	}
 
-	/// Execute a call to a contract on top of state in a block of given hash.
+	/// Execute a call to a contract on top of state in a block of given id.
 	///
 	/// No changes are made.
-	pub fn call(&self, hash: &block::HeaderHash, method: &str, call_data: &[u8]) -> error::Result<CallResult> {
-		let state = self.state_at(hash)?;
+	pub fn call(&self, id: &BlockId, method: &str, call_data: &[u8]) -> error::Result<CallResult> {
+		let state = self.state_at(*id)?;
 		let mut changes = state_machine::OverlayedChanges::default();
 
 		let _ = state_machine::execute(
@@ -176,7 +181,7 @@ impl<B, E> Client<B, E> where
 			blockchain::BlockStatus::Unknown => return Ok(ImportResult::UnknownParent),
 		}
 
-		let mut transaction = self.backend.begin_transaction(BlockId::Number(header.number))?;
+		let mut transaction = self.backend.begin_transaction(BlockId::Hash(header.parent_hash))?;
 		let mut _state = transaction.state()?;
 		// TODO: execute block on _state
 
@@ -197,9 +202,9 @@ impl<B, E> Client<B, E> where
 	}
 
 	/// Get block status.
-	pub fn block_status(&self, hash: &block::HeaderHash) -> error::Result<BlockStatus> {
+	pub fn block_status(&self, id: &BlockId) -> error::Result<BlockStatus> {
 		// TODO: more efficient implementation
-		match self.backend.blockchain().header(BlockId::Hash(*hash)).map_err(|e| error::Error::from_blockchain(Box::new(e)))?.is_some() {
+		match self.backend.blockchain().header(*id).map_err(|e| error::Error::from_blockchain(Box::new(e)))?.is_some() {
 			true => Ok(BlockStatus::InChain),
 			false => Ok(BlockStatus::Unknown),
 		}
@@ -210,8 +215,13 @@ impl<B, E> Client<B, E> where
 		self.backend.blockchain().hash(block_number)
 	}
 
-	/// Get block header by hash.
-	pub fn header(&self, hash: &block::HeaderHash) -> error::Result<Option<block::Header>> {
-		self.backend.blockchain().header(BlockId::Hash(*hash))
+	/// Get block header by id.
+	pub fn header(&self, id: &BlockId) -> error::Result<Option<block::Header>> {
+		self.backend.blockchain().header(*id)
+	}
+
+	/// Get block body by id.
+	pub fn body(&self, id: &BlockId) -> error::Result<Option<block::Body>> {
+		self.backend.blockchain().body(*id)
 	}
 }

substrate/executor/src/wasm_executor.rs

@@ -330,9 +330,8 @@ impl CodeExecutor for WasmExecutor {
 mod tests {
 	use super::*;
 	use rustc_hex::FromHex;
-	use codec::{Slicable, Joiner};
+	use codec::Slicable;
 	use state_machine::TestExternalities;
-	use primitives::Header;
 
 	#[test]
 	fn returning_should_work() {