Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chaingen: implement DCP0001 for generator. #2329

Merged
merged 1 commit into from
Oct 1, 2020

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Aug 14, 2020

This ports the current stake difficulty algorithm (DCP0001) to the generator.

Work towards #2090.

@davecgh davecgh added the test coverage Discussion and pull requests for improving test coverage. label Aug 16, 2020
@davecgh davecgh modified the milestones: 1.7.0, 1.6.0 Aug 17, 2020
Copy link
Member

@rstaudt2 rstaudt2 left a comment

Choose a reason for hiding this comment

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

This looks good to me, I went through all of the updated generator logic and compared it to the actual difficulty logic and it all looks accurate. Left a couple of really minor comments, and also noticed there is an extra space in the commit message.

blockchain/chaingen/generator.go Outdated Show resolved Hide resolved
blockchain/fullblocksstakeversion_test.go Outdated Show resolved Hide resolved
@davecgh davecgh force-pushed the update_gen_stake_diff branch 3 times, most recently from 502a961 to debc4f1 Compare October 1, 2020 19:42
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

So, something that this does which I went back and forth a bit on is that it completely removes the old algorithm in favor of the new one which means the generator can't be used to properly test that aspect of consensus prior to activation of the DCP and testing consensus is one of the major reasons for having the generator and the fullblocktests to begin with.

However, upon closer inspection, none of the tests fail under the new algorithm because they either don't produce enough blocks to hit stake validation height, or, they maintain a steady state of votes, so the difficulty never changes.

Further, given that any alternative implementations will necessarily have to be able to sync mainnet and any issues with the old difficulty calculation code in said implementations will clearly fail if they are not correct, all new blocks are required to use the new algorithm now, and it's not a case of some easily missable corner case, this change seems acceptable.

This ports the current stake difficulty algorithm (DCP0001) to the
generator.
@davecgh
Copy link
Member

davecgh commented Oct 1, 2020

I fixed the commit message as well.

@davecgh davecgh merged commit bf7072f into decred:master Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test coverage Discussion and pull requests for improving test coverage.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants