Skip to content

Commit

Permalink
Merge pull request #3 from crescent-network/custom/backport-barberry-…
Browse files Browse the repository at this point in the history
…vesting-feegrant-bug

fix!: barberry vesting, feegrant bug (backport)
  • Loading branch information
crypin authored Jun 8, 2023
2 parents cd29fbb + d56a3cd commit d5670a7
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 1 deletion.
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)
}
})
}
}

0 comments on commit d5670a7

Please sign in to comment.