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
Merged

Generalized blockReward and difficultyBombDelays config #9480

merged 4 commits into from Sep 8, 2018

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Sep 5, 2018 •
edited

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 •
edited

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?

cheme
cheme approved these changes Sep 7, 2018
View reviewed changes
Copy link
Contributor

@cheme cheme left a comment •
edited

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
5chdn
5chdn reviewed Sep 8, 2018
View reviewed changes
ethcore/res/ethereum/constantinople_test.json
"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).

This was referenced Sep 8, 2018
Closed
Closed
@5chdn 5chdn merged commit e1f3330 into master Sep 8, 2018
@5chdn 5chdn deleted the sp-reward-difficulty branch September 8, 2018 22:38
@5chdn 5chdn mentioned this pull request Sep 9, 2018
Merged
ChainSafeSystems pushed a commit to goerli/parity-goerli that referenced this pull request Sep 9, 2018
@5chdn @ChainSafeSystems
backports from upstream (#3)
5db7277 
* 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

@sorpaas sorpaas mentioned this pull request Sep 10, 2018
Merged
@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
Handle openethereum/parity-ethereum#9480, updates to parity config
dce0089
@holiman holiman mentioned this pull request Sep 10, 2018
Merged
holiman added a commit to ethereum/hive that referenced this pull request Sep 10, 2018
@holiman
Handle openethereum/parity-ethereum#9480, updates to parity config (#123
4ab3863 
)
@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 :)

@5chdn 5chdn mentioned this pull request Sep 12, 2018
Merged
@andresilva andresilva mentioned this pull request Oct 9, 2018
Merged
16 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cheme cheme cheme approved these changes

@5chdn 5chdn 5chdn left review comments

@NikVolf NikVolf NikVolf approved these changes

Assignees
No one assigned
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
Milestone
2.1 Whatever
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@sorpaas @5chdn @NikVolf @Tbaut @holiman @cheme