From 7106134b05fa763030dfdc7ddd2e77cb974c8211 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 14 Mar 2023 09:57:02 +0000 Subject: [PATCH] fix: add extra check in vesting (backport #15373) (#15382) Co-authored-by: Julien Robert --- CHANGELOG.md | 1 + RELEASE_NOTES.md | 3 ++- x/auth/posthandler/tips.go | 9 ++++++++- x/auth/testutil/expected_keepers_mocks.go | 19 +++++++++++++++++++ x/auth/types/expected_keepers.go | 1 + x/auth/vesting/msg_server.go | 14 +++++++------- x/auth/vesting/msg_server_test.go | 14 +++++++++----- 7 files changed, 47 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e1021f2510b..b6c6a269b7ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -237,6 +237,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf ### Bug Fixes +* (x/auth/vesting) [#15373](https://github.com/cosmos/cosmos-sdk/pull/15373) Add extra checks when creating a periodic vesting account. * (x/auth) [#13838](https://github.com/cosmos/cosmos-sdk/pull/13838) Fix calling `String()` and `MarshalYAML` panics when pubkey is set on a `BaseAccount``. * (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) Fix evidence query API to decode the hash properly. * (bank) [#13691](https://github.com/cosmos/cosmos-sdk/issues/13691) Fix unhandled error for vesting account transfers, when total vesting amount exceeds total balance. diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index b919b52aaa60..3168fa56054f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -47,10 +47,11 @@ Refer to the [upgrading guide](https://github.com/cosmos/cosmos-sdk/blob/release * Binary Builders ([@binary_builders](https://twitter.com/binary_builders)) * Crypto.com ([@cronos_chain](https://twitter.com/cronos_chain)) -* Interchain GmbH ([@interchainio](https://twitter.com/interchainio)) +* Interchain GmbH ([@interchain_io](https://twitter.com/interchain_io)) * Notional ([@notionaldao](https://twitter.com/notionaldao)) * Osmosis ([@osmosiszone](https://twitter.com/osmosiszone)) * Regen Network ([@regen_network](https://twitter.com/RegenNetworkDev)) +* VitWit ([@vitwit_](https://twitter.com/vitwit_)) This list is non exhaustive and ordered alphabetically. Thank you to everyone who contributed to this release! diff --git a/x/auth/posthandler/tips.go b/x/auth/posthandler/tips.go index f956d53e5442..33a5f69e9081 100644 --- a/x/auth/posthandler/tips.go +++ b/x/auth/posthandler/tips.go @@ -1,6 +1,8 @@ package posthandler import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -47,5 +49,10 @@ func (d tipDecorator) transferTip(ctx sdk.Context, sdkTx sdk.Tx) error { return err } - return d.bankKeeper.SendCoins(ctx, tipper, tipTx.FeePayer(), tipTx.GetTip().Amount) + coins := tipTx.GetTip().Amount + if err := d.bankKeeper.IsSendEnabledCoins(ctx, coins...); err != nil { + return fmt.Errorf("cannot tip these coins: %w", err) + } + + return d.bankKeeper.SendCoins(ctx, tipper, tipTx.FeePayer(), coins) } diff --git a/x/auth/testutil/expected_keepers_mocks.go b/x/auth/testutil/expected_keepers_mocks.go index 97c2beed47e0..ae7f6c5dfa90 100644 --- a/x/auth/testutil/expected_keepers_mocks.go +++ b/x/auth/testutil/expected_keepers_mocks.go @@ -34,6 +34,25 @@ func (m *MockBankKeeper) EXPECT() *MockBankKeeperMockRecorder { return m.recorder } +// IsSendEnabledCoins mocks base method. +func (m *MockBankKeeper) IsSendEnabledCoins(ctx types.Context, coins ...types.Coin) error { + m.ctrl.T.Helper() + varargs := []interface{}{ctx} + for _, a := range coins { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "IsSendEnabledCoins", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// IsSendEnabledCoins indicates an expected call of IsSendEnabledCoins. +func (mr *MockBankKeeperMockRecorder) IsSendEnabledCoins(ctx interface{}, coins ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx}, coins...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsSendEnabledCoins", reflect.TypeOf((*MockBankKeeper)(nil).IsSendEnabledCoins), varargs...) +} + // SendCoins mocks base method. func (m *MockBankKeeper) SendCoins(ctx types.Context, from, to types.AccAddress, amt types.Coins) error { m.ctrl.T.Helper() diff --git a/x/auth/types/expected_keepers.go b/x/auth/types/expected_keepers.go index 7f242c63fd3a..b83b00aa1a92 100644 --- a/x/auth/types/expected_keepers.go +++ b/x/auth/types/expected_keepers.go @@ -6,6 +6,7 @@ import ( // BankKeeper defines the contract needed for supply related APIs (noalias) type BankKeeper interface { + IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error SendCoins(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.Coins) error SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error } diff --git a/x/auth/vesting/msg_server.go b/x/auth/vesting/msg_server.go index 2ec28589081c..33dd56861cb1 100644 --- a/x/auth/vesting/msg_server.go +++ b/x/auth/vesting/msg_server.go @@ -79,8 +79,7 @@ func (s msgServer) CreateVestingAccount(goCtx context.Context, msg *types.MsgCre } }() - err = bk.SendCoins(ctx, from, to, msg.Amount) - if err != nil { + if err = bk.SendCoins(ctx, from, to, msg.Amount); err != nil { return nil, err } @@ -133,8 +132,7 @@ func (s msgServer) CreatePermanentLockedAccount(goCtx context.Context, msg *type } }() - err = bk.SendCoins(ctx, from, to, msg.Amount) - if err != nil { + if err = bk.SendCoins(ctx, from, to, msg.Amount); err != nil { return nil, err } @@ -161,11 +159,14 @@ func (s msgServer) CreatePeriodicVestingAccount(goCtx context.Context, msg *type } var totalCoins sdk.Coins - for _, period := range msg.VestingPeriods { totalCoins = totalCoins.Add(period.Amount...) } + if err := bk.IsSendEnabledCoins(ctx, totalCoins...); err != nil { + return nil, err + } + baseAccount := authtypes.NewBaseAccountWithAddress(to) baseAccount = ak.NewAccount(ctx, baseAccount).(*authtypes.BaseAccount) vestingAccount := types.NewPeriodicVestingAccount(baseAccount, totalCoins.Sort(), msg.StartTime, msg.VestingPeriods) @@ -186,8 +187,7 @@ func (s msgServer) CreatePeriodicVestingAccount(goCtx context.Context, msg *type } }() - err = bk.SendCoins(ctx, from, to, totalCoins) - if err != nil { + if err = bk.SendCoins(ctx, from, to, totalCoins); err != nil { return nil, err } diff --git a/x/auth/vesting/msg_server_test.go b/x/auth/vesting/msg_server_test.go index 9219b9f0483d..18205f705b2a 100644 --- a/x/auth/vesting/msg_server_test.go +++ b/x/auth/vesting/msg_server_test.go @@ -186,13 +186,15 @@ func (s *VestingTestSuite) TestCreatePermanentLockedAccount() { } func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() { - testCases := map[string]struct { + testCases := []struct { + name string preRun func() input *vestingtypes.MsgCreatePeriodicVestingAccount expErr bool expErrMsg string }{ - "create for existing account": { + { + name: "create for existing account", preRun: func() { toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) s.accountKeeper.SetAccount(s.ctx, toAcc) @@ -211,8 +213,10 @@ func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() { expErr: true, expErrMsg: "already exists", }, - "create a valid periodic vesting account": { + { + name: "create a valid periodic vesting account", preRun: func() { + s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), periodCoin.Add(fooCoin)).Return(nil) s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to2Addr, gomock.Any()).Return(nil) }, input: vestingtypes.NewMsgCreatePeriodicVestingAccount( @@ -235,8 +239,8 @@ func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() { }, } - for name, tc := range testCases { - s.Run(name, func() { + for _, tc := range testCases { + s.Run(tc.name, func() { tc.preRun() _, err := s.msgServer.CreatePeriodicVestingAccount(s.ctx, tc.input) if tc.expErr {