From b3c4831538eddbb1b7c65892d99e508407e87cfa Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Sat, 29 Jan 2022 20:41:32 -0800 Subject: [PATCH] fix: reset delegation bookkeeping when adding new grants Fixes issue https://github.com/Agoric/agoric-sdk/issues/4300 --- x/auth/vesting/types/vesting_account.go | 37 ++++- x/auth/vesting/types/vesting_account_test.go | 155 +++++++++++++++++++ 2 files changed, 184 insertions(+), 8 deletions(-) diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index 88b16c008781..e0815913b0f1 100644 --- a/x/auth/vesting/types/vesting_account.go +++ b/x/auth/vesting/types/vesting_account.go @@ -365,7 +365,18 @@ func (pva PeriodicVestingAccount) Validate() error { } // AddGrant merges a new periodic vesting grant into an existing PeriodicVestingAccount. -func (pva *PeriodicVestingAccount) AddGrant(grantStartTime int64, grantVestingPeriods []Period, grantCoins sdk.Coins) { +func (pva *PeriodicVestingAccount) AddGrant(ctx sdk.Context, bk BankKeeper, sk StakingKeeper, grantStartTime int64, grantVestingPeriods []Period, grantCoins sdk.Coins) { + // how much is really delegated? + bondedAmt, unbondingAmt, _ := findBalance(ctx, pva.GetAddress(), bk, sk) + delegatedAmt := bondedAmt.Add(unbondingAmt) + delegated := sdk.NewCoins(sdk.NewCoin(sk.BondDenom(ctx), delegatedAmt)) + + // cap DV at the current unvested amount, DF rounds out to current delegated + unvested := pva.GetVestingCoins(ctx.BlockTime()) + pva.DelegatedVesting = coinsMin(delegated, unvested) + pva.DelegatedFree = delegated.Sub(pva.DelegatedVesting) + + // modify vesting schedule for the new grant newStart, newEnd, newPeriods := DisjunctPeriods(pva.StartTime, grantStartTime, pva.GetVestingPeriods(), grantVestingPeriods) pva.StartTime = newStart @@ -669,7 +680,17 @@ func (va ClawbackVestingAccount) MarshalYAML() (interface{}, error) { } // AddGrant merges a new clawback vesting grant into an existing ClawbackVestingAccount. -func (va *ClawbackVestingAccount) AddGrant(grantStartTime int64, grantLockupPeriods, grantVestingPeriods []Period, grantCoins sdk.Coins) { +func (va *ClawbackVestingAccount) AddGrant(ctx sdk.Context, bk BankKeeper, sk StakingKeeper, grantStartTime int64, grantLockupPeriods, grantVestingPeriods []Period, grantCoins sdk.Coins) { + // how much is really delegated? + bondedAmt, unbondingAmt, _ := findBalance(ctx, va.GetAddress(), bk, sk) + delegatedAmt := bondedAmt.Add(unbondingAmt) + delegated := sdk.NewCoins(sdk.NewCoin(sk.BondDenom(ctx), delegatedAmt)) + + // cap DV at the current unvested amount, DF rounds out to current delegated + unvested := va.GetVestingCoins(ctx.BlockTime()) + va.DelegatedVesting = coinsMin(delegated, unvested) + va.DelegatedFree = delegated.Sub(va.DelegatedVesting) + newLockupStart, newLockupEnd, newLockupPeriods := DisjunctPeriods(va.StartTime, grantStartTime, va.LockupPeriods, grantLockupPeriods) newVestingStart, newVestingEnd, newVestingPeriods := DisjunctPeriods(va.StartTime, grantStartTime, va.GetVestingPeriods(), grantVestingPeriods) @@ -778,7 +799,7 @@ func (va ClawbackVestingAccount) Clawback(ctx sdk.Context, dest sdk.AccAddress, // Compute the clawback based on bank balance and delegation, and update account encumbered := updatedAcc.GetVestingCoins(ctx.BlockTime()) - bondedAmt, unbondingAmt, _ := updatedAcc.findBalance(ctx, bk, sk) + bondedAmt, unbondingAmt, _ := findBalance(ctx, addr, bk, sk) bonded := sdk.NewCoins(sdk.NewCoin(bondDenom, bondedAmt)) unbonding := sdk.NewCoins(sdk.NewCoin(bondDenom, unbondingAmt)) unbonded := bk.GetAllBalances(ctx, addr) @@ -857,12 +878,12 @@ func (va ClawbackVestingAccount) Clawback(ctx sdk.Context, dest sdk.AccAddress, // Returns the number of bonded, unbonding, and unbonded statking tokens. // Rounds down when computing the bonded tokens to err on the side of vested fraction // (smaller number of bonded tokens means vested amount covers more of them). -func (va ClawbackVestingAccount) findBalance(ctx sdk.Context, bk BankKeeper, sk StakingKeeper) (bonded, unbonding, unbonded sdk.Int) { +func findBalance(ctx sdk.Context, addr sdk.AccAddress, bk BankKeeper, sk StakingKeeper) (bonded, unbonding, unbonded sdk.Int) { bondDenom := sk.BondDenom(ctx) - unbonded = bk.GetBalance(ctx, va.GetAddress(), bondDenom).Amount + unbonded = bk.GetBalance(ctx, addr, bondDenom).Amount unbonding = sdk.ZeroInt() - unbondings := sk.GetUnbondingDelegations(ctx, va.GetAddress(), math.MaxUint16) + unbondings := sk.GetUnbondingDelegations(ctx, addr, math.MaxUint16) for _, ubd := range unbondings { for _, entry := range ubd.Entries { unbonding = unbonding.Add(entry.Balance) @@ -870,7 +891,7 @@ func (va ClawbackVestingAccount) findBalance(ctx sdk.Context, bk BankKeeper, sk } bonded = sdk.ZeroInt() - delegations := sk.GetDelegatorDelegations(ctx, va.GetAddress(), math.MaxUint16) + delegations := sk.GetDelegatorDelegations(ctx, addr, math.MaxUint16) for _, delegation := range delegations { validatorAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) if err != nil { @@ -959,7 +980,7 @@ func (va ClawbackVestingAccount) PostReward(ctx sdk.Context, reward sdk.Coins, a } // Find current split of account balance on staking axis - bonded, unbonding, unbonded := va.findBalance(ctx, bk, sk) + bonded, unbonding, unbonded := findBalance(ctx, va.GetAddress(), bk, sk) total := bonded.Add(unbonding).Add(unbonded) total = total.Sub(minInt(total, reward.AmountOf(bondDenom))) // look at pre-reward total diff --git a/x/auth/vesting/types/vesting_account_test.go b/x/auth/vesting/types/vesting_account_test.go index b01710c33911..8d8b59cbd3ba 100644 --- a/x/auth/vesting/types/vesting_account_test.go +++ b/x/auth/vesting/types/vesting_account_test.go @@ -372,6 +372,49 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 25)}, dva.DelegatedVesting) } +func TestAddGrantPeriodicVestingAcc(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(650), stake(40))}, + {Length: 100, Amount: c(fee(350), stake(60))}, + } + bacc, origCoins := initBaseAccount() + pva := types.NewPeriodicVestingAccount(bacc, origCoins, now.Unix(), periods) + addr := pva.GetAddress() + + // simulate 54stake (unvested) lost to slashing + pva.DelegatedVesting = c(stake(54)) + + // Add a new grant of 50stake + newGrant := c(stake(50)) + pva.AddGrant(ctx, app.BankKeeper, app.StakingKeeper, now.Add(500*time.Second).Unix(), []types.Period{{Length: 50, Amount: newGrant}}, newGrant) + app.AccountKeeper.SetAccount(ctx, pva) + + // fund the vesting account with new grant (old has vested and transferred out) + ctx = ctx.WithBlockTime(now.Add(500 * time.Second)) + err := simapp.FundAccount(app.BankKeeper, ctx, addr, newGrant) + require.NoError(t, err) + require.Equal(t, int64(50), app.BankKeeper.GetBalance(ctx, addr, stakeDenom).Amount.Int64()) + + // we should not be able to transfer the new grant out until it has vested + _, _, dest := testdata.KeyTestPubAddr() + err = app.BankKeeper.SendCoins(ctx, addr, dest, c(stake(1))) + require.Error(t, err) + ctx = ctx.WithBlockTime(now.Add(600 * time.Second)) + err = app.BankKeeper.SendCoins(ctx, addr, dest, newGrant) + require.NoError(t, err) +} + func TestGetVestedCoinsPeriodicVestingAcc(t *testing.T) { now := time.Now() endTime := now.Add(24 * time.Hour) @@ -1225,6 +1268,118 @@ func TestRewards(t *testing.T) { } } +func TestRewards_PostSlash(t *testing.T) { + c := sdk.NewCoins + stake := func(x int64) sdk.Coin { return sdk.NewInt64Coin(stakeDenom, x) } + now := tmtime.Now() + + // set up simapp and validators + app := simapp.Setup(false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockTime((now)) + _, val := createValidator(t, ctx, app, 100) + require.Equal(t, "stake", app.StakingKeeper.BondDenom(ctx)) + + // create vesting account with a simulated 350stake lost to slashing + lockupPeriods := types.Periods{ + {Length: 1, Amount: c(stake(4000))}, + } + vestingPeriods := types.Periods{ + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + {Length: int64(100), Amount: c(stake(500))}, + } + bacc, _ := initBaseAccount() + origCoins := c(stake(4000)) + _, _, funder := testdata.KeyTestPubAddr() + va := types.NewClawbackVestingAccount(bacc, funder, origCoins, now.Unix(), lockupPeriods, vestingPeriods) + addr := va.GetAddress() + va.DelegatedVesting = c(stake(350)) + app.AccountKeeper.SetAccount(ctx, va) + + // fund the vesting account with 350 stake lost to slashing + err := simapp.FundAccount(app.BankKeeper, ctx, addr, c(stake(3650))) + require.NoError(t, err) + require.Equal(t, int64(3650), app.BankKeeper.GetBalance(ctx, addr, stakeDenom).Amount.Int64()) + + // delegate all 3650stake + shares, err := app.StakingKeeper.Delegate(ctx, addr, sdk.NewInt(3650), stakingtypes.Unbonded, val, true) + require.NoError(t, err) + require.Equal(t, sdk.NewInt(3650), shares.TruncateInt()) + require.Equal(t, int64(0), app.BankKeeper.GetBalance(ctx, addr, stakeDenom).Amount.Int64()) + va = app.AccountKeeper.GetAccount(ctx, addr).(*types.ClawbackVestingAccount) + + // distribute a reward of 160stake - should all be unvested + err = simapp.FundAccount(app.BankKeeper, ctx, addr, c(stake(160))) + require.NoError(t, err) + va.PostReward(ctx, c(stake(160)), app.AccountKeeper, app.BankKeeper, app.StakingKeeper) + va = app.AccountKeeper.GetAccount(ctx, addr).(*types.ClawbackVestingAccount) + require.Equal(t, int64(4160), va.OriginalVesting.AmountOf(stakeDenom).Int64()) + require.Equal(t, 8, len(va.VestingPeriods)) + for i := 0; i < 8; i++ { + require.Equal(t, int64(520), va.VestingPeriods[i].Amount.AmountOf(stakeDenom).Int64()) + } + + // must not be able to transfer reward until it vests + _, _, dest := testdata.KeyTestPubAddr() + err = app.BankKeeper.SendCoins(ctx, addr, dest, c(stake(1))) + require.Error(t, err) + ctx = ctx.WithBlockTime(now.Add(600 * time.Second)) + err = app.BankKeeper.SendCoins(ctx, addr, dest, c(stake(160))) + require.NoError(t, err) +} + +func TestAddGrantClawbackVestingAcc(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 + _, _, funder := testdata.KeyTestPubAddr() + lockupPeriods := types.Periods{{Length: 1, Amount: c(fee(1000), stake(100))}} + vestingPeriods := types.Periods{ + {Length: 100, Amount: c(fee(650), stake(40))}, + {Length: 100, Amount: c(fee(350), stake(60))}, + } + bacc, origCoins := initBaseAccount() + va := types.NewClawbackVestingAccount(bacc, funder, origCoins, now.Unix(), lockupPeriods, vestingPeriods) + addr := va.GetAddress() + + // simulate 54stake (unvested) lost to slashing + va.DelegatedVesting = c(stake(54)) + + // Add a new grant of 50stake + newGrant := c(stake(50)) + va.AddGrant(ctx, app.BankKeeper, app.StakingKeeper, now.Add(500*time.Second).Unix(), + []types.Period{{Length: 1, Amount: newGrant}}, + []types.Period{{Length: 50, Amount: newGrant}}, newGrant) + app.AccountKeeper.SetAccount(ctx, va) + + // fund the vesting account with new grant (old has vested and transferred out) + ctx = ctx.WithBlockTime(now.Add(500 * time.Second)) + err := simapp.FundAccount(app.BankKeeper, ctx, addr, newGrant) + require.NoError(t, err) + require.Equal(t, int64(50), app.BankKeeper.GetBalance(ctx, addr, stakeDenom).Amount.Int64()) + + // we should not be able to transfer the new grant out until it has vested + _, _, dest := testdata.KeyTestPubAddr() + err = app.BankKeeper.SendCoins(ctx, addr, dest, c(stake(1))) + require.Error(t, err) + ctx = ctx.WithBlockTime(now.Add(600 * time.Second)) + err = app.BankKeeper.SendCoins(ctx, addr, dest, newGrant) + require.NoError(t, err) +} + func TestGenesisAccountValidate(t *testing.T) { pubkey := secp256k1.GenPrivKey().PubKey() addr := sdk.AccAddress(pubkey.Address())