From 994b7731c9c5144fa8d2dd60784f12a3e9ae4ceb Mon Sep 17 00:00:00 2001 From: alpo Date: Tue, 19 Mar 2024 20:04:27 -0700 Subject: [PATCH] implement retaining rewards for transfers --- x/concentrated-liquidity/msg_server_test.go | 10 ++++---- x/concentrated-liquidity/position.go | 8 ------ x/concentrated-liquidity/position_test.go | 27 ++++++++++++--------- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/x/concentrated-liquidity/msg_server_test.go b/x/concentrated-liquidity/msg_server_test.go index 10d5b5b1a2d..45104ea6357 100644 --- a/x/concentrated-liquidity/msg_server_test.go +++ b/x/concentrated-liquidity/msg_server_test.go @@ -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 }{ @@ -572,14 +572,14 @@ 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}, @@ -587,7 +587,7 @@ func (s *KeeperTestSuite) TestTransferPositions_Events() { 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}, @@ -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}, diff --git a/x/concentrated-liquidity/position.go b/x/concentrated-liquidity/position.go index efc7abcf467..7a1a3ddb7b6 100644 --- a/x/concentrated-liquidity/position.go +++ b/x/concentrated-liquidity/position.go @@ -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 { diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index 0a8d52eb1bc..d8d8e6e45aa 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -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 {