deserialize block only once during verification (#9161)

Author: debris

ethcore/src/client/client.rs

@@ -73,15 +73,14 @@ use transaction::{self, LocalizedTransaction, UnverifiedTransaction, SignedTrans
 use types::filter::Filter;
 use types::ancestry_action::AncestryAction;
 use verification;
-use verification::{PreverifiedBlock, Verifier};
-use verification::queue::BlockQueue;
+use verification::{PreverifiedBlock, Verifier, BlockQueue};
 use views::BlockView;
 
 // re-export
 pub use types::blockchain_info::BlockChainInfo;
 pub use types::block_status::BlockStatus;
 pub use blockchain::CacheSize as BlockChainCacheSize;
-pub use verification::queue::QueueInfo as BlockQueueInfo;
+pub use verification::QueueInfo as BlockQueueInfo;
 
 use_contract!(registry, "Registry", "res/contracts/registrar.json");
 
@@ -371,8 +370,7 @@ impl Importer {
 			&parent,
 			engine,
 			Some(verification::FullFamilyParams {
-				block_bytes: &block.bytes,
-				transactions: &block.transactions,
+				block: &block,
 				block_provider: &**chain,
 				client
 			}),

ethcore/src/verification/mod.rs

@@ -16,8 +16,8 @@
 
 //! Block verification utilities.
 
-pub mod verification;
-pub mod verifier;
+mod verification;
+mod verifier;
 pub mod queue;
 mod canon_verifier;
 mod noop_verifier;

ethcore/src/verification/queue/kind.rs

@@ -72,6 +72,7 @@ pub mod blocks {
 	use error::{Error, ErrorKind, BlockError};
 	use header::Header;
 	use verification::{PreverifiedBlock, verify_block_basic, verify_block_unordered};
+	use transaction::UnverifiedTransaction;
 
 	use heapsize::HeapSizeOf;
 	use ethereum_types::{H256, U256};
@@ -86,7 +87,7 @@ pub mod blocks {
 		type Verified = PreverifiedBlock;
 
 		fn create(input: Self::Input, engine: &EthEngine) -> Result<Self::Unverified, Error> {
-			match verify_block_basic(&input.header, &input.bytes, engine) {
+			match verify_block_basic(&input, engine) {
 				Ok(()) => Ok(input),
 				Err(Error(ErrorKind::Block(BlockError::TemporarilyInvalid(oob)), _)) => {
 					debug!(target: "client", "Block received too early {}: {:?}", input.hash(), oob);
@@ -101,7 +102,7 @@ pub mod blocks {
 
 		fn verify(un: Self::Unverified, engine: &EthEngine, check_seal: bool) -> Result<Self::Verified, Error> {
 			let hash = un.hash();
-			match verify_block_unordered(un.header, un.bytes, engine, check_seal) {
+			match verify_block_unordered(un, engine, check_seal) {
 				Ok(verified) => Ok(verified),
 				Err(e) => {
 					warn!(target: "client", "Stage 2 block verification failed for {}: {:?}", hash, e);
@@ -113,25 +114,43 @@ pub mod blocks {
 
 	/// An unverified block.
 	pub struct Unverified {
-		header: Header,
-		bytes: Bytes,
+		/// Unverified block header.
+		pub header: Header,
+		/// Unverified block transactions.
+		pub transactions: Vec<UnverifiedTransaction>,
+		/// Unverified block uncles.
+		pub uncles: Vec<Header>,
+		/// Raw block bytes.
+		pub bytes: Bytes,
 	}
 
 	impl Unverified {
 		/// Create an `Unverified` from raw bytes.
 		pub fn from_rlp(bytes: Bytes) -> Result<Self, ::rlp::DecoderError> {
+			use rlp::Rlp;
+			let (header, transactions, uncles) = {
+				let rlp = Rlp::new(&bytes);
+				let header = rlp.val_at(0)?;
+				let transactions = rlp.list_at(1)?;
+				let uncles = rlp.list_at(2)?;
+				(header, transactions, uncles)
+			};
 
-			let header = ::rlp::Rlp::new(&bytes).val_at(0)?; 
 			Ok(Unverified {
-				header: header,
-				bytes: bytes,
+				header,
+				transactions,
+				uncles,
+				bytes,
 			})
 		}
 	}
 
 	impl HeapSizeOf for Unverified {
 		fn heap_size_of_children(&self) -> usize {
-			self.header.heap_size_of_children() + self.bytes.heap_size_of_children()
+			self.header.heap_size_of_children()
+				+ self.transactions.heap_size_of_children()
+				+ self.uncles.heap_size_of_children()
+				+ self.bytes.heap_size_of_children()
 		}
 	}
 

ethcore/src/verification/verification.rs

@@ -25,7 +25,6 @@ use std::collections::HashSet;
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
 use bytes::Bytes;
-use ethereum_types::H256;
 use hash::keccak;
 use heapsize::HeapSizeOf;
 use rlp::Rlp;
@@ -37,15 +36,17 @@ use client::{BlockInfo, CallContract};
 use engines::EthEngine;
 use error::{BlockError, Error};
 use header::{BlockNumber, Header};
-use transaction::{SignedTransaction, UnverifiedTransaction};
-use views::BlockView;
+use transaction::SignedTransaction;
+use verification::queue::kind::blocks::Unverified;
 
 /// Preprocessed block data gathered in `verify_block_unordered` call
 pub struct PreverifiedBlock {
 	/// Populated block header
 	pub header: Header,
 	/// Populated block transactions
 	pub transactions: Vec<SignedTransaction>,
+	/// Populated block uncles
+	pub uncles: Vec<Header>,
 	/// Block bytes
 	pub bytes: Bytes,
 }
@@ -59,63 +60,66 @@ impl HeapSizeOf for PreverifiedBlock {
 }
 
 /// Phase 1 quick block verification. Only does checks that are cheap. Operates on a single block
-pub fn verify_block_basic(header: &Header, bytes: &[u8], engine: &EthEngine) -> Result<(), Error> {
-	verify_header_params(&header, engine, true)?;
-	verify_block_integrity(bytes, &header.transactions_root(), &header.uncles_hash())?;
-	engine.verify_block_basic(&header)?;
-	for u in Rlp::new(bytes).at(2)?.iter().map(|rlp| rlp.as_val::<Header>()) {
-		let u = u?;
-		verify_header_params(&u, engine, false)?;
-		engine.verify_block_basic(&u)?;
+pub fn verify_block_basic(block: &Unverified, engine: &EthEngine) -> Result<(), Error> {
+	verify_header_params(&block.header, engine, true)?;
+	verify_block_integrity(block)?;
+	engine.verify_block_basic(&block.header)?;
+
+	for uncle in &block.uncles {
+		verify_header_params(uncle, engine, false)?;
+		engine.verify_block_basic(uncle)?;
 	}
 
-	for t in Rlp::new(bytes).at(1)?.iter().map(|rlp| rlp.as_val::<UnverifiedTransaction>()) {
-		engine.verify_transaction_basic(&t?, &header)?;
+	for t in &block.transactions {
+		engine.verify_transaction_basic(t, &block.header)?;
 	}
+
 	Ok(())
 }
 
 /// Phase 2 verification. Perform costly checks such as transaction signatures and block nonce for ethash.
 /// Still operates on a individual block
 /// Returns a `PreverifiedBlock` structure populated with transactions
-pub fn verify_block_unordered(header: Header, bytes: Bytes, engine: &EthEngine, check_seal: bool) -> Result<PreverifiedBlock, Error> {
+pub fn verify_block_unordered(block: Unverified, engine: &EthEngine, check_seal: bool) -> Result<PreverifiedBlock, Error> {
+	let header = block.header;
 	if check_seal {
 		engine.verify_block_unordered(&header)?;
-		for u in Rlp::new(&bytes).at(2)?.iter().map(|rlp| rlp.as_val::<Header>()) {
-			engine.verify_block_unordered(&u?)?;
+		for uncle in &block.uncles {
+			engine.verify_block_unordered(uncle)?;
 		}
 	}
 	// Verify transactions.
-	let mut transactions = Vec::new();
 	let nonce_cap = if header.number() >= engine.params().dust_protection_transition {
 		Some((engine.params().nonce_cap_increment * header.number()).into())
-	} else { None };
-	{
-		let v = view!(BlockView, &bytes);
-		for t in v.transactions() {
+	} else {
+		None
+	};
+
+	let transactions = block.transactions
+		.into_iter()
+		.map(|t| {
 			let t = engine.verify_transaction_unordered(t, &header)?;
 			if let Some(max_nonce) = nonce_cap {
 				if t.nonce >= max_nonce {
 					return Err(BlockError::TooManyTransactions(t.sender()).into());
 				}
 			}
-			transactions.push(t);
-		}
-	}
+			Ok(t)
+		})
+		.collect::<Result<Vec<_>, Error>>()?;
+
 	Ok(PreverifiedBlock {
-		header: header,
-		transactions: transactions,
-		bytes: bytes,
+		header,
+		transactions,
+		uncles: block.uncles,
+		bytes: block.bytes,
 	})
 }
 
 /// Parameters for full verification of block family
 pub struct FullFamilyParams<'a, C: BlockInfo + CallContract + 'a> {
-	/// Serialized block bytes
-	pub block_bytes: &'a [u8],
-
-	/// Signed transactions
-	pub transactions: &'a [SignedTransaction],
+	/// Preverified block
+	pub block: &'a PreverifiedBlock,
 
 	/// Block provider to use during verification
 	pub block_provider: &'a BlockProvider,
@@ -135,17 +139,18 @@ pub fn verify_block_family<C: BlockInfo + CallContract>(header: &Header, parent:
 		None => return Ok(()),
 	};
 
-	verify_uncles(header, params.block_bytes, params.block_provider, engine)?;
+	verify_uncles(params.block, params.block_provider, engine)?;
 
-	for transaction in params.transactions {
-		engine.machine().verify_transaction(transaction, header, params.client)?;
+	for tx in &params.block.transactions {
+		engine.machine().verify_transaction(tx, header, params.client)?;
 	}
 
 	Ok(())
 }
 
-fn verify_uncles(header: &Header, bytes: &[u8], bc: &BlockProvider, engine: &EthEngine) -> Result<(), Error> {
-	let num_uncles = Rlp::new(bytes).at(2)?.item_count()?;
+fn verify_uncles(block: &PreverifiedBlock, bc: &BlockProvider, engine: &EthEngine) -> Result<(), Error> {
+	let header = &block.header;
+	let num_uncles = block.uncles.len();
 	let max_uncles = engine.maximum_uncle_count(header.number());
 	if num_uncles != 0 {
 		if num_uncles > max_uncles {
@@ -174,8 +179,7 @@ fn verify_uncles(header: &Header, bytes: &[u8], bc: &BlockProvider, engine: &Eth
 		}
 
 		let mut verified = HashSet::new();
-		for uncle in Rlp::new(bytes).at(2)?.iter().map(|rlp| rlp.as_val::<Header>()) {
-			let uncle = uncle?;
+		for uncle in &block.uncles {
 			if excluded.contains(&uncle.hash()) {
 				return Err(From::from(BlockError::UncleInChain(uncle.hash())))
 			}
@@ -332,16 +336,22 @@ fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result
 }
 
 /// Verify block data against header: transactions root and uncles hash.
-fn verify_block_integrity(block: &[u8], transactions_root: &H256, uncles_hash: &H256) -> Result<(), Error> {
-	let block = Rlp::new(block);
-	let tx = block.at(1)?;
-	let expected_root = &ordered_trie_root(tx.iter().map(|r| r.as_raw()));
-	if expected_root != transactions_root {
-		return Err(From::from(BlockError::InvalidTransactionsRoot(Mismatch { expected: expected_root.clone(), found: transactions_root.clone() })))
-	}
-	let expected_uncles = &keccak(block.at(2)?.as_raw());
-	if expected_uncles != uncles_hash {
-		return Err(From::from(BlockError::InvalidUnclesHash(Mismatch { expected: expected_uncles.clone(), found: uncles_hash.clone() })))
+fn verify_block_integrity(block: &Unverified) -> Result<(), Error> {
+	let block_rlp = Rlp::new(&block.bytes);
+	let tx = block_rlp.at(1)?;
+	let expected_root = ordered_trie_root(tx.iter().map(|r| r.as_raw()));
+	if &expected_root != block.header.transactions_root() {
+		bail!(BlockError::InvalidTransactionsRoot(Mismatch {
+			expected: expected_root,
+			found: *block.header.transactions_root(),
+		}));
+	}
+	let expected_uncles = keccak(block_rlp.at(2)?.as_raw());
+	if &expected_uncles != block.header.uncles_hash(){
+		bail!(BlockError::InvalidUnclesHash(Mismatch {
+			expected: expected_uncles,
+			found: *block.header.uncles_hash(),
+		}));
 	}
 	Ok(())
 }
@@ -366,6 +376,7 @@ mod tests {
 	use types::log_entry::{LogEntry, LocalizedLogEntry};
 	use rlp;
 	use triehash::ordered_trie_root;
+	use views::BlockView;
 
 	fn check_ok(result: Result<(), Error>) {
 		result.unwrap_or_else(|e| panic!("Block verification failed: {:?}", e));
@@ -486,8 +497,8 @@ mod tests {
 	}
 
 	fn basic_test(bytes: &[u8], engine: &EthEngine) -> Result<(), Error> {
-		let header = view!(BlockView, bytes).header();
-		verify_block_basic(&header, bytes, engine)
+		let unverified = Unverified::from_rlp(bytes.to_vec())?;
+		verify_block_basic(&unverified, engine)
 	}
 
 	fn family_test<BC>(bytes: &[u8], engine: &EthEngine, bc: &BC) -> Result<(), Error> where BC: BlockProvider {
@@ -507,18 +518,25 @@ mod tests {
 			.ok_or(BlockError::UnknownParent(header.parent_hash().clone()))?
 			.decode()?;
 
+		let block = PreverifiedBlock {
+			header,
+			transactions,
+			uncles: view.uncles(),
+			bytes: bytes.to_vec(),
+		};
+
 		let full_params = FullFamilyParams {
-			block_bytes: bytes,
-			transactions: &transactions[..],
+			block: &block,
 			block_provider: bc as &BlockProvider,
 			client: &client,
 		};
-		verify_block_family(&header, &parent, engine, Some(full_params))
+		verify_block_family(&block.header, &parent, engine, Some(full_params))
 	}
 
 	fn unordered_test(bytes: &[u8], engine: &EthEngine) -> Result<(), Error> {
-		let header = view!(BlockView, bytes).header();
-		verify_block_unordered(header, bytes.to_vec(), engine, false)?;
+		use verification::queue::kind::blocks::Unverified;
+		let un = Unverified::from_rlp(bytes.to_vec())?;
+		verify_block_unordered(un, engine, false)?;
 		Ok(())
 	}
 

ethcore/transaction/src/transaction.rs

@@ -282,6 +282,12 @@ pub struct UnverifiedTransaction {
 	hash: H256,
 }
 
+impl HeapSizeOf for UnverifiedTransaction {
+	fn heap_size_of_children(&self) -> usize {
+		self.unsigned.heap_size_of_children()
+	}
+}
+
 impl Deref for UnverifiedTransaction {
 	type Target = Transaction;
 
@@ -436,7 +442,7 @@ pub struct SignedTransaction {
 
 impl HeapSizeOf for SignedTransaction {
 	fn heap_size_of_children(&self) -> usize {
-		self.transaction.unsigned.heap_size_of_children()
+		self.transaction.heap_size_of_children()
 	}
 }