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 <jack.zampolin@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

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{

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

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,

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 {

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)