Skip to content

Commit

Permalink
allow gov module account to transfer any position
Browse files Browse the repository at this point in the history
  • Loading branch information
czarcas7ic committed Mar 18, 2024
1 parent be76d37 commit 46421f1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
21 changes: 12 additions & 9 deletions x/concentrated-liquidity/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
sdkprefix "github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/query"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/osmoutils"
Expand Down Expand Up @@ -669,7 +670,7 @@ func (k Keeper) updateFullRangeLiquidityInPool(ctx sdk.Context, poolId uint64, l
// transferPositions transfers ownership of a set of positions from a sender to a recipient.
// It first checks if the provided position IDs are unique. If not, it returns a DuplicatePositionIdsError.
// For each position ID, it retrieves the corresponding position and checks if the sender is the owner of the position.
// If the sender is not the owner, it returns an error.
// If the sender is not the owner (or the governance module account), it returns an error.
// It then checks if the position has an active underlying lock, and if so, returns an error.
// It then collects any outstanding incentives and rewards for the position, deletes the KVStore entries for the position,
// and restores the position under the recipient's account.
Expand All @@ -684,14 +685,17 @@ func (k Keeper) transferPositions(ctx sdk.Context, positionIds []uint64, sender
return types.DuplicatePositionIdsError{PositionIds: positionIds}
}

// Check if the sender is the governance module account.
isGovModuleSender := sender.Equals(k.accountKeeper.GetModuleAccount(ctx, govtypes.ModuleName).GetAddress())

for _, positionId := range positionIds {
position, err := k.GetPosition(ctx, positionId)
if err != nil {
return err
}

// Check that the sender is the owner of the position.
if position.Address != sender.String() {
// If the sender is not the governance module, verify that the sender is the owner of the position.
if !isGovModuleSender && position.Address != sender.String() {
return types.PositionOwnerMismatchError{PositionOwner: position.Address, Sender: sender.String()}
}

Expand All @@ -705,22 +709,21 @@ func (k Keeper) transferPositions(ctx sdk.Context, positionIds []uint64, sender
}

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

// Delete the KVStore entries for the position.
err = k.deletePosition(ctx, positionId, sender, position.PoolId)
err = k.deletePosition(ctx, positionId, positionOwnerAddr, position.PoolId)
if err != nil {
return err
}

// There exists special logic branches we take if a position is the last position in a pool and it is withdrawn.
// It makes sense to accept the small annoyance of preventing a position from being transferred if it is the last position in a pool
// instead of considering all the edge cases that would arise if we allowed it.
// Check if transferring the last position in a pool.
anyPositionsRemainingInPool, err := k.HasAnyPositionForPool(ctx, position.PoolId)
if err != nil {
return err
Expand Down
16 changes: 15 additions & 1 deletion x/concentrated-liquidity/position_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/query"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/osmoutils"
Expand Down Expand Up @@ -2279,6 +2280,7 @@ func (s *KeeperTestSuite) TestTransferPositions() {
positionsToTransfer []uint64
setupUnownedPosition bool
isLastPositionInPool bool
isGovAddress bool

expectedError error
}{
Expand Down Expand Up @@ -2314,6 +2316,13 @@ func (s *KeeperTestSuite) TestTransferPositions() {
outOfRangePositions: []uint64{DefaultPositionId + 1, DefaultPositionId + 2},
positionsToTransfer: []uint64{DefaultPositionId, DefaultPositionId + 2},
},
"three position IDs, not an owner of one of them but caller is gov address": {
inRangePositions: []uint64{DefaultPositionId, DefaultPositionId + 1},
outOfRangePositions: []uint64{DefaultPositionId + 2},
positionsToTransfer: []uint64{DefaultPositionId, DefaultPositionId + 1, DefaultPositionId + 2},
setupUnownedPosition: true,
isGovAddress: true,
},
"error: two position IDs, second ID does not exist": {
inRangePositions: []uint64{DefaultPositionId, DefaultPositionId + 1},
positionsToTransfer: []uint64{DefaultPositionId, DefaultPositionId + 3},
Expand Down Expand Up @@ -2387,8 +2396,13 @@ func (s *KeeperTestSuite) TestTransferPositions() {
// Account funds of new owner
preTransferNewOwnerFunds := s.App.BankKeeper.GetAllBalances(s.Ctx, newOwner)

transferCaller := oldOwner
if tc.isGovAddress {
transferCaller = s.App.AccountKeeper.GetModuleAddress(govtypes.ModuleName)
}

// System under test
err = s.App.ConcentratedLiquidityKeeper.TransferPositions(s.Ctx, tc.positionsToTransfer, oldOwner, newOwner)
err = s.App.ConcentratedLiquidityKeeper.TransferPositions(s.Ctx, tc.positionsToTransfer, transferCaller, newOwner)

if tc.expectedError != nil {
s.Require().Error(err)
Expand Down

0 comments on commit 46421f1

Please sign in to comment.