Make usage of delegate list BFT compliant - Closes #3681 #4397
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
This reverts commit c23651f.
yatki
reviewed
Oct 16, 2019
Invert params on Dpos methods to make use of default value of delegateListRoundOffset
lsilvs
force-pushed
the
3681-failed_verify_slot_after_101
branch
from
fcc62f5
to
7dd644a
Compare
… into 3681-failed_verify_slot_after_101
lsilvs
force-pushed
the
3681-failed_verify_slot_after_101
branch
from
f23d160
to
a217e92
Compare
shuse2
approved these changes
Oct 18, 2019
yatki
suggested changes
Oct 18, 2019
| async getForgerPublicKeysForRound(round) { | ||
| async getForgerPublicKeysForRound(round, delegateListRoundOffset) { | ||
| // Delegate list is generated from round 1 hence `roundWithOffset` can't be less than 1 | ||
| const roundWithOffset = Math.max(round - delegateListRoundOffset, 1); |
There was a problem hiding this comment.
Do you think adding a check against negative offset values make sense? For instance, if round: 6 and I provide offset: -2 I can get the list for round 8. Just saying..
There was a problem hiding this comment.
That's a good point but I would prefer doing this in another ticket to avoid more changes and tests broken in this PR 😬
yatki
approved these changes
Oct 18, 2019
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.
What was the problem?
List of delegates used to verify block forger was different from the on used to forge.
The delegate list round offset was not being used to calculate delegates who missed the slot in order to update
missedBlocksproperty.How did I solve it?
delegateListRoundOffsetto apply/undo/getForgerPublicKeysForRound.DELEGATE_LIST_ROUND_OFFSETto set thedelegateListRoundOffsetdefault value for Dpos moduleHow to manually test it?
Review checklist