From 052d7165d6f6d68dd113f531c7f7df3dac0f4fde Mon Sep 17 00:00:00 2001 From: stackman27 Date: Wed, 10 May 2023 18:21:49 -0700 Subject: [PATCH 01/12] progress --- x/concentrated-liquidity/swaps.go | 2 ++ x/poolmanager/router.go | 30 +++++++++++++++++++----------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index df1ac17c626..e0cb996c155 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -114,6 +114,8 @@ func (k Keeper) SwapExactAmountIn( return tokenOutAmount, nil } +// TODO: add godoc +// SwapExactAmountOut does func (k Keeper) SwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index c4987e4856f..654d8f88888 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -285,7 +285,9 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( // MultihopSwapExactAmountOut defines the output denom and output amount for the last pool. // Calculation starts by providing the tokenOutAmount of the final pool to calculate the required tokenInAmount // the calculated tokenInAmount is used as defined tokenOutAmount of the previous pool, calculating in reverse order of the swap -// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined tokenInMaxAmount defined. +// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined tokenInMaxAmount defined and no errors +// are encountered along the way + func (k Keeper) RouteExactAmountOut(ctx sdk.Context, sender sdk.AccAddress, route []types.SwapAmountOutRoute, @@ -293,25 +295,28 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, tokenOut sdk.Coin, ) (tokenInAmount sdk.Int, err error) { isMultiHopRouted, routeSwapFee, sumOfSwapFees := false, sdk.Dec{}, sdk.Dec{} + // Ensure that provided route is not empty and has valid denom format. routeStep := types.SwapAmountOutRoutes(route) if err := routeStep.Validate(); err != nil { return sdk.Int{}, err } - defer func() { - if r := recover(); r != nil { - tokenInAmount = sdk.Int{} - err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) - } - }() + // ? removing defer function since we donot use panic anywhere now + // defer func() { + // if r := recover(); r != nil { + // tokenInAmount = sdk.Int{} + // err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) + // } + // }() - // in this loop, we check if: + // In this loop (isOsmoRoutedMultihop), we check if: // - the routeStep is of length 2 // - routeStep 1 and routeStep 2 don't trade via the same pool // - routeStep 1 contains uosmo // - both routeStep 1 and routeStep 2 are incentivized pools + // // if all of the above is true, then we collect the additive and max fee between the two pools to later calculate the following: - // total_swap_fee = total_swap_fee = max(swapfee1, swapfee2) + // total_swap_fee = max(swapfee1, swapfee2) // fee_per_pool = total_swap_fee * ((pool_fee) / (swapfee1 + swapfee2)) if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) { isMultiHopRouted = true @@ -343,6 +348,7 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, // value of this method is done when we calculate insExpected – this for loop primarily serves to execute the actual // swaps on each pool. for i, routeStep := range route { + // Get underlying pool type corresponding to the pool ID at the current routeStep. swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) if err != nil { return sdk.Int{}, err @@ -362,12 +368,14 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, return sdk.Int{}, poolErr } - // check if pool is active, if not error + // check if pool has swaps enabled if !pool.IsActive(ctx) { - return sdk.Int{}, fmt.Errorf("pool %d is not active", pool.GetId()) + return sdk.Int{}, types.InactivePoolError{PoolId: pool.GetId()} } swapFee := pool.GetSwapFee(ctx) + // If we determined the routeStep is an osmo multi-hop and both route are incentivized, + // we modify the swap fee accordingly. if isMultiHopRouted { swapFee = routeSwapFee.Mul((swapFee.Quo(sumOfSwapFees))) } From be1b17afffa4aa258ceaff188e730233b7622a77 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Thu, 11 May 2023 14:30:39 -0700 Subject: [PATCH 02/12] more checks --- x/concentrated-liquidity/swaps.go | 9 ++++++--- x/poolmanager/router.go | 31 +++++++++++++++---------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index e0cb996c155..120df53d5f5 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -114,8 +114,8 @@ func (k Keeper) SwapExactAmountIn( return tokenOutAmount, nil } -// TODO: add godoc -// SwapExactAmountOut does +// SwapExactAmountOut allows users to specify the output token amount they want to receive from a swap and get the exact +// input token amount they need to provide based on the current pool prices and any applicable fees. func (k Keeper) SwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, @@ -140,6 +140,7 @@ func (k Keeper) SwapExactAmountOut( zeroForOne := tokenOut.Denom == asset1 // change priceLimit based on which direction we are swapping + // if zeroForOne == true, use MinSpotPrice else use MaxSpotPrice priceLimit := swapstrategy.GetPriceLimit(zeroForOne) tokenIn, tokenOut, _, _, _, err := k.swapInAmtGivenOut(ctx, sender, pool, tokenOut, tokenInDenom, swapFee, priceLimit) if err != nil { @@ -155,7 +156,7 @@ func (k Keeper) SwapExactAmountOut( return tokenInAmount, nil } -// SwapOutAmtGivenIn is the internal mutative method for CalcOutAmtGivenIn. Utilizing CalcOutAmtGivenIn's output, this function applies the +// swapOutAmtGivenIn is the internal mutative method for CalcOutAmtGivenIn. Utilizing CalcOutAmtGivenIn's output, this function applies the // new tick, liquidity, and sqrtPrice to the respective pool func (k Keeper) swapOutAmtGivenIn( ctx sdk.Context, @@ -184,6 +185,8 @@ func (k Keeper) swapOutAmtGivenIn( return tokenIn, tokenOut, newCurrentTick, newLiquidity, newSqrtPrice, nil } +// swapInAmtGivenOut is the internal mutative method for calcInAmtGivenOut. Utilizing calcInAmtGivenOut's output, this function applies the +// new tick, liquidity, and sqrtPrice to the respective pool. func (k *Keeper) swapInAmtGivenOut( ctx sdk.Context, sender sdk.AccAddress, diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index 654d8f88888..b33b5d0d5cb 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -282,12 +282,12 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( return tokenOutAmount, err } -// MultihopSwapExactAmountOut defines the output denom and output amount for the last pool. -// Calculation starts by providing the tokenOutAmount of the final pool to calculate the required tokenInAmount -// the calculated tokenInAmount is used as defined tokenOutAmount of the previous pool, calculating in reverse order of the swap -// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined tokenInMaxAmount defined and no errors -// are encountered along the way - +// RouteExactAmountIn processes a swap along the given route using the swap function corresponding +// to poolID's pool type. This function is responsible for computing the optimal input amount +// for a given output amount when swapping tokens, taking into account the current price of the +// tokens in the pool and any slippage. +// Transaction succeeds if final amount out is greater than tokenOutMinAmount defined +// and no errors are encountered along the way. func (k Keeper) RouteExactAmountOut(ctx sdk.Context, sender sdk.AccAddress, route []types.SwapAmountOutRoute, @@ -318,30 +318,28 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, // if all of the above is true, then we collect the additive and max fee between the two pools to later calculate the following: // total_swap_fee = max(swapfee1, swapfee2) // fee_per_pool = total_swap_fee * ((pool_fee) / (swapfee1 + swapfee2)) - if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) { - isMultiHopRouted = true - routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep) - if err != nil { - return sdk.Int{}, err - } - } + var insExpected []sdk.Int + isMultiHopRouted = k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) // Determine what the estimated input would be for each pool along the multi-hop routeStep // if we determined the routeStep is an osmo multi-hop and both route are incentivized, // we utilize a separate function that calculates the discounted swap fees - var insExpected []sdk.Int if isMultiHopRouted { + routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep) + if err != nil { + return sdk.Int{}, err + } insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSwapFee, sumOfSwapFees) } else { insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) } + if err != nil { return sdk.Int{}, err } if len(insExpected) == 0 { return sdk.Int{}, nil } - insExpected[0] = tokenInMaxAmount // Iterates through each routed pool and executes their respective swaps. Note that all of the work to get the return @@ -585,6 +583,7 @@ func (k Keeper) AllPools( return sortedPools, nil } +// IsOsmoRoutedMultihop determines if a multi-hop swap involves OSMO, as one of the intermediary tokens. func (k Keeper) isOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, inDenom, outDenom string) (isRouted bool) { if route.Length() != 2 { return false @@ -678,7 +677,7 @@ func (k Keeper) createMultihopExpectedSwapOuts( return insExpected, nil } -// createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee +// createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee. func (k Keeper) createOsmoMultihopExpectedSwapOuts( ctx sdk.Context, route []types.SwapAmountOutRoute, From ab048e4ba1f20f3368c8ca54ac698e550a686a08 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Thu, 11 May 2023 14:46:42 -0700 Subject: [PATCH 03/12] checked logic --- x/concentrated-liquidity/swaps.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 120df53d5f5..3b378e3e114 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -531,6 +531,7 @@ func (k Keeper) computeInAmtGivenOut( feeGrowthGlobal: sdk.ZeroDec(), } + // ? is this todo still valid? // TODO: This should be GT 0 but some instances have very small remainder // need to look into fixing this for swapState.amountSpecifiedRemaining.GT(smallestDec) && !swapState.sqrtPrice.Equal(sqrtPriceLimit) { From 87efed6b931673f0eaaf4231e5e737b7e4e6198c Mon Sep 17 00:00:00 2001 From: stackman27 Date: Fri, 12 May 2023 18:44:25 -0700 Subject: [PATCH 04/12] moved all fmterror to errors --- x/concentrated-liquidity/swaps.go | 6 +++--- x/concentrated-liquidity/types/errors.go | 24 ++++++++++++++++++++++++ x/poolmanager/router.go | 14 +++++++------- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 3b378e3e114..cfb3d9e2bca 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -494,7 +494,7 @@ func (k Keeper) computeInAmtGivenOut( // take provided price limit and turn this into a sqrt price limit since formulas use sqrtPrice sqrtPriceLimit, err := priceLimit.ApproxSqrt() if err != nil { - return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("issue calculating square root of price limit") + return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.SqrtRootCalculationError{SqrtPriceLimit: sqrtPriceLimit} } // set the swap strategy @@ -544,13 +544,13 @@ func (k Keeper) computeInAmtGivenOut( // if no ticks are initialized (no users have created liquidity positions) then we return an error nextTick, ok := swapStrategy.NextInitializedTick(ctx, poolId, swapState.tick) if !ok { - return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("there are no more ticks initialized to fill the swap") + return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.UninitilizedTickError{NextTick: nextTick} } // utilizing the next initialized tick, we find the corresponding nextPrice (the target price) _, sqrtPriceNextTick, err := math.TickToSqrtPrice(nextTick) if err != nil { - return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("could not convert next tick (%v) to nextSqrtPrice", nextTick) + return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.TickToSqrtPriceConversionError{SqrtPriceNextTick: sqrtPriceNextTick} } sqrtPriceTarget := swapStrategy.GetSqrtTargetPrice(sqrtPriceNextTick) diff --git a/x/concentrated-liquidity/types/errors.go b/x/concentrated-liquidity/types/errors.go index 2a450cb5e0d..c812ce348bb 100644 --- a/x/concentrated-liquidity/types/errors.go +++ b/x/concentrated-liquidity/types/errors.go @@ -815,3 +815,27 @@ type RanOutOfTicksForPoolError struct { func (e RanOutOfTicksForPoolError) Error() string { return fmt.Sprintf("ran out of ticks for pool (%d) during swap", e.PoolId) } + +type SqrtRootCalculationError struct { + SqrtPriceLimit sdk.Dec +} + +func (e SqrtRootCalculationError) Error() string { + return fmt.Sprintf("issue calculating square root of price limit %s", e.SqrtPriceLimit) +} + +type UninitilizedTickError struct { + NextTick sdk.Int +} + +func (e UninitilizedTickError) Error() string { + return fmt.Sprintf("there are no more ticks initialized to fill the swap %s", e.NextTick) +} + +type TickToSqrtPriceConversionError struct { + SqrtPriceNextTick sdk.Dec +} + +func (e TickToSqrtPriceConversionError) Error() string { + return fmt.Sprintf("could not convert next tick (%v) to nextSqrtPrice", e.SqrtPriceNextTick) +} diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index b33b5d0d5cb..9d96ef29aee 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -301,13 +301,13 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, return sdk.Int{}, err } - // ? removing defer function since we donot use panic anywhere now - // defer func() { - // if r := recover(); r != nil { - // tokenInAmount = sdk.Int{} - // err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) - // } - // }() + // ? should we have defer in RouteExactAmountIn too? + defer func() { + if r := recover(); r != nil { + tokenInAmount = sdk.Int{} + err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) + } + }() // In this loop (isOsmoRoutedMultihop), we check if: // - the routeStep is of length 2 From f81d38a2cae941673c6c99d2c97875af85befb2b Mon Sep 17 00:00:00 2001 From: stackman27 Date: Sun, 14 May 2023 14:01:54 -0700 Subject: [PATCH 05/12] added test --- x/concentrated-liquidity/swaps.go | 2 + x/poolmanager/export_test.go | 17 ++++ x/poolmanager/router.go | 2 +- x/poolmanager/router_test.go | 136 ++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 1 deletion(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index cfb3d9e2bca..740cd08d3fe 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -507,6 +507,8 @@ func (k Keeper) computeInAmtGivenOut( return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, err } + fmt.Println("Desired: ", desiredTokenOut.Denom, tokenInDenom, asset0, asset1) + // check that the specified tokenOut matches one of the assets in the specified pool if desiredTokenOut.Denom != asset0 && desiredTokenOut.Denom != asset1 { return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, types.TokenOutDenomNotInPoolError{TokenOutDenom: desiredTokenOut.Denom} diff --git a/x/poolmanager/export_test.go b/x/poolmanager/export_test.go index a88f76cfb0a..2aa856a9f79 100644 --- a/x/poolmanager/export_test.go +++ b/x/poolmanager/export_test.go @@ -45,3 +45,20 @@ func (k Keeper) ValidateCreatedPool(ctx sdk.Context, poolId uint64, pool types.P func (k Keeper) IsOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, inDenom, outDenom string) (isRouted bool) { return k.isOsmoRoutedMultihop(ctx, route, inDenom, outDenom) } + +func (k Keeper) CreateMultihopExpectedSwapOuts( + ctx sdk.Context, + route []types.SwapAmountOutRoute, + tokenOut sdk.Coin, +) ([]sdk.Int, error) { + return k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) +} + +func (k Keeper) CreateOsmoMultihopExpectedSwapOuts( + ctx sdk.Context, + route []types.SwapAmountOutRoute, + tokenOut sdk.Coin, + cumulativeRouteSwapFee, sumOfSwapFees sdk.Dec, +) ([]sdk.Int, error) { + return k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, cumulativeRouteSwapFee, sumOfSwapFees) +} diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index 9d96ef29aee..ab84e74aa0c 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -645,7 +645,6 @@ func (k Keeper) getOsmoRoutedMultihopTotalSwapFee(ctx sdk.Context, route types.M // amount for this last pool and then chains that input as the output of the previous pool in the routeStep, repeating // until the first pool is reached. It returns an array of inputs, each of which correspond to a pool ID in the // routeStep of pools for the original multihop transaction. -// TODO: test this. func (k Keeper) createMultihopExpectedSwapOuts( ctx sdk.Context, route []types.SwapAmountOutRoute, @@ -711,6 +710,7 @@ func (k Keeper) createOsmoMultihopExpectedSwapOuts( return insExpected, nil } +// GetTotalPoolLiquidity gets the total liquidity for a given poolId. func (k Keeper) GetTotalPoolLiquidity(ctx sdk.Context, poolId uint64) (sdk.Coins, error) { swapModule, err := k.GetPoolModule(ctx, poolId) if err != nil { diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 0a1d6b6586f..b484a43a4a2 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -2442,3 +2442,139 @@ func (s *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { }) } } + +func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { + tests := map[string]struct { + route []types.SwapAmountOutRoute + tokenOut sdk.Coin + balancerPoolCoins []sdk.Coins + concentratedPoolDenoms [][]string + poolCoins []sdk.Coins + cumulativeRouteSwapFee sdk.Dec + sumOfSwapFees sdk.Dec + + expectedSwapOuts []sdk.Int + expectedError bool + }{ + "happy path: one route": { + route: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: bar, + }, + }, + poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount))}, + + tokenOut: sdk.NewCoin(foo, defaultSwapAmount), + expectedSwapOuts: []sdk.Int{sdk.NewInt(1000002)}, + }, + "happy path: two route": { + route: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: foo, + }, + { + PoolId: 2, + TokenInDenom: bar, + }, + }, + + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount)), // pool 2. + }, + tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), + expectedSwapOuts: []sdk.Int{sdk.NewInt(100002), sdk.NewInt(100001)}, + }, + "happy path: one route with swap Fee": { + route: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: bar, + }, + }, + poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount))}, + cumulativeRouteSwapFee: sdk.NewDec(100), + sumOfSwapFees: sdk.NewDec(500), + + tokenOut: sdk.NewCoin(uosmo, defaultSwapAmount), + expectedSwapOuts: []sdk.Int{sdk.NewInt(1000002)}, + }, + "happy path: two route with swap Fee": { + route: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: foo, + }, + { + PoolId: 2, + TokenInDenom: bar, + }, + }, + + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. + }, + cumulativeRouteSwapFee: sdk.NewDec(100), + sumOfSwapFees: sdk.NewDec(500), + + tokenOut: sdk.NewCoin(uosmo, sdk.NewInt(100000)), + expectedSwapOuts: []sdk.Int{sdk.NewInt(100002), sdk.NewInt(100001)}, + }, + "error: Invalid Pool": { + route: []types.SwapAmountOutRoute{ + { + PoolId: 100, + TokenInDenom: foo, + }, + }, + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. + }, + tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), + expectedError: true, + }, + "error: calculating in given out": { + route: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: uosmo, + }, + }, + + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. + }, + tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), + expectedSwapOuts: []sdk.Int{}, + + expectedError: true, + }, + } + + for name, tc := range tests { + suite.Run(name, func() { + suite.SetupTest() + + suite.createBalancerPoolsFromCoins(tc.poolCoins) + + var actualSwapOuts []sdk.Int + var err error + + if !tc.sumOfSwapFees.IsNil() && !tc.cumulativeRouteSwapFee.IsNil() { + actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateOsmoMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut, tc.cumulativeRouteSwapFee, tc.sumOfSwapFees) + } else { + actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut) + } + + if tc.expectedError { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + suite.Require().Equal(tc.expectedSwapOuts, actualSwapOuts) + } + }) + } +} From 46305212641943c7c751ec7221a46980cd3c2772 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Sun, 14 May 2023 14:27:21 -0700 Subject: [PATCH 06/12] cleanup --- x/concentrated-liquidity/swaps.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 740cd08d3fe..aab434e5e22 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -533,7 +533,6 @@ func (k Keeper) computeInAmtGivenOut( feeGrowthGlobal: sdk.ZeroDec(), } - // ? is this todo still valid? // TODO: This should be GT 0 but some instances have very small remainder // need to look into fixing this for swapState.amountSpecifiedRemaining.GT(smallestDec) && !swapState.sqrtPrice.Equal(sqrtPriceLimit) { From 8ee2859adf39be1ad1f07e4c84f5814f05dd86e3 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Mon, 15 May 2023 10:40:09 -0700 Subject: [PATCH 07/12] cli tested --- x/concentrated-liquidity/client/cli/tx.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/concentrated-liquidity/client/cli/tx.go b/x/concentrated-liquidity/client/cli/tx.go index 3f750d23c33..e3032dd5d88 100644 --- a/x/concentrated-liquidity/client/cli/tx.go +++ b/x/concentrated-liquidity/client/cli/tx.go @@ -49,9 +49,9 @@ func NewCreateConcentratedPoolCmd() (*osmocli.TxCliDesc, *clmodel.MsgCreateConce func NewCreatePositionCmd() (*osmocli.TxCliDesc, *types.MsgCreatePosition) { return &osmocli.TxCliDesc{ - Use: "create-position [pool-id] [lower-tick] [upper-tick] [coins] [token-0-min-amount] [token-1-min-amount]", + Use: "create-position [pool-id] [lower-tick] [upper-tick] [tokensProvided] [token-0-min-amount] [token-1-min-amount]", Short: "create or add to existing concentrated liquidity position", - Example: "create-position 1 \"[-69082]\" 69082 1000000000uosmo,10000000uion 0 0 --from val --chain-id osmosis-1", + Example: "create-position 1 \"[-69082]\" 69082 10000uosmo,10000uion 0 0 --from val --chain-id osmosis-1", }, &types.MsgCreatePosition{} } From 8337e5bfe3d0220bd0aa5c930e42c8ec915fe734 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 16 May 2023 21:28:41 -0700 Subject: [PATCH 08/12] resolved feedbacks --- x/poolmanager/router.go | 11 ++++---- x/poolmanager/router_test.go | 51 ++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index ab84e74aa0c..c66fbac1a28 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -282,12 +282,12 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( return tokenOutAmount, err } -// RouteExactAmountIn processes a swap along the given route using the swap function corresponding -// to poolID's pool type. This function is responsible for computing the optimal input amount -// for a given output amount when swapping tokens, taking into account the current price of the +// RouteExactAmountOut processes a swap along the given route using the swap function corresponding +// to poolID's pool type. This function is responsible for computing the optimal output amount +// for a given input amount when swapping tokens, taking into account the current price of the // tokens in the pool and any slippage. -// Transaction succeeds if final amount out is greater than tokenOutMinAmount defined -// and no errors are encountered along the way. +// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined +// tokenInMaxAmount defined. func (k Keeper) RouteExactAmountOut(ctx sdk.Context, sender sdk.AccAddress, route []types.SwapAmountOutRoute, @@ -301,7 +301,6 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, return sdk.Int{}, err } - // ? should we have defer in RouteExactAmountIn too? defer func() { if r := recover(); r != nil { tokenInAmount = sdk.Int{} diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index b484a43a4a2..a20910abcf4 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -2453,8 +2453,8 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { cumulativeRouteSwapFee sdk.Dec sumOfSwapFees sdk.Dec - expectedSwapOuts []sdk.Int - expectedError bool + expectedSwapIns []sdk.Int + expectedError bool }{ "happy path: one route": { route: []types.SwapAmountOutRoute{ @@ -2463,10 +2463,12 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { TokenInDenom: bar, }, }, - poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount))}, + poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100)))}, - tokenOut: sdk.NewCoin(foo, defaultSwapAmount), - expectedSwapOuts: []sdk.Int{sdk.NewInt(1000002)}, + tokenOut: sdk.NewCoin(foo, sdk.NewInt(10)), + // expectedSwapIns = (tokenOut * (poolTokenOutBalance / poolPostSwapOutBalance)).ceil() + // foo token = 10 * (100 / 90) ~ 12 + expectedSwapIns: []sdk.Int{sdk.NewInt(12)}, }, "happy path: two route": { route: []types.SwapAmountOutRoute{ @@ -2481,11 +2483,14 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { }, poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount)), // pool 2. + sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1. + sdk.NewCoins(sdk.NewCoin(bar, sdk.NewInt(100)), sdk.NewCoin(baz, sdk.NewInt(100))), // pool 2. }, - tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), - expectedSwapOuts: []sdk.Int{sdk.NewInt(100002), sdk.NewInt(100001)}, + tokenOut: sdk.NewCoin(baz, sdk.NewInt(10)), + // expectedSwapIns = (tokenOut * (poolTokenOutBalance / poolPostSwapOutBalance)).ceil() + // foo token = 10 * (100 / 90) ~ 12 + // bar token = 12 * (100 / 88) ~ 14 + expectedSwapIns: []sdk.Int{sdk.NewInt(14), sdk.NewInt(12)}, }, "happy path: one route with swap Fee": { route: []types.SwapAmountOutRoute{ @@ -2494,12 +2499,12 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { TokenInDenom: bar, }, }, - poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount))}, + poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(uosmo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100)))}, cumulativeRouteSwapFee: sdk.NewDec(100), sumOfSwapFees: sdk.NewDec(500), - tokenOut: sdk.NewCoin(uosmo, defaultSwapAmount), - expectedSwapOuts: []sdk.Int{sdk.NewInt(1000002)}, + tokenOut: sdk.NewCoin(uosmo, sdk.NewInt(10)), + expectedSwapIns: []sdk.Int{sdk.NewInt(12)}, }, "happy path: two route with swap Fee": { route: []types.SwapAmountOutRoute{ @@ -2514,14 +2519,14 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { }, poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. + sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1. + sdk.NewCoins(sdk.NewCoin(bar, sdk.NewInt(100)), sdk.NewCoin(uosmo, sdk.NewInt(100))), // pool 2. }, cumulativeRouteSwapFee: sdk.NewDec(100), sumOfSwapFees: sdk.NewDec(500), - tokenOut: sdk.NewCoin(uosmo, sdk.NewInt(100000)), - expectedSwapOuts: []sdk.Int{sdk.NewInt(100002), sdk.NewInt(100001)}, + tokenOut: sdk.NewCoin(uosmo, sdk.NewInt(10)), + expectedSwapIns: []sdk.Int{sdk.NewInt(14), sdk.NewInt(12)}, }, "error: Invalid Pool": { route: []types.SwapAmountOutRoute{ @@ -2531,9 +2536,9 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { }, }, poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1. }, - tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), + tokenOut: sdk.NewCoin(baz, sdk.NewInt(10)), expectedError: true, }, "error: calculating in given out": { @@ -2545,10 +2550,10 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { }, poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1. }, - tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), - expectedSwapOuts: []sdk.Int{}, + tokenOut: sdk.NewCoin(baz, sdk.NewInt(10)), + expectedSwapIns: []sdk.Int{}, expectedError: true, }, @@ -2568,12 +2573,12 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { } else { actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut) } - + fmt.Println("SISHIR", actualSwapOuts) if tc.expectedError { suite.Require().Error(err) } else { suite.Require().NoError(err) - suite.Require().Equal(tc.expectedSwapOuts, actualSwapOuts) + suite.Require().Equal(tc.expectedSwapIns, actualSwapOuts) } }) } From 7a37f0b04d1635fa0a3fbce26fba47e428fb71be Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 16 May 2023 21:28:56 -0700 Subject: [PATCH 09/12] resolved feedbacks --- x/concentrated-liquidity/swaps.go | 6 ++---- x/concentrated-liquidity/types/errors.go | 12 ++---------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index aab434e5e22..d00f9517748 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -507,8 +507,6 @@ func (k Keeper) computeInAmtGivenOut( return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, err } - fmt.Println("Desired: ", desiredTokenOut.Denom, tokenInDenom, asset0, asset1) - // check that the specified tokenOut matches one of the assets in the specified pool if desiredTokenOut.Denom != asset0 && desiredTokenOut.Denom != asset1 { return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, types.TokenOutDenomNotInPoolError{TokenOutDenom: desiredTokenOut.Denom} @@ -545,13 +543,13 @@ func (k Keeper) computeInAmtGivenOut( // if no ticks are initialized (no users have created liquidity positions) then we return an error nextTick, ok := swapStrategy.NextInitializedTick(ctx, poolId, swapState.tick) if !ok { - return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.UninitilizedTickError{NextTick: nextTick} + return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.InvalidTickError{} } // utilizing the next initialized tick, we find the corresponding nextPrice (the target price) _, sqrtPriceNextTick, err := math.TickToSqrtPrice(nextTick) if err != nil { - return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.TickToSqrtPriceConversionError{SqrtPriceNextTick: sqrtPriceNextTick} + return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.TickToSqrtPriceConversionError{NextTick: nextTick} } sqrtPriceTarget := swapStrategy.GetSqrtTargetPrice(sqrtPriceNextTick) diff --git a/x/concentrated-liquidity/types/errors.go b/x/concentrated-liquidity/types/errors.go index c812ce348bb..5ffc41bf20a 100644 --- a/x/concentrated-liquidity/types/errors.go +++ b/x/concentrated-liquidity/types/errors.go @@ -824,18 +824,10 @@ func (e SqrtRootCalculationError) Error() string { return fmt.Sprintf("issue calculating square root of price limit %s", e.SqrtPriceLimit) } -type UninitilizedTickError struct { - NextTick sdk.Int -} - -func (e UninitilizedTickError) Error() string { - return fmt.Sprintf("there are no more ticks initialized to fill the swap %s", e.NextTick) -} - type TickToSqrtPriceConversionError struct { - SqrtPriceNextTick sdk.Dec + NextTick sdk.Int } func (e TickToSqrtPriceConversionError) Error() string { - return fmt.Sprintf("could not convert next tick (%v) to nextSqrtPrice", e.SqrtPriceNextTick) + return fmt.Sprintf("could not convert next tick to nextSqrtPrice (%v)", e.NextTick) } From c7e9d9367aefd45b0b2d310f13960886c3825a0a Mon Sep 17 00:00:00 2001 From: stackman27 Date: Wed, 17 May 2023 13:01:07 -0700 Subject: [PATCH 10/12] fixed ci --- x/concentrated-liquidity/swaps.go | 6 +++--- x/concentrated-liquidity/types/errors.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index d00f9517748..cdfde5edae5 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -494,7 +494,7 @@ func (k Keeper) computeInAmtGivenOut( // take provided price limit and turn this into a sqrt price limit since formulas use sqrtPrice sqrtPriceLimit, err := priceLimit.ApproxSqrt() if err != nil { - return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.SqrtRootCalculationError{SqrtPriceLimit: sqrtPriceLimit} + return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, types.SqrtRootCalculationError{SqrtPriceLimit: sqrtPriceLimit} } // set the swap strategy @@ -543,13 +543,13 @@ func (k Keeper) computeInAmtGivenOut( // if no ticks are initialized (no users have created liquidity positions) then we return an error nextTick, ok := swapStrategy.NextInitializedTick(ctx, poolId, swapState.tick) if !ok { - return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.InvalidTickError{} + return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, types.InvalidTickError{} } // utilizing the next initialized tick, we find the corresponding nextPrice (the target price) _, sqrtPriceNextTick, err := math.TickToSqrtPrice(nextTick) if err != nil { - return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.TickToSqrtPriceConversionError{NextTick: nextTick} + return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, types.TickToSqrtPriceConversionError{NextTick: nextTick} } sqrtPriceTarget := swapStrategy.GetSqrtTargetPrice(sqrtPriceNextTick) diff --git a/x/concentrated-liquidity/types/errors.go b/x/concentrated-liquidity/types/errors.go index 5ffc41bf20a..90e2d3dbfcf 100644 --- a/x/concentrated-liquidity/types/errors.go +++ b/x/concentrated-liquidity/types/errors.go @@ -825,7 +825,7 @@ func (e SqrtRootCalculationError) Error() string { } type TickToSqrtPriceConversionError struct { - NextTick sdk.Int + NextTick int64 } func (e TickToSqrtPriceConversionError) Error() string { From c3475a1839e5b49b4c16447b9c1aca7d99b6140d Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 19 May 2023 16:10:44 -0400 Subject: [PATCH 11/12] Update x/poolmanager/router_test.go --- x/poolmanager/router_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index a20910abcf4..7955fa6d96b 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -2573,7 +2573,6 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { } else { actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut) } - fmt.Println("SISHIR", actualSwapOuts) if tc.expectedError { suite.Require().Error(err) } else { From 32485a49496705df384a8f66ca35d792d6104937 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 19 May 2023 16:14:36 -0400 Subject: [PATCH 12/12] Update x/poolmanager/router.go --- x/poolmanager/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index c66fbac1a28..baf882033ee 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -365,7 +365,7 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, return sdk.Int{}, poolErr } - // check if pool has swaps enabled + // check if pool is active, if not error if !pool.IsActive(ctx) { return sdk.Int{}, types.InactivePoolError{PoolId: pool.GetId()} }