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

Generalized blockReward and difficultyBombDelays config #9480

Merged
merged 4 commits into from
Sep 8, 2018

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Sep 5, 2018

Replaces #9187. Rel #8427

Make blockReward config in ethash to accept multi, and add difficultyBombDelays config. Removes EIP649 configurations.

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 5, 2018
@sorpaas sorpaas added this to the 2.1 milestone Sep 5, 2018
@5chdn
Copy link
Contributor

5chdn commented Sep 6, 2018

Do we want to backport this to 2.0 to have Constantinople available on the future stable branch?

Edit, actually, I mark this blocker because it's required for the hard fork

@5chdn 5chdn added B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. labels Sep 6, 2018
@NikVolf
Copy link
Contributor

NikVolf commented Sep 6, 2018

Can you link all relevant information in the description so that it can be navigated from the PR?

Copy link
Contributor

@cheme cheme left a comment

Choose a reason for hiding this comment

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

LGTM. Also, I think it is important to document (in release note and wiki) the fact that difficulty bomb delays are cumulative (I would have make the mistake of thinking it is not). Do :

"difficultyBombDelays": {
  "0xC3500": 3000000,
  "constant_bl": 2000000
},

and not

"difficultyBombDelays": {
  "0xC3500": 3000000,
  "constant_bl": 5000000
},

(constant_bl being constantinople start block value).

@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 8, 2018
"eip100bTransition": "0x0",
"eip649Transition": "0x0"
"difficultyBombDelays": {
"0": 3000000
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add the 1234 portion here for constantinople?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this constantinople_test is totally unsynch (see the name), currently it's basically the byzantium one. I could indeed be a good idea to get it ready for actual constantinople (if all pr are merged).

@5chdn 5chdn merged commit e1f3330 into master Sep 8, 2018
@5chdn 5chdn deleted the sp-reward-difficulty branch September 8, 2018 22:38
ChainSafeSystems pushed a commit to goerli/parity-goerli that referenced this pull request Sep 9, 2018
* rpc(debug_getBadBlocks): fix test (openethereum#9502)

* Generalized blockReward and difficultyBombDelays config (openethereum#9480)

* Implement multi blockReward

* Implement difficultyBombDelays

* Fix json crate compile

* json keys can only be string

* parity: print correct keys path on startup (openethereum#9501)
@Tbaut
Copy link
Contributor

Tbaut commented Sep 9, 2018

@sorpaas Can you add this to the wiki please? I believe it should be in https://github.com/paritytech/wiki/blob/master/Chain-specification.md

@holiman
Copy link
Contributor

holiman commented Sep 10, 2018

On hive, parity went from 72 to 5113 errors at this commit. Example error message on a block that suddenly is not accepted any longer: http://hivetests.ethstats.net/#displayLog=artefacts/20180909_033154-parity%3Amaster/client-d46f7870.log

Please send me a ping or mention when changes are made to the config/genesis spec, since several tools needs to be updated; such as hive, evmlab and puppeth.

holiman added a commit to ethereum/hive that referenced this pull request Sep 10, 2018
holiman added a commit to ethereum/hive that referenced this pull request Sep 10, 2018
@sorpaas
Copy link
Collaborator Author

sorpaas commented Sep 10, 2018

Really sorry @holiman. Will make sure to ping you next time.

@holiman
Copy link
Contributor

holiman commented Sep 10, 2018

Yeah no hard feelings, just a reminder :)

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. B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants