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

Generalize engine trait #6591

Merged
merged 38 commits into from
Sep 26, 2017
Merged

Generalize engine trait #6591

merged 38 commits into from
Sep 26, 2017

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Sep 25, 2017

This introduces the parity-machine crate, which defines traits for blockchain headers and live blocks.

Consensus engines are intended to be generic over different kinds of state machines which fit the requirements they need. For example, ethash could potentially be run over any block chain type with a difficulty and balances (for block and uncle reward).

Verification and block population logic which was fragmented among engines has now been unified in two places: ethcore/machine.rs and ethcore/verification/verification.rs. This eliminates a common source of bugs stemming from protocol upgrades being implemented for some engines but not others.

state, executive, and externalities are now fully decoupled from consensus logic.

ValidatorSet based engines haven't been generalized yet, and currently are implemented only for the EthereumMachine. In a future PR, the ValidatorSet framework will be generalized to anything which supports a generalized notion of smart contracts and "signals" (implemented via ethabi and log events in Ethereum).

The snapshot module can is also a prime candidate for refactor. Currently there are two kinds of chunks: "block chunks" and "corroboration chunks". Corroboration chunks vary from engine to engine, and now we can make "block chunks" vary from machine to machine. This will give us secure warp sync with minimal effort for any new blockchains we create.

rphmeier added 30 commits July 24, 2017 14:00
Including metropolis updates
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 25, 2017
@@ -5,6 +5,7 @@
"authorityRound": {
"params": {
"stepDuration": "4",
"blockReward": "0x4563918244F40000",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indentation

fn hash(&self) -> H256;

/// Get a reference to the seal fields.
fn seal(&self) -> &[Vec<u8>];
Copy link
Collaborator

@arkpar arkpar Sep 25, 2017

Choose a reason for hiding this comment

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

Spaces are used instead of tabs on a few lines in this file

fn builtins(&self) -> &BTreeMap<Address, Builtin> {
&self.builtins
}
fn on_close_block(&self, block: &mut M::LiveBlock) -> Result<(), M::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use ::engines::common::bestow_block_reward here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since it handles uncle reward as well and there is a test that depends on that behavior

@arkpar
Copy link
Collaborator

arkpar commented Sep 26, 2017

Looks good in general, mostly just code moved around.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 26, 2017
@arkpar
Copy link
Collaborator

arkpar commented Sep 26, 2017

Could use a doc describing migration of existing spec files though. From 1.6 and 1.7

@gavofyork
Copy link
Contributor

@rphmeier ^^^

@gavofyork gavofyork merged commit bc167a2 into master Sep 26, 2017
@gavofyork gavofyork deleted the general-engine branch September 26, 2017 12:19
@5chdn
Copy link
Contributor

5chdn commented Sep 27, 2017

Martin Holst Swende @holiman 13:55 (Gitter)

It seems that #6591 blew the existing chain specifications out of the water; I'm getting quite a lot of errors when running parity on hive now. Is there some documentation about what's changed ?

I can help with docs if I get some pointers @rphmeier .

@bjornwgnr
Copy link
Collaborator

Are there any docs yet?

@5chdn
Copy link
Contributor

5chdn commented Oct 12, 2017

No but I can help drafting something if anyone asks.

@rphmeier
Copy link
Contributor Author

Move registrar, gasLimitBoundDivisor, and eip155Transition from engine (Ethash/AuthorityRound/etc.) to general params section.

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. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants