decode block rlp less often

ethcore/src/block.rs

@@ -40,7 +40,7 @@ use engines::EthEngine;
 use error::{Error, BlockError};
 use ethereum_types::{H256, U256, Address, Bloom};
 use factory::Factories;
-use hash::{keccak, KECCAK_NULL_RLP, KECCAK_EMPTY_LIST_RLP};
+use hash::keccak;
 use header::{Header, ExtendedHeader};
 use receipt::{Receipt, TransactionOutcome};
 use rlp::{Rlp, RlpStream, Encodable, Decodable, DecoderError, encode_list};
@@ -51,7 +51,6 @@ use transaction::{UnverifiedTransaction, SignedTransaction, Error as Transaction
 use triehash::ordered_trie_root;
 use unexpected::{Mismatch, OutOfBounds};
 use verification::PreverifiedBlock;
-use views::BlockView;
 use vm::{EnvInfo, LastHashes};
 
 /// A block, encoded as it is on the block chain.
@@ -66,11 +65,6 @@ pub struct Block {
 }
 
 impl Block {
-	/// Returns true if the given bytes form a valid encoding of a block in RLP.
-	pub fn is_good(b: &[u8]) -> bool {
-		Rlp::new(b).as_val::<Block>().is_ok()
-	}
-
 	/// Get the RLP-encoding of the block with the seal.
 	pub fn rlp_bytes(&self) -> Bytes {
 		let mut block_rlp = RlpStream::new_list(3);
@@ -398,26 +392,11 @@ impl<'x> OpenBlock<'x> {
 
 	/// Turn this into a `ClosedBlock`.
 	pub fn close(self) -> Result<ClosedBlock, Error> {
-		let mut s = self;
-
-		let unclosed_state = s.block.state.clone();
-
-		s.engine.on_close_block(&mut s.block)?;
-		s.block.state.commit()?;
-
-		s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes())));
-		let uncle_bytes = encode_list(&s.block.uncles);
-		s.block.header.set_uncles_hash(keccak(&uncle_bytes));
-		s.block.header.set_state_root(s.block.state.root().clone());
-		s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes())));
-		s.block.header.set_log_bloom(s.block.receipts.iter().fold(Bloom::zero(), |mut b, r| {
-			b.accrue_bloom(&r.log_bloom);
-			b
-		}));
-		s.block.header.set_gas_used(s.block.receipts.last().map_or_else(U256::zero, |r| r.gas_used));
+		let unclosed_state = self.block.state.clone();
+		let locked = self.close_and_lock()?;
 
 		Ok(ClosedBlock {
-			block: s.block,
+			block: locked.block,
 			unclosed_state,
 		})
 	}
@@ -429,18 +408,11 @@ impl<'x> OpenBlock<'x> {
 		s.engine.on_close_block(&mut s.block)?;
 		s.block.state.commit()?;
 
-		if s.block.header.transactions_root().is_zero() || s.block.header.transactions_root() == &KECCAK_NULL_RLP {
-			s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes())));
-		}
-		if s.block.header.uncles_hash().is_zero() || s.block.header.uncles_hash() == &KECCAK_EMPTY_LIST_RLP {
-			let uncle_bytes = encode_list(&s.block.uncles);
-			s.block.header.set_uncles_hash(keccak(&uncle_bytes));
-		}
-		if s.block.header.receipts_root().is_zero() || s.block.header.receipts_root() == &KECCAK_NULL_RLP {
-			s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes())));
-		}
-
+		s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes())));
+		let uncle_bytes = encode_list(&s.block.uncles);
+		s.block.header.set_uncles_hash(keccak(&uncle_bytes));
 		s.block.header.set_state_root(s.block.state.root().clone());
