Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
fix bad-block reporting no reason (#9638)
Browse files Browse the repository at this point in the history
  • Loading branch information
debris authored and sorpaas committed Sep 25, 2018
1 parent 4c2301f commit 8875dcc
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 20 deletions.
29 changes: 16 additions & 13 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ impl Importer {
continue;
}

let raw = block.bytes.clone();
match self.check_and_lock_block(block, client) {
Ok(closed_block) => {
if self.engine.is_proposal(&header) {
Expand All @@ -312,8 +311,8 @@ impl Importer {
}
},
Err(err) => {
self.bad_blocks.report(raw, format!("{:?}", err));
invalid_blocks.insert(header.hash());
self.bad_blocks.report(bytes, format!("{:?}", err));
invalid_blocks.insert(hash);
},
}
}
Expand Down Expand Up @@ -354,23 +353,23 @@ impl Importer {
imported
}

fn check_and_lock_block(&self, block: PreverifiedBlock, client: &Client) -> Result<LockedBlock, ()> {
fn check_and_lock_block(&self, block: PreverifiedBlock, client: &Client) -> EthcoreResult<LockedBlock> {
let engine = &*self.engine;
let header = block.header.clone();

// Check the block isn't so old we won't be able to enact it.
let best_block_number = client.chain.read().best_block_number();
if client.pruning_info().earliest_state > header.number() {
warn!(target: "client", "Block import failed for #{} ({})\nBlock is ancient (current best block: #{}).", header.number(), header.hash(), best_block_number);
return Err(());
bail!("Block is ancient");
}

// Check if parent is in chain
let parent = match client.block_header_decoded(BlockId::Hash(*header.parent_hash())) {
Some(h) => h,
None => {
warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash());
return Err(());
bail!("Parent not found");
}
};

Expand All @@ -389,13 +388,13 @@ impl Importer {

if let Err(e) = verify_family_result {
warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
return Err(());
bail!(e);
};

let verify_external_result = self.verifier.verify_block_external(&header, engine);
if let Err(e) = verify_external_result {
warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
return Err(());
bail!(e);
};

// Enact Verified Block
Expand All @@ -415,9 +414,13 @@ impl Importer {
&mut chain.ancestry_with_metadata_iter(*header.parent_hash()),
);

let mut locked_block = enact_result.map_err(|e| {
warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
})?;
let mut locked_block = match enact_result {
Ok(b) => b,
Err(e) => {
warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
bail!(e);
}
};

// Strip receipts for blocks before validate_receipts_transition,
// if the expected receipts root header does not match.
Expand All @@ -431,7 +434,7 @@ impl Importer {
// Final Verification
if let Err(e) = self.verifier.verify_block_final(&header, locked_block.block().header()) {
warn!(target: "client", "Stage 5 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
return Err(());
bail!(e);
}

Ok(locked_block)
Expand All @@ -441,7 +444,7 @@ impl Importer {
///
/// The block is guaranteed to be the next best blocks in the
/// first block sequence. Does no sealing or transaction validation.
fn import_old_block(&self, unverified: Unverified, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> Result<(), ::error::Error> {
fn import_old_block(&self, unverified: Unverified, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> EthcoreResult<()> {
let receipts = ::rlp::decode_list(receipts_bytes);
let _import_lock = self.import_lock.lock();

Expand Down
14 changes: 7 additions & 7 deletions ethcore/src/verification/queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,19 +465,19 @@ impl<K: Kind> VerificationQueue<K> {

/// Add a block to the queue.
pub fn import(&self, input: K::Input) -> EthcoreResult<H256> {
let h = input.hash();
let hash = input.hash();
{
if self.processing.read().contains_key(&h) {
if self.processing.read().contains_key(&hash) {
bail!(ErrorKind::Import(ImportErrorKind::AlreadyQueued));
}

let mut bad = self.verification.bad.lock();
if bad.contains(&h) {
if bad.contains(&hash) {
bail!(ErrorKind::Import(ImportErrorKind::KnownBad));
}

if bad.contains(&input.parent_hash()) {
bad.insert(h.clone());
bad.insert(hash);
bail!(ErrorKind::Import(ImportErrorKind::KnownBad));
}
}
Expand All @@ -486,21 +486,21 @@ impl<K: Kind> VerificationQueue<K> {
Ok(item) => {
self.verification.sizes.unverified.fetch_add(item.heap_size_of_children(), AtomicOrdering::SeqCst);

self.processing.write().insert(h.clone(), item.difficulty());
self.processing.write().insert(hash, item.difficulty());
{
let mut td = self.total_difficulty.write();
*td = *td + item.difficulty();
}
self.verification.unverified.lock().push_back(item);
self.more_to_verify.notify_all();
Ok(h)
Ok(hash)
},
Err(err) => {
match err {
// Don't mark future blocks as bad.
Error(ErrorKind::Block(BlockError::TemporarilyInvalid(_)), _) => {},
_ => {
self.verification.bad.lock().insert(h.clone());
self.verification.bad.lock().insert(hash);
}
}
Err(err)
Expand Down

0 comments on commit 8875dcc

Please sign in to comment.