Bad blocks RPC + reporting (#9433)

* Bad blocks RPC.

* Return bad blocks via RPC.

* Fix test.

* More verbose bad block message.

* Expose via CLI.

* Remove stray whitespace.

* Remove stray newline.

* Fix tests.

ethcore/src/client/bad_blocks.rs

@@ -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) {
+		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()
+	}
+}

ethcore/src/client/client.rs

@@ -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));
+						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 {
 			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)?;

ethcore/src/client/mod.rs

@@ -17,6 +17,7 @@
 //! Blockchain database client.
 
 mod ancient_import;
+mod bad_blocks;
 mod client;
 mod config;
 #[cfg(any(test, feature = "test-helpers"))]
@@ -36,7 +37,7 @@ pub use self::test_client::{TestBlockChainClient, EachBlockWith};
 pub use self::chain_notify::{ChainNotify, ChainRoute, ChainRouteType, ChainMessageType};
 pub use self::traits::{
     Nonce, Balance, ChainInfo, BlockInfo, ReopenBlock, PrepareOpenBlock, CallContract, TransactionInfo, RegistryInfo, ScheduleInfo, ImportSealedBlock, BroadcastProposalBlock, ImportBlock,
-    StateOrBlock, StateClient, Call, EngineInfo, AccountData, BlockChain, BlockProducer, SealedBlockImporter
+    StateOrBlock, StateClient, Call, EngineInfo, AccountData, BlockChain, BlockProducer, SealedBlockImporter, BadBlocks,
 };
 pub use state::StateInfo;
 pub use self::traits::{BlockChainClient, EngineClient, ProvingBlockChainClient, IoClient};

ethcore/src/client/test_client.rs

@@ -39,7 +39,8 @@ use client::{
 	PrepareOpenBlock, BlockChainClient, BlockChainInfo, BlockStatus, BlockId, Mode,
 	TransactionId, UncleId, TraceId, TraceFilter, LastHashes, CallAnalytics, BlockImportError,
 	ProvingBlockChainClient, ScheduleInfo, ImportSealedBlock, BroadcastProposalBlock, ImportBlock, StateOrBlock,
-	Call, StateClient, EngineInfo, AccountData, BlockChain, BlockProducer, SealedBlockImporter, IoClient
+	Call, StateClient, EngineInfo, AccountData, BlockChain, BlockProducer, SealedBlockImporter, IoClient,
+	BadBlocks,
 };
 use db::{NUM_COLUMNS, COL_STATE};
 use header::{Header as BlockHeader, BlockNumber};
@@ -615,6 +616,19 @@ impl EngineInfo for TestBlockChainClient {
 	}
 }
 
+impl BadBlocks for TestBlockChainClient {
+	fn bad_blocks(&self) -> Vec<(Unverified, String)> {
+		vec![
+			(Unverified {
+				header: Default::default(),
+				transactions: vec![],
+				uncles: vec![],
+				bytes: vec![1, 2, 3],
+			}, "Invalid block".into())
+		]
+	}
+}
+
 impl BlockChainClient for TestBlockChainClient {
 	fn replay(&self, _id: TransactionId, _analytics: CallAnalytics) -> Result<Executed, CallError> {
 		self.execution_result.read().clone().unwrap()

ethcore/src/client/traits.rs

@@ -211,9 +211,15 @@ pub trait IoClient: Sync + Send {
 	fn queue_consensus_message(&self, message: Bytes);
 }
 
+/// Provides recently seen bad blocks.
+pub trait BadBlocks {
+	/// Returns a list of blocks that were recently not imported because they were invalid.
+	fn bad_blocks(&self) -> Vec<(Unverified, String)>;
+}
+
 /// Blockchain database client. Owns and manages a blockchain and a block queue.
 pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContract + RegistryInfo + ImportBlock
-+ IoClient {
++ IoClient + BadBlocks {
 	/// Look up the block number for the given block ID.
 	fn block_number(&self, id: BlockId) -> Option<BlockNumber>;
 

parity/cli/mod.rs

@@ -477,7 +477,7 @@ usage! {
 
 			ARG arg_jsonrpc_apis: (String) = "web3,eth,pubsub,net,parity,private,parity_pubsub,traces,rpc,shh,shh_pubsub", or |c: &Config| c.rpc.as_ref()?.apis.as_ref().map(|vec| vec.join(",")),
 			"--jsonrpc-apis=[APIS]",
-			"Specify the APIs available through the HTTP JSON-RPC interface using a comma-delimited list of API names. Possible names are: all, safe, web3, net, eth, pubsub, personal, signer, parity, parity_pubsub, parity_accounts, parity_set, traces, rpc, secretstore, shh, shh_pubsub. You can also disable a specific API by putting '-' in the front, example: all,-personal. 'safe' enables the following APIs: web3, net, eth, pubsub, parity, parity_pubsub, traces, rpc, shh, shh_pubsub",
+			"Specify the APIs available through the HTTP JSON-RPC interface using a comma-delimited list of API names. Possible names are: all, safe, debug, web3, net, eth, pubsub, personal, signer, parity, parity_pubsub, parity_accounts, parity_set, traces, rpc, secretstore, shh, shh_pubsub. You can also disable a specific API by putting '-' in the front, example: all,-personal. 'safe' enables the following APIs: web3, net, eth, pubsub, parity, parity_pubsub, traces, rpc, shh, shh_pubsub",
 
 			ARG arg_jsonrpc_hosts: (String) = "none", or |c: &Config| c.rpc.as_ref()?.hosts.as_ref().map(|vec| vec.join(",")),
 			"--jsonrpc-hosts=[HOSTS]",