diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f9f64ca23..eb8a1117d 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -11,6 +11,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - CLI/RPC/Config - [evidence] \#4725 Remove `Pubkey` from DuplicateVoteEvidence + - [rpc] [\#4792](https://github.com/tendermint/tendermint/pull/4792) `/validators` are now sorted by voting power (@melekes) - Apps @@ -25,6 +26,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - Blockchain Protocol + - [types] [\#4792](https://github.com/tendermint/tendermint/pull/4792) Sort validators by voting power to enable faster commit verification (@melekes) + ### FEATURES: - [statesync] Add state sync support, where a new node can be rapidly bootstrapped by fetching state snapshots from peers instead of replaying blocks. See the `[statesync]` config section. diff --git a/lite2/helpers_test.go b/lite2/helpers_test.go index 1e1e022d4..4a072cb3e 100644 --- a/lite2/helpers_test.go +++ b/lite2/helpers_test.go @@ -78,7 +78,7 @@ func (pkz privKeys) signHeader(header *types.Header, first, last int) *types.Com } // We need this list to keep the ordering. - vset := pkz.ToValidators(1, 0) + vset := pkz.ToValidators(1, 1) blockID := types.BlockID{ Hash: header.Hash(), diff --git a/rpc/swagger/swagger.yaml b/rpc/swagger/swagger.yaml index f2631c2e9..af6306890 100644 --- a/rpc/swagger/swagger.yaml +++ b/rpc/swagger/swagger.yaml @@ -669,7 +669,7 @@ paths: tags: - Info description: | - Get Validators. + Get Validators. Validators are sorted by voting power. responses: 200: description: Commit results. diff --git a/state/validation.go b/state/validation.go index c3231c270..221927af3 100644 --- a/state/validation.go +++ b/state/validation.go @@ -148,9 +148,14 @@ func validateBlock(evidencePool EvidencePool, stateDB dbm.DB, state State, block // NOTE: We can't actually verify it's the right proposer because we dont // know what round the block was first proposed. So just check that it's // a legit address and a known validator. - if len(block.ProposerAddress) != crypto.AddressSize || - !state.Validators.HasAddress(block.ProposerAddress) { - return fmt.Errorf("block.Header.ProposerAddress, %X, is not a validator", + if len(block.ProposerAddress) != crypto.AddressSize { + return fmt.Errorf("expected ProposerAddress size %d, got %d", + crypto.AddressSize, + len(block.ProposerAddress), + ) + } + if !state.Validators.HasAddress(block.ProposerAddress) { + return fmt.Errorf("block.Header.ProposerAddress %X is not a validator", block.ProposerAddress, ) } diff --git a/types/priv_validator.go b/types/priv_validator.go index fbe8cebf0..3d8e72bba 100644 --- a/types/priv_validator.go +++ b/types/priv_validator.go @@ -18,9 +18,6 @@ type PrivValidator interface { SignProposal(chainID string, proposal *Proposal) error } -//---------------------------------------- -// Misc. - type PrivValidatorsByAddress []PrivValidator func (pvs PrivValidatorsByAddress) Len() int { diff --git a/types/validator_set.go b/types/validator_set.go index 2ce466250..07b50f5ff 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -24,19 +24,23 @@ const ( // and leaves room for defensive purposes. MaxTotalVotingPower = int64(math.MaxInt64) / 8 - // PriorityWindowSizeFactor - is a constant that when multiplied with the total voting power gives - // the maximum allowed distance between validator priorities. + // PriorityWindowSizeFactor - is a constant that when multiplied with the + // total voting power gives the maximum allowed distance between validator + // priorities. PriorityWindowSizeFactor = 2 ) // ValidatorSet represent a set of *Validator at a given height. +// // The validators can be fetched by address or index. -// The index is in order of .Address, so the indices are fixed -// for all rounds of a given blockchain height - ie. the validators -// are sorted by their address. -// On the other hand, the .ProposerPriority of each validator and -// the designated .GetProposer() of a set changes every round, -// upon calling .IncrementProposerPriority(). +// The index is in order of .VotingPower, so the indices are fixed for all +// rounds of a given blockchain height - ie. the validators are sorted by their +// voting power (descending). Secondary index - .Address (ascending). +// +// On the other hand, the .ProposerPriority of each validator and the +// designated .GetProposer() of a set changes every round, upon calling +// .IncrementProposerPriority(). +// // NOTE: Not goroutine-safe. // NOTE: All get/set to validators should copy the value for safety. type ValidatorSet struct { @@ -48,18 +52,21 @@ type ValidatorSet struct { totalVotingPower int64 } -// NewValidatorSet initializes a ValidatorSet by copying over the -// values from `valz`, a list of Validators. If valz is nil or empty, -// the new ValidatorSet will have an empty list of Validators. -// The addresses of validators in `valz` must be unique otherwise the -// function panics. -// Note the validator set size has an implied limit equal to that of the MaxVotesCount - -// commits by a validator set larger than this will fail validation. +// NewValidatorSet initializes a ValidatorSet by copying over the values from +// `valz`, a list of Validators. If valz is nil or empty, the new ValidatorSet +// will have an empty list of Validators. +// +// The addresses of validators in `valz` must be unique otherwise the function +// panics. +// +// Note the validator set size has an implied limit equal to that of the +// MaxVotesCount - commits by a validator set larger than this will fail +// validation. func NewValidatorSet(valz []*Validator) *ValidatorSet { vals := &ValidatorSet{} err := vals.updateWithChangeSet(valz, false) if err != nil { - panic(fmt.Sprintf("cannot create validator set: %s", err)) + panic(fmt.Sprintf("Cannot create validator set: %v", err)) } if len(valz) > 0 { vals.IncrementProposerPriority(1) @@ -80,8 +87,8 @@ func (vals *ValidatorSet) CopyIncrementProposerPriority(times int) *ValidatorSet return copy } -// IncrementProposerPriority increments ProposerPriority of each validator and updates the -// proposer. Panics if validator set is empty. +// IncrementProposerPriority increments ProposerPriority of each validator and +// updates the proposer. Panics if validator set is empty. // `times` must be positive. func (vals *ValidatorSet) IncrementProposerPriority(times int) { if vals.IsNilOrEmpty() { @@ -107,8 +114,9 @@ func (vals *ValidatorSet) IncrementProposerPriority(times int) { vals.Proposer = proposer } -// RescalePriorities rescales the priorities such that the distance between the maximum and minimum -// is smaller than `diffMax`. +// RescalePriorities rescales the priorities such that the distance between the +// maximum and minimum is smaller than `diffMax`. Panics if validator set is +// empty. func (vals *ValidatorSet) RescalePriorities(diffMax int64) { if vals.IsNilOrEmpty() { panic("empty validator set") @@ -226,25 +234,27 @@ func (vals *ValidatorSet) Copy() *ValidatorSet { // HasAddress returns true if address given is in the validator set, false - // otherwise. func (vals *ValidatorSet) HasAddress(address []byte) bool { - idx := sort.Search(len(vals.Validators), func(i int) bool { - return bytes.Compare(address, vals.Validators[i].Address) <= 0 - }) - return idx < len(vals.Validators) && bytes.Equal(vals.Validators[idx].Address, address) + for _, val := range vals.Validators { + if bytes.Equal(val.Address, address) { + return true + } + } + return false } // GetByAddress returns an index of the validator with address and validator -// itself if found. Otherwise, -1 and nil are returned. +// itself (copy) if found. Otherwise, -1 and nil are returned. func (vals *ValidatorSet) GetByAddress(address []byte) (index int, val *Validator) { - idx := sort.Search(len(vals.Validators), func(i int) bool { - return bytes.Compare(address, vals.Validators[i].Address) <= 0 - }) - if idx < len(vals.Validators) && bytes.Equal(vals.Validators[idx].Address, address) { - return idx, vals.Validators[idx].Copy() + for idx, val := range vals.Validators { + if bytes.Equal(val.Address, address) { + return idx, val.Copy() + } } return -1, nil } -// GetByIndex returns the validator's address and validator itself by index. +// GetByIndex returns the validator's address and validator itself (copy) by +// index. // It returns nil values if index is less than 0 or greater or equal to // len(ValidatorSet.Validators). func (vals *ValidatorSet) GetByIndex(index int) (address []byte, val *Validator) { @@ -263,7 +273,6 @@ func (vals *ValidatorSet) Size() int { // Forces recalculation of the set's total voting power. // Panics if total voting power is bigger than MaxTotalVotingPower. func (vals *ValidatorSet) updateTotalVotingPower() { - sum := int64(0) for _, val := range vals.Validators { // mind overflow @@ -445,7 +454,6 @@ func numNewValidators(updates []*Validator, vals *ValidatorSet) int { // // No changes are made to the validator set 'vals'. func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotalVotingPower int64) { - for _, valUpdate := range updates { address := valUpdate.Address _, val := vals.GetByAddress(address) @@ -471,8 +479,9 @@ func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotal // Expects updates to be a list of updates sorted by address with no duplicates or errors, // must have been validated with verifyUpdates() and priorities computed with computeNewPriorities(). func (vals *ValidatorSet) applyUpdates(updates []*Validator) { - existing := vals.Validators + sort.Sort(ValidatorsByAddress(existing)) + merged := make([]*Validator, len(existing)+len(updates)) i := 0 @@ -509,7 +518,6 @@ func (vals *ValidatorSet) applyUpdates(updates []*Validator) { // Checks that the validators to be removed are part of the validator set. // No changes are made to the validator set 'vals'. func verifyRemovals(deletes []*Validator, vals *ValidatorSet) (votingPower int64, err error) { - removedVotingPower := int64(0) for _, valUpdate := range deletes { address := valUpdate.Address @@ -527,8 +535,8 @@ func verifyRemovals(deletes []*Validator, vals *ValidatorSet) (votingPower int64 // Removes the validators specified in 'deletes' from validator set 'vals'. // Should not fail as verification has been done before. +// Expects vals to be sorted by address (done by applyUpdates). func (vals *ValidatorSet) applyRemovals(deletes []*Validator) { - existing := vals.Validators merged := make([]*Validator, len(existing)-len(deletes)) @@ -559,7 +567,6 @@ func (vals *ValidatorSet) applyRemovals(deletes []*Validator) { // are not allowed and will trigger an error if present in 'changes'. // The 'allowDeletes' flag is set to false by NewValidatorSet() and to true by UpdateWithChangeSet(). func (vals *ValidatorSet) updateWithChangeSet(changes []*Validator, allowDeletes bool) error { - if len(changes) == 0 { return nil } @@ -606,6 +613,8 @@ func (vals *ValidatorSet) updateWithChangeSet(changes []*Validator, allowDeletes vals.RescalePriorities(PriorityWindowSizeFactor * vals.TotalVotingPower()) vals.shiftByAvgProposerPriority() + sort.Sort(ValidatorsByVotingPower(vals.Validators)) + return nil } @@ -883,42 +892,59 @@ func (vals *ValidatorSet) StringIndented(indent string) string { } //------------------------------------- -// Implements sort for sorting validators by address. -// ValidatorsByAddress implements the sort of validators by address. -type ValidatorsByAddress []*Validator +// ValidatorsByVotingPower implements sort.Interface for []*Validator based on +// the VotingPower and Address fields. +type ValidatorsByVotingPower []*Validator + +func (valz ValidatorsByVotingPower) Len() int { return len(valz) } -func (valz ValidatorsByAddress) Len() int { - return len(valz) +func (valz ValidatorsByVotingPower) Less(i, j int) bool { + if valz[i].VotingPower == valz[j].VotingPower { + return bytes.Compare(valz[i].Address, valz[j].Address) == -1 + } + return valz[i].VotingPower > valz[j].VotingPower +} + +func (valz ValidatorsByVotingPower) Swap(i, j int) { + valz[i], valz[j] = valz[j], valz[i] } +// ValidatorsByAddress implements sort.Interface for []*Validator based on +// the Address field. +type ValidatorsByAddress []*Validator + +func (valz ValidatorsByAddress) Len() int { return len(valz) } + func (valz ValidatorsByAddress) Less(i, j int) bool { return bytes.Compare(valz[i].Address, valz[j].Address) == -1 } func (valz ValidatorsByAddress) Swap(i, j int) { - it := valz[i] - valz[i] = valz[j] - valz[j] = it + valz[i], valz[j] = valz[j], valz[i] } //---------------------------------------- -// for testing -// RandValidatorSet returns a randomized validator set, useful for testing. -// NOTE: PrivValidator are in order. -// UNSTABLE +// RandValidatorSet returns a randomized validator set (size: +numValidators+), +// where each validator has a voting power of +votingPower+. +// +// EXPOSED FOR TESTING. func RandValidatorSet(numValidators int, votingPower int64) (*ValidatorSet, []PrivValidator) { - valz := make([]*Validator, numValidators) - privValidators := make([]PrivValidator, numValidators) + var ( + valz = make([]*Validator, numValidators) + privValidators = make([]PrivValidator, numValidators) + ) + for i := 0; i < numValidators; i++ { val, privValidator := RandValidator(false, votingPower) valz[i] = val privValidators[i] = privValidator } - vals := NewValidatorSet(valz) + sort.Sort(PrivValidatorsByAddress(privValidators)) - return vals, privValidators + + return NewValidatorSet(valz), privValidators } /////////////////////////////////////////////////////////////////////////////// diff --git a/types/validator_set_test.go b/types/validator_set_test.go index 3dbdbabed..0e73e800a 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -929,28 +929,28 @@ func TestValSetUpdatesBasicTestsExecute(t *testing.T) { }, { // voting power changes testValSet(2, 10), - []testVal{{"v1", 11}, {"v2", 22}}, - []testVal{{"v1", 11}, {"v2", 22}}, + []testVal{{"v2", 22}, {"v1", 11}}, + []testVal{{"v2", 22}, {"v1", 11}}, }, { // add new validators - []testVal{{"v1", 10}, {"v2", 20}}, - []testVal{{"v3", 30}, {"v4", 40}}, - []testVal{{"v1", 10}, {"v2", 20}, {"v3", 30}, {"v4", 40}}, + []testVal{{"v2", 20}, {"v1", 10}}, + []testVal{{"v4", 40}, {"v3", 30}}, + []testVal{{"v4", 40}, {"v3", 30}, {"v2", 20}, {"v1", 10}}, }, { // add new validator to middle - []testVal{{"v1", 10}, {"v3", 20}}, + []testVal{{"v3", 20}, {"v1", 10}}, []testVal{{"v2", 30}}, - []testVal{{"v1", 10}, {"v2", 30}, {"v3", 20}}, + []testVal{{"v2", 30}, {"v3", 20}, {"v1", 10}}, }, { // add new validator to beginning - []testVal{{"v2", 10}, {"v3", 20}}, + []testVal{{"v3", 20}, {"v2", 10}}, []testVal{{"v1", 30}}, - []testVal{{"v1", 30}, {"v2", 10}, {"v3", 20}}, + []testVal{{"v1", 30}, {"v3", 20}, {"v2", 10}}, }, { // delete validators - []testVal{{"v1", 10}, {"v2", 20}, {"v3", 30}}, + []testVal{{"v3", 30}, {"v2", 20}, {"v1", 10}}, []testVal{{"v2", 0}}, - []testVal{{"v1", 10}, {"v3", 30}}, + []testVal{{"v3", 30}, {"v1", 10}}, }, } @@ -987,19 +987,19 @@ func TestValSetUpdatesOrderIndependenceTestsExecute(t *testing.T) { updateVals []testVal }{ 0: { // order of changes should not matter, the final validator sets should be the same - []testVal{{"v1", 10}, {"v2", 10}, {"v3", 30}, {"v4", 40}}, - []testVal{{"v1", 11}, {"v2", 22}, {"v3", 33}, {"v4", 44}}}, + []testVal{{"v4", 40}, {"v3", 30}, {"v2", 10}, {"v1", 10}}, + []testVal{{"v4", 44}, {"v3", 33}, {"v2", 22}, {"v1", 11}}}, 1: { // order of additions should not matter - []testVal{{"v1", 10}, {"v2", 20}}, + []testVal{{"v2", 20}, {"v1", 10}}, []testVal{{"v3", 30}, {"v4", 40}, {"v5", 50}, {"v6", 60}}}, 2: { // order of removals should not matter - []testVal{{"v1", 10}, {"v2", 20}, {"v3", 30}, {"v4", 40}}, + []testVal{{"v4", 40}, {"v3", 30}, {"v2", 20}, {"v1", 10}}, []testVal{{"v1", 0}, {"v3", 0}, {"v4", 0}}}, 3: { // order of mixed operations should not matter - []testVal{{"v1", 10}, {"v2", 20}, {"v3", 30}, {"v4", 40}}, + []testVal{{"v4", 40}, {"v3", 30}, {"v2", 20}, {"v1", 10}}, []testVal{{"v1", 0}, {"v3", 0}, {"v2", 22}, {"v5", 50}, {"v4", 44}}}, } @@ -1150,11 +1150,11 @@ func randTestVSetCfg(t *testing.T, nBase, nAddMax int) testVSetCfg { cfg.expectedVals[i-nDel] = cfg.addedVals[i-nBase] } - sort.Sort(testValsByAddress(cfg.startVals)) - sort.Sort(testValsByAddress(cfg.deletedVals)) - sort.Sort(testValsByAddress(cfg.updatedVals)) - sort.Sort(testValsByAddress(cfg.addedVals)) - sort.Sort(testValsByAddress(cfg.expectedVals)) + sort.Sort(testValsByVotingPower(cfg.startVals)) + sort.Sort(testValsByVotingPower(cfg.deletedVals)) + sort.Sort(testValsByVotingPower(cfg.updatedVals)) + sort.Sort(testValsByVotingPower(cfg.addedVals)) + sort.Sort(testValsByVotingPower(cfg.expectedVals)) return cfg @@ -1175,25 +1175,25 @@ func TestValSetUpdatePriorityOrderTests(t *testing.T) { testCases := []testVSetCfg{ 0: { // remove high power validator, keep old equal lower power validators - startVals: []testVal{{"v1", 1}, {"v2", 1}, {"v3", 1000}}, + startVals: []testVal{{"v3", 1000}, {"v1", 1}, {"v2", 1}}, deletedVals: []testVal{{"v3", 0}}, updatedVals: []testVal{}, addedVals: []testVal{}, expectedVals: []testVal{{"v1", 1}, {"v2", 1}}, }, 1: { // remove high power validator, keep old different power validators - startVals: []testVal{{"v1", 1}, {"v2", 10}, {"v3", 1000}}, + startVals: []testVal{{"v3", 1000}, {"v2", 10}, {"v1", 1}}, deletedVals: []testVal{{"v3", 0}}, updatedVals: []testVal{}, addedVals: []testVal{}, - expectedVals: []testVal{{"v1", 1}, {"v2", 10}}, + expectedVals: []testVal{{"v2", 10}, {"v1", 1}}, }, 2: { // remove high power validator, add new low power validators, keep old lower power - startVals: []testVal{{"v1", 1}, {"v2", 2}, {"v3", 1000}}, + startVals: []testVal{{"v3", 1000}, {"v2", 2}, {"v1", 1}}, deletedVals: []testVal{{"v3", 0}}, updatedVals: []testVal{{"v2", 1}}, - addedVals: []testVal{{"v4", 40}, {"v5", 50}}, - expectedVals: []testVal{{"v1", 1}, {"v2", 1}, {"v4", 40}, {"v5", 50}}, + addedVals: []testVal{{"v5", 50}, {"v4", 40}}, + expectedVals: []testVal{{"v5", 50}, {"v4", 40}, {"v1", 1}, {"v2", 1}}, }, // generate a configuration with 100 validators, @@ -1244,7 +1244,7 @@ func verifyValSetUpdatePriorityOrder(t *testing.T, valSet *ValidatorSet, cfg tes // - they should be at the beginning of valListNewPriority since it is sorted by priority if len(cfg.addedVals) > 0 { addedValsPriSlice := updatedValsPriSorted[:len(cfg.addedVals)] - sort.Sort(ValidatorsByAddress(addedValsPriSlice)) + sort.Sort(ValidatorsByVotingPower(addedValsPriSlice)) assert.Equal(t, cfg.addedVals, toTestValList(addedValsPriSlice)) // - and should all have the same priority @@ -1259,7 +1259,7 @@ func TestValSetUpdateOverflowRelated(t *testing.T) { testCases := []testVSetCfg{ { name: "1 no false overflow error messages for updates", - startVals: []testVal{{"v1", 1}, {"v2", MaxTotalVotingPower - 1}}, + startVals: []testVal{{"v2", MaxTotalVotingPower - 1}, {"v1", 1}}, updatedVals: []testVal{{"v1", MaxTotalVotingPower - 1}, {"v2", 1}}, expectedVals: []testVal{{"v1", MaxTotalVotingPower - 1}, {"v2", 1}}, wantErr: false, @@ -1268,9 +1268,9 @@ func TestValSetUpdateOverflowRelated(t *testing.T) { // this test shows that it is important to apply the updates in the order of the change in power // i.e. apply first updates with decreases in power, v2 change in this case. name: "2 no false overflow error messages for updates", - startVals: []testVal{{"v1", 1}, {"v2", MaxTotalVotingPower - 1}}, + startVals: []testVal{{"v2", MaxTotalVotingPower - 1}, {"v1", 1}}, updatedVals: []testVal{{"v1", MaxTotalVotingPower/2 - 1}, {"v2", MaxTotalVotingPower / 2}}, - expectedVals: []testVal{{"v1", MaxTotalVotingPower/2 - 1}, {"v2", MaxTotalVotingPower / 2}}, + expectedVals: []testVal{{"v2", MaxTotalVotingPower / 2}, {"v1", MaxTotalVotingPower/2 - 1}}, wantErr: false, }, { @@ -1278,7 +1278,7 @@ func TestValSetUpdateOverflowRelated(t *testing.T) { startVals: []testVal{{"v1", MaxTotalVotingPower - 2}, {"v2", 1}, {"v3", 1}}, deletedVals: []testVal{{"v1", 0}}, addedVals: []testVal{{"v4", MaxTotalVotingPower - 2}}, - expectedVals: []testVal{{"v2", 1}, {"v3", 1}, {"v4", MaxTotalVotingPower - 2}}, + expectedVals: []testVal{{"v4", MaxTotalVotingPower - 2}, {"v2", 1}, {"v3", 1}}, wantErr: false, }, { @@ -1291,7 +1291,7 @@ func TestValSetUpdateOverflowRelated(t *testing.T) { {"v1", MaxTotalVotingPower/2 - 2}, {"v3", MaxTotalVotingPower/2 - 3}, {"v4", 2}}, addedVals: []testVal{{"v5", 3}}, expectedVals: []testVal{ - {"v1", MaxTotalVotingPower/2 - 2}, {"v3", MaxTotalVotingPower/2 - 3}, {"v4", 2}, {"v5", 3}}, + {"v1", MaxTotalVotingPower/2 - 2}, {"v3", MaxTotalVotingPower/2 - 3}, {"v5", 3}, {"v4", 2}}, wantErr: false, }, { @@ -1433,18 +1433,21 @@ func (valz validatorsByPriority) Swap(i, j int) { } //------------------------------------- -// Sort testVal-s by address. -type testValsByAddress []testVal -func (tvals testValsByAddress) Len() int { +type testValsByVotingPower []testVal + +func (tvals testValsByVotingPower) Len() int { return len(tvals) } -func (tvals testValsByAddress) Less(i, j int) bool { - return bytes.Compare([]byte(tvals[i].name), []byte(tvals[j].name)) == -1 +func (tvals testValsByVotingPower) Less(i, j int) bool { + if tvals[i].power == tvals[j].power { + return bytes.Compare([]byte(tvals[i].name), []byte(tvals[j].name)) == -1 + } + return tvals[i].power > tvals[j].power } -func (tvals testValsByAddress) Swap(i, j int) { +func (tvals testValsByVotingPower) Swap(i, j int) { it := tvals[i] tvals[i] = tvals[j] tvals[j] = it