+		s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes())));
 		s.block.header.set_log_bloom(s.block.receipts.iter().fold(Bloom::zero(), |mut b, r| {
 			b.accrue_bloom(&r.log_bloom);
 			b
@@ -537,18 +509,16 @@ impl LockedBlock {
 		self,
 		engine: &EthEngine,
 		seal: Vec<Bytes>,
-	) -> Result<SealedBlock, (Error, LockedBlock)> {
+	) -> Result<SealedBlock, Error> {
 		let mut s = self;
 		s.block.header.set_seal(seal);
 		s.block.header.compute_hash();
 
 		// TODO: passing state context to avoid engines owning it?
-		match engine.verify_local_seal(&s.block.header) {
-			Err(e) => Err((e, s)),
-			_ => Ok(SealedBlock {
-				block: s.block
-			}),
-		}
+		engine.verify_local_seal(&s.block.header)?;
+		Ok(SealedBlock {
+			block: s.block
+		})
 	}
 }
 
@@ -637,12 +607,11 @@ pub fn enact_verified(
 	is_epoch_begin: bool,
 	ancestry: &mut Iterator<Item=ExtendedHeader>,
 ) -> Result<LockedBlock, Error> {
-	let view = view!(BlockView, &block.bytes);
 
 	enact(
 		block.header,
 		block.transactions,
-		view.uncles(),
+		block.uncles,
 		engine,
 		tracing,
 		db,

ethcore/src/client/client.rs

@@ -74,6 +74,8 @@ use types::filter::Filter;
 use types::ancestry_action::AncestryAction;
 use verification;
 use verification::{PreverifiedBlock, Verifier, BlockQueue};
+use verification::queue::kind::blocks::Unverified;
+use verification::queue::kind::BlockLike;
 
 // re-export
 pub use types::blockchain_info::BlockChainInfo;
@@ -209,7 +211,7 @@ pub struct Client {
 	/// Queued ancient blocks, make sure they are imported in order.
 	queued_ancient_blocks: Arc<RwLock<(
 		HashSet<H256>,
-		VecDeque<(Header, encoded::Block, Bytes)>
+		VecDeque<(Unverified, Bytes)>
 	)>>,
 	ancient_blocks_import_lock: Arc<Mutex<()>>,
 	/// Consensus messages import queue
@@ -429,19 +431,19 @@ 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, header: &Header, block: encoded::Block, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> Result<(), ::error::Error> {
+	fn import_old_block(&self, unverified: Unverified, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> Result<(), ::error::Error> {
 		let receipts = ::rlp::decode_list(receipts_bytes);
 		let _import_lock = self.import_lock.lock();
 
 		{
 			trace_time!("import_old_block");
 			// verify the block, passing the chain for updating the epoch verifier.
 			let mut rng = OsRng::new()?;
-			self.ancient_verifier.verify(&mut rng, &header, &chain)?;
+			self.ancient_verifier.verify(&mut rng, &unverified.header, &chain)?;
 
 			// Commit results
 			let mut batch = DBTransaction::new();
-			chain.insert_unordered_block(&mut batch, block, receipts, None, false, true);
+			chain.insert_unordered_block(&mut batch, encoded::Block::new(unverified.bytes), receipts, None, false, true);
 			// Final commit to the DB
 			db.write_buffered(batch);
 			chain.commit();
@@ -1382,22 +1384,15 @@ impl CallContract for Client {
 }
 
 impl ImportBlock for Client {
-	fn import_block(&self, bytes: Bytes) -> Result<H256, BlockImportError> {
-		use verification::queue::kind::BlockLike;
-		use verification::queue::kind::blocks::Unverified;
-
-		// create unverified block here so the `keccak` calculation can be cached.
-		let unverified = Unverified::from_rlp(bytes)?;
-
-		{
-			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())));
-			}
+	fn import_block(&self, unverified: Unverified) -> Result<H256, BlockImportError> {
+		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)?)
 	}
 }
@@ -2022,24 +2017,23 @@ impl IoClient for Client {
 		});
 	}
 
-	fn queue_ancient_block(&self, block_bytes: Bytes, receipts_bytes: Bytes) -> Result<H256, BlockImportError> {
+	fn queue_ancient_block(&self, unverified: Unverified, receipts_bytes: Bytes) -> Result<H256, BlockImportError> {
 		trace_time!("queue_ancient_block");
-		let header: Header = ::rlp::Rlp::new(&block_bytes).val_at(0)?;
-		let hash = header.hash();
 
+		let hash = unverified.hash();
 		{
 			// check block order
 			if self.chain.read().is_known(&hash) {
 				bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain));
 			}
-			let parent_hash = header.parent_hash();
+			let parent_hash = unverified.parent_hash();
 			// NOTE To prevent race condition with import, make sure to check queued blocks first
 			// (and attempt to acquire lock)
-			let is_parent_pending = self.queued_ancient_blocks.read().0.contains(parent_hash);
+			let is_parent_pending = self.queued_ancient_blocks.read().0.contains(&parent_hash);
 			if !is_parent_pending {
-				let status = self.block_status(BlockId::Hash(*parent_hash));
+				let status = self.block_status(BlockId::Hash(parent_hash));
 				if  status == BlockStatus::Unknown || status == BlockStatus::Pending {
-					bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(*parent_hash)));
+					bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(parent_hash)));
 				}
 			}
 		}
