Refine SubList and SubListWithProposer #551
Conversation
aidan-kwon
requested review from
ehnuje,
jeongkyun-oh,
KimKyungup,
kjhman21 and
joowon-byun
as code owners
|
The last commit revoked changes in the previous commit restricting committee size. |
| if committeeSize >= validatorSize { | ||
| return validators | ||
| } |
There was a problem hiding this comment.
how about moving these codes to top of the function.
we can just return if the commiteeSize is bigger than validatorSize
There was a problem hiding this comment.
Thank you. I moved it!
| // seed will be used to select a random committee | ||
| seed, err := ConvertHashToSeed(prevHash) | ||
| if err != nil { | ||
| logger.Error("failed to covert hash to seed", "prevHash", prevHash, "err", err) |
There was a problem hiding this comment.
| logger.Error("failed to covert hash to seed", "prevHash", prevHash, "err", err) | |
| logger.Error("failed to convert hash to seed", "prevHash", prevHash, "err", err) |
|
Reviewers, please take a look. |
| // SelectRandomCommittee composes a committee selecting validators randomly based on the seed value. | ||
| // It returns nil if the given committeeSize is bigger than validatorSize or proposer indexes are invalid. | ||
| func SelectRandomCommittee(validators []istanbul.Validator, committeeSize uint64, seed int64, proposerIdx int, nextProposerIdx int) []istanbul.Validator { | ||
| // ensure validator indexes are valid |
There was a problem hiding this comment.
| // ensure validator indexes are valid | |
| // ensure proposer indexes are valid |
There was a problem hiding this comment.
I think "validator" is good to go because the index is indexes of validators variable
| func (valSet *weightedCouncil) SetSubGroupSize(size uint64) { | ||
| if size == 0 { | ||
| logger.Error("cannot assign committee size to 0") |
There was a problem hiding this comment.
Crit?
| logger.Error("cannot assign committee size to 0") | |
| logger.Crit("cannot assign committee size to 0") |
There was a problem hiding this comment.
I think error is enough. It doesn't change valSet.subSize value when size is 0.
|
@ehnuje PTAL. :) |
Proposed changes
Refine
SubListandSubListWithProposer.ConvertHashToSeed,SelectRandomCommitteecommitteeSizeinSubListWithProposerSetSubGroupSizeI tried to maintain the semantic of the
SubListWithProposerexcept for invalid input handling.Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test)