Skip to content

Commit

Permalink
feat: allow gov module account to transfer any CL position (#7768)
Browse files Browse the repository at this point in the history
* allow gov module account to transfer any position

* add changlelog

* Update position_test.go

* fix test
  • Loading branch information
czarcas7ic authored Mar 22, 2024
1 parent 4fe2498 commit e3f0689
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#7689](https://github.com/osmosis-labs/osmosis/pull/7689) Make CL price estimations not cause state writes (speed and gas improvements)
* [#7745](https://github.com/osmosis-labs/osmosis/pull/7745) Add gauge id query to stargate whitelist
* [#7747](https://github.com/osmosis-labs/osmosis/pull/7747) Remove redundant call to incentive collection in CL position withdrawal logic
* [#7768](https://github.com/osmosis-labs/osmosis/pull/7768) Allow governance module account to transfer any CL position
* [#7746](https://github.com/osmosis-labs/osmosis/pull/7746) Make forfeited incentives redeposit into the pool instead of sending to community pool
* [#7785](https://github.com/osmosis-labs/osmosis/pull/7785) Remove reward claiming during position transfers

Expand Down
19 changes: 12 additions & 7 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 deletes the KVStore entries for the position, and restores the position under the recipient's account.
// If any of these operations fail, it returns the corresponding error.
Expand All @@ -683,14 +684,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 @@ -703,15 +707,16 @@ func (k Keeper) transferPositions(ctx sdk.Context, positionIds []uint64, sender
return types.LockNotMatureError{PositionId: position.PositionId, LockId: lockId}
}

// Since the caller can be either the owner or the governance module (verified above), we can safely utilize the address directly from the position.
positionOwnerAddr := sdk.MustAccAddressFromBech32(position.Address)

// 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
15 changes: 14 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 @@ -2280,6 +2281,7 @@ func (s *KeeperTestSuite) TestTransferPositions() {
positionsToTransfer []uint64
setupUnownedPosition bool
isLastPositionInPool bool
isGovAddress bool

expectedError error
}{
Expand Down Expand Up @@ -2315,6 +2317,12 @@ func (s *KeeperTestSuite) TestTransferPositions() {
outOfRangePositions: []uint64{DefaultPositionId + 1, DefaultPositionId + 2},
positionsToTransfer: []uint64{DefaultPositionId, DefaultPositionId + 2},
},
"three position IDs, not an owner of any of them but caller is gov address": {
inRangePositions: []uint64{DefaultPositionId, DefaultPositionId + 1},
outOfRangePositions: []uint64{DefaultPositionId + 2},
positionsToTransfer: []uint64{DefaultPositionId, DefaultPositionId + 1, DefaultPositionId + 2},
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 @@ -2388,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 e3f0689

Please sign in to comment.