Skip to content
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

Merged
merged 40 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
fbc390c
refactor: rename VotingPower to StakingPower
May 25, 2020
9b8d758
fix: separate StakingPower and VotingPower
May 25, 2020
d68124d
feat: implement RandomSamplingWithoutReplacement
May 29, 2020
3749b65
fix: lint error
May 29, 2020
521cdc1
feat: implement assigning voting power
Jun 2, 2020
5a4cf87
fix: merge conflict
Jun 2, 2020
85a08e1
fix: lint error
Jun 2, 2020
c99b7bd
fix: lint error
Jun 2, 2020
41fe4c2
fix: lite2 test failure
Jun 2, 2020
0f2d198
fix: proto generated file
Jun 2, 2020
37a2431
fix: diable proto-checking of circle-ci
Jun 3, 2020
52de880
fix: apply comment; use VotingPower on adding vote
Jun 3, 2020
b2247ba
fix: apply comment; remove totalStakingPower from VoterSet
Jun 3, 2020
8e74da4
fix: apply comment; fix NewVoterSet
Jun 3, 2020
8eda4c1
fix: apply comment; rename validatorSet to voterSet and fix compile e…
Jun 3, 2020
b85eedb
fix: apply comment; use VotingPower on consensus
Jun 3, 2020
fcd158b
fix: lint error
Jun 3, 2020
af8ea82
fix: lint error
Jun 3, 2020
aac3c8b
fix: lite test compile error
Jun 3, 2020
9e8dc55
fix: remove unused function
Jun 3, 2020
d5cf9b8
fix: modify validator to voter in comments
Jun 4, 2020
cbe2847
fix: total voting power overflow
Jun 5, 2020
45e573e
fix: update total voting power if 0
Jun 5, 2020
cb7f045
docs: change log
Jun 8, 2020
a3835cb
fix: apply comments
Jun 11, 2020
b1f68f5
fix: lint error
Jun 11, 2020
d930617
Merge branch 'develop' of https://github.com/line/tendermint into fea…
Jun 11, 2020
083f637
fix: rewrite randomThreshold; remove priorityRateThreshold; some test…
Jun 11, 2020
fdb7a02
fix: lint error
Jun 11, 2020
e33896f
test: add test for randomThreshold
Jun 12, 2020
0a18969
test: add testing for verifying idempotence of randomThreshold
Jun 12, 2020
f08edeb
fix: lint error
Jun 12, 2020
08b7835
fix: improve voting power polacy
Jun 15, 2020
deb89eb
fix: compile error
Jun 15, 2020
d30feec
fix: lint error
Jun 15, 2020
06e84e8
fix: test case
Jun 15, 2020
4387cb6
test: add comment
Jun 15, 2020
94b2975
fix: remove unused function
Jun 15, 2020
3efc1d7
fix: define MaxTotalVotingPower
Jun 16, 2020
1c759e3
fix: remove useless test case, and leave todo
Jun 16, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/proto.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ jobs:
- uses: docker-practice/actions-setup-docker@master
- name: lint
run: make proto-lint
- name: check-breakage
run: make proto-check-breaking-ci
# - name: check-breakage
# run: make proto-check-breaking-ci
2 changes: 1 addition & 1 deletion blockchain/v0/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func randGenesisDoc(numValidators int, randPower bool, minPower int64) (*types.G
val, privVal := types.RandValidator(randPower, minPower)
validators[i] = types.GenesisValidator{
PubKey: val.PubKey,
Power: val.VotingPower,
Power: val.StakingPower,
}
privValidators[i] = privVal
}
Expand Down
2 changes: 1 addition & 1 deletion blockchain/v1/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func randGenesisDoc(numValidators int, randPower bool, minPower int64) (*types.G
val, privVal := types.RandValidator(randPower, minPower)
validators[i] = types.GenesisValidator{
PubKey: val.PubKey,
Power: val.VotingPower,
Power: val.StakingPower,
}
privValidators[i] = privVal
}
Expand Down
2 changes: 1 addition & 1 deletion blockchain/v2/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ func randGenesisDoc(chainID string, numValidators int, randPower bool, minPower
val, privVal := types.RandValidator(randPower, minPower)
validators[i] = types.GenesisValidator{
PubKey: val.PubKey,
Power: val.VotingPower,
Power: val.StakingPower,
}
privValidators[i] = privVal
}
Expand Down
4 changes: 2 additions & 2 deletions consensus/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func loadPrivValidator(config *cfg.Config) *privval.FilePV {
func randState(nValidators int) (*State, []*validatorStub) {
// Get State
state, privVals := randGenesisState(nValidators, false, 10)
state.LastProofHash = []byte{2}
state.LastProofHash = []byte{3}

vss := make([]*validatorStub, nValidators)

Expand Down Expand Up @@ -804,7 +804,7 @@ func randGenesisDoc(numValidators int, randPower bool, minPower int64) (*types.G
val, privVal := types.RandValidator(randPower, minPower)
validators[i] = types.GenesisValidator{
PubKey: val.PubKey,
Power: val.VotingPower,
Power: val.StakingPower,
}
privValidators[i] = privVal
}
Expand Down
6 changes: 3 additions & 3 deletions consensus/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ func TestReactorRecordsVotesAndBlockParts(t *testing.T) {
//-------------------------------------------------------------
// ensure we can make blocks despite cycling a validator set

func TestReactorVotingPowerChange(t *testing.T) {
func TestReactorStakingPowerChange(t *testing.T) {
nVals := 4
logger := log.TestingLogger()
css, cleanup := randConsensusNet(
Expand Down Expand Up @@ -377,7 +377,7 @@ func TestReactorVotingPowerChange(t *testing.T) {

if css[0].GetRoundState().LastVoters.TotalVotingPower() == previousTotalVotingPower {
t.Fatalf(
"expected voting power to change (before: %d, after: %d)",
"expected staking power to change (before: %d, after: %d)",
previousTotalVotingPower,
css[0].GetRoundState().LastVoters.TotalVotingPower())
}
Expand Down Expand Up @@ -486,7 +486,7 @@ func TestReactorValidatorSetChanges(t *testing.T) {

if css[nVals].GetRoundState().LastVoters.TotalVotingPower() == previousTotalVotingPower {
t.Errorf(
"expected voting power to change (before: %d, after: %d)",
"expected staking power to change (before: %d, after: %d)",
previousTotalVotingPower,
css[nVals].GetRoundState().LastVoters.TotalVotingPower())
}
Expand Down
2 changes: 1 addition & 1 deletion consensus/replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func (h *Handshaker) ReplayBlocks(
return nil, err
}
state.Validators = types.NewValidatorSet(vals)
state.Voters = types.ToVoterAll(state.Validators)
state.Voters = types.ToVoterAll(state.Validators.Validators)
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToVoterAll() needs []*Validator not ValidatorSet.

// Should sync it with MakeGenesisState()
state.NextValidators = types.NewValidatorSet(vals)
state.NextVoters = types.SelectVoter(state.NextValidators, h.genDoc.Hash())
Expand Down
3 changes: 1 addition & 2 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func StateMetrics(metrics *Metrics) StateOption {
// String returns a string.
func (cs *State) String() string {
// better not to access shared variables
return "ConsensusState" //(H:%v R:%v S:%v", cs.Height, cs.Round, cs.Step)
return "ConsensusState" // (H:%v R:%v S:%v", cs.Height, cs.Round, cs.Step)
}

// GetState returns a copy of the chain state.
Expand Down Expand Up @@ -1557,7 +1557,6 @@ func (cs *State) recordMetrics(height int64, block *types.Block) {
cs.Logger.Error("Error on retrival of pubkey", "err", err)
continue
}

if bytes.Equal(val.Address, pubKey.Address()) {
label := []string{
"validator_address", val.Address.String(),
Expand Down
2 changes: 1 addition & 1 deletion consensus/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func TestStateBadProposal(t *testing.T) {
proposalCh := subscribe(cs1.eventBus, types.EventQueryCompleteProposal)
voteCh := subscribe(cs1.eventBus, types.EventQueryVote)

propBlock, _ := cs1.createProposalBlock(round) //changeProposer(t, cs1, vs2)
propBlock, _ := cs1.createProposalBlock(round) // changeProposer(t, cs1, vs2)

// make the second validator the proposer by incrementing round
round++
Expand Down
2 changes: 1 addition & 1 deletion evidence/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (evpool *Pool) AddEvidence(evidence types.Evidence) error {
return err
}
_, val := valSet.GetByAddress(evidence.Address())
priority := val.VotingPower
priority := val.StakingPower

_, err = evpool.store.AddNewEvidence(evidence, priority)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion evidence/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func initializeValidatorState(valAddr []byte, height int64) dbm.DB {

// create validator set and state
vals := []*types.Validator{
{Address: valAddr, VotingPower: 1},
{Address: valAddr, StakingPower: 1},
}
state := sm.State{
LastBlockHeight: 0,
Expand Down
90 changes: 63 additions & 27 deletions libs/rand/sampling.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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] })

Expand Down Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think MaxFloat64Significant masking has much effect, since the division is done afterward. If we should eliminate the uncertainty introduced by floating-point arithmetic rounding, it's best to use big.Int, as shown below.

// (rand(seed) % Max) * total / (Max+1)
a := big.NewInt(nextRandom(seed) & MaxBits)
a.Mul(a, total)
a.Div(a, MaxBits + 1)
a.Uint64()

Copy link
Contributor Author

@egonspace egonspace Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll apply it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make sure that the idempotency with staking is not broken?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting the winners after the candidates array seems to be a bit of a tricky code, but what's the purpose? I don't think the effect of saving small buffer space is significant, and the risk of allowing candidates to overrun references seems to be greater.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 moveWinnerToLast, the winner's area also moves.

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")
}
Copy link
Contributor

Choose a reason for hiding this comment

The 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 winner.Priority() != 0 when a winner was selected.

Copy link
Contributor Author

@egonspace egonspace Jun 10, 2020

Choose a reason for hiding this comment

The 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.

if totalPriority-winnersPriority == 0 {
	break
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 ToVoterAll() function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some test cases having 0 staking power.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 assert purposes.

sum += candi.Priority()
}
return
return sum
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sum isn't necessary because it's declared in the return value. In golang fashion, I think it should be specified explicitly by return only when returning a value other than the one declared in the return value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

}

// SplitMix64
Expand Down
Loading