Skip to content

Commit

Permalink
refactor: reduce number of returns for creating full range position (#…
Browse files Browse the repository at this point in the history
…6004)

* refactor: reduce number of returns for creating full range position

* update changelog

---------

Co-authored-by: devbot-wizard <141283918+devbot-wizard@users.noreply.github.com>
(cherry picked from commit 07c8556)

# Conflicts:
#	CHANGELOG.md
#	app/upgrades/v17/upgrades.go
#	x/concentrated-liquidity/lp_test.go
#	x/concentrated-liquidity/position.go
#	x/concentrated-liquidity/position_test.go
#	x/superfluid/keeper/export_test.go
#	x/superfluid/keeper/migrate.go
  • Loading branch information
p0mvn authored and mergify[bot] committed Aug 9, 2023
1 parent 5da2a20 commit a1c812d
Show file tree
Hide file tree
Showing 23 changed files with 473 additions and 219 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### State Breaking

<<<<<<< HEAD
### Bug Fixes
=======
* [#5983](https://github.com/osmosis-labs/osmosis/pull/5983) refactor(CL): 6 return values in CL CreatePosition with a struct
* [#6004](https://github.com/osmosis-labs/osmosis/pull/6004) reduce number of returns for creating full range position
>>>>>>> 07c85560 (refactor: reduce number of returns for creating full range position (#6004))
### Misc Improvements
* [#5976](https://github.com/osmosis-labs/osmosis/pull/5976) chore[e2e]: faster epoch
Expand Down
8 changes: 4 additions & 4 deletions app/apptesting/concentrated_liquidity.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ func (s *KeeperTestHelper) PrepareConcentratedPoolWithCoinsAndLockedFullRangePos
clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], denom1, denom2, DefaultTickSpacing, sdk.ZeroDec())
fundCoins := sdk.NewCoins(sdk.NewCoin(denom1, DefaultCoinAmount), sdk.NewCoin(denom2, DefaultCoinAmount))
s.FundAcc(s.TestAccs[0], fundCoins)
positionId, _, _, _, concentratedLockId, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), s.TestAccs[0], fundCoins, time.Hour*24*14)
positionData, concentratedLockId, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), s.TestAccs[0], fundCoins, time.Hour*24*14)
s.Require().NoError(err)
clPool, err = s.App.ConcentratedLiquidityKeeper.GetConcentratedPoolById(s.Ctx, clPool.GetId())
s.Require().NoError(err)
return clPool, concentratedLockId, positionId
return clPool, concentratedLockId, positionData.ID
}

// PrepareCustomConcentratedPool sets up a concentrated liquidity pool with the custom parameters.
Expand Down Expand Up @@ -113,9 +113,9 @@ func (s *KeeperTestHelper) PrepareMultipleConcentratedPools(poolsToCreate uint16
// CreateFullRangePosition creates a full range position and returns position id and the liquidity created.
func (s *KeeperTestHelper) CreateFullRangePosition(pool types.ConcentratedPoolExtension, coins sdk.Coins) (uint64, sdk.Dec) {
s.FundAcc(s.TestAccs[0], coins)
positionId, _, _, liquidity, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coins)
positionData, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coins)
s.Require().NoError(err)
return positionId, liquidity
return positionData.ID, positionData.Liquidity
}

