add sorting into AccumulateChanges and add a unit test for it (#584)

* add sorting into AccumulateChanges and add a unit test for it

* Fix sorting to make it more understandable

x/ccv/utils/utils.go

@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"sort"
 	"time"
 
 	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
@@ -30,6 +31,16 @@ func AccumulateChanges(currentChanges, newChanges []abci.ValidatorUpdate) []abci
 	for _, update := range m {
 		out = append(out, update)
 	}
+
+	// The list of tendermint updates should hash the same across all consensus nodes
+	// that means it is necessary to sort for determinism.
+	sort.Slice(out, func(i, j int) bool {
+		if out[i].Power != out[j].Power {
+			return out[i].Power > out[j].Power
+		}
+		return out[i].PubKey.String() > out[j].PubKey.String()
+	})
+
 	return out
 }
 

x/ccv/utils/utils_test.go

@@ -0,0 +1,87 @@
+package utils_test
+
+import (
+	"testing"
+
+	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
+	ibcsimapp "github.com/cosmos/ibc-go/v3/testing/simapp"
+	"github.com/cosmos/interchain-security/x/ccv/utils"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+func TestAccumulateChanges(t *testing.T) {
+	testKeys := ibcsimapp.CreateTestPubKeys(2)
+
+	tmPubKey, _ := cryptocodec.ToTmProtoPublicKey(testKeys[0])
+	tmPubKey2, _ := cryptocodec.ToTmProtoPublicKey(testKeys[1])
+
+	testCases := []struct {
+		name     string
+		changes1 []abci.ValidatorUpdate
+		changes2 []abci.ValidatorUpdate
+		expected []abci.ValidatorUpdate
+	}{
+		{
+			name:     "no changes",
+			changes1: []abci.ValidatorUpdate{},
+			changes2: []abci.ValidatorUpdate{},
+			expected: []abci.ValidatorUpdate(nil),
+		},
+		{
+			name: "one change",
+			changes1: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 1},
+			},
+			changes2: []abci.ValidatorUpdate{},
+			expected: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 1},
+			},
+		},
+		{
+			name: "two changes",
+			changes1: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 1},
+			},
+			changes2: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 2},
+			},
+			expected: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 2},
+			},
+		},
+		{
+			name: "two changes with different pubkeys",
+			changes1: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 1},
+			},
+			changes2: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey2, Power: 2},
+			},
+			expected: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey2, Power: 2},
+				{PubKey: tmPubKey, Power: 1},
+			},
+		},
+		{
+			name: "two changes with different pubkeys and same power",
+			changes1: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey, Power: 1},
+			},
+			changes2: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey2, Power: 1},
+			},
+			expected: []abci.ValidatorUpdate{
+				{PubKey: tmPubKey2, Power: 1},
+				{PubKey: tmPubKey, Power: 1},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			changes := utils.AccumulateChanges(tc.changes1, tc.changes2)
+			require.Equal(t, tc.expected, changes)
+		})
+	}
+}