ethcore: fix pow difficulty validation

ethcore/src/ethereum/ethash.rs

@@ -21,7 +21,7 @@ use std::sync::Arc;
 use hash::{KECCAK_EMPTY_LIST_RLP};
 use engines::block_reward::{self, RewardKind};
 use ethash::{quick_get_difficulty, slow_hash_block_number, EthashManager, OptimizeFor};
-use ethereum_types::{H256, H64, U256, Address};
+use ethereum_types::{H256, H64, U512, U256, Address};
 use unexpected::{OutOfBounds, Mismatch};
 use block::*;
 use error::{BlockError, Error};
@@ -450,20 +450,27 @@ impl Ethash {
 
 	/// Convert an Ethash boundary to its original difficulty. Basically just `f(x) = 2^256 / x`.
 	pub fn boundary_to_difficulty(boundary: &H256) -> U256 {
-		let d = U256::from(*boundary);
-		if d <= U256::one() {
+		assert!(!boundary.is_zero());
+
+		let d = U512::from(&**boundary);
+		if d == U512::one() {
 			U256::max_value()
 		} else {
-			((U256::one() << 255) / d) << 1
+			// d > 1, so result should never overflow 256 bits
+			U256::from((U512::one() << 256) / d)
 		}
 	}
 
 	/// Convert an Ethash difficulty to the target boundary. Basically just `f(x) = 2^256 / x`.
 	pub fn difficulty_to_boundary(difficulty: &U256) -> H256 {
-		if *difficulty <= U256::one() {
+		assert!(!difficulty.is_zero());
+
+		if *difficulty == U256::one() {
 			U256::max_value().into()
 		} else {
-			(((U256::one() << 255) / *difficulty) << 1).into()
+			let d = U512::from(difficulty);
+			// d > 1, so result should never overflow 256 bits
+			U256::from((U512::one() << 256) / d).into()
 		}
 	}
 }
@@ -768,14 +775,36 @@ mod tests {
 
 	#[test]
 	fn test_difficulty_to_boundary() {
-		// result of f(0) is undefined, so do not assert the result
-		let _ = Ethash::difficulty_to_boundary(&U256::from(0));
 		assert_eq!(Ethash::difficulty_to_boundary(&U256::from(1)), H256::from(U256::max_value()));
 		assert_eq!(Ethash::difficulty_to_boundary(&U256::from(2)), H256::from_str("8000000000000000000000000000000000000000000000000000000000000000").unwrap());
 		assert_eq!(Ethash::difficulty_to_boundary(&U256::from(4)), H256::from_str("4000000000000000000000000000000000000000000000000000000000000000").unwrap());
 		assert_eq!(Ethash::difficulty_to_boundary(&U256::from(32)), H256::from_str("0800000000000000000000000000000000000000000000000000000000000000").unwrap());
 	}
 
+	#[test]
+	fn test_difficulty_to_boundary_regression() {
+		// the last bit was originally being truncated when performing the conversion
+		// https://github.com/paritytech/parity-ethereum/issues/8397
+		for difficulty in 1..9 {
+			assert_eq!(U256::from(difficulty), Ethash::boundary_to_difficulty(&Ethash::difficulty_to_boundary(&difficulty.into())));
+			assert_eq!(H256::from(difficulty), Ethash::difficulty_to_boundary(&Ethash::boundary_to_difficulty(&difficulty.into())));
+			assert_eq!(U256::from(difficulty), Ethash::boundary_to_difficulty(&Ethash::boundary_to_difficulty(&difficulty.into()).into()));
+			assert_eq!(H256::from(difficulty), Ethash::difficulty_to_boundary(&Ethash::difficulty_to_boundary(&difficulty.into()).into()));
+		}
+	}
+
+	#[test]
+	#[should_panic]
+	fn test_difficulty_to_boundary_panics_on_zero() {
+		Ethash::difficulty_to_boundary(&U256::from(0));
+	}
+
+	#[test]
+	#[should_panic]
+	fn test_boundary_to_difficulty_panics_on_zero() {
+		Ethash::boundary_to_difficulty(&H256::from(0));
+	}
+
 	#[test]
 	fn difficulty_frontier() {
 		let machine = new_homestead_test_machine();

json/src/spec/ethash.rs

@@ -24,6 +24,7 @@ use hash::Address;
 pub struct EthashParams {
 	/// See main EthashParams docs.
 	#[serde(rename="minimumDifficulty")]
+	#[serde(deserialize_with="uint::validate_non_zero")]
 	pub minimum_difficulty: Uint,
 	/// See main EthashParams docs.
 	#[serde(rename="difficultyBoundDivisor")]

miner/src/work_notify.rs

@@ -28,7 +28,7 @@ use self::ethash::SeedHashCompute;
 use self::url::Url;
 use self::hyper::header::ContentType;
 
-use ethereum_types::{H256, U256};
+use ethereum_types::{H256, U256, U512};
 use parking_lot::Mutex;
 use futures::Future;
 
@@ -69,10 +69,14 @@ impl WorkPoster {
 
 /// Convert an Ethash difficulty to the target boundary. Basically just `f(x) = 2^256 / x`.
 fn difficulty_to_boundary(difficulty: &U256) -> H256 {
-	if *difficulty <= U256::one() {
+	assert!(!difficulty.is_zero());
+
+	if *difficulty == U256::one() {
 		U256::max_value().into()
 	} else {
-		(((U256::one() << 255) / *difficulty) << 1).into()
+		let d = U512::from(difficulty);
+		// d > 1, so result should never overflow 256 bits
+		U256::from((U512::one() << 256) / d).into()
 	}
 }