diff --git a/CHANGELOG.md b/CHANGELOG.md index d339721e00..882aeac143 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -252,6 +252,8 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf ### Bug Fixes +* (types/coin) [#14715](https://github.com/cosmos/cosmos-sdk/pull/14715) `sdk.Coins.Add` now returns an empty set of coins `sdk.Coins{}` if both coins set are empty. + * This is a behavior change, as previously `sdk.Coins.Add` would return `nil` in this case. * (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. diff --git a/types/coin.go b/types/coin.go index ca7d7d8c3e..79c0da1f16 100644 --- a/types/coin.go +++ b/types/coin.go @@ -340,6 +340,9 @@ func (coins Coins) safeAdd(coinsB Coins) (coalesced Coins) { coalesced = append(coalesced, comboCoin) } } + if coalesced == nil { + return Coins{} + } return coalesced.Sort() } diff --git a/types/coin_test.go b/types/coin_test.go index 0427dcdf4f..13a20eee6e 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -21,6 +21,7 @@ var ( type coinTestSuite struct { suite.Suite ca0, ca1, ca2, ca4, cm0, cm1, cm2, cm4 sdk.Coin + emptyCoins sdk.Coins } func TestCoinTestSuite(t *testing.T) { @@ -35,6 +36,7 @@ func (s *coinTestSuite) SetupSuite() { s.ca0, s.ca1, s.ca2, s.ca4 = sdk.NewCoin(testDenom1, zero), sdk.NewCoin(testDenom1, one), sdk.NewCoin(testDenom1, two), sdk.NewCoin(testDenom1, four) s.cm0, s.cm1, s.cm2, s.cm4 = sdk.NewCoin(testDenom2, zero), sdk.NewCoin(testDenom2, one), sdk.NewCoin(testDenom2, two), sdk.NewCoin(testDenom2, four) + s.emptyCoins = sdk.Coins{} } // ---------------------------------------------------------------------------- @@ -524,24 +526,56 @@ func (s *coinTestSuite) TestEqualCoins() { } func (s *coinTestSuite) TestAddCoins() { + cA0M0 := sdk.Coins{s.ca0, s.cm0} + cA0M1 := sdk.Coins{s.ca0, s.cm1} + cA1M1 := sdk.Coins{s.ca1, s.cm1} cases := []struct { name string inputOne sdk.Coins inputTwo sdk.Coins expected sdk.Coins + msg string }{ - {"{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)}, + {"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", + }, } 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") + require.Equal(t, tc.expected, res, tc.msg) }) } } @@ -573,12 +607,19 @@ func TestCoinsAddCoalescesDuplicateDenominations(t *testing.T) { } func (s *coinTestSuite) TestSubCoins() { + cA0M0 := sdk.Coins{s.ca0, s.cm0} + cA0M1 := sdk.Coins{s.ca0, s.cm1} testCases := []struct { inputOne sdk.Coins inputTwo sdk.Coins expected sdk.Coins shouldPanic bool }{ + {s.emptyCoins, s.emptyCoins, s.emptyCoins, false}, + {cA0M0, s.emptyCoins, s.emptyCoins, false}, + {cA0M0, sdk.Coins{s.cm0}, s.emptyCoins, false}, + {sdk.Coins{s.cm0}, cA0M0, s.emptyCoins, false}, + {cA0M1, s.emptyCoins, sdk.Coins{s.cm1}, false}, // denoms are not sorted - should panic {sdk.Coins{s.ca2}, sdk.Coins{s.cm2, s.ca1}, sdk.Coins{}, true}, {sdk.Coins{s.cm2, s.ca2}, sdk.Coins{s.ca1}, sdk.Coins{}, true}, diff --git a/x/auth/vesting/types/vesting_account_test.go b/x/auth/vesting/types/vesting_account_test.go index a96ef2d5c8..088395cd8f 100644 --- a/x/auth/vesting/types/vesting_account_test.go +++ b/x/auth/vesting/types/vesting_account_test.go @@ -25,6 +25,7 @@ import ( var ( stakeDenom = "stake" feeDenom = "fee" + emptyCoins = sdk.Coins{} ) type VestingAccountTestSuite struct { @@ -97,7 +98,7 @@ func TestGetVestingCoinsContVestingAcc(t *testing.T) { // require no coins vesting at the end of the vesting schedule vestingCoins = cva.GetVestingCoins(endTime) - require.Nil(t, vestingCoins) + require.Equal(t, emptyCoins, vestingCoins) // require 50% of coins vesting vestingCoins = cva.GetVestingCoins(now.Add(12 * time.Hour)) @@ -173,14 +174,14 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { cva.TrackDelegation(now, origCoins, origCoins) cva.TrackUndelegation(origCoins) require.Nil(t, cva.DelegatedFree) - require.Nil(t, cva.DelegatedVesting) + require.Equal(t, emptyCoins, cva.DelegatedVesting) // require the ability to undelegate all vested coins cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) cva.TrackDelegation(endTime, origCoins, origCoins) cva.TrackUndelegation(origCoins) - require.Nil(t, cva.DelegatedFree) + require.Equal(t, emptyCoins, cva.DelegatedFree) require.Nil(t, cva.DelegatedVesting) // require no modifications when the undelegation amount is zero @@ -204,7 +205,7 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { // undelegate from the other validator that did not get slashed cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) - require.Nil(t, cva.DelegatedFree) + require.Equal(t, emptyCoins, cva.DelegatedFree) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 25)}, cva.DelegatedVesting) } @@ -237,7 +238,7 @@ func TestGetVestingCoinsDelVestingAcc(t *testing.T) { // require no coins vesting at schedule maturation vestingCoins = dva.GetVestingCoins(endTime) - require.Nil(t, vestingCoins) + require.Equal(t, emptyCoins, vestingCoins) } func TestSpendableCoinsDelVestingAcc(t *testing.T) { @@ -315,13 +316,13 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { dva.TrackDelegation(now, origCoins, origCoins) dva.TrackUndelegation(origCoins) require.Nil(t, dva.DelegatedFree) - require.Nil(t, dva.DelegatedVesting) + require.Equal(t, emptyCoins, dva.DelegatedVesting) // require the ability to undelegate all vested coins dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) dva.TrackDelegation(endTime, origCoins, origCoins) dva.TrackUndelegation(origCoins) - require.Nil(t, dva.DelegatedFree) + require.Equal(t, emptyCoins, dva.DelegatedFree) require.Nil(t, dva.DelegatedVesting) // require no modifications when the undelegation amount is zero @@ -412,7 +413,7 @@ func TestGetVestingCoinsPeriodicVestingAcc(t *testing.T) { // require no coins vesting at the end of the vesting schedule vestingCoins = pva.GetVestingCoins(endTime) - require.Nil(t, vestingCoins) + require.Equal(t, emptyCoins, vestingCoins) // require 50% of coins vesting vestingCoins = pva.GetVestingCoins(now.Add(12 * time.Hour)) @@ -428,7 +429,7 @@ func TestGetVestingCoinsPeriodicVestingAcc(t *testing.T) { // require 0% of coins vesting after vesting complete vestingCoins = pva.GetVestingCoins(now.Add(48 * time.Hour)) - require.Nil(t, vestingCoins) + require.Equal(t, emptyCoins, vestingCoins) } func TestSpendableCoinsPeriodicVestingAcc(t *testing.T) { @@ -530,21 +531,21 @@ func TestTrackUndelegationPeriodicVestingAcc(t *testing.T) { pva.TrackDelegation(now, origCoins, origCoins) pva.TrackUndelegation(origCoins) require.Nil(t, pva.DelegatedFree) - require.Nil(t, pva.DelegatedVesting) + require.Equal(t, emptyCoins, pva.DelegatedVesting) // require the ability to undelegate all vested coins at the end of vesting pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) pva.TrackDelegation(endTime, origCoins, origCoins) pva.TrackUndelegation(origCoins) - require.Nil(t, pva.DelegatedFree) + require.Equal(t, emptyCoins, pva.DelegatedFree) require.Nil(t, pva.DelegatedVesting) // require the ability to undelegate half of coins pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) pva.TrackDelegation(endTime, origCoins, periods[0].Amount) pva.TrackUndelegation(periods[0].Amount) - require.Nil(t, pva.DelegatedFree) + require.Equal(t, emptyCoins, pva.DelegatedFree) require.Nil(t, pva.DelegatedVesting) // require no modifications when the undelegation amount is zero @@ -568,7 +569,7 @@ func TestTrackUndelegationPeriodicVestingAcc(t *testing.T) { // undelegate from the other validator that did not get slashed pva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) - require.Nil(t, pva.DelegatedFree) + require.Equal(t, emptyCoins, pva.DelegatedFree) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 25)}, pva.DelegatedVesting) } @@ -667,14 +668,14 @@ func TestTrackUndelegationPermLockedVestingAcc(t *testing.T) { plva.TrackDelegation(now, origCoins, origCoins) plva.TrackUndelegation(origCoins) require.Nil(t, plva.DelegatedFree) - require.Nil(t, plva.DelegatedVesting) + require.Equal(t, emptyCoins, plva.DelegatedVesting) // require the ability to undelegate all vesting coins at endTime plva = types.NewPermanentLockedAccount(bacc, origCoins) plva.TrackDelegation(endTime, origCoins, origCoins) plva.TrackUndelegation(origCoins) require.Nil(t, plva.DelegatedFree) - require.Nil(t, plva.DelegatedVesting) + require.Equal(t, emptyCoins, plva.DelegatedVesting) // require no modifications when the undelegation amount is zero plva = types.NewPermanentLockedAccount(bacc, origCoins) diff --git a/x/authz/simulation/operations.go b/x/authz/simulation/operations.go index ab2e0ac845..39172a1e5e 100644 --- a/x/authz/simulation/operations.go +++ b/x/authz/simulation/operations.go @@ -110,7 +110,7 @@ func SimulateMsgGrant(cdc *codec.ProtoCodec, ak authz.AccountKeeper, bk authz.Ba } spendLimit := spendableCoins.Sub(fees...) - if spendLimit == nil { + if len(spendLimit) == 0 { return simtypes.NoOpMsg(authz.ModuleName, TypeMsgGrant, "spend limit is nil"), nil, nil } diff --git a/x/bank/types/send_authorization.go b/x/bank/types/send_authorization.go index 81f5118f0f..ba2796fe9d 100644 --- a/x/bank/types/send_authorization.go +++ b/x/bank/types/send_authorization.go @@ -62,7 +62,7 @@ func (a SendAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRes // ValidateBasic implements Authorization.ValidateBasic. func (a SendAuthorization) ValidateBasic() error { - if a.SpendLimit == nil { + if len(a.SpendLimit) == 0 { return sdkerrors.ErrInvalidCoins.Wrap("spend limit cannot be nil") } if !a.SpendLimit.IsAllPositive() { diff --git a/x/feegrant/periodic_fee_test.go b/x/feegrant/periodic_fee_test.go index 0e229ea93a..ca2db8ad84 100644 --- a/x/feegrant/periodic_fee_test.go +++ b/x/feegrant/periodic_fee_test.go @@ -25,6 +25,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { leftAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 512)) oneAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 1)) eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 1)) + emptyCoins := sdk.Coins{} now := ctx.BlockTime() oneHour := now.Add(1 * time.Hour) @@ -61,11 +62,12 @@ func TestPeriodicFeeValidAllow(t *testing.T) { PeriodSpendLimit: smallAtom, PeriodReset: now.Add(30 * time.Minute), }, - blockTime: now, - valid: true, - accept: true, - remove: false, - periodReset: now.Add(30 * time.Minute), + blockTime: now, + valid: true, + accept: true, + remove: false, + remainsPeriod: emptyCoins, + periodReset: now.Add(30 * time.Minute), }, "mismatched currencies": { allow: feegrant.PeriodicAllowance{ @@ -94,7 +96,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { blockTime: now, accept: true, remove: false, - remainsPeriod: nil, + remainsPeriod: emptyCoins, remains: leftAtom, periodReset: now.Add(1 * time.Hour), }, @@ -113,7 +115,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { blockTime: now.Add(1 * time.Hour), accept: true, remove: false, - remainsPeriod: nil, + remainsPeriod: emptyCoins, remains: smallAtom, periodReset: oneHour.Add(tenMinutes), // one step from last reset, not now }, @@ -142,12 +144,13 @@ func TestPeriodicFeeValidAllow(t *testing.T) { PeriodReset: now, PeriodSpendLimit: atom, }, - valid: true, - fee: atom, - blockTime: oneHour, - accept: true, - remove: false, - periodReset: oneHour.Add(tenMinutes), // one step from last reset, not now + valid: true, + fee: atom, + blockTime: oneHour, + accept: true, + remove: false, + remainsPeriod: emptyCoins, + periodReset: oneHour.Add(tenMinutes), // one step from last reset, not now }, "expired": { allow: feegrant.PeriodicAllowance{