-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: implement random sampling without replacement and staking power #83
Changes from 19 commits
fbc390c
9b8d758
d68124d
3749b65
521cdc1
5a4cf87
85a08e1
c99b7bd
41fe4c2
0f2d198
37a2431
52de880
b2247ba
8e74da4
8eda4c1
b85eedb
fcd158b
af8ea82
aac3c8b
9e8dc55
d5cf9b8
cbe2847
45e573e
cb7f045
a3835cb
b1f68f5
d930617
083f637
fdb7a02
e33896f
0a18969
f08edeb
08b7835
deb89eb
d30feec
06e84e8
4387cb6
94b2975
3efc1d7
1c759e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,13 @@ import ( | |
type Candidate interface { | ||
Priority() uint64 | ||
LessThan(other Candidate) bool | ||
IncreaseWin() | ||
SetWinPoint(winPoint uint64) | ||
} | ||
|
||
const uint64Mask = uint64(0x7FFFFFFFFFFFFFFF) | ||
const ( | ||
// Casting a larger number than this as float64 can result in that lower bytes can be truncated | ||
MaxFloat64Significant = uint64(0x1FFFFFFFFFFFFF) | ||
) | ||
|
||
// Select a specified number of candidates randomly from the candidate set based on each priority. This function is | ||
// deterministic and will produce the same result for the same input. | ||
|
@@ -33,7 +36,7 @@ func RandomSamplingWithPriority( | |
thresholds := make([]uint64, sampleSize) | ||
for i := 0; i < sampleSize; i++ { | ||
// calculating [gross weights] × [(0,1] random number] | ||
thresholds[i] = uint64(float64(nextRandom(&seed)&uint64Mask) / float64(uint64Mask+1) * float64(totalPriority)) | ||
thresholds[i] = randomThreshold(&seed, totalPriority) | ||
} | ||
s.Slice(thresholds, func(i, j int) bool { return thresholds[i] < thresholds[j] }) | ||
|
||
|
@@ -66,53 +69,86 @@ func RandomSamplingWithPriority( | |
totalPriority, actualTotalPriority, seed, sampleSize, undrawn, undrawn, thresholds[undrawn], len(candidates))) | ||
} | ||
|
||
const MaxSamplingLoopTry = 1000 | ||
func moveWinnerToLast(candidates []Candidate, winner int) { | ||
winnerCandidate := candidates[winner] | ||
copy(candidates[winner:], candidates[winner+1:]) | ||
candidates[len(candidates)-1] = winnerCandidate | ||
} | ||
|
||
func randomThreshold(seed *uint64, total uint64) uint64 { | ||
return uint64(float64(nextRandom(seed)&MaxFloat64Significant) / | ||
float64(MaxFloat64Significant+1) * float64(total)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think // (rand(seed) % Max) * total / (Max+1)
a := big.NewInt(nextRandom(seed) & MaxBits)
a.Mul(a, total)
a.Div(a, MaxBits + 1)
a.Uint64() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I'll apply it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you make sure that the idempotency with staking is not broken? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Okay. |
||
} | ||
|
||
// `RandomSamplingWithoutReplacement` elects winners among candidates without replacement | ||
// so it updates rewards of winners. This function continues to elect winners until the both of two | ||
// conditions(minSamplingCount, minPriorityPercent) are met. | ||
func RandomSamplingWithoutReplacement( | ||
seed uint64, candidates []Candidate, minSamplingCount int, minPriorityPercent uint, winPointUnit uint64) ( | ||
winners []Candidate) { | ||
zemyblue marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// `RandomSamplingToMax` elects voters among candidates so it updates wins of candidates | ||
// Voters can be elected by a maximum `limitCandidates`. | ||
// However, if the likely candidates are less than the `limitCandidates`, | ||
// the number of voters may be less than the `limitCandidates`. | ||
// This is to prevent falling into an infinite loop. | ||
func RandomSamplingToMax( | ||
seed uint64, candidates []Candidate, limitCandidates int, totalPriority uint64) uint64 { | ||
if len(candidates) < minSamplingCount { | ||
panic(fmt.Sprintf("The number of candidates(%d) cannot be less minSamplingCount %d", | ||
len(candidates), minSamplingCount)) | ||
} | ||
|
||
if len(candidates) < limitCandidates { | ||
panic("The number of candidates cannot be less limitCandidate") | ||
if minPriorityPercent > 100 { | ||
panic(fmt.Sprintf("minPriorityPercent must be equal or less than 100: %d", minPriorityPercent)) | ||
} | ||
|
||
totalPriority := sumTotalPriority(candidates) | ||
if totalPriority > MaxFloat64Significant { | ||
// totalPriority will be casting to float64, so it must be less than 0x1FFFFFFFFFFFFF(53bits) | ||
panic(fmt.Sprintf("total priority cannot exceed %d: %d", MaxFloat64Significant, totalPriority)) | ||
} | ||
|
||
priorityThreshold := totalPriority * uint64(minPriorityPercent) / 100 | ||
candidates = sort(candidates) | ||
totalSampling := uint64(0) | ||
winCandidates := make(map[Candidate]bool) | ||
for len(winCandidates) < limitCandidates && totalSampling < MaxSamplingLoopTry { | ||
threshold := uint64(float64(nextRandom(&seed)&uint64Mask) / float64(uint64Mask+1) * float64(totalPriority)) | ||
winnersPriority := uint64(0) | ||
losersPriorities := make([]uint64, len(candidates)) | ||
winnerNum := 0 | ||
for winnerNum < minSamplingCount || winnersPriority < priorityThreshold { | ||
threshold := randomThreshold(&seed, totalPriority-winnersPriority) | ||
cumulativePriority := uint64(0) | ||
found := false | ||
for _, candidate := range candidates { | ||
for i, candidate := range candidates[:len(candidates)-winnerNum] { | ||
if threshold < cumulativePriority+candidate.Priority() { | ||
if !winCandidates[candidate] { | ||
winCandidates[candidate] = true | ||
} | ||
candidate.IncreaseWin() | ||
totalSampling++ | ||
moveWinnerToLast(candidates, i) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Putting the winners after the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if we create a new array for winners and take winners out of the candidates and put them in the winner list, the overrun references problem is the same unless we reduce the capacity of the candidates(we should re-allocate array to reduce the capacity). I don't know why I have to make a new array for winners. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, this is a trivial point, so I think it's also a good idea to reuse the tail in the area of the winner. I'm also slightly concerned that when it |
||
winnersPriority += candidate.Priority() | ||
losersPriorities[winnerNum] = totalPriority - winnersPriority | ||
winnerNum++ | ||
found = true | ||
break | ||
} | ||
cumulativePriority += candidate.Priority() | ||
} | ||
|
||
if !found { | ||
panic(fmt.Sprintf("Cannot find random sample. totalPriority may be wrong: totalPriority=%d, "+ | ||
"actualTotalPriority=%d, threshold=%d", totalPriority, sumTotalPriority(candidates), threshold)) | ||
panic(fmt.Sprintf("Cannot find random sample. winnerNum=%d, minSamplingCount=%d, "+ | ||
"winnersPriority=%d, priorityThreshold=%d, totalPriority=%d, threshold=%d", | ||
winnerNum, minSamplingCount, winnersPriority, priorityThreshold, totalPriority, threshold)) | ||
} | ||
} | ||
return totalSampling | ||
compensationProportions := make([]float64, winnerNum) | ||
for i := winnerNum - 2; i >= 0; i-- { // last winner doesn't get compensation reward | ||
compensationProportions[i] = compensationProportions[i+1] + 1/float64(losersPriorities[i]) | ||
} | ||
winners = candidates[len(candidates)-winnerNum:] | ||
for i, winner := range winners { | ||
winner.SetWinPoint(winPointUnit + | ||
uint64(float64(winner.Priority())*compensationProportions[i]*float64(winPointUnit))) | ||
} | ||
return winners | ||
} | ||
|
||
func sumTotalPriority(candidates []Candidate) (sum uint64) { | ||
for _, candi := range candidates { | ||
if candi.Priority() == 0 { | ||
panic("candidate(%d) priority must not be 0") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like that the boundary condition for the cumulative summation is designed not to select a zero-priority candidate. If an assertion is required, I think it's sufficient to make sure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you said, the restriction that a candidate's priority cannot be zero seems to be bad. However, it seems that the loop escape condition should be added from the sampling algorithm to assume zero.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And I think we should remove validator having 0 staking power from voter set in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added some test cases having 0 staking power. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first comment means that a Candidate with Priority() of 0 would fundamentally not be selected in the above code (in other words, the presence or absence of a candidate with Priority=0 doesn't affect the results). So this is a comment that I think it seems to be enough to verify Priority!=0 only to the winner for |
||
sum += candi.Priority() | ||
} | ||
return | ||
return sum | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay |
||
} | ||
|
||
// SplitMix64 | ||
|
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.
What is this change for?
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.
ToVoterAll()
needs[]*Validator
notValidatorSet
.