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

Fix bugfix hard fork logic #9138

Merged
merged 3 commits into from
Jul 19, 2018
Merged

Fix bugfix hard fork logic #9138

merged 3 commits into from
Jul 19, 2018

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Jul 17, 2018

A bugfix hard fork is something that should always be on by default. When checking, it should use || instead of &&. In addition, EIP86, EIP98, etc do not belong to this category.

This PR also changes the function to return a string instead, so it can tell the operator what needs to be changed in the chain spec config.

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jul 17, 2018
@sorpaas sorpaas added this to the 2.1 milestone Jul 17, 2018
EIP-168 is not enabled by default
@5chdn
Copy link
Contributor

5chdn commented Jul 17, 2018

does this require a backport?

@sorpaas
Copy link
Collaborator Author

sorpaas commented Jul 17, 2018

Given this is a bugfix for bugfix hard fork and the change is simple, backporting it would be fine. :)

Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

I had a look at all the EIPs that you removed from the "bugfix" list and it lgtm, but I'd appreciate that someone with more knowledge about them also take a look.

@5chdn 5chdn modified the milestones: 2.1, 2.2 Jul 17, 2018
self.eip211_transition != 0 && self.eip214_transition != 0 &&
self.validate_chain_id_transition != 0 && self.dust_protection_transition != 0
/// Return Some if the current parameters contain a bugfix hard fork not on block 0.
pub fn nonzero_bugfix_hard_fork(&self) -> Option<&'static str> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

static lifetime annotation is not required anymore so Option<&str> should be enough!

Copy link
Collaborator

@debris debris left a comment

Choose a reason for hiding this comment

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

I also had to double-check everything, but it LGTM

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 19, 2018
@debris debris merged commit b914912 into master Jul 19, 2018
@debris debris deleted the sp-bugfix-hard-fork branch July 19, 2018 14:43
5chdn pushed a commit that referenced this pull request Jul 24, 2018
* Fix bugfix hard fork logic

* Remove dustProtectionTransition from bugfix category

EIP-168 is not enabled by default

* Remove unnecessary 'static
5chdn pushed a commit that referenced this pull request Jul 24, 2018
* Fix bugfix hard fork logic

* Remove dustProtectionTransition from bugfix category

EIP-168 is not enabled by default

* Remove unnecessary 'static
5chdn added a commit that referenced this pull request Jul 25, 2018
* parity-version: bump stable to 1.11.8

* ci: update version strings for snaps

* Be more graceful on Aura difficulty validation (#9164)

* Be more graceful on Aura difficulty validation

* test: rejects_step_backwards

* test: proposer_switching

* test: rejects_future_block

* test: reports_skipped

* test: verify_empty_seal_steps

* parity: fix UserDefaults json parser (#9189)

* parity: fix UserDefaults json parser

* parity: use serde_derive for UserDefaults

* parity: support deserialization of old UserDefault json format

* parity: make UserDefaults serde backwards compatible

* parity: tabify indentation in UserDefaults

* Fix bugfix hard fork logic (#9138)

* Fix bugfix hard fork logic

* Remove dustProtectionTransition from bugfix category

EIP-168 is not enabled by default

* Remove unnecessary 'static

* Disable per-sender limit for local transactions. (#9148)

* Disable per-sender limit for local transactions.

* Add a missing new line.

* rpc: fix is_major_importing sync state condition (#9112)

* rpc: fix is_major_importing sync state condition

* rpc: fix informant printout when waiting for peers

* fix verification in ethcore-sync collect_blocks (#9135)

* docker: update hub dockerfile (#9173)

* update Dockerfile for hub

update to Ubuntu Xenial 16.04
fix cmake version

* docker: fix tab indentation in hub dockerfile

* ethcore: update to parity-wasm 0.31

* rpc: fix broken merge
5chdn added a commit that referenced this pull request Jul 26, 2018
* parity-version: bump beta to 2.0.1

* ci: update version strings for snaps

* Be more graceful on Aura difficulty validation (#9164)

* Be more graceful on Aura difficulty validation

* test: rejects_step_backwards

* test: proposer_switching

* test: rejects_future_block

* test: reports_skipped

* test: verify_empty_seal_steps

* Remove node-health (#9119)

* Remove node-health

* Remove ntp_servers

* Add --ntp-servers as legacy instead of removing it

* Add --ntp-servers to deprecated args

* Remove unused stuff

* Remove _legacy_ntp_servers

* parity: fix UserDefaults json parser (#9189)

* parity: fix UserDefaults json parser

* parity: use serde_derive for UserDefaults

* parity: support deserialization of old UserDefault json format

* parity: make UserDefaults serde backwards compatible

* parity: tabify indentation in UserDefaults

* Fix bugfix hard fork logic (#9138)

* Fix bugfix hard fork logic

* Remove dustProtectionTransition from bugfix category

EIP-168 is not enabled by default

* Remove unnecessary 'static

* Disable per-sender limit for local transactions. (#9148)

* Disable per-sender limit for local transactions.

* Add a missing new line.

* rpc: fix is_major_importing sync state condition (#9112)

* rpc: fix is_major_importing sync state condition

* rpc: fix informant printout when waiting for peers

* fix verification in ethcore-sync collect_blocks (#9135)

* docker: update hub dockerfile (#9173)

* update Dockerfile for hub

update to Ubuntu Xenial 16.04
fix cmake version

* docker: fix tab indentation in hub dockerfile

* rpc: fix broken merge

* rcp: remove node_health leftover from merge

* rpc: remove dapps leftover from merge
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.

5 participants