diff --git a/CHANGELOG.md b/CHANGELOG.md index 92e1ba338a..8ac88f240c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [\#1301](https://github.com/Finschia/finschia-sdk/pull/1301) Use bytes instead of string comparison in delete validator queue (backport cosmos/cosmos-sdk#12303) * (client/keys) [#1312](https://github.com/Finschia/finschia-sdk/pull/1312) ignore error when key not found in `keys delete` * (store) [\#1310](https://github.com/Finschia/finschia-sdk/pull/1310) fix app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) +* (types) [\#1313](https://github.com/Finschia/finschia-sdk/pull/1313) fix correctly coalesce coins even with repeated denominations(backport cosmos/cosmos-sdk#13265) ### Removed diff --git a/types/coin.go b/types/coin.go index 1ff62a128a..1c86b46a9a 100644 --- a/types/coin.go +++ b/types/coin.go @@ -292,7 +292,7 @@ func (coins Coins) Add(coinsB ...Coin) Coins { // denomination and addition only occurs when the denominations match, otherwise // the coin is simply added to the sum assuming it's not zero. // The function panics if `coins` or `coinsB` are not sorted (ascending). -func (coins Coins) safeAdd(coinsB Coins) Coins { +func (coins Coins) safeAdd(coinsB Coins) (coalesced Coins) { // probably the best way will be to make Coins and interface and hide the structure // definition (type alias) if !coins.isSorted() { @@ -302,51 +302,24 @@ func (coins Coins) safeAdd(coinsB Coins) Coins { panic("Wrong argument: coins must be sorted") } - sum := ([]Coin)(nil) - indexA, indexB := 0, 0 - lenA, lenB := len(coins), len(coinsB) - - for { - if indexA == lenA { - if indexB == lenB { - // return nil coins if both sets are empty - return sum - } - - // return set B (excluding zero coins) if set A is empty - return append(sum, removeZeroCoins(coinsB[indexB:])...) - } else if indexB == lenB { - // return set A (excluding zero coins) if set B is empty - return append(sum, removeZeroCoins(coins[indexA:])...) + 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 { + uniqCoins[c.Denom] = append(uniqCoins[c.Denom], c) } + } - coinA, coinB := coins[indexA], coinsB[indexB] - - switch strings.Compare(coinA.Denom, coinB.Denom) { - case -1: // coin A denom < coin B denom - if !coinA.IsZero() { - sum = append(sum, coinA) - } - - indexA++ - - case 0: // coin A denom == coin B denom - res := coinA.Add(coinB) - if !res.IsZero() { - sum = append(sum, res) - } - - indexA++ - indexB++ - - case 1: // coin A denom > coin B denom - if !coinB.IsZero() { - sum = append(sum, coinB) - } - - indexB++ + 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) } } + return coalesced.Sort() } // DenomsSubsetOf returns true if receiver's denom set diff --git a/types/coin_test.go b/types/coin_test.go index 352ab331fb..4af7a46e00 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/Finschia/finschia-sdk/codec" @@ -417,21 +418,50 @@ func (s *coinTestSuite) TestEqualCoins() { func (s *coinTestSuite) TestAddCoins() { cases := []struct { + name string inputOne sdk.Coins inputTwo sdk.Coins expected sdk.Coins }{ - {sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca2, s.cm2}}, - {sdk.Coins{s.ca0, s.cm1}, sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.cm1}}, - {sdk.Coins{s.ca2}, sdk.Coins{s.cm0}, sdk.Coins{s.ca2}}, - {sdk.Coins{s.ca1}, sdk.Coins{s.ca1, s.cm2}, sdk.Coins{s.ca2, s.cm2}}, - {sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.ca0, s.cm0}, sdk.Coins(nil)}, + {"{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 tcIndex, tc := range cases { - res := tc.inputOne.Add(tc.inputTwo...) - s.Require().True(res.IsValid()) - s.Require().Equal(tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex) + 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, "sum of coins is incorrect") + }) + } +} + +// Tests that even if coins with repeated denominations are passed into .Add that they +// are correctly coalesced. Please see issue https://github.com/cosmos/cosmos-sdk/issues/13234 +func TestCoinsAddCoalescesDuplicateDenominations(t *testing.T) { + A := sdk.Coins{ + {"den", sdk.NewInt(2)}, + {"den", sdk.NewInt(3)}, + } + B := sdk.Coins{ + {"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", sdk.NewInt(11)}, + } + + if !got.Equal(want) { + t.Fatalf("Wrong result\n\tGot: %s\n\tWant: %s", got, want) } } diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index 9571568ba2..c25882316b 100644 --- a/x/auth/vesting/types/vesting_account.go +++ b/x/auth/vesting/types/vesting_account.go @@ -4,7 +4,7 @@ import ( "errors" "time" - yaml "gopkg.in/yaml.v2" + "gopkg.in/yaml.v2" cryptotypes "github.com/Finschia/finschia-sdk/crypto/types" sdk "github.com/Finschia/finschia-sdk/types" diff --git a/x/gov/types/params.go b/x/gov/types/params.go index 1f4111f8da..115bf413c0 100644 --- a/x/gov/types/params.go +++ b/x/gov/types/params.go @@ -4,7 +4,7 @@ import ( "fmt" "time" - yaml "gopkg.in/yaml.v2" + "gopkg.in/yaml.v2" sdk "github.com/Finschia/finschia-sdk/types" paramtypes "github.com/Finschia/finschia-sdk/x/params/types"