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

Engine block ordering #3719

Merged
merged 3 commits into from
Dec 5, 2016
Merged

Engine block ordering #3719

merged 3 commits into from
Dec 5, 2016

Conversation

keorn
Copy link

@keorn keorn commented Dec 5, 2016

New method on Engine that lets Blockchain know about the ordering. #3489

@keorn keorn added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Dec 5, 2016
@@ -325,6 +327,15 @@ impl Engine for Ethash {
fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
t.sender().map(|_|()) // Perform EC recovery and cache sender
}

fn is_new_best_block(&self, best_total_difficulty: U256, _best_header: HeaderView, parent_details: &BlockDetails, new_header: &HeaderView) -> bool {
is_new_best_block(best_total_difficulty, parent_details, new_header)
Copy link
Author

Choose a reason for hiding this comment

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

The same as default, but kept to be explicit.

@rphmeier
Copy link
Contributor

rphmeier commented Dec 5, 2016

nice! I wonder if in the future it'll be feasible to extend this abstraction even to stuff like difficulty which is only relevant to PoW. This would require a strong database abstraction and to extend Engine into something which specifies block layout beyond just the seal.

@keorn
Copy link
Author

keorn commented Dec 5, 2016

Yep, that would be nice (and will probably need to be done). Currently all additional info has to be stuffed into the seal.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 53b479f on engine-block-ordering into ** on master**.

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 5, 2016
@gavofyork gavofyork merged commit ad36743 into master Dec 5, 2016
@gavofyork gavofyork deleted the engine-block-ordering branch December 5, 2016 18:37
@gavofyork
Copy link
Contributor

this still compiles with IPC enabled, right?

@keorn
Copy link
Author

keorn commented Dec 5, 2016

Ah, you are right snapshot_service_trait does not compile. Not too familiar with the IPC setup.

@keorn
Copy link
Author

keorn commented Dec 5, 2016

Actually it was broken before this PR.

@rphmeier
Copy link
Contributor

rphmeier commented Dec 5, 2016

I broke it earlier with #2935 and also the list_storage stuff broke it as well

Preparing workarounds. The codegen implementation could really use some love...

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.

None yet

4 participants