Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: barberry vesting, feegrant bug (backport) #3

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions x/auth/legacy/v043/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions x/auth/vesting/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions x/auth/vesting/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 11 additions & 1 deletion x/feegrant/filtered_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
82 changes: 82 additions & 0 deletions x/feegrant/filtered_fee_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}