From 50f5d6bdc8a6c4a4ec4e2fee592db950630ff846 Mon Sep 17 00:00:00 2001 From: Roman Date: Sat, 20 May 2023 00:30:14 -0400 Subject: [PATCH] refactor: reinvest dust fees into pool accumulator (#5245) * refactor: reinvest dust fees into pool accumulator * updates * updates * Update x/concentrated-liquidity/fees_test.go Co-authored-by: Adam Tucker --------- Co-authored-by: Adam Tucker --- x/concentrated-liquidity/fees.go | 28 ++++++++++++++-- x/concentrated-liquidity/fees_test.go | 47 ++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/x/concentrated-liquidity/fees.go b/x/concentrated-liquidity/fees.go index b88417c7605..21dc21fcb46 100644 --- a/x/concentrated-liquidity/fees.go +++ b/x/concentrated-liquidity/fees.go @@ -237,7 +237,9 @@ func (k Keeper) GetClaimableFees(ctx sdk.Context, positionId uint64) (sdk.Coins, // prepareClaimableFees returns the amount of fees that a position is eligible to claim. // Note that it mutates the internal state of the fee accumulator by setting the position's // unclaimed rewards to zero and update the position's accumulator value to reflect the -// current pool fee accumulator value. +// current pool fee accumulator value. If there is any dust left over, it is added back to the +// global accumulator as long as there are shares remaining in the accumulator. If not, the dust +// is ignored. // // Returns error if: // - pool with the given id does not exist @@ -275,11 +277,33 @@ func (k Keeper) prepareClaimableFees(ctx sdk.Context, positionId uint64) (sdk.Co } // Claim rewards, set the unclaimed rewards to zero, and update the position's accumulator value to reflect the current accumulator value. - feesClaimed, _, err := updateAccumAndClaimRewards(feeAccumulator, positionKey, feeGrowthOutside) + feesClaimed, forfeitedDust, err := updateAccumAndClaimRewards(feeAccumulator, positionKey, feeGrowthOutside) if err != nil { return nil, err } + // add foreited dust back to the global accumulator + if !forfeitedDust.IsZero() { + // Refetch the fee accumulator as the number of shares has changed after claiming. + feeAccumulator, err := k.GetFeeAccumulator(ctx, position.PoolId) + if err != nil { + return nil, err + } + + totalSharesRemaining, err := feeAccumulator.GetTotalShares() + if err != nil { + return nil, err + } + + // if there are no shares remaining, the dust is ignored. Otherwise, it is added back to the global accumulator. + // Total shares remaining can be zero if we claim in withdrawPosition for the last position in the pool. + // The shares are decremented in osmoutils/accum.ClaimRewards. + if !totalSharesRemaining.IsZero() { + forfeitedDustPerShare := forfeitedDust.QuoDecTruncate(totalSharesRemaining) + feeAccumulator.AddToAccumulator(forfeitedDustPerShare) + } + } + return feesClaimed, nil } diff --git a/x/concentrated-liquidity/fees_test.go b/x/concentrated-liquidity/fees_test.go index b7e1107be21..ffa0ea1a347 100644 --- a/x/concentrated-liquidity/fees_test.go +++ b/x/concentrated-liquidity/fees_test.go @@ -980,12 +980,13 @@ func (s *KeeperTestSuite) TestPrepareClaimableFees() { tests := map[string]struct { // setup parameters. - initialLiquidity sdk.Dec - lowerTickFeeGrowthOutside sdk.DecCoins - upperTickFeeGrowthOutside sdk.DecCoins - globalFeeGrowth sdk.DecCoins - currentTick int64 - isInvalidPoolIdGiven bool + initialLiquidity sdk.Dec + lowerTickFeeGrowthOutside sdk.DecCoins + upperTickFeeGrowthOutside sdk.DecCoins + globalFeeGrowth sdk.DecCoins + expectedReinvestedDustAmount sdk.Dec + currentTick int64 + isInvalidPoolIdGiven bool // inputs parameters. lowerTick int64 @@ -1109,6 +1110,27 @@ func (s *KeeperTestSuite) TestPrepareClaimableFees() { expectedInitAccumValue: sdk.NewDecCoins(sdk.NewDecCoin(ETH, sdk.NewInt(10))), }, + "dust reinvested: single swap right -> left: 2 ticks, two shares, current tick in between lower and upper tick": { + initialLiquidity: sdk.NewDec(2), + + lowerTickFeeGrowthOutside: sdk.NewDecCoins(sdk.NewDecCoin(ETH, sdk.NewInt(0))), + upperTickFeeGrowthOutside: sdk.NewDecCoins(sdk.NewDecCoinFromDec(ETH, sdk.MustNewDecFromStr("3.3"))), + + globalFeeGrowth: sdk.NewDecCoins(sdk.NewDecCoin(ETH, sdk.NewInt(10))), + + lowerTick: 0, + upperTick: 2, + positionIdToPrepare: DefaultPositionId, + + currentTick: 1, + + // expected = global - below lower - above upper = 10 - 3.3 = 6.7 + expectedInitAccumValue: sdk.NewDecCoins(sdk.NewDecCoinFromDec(ETH, sdk.MustNewDecFromStr("6.7"))), + // expected reinvested dust = (6.7 * 2 % floor(6.7 * 2)) / 2 + // This can be thought of as the diffence between the non-truncated total amount of fees and the truncated toal amount of fees + // divided by the number of shares. + expectedReinvestedDustAmount: sdk.MustNewDecFromStr("0.2"), + }, "swap occurs above the position, current tick > upper tick": { initialLiquidity: sdk.OneDec(), @@ -1189,9 +1211,11 @@ func (s *KeeperTestSuite) TestPrepareClaimableFees() { positionKey := types.KeyFeePositionAccumulator(DefaultPositionId) // Note the position accumulator before calling prepare - _, err = s.App.ConcentratedLiquidityKeeper.GetFeeAccumulator(ctx, validPoolId) + originalAccum, err := s.App.ConcentratedLiquidityKeeper.GetFeeAccumulator(ctx, validPoolId) s.Require().NoError(err) + originalAccumValue := originalAccum.GetValue() + // System under test actualFeesClaimed, err := clKeeper.PrepareClaimableFees(ctx, tc.positionIdToPrepare) @@ -1211,8 +1235,15 @@ func (s *KeeperTestSuite) TestPrepareClaimableFees() { s.Require().Equal(tc.expectedInitAccumValue, postPreparePosition.AccumValuePerShare) s.Require().Equal(tc.initialLiquidity, postPreparePosition.NumShares) - expectedFeeClaimAmount := tc.expectedInitAccumValue.AmountOf(ETH).Mul(tc.initialLiquidity).TruncateInt() + expectedClaimedAmountDec := tc.expectedInitAccumValue.AmountOf(ETH).Mul(tc.initialLiquidity) + expectedFeeClaimAmount := expectedClaimedAmountDec.TruncateInt() s.Require().Equal(expectedFeeClaimAmount, actualFeesClaimed.AmountOf(ETH)) + + // validate that truncated dust amount is reinvested back into the global accumulator + if expectedClaimedAmountDec.GT(expectedFeeClaimAmount.ToDec()) { + accumDelta, _ := accum.GetValue().SafeSub(originalAccumValue) + s.Require().Equal(tc.expectedReinvestedDustAmount, accumDelta.AmountOf(ETH)) + } }) } }