Skip to content

Commit

Permalink
refactor: reinvest dust fees into pool accumulator (#5245)
Browse files Browse the repository at this point in the history
* refactor: reinvest dust fees into pool accumulator

* updates

* updates

* Update x/concentrated-liquidity/fees_test.go

Co-authored-by: Adam Tucker <[email protected]>

---------

Co-authored-by: Adam Tucker <[email protected]>
  • Loading branch information
p0mvn and czarcas7ic authored May 20, 2023
1 parent 53b56a7 commit 50f5d6b
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 10 deletions.
28 changes: 26 additions & 2 deletions x/concentrated-liquidity/fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
47 changes: 39 additions & 8 deletions x/concentrated-liquidity/fees_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),

Expand Down Expand Up @@ -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)

Expand All @@ -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))
}
})
}
}
Expand Down

0 comments on commit 50f5d6b

Please sign in to comment.