// WithdrawFullRangePosition withdraws given liquidity from a position specified by id.
Expand Down
4 changes: 2 additions & 2 deletions app/upgrades/v16/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func CreateUpgradeHandler(

// Create a full range position via the community pool with the funds that were swapped.
fullRangeOsmoDaiCoins := sdk.NewCoins(respectiveOsmo, oneDai)
_, actualOsmoAmtUsed, actualDaiAmtUsed, _, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeOsmoDaiCoins)
positionData, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeOsmoDaiCoins)
if err != nil {
return nil, err
}
Expand All @@ -157,7 +157,7 @@ func CreateUpgradeHandler(

// Remove coins we used from the community pool to make the CL position
feePool := keepers.DistrKeeper.GetFeePool(ctx)
fulllRangeOsmoDaiCoinsUsed := sdk.NewCoins(sdk.NewCoin(DesiredDenom0, actualOsmoAmtUsed), sdk.NewCoin(DAIIBCDenom, actualDaiAmtUsed))
fulllRangeOsmoDaiCoinsUsed := sdk.NewCoins(sdk.NewCoin(DesiredDenom0, positionData.Amount0), sdk.NewCoin(DAIIBCDenom, positionData.Amount1))
newPool, negative := feePool.CommunityPool.SafeSub(sdk.NewDecCoinsFromCoins(fulllRangeOsmoDaiCoinsUsed...))
if negative {
return nil, fmt.Errorf("community pool cannot be negative: %s", newPool)
Expand Down
182 changes: 182 additions & 0 deletions app/upgrades/v17/upgrades.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
package v17

import (
"fmt"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"

cltypes "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types"
gammmigration "github.com/osmosis-labs/osmosis/v17/x/gamm/types/migration"
superfluidtypes "github.com/osmosis-labs/osmosis/v17/x/superfluid/types"

"github.com/osmosis-labs/osmosis/v17/app/keepers"
"github.com/osmosis-labs/osmosis/v17/app/upgrades"
"github.com/osmosis-labs/osmosis/v17/x/protorev/types"

poolmanagertypes "github.com/osmosis-labs/osmosis/v17/x/poolmanager/types"
)

func CreateUpgradeHandler(
mm *module.Manager,
configurator module.Configurator,
bpm upgrades.BaseAppParamManager,
keepers *keepers.AppKeepers,
) upgradetypes.UpgradeHandler {
return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
// Run migrations before applying any other state changes.
// NOTE: DO NOT PUT ANY STATE CHANGES BEFORE RunMigrations().
migrations, err := mm.RunMigrations(ctx, configurator, fromVM)
if err != nil {
return nil, err
}

// get all the existing CL pools
pools, err := keepers.ConcentratedLiquidityKeeper.GetPools(ctx)
if err != nil {
return nil, err
}

// migrate twap records for CL Pools
err = FlipTwapSpotPriceRecords(ctx, pools, keepers)
if err != nil {
return nil, err
}

// Get community pool address.
communityPoolAddress := keepers.AccountKeeper.GetModuleAddress(distrtypes.ModuleName)

// fullRangeCoinsUsed tracks the coins we use in the below for loop from the community pool to create the full range position for each new pool.
fullRangeCoinsUsed := sdk.NewCoins()

poolLinks := []gammmigration.BalancerToConcentratedPoolLink{}

assetPairs := InitializeAssetPairs(ctx, keepers)

for _, assetPair := range assetPairs {
// Create a concentrated liquidity pool for asset pair.
clPool, err := keepers.GAMMKeeper.CreateConcentratedPoolFromCFMM(ctx, assetPair.LinkedClassicPool, assetPair.BaseAsset, assetPair.SpreadFactor, TickSpacing)
if err != nil {
return nil, err
}
clPoolId := clPool.GetId()
clPoolDenom := cltypes.GetConcentratedLockupDenomFromPoolId(clPoolId)

// Add the pool link to the list of pool links (we set them all at once later)
poolLinks = append(poolLinks, gammmigration.BalancerToConcentratedPoolLink{
BalancerPoolId: assetPair.LinkedClassicPool,
ClPoolId: clPoolId,
})

// Determine the amount of baseAsset that can be bought with 1 OSMO.
oneOsmo := sdk.NewCoin(QuoteAsset, sdk.NewInt(1000000))
linkedClassicPool, err := keepers.PoolManagerKeeper.GetPool(ctx, assetPair.LinkedClassicPool)
if err != nil {
return nil, err
}
respectiveBaseAsset, err := keepers.GAMMKeeper.CalcOutAmtGivenIn(ctx, linkedClassicPool, oneOsmo, assetPair.BaseAsset, sdk.ZeroDec())
if err != nil {
return nil, err
}

// Create a full range position via the community pool with the funds we calculated above.
fullRangeCoins := sdk.NewCoins(respectiveBaseAsset, oneOsmo)
positionData, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeCoins)
if err != nil {
return nil, err
}

// Track the coins used to create the full range position (we manually update the fee pool later all at once).
fullRangeCoinsUsed = fullRangeCoinsUsed.Add(sdk.NewCoins(sdk.NewCoin(QuoteAsset, positionData.Amount1), sdk.NewCoin(assetPair.BaseAsset, positionData.Amount0))...)

// If pair was previously superfluid enabled, add the cl pool's full range denom as an authorized superfluid asset.
if assetPair.Superfluid {
superfluidAsset := superfluidtypes.SuperfluidAsset{
Denom: clPoolDenom,
AssetType: superfluidtypes.SuperfluidAssetTypeConcentratedShare,
}
err = keepers.SuperfluidKeeper.AddNewSuperfluidAsset(ctx, superfluidAsset)
if err != nil {
return nil, err
}
}

clPoolTwapRecords, err := keepers.TwapKeeper.GetAllMostRecentRecordsForPool(ctx, clPoolId)
if err != nil {
return nil, err
}

for _, twapRecord := range clPoolTwapRecords {
twapRecord.LastErrorTime = time.Time{}
keepers.TwapKeeper.StoreNewRecord(ctx, twapRecord)
}
}

// Set the migration links in x/gamm.
// This will also migrate the CFMM distribution records to point to the new CL pools.
err = keepers.GAMMKeeper.UpdateMigrationRecords(ctx, poolLinks)
if err != nil {
return nil, err
}

// Because we had done direct sends from the community pool, we need to manually change the fee pool to reflect the change in balance.

// Remove coins we used from the community pool to make the CL positions
feePool := keepers.DistrKeeper.GetFeePool(ctx)
newPool, negative := feePool.CommunityPool.SafeSub(sdk.NewDecCoinsFromCoins(fullRangeCoinsUsed...))
if negative {
return nil, fmt.Errorf("community pool cannot be negative: %s", newPool)
}

// Update and set the new fee pool
feePool.CommunityPool = newPool
keepers.DistrKeeper.SetFeePool(ctx, feePool)

// Reset the pool weights upon upgrade. This will add support for CW pools on ProtoRev.
keepers.ProtoRevKeeper.SetInfoByPoolType(ctx, types.DefaultPoolTypeInfo)

return migrations, nil
}
}

