From b7b925a1acf6d48e4ba08737b65bb540ee0920a7 Mon Sep 17 00:00:00 2001 From: Aditya Date: Thu, 29 Oct 2020 12:17:37 +0000 Subject: [PATCH] Sort validators like tendermint in HistoricalInfo (#7691) * sort validators like tendermint * address comments * switch ordering in tests * change sort logic in error case * don't change test validators array order Co-authored-by: Jack Zampolin Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- x/staking/keeper/historical_info_test.go | 11 +++++---- x/staking/keeper/keeper_test.go | 6 ++++- x/staking/types/historical_info.go | 3 ++- x/staking/types/validator.go | 25 +++++++++++++++++++ x/staking/types/validator_test.go | 31 ++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 7 deletions(-) diff --git a/x/staking/keeper/historical_info_test.go b/x/staking/keeper/historical_info_test.go index 0ccdee9408..a9411d0db9 100644 --- a/x/staking/keeper/historical_info_test.go +++ b/x/staking/keeper/historical_info_test.go @@ -31,7 +31,7 @@ func TestHistoricalInfo(t *testing.T) { recv, found := app.StakingKeeper.GetHistoricalInfo(ctx, 2) require.True(t, found, "HistoricalInfo not found after set") require.Equal(t, hi, recv, "HistoricalInfo not equal") - require.True(t, sort.IsSorted(types.Validators(recv.Valset)), "HistoricalInfo validators is not sorted") + require.True(t, sort.IsSorted(types.ValidatorsByVotingPower(recv.Valset)), "HistoricalInfo validators is not sorted") app.StakingKeeper.DeleteHistoricalInfo(ctx, 2) @@ -76,15 +76,16 @@ func TestTrackHistoricalInfo(t *testing.T) { require.True(t, found) require.Equal(t, hi5, recv) - // Set last validators in keeper + // Set bonded validators in keeper val1 := teststaking.NewValidator(t, addrVals[2], PKs[2]) app.StakingKeeper.SetValidator(ctx, val1) app.StakingKeeper.SetLastValidatorPower(ctx, val1.GetOperator(), 10) val2 := teststaking.NewValidator(t, addrVals[3], PKs[3]) - vals := []types.Validator{val1, val2} - sort.Sort(types.Validators(vals)) app.StakingKeeper.SetValidator(ctx, val2) - app.StakingKeeper.SetLastValidatorPower(ctx, val2.GetOperator(), 8) + app.StakingKeeper.SetLastValidatorPower(ctx, val2.GetOperator(), 80) + + vals := []types.Validator{val1, val2} + sort.Sort(types.ValidatorsByVotingPower(vals)) // Set Header for BeginBlock context header := tmproto.Header{ diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index 0342dfea9b..2f9819989f 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -41,7 +41,11 @@ func (suite *KeeperTestSuite) SetupTest() { Height: 5, } - hi := types.NewHistoricalInfo(header, validators) + // sort a copy of the validators, so that original validators does not + // have its order changed + sortedVals := make([]types.Validator, len(validators)) + copy(sortedVals, validators) + hi := types.NewHistoricalInfo(header, sortedVals) app.StakingKeeper.SetHistoricalInfo(ctx, 5, &hi) suite.app, suite.ctx, suite.queryClient, suite.addrs, suite.vals = app, ctx, queryClient, addrs, validators diff --git a/x/staking/types/historical_info.go b/x/staking/types/historical_info.go index 10df3ced02..1a8461ad13 100644 --- a/x/staking/types/historical_info.go +++ b/x/staking/types/historical_info.go @@ -14,7 +14,8 @@ import ( // NewHistoricalInfo will create a historical information struct from header and valset // it will first sort valset before inclusion into historical info func NewHistoricalInfo(header tmproto.Header, valSet Validators) HistoricalInfo { - sort.Sort(valSet) + // Must sort in the same way that tendermint does + sort.Sort(ValidatorsByVotingPower(valSet)) return HistoricalInfo{ Header: header, diff --git a/x/staking/types/validator.go b/x/staking/types/validator.go index 8b5cfcdf14..19e032dbab 100644 --- a/x/staking/types/validator.go +++ b/x/staking/types/validator.go @@ -123,6 +123,31 @@ func (v Validators) Swap(i, j int) { v[j] = it } +// ValidatorsByVotingPower implements sort.Interface for []Validator based on +// the VotingPower and Address fields. +// The validators are sorted first by their voting power (descending). Secondary index - Address (ascending). +// Copied from tendermint/types/validator_set.go +type ValidatorsByVotingPower []Validator + +func (valz ValidatorsByVotingPower) Len() int { return len(valz) } + +func (valz ValidatorsByVotingPower) Less(i, j int) bool { + if valz[i].ConsensusPower() == valz[j].ConsensusPower() { + addrI, errI := valz[i].GetConsAddr() + addrJ, errJ := valz[j].GetConsAddr() + // If either returns error, then return false + if errI != nil || errJ != nil { + return false + } + return bytes.Compare(addrI, addrJ) == -1 + } + return valz[i].ConsensusPower() > valz[j].ConsensusPower() +} + +func (valz ValidatorsByVotingPower) Swap(i, j int) { + valz[i], valz[j] = valz[j], valz[i] +} + // UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces func (v Validators) UnpackInterfaces(c codectypes.AnyUnpacker) error { for i := range v { diff --git a/x/staking/types/validator_test.go b/x/staking/types/validator_test.go index 2e9d961169..64afc97681 100644 --- a/x/staking/types/validator_test.go +++ b/x/staking/types/validator_test.go @@ -274,6 +274,37 @@ func TestValidatorsSortDeterminism(t *testing.T) { } } +// Check SortTendermint sorts the same as tendermint +func TestValidatorsSortTendermint(t *testing.T) { + vals := make([]Validator, 100) + + for i := range vals { + pk := ed25519.GenPrivKey().PubKey() + pk2 := ed25519.GenPrivKey().PubKey() + vals[i] = newValidator(t, sdk.ValAddress(pk2.Address()), pk) + vals[i].Status = Bonded + vals[i].Tokens = sdk.NewInt(rand.Int63()) + } + // create some validators with the same power + for i := 0; i < 10; i++ { + vals[i].Tokens = sdk.NewInt(1000000) + } + + valz := Validators(vals) + + // create expected tendermint validators by converting to tendermint then sorting + expectedVals, err := valz.ToTmValidators() + require.NoError(t, err) + sort.Sort(tmtypes.ValidatorsByVotingPower(expectedVals)) + + // sort in SDK and then convert to tendermint + sort.Sort(ValidatorsByVotingPower(valz)) + actualVals, err := valz.ToTmValidators() + require.NoError(t, err) + + require.Equal(t, expectedVals, actualVals, "sorting in SDK is not the same as sorting in Tendermint") +} + func TestValidatorToTm(t *testing.T) { vals := make(Validators, 10) expected := make([]*tmtypes.Validator, 10)