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

fix bad-block reporting no reason #9638

Merged
merged 1 commit into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block bytes is already cloned

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));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_and_lock_block always returned () as an error and it was reported as a reason

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();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just trying to make code more neat

{
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