Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

removed redundant fork choice abstraction #10849

Merged
merged 1 commit into from
Jul 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -501,32 +501,24 @@ impl Importer {
let traces = block.traces.drain();
let best_hash = chain.best_block_hash();

let new = ExtendedHeader {
header: header.clone(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant 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");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant database query and block header decoding

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
Expand Down
4 changes: 0 additions & 4 deletions ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item=ExtendedHeader>) -> Vec<AncestryAction> {
let finalized = self.build_finality(
header,
Expand Down
6 changes: 1 addition & 5 deletions ethcore/src/engines/basic_authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -208,10 +208,6 @@ impl Engine for BasicAuthority {
fn snapshot_components(&self) -> Option<Box<dyn (::snapshot::SnapshotComponents)>> {
None
}

fn fork_choice(&self, new: &ExtendedHeader, current: &ExtendedHeader) -> super::ForkChoice {
super::total_difficulty_fork_choice(new, current)
}
}

#[cfg(test)]
Expand Down
6 changes: 1 addition & 5 deletions ethcore/src/engines/clique/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Address, Error> {
Expand Down
6 changes: 1 addition & 5 deletions ethcore/src/engines/instant_seal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)]
Expand Down
12 changes: 0 additions & 12 deletions ethcore/src/engines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Address, Error> {
Ok(*header.author())
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 1 addition & 5 deletions ethcore/src/engines/null_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -101,8 +101,4 @@ impl Engine for NullEngine {
fn snapshot_components(&self) -> Option<Box<dyn (::snapshot::SnapshotComponents)>> {
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)
}
}
6 changes: 1 addition & 5 deletions ethcore/src/ethereum/ethash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -380,10 +380,6 @@ impl Engine for Arc<Ethash> {
fn snapshot_components(&self) -> Option<Box<dyn (::snapshot::SnapshotComponents)>> {
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 {
Expand Down