From a36c1a49306475e15abccb33398fde1b18aa16b7 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 14 Mar 2023 10:18:12 +0100 Subject: [PATCH 1/3] fix: add extra check in vesting (#15373) ## Description Closes: #XXXX --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 721c24a1ae08e8f8b2f3f70104236562859cc846) # Conflicts: # x/auth/testutil/expected_keepers_mocks.go # x/auth/vesting/msg_server_test.go --- x/auth/posthandler/tips.go | 9 +- x/auth/testutil/expected_keepers_mocks.go | 82 +++++++ x/auth/types/expected_keepers.go | 1 + x/auth/vesting/msg_server.go | 14 +- x/auth/vesting/msg_server_test.go | 260 ++++++++++++++++++++++ 5 files changed, 358 insertions(+), 8 deletions(-) create mode 100644 x/auth/testutil/expected_keepers_mocks.go create mode 100644 x/auth/vesting/msg_server_test.go 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 new file mode 100644 index 000000000000..ae7f6c5dfa90 --- /dev/null +++ b/x/auth/testutil/expected_keepers_mocks.go @@ -0,0 +1,82 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: x/auth/types/expected_keepers.go + +// Package testutil is a generated GoMock package. +package testutil + +import ( + reflect "reflect" + + types "github.com/cosmos/cosmos-sdk/types" + gomock "github.com/golang/mock/gomock" +) + +// MockBankKeeper is a mock of BankKeeper interface. +type MockBankKeeper struct { + ctrl *gomock.Controller + recorder *MockBankKeeperMockRecorder +} + +// MockBankKeeperMockRecorder is the mock recorder for MockBankKeeper. +type MockBankKeeperMockRecorder struct { + mock *MockBankKeeper +} + +// NewMockBankKeeper creates a new mock instance. +func NewMockBankKeeper(ctrl *gomock.Controller) *MockBankKeeper { + mock := &MockBankKeeper{ctrl: ctrl} + mock.recorder = &MockBankKeeperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +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() + ret := m.ctrl.Call(m, "SendCoins", ctx, from, to, amt) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendCoins indicates an expected call of SendCoins. +func (mr *MockBankKeeperMockRecorder) SendCoins(ctx, from, to, amt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoins", reflect.TypeOf((*MockBankKeeper)(nil).SendCoins), ctx, from, to, amt) +} + +// SendCoinsFromAccountToModule mocks base method. +func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx types.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SendCoinsFromAccountToModule", ctx, senderAddr, recipientModule, amt) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendCoinsFromAccountToModule indicates an expected call of SendCoinsFromAccountToModule. +func (mr *MockBankKeeperMockRecorder) SendCoinsFromAccountToModule(ctx, senderAddr, recipientModule, amt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromAccountToModule", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromAccountToModule), ctx, senderAddr, recipientModule, amt) +} 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 afcd2681e1bb..c3dd5785163d 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 } @@ -140,8 +139,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 } @@ -175,11 +173,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) @@ -200,8 +201,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 new file mode 100644 index 000000000000..f5e6fe58213a --- /dev/null +++ b/x/auth/vesting/msg_server_test.go @@ -0,0 +1,260 @@ +package vesting_test + +import ( + "testing" + "time" + + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + cmttime "github.com/cometbft/cometbft/types/time" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" + + storetypes "cosmossdk.io/store/types" + + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/auth/vesting" + vestingtestutil "github.com/cosmos/cosmos-sdk/x/auth/vesting/testutil" + vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" +) + +var ( + fromAddr = sdk.AccAddress([]byte("from1________________")) + to1Addr = sdk.AccAddress([]byte("to1__________________")) + to2Addr = sdk.AccAddress([]byte("to2__________________")) + to3Addr = sdk.AccAddress([]byte("to3__________________")) + fooCoin = sdk.NewInt64Coin("foo", 100) + periodCoin = sdk.NewInt64Coin("foo", 20) +) + +type VestingTestSuite struct { + suite.Suite + + ctx sdk.Context + accountKeeper authkeeper.AccountKeeper + bankKeeper *vestingtestutil.MockBankKeeper + msgServer vestingtypes.MsgServer +} + +func (s *VestingTestSuite) SetupTest() { + key := storetypes.NewKVStoreKey(authtypes.StoreKey) + testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) + s.ctx = testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) + encCfg := moduletestutil.MakeTestEncodingConfig() + + maccPerms := map[string][]string{} + + ctrl := gomock.NewController(s.T()) + s.bankKeeper = vestingtestutil.NewMockBankKeeper(ctrl) + s.accountKeeper = authkeeper.NewAccountKeeper( + encCfg.Codec, + key, + authtypes.ProtoBaseAccount, + maccPerms, + "cosmos", + authtypes.NewModuleAddress("gov").String(), + ) + + vestingtypes.RegisterInterfaces(encCfg.InterfaceRegistry) + authtypes.RegisterInterfaces(encCfg.InterfaceRegistry) + s.msgServer = vesting.NewMsgServerImpl(s.accountKeeper, s.bankKeeper) +} + +func (s *VestingTestSuite) TestCreateVestingAccount() { + testCases := map[string]struct { + preRun func() + input *vestingtypes.MsgCreateVestingAccount + expErr bool + expErrMsg string + }{ + "create for existing account": { + preRun: func() { + toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) + s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) + s.accountKeeper.SetAccount(s.ctx, toAcc) + s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(false) + }, + input: vestingtypes.NewMsgCreateVestingAccount( + fromAddr, + to1Addr, + sdk.Coins{fooCoin}, + time.Now().Unix(), + true, + ), + expErr: true, + expErrMsg: "already exists", + }, + "create a valid delayed vesting account": { + preRun: func() { + s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) + s.bankKeeper.EXPECT().BlockedAddr(to2Addr).Return(false) + s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to2Addr, sdk.Coins{fooCoin}).Return(nil) + }, + input: vestingtypes.NewMsgCreateVestingAccount( + fromAddr, + to2Addr, + sdk.Coins{fooCoin}, + time.Now().Unix(), + true, + ), + expErr: false, + expErrMsg: "", + }, + "create a valid continuous vesting account": { + preRun: func() { + s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) + s.bankKeeper.EXPECT().BlockedAddr(to3Addr).Return(false) + s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to3Addr, sdk.Coins{fooCoin}).Return(nil) + }, + input: vestingtypes.NewMsgCreateVestingAccount( + fromAddr, + to3Addr, + sdk.Coins{fooCoin}, + time.Now().Unix(), + false, + ), + expErr: false, + expErrMsg: "", + }, + } + + for name, tc := range testCases { + s.Run(name, func() { + tc.preRun() + _, err := s.msgServer.CreateVestingAccount(s.ctx, tc.input) + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.expErrMsg) + } else { + s.Require().NoError(err) + } + }) + } +} + +func (s *VestingTestSuite) TestCreatePermanentLockedAccount() { + testCases := map[string]struct { + preRun func() + input *vestingtypes.MsgCreatePermanentLockedAccount + expErr bool + expErrMsg string + }{ + "create for existing account": { + preRun: func() { + toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) + s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) + s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(false) + s.accountKeeper.SetAccount(s.ctx, toAcc) + }, + input: vestingtypes.NewMsgCreatePermanentLockedAccount( + fromAddr, + to1Addr, + sdk.Coins{fooCoin}, + ), + expErr: true, + expErrMsg: "already exists", + }, + "create a valid permanent locked account": { + preRun: func() { + s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) + s.bankKeeper.EXPECT().BlockedAddr(to2Addr).Return(false) + s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to2Addr, sdk.Coins{fooCoin}).Return(nil) + }, + input: vestingtypes.NewMsgCreatePermanentLockedAccount( + fromAddr, + to2Addr, + sdk.Coins{fooCoin}, + ), + expErr: false, + expErrMsg: "", + }, + } + + for name, tc := range testCases { + s.Run(name, func() { + tc.preRun() + _, err := s.msgServer.CreatePermanentLockedAccount(s.ctx, tc.input) + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.expErrMsg) + } else { + s.Require().NoError(err) + } + }) + } +} + +func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() { + testCases := []struct { + name string + preRun func() + input *vestingtypes.MsgCreatePeriodicVestingAccount + expErr bool + expErrMsg string + }{ + { + name: "create for existing account", + preRun: func() { + toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) + s.accountKeeper.SetAccount(s.ctx, toAcc) + }, + input: vestingtypes.NewMsgCreatePeriodicVestingAccount( + fromAddr, + to1Addr, + time.Now().Unix(), + []vestingtypes.Period{ + { + Length: 10, + Amount: sdk.NewCoins(periodCoin), + }, + }, + ), + expErr: true, + expErrMsg: "already exists", + }, + { + 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( + fromAddr, + to2Addr, + time.Now().Unix(), + []vestingtypes.Period{ + { + Length: 10, + Amount: sdk.NewCoins(periodCoin), + }, + { + Length: 20, + Amount: sdk.NewCoins(fooCoin), + }, + }, + ), + expErr: false, + expErrMsg: "", + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + tc.preRun() + _, err := s.msgServer.CreatePeriodicVestingAccount(s.ctx, tc.input) + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.expErrMsg) + } else { + s.Require().NoError(err) + } + }) + } +} + +func TestVestingTestSuite(t *testing.T) { + suite.Run(t, new(VestingTestSuite)) +} From 0c5ab5e324af703ff6cb0727587605a9b7e2a57d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 14 Mar 2023 10:25:19 +0100 Subject: [PATCH 2/3] fix conflicts --- x/auth/posthandler/tips.go | 9 +- x/auth/testutil/expected_keepers_mocks.go | 82 ------- x/auth/types/expected_keepers.go | 1 - x/auth/vesting/msg_server_test.go | 260 ---------------------- 4 files changed, 1 insertion(+), 351 deletions(-) delete mode 100644 x/auth/testutil/expected_keepers_mocks.go delete mode 100644 x/auth/vesting/msg_server_test.go diff --git a/x/auth/posthandler/tips.go b/x/auth/posthandler/tips.go index 33a5f69e9081..f956d53e5442 100644 --- a/x/auth/posthandler/tips.go +++ b/x/auth/posthandler/tips.go @@ -1,8 +1,6 @@ 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" @@ -49,10 +47,5 @@ func (d tipDecorator) transferTip(ctx sdk.Context, sdkTx sdk.Tx) error { return err } - 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) + return d.bankKeeper.SendCoins(ctx, tipper, tipTx.FeePayer(), tipTx.GetTip().Amount) } diff --git a/x/auth/testutil/expected_keepers_mocks.go b/x/auth/testutil/expected_keepers_mocks.go deleted file mode 100644 index ae7f6c5dfa90..000000000000 --- a/x/auth/testutil/expected_keepers_mocks.go +++ /dev/null @@ -1,82 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: x/auth/types/expected_keepers.go - -// Package testutil is a generated GoMock package. -package testutil - -import ( - reflect "reflect" - - types "github.com/cosmos/cosmos-sdk/types" - gomock "github.com/golang/mock/gomock" -) - -// MockBankKeeper is a mock of BankKeeper interface. -type MockBankKeeper struct { - ctrl *gomock.Controller - recorder *MockBankKeeperMockRecorder -} - -// MockBankKeeperMockRecorder is the mock recorder for MockBankKeeper. -type MockBankKeeperMockRecorder struct { - mock *MockBankKeeper -} - -// NewMockBankKeeper creates a new mock instance. -func NewMockBankKeeper(ctrl *gomock.Controller) *MockBankKeeper { - mock := &MockBankKeeper{ctrl: ctrl} - mock.recorder = &MockBankKeeperMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -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() - ret := m.ctrl.Call(m, "SendCoins", ctx, from, to, amt) - ret0, _ := ret[0].(error) - return ret0 -} - -// SendCoins indicates an expected call of SendCoins. -func (mr *MockBankKeeperMockRecorder) SendCoins(ctx, from, to, amt interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoins", reflect.TypeOf((*MockBankKeeper)(nil).SendCoins), ctx, from, to, amt) -} - -// SendCoinsFromAccountToModule mocks base method. -func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx types.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SendCoinsFromAccountToModule", ctx, senderAddr, recipientModule, amt) - ret0, _ := ret[0].(error) - return ret0 -} - -// SendCoinsFromAccountToModule indicates an expected call of SendCoinsFromAccountToModule. -func (mr *MockBankKeeperMockRecorder) SendCoinsFromAccountToModule(ctx, senderAddr, recipientModule, amt interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromAccountToModule", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromAccountToModule), ctx, senderAddr, recipientModule, amt) -} diff --git a/x/auth/types/expected_keepers.go b/x/auth/types/expected_keepers.go index b83b00aa1a92..7f242c63fd3a 100644 --- a/x/auth/types/expected_keepers.go +++ b/x/auth/types/expected_keepers.go @@ -6,7 +6,6 @@ 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_test.go b/x/auth/vesting/msg_server_test.go deleted file mode 100644 index f5e6fe58213a..000000000000 --- a/x/auth/vesting/msg_server_test.go +++ /dev/null @@ -1,260 +0,0 @@ -package vesting_test - -import ( - "testing" - "time" - - cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" - cmttime "github.com/cometbft/cometbft/types/time" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/suite" - - storetypes "cosmossdk.io/store/types" - - "github.com/cosmos/cosmos-sdk/testutil" - sdk "github.com/cosmos/cosmos-sdk/types" - moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" - authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/cosmos/cosmos-sdk/x/auth/vesting" - vestingtestutil "github.com/cosmos/cosmos-sdk/x/auth/vesting/testutil" - vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" -) - -var ( - fromAddr = sdk.AccAddress([]byte("from1________________")) - to1Addr = sdk.AccAddress([]byte("to1__________________")) - to2Addr = sdk.AccAddress([]byte("to2__________________")) - to3Addr = sdk.AccAddress([]byte("to3__________________")) - fooCoin = sdk.NewInt64Coin("foo", 100) - periodCoin = sdk.NewInt64Coin("foo", 20) -) - -type VestingTestSuite struct { - suite.Suite - - ctx sdk.Context - accountKeeper authkeeper.AccountKeeper - bankKeeper *vestingtestutil.MockBankKeeper - msgServer vestingtypes.MsgServer -} - -func (s *VestingTestSuite) SetupTest() { - key := storetypes.NewKVStoreKey(authtypes.StoreKey) - testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) - s.ctx = testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) - encCfg := moduletestutil.MakeTestEncodingConfig() - - maccPerms := map[string][]string{} - - ctrl := gomock.NewController(s.T()) - s.bankKeeper = vestingtestutil.NewMockBankKeeper(ctrl) - s.accountKeeper = authkeeper.NewAccountKeeper( - encCfg.Codec, - key, - authtypes.ProtoBaseAccount, - maccPerms, - "cosmos", - authtypes.NewModuleAddress("gov").String(), - ) - - vestingtypes.RegisterInterfaces(encCfg.InterfaceRegistry) - authtypes.RegisterInterfaces(encCfg.InterfaceRegistry) - s.msgServer = vesting.NewMsgServerImpl(s.accountKeeper, s.bankKeeper) -} - -func (s *VestingTestSuite) TestCreateVestingAccount() { - testCases := map[string]struct { - preRun func() - input *vestingtypes.MsgCreateVestingAccount - expErr bool - expErrMsg string - }{ - "create for existing account": { - preRun: func() { - toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) - s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) - s.accountKeeper.SetAccount(s.ctx, toAcc) - s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(false) - }, - input: vestingtypes.NewMsgCreateVestingAccount( - fromAddr, - to1Addr, - sdk.Coins{fooCoin}, - time.Now().Unix(), - true, - ), - expErr: true, - expErrMsg: "already exists", - }, - "create a valid delayed vesting account": { - preRun: func() { - s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) - s.bankKeeper.EXPECT().BlockedAddr(to2Addr).Return(false) - s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to2Addr, sdk.Coins{fooCoin}).Return(nil) - }, - input: vestingtypes.NewMsgCreateVestingAccount( - fromAddr, - to2Addr, - sdk.Coins{fooCoin}, - time.Now().Unix(), - true, - ), - expErr: false, - expErrMsg: "", - }, - "create a valid continuous vesting account": { - preRun: func() { - s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) - s.bankKeeper.EXPECT().BlockedAddr(to3Addr).Return(false) - s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to3Addr, sdk.Coins{fooCoin}).Return(nil) - }, - input: vestingtypes.NewMsgCreateVestingAccount( - fromAddr, - to3Addr, - sdk.Coins{fooCoin}, - time.Now().Unix(), - false, - ), - expErr: false, - expErrMsg: "", - }, - } - - for name, tc := range testCases { - s.Run(name, func() { - tc.preRun() - _, err := s.msgServer.CreateVestingAccount(s.ctx, tc.input) - if tc.expErr { - s.Require().Error(err) - s.Require().Contains(err.Error(), tc.expErrMsg) - } else { - s.Require().NoError(err) - } - }) - } -} - -func (s *VestingTestSuite) TestCreatePermanentLockedAccount() { - testCases := map[string]struct { - preRun func() - input *vestingtypes.MsgCreatePermanentLockedAccount - expErr bool - expErrMsg string - }{ - "create for existing account": { - preRun: func() { - toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) - s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) - s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(false) - s.accountKeeper.SetAccount(s.ctx, toAcc) - }, - input: vestingtypes.NewMsgCreatePermanentLockedAccount( - fromAddr, - to1Addr, - sdk.Coins{fooCoin}, - ), - expErr: true, - expErrMsg: "already exists", - }, - "create a valid permanent locked account": { - preRun: func() { - s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil) - s.bankKeeper.EXPECT().BlockedAddr(to2Addr).Return(false) - s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to2Addr, sdk.Coins{fooCoin}).Return(nil) - }, - input: vestingtypes.NewMsgCreatePermanentLockedAccount( - fromAddr, - to2Addr, - sdk.Coins{fooCoin}, - ), - expErr: false, - expErrMsg: "", - }, - } - - for name, tc := range testCases { - s.Run(name, func() { - tc.preRun() - _, err := s.msgServer.CreatePermanentLockedAccount(s.ctx, tc.input) - if tc.expErr { - s.Require().Error(err) - s.Require().Contains(err.Error(), tc.expErrMsg) - } else { - s.Require().NoError(err) - } - }) - } -} - -func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() { - testCases := []struct { - name string - preRun func() - input *vestingtypes.MsgCreatePeriodicVestingAccount - expErr bool - expErrMsg string - }{ - { - name: "create for existing account", - preRun: func() { - toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr) - s.accountKeeper.SetAccount(s.ctx, toAcc) - }, - input: vestingtypes.NewMsgCreatePeriodicVestingAccount( - fromAddr, - to1Addr, - time.Now().Unix(), - []vestingtypes.Period{ - { - Length: 10, - Amount: sdk.NewCoins(periodCoin), - }, - }, - ), - expErr: true, - expErrMsg: "already exists", - }, - { - 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( - fromAddr, - to2Addr, - time.Now().Unix(), - []vestingtypes.Period{ - { - Length: 10, - Amount: sdk.NewCoins(periodCoin), - }, - { - Length: 20, - Amount: sdk.NewCoins(fooCoin), - }, - }, - ), - expErr: false, - expErrMsg: "", - }, - } - - for _, tc := range testCases { - s.Run(tc.name, func() { - tc.preRun() - _, err := s.msgServer.CreatePeriodicVestingAccount(s.ctx, tc.input) - if tc.expErr { - s.Require().Error(err) - s.Require().Contains(err.Error(), tc.expErrMsg) - } else { - s.Require().NoError(err) - } - }) - } -} - -func TestVestingTestSuite(t *testing.T) { - suite.Run(t, new(VestingTestSuite)) -} From eecf548474e934140356486a8788d0c2f0520d0d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 14 Mar 2023 10:26:34 +0100 Subject: [PATCH 3/3] add changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96f0e0c451bb..3d62f194f5d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,8 +38,13 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] ### Improvements + * (simapp) [#15305](https://github.com/cosmos/cosmos-sdk/pull/15305) Add `AppStateFnWithExtendedCb` with callback function to extend rawState and `AppStateRandomizedFnWithState` with extra genesisState argument which is the genesis state of the app. +### Bug Fixes + +* (x/auth/vesting) [#15383](https://github.com/cosmos/cosmos-sdk/pull/15383) Add extra checks when creating a periodic vesting account. + ## [v0.46.11](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.11) - 2022-03-03 ### Improvements