Skip to content
This repository has been archived by the owner on Jul 18, 2018. It is now read-only.

[MASTER-RELEASE] Metro-Byzantium changes #6

Merged
merged 5 commits into from
Sep 25, 2017
Merged

Conversation

jwasinger
Copy link

@jwasinger jwasinger commented Sep 11, 2017

EIP 649: block reward reduction

@holgerd77
Copy link
Member

holgerd77 commented Sep 22, 2017

@jwasinger ommerReward and niblingReward are constants actively being used in the current wip/metropolis branch: https://github.com/jwasinger/ethereumjs-vm/blob/wip/metropolis/lib/runBlock.js#L10 this will break the whole test setup, not sure what drove you to this decision? I will revert this for being able to work today.

This is generally not a good idea to just remove existing constants from a shared library others might depend on. These constants are not really hurting or being outdated, or are they?

@jwasinger
Copy link
Author

My reasoning for that was that the ommer reward and nibbler reward can be derived from the Block reward. Thus, it does not make sense to have constants for them. I removed these in https://github.com/jwasinger/ethereumjs#wip/metropolis

@holgerd77 holgerd77 changed the title [WIP] Metro-Byzantium changes [MASTER-RELEASE] Metro-Byzantium changes Sep 25, 2017
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.

Everything looks good to me here, have also updated/completed the CHANGELOG (for "Unreleased (Master)").

@cdetrio
Copy link

cdetrio commented Sep 25, 2017

Can we remove ommerReward and niblingReward without breaking ethereumjs-vm/block now? They are indeed outdated after the change to 3 ETH block reward.

@holgerd77
Copy link
Member

@cdetrio yes, we can, had removed the includes after the change rollback here.

@cdetrio cdetrio merged commit 59d0399 into master Sep 25, 2017
@jwasinger jwasinger deleted the block-reward-reduction branch September 25, 2017 18:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants