-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Bad blocks RPC + reporting #9433
Changes from 5 commits
c96ee74
eaf9928
b22388d
e67f5ce
6d2bd28
aa46cd9
1686698
395ec14
023da5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
// Copyright 2015-2018 Parity Technologies (UK) Ltd. | ||
// This file is part of Parity. | ||
|
||
// Parity is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
|
||
// Parity is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
|
||
// You should have received a copy of the GNU General Public License | ||
// along with Parity. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
//! Stores recently seen bad blocks. | ||
|
||
use bytes::{Bytes, ToPretty}; | ||
use ethereum_types::H256; | ||
use itertools::Itertools; | ||
use memory_cache::MemoryLruCache; | ||
use parking_lot::RwLock; | ||
use verification::queue::kind::blocks::Unverified; | ||
|
||
/// Recently seen bad blocks. | ||
pub struct BadBlocks { | ||
last_blocks: RwLock<MemoryLruCache<H256, (Unverified, String)>>, | ||
} | ||
|
||
impl Default for BadBlocks { | ||
fn default() -> Self { | ||
BadBlocks { | ||
last_blocks: RwLock::new(MemoryLruCache::new(8 * 1024 * 1024)), | ||
} | ||
} | ||
} | ||
|
||
impl BadBlocks { | ||
/// Reports given RLP as invalid block. | ||
pub fn report(&self, raw: Bytes, message: String) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra whitespace. |
||
match Unverified::from_rlp(raw) { | ||
Ok(unverified) => { | ||
error!( | ||
target: "client", | ||
"\nBad block detected: {}\nRLP: {}\nHeader: {:?}\nUncles: {}\nTransactions:{}\n", | ||
message, | ||
unverified.bytes.to_hex(), | ||
unverified.header, | ||
unverified.uncles | ||
.iter() | ||
.enumerate() | ||
.map(|(index, uncle)| format!("[Uncle {}] {:?}", index, uncle)) | ||
.join("\n"), | ||
unverified.transactions | ||
.iter() | ||
.enumerate() | ||
.map(|(index, tx)| format!("[Tx {}] {:?}", index, tx)) | ||
.join("\n"), | ||
); | ||
self.last_blocks.write().insert(unverified.header.hash(), (unverified, message)); | ||
}, | ||
Err(err) => { | ||
error!(target: "client", "Bad undecodable block detected: {}\n{:?}", message, err); | ||
}, | ||
} | ||
} | ||
|
||
/// Returns a list of recently detected bad blocks with error descriptions. | ||
pub fn bad_blocks(&self) -> Vec<(Unverified, String)> { | ||
self.last_blocks.read() | ||
.backstore() | ||
.iter() | ||
.map(|(_k, (unverified, message))| ( | ||
Unverified::from_rlp(unverified.bytes.clone()) | ||
.expect("Bytes coming from UnverifiedBlock so decodable; qed"), | ||
message.clone(), | ||
)) | ||
.collect() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,14 +39,15 @@ use client::{ | |
RegistryInfo, ReopenBlock, PrepareOpenBlock, ScheduleInfo, ImportSealedBlock, | ||
BroadcastProposalBlock, ImportBlock, StateOrBlock, StateInfo, StateClient, Call, | ||
AccountData, BlockChain as BlockChainTrait, BlockProducer, SealedBlockImporter, | ||
ClientIoMessage | ||
ClientIoMessage, | ||
}; | ||
use client::{ | ||
BlockId, TransactionId, UncleId, TraceId, ClientConfig, BlockChainClient, | ||
TraceFilter, CallAnalytics, BlockImportError, Mode, | ||
ChainNotify, ChainRoute, PruningInfo, ProvingBlockChainClient, EngineInfo, ChainMessageType, | ||
IoClient, | ||
IoClient, BadBlocks, | ||
}; | ||
use client::bad_blocks; | ||
use encoded; | ||
use engines::{EthEngine, EpochTransition, ForkChoice}; | ||
use error::{ | ||
|
@@ -163,6 +164,9 @@ struct Importer { | |
|
||
/// Ethereum engine to be used during import | ||
pub engine: Arc<EthEngine>, | ||
|
||
/// A lru cache of recently detected bad blocks | ||
pub bad_blocks: bad_blocks::BadBlocks, | ||
} | ||
|
||
/// Blockchain database client backed by a persistent database. Owns and manages a blockchain and a block queue. | ||
|
@@ -254,6 +258,7 @@ impl Importer { | |
miner, | ||
ancient_verifier: AncientVerifier::new(engine.clone()), | ||
engine, | ||
bad_blocks: Default::default(), | ||
}) | ||
} | ||
|
||
|
@@ -291,22 +296,27 @@ impl Importer { | |
continue; | ||
} | ||
|
||
if let Ok(closed_block) = self.check_and_lock_block(block, client) { | ||
if self.engine.is_proposal(&header) { | ||
self.block_queue.mark_as_good(&[hash]); | ||
proposed_blocks.push(bytes); | ||
} else { | ||
imported_blocks.push(hash); | ||
let raw = block.bytes.clone(); | ||
match self.check_and_lock_block(block, client) { | ||
Ok(closed_block) => { | ||
if self.engine.is_proposal(&header) { | ||
self.block_queue.mark_as_good(&[hash]); | ||
proposed_blocks.push(bytes); | ||
} else { | ||
imported_blocks.push(hash); | ||
|
||
let transactions_len = closed_block.transactions().len(); | ||
let transactions_len = closed_block.transactions().len(); | ||
|
||
let route = self.commit_block(closed_block, &header, encoded::Block::new(bytes), client); | ||
import_results.push(route); | ||
let route = self.commit_block(closed_block, &header, encoded::Block::new(bytes), client); | ||
import_results.push(route); | ||
|
||
client.report.write().accrue_block(&header, transactions_len); | ||
} | ||
} else { | ||
invalid_blocks.insert(header.hash()); | ||
client.report.write().accrue_block(&header, transactions_len); | ||
} | ||
}, | ||
Err(err) => { | ||
self.bad_blocks.report(raw, format!("{:?}", err)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make this configurable? Such as only enable bad blocks reporting when the debug namespace RPC is enabled, or just use another cli flag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the whole point is to have it running by default, so that anyone (that have seen such message) can be asked to get bad blocks from RPC and then report them for the analysis. |
||
invalid_blocks.insert(header.hash()); | ||
}, | ||
} | ||
} | ||
|
||
|
@@ -1390,12 +1400,22 @@ impl ImportBlock for Client { | |
if self.chain.read().is_known(&unverified.hash()) { | ||
bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain)); | ||
} | ||
|
||
let status = self.block_status(BlockId::Hash(unverified.parent_hash())); | ||
if status == BlockStatus::Unknown || status == BlockStatus::Pending { | ||
bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(unverified.parent_hash()))); | ||
} | ||
|
||
Ok(self.importer.block_queue.import(unverified)?) | ||
let raw = unverified.bytes.clone(); | ||
match self.importer.block_queue.import(unverified).map_err(Into::into) { | ||
Ok(res) => Ok(res), | ||
// we only care about block errors (not import errors) | ||
Err(BlockImportError(BlockImportErrorKind::Block(err), _))=> { | ||
self.importer.bad_blocks.report(raw, format!("{:?}", err)); | ||
bail!(BlockImportErrorKind::Block(err)) | ||
}, | ||
Err(e) => Err(e), | ||
} | ||
} | ||
} | ||
|
||
|
@@ -1532,6 +1552,12 @@ impl EngineInfo for Client { | |
} | ||
} | ||
|
||
impl BadBlocks for Client { | ||
fn bad_blocks(&self) -> Vec<(Unverified, String)> { | ||
self.importer.bad_blocks.bad_blocks() | ||
} | ||
} | ||
|
||
impl BlockChainClient for Client { | ||
fn replay(&self, id: TransactionId, analytics: CallAnalytics) -> Result<Executed, CallError> { | ||
let address = self.transaction_address(id).ok_or(CallError::TransactionNotFound)?; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
// Copyright 2015-2018 Parity Technologies (UK) Ltd. | ||
// This file is part of Parity. | ||
|
||
// Parity is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
|
||
// Parity is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
|
||
// You should have received a copy of the GNU General Public License | ||
// along with Parity. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
//! Debug APIs RPC implementation | ||
|
||
use std::sync::Arc; | ||
|
||
use ethcore::client::BlockChainClient; | ||
use transaction::LocalizedTransaction; | ||
|
||
use jsonrpc_core::Result; | ||
use v1::traits::Debug; | ||
use v1::types::{Block, Bytes, RichBlock, BlockTransactions, Transaction}; | ||
|
||
/// Debug rpc implementation. | ||
pub struct DebugClient<C> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this mod needs to be enabled in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, that's 100% true. Haven't tested it properly, will add to CLI and actually check if it's queryable through RPC. |
||
client: Arc<C>, | ||
} | ||
|
||
impl<C> DebugClient<C> { | ||
/// Creates new debug client. | ||
pub fn new(client: Arc<C>) -> Self { | ||
Self { | ||
client, | ||
} | ||
} | ||
} | ||
|
||
impl<C: BlockChainClient + 'static> Debug for DebugClient<C> { | ||
fn bad_blocks(&self) -> Result<Vec<RichBlock>> { | ||
fn cast<O, T: Copy + Into<O>>(t: &T) -> O { | ||
(*t).into() | ||
} | ||
|
||
Ok(self.client.bad_blocks().into_iter().map(|(block, reason)| { | ||
let number = block.header.number(); | ||
let hash = block.header.hash(); | ||
RichBlock { | ||
inner: Block { | ||
hash: Some(hash.into()), | ||
size: Some(block.bytes.len().into()), | ||
parent_hash: cast(block.header.parent_hash()), | ||
uncles_hash: cast(block.header.uncles_hash()), | ||
author: cast(block.header.author()), | ||
miner: cast(block.header.author()), | ||
state_root: cast(block.header.state_root()), | ||
receipts_root: cast(block.header.receipts_root()), | ||
number: Some(number.into()), | ||
gas_used: cast(block.header.gas_used()), | ||
gas_limit: cast(block.header.gas_limit()), | ||
logs_bloom: Some(cast(block.header.log_bloom())), | ||
timestamp: block.header.timestamp().into(), | ||
difficulty: cast(block.header.difficulty()), | ||
total_difficulty: None, | ||
seal_fields: block.header.seal().into_iter().cloned().map(Into::into).collect(), | ||
uncles: block.uncles.into_iter().map(|u| u.hash().into()).collect(), | ||
transactions: BlockTransactions::Full(block.transactions | ||
.into_iter() | ||
.enumerate() | ||
.map(|(transaction_index, signed)| Transaction::from_localized(LocalizedTransaction { | ||
block_number: number.into(), | ||
block_hash: hash.into(), | ||
transaction_index, | ||
signed, | ||
cached_sender: None, | ||
})).collect() | ||
), | ||
transactions_root: cast(block.header.transactions_root()), | ||
extra_data: block.header.extra_data().clone().into(), | ||
}, | ||
extra_info: vec![ | ||
("reason".to_owned(), reason), | ||
("rlp".to_owned(), serialize(&Bytes(block.bytes))), | ||
("hash".to_owned(), format!("{:#x}", hash)), | ||
].into_iter().collect(), | ||
} | ||
}).collect()) | ||
} | ||
} | ||
|
||
fn serialize<T: ::serde::Serialize>(t: &T) -> String { | ||
::serde_json::to_string(t).expect("RPC types serialization is non-fallible.") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about to add the raw_block data (Vec) into the tuple also, to be able to save "undecodable" BadBlocks? Is that makes any sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I though about it actually. It doesn't allow us to return detailed block in the RPC response though. I think currently it makes sense to have something with the same results as geth to start detecting issues asap.
That said, feel free to log it as a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also thought about this and checked the geth implementation and it seems they also don't store block rlp's that aren't decodable.