From 56d6f062206ed4ac35afa88977d4aa18cbd161d9 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 15 Sep 2022 16:43:38 +0200 Subject: [PATCH] fix: types: correctly coalesce coins even with repeated denominations & simplify logic (backport #13265) (#13302) * fix: types: correctly coalesce coins even with repeated denominations & simplify logic (#13265) (cherry picked from commit 83f88a68208d0c80d525de1241820a3a77261c97) # Conflicts: # types/coin_test.go * fix conflict * add changelog Co-authored-by: Emmanuel T Odeke Co-authored-by: Julien Robert --- CHANGELOG.md | 1 + types/coin.go | 19 ++++++++-------- types/coin_test.go | 54 +++++++++++----------------------------------- 3 files changed, 23 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 029403245bd9..ace2e32e7141 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/types/coin.go b/types/coin.go index f7ae87b7056a..03ecd6345193 100644 --- a/types/coin.go +++ b/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) diff --git a/types/coin_test.go b/types/coin_test.go index 04a7d50189e6..96076f517bd3 100644 --- a/types/coin_test.go +++ b/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,13 +583,13 @@ 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() @@ -625,10 +597,10 @@ func TestCoinsAddCoalescesDuplicateDenominations(t *testing.T) { 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) } }