fix: types: correctly coalesce coins even with repeated denominations & simplify logic (#13265)
This commit is contained in:
Emmanuel T Odeke
GitHub
parent
fd028db79a
commit
83f88a6820
@ -319,7 +319,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() {
|
||||
@ -329,51 +329,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:])...)
|
||||
}
|
||||
|
||||
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++
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
@ -6,6 +6,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"cosmossdk.io/math"
|
||||
"github.com/stretchr/testify/require"
|
||||
"github.com/stretchr/testify/suite"
|
||||
|
||||
"github.com/cosmos/cosmos-sdk/codec"
|
||||
@ -536,21 +537,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.IsEqual(want) {
|
||||
t.Fatalf("Wrong result\n\tGot: %s\n\tWant: %s", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user