From 0b550a6f22b25050b86b88be3c9af5abb805d325 Mon Sep 17 00:00:00 2001 From: debris Date: Thu, 21 Nov 2019 13:15:58 +0800 Subject: [PATCH 1/4] fixed verify_uncles error type --- ethcore/src/client/client.rs | 6 +++--- ethcore/types/src/engines/mod.rs | 2 +- ethcore/verification/src/verification.rs | 21 +++++++++++---------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 3a80afe874f..7843acf66df 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1938,7 +1938,7 @@ impl BlockChainClient for Client { } fn find_uncles(&self, hash: &H256) -> Option> { - self.chain.read().find_uncle_hashes(hash, MAX_UNCLE_AGE) + self.chain.read().find_uncle_hashes(hash, MAX_UNCLE_AGE as usize) } fn state_data(&self, hash: &H256) -> Option { @@ -2306,7 +2306,7 @@ impl ReopenBlock for Client { let h = chain.best_block_hash(); // Add new uncles let uncles = chain - .find_uncle_hashes(&h, MAX_UNCLE_AGE) + .find_uncle_hashes(&h, MAX_UNCLE_AGE as usize) .unwrap_or_else(Vec::new); for h in uncles { @@ -2349,7 +2349,7 @@ impl PrepareOpenBlock for Client { // Add uncles chain - .find_uncle_headers(&h, MAX_UNCLE_AGE) + .find_uncle_headers(&h, MAX_UNCLE_AGE as usize) .unwrap_or_else(Vec::new) .into_iter() .take(engine.maximum_uncle_count(open_block.header.number())) diff --git a/ethcore/types/src/engines/mod.rs b/ethcore/types/src/engines/mod.rs index 94aaa445319..1cabe5a2456 100644 --- a/ethcore/types/src/engines/mod.rs +++ b/ethcore/types/src/engines/mod.rs @@ -92,7 +92,7 @@ pub enum SealingState { } /// The number of generations back that uncles can be. -pub const MAX_UNCLE_AGE: usize = 6; +pub const MAX_UNCLE_AGE: u64 = 6; /// Default EIP-210 contract code. /// As defined in https://github.com/ethereum/EIPs/pull/210 diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index 4f2c055a482..870341cf7db 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -187,18 +187,19 @@ fn verify_uncles(block: &PreverifiedBlock, bc: &dyn BlockProvider, engine: &dyn // 5 // 6 7 // (8 Invalid) - - let depth = if header.number() > uncle.number() { header.number() - uncle.number() } else { 0 }; - if depth > MAX_UNCLE_AGE as u64 { - return Err(From::from(BlockError::UncleTooOld(OutOfBounds { - min: Some(header.number() - depth), + let depth = if header.number() > uncle.number() { + header.number() - uncle.number() + } else { + return Err(From::from(BlockError::UncleIsBrother(OutOfBounds { + min: Some(header.number() - MAX_UNCLE_AGE ), max: Some(header.number() - 1), found: uncle.number() }))); - } - else if depth < 1 { - return Err(From::from(BlockError::UncleIsBrother(OutOfBounds { - min: Some(header.number() - depth), + }; + + if depth > MAX_UNCLE_AGE { + return Err(From::from(BlockError::UncleTooOld(OutOfBounds { + min: Some(header.number() - MAX_UNCLE_AGE), max: Some(header.number() - 1), found: uncle.number() }))); @@ -215,7 +216,7 @@ fn verify_uncles(block: &PreverifiedBlock, bc: &dyn BlockProvider, engine: &dyn // cB.p^8 let mut expected_uncle_parent = header.parent_hash().clone(); let uncle_parent = bc.block_header_data(&uncle.parent_hash()) - .ok_or_else(|| Error::from(BlockError::UnknownUncleParent(*uncle.parent_hash())))?; + .ok_or_else(|| BlockError::UnknownUncleParent(*uncle.parent_hash()))?; for _ in 0..depth { match bc.block_details(&expected_uncle_parent) { Some(details) => { From 19010df3756e25f73f5136295d43505a8d414806 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 22 Nov 2019 14:58:01 +0800 Subject: [PATCH 2/4] cleanup and document fn verify_uncles bounds checking --- ethcore/types/src/errors/block_error.rs | 7 ++---- ethcore/verification/src/verification.rs | 31 +++++++++--------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/ethcore/types/src/errors/block_error.rs b/ethcore/types/src/errors/block_error.rs index 31baceff39f..3883de596b2 100644 --- a/ethcore/types/src/errors/block_error.rs +++ b/ethcore/types/src/errors/block_error.rs @@ -44,12 +44,9 @@ pub enum BlockError { /// Uncles hash in header is invalid. #[display(fmt = "Block has invalid uncles hash: {}", _0)] InvalidUnclesHash(Mismatch), - /// An uncle is from a generation too old. + /// An uncle is from a wrong generation. #[display(fmt = "Uncle block is too old. {}", _0)] - UncleTooOld(OutOfBounds), - /// An uncle is from the same generation as the block. - #[display(fmt = "Uncle from same generation as block. {}", _0)] - UncleIsBrother(OutOfBounds), + UncleOutOfBounds(OutOfBounds), /// An uncle is already in the chain. #[display(fmt = "Uncle {} already in chain", _0)] UncleInChain(H256), diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index 870341cf7db..92b0f02cb46 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -179,31 +179,22 @@ fn verify_uncles(block: &PreverifiedBlock, bc: &dyn BlockProvider, engine: &dyn return Err(From::from(BlockError::DuplicateUncle(uncle.hash()))) } - // m_currentBlock.number() - uncle.number() m_cB.n - uP.n() - // 1 2 - // 2 - // 3 - // 4 - // 5 - // 6 7 - // (8 Invalid) - let depth = if header.number() > uncle.number() { + // uncle.number() needs to be within specific number range which is + // [header.number() - MAX_UNCLE_AGE, header.number() - 1] + // + // depth is a difference between uncle.number() and header.number() + // and the previous condition implies that it is always in range + // [1, MAX_UNCLE_AGE] + let depth = if header.number() > uncle.number() && + uncle.number() + MAX_UNCLE_AGE >= header.number() { header.number() - uncle.number() } else { - return Err(From::from(BlockError::UncleIsBrother(OutOfBounds { - min: Some(header.number() - MAX_UNCLE_AGE ), - max: Some(header.number() - 1), - found: uncle.number() - }))); - }; - - if depth > MAX_UNCLE_AGE { - return Err(From::from(BlockError::UncleTooOld(OutOfBounds { + return Err(BlockError::UncleOutOfBounds(OutOfBounds { min: Some(header.number() - MAX_UNCLE_AGE), max: Some(header.number() - 1), found: uncle.number() - }))); - } + }).into()); + }; // cB // cB.p^1 1 depth, valid uncle From 304117d323b9a9a85c7f0651eeadf51e8e7b16b4 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 22 Nov 2019 15:14:31 +0800 Subject: [PATCH 3/4] find_uncle_headers and find_uncle_hashes take u64 instead of u32 as an input param --- ethcore/blockchain/src/blockchain.rs | 5 +++-- ethcore/src/client/client.rs | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ethcore/blockchain/src/blockchain.rs b/ethcore/blockchain/src/blockchain.rs index aae60f66b17..362e14cf2fc 100644 --- a/ethcore/blockchain/src/blockchain.rs +++ b/ethcore/blockchain/src/blockchain.rs @@ -1323,13 +1323,14 @@ impl BlockChain { } /// Given a block's `parent`, find every block header which represents a valid possible uncle. - pub fn find_uncle_headers(&self, parent: &H256, uncle_generations: usize) -> Option> { + pub fn find_uncle_headers(&self, parent: &H256, uncle_generations: u64) -> Option> { self.find_uncle_hashes(parent, uncle_generations) .map(|v| v.into_iter().filter_map(|h| self.block_header_data(&h)).collect()) } /// Given a block's `parent`, find every block hash which represents a valid possible uncle. - pub fn find_uncle_hashes(&self, parent: &H256, uncle_generations: usize) -> Option> { + pub fn find_uncle_hashes(&self, parent: &H256, uncle_generations: u64) -> Option> { + let uncle_generations = uncle_generations as usize; if !self.is_known(parent) { return None; } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 7843acf66df..3a80afe874f 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1938,7 +1938,7 @@ impl BlockChainClient for Client { } fn find_uncles(&self, hash: &H256) -> Option> { - self.chain.read().find_uncle_hashes(hash, MAX_UNCLE_AGE as usize) + self.chain.read().find_uncle_hashes(hash, MAX_UNCLE_AGE) } fn state_data(&self, hash: &H256) -> Option { @@ -2306,7 +2306,7 @@ impl ReopenBlock for Client { let h = chain.best_block_hash(); // Add new uncles let uncles = chain - .find_uncle_hashes(&h, MAX_UNCLE_AGE as usize) + .find_uncle_hashes(&h, MAX_UNCLE_AGE) .unwrap_or_else(Vec::new); for h in uncles { @@ -2349,7 +2349,7 @@ impl PrepareOpenBlock for Client { // Add uncles chain - .find_uncle_headers(&h, MAX_UNCLE_AGE as usize) + .find_uncle_headers(&h, MAX_UNCLE_AGE) .unwrap_or_else(Vec::new) .into_iter() .take(engine.maximum_uncle_count(open_block.header.number())) From 45588354c9264944c19ec8c6435d2ef0f85b5472 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Fri, 22 Nov 2019 15:16:22 +0800 Subject: [PATCH 4/4] Update ethcore/verification/src/verification.rs Co-Authored-By: David --- ethcore/verification/src/verification.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index 92b0f02cb46..aa34b8e8415 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -182,7 +182,7 @@ fn verify_uncles(block: &PreverifiedBlock, bc: &dyn BlockProvider, engine: &dyn // uncle.number() needs to be within specific number range which is // [header.number() - MAX_UNCLE_AGE, header.number() - 1] // - // depth is a difference between uncle.number() and header.number() + // depth is the difference between uncle.number() and header.number() // and the previous condition implies that it is always in range // [1, MAX_UNCLE_AGE] let depth = if header.number() > uncle.number() &&