From 1986c4ee79cd6d3b42306f72e7061d9b75247358 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Fri, 22 Nov 2019 17:26:50 +0800 Subject: [PATCH] fixed verify_uncles error type (#11276) * fixed verify_uncles error type * cleanup and document fn verify_uncles bounds checking * find_uncle_headers and find_uncle_hashes take u64 instead of u32 as an input param * Update ethcore/verification/src/verification.rs Co-Authored-By: David --- ethcore/blockchain/src/blockchain.rs | 5 ++-- ethcore/types/src/engines/mod.rs | 2 +- ethcore/types/src/errors/block_error.rs | 7 ++--- ethcore/verification/src/verification.rs | 38 ++++++++++-------------- 4 files changed, 21 insertions(+), 31 deletions(-) diff --git a/ethcore/blockchain/src/blockchain.rs b/ethcore/blockchain/src/blockchain.rs index aae60f66b..362e14cf2 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/types/src/engines/mod.rs b/ethcore/types/src/engines/mod.rs index 94aaa4453..1cabe5a24 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/types/src/errors/block_error.rs b/ethcore/types/src/errors/block_error.rs index 31baceff3..3883de596 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 4f2c055a4..aa34b8e84 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -179,30 +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() { 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), + // uncle.number() needs to be within specific number range which is + // [header.number() - MAX_UNCLE_AGE, header.number() - 1] + // + // 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() && + uncle.number() + MAX_UNCLE_AGE >= header.number() { + header.number() - uncle.number() + } else { + return Err(BlockError::UncleOutOfBounds(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), - max: Some(header.number() - 1), - found: uncle.number() - }))); - } + }).into()); + }; // cB // cB.p^1 1 depth, valid uncle @@ -215,7 +207,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) => {