From 930bff826f4b81fcfc76a808259cf771263fd353 Mon Sep 17 00:00:00 2001 From: Assaf Morami Date: Thu, 11 May 2023 17:35:12 +0300 Subject: [PATCH 1/2] fix(feegrant): infinite feegrant bug (#16097) (cherry picked from commit d3370f231aaf4876649b4d88993b7ff3a3ff676d) (cherry picked from commit 5b1bd64aaad9a679d5bc3d63f0770a4c64781cf5) --- x/feegrant/filtered_fee.go | 12 ++++- x/feegrant/filtered_fee_test.go | 82 +++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 x/feegrant/filtered_fee_test.go diff --git a/x/feegrant/filtered_fee.go b/x/feegrant/filtered_fee.go index f83f3c5f6277..b9eaff18cbd0 100644 --- a/x/feegrant/filtered_fee.go +++ b/x/feegrant/filtered_fee.go @@ -63,7 +63,17 @@ func (a *AllowedMsgAllowance) Accept(ctx sdk.Context, fee sdk.Coins, msgs []sdk. return false, err } - return allowance.Accept(ctx, fee, msgs) + remove, err := allowance.Accept(ctx, fee, msgs) + if err != nil { + return false, err + } + + a.Allowance, err = types.NewAnyWithValue(allowance.(proto.Message)) + if err != nil { + return false, err + } + + return remove, nil } func (a *AllowedMsgAllowance) allowedMsgsToMap(ctx sdk.Context) map[string]bool { diff --git a/x/feegrant/filtered_fee_test.go b/x/feegrant/filtered_fee_test.go new file mode 100644 index 000000000000..a1be11fa65ab --- /dev/null +++ b/x/feegrant/filtered_fee_test.go @@ -0,0 +1,82 @@ +package feegrant_test + +import ( + "testing" + "time" + + bank "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/simapp" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/feegrant" +) + +func TestFilteredFeeValidAllow(t *testing.T) { + app := simapp.Setup(false) + + smallAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 488)) + bigAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 1000)) + leftAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 512)) + + basicAllowance, _ := codectypes.NewAnyWithValue(&feegrant.BasicAllowance{ + SpendLimit: bigAtom, + }) + + cases := map[string]struct { + allowance *feegrant.AllowedMsgAllowance + // all other checks are ignored if valid=false + fee sdk.Coins + blockTime time.Time + valid bool + accept bool + remove bool + remains sdk.Coins + }{ + "internal fee is updated": { + allowance: &feegrant.AllowedMsgAllowance{ + Allowance: basicAllowance, + AllowedMessages: []string{"/cosmos.bank.v1beta1.MsgSend"}, + }, + fee: smallAtom, + accept: true, + remove: false, + remains: leftAtom, + }, + } + + for name, stc := range cases { + tc := stc // to make scopelint happy + t.Run(name, func(t *testing.T) { + err := tc.allowance.ValidateBasic() + require.NoError(t, err) + + ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockTime(tc.blockTime) + + // now try to deduct + removed, err := tc.allowance.Accept(ctx, tc.fee, []sdk.Msg{ + &bank.MsgSend{ + FromAddress: "gm", + ToAddress: "gn", + Amount: tc.fee, + }, + }) + if !tc.accept { + require.Error(t, err) + return + } + require.NoError(t, err) + + require.Equal(t, tc.remove, removed) + if !removed { + var basicAllowanceLeft feegrant.BasicAllowance + app.AppCodec().Unmarshal(tc.allowance.Allowance.Value, &basicAllowanceLeft) + + assert.Equal(t, tc.remains, basicAllowanceLeft.SpendLimit) + } + }) + } +} From d56a3cd71e58b918056ea3ab201cced01c205b99 Mon Sep 17 00:00:00 2001 From: crypin Date: Fri, 9 Jun 2023 04:47:18 +0900 Subject: [PATCH 2/2] fix!: patch barberry vesting bug (backport) --- x/auth/legacy/v043/store_test.go | 2 ++ x/auth/vesting/msg_server.go | 8 ++++++++ x/auth/vesting/types/msgs.go | 8 ++++++++ 3 files changed, 18 insertions(+) diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go index 6fbece49e8ad..1a9fb88660ed 100644 --- a/x/auth/legacy/v043/store_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -332,6 +332,8 @@ func TestMigrateVestingAccounts(t *testing.T) { delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + ctx = ctx.WithBlockTime(time.Unix(1601042400, 0)) + app.AccountKeeper.SetAccount(ctx, delayedAccount) // delegation of the original vesting, failed because of no spendable balances diff --git a/x/auth/vesting/msg_server.go b/x/auth/vesting/msg_server.go index 5557b96ba4a3..66ceef463706 100644 --- a/x/auth/vesting/msg_server.go +++ b/x/auth/vesting/msg_server.go @@ -114,6 +114,10 @@ func (s msgServer) CreatePeriodicVestingAccount(goCtx context.Context, msg *type return nil, err } + if bk.BlockedAddr(to) { + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", msg.ToAddress) + } + if acc := ak.GetAccount(ctx, to); acc != nil { return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "account %s already exists", msg.ToAddress) } @@ -124,6 +128,10 @@ func (s msgServer) CreatePeriodicVestingAccount(goCtx context.Context, msg *type totalCoins = totalCoins.Add(period.Amount...) } + if err := bk.IsSendEnabledCoins(ctx, totalCoins...); err != nil { + return nil, err + } + baseAccount := ak.NewAccountWithAddress(ctx, to) acc := types.NewPeriodicVestingAccount(baseAccount.(*authtypes.BaseAccount), totalCoins.Sort(), msg.StartTime, msg.VestingPeriods) diff --git a/x/auth/vesting/types/msgs.go b/x/auth/vesting/types/msgs.go index 31c0fe53e927..1535f7880597 100644 --- a/x/auth/vesting/types/msgs.go +++ b/x/auth/vesting/types/msgs.go @@ -139,6 +139,14 @@ func (msg MsgCreatePeriodicVestingAccount) ValidateBasic() error { } for i, period := range msg.VestingPeriods { + if !period.Amount.IsValid() { + return sdkerrors.ErrInvalidCoins.Wrap(period.Amount.String()) + } + + if !period.Amount.IsAllPositive() { + return sdkerrors.ErrInvalidCoins.Wrap(period.Amount.String()) + } + if period.Length < 1 { return fmt.Errorf("invalid period length of %d in period %d, length must be greater than 0", period.Length, i) }