8000 remove old fields `consensus` and `finality` by ryanio · Pull Request #120 · ethereumjs/ethereumjs-client · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Dec 10, 2020. It is now read-only.

Conversation

@ryanio
Copy link
Contributor
@ryanio ryanio commented Jun 2, 2020

This PR follows ethereumjs/ethereumjs-monorepo#758 to remove unused fields consensus and finality.

@ryanio ryanio force-pushed the remove-old-fields branch from 421f3a9 to 04be72a Compare June 2, 2020 23:30
@coveralls
Copy link
coveralls commented Jun 2, 2020

Coverage Status

Coverage increased (+0.2%) to 92.711% when pulling 04be72a on remove-old-fields into 3af85bc on master.

Copy link
Member
@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Looks good, good catch, didn't know this was consumed here

params.hardforks = hardforks.map(name => ({
name: name,
block: name === 'chainstart' ? 0 : json.config[forkMap[name]] || null,
consensus: json.config.clique ? 'poa' : 'pow',
Copy link
Member

Choose a reason for hiding this comment

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

Was a bit unsure about the consensus part, since the PoA information is directly extracted from a geth configuration file, but the information is nowhere used within the client so I think it should be safe to remove. Can also be re-added later if necessary, eventually in another context then.

@holgerd77 holgerd77 merged commit f15259a into master Jun 3, 2020
@holgerd77 holgerd77 deleted the remove-old-fields branch June 3, 2020 11:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0