-
Notifications
You must be signed in to change notification settings - Fork 112
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
Prevent inappropriate byes being awarded to submissions during dispute #810
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
area
force-pushed
the
fix/mining-dispute-byes
branch
2 times, most recently
from
February 15, 2020 12:37
8122958
to
eb93fa1
Compare
area
changed the title
Prevent inappropriate byes being awarded to submissions during dsipute
Prevent inappropriate byes being awarded to submissions during dispute
Feb 19, 2020
area
force-pushed
the
fix/mining-dispute-byes
branch
2 times, most recently
from
February 24, 2020 11:31
3561fa5
to
420db8f
Compare
area
force-pushed
the
fix/mining-dispute-byes
branch
from
February 27, 2020 14:39
2fdd300
to
b0eb14c
Compare
kronosapiens
approved these changes
Feb 27, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Byes were able to be awarded too liberally in the current codebase. At the very least, byes should never have been able to be claimed if the submission window was still open (because more submissions might be made). In addition, byes should never be able to be awarded if there are any submissions from any previous rounds still not eliminated.
I took several cracks at this before settling on this solution, but I'm still not happy with it. The tricky bit is preventing the award of a bye if there are any submissions from previous rounds not eliminated yet. We can't directly track if a round is complete with a flag, because multiple rounds may silently complete if the window closes and all existing disputes have completed.
A loop through all previous rounds is the natural way of solving this, but loops are bad. I've allowed myself to be a little clever and short-circuit it a bit so that if there are a series of disputes where multiple byes should be awarded (as is the case if there are e.g. 11 submissions and none time out). In that scenario, only the previous rounds that weren't complete before are examined. While the unbounded loop should be fine with our current configuration (because the maximum number of rounds should be capped at 14, based on the block gas limit and the current timeout), I've added this extra element in in an attempt to harden the approach if the parameters change in the future.
Also fixed a bug in
accommodateChallenge...
with the return frommod(2)
- at some point, it was a JS number and is now not? This was noted as a breaking change for v5 of bn.js but I can't figure out how (if?) we were exposed to that...