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 <[email protected]>

Author: 2 people

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<Vec<encoded::Header>> {
+	pub fn find_uncle_headers(&self, parent: &H256, uncle_generations: u64) -> Option<Vec<encoded::Header>> {
 		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<Vec<H256>> {
+	pub fn find_uncle_hashes(&self, parent: &H256, uncle_generations: u64) -> Option<Vec<H256>> {
+		let uncle_generations = uncle_generations as usize;
 		if !self.is_known(parent) {
 			return None;
 		}

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

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<H256>),
-	/// 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<BlockNumber>),
-	/// An uncle is from the same generation as the block.
-	#[display(fmt = "Uncle from same generation as block. {}", _0)]
-	UncleIsBrother(OutOfBounds<BlockNumber>),
+	UncleOutOfBounds(OutOfBounds<BlockNumber>),
 	/// An uncle is already in the chain.
 	#[display(fmt = "Uncle {} already in chain", _0)]
 	UncleInChain(H256),

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) => {