Skip to content

Commit

Permalink
fix: add extra check in vesting (backport #15373) (#15382)
Browse files Browse the repository at this point in the history
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
mergify[bot] and julienrbrt authored Mar 14, 2023
1 parent 3e6bf5c commit 7106134
Showing 7 changed files with 47 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 2 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -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!
9 changes: 8 additions & 1 deletion x/auth/posthandler/tips.go
Original file line number Diff line number Diff line change
@@ -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)
}
19 changes: 19 additions & 0 deletions x/auth/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions x/auth/types/expected_keepers.go
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 7 additions & 7 deletions x/auth/vesting/msg_server.go
Original file line number Diff line number Diff line change
@@ -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
}

14 changes: 9 additions & 5 deletions x/auth/vesting/msg_server_test.go
Original file line number Diff line number Diff line change
@@ -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 {

0 comments on commit 7106134

Please sign in to comment.