Skip to content

avoid div by zero in extreme balance case#1664

Merged
djrtwo merged 2 commits into
v011xfrom
div-zero-total-balance
Mar 19, 2020
Merged

avoid div by zero in extreme balance case#1664
djrtwo merged 2 commits into
v011xfrom
div-zero-total-balance

Conversation

@djrtwo

@djrtwo djrtwo commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

Address #1663

  • get_total_balance returns a minimum of EFFECTIVE_BALANCE_INCREMENT to avoid div by zero in extreme cases
  • add rewards test vector to test for this extreme case (it did in fact div by 0 before the fix was in place)
  • Add another rewards test for empty attestations (because I noticed it was needed... )

@djrtwo djrtwo requested a review from protolambda March 16, 2020 17:25
@djrtwo djrtwo changed the base branch from dev to v011x March 16, 2020 17:26
@djrtwo djrtwo mentioned this pull request Mar 16, 2020
Closed
@djrtwo djrtwo requested a review from hwwhww March 17, 2020 16:13
hwwhww
hwwhww approved these changes Mar 18, 2020
View reviewed changes

@hwwhww hwwhww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Nice catch @MrChico!

Comment thread tests/core/pyspec/eth2spec/test/context.py Outdated
Comment thread tests/core/pyspec/eth2spec/test/context.py Outdated
Comment thread specs/phase0/beacon-chain.md
@djrtwo

djrtwo commented Mar 18, 2020

Copy link
Copy Markdown
Contributor Author

Thanks @hwwhww!
Made the suggested changes. Merging after tests run

@djrtwo djrtwo merged commit 2b8c32a into v011x Mar 19, 2020
@djrtwo djrtwo deleted the div-zero-total-balance branch March 19, 2020 14:50
@terencechain terencechain mentioned this pull request Mar 27, 2020
Closed
9 tasks
This was referenced Apr 10, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hwwhww hwwhww hwwhww approved these changes

@protolambda protolambda Awaiting requested review from protolambda

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@djrtwo @hwwhww