From fe0363e230440341d7540444dd295bdbef2b7e16 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 27 Jan 2016 13:28:15 +0100 Subject: [PATCH] Fix import for bcMultiChainTest. Fixes #223 --- src/block_queue.rs | 14 ++++++++------ src/client.rs | 15 +++++++++------ src/error.rs | 2 +- src/sync/chain.rs | 4 ++-- src/sync/tests.rs | 11 ++++++----- src/tests/chain.rs | 20 +++++++++++++------- 6 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/block_queue.rs b/src/block_queue.rs index e0868c011..dab3a5fe3 100644 --- a/src/block_queue.rs +++ b/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. diff --git a/src/client.rs b/src/client.rs index a83ff554e..8aea437e4 100644 --- a/src/client.rs +++ b/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) { + pub fn import_verified_blocks(&self, _io: &IoChannel) -> 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 diff --git a/src/error.rs b/src/error.rs index bc2bdfe97..cdc0c2d25 100644 --- a/src/error.rs +++ b/src/error.rs @@ -145,7 +145,7 @@ impl From for ImportError { } /// Result of import block operation. -pub type ImportResult = Result<(), ImportError>; +pub type ImportResult = Result; #[derive(Debug)] /// General error type which should be capable of representing all errors in ethcore. diff --git a/src/sync/chain.rs b/src/sync/chain.rs index aaba701c2..1f79477af 100644 --- a/src/sync/chain.rs +++ b/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(); diff --git a/src/sync/tests.rs b/src/sync/tests.rs index 50d6efab2..cc0645b50 100644 --- a/src/sync/tests.rs +++ b/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::(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()); - self.blocks.write().unwrap().insert(header.hash(), b); - self.numbers.write().unwrap().insert(number, header.hash()); + mem::replace(self.last_hash.write().unwrap().deref_mut(), h.clone()); + self.blocks.write().unwrap().insert(h.clone(), b); + 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 { diff --git a/src/tests/chain.rs b/src/tests/chain.rs index ec899c73b..8ba5e3671 100644 --- a/src/tests/chain.rs +++ b/src/tests/chain.rs @@ -21,7 +21,7 @@ fn do_json_test(json_data: &[u8]) -> Vec { flush(format!(" - {}...", name)); - let blocks: Vec = 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 { 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)) { - client.import_block(b).unwrap(); + for (b, is_valid) in blocks.into_iter() { + 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_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"} // FAILS +declare_test!{BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"} +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"}