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.
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
Refactor bootstrapper implementation into consensus #2300
Refactor bootstrapper implementation into consensus #2300
Changes from all commits
c0cfff8
c1c84a6
b0db181
940b195
d13664d
0ba0384
c887265
fd41c5a
a629c00
34852b6
881055d
5b9ae56
38bff63
c8154cb
bffbb50
f990fa1
1c8d8f0
a9b96c8
02caaa8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should also consider adding
bool
to the return signature here so instead of calling bothRecordOpinion
andResult
to figure out if we need to keep poling peers we can just callRecordOpinion
in a loop. This way we can also move all this code intoResult
, and get rid of them.accepted
field.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left an offline comment on slack but we can also consider supplying a filter function
func(map[ids.ID]uint64) bool
thatResult
can use to filter out any valid blocks which will get rid of the duplicated code inMinority
and thePoll
interface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need
Result
on the interface for theMinority
implementation to be usable because the minority result is used repeatedly to send the majority requests. I'm sure that we could combine the implementations somehow... But I feel like currently there really isn't much duplicated code... The structure of the code may look similar but the logic is pretty different