From 13f09218ebfdec88ed062109f8335bb867abb3d2 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Thu, 5 Mar 2020 11:33:34 +0100 Subject: [PATCH 1/5] ethcore: cleanup after #11531 --- ethcore/src/block.rs | 27 ++++++++++++++++++++++++ ethcore/src/client/client.rs | 10 +++------ ethcore/types/src/block.rs | 9 +++++++- ethcore/verification/src/queue/kind.rs | 12 +++++------ ethcore/verification/src/verification.rs | 17 ++++++++------- 5 files changed, 53 insertions(+), 22 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 90b9a9c76e5..4ccc1c6e392 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -49,6 +49,7 @@ use vm::LastHashes; use hash::keccak; use rlp::{RlpStream, Encodable, encode_list}; use types::{ + block::PreverifiedBlock, errors::{EthcoreError as Error, BlockError}, transaction::{SignedTransaction, Error as TransactionError}, header::Header, @@ -459,6 +460,32 @@ pub(crate) fn enact( b.close_and_lock() } +/// Enact the block given by `block_bytes` using `engine` on the database `db` with the given `parent` block header +pub fn enact_verified( + block: PreverifiedBlock, + engine: &dyn Engine, + tracing: bool, + db: StateDB, + parent: &Header, + last_hashes: Arc, + factories: Factories, + is_epoch_begin: bool, +) -> Result { + + enact( + &block.header, + block.transactions, + block.uncles, + engine, + tracing, + db, + parent, + last_hashes, + factories, + is_epoch_begin, + ) +} + #[cfg(test)] mod tests { use test_helpers::get_temp_state_db; diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 5211e4178bb..366e1b69614 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -296,7 +296,9 @@ impl Importer { trace_time!("import_verified_blocks"); let start = Instant::now(); - for mut block in blocks { + for block in blocks { + let block_bytes = block.bytes; + let block = block.preverified_block; let hash = block.header.hash(); let is_invalid = invalid_blocks.contains(block.header.parent_hash()); @@ -305,12 +307,6 @@ impl Importer { continue; } - // -------------------------------------------------------- - // NOTE: this will remove the RLP bytes from the - // `PreverifiedBlock` so be careful not to use the bytes - // anywhere after this, it will be an empty `Vec`. - // -------------------------------------------------------- - let block_bytes = std::mem::take(&mut block.bytes); match self.check_and_lock_block(block, client) { Ok((locked_block, pending)) => { imported_blocks.push(hash); diff --git a/ethcore/types/src/block.rs b/ethcore/types/src/block.rs index 9eba9baefde..bd9190ee69f 100644 --- a/ethcore/types/src/block.rs +++ b/ethcore/types/src/block.rs @@ -67,7 +67,7 @@ impl Decodable for Block { } } -/// Preprocessed block data gathered in `verify_block_unordered` call +/// Preprocessed block data without block bytes #[derive(MallocSizeOf)] pub struct PreverifiedBlock { /// Populated block header @@ -76,6 +76,13 @@ pub struct PreverifiedBlock { pub transactions: Vec, /// Populated block uncles pub uncles: Vec
, +} + +/// Preprocessed block data gathered in `verify_block_unordered` call +#[derive(MallocSizeOf)] +pub struct PreverifiedBlockWithBytes { + /// Preprocessed block without block bytes + pub preverified_block: PreverifiedBlock, /// Block bytes pub bytes: Bytes, } diff --git a/ethcore/verification/src/queue/kind.rs b/ethcore/verification/src/queue/kind.rs index 546df2d5a50..6deeaea2946 100644 --- a/ethcore/verification/src/queue/kind.rs +++ b/ethcore/verification/src/queue/kind.rs @@ -81,7 +81,7 @@ pub mod blocks { use engine::Engine; use common_types::{ - block::PreverifiedBlock, + block::PreverifiedBlockWithBytes, errors::{EthcoreError as Error, BlockError}, verification::Unverified, }; @@ -96,7 +96,7 @@ pub mod blocks { impl Kind for Blocks { type Input = Unverified; type Unverified = Unverified; - type Verified = PreverifiedBlock; + type Verified = PreverifiedBlockWithBytes; fn create( input: Self::Input, @@ -146,9 +146,9 @@ pub mod blocks { } } - impl BlockLike for PreverifiedBlock { + impl BlockLike for PreverifiedBlockWithBytes { fn hash(&self) -> H256 { - self.header.hash() + self.preverified_block.header.hash() } fn raw_hash(&self) -> H256 { @@ -156,11 +156,11 @@ pub mod blocks { } fn parent_hash(&self) -> H256 { - *self.header.parent_hash() + *self.preverified_block.header.parent_hash() } fn difficulty(&self) -> U256 { - *self.header.difficulty() + *self.preverified_block.header.difficulty() } } } diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index b67cc127b23..a86b520436b 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -38,7 +38,7 @@ use common_types::{ header::Header, errors::{EthcoreError as Error, BlockError}, engines::MAX_UNCLE_AGE, - block::PreverifiedBlock, + block::{PreverifiedBlock, PreverifiedBlockWithBytes}, verification::Unverified, }; @@ -78,8 +78,8 @@ pub fn verify_block_basic(block: &Unverified, engine: &dyn Engine, check_seal: b /// 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(block: Unverified, engine: &dyn Engine, check_seal: bool) -> Result { +/// Returns a `PreverifiedBlockWithBytes` structure populated with transactions +pub fn verify_block_unordered(block: Unverified, engine: &dyn Engine, check_seal: bool) -> Result { let header = block.header; if check_seal { engine.verify_block_unordered(&header)?; @@ -107,10 +107,12 @@ pub fn verify_block_unordered(block: Unverified, engine: &dyn Engine, check_seal }) .collect::, Error>>()?; - Ok(PreverifiedBlock { - header, - transactions, - uncles: block.uncles, + Ok(PreverifiedBlockWithBytes { + preverified_block: PreverifiedBlock { + header, + transactions, + uncles: block.uncles, + }, bytes: block.bytes, }) } @@ -502,7 +504,6 @@ mod tests { header, transactions, uncles: block.uncles, - bytes: bytes.to_vec(), }; let full_params = FullFamilyParams { From 4f31d52f66813801b6fe3a8728ddf9b791b6f69e Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Thu, 5 Mar 2020 12:06:50 +0100 Subject: [PATCH 2/5] fix verification benches --- ethcore/verification/benches/verification.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ethcore/verification/benches/verification.rs b/ethcore/verification/benches/verification.rs index 81477cc48c0..ca17735393f 100644 --- a/ethcore/verification/benches/verification.rs +++ b/ethcore/verification/benches/verification.rs @@ -120,6 +120,7 @@ fn block_verification(c: &mut Criterion) { // Phase 3 verification let block = Unverified::from_rlp(rlp_8481476.clone()).expect(PROOF); let preverified = verification::verify_block_unordered(block, ðash, true).expect(PROOF); + let preverified = preverified.preverified_block; let parent = Unverified::from_rlp(rlp_8481475.clone()).expect(PROOF); let mut block_provider = TestBlockChain::new(); From bfb2525cb3087158b58ea962c1ce94b8210c2c66 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Thu, 5 Mar 2020 14:27:32 +0100 Subject: [PATCH 3/5] ethcore: remove enact_verified --- ethcore/src/block.rs | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 4ccc1c6e392..90b9a9c76e5 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -49,7 +49,6 @@ use vm::LastHashes; use hash::keccak; use rlp::{RlpStream, Encodable, encode_list}; use types::{ - block::PreverifiedBlock, errors::{EthcoreError as Error, BlockError}, transaction::{SignedTransaction, Error as TransactionError}, header::Header, @@ -460,32 +459,6 @@ pub(crate) fn enact( b.close_and_lock() } -/// Enact the block given by `block_bytes` using `engine` on the database `db` with the given `parent` block header -pub fn enact_verified( - block: PreverifiedBlock, - engine: &dyn Engine, - tracing: bool, - db: StateDB, - parent: &Header, - last_hashes: Arc, - factories: Factories, - is_epoch_begin: bool, -) -> Result { - - enact( - &block.header, - block.transactions, - block.uncles, - engine, - tracing, - db, - parent, - last_hashes, - factories, - is_epoch_begin, - ) -} - #[cfg(test)] mod tests { use test_helpers::get_temp_state_db; From 2d9c266082239011c07fae4cdea1e6013ffde353 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Thu, 5 Mar 2020 14:34:04 +0100 Subject: [PATCH 4/5] ethcore: replace PreverifiedBlock with a tuple --- ethcore/src/client/client.rs | 4 +--- ethcore/types/src/block.rs | 11 +---------- ethcore/verification/benches/verification.rs | 3 +-- ethcore/verification/src/queue/kind.rs | 14 +++++++------- ethcore/verification/src/verification.rs | 13 ++++++------- 5 files changed, 16 insertions(+), 29 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 366e1b69614..4f879035646 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -296,9 +296,7 @@ impl Importer { trace_time!("import_verified_blocks"); let start = Instant::now(); - for block in blocks { - let block_bytes = block.bytes; - let block = block.preverified_block; + for (block, block_bytes) in blocks { let hash = block.header.hash(); let is_invalid = invalid_blocks.contains(block.header.parent_hash()); diff --git a/ethcore/types/src/block.rs b/ethcore/types/src/block.rs index bd9190ee69f..9a1a29b394c 100644 --- a/ethcore/types/src/block.rs +++ b/ethcore/types/src/block.rs @@ -67,7 +67,7 @@ impl Decodable for Block { } } -/// Preprocessed block data without block bytes +/// Preprocessed block data gathered in `verify_block_unordered` call #[derive(MallocSizeOf)] pub struct PreverifiedBlock { /// Populated block header @@ -78,15 +78,6 @@ pub struct PreverifiedBlock { pub uncles: Vec
, } -/// Preprocessed block data gathered in `verify_block_unordered` call -#[derive(MallocSizeOf)] -pub struct PreverifiedBlockWithBytes { - /// Preprocessed block without block bytes - pub preverified_block: PreverifiedBlock, - /// Block bytes - pub bytes: Bytes, -} - /// Brief info about inserted block. #[derive(Clone)] pub struct BlockInfo { diff --git a/ethcore/verification/benches/verification.rs b/ethcore/verification/benches/verification.rs index ca17735393f..21c4bfdf7d1 100644 --- a/ethcore/verification/benches/verification.rs +++ b/ethcore/verification/benches/verification.rs @@ -119,8 +119,7 @@ fn block_verification(c: &mut Criterion) { // Phase 3 verification let block = Unverified::from_rlp(rlp_8481476.clone()).expect(PROOF); - let preverified = verification::verify_block_unordered(block, ðash, true).expect(PROOF); - let preverified = preverified.preverified_block; + let preverified = verification::verify_block_unordered(block, ðash, true).expect(PROOF).0; let parent = Unverified::from_rlp(rlp_8481475.clone()).expect(PROOF); let mut block_provider = TestBlockChain::new(); diff --git a/ethcore/verification/src/queue/kind.rs b/ethcore/verification/src/queue/kind.rs index 6deeaea2946..2dbd84ff1ed 100644 --- a/ethcore/verification/src/queue/kind.rs +++ b/ethcore/verification/src/queue/kind.rs @@ -81,7 +81,7 @@ pub mod blocks { use engine::Engine; use common_types::{ - block::PreverifiedBlockWithBytes, + block::PreverifiedBlock, errors::{EthcoreError as Error, BlockError}, verification::Unverified, }; @@ -96,7 +96,7 @@ pub mod blocks { impl Kind for Blocks { type Input = Unverified; type Unverified = Unverified; - type Verified = PreverifiedBlockWithBytes; + type Verified = (PreverifiedBlock, Vec); fn create( input: Self::Input, @@ -146,21 +146,21 @@ pub mod blocks { } } - impl BlockLike for PreverifiedBlockWithBytes { + impl BlockLike for (PreverifiedBlock, Vec) { fn hash(&self) -> H256 { - self.preverified_block.header.hash() + self.0.header.hash() } fn raw_hash(&self) -> H256 { - keccak_hash::keccak(&self.bytes) + keccak_hash::keccak(&self.1) } fn parent_hash(&self) -> H256 { - *self.preverified_block.header.parent_hash() + *self.0.header.parent_hash() } fn difficulty(&self) -> U256 { - *self.preverified_block.header.difficulty() + *self.0.header.difficulty() } } } diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index a86b520436b..7e2f26620aa 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -38,7 +38,7 @@ use common_types::{ header::Header, errors::{EthcoreError as Error, BlockError}, engines::MAX_UNCLE_AGE, - block::{PreverifiedBlock, PreverifiedBlockWithBytes}, + block::PreverifiedBlock, verification::Unverified, }; @@ -78,8 +78,8 @@ pub fn verify_block_basic(block: &Unverified, engine: &dyn Engine, check_seal: b /// Phase 2 verification. Perform costly checks such as transaction signatures and block nonce for ethash. /// Still operates on a individual block -/// Returns a `PreverifiedBlockWithBytes` structure populated with transactions -pub fn verify_block_unordered(block: Unverified, engine: &dyn Engine, check_seal: bool) -> Result { +/// Returns a `PreverifiedBlock` structure populated with transactions along with the block bytes. +pub fn verify_block_unordered(block: Unverified, engine: &dyn Engine, check_seal: bool) -> Result<(PreverifiedBlock, Vec), Error> { let header = block.header; if check_seal { engine.verify_block_unordered(&header)?; @@ -107,14 +107,13 @@ pub fn verify_block_unordered(block: Unverified, engine: &dyn Engine, check_seal }) .collect::, Error>>()?; - Ok(PreverifiedBlockWithBytes { - preverified_block: PreverifiedBlock { + Ok((PreverifiedBlock { header, transactions, uncles: block.uncles, }, - bytes: block.bytes, - }) + block.bytes, + )) } /// Parameters for full verification of block family From bad23fb5f3fbde48289ede4c874bc959a0d30777 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Thu, 5 Mar 2020 15:24:51 +0100 Subject: [PATCH 5/5] ethcore: more descriptive Vec type alias --- ethcore/types/src/block.rs | 3 +++ ethcore/verification/src/queue/kind.rs | 6 +++--- ethcore/verification/src/verification.rs | 10 +++++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ethcore/types/src/block.rs b/ethcore/types/src/block.rs index 9a1a29b394c..3045c2efdb8 100644 --- a/ethcore/types/src/block.rs +++ b/ethcore/types/src/block.rs @@ -78,6 +78,9 @@ pub struct PreverifiedBlock { pub uncles: Vec
, } +/// The RLP representation of a block. +pub type BlockRlpRepresentation = Vec; + /// Brief info about inserted block. #[derive(Clone)] pub struct BlockInfo { diff --git a/ethcore/verification/src/queue/kind.rs b/ethcore/verification/src/queue/kind.rs index 2dbd84ff1ed..49aec3d494b 100644 --- a/ethcore/verification/src/queue/kind.rs +++ b/ethcore/verification/src/queue/kind.rs @@ -81,7 +81,7 @@ pub mod blocks { use engine::Engine; use common_types::{ - block::PreverifiedBlock, + block::{BlockRlpRepresentation, PreverifiedBlock}, errors::{EthcoreError as Error, BlockError}, verification::Unverified, }; @@ -96,7 +96,7 @@ pub mod blocks { impl Kind for Blocks { type Input = Unverified; type Unverified = Unverified; - type Verified = (PreverifiedBlock, Vec); + type Verified = (PreverifiedBlock, BlockRlpRepresentation); fn create( input: Self::Input, @@ -146,7 +146,7 @@ pub mod blocks { } } - impl BlockLike for (PreverifiedBlock, Vec) { + impl BlockLike for (PreverifiedBlock, BlockRlpRepresentation) { fn hash(&self) -> H256 { self.0.header.hash() } diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index 7e2f26620aa..8786f627d01 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -38,7 +38,7 @@ use common_types::{ header::Header, errors::{EthcoreError as Error, BlockError}, engines::MAX_UNCLE_AGE, - block::PreverifiedBlock, + block::{BlockRlpRepresentation, PreverifiedBlock}, verification::Unverified, }; @@ -78,8 +78,12 @@ pub fn verify_block_basic(block: &Unverified, engine: &dyn Engine, check_seal: b /// 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 along with the block bytes. -pub fn verify_block_unordered(block: Unverified, engine: &dyn Engine, check_seal: bool) -> Result<(PreverifiedBlock, Vec), Error> { +/// Returns a `PreverifiedBlock` structure populated with transactions along with the RLP representation of the block. +pub fn verify_block_unordered( + block: Unverified, + engine: &dyn Engine, + check_seal: bool, +) -> Result<(PreverifiedBlock, BlockRlpRepresentation), Error> { let header = block.header; if check_seal { engine.verify_block_unordered(&header)?;