Skip to content

Commit

Permalink
fix: replace IsEqual with Equal (#14739)
Browse files Browse the repository at this point in the history 
Co-authored-by: Marko <marko@baricevic.me>
Co-authored-by: Marko <marbar3778@yahoo.com>
Closes #3246
  • Loading branch information
@EmilGeorgiev
EmilGeorgiev authored Jan 24, 2023
1 parent 6674402 commit e9fbb01
Show file tree
Hide file tree
Showing 28 changed files with 99 additions and 126 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf

### Bug Fixes

* (types/coin) [#14739](https://github.com/cosmos/cosmos-sdk/pull/14739) Deprecate the method `Coin.IsEqual` in favour of `Coin.Equal`. The difference between the two methods is that the first one results in a panic when denoms are not equal. This panic lead to unexpected behaviour
* (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
View file
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
View file
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]) {
return false
}
}
Expand Down
48 changes: 18 additions & 30 deletions types/coin_test.go
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit e9fbb01

Please sign in to comment.