From fdc7b0fdaa386faed5a74bc61b8ffba2ac7d7f9f Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Tue, 9 Jul 2019 04:27:33 +0200 Subject: [PATCH] removed redundant fork choice abstraction (#10849) --- ethcore/src/client/client.rs | 32 ++++++++-------------- ethcore/src/engines/authority_round/mod.rs | 4 --- ethcore/src/engines/basic_authority.rs | 6 +--- ethcore/src/engines/clique/mod.rs | 6 +--- ethcore/src/engines/instant_seal.rs | 6 +--- ethcore/src/engines/mod.rs | 12 -------- ethcore/src/engines/null_engine.rs | 6 +--- ethcore/src/ethereum/ethash.rs | 6 +--- 8 files changed, 17 insertions(+), 61 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 92a63b5d7..3026290b5 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -40,7 +40,7 @@ use types::encoded; use types::filter::Filter; use types::log_entry::LocalizedLogEntry; use types::receipt::{Receipt, LocalizedReceipt}; -use types::{BlockNumber, header::{Header, ExtendedHeader}}; +use types::{BlockNumber, header::Header}; use vm::{EnvInfo, LastHashes}; use hash_db::EMPTY_PREFIX; use block::{LockedBlock, Drain, ClosedBlock, OpenBlock, enact_verified, SealedBlock}; @@ -502,32 +502,24 @@ impl Importer { let traces = block.traces.drain(); let best_hash = chain.best_block_hash(); - let new = ExtendedHeader { - header: header.clone(), - is_finalized, - parent_total_difficulty: chain.block_details(&parent).expect("Parent block is in the database; qed").total_difficulty + let new_total_difficulty = { + let parent_total_difficulty = chain.block_details(&parent) + .expect("Parent block is in the database; qed") + .total_difficulty; + parent_total_difficulty + header.difficulty() }; - let best = { - let header = chain.block_header_data(&best_hash) - .expect("Best block is in the database; qed") - .decode() - .expect("Stored block header is valid RLP; qed"); - let details = chain.block_details(&best_hash) - .expect("Best block is in the database; qed"); - - ExtendedHeader { - parent_total_difficulty: details.total_difficulty - *header.difficulty(), - is_finalized: details.is_finalized, - header, - } - }; + let best_total_difficulty = chain.block_details(&best_hash) + .expect("Best block is in the database; qed") + .total_difficulty; let route = chain.tree_route(best_hash, *parent).expect("forks are only kept when it has common ancestors; tree route from best to prospective's parent always exists; qed"); let fork_choice = if route.is_from_route_finalized { ForkChoice::Old + } else if new_total_difficulty > best_total_difficulty { + ForkChoice::New } else { - self.engine.fork_choice(&new, &best) + ForkChoice::Old }; // CHECK! I *think* this is fine, even if the state_root is equal to another diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 840f689b9..360ec0443 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -1592,10 +1592,6 @@ impl Engine for AuthorityRound { } } - fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> super::ForkChoice { - super::total_difficulty_fork_choice(new, current) - } - fn ancestry_actions(&self, header: &Header, ancestry: &mut dyn Iterator) -> Vec { let finalized = self.build_finality( header, diff --git a/ethcore/src/engines/basic_authority.rs b/ethcore/src/engines/basic_authority.rs index b08512b79..8232c0ff2 100644 --- a/ethcore/src/engines/basic_authority.rs +++ b/ethcore/src/engines/basic_authority.rs @@ -27,7 +27,7 @@ use error::{BlockError, Error}; use ethjson; use client::EngineClient; use machine::{AuxiliaryData, Call, Machine}; -use types::header::{Header, ExtendedHeader}; +use types::header::Header; use super::validator_set::{ValidatorSet, SimpleList, new_validator_set}; /// `BasicAuthority` params. @@ -208,10 +208,6 @@ impl Engine for BasicAuthority { fn snapshot_components(&self) -> Option> { None } - - fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> super::ForkChoice { - super::total_difficulty_fork_choice(new, current) - } } #[cfg(test)] diff --git a/ethcore/src/engines/clique/mod.rs b/ethcore/src/engines/clique/mod.rs index 89bcef94c..adfcf88f0 100644 --- a/ethcore/src/engines/clique/mod.rs +++ b/ethcore/src/engines/clique/mod.rs @@ -83,7 +83,7 @@ use super::signer::EngineSigner; use unexpected::{Mismatch, OutOfBounds}; use time_utils::CheckedSystemTime; use types::BlockNumber; -use types::header::{ExtendedHeader, Header}; +use types::header::Header; use self::block_state::CliqueBlockState; use self::params::CliqueParams; @@ -766,10 +766,6 @@ impl Engine for Clique { header_timestamp >= parent_timestamp.saturating_add(self.period) } - fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> super::ForkChoice { - super::total_difficulty_fork_choice(new, current) - } - // Clique uses the author field for voting, the real author is hidden in the `extra_data` field. // So when executing tx's (like in `enact()`) we want to use the executive author fn executive_author(&self, header: &Header) -> Result { diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index 58fbfeed1..125a4edc2 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -16,7 +16,7 @@ use engines::{Engine, Seal, SealingState}; use machine::Machine; -use types::header::{Header, ExtendedHeader}; +use types::header::Header; use block::ExecutedBlock; use error::Error; @@ -87,10 +87,6 @@ impl Engine for InstantSeal { fn is_timestamp_valid(&self, header_timestamp: u64, parent_timestamp: u64) -> bool { header_timestamp >= parent_timestamp } - - fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> super::ForkChoice { - super::total_difficulty_fork_choice(new, current) - } } #[cfg(test)] diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index b5d7e774b..9cad9bdce 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -469,9 +469,6 @@ pub trait Engine: Sync + Send { Vec::new() } - /// Check whether the given new block is the best block, after finalization check. - fn fork_choice(&self, new: &ExtendedHeader, best: &ExtendedHeader) -> ForkChoice; - /// Returns author should used when executing tx's for this block. fn executive_author(&self, header: &Header) -> Result { Ok(*header.author()) @@ -550,15 +547,6 @@ pub trait Engine: Sync + Send { } } -/// Check whether a given block is the best block based on the default total difficulty rule. -pub fn total_difficulty_fork_choice(new: &ExtendedHeader, best: &ExtendedHeader) -> ForkChoice { - if new.total_score() > best.total_score() { - ForkChoice::New - } else { - ForkChoice::Old - } -} - /// Verifier for all blocks within an epoch with self-contained state. pub trait EpochVerifier: Send + Sync { /// Lightly verify the next block header. diff --git a/ethcore/src/engines/null_engine.rs b/ethcore/src/engines/null_engine.rs index 71cb9d459..2d81e4400 100644 --- a/ethcore/src/engines/null_engine.rs +++ b/ethcore/src/engines/null_engine.rs @@ -19,7 +19,7 @@ use engines::block_reward::{self, RewardKind}; use ethereum_types::U256; use machine::Machine; use types::BlockNumber; -use types::header::{Header, ExtendedHeader}; +use types::header::Header; use block::ExecutedBlock; use error::Error; @@ -101,8 +101,4 @@ impl Engine for NullEngine { fn snapshot_components(&self) -> Option> { Some(Box::new(::snapshot::PowSnapshot::new(10000, 10000))) } - - fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> super::ForkChoice { - super::total_difficulty_fork_choice(new, current) - } } diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index a6540ab79..91964a11e 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -23,7 +23,7 @@ use ethereum_types::{H256, H64, U256}; use ethjson; use hash::{KECCAK_EMPTY_LIST_RLP}; use rlp::Rlp; -use types::header::{Header, ExtendedHeader}; +use types::header::Header; use types::BlockNumber; use unexpected::{OutOfBounds, Mismatch}; @@ -380,10 +380,6 @@ impl Engine for Arc { fn snapshot_components(&self) -> Option> { Some(Box::new(::snapshot::PowSnapshot::new(SNAPSHOT_BLOCKS, MAX_SNAPSHOT_BLOCKS))) } - - fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> engines::ForkChoice { - engines::total_difficulty_fork_choice(new, current) - } } impl Ethash {