From adc7451ad7ba533af2e47815254cda53029cb1ce Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 29 Jun 2023 12:51:04 +0200 Subject: [PATCH] refactor(vesting)!: improve validate functions and return error in constructor (#16741) --- CHANGELOG.md | 3 +- .../auth/migrations/v2/store_test.go | 57 +++-- x/auth/simulation/genesis.go | 5 +- x/auth/vesting/msg_server.go | 18 +- x/auth/vesting/types/genesis_test.go | 3 +- x/auth/vesting/types/vesting_account.go | 63 ++++-- x/auth/vesting/types/vesting_account_test.go | 196 +++++++++++------- x/bank/keeper/grpc_query_test.go | 6 +- x/bank/keeper/keeper_test.go | 27 ++- x/genutil/genaccounts.go | 5 +- 10 files changed, 246 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f206ee94dcd1..f3d100a85d47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,7 +66,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * remove `Keeper`: `IterateValidatorOutstandingRewards`, `GetValidatorOutstandingRewards`, `SetValidatorOutstandingRewards`, `DeleteValidatorOutstandingRewards` * (x/distribution) [#16607](https://github.com/cosmos/cosmos-sdk/pull/16607) use collections for `ValidatorHistoricalRewards` state management: * remove `Keeper`: `IterateValidatorHistoricalRewards`, `GetValidatorHistoricalRewards`, `SetValidatorHistoricalRewards`, `DeleteValidatorHistoricalRewards`, `DeleteValidatorHistoricalReward`, `DeleteAllValidatorHistoricalRewards` -* (x/auth) [#16621](https://github.com/cosmos/cosmos-sdk/pull/16621) Pass address codec to auth new keeper constructor +* (x/auth) [#16621](https://github.com/cosmos/cosmos-sdk/pull/16621) Pass address codec to auth new keeper constructor. +* (x/auth/vesting) [#16741](https://github.com/cosmos/cosmos-sdk/pull/16741) Vesting account constructor now return an error with the result of their validate function. ### Bug Fixes diff --git a/tests/integration/auth/migrations/v2/store_test.go b/tests/integration/auth/migrations/v2/store_test.go index 24abfade89e1..32f628c93563 100644 --- a/tests/integration/auth/migrations/v2/store_test.go +++ b/tests/integration/auth/migrations/v2/store_test.go @@ -98,7 +98,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(200))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -127,7 +128,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) baseAccount := createBaseAccount(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(200))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -149,7 +151,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(200))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -175,7 +178,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(200))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) @@ -195,7 +199,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(200))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) @@ -219,7 +224,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) @@ -243,7 +249,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) @@ -263,7 +270,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -287,7 +295,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(300))) - delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + delayedAccount, err := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -311,7 +320,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(300))) - delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + delayedAccount, err := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -335,7 +345,8 @@ func TestMigrateVestingAccounts(t *testing.T) { bondDenom, err := stakingKeeper.BondDenom(ctx) require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdkmath.NewInt(300))) - delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + delayedAccount, err := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + require.NoError(t, err) ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) @@ -367,7 +378,8 @@ func TestMigrateVestingAccounts(t *testing.T) { }, } - account := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, start, periods) + account, err := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, start, periods) + require.NoError(t, err) accountKeeper.SetAccount(ctx, account) @@ -412,7 +424,8 @@ func TestMigrateVestingAccounts(t *testing.T) { }, } - delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + delayedAccount, err := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) @@ -458,7 +471,8 @@ func TestMigrateVestingAccounts(t *testing.T) { }, } - delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + delayedAccount, err := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + require.NoError(t, err) ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+15897600+15897600+1, 0)) @@ -506,7 +520,8 @@ func TestMigrateVestingAccounts(t *testing.T) { }, } - delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + delayedAccount, err := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + require.NoError(t, err) ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+1, 0)) @@ -554,7 +569,8 @@ func TestMigrateVestingAccounts(t *testing.T) { }, } - delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + delayedAccount, err := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + require.NoError(t, err) ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+15638400+1, 0)) @@ -578,7 +594,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdk.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) @@ -609,7 +626,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdk.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) }, @@ -627,7 +645,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) vestedCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, sdk.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + delayedAccount, err := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + require.NoError(t, err) accountKeeper.SetAccount(ctx, delayedAccount) }, diff --git a/x/auth/simulation/genesis.go b/x/auth/simulation/genesis.go index 1f9d45e55275..c6984cc355e8 100644 --- a/x/auth/simulation/genesis.go +++ b/x/auth/simulation/genesis.go @@ -47,7 +47,10 @@ func RandomGenesisAccounts(simState *module.SimulationState) types.GenesisAccoun endTime = int64(simulation.RandIntBetween(simState.Rand, int(startTime)+1, int(startTime+(60*60*12)))) } - bva := vestingtypes.NewBaseVestingAccount(bacc, initialVesting, endTime) + bva, err := vestingtypes.NewBaseVestingAccount(bacc, initialVesting, endTime) + if err != nil { + panic(err) + } if simState.Rand.Intn(100) < 50 { genesisAccs[i] = vestingtypes.NewContinuousVestingAccountRaw(bva, startTime) diff --git a/x/auth/vesting/msg_server.go b/x/auth/vesting/msg_server.go index 1471496171b6..7864f6f3951b 100644 --- a/x/auth/vesting/msg_server.go +++ b/x/auth/vesting/msg_server.go @@ -62,7 +62,10 @@ func (s msgServer) CreateVestingAccount(goCtx context.Context, msg *types.MsgCre baseAccount := authtypes.NewBaseAccountWithAddress(to) baseAccount = s.AccountKeeper.NewAccount(ctx, baseAccount).(*authtypes.BaseAccount) - baseVestingAccount := types.NewBaseVestingAccount(baseAccount, msg.Amount.Sort(), msg.EndTime) + baseVestingAccount, err := types.NewBaseVestingAccount(baseAccount, msg.Amount.Sort(), msg.EndTime) + if err != nil { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) + } var vestingAccount sdk.AccountI if msg.Delayed { @@ -124,7 +127,10 @@ func (s msgServer) CreatePermanentLockedAccount(goCtx context.Context, msg *type baseAccount := authtypes.NewBaseAccountWithAddress(to) baseAccount = s.AccountKeeper.NewAccount(ctx, baseAccount).(*authtypes.BaseAccount) - vestingAccount := types.NewPermanentLockedAccount(baseAccount, msg.Amount) + vestingAccount, err := types.NewPermanentLockedAccount(baseAccount, msg.Amount) + if err != nil { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) + } s.AccountKeeper.SetAccount(ctx, vestingAccount) @@ -188,11 +194,9 @@ func (s msgServer) CreatePeriodicVestingAccount(goCtx context.Context, msg *type baseAccount := authtypes.NewBaseAccountWithAddress(to) baseAccount = s.AccountKeeper.NewAccount(ctx, baseAccount).(*authtypes.BaseAccount) - vestingAccount := types.NewPeriodicVestingAccount(baseAccount, totalCoins.Sort(), msg.StartTime, msg.VestingPeriods) - - // Enforce and sanity check that we don't have any negative endTime. - if bva := vestingAccount.BaseVestingAccount; bva != nil && bva.EndTime < 0 { - return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "cumulative endtime is negative") + vestingAccount, err := types.NewPeriodicVestingAccount(baseAccount, totalCoins.Sort(), msg.StartTime, msg.VestingPeriods) + if err != nil { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) } s.AccountKeeper.SetAccount(ctx, vestingAccount) diff --git a/x/auth/vesting/types/genesis_test.go b/x/auth/vesting/types/genesis_test.go index 98aabb15cfde..31971396dd8f 100644 --- a/x/auth/vesting/types/genesis_test.go +++ b/x/auth/vesting/types/genesis_test.go @@ -21,7 +21,8 @@ var ( func TestValidateGenesisInvalidAccounts(t *testing.T) { acc1 := authtypes.NewBaseAccountWithAddress(sdk.AccAddress(addr1)) acc1Balance := sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 150)) - baseVestingAcc := NewBaseVestingAccount(acc1, acc1Balance, 1548775410) + baseVestingAcc, err := NewBaseVestingAccount(acc1, acc1Balance, 1548775410) + require.NoError(t, err) // invalid delegated vesting baseVestingAcc.DelegatedVesting = acc1Balance.Add(acc1Balance...) diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index 21bd35540914..f9cc0e227e81 100644 --- a/x/auth/vesting/types/vesting_account.go +++ b/x/auth/vesting/types/vesting_account.go @@ -25,14 +25,16 @@ var ( // NewBaseVestingAccount creates a new BaseVestingAccount object. It is the // callers responsibility to ensure the base account has sufficient funds with // regards to the original vesting amount. -func NewBaseVestingAccount(baseAccount *authtypes.BaseAccount, originalVesting sdk.Coins, endTime int64) *BaseVestingAccount { - return &BaseVestingAccount{ +func NewBaseVestingAccount(baseAccount *authtypes.BaseAccount, originalVesting sdk.Coins, endTime int64) (*BaseVestingAccount, error) { + baseVestingAccount := &BaseVestingAccount{ BaseAccount: baseAccount, OriginalVesting: originalVesting, DelegatedFree: sdk.NewCoins(), DelegatedVesting: sdk.NewCoins(), EndTime: endTime, } + + return baseVestingAccount, baseVestingAccount.Validate() } // LockedCoinsFromVesting returns all the coins that are not spendable (i.e. locked) @@ -145,9 +147,18 @@ func (bva BaseVestingAccount) GetEndTime() int64 { // Validate checks for errors on the account fields func (bva BaseVestingAccount) Validate() error { + if bva.EndTime < 0 { + return errors.New("end time cannot be negative") + } + + if !bva.OriginalVesting.IsValid() || !bva.OriginalVesting.IsAllPositive() { + return fmt.Errorf("invalid coins: %s", bva.OriginalVesting.String()) + } + if !(bva.DelegatedVesting.IsAllLTE(bva.OriginalVesting)) { return errors.New("delegated vesting amount cannot be greater than original vesting amount") } + return bva.BaseAccount.Validate() } @@ -167,17 +178,19 @@ func NewContinuousVestingAccountRaw(bva *BaseVestingAccount, startTime int64) *C } // NewContinuousVestingAccount returns a new ContinuousVestingAccount -func NewContinuousVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, startTime, endTime int64) *ContinuousVestingAccount { +func NewContinuousVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, startTime, endTime int64) (*ContinuousVestingAccount, error) { baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, OriginalVesting: originalVesting, EndTime: endTime, } - return &ContinuousVestingAccount{ + continuousVestingAccount := &ContinuousVestingAccount{ StartTime: startTime, BaseVestingAccount: baseVestingAcc, } + + return continuousVestingAccount, continuousVestingAccount.Validate() } // GetVestedCoins returns the total number of vested coins. If no coins are vested, @@ -258,28 +271,25 @@ func NewPeriodicVestingAccountRaw(bva *BaseVestingAccount, startTime int64, peri } // NewPeriodicVestingAccount returns a new PeriodicVestingAccount -func NewPeriodicVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, startTime int64, periods Periods) *PeriodicVestingAccount { +func NewPeriodicVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, startTime int64, periods Periods) (*PeriodicVestingAccount, error) { endTime := startTime - for i, p := range periods { - if p.Length < 0 { - panic(fmt.Sprintf("period #%d has a negative length: %d", i, p.Length)) - } + for _, p := range periods { endTime += p.Length } - if endTime < 0 || endTime < startTime { - panic("cumulative endTime overflowed, and/or is less than startTime") - } + baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, OriginalVesting: originalVesting, EndTime: endTime, } - return &PeriodicVestingAccount{ + periodicVestingAccount := &PeriodicVestingAccount{ BaseVestingAccount: baseVestingAcc, StartTime: startTime, VestingPeriods: periods, } + + return periodicVestingAccount, periodicVestingAccount.Validate() } // GetVestedCoins returns the total number of vested coins. If no coins are vested, @@ -352,15 +362,26 @@ func (pva PeriodicVestingAccount) Validate() error { } endTime := pva.StartTime originalVesting := sdk.NewCoins() - for _, p := range pva.VestingPeriods { + for i, p := range pva.VestingPeriods { + if p.Length < 0 { + return fmt.Errorf("period #%d has a negative length: %d", i, p.Length) + } endTime += p.Length + + if !p.Amount.IsValid() || !p.Amount.IsAllPositive() { + return fmt.Errorf("period #%d has invalid coins: %s", i, p.Amount.String()) + } + originalVesting = originalVesting.Add(p.Amount...) } if endTime != pva.EndTime { return errors.New("vesting end time does not match length of all vesting periods") } + if endTime < pva.GetStartTime() { + return errors.New("cumulative endTime overflowed, and/or is less than startTime") + } if !originalVesting.Equal(pva.OriginalVesting) { - return errors.New("original vesting coins does not match the sum of all coins in vesting periods") + return fmt.Errorf("original vesting coins (%v) does not match the sum of all coins in vesting periods (%v)", pva.OriginalVesting, originalVesting) } return pva.BaseVestingAccount.Validate() @@ -381,14 +402,16 @@ func NewDelayedVestingAccountRaw(bva *BaseVestingAccount) *DelayedVestingAccount } // NewDelayedVestingAccount returns a DelayedVestingAccount -func NewDelayedVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, endTime int64) *DelayedVestingAccount { +func NewDelayedVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, endTime int64) (*DelayedVestingAccount, error) { baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, OriginalVesting: originalVesting, EndTime: endTime, } - return &DelayedVestingAccount{baseVestingAcc} + delayedVestingAccount := &DelayedVestingAccount{baseVestingAcc} + + return delayedVestingAccount, delayedVestingAccount.Validate() } // GetVestedCoins returns the total amount of vested coins for a delayed vesting @@ -439,14 +462,16 @@ var ( ) // NewPermanentLockedAccount returns a PermanentLockedAccount -func NewPermanentLockedAccount(baseAcc *authtypes.BaseAccount, coins sdk.Coins) *PermanentLockedAccount { +func NewPermanentLockedAccount(baseAcc *authtypes.BaseAccount, coins sdk.Coins) (*PermanentLockedAccount, error) { baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, OriginalVesting: coins, EndTime: 0, // ensure EndTime is set to 0, as PermanentLockedAccount's do not have an EndTime } - return &PermanentLockedAccount{baseVestingAcc} + permanentLockedAccount := &PermanentLockedAccount{baseVestingAcc} + + return permanentLockedAccount, permanentLockedAccount.Validate() } // GetVestedCoins returns the total amount of vested coins for a permanent locked vesting diff --git a/x/auth/vesting/types/vesting_account_test.go b/x/auth/vesting/types/vesting_account_test.go index 6d8e91fb2468..74cdfe59c5a5 100644 --- a/x/auth/vesting/types/vesting_account_test.go +++ b/x/auth/vesting/types/vesting_account_test.go @@ -70,7 +70,8 @@ func TestGetVestedCoinsContVestingAcc(t *testing.T) { endTime := now.Add(24 * time.Hour) bacc, origCoins := initBaseAccount() - cva := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) // require no coins vested in the very beginning of the vesting schedule vestedCoins := cva.GetVestedCoins(now) @@ -94,7 +95,8 @@ func TestGetVestingCoinsContVestingAcc(t *testing.T) { endTime := now.Add(24 * time.Hour) bacc, origCoins := initBaseAccount() - cva := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) // require all coins vesting in the beginning of the vesting schedule vestingCoins := cva.GetVestingCoins(now) @@ -114,7 +116,8 @@ func TestSpendableCoinsContVestingAcc(t *testing.T) { endTime := now.Add(24 * time.Hour) bacc, origCoins := initBaseAccount() - cva := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) // require that all original coins are locked at the end of the vesting // schedule @@ -137,19 +140,22 @@ func TestTrackDelegationContVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to delegate all vesting coins - cva := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) cva.TrackDelegation(now, origCoins, origCoins) require.Equal(t, origCoins, cva.DelegatedVesting) require.Nil(t, cva.DelegatedFree) // require the ability to delegate all vested coins - cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) cva.TrackDelegation(endTime, origCoins, origCoins) require.Nil(t, cva.DelegatedVesting) require.Equal(t, origCoins, cva.DelegatedFree) // require the ability to delegate all vesting coins (50%) and all vested coins (50%) - cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) cva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, cva.DelegatedVesting) require.Nil(t, cva.DelegatedFree) @@ -159,7 +165,8 @@ func TestTrackDelegationContVestingAcc(t *testing.T) { require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, cva.DelegatedFree) // require no modifications when delegation amount is zero or not enough funds - cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) require.Panics(t, func() { cva.TrackDelegation(endTime, origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 1000000)}) }) @@ -174,23 +181,24 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to undelegate all vesting coins - cva := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err := types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) cva.TrackDelegation(now, origCoins, origCoins) cva.TrackUndelegation(origCoins) require.Nil(t, cva.DelegatedFree) require.Equal(t, emptyCoins, cva.DelegatedVesting) // require the ability to undelegate all vested coins - cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) - + cva, err = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) cva.TrackDelegation(endTime, origCoins, origCoins) cva.TrackUndelegation(origCoins) require.Equal(t, emptyCoins, cva.DelegatedFree) require.Nil(t, cva.DelegatedVesting) // require no modifications when the undelegation amount is zero - cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) - + cva, err = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) require.Panics(t, func() { cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(stakeDenom, 0)}) }) @@ -198,7 +206,8 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { require.Nil(t, cva.DelegatedVesting) // vest 50% and delegate to two validators - cva = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + cva, err = types.NewContinuousVestingAccount(bacc, origCoins, now.Unix(), endTime.Unix()) + require.NoError(t, err) cva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) cva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) @@ -220,7 +229,8 @@ func TestGetVestedCoinsDelVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require no coins are vested until schedule maturation - dva := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) vestedCoins := dva.GetVestedCoins(now) require.Nil(t, vestedCoins) @@ -236,7 +246,8 @@ func TestGetVestingCoinsDelVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require all coins vesting at the beginning of the schedule - dva := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) vestingCoins := dva.GetVestingCoins(now) require.Equal(t, origCoins, vestingCoins) @@ -253,7 +264,8 @@ func TestSpendableCoinsDelVestingAcc(t *testing.T) { // require that all coins are locked in the beginning of the vesting // schedule - dva := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) lockedCoins := dva.LockedCoins(now) require.True(t, lockedCoins.Equal(origCoins)) @@ -281,27 +293,30 @@ func TestTrackDelegationDelVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to delegate all vesting coins - dva := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) dva.TrackDelegation(now, origCoins, origCoins) require.Equal(t, origCoins, dva.DelegatedVesting) require.Nil(t, dva.DelegatedFree) // require the ability to delegate all vested coins - dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) dva.TrackDelegation(endTime, origCoins, origCoins) require.Nil(t, dva.DelegatedVesting) require.Equal(t, origCoins, dva.DelegatedFree) // require the ability to delegate all coins half way through the vesting // schedule - dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) dva.TrackDelegation(now.Add(12*time.Hour), origCoins, origCoins) require.Equal(t, origCoins, dva.DelegatedVesting) require.Nil(t, dva.DelegatedFree) // require no modifications when delegation amount is zero or not enough funds - dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) - + dva, err = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) require.Panics(t, func() { dva.TrackDelegation(endTime, origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 1000000)}) }) @@ -316,22 +331,24 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to undelegate all vesting coins - dva := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err := types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) dva.TrackDelegation(now, origCoins, origCoins) dva.TrackUndelegation(origCoins) require.Nil(t, dva.DelegatedFree) require.Equal(t, emptyCoins, dva.DelegatedVesting) // require the ability to undelegate all vested coins - dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) dva.TrackDelegation(endTime, origCoins, origCoins) dva.TrackUndelegation(origCoins) require.Equal(t, emptyCoins, dva.DelegatedFree) require.Nil(t, dva.DelegatedVesting) // require no modifications when the undelegation amount is zero - dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) - + dva, err = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) require.Panics(t, func() { dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(stakeDenom, 0)}) }) @@ -339,7 +356,8 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { require.Nil(t, dva.DelegatedVesting) // vest 50% and delegate to two validators - dva = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + dva, err = types.NewDelayedVestingAccount(bacc, origCoins, endTime.Unix()) + require.NoError(t, err) dva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) dva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) @@ -365,7 +383,8 @@ func TestGetVestedCoinsPeriodicVestingAcc(t *testing.T) { } bacc, origCoins := initBaseAccount() - pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) // require no coins vested at the beginning of the vesting schedule vestedCoins := pva.GetVestedCoins(now) @@ -402,9 +421,9 @@ func TestGetVestedCoinsPeriodicVestingAcc(t *testing.T) { func TestOverflowAndNegativeVestedCoinsPeriods(t *testing.T) { now := tmtime.Now() tests := []struct { - name string - periods []types.Period - wantPanic string + name string + periods []types.Period + wantErr string }{ { "negative .Length", @@ -420,13 +439,14 @@ func TestOverflowAndNegativeVestedCoinsPeriods(t *testing.T) { types.Period{Length: 9223372036854775108, Amount: sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}}, types.Period{Length: 6 * 60 * 60, Amount: sdk.Coins{sdk.NewInt64Coin(feeDenom, 250), sdk.NewInt64Coin(stakeDenom, 25)}}, }, - "cumulative endTime overflowed, and/or is less than startTime", + "vesting start-time cannot be before end-time", // it overflow to a negative number, making start-time > end-time }, { "good periods that are not negative nor overflow", types.Periods{ types.Period{Length: now.Unix() - 1000, Amount: sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}}, types.Period{Length: 60, Amount: sdk.Coins{sdk.NewInt64Coin(feeDenom, 250), sdk.NewInt64Coin(stakeDenom, 25)}}, + types.Period{Length: 30, Amount: sdk.Coins{sdk.NewInt64Coin(feeDenom, 250), sdk.NewInt64Coin(stakeDenom, 25)}}, }, "", }, @@ -435,24 +455,13 @@ func TestOverflowAndNegativeVestedCoinsPeriods(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { bacc, origCoins := initBaseAccount() - defer func() { - r := recover() - if r == nil { - if tt.wantPanic != "" { - t.Fatalf("expected a panic with substring: %q", tt.wantPanic) - } - return - } - - // Otherwise ensure we match the panic substring. - require.Contains(t, r, tt.wantPanic) - }() - - pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), tt.periods) - if pva == nil { + pva, err := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), tt.periods) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) return } + require.NoError(t, err) if pbva := pva.BaseVestingAccount; pbva.EndTime < 0 { t.Fatalf("Unfortunately we still have negative .EndTime :-(: %d", pbva.EndTime) } @@ -470,7 +479,8 @@ func TestGetVestingCoinsPeriodicVestingAcc(t *testing.T) { } bacc, origCoins := initBaseAccount() - pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) // require all coins vesting at the beginning of the vesting schedule vestingCoins := pva.GetVestingCoins(now) @@ -507,7 +517,8 @@ func TestSpendableCoinsPeriodicVestingAcc(t *testing.T) { } bacc, origCoins := initBaseAccount() - pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) // require that there exist no spendable coins at the beginning of the // vesting schedule @@ -536,33 +547,38 @@ func TestTrackDelegationPeriodicVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to delegate all vesting coins - pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(now, origCoins, origCoins) require.Equal(t, origCoins, pva.DelegatedVesting) require.Nil(t, pva.DelegatedFree) // require the ability to delegate all vested coins - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(endTime, origCoins, origCoins) require.Nil(t, pva.DelegatedVesting) require.Equal(t, origCoins, pva.DelegatedFree) // delegate half of vesting coins - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(now, origCoins, periods[0].Amount) // require that all delegated coins are delegated vesting require.Equal(t, pva.DelegatedVesting, periods[0].Amount) require.Nil(t, pva.DelegatedFree) // delegate 75% of coins, split between vested and vesting - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(now.Add(12*time.Hour), origCoins, periods[0].Amount.Add(periods[1].Amount...)) // require that the maximum possible amount of vesting coins are chosen for delegation. require.Equal(t, pva.DelegatedFree, periods[1].Amount) require.Equal(t, pva.DelegatedVesting, periods[0].Amount) // require the ability to delegate all vesting coins (50%) and all vested coins (50%) - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, pva.DelegatedVesting) require.Nil(t, pva.DelegatedFree) @@ -572,7 +588,8 @@ func TestTrackDelegationPeriodicVestingAcc(t *testing.T) { require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, pva.DelegatedFree) // require no modifications when delegation amount is zero or not enough funds - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) require.Panics(t, func() { pva.TrackDelegation(endTime, origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 1000000)}) }) @@ -592,30 +609,32 @@ func TestTrackUndelegationPeriodicVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to undelegate all vesting coins at the beginning of vesting - pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(now, origCoins, origCoins) pva.TrackUndelegation(origCoins) require.Nil(t, pva.DelegatedFree) require.Equal(t, emptyCoins, pva.DelegatedVesting) // require the ability to undelegate all vested coins at the end of vesting - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) - + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(endTime, origCoins, origCoins) pva.TrackUndelegation(origCoins) require.Equal(t, emptyCoins, pva.DelegatedFree) require.Nil(t, pva.DelegatedVesting) // require the ability to undelegate half of coins - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(endTime, origCoins, periods[0].Amount) pva.TrackUndelegation(periods[0].Amount) require.Equal(t, emptyCoins, pva.DelegatedFree) require.Nil(t, pva.DelegatedVesting) // require no modifications when the undelegation amount is zero - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) - + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) require.Panics(t, func() { pva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(stakeDenom, 0)}) }) @@ -623,7 +642,8 @@ func TestTrackUndelegationPeriodicVestingAcc(t *testing.T) { require.Nil(t, pva.DelegatedVesting) // vest 50% and delegate to two validators - pva = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + pva, err = types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + require.NoError(t, err) pva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) pva.TrackDelegation(now.Add(12*time.Hour), origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) @@ -645,7 +665,8 @@ func TestGetVestedCoinsPermLockedVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require no coins are vested - plva := types.NewPermanentLockedAccount(bacc, origCoins) + plva, err := types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) vestedCoins := plva.GetVestedCoins(now) require.Nil(t, vestedCoins) @@ -661,7 +682,8 @@ func TestGetVestingCoinsPermLockedVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require all coins vesting at the beginning of the schedule - plva := types.NewPermanentLockedAccount(bacc, origCoins) + plva, err := types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) vestingCoins := plva.GetVestingCoins(now) require.Equal(t, origCoins, vestingCoins) @@ -678,7 +700,8 @@ func TestSpendableCoinsPermLockedVestingAcc(t *testing.T) { // require that all coins are locked in the beginning of the vesting // schedule - plva := types.NewPermanentLockedAccount(bacc, origCoins) + plva, err := types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) lockedCoins := plva.LockedCoins(now) require.True(t, lockedCoins.Equal(origCoins)) @@ -701,20 +724,22 @@ func TestTrackDelegationPermLockedVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to delegate all vesting coins - plva := types.NewPermanentLockedAccount(bacc, origCoins) + plva, err := types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) plva.TrackDelegation(now, origCoins, origCoins) require.Equal(t, origCoins, plva.DelegatedVesting) require.Nil(t, plva.DelegatedFree) // require the ability to delegate all vested coins at endTime - plva = types.NewPermanentLockedAccount(bacc, origCoins) + plva, err = types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) plva.TrackDelegation(endTime, origCoins, origCoins) require.Equal(t, origCoins, plva.DelegatedVesting) require.Nil(t, plva.DelegatedFree) // require no modifications when delegation amount is zero or not enough funds - plva = types.NewPermanentLockedAccount(bacc, origCoins) - + plva, err = types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) require.Panics(t, func() { plva.TrackDelegation(endTime, origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 1000000)}) }) @@ -729,21 +754,24 @@ func TestTrackUndelegationPermLockedVestingAcc(t *testing.T) { bacc, origCoins := initBaseAccount() // require the ability to undelegate all vesting coins - plva := types.NewPermanentLockedAccount(bacc, origCoins) + plva, err := types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) plva.TrackDelegation(now, origCoins, origCoins) plva.TrackUndelegation(origCoins) require.Nil(t, plva.DelegatedFree) require.Equal(t, emptyCoins, plva.DelegatedVesting) // require the ability to undelegate all vesting coins at endTime - plva = types.NewPermanentLockedAccount(bacc, origCoins) + plva, err = types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) plva.TrackDelegation(endTime, origCoins, origCoins) plva.TrackUndelegation(origCoins) require.Nil(t, plva.DelegatedFree) require.Equal(t, emptyCoins, plva.DelegatedVesting) // require no modifications when the undelegation amount is zero - plva = types.NewPermanentLockedAccount(bacc, origCoins) + plva, err = types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) require.Panics(t, func() { plva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(stakeDenom, 0)}) }) @@ -751,7 +779,8 @@ func TestTrackUndelegationPermLockedVestingAcc(t *testing.T) { require.Nil(t, plva.DelegatedVesting) // delegate to two validators - plva = types.NewPermanentLockedAccount(bacc, origCoins) + plva, err = types.NewPermanentLockedAccount(bacc, origCoins) + require.NoError(t, err) plva.TrackDelegation(now, origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) plva.TrackDelegation(now, origCoins, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) @@ -772,7 +801,8 @@ func TestGenesisAccountValidate(t *testing.T) { addr := sdk.AccAddress(pubkey.Address()) baseAcc := authtypes.NewBaseAccount(addr, pubkey, 0, 0) initialVesting := sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 50)) - baseVestingWithCoins := types.NewBaseVestingAccount(baseAcc, initialVesting, 100) + baseVestingWithCoins, err := types.NewBaseVestingAccount(baseAcc, initialVesting, 100) + require.NoError(t, err) tests := []struct { name string acc authtypes.GenesisAccount @@ -795,17 +825,26 @@ func TestGenesisAccountValidate(t *testing.T) { }, { "valid continuous vesting account", - types.NewContinuousVestingAccount(baseAcc, initialVesting, 100, 200), + func() authtypes.GenesisAccount { + acc, _ := types.NewContinuousVestingAccount(baseAcc, initialVesting, 100, 200) + return acc + }(), false, }, { "invalid vesting times", - types.NewContinuousVestingAccount(baseAcc, initialVesting, 1654668078, 1554668078), + func() authtypes.GenesisAccount { + acc, _ := types.NewContinuousVestingAccount(baseAcc, initialVesting, 1654668078, 1554668078) + return acc + }(), true, }, { "valid periodic vesting account", - types.NewPeriodicVestingAccount(baseAcc, initialVesting, 0, types.Periods{types.Period{Length: int64(100), Amount: sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 50)}}}), + func() authtypes.GenesisAccount { + acc, _ := types.NewPeriodicVestingAccount(baseAcc, initialVesting, 0, types.Periods{types.Period{Length: int64(100), Amount: sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 50)}}}) + return acc + }(), false, }, { @@ -824,7 +863,10 @@ func TestGenesisAccountValidate(t *testing.T) { }, { "valid permanent locked vesting account", - types.NewPermanentLockedAccount(baseAcc, initialVesting), + func() authtypes.GenesisAccount { + acc, _ := types.NewPermanentLockedAccount(baseAcc, initialVesting) + return acc + }(), false, }, { diff --git a/x/bank/keeper/grpc_query_test.go b/x/bank/keeper/grpc_query_test.go index 2bb6bbc3e301..2a7585e5f509 100644 --- a/x/bank/keeper/grpc_query_test.go +++ b/x/bank/keeper/grpc_query_test.go @@ -147,12 +147,13 @@ func (suite *KeeperTestSuite) TestSpendableBalances() { barCoins := newBarCoin(30) origCoins := sdk.NewCoins(fooCoins, barCoins) - vacc := vestingtypes.NewContinuousVestingAccount( + vacc, err := vestingtypes.NewContinuousVestingAccount( acc, sdk.NewCoins(fooCoins), ctx.BlockTime().Unix(), ctx.BlockTime().Add(time.Hour).Unix(), ) + suite.Require().NoError(err) suite.mockFundAccount(addr) suite.Require().NoError(testutil.FundAccount(suite.ctx, suite.bankKeeper, addr, origCoins)) @@ -194,12 +195,13 @@ func (suite *KeeperTestSuite) TestSpendableBalanceByDenom() { barCoins := newBarCoin(30) origCoins := sdk.NewCoins(fooCoins, barCoins) - vacc := vestingtypes.NewContinuousVestingAccount( + vacc, err := vestingtypes.NewContinuousVestingAccount( acc, sdk.NewCoins(fooCoins), ctx.BlockTime().Unix(), ctx.BlockTime().Add(time.Hour).Unix(), ) + suite.Require().NoError(err) suite.mockFundAccount(addr) suite.Require().NoError(testutil.FundAccount(suite.ctx, suite.bankKeeper, addr, origCoins)) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 425c1a3a6f1e..bf2c9ae1228f 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -692,7 +692,8 @@ func (suite *KeeperTestSuite) TestSendCoins_Invalid_SendLockedCoins() { sendCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 50)) acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) - vacc := vesting.NewContinuousVestingAccount(acc0, origCoins, now.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc0, origCoins, now.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[1]) suite.Require().NoError(banktestutil.FundAccount(suite.ctx, suite.bankKeeper, accAddrs[1], balances)) @@ -719,7 +720,8 @@ func (suite *KeeperTestSuite) TestValidateBalance() { require.NoError(suite.bankKeeper.ValidateBalance(ctx, accAddrs[0])) acc1 := authtypes.NewBaseAccountWithAddress(accAddrs[1]) - vacc := vesting.NewContinuousVestingAccount(acc1, balances.Add(balances...), now.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc1, balances.Add(balances...), now.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[1]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[1], balances)) @@ -930,7 +932,8 @@ func (suite *KeeperTestSuite) TestSpendableCoins() { acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) acc1 := authtypes.NewBaseAccountWithAddress(accAddrs[1]) - vacc := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) @@ -962,7 +965,8 @@ func (suite *KeeperTestSuite) TestVestingAccountSend() { sendCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 50)) acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) - vacc := vesting.NewContinuousVestingAccount(acc0, origCoins, now.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc0, origCoins, now.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) @@ -995,7 +999,8 @@ func (suite *KeeperTestSuite) TestPeriodicVestingAccountSend() { } acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) - vacc := vesting.NewPeriodicVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), periods) + vacc, err := vesting.NewPeriodicVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), periods) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) @@ -1026,7 +1031,8 @@ func (suite *KeeperTestSuite) TestVestingAccountReceive() { acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) acc1 := authtypes.NewBaseAccountWithAddress(accAddrs[1]) - vacc := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) @@ -1063,7 +1069,8 @@ func (suite *KeeperTestSuite) TestPeriodicVestingAccountReceive() { vesting.Period{Length: int64(6 * 60 * 60), Amount: sdk.Coins{sdk.NewInt64Coin("stake", 25)}}, } - vacc := vesting.NewPeriodicVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), periods) + vacc, err := vesting.NewPeriodicVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), periods) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) @@ -1095,7 +1102,8 @@ func (suite *KeeperTestSuite) TestDelegateCoins() { acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) acc1 := authtypes.NewBaseAccountWithAddress(accAddrs[1]) - vacc := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) @@ -1152,7 +1160,8 @@ func (suite *KeeperTestSuite) TestUndelegateCoins() { acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) acc1 := authtypes.NewBaseAccountWithAddress(accAddrs[1]) - vacc := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + vacc, err := vesting.NewContinuousVestingAccount(acc0, origCoins, ctx.BlockHeader().Time.Unix(), endTime.Unix()) + suite.Require().NoError(err) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], origCoins)) diff --git a/x/genutil/genaccounts.go b/x/genutil/genaccounts.go index 8e92ab61e9a9..c9a2ad250120 100644 --- a/x/genutil/genaccounts.go +++ b/x/genutil/genaccounts.go @@ -45,7 +45,10 @@ func AddGenesisAccount( baseAccount := authtypes.NewBaseAccount(accAddr, nil, 0, 0) if !vestingAmt.IsZero() { - baseVestingAccount := authvesting.NewBaseVestingAccount(baseAccount, vestingAmt.Sort(), vestingEnd) + baseVestingAccount, err := authvesting.NewBaseVestingAccount(baseAccount, vestingAmt.Sort(), vestingEnd) + if err != nil { + return fmt.Errorf("failed to create base vesting account: %w", err) + } if (balances.Coins.IsZero() && !baseVestingAccount.OriginalVesting.IsZero()) || baseVestingAccount.OriginalVesting.IsAnyGT(balances.Coins) {