Fix import for bcMultiChainTest.

Fixes #223

src/block_queue.rs

@@ -200,34 +200,36 @@ impl BlockQueue {
 	/// Add a block to the queue.
 	pub fn import_block(&mut self, bytes: Bytes) -> ImportResult {
 		let header = BlockView::new(&bytes).header();
-		if self.processing.contains(&header.hash()) {
+		let h = header.hash();
+		if self.processing.contains(&h) {
 			return Err(ImportError::AlreadyQueued);
 		}
 		{
 			let mut verification = self.verification.lock().unwrap();
-			if verification.bad.contains(&header.hash()) {
+			if verification.bad.contains(&h) {
 				return Err(ImportError::Bad(None));
 			}
 
 			if verification.bad.contains(&header.parent_hash) {
-				verification.bad.insert(header.hash());
+				verification.bad.insert(h.clone());
 				return Err(ImportError::Bad(None));
 			}
 		}
 
 		match verify_block_basic(&header, &bytes, self.engine.deref().deref()) {
 			Ok(()) => {
-				self.processing.insert(header.hash());
+				self.processing.insert(h.clone());
 				self.verification.lock().unwrap().unverified.push_back(UnVerifiedBlock { header: header, bytes: bytes });
 				self.more_to_verify.notify_all();
+				Ok(h)
 			},
 			Err(err) => {
 				flushln!("Stage 1 block verification failed for {}\nError: {:?}", BlockView::new(&bytes).header_view().sha3(), err);
 				warn!(target: "client", "Stage 1 block verification failed for {}\nError: {:?}", BlockView::new(&bytes).header_view().sha3(), err);
-				self.verification.lock().unwrap().bad.insert(header.hash());
+				self.verification.lock().unwrap().bad.insert(h.clone());
+				Err(From::from(err))
 			}
 		}
-		Ok(())
 	}
 
 	/// Mark given block and all its children as bad. Stops verification.

src/client.rs

@@ -193,7 +193,8 @@ impl Client {
 	}
 
 	/// This is triggered by a message coming from a block queue when the block is ready for insertion
-	pub fn import_verified_blocks(&self, _io: &IoChannel<NetSyncMessage>) {
+	pub fn import_verified_blocks(&self, _io: &IoChannel<NetSyncMessage>) -> usize {
+		let mut ret = 0;
 		let mut bad = HashSet::new();
 		let _import_lock = self.import_lock.lock();
 		let blocks = self.block_queue.write().unwrap().drain(128);
@@ -211,7 +212,7 @@ impl Client {
 				warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
 				self.block_queue.write().unwrap().mark_as_bad(&header.hash());
 				bad.insert(block.header.hash());
-				return;
+				break;
 			};
 			let parent = match self.chain.read().unwrap().block_header(&header.parent_hash) {
 				Some(p) => p,
@@ -220,7 +221,7 @@ impl Client {
 					warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash);
 					self.block_queue.write().unwrap().mark_as_bad(&header.hash());
 					bad.insert(block.header.hash());
-					return;
+					break;
 				},
 			};
 			// build last hashes
@@ -244,14 +245,14 @@ impl Client {
 					warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
 					bad.insert(block.header.hash());
 					self.block_queue.write().unwrap().mark_as_bad(&header.hash());
-					return;
+					break;
 				}
 			};
 			if let Err(e) = verify_block_final(&header, result.block().header()) {
 				flushln!("Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
 				warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
 				self.block_queue.write().unwrap().mark_as_bad(&header.hash());
-				return;
+				break;
 			}
 
 			self.chain.write().unwrap().insert_block(&block.bytes); //TODO: err here?
@@ -260,12 +261,14 @@ impl Client {
 				Ok(_) => (),
 				Err(e) => {
 					warn!(target: "client", "State DB commit failed: {:?}", e);
-					return;
+					break;
 				}
 			}
 			self.report.write().unwrap().accrue_block(&block);
 			trace!(target: "client", "Imported #{} ({})", header.number(), header.hash());
+			ret += 1;
 		}
+		ret
 	}
 
 	/// Clear cached state overlay 

src/error.rs

@@ -145,7 +145,7 @@ impl From<Error> for ImportError {
 }
 
 /// Result of import block operation.
-pub type ImportResult = Result<(), ImportError>;
+pub type ImportResult = Result<H256, ImportError>;
 
 #[derive(Debug)]
 /// General error type which should be capable of representing all errors in ethcore.

src/sync/chain.rs

@@ -415,7 +415,7 @@ impl ChainSync {
 				Err(ImportError::AlreadyQueued) => {
 					trace!(target: "sync", "New block already queued {:?}", h);
 				},
-				Ok(()) => {
+				Ok(_) => {
 					trace!(target: "sync", "New block queued {:?}", h);
 				},
 				Err(e) => {
@@ -680,7 +680,7 @@ impl ChainSync {
 						self.last_imported_block = headers.0 + i as BlockNumber;
 						self.last_imported_hash = h.clone();
 					},
-					Ok(()) => {
+					Ok(_) => {
 						trace!(target: "sync", "Block queued {:?}", h);
 						self.last_imported_block = headers.0 + i as BlockNumber;
 						self.last_imported_hash = h.clone();

src/sync/tests.rs

@@ -114,6 +114,7 @@ impl BlockChainClient for TestBlockChainClient {
 
 	fn import_block(&self, b: Bytes) -> ImportResult {
 		let header = Rlp::new(&b).val_at::<BlockHeader>(0);
+		let h = header.hash();
 		let number: usize = header.number as usize;
 		if number > self.blocks.read().unwrap().len() {
 			panic!("Unexpected block number. Expected {}, got {}", self.blocks.read().unwrap().len(), number);
@@ -134,9 +135,9 @@ impl BlockChainClient for TestBlockChainClient {
 		let len = self.numbers.read().unwrap().len();
 		if number == len {
 			*self.difficulty.write().unwrap().deref_mut() += header.difficulty;
-			mem::replace(self.last_hash.write().unwrap().deref_mut(), header.hash());
+			mem::replace(self.last_hash.write().unwrap().deref_mut(), h.clone());
-			self.blocks.write().unwrap().insert(header.hash(), b);
+			self.blocks.write().unwrap().insert(h.clone(), b);
-			self.numbers.write().unwrap().insert(number, header.hash());
+			self.numbers.write().unwrap().insert(number, h.clone());
 			let mut parent_hash = header.parent_hash;
 			if number > 0 {
 				let mut n = number - 1;
@@ -148,9 +149,9 @@ impl BlockChainClient for TestBlockChainClient {
 			}
 		}
 		else {
-			self.blocks.write().unwrap().insert(header.hash(), b.to_vec());
+			self.blocks.write().unwrap().insert(h.clone(), b.to_vec());
 		}
-		Ok(())
+		Ok(h)
 	}
 
 	fn queue_info(&self) -> BlockQueueInfo {

src/tests/chain.rs

@@ -21,7 +21,7 @@ fn do_json_test(json_data: &[u8]) -> Vec<String> {
 
 			flush(format!("   - {}...", name));
 
-			let blocks: Vec<Bytes> = test["blocks"].as_array().unwrap().iter().map(|e| xjson!(&e["rlp"])).collect();
+			let blocks: Vec<(Bytes, bool)> = test["blocks"].as_array().unwrap().iter().map(|e| (xjson!(&e["rlp"]), e.find("blockHeader").is_some())).collect();
 			let mut spec = ethereum::new_frontier_like_test();
 			let s = PodState::from_json(test.find("pre").unwrap());
 			spec.set_genesis_state(s);
@@ -32,11 +32,17 @@ fn do_json_test(json_data: &[u8]) -> Vec<String> {
 			dir.push(H32::random().hex());
 			{
 				let client = Client::new(spec, &dir, IoChannel::disconnected()).unwrap();
-				for b in blocks.into_iter().filter(|ref b| Block::is_good(b)) {
+				for (b, is_valid) in blocks.into_iter() {
-					client.import_block(b).unwrap();
+					let mut hash = H256::new();
+					if Block::is_good(&b) {
+						if let Ok(h) = client.import_block(b.clone()) {
+							hash = h;
+						}
+					}
+					client.flush_queue();
+					let imported_ok = client.import_verified_blocks(&IoChannel::disconnected()) > 0;
+					assert_eq!(imported_ok, is_valid);	// may yet be invalid for the later stages, so can't do a hard check.
 				}
-				client.flush_queue();
-				client.import_verified_blocks(&IoChannel::disconnected());
 				fail_unless(client.chain_info().best_block_hash == H256::from_json(&test["lastblockhash"]));
 			}
 			fs::remove_dir_all(&dir).unwrap();
@@ -55,8 +61,8 @@ declare_test!{BlockchainTests_bcForkStressTest, "BlockchainTests/bcForkStressTes
 declare_test!{BlockchainTests_bcForkUncle, "BlockchainTests/bcForkUncle"}						// STILL FAILS
 declare_test!{BlockchainTests_bcGasPricerTest, "BlockchainTests/bcGasPricerTest"}
 declare_test!{BlockchainTests_bcInvalidHeaderTest, "BlockchainTests/bcInvalidHeaderTest"}
-declare_test!{BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"}				// FAILS
+declare_test!{BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"}
-declare_test!{BlockchainTests_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"}				// FAILS
+declare_test!{BlockchainTests_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"}
 declare_test!{BlockchainTests_bcRPC_API_Test, "BlockchainTests/bcRPC_API_Test"}
 declare_test!{BlockchainTests_bcStateTest, "BlockchainTests/bcStateTest"}
 declare_test!{BlockchainTests_bcTotalDifficultyTest, "BlockchainTests/bcTotalDifficultyTest"}