feat: Do not unbond when validator inactive (#793)

persistenceOne · Mar 21, 2024 · 11191ee · 11191ee
1 parent 547b8dc
commit 11191ee
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 82 deletions.
diff --git a/x/liquidstake/keeper/liquidstake.go b/x/liquidstake/keeper/liquidstake.go
@@ -2,13 +2,13 @@ package keeper
 
 import (
 	"encoding/json"
-	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"sort"
 	"time"
 
 	errorsmod "cosmossdk.io/errors"
 	"cosmossdk.io/math"
 	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

diff --git a/x/liquidstake/keeper/liquidstake_test.go b/x/liquidstake/keeper/liquidstake_test.go
@@ -382,17 +382,18 @@ func (s *KeeperTestSuite) TestLiquidUnstakeEdgeCases() {
 	s.keeper.UpdateLiquidValidatorSet(s.ctx)
 
 	// error case where there is a quantity that are unbonding balance or remaining rewards that is not re-stake or withdrawn in netAmount.
-	_, _, _, _, err = s.liquidUnstakingWithResult(s.delAddrs[0], sdk.NewCoin(params.LiquidBondDenom, math.NewInt(1000)))
-	s.Require().ErrorIs(err, types.ErrInsufficientProxyAccBalance)
+	// NOT APPLICABLE since we do not validator unbond if validator goes inactive.
+	//_, _, _, _, err = s.liquidUnstakingWithResult(s.delAddrs[0], sdk.NewCoin(params.LiquidBondDenom, math.NewInt(1000)))
+	//s.Require().ErrorIs(err, types.ErrInsufficientProxyAccBalance)
 
-	// success after complete unbonding
+	// success after complete unbonding, Not applicable
 	s.completeRedelegationUnbonding()
-	ubdTime, unbondingAmt, ubds, unbondedAmt, err := s.liquidUnstakingWithResult(s.delAddrs[0], sdk.NewCoin(params.LiquidBondDenom, math.NewInt(1000)))
-	s.Require().NoError(err)
-	s.Require().EqualValues(unbondedAmt, math.NewInt(1000))
-	s.Require().EqualValues(unbondingAmt, sdk.ZeroInt())
-	s.Require().EqualValues(ubdTime, time.Time{})
-	s.Require().Len(ubds, 0)
+	// ubdTime, unbondingAmt, ubds, unbondedAmt, err := s.liquidUnstakingWithResult(s.delAddrs[0], sdk.NewCoin(params.LiquidBondDenom, math.NewInt(1000)))
+	// s.Require().NoError(err)
+	// s.Require().EqualValues(unbondedAmt, math.NewInt(1000))
+	// s.Require().EqualValues(unbondingAmt, sdk.ZeroInt())
+	// s.Require().EqualValues(ubdTime, time.Time{})
+	// s.Require().Len(ubds, 0)
 }
 
 func (s *KeeperTestSuite) TestShareInflation() {

diff --git a/x/liquidstake/keeper/rebalancing.go b/x/liquidstake/keeper/rebalancing.go
@@ -180,45 +180,8 @@ func (k Keeper) UpdateLiquidValidatorSet(ctx sdk.Context) (redelegations []types
 		types.RebalancingTrigger,
 	)
 
-	// unbond all delShares to proxyAcc if delShares exist on inactive liquid validators
-	for _, lv := range liquidValidators {
-		if !k.IsActiveLiquidValidator(ctx, lv, whitelistedValsMap) {
-			delShares := lv.GetDelShares(ctx, k.stakingKeeper)
-			if delShares.IsPositive() {
-				cachedCtx, writeCache := ctx.CacheContext()
-				completionTime, returnAmount, _, err := k.LiquidUnbond(cachedCtx, types.LiquidStakeProxyAcc, types.LiquidStakeProxyAcc, lv.GetOperator(), delShares, false)
-				if err != nil {
-					logger.Error("liquid unbonding of inactive liquid validator failed", "error", err)
-					continue
-				}
-				writeCache()
-				unbondingAmount := sdk.Coin{Denom: k.stakingKeeper.BondDenom(ctx), Amount: returnAmount}.String()
-				ctx.EventManager().EmitEvents(sdk.Events{
-					sdk.NewEvent(
-						types.EventTypeUnbondInactiveLiquidTokens,
-						sdk.NewAttribute(types.AttributeKeyLiquidValidator, lv.OperatorAddress),
-						sdk.NewAttribute(types.AttributeKeyUnbondingAmount, unbondingAmount),
-						sdk.NewAttribute(types.AttributeKeyCompletionTime, completionTime.Format(time.RFC3339)),
-					),
-				})
-				logger.Info(types.EventTypeUnbondInactiveLiquidTokens,
-					types.AttributeKeyLiquidValidator, lv.OperatorAddress,
-					types.AttributeKeyUnbondingAmount, unbondingAmount,
-					types.AttributeKeyCompletionTime, completionTime.Format(time.RFC3339))
-			}
-			_, found := k.stakingKeeper.GetDelegation(ctx, types.LiquidStakeProxyAcc, lv.GetOperator())
-			if !found {
-				k.RemoveLiquidValidator(ctx, lv)
-				ctx.EventManager().EmitEvents(sdk.Events{
-					sdk.NewEvent(
-						types.EventTypeRemoveLiquidValidator,
-						sdk.NewAttribute(types.AttributeKeyLiquidValidator, lv.OperatorAddress),
-					),
-				})
-				logger.Info(types.EventTypeRemoveLiquidValidator, types.AttributeKeyLiquidValidator, lv.OperatorAddress)
-			}
-		}
-	}
+	// if there are inactive liquid validators, do not unbond,
+	// instead let validator selection and rebalancing take care of it.
 
 	return redelegations
 }

diff --git a/x/liquidstake/keeper/rebalancing_test.go b/x/liquidstake/keeper/rebalancing_test.go
@@ -217,28 +217,25 @@ func (s *KeeperTestSuite) TestRebalancingCase1() {
 
 	// liquid validator removed, invalid after tombstoned
 	lvState, found = s.keeper.GetLiquidValidatorState(s.ctx, valOpers[1])
-	s.Require().False(found)
+	s.Require().True(found)
 	s.Require().Equal(lvState.OperatorAddress, valOpers[1].String())
-	s.Require().Equal(lvState.Status, types.ValidatorStatusUnspecified)
+	s.Require().Equal(lvState.Status, types.ValidatorStatusInactive)
 	s.Require().EqualValues(lvState.DelShares, sdk.ZeroDec())
 	s.Require().EqualValues(lvState.LiquidTokens, sdk.ZeroInt())
 
 	// jail last liquid validator, undelegate all liquid tokens to proxy acc
+	nasBefore := s.keeper.GetNetAmountState(s.ctx)
 	s.doubleSign(valOpers[0], sdk.ConsAddress(pks[0].Address()))
 	reds = s.keeper.UpdateLiquidValidatorSet(s.ctx)
 	s.Require().Len(reds, 0)
 
 	// no delegation of proxy acc
 	proxyAccDel1, found = s.app.StakingKeeper.GetDelegation(s.ctx, types.LiquidStakeProxyAcc, valOpers[0])
-	s.Require().False(found)
+	s.Require().True(found)
 	val1, found := s.app.StakingKeeper.GetValidator(s.ctx, valOpers[0])
 	s.Require().True(found)
 	s.Require().Equal(val1.Status, stakingtypes.Unbonding)
 
-	// check unbonding delegation to proxy acc
-	ubd, found := s.app.StakingKeeper.GetUnbondingDelegation(s.ctx, types.LiquidStakeProxyAcc, val1.GetOperator())
-	s.Require().True(found)
-
 	// complete unbonding
 	s.completeRedelegationUnbonding()
 
@@ -247,31 +244,16 @@ func (s *KeeperTestSuite) TestRebalancingCase1() {
 	s.Require().True(found)
 	s.Require().Equal(val1.Status, stakingtypes.Unbonded)
 
-	// no rewards, delShares, liquid tokens
+	// no rewards, same delShares, liquid tokens as we do not unbond now
 	nas := s.keeper.GetNetAmountState(s.ctx)
 	s.Require().EqualValues(nas.TotalRemainingRewards, sdk.ZeroDec())
-	s.Require().EqualValues(nas.TotalDelShares, sdk.ZeroDec())
-	s.Require().EqualValues(nas.TotalLiquidTokens, sdk.ZeroInt())
-
-	// unbonded to balance, equal with netAmount
-	s.Require().EqualValues(ubd.Entries[0].Balance, nas.ProxyAccBalance)
-	s.Require().EqualValues(nas.NetAmount.TruncateInt(), nas.ProxyAccBalance)
+	s.Require().EqualValues(nas.TotalDelShares, nasBefore.TotalDelShares)
+	s.Require().LessOrEqual(nas.TotalLiquidTokens.Int64(), nasBefore.TotalLiquidTokens.Int64()) // slashing
 
 	// mintRate over 1 due to slashing
 	s.Require().True(nas.MintRate.GT(sdk.OneDec()))
 	stkXPRTBalanceBefore := s.app.BankKeeper.GetBalance(s.ctx, s.delAddrs[0], params.LiquidBondDenom).Amount
-	nativeTokenBalanceBefore := s.app.BankKeeper.GetBalance(s.ctx, s.delAddrs[0], sdk.DefaultBondDenom).Amount
 	s.Require().EqualValues(nas.StkxprtTotalSupply, stkXPRTBalanceBefore)
-
-	// withdraw directly unstaking when no totalLiquidTokens
-	s.Require().NoError(s.liquidUnstaking(s.delAddrs[0], stkXPRTBalanceBefore, false))
-	stkXPRTBalanceAfter := s.app.BankKeeper.GetBalance(s.ctx, s.delAddrs[0], params.LiquidBondDenom).Amount
-	nativeTokenBalanceAfter := s.app.BankKeeper.GetBalance(s.ctx, s.delAddrs[0], sdk.DefaultBondDenom).Amount
-	s.Require().EqualValues(stkXPRTBalanceAfter, sdk.ZeroInt())
-	s.Require().EqualValues(nativeTokenBalanceAfter.Sub(nativeTokenBalanceBefore), nas.NetAmount.TruncateInt())
-
-	// zero net amount states
-	s.RequireNetAmountStateZero()
 }
 
 func (s *KeeperTestSuite) TestRebalancingConsecutiveCase() {
@@ -545,19 +527,16 @@ func (s *KeeperTestSuite) TestRemoveAllLiquidValidator() {
 
 	// no liquid validator
 	lvs := s.keeper.GetAllLiquidValidators(s.ctx)
-	s.Require().Len(lvs, 0)
+	s.Require().Len(lvs, 3) // now we do not remove inactive validators
 
 	nasAfter := s.keeper.GetNetAmountState(s.ctx)
-	s.Require().EqualValues(sdk.ZeroDec(), nasAfter.TotalRemainingRewards)
-	s.Require().EqualValues(nasBefore.TotalRemainingRewards.TruncateInt(), nasAfter.ProxyAccBalance)
-	s.Require().EqualValues(sdk.ZeroDec(), nasAfter.TotalDelShares)
-	s.Require().EqualValues(sdk.ZeroInt(), nasAfter.TotalLiquidTokens)
-	s.Require().EqualValues(nasBefore.NetAmount.Add(nasBefore.TotalRemainingRewards).TruncateInt(), nasAfter.NetAmount.TruncateInt())
+
+	s.Require().EqualValues(nasBefore.NetAmount.TruncateInt(), nasAfter.NetAmount.TruncateInt())
 
 	s.completeRedelegationUnbonding()
 	nasAfter2 := s.keeper.GetNetAmountState(s.ctx)
-	s.Require().EqualValues(nasAfter.ProxyAccBalance.Add(nasBefore.TotalLiquidTokens), nasAfter2.ProxyAccBalance)
-	s.Require().EqualValues(nasBefore.NetAmount.Add(nasBefore.TotalRemainingRewards).TruncateInt(), nasAfter2.NetAmount.TruncateInt())
+	s.Require().EqualValues(nasAfter.ProxyAccBalance, nasAfter2.ProxyAccBalance)                  // should be equal since no unbonding
+	s.Require().EqualValues(nasBefore.NetAmount.TruncateInt(), nasAfter2.NetAmount.TruncateInt()) // should be equal since no unbonding
 }
 
 func (s *KeeperTestSuite) TestUndelegatedFundsNotBecomeFees() {