decode block rlp less often (#9252)

in total:
- removed 4 redundant rlp deserializations
- avoid 1 redundant block data copy

ethcore/sync/src/block_sync.rs

@@ -23,11 +23,10 @@ use std::cmp;
 use heapsize::HeapSizeOf;
 use ethereum_types::H256;
 use rlp::{self, Rlp};
-use ethcore::views::BlockView;
 use ethcore::header::{BlockNumber, Header as BlockHeader};
 use ethcore::client::{BlockStatus, BlockId, BlockImportError, BlockImportErrorKind};
-use ethcore::block::Block;
 use ethcore::error::{ImportErrorKind, BlockError};
+use ethcore::verification::queue::kind::blocks::Unverified;
 use sync_io::SyncIo;
 use blocks::BlockCollection;
 
@@ -484,18 +483,19 @@ impl BlockDownloader {
 			let block = block_and_receipts.block;
 			let receipts = block_and_receipts.receipts;
 
-			// Perform basic block verification
-			if !Block::is_good(&block) {
-				debug!(target: "sync", "Bad block rlp: {:?}", block);
-				bad = true;
-				break;
-			}
-
-			let (h, number, parent) = {
-				let header = view!(BlockView, &block).header_view();
-				(header.hash(), header.number(), header.parent_hash())
+			let block = match Unverified::from_rlp(block) {
+				Ok(block) => block,
+				Err(_) => {
+					debug!(target: "sync", "Bad block rlp");
+					bad = true;
+					break;
+				}
 			};
 
+			let h = block.header.hash();
+			let number = block.header.number();
+			let parent = *block.header.parent_hash();
+
 			if self.target_hash.as_ref().map_or(false, |t| t == &h) {
 				self.state = State::Complete;
 				trace!(target: "sync", "Sync target reached");

ethcore/sync/src/blocks.rs

@@ -294,7 +294,7 @@ impl BlockCollection {
 				let header = view!(HeaderView, &block.header);
 				let block_view = Block::new_from_header_and_body(&header, &body);
 				drained.push(BlockAndReceipts {
-					block: block_view.rlp().as_raw().to_vec(),
+					block: block_view.into_inner(),
 					receipts: block.receipts.clone(),
 				});
 			}

ethcore/sync/src/chain/handler.rs

@@ -19,8 +19,9 @@ use block_sync::{BlockDownloaderImportError as DownloaderImportError, DownloadAc
 use bytes::Bytes;
 use ethcore::client::{BlockStatus, BlockId, BlockImportError, BlockImportErrorKind};
 use ethcore::error::*;
-use ethcore::header::{BlockNumber, Header as BlockHeader};
+use ethcore::header::BlockNumber;
 use ethcore::snapshot::{ManifestData, RestorationStatus};
+use ethcore::verification::queue::kind::blocks::Unverified;
 use ethereum_types::{H256, U256};
 use hash::keccak;
 use network::PeerId;
@@ -162,44 +163,43 @@ impl SyncHandler {
 				peer.difficulty = Some(difficulty);
 			}
 		}
-		let block_rlp = r.at(0)?;
-		let header_rlp = block_rlp.at(0)?;
-		let h = keccak(&header_rlp.as_raw());
-		trace!(target: "sync", "{} -> NewBlock ({})", peer_id, h);
-		let header: BlockHeader = header_rlp.as_val()?;
-		if header.number() > sync.highest_block.unwrap_or(0) {
-			sync.highest_block = Some(header.number());
+		let block = Unverified::from_rlp(r.at(0)?.as_raw().to_vec())?;
+		let hash = block.header.hash();
+		let number = block.header.number();
+		trace!(target: "sync", "{} -> NewBlock ({})", peer_id, hash);
+		if number > sync.highest_block.unwrap_or(0) {
+			sync.highest_block = Some(number);
 		}
 		let mut unknown = false;
-		{
-			if let Some(ref mut peer) = sync.peers.get_mut(&peer_id) {
-				peer.latest_hash = header.hash();
-			}
+
+		if let Some(ref mut peer) = sync.peers.get_mut(&peer_id) {
+			peer.latest_hash = hash;
 		}
+
 		let last_imported_number = sync.new_blocks.last_imported_block_number();
-		if last_imported_number > header.number() && last_imported_number - header.number() > MAX_NEW_BLOCK_AGE {
-			trace!(target: "sync", "Ignored ancient new block {:?}", h);
+		if last_imported_number > number && last_imported_number - number > MAX_NEW_BLOCK_AGE {
+			trace!(target: "sync", "Ignored ancient new block {:?}", hash);
 			return Err(DownloaderImportError::Invalid);
 		}
-		match io.chain().import_block(block_rlp.as_raw().to_vec()) {
+		match io.chain().import_block(block) {
 			Err(BlockImportError(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain), _)) => {
-				trace!(target: "sync", "New block already in chain {:?}", h);
+				trace!(target: "sync", "New block already in chain {:?}", hash);
 			},
 			Err(BlockImportError(BlockImportErrorKind::Import(ImportErrorKind::AlreadyQueued), _)) => {
-				trace!(target: "sync", "New block already queued {:?}", h);
+				trace!(target: "sync", "New block already queued {:?}", hash);
 			},
 			Ok(_) => {
 				// abort current download of the same block
 				sync.complete_sync(io);
-				sync.new_blocks.mark_as_known(&header.hash(), header.number());
-				trace!(target: "sync", "New block queued {:?} ({})", h, header.number());
+				sync.new_blocks.mark_as_known(&hash, number);
+				trace!(target: "sync", "New block queued {:?} ({})", hash, number);
 			},
 			Err(BlockImportError(BlockImportErrorKind::Block(BlockError::UnknownParent(p)), _)) => {
 				unknown = true;
-				trace!(target: "sync", "New block with unknown parent ({:?}) {:?}", p, h);
+				trace!(target: "sync", "New block with unknown parent ({:?}) {:?}", p, hash);
 			},
 			Err(e) => {
-				debug!(target: "sync", "Bad new block {:?} : {:?}", h, e);
+				debug!(target: "sync", "Bad new block {:?} : {:?}", hash, e);
 				return Err(DownloaderImportError::Invalid);
 			}
 		};
@@ -207,7 +207,7 @@ impl SyncHandler {
 			if sync.state != SyncState::Idle {
 				trace!(target: "sync", "NewBlock ignored while seeking");
 			} else {
-				trace!(target: "sync", "New unknown block {:?}", h);
+				trace!(target: "sync", "New unknown block {:?}", hash);
 				//TODO: handle too many unknown blocks
 				sync.sync_peer(io, peer_id, true);
 			}