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

removed redundant fork choice abstraction #10849

merged 1 commit into from
Jul 9, 2019

Conversation

debris
Copy link
Collaborator

@debris debris commented Jul 5, 2019

fork_choice is always based on total difficulty, so it does not need to be abstracted

@debris debris added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jul 5, 2019
@debris debris requested a review from dvdplm July 5, 2019 16:22
@@ -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

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

@dvdplm dvdplm requested review from gnunicorn and ngotchac July 5, 2019 17:08
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Seems like a good simplification.

@ordian ordian added this to the 2.7 milestone Jul 6, 2019
@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 8, 2019
@sorpaas sorpaas merged commit fdc7b0f into master Jul 9, 2019
@sorpaas sorpaas deleted the fork-choice branch July 9, 2019 02:27
ordian added a commit that referenced this pull request Jul 9, 2019
* master:
  Run cargo fix on a few of the worst offenders (#10854)
  removed redundant fork choice abstraction (#10849)
  Extract state-db from ethcore (#10858)
  Fix fork choice (#10837)
  Move more code into state-account (#10840)
  Remove compiler warning (#10865)
  [ethash] use static_assertions crate (#10860)
dvdplm added a commit that referenced this pull request Jul 9, 2019
* master:
  Run cargo fix on a few of the worst offenders (#10854)
  removed redundant fork choice abstraction (#10849)
  Extract state-db from ethcore (#10858)
  Fix fork choice (#10837)
  Move more code into state-account (#10840)
  Remove compiler warning (#10865)
  [ethash] use static_assertions crate (#10860)
dvdplm added a commit that referenced this pull request Jul 11, 2019
* master:
  Docker images renaming (#10863)
  Move the substate module into ethcore/executive (#10867)
  Run cargo fix on a few of the worst offenders (#10854)
  removed redundant fork choice abstraction (#10849)
  Extract state-db from ethcore (#10858)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants