Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace IsEqual with Equal #14739

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf

### Bug Fixes

* (x/auth x/bank x/distribution x/gov x/slashing x/staking x/group) [#14739](https://github.com/cosmos/cosmos-sdk/pull/14739) Replace the method Coin.IsEqual with Coin.Equal. The main difference between the two methods is that the first one rise a panic when denoms are not equal. This panic lead to unexpected behavior
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
* (x/bank) [#14538](https://github.com/cosmos/cosmos-sdk/pull/14538) Validate denom in bank balances GRPC queries.
* (baseapp) [#14505](https://github.com/cosmos/cosmos-sdk/pull/14505) PrepareProposal and ProcessProposal now use deliverState for the first block in order to access changes made in InitChain.
* (server) [#14441](https://github.com/cosmos/cosmos-sdk/pull/14441) Fix `--log_format` flag not working.
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/distribution/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ func Test100PercentCommissionReward(t *testing.T) {
assert.NilError(t, err)

zeroRewards := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, math.ZeroInt())}
assert.Assert(t, rewards.IsEqual(zeroRewards))
assert.Assert(t, rewards.Equal(zeroRewards))

events := ctx.EventManager().Events()
lastEvent := events[len(events)-1]
Expand Down
17 changes: 7 additions & 10 deletions types/coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,9 @@ func (coin Coin) IsLTE(other Coin) bool {
}

// IsEqual returns true if the two sets of Coins have the same value
// Deprecated: Use Coin.Equal instead.
func (coin Coin) IsEqual(other Coin) bool {
if coin.Denom != other.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
}

return coin.Amount.Equal(other.Amount)
return coin.Equal(other)
}

// Add adds amounts of two coins with same denom. If the coins differ in denom then
Expand Down Expand Up @@ -463,7 +460,7 @@ func (coins Coins) SafeQuoInt(x Int) (Coins, bool) {
// a.IsAllLTE(a.Max(b))
// b.IsAllLTE(a.Max(b))
// a.IsAllLTE(c) && b.IsAllLTE(c) == a.Max(b).IsAllLTE(c)
// a.Add(b...).IsEqual(a.Min(b).Add(a.Max(b)...))
// a.Add(b...).Equal(a.Min(b).Add(a.Max(b)...))
//
// E.g.
// {1A, 3B, 2C}.Max({4A, 2B, 2C} == {4A, 3B, 2C})
Expand Down Expand Up @@ -509,7 +506,7 @@ func (coins Coins) Max(coinsB Coins) Coins {
// a.Min(b).IsAllLTE(a)
// a.Min(b).IsAllLTE(b)
// c.IsAllLTE(a) && c.IsAllLTE(b) == c.IsAllLTE(a.Min(b))
// a.Add(b...).IsEqual(a.Min(b).Add(a.Max(b)...))
// a.Add(b...).Equal(a.Min(b).Add(a.Max(b)...))
//
// E.g.
// {1A, 3B, 2C}.Min({4A, 2B, 2C} == {1A, 2B, 2C})
Expand Down Expand Up @@ -652,8 +649,8 @@ func (coins Coins) IsZero() bool {
return true
}

// IsEqual returns true if the two sets of Coins have the same value
func (coins Coins) IsEqual(coinsB Coins) bool {
// Equal returns true if the two sets of Coins have the same value
func (coins Coins) Equal(coinsB Coins) bool {
if len(coins) != len(coinsB) {
return false
}
Expand All @@ -662,7 +659,7 @@ func (coins Coins) IsEqual(coinsB Coins) bool {
coinsB = coinsB.Sort()

for i := 0; i < len(coins); i++ {
if !coins[i].IsEqual(coinsB[i]) {
if !coins[i].Equal(coinsB[i]) {
EmilGeorgiev marked this conversation as resolved.
Show resolved Hide resolved
return false
}
}
Expand Down
48 changes: 18 additions & 30 deletions types/coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,15 @@ func (s *coinTestSuite) TestIsEqualCoin() {
inputOne sdk.Coin
inputTwo sdk.Coin
expected bool
panics bool
}{
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom1, 1), true, false},
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom2, 1), false, true},
{sdk.NewInt64Coin("stake", 1), sdk.NewInt64Coin("stake", 10), false, false},
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom1, 1), true},
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom2, 1), false},
{sdk.NewInt64Coin("stake", 1), sdk.NewInt64Coin("stake", 10), false},
}

for tcIndex, tc := range cases {
tc := tc
if tc.panics {
s.Require().Panics(func() { tc.inputOne.IsEqual(tc.inputTwo) })
} else {
res := tc.inputOne.IsEqual(tc.inputTwo)
s.Require().Equal(tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex)
}
res := tc.inputOne.IsEqual(tc.inputTwo)
s.Require().Equal(tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex)
}
}

Expand Down Expand Up @@ -513,25 +507,19 @@ func (s *coinTestSuite) TestEqualCoins() {
inputOne sdk.Coins
inputTwo sdk.Coins
expected bool
panics bool
}{
{sdk.Coins{}, sdk.Coins{}, true, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, true, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, true, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom2, 0)}, false, true},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 1)}, false, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, false, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, true, false},
{sdk.Coins{}, sdk.Coins{}, true},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, true},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, true},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom2, 0)}, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 1)}, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, false},
{sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, sdk.Coins{sdk.NewInt64Coin(testDenom1, 0), sdk.NewInt64Coin(testDenom2, 1)}, true},
}

for tcnum, tc := range cases {
tc := tc
if tc.panics {
s.Require().Panics(func() { tc.inputOne.IsEqual(tc.inputTwo) })
} else {
res := tc.inputOne.IsEqual(tc.inputTwo)
s.Require().Equal(tc.expected, res, "Equality is differed from exported. tc #%d, expected %b, actual %b.", tcnum, tc.expected, res)
}
res := tc.inputOne.Equal(tc.inputTwo)
s.Require().Equal(tc.expected, res, "Equality is differed from exported. tc #%d, expected %b, actual %b.", tcnum, tc.expected, res)
}
}

Expand Down Expand Up @@ -579,7 +567,7 @@ func TestCoinsAddCoalescesDuplicateDenominations(t *testing.T) {
{"den", sdk.NewInt(11)},
}

if !got.IsEqual(want) {
if !got.Equal(want) {
t.Fatalf("Wrong result\n\tGot: %s\n\tWant: %s", got, want)
}
}
Expand Down Expand Up @@ -871,8 +859,8 @@ func (s *coinTestSuite) TestMinMax() {
for _, tc := range cases {
min := tc.input1.Min(tc.input2)
max := tc.input1.Max(tc.input2)
s.Require().True(min.IsEqual(tc.min), tc.name)
s.Require().True(max.IsEqual(tc.max), tc.name)
s.Require().True(min.Equal(tc.min), tc.name)
s.Require().True(max.Equal(tc.max), tc.name)
}
}

Expand Down Expand Up @@ -1174,7 +1162,7 @@ func (s *coinTestSuite) TestNewCoins() {
continue
}
got := sdk.NewCoins(tt.coins...)
s.Require().True(got.IsEqual(tt.want))
s.Require().True(got.Equal(tt.want))
}
}

Expand Down
11 changes: 4 additions & 7 deletions types/dec_coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,9 @@ func (coin DecCoin) IsLT(other DecCoin) bool {
}

// IsEqual returns true if the two sets of Coins have the same value.
// Deprecated: Use DecCoin.Equal instead.
func (coin DecCoin) IsEqual(other DecCoin) bool {
if coin.Denom != other.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
}

return coin.Amount.Equal(other.Amount)
return coin.Equal(other)
}

// Add adds amounts of two decimal coins with same denom.
Expand Down Expand Up @@ -483,8 +480,8 @@ func (coins DecCoins) AmountOf(denom string) Dec {
}
}

// IsEqual returns true if the two sets of DecCoins have the same value.
func (coins DecCoins) IsEqual(coinsB DecCoins) bool {
// Equal returns true if the two sets of DecCoins have the same value.
func (coins DecCoins) Equal(coinsB DecCoins) bool {
if len(coins) != len(coinsB) {
return false
}
Expand Down
44 changes: 17 additions & 27 deletions types/dec_coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ func (s *decCoinTestSuite) TestDecCoinsIntersect() {
s.Require().NoError(err, "unexpected parse error in %v", i)
exr, err := sdk.ParseDecCoins(tc.expectedResult)
s.Require().NoError(err, "unexpected parse error in %v", i)
s.Require().True(in1.Intersect(in2).IsEqual(exr), "in1.cap(in2) != exr in %v", i)
s.Require().True(in1.Intersect(in2).Equal(exr), "in1.cap(in2) != exr in %v", i)
}
}

Expand Down Expand Up @@ -1006,48 +1006,43 @@ func (s *decCoinTestSuite) TestDecCoin_IsEqual() {
coin sdk.DecCoin
otherCoin sdk.DecCoin
expectedResult bool
expectedPanic bool
}{
{
"Different Denom Same Amount",
sdk.DecCoin{testDenom1, math.LegacyNewDec(20)},
sdk.DecCoin{testDenom2, math.LegacyNewDec(20)},
false, true,
false,
},

{
"Different Denom Different Amount",
sdk.DecCoin{testDenom1, math.LegacyNewDec(20)},
sdk.DecCoin{testDenom2, math.LegacyNewDec(10)},
false, true,
false,
},

{
"Same Denom Different Amount",
sdk.DecCoin{testDenom1, math.LegacyNewDec(20)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(10)},
false, false,
false,
},

{
"Same Denom Same Amount",
sdk.DecCoin{testDenom1, math.LegacyNewDec(20)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(20)},
true, false,
true,
},
}

for i, tc := range testCases {
s.T().Run(tc.name, func(t *testing.T) {
if tc.expectedPanic {
s.Require().Panics(func() { tc.coin.IsEqual(tc.otherCoin) }, "Test case #%d: %s", i, tc.name)
res := tc.coin.IsEqual(tc.otherCoin)
if tc.expectedResult {
s.Require().True(res, "Test case #%d: %s", i, tc.name)
} else {
res := tc.coin.IsEqual(tc.otherCoin)
if tc.expectedResult {
s.Require().True(res, "Test case #%d: %s", i, tc.name)
} else {
s.Require().False(res, "Test case #%d: %s", i, tc.name)
}
s.Require().False(res, "Test case #%d: %s", i, tc.name)
}
})
}
Expand All @@ -1059,51 +1054,46 @@ func (s *decCoinTestSuite) TestDecCoins_IsEqual() {
coinsA sdk.DecCoins
coinsB sdk.DecCoins
expectedResult bool
expectedPanic bool
}{
{"Different length sets", sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(3)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(4)},
}, sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(35)},
}, false, false},
}, false},

{"Same length - different denoms", sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(3)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(4)},
}, sdk.DecCoins{
sdk.DecCoin{testDenom2, math.LegacyNewDec(3)},
sdk.DecCoin{testDenom2, math.LegacyNewDec(4)},
}, false, true},
}, false},

{"Same length - different amounts", sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(3)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(4)},
}, sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(41)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(343)},
}, false, false},
}, false},

{"Same length - same amounts", sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(33)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(344)},
}, sdk.DecCoins{
sdk.DecCoin{testDenom1, math.LegacyNewDec(33)},
sdk.DecCoin{testDenom1, math.LegacyNewDec(344)},
}, true, false},
}, true},
}

