From 4eab8672b840b86007bb71878f3f77b096f354ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 10 Aug 2018 14:36:19 +0100 Subject: [PATCH] ethcore: fix pow difficulty validation (#9328) * ethcore: fix pow difficulty validation * ethcore: validate difficulty is not zero * ethcore: add issue link to regression test * ethcore: fix tests * ethcore: move difficulty_to_boundary to ethash crate * ethcore: reuse difficulty_to_boundary and boundary_to_difficulty * ethcore: fix grumbles in difficulty_to_boundary_aux --- Cargo.lock | 1 + ethash/Cargo.toml | 11 +++--- ethash/src/lib.rs | 66 ++++++++++++++++++++++++++++++++-- ethcore/src/ethereum/ethash.rs | 35 ++---------------- ethcore/src/miner/stratum.rs | 5 ++- json/src/spec/ethash.rs | 1 + miner/src/work_notify.rs | 11 +----- rpc/src/v1/impls/eth.rs | 5 ++- 8 files changed, 80 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa96f9fe2..9e95c9ea8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -473,6 +473,7 @@ version = "1.12.0" dependencies = [ "crunchy 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "either 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "keccak-hash 0.1.2 (git+https://github.com/paritytech/parity-common)", "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ethash/Cargo.toml b/ethash/Cargo.toml index f4a99cd5e..772f12d4b 100644 --- a/ethash/Cargo.toml +++ b/ethash/Cargo.toml @@ -6,13 +6,14 @@ authors = ["Parity Technologies "] [lib] [dependencies] -log = "0.4" -keccak-hash = { git = "https://github.com/paritytech/parity-common" } -primal = "0.2.3" -parking_lot = "0.6" crunchy = "0.1.0" -memmap = "0.6" either = "1.0.0" +ethereum-types = "0.3" +keccak-hash = { git = "https://github.com/paritytech/parity-common" } +log = "0.4" +memmap = "0.6" +parking_lot = "0.6" +primal = "0.2.3" [dev-dependencies] tempdir = "0.3" diff --git a/ethash/src/lib.rs b/ethash/src/lib.rs index 69b5a1d11..29361ad5c 100644 --- a/ethash/src/lib.rs +++ b/ethash/src/lib.rs @@ -16,10 +16,11 @@ #![cfg_attr(feature = "benches", feature(test))] -extern crate primal; -extern crate parking_lot; extern crate either; +extern crate ethereum_types; extern crate memmap; +extern crate parking_lot; +extern crate primal; #[macro_use] extern crate crunchy; @@ -38,6 +39,7 @@ mod shared; pub use cache::{NodeCacheBuilder, OptimizeFor}; pub use compute::{ProofOfWork, quick_get_difficulty, slow_hash_block_number}; use compute::Light; +use ethereum_types::{U256, U512}; use keccak::H256; use parking_lot::Mutex; pub use seed_compute::SeedHashCompute; @@ -136,6 +138,29 @@ impl EthashManager { } } +/// Convert an Ethash boundary to its original difficulty. Basically just `f(x) = 2^256 / x`. +pub fn boundary_to_difficulty(boundary: ðereum_types::H256) -> U256 { + difficulty_to_boundary_aux(&**boundary) +} + +/// Convert an Ethash difficulty to the target boundary. Basically just `f(x) = 2^256 / x`. +pub fn difficulty_to_boundary(difficulty: &U256) -> ethereum_types::H256 { + difficulty_to_boundary_aux(difficulty).into() +} + +fn difficulty_to_boundary_aux>(difficulty: T) -> ethereum_types::U256 { + let difficulty = difficulty.into(); + + assert!(!difficulty.is_zero()); + + if difficulty == U512::one() { + U256::max_value() + } else { + // difficulty > 1, so result should never overflow 256 bits + U256::from((U512::one() << 256) / difficulty) + } +} + #[test] fn test_lru() { use tempdir::TempDir; @@ -155,6 +180,43 @@ fn test_lru() { assert_eq!(ethash.cache.lock().prev_epoch.unwrap(), 0); } +#[test] +fn test_difficulty_to_boundary() { + use ethereum_types::H256; + use std::str::FromStr; + + assert_eq!(difficulty_to_boundary(&U256::from(1)), H256::from(U256::max_value())); + assert_eq!(difficulty_to_boundary(&U256::from(2)), H256::from_str("8000000000000000000000000000000000000000000000000000000000000000").unwrap()); + assert_eq!(difficulty_to_boundary(&U256::from(4)), H256::from_str("4000000000000000000000000000000000000000000000000000000000000000").unwrap()); + assert_eq!(difficulty_to_boundary(&U256::from(32)), H256::from_str("0800000000000000000000000000000000000000000000000000000000000000").unwrap()); +} + +#[test] +fn test_difficulty_to_boundary_regression() { + use ethereum_types::H256; + + // 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), boundary_to_difficulty(&difficulty_to_boundary(&difficulty.into()))); + assert_eq!(H256::from(difficulty), difficulty_to_boundary(&boundary_to_difficulty(&difficulty.into()))); + assert_eq!(U256::from(difficulty), boundary_to_difficulty(&boundary_to_difficulty(&difficulty.into()).into())); + assert_eq!(H256::from(difficulty), difficulty_to_boundary(&difficulty_to_boundary(&difficulty.into()).into())); + } +} + +#[test] +#[should_panic] +fn test_difficulty_to_boundary_panics_on_zero() { + difficulty_to_boundary(&U256::from(0)); +} + +#[test] +#[should_panic] +fn test_boundary_to_difficulty_panics_on_zero() { + boundary_to_difficulty(ðereum_types::H256::from(0)); +} + #[cfg(feature = "benches")] mod benchmarks { extern crate test; diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index ca19983d3..16069c327 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -20,7 +20,7 @@ use std::collections::BTreeMap; 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 ethash::{self, quick_get_difficulty, slow_hash_block_number, EthashManager, OptimizeFor}; use ethereum_types::{H256, H64, U256, Address}; use unexpected::{OutOfBounds, Mismatch}; use block::*; @@ -302,7 +302,7 @@ impl Engine for Arc { return Err(From::from(BlockError::DifficultyOutOfBounds(OutOfBounds { min: Some(min_difficulty), max: None, found: header.difficulty().clone() }))) } - let difficulty = Ethash::boundary_to_difficulty(&H256(quick_get_difficulty( + let difficulty = ethash::boundary_to_difficulty(&H256(quick_get_difficulty( &header.bare_hash().0, seal.nonce.low_u64(), &seal.mix_hash.0 @@ -324,7 +324,7 @@ impl Engine for Arc { let result = self.pow.compute_light(header.number() as u64, &header.bare_hash().0, seal.nonce.low_u64()); let mix = H256(result.mix_hash); - let difficulty = Ethash::boundary_to_difficulty(&H256(result.value)); + let difficulty = ethash::boundary_to_difficulty(&H256(result.value)); trace!(target: "miner", "num: {num}, seed: {seed}, h: {h}, non: {non}, mix: {mix}, res: {res}", num = header.number() as u64, seed = H256(slow_hash_block_number(header.number() as u64)), @@ -447,25 +447,6 @@ impl Ethash { } target } - - /// 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() { - U256::max_value() - } else { - ((U256::one() << 255) / d) << 1 - } - } - - /// 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() { - U256::max_value().into() - } else { - (((U256::one() << 255) / *difficulty) << 1).into() - } - } } fn ecip1017_eras_block_reward(era_rounds: u64, mut reward: U256, block_number:u64) -> (u64, U256) { @@ -766,16 +747,6 @@ 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 difficulty_frontier() { let machine = new_homestead_test_machine(); diff --git a/ethcore/src/miner/stratum.rs b/ethcore/src/miner/stratum.rs index ca7443279..38c881bb1 100644 --- a/ethcore/src/miner/stratum.rs +++ b/ethcore/src/miner/stratum.rs @@ -22,8 +22,7 @@ use std::fmt; use client::{Client, ImportSealedBlock}; use ethereum_types::{H64, H256, clean_0x, U256}; -use ethereum::ethash::Ethash; -use ethash::SeedHashCompute; +use ethash::{self, SeedHashCompute}; #[cfg(feature = "work-notify")] use ethcore_miner::work_notify::NotifyWork; #[cfg(feature = "work-notify")] @@ -167,7 +166,7 @@ impl StratumJobDispatcher { /// Serializes payload for stratum service fn payload(&self, pow_hash: H256, difficulty: U256, number: u64) -> String { // TODO: move this to engine - let target = Ethash::difficulty_to_boundary(&difficulty); + let target = ethash::difficulty_to_boundary(&difficulty); let seed_hash = &self.seed_compute.lock().hash_block_number(number); let seed_hash = H256::from_slice(&seed_hash[..]); format!( diff --git a/json/src/spec/ethash.rs b/json/src/spec/ethash.rs index 19fd09662..fd6b9fca5 100644 --- a/json/src/spec/ethash.rs +++ b/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")] diff --git a/miner/src/work_notify.rs b/miner/src/work_notify.rs index efae26ff1..522901982 100644 --- a/miner/src/work_notify.rs +++ b/miner/src/work_notify.rs @@ -67,19 +67,10 @@ 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() { - U256::max_value().into() - } else { - (((U256::one() << 255) / *difficulty) << 1).into() - } -} - impl NotifyWork for WorkPoster { fn notify(&self, pow_hash: H256, difficulty: U256, number: u64) { // TODO: move this to engine - let target = difficulty_to_boundary(&difficulty); + let target = ethash::difficulty_to_boundary(&difficulty); let seed_hash = &self.seed_compute.lock().hash_block_number(number); let seed_hash = H256::from_slice(&seed_hash[..]); let body = format!( diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 1e4ad87a5..67dc640d8 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -24,10 +24,9 @@ use rlp::{self, Rlp}; use ethereum_types::{U256, H64, H256, Address}; use parking_lot::Mutex; -use ethash::SeedHashCompute; +use ethash::{self, SeedHashCompute}; use ethcore::account_provider::AccountProvider; use ethcore::client::{BlockChainClient, BlockId, TransactionId, UncleId, StateOrBlock, StateClient, StateInfo, Call, EngineInfo}; -use ethcore::ethereum::Ethash; use ethcore::filter::Filter as EthcoreFilter; use ethcore::header::{BlockNumber as EthBlockNumber}; use ethcore::log_entry::LogEntry; @@ -758,7 +757,7 @@ impl Eth for EthClient< })?; let (pow_hash, number, timestamp, difficulty) = work; - let target = Ethash::difficulty_to_boundary(&difficulty); + let target = ethash::difficulty_to_boundary(&difficulty); let seed_hash = self.seed_compute.lock().hash_block_number(number); let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs();