From eb78076a2bfed494a6ee64f6531d4c1594812907 Mon Sep 17 00:00:00 2001 From: debris Date: Wed, 18 Jul 2018 10:48:49 +0200 Subject: [PATCH 1/6] deserialize block only once during verification --- ethcore/src/client/client.rs | 8 +- ethcore/src/verification/mod.rs | 4 +- ethcore/src/verification/queue/kind.rs | 35 ++++-- ethcore/src/verification/verification.rs | 130 +++++++++++++---------- ethcore/transaction/src/transaction.rs | 8 +- 5 files changed, 113 insertions(+), 72 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 56c71a86ec9..e5e6c5dd984 100644 --- a/ethcore/src/client/client.rs +++ b/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 }), diff --git a/ethcore/src/verification/mod.rs b/ethcore/src/verification/mod.rs index 56cb2ade773..fdb04df8647 100644 --- a/ethcore/src/verification/mod.rs +++ b/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; diff --git a/ethcore/src/verification/queue/kind.rs b/ethcore/src/verification/queue/kind.rs index 2d89f11a33d..fbc6346c9c0 100644 --- a/ethcore/src/verification/queue/kind.rs +++ b/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 { - 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 { 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, + /// Unverified block uncles. + pub uncles: Vec
, + /// Raw block bytes. + pub bytes: Bytes, } impl Unverified { /// Create an `Unverified` from raw bytes. pub fn from_rlp(bytes: Bytes) -> Result { + 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() } } diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index ebb8e79a704..a021013543c 100644 --- a/ethcore/src/verification/verification.rs +++ b/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,8 +36,8 @@ 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 { @@ -46,6 +45,8 @@ pub struct PreverifiedBlock { pub header: Header, /// Populated block transactions pub transactions: Vec, + /// Populated block uncles + pub uncles: Vec
, /// 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::
()) { - 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::()) { - 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 { +pub fn verify_block_unordered(block: Unverified, engine: &EthEngine, check_seal: bool) -> Result { + 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::
()) { - 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::, 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(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 ¶ms.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::
()) { - 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(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(()) } diff --git a/ethcore/transaction/src/transaction.rs b/ethcore/transaction/src/transaction.rs index 27b8b346cf2..49804f1870f 100644 --- a/ethcore/transaction/src/transaction.rs +++ b/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() } } From 91edc42e0f88e69c0233b7211939797a2275eca7 Mon Sep 17 00:00:00 2001 From: debris Date: Wed, 18 Jul 2018 14:55:40 +0200 Subject: [PATCH 2/6] ethcore-sync uses Unverified --- ethcore/src/client/client.rs | 22 +++++++--------- ethcore/src/client/test_client.rs | 14 +++++----- ethcore/src/client/traits.rs | 3 ++- ethcore/src/test_helpers.rs | 9 ++++--- ethcore/sync/src/block_sync.rs | 24 ++++++++--------- ethcore/sync/src/chain/handler.rs | 43 +++++++++++++++---------------- 6 files changed, 57 insertions(+), 58 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index e5e6c5dd984..f8b7cb8ea0f 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -74,6 +74,7 @@ use types::filter::Filter; use types::ancestry_action::AncestryAction; use verification; use verification::{PreverifiedBlock, Verifier, BlockQueue}; +use verification::queue::kind::blocks::Unverified; use views::BlockView; // re-export @@ -1384,22 +1385,17 @@ impl CallContract for Client { } impl ImportBlock for Client { - fn import_block(&self, bytes: Bytes) -> Result { + fn import_block(&self, unverified: Unverified) -> Result { 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()))); - } + 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)?) } } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 7a3dfcd0075..8d23c84d321 100644 --- a/ethcore/src/client/test_client.rs +++ b/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 { - let header = view!(BlockView, &b).header(); + fn import_block(&self, unverified: Unverified) -> Result { + 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) } @@ -857,7 +859,7 @@ impl IoClient for TestBlockChainClient { } fn queue_ancient_block(&self, b: Bytes, _r: Bytes) -> Result { - self.import_block(b) + self.import_block(Unverified::from_rlp(b)?) } fn queue_consensus_message(&self, message: Bytes) { diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 65bf0092118..d70a8014d5f 100644 --- a/ethcore/src/client/traits.rs +++ b/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; + fn import_block(&self, block: Unverified) -> Result; } /// Provides `call_contract` method diff --git a/ethcore/src/test_helpers.rs b/ethcore/src/test_helpers.rs index 443e0418b55..634e4f114cb 100644 --- a/ethcore/src/test_helpers.rs +++ b/ethcore/src/test_helpers.rs @@ -43,6 +43,7 @@ use blooms_db; use kvdb::KeyValueDB; use kvdb_rocksdb; use tempdir::TempDir; +use verification::queue::kind::blocks::Unverified; /// Creates test block with corresponding header pub fn create_test_block(header: &Header) -> Bytes { @@ -174,7 +175,7 @@ pub fn generate_dummy_client_with_spec_accounts_and_data(test_spec: F, accoun let b = b.close_and_lock().unwrap().seal(test_engine, vec![]).unwrap(); - if let Err(e) = client.import_block(b.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(b.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -210,7 +211,7 @@ pub fn push_blocks_to_client(client: &Arc, timestamp_salt: u64, starting rolling_block_number = rolling_block_number + 1; rolling_timestamp = rolling_timestamp + 10; - if let Err(e) = client.import_block(create_test_block(&header)) { + if let Err(e) = client.import_block(Unverified::from_rlp(create_test_block(&header)).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } } @@ -230,7 +231,7 @@ pub fn push_block_with_transactions(client: &Arc, transactions: &[Signed } let b = b.close_and_lock().unwrap().seal(test_engine, vec![]).unwrap(); - if let Err(e) = client.import_block(b.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(b.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -252,7 +253,7 @@ pub fn get_test_client_with_blocks(blocks: Vec) -> Arc { ).unwrap(); for block in blocks { - if let Err(e) = client.import_block(block) { + if let Err(e) = client.import_block(Unverified::from_rlp(block).unwrap()) { panic!("error importing block which is well-formed: {:?}", e); } } diff --git a/ethcore/sync/src/block_sync.rs b/ethcore/sync/src/block_sync.rs index 2e508f4d69f..67e3d8ac3a8 100644 --- a/ethcore/sync/src/block_sync.rs +++ b/ethcore/sync/src/block_sync.rs @@ -23,11 +23,10 @@ use std::cmp; use heapsize::HeapSizeOf; use ethereum_types::H256; use rlp::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; @@ -478,18 +477,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: {:?}", block); + 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"); diff --git a/ethcore/sync/src/chain/handler.rs b/ethcore/sync/src/chain/handler.rs index 75111a4d4a6..96c4108566c 100644 --- a/ethcore/sync/src/chain/handler.rs +++ b/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; @@ -148,45 +149,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(); + trace!(target: "sync", "{} -> NewBlock ({})", peer_id, hash); + if block.header.number() > sync.highest_block.unwrap_or(0) { + sync.highest_block = Some(block.header.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 > block.header.number() && last_imported_number - block.header.number() > MAX_NEW_BLOCK_AGE { + trace!(target: "sync", "Ignored ancient new block {:?}", hash); io.disable_peer(peer_id); return Ok(()); } - 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, block.header.number()); + trace!(target: "sync", "New block queued {:?} ({})", hash, block.header.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); io.disable_peer(peer_id); } }; @@ -194,7 +193,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); } From 8290fb764acb7448e7246746956ae564f7fa9733 Mon Sep 17 00:00:00 2001 From: debris Date: Mon, 30 Jul 2018 16:15:34 +0200 Subject: [PATCH 3/6] ethcore-sync uses Unverified --- ethcore/src/block.rs | 44 ++++--------------- ethcore/src/client/client.rs | 33 +++++++------- ethcore/src/client/test_client.rs | 4 +- ethcore/src/client/traits.rs | 2 +- ethcore/src/engines/validator_set/multi.rs | 3 +- .../engines/validator_set/safe_contract.rs | 3 +- ethcore/src/miner/miner.rs | 2 +- ethcore/src/tests/client.rs | 22 +--------- ethcore/src/tests/trace.rs | 7 +-- ethcore/sync/src/block_sync.rs | 2 +- ethcore/sync/src/blocks.rs | 2 +- ethcore/sync/src/chain/handler.rs | 13 +++--- 12 files changed, 47 insertions(+), 90 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 6f1ba7a2f00..c1c61227ccc 100644 --- a/ethcore/src/block.rs +++ b/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. @@ -424,31 +423,9 @@ impl<'x> OpenBlock<'x> { /// Turn this into a `LockedBlock`. pub fn close_and_lock(self) -> Result { - let mut s = self; - - 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_state_root(s.block.state.root().clone()); - 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 closed = self.close()?; Ok(LockedBlock { - block: s.block, + block: closed.block, }) } @@ -537,18 +514,16 @@ impl LockedBlock { self, engine: &EthEngine, seal: Vec, - ) -> Result { + ) -> Result { 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 +612,11 @@ pub fn enact_verified( is_epoch_begin: bool, ancestry: &mut Iterator, ) -> Result { - let view = view!(BlockView, &block.bytes); enact( block.header, block.transactions, - view.uncles(), + block.uncles, engine, tracing, db, diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 5c86654dbd5..71ddd98ddb7 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -75,6 +75,7 @@ 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; @@ -210,7 +211,7 @@ pub struct Client { /// Queued ancient blocks, make sure they are imported in order. queued_ancient_blocks: Arc, - VecDeque<(Header, encoded::Block, Bytes)> + VecDeque<(Unverified, Bytes)> )>>, ancient_blocks_import_lock: Arc>, /// Consensus messages import queue @@ -430,7 +431,7 @@ 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(); @@ -438,11 +439,11 @@ impl Importer { 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(); @@ -1384,8 +1385,6 @@ impl CallContract for Client { impl ImportBlock for Client { fn import_block(&self, unverified: Unverified) -> Result { - use verification::queue::kind::BlockLike; - if self.chain.read().is_known(&unverified.hash()) { bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain)); } @@ -2018,24 +2017,23 @@ impl IoClient for Client { }); } - fn queue_ancient_block(&self, block_bytes: Bytes, receipts_bytes: Bytes) -> Result { + fn queue_ancient_block(&self, unverified: Unverified, receipts_bytes: Bytes) -> Result { 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))); } } } @@ -2044,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(); @@ -2056,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(), diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 8d23c84d321..627d844aeb6 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -858,8 +858,8 @@ impl IoClient for TestBlockChainClient { self.miner.import_external_transactions(self, txs); } - fn queue_ancient_block(&self, b: Bytes, _r: Bytes) -> Result { - self.import_block(Unverified::from_rlp(b)?) + fn queue_ancient_block(&self, unverified: Unverified, _r: Bytes) -> Result { + self.import_block(unverified) } fn queue_consensus_message(&self, message: Bytes) { diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index d70a8014d5f..3f5595f6193 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -205,7 +205,7 @@ pub trait IoClient: Sync + Send { fn queue_transactions(&self, transactions: Vec, 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; + fn queue_ancient_block(&self, block_bytes: Unverified, receipts_bytes: Bytes) -> Result; /// Queue conensus engine message. fn queue_consensus_message(&self, message: Bytes); diff --git a/ethcore/src/engines/validator_set/multi.rs b/ethcore/src/engines/validator_set/multi.rs index a23208cd2a8..24fb2d890bd 100644 --- a/ethcore/src/engines/validator_set/multi.rs +++ b/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); diff --git a/ethcore/src/engines/validator_set/safe_contract.rs b/ethcore/src/engines/validator_set/safe_contract.rs index a7f4f2c731a..adaa63f0b42 100644 --- a/ethcore/src/engines/validator_set/safe_contract.rs +++ b/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); diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 233e28139be..38acf9e1e50 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1148,7 +1148,7 @@ impl miner::MinerService for Miner { |b| &b.hash() == &block_hash ) { trace!(target: "miner", "Submitted block {}={}={} with seal {:?}", block_hash, b.hash(), b.header().bare_hash(), seal); - b.lock().try_seal(&*self.engine, seal).or_else(|(e, _)| { + b.lock().try_seal(&*self.engine, seal).or_else(|e| { warn!(target: "miner", "Mined solution rejected: {}", e); Err(ErrorKind::PowInvalid.into()) }) diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index a7629313d17..4031d30b08e 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -35,9 +35,9 @@ use views::BlockView; use ethkey::KeyPair; use transaction::{PendingTransaction, Transaction, Action, Condition}; use miner::MinerService; -use rlp::{RlpStream, EMPTY_LIST_RLP}; use tempdir::TempDir; use test_helpers; +use verification::queue::kind::blocks::Unverified; #[test] fn imports_from_empty() { @@ -97,7 +97,7 @@ fn imports_good_block() { IoChannel::disconnected(), ).unwrap(); let good_block = get_good_dummy_block(); - if client.import_block(good_block).is_err() { + if client.import_block(Unverified::from_rlp(good_block).unwrap()).is_err() { panic!("error importing block being good by definition"); } client.flush_queue(); @@ -107,24 +107,6 @@ fn imports_good_block() { assert!(!block.into_inner().is_empty()); } -#[test] -fn fails_to_import_block_with_invalid_rlp() { - use error::{BlockImportError, BlockImportErrorKind}; - - let client = generate_dummy_client(6); - let mut rlp = RlpStream::new_list(3); - rlp.append_raw(&EMPTY_LIST_RLP, 1); // empty header - rlp.append_raw(&EMPTY_LIST_RLP, 1); - rlp.append_raw(&EMPTY_LIST_RLP, 1); - let invalid_header_block = rlp.out(); - - match client.import_block(invalid_header_block) { - Err(BlockImportError(BlockImportErrorKind::Decoder(_), _)) => (), // all good - Err(_) => panic!("Should fail with a decoder error"), - Ok(_) => panic!("Should not import block with invalid header"), - } -} - #[test] fn query_none_block() { let db = test_helpers::new_db(); diff --git a/ethcore/src/tests/trace.rs b/ethcore/src/tests/trace.rs index fd7c932cbdc..24ef3780043 100644 --- a/ethcore/src/tests/trace.rs +++ b/ethcore/src/tests/trace.rs @@ -33,6 +33,7 @@ use views::BlockView; use trace::{RewardType, LocalizedTrace}; use trace::trace::Action::Reward; use test_helpers; +use verification::queue::kind::blocks::Unverified; #[test] fn can_trace_block_and_uncle_reward() { @@ -91,7 +92,7 @@ fn can_trace_block_and_uncle_reward() { let root_block = root_block.close_and_lock().unwrap().seal(engine, vec![]).unwrap(); - if let Err(e) = client.import_block(root_block.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(root_block.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -120,7 +121,7 @@ fn can_trace_block_and_uncle_reward() { let parent_block = parent_block.close_and_lock().unwrap().seal(engine, vec![]).unwrap(); - if let Err(e) = client.import_block(parent_block.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(parent_block.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -170,7 +171,7 @@ fn can_trace_block_and_uncle_reward() { let block = block.close_and_lock().unwrap().seal(engine, vec![]).unwrap(); - let res = client.import_block(block.rlp_bytes()); + let res = client.import_block(Unverified::from_rlp(block.rlp_bytes()).unwrap()); if res.is_err() { panic!("error importing block: {:#?}", res.err().unwrap()); } diff --git a/ethcore/sync/src/block_sync.rs b/ethcore/sync/src/block_sync.rs index 8a73bfb1e55..588bfc0c711 100644 --- a/ethcore/sync/src/block_sync.rs +++ b/ethcore/sync/src/block_sync.rs @@ -486,7 +486,7 @@ impl BlockDownloader { let block = match Unverified::from_rlp(block) { Ok(block) => block, Err(_) => { - debug!(target: "sync", "Bad block rlp: {:?}", block); + debug!(target: "sync", "Bad block rlp"); bad = true; break; } diff --git a/ethcore/sync/src/blocks.rs b/ethcore/sync/src/blocks.rs index df7d7a3bfde..248180b281d 100644 --- a/ethcore/sync/src/blocks.rs +++ b/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(), }); } diff --git a/ethcore/sync/src/chain/handler.rs b/ethcore/sync/src/chain/handler.rs index 8d2bd2c0e08..8547be7b3ff 100644 --- a/ethcore/sync/src/chain/handler.rs +++ b/ethcore/sync/src/chain/handler.rs @@ -165,9 +165,10 @@ impl SyncHandler { } 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 block.header.number() > sync.highest_block.unwrap_or(0) { - sync.highest_block = Some(block.header.number()); + if number > sync.highest_block.unwrap_or(0) { + sync.highest_block = Some(number); } let mut unknown = false; @@ -176,8 +177,8 @@ impl SyncHandler { } let last_imported_number = sync.new_blocks.last_imported_block_number(); - if last_imported_number > block.header.number() && last_imported_number - block.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) { @@ -190,8 +191,8 @@ impl SyncHandler { Ok(_) => { // abort current download of the same block sync.complete_sync(io); - sync.new_blocks.mark_as_known(&hash, block.header.number()); - trace!(target: "sync", "New block queued {:?} ({})", hash, block.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; From 35fcb8123db7e5567394acf1ee4e1d9ac038a1da Mon Sep 17 00:00:00 2001 From: debris Date: Mon, 30 Jul 2018 16:38:44 +0200 Subject: [PATCH 4/6] fixed build error --- parity/blockchain.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/parity/blockchain.rs b/parity/blockchain.rs index 21af2968ef5..cc92419dabf 100644 --- a/parity/blockchain.rs +++ b/parity/blockchain.rs @@ -30,6 +30,7 @@ use ethcore::client::{Mode, DatabaseCompactionProfile, VMType, BlockImportError, use ethcore::error::{ImportErrorKind, BlockImportErrorKind}; use ethcore::miner::Miner; use ethcore::verification::queue::VerifierSettings; +use ethcore::verification::queue::kind::blocks::Unverified; use ethcore_service::ClientService; use cache::CacheConfig; use informant::{Informant, FullNodeInformantData, MillisecondDuration}; @@ -417,8 +418,9 @@ fn execute_import(cmd: ImportBlockchain) -> Result<(), String> { service.register_io_handler(informant).map_err(|_| "Unable to register informant handler".to_owned())?; let do_import = |bytes| { + let block = Unverified::from_rlp(bytes).map_err(|_| "Invalid block rlp")?; while client.queue_info().is_full() { sleep(Duration::from_secs(1)); } - match client.import_block(bytes) { + match client.import_block(block) { Err(BlockImportError(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain), _)) => { trace!("Skipping block already in chain."); } From a19faf01f347e7f485a1e1a82c47e7715e132dba Mon Sep 17 00:00:00 2001 From: debris Date: Mon, 30 Jul 2018 20:33:24 +0200 Subject: [PATCH 5/6] removed Block::is_good --- ethcore/src/block.rs | 5 ----- ethcore/src/json_tests/chain.rs | 8 ++++---- rpc/src/lib.rs | 1 - rpc/src/v1/tests/eth.rs | 17 ++++++++--------- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index c1c61227ccc..449e851065e 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -65,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::().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); diff --git a/ethcore/src/json_tests/chain.rs b/ethcore/src/json_tests/chain.rs index 2d643e75fe4..83a940fcb64 100644 --- a/ethcore/src/json_tests/chain.rs +++ b/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(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(); } diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 96926700c06..2e731cd3475 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -43,7 +43,6 @@ extern crate jsonrpc_ipc_server as ipc; extern crate jsonrpc_pubsub; extern crate ethash; -#[cfg_attr(test, macro_use)] extern crate ethcore; extern crate parity_bytes as bytes; extern crate parity_crypto as crypto; diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 6e3b9855d65..217e032902c 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -20,14 +20,13 @@ use std::sync::Arc; use ethereum_types::{H256, Address}; use ethcore::account_provider::AccountProvider; -use ethcore::block::Block; use ethcore::client::{BlockChainClient, Client, ClientConfig, ChainInfo, ImportBlock}; use ethcore::ethereum; use ethcore::ids::BlockId; use ethcore::miner::Miner; use ethcore::spec::{Genesis, Spec}; use ethcore::test_helpers; -use ethcore::views::BlockView; +use ethcore::verification::queue::kind::blocks::Unverified; use ethjson::blockchain::BlockChain; use ethjson::state::test::ForkSpec; use io::IoChannel; @@ -85,9 +84,9 @@ impl EthTester { fn from_chain(chain: &BlockChain) -> Self { let tester = Self::from_spec(make_spec(chain)); - for b in &chain.blocks_rlp() { - if Block::is_good(&b) { - let _ = tester.client.import_block(b.clone()); + for b in chain.blocks_rlp() { + if let Ok(block) = Unverified::from_rlp(b) { + let _ = tester.client.import_block(block); tester.client.flush_queue(); tester.client.import_verified_blocks(); } @@ -423,11 +422,11 @@ fn verify_transaction_counts(name: String, chain: BlockChain) { let tester = EthTester::from_chain(&chain); let mut id = 1; - for b in chain.blocks_rlp().iter().filter(|b| Block::is_good(b)).map(|b| view!(BlockView, b)) { - let count = b.transactions_count(); + for b in chain.blocks_rlp().into_iter().filter_map(|b| Unverified::from_rlp(b).ok()) { + let count = b.transactions.len(); - let hash = b.hash(); - let number = b.header_view().number(); + let hash = b.header.hash(); + let number = b.header.number(); let (req, res) = by_hash(hash, count, &mut id); assert_eq!(tester.handler.handle_request_sync(&req), Some(res)); From 32f7b38905315f745db8b2d3fa547bc0879441b6 Mon Sep 17 00:00:00 2001 From: debris Date: Tue, 31 Jul 2018 10:55:10 +0200 Subject: [PATCH 6/6] applied review suggestions --- ethcore/src/block.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 449e851065e..9fd3957fb34 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -392,9 +392,18 @@ impl<'x> OpenBlock<'x> { /// Turn this into a `ClosedBlock`. pub fn close(self) -> Result { - let mut s = self; + let unclosed_state = self.block.state.clone(); + let locked = self.close_and_lock()?; - let unclosed_state = s.block.state.clone(); + Ok(ClosedBlock { + block: locked.block, + unclosed_state, + }) + } + + /// Turn this into a `LockedBlock`. + pub fn close_and_lock(self) -> Result { + let mut s = self; s.engine.on_close_block(&mut s.block)?; s.block.state.commit()?; @@ -410,17 +419,8 @@ impl<'x> OpenBlock<'x> { })); s.block.header.set_gas_used(s.block.receipts.last().map_or_else(U256::zero, |r| r.gas_used)); - Ok(ClosedBlock { - block: s.block, - unclosed_state, - }) - } - - /// Turn this into a `LockedBlock`. - pub fn close_and_lock(self) -> Result { - let closed = self.close()?; Ok(LockedBlock { - block: closed.block, + block: s.block, }) }