Merge pull request #302 from agoric-labs/jimlarson-barberry

fix: Validate coins when creating vesting schedules

x/auth/vesting/client/testutil/suite.go

@@ -181,13 +181,20 @@ func (s *IntegrationTestSuite) TestNewMsgCreatePeriodicVestingAccountCmd() {
 			},
 			expectErr: true,
 		},
-		"bad periods": {
+		"bad periods length": {
 			args: []string{
 				sdk.AccAddress("addr8_______________").String(),
 				"testdata/badperiod.json",
 			},
 			expectErr: true,
 		},
+		"bad periods amount": {
+			args: []string{
+				sdk.AccAddress("addr9_______________").String(),
+				"testdata/badperiod2.json",
+			},
+			expectErr: true,
+		},
 		"merge": {
 			args: []string{
 				sdk.AccAddress("addr9_______________").String(),
@@ -347,13 +354,21 @@ func (s *IntegrationTestSuite) TestNewMsgCreateClawbackVestingAccountCmd() {
 			expectErr: true,
 		},
 		{
-			name: "bad vesting periods",
+			name: "bad vesting periods length",
 			args: []string{
 				sdk.AccAddress("addr13______________").String(),
 				fmt.Sprintf("--%s=%s", cli.FlagVesting, "testdata/badperiod.json"),
 			},
 			expectErr: true,
 		},
+		{
+			name: "bad vesting periods amount",
+			args: []string{
+				sdk.AccAddress("addr13______________").String(),
+				fmt.Sprintf("--%s=%s", cli.FlagVesting, "testdata/badperiod2.json"),
+			},
+			expectErr: true,
+		},
 	} {
 		s.Run(tc.name, func() {
 			clientCtx := val.ClientCtx

x/auth/vesting/client/testutil/testdata/badperiod2.json

@@ -0,0 +1,9 @@
+{
+    "start_time": 1625204910,
+    "periods": [
+        {
+            "coins": "-10test",
+            "length_seconds": 500
+        }
+    ]
+}

x/auth/vesting/types/msgs.go

@@ -149,6 +149,9 @@ func (msg MsgCreatePeriodicVestingAccount) ValidateBasic() error {
 		if period.Length < 1 {
 			return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid period length of %d in period %d, length must be greater than 0", period.Length, i)
 		}
+		if !period.Amount.IsValid() {
+			return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid period amount in period %d: %s", i, err)
+		}
 	}
 
 	return nil
@@ -212,6 +215,9 @@ func (msg MsgCreateClawbackVestingAccount) ValidateBasic() error {
 		if period.Length < 1 {
 			return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid period length of %d in period %d, length must be greater than 0", period.Length, i)
 		}
+		if !period.Amount.IsValid() {
+			return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid period amount in period %d: %s", i, err)
+		}
 		lockupCoins = lockupCoins.Add(period.Amount...)
 	}
 
@@ -220,6 +226,9 @@ func (msg MsgCreateClawbackVestingAccount) ValidateBasic() error {
 		if period.Length < 1 {
 			return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid period length of %d in period %d, length must be greater than 0", period.Length, i)
 		}
+		if !period.Amount.IsValid() {
+			return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid period amount in period %d: %s", i, err)
+		}
 		vestingCoins = vestingCoins.Add(period.Amount...)
 	}
 

x/auth/vesting/types/msgs_test.go

@@ -3,6 +3,7 @@ package types
 import (
 	"testing"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
 )
 
@@ -82,6 +83,13 @@ func TestPeriodicVestingAccountMsg(t *testing.T) {
 	}, false)
 	err = badPeriods.ValidateBasic()
 	require.Error(t, err)
+
+	badPeriodAmount := NewMsgCreatePeriodicVestingAccount(fromAddr, toAddr, startTime, []Period{
+		{Length: 1, Amount: []sdk.Coin{{Denom: "atom", Amount: sdk.NewInt(1000)}}},
+		{Length: 1000, Amount: []sdk.Coin{{Denom: "atom", Amount: sdk.NewInt(-999)}}},
+	}, true)
+	err = badPeriodAmount.ValidateBasic()
+	require.Error(t, err)
 }
 
 func TestClawbackVestingAccountMsg(t *testing.T) {
@@ -146,6 +154,19 @@ func TestClawbackVestingAccountMsg(t *testing.T) {
 	noVestingOk := NewMsgCreateClawbackVestingAccount(fromAddr, toAddr, startTime, lockupPeriods, emptyPeriods, false)
 	err = noVestingOk.ValidateBasic()
 	require.NoError(t, err)
+
+	badPeriodsAmount := []Period{
+		{Length: 1, Amount: []sdk.Coin{{Denom: "atom", Amount: sdk.NewInt(20000000)}}},
+		{Length: 1000, Amount: []sdk.Coin{{Denom: "atom", Amount: sdk.NewInt(-10000000)}}},
+	}
+
+	negLockupEvent := NewMsgCreateClawbackVestingAccount(fromAddr, toAddr, startTime, badPeriodsAmount, vestingPeriods, false)
+	err = negLockupEvent.ValidateBasic()
+	require.Error(t, err)
+
+	negVestingEvent := NewMsgCreateClawbackVestingAccount(fromAddr, toAddr, startTime, lockupPeriods, badPeriodsAmount, false)
+	err = negVestingEvent.ValidateBasic()
+	require.Error(t, err)
 }
 
 func TestClawbackMsg(t *testing.T) {

x/auth/vesting/types/vesting_account_test.go

@@ -316,6 +316,35 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) {
 	require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 25)}, dva.DelegatedVesting)
 }
 
+func TestCreatePeriodicVestingAccBricked(t *testing.T) {
+	c := sdk.NewCoins
+	fee := func(amt int64) sdk.Coin { return sdk.NewInt64Coin(feeDenom, amt) }
+	stake := func(amt int64) sdk.Coin { return sdk.NewInt64Coin(stakeDenom, amt) }
+	now := tmtime.Now()
+
+	// set up simapp
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockTime((now))
+	require.Equal(t, "stake", app.StakingKeeper.BondDenom(ctx))
+
+	bogusPeriods := types.Periods{
+		{Length: 1, Amount: c(fee(10000), stake(100))},
+		{Length: 1000, Amount: []sdk.Coin{{Denom: feeDenom, Amount: sdk.NewInt(-9000)}}},
+	}
+	bacc, origCoins := initBaseAccount()
+	pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), bogusPeriods)
+	addr := pva.GetAddress()
+	app.AccountKeeper.SetAccount(ctx, pva)
+
+	err := simapp.FundAccount(app.BankKeeper, ctx, addr, c(fee(6000), stake(100)))
+	require.NoError(t, err)
+	require.Equal(t, int64(100), app.BankKeeper.GetBalance(ctx, addr, stakeDenom).Amount.Int64())
+
+	ctx = ctx.WithBlockTime(now.Add(160 * time.Second))
+	_, _, dest := testdata.KeyTestPubAddr()
+	require.Panics(t, func() { app.BankKeeper.SendCoins(ctx, addr, dest, c(fee(750))) })
+}
+
 func TestAddGrantPeriodicVestingAcc(t *testing.T) {
 	c := sdk.NewCoins
 	fee := func(amt int64) sdk.Coin { return sdk.NewInt64Coin(feeDenom, amt) }
@@ -439,6 +468,51 @@ func TestAddGrantPeriodicVestingAcc_FullSlash(t *testing.T) {
 	require.Equal(t, int64(10), pva.LockedCoins(ctx.WithBlockTime(now.Add(150*time.Second))).AmountOf(stakeDenom).Int64())
 }
 
+func TestAddGrantPeriodicVestingAcc_negAmount(t *testing.T) {
+	c := sdk.NewCoins
+	fee := func(amt int64) sdk.Coin { return sdk.NewInt64Coin(feeDenom, amt) }
+	stake := func(amt int64) sdk.Coin { return sdk.NewInt64Coin(stakeDenom, amt) }
+	now := tmtime.Now()
+
+	// set up simapp
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockTime((now))
+	require.Equal(t, "stake", app.StakingKeeper.BondDenom(ctx))
+
+	// create an account with an initial grant
+	periods := types.Periods{
+		{Length: 100, Amount: c(fee(250), stake(25))},
+		{Length: 100, Amount: c(fee(250), stake(25))},
+		{Length: 100, Amount: c(fee(250), stake(25))},
+		{Length: 100, Amount: c(fee(250), stake(25))},
+	}
+	bacc, origCoins := initBaseAccount()
+	pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods)
+	addr := pva.GetAddress()
+
+	// At now+150, add a new grant which attempts to prematurely vest the grant
+	bogusPeriods := types.Periods{
+		{Length: 1, Amount: c(fee(750))},
+		{Length: 1000, Amount: []sdk.Coin{{Denom: feeDenom, Amount: sdk.NewInt(-749)}}},
+	}
+	ctx = ctx.WithBlockTime(now.Add(150 * time.Second))
+	grantAction := types.NewPeriodicGrantAction(app.StakingKeeper, ctx.BlockTime().Unix(), bogusPeriods, c(fee(1)))
+	err := pva.AddGrant(ctx, grantAction)
+	require.NoError(t, err)
+	app.AccountKeeper.SetAccount(ctx, pva)
+
+	// fund the vesting account with new grant (old has vested and transferred out)
+	err = simapp.FundAccount(app.BankKeeper, ctx, addr, c(fee(1001), stake(100)))
+	require.NoError(t, err)
+	require.Equal(t, int64(100), app.BankKeeper.GetBalance(ctx, addr, stakeDenom).Amount.Int64())
+
+	// try to transfer the original grant before its time
+	ctx = ctx.WithBlockTime(now.Add(160 * time.Second))
+	_, _, dest := testdata.KeyTestPubAddr()
+	err = app.BankKeeper.SendCoins(ctx, addr, dest, c(fee(750)))
+	require.NoError(t, err)
+}
+
 func TestGetVestedCoinsPeriodicVestingAcc(t *testing.T) {
 	now := tmtime.Now()
 	endTime := now.Add(24 * time.Hour)