fix: types: correctly coalesce coins even with repeated denominations…

… & simplify logic (backport cosmos#13265) (cosmos#13302)

* fix: types: correctly coalesce coins even with repeated denominations & simplify logic (cosmos#13265)

(cherry picked from commit 83f88a6)

# Conflicts:
#	types/coin_test.go

* fix conflict

* add changelog

Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>

CHANGELOG.md

@@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (types) [#13265](https://github.com/cosmos/cosmos-sdk/pull/13265) Correctly coalesce coins even with repeated denominations & simplify logic
 * (x/auth) [#13200](https://github.com/cosmos/cosmos-sdk/pull/13200) Fix wrong sequences in `sign-batch`.
 * (export) [#13029](https://github.com/cosmos/cosmos-sdk/pull/13029) Fix exporting the blockParams regression.
 * [#13046](https://github.com/cosmos/cosmos-sdk/pull/13046) Fix missing return statement in BaseApp.Query.

types/coin.go

@@ -342,22 +342,21 @@ func (coins Coins) safeAdd(coinsB Coins) (coalesced Coins) {
 		panic("Wrong argument: coins must be sorted")
 	}
 
-	uniqCoins := make(map[string]Coin, len(coins)+len(coinsB))
+	uniqCoins := make(map[string]Coins, len(coins)+len(coinsB))
 	// Traverse all the coins for each of the coins and coinsB.
 	for _, cL := range []Coins{coins, coinsB} {
 		for _, c := range cL {
-			if uc, ok := uniqCoins[c.Denom]; ok {
-				uniqCoins[c.Denom] = uc.Add(c)
-			} else {
-				uniqCoins[c.Denom] = c
-			}
+			uniqCoins[c.Denom] = append(uniqCoins[c.Denom], c)
 		}
 	}
 
-	coalesced = make(Coins, 0, len(uniqCoins))
-	for denom, c := range uniqCoins { //#nosec
-		if c.IsZero() {
-			continue
+	for denom, cL := range uniqCoins {
+		comboCoin := Coin{Denom: denom, Amount: NewInt(0)}
+		for _, c := range cL {
+			comboCoin = comboCoin.Add(c)
+		}
+		if !comboCoin.IsZero() {
+			coalesced = append(coalesced, comboCoin)
 		}
 		c.Denom = denom
 		coalesced = append(coalesced, c)

types/coin_test.go

@@ -563,46 +563,18 @@ func (s *coinTestSuite) TestAddCoins() {
 		expected sdk.Coins
 		msg      string
 	}{
-		{"adding two empty lists", s.emptyCoins, s.emptyCoins, s.emptyCoins, "empty, non list should be returned"},
-		{"empty list + set", s.emptyCoins, cA0M1, sdk.Coins{s.cm1}, "zero coins should be removed"},
-		{"empty list + set", s.emptyCoins, cA1M1, cA1M1, "zero + a_non_zero = a_non_zero"},
-		{"set + empty list", cA0M1, s.emptyCoins, sdk.Coins{s.cm1}, "zero coins should be removed"},
-		{"set + empty list", cA1M1, s.emptyCoins, cA1M1, "a_non_zero + zero  = a_non_zero"},
-		{
-			"{1atom,1muon}+{1atom,1muon}", cA1M1, cA1M1,
-			sdk.Coins{s.ca2, s.cm2},
-			"a + a = 2a",
-		},
-		{
-			"{0atom,1muon}+{0atom,0muon}", cA0M1, cA0M0,
-			sdk.Coins{s.cm1},
-			"zero coins should be removed",
-		},
-		{
-			"{2atom}+{0muon}",
-			sdk.Coins{s.ca2},
-			sdk.Coins{s.cm0},
-			sdk.Coins{s.ca2},
-			"zero coins should be removed",
-		},
-		{
-			"{1atom}+{1atom,2muon}",
-			sdk.Coins{s.ca1},
-			sdk.Coins{s.ca1, s.cm2},
-			sdk.Coins{s.ca2, s.cm2},
-			"should be correctly added",
-		},
-		{
-			"{0atom,0muon}+{0atom,0muon}", cA0M0, cA0M0, s.emptyCoins,
-			"sets with zero coins should return empty set",
-		},
+		{"{1atom,1muon}+{1atom,1muon}", sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca2, s.cm2}},
+		{"{0atom,1muon}+{0atom,0muon}", sdk.Coins{s.ca0, s.cm1}, sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.cm1}},
+		{"{2atom}+{0muon}", sdk.Coins{s.ca2}, sdk.Coins{s.cm0}, sdk.Coins{s.ca2}},
+		{"{1atom}+{1atom,2muon}", sdk.Coins{s.ca1}, sdk.Coins{s.ca1, s.cm2}, sdk.Coins{s.ca2, s.cm2}},
+		{"{0atom,0muon}+{0atom,0muon}", sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.ca0, s.cm0}, sdk.Coins(nil)},
 	}
 
 	for _, tc := range cases {
 		s.T().Run(tc.name, func(t *testing.T) {
 			res := tc.inputOne.Add(tc.inputTwo...)
 			require.True(t, res.IsValid(), fmt.Sprintf("%s + %s = %s", tc.inputOne, tc.inputTwo, res))
-			require.Equal(t, tc.expected, res, tc.msg)
+			require.Equal(t, tc.expected, res, "sum of coins is incorrect")
 		})
 	}
 }
@@ -611,24 +583,24 @@ func (s *coinTestSuite) TestAddCoins() {
 // are correctly coalesced. Please see issue https://github.com/cosmos/cosmos-sdk/issues/13234
 func TestCoinsAddCoalescesDuplicateDenominations(t *testing.T) {
 	A := sdk.Coins{
-		{"den", math.NewInt(2)},
-		{"den", math.NewInt(3)},
+		{"den", sdk.NewInt(2)},
+		{"den", sdk.NewInt(3)},
 	}
 	B := sdk.Coins{
-		{"den", math.NewInt(3)},
-		{"den", math.NewInt(2)},
-		{"den", math.NewInt(1)},
+		{"den", sdk.NewInt(3)},
+		{"den", sdk.NewInt(2)},
+		{"den", sdk.NewInt(1)},
 	}
 
 	A = A.Sort()
 	B = B.Sort()
 	got := A.Add(B...)
 
 	want := sdk.Coins{
-		{"den", math.NewInt(11)},
+		{"den", sdk.NewInt(11)},
 	}
 
-	if !got.Equal(want) {
+	if !got.IsEqual(want) {
 		t.Fatalf("Wrong result\n\tGot:   %s\n\tWant: %s", got, want)
 	}
 }