for i, tc := range testCases {
s.T().Run(tc.name, func(t *testing.T) {
if tc.expectedPanic {
s.Require().Panics(func() { tc.coinsA.IsEqual(tc.coinsB) }, "Test case #%d: %s", i, tc.name)
res := tc.coinsA.Equal(tc.coinsB)
if tc.expectedResult {
s.Require().True(res, "Test case #%d: %s", i, tc.name)
} else {
res := tc.coinsA.IsEqual(tc.coinsB)
if tc.expectedResult {
s.Require().True(res, "Test case #%d: %s", i, tc.name)
} else {
s.Require().False(res, "Test case #%d: %s", i, tc.name)
}
s.Require().False(res, "Test case #%d: %s", i, tc.name)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions x/auth/migrations/v2/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,8 +645,8 @@ func trackingCorrected(ctx sdk.Context, t *testing.T, ak keeper.AccountKeeper, a
vDA, ok := baseAccount.(exported.VestingAccount)
require.True(t, ok)

vestedOk := expDelVesting.IsEqual(vDA.GetDelegatedVesting())
freeOk := expDelFree.IsEqual(vDA.GetDelegatedFree())
vestedOk := expDelVesting.Equal(vDA.GetDelegatedVesting())
freeOk := expDelFree.Equal(vDA.GetDelegatedFree())
require.True(t, vestedOk, vDA.GetDelegatedVesting().String())
require.True(t, freeOk, vDA.GetDelegatedFree().String())
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/tx/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ func (w *wrapper) AddAuxSignerData(data tx.AuxSignerData) error {
}
}
if w.tx.AuthInfo.Tip != nil && data.SignDoc.Tip != nil {
if !w.tx.AuthInfo.Tip.Amount.IsEqual(data.SignDoc.Tip.Amount) {
if !w.tx.AuthInfo.Tip.Amount.Equal(data.SignDoc.Tip.Amount) {
return sdkerrors.ErrInvalidRequest.Wrapf("TxBuilder has tip %+v, got %+v in AuxSignerData", w.tx.AuthInfo.Tip.Amount, data.SignDoc.Tip.Amount)
}
if w.tx.AuthInfo.Tip.Tipper != data.SignDoc.Tip.Tipper {
Expand Down
2 changes: 1 addition & 1 deletion x/auth/vesting/types/vesting_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (pva PeriodicVestingAccount) Validate() error {
if endTime != pva.EndTime {
return errors.New("vesting end time does not match length of all vesting periods")
}
if !originalVesting.IsEqual(pva.OriginalVesting) {
if !originalVesting.Equal(pva.OriginalVesting) {
return errors.New("original vesting coins does not match the sum of all coins in vesting periods")
}

Expand Down
12 changes: 6 additions & 6 deletions x/auth/vesting/types/vesting_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func TestSpendableCoinsDelVestingAcc(t *testing.T) {
// schedule
dva := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix())
lockedCoins := dva.LockedCoins(now)
require.True(t, lockedCoins.IsEqual(origCoins))
require.True(t, lockedCoins.Equal(origCoins))

// require that all coins are spendable after the maturation of the vesting
// schedule
Expand All @@ -258,14 +258,14 @@ func TestSpendableCoinsDelVestingAcc(t *testing.T) {

// require that all coins are still vesting after some time
lockedCoins = dva.LockedCoins(now.Add(12 * time.Hour))
require.True(t, lockedCoins.IsEqual(origCoins))
require.True(t, lockedCoins.Equal(origCoins))

// delegate some locked coins
// require that locked is reduced
delegatedAmount := sdk.NewCoins(sdk.NewInt64Coin(stakeDenom, 50))
dva.TrackDelegation(now.Add(12*time.Hour), origCoins, delegatedAmount)
lockedCoins = dva.LockedCoins(now.Add(12 * time.Hour))
require.True(t, lockedCoins.IsEqual(origCoins.Sub(delegatedAmount...)))
require.True(t, lockedCoins.Equal(origCoins.Sub(delegatedAmount...)))
}

func TestTrackDelegationDelVestingAcc(t *testing.T) {
Expand Down Expand Up @@ -613,18 +613,18 @@ func TestSpendableCoinsPermLockedVestingAcc(t *testing.T) {
// schedule
plva := types.NewPermanentLockedAccount(bacc, origCoins)
lockedCoins := plva.LockedCoins(now)
require.True(t, lockedCoins.IsEqual(origCoins))
require.True(t, lockedCoins.Equal(origCoins))

// require that all coins are still locked at end time
lockedCoins = plva.LockedCoins(endTime)
require.True(t, lockedCoins.IsEqual(origCoins))
require.True(t, lockedCoins.Equal(origCoins))

// delegate some locked coins
// require that locked is reduced
delegatedAmount := sdk.NewCoins(sdk.NewInt64Coin(stakeDenom, 50))
plva.TrackDelegation(now.Add(12*time.Hour), origCoins, delegatedAmount)
lockedCoins = plva.LockedCoins(now.Add(12 * time.Hour))
require.True(t, lockedCoins.IsEqual(origCoins.Sub(delegatedAmount...)))
require.True(t, lockedCoins.Equal(origCoins.Sub(delegatedAmount...)))
}

func TestTrackDelegationPermLockedVestingAcc(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion x/bank/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func createTestSuite(t *testing.T, genesisAccounts []authtypes.GenesisAccount) s
// CheckBalance checks the balance of an account.
func checkBalance(t *testing.T, baseApp *baseapp.BaseApp, addr sdk.AccAddress, balances sdk.Coins, keeper bankkeeper.Keeper) {
ctxCheck := baseApp.NewContext(true, tmproto.Header{})
require.True(t, balances.IsEqual(keeper.GetAllBalances(ctxCheck, addr)))
require.True(t, balances.Equal(keeper.GetAllBalances(ctxCheck, addr)))
}

func TestSendNotEnoughBalance(t *testing.T) {
Expand Down
Loading