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

Ethereum Classic Monetary Policy #5741

Merged
merged 5 commits into from
Jun 19, 2017
Merged

Ethereum Classic Monetary Policy #5741

merged 5 commits into from
Jun 19, 2017

Conversation

sjmackenzie
Copy link
Contributor

Create a new parameter ecip1017EraRounds. When the block number
passes one era rounds, the reward is reduced by 20%.

See https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md

@parity-cla-bot
Copy link

It looks like @sjmackenzie hasn'signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, plesae reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@rphmeier rphmeier added A4-clasignoffneeded 📛 Pull request requires author to sign off on CLA before review/merge can begin. M4-core ⛓ Core client code / Rust. labels Jun 1, 2017
@sorpaas @sjmackenzie
Ethereum Classic Monetary Policy
ff36274 
Create a new parameter `ecip1017EraRounds`. When the block number
passes one era rounds, the reward is reduced by 20%.

See https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md
@sjmackenzie
Copy link
Contributor Author

[clabot:check]

@parity-cla-bot
Copy link

It looks like @sjmackenzie signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A4-clasignoffneeded 📛 Pull request requires author to sign off on CLA before review/merge can begin. labels Jun 1, 2017
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 1, 2017
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jun 1, 2017
@sjmackenzie
Copy link
Contributor Author

not ready yet

@rphmeier rphmeier added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 1, 2017
@sorpaas @sjmackenzie
Update rewards for uncle miners for ECIP1017
89849e5 
In the monetary policy, the rewards are changed from "up to 7/8 of the
reward" to "1/32 of the reward".
@sjmackenzie
Copy link
Contributor Author

Okay it's good to go.

whilei
whilei approved these changes Jun 1, 2017
View reviewed changes
ethcore/src/ethereum/ethash.rs Outdated
let fields = block.fields_mut();

let eras = fields.header.number() / self.ethash_params.ecip1017_era_rounds;
for _ in 0..eras {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this make 4/5 * reward also for "Era 0"?

Winner reward should be full (5 eth) for "Era 0", and 4 eth for Era 1; where "Era 0" == 0-5million[or configured], "Era 1" == 5-10million, etc.

My rust is rusty... maybe should be 1..eras?

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 it's OK:

for _ in 0..0 {}

is == for i := 0; i < 0; i++ {}, so it won't be called as long as eras is zero-indexed (which #L294 confirms).

@sorpaas @sjmackenzie
Fix an off-by-one error in ECIP1017 era calculation
f962412 
According to
https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md,
when in block number 5,000,000, it should still be in Era 1 (which in
our code `era == 0`). So we need to check whether the `rem` equals to
zero and act accordingly when calculating the era.
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jun 2, 2017
@gavofyork
Copy link
Contributor

needs to be placed in build bot.

@sjmackenzie
Copy link
Contributor Author

please run build bot again.

@rphmeier
Copy link
Contributor

rphmeier commented Jun 7, 2017 •
edited
Loading

would prefer to have eras logic in a separate function
fn eras_block_reward(era_rounds, initial_reward, block_number) -> new_reward and that unit tests be added for this function's behavior.

CI is undergoing some changes right now, but it should trigger again if you push

@rphmeier rphmeier added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jun 7, 2017
@jesuscript
Copy link
Contributor

what Rob said :) Please push again and it'll run a CI pipeline for you

@sjmackenzie
Copy link
Contributor Author

oh it seems you've got a A5-grumble tag :-), oh dear! Please have a test run now, and sorry for the delay!

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jun 12, 2017
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 19, 2017
@gavofyork gavofyork merged commit d152fa3 into openethereum:master Jun 19, 2017
arkpar pushed a commit that referenced this pull request Jul 14, 2017
@sjmackenzie @arkpar
Ethereum Classic Monetary Policy (#5741)
4518bff 
* Ethereum Classic Monetary Policy

Create a new parameter `ecip1017EraRounds`. When the block number
passes one era rounds, the reward is reduced by 20%.

See https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md

* Update rewards for uncle miners for ECIP1017

In the monetary policy, the rewards are changed from "up to 7/8 of the
reward" to "1/32 of the reward".

* Fix an off-by-one error in ECIP1017 era calculation

According to
https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md,
when in block number 5,000,000, it should still be in Era 1 (which in
our code `era == 0`). So we need to check whether the `rem` equals to
zero and act accordingly when calculating the era.

* `ecip1017_era_rounds` missing from EthashParams when run in build bot

* strip out ecip1017_eras_block_reward function and add unit test
arkpar added a commit that referenced this pull request Jul 14, 2017
@arkpar
More backports into stable (#6061)
aa7dfdf 
* Ethereum Classic Monetary Policy (#5741)

* Ethereum Classic Monetary Policy

Create a new parameter `ecip1017EraRounds`. When the block number
passes one era rounds, the reward is reduced by 20%.

See https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md

* Update rewards for uncle miners for ECIP1017

In the monetary policy, the rewards are changed from "up to 7/8 of the
reward" to "1/32 of the reward".

* Fix an off-by-one error in ECIP1017 era calculation

According to
https://github.com/ethereumproject/ECIPs/blob/master/ECIPs/ECIP-1017.md,
when in block number 5,000,000, it should still be in Era 1 (which in
our code `era == 0`). So we need to check whether the `rem` equals to
zero and act accordingly when calculating the era.

* `ecip1017_era_rounds` missing from EthashParams when run in build bot

* strip out ecip1017_eras_block_reward function and add unit test

* JS precompiled set to stable
@whilei whilei mentioned this pull request Aug 28, 2017
Merged
1 task
@whilei whilei mentioned this pull request Nov 24, 2017
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@whilei whilei whilei approved these changes

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sjmackenzie @parity-cla-bot @gavofyork @rphmeier @jesuscript @whilei @arkpar @sorpaas