From 063602cc0dbf3c200390446461c485a5c79fdb13 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Fri, 28 Jul 2023 18:15:59 -0400 Subject: [PATCH 01/18] init --- x/protorev/keeper/keeper.go | 29 ++++--- x/protorev/keeper/rebalance.go | 114 ++++++++++++++++++++++++++- x/protorev/types/constants.go | 4 + x/protorev/types/expected_keepers.go | 11 +++ 4 files changed, 142 insertions(+), 16 deletions(-) diff --git a/x/protorev/keeper/keeper.go b/x/protorev/keeper/keeper.go index 7145aba4e58..ef7f0e75f59 100644 --- a/x/protorev/keeper/keeper.go +++ b/x/protorev/keeper/keeper.go @@ -19,11 +19,12 @@ type ( storeKey storetypes.StoreKey paramstore paramtypes.Subspace - accountKeeper types.AccountKeeper - bankKeeper types.BankKeeper - gammKeeper types.GAMMKeeper - epochKeeper types.EpochKeeper - poolmanagerKeeper types.PoolManagerKeeper + accountKeeper types.AccountKeeper + bankKeeper types.BankKeeper + gammKeeper types.GAMMKeeper + epochKeeper types.EpochKeeper + poolmanagerKeeper types.PoolManagerKeeper + concentratedLiquidityKeeper types.ConcentratedLiquidityKeeper } ) @@ -36,6 +37,7 @@ func NewKeeper( gammKeeper types.GAMMKeeper, epochKeeper types.EpochKeeper, poolmanagerKeeper types.PoolManagerKeeper, + concentratedLiquidityKeeper types.ConcentratedLiquidityKeeper, ) Keeper { // set KeyTable if it has not already been set if !ps.HasKeyTable() { @@ -43,14 +45,15 @@ func NewKeeper( } return Keeper{ - cdc: cdc, - storeKey: storeKey, - paramstore: ps, - accountKeeper: accountKeeper, - bankKeeper: bankKeeper, - gammKeeper: gammKeeper, - epochKeeper: epochKeeper, - poolmanagerKeeper: poolmanagerKeeper, + cdc: cdc, + storeKey: storeKey, + paramstore: ps, + accountKeeper: accountKeeper, + bankKeeper: bankKeeper, + gammKeeper: gammKeeper, + epochKeeper: epochKeeper, + poolmanagerKeeper: poolmanagerKeeper, + concentratedLiquidityKeeper: concentratedLiquidityKeeper, } } diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index e3b658929ae..0084acaa82f 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -133,7 +133,7 @@ func (k Keeper) FindMaxProfitForRoute(ctx sdk.Context, route RouteMetaData, rema } // Extend the search range if the max input amount is too small - curLeft, curRight = k.ExtendSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight) + curLeft, curRight = k.UpdateSearchRange(ctx, route, inputDenom, curLeft, curRight) // Binary search to find the max profit for iteration := 0; curLeft.LT(curRight) && iteration < types.MaxIterations; iteration++ { @@ -167,8 +167,116 @@ func (k Keeper) FindMaxProfitForRoute(ctx sdk.Context, route RouteMetaData, rema return tokenIn, profit, nil } +// UpdateSearchRange updates the search range for the binary search. +func (k Keeper) UpdateSearchRange( + ctx sdk.Context, + route RouteMetaData, + inputDenom string, + curLeft, curRight sdk.Int, +) (sdk.Int, sdk.Int) { + // Retrieve all concentrated liquidity pools in the route + clPools := make([]uint64, 0) + inputMap := make(map[uint64]string) + prevInput := inputDenom + + for _, route := range route.Route { + pool, err := k.poolmanagerKeeper.GetPool(ctx, route.PoolId) + if err != nil { + return sdk.OneInt(), sdk.OneInt() + } + + if pool.GetType() == poolmanagertypes.Concentrated { + clPools = append(clPools, route.PoolId) + inputMap[route.PoolId] = prevInput + } + + prevInput = route.TokenOutDenom + } + + // If there are any concentrated liquidity pools in the route, then we might want + // to reduce the search range since it is gas intensive to move across several ticks + // + // TODO: Refactor a bit so that its clear that its only the use case for CL pools + updatedMax := k.ReduceSearchRangeIfNeeded(ctx, clPools, inputDenom, inputMap, curLeft, curRight) + if updatedMax.LT(curRight) { + return curLeft, updatedMax + } + + if updatedMax.GT(types.MaxInputAmount) { + updatedMax = types.MaxInputAmount + } + + return k.ExtendSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight, updatedMax) +} + +// ReduceSearchRangeIfNeeded reduces the search range if the max input amount is too large. This is +// particularly useful for concentrated liquidity pools where the max input amount is very large and +// the binary search is too gas intensive. +func (k Keeper) ReduceSearchRangeIfNeeded( + ctx sdk.Context, + clPools []uint64, + inputDenom string, + inputMap map[uint64]string, + curLeft, curRight sdk.Int, +) sdk.Int { + // Iterate through all CL pools and determine the maximal amount of input that can be used + // respecting the max ticks moved + baseDenom, err := k.GetAllBaseDenoms(ctx) + if err != nil { + return sdk.ZeroInt() + } + + uosmoStepSize := sdk.ZeroInt() + foundUosmo := false + for _, denom := range baseDenom { + if denom.Denom == types.OsmosisDenomination { + uosmoStepSize = denom.StepSize + foundUosmo = true + break + } + } + + if !foundUosmo { + return sdk.ZeroInt() + } + + maxAmountIn := types.MaxInputAmount.Mul(uosmoStepSize) + for _, poolId := range clPools { + maxInAmount, _, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( + ctx, + poolId, + inputMap[poolId], + types.MaxTicksMoved, + ) + + // In the case where we cannot calculate the max in amount, we short circuit the search + // and return the current left and right bounds as equal. This means no search will be executed. + if err != nil { + return sdk.ZeroInt() + } + + // Convert the input amount for comparison + amount, err := k.ConvertProfits(ctx, maxInAmount, maxInAmount.Amount) + if err != nil { + return sdk.ZeroInt() + } + + // If the max input amount is less than the current max input amount, then we update the current max input amount + if amount.LT(maxAmountIn) { + maxAmountIn = amount + } + } + + return maxAmountIn.Quo(uosmoStepSize) +} + // Determine if the binary search range needs to be extended -func (k Keeper) ExtendSearchRangeIfNeeded(ctx sdk.Context, route RouteMetaData, inputDenom string, curLeft, curRight sdk.Int) (sdk.Int, sdk.Int) { +func (k Keeper) ExtendSearchRangeIfNeeded( + ctx sdk.Context, + route RouteMetaData, + inputDenom string, + curLeft, curRight, updatedMax sdk.Int, +) (sdk.Int, sdk.Int) { // Get the profit for the maximum amount in _, maxInProfit, err := k.EstimateMultihopProfit(ctx, inputDenom, curRight.Mul(route.StepSize), route.Route) if err != nil { @@ -186,7 +294,7 @@ func (k Keeper) ExtendSearchRangeIfNeeded(ctx sdk.Context, route RouteMetaData, // Change the range of the binary search if the profit is still increasing if maxInProfitPlusOne.GT(maxInProfit) { curLeft = curRight - curRight = types.ExtendedMaxInputAmount + curRight = updatedMax } } diff --git a/x/protorev/types/constants.go b/x/protorev/types/constants.go index 254eefb8013..ecb2de1bfb9 100644 --- a/x/protorev/types/constants.go +++ b/x/protorev/types/constants.go @@ -27,6 +27,10 @@ const MaxPoolPointsPerTx uint64 = 50 // to the maximum execution time (in ms) of protorev per block const MaxPoolPointsPerBlock uint64 = 200 +// Max number of ticks we can move in a concentrated pool swap. This will be parameterized in a +// follow up PR. +const MaxTicksMoved uint64 = 5 + // ---------------- Module Profit Splitting Constants ---------------- // // Year 1 (20% of total profit) diff --git a/x/protorev/types/expected_keepers.go b/x/protorev/types/expected_keepers.go index 03b7ba41eb3..8b12c09085f 100644 --- a/x/protorev/types/expected_keepers.go +++ b/x/protorev/types/expected_keepers.go @@ -61,3 +61,14 @@ type PoolManagerKeeper interface { type EpochKeeper interface { GetEpochInfo(ctx sdk.Context, identifier string) epochtypes.EpochInfo } + +// ConcentratedLiquidityKeeper defines the ConcentratedLiquidity contract that must be fulfilled when +// creating a x/protorev keeper. +type ConcentratedLiquidityKeeper interface { + ComputeMaxInAmtGivenMaxTicksCrossed( + ctx sdk.Context, + poolId uint64, + tokenInDenom string, + maxTicksCrossed uint64, + ) (maxTokenIn, resultingTokenOut sdk.Coin, err error) +} From e2b628e2d8225f852edb5982ef24f86b1f56c6f0 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Mon, 31 Jul 2023 14:03:57 -0400 Subject: [PATCH 02/18] testing update --- app/keepers/keepers.go | 8 +- x/protorev/keeper/epoch_hook_test.go | 6 +- x/protorev/keeper/hooks_test.go | 6 +- x/protorev/keeper/keeper_test.go | 34 +++++++- x/protorev/keeper/posthandler_test.go | 2 +- x/protorev/keeper/rebalance.go | 111 ++++++++++++++------------ x/protorev/keeper/rebalance_test.go | 44 ++++++++++ x/protorev/keeper/routes_test.go | 4 +- 8 files changed, 153 insertions(+), 62 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 38709d660f2..c46085b0648 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -357,7 +357,13 @@ func (appKeepers *AppKeepers) InitNormalKeepers( protorevKeeper := protorevkeeper.NewKeeper( appCodec, appKeepers.keys[protorevtypes.StoreKey], appKeepers.GetSubspace(protorevtypes.ModuleName), - appKeepers.AccountKeeper, appKeepers.BankKeeper, appKeepers.GAMMKeeper, appKeepers.EpochsKeeper, appKeepers.PoolManagerKeeper) + appKeepers.AccountKeeper, + appKeepers.BankKeeper, + appKeepers.GAMMKeeper, + appKeepers.EpochsKeeper, + appKeepers.PoolManagerKeeper, + appKeepers.ConcentratedLiquidityKeeper, + ) appKeepers.ProtoRevKeeper = &protorevKeeper txFeesKeeper := txfeeskeeper.NewKeeper( diff --git a/x/protorev/keeper/epoch_hook_test.go b/x/protorev/keeper/epoch_hook_test.go index d3f135514be..81b8f44ce66 100644 --- a/x/protorev/keeper/epoch_hook_test.go +++ b/x/protorev/keeper/epoch_hook_test.go @@ -152,8 +152,8 @@ func (s *KeeperTestSuite) TestUpdateHighestLiquidityPools() { // There are 2 pools with epochTwo and uosmo as denoms, // One in the GAMM module and one in the Concentrated Liquidity module. // pool with ID 48 has a liquidity value of 1,000,000 - // pool with ID 49 has a liquidity value of 2,000,000 - // pool with ID 49 should be returned as the highest liquidity pool + // pool with ID 50 has a liquidity value of 2,000,000 + // pool with ID 50 should be returned as the highest liquidity pool // We provide epochTwo as the input base denom, to test the method chooses the correct pool // across the GAMM and Concentrated Liquidity modules name: "Get highest liquidity pools for one GAMM pool and one Concentrated Liquidity pool", @@ -162,7 +162,7 @@ func (s *KeeperTestSuite) TestUpdateHighestLiquidityPools() { }, expectedBaseDenomPools: map[string]map[string]keeper.LiquidityPoolStruct{ "epochTwo": { - "uosmo": {Liquidity: sdk.Int(sdk.NewUintFromString("999999000000000001000000000000000000")), PoolId: 49}, + "uosmo": {Liquidity: sdk.Int(sdk.NewUintFromString("999999000000000001000000000000000000")), PoolId: 50}, }, }, }, diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 76f620c45cb..17c3e683147 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -108,14 +108,14 @@ func (s *KeeperTestSuite) TestSwapping() { param: param{ expectedTrades: []types.Trade{ { - Pool: 49, + Pool: 50, TokenIn: "uosmo", TokenOut: "epochTwo", }, }, executeSwap: func() { - route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 49, TokenOutDenom: "epochTwo"}} + route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 50, TokenOutDenom: "epochTwo"}} _, err := s.App.PoolManagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], route, sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewInt(1)) s.Require().NoError(err) @@ -606,7 +606,7 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { { name: "Non-Gamm Pool, Return Early Do Not Store Any Swaps", param: param{ - poolId: 49, + poolId: 50, denom: "uosmo", isJoin: true, expectedSwap: types.Trade{}, diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 1c378d5a025..780ee662839 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -888,6 +888,17 @@ func (s *KeeperTestSuite) setUpPools() { }, scalingFactors: []uint64{1, 1}, }, + { // Pool 49 - Used for CL testing + initialLiquidity: sdk.NewCoins( + sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000)), + sdk.NewCoin("epochTwo", sdk.NewInt(8_000_000_000_000)), + ), + poolParams: stableswap.PoolParams{ + SwapFee: sdk.NewDecWithPrec(0, 2), + ExitFee: sdk.NewDecWithPrec(0, 2), + }, + scalingFactors: []uint64{1, 1}, + }, } for _, pool := range s.stableSwapPools { @@ -895,13 +906,32 @@ func (s *KeeperTestSuite) setUpPools() { } // Create a concentrated liquidity pool for epoch_hook testing - // Pool 49 + // Pool 50 s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("epochTwo", "uosmo") // Create a cosmwasm pool for testing - // Pool 50 + // Pool 51 s.PrepareCosmWasmPool() + // Create a concentrated liquidity pool for range testing + // Pool 52 + // Create the CL pool + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[2], "epochTwo", "uosmo", 100, sdk.NewDecWithPrec(2, 3)) + fundCoins := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(1000000000000000000)), sdk.NewCoin("epochTwo", sdk.NewInt(1000000000000000000))) + s.FundAcc(s.TestAccs[2], fundCoins) + + // Create 100 ticks in the CL pool, 50 on each side + tokensProvided := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000000)), sdk.NewCoin("epochTwo", sdk.NewInt(10000000))) + amount0Min := sdk.NewInt(0) + amount1Min := sdk.NewInt(0) + lowerTick := int64(0) + upperTick := int64(100) + + for i := int64(0); i < 50; i++ { + s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, clPool.GetId(), s.TestAccs[2], tokensProvided, amount0Min, amount1Min, lowerTick-(100*i), upperTick-(100*i)) + s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, clPool.GetId(), s.TestAccs[2], tokensProvided, amount0Min, amount1Min, lowerTick+(100*i), upperTick+(100*i)) + } + // Set all of the pool info into the stores err := s.App.ProtoRevKeeper.UpdatePools(s.Ctx) s.Require().NoError(err) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 725652e7e66..1ac83b5153a 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -340,7 +340,7 @@ func (s *KeeperTestSuite) TestAnteHandle() { params: param{ trades: []types.Trade{ { - Pool: 51, + Pool: 53, TokenOut: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", TokenIn: "uosmo", }, diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 0084acaa82f..b0b68248109 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -1,6 +1,8 @@ package keeper import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v17/x/poolmanager/types" @@ -30,13 +32,10 @@ func (k Keeper) IterateRoutes(ctx sdk.Context, routes []RouteMetaData, remaining // If the profit is greater than zero, then we convert the profits to uosmo and compare profits in terms of uosmo if profit.GT(sdk.ZeroInt()) { - if inputCoin.Denom != types.OsmosisDenomination { - uosmoProfit, err := k.ConvertProfits(ctx, inputCoin, profit) - if err != nil { - k.Logger(ctx).Error("Error converting profits: ", err) - continue - } - profit = uosmoProfit + profit, err := k.ConvertProfits(ctx, inputCoin, profit) + if err != nil { + k.Logger(ctx).Error("Error converting profits: ", err) + continue } // Select the optimal route King of the Hill style (route with the highest profit will be executed) @@ -53,6 +52,10 @@ func (k Keeper) IterateRoutes(ctx sdk.Context, routes []RouteMetaData, remaining // ConvertProfits converts the profit denom to uosmo to allow for a fair comparison of profits func (k Keeper) ConvertProfits(ctx sdk.Context, inputCoin sdk.Coin, profit sdk.Int) (sdk.Int, error) { + if inputCoin.Denom == types.OsmosisDenomination { + return profit, nil + } + // Get highest liquidity pool ID for the input coin and uosmo conversionPoolID, err := k.GetPoolForDenomPair(ctx, types.OsmosisDenomination, inputCoin.Denom) if err != nil { @@ -132,9 +135,8 @@ func (k Keeper) FindMaxProfitForRoute(ctx sdk.Context, route RouteMetaData, rema return sdk.Coin{}, sdk.ZeroInt(), err } - // Extend the search range if the max input amount is too small - curLeft, curRight = k.UpdateSearchRange(ctx, route, inputDenom, curLeft, curRight) - + // Update the search range if the max input amount is too small/large + curLeft, curRight = k.UpdateSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight) // Binary search to find the max profit for iteration := 0; curLeft.LT(curRight) && iteration < types.MaxIterations; iteration++ { curMid := (curLeft.Add(curRight)).Quo(sdk.NewInt(2)) @@ -167,8 +169,8 @@ func (k Keeper) FindMaxProfitForRoute(ctx sdk.Context, route RouteMetaData, rema return tokenIn, profit, nil } -// UpdateSearchRange updates the search range for the binary search. -func (k Keeper) UpdateSearchRange( +// UpdateSearchRangeIfNeeded updates the search range for the binary search. +func (k Keeper) UpdateSearchRangeIfNeeded( ctx sdk.Context, route RouteMetaData, inputDenom string, @@ -194,16 +196,14 @@ func (k Keeper) UpdateSearchRange( } // If there are any concentrated liquidity pools in the route, then we might want - // to reduce the search range since it is gas intensive to move across several ticks - // - // TODO: Refactor a bit so that its clear that its only the use case for CL pools - updatedMax := k.ReduceSearchRangeIfNeeded(ctx, clPools, inputDenom, inputMap, curLeft, curRight) - if updatedMax.LT(curRight) { - return curLeft, updatedMax + // to reduce the search range since it is gas intensive to move across several ticks. + updatedMax, err := k.ReduceSearchRangeIfNeeded(ctx, clPools, inputDenom, inputMap, curLeft, curRight) + if err != nil { + return sdk.OneInt(), sdk.OneInt() } - if updatedMax.GT(types.MaxInputAmount) { - updatedMax = types.MaxInputAmount + if updatedMax.LT(curRight) { + return curLeft, updatedMax } return k.ExtendSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight, updatedMax) @@ -218,31 +218,17 @@ func (k Keeper) ReduceSearchRangeIfNeeded( inputDenom string, inputMap map[uint64]string, curLeft, curRight sdk.Int, -) sdk.Int { - // Iterate through all CL pools and determine the maximal amount of input that can be used - // respecting the max ticks moved - baseDenom, err := k.GetAllBaseDenoms(ctx) +) (sdk.Int, error) { + // Find the max amount in that can be used for the binary search + maxInputAmount, stepSize, err := k.MaxInputAmount(ctx) if err != nil { - return sdk.ZeroInt() - } - - uosmoStepSize := sdk.ZeroInt() - foundUosmo := false - for _, denom := range baseDenom { - if denom.Denom == types.OsmosisDenomination { - uosmoStepSize = denom.StepSize - foundUosmo = true - break - } - } - - if !foundUosmo { - return sdk.ZeroInt() + return sdk.ZeroInt(), err } - maxAmountIn := types.MaxInputAmount.Mul(uosmoStepSize) + // Iterate through all CL pools and determine the maximal amount of input that can be used + // respecting the max ticks moved. for _, poolId := range clPools { - maxInAmount, _, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( + maxInForPool, _, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( ctx, poolId, inputMap[poolId], @@ -250,24 +236,22 @@ func (k Keeper) ReduceSearchRangeIfNeeded( ) // In the case where we cannot calculate the max in amount, we short circuit the search - // and return the current left and right bounds as equal. This means no search will be executed. + // and return an upper bound of 0. This will cause the binary search to not run. if err != nil { - return sdk.ZeroInt() + return sdk.ZeroInt(), err } - // Convert the input amount for comparison - amount, err := k.ConvertProfits(ctx, maxInAmount, maxInAmount.Amount) + maxInForPoolInOsmo, err := k.ConvertProfits(ctx, maxInForPool, maxInForPool.Amount) if err != nil { - return sdk.ZeroInt() + return sdk.ZeroInt(), err } - // If the max input amount is less than the current max input amount, then we update the current max input amount - if amount.LT(maxAmountIn) { - maxAmountIn = amount + if maxInForPoolInOsmo.LT(maxInputAmount) { + maxInputAmount = maxInForPoolInOsmo } } - return maxAmountIn.Quo(uosmoStepSize) + return maxInputAmount.Quo(stepSize), nil } // Determine if the binary search range needs to be extended @@ -301,6 +285,33 @@ func (k Keeper) ExtendSearchRangeIfNeeded( return curLeft, curRight } +// MaxInputAmount returns the max input amount that can be used in any route. We use uosmo as the base +// unit of account for the max input amount. +func (k Keeper) MaxInputAmount(ctx sdk.Context) (sdk.Int, sdk.Int, error) { + // Determine the max amount in that can be used for the binary search + // in terms of uosmo. + baseDenom, err := k.GetAllBaseDenoms(ctx) + if err != nil { + return sdk.ZeroInt(), sdk.ZeroInt(), err + } + + uosmoStepSize := sdk.ZeroInt() + foundUosmo := false + for _, denom := range baseDenom { + if denom.Denom == types.OsmosisDenomination { + uosmoStepSize = denom.StepSize + foundUosmo = true + break + } + } + + if !foundUosmo { + return sdk.ZeroInt(), sdk.ZeroInt(), fmt.Errorf("uosmo not found in base denoms") + } + + return types.ExtendedMaxInputAmount.Mul(uosmoStepSize), uosmoStepSize, nil +} + // ExecuteTrade inputs a route, amount in, and rebalances the pool func (k Keeper) ExecuteTrade(ctx sdk.Context, route poolmanagertypes.SwapAmountInRoutes, inputCoin sdk.Coin, pool SwapToBackrun, remainingTxPoolPoints, remainingBlockPoolPoints uint64) error { // Get the module address which will execute the trade diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index 26c83e835c5..68a5a5b7192 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -165,6 +165,30 @@ var extendedRangeRoute = poolmanagertypes.SwapAmountInRoutes{ }, } +// Tests the binary search range for CL pools +var clPoolRoute = poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountInRoute{ + PoolId: 49, + TokenOutDenom: "uosmo", + }, + poolmanagertypes.SwapAmountInRoute{ + PoolId: 50, + TokenOutDenom: "epochTwo", + }, +} + +// Tests reducing the binary search range +var reducedRangeRoute = poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountInRoute{ + PoolId: 49, + TokenOutDenom: "uosmo", + }, + poolmanagertypes.SwapAmountInRoute{ + PoolId: 52, + TokenOutDenom: "epochTwo", + }, +} + // EstimateMultiHopSwap Panic catching test var panicRoute = poolmanagertypes.SwapAmountInRoutes{ poolmanagertypes.SwapAmountInRoute{ @@ -290,6 +314,26 @@ func (s *KeeperTestSuite) TestFindMaxProfitRoute() { }, expectPass: false, }, + { + name: "CL Route", + param: param{ + route: clPoolRoute, + expectedAmtIn: sdk.NewInt(131_072_000_000), + expectedProfit: sdk.NewInt(295_125_808), + routePoolPoints: 7, + }, + expectPass: true, + }, + { + name: "Reduced Range Route", // This will search up to 60_000_000uosmo + param: param{ + route: reducedRangeRoute, + expectedAmtIn: sdk.NewInt(60_000_000), + expectedProfit: sdk.NewInt(31_474), + routePoolPoints: 7, + }, + expectPass: true, + }, } for _, test := range tests { diff --git a/x/protorev/keeper/routes_test.go b/x/protorev/keeper/routes_test.go index 296b52d3a2f..70ac96276a0 100644 --- a/x/protorev/keeper/routes_test.go +++ b/x/protorev/keeper/routes_test.go @@ -349,13 +349,13 @@ func (s *KeeperTestSuite) TestCalculateRoutePoolPoints() { }, { description: "Valid route with a cosmwasm pool", - route: []poolmanagertypes.SwapAmountInRoute{{PoolId: 1, TokenOutDenom: ""}, {PoolId: 50, TokenOutDenom: ""}, {PoolId: 2, TokenOutDenom: ""}}, + route: []poolmanagertypes.SwapAmountInRoute{{PoolId: 1, TokenOutDenom: ""}, {PoolId: 51, TokenOutDenom: ""}, {PoolId: 2, TokenOutDenom: ""}}, expectedRoutePoolPoints: 8, expectedPass: true, }, { description: "Valid route with cw pool, balancer, stable swap and cl pool", - route: []poolmanagertypes.SwapAmountInRoute{{PoolId: 1, TokenOutDenom: ""}, {PoolId: 50, TokenOutDenom: ""}, {PoolId: 40, TokenOutDenom: ""}, {PoolId: 49, TokenOutDenom: ""}}, + route: []poolmanagertypes.SwapAmountInRoute{{PoolId: 1, TokenOutDenom: ""}, {PoolId: 51, TokenOutDenom: ""}, {PoolId: 40, TokenOutDenom: ""}, {PoolId: 50, TokenOutDenom: ""}}, expectedRoutePoolPoints: 10, expectedPass: true, }, From c9b13cd57eac44ed32af08f34d2e5052a4e75abe Mon Sep 17 00:00:00 2001 From: David Terpay Date: Mon, 31 Jul 2023 14:20:59 -0400 Subject: [PATCH 03/18] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bce979b7c5..a827b0e319c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#5831](https://github.com/osmosis-labs/osmosis/pull/5831) Fix superfluid_delegations query * [#5835](https://github.com/osmosis-labs/osmosis/pull/5835) Fix println's for "amountZeroInRemainingBigDec before fee" making it into production * [#5841] (https://github.com/osmosis-labs/osmosis/pull/5841) Fix protorev's out of gas erroring of the user's transcation. +* [#5930] (https://github.com/osmosis-labs/osmosis/pull/5930) Updating Binary Search Range with CL Pools ### Misc Improvements From a706848d8005f71a808adbf63d1708bc7a5b3a2c Mon Sep 17 00:00:00 2001 From: David Terpay Date: Mon, 31 Jul 2023 14:42:35 -0400 Subject: [PATCH 04/18] comments --- x/protorev/keeper/rebalance.go | 46 ++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index b0b68248109..da744a95afc 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -136,7 +136,11 @@ func (k Keeper) FindMaxProfitForRoute(ctx sdk.Context, route RouteMetaData, rema } // Update the search range if the max input amount is too small/large - curLeft, curRight = k.UpdateSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight) + curLeft, curRight, err = k.UpdateSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight) + if err != nil { + return sdk.Coin{}, sdk.ZeroInt(), err + } + // Binary search to find the max profit for iteration := 0; curLeft.LT(curRight) && iteration < types.MaxIterations; iteration++ { curMid := (curLeft.Add(curRight)).Quo(sdk.NewInt(2)) @@ -169,13 +173,16 @@ func (k Keeper) FindMaxProfitForRoute(ctx sdk.Context, route RouteMetaData, rema return tokenIn, profit, nil } -// UpdateSearchRangeIfNeeded updates the search range for the binary search. +// UpdateSearchRangeIfNeeded updates the search range for the binary search. First, we check if there are any +// concentrated liquidity pools in the route. If there are, then we may need to reduce the upper bound of the +// binary search since it is gas intensive to move across several ticks. Next, we determine if the current bound +// includes the optimal amount in. If it does not, then we can extend the search range to capture more profits. func (k Keeper) UpdateSearchRangeIfNeeded( ctx sdk.Context, route RouteMetaData, inputDenom string, curLeft, curRight sdk.Int, -) (sdk.Int, sdk.Int) { +) (sdk.Int, sdk.Int, error) { // Retrieve all concentrated liquidity pools in the route clPools := make([]uint64, 0) inputMap := make(map[uint64]string) @@ -184,7 +191,7 @@ func (k Keeper) UpdateSearchRangeIfNeeded( for _, route := range route.Route { pool, err := k.poolmanagerKeeper.GetPool(ctx, route.PoolId) if err != nil { - return sdk.OneInt(), sdk.OneInt() + return sdk.ZeroInt(), sdk.ZeroInt(), err } if pool.GetType() == poolmanagertypes.Concentrated { @@ -195,23 +202,21 @@ func (k Keeper) UpdateSearchRangeIfNeeded( prevInput = route.TokenOutDenom } - // If there are any concentrated liquidity pools in the route, then we might want - // to reduce the search range since it is gas intensive to move across several ticks. + // If there are concentrated liquidity pools in the route, then we may need to reduce the upper bound of the binary search. updatedMax, err := k.ReduceSearchRangeIfNeeded(ctx, clPools, inputDenom, inputMap, curLeft, curRight) if err != nil { - return sdk.OneInt(), sdk.OneInt() + return sdk.ZeroInt(), sdk.ZeroInt(), err } if updatedMax.LT(curRight) { - return curLeft, updatedMax + return curLeft, updatedMax, nil } return k.ExtendSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight, updatedMax) } -// ReduceSearchRangeIfNeeded reduces the search range if the max input amount is too large. This is -// particularly useful for concentrated liquidity pools where the max input amount is very large and -// the binary search is too gas intensive. +// ReduceSearchRangeIfNeeded returns the max amount in that can be used for the binary search +// respecting the max ticks moved across all concentrated liquidity pools in the route. func (k Keeper) ReduceSearchRangeIfNeeded( ctx sdk.Context, clPools []uint64, @@ -219,7 +224,7 @@ func (k Keeper) ReduceSearchRangeIfNeeded( inputMap map[uint64]string, curLeft, curRight sdk.Int, ) (sdk.Int, error) { - // Find the max amount in that can be used for the binary search + // Find the max amount in that can be used for the binary search (in terms of uosmo) maxInputAmount, stepSize, err := k.MaxInputAmount(ctx) if err != nil { return sdk.ZeroInt(), err @@ -260,19 +265,24 @@ func (k Keeper) ExtendSearchRangeIfNeeded( route RouteMetaData, inputDenom string, curLeft, curRight, updatedMax sdk.Int, -) (sdk.Int, sdk.Int) { +) (sdk.Int, sdk.Int, error) { // Get the profit for the maximum amount in _, maxInProfit, err := k.EstimateMultihopProfit(ctx, inputDenom, curRight.Mul(route.StepSize), route.Route) if err != nil { - return curLeft, curRight + return sdk.ZeroInt(), sdk.ZeroInt(), err } // If the profit for the maximum amount in is still increasing, then we can increase the range of the binary search if maxInProfit.GTE(sdk.ZeroInt()) { + maxInPlusOne := curRight.Add(sdk.OneInt()).Mul(route.StepSize) + if updatedMax.LT(maxInPlusOne) { + return curLeft, curRight, nil + } + // Get the profit for the maximum amount in + 1 - _, maxInProfitPlusOne, err := k.EstimateMultihopProfit(ctx, inputDenom, curRight.Add(sdk.OneInt()).Mul(route.StepSize), route.Route) + _, maxInProfitPlusOne, err := k.EstimateMultihopProfit(ctx, inputDenom, maxInPlusOne, route.Route) if err != nil { - return curLeft, curRight + return sdk.ZeroInt(), sdk.ZeroInt(), err } // Change the range of the binary search if the profit is still increasing @@ -282,14 +292,12 @@ func (k Keeper) ExtendSearchRangeIfNeeded( } } - return curLeft, curRight + return curLeft, curRight, nil } // MaxInputAmount returns the max input amount that can be used in any route. We use uosmo as the base // unit of account for the max input amount. func (k Keeper) MaxInputAmount(ctx sdk.Context) (sdk.Int, sdk.Int, error) { - // Determine the max amount in that can be used for the binary search - // in terms of uosmo. baseDenom, err := k.GetAllBaseDenoms(ctx) if err != nil { return sdk.ZeroInt(), sdk.ZeroInt(), err From efa68d2dc36211e1020e2c796e58fe4c788bcab5 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Mon, 31 Jul 2023 14:55:56 -0400 Subject: [PATCH 05/18] nit --- CHANGELOG.md | 2 +- x/protorev/keeper/rebalance.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a827b0e319c..90d74767ad5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#5831](https://github.com/osmosis-labs/osmosis/pull/5831) Fix superfluid_delegations query * [#5835](https://github.com/osmosis-labs/osmosis/pull/5835) Fix println's for "amountZeroInRemainingBigDec before fee" making it into production * [#5841] (https://github.com/osmosis-labs/osmosis/pull/5841) Fix protorev's out of gas erroring of the user's transcation. -* [#5930] (https://github.com/osmosis-labs/osmosis/pull/5930) Updating Binary Search Range with CL Pools +* [#5930] (https://github.com/osmosis-labs/osmosis/pull/5930) Updating Protorev Binary Search Range Logic with CL Pools ### Misc Improvements diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index da744a95afc..7e23a3ad296 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -275,7 +275,7 @@ func (k Keeper) ExtendSearchRangeIfNeeded( // If the profit for the maximum amount in is still increasing, then we can increase the range of the binary search if maxInProfit.GTE(sdk.ZeroInt()) { maxInPlusOne := curRight.Add(sdk.OneInt()).Mul(route.StepSize) - if updatedMax.LT(maxInPlusOne) { + if updatedMax.Mul(route.StepSize).LT(maxInPlusOne) { return curLeft, curRight, nil } From d25d53eff400f5cff0a9abeae9eaf5ed126ef0c0 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 14:06:10 -0400 Subject: [PATCH 06/18] refactor --- x/protorev/keeper/keeper_test.go | 25 ++-- x/protorev/keeper/posthandler_test.go | 6 +- x/protorev/keeper/rebalance.go | 193 +++++++++++++++----------- x/protorev/keeper/rebalance_test.go | 38 +++-- 4 files changed, 156 insertions(+), 106 deletions(-) diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 780ee662839..2cc6c913b9c 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -916,21 +916,18 @@ func (s *KeeperTestSuite) setUpPools() { // Create a concentrated liquidity pool for range testing // Pool 52 // Create the CL pool - clPool := s.PrepareCustomConcentratedPool(s.TestAccs[2], "epochTwo", "uosmo", 100, sdk.NewDecWithPrec(2, 3)) - fundCoins := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(1000000000000000000)), sdk.NewCoin("epochTwo", sdk.NewInt(1000000000000000000))) - s.FundAcc(s.TestAccs[2], fundCoins) - - // Create 100 ticks in the CL pool, 50 on each side - tokensProvided := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000000)), sdk.NewCoin("epochTwo", sdk.NewInt(10000000))) - amount0Min := sdk.NewInt(0) - amount1Min := sdk.NewInt(0) - lowerTick := int64(0) - upperTick := int64(100) + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) + fundCoins := sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(10_000_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000))) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool, fundCoins) - for i := int64(0); i < 50; i++ { - s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, clPool.GetId(), s.TestAccs[2], tokensProvided, amount0Min, amount1Min, lowerTick-(100*i), upperTick-(100*i)) - s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, clPool.GetId(), s.TestAccs[2], tokensProvided, amount0Min, amount1Min, lowerTick+(100*i), upperTick+(100*i)) - } + // Create a concentrated liquidity pool for range testing + // Pool 52 + // Create the CL pool + clPool = s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) + fundCoins = sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(2_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(1_000_000_000))) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool, fundCoins) // Set all of the pool info into the stores err := s.App.ProtoRevKeeper.UpdatePools(s.Ctx) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 1ac83b5153a..bf2beb0e4fd 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -340,12 +340,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { params: param{ trades: []types.Trade{ { - Pool: 53, + Pool: 54, TokenOut: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", TokenIn: "uosmo", }, }, - expectedNumOfTrades: sdk.NewInt(5), + expectedNumOfTrades: sdk.NewInt(6), expectedProfits: []sdk.Coin{ { Denom: "Atom", @@ -357,7 +357,7 @@ func (s *KeeperTestSuite) TestAnteHandle() { }, { Denom: types.OsmosisDenomination, - Amount: sdk.NewInt(56_609_900), + Amount: sdk.NewInt(56_653_504), }, }, expectedPoolPoints: 37, diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 7e23a3ad296..0aa3d8423d8 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -1,8 +1,6 @@ package keeper import ( - "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v17/x/poolmanager/types" @@ -26,7 +24,7 @@ func (k Keeper) IterateRoutes(ctx sdk.Context, routes []RouteMetaData, remaining // Find the max profit for the route if it exists inputCoin, profit, err := k.FindMaxProfitForRoute(ctx, routes[index], remainingTxPoolPoints, remainingBlockPoolPoints) if err != nil { - k.Logger(ctx).Error("Error finding max profit for route: ", err) + k.Logger(ctx).Error("Error finding max profit for route: " + err.Error()) continue } @@ -34,7 +32,7 @@ func (k Keeper) IterateRoutes(ctx sdk.Context, routes []RouteMetaData, remaining if profit.GT(sdk.ZeroInt()) { profit, err := k.ConvertProfits(ctx, inputCoin, profit) if err != nil { - k.Logger(ctx).Error("Error converting profits: ", err) + k.Logger(ctx).Error("Error converting profits: " + err.Error()) continue } @@ -183,31 +181,14 @@ func (k Keeper) UpdateSearchRangeIfNeeded( inputDenom string, curLeft, curRight sdk.Int, ) (sdk.Int, sdk.Int, error) { - // Retrieve all concentrated liquidity pools in the route - clPools := make([]uint64, 0) - inputMap := make(map[uint64]string) - prevInput := inputDenom - - for _, route := range route.Route { - pool, err := k.poolmanagerKeeper.GetPool(ctx, route.PoolId) - if err != nil { - return sdk.ZeroInt(), sdk.ZeroInt(), err - } - - if pool.GetType() == poolmanagertypes.Concentrated { - clPools = append(clPools, route.PoolId) - inputMap[route.PoolId] = prevInput - } - - prevInput = route.TokenOutDenom - } - // If there are concentrated liquidity pools in the route, then we may need to reduce the upper bound of the binary search. - updatedMax, err := k.ReduceSearchRangeIfNeeded(ctx, clPools, inputDenom, inputMap, curLeft, curRight) + updatedMax, err := k.CalculateUpperBoundForSearch(ctx, route, inputDenom) if err != nil { return sdk.ZeroInt(), sdk.ZeroInt(), err } + // In the case where the updated upper bound is less than the current upper bound, we know we will not extend + // the search range so we can short-circuit return. if updatedMax.LT(curRight) { return curLeft, updatedMax, nil } @@ -215,50 +196,129 @@ func (k Keeper) UpdateSearchRangeIfNeeded( return k.ExtendSearchRangeIfNeeded(ctx, route, inputDenom, curLeft, curRight, updatedMax) } -// ReduceSearchRangeIfNeeded returns the max amount in that can be used for the binary search +// CalculateUpperBoundForSearch returns the max amount in that can be used for the binary search // respecting the max ticks moved across all concentrated liquidity pools in the route. -func (k Keeper) ReduceSearchRangeIfNeeded( +func (k Keeper) CalculateUpperBoundForSearch( ctx sdk.Context, - clPools []uint64, + route RouteMetaData, inputDenom string, - inputMap map[uint64]string, - curLeft, curRight sdk.Int, ) (sdk.Int, error) { - // Find the max amount in that can be used for the binary search (in terms of uosmo) - maxInputAmount, stepSize, err := k.MaxInputAmount(ctx) - if err != nil { - return sdk.ZeroInt(), err - } + var intermidiateCoin sdk.Coin // Iterate through all CL pools and determine the maximal amount of input that can be used // respecting the max ticks moved. - for _, poolId := range clPools { - maxInForPool, _, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( - ctx, - poolId, - inputMap[poolId], - types.MaxTicksMoved, - ) - - // In the case where we cannot calculate the max in amount, we short circuit the search - // and return an upper bound of 0. This will cause the binary search to not run. + for index := route.Route.Length() - 1; index >= 0; index-- { + hop := route.Route[index] + pool, err := k.poolmanagerKeeper.GetPool(ctx, hop.PoolId) if err != nil { return sdk.ZeroInt(), err } - maxInForPoolInOsmo, err := k.ConvertProfits(ctx, maxInForPool, maxInForPool.Amount) - if err != nil { - return sdk.ZeroInt(), err + tokenOut := inputDenom + if index > 0 { + tokenOut = route.Route[index-1].TokenOutDenom } - if maxInForPoolInOsmo.LT(maxInputAmount) { - maxInputAmount = maxInForPoolInOsmo + switch { + case pool.GetType() == poolmanagertypes.Concentrated: + // If the pool is a concentrated liquidity pool, then check the maximum amount in that can be used + // and determine what this amount is as an input at the previous pool (working all the way up to the + // beginning of the route). + _, maxTokenOut, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( + ctx, + pool.GetId(), + hop.TokenOutDenom, + types.MaxTicksMoved, + ) + if err != nil { + return sdk.ZeroInt(), err + } + + // if there have been no other CL pools in the route, then we can set the intermediate coin to the max input amount + if intermidiateCoin.IsNil() { + intermidiateCoin = maxTokenOut + continue + } + + // In the scenario where there are multiple CL pools in a route, we select the one that inputs + // the smaller amount to ensure we do not overstep the max ticks moved. + liquidity, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, pool.GetId()) + if err != nil { + return sdk.ZeroInt(), err + } + + liquidTokenAmt := liquidity.AmountOf(intermidiateCoin.Denom) + if liquidTokenAmt.LT(intermidiateCoin.Amount) { + intermidiateCoin.Amount = liquidTokenAmt.Sub(sdk.OneInt().Mul(route.StepSize)) + } + + amtOut, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( + ctx, + poolmanagertypes.SwapAmountInRoutes{ + { + PoolId: pool.GetId(), + TokenOutDenom: tokenOut, + }, + }, + sdk.NewCoin(intermidiateCoin.Denom, intermidiateCoin.Amount), + ) + if err != nil { + return sdk.ZeroInt(), err + } + + if amtOut.GT(maxTokenOut.Amount) { + intermidiateCoin = maxTokenOut + } else { + intermidiateCoin = sdk.NewCoin(tokenOut, amtOut) + } + case !intermidiateCoin.IsNil(): + // In the case where the liquidity in the pool is less than the max amount out, we want to + // use the liquidity - 1 to ensure we do not attempt to move more liquidity than is available. + liquidity, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, pool.GetId()) + if err != nil { + return sdk.ZeroInt(), err + } + + liquidTokenAmt := liquidity.AmountOf(intermidiateCoin.Denom) + if liquidTokenAmt.LT(intermidiateCoin.Amount) { + intermidiateCoin.Amount = liquidTokenAmt.Sub(sdk.OneInt().Mul(route.StepSize)) + } + + amtOut, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( + ctx, + poolmanagertypes.SwapAmountInRoutes{ + { + PoolId: pool.GetId(), + TokenOutDenom: tokenOut, + }, + }, + sdk.NewCoin(intermidiateCoin.Denom, intermidiateCoin.Amount), + ) + if err != nil { + return sdk.ZeroInt(), err + } + + intermidiateCoin = sdk.NewCoin(tokenOut, amtOut) } } - return maxInputAmount.Quo(stepSize), nil + // In the case where there are no CL pools, we want to return the extended max input amount + if intermidiateCoin.IsNil() { + return types.ExtendedMaxInputAmount, nil + } + + // Ensure that the normalized upper bound is not greater than the extended max input amount + upperBound := intermidiateCoin.Amount.Quo(route.StepSize) + if upperBound.GT(types.ExtendedMaxInputAmount) { + return types.ExtendedMaxInputAmount, nil + } + + return upperBound, nil } +// ExecuteSafeSwap executes a safe swap by first ensuring the swap amount is less than the +// amount of total liquidity in the pool. + // Determine if the binary search range needs to be extended func (k Keeper) ExtendSearchRangeIfNeeded( ctx sdk.Context, @@ -274,13 +334,9 @@ func (k Keeper) ExtendSearchRangeIfNeeded( // If the profit for the maximum amount in is still increasing, then we can increase the range of the binary search if maxInProfit.GTE(sdk.ZeroInt()) { - maxInPlusOne := curRight.Add(sdk.OneInt()).Mul(route.StepSize) - if updatedMax.Mul(route.StepSize).LT(maxInPlusOne) { - return curLeft, curRight, nil - } - // Get the profit for the maximum amount in + 1 - _, maxInProfitPlusOne, err := k.EstimateMultihopProfit(ctx, inputDenom, maxInPlusOne, route.Route) + maxInPlusOne := curRight.Add(sdk.OneInt().Mul(route.StepSize)) + _, maxInProfitPlusOne, err := k.EstimateMultihopProfit(ctx, inputDenom, maxInPlusOne.Mul(route.StepSize), route.Route) if err != nil { return sdk.ZeroInt(), sdk.ZeroInt(), err } @@ -295,31 +351,6 @@ func (k Keeper) ExtendSearchRangeIfNeeded( return curLeft, curRight, nil } -// MaxInputAmount returns the max input amount that can be used in any route. We use uosmo as the base -// unit of account for the max input amount. -func (k Keeper) MaxInputAmount(ctx sdk.Context) (sdk.Int, sdk.Int, error) { - baseDenom, err := k.GetAllBaseDenoms(ctx) - if err != nil { - return sdk.ZeroInt(), sdk.ZeroInt(), err - } - - uosmoStepSize := sdk.ZeroInt() - foundUosmo := false - for _, denom := range baseDenom { - if denom.Denom == types.OsmosisDenomination { - uosmoStepSize = denom.StepSize - foundUosmo = true - break - } - } - - if !foundUosmo { - return sdk.ZeroInt(), sdk.ZeroInt(), fmt.Errorf("uosmo not found in base denoms") - } - - return types.ExtendedMaxInputAmount.Mul(uosmoStepSize), uosmoStepSize, nil -} - // ExecuteTrade inputs a route, amount in, and rebalances the pool func (k Keeper) ExecuteTrade(ctx sdk.Context, route poolmanagertypes.SwapAmountInRoutes, inputCoin sdk.Coin, pool SwapToBackrun, remainingTxPoolPoints, remainingBlockPoolPoints uint64) error { // Get the module address which will execute the trade diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index 68a5a5b7192..ac7fd0a32ad 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -166,7 +166,7 @@ var extendedRangeRoute = poolmanagertypes.SwapAmountInRoutes{ } // Tests the binary search range for CL pools -var clPoolRoute = poolmanagertypes.SwapAmountInRoutes{ +var clPoolRouteExtended = poolmanagertypes.SwapAmountInRoutes{ poolmanagertypes.SwapAmountInRoute{ PoolId: 49, TokenOutDenom: "uosmo", @@ -177,8 +177,20 @@ var clPoolRoute = poolmanagertypes.SwapAmountInRoutes{ }, } +// Tests multiple CL pools in the same route +var clPoolRouteMulti = poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountInRoute{ + PoolId: 52, + TokenOutDenom: "uosmo", + }, + poolmanagertypes.SwapAmountInRoute{ + PoolId: 53, + TokenOutDenom: "epochTwo", + }, +} + // Tests reducing the binary search range -var reducedRangeRoute = poolmanagertypes.SwapAmountInRoutes{ +var clPoolRoute = poolmanagertypes.SwapAmountInRoutes{ poolmanagertypes.SwapAmountInRoute{ PoolId: 49, TokenOutDenom: "uosmo", @@ -315,9 +327,9 @@ func (s *KeeperTestSuite) TestFindMaxProfitRoute() { expectPass: false, }, { - name: "CL Route", + name: "CL Route (extended range)", param: param{ - route: clPoolRoute, + route: clPoolRouteExtended, expectedAmtIn: sdk.NewInt(131_072_000_000), expectedProfit: sdk.NewInt(295_125_808), routePoolPoints: 7, @@ -325,15 +337,25 @@ func (s *KeeperTestSuite) TestFindMaxProfitRoute() { expectPass: true, }, { - name: "Reduced Range Route", // This will search up to 60_000_000uosmo + name: "CL Route", // This will search up to 131072 * stepsize param: param{ - route: reducedRangeRoute, - expectedAmtIn: sdk.NewInt(60_000_000), - expectedProfit: sdk.NewInt(31_474), + route: clPoolRoute, + expectedAmtIn: sdk.NewInt(13_159_000_000), + expectedProfit: sdk.NewInt(18_055_586), routePoolPoints: 7, }, expectPass: true, }, + { + name: "CL Route Multi", + param: param{ + route: clPoolRouteMulti, // this will search up to 999 * stepsize + expectedAmtIn: sdk.NewInt(414_000_000), + expectedProfit: sdk.NewInt(171_555_698), + routePoolPoints: 12, + }, + expectPass: true, + }, } for _, test := range tests { From bde5c73405bb9003f424026a9d2dbbc12e3917c1 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 14:24:39 -0400 Subject: [PATCH 07/18] clean up --- x/protorev/keeper/rebalance.go | 87 +++++++++++++++------------------- 1 file changed, 39 insertions(+), 48 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 0aa3d8423d8..8c7b57edf35 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -242,63 +242,21 @@ func (k Keeper) CalculateUpperBoundForSearch( // In the scenario where there are multiple CL pools in a route, we select the one that inputs // the smaller amount to ensure we do not overstep the max ticks moved. - liquidity, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, pool.GetId()) + intermidiateCoin, err = k.executeSafeSwap(ctx, pool.GetId(), intermidiateCoin, tokenOut, route.StepSize) if err != nil { return sdk.ZeroInt(), err } - liquidTokenAmt := liquidity.AmountOf(intermidiateCoin.Denom) - if liquidTokenAmt.LT(intermidiateCoin.Amount) { - intermidiateCoin.Amount = liquidTokenAmt.Sub(sdk.OneInt().Mul(route.StepSize)) - } - - amtOut, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( - ctx, - poolmanagertypes.SwapAmountInRoutes{ - { - PoolId: pool.GetId(), - TokenOutDenom: tokenOut, - }, - }, - sdk.NewCoin(intermidiateCoin.Denom, intermidiateCoin.Amount), - ) - if err != nil { - return sdk.ZeroInt(), err - } - - if amtOut.GT(maxTokenOut.Amount) { + if intermidiateCoin.Amount.GT(maxTokenOut.Amount) { intermidiateCoin = maxTokenOut - } else { - intermidiateCoin = sdk.NewCoin(tokenOut, amtOut) } case !intermidiateCoin.IsNil(): - // In the case where the liquidity in the pool is less than the max amount out, we want to - // use the liquidity - 1 to ensure we do not attempt to move more liquidity than is available. - liquidity, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, pool.GetId()) + // If we have already seen a CL pool in the route, then simply propagate the intermediate coin up + // the route. + intermidiateCoin, err = k.executeSafeSwap(ctx, pool.GetId(), intermidiateCoin, tokenOut, route.StepSize) if err != nil { return sdk.ZeroInt(), err } - - liquidTokenAmt := liquidity.AmountOf(intermidiateCoin.Denom) - if liquidTokenAmt.LT(intermidiateCoin.Amount) { - intermidiateCoin.Amount = liquidTokenAmt.Sub(sdk.OneInt().Mul(route.StepSize)) - } - - amtOut, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( - ctx, - poolmanagertypes.SwapAmountInRoutes{ - { - PoolId: pool.GetId(), - TokenOutDenom: tokenOut, - }, - }, - sdk.NewCoin(intermidiateCoin.Denom, intermidiateCoin.Amount), - ) - if err != nil { - return sdk.ZeroInt(), err - } - - intermidiateCoin = sdk.NewCoin(tokenOut, amtOut) } } @@ -316,8 +274,41 @@ func (k Keeper) CalculateUpperBoundForSearch( return upperBound, nil } -// ExecuteSafeSwap executes a safe swap by first ensuring the swap amount is less than the +// executeSafeSwap executes a safe swap by first ensuring the swap amount is less than the // amount of total liquidity in the pool. +func (k Keeper) executeSafeSwap( + ctx sdk.Context, + poolID uint64, + inputCoin sdk.Coin, + tokenOutDenom string, + stepSize sdk.Int, +) (sdk.Coin, error) { + liquidity, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolID) + if err != nil { + return sdk.NewCoin(tokenOutDenom, sdk.ZeroInt()), err + } + + liquidTokenAmt := liquidity.AmountOf(inputCoin.Denom) + if liquidTokenAmt.LT(inputCoin.Amount) { + inputCoin.Amount = liquidTokenAmt.Sub(sdk.OneInt().Mul(stepSize)) + } + + amt, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( + ctx, + poolmanagertypes.SwapAmountInRoutes{ + { + PoolId: poolID, + TokenOutDenom: tokenOutDenom, + }, + }, + inputCoin, + ) + if err != nil { + return sdk.NewCoin(tokenOutDenom, amt), err + } + + return sdk.NewCoin(tokenOutDenom, amt), nil +} // Determine if the binary search range needs to be extended func (k Keeper) ExtendSearchRangeIfNeeded( From 2f314fae1fe3625506652616615c08a932459c4a Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 14:35:15 -0400 Subject: [PATCH 08/18] nit --- x/protorev/keeper/rebalance.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 8c7b57edf35..4ec7eaa9c97 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -326,8 +326,7 @@ func (k Keeper) ExtendSearchRangeIfNeeded( // If the profit for the maximum amount in is still increasing, then we can increase the range of the binary search if maxInProfit.GTE(sdk.ZeroInt()) { // Get the profit for the maximum amount in + 1 - maxInPlusOne := curRight.Add(sdk.OneInt().Mul(route.StepSize)) - _, maxInProfitPlusOne, err := k.EstimateMultihopProfit(ctx, inputDenom, maxInPlusOne.Mul(route.StepSize), route.Route) + _, maxInProfitPlusOne, err := k.EstimateMultihopProfit(ctx, inputDenom, curRight.Add(sdk.OneInt()).Mul(route.StepSize), route.Route) if err != nil { return sdk.ZeroInt(), sdk.ZeroInt(), err } From 1ede81de0ed16ac685da2ded18ae88caf358a18a Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 14:36:31 -0400 Subject: [PATCH 09/18] comment --- x/protorev/keeper/rebalance_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index ac7fd0a32ad..142d7d5cf25 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -327,7 +327,7 @@ func (s *KeeperTestSuite) TestFindMaxProfitRoute() { expectPass: false, }, { - name: "CL Route (extended range)", + name: "CL Route (extended range)", // This will search up to 131072 * stepsize param: param{ route: clPoolRouteExtended, expectedAmtIn: sdk.NewInt(131_072_000_000), @@ -347,7 +347,7 @@ func (s *KeeperTestSuite) TestFindMaxProfitRoute() { expectPass: true, }, { - name: "CL Route Multi", + name: "CL Route Reduced Range", param: param{ route: clPoolRouteMulti, // this will search up to 999 * stepsize expectedAmtIn: sdk.NewInt(414_000_000), From b0c92f30915b9552b953e1c93b4b52e02fa57fc0 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 14:47:01 -0400 Subject: [PATCH 10/18] cr --- x/protorev/keeper/keeper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 2cc6c913b9c..0cb50715022 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -922,7 +922,7 @@ func (s *KeeperTestSuite) setUpPools() { s.CreateFullRangePosition(clPool, fundCoins) // Create a concentrated liquidity pool for range testing - // Pool 52 + // Pool 53 // Create the CL pool clPool = s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) fundCoins = sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(2_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(1_000_000_000))) From 1c784853b5d94f1669a47152dc3399611d276334 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 15:08:57 -0400 Subject: [PATCH 11/18] cl are not symetric --- x/protorev/keeper/rebalance.go | 12 ++++++------ x/protorev/types/constants.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 4ec7eaa9c97..ec26af876a5 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -224,11 +224,11 @@ func (k Keeper) CalculateUpperBoundForSearch( // If the pool is a concentrated liquidity pool, then check the maximum amount in that can be used // and determine what this amount is as an input at the previous pool (working all the way up to the // beginning of the route). - _, maxTokenOut, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( + maxTokenIn, _, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( ctx, pool.GetId(), - hop.TokenOutDenom, - types.MaxTicksMoved, + tokenOut, + types.MaxTicksCrossed, ) if err != nil { return sdk.ZeroInt(), err @@ -236,7 +236,7 @@ func (k Keeper) CalculateUpperBoundForSearch( // if there have been no other CL pools in the route, then we can set the intermediate coin to the max input amount if intermidiateCoin.IsNil() { - intermidiateCoin = maxTokenOut + intermidiateCoin = maxTokenIn continue } @@ -247,8 +247,8 @@ func (k Keeper) CalculateUpperBoundForSearch( return sdk.ZeroInt(), err } - if intermidiateCoin.Amount.GT(maxTokenOut.Amount) { - intermidiateCoin = maxTokenOut + if intermidiateCoin.Amount.GT(maxTokenIn.Amount) { + intermidiateCoin = maxTokenIn } case !intermidiateCoin.IsNil(): // If we have already seen a CL pool in the route, then simply propagate the intermediate coin up diff --git a/x/protorev/types/constants.go b/x/protorev/types/constants.go index ecb2de1bfb9..ffdc56c2a7f 100644 --- a/x/protorev/types/constants.go +++ b/x/protorev/types/constants.go @@ -29,7 +29,7 @@ const MaxPoolPointsPerBlock uint64 = 200 // Max number of ticks we can move in a concentrated pool swap. This will be parameterized in a // follow up PR. -const MaxTicksMoved uint64 = 5 +const MaxTicksCrossed uint64 = 5 // ---------------- Module Profit Splitting Constants ---------------- // From 64a68915680d471716e4282e285a930ab7f003b4 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 16:37:39 -0400 Subject: [PATCH 12/18] smh more symetric changes --- x/protorev/keeper/rebalance.go | 49 ++++++++++++++-------------- x/protorev/types/expected_keepers.go | 5 +++ 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index ec26af876a5..5a8af454dbb 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -214,9 +214,9 @@ func (k Keeper) CalculateUpperBoundForSearch( return sdk.ZeroInt(), err } - tokenOut := inputDenom + tokenInDenom := inputDenom if index > 0 { - tokenOut = route.Route[index-1].TokenOutDenom + tokenInDenom = route.Route[index-1].TokenOutDenom } switch { @@ -224,36 +224,35 @@ func (k Keeper) CalculateUpperBoundForSearch( // If the pool is a concentrated liquidity pool, then check the maximum amount in that can be used // and determine what this amount is as an input at the previous pool (working all the way up to the // beginning of the route). - maxTokenIn, _, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( + maxTokenIn, maxTokenOut, err := k.concentratedLiquidityKeeper.ComputeMaxInAmtGivenMaxTicksCrossed( ctx, pool.GetId(), - tokenOut, + tokenInDenom, types.MaxTicksCrossed, ) if err != nil { return sdk.ZeroInt(), err } - // if there have been no other CL pools in the route, then we can set the intermediate coin to the max input amount - if intermidiateCoin.IsNil() { + // if there have been no other CL pools in the route, then we can set the intermediate coin to the max input amount. + // Additionally, if the amount of the previous token is greater than the possible amount from this pool, then we + // can set the intermediate coin to the max input amount (from the current pool). Otherwise we have to do a + // safe swap given the previous max amount. + if intermidiateCoin.IsNil() || maxTokenOut.Amount.LT(intermidiateCoin.Amount) { intermidiateCoin = maxTokenIn continue } // In the scenario where there are multiple CL pools in a route, we select the one that inputs // the smaller amount to ensure we do not overstep the max ticks moved. - intermidiateCoin, err = k.executeSafeSwap(ctx, pool.GetId(), intermidiateCoin, tokenOut, route.StepSize) + intermidiateCoin, err = k.executeSafeSwap(ctx, pool.GetId(), intermidiateCoin, tokenInDenom, route.StepSize) if err != nil { return sdk.ZeroInt(), err } - - if intermidiateCoin.Amount.GT(maxTokenIn.Amount) { - intermidiateCoin = maxTokenIn - } case !intermidiateCoin.IsNil(): // If we have already seen a CL pool in the route, then simply propagate the intermediate coin up // the route. - intermidiateCoin, err = k.executeSafeSwap(ctx, pool.GetId(), intermidiateCoin, tokenOut, route.StepSize) + intermidiateCoin, err = k.executeSafeSwap(ctx, pool.GetId(), intermidiateCoin, tokenInDenom, route.StepSize) if err != nil { return sdk.ZeroInt(), err } @@ -279,35 +278,35 @@ func (k Keeper) CalculateUpperBoundForSearch( func (k Keeper) executeSafeSwap( ctx sdk.Context, poolID uint64, - inputCoin sdk.Coin, - tokenOutDenom string, + outputCoin sdk.Coin, + tokenInDenom string, stepSize sdk.Int, ) (sdk.Coin, error) { liquidity, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolID) if err != nil { - return sdk.NewCoin(tokenOutDenom, sdk.ZeroInt()), err + return sdk.NewCoin(tokenInDenom, sdk.ZeroInt()), err } - liquidTokenAmt := liquidity.AmountOf(inputCoin.Denom) - if liquidTokenAmt.LT(inputCoin.Amount) { - inputCoin.Amount = liquidTokenAmt.Sub(sdk.OneInt().Mul(stepSize)) + liquidTokenAmt := liquidity.AmountOf(outputCoin.Denom) + if liquidTokenAmt.LT(outputCoin.Amount) { + outputCoin.Amount = liquidTokenAmt.Quo(sdk.NewInt(2)) } - amt, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( + amt, err := k.poolmanagerKeeper.MultihopEstimateInGivenExactAmountOut( ctx, - poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountOutRoutes{ { - PoolId: poolID, - TokenOutDenom: tokenOutDenom, + PoolId: poolID, + TokenInDenom: tokenInDenom, }, }, - inputCoin, + outputCoin, ) if err != nil { - return sdk.NewCoin(tokenOutDenom, amt), err + return sdk.NewCoin(tokenInDenom, sdk.ZeroInt()), err } - return sdk.NewCoin(tokenOutDenom, amt), nil + return sdk.NewCoin(tokenInDenom, amt), nil } // Determine if the binary search range needs to be extended diff --git a/x/protorev/types/expected_keepers.go b/x/protorev/types/expected_keepers.go index 8b12c09085f..4e80c5acdd2 100644 --- a/x/protorev/types/expected_keepers.go +++ b/x/protorev/types/expected_keepers.go @@ -44,6 +44,11 @@ type PoolManagerKeeper interface { tokenIn sdk.Coin, ) (tokenOutAmount sdk.Int, err error) + MultihopEstimateInGivenExactAmountOut( + ctx sdk.Context, + routes []poolmanagertypes.SwapAmountOutRoute, + tokenOut sdk.Coin) (tokenInAmount sdk.Int, err error) + AllPools( ctx sdk.Context, ) ([]poolmanagertypes.PoolI, error) From b8e3f1fec856b5734d4911eec0a83eb1e861f805 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 17:05:02 -0400 Subject: [PATCH 13/18] simpler safe swap --- x/protorev/keeper/rebalance.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 5a8af454dbb..5c9b611ee6a 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -287,9 +287,10 @@ func (k Keeper) executeSafeSwap( return sdk.NewCoin(tokenInDenom, sdk.ZeroInt()), err } - liquidTokenAmt := liquidity.AmountOf(outputCoin.Denom) + // At most we can swap half of the liquidity in the pool + liquidTokenAmt := liquidity.AmountOf(outputCoin.Denom).Quo(sdk.NewInt(2)) if liquidTokenAmt.LT(outputCoin.Amount) { - outputCoin.Amount = liquidTokenAmt.Quo(sdk.NewInt(2)) + outputCoin.Amount = liquidTokenAmt } amt, err := k.poolmanagerKeeper.MultihopEstimateInGivenExactAmountOut( From da72dd0dd789cd365cf4722bc0d0a11412aacb1f Mon Sep 17 00:00:00 2001 From: David Terpay Date: Tue, 1 Aug 2023 18:48:22 -0400 Subject: [PATCH 14/18] more stuff --- x/protorev/keeper/posthandler_test.go | 29 --------------------------- 1 file changed, 29 deletions(-) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index bf2beb0e4fd..1897f807669 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -335,35 +335,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { }, expectPass: true, }, - { - name: "Concentrated Liquidity - Many Ticks Run Out of Gas While Rebalacing, Ensure Doesn't Fail User Tx", - params: param{ - trades: []types.Trade{ - { - Pool: 54, - TokenOut: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", - TokenIn: "uosmo", - }, - }, - expectedNumOfTrades: sdk.NewInt(6), - expectedProfits: []sdk.Coin{ - { - Denom: "Atom", - Amount: sdk.NewInt(15_767_231), - }, - { - Denom: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", - Amount: sdk.NewInt(218_149_058), - }, - { - Denom: types.OsmosisDenomination, - Amount: sdk.NewInt(56_653_504), - }, - }, - expectedPoolPoints: 37, - }, - expectPass: true, - }, } // Ensure that the max points per tx is enough for the test suite From 7ef7106ef3b5f944cafd96e8ed30b00034cc4fd9 Mon Sep 17 00:00:00 2001 From: David Terpay Date: Wed, 2 Aug 2023 16:41:54 -0400 Subject: [PATCH 15/18] more testing --- x/protorev/keeper/rebalance_test.go | 109 +++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index 142d7d5cf25..56e342f8b17 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -4,7 +4,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/osmosis-labs/osmosis/v17/app/apptesting" + "github.com/osmosis-labs/osmosis/v17/x/gamm/pool-models/stableswap" poolmanagertypes "github.com/osmosis-labs/osmosis/v17/x/poolmanager/types" + "github.com/osmosis-labs/osmosis/v17/x/protorev/keeper" protorevtypes "github.com/osmosis-labs/osmosis/v17/x/protorev/keeper" "github.com/osmosis-labs/osmosis/v17/x/protorev/types" ) @@ -347,9 +349,9 @@ func (s *KeeperTestSuite) TestFindMaxProfitRoute() { expectPass: true, }, { - name: "CL Route Reduced Range", + name: "CL Route Multi", // This will search up to 131072 * stepsize param: param{ - route: clPoolRouteMulti, // this will search up to 999 * stepsize + route: clPoolRouteMulti, expectedAmtIn: sdk.NewInt(414_000_000), expectedProfit: sdk.NewInt(171_555_698), routePoolPoints: 12, @@ -733,3 +735,106 @@ func (s *KeeperTestSuite) TestRemainingPoolPointsForTx() { }) } } + +func (s *KeeperTestSuite) TestUpdateSearchRangeIfNeeded() { + s.Run("Extended search on stable pools", func() { + route := keeper.RouteMetaData{ + Route: extendedRangeRoute, + StepSize: sdk.NewInt(1_000_000), + } + + curLeft, curRight, err := s.App.ProtoRevKeeper.UpdateSearchRangeIfNeeded( + s.Ctx, + route, + "usdx", + sdk.OneInt(), + types.MaxInputAmount, + ) + s.Require().NoError(err) + s.Require().Equal(types.MaxInputAmount, curLeft) + s.Require().Equal(types.ExtendedMaxInputAmount, curRight) + }) + + s.Run("Extended search on CL pools", func() { + // Create two massive CL pools with a massive arb + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], "atom", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) + fundCoins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10_000_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000))) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool, fundCoins) + + clPool2 := s.PrepareCustomConcentratedPool(s.TestAccs[0], "atom", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) + fundCoins = sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(20_000_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000))) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool2, fundCoins) + + route := keeper.RouteMetaData{ + Route: poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountInRoute{ + PoolId: clPool.GetId(), + TokenOutDenom: "uosmo", + }, + poolmanagertypes.SwapAmountInRoute{ + PoolId: clPool2.GetId(), + TokenOutDenom: "atom", + }, + }, + StepSize: sdk.NewInt(1_000_000), + } + + curLeft, curRight, err := s.App.ProtoRevKeeper.UpdateSearchRangeIfNeeded( + s.Ctx, + route, + "atom", + sdk.OneInt(), + types.MaxInputAmount, + ) + s.Require().NoError(err) + s.Require().Equal(types.MaxInputAmount, curLeft) + s.Require().Equal(types.ExtendedMaxInputAmount, curRight) + }) + + s.Run("Reduced search on CL pools", func() { + stablePool := s.createStableswapPool( + sdk.NewCoins( + sdk.NewCoin("uosmo", sdk.NewInt(25_000_000_000)), + sdk.NewCoin("eth", sdk.NewInt(20_000_000_000)), + ), + stableswap.PoolParams{ + SwapFee: sdk.NewDecWithPrec(0, 2), + ExitFee: sdk.NewDecWithPrec(0, 2), + }, + []uint64{1, 1}, + ) + + // Create two massive CL pools with a massive arb + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], "eth", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) + fundCoins := sdk.NewCoins(sdk.NewCoin("eth", sdk.NewInt(10_000_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000))) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool, fundCoins) + + route := keeper.RouteMetaData{ + Route: poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountInRoute{ + PoolId: stablePool, + TokenOutDenom: "eth", + }, + poolmanagertypes.SwapAmountInRoute{ + PoolId: clPool.GetId(), + TokenOutDenom: "uosmo", + }, + }, + StepSize: sdk.NewInt(1_000_000), + } + + curLeft, curRight, err := s.App.ProtoRevKeeper.UpdateSearchRangeIfNeeded( + s.Ctx, + route, + "uosmo", + sdk.OneInt(), + types.MaxInputAmount, + ) + s.Require().NoError(err) + s.Require().Equal(sdk.OneInt(), curLeft) + s.Require().Equal(sdk.NewInt(11247), curRight) + }) +} From aadbcc79b5e38cf609523bee59b938cda83d513f Mon Sep 17 00:00:00 2001 From: David Terpay Date: Wed, 2 Aug 2023 18:24:51 -0400 Subject: [PATCH 16/18] adding comments --- x/protorev/keeper/rebalance.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 5c9b611ee6a..8c4d17247d1 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -49,6 +49,8 @@ func (k Keeper) IterateRoutes(ctx sdk.Context, routes []RouteMetaData, remaining } // ConvertProfits converts the profit denom to uosmo to allow for a fair comparison of profits +// +// NOTE: This does not check the underlying pool before swapping so this may go over the MaxTicksCrossed. func (k Keeper) ConvertProfits(ctx sdk.Context, inputCoin sdk.Coin, profit sdk.Int) (sdk.Int, error) { if inputCoin.Denom == types.OsmosisDenomination { return profit, nil From 9f1e6e427d9f71ad6a63a36f952cf058d99f3cde Mon Sep 17 00:00:00 2001 From: David Terpay Date: Thu, 3 Aug 2023 20:12:14 -0400 Subject: [PATCH 17/18] updating max range wit pool liq --- x/protorev/keeper/rebalance.go | 2 +- x/protorev/keeper/rebalance_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 8c4d17247d1..f1df9655f2f 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -290,7 +290,7 @@ func (k Keeper) executeSafeSwap( } // At most we can swap half of the liquidity in the pool - liquidTokenAmt := liquidity.AmountOf(outputCoin.Denom).Quo(sdk.NewInt(2)) + liquidTokenAmt := liquidity.AmountOf(outputCoin.Denom).Quo(sdk.NewInt(4)) if liquidTokenAmt.LT(outputCoin.Amount) { outputCoin.Amount = liquidTokenAmt } diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index 56e342f8b17..c6753381326 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -835,6 +835,6 @@ func (s *KeeperTestSuite) TestUpdateSearchRangeIfNeeded() { ) s.Require().NoError(err) s.Require().Equal(sdk.OneInt(), curLeft) - s.Require().Equal(sdk.NewInt(11247), curRight) + s.Require().Equal(sdk.NewInt(5141), curRight) }) } From b890a439bfee574a870b31d89dc9186a4a6c89df Mon Sep 17 00:00:00 2001 From: David Terpay Date: Fri, 4 Aug 2023 10:17:33 -0400 Subject: [PATCH 18/18] updating readme --- x/protorev/protorev.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/protorev.md b/x/protorev/protorev.md index f63e824841c..f1b023b6c79 100644 --- a/x/protorev/protorev.md +++ b/x/protorev/protorev.md @@ -314,7 +314,7 @@ IterateRoutes iterates through a list of routes, determining the route and input ### FindMaxProfitForRoute -This will take in a route and determine the optimal amount to swap in to maximize profits, given the reserves of all of the pools that are swapped against in the route. +This will take in a route and determine the optimal amount to swap in to maximize profits, given the reserves of all of the pools that are swapped against in the route. The bounds of the binary search are dynamic and update per route (see `UpdateSearchRangeIfNeeded`) based on how computationally expensive (in terms of gas) swapping can be on that route. For instance, moving across several ticks on a concentrated pool is relatively expensive, so the bounds of the binary search with a route that includes that pool type may be smaller than a route that does not include that pool type. ### ExecuteTrade