From 0b511a180acfe509f6bbb44d68a68281d1fd21bf Mon Sep 17 00:00:00 2001 From: arkpar Date: Fri, 15 Jan 2016 12:26:04 +0100 Subject: [PATCH] Bad block reporting --- src/client.rs | 7 ++++-- src/error.rs | 4 ++-- src/queue.rs | 25 +++++++++++++++++---- src/sync/chain.rs | 1 + src/verification.rs | 53 +++++++++++++++++++++++++-------------------- 5 files changed, 58 insertions(+), 32 deletions(-) diff --git a/src/client.rs b/src/client.rs index 59734b387..6a8acf6d3 100644 --- a/src/client.rs +++ b/src/client.rs @@ -128,15 +128,16 @@ impl Client { pub fn import_verified_block(&mut self, bytes: Bytes) { let block = BlockView::new(&bytes); let header = block.header(); - if let Err(e) = verify_block_family(&bytes, self.engine.deref().deref(), self.chain.read().unwrap().deref()) { + if let Err(e) = verify_block_family(&header, &bytes, self.engine.deref().deref(), self.chain.read().unwrap().deref()) { warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - // TODO: mark as bad + self.queue.mark_as_bad(&header.hash()); return; }; let parent = match self.chain.read().unwrap().block_header(&header.parent_hash) { Some(p) => p, None => { warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); + self.queue.mark_as_bad(&header.hash()); return; }, }; @@ -158,11 +159,13 @@ impl Client { Ok(b) => b, Err(e) => { warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); + self.queue.mark_as_bad(&header.hash()); return; } }; if let Err(e) = verify_block_final(&header, result.block().header()) { warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); + self.queue.mark_as_bad(&header.hash()); return; } diff --git a/src/error.rs b/src/error.rs index 9973cb91c..dec13288d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -68,14 +68,14 @@ pub enum BlockError { #[derive(Debug)] pub enum ImportError { - Bad(Error), + Bad(Option), AlreadyInChain, AlreadyQueued, } impl From for ImportError { fn from(err: Error) -> ImportError { - ImportError::Bad(err) + ImportError::Bad(Some(err)) } } diff --git a/src/queue.rs b/src/queue.rs index 6c398e6f5..5ca361834 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -9,7 +9,8 @@ use views::*; /// Sorts them ready for blockchain insertion. pub struct BlockQueue { engine: Arc>, - message_channel: IoChannel + message_channel: IoChannel, + bad: HashSet, } impl BlockQueue { @@ -17,7 +18,8 @@ impl BlockQueue { pub fn new(engine: Arc>, message_channel: IoChannel) -> BlockQueue { BlockQueue { engine: engine, - message_channel: message_channel + message_channel: message_channel, + bad: HashSet::new(), } } @@ -27,16 +29,31 @@ impl BlockQueue { /// Add a block to the queue. pub fn import_block(&mut self, bytes: &[u8]) -> ImportResult { - try!(verify_block_basic(bytes, self.engine.deref().deref()).map_err(|e| { + let header = BlockView::new(bytes).header(); + if self.bad.contains(&header.hash()) { + return Err(ImportError::Bad(None)); + } + + if self.bad.contains(&header.parent_hash) { + self.bad.insert(header.hash()); + return Err(ImportError::Bad(None)); + } + + try!(verify_block_basic(&header, bytes, self.engine.deref().deref()).map_err(|e| { warn!(target: "client", "Stage 1 block verification failed for {}\nError: {:?}", BlockView::new(&bytes).header_view().sha3(), e); e })); - try!(verify_block_unordered(bytes, self.engine.deref().deref()).map_err(|e| { + try!(verify_block_unordered(&header, bytes, self.engine.deref().deref()).map_err(|e| { warn!(target: "client", "Stage 2 block verification failed for {}\nError: {:?}", BlockView::new(&bytes).header_view().sha3(), e); e })); try!(self.message_channel.send(UserMessage(SyncMessage::BlockVerified(bytes.to_vec()))).map_err(|e| Error::from(e))); Ok(()) } + + pub fn mark_as_bad(&mut self, hash: &H256) { + self.bad.insert(hash.clone()); + //TODO: walk the queue + } } diff --git a/src/sync/chain.rs b/src/sync/chain.rs index ce567556c..43f5968f4 100644 --- a/src/sync/chain.rs +++ b/src/sync/chain.rs @@ -473,6 +473,7 @@ impl ChainSync { if self.peers.contains_key(&peer) { info!(target: "sync", "Disconneced {}:{}", peer, io.peer_info(peer)); self.clear_peer_download(peer); + self.peers.remove(&peer); self.continue_sync(io); } } diff --git a/src/verification.rs b/src/verification.rs index bfc97e1eb..4383f5a4a 100644 --- a/src/verification.rs +++ b/src/verification.rs @@ -10,9 +10,7 @@ use engine::Engine; use blockchain::*; /// Phase 1 quick block verification. Only does checks that are cheap. Operates on a single block -pub fn verify_block_basic(bytes: &[u8], engine: &Engine) -> Result<(), Error> { - let block = BlockView::new(bytes); - let header = block.header(); +pub fn verify_block_basic(header: &Header, bytes: &[u8], engine: &Engine) -> Result<(), Error> { try!(verify_header(&header, engine)); try!(verify_block_integrity(bytes, &header.transactions_root, &header.uncles_hash)); try!(engine.verify_block_basic(&header, Some(bytes))); @@ -26,9 +24,7 @@ pub fn verify_block_basic(bytes: &[u8], engine: &Engine) -> Result<(), Error> { /// Phase 2 verification. Perform costly checks such as transaction signatures and block nonce for ethash. /// Still operates on a individual block /// TODO: return cached transactions, header hash. -pub fn verify_block_unordered(bytes: &[u8], engine: &Engine) -> Result<(), Error> { - let block = BlockView::new(bytes); - let header = block.header(); +pub fn verify_block_unordered(header: &Header, bytes: &[u8], engine: &Engine) -> Result<(), Error> { try!(engine.verify_block_unordered(&header, Some(bytes))); for u in Rlp::new(bytes).at(2).iter().map(|rlp| rlp.as_val::
()) { try!(engine.verify_block_unordered(&u, None)); @@ -37,10 +33,8 @@ pub fn verify_block_unordered(bytes: &[u8], engine: &Engine) -> Result<(), Error } /// Phase 3 verification. Check block information against parent and uncles. -pub fn verify_block_family(bytes: &[u8], engine: &Engine, bc: &BC) -> Result<(), Error> where BC: BlockProvider { +pub fn verify_block_family(header: &Header, bytes: &[u8], engine: &Engine, bc: &BC) -> Result<(), Error> where BC: BlockProvider { // TODO: verify timestamp - let block = BlockView::new(bytes); - let header = block.header(); let parent = try!(bc.block_header(&header.parent_hash).ok_or::(From::from(BlockError::UnknownParent(header.parent_hash.clone())))); try!(verify_parent(&header, &parent)); try!(engine.verify_block_family(&header, &parent, Some(bytes))); @@ -194,6 +188,7 @@ mod tests { use error::BlockError::*; use views::*; use blockchain::*; + use engine::*; use ethereum; fn create_test_block(header: &Header) -> Bytes { @@ -280,6 +275,16 @@ mod tests { } } + fn basic_test(bytes: &[u8], engine: &Engine) -> Result<(), Error> { + let header = BlockView::new(bytes).header(); + verify_block_basic(&header, bytes, engine) + } + + fn family_test(bytes: &[u8], engine: &Engine, bc: &BC) -> Result<(), Error> where BC: BlockProvider { + let header = BlockView::new(bytes).header(); + verify_block_family(&header, bytes, engine, bc) + } + #[test] fn test_verify_block() { // Test against morden @@ -348,69 +353,69 @@ mod tests { bc.insert(create_test_block(&parent7)); bc.insert(create_test_block(&parent8)); - check_ok(verify_block_basic(&create_test_block(&good), engine.deref())); + check_ok(basic_test(&create_test_block(&good), engine.deref())); let mut header = good.clone(); header.transactions_root = good_transactions_root.clone(); header.uncles_hash = good_uncles_hash.clone(); - check_ok(verify_block_basic(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref())); + check_ok(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref())); header.gas_limit = min_gas_limit - From::from(1); - check_fail(verify_block_basic(&create_test_block(&header), engine.deref()), + check_fail(basic_test(&create_test_block(&header), engine.deref()), InvalidGasLimit(OutOfBounds { min: Some(min_gas_limit), max: None, found: header.gas_limit })); header = good.clone(); header.number = BlockNumber::max_value(); - check_fail(verify_block_basic(&create_test_block(&header), engine.deref()), + check_fail(basic_test(&create_test_block(&header), engine.deref()), InvalidNumber(OutOfBounds { max: Some(BlockNumber::max_value()), min: None, found: header.number })); header = good.clone(); header.gas_used = header.gas_limit + From::from(1); - check_fail(verify_block_basic(&create_test_block(&header), engine.deref()), + check_fail(basic_test(&create_test_block(&header), engine.deref()), TooMuchGasUsed(OutOfBounds { max: Some(header.gas_limit), min: None, found: header.gas_used })); header = good.clone(); header.extra_data.resize(engine.maximum_extra_data_size() + 1, 0u8); - check_fail(verify_block_basic(&create_test_block(&header), engine.deref()), + check_fail(basic_test(&create_test_block(&header), engine.deref()), ExtraDataOutOfBounds(OutOfBounds { max: Some(engine.maximum_extra_data_size()), min: None, found: header.extra_data.len() })); header = good.clone(); header.extra_data.resize(engine.maximum_extra_data_size() + 1, 0u8); - check_fail(verify_block_basic(&create_test_block(&header), engine.deref()), + check_fail(basic_test(&create_test_block(&header), engine.deref()), ExtraDataOutOfBounds(OutOfBounds { max: Some(engine.maximum_extra_data_size()), min: None, found: header.extra_data.len() })); header = good.clone(); header.uncles_hash = good_uncles_hash.clone(); - check_fail(verify_block_basic(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref()), + check_fail(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref()), InvalidTransactionsRoot(Mismatch { expected: good_transactions_root.clone(), found: header.transactions_root })); header = good.clone(); header.transactions_root = good_transactions_root.clone(); - check_fail(verify_block_basic(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref()), + check_fail(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref()), InvalidUnclesHash(Mismatch { expected: good_uncles_hash.clone(), found: header.uncles_hash })); - check_ok(verify_block_family(&create_test_block(&good), engine.deref(), &bc)); - check_ok(verify_block_family(&create_test_block_with_data(&good, &good_transactions, &good_uncles), engine.deref(), &bc)); + check_ok(family_test(&create_test_block(&good), engine.deref(), &bc)); + check_ok(family_test(&create_test_block_with_data(&good, &good_transactions, &good_uncles), engine.deref(), &bc)); header = good.clone(); header.parent_hash = H256::random(); - check_fail(verify_block_family(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref(), &bc), + check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref(), &bc), UnknownParent(header.parent_hash)); header = good.clone(); header.timestamp = 10; - check_fail(verify_block_family(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref(), &bc), + check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref(), &bc), InvalidTimestamp(OutOfBounds { max: None, min: Some(parent.timestamp + 1), found: header.timestamp })); header = good.clone(); header.number = 9; - check_fail(verify_block_family(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref(), &bc), + check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine.deref(), &bc), InvalidNumber(OutOfBounds { max: None, min: Some(parent.number + 1), found: header.number })); header = good.clone(); let mut bad_uncles = good_uncles.clone(); bad_uncles.push(good_uncle1.clone()); - check_fail(verify_block_family(&create_test_block_with_data(&header, &good_transactions, &bad_uncles), engine.deref(), &bc), + check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &bad_uncles), engine.deref(), &bc), TooManyUncles(OutOfBounds { max: Some(engine.maximum_uncle_count()), min: None, found: bad_uncles.len() })); // TODO: some additional uncle checks