From 5e32f447a5f212469b4d4784c71ca9c4442bbaa4 Mon Sep 17 00:00:00 2001 From: Marc Puig Date: Sun, 24 Mar 2024 08:39:35 +0100 Subject: [PATCH] fix: remove proxy acc usage (#799) * remove proxy account usage * tests * changelog * set APY limit for auto compounded rewards --- CHANGELOG.md | 4 ++ app/app.go | 1 + x/liquidstake/keeper/keeper.go | 3 ++ x/liquidstake/keeper/keeper_test.go | 2 +- x/liquidstake/keeper/liquidstake.go | 8 +--- x/liquidstake/keeper/liquidstake_test.go | 6 +-- x/liquidstake/keeper/rebalancing.go | 48 +++++++++++++++++------- x/liquidstake/keeper/rebalancing_test.go | 37 +++++++++++++++++- x/liquidstake/types/expected_keepers.go | 8 ++++ x/liquidstake/types/liquidstake.go | 2 +- x/liquidstake/types/params.go | 6 +++ 11 files changed, 98 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 495510a38..e1c4ddce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +### Improvements + +- [790](https://github.com/persistenceOne/pstake-native/pull/790) Make liquidstake module LSM Cap compliant. + ### Bug Fixes - [792](https://github.com/persistenceOne/pstake-native/pull/792) Use GetHostChainFromHostDenom in ICA Transfer diff --git a/app/app.go b/app/app.go index 98f935bfb..1caf8a79e 100644 --- a/app/app.go +++ b/app/app.go @@ -434,6 +434,7 @@ func NewpStakeApp( app.AccountKeeper, app.BankKeeper, app.StakingKeeper, + app.MintKeeper, app.DistrKeeper, app.SlashingKeeper, app.MsgServiceRouter(), diff --git a/x/liquidstake/keeper/keeper.go b/x/liquidstake/keeper/keeper.go index 91fafc4ba..cff58b8e8 100644 --- a/x/liquidstake/keeper/keeper.go +++ b/x/liquidstake/keeper/keeper.go @@ -20,6 +20,7 @@ type Keeper struct { accountKeeper types.AccountKeeper bankKeeper types.BankKeeper stakingKeeper types.StakingKeeper + mintKeeper types.MintKeeper distrKeeper types.DistrKeeper slashingKeeper types.SlashingKeeper @@ -34,6 +35,7 @@ func NewKeeper( accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, stakingKeeper types.StakingKeeper, + mintKeeper types.MintKeeper, distrKeeper types.DistrKeeper, slashingKeeper types.SlashingKeeper, router *baseapp.MsgServiceRouter, @@ -50,6 +52,7 @@ func NewKeeper( accountKeeper: accountKeeper, bankKeeper: bankKeeper, stakingKeeper: stakingKeeper, + mintKeeper: mintKeeper, distrKeeper: distrKeeper, slashingKeeper: slashingKeeper, router: router, diff --git a/x/liquidstake/keeper/keeper_test.go b/x/liquidstake/keeper/keeper_test.go index fbcc8c3b2..cfd1da6cd 100644 --- a/x/liquidstake/keeper/keeper_test.go +++ b/x/liquidstake/keeper/keeper_test.go @@ -28,7 +28,7 @@ import ( "github.com/persistenceOne/pstake-native/v2/x/liquidstake/types" ) -var BlockTime = 10 * time.Second +var BlockTime = 6 * time.Second type KeeperTestSuite struct { suite.Suite diff --git a/x/liquidstake/keeper/liquidstake.go b/x/liquidstake/keeper/liquidstake.go index 493bbd6ed..ceb9f1e77 100644 --- a/x/liquidstake/keeper/liquidstake.go +++ b/x/liquidstake/keeper/liquidstake.go @@ -789,22 +789,18 @@ func (k Keeper) CheckDelegationStates(ctx sdk.Context, proxyAcc sdk.AccAddress) return totalRewards, totalDelShares, totalLiquidTokens } -func (k Keeper) WithdrawLiquidRewards(ctx sdk.Context, proxyAcc sdk.AccAddress) math.Int { - totalRewards := sdk.ZeroInt() - bondDenom := k.stakingKeeper.BondDenom(ctx) +func (k Keeper) WithdrawLiquidRewards(ctx sdk.Context, proxyAcc sdk.AccAddress) { k.stakingKeeper.IterateDelegations( ctx, proxyAcc, func(_ int64, del stakingtypes.DelegationI) (stop bool) { valAddr := del.GetValidatorAddr() - reward, err := k.distrKeeper.WithdrawDelegationRewards(ctx, proxyAcc, valAddr) + _, err := k.distrKeeper.WithdrawDelegationRewards(ctx, proxyAcc, valAddr) if err != nil { panic(err) } - totalRewards = totalRewards.Add(reward.AmountOf(bondDenom)) return false }, ) - return totalRewards } // GetLiquidValidator get a single liquid validator diff --git a/x/liquidstake/keeper/liquidstake_test.go b/x/liquidstake/keeper/liquidstake_test.go index f42916aa0..0da3ef7d7 100644 --- a/x/liquidstake/keeper/liquidstake_test.go +++ b/x/liquidstake/keeper/liquidstake_test.go @@ -443,11 +443,11 @@ func (s *KeeperTestSuite) TestShareInflation() { s.app.BankKeeper.SendCoins(s.ctx, attacker, types.LiquidStakeProxyAcc, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, attackerTransferAmount))) - // 4. user tx went through but receives fewer shares than intended - // stkXPRT to mint = 1 * 1000 / (1+500) = 1.99 = 1 + // 4. user tx went through and the mint rate is not affected by the XPRT sent by the attacker + // stkXPRT to mint = 1 * 1000 / 1 = 1 mintAmount, err = s.keeper.LiquidStake(s.ctx, types.LiquidStakeProxyAcc, user, sdk.NewCoin(sdk.DefaultBondDenom, userStakeAmount)) s.Require().NoError(err) - s.Require().Equal(mintAmount, math.NewInt(952)) + s.Require().Equal(mintAmount, math.NewInt(1_000)) // 5. attacker unstakes the shares immediately liquidBondDenom := s.keeper.LiquidBondDenom(s.ctx) diff --git a/x/liquidstake/keeper/rebalancing.go b/x/liquidstake/keeper/rebalancing.go index e5daab7a1..99b6f92a4 100644 --- a/x/liquidstake/keeper/rebalancing.go +++ b/x/liquidstake/keeper/rebalancing.go @@ -188,19 +188,8 @@ func (k Keeper) UpdateLiquidValidatorSet(ctx sdk.Context) (redelegations []types // AutocompoundStakingRewards withdraws staking rewards and re-stakes when over threshold. func (k Keeper) AutocompoundStakingRewards(ctx sdk.Context, whitelistedValsMap types.WhitelistedValsMap) { - // Withdraw rewards of LiquidStakeProxyAcc and re-staking - totalRewardsWithdrawn := k.WithdrawLiquidRewards(ctx, types.LiquidStakeProxyAcc) - - // prepare to re-staking with proxyAccBalance - proxyAccBalance := k.GetProxyAccBalance(ctx, types.LiquidStakeProxyAcc) - - // calculate autocompounding fee - params := k.GetParams(ctx) - - autocompoundFee := sdk.NewCoin(proxyAccBalance.Denom, math.ZeroInt()) - if !params.AutocompoundFeeRate.IsZero() && totalRewardsWithdrawn.IsPositive() { - autocompoundFee = sdk.NewCoin(proxyAccBalance.Denom, params.AutocompoundFeeRate.MulInt(totalRewardsWithdrawn).TruncateInt()) - } + // withdraw rewards of LiquidStakeProxyAcc + k.WithdrawLiquidRewards(ctx, types.LiquidStakeProxyAcc) // skip when no active liquid validator activeVals := k.GetActiveLiquidValidators(ctx, whitelistedValsMap) @@ -208,9 +197,40 @@ func (k Keeper) AutocompoundStakingRewards(ctx sdk.Context, whitelistedValsMap t return } + // get all the APY components + bondDenom := k.stakingKeeper.BondDenom(ctx) + totalSupply := k.bankKeeper.GetSupply(ctx, bondDenom).Amount + bondedTokens := k.bankKeeper.GetBalance(ctx, k.stakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + inflation := k.mintKeeper.GetMinter(ctx).Inflation + + // calculate the hourly APY + bondRatio := math.LegacyDec(bondedTokens).Quo(math.LegacyDec(totalSupply)) + hourlyApy := inflation.Quo(bondRatio). + Quo(types.DefaultLimitAutocompoundPeriodDays). + Quo(types.DefaultLimitAutocompoundPeriodHours) + + // calculate autocompoundable amount by limiting the current net amount with the calculated APY + autoCompoundableAmount := k.GetNetAmountState(ctx).NetAmount.Mul(hourlyApy).TruncateInt() + + // use the calculated autocompoundable amount as the limit for the transfer + proxyAccBalance := k.GetProxyAccBalance(ctx, types.LiquidStakeProxyAcc) + if proxyAccBalance.Amount.LT(autoCompoundableAmount) { + autoCompoundableAmount = proxyAccBalance.Amount + } + + // calculate autocompounding fee + params := k.GetParams(ctx) + autocompoundFee := sdk.NewCoin(k.stakingKeeper.BondDenom(ctx), math.ZeroInt()) + if !params.AutocompoundFeeRate.IsZero() && autoCompoundableAmount.IsPositive() { + autocompoundFee = sdk.NewCoin( + k.stakingKeeper.BondDenom(ctx), + params.AutocompoundFeeRate.MulInt(autoCompoundableAmount).TruncateInt(), + ) + } + // re-staking of the accumulated rewards cachedCtx, writeCache := ctx.CacheContext() - delegableAmount := proxyAccBalance.Amount.Sub(autocompoundFee.Amount) + delegableAmount := autoCompoundableAmount.Sub(autocompoundFee.Amount) err := k.LiquidDelegate(cachedCtx, types.LiquidStakeProxyAcc, activeVals, delegableAmount, whitelistedValsMap) if err != nil { logger := k.Logger(ctx) diff --git a/x/liquidstake/keeper/rebalancing_test.go b/x/liquidstake/keeper/rebalancing_test.go index 0fa13b0f4..20848155b 100644 --- a/x/liquidstake/keeper/rebalancing_test.go +++ b/x/liquidstake/keeper/rebalancing_test.go @@ -472,10 +472,10 @@ func (s *KeeperTestSuite) TestAutocompoundStakingRewards() { s.EqualValues(totalDelShares, stakingAmt.ToLegacyDec(), totalLiquidTokens) // allocate rewards - s.advanceHeight(100, false) + s.advanceHeight(360, false) totalRewards, totalDelShares, totalLiquidTokens = s.keeper.CheckDelegationStates(s.ctx, types.LiquidStakeProxyAcc) s.NotEqualValues(totalRewards, sdk.ZeroDec()) - s.NotEqualValues(totalLiquidTokens, sdk.ZeroDec()) + s.Equal(totalLiquidTokens, stakingAmt) // withdraw rewards and re-staking whitelistedValsMap := types.GetWhitelistedValsMap(params.WhitelistedValidators) @@ -495,6 +495,39 @@ func (s *KeeperTestSuite) TestAutocompoundStakingRewards() { s.EqualValues(autocompoundFee.TruncateInt(), feeAccountBalance.Amount) } +func (s *KeeperTestSuite) TestLimitAutocompoundStakingRewards() { + _, valOpers, _ := s.CreateValidators([]int64{2000000, 2000000, 2000000}) + params := s.keeper.GetParams(s.ctx) + + params.WhitelistedValidators = []types.WhitelistedValidator{ + {ValidatorAddress: valOpers[0].String(), TargetWeight: math.NewInt(5000)}, + {ValidatorAddress: valOpers[1].String(), TargetWeight: math.NewInt(5000)}, + } + params.ModulePaused = false + s.keeper.SetParams(s.ctx, params) + s.keeper.UpdateLiquidValidatorSet(s.ctx) + + stakingAmt := math.NewInt(100000000) + s.Require().NoError(s.liquidStaking(s.delAddrs[0], stakingAmt)) + + // allocate rewards + s.advanceHeight(360, false) + totalRewards, _, totalLiquidTokens := s.keeper.CheckDelegationStates(s.ctx, types.LiquidStakeProxyAcc) + s.NotEqualValues(totalRewards, sdk.ZeroDec()) + s.Equal(totalLiquidTokens, stakingAmt) + + // unilaterally send tokens to the proxy account + s.fundAddr(types.LiquidStakeProxyAcc, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1000000000)))) + + // withdraw rewards and re-stake + whitelistedValsMap := types.GetWhitelistedValsMap(params.WhitelistedValidators) + s.keeper.AutocompoundStakingRewards(s.ctx, whitelistedValsMap) + + // tokens still remaining in the proxy account as the balance was higher than the APY limit + proxyAccBalanceAfter := s.keeper.GetProxyAccBalance(s.ctx, types.LiquidStakeProxyAcc) + s.NotEqual(proxyAccBalanceAfter.Amount, sdk.ZeroInt()) +} + func (s *KeeperTestSuite) TestRemoveAllLiquidValidator() { _, valOpers, _ := s.CreateValidators([]int64{2000000, 2000000, 2000000}) params := s.keeper.GetParams(s.ctx) diff --git a/x/liquidstake/types/expected_keepers.go b/x/liquidstake/types/expected_keepers.go index a3abdfe90..a15a80414 100644 --- a/x/liquidstake/types/expected_keepers.go +++ b/x/liquidstake/types/expected_keepers.go @@ -7,6 +7,7 @@ import ( abci "github.com/cometbft/cometbft/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/mint/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) @@ -21,6 +22,7 @@ type BankKeeper interface { BurnCoins(ctx sdk.Context, name string, amt sdk.Coins) error MintCoins(ctx sdk.Context, name string, amt sdk.Coins) error SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins + GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin } // AccountKeeper defines the expected account keeper @@ -78,6 +80,12 @@ type StakingKeeper interface { delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress) bool SafelyIncreaseTotalLiquidStakedTokens(ctx sdk.Context, amount math.Int, sharesAlreadyBonded bool) error DecreaseTotalLiquidStakedTokens(ctx sdk.Context, amount math.Int) error + GetBondedPool(ctx sdk.Context) (bondedPool authtypes.ModuleAccountI) +} + +// MintKeeper expected minting keeper (noalias) +type MintKeeper interface { + GetMinter(ctx sdk.Context) (minter types.Minter) } // DistrKeeper expected distribution keeper (noalias) diff --git a/x/liquidstake/types/liquidstake.go b/x/liquidstake/types/liquidstake.go index be729235b..1454e4a58 100644 --- a/x/liquidstake/types/liquidstake.go +++ b/x/liquidstake/types/liquidstake.go @@ -182,7 +182,7 @@ func DeductFeeRate(input, feeRate math.LegacyDec) (feeDeductedOutput math.Legacy } func (nas NetAmountState) CalcNetAmount() math.LegacyDec { - return math.LegacyNewDecFromInt(nas.ProxyAccBalance.Add(nas.TotalLiquidTokens).Add(nas.TotalUnbondingBalance)) + return math.LegacyNewDecFromInt(nas.TotalLiquidTokens.Add(nas.TotalUnbondingBalance)) } func (nas NetAmountState) CalcMintRate() math.LegacyDec { diff --git a/x/liquidstake/types/params.go b/x/liquidstake/types/params.go index 072886d5c..e40aeab6a 100644 --- a/x/liquidstake/types/params.go +++ b/x/liquidstake/types/params.go @@ -23,6 +23,12 @@ var ( // DefaultMinLiquidStakeAmount is the default minimum liquid stake amount. DefaultMinLiquidStakeAmount = math.NewInt(1000) + // DefaultLimitAutocompoundPeriodDays is the number of days for which APY autocompound limit is calculated. + DefaultLimitAutocompoundPeriodDays = math.LegacyNewDec(365) + + // DefaultLimitAutocompoundPeriodHours is the number of hours for which APY autocompound limit is calculated. + DefaultLimitAutocompoundPeriodHours = math.LegacyNewDec(24) + // Const variables // TotalValidatorWeight specifies the target sum of validator weights in the whitelist.