Skip to content

Commit

Permalink
implement retaining rewards for transfers
Browse files Browse the repository at this point in the history
  • Loading branch information
AlpinYukseloglu committed Mar 20, 2024
1 parent 452f273 commit 994b773
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 25 deletions.
10 changes: 5 additions & 5 deletions x/concentrated-liquidity/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ func (s *KeeperTestSuite) TestTransferPositions_Events() {
hasIncentivesToClaim bool
hasSpreadRewardsToClaim bool
expectedTransferPositionsEvent int
expectedMessageEvents int
expectedMessageEvents int // We expect these to always be 0 because no additional events are being triggered
isLastPositionInPool bool
expectedError error
}{
Expand All @@ -572,22 +572,22 @@ func (s *KeeperTestSuite) TestTransferPositions_Events() {
hasIncentivesToClaim: true,
numPositionsToCreate: 1,
expectedTransferPositionsEvent: 1,
expectedMessageEvents: 2, // 1 for collect incentives claim send, 1 for collect incentives forfeit send
expectedMessageEvents: 0,
},
"single position ID with claimable spread rewards": {
positionIds: []uint64{DefaultPositionId},
hasSpreadRewardsToClaim: true,
numPositionsToCreate: 1,
expectedTransferPositionsEvent: 1,
expectedMessageEvents: 1, // 1 for collect spread rewards claim send
expectedMessageEvents: 0,
},
"single position ID with claimable incentives and spread rewards": {
positionIds: []uint64{DefaultPositionId},
hasIncentivesToClaim: true,
hasSpreadRewardsToClaim: true,
numPositionsToCreate: 1,
expectedTransferPositionsEvent: 1,
expectedMessageEvents: 3, // 1 for collect incentives claim send, 1 for collect incentives forfeit send, 1 for collect spread rewards claim send
expectedMessageEvents: 0,
},
"two position IDs": {
positionIds: []uint64{DefaultPositionId, DefaultPositionId + 1},
Expand All @@ -605,7 +605,7 @@ func (s *KeeperTestSuite) TestTransferPositions_Events() {
hasSpreadRewardsToClaim: true,
numPositionsToCreate: 3,
expectedTransferPositionsEvent: 1,
expectedMessageEvents: 9, // 3 for collect incentives claim send, 3 for collect incentives forfeit send, 3 for collect spread rewards claim send
expectedMessageEvents: 0,
},
"two position IDs, second ID does not exist": {
positionIds: []uint64{DefaultPositionId, DefaultPositionId + 1},
Expand Down
8 changes: 0 additions & 8 deletions x/concentrated-liquidity/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,14 +704,6 @@ func (k Keeper) transferPositions(ctx sdk.Context, positionIds []uint64, sender
return types.LockNotMatureError{PositionId: position.PositionId, LockId: lockId}
}

// Collect any outstanding incentives and rewards for the position.
if _, err := k.collectSpreadRewards(ctx, sender, positionId); err != nil {
return err
}
if _, _, err := k.collectIncentives(ctx, sender, positionId); err != nil {
return err
}

// Delete the KVStore entries for the position.
err = k.deletePosition(ctx, positionId, sender, position.PoolId)
if err != nil {
Expand Down
27 changes: 15 additions & 12 deletions x/concentrated-liquidity/position_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2440,31 +2440,34 @@ func (s *KeeperTestSuite) TestTransferPositions() {
s.Require().Equal(oldPosition, newPosition)
}

// Check that the incentives and spread rewards went to the old owner
// Check that the old owner's balance did not change due to the transfer
postTransferOriginalOwnerFunds := s.App.BankKeeper.GetAllBalances(s.Ctx, oldOwner)
expectedTransferOriginalOwnerFunds := preTransferOwnerFunds.Add(totalExpectedRewards...)
s.Require().Equal(expectedTransferOriginalOwnerFunds.String(), postTransferOriginalOwnerFunds.String())
s.Require().Equal(preTransferOwnerFunds.String(), postTransferOriginalOwnerFunds.String())

// Check that the new owner does not have any new funds
// Check that the new owner's balance did not change due to the transfer
postTransferNewOwnerFunds := s.App.BankKeeper.GetAllBalances(s.Ctx, newOwner)
s.Require().Equal(preTransferNewOwnerFunds, postTransferNewOwnerFunds)

// Test that claiming incentives/spread rewards with the new owner returns nothing
// Claim rewards and ensure that previously accrued incentives and spread rewards go to the new owner
for _, positionId := range tc.positionsToTransfer {
fundsToClaim, fundsToForefeit, err := s.App.ConcentratedLiquidityKeeper.GetClaimableIncentives(s.Ctx, positionId)
_, _, err := s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, newOwner, positionId)
s.Require().NoError(err)
s.Require().Equal(sdk.Coins{}, fundsToClaim)
s.Require().Equal(sdk.Coins{}, fundsToForefeit)

spreadRewards, err := s.App.ConcentratedLiquidityKeeper.GetClaimableSpreadRewards(s.Ctx, positionId)
_, err = s.App.ConcentratedLiquidityKeeper.CollectSpreadRewards(s.Ctx, newOwner, positionId)
s.Require().NoError(err)
s.Require().Equal(sdk.Coins(nil), spreadRewards)
}

// Ensure all rewards went to the new owner
postClaimRewardsNewOwnerBalance := s.App.BankKeeper.GetAllBalances(s.Ctx, newOwner)
s.Require().Equal(totalExpectedRewards, postClaimRewardsNewOwnerBalance)

// Ensure no rewards went to the old owner
postClaimRewardsOldOwnerBalance := s.App.BankKeeper.GetAllBalances(s.Ctx, oldOwner)
s.Require().Equal(preTransferOwnerFunds.String(), postClaimRewardsOldOwnerBalance.String())

// Test that adding incentives/spread rewards and then claiming returns it to the new owner, and the old owner does not get anything
totalSpreadRewards := s.fundSpreadRewardsAddr(s.Ctx, pool.GetSpreadRewardsAddress(), tc.inRangePositions)
totalIncentives := s.fundIncentiveAddr(pool.GetIncentivesAddress(), tc.inRangePositions)
totalExpectedRewards := totalSpreadRewards.Add(totalIncentives...)
totalExpectedRewards := totalSpreadRewards.Add(totalIncentives...).Add(totalExpectedRewards...)
s.addUptimeGrowthInsideRange(s.Ctx, pool.GetId(), apptesting.DefaultLowerTick+1, DefaultLowerTick, DefaultUpperTick, expectedUptimes.hundredTokensMultiDenom)
s.AddToSpreadRewardAccumulator(pool.GetId(), sdk.NewDecCoin(ETH, osmomath.NewInt(10)))
for _, positionId := range tc.positionsToTransfer {
Expand Down

0 comments on commit 994b773

Please sign in to comment.