@@ -2048,7 +2042,7 @@ impl IoClient for Client {
 		{
 			let mut queued = self.queued_ancient_blocks.write();
 			queued.0.insert(hash);
-			queued.1.push_back((header, encoded::Block::new(block_bytes), receipts_bytes));
+			queued.1.push_back((unverified, receipts_bytes));
 		}
 
 		let queued = self.queued_ancient_blocks.clone();
@@ -2060,11 +2054,10 @@ impl IoClient for Client {
 			let _lock = lock.lock();
 			for _i in 0..MAX_ANCIENT_BLOCKS_TO_IMPORT {
 				let first = queued.write().1.pop_front();
-				if let Some((header, block_bytes, receipts_bytes)) = first {
-					let hash = header.hash();
+				if let Some((unverified, receipts_bytes)) = first {
+					let hash = unverified.hash();
 					let result = client.importer.import_old_block(
-						&header,
-						block_bytes,
+						unverified,
 						&receipts_bytes,
 						&**client.db.read().key_value(),
 						&*client.chain.read(),

ethcore/src/client/test_client.rs

@@ -53,6 +53,7 @@ use spec::Spec;
 use types::basic_account::BasicAccount;
 use types::pruning_info::PruningInfo;
 use verification::queue::QueueInfo;
+use verification::queue::kind::blocks::Unverified;
 use block::{OpenBlock, SealedBlock, ClosedBlock};
 use executive::Executed;
 use error::CallError;
@@ -280,7 +281,8 @@ impl TestBlockChainClient {
 			rlp.append(&header);
 			rlp.append_raw(&txs, 1);
 			rlp.append_raw(uncles.as_raw(), 1);
-			self.import_block(rlp.as_raw().to_vec()).unwrap();
+			let unverified = Unverified::from_rlp(rlp.out()).unwrap();
+			self.import_block(unverified).unwrap();
 		}
 	}
 
@@ -512,8 +514,8 @@ impl RegistryInfo for TestBlockChainClient {
 }
 
 impl ImportBlock for TestBlockChainClient {
-	fn import_block(&self, b: Bytes) -> Result<H256, BlockImportError> {
-		let header = view!(BlockView, &b).header();
+	fn import_block(&self, unverified: Unverified) -> Result<H256, BlockImportError> {
+		let header = unverified.header;
 		let h = header.hash();
 		let number: usize = header.number() as usize;
 		if number > self.blocks.read().len() {
@@ -539,7 +541,7 @@ impl ImportBlock for TestBlockChainClient {
 				*difficulty = *difficulty + header.difficulty().clone();
 			}
 			mem::replace(&mut *self.last_hash.write(), h.clone());
-			self.blocks.write().insert(h.clone(), b);
+			self.blocks.write().insert(h.clone(), unverified.bytes);
 			self.numbers.write().insert(number, h.clone());
 			let mut parent_hash = header.parent_hash().clone();
 			if number > 0 {
@@ -552,7 +554,7 @@ impl ImportBlock for TestBlockChainClient {
 			}
 		}
 		else {
-			self.blocks.write().insert(h.clone(), b.to_vec());
+			self.blocks.write().insert(h.clone(), unverified.bytes);
 		}
 		Ok(h)
 	}
@@ -856,8 +858,8 @@ impl IoClient for TestBlockChainClient {
 		self.miner.import_external_transactions(self, txs);
 	}
 
-	fn queue_ancient_block(&self, b: Bytes, _r: Bytes) -> Result<H256, BlockImportError> {
-		self.import_block(b)
+	fn queue_ancient_block(&self, unverified: Unverified, _r: Bytes) -> Result<H256, BlockImportError> {
+		self.import_block(unverified)
 	}
 
 	fn queue_consensus_message(&self, message: Bytes) {

ethcore/src/client/traits.rs

@@ -34,6 +34,7 @@ use receipt::LocalizedReceipt;
 use trace::LocalizedTrace;
 use transaction::{self, LocalizedTransaction, SignedTransaction};
 use verification::queue::QueueInfo as BlockQueueInfo;
+use verification::queue::kind::blocks::Unverified;
 use state::StateInfo;
 use header::Header;
 use engines::EthEngine;
@@ -167,7 +168,7 @@ pub trait RegistryInfo {
 /// Provides methods to import block into blockchain
 pub trait ImportBlock {
 	/// Import a block into the blockchain.
-	fn import_block(&self, bytes: Bytes) -> Result<H256, BlockImportError>;
+	fn import_block(&self, block: Unverified) -> Result<H256, BlockImportError>;
 }
 
 /// Provides `call_contract` method
@@ -204,7 +205,7 @@ pub trait IoClient: Sync + Send {
 	fn queue_transactions(&self, transactions: Vec<Bytes>, peer_id: usize);
 
 	/// Queue block import with transaction receipts. Does no sealing and transaction validation.
-	fn queue_ancient_block(&self, block_bytes: Bytes, receipts_bytes: Bytes) -> Result<H256, BlockImportError>;
+	fn queue_ancient_block(&self, block_bytes: Unverified, receipts_bytes: Bytes) -> Result<H256, BlockImportError>;
 
 	/// Queue conensus engine message.
 	fn queue_consensus_message(&self, message: Bytes);

ethcore/src/engines/validator_set/multi.rs

@@ -158,6 +158,7 @@ mod tests {
 	use test_helpers::{generate_dummy_client_with_spec_and_accounts, generate_dummy_client_with_spec_and_data};
 	use types::ids::BlockId;
 	use ethereum_types::Address;
+	use verification::queue::kind::blocks::Unverified;
 
 	use super::Multi;
 
@@ -198,7 +199,7 @@ mod tests {
 		let sync_client = generate_dummy_client_with_spec_and_data(Spec::new_validator_multi, 0, 0, &[]);
 		sync_client.engine().register_client(Arc::downgrade(&sync_client) as _);
 		for i in 1..4 {
-			sync_client.import_block(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap();
+			sync_client.import_block(Unverified::from_rlp(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap()).unwrap();
 		}
 		sync_client.flush_queue();
 		assert_eq!(sync_client.chain_info().best_block_number, 3);

ethcore/src/engines/validator_set/safe_contract.rs

@@ -458,6 +458,7 @@ mod tests {
 	use test_helpers::{generate_dummy_client_with_spec_and_accounts, generate_dummy_client_with_spec_and_data};
 	use super::super::ValidatorSet;
 	use super::{ValidatorSafeContract, EVENT_NAME_HASH};
+	use verification::queue::kind::blocks::Unverified;
 
 	#[test]
 	fn fetches_validators() {
@@ -530,7 +531,7 @@ mod tests {
 		let sync_client = generate_dummy_client_with_spec_and_data(Spec::new_validator_safe_contract, 0, 0, &[]);
 		sync_client.engine().register_client(Arc::downgrade(&sync_client) as _);
 		for i in 1..4 {
-			sync_client.import_block(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap();
+			sync_client.import_block(Unverified::from_rlp(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap()).unwrap();
 		}
 		sync_client.flush_queue();
 		assert_eq!(sync_client.chain_info().best_block_number, 3);

ethcore/src/json_tests/chain.rs

@@ -17,12 +17,12 @@
 use std::path::Path;
 use std::sync::Arc;
 use client::{EvmTestClient, Client, ClientConfig, ChainInfo, ImportBlock};
-use block::Block;
 use spec::Genesis;
 use ethjson;
 use miner::Miner;
 use io::IoChannel;
 use test_helpers;
+use verification::queue::kind::blocks::Unverified;
 
 use super::HookType;
 
@@ -83,9 +83,9 @@ pub fn json_chain_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_ho
 					Arc::new(Miner::new_for_tests(&spec, None)),
 					IoChannel::disconnected(),
 				).unwrap();
-				for b in &blockchain.blocks_rlp() {
-					if Block::is_good(&b) {
-						let _ = client.import_block(b.clone());
+				for b in blockchain.blocks_rlp() {
+					if let Ok(block) = Unverified::from_rlp(b) {
+						let _ = client.import_block(block);
 						client.flush_queue();
 						client.import_verified_blocks();
 					}