// FlipTwapSpotPriceRecords flips the denoms and spot price of twap record of a given pool.
func FlipTwapSpotPriceRecords(ctx sdk.Context, pools []poolmanagertypes.PoolI, keepers *keepers.AppKeepers) error {
for _, pool := range pools {
poolId := pool.GetId()
twapRecordHistoricalPoolIndexed, err := keepers.TwapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(ctx, poolId)
if err != nil {
return err
}

for _, historicalTwapRecord := range twapRecordHistoricalPoolIndexed {
oldRecord := historicalTwapRecord
historicalTwapRecord.Asset0Denom, historicalTwapRecord.Asset1Denom = oldRecord.Asset1Denom, oldRecord.Asset0Denom
historicalTwapRecord.P0LastSpotPrice, historicalTwapRecord.P1LastSpotPrice = oldRecord.P1LastSpotPrice, oldRecord.P0LastSpotPrice

keepers.TwapKeeper.StoreHistoricalTWAP(ctx, historicalTwapRecord)
keepers.TwapKeeper.DeleteHistoricalRecord(ctx, oldRecord)
}

clPoolTwapRecords, err := keepers.TwapKeeper.GetAllMostRecentRecordsForPool(ctx, poolId)
if err != nil {
return err
}

for _, twapRecord := range clPoolTwapRecords {
twapRecord.LastErrorTime = time.Time{}
oldRecord := twapRecord

twapRecord.Asset0Denom, twapRecord.Asset1Denom = oldRecord.Asset1Denom, oldRecord.Asset0Denom
twapRecord.P0LastSpotPrice, twapRecord.P1LastSpotPrice = oldRecord.P1LastSpotPrice, oldRecord.P0LastSpotPrice

keepers.TwapKeeper.StoreNewRecord(ctx, twapRecord)
keepers.TwapKeeper.DeleteMostRecentRecord(ctx, oldRecord)
}
}

return nil
}
52 changes: 35 additions & 17 deletions x/concentrated-liquidity/lp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@ import (

"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/osmoutils"
<<<<<<< HEAD
cl "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity"
"github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/model"
clmodel "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/model"
types "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/types"
=======
cl "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity"
"github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/model"
clmodel "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/model"
cltypes "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types"
types "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types"
>>>>>>> 07c85560 (refactor: reduce number of returns for creating full range position (#6004))
)

type lpTest struct {
Expand Down Expand Up @@ -381,20 +389,36 @@ const (
)

func (s *KeeperTestSuite) createPositionWithLockState(ls lockState, poolId uint64, owner sdk.AccAddress, providedCoins sdk.Coins, dur time.Duration) (uint64, sdk.Dec) {
<<<<<<< HEAD
var liquidityCreated sdk.Dec
var positionId uint64
var err error
=======
var (
positionData cl.CreatePositionData
fullRangePositionData cltypes.CreateFullRangePositionData
err error
)

>>>>>>> 07c85560 (refactor: reduce number of returns for creating full range position (#6004))
if ls == locked {
positionId, _, _, liquidityCreated, _, err = s.clk.CreateFullRangePositionLocked(s.Ctx, poolId, owner, providedCoins, dur)
fullRangePositionData, _, err = s.clk.CreateFullRangePositionLocked(s.Ctx, poolId, owner, providedCoins, dur)
} else if ls == unlocking {
positionId, _, _, liquidityCreated, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur+time.Hour)
fullRangePositionData, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur+time.Hour)
} else if ls == unlocked {
positionId, _, _, liquidityCreated, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur-time.Hour)
fullRangePositionData, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur-time.Hour)
} else {
<<<<<<< HEAD
positionId, _, _, liquidityCreated, _, _, err = s.clk.CreatePosition(s.Ctx, poolId, owner, providedCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
=======
positionData, err = s.clk.CreatePosition(s.Ctx, poolId, owner, providedCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
s.Require().NoError(err)
return positionData.ID, positionData.Liquidity
>>>>>>> 07c85560 (refactor: reduce number of returns for creating full range position (#6004))
}
// full range case
s.Require().NoError(err)
return positionId, liquidityCreated
return fullRangePositionData.ID, fullRangePositionData.Liquidity
}

func (s *KeeperTestSuite) TestWithdrawPosition() {
Expand Down Expand Up @@ -1952,8 +1976,8 @@ func (s *KeeperTestSuite) TestIsLockMature() {
s.Run(name, func() {
tc := tc
var (
positionId uint64
concentratedLockId uint64
positionData types.CreateFullRangePositionData
err error
)
s.SetupTest()
Expand All @@ -1965,17 +1989,15 @@ func (s *KeeperTestSuite) TestIsLockMature() {
s.FundAcc(s.TestAccs[0], coinsToFund)

if tc.unlockingPosition {
positionId, _, _, _, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
s.Require().NoError(err)
positionData, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
} else if tc.lockedPosition {
positionId, _, _, _, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
s.Require().NoError(err)
positionData, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
} else {
positionId, _, _, _, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund)
s.Require().NoError(err)
positionData, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund)
}
s.Require().NoError(err)

_, err = s.App.ConcentratedLiquidityKeeper.GetPosition(s.Ctx, positionId)
_, err = s.App.ConcentratedLiquidityKeeper.GetPosition(s.Ctx, positionData.ID)
s.Require().NoError(err)

// Increment block time by a second to ensure test cases with zero lock duration are in the past
Expand All @@ -1984,11 +2006,7 @@ func (s *KeeperTestSuite) TestIsLockMature() {
// System under test
lockIsMature, _ := s.App.ConcentratedLiquidityKeeper.IsLockMature(s.Ctx, concentratedLockId)

if tc.expectedLockIsMature {
s.Require().True(lockIsMature)
} else {
s.Require().False(lockIsMature)
}
s.Require().Equal(tc.expectedLockIsMature, lockIsMature)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion x/concentrated-liquidity/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ func (s *KeeperTestSuite) TestGetUserUnbondingPositions() {

// Create 3 locked positions
for i := 0; i < 3; i++ {
_, _, _, _, _, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), defaultAddress, defaultFunds, time.Hour)
_, _, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), defaultAddress, defaultFunds, time.Hour)
s.Require().NoError(err)
}

Expand Down
Loading

0 comments on commit a1c812d

Please sign in to comment.