From 59c5a7111d4ddd6ef3ac2d1040c447404ac67a27 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 5 May 2023 11:27:17 -0700 Subject: [PATCH 1/7] add comments from audit --- x/concentrated-liquidity/math/tick.go | 2 +- x/concentrated-liquidity/swaps.go | 69 +++--- .../swapstrategy/swap_strategy.go | 2 +- x/poolmanager/router.go | 209 ++++++++++-------- 4 files changed, 151 insertions(+), 131 deletions(-) diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 49766674bac..a65ae40d611 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -46,7 +46,7 @@ func TickToSqrtPrice(tickIndex sdk.Int) (sdk.Dec, error) { return sqrtPrice, nil } -// TickToSqrtPrice returns the sqrtPrice given a tickIndex +// TickToPrice returns the price given a tickIndex // If tickIndex is zero, the function returns sdk.OneDec(). func TickToPrice(tickIndex sdk.Int) (price sdk.Dec, err error) { if tickIndex.IsZero() { diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index f370c5fb858..494c03325f2 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -58,7 +58,7 @@ type SwapState struct { // when liquidity is positive. // // If the liquidity is zero, this is a no-op. This case may occur when there is no liquidity -// between the ticks.This is possible when there are only 2 positions with no overlapping ranges. +// between the ticks. This is possible when there are only 2 positions with no overlapping ranges. // As a result, the range from the end of position one to the beginning of position // two has no liquidity and can be skipped. // TODO: test @@ -83,16 +83,17 @@ func (k Keeper) SwapExactAmountIn( return sdk.Int{}, types.DenomDuplicatedError{TokenInDenom: tokenIn.Denom, TokenOutDenom: tokenOutDenom} } + // Convert pool interface to CL pool type pool, err := convertPoolInterfaceToConcentrated(poolI) if err != nil { return sdk.Int{}, err } - // determine if we are swapping asset0 for asset1 or vice versa + // Determine if we are swapping asset0 for asset1 or vice versa asset0 := pool.GetToken0() zeroForOne := tokenIn.Denom == asset0 - // change priceLimit based on which direction we are swapping + // Change priceLimit based on which direction we are swapping priceLimit := swapstrategy.GetPriceLimit(zeroForOne) tokenIn, tokenOut, _, _, _, err := k.swapOutAmtGivenIn(ctx, sender, pool, tokenIn, tokenOutDenom, swapFee, priceLimit) if err != nil { @@ -246,6 +247,7 @@ func (k Keeper) CalcInAmtGivenOut( // what the updated tick, liquidity, and currentSqrtPrice for the pool would be after this swap. // Note this method is non-mutative, so the values returned by CalcOutAmtGivenIn do not get stored // Instead, we return writeCtx function so that the caller of this method can decide to write the cached ctx to store or not. +// Note that passing in 0 for `priceLimit` will result in the price limit being set to the max/min value based on swap direction func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, tokenInMin sdk.Coin, tokenOutDenom string, @@ -254,6 +256,8 @@ func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, poolId uint64, ) (writeCtx func(), tokenIn, tokenOut sdk.Coin, updatedTick sdk.Int, updatedLiquidity, updatedSqrtPrice sdk.Dec, err error) { ctx, writeCtx = ctx.CacheContext() + + // Get pool and asset info p, err := k.getPoolById(ctx, poolId) if err != nil { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, err @@ -262,40 +266,41 @@ func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, asset1 := p.GetToken1() tokenAmountInSpecified := tokenInMin.Amount.ToDec() - // if swapping asset0 for asset1, zeroForOne is true + // If swapping asset0 for asset1, zeroForOne is true zeroForOne := tokenInMin.Denom == asset0 - // if priceLimit not set, set to max/min value based on swap direction + // If priceLimit not set (i.e. set to zero), set to max/min value based on swap direction if zeroForOne && priceLimit.Equal(sdk.ZeroDec()) { priceLimit = types.MinSpotPrice } else if !zeroForOne && priceLimit.Equal(sdk.ZeroDec()) { priceLimit = types.MaxSpotPrice } - // take provided price limit and turn this into a sqrt price limit since formulas use sqrtPrice + // 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{}, fmt.Errorf("issue calculating square root of price limit") } - // set the swap strategy + // Set the swap strategy swapStrategy := swapstrategy.New(zeroForOne, sqrtPriceLimit, k.storeKey, swapFee, p.GetTickSpacing()) - // get current sqrt price from pool + // Get current sqrt price from pool and run sanity check that current sqrt price is + // on the correct side of the price limit given swap direction. curSqrtPrice := p.GetCurrentSqrtPrice() if err := swapStrategy.ValidateSqrtPrice(sqrtPriceLimit, curSqrtPrice); err != nil { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, err } - // check that the specified tokenIn matches one of the assets in the specified pool + // Check that the specified tokenIn matches one of the assets in the specified pool if tokenInMin.Denom != asset0 && tokenInMin.Denom != asset1 { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.TokenInDenomNotInPoolError{TokenInDenom: tokenInMin.Denom} } - // check that the specified tokenOut matches one of the assets in the specified pool + // Check that the specified tokenOut matches one of the assets in the specified pool if tokenOutDenom != asset0 && tokenOutDenom != asset1 { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.TokenOutDenomNotInPoolError{TokenOutDenom: tokenOutDenom} } - // check that token in and token out are different denominations + // Check that token in and token out are different denominations if tokenInMin.Denom == tokenOutDenom { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, types.DenomDuplicatedError{TokenInDenom: tokenInMin.Denom, TokenOutDenom: tokenOutDenom} } @@ -306,20 +311,21 @@ func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, amountSpecifiedRemaining: tokenAmountInSpecified, // tokenIn amountCalculated: sdk.ZeroDec(), // tokenOut sqrtPrice: curSqrtPrice, - tick: swapStrategy.InitializeTickValue(p.GetCurrentTick()), - liquidity: p.GetLiquidity(), - feeGrowthGlobal: sdk.ZeroDec(), + // Pad (or don't pad) current tick based on swap direction to avoid off-by-one errors + tick: swapStrategy.InitializeTickValue(p.GetCurrentTick()), + liquidity: p.GetLiquidity(), + feeGrowthGlobal: sdk.ZeroDec(), } - // iterate and update swapState until we swap all tokenIn or we reach the specific sqrtPriceLimit + // Iterate and update swapState until we swap all tokenIn or we reach the specific sqrtPriceLimit // TODO: for now, we check if amountSpecifiedRemaining is GT 0.0000001. This is because there are times when the remaining // amount may be extremely small, and that small amount cannot generate and amountIn/amountOut and we are therefore left // in an infinite loop. for swapState.amountSpecifiedRemaining.GT(sdk.SmallestDec()) && !swapState.sqrtPrice.Equal(sqrtPriceLimit) { - // log the sqrtPrice we start the iteration with + // Log the sqrtPrice we start the iteration with sqrtPriceStart := swapState.sqrtPrice - // we first check to see what the position of the nearest initialized tick is + // We first check to see what the position of the nearest initialized tick is // if zeroForOneStrategy, we look to the left of the tick the current sqrt price is at // if oneForZeroStrategy, we look to the right of the tick the current sqrt price is at // if no ticks are initialized (no users have created liquidity positions) then we return an error @@ -328,15 +334,16 @@ func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("there are no more ticks initialized to fill the swap") } - // utilizing the next initialized tick, we find the corresponding nextPrice (the target price) + // Utilizing the next initialized tick, we find the corresponding nextPrice (the target price). nextTickSqrtPrice, err := math.TickToSqrtPrice(nextTick) if err != nil { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("could not convert next tick (%v) to nextSqrtPrice", nextTick) } + // If nextSqrtPrice exceeds the price limit, we set the nextSqrtPrice to the price limit. sqrtPriceTarget := swapStrategy.GetSqrtTargetPrice(nextTickSqrtPrice) - // utilizing the bucket's liquidity and knowing the price target, we calculate the how much tokenOut we get from the tokenIn + // Utilizing the bucket's liquidity and knowing the price target, we calculate the how much tokenOut we get from the tokenIn // we also calculate the swap state's new sqrtPrice after this swap sqrtPrice, amountIn, amountOut, feeCharge := swapStrategy.ComputeSwapStepOutGivenIn( swapState.sqrtPrice, @@ -356,30 +363,30 @@ func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, ctx.Logger().Debug("amountOut", amountOut) ctx.Logger().Debug("feeCharge", feeCharge) - // update the swapState with the new sqrtPrice from the above swap + // Update the swapState with the new sqrtPrice from the above swap swapState.sqrtPrice = sqrtPrice - // we deduct the amount of tokens we input in the computeSwapStep above from the user's defined tokenIn amount + // We deduct the amount of tokens we input in the computeSwapStep above from the user's defined tokenIn amount swapState.amountSpecifiedRemaining = swapState.amountSpecifiedRemaining.Sub(amountIn.Add(feeCharge)) - // we add the amount of tokens we received (amountOut) from the computeSwapStep above to the amountCalculated accumulator + // We add the amount of tokens we received (amountOut) from the computeSwapStep above to the amountCalculated accumulator swapState.amountCalculated = swapState.amountCalculated.Add(amountOut) - // if the computeSwapStep calculated a sqrtPrice that is equal to the nextSqrtPrice, this means all liquidity in the current + // If the computeSwapStep calculated a sqrtPrice that is equal to the nextSqrtPrice, this means all liquidity in the current // tick has been consumed and we must move on to the next tick to complete the swap if nextTickSqrtPrice.Equal(sqrtPrice) { - // retrieve the liquidity held in the next closest initialized tick + // Retrieve the liquidity held in the next closest initialized tick liquidityNet, err := k.crossTick(ctx, p.GetId(), nextTick.Int64(), sdk.NewDecCoinFromDec(tokenInMin.Denom, swapState.feeGrowthGlobal)) if err != nil { return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, err } liquidityNet = swapStrategy.SetLiquidityDeltaSign(liquidityNet) - // update the swapState's liquidity with the new tick's liquidity + // Update the swapState's liquidity with the new tick's liquidity newLiquidity := math.AddLiquidity(swapState.liquidity, liquidityNet) swapState.liquidity = newLiquidity - // update the swapState's tick with the tick we retrieved liquidity from + // Update the swapState's tick with the tick we retrieved liquidity from swapState.tick = nextTick } else if !sqrtPriceStart.Equal(sqrtPrice) { - // otherwise if the sqrtPrice calculated from computeSwapStep does not equal the sqrtPrice we started with at the + // Otherwise if the sqrtPrice calculated from computeSwapStep does not equal the sqrtPrice we started with at the // beginning of this iteration, we set the swapState tick to the corresponding tick of the sqrtPrice calculated from computeSwapStep price := sqrtPrice.Mul(sqrtPrice) swapState.tick, err = math.PriceToTickRoundDown(price, p.GetTickSpacing()) @@ -393,10 +400,10 @@ func (k Keeper) calcOutAmtGivenIn(ctx sdk.Context, return writeCtx, sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, err } - // coin amounts require int values - // round amountIn up to avoid under charging - amt0 := tokenAmountInSpecified.Sub(swapState.amountSpecifiedRemaining).Ceil().TruncateInt() - // round amountOut down to avoid over refunding. + // Coin amounts require int values + // Round amountIn up to avoid under charging + amt0 := (tokenAmountInSpecified.Sub(swapState.amountSpecifiedRemaining)).Ceil().TruncateInt() + // Round amountOut down to avoid over distributing. amt1 := swapState.amountCalculated.TruncateInt() ctx.Logger().Debug("final amount in", amt0) diff --git a/x/concentrated-liquidity/swapstrategy/swap_strategy.go b/x/concentrated-liquidity/swapstrategy/swap_strategy.go index b9c06dff755..367e88319c2 100644 --- a/x/concentrated-liquidity/swapstrategy/swap_strategy.go +++ b/x/concentrated-liquidity/swapstrategy/swap_strategy.go @@ -64,7 +64,7 @@ type swapStrategy interface { // and subtract from the upper tick to reflect that this new // liquidity would be added when the price crosses the lower tick // going up, and subtracted when the price crosses the upper tick - // going up. As a result, the sign depend on the direction we are moving. + // going up. As a result, the sign depends on the direction we are moving. // See oneForZeroStrategy or zeroForOneStrategy for implementation details. SetLiquidityDeltaSign(liquidityDelta sdk.Dec) sdk.Dec // ValidateSqrtPrice validates the given square root price diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index aa103f154f6..a5c1cdb7774 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -17,13 +17,16 @@ var ( intMaxValue = sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1))) ) -// RouteExactAmountIn defines the input denom and input amount for the first pool, -// the output of the first pool is chained as the input for the next routed pool -// transaction succeeds when final amount out is greater than tokenOutMinAmount defined. +// RouteExactAmountIn processes a swap along the given route using the swap function +// corresponding to poolID's pool type. It takes in the input denom and amount for +// the initial swap against the first pool and chains the output as the input for the +// next routed pool until the last pool is reached. +// Transaction succeeds if final amount out is greater than tokenOutMinAmount defined +// and no errors are encountered along the way. func (k Keeper) RouteExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, - routes []types.SwapAmountInRoute, + route []types.SwapAmountInRoute, tokenIn sdk.Coin, tokenOutMinAmount sdk.Int, ) (tokenOutAmount sdk.Int, err error) { @@ -33,95 +36,99 @@ func (k Keeper) RouteExactAmountIn( sumOfSwapFees sdk.Dec ) - route := types.SwapAmountInRoutes(routes) - if err := route.Validate(); err != nil { + // Ensure that provided route is not empty and has valid denom format. + routeStep := types.SwapAmountInRoutes(route) + if err := routeStep.Validate(); err != nil { return sdk.Int{}, err } - // In this loop, we check if: - // - the route is of length 2 - // - route 1 and route 2 don't trade via the same pool - // - route 1 contains uosmo - // - both route 1 and route 2 are incentivized pools + // 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, route, routes[0].TokenOutDenom, tokenIn.Denom) { + if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenOutDenom, tokenIn.Denom) { isMultiHopRouted = true - routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, route) + routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep) if err != nil { return sdk.Int{}, err } } - for i, route := range routes { + // Iterate through the route and execute a series of swaps through each pool. + for i, routeStep := range route { // To prevent the multihop swap from being interrupted prematurely, we keep // the minimum expected output at a very low number until the last pool _outMinAmount := sdk.NewInt(1) - if len(routes)-1 == i { + if len(route)-1 == i { _outMinAmount = tokenOutMinAmount } - swapModule, err := k.GetPoolModule(ctx, route.PoolId) + // 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 } // Execute the expected swap on the current routed pool - pool, poolErr := swapModule.GetPool(ctx, route.PoolId) + pool, poolErr := swapModule.GetPool(ctx, routeStep.PoolId) if poolErr != nil { 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()) } swapFee := pool.GetSwapFee(ctx) - // If we determined the route is an osmo multi-hop and both routes are incentivized, + // If we determined the routeStep is an osmo multi-hop and both route are incentivized, // we modify the swap fee accordingly. + // TODO: evaluate rounding if isMultiHopRouted { swapFee = routeSwapFee.Mul((swapFee.Quo(sumOfSwapFees))) } - tokenOutAmount, err = swapModule.SwapExactAmountIn(ctx, sender, pool, tokenIn, route.TokenOutDenom, _outMinAmount, swapFee) + tokenOutAmount, err = swapModule.SwapExactAmountIn(ctx, sender, pool, tokenIn, routeStep.TokenOutDenom, _outMinAmount, swapFee) if err != nil { return sdk.Int{}, err } // Chain output of current pool as the input for the next routed pool - tokenIn = sdk.NewCoin(route.TokenOutDenom, tokenOutAmount) + tokenIn = sdk.NewCoin(routeStep.TokenOutDenom, tokenOutAmount) } return tokenOutAmount, nil } -// SplitRouteExactAmountIn routes the swap across multiple multihop paths +// SplitRouteExactAmountIn route the swap across multiple multihop paths // to get the desired token out. This is useful for achieving the most optimal execution. However, note that the responsibility -// of determining the optimal split is left to the client. This method simply routes the swap across the given routes. -// The routes must end with the same token out and begin with the same token in. +// of determining the optimal split is left to the client. This method simply route the swap across the given route. +// The route must end with the same token out and begin with the same token in. // // It performs the price impact protection check on the combination of tokens out from all multihop paths. The given tokenOutMinAmount // is used for comparison. // // Returns error if: -// - routes are empty -// - routes contain duplicate multihop paths -// - last token out denom is not the same for all multihop paths in route +// - route are empty +// - route contain duplicate multihop paths +// - last token out denom is not the same for all multihop paths in routeStep // - one of the multihop swaps fails for internal reasons // - final token out computed is not positive // - final token out computed is smaller than tokenOutMinAmount func (k Keeper) SplitRouteExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, - routes []types.SwapAmountInSplitRoute, + route []types.SwapAmountInSplitRoute, tokenInDenom string, tokenOutMinAmount sdk.Int, ) (sdk.Int, error) { - if err := types.ValidateSwapAmountInSplitRoute(routes); err != nil { + if err := types.ValidateSwapAmountInSplitRoute(route); err != nil { return sdk.Int{}, err } @@ -133,7 +140,7 @@ func (k Keeper) SplitRouteExactAmountIn( totalOutAmount = sdk.ZeroInt() ) - for _, multihopRoute := range routes { + for _, multihopRoute := range route { tokenOutAmount, err := k.RouteExactAmountIn( ctx, sender, @@ -171,23 +178,28 @@ func (k Keeper) SwapExactAmountIn( tokenOutDenom string, tokenOutMinAmount sdk.Int, ) (tokenOutAmount sdk.Int, err error) { + // Get the pool-specific module implementation to ensure that + // swaps are routed to the pool type corresponding to pool ID's pool. swapModule, err := k.GetPoolModule(ctx, poolId) if err != nil { return sdk.Int{}, err } + // Get pool as a general pool type. Note that the underlying function used + // still varies with the pool type. pool, poolErr := swapModule.GetPool(ctx, poolId) if poolErr != nil { 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()) } swapFee := pool.GetSwapFee(ctx) + // routeStep to the pool-specific SwapExactAmountIn implementation. tokenOutAmount, err = swapModule.SwapExactAmountIn(ctx, sender, pool, tokenIn, tokenOutDenom, tokenOutMinAmount, swapFee) if err != nil { return sdk.Int{}, err @@ -198,7 +210,7 @@ func (k Keeper) SwapExactAmountIn( func (k Keeper) MultihopEstimateOutGivenExactAmountIn( ctx sdk.Context, - routes []types.SwapAmountInRoute, + route []types.SwapAmountInRoute, tokenIn sdk.Coin, ) (tokenOutAmount sdk.Int, err error) { var ( @@ -215,40 +227,40 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( } }() - route := types.SwapAmountInRoutes(routes) - if err := route.Validate(); err != nil { + routeStep := types.SwapAmountInRoutes(route) + if err := routeStep.Validate(); err != nil { return sdk.Int{}, err } - if k.isOsmoRoutedMultihop(ctx, route, routes[0].TokenOutDenom, tokenIn.Denom) { + if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenOutDenom, tokenIn.Denom) { isMultiHopRouted = true - routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, route) + routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep) if err != nil { return sdk.Int{}, err } } - for _, route := range routes { - swapModule, err := k.GetPoolModule(ctx, route.PoolId) + for _, routeStep := range route { + swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) if err != nil { return sdk.Int{}, err } // Execute the expected swap on the current routed pool - poolI, poolErr := swapModule.GetPool(ctx, route.PoolId) + poolI, poolErr := swapModule.GetPool(ctx, routeStep.PoolId) if poolErr != nil { return sdk.Int{}, poolErr } swapFee := poolI.GetSwapFee(ctx) - // If we determined the route is an osmo multi-hop and both routes are incentivized, + // 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))) } - tokenOut, err := swapModule.CalcOutAmtGivenIn(ctx, poolI, tokenIn, route.TokenOutDenom, swapFee) + tokenOut, err := swapModule.CalcOutAmtGivenIn(ctx, poolI, tokenIn, routeStep.TokenOutDenom, swapFee) if err != nil { return sdk.Int{}, err } @@ -259,7 +271,7 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( } // Chain output of current pool as the input for the next routed pool - tokenIn = sdk.NewCoin(route.TokenOutDenom, tokenOutAmount) + tokenIn = sdk.NewCoin(routeStep.TokenOutDenom, tokenOutAmount) } return tokenOutAmount, err } @@ -270,13 +282,13 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( // 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, - routes []types.SwapAmountOutRoute, + route []types.SwapAmountOutRoute, tokenInMaxAmount sdk.Int, tokenOut sdk.Coin, ) (tokenInAmount sdk.Int, err error) { isMultiHopRouted, routeSwapFee, sumOfSwapFees := false, sdk.Dec{}, sdk.Dec{} - route := types.SwapAmountOutRoutes(routes) - if err := route.Validate(); err != nil { + routeStep := types.SwapAmountOutRoutes(route) + if err := routeStep.Validate(); err != nil { return sdk.Int{}, err } @@ -288,29 +300,29 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, }() // in this loop, we check if: - // - the route is of length 2 - // - route 1 and route 2 don't trade via the same pool - // - route 1 contains uosmo - // - both route 1 and route 2 are incentivized pools + // - 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) // fee_per_pool = total_swap_fee * ((pool_fee) / (swapfee1 + swapfee2)) - if k.isOsmoRoutedMultihop(ctx, route, routes[0].TokenInDenom, tokenOut.Denom) { + if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) { isMultiHopRouted = true - routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, route) + routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep) if err != nil { return sdk.Int{}, err } } - // Determine what the estimated input would be for each pool along the multi-hop route - // if we determined the route is an osmo multi-hop and both routes are incentivized, + // 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 { - insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, routes, tokenOut, routeSwapFee, sumOfSwapFees) + insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSwapFee, sumOfSwapFees) } else { - insExpected, err = k.createMultihopExpectedSwapOuts(ctx, routes, tokenOut) + insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) } if err != nil { return sdk.Int{}, err @@ -324,22 +336,22 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, // Iterates through each routed pool and executes their respective swaps. Note that all of the work to get the return // 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, route := range routes { - swapModule, err := k.GetPoolModule(ctx, route.PoolId) + for i, routeStep := range route { + swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) if err != nil { return sdk.Int{}, err } _tokenOut := tokenOut - // If there is one pool left in the route, set the expected output of the current swap + // If there is one pool left in the routeStep, set the expected output of the current swap // to the estimated input of the final pool. - if i != len(routes)-1 { - _tokenOut = sdk.NewCoin(routes[i+1].TokenInDenom, insExpected[i+1]) + if i != len(route)-1 { + _tokenOut = sdk.NewCoin(route[i+1].TokenInDenom, insExpected[i+1]) } // Execute the expected swap on the current routed pool - pool, poolErr := swapModule.GetPool(ctx, route.PoolId) + pool, poolErr := swapModule.GetPool(ctx, routeStep.PoolId) if poolErr != nil { return sdk.Int{}, poolErr } @@ -354,7 +366,7 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, swapFee = routeSwapFee.Mul((swapFee.Quo(sumOfSwapFees))) } - _tokenInAmount, swapErr := swapModule.SwapExactAmountOut(ctx, sender, pool, route.TokenInDenom, insExpected[i], _tokenOut, swapFee) + _tokenInAmount, swapErr := swapModule.SwapExactAmountOut(ctx, sender, pool, routeStep.TokenInDenom, insExpected[i], _tokenOut, swapFee) if swapErr != nil { return sdk.Int{}, swapErr } @@ -370,29 +382,29 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, return tokenInAmount, nil } -// SplitRouteExactAmountOut routes the swap across multiple multihop paths +// SplitRouteExactAmountOut route the swap across multiple multihop paths // to get the desired token in. This is useful for achieving the most optimal execution. However, note that the responsibility -// of determining the optimal split is left to the client. This method simply routes the swap across the given routes. -// The routes must end with the same token out and begin with the same token in. +// of determining the optimal split is left to the client. This method simply route the swap across the given route. +// The route must end with the same token out and begin with the same token in. // // It performs the price impact protection check on the combination of tokens in from all multihop paths. The given tokenInMaxAmount // is used for comparison. // // Returns error if: -// - routes are empty -// - routes contain duplicate multihop paths -// - last token out denom is not the same for all multihop paths in route +// - route are empty +// - route contain duplicate multihop paths +// - last token out denom is not the same for all multihop paths in routeStep // - one of the multihop swaps fails for internal reasons // - final token out computed is not positive // - final token out computed is smaller than tokenInMaxAmount func (k Keeper) SplitRouteExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, - routes []types.SwapAmountOutSplitRoute, + route []types.SwapAmountOutSplitRoute, tokenOutDenom string, tokenInMaxAmount sdk.Int, ) (sdk.Int, error) { - if err := types.ValidateSwapAmountOutSplitRoute(routes); err != nil { + if err := types.ValidateSwapAmountOutSplitRoute(route); err != nil { return sdk.Int{}, err } @@ -405,7 +417,7 @@ func (k Keeper) SplitRouteExactAmountOut( totalInAmount = sdk.ZeroInt() ) - for _, multihopRoute := range routes { + for _, multihopRoute := range route { tokenOutAmount, err := k.RouteExactAmountOut( ctx, sender, @@ -468,7 +480,7 @@ func (k Keeper) RouteCalculateSpotPrice( func (k Keeper) MultihopEstimateInGivenExactAmountOut( ctx sdk.Context, - routes []types.SwapAmountOutRoute, + route []types.SwapAmountOutRoute, tokenOut sdk.Coin, ) (tokenInAmount sdk.Int, err error) { isMultiHopRouted, routeSwapFee, sumOfSwapFees := false, sdk.Dec{}, sdk.Dec{} @@ -482,26 +494,26 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( } }() - route := types.SwapAmountOutRoutes(routes) - if err := route.Validate(); err != nil { + routeStep := types.SwapAmountOutRoutes(route) + if err := routeStep.Validate(); err != nil { return sdk.Int{}, err } - if k.isOsmoRoutedMultihop(ctx, route, routes[0].TokenInDenom, tokenOut.Denom) { + if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) { isMultiHopRouted = true - routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, route) + routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep) if err != nil { return sdk.Int{}, err } } - // Determine what the estimated input would be for each pool along the multi-hop route - // if we determined the route is an osmo multi-hop and both routes are incentivized, + // 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 if isMultiHopRouted { - insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, routes, tokenOut, routeSwapFee, sumOfSwapFees) + insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSwapFee, sumOfSwapFees) } else { - insExpected, err = k.createMultihopExpectedSwapOuts(ctx, routes, tokenOut) + insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) } if err != nil { return sdk.Int{}, err @@ -572,6 +584,7 @@ func (k Keeper) isOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, return route0Incentivized && route1Incentivized } +// getOsmoRoutedMultihopTotalSwapFee calculates the effective swap fees and the sum of swap fees for the route of pools. func (k Keeper) getOsmoRoutedMultihopTotalSwapFee(ctx sdk.Context, route types.MultihopRoute) ( totalPathSwapFee sdk.Dec, sumOfSwapFees sdk.Dec, err error, ) { @@ -598,31 +611,31 @@ func (k Keeper) getOsmoRoutedMultihopTotalSwapFee(ctx sdk.Context, route types.M } // createMultihopExpectedSwapOuts defines the output denom and output amount for the last pool in -// the route of pools the caller is intending to hop through in a fixed-output multihop tx. It estimates the input -// amount for this last pool and then chains that input as the output of the previous pool in the route, repeating +// the routeStep of pools the caller is intending to hop through in a fixed-output multihop tx. It estimates the input +// 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 -// route of pools for the original multihop transaction. +// routeStep of pools for the original multihop transaction. // TODO: test this. func (k Keeper) createMultihopExpectedSwapOuts( ctx sdk.Context, - routes []types.SwapAmountOutRoute, + route []types.SwapAmountOutRoute, tokenOut sdk.Coin, ) ([]sdk.Int, error) { - insExpected := make([]sdk.Int, len(routes)) - for i := len(routes) - 1; i >= 0; i-- { - route := routes[i] + insExpected := make([]sdk.Int, len(route)) + for i := len(route) - 1; i >= 0; i-- { + routeStep := route[i] - swapModule, err := k.GetPoolModule(ctx, route.PoolId) + swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) if err != nil { return nil, err } - poolI, err := swapModule.GetPool(ctx, route.PoolId) + poolI, err := swapModule.GetPool(ctx, routeStep.PoolId) if err != nil { return nil, err } - tokenIn, err := swapModule.CalcInAmtGivenOut(ctx, poolI, tokenOut, route.TokenInDenom, poolI.GetSwapFee(ctx)) + tokenIn, err := swapModule.CalcInAmtGivenOut(ctx, poolI, tokenOut, routeStep.TokenInDenom, poolI.GetSwapFee(ctx)) if err != nil { return nil, err } @@ -637,26 +650,26 @@ func (k Keeper) createMultihopExpectedSwapOuts( // createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee func (k Keeper) createOsmoMultihopExpectedSwapOuts( ctx sdk.Context, - routes []types.SwapAmountOutRoute, + route []types.SwapAmountOutRoute, tokenOut sdk.Coin, cumulativeRouteSwapFee, sumOfSwapFees sdk.Dec, ) ([]sdk.Int, error) { - insExpected := make([]sdk.Int, len(routes)) - for i := len(routes) - 1; i >= 0; i-- { - route := routes[i] + insExpected := make([]sdk.Int, len(route)) + for i := len(route) - 1; i >= 0; i-- { + routeStep := route[i] - swapModule, err := k.GetPoolModule(ctx, route.PoolId) + swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) if err != nil { return nil, err } - poolI, err := swapModule.GetPool(ctx, route.PoolId) + poolI, err := swapModule.GetPool(ctx, routeStep.PoolId) if err != nil { return nil, err } swapFee := poolI.GetSwapFee(ctx) - tokenIn, err := swapModule.CalcInAmtGivenOut(ctx, poolI, tokenOut, route.TokenInDenom, cumulativeRouteSwapFee.Mul((swapFee.Quo(sumOfSwapFees)))) + tokenIn, err := swapModule.CalcInAmtGivenOut(ctx, poolI, tokenOut, routeStep.TokenInDenom, cumulativeRouteSwapFee.Mul((swapFee.Quo(sumOfSwapFees)))) if err != nil { return nil, err } From bc6e958f07293effd9f39147911c077b80c7cc62 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 7 May 2023 19:50:08 -0700 Subject: [PATCH 2/7] add tests for untested files and fix minor bugs and issues --- app/apptesting/concentrated_liquidity.go | 24 ++ x/concentrated-liquidity/fees_test.go | 7 +- x/concentrated-liquidity/math/math_test.go | 43 +++ x/concentrated-liquidity/swaps.go | 5 +- x/concentrated-liquidity/swaps_test.go | 9 + x/pool-incentives/keeper/keeper.go | 23 +- x/poolmanager/export_test.go | 4 + x/poolmanager/keeper_test.go | 13 + x/poolmanager/router.go | 23 +- x/poolmanager/router_test.go | 413 ++++++++++++++++++++- x/poolmanager/types/errors.go | 8 + x/poolmanager/types/routes_test.go | 64 ++++ 12 files changed, 617 insertions(+), 19 deletions(-) diff --git a/app/apptesting/concentrated_liquidity.go b/app/apptesting/concentrated_liquidity.go index 41502974c9a..e445cc60beb 100644 --- a/app/apptesting/concentrated_liquidity.go +++ b/app/apptesting/concentrated_liquidity.go @@ -41,6 +41,30 @@ func (s *KeeperTestHelper) PrepareConcentratedPoolWithCoinsAndFullRangePosition( return clPool } +// createConcentratedPoolsFromCoinsWithSwapFee creates CL pools from given sets of coins and respective swap fees. +// Where element 1 of the input corresponds to the first pool created, element 2 to the second pool created etc. +func (s *KeeperTestHelper) CreateConcentratedPoolsAndFullRangePositionWithSwapFee(poolDenoms [][]string, swapFee []sdk.Dec) { + for i, curPoolDenoms := range poolDenoms { + s.Require().Equal(2, len(curPoolDenoms)) + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], curPoolDenoms[0], curPoolDenoms[1], DefaultTickSpacing, swapFee[i]) + fundCoins := sdk.NewCoins(sdk.NewCoin(curPoolDenoms[0], DefaultCoinAmount), sdk.NewCoin(curPoolDenoms[1], DefaultCoinAmount)) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool, fundCoins) + } +} + +// createConcentratedPoolsFromCoins creates CL pools from given sets of coins (with zero swap fees). +// Where element 1 of the input corresponds to the first pool created, element 2 to the second pool created etc. +func (s *KeeperTestHelper) CreateConcentratedPoolsAndFullRangePosition(poolDenoms [][]string) { + for _, curPoolDenoms := range poolDenoms { + s.Require().Equal(2, len(curPoolDenoms)) + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], curPoolDenoms[0], curPoolDenoms[1], DefaultTickSpacing, sdk.ZeroDec()) + fundCoins := sdk.NewCoins(sdk.NewCoin(curPoolDenoms[0], DefaultCoinAmount), sdk.NewCoin(curPoolDenoms[1], DefaultCoinAmount)) + s.FundAcc(s.TestAccs[0], fundCoins) + s.CreateFullRangePosition(clPool, fundCoins) + } +} + // PrepareConcentratedPoolWithCoinsAndLockedFullRangePosition sets up a concentrated liquidity pool with custom denoms. // It also creates a full range position and locks it for 14 days. func (s *KeeperTestHelper) PrepareConcentratedPoolWithCoinsAndLockedFullRangePosition(denom1, denom2 string) (types.ConcentratedPoolExtension, uint64, uint64) { diff --git a/x/concentrated-liquidity/fees_test.go b/x/concentrated-liquidity/fees_test.go index f65e4a9db92..fa2925e67c0 100644 --- a/x/concentrated-liquidity/fees_test.go +++ b/x/concentrated-liquidity/fees_test.go @@ -1523,7 +1523,12 @@ func (s *KeeperTestSuite) TestFunctional_Fees_LP() { // Collect fees. feesCollected := s.collectFeesAndCheckInvariance(ctx, 0, DefaultMinTick, DefaultMaxTick, positionIdOne, sdk.NewCoins(), []string{USDC}, [][]sdk.Int{ticksActivatedAfterEachSwap}) - s.Require().Equal(totalFeesExpected, feesCollected) + expectedFeesTruncated := totalFeesExpected + for i, feeToken := range totalFeesExpected { + // We run expected fees through a cycle of divison and multiplication by liquidity to capture appropriate rounding behavior + expectedFeesTruncated[i] = sdk.NewCoin(feeToken.Denom, feeToken.Amount.ToDec().QuoTruncate(liquidity).MulTruncate(liquidity).TruncateInt()) + } + s.Require().Equal(expectedFeesTruncated, feesCollected) // Unclaimed rewards should be emptied since fees were collected. s.validatePositionFeeGrowth(pool.GetId(), positionIdOne, cl.EmptyCoins) diff --git a/x/concentrated-liquidity/math/math_test.go b/x/concentrated-liquidity/math/math_test.go index a5854549446..ca422c633cb 100644 --- a/x/concentrated-liquidity/math/math_test.go +++ b/x/concentrated-liquidity/math/math_test.go @@ -79,6 +79,49 @@ func (suite *ConcentratedMathTestSuite) TestLiquidity0() { } } +func (suite *ConcentratedMathTestSuite) TestAddLiquidity() { + testCases := map[string]struct { + inputLiqA sdk.Dec + inputLiqB sdk.Dec + + expectedOutout sdk.Dec + }{ + "happy path": { + inputLiqA: sdk.MustNewDecFromStr("1000000000"), + inputLiqB: sdk.MustNewDecFromStr("300000999"), + + expectedOutout: sdk.MustNewDecFromStr("1300000999"), + }, + "second value negative": { + inputLiqA: sdk.MustNewDecFromStr("1000000000"), + inputLiqB: sdk.MustNewDecFromStr("-300000999"), + + expectedOutout: sdk.MustNewDecFromStr("699999001"), + }, + "first value negative": { + inputLiqA: sdk.MustNewDecFromStr("-1000000000"), + inputLiqB: sdk.MustNewDecFromStr("300000999"), + + expectedOutout: sdk.MustNewDecFromStr("-699999001"), + }, + "both values negative": { + inputLiqA: sdk.MustNewDecFromStr("-1000000000"), + inputLiqB: sdk.MustNewDecFromStr("-300000999"), + + expectedOutout: sdk.MustNewDecFromStr("-1300000999"), + }, + } + + for name, tc := range testCases { + tc := tc + + suite.Run(name, func() { + actualOutput := math.AddLiquidity(tc.inputLiqA, tc.inputLiqB) + suite.Require().Equal(tc.expectedOutout, actualOutput) + }) + } +} + // TestGetNextSqrtPriceFromAmount0RoundingUp tests that getNextSqrtPriceFromAmount0RoundingUp utilizes // the current squareRootPrice, liquidity of denom0, and amount of denom0 that still needs // to be swapped in order to determine the next squareRootPrice diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 494c03325f2..f2a114b318f 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -61,10 +61,11 @@ type SwapState struct { // between the ticks. This is possible when there are only 2 positions with no overlapping ranges. // As a result, the range from the end of position one to the beginning of position // two has no liquidity and can be skipped. -// TODO: test func (ss *SwapState) updateFeeGrowthGlobal(feeChargeTotal sdk.Dec) { if !ss.liquidity.IsZero() { - feeChargePerUnitOfLiquidity := feeChargeTotal.Quo(ss.liquidity) + // We round down here since we want to avoid overdistributing (the "fee charge" refers to + // the total fees that will be accrued to the fee accumulator) + feeChargePerUnitOfLiquidity := feeChargeTotal.QuoTruncate(ss.liquidity) ss.feeGrowthGlobal = ss.feeGrowthGlobal.Add(feeChargePerUnitOfLiquidity) return } diff --git a/x/concentrated-liquidity/swaps_test.go b/x/concentrated-liquidity/swaps_test.go index 2b2a56fa6ea..0ac11f7eb47 100644 --- a/x/concentrated-liquidity/swaps_test.go +++ b/x/concentrated-liquidity/swaps_test.go @@ -2621,6 +2621,15 @@ func (suite *KeeperTestSuite) TestUpdatePoolForSwap() { newLiquidity: sdk.NewDec(2), newSqrtPrice: sdk.NewDec(2), }, + "success case with different/uneven numbers": { + senderInitialBalance: defaultInitialBalance.Add(defaultInitialBalance...), + poolInitialBalance: defaultInitialBalance, + tokenIn: oneHundredETH.Add(oneHundredETH), + tokenOut: oneHundredUSDC, + newCurrentTick: sdk.NewInt(8), + newLiquidity: sdk.NewDec(37), + newSqrtPrice: sdk.NewDec(91), + }, "sender does not have enough balance": { senderInitialBalance: defaultInitialBalance, poolInitialBalance: defaultInitialBalance, diff --git a/x/pool-incentives/keeper/keeper.go b/x/pool-incentives/keeper/keeper.go index 7035fe27019..47074bd4b9e 100644 --- a/x/pool-incentives/keeper/keeper.go +++ b/x/pool-incentives/keeper/keeper.go @@ -217,13 +217,30 @@ func (k Keeper) GetAllGauges(ctx sdk.Context) []incentivestypes.Gauge { return gauges } +// IsPoolIncentivized returns a boolean representing whether the given pool ID +// corresponds to an incentivized pool. It fails quietly by returning false if +// the pool does not exist or does not have any records, as this is technically +// equivalent to the pool not being incentivized. func (k Keeper) IsPoolIncentivized(ctx sdk.Context, poolId uint64) bool { - lockableDurations := k.GetLockableDurations(ctx) + pool, err := k.poolmanagerKeeper.GetPool(ctx, poolId) + if err != nil { + return false + } + isCLPool := pool.GetType() == poolmanagertypes.Concentrated + + var gaugeDurations []time.Duration + if isCLPool { + incParams := k.incentivesKeeper.GetEpochInfo(ctx) + gaugeDurations = []time.Duration{incParams.Duration} + } else { + gaugeDurations = k.GetLockableDurations(ctx) + } + distrInfo := k.GetDistrInfo(ctx) candidateGaugeIds := []uint64{} - for _, lockableDuration := range lockableDurations { - gaugeId, err := k.GetPoolGaugeId(ctx, poolId, lockableDuration) + for _, gaugeDuration := range gaugeDurations { + gaugeId, err := k.GetPoolGaugeId(ctx, poolId, gaugeDuration) if err == nil { candidateGaugeIds = append(candidateGaugeIds, gaugeId) } diff --git a/x/poolmanager/export_test.go b/x/poolmanager/export_test.go index 9b0faad18b2..4b9071b2d54 100644 --- a/x/poolmanager/export_test.go +++ b/x/poolmanager/export_test.go @@ -42,3 +42,7 @@ func (k Keeper) GetAllPoolRoutes(ctx sdk.Context) []types.ModuleRoute { func (k Keeper) ValidateCreatedPool(ctx sdk.Context, poolId uint64, pool types.PoolI) error { return k.validateCreatedPool(ctx, poolId, pool) } + +func (k Keeper) IsOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, inDenom, outDenom string) (isRouted bool) { + return k.isOsmoRoutedMultihop(ctx, route, inDenom, outDenom) +} diff --git a/x/poolmanager/keeper_test.go b/x/poolmanager/keeper_test.go index ce5958b9e5f..0b9eb7a7fc5 100644 --- a/x/poolmanager/keeper_test.go +++ b/x/poolmanager/keeper_test.go @@ -52,6 +52,19 @@ func (suite *KeeperTestSuite) createBalancerPoolsFromCoinsWithSwapFee(poolCoins } } +// createBalancerPoolsFromCoins creates balancer pools from given sets of coins and zero swap fees. +// Where element 1 of the input corresponds to the first pool created, +// element 2 to the second pool created, up until the last element. +func (suite *KeeperTestSuite) createBalancerPoolsFromCoins(poolCoins []sdk.Coins) { + for _, curPoolCoins := range poolCoins { + suite.FundAcc(suite.TestAccs[0], curPoolCoins) + suite.PrepareCustomBalancerPoolFromCoins(curPoolCoins, balancer.PoolParams{ + SwapFee: sdk.ZeroDec(), + ExitFee: sdk.ZeroDec(), + }) + } +} + func (suite *KeeperTestSuite) TestInitGenesis() { suite.Setup() diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index a5c1cdb7774..c025f40884f 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -83,16 +83,15 @@ func (k Keeper) RouteExactAmountIn( // 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. - // TODO: evaluate rounding if isMultiHopRouted { - swapFee = routeSwapFee.Mul((swapFee.Quo(sumOfSwapFees))) + swapFee = routeSwapFee.MulRoundUp((swapFee.QuoRoundUp(sumOfSwapFees))) } tokenOutAmount, err = swapModule.SwapExactAmountIn(ctx, sender, pool, tokenIn, routeStep.TokenOutDenom, _outMinAmount, swapFee) @@ -584,12 +583,14 @@ func (k Keeper) isOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, return route0Incentivized && route1Incentivized } -// getOsmoRoutedMultihopTotalSwapFee calculates the effective swap fees and the sum of swap fees for the route of pools. +// getOsmoRoutedMultihopTotalSwapFee calculates and returns the average swap fee and the sum of swap fees for +// a given route. For the former, it sets a lower bound of the highest swap fee pool in the route to ensure total +// swap fees for a route are never more than halved. func (k Keeper) getOsmoRoutedMultihopTotalSwapFee(ctx sdk.Context, route types.MultihopRoute) ( totalPathSwapFee sdk.Dec, sumOfSwapFees sdk.Dec, err error, ) { additiveSwapFee := sdk.ZeroDec() - maxSwapFee := sdk.ZeroDec() + highestSwapFee := sdk.ZeroDec() for _, poolId := range route.PoolIds() { swapModule, err := k.GetPoolModule(ctx, poolId) @@ -603,11 +604,17 @@ func (k Keeper) getOsmoRoutedMultihopTotalSwapFee(ctx sdk.Context, route types.M } swapFee := pool.GetSwapFee(ctx) additiveSwapFee = additiveSwapFee.Add(swapFee) - maxSwapFee = sdk.MaxDec(maxSwapFee, swapFee) + highestSwapFee = sdk.MaxDec(highestSwapFee, swapFee) } + + // We divide by 2 to get the average since OSMO-routed multihops always have exactly 2 pools. averageSwapFee := additiveSwapFee.QuoInt64(2) - maxSwapFee = sdk.MaxDec(maxSwapFee, averageSwapFee) - return maxSwapFee, additiveSwapFee, nil + + // We take the max here as a guardrail to ensure that there is a lowerbound on the swap fee for the + // whole route equivalent to the highest fee pool + routeSwapFee := sdk.MaxDec(highestSwapFee, averageSwapFee) + + return routeSwapFee, additiveSwapFee, nil } // createMultihopExpectedSwapOuts defines the output denom and output amount for the last pool in diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 2118a01abd1..0f48ec14334 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -112,6 +112,11 @@ func (suite *KeeperTestSuite) TestGetPoolModule() { poolId: 1, expectedModule: gammKeeperType, }, + "valid concentrated liquidity pool": { + preCreatePoolType: types.Concentrated, + poolId: 1, + expectedModule: concentratedKeeperType, + }, "non-existent pool": { preCreatePoolType: types.Balancer, poolId: 2, @@ -128,7 +133,6 @@ func (suite *KeeperTestSuite) TestGetPoolModule() { expectError: types.UndefinedRouteError{PoolId: 1, PoolType: types.Balancer}, }, - // TODO: valid concentrated liquidity test case. } for name, tc := range tests { @@ -341,12 +345,14 @@ func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { tests := []struct { name string poolCoins []sdk.Coins + poolDenoms [][]string poolFee []sdk.Dec routes []poolmanagertypes.SwapAmountInRoute incentivizedGauges []uint64 tokenIn sdk.Coin tokenOutMinAmount sdk.Int swapFee sdk.Dec + isConcentrated bool expectError bool expectReducedFeeApplied bool }{ @@ -533,8 +539,50 @@ func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { tokenOutMinAmount: sdk.NewInt(1), expectReducedFeeApplied: false, }, + { + name: "[Concentrated] One route: Swap - [foo -> bar], 1 percent fee", + isConcentrated: true, + poolDenoms: [][]string{ + {foo, bar}, + }, + poolFee: []sdk.Dec{defaultPoolSwapFee}, + routes: []poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + }, + tokenIn: sdk.NewCoin(foo, sdk.NewInt(100000)), + tokenOutMinAmount: sdk.NewInt(1), + }, + { + name: "[Concentrated[ Three routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", + isConcentrated: true, + poolDenoms: [][]string{ + {foo, uosmo}, // pool 1. + {baz, uosmo}, // pool 2. + {bar, baz}, // pool 3. + }, + poolFee: []sdk.Dec{defaultPoolSwapFee, defaultPoolSwapFee, defaultPoolSwapFee}, + routes: []types.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + { + PoolId: 3, + TokenOutDenom: bar, + }, + }, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenIn: sdk.NewCoin(foo, sdk.NewInt(100000)), + tokenOutMinAmount: sdk.NewInt(1), + }, // TODO: - // tests for concentrated liquidity // change values in and out to be different with each swap module type // tests for stable-swap pools // edge cases: @@ -548,7 +596,12 @@ func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { suite.SetupTest() poolmanagerKeeper := suite.App.PoolManagerKeeper - suite.createBalancerPoolsFromCoinsWithSwapFee(tc.poolCoins, tc.poolFee) + if tc.isConcentrated { + // create a concentrated pool with a full range position + suite.CreateConcentratedPoolsAndFullRangePositionWithSwapFee(tc.poolDenoms, tc.poolFee) + } else { + suite.createBalancerPoolsFromCoinsWithSwapFee(tc.poolCoins, tc.poolFee) + } // if test specifies incentivized gauges, set them here if len(tc.incentivizedGauges) > 0 { @@ -561,7 +614,12 @@ func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { suite.Require().Error(err) } else { // calculate the swap as separate swaps with either the reduced swap fee or normal fee - expectedMultihopTokenOutAmount := suite.calcInAmountAsSeparateSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) + var expectedMultihopTokenOutAmount sdk.Coin + if tc.isConcentrated { + expectedMultihopTokenOutAmount = suite.calcInAmountAsSeparateConcentratedSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) + } else { + expectedMultihopTokenOutAmount = suite.calcInAmountAsSeparateBalancerSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) + } // execute the swap multihopTokenOutAmount, err := poolmanagerKeeper.RouteExactAmountIn(suite.Ctx, suite.TestAccs[0], tc.routes, tc.tokenIn, tc.tokenOutMinAmount) // compare the expected tokenOut to the actual tokenOut @@ -1229,7 +1287,9 @@ func (suite *KeeperTestSuite) calcOutAmountAsSeparateSwaps(osmoFeeReduced bool, } } -func (suite *KeeperTestSuite) calcInAmountAsSeparateSwaps(osmoFeeReduced bool, routes []poolmanagertypes.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { +// calcInAmountAsSeparateBalancerSwaps calculates the output amount of a series of swaps on Balancer pools while factoring in reduces swap fee changes. +// It uses GAMM functions directly to ensure the poolmanager functions route to the correct modules. +func (suite *KeeperTestSuite) calcInAmountAsSeparateBalancerSwaps(osmoFeeReduced bool, routes []poolmanagertypes.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { cacheCtx, _ := suite.Ctx.CacheContext() if osmoFeeReduced { // extract route from swap @@ -1267,6 +1327,48 @@ func (suite *KeeperTestSuite) calcInAmountAsSeparateSwaps(osmoFeeReduced bool, r } } +// calcInAmountAsSeparateBalancerSwaps calculates the output amount of a series of swaps on Concentrated pools while factoring in reduces swap fee changes. +// It uses CL functions directly to ensure the poolmanager functions route to the correct modules. +func (suite *KeeperTestSuite) calcInAmountAsSeparateConcentratedSwaps(osmoFeeReduced bool, routes []poolmanagertypes.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { + cacheCtx, _ := suite.Ctx.CacheContext() + if osmoFeeReduced { + // extract route from swap + route := types.SwapAmountInRoutes(routes) + // utilizing the extracted route, determine the routeSwapFee and sumOfSwapFees + // these two variables are used to calculate the overall swap fee utilizing the following formula + // swapFee = routeSwapFee * ((pool_fee) / (sumOfSwapFees)) + routeSwapFee, sumOfSwapFees, err := suite.App.PoolManagerKeeper.GetOsmoRoutedMultihopTotalSwapFee(suite.Ctx, route) + suite.Require().NoError(err) + nextTokenIn := tokenIn + for _, hop := range routes { + // extract the current pool's swap fee + hopPool, err := suite.App.ConcentratedLiquidityKeeper.GetPool(cacheCtx, hop.PoolId) + suite.Require().NoError(err) + currentPoolSwapFee := hopPool.GetSwapFee(cacheCtx) + // utilize the routeSwapFee, sumOfSwapFees, and current pool swap fee to calculate the new reduced swap fee + swapFee := routeSwapFee.Mul((currentPoolSwapFee.Quo(sumOfSwapFees))) + // we then do individual swaps until we reach the end of the swap route + tokenOut, err := suite.App.ConcentratedLiquidityKeeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hopPool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), swapFee) + suite.Require().NoError(err) + nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) + } + return nextTokenIn + } else { + nextTokenIn := tokenIn + for _, hop := range routes { + hopPool, err := suite.App.ConcentratedLiquidityKeeper.GetPool(cacheCtx, hop.PoolId) + suite.Require().NoError(err) + updatedPoolSwapFee := hopPool.GetSwapFee(cacheCtx) + tokenOut, err := suite.App.ConcentratedLiquidityKeeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hopPool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), updatedPoolSwapFee) + suite.Require().NoError(err) + nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) + } + return nextTokenIn + } +} + +// TODO: abstract SwapAgainstBalancerPool and SwapAgainstConcentratedPool + func (suite *KeeperTestSuite) TestSingleSwapExactAmountIn() { tests := []struct { name string @@ -2072,3 +2174,304 @@ func (s *KeeperTestSuite) TestGetTotalPoolLiquidity() { }) } } + +func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { + tests := map[string]struct { + route types.MultihopRoute + balancerPoolCoins []sdk.Coins + concentratedPoolDenoms [][]string + incentivizedGauges []uint64 + inDenom string + outDenom string + expectIsRouted bool + }{ + "happy path: osmo routed (balancer)": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + balancerPoolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 2. + }, + // Note that we incentivize all candidate gauges for the sake of test readability. + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + inDenom: foo, + outDenom: bar, + + expectIsRouted: true, + }, + "happy path: osmo routed (balancer, only one active gauge for each pool)": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + balancerPoolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 2. + }, + incentivizedGauges: []uint64{1, 4}, + inDenom: foo, + outDenom: bar, + + expectIsRouted: true, + }, + "osmo routed (concentrated)": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + concentratedPoolDenoms: [][]string{ + {foo, uosmo}, // pool 1. + {uosmo, baz}, // pool 2. + }, + incentivizedGauges: []uint64{1, 2}, + inDenom: foo, + outDenom: bar, + + expectIsRouted: true, + }, + "osmo routed (mixed concentrated and balancer)": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + concentratedPoolDenoms: [][]string{ + {foo, uosmo}, // pool 1. + }, + balancerPoolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 2. + }, + + incentivizedGauges: []uint64{1, 2}, + inDenom: foo, + outDenom: bar, + + expectIsRouted: true, + }, + "not osmo routed (single pool)": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + }), + inDenom: foo, + outDenom: bar, + + expectIsRouted: false, + }, + "not osmo routed (two pools)": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + }), + inDenom: foo, + outDenom: baz, + + expectIsRouted: false, + }, + } + + for name, tc := range tests { + suite.Run(name, func() { + suite.SetupTest() + poolManagerKeeper := suite.App.PoolManagerKeeper + + // Create pools to route through + if tc.concentratedPoolDenoms != nil { + suite.CreateConcentratedPoolsAndFullRangePosition(tc.concentratedPoolDenoms) + } + + if tc.balancerPoolCoins != nil { + suite.createBalancerPoolsFromCoins(tc.balancerPoolCoins) + } + + // If test specifies incentivized gauges, set them here + if len(tc.incentivizedGauges) > 0 { + suite.makeGaugesIncentivized(tc.incentivizedGauges) + } + + // System under test + isRouted := poolManagerKeeper.IsOsmoRoutedMultihop(suite.Ctx, tc.route, tc.inDenom, tc.outDenom) + + // Check output + suite.Require().Equal(tc.expectIsRouted, isRouted) + }) + } +} + +func (suite *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { + + tests := map[string]struct { + route types.MultihopRoute + balancerPoolCoins []sdk.Coins + concentratedPoolDenoms [][]string + poolFees []sdk.Dec + + expectedRouteFee sdk.Dec + expectedTotalFee sdk.Dec + expectedError error + }{ + "happy path: balancer route": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + poolFees: []sdk.Dec{defaultPoolSwapFee, defaultPoolSwapFee}, + balancerPoolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 2. + }, + + expectedRouteFee: defaultPoolSwapFee, + expectedTotalFee: defaultPoolSwapFee.Add(defaultPoolSwapFee), + }, + "concentrated route": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + poolFees: []sdk.Dec{defaultPoolSwapFee, defaultPoolSwapFee}, + concentratedPoolDenoms: [][]string{ + {foo, uosmo}, // pool 1. + {uosmo, baz}, // pool 2. + }, + + expectedRouteFee: defaultPoolSwapFee, + expectedTotalFee: defaultPoolSwapFee.Add(defaultPoolSwapFee), + }, + "mixed concentrated and balancer route": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + poolFees: []sdk.Dec{defaultPoolSwapFee, defaultPoolSwapFee}, + concentratedPoolDenoms: [][]string{ + {foo, uosmo}, // pool 1. + }, + balancerPoolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 2. + }, + + expectedRouteFee: defaultPoolSwapFee, + expectedTotalFee: defaultPoolSwapFee.Add(defaultPoolSwapFee), + }, + "edge case: average fee is lower than highest pool fee": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + // Note that pool 2 has 5x the swap fee of pool 1 + poolFees: []sdk.Dec{defaultPoolSwapFee, defaultPoolSwapFee.Mul(sdk.NewDec(5))}, + concentratedPoolDenoms: [][]string{ + {foo, uosmo}, // pool 1. + {uosmo, baz}, // pool 2. + }, + + expectedRouteFee: defaultPoolSwapFee.Mul(sdk.NewDec(5)), + expectedTotalFee: defaultPoolSwapFee.Mul(sdk.NewDec(6)), + }, + "error: pool does not exist": { + route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: bar, + }, + }), + poolFees: []sdk.Dec{defaultPoolSwapFee, defaultPoolSwapFee}, + + expectedError: types.FailedToFindRouteError{PoolId: 1}, + }, + } + + for name, tc := range tests { + suite.Run(name, func() { + suite.SetupTest() + poolManagerKeeper := suite.App.PoolManagerKeeper + + // Create pools for test route + if tc.concentratedPoolDenoms != nil { + suite.CreateConcentratedPoolsAndFullRangePositionWithSwapFee(tc.concentratedPoolDenoms, tc.poolFees) + } + + if tc.balancerPoolCoins != nil { + suite.createBalancerPoolsFromCoinsWithSwapFee(tc.balancerPoolCoins, tc.poolFees) + } + + // System under test + routeFee, totalFee, err := poolManagerKeeper.GetOsmoRoutedMultihopTotalSwapFee(suite.Ctx, tc.route) + + // Assertions + if tc.expectedError != nil { + suite.Require().Error(err) + suite.Require().Equal(tc.expectedError.Error(), err.Error()) + suite.Require().Equal(sdk.Dec{}, routeFee) + suite.Require().Equal(sdk.Dec{}, totalFee) + return + } + + suite.Require().NoError(err) + suite.Require().Equal(tc.expectedRouteFee, routeFee) + suite.Require().Equal(tc.expectedTotalFee, totalFee) + }) + } +} diff --git a/x/poolmanager/types/errors.go b/x/poolmanager/types/errors.go index 784d6eb5658..3505923958e 100644 --- a/x/poolmanager/types/errors.go +++ b/x/poolmanager/types/errors.go @@ -125,3 +125,11 @@ type IncorrectPoolAddressError struct { func (e IncorrectPoolAddressError) Error() string { return fmt.Sprintf("Pool was attempted to be created with incorrect pool address. Expected (%s), actual (%s)", e.ExpectedPoolAddress, e.ActualPoolAddress) } + +type InactivePoolError struct { + PoolId uint64 +} + +func (e InactivePoolError) Error() string { + return fmt.Sprintf("Pool %d is not active.", e.PoolId) +} diff --git a/x/poolmanager/types/routes_test.go b/x/poolmanager/types/routes_test.go index 66961215e70..71653c8825c 100644 --- a/x/poolmanager/types/routes_test.go +++ b/x/poolmanager/types/routes_test.go @@ -298,3 +298,67 @@ func TestValidateSwapAmountOutSplitRoute(t *testing.T) { }) } } + +func TestIntermediateDenoms(t *testing.T) { + + tests := map[string]struct { + route SwapAmountInRoutes + expectedDenoms []string + }{ + "happy path: one intermediate denom": { + route: SwapAmountInRoutes([]SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + }), + + expectedDenoms: []string{bar}, + }, + "multiple intermediate denoms": { + route: SwapAmountInRoutes([]SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + { + PoolId: 5, + TokenOutDenom: uosmo, + }, + { + PoolId: 3, + TokenOutDenom: foo, + }, + }), + + expectedDenoms: []string{bar, baz, uosmo}, + }, + "no intermediate denoms (single pool)": { + route: SwapAmountInRoutes([]SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + }), + + // Note that we expect the function to "fail" quietly and + // simply return nil + expectedDenoms: nil, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + actualIntermediateDenoms := tc.route.IntermediateDenoms() + require.Equal(t, tc.expectedDenoms, actualIntermediateDenoms) + }) + } +} From 67dfe521d5f92d34b63a9e6177419cc5832da1e0 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 7 May 2023 20:12:35 -0700 Subject: [PATCH 3/7] add test for rounding behavior --- x/concentrated-liquidity/swaps_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x/concentrated-liquidity/swaps_test.go b/x/concentrated-liquidity/swaps_test.go index 0ac11f7eb47..e12493d1d84 100644 --- a/x/concentrated-liquidity/swaps_test.go +++ b/x/concentrated-liquidity/swaps_test.go @@ -2523,6 +2523,12 @@ func (suite *KeeperTestSuite) TestUpdateFeeGrowthGlobal() { // 10 / 10 = 1 expectedFeeGrowthGlobal: sdk.OneDec(), }, + "rounding test: boundary fee growth": { + liquidity: ten.Add(ten).Mul(sdk.NewDec(1e18)), + feeChargeTotal: ten, + // 10 / (20 * 10^18) = 5 * 10^-19, which we expect to truncate and leave 0. + expectedFeeGrowthGlobal: sdk.ZeroDec(), + }, } for name, tc := range tests { From 7b137dfd081aaa3f37cfb5992a04e5f96364be30 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 7 May 2023 20:15:26 -0700 Subject: [PATCH 4/7] clean up comments --- x/poolmanager/types/routes_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/poolmanager/types/routes_test.go b/x/poolmanager/types/routes_test.go index 71653c8825c..c6e6530d8c8 100644 --- a/x/poolmanager/types/routes_test.go +++ b/x/poolmanager/types/routes_test.go @@ -349,8 +349,7 @@ func TestIntermediateDenoms(t *testing.T) { }, }), - // Note that we expect the function to "fail" quietly and - // simply return nil + // Note that we expect the function to fail quietly expectedDenoms: nil, }, } From 3267d9b480c08a048680fa4b56557c75de110775 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 7 May 2023 20:26:54 -0700 Subject: [PATCH 5/7] fix conflicts --- x/poolmanager/router_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 7ba353113ea..7990db8c467 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -545,7 +545,7 @@ func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { {foo, bar}, }, poolFee: []sdk.Dec{defaultPoolSwapFee}, - routes: []poolmanagertypes.SwapAmountInRoute{ + routes: []types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: bar, @@ -1328,7 +1328,7 @@ func (suite *KeeperTestSuite) calcInAmountAsSeparateBalancerSwaps(osmoFeeReduced // calcInAmountAsSeparateBalancerSwaps calculates the output amount of a series of swaps on Concentrated pools while factoring in reduces swap fee changes. // It uses CL functions directly to ensure the poolmanager functions route to the correct modules. -func (suite *KeeperTestSuite) calcInAmountAsSeparateConcentratedSwaps(osmoFeeReduced bool, routes []poolmanagertypes.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { +func (suite *KeeperTestSuite) calcInAmountAsSeparateConcentratedSwaps(osmoFeeReduced bool, routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { cacheCtx, _ := suite.Ctx.CacheContext() if osmoFeeReduced { // extract route from swap @@ -2183,7 +2183,7 @@ func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { expectIsRouted bool }{ "happy path: osmo routed (balancer)": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2205,7 +2205,7 @@ func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { expectIsRouted: true, }, "happy path: osmo routed (balancer, only one active gauge for each pool)": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2226,7 +2226,7 @@ func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { expectIsRouted: true, }, "osmo routed (concentrated)": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2247,7 +2247,7 @@ func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { expectIsRouted: true, }, "osmo routed (mixed concentrated and balancer)": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2271,7 +2271,7 @@ func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { expectIsRouted: true, }, "not osmo routed (single pool)": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: bar, @@ -2283,7 +2283,7 @@ func (suite *KeeperTestSuite) TestIsOsmoRoutedMultihop() { expectIsRouted: false, }, "not osmo routed (two pools)": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: bar, @@ -2341,7 +2341,7 @@ func (suite *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { expectedError error }{ "happy path: balancer route": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2361,7 +2361,7 @@ func (suite *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { expectedTotalFee: defaultPoolSwapFee.Add(defaultPoolSwapFee), }, "concentrated route": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2381,7 +2381,7 @@ func (suite *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { expectedTotalFee: defaultPoolSwapFee.Add(defaultPoolSwapFee), }, "mixed concentrated and balancer route": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2403,7 +2403,7 @@ func (suite *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { expectedTotalFee: defaultPoolSwapFee.Add(defaultPoolSwapFee), }, "edge case: average fee is lower than highest pool fee": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, @@ -2424,7 +2424,7 @@ func (suite *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() { expectedTotalFee: defaultPoolSwapFee.Mul(sdk.NewDec(6)), }, "error: pool does not exist": { - route: types.SwapAmountInRoutes([]poolmanagertypes.SwapAmountInRoute{ + route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ { PoolId: 1, TokenOutDenom: uosmo, From a4deb90590084c35fcf92f97a6331bc3c83de94e Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 7 May 2023 20:43:27 -0700 Subject: [PATCH 6/7] fix conflicts in tests relating to rounding changes --- x/concentrated-liquidity/position_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index ad5cc92a270..e1e816f2f1b 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -1102,6 +1102,10 @@ func (s *KeeperTestSuite) TestFungifyChargedPositions_SwapAndClaimFees() { // Perform a swap to earn fees swapAmountIn := sdk.NewCoin(ETH, sdk.NewInt(swapAmount)) expectedFee := swapAmountIn.Amount.ToDec().Mul(swapFee) + // We run expected fees through a cycle of divison and multiplication by liquidity to capture appropriate rounding behavior. + // Note that we truncate to int at the end since it is not possible to have a decimal fee amount collected (the QuoTruncate + // and MulTruncates are much smaller operations that round down for values past the 18th decimal place). + expectedFeeTruncated := expectedFee.QuoTruncate(totalLiquidity).MulTruncate(totalLiquidity).TruncateInt() s.FundAcc(s.TestAccs[0], sdk.NewCoins(swapAmountIn)) s.swapAndTrackXTimesInARow(defaultPoolId, swapAmountIn, USDC, types.MinSpotPrice, 1) @@ -1117,7 +1121,7 @@ func (s *KeeperTestSuite) TestFungifyChargedPositions_SwapAndClaimFees() { s.Require().NoError(err) // Validate that the correct fee amount was collected. - s.Require().Equal(expectedFee, collected.AmountOf(swapAmountIn.Denom).ToDec()) + s.Require().Equal(expectedFeeTruncated, collected.AmountOf(swapAmountIn.Denom)) // Check that cannot claim again. collected, err = s.App.ConcentratedLiquidityKeeper.CollectFees(s.Ctx, defaultAddress, newPositionId) From 4a53acd4b04a845607da323b6134b3819b8bbe9a Mon Sep 17 00:00:00 2001 From: stackman27 Date: Thu, 11 May 2023 18:02:22 -0700 Subject: [PATCH 7/7] romans and adams feedback --- app/apptesting/concentrated_liquidity.go | 17 ++--- x/concentrated-liquidity/position_test.go | 2 +- x/concentrated-liquidity/swaps.go | 4 +- x/pool-incentives/keeper/keeper.go | 8 +-- x/pool-incentives/keeper/keeper_test.go | 43 ++++++++++++- x/poolmanager/router_test.go | 77 +++++++---------------- 6 files changed, 82 insertions(+), 69 deletions(-) diff --git a/app/apptesting/concentrated_liquidity.go b/app/apptesting/concentrated_liquidity.go index e445cc60beb..1b192a067d1 100644 --- a/app/apptesting/concentrated_liquidity.go +++ b/app/apptesting/concentrated_liquidity.go @@ -46,7 +46,14 @@ func (s *KeeperTestHelper) PrepareConcentratedPoolWithCoinsAndFullRangePosition( func (s *KeeperTestHelper) CreateConcentratedPoolsAndFullRangePositionWithSwapFee(poolDenoms [][]string, swapFee []sdk.Dec) { for i, curPoolDenoms := range poolDenoms { s.Require().Equal(2, len(curPoolDenoms)) - clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], curPoolDenoms[0], curPoolDenoms[1], DefaultTickSpacing, swapFee[i]) + var curSwapFee sdk.Dec + if len(swapFee) > i { + curSwapFee = swapFee[i] + } else { + curSwapFee = sdk.ZeroDec() + } + + clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], curPoolDenoms[0], curPoolDenoms[1], DefaultTickSpacing, curSwapFee) fundCoins := sdk.NewCoins(sdk.NewCoin(curPoolDenoms[0], DefaultCoinAmount), sdk.NewCoin(curPoolDenoms[1], DefaultCoinAmount)) s.FundAcc(s.TestAccs[0], fundCoins) s.CreateFullRangePosition(clPool, fundCoins) @@ -56,13 +63,7 @@ func (s *KeeperTestHelper) CreateConcentratedPoolsAndFullRangePositionWithSwapFe // createConcentratedPoolsFromCoins creates CL pools from given sets of coins (with zero swap fees). // Where element 1 of the input corresponds to the first pool created, element 2 to the second pool created etc. func (s *KeeperTestHelper) CreateConcentratedPoolsAndFullRangePosition(poolDenoms [][]string) { - for _, curPoolDenoms := range poolDenoms { - s.Require().Equal(2, len(curPoolDenoms)) - clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], curPoolDenoms[0], curPoolDenoms[1], DefaultTickSpacing, sdk.ZeroDec()) - fundCoins := sdk.NewCoins(sdk.NewCoin(curPoolDenoms[0], DefaultCoinAmount), sdk.NewCoin(curPoolDenoms[1], DefaultCoinAmount)) - s.FundAcc(s.TestAccs[0], fundCoins) - s.CreateFullRangePosition(clPool, fundCoins) - } + s.CreateConcentratedPoolsAndFullRangePositionWithSwapFee(poolDenoms, []sdk.Dec{sdk.ZeroDec()}) } // PrepareConcentratedPoolWithCoinsAndLockedFullRangePosition sets up a concentrated liquidity pool with custom denoms. diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index e1e816f2f1b..02900a9ef01 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -1103,7 +1103,7 @@ func (s *KeeperTestSuite) TestFungifyChargedPositions_SwapAndClaimFees() { swapAmountIn := sdk.NewCoin(ETH, sdk.NewInt(swapAmount)) expectedFee := swapAmountIn.Amount.ToDec().Mul(swapFee) // We run expected fees through a cycle of divison and multiplication by liquidity to capture appropriate rounding behavior. - // Note that we truncate to int at the end since it is not possible to have a decimal fee amount collected (the QuoTruncate + // Note that we truncate the int at the end since it is not possible to have a decimal fee amount collected (the QuoTruncate // and MulTruncates are much smaller operations that round down for values past the 18th decimal place). expectedFeeTruncated := expectedFee.QuoTruncate(totalLiquidity).MulTruncate(totalLiquidity).TruncateInt() s.FundAcc(s.TestAccs[0], sdk.NewCoins(swapAmountIn)) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index f2a114b318f..262cc45e2a9 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -65,8 +65,8 @@ func (ss *SwapState) updateFeeGrowthGlobal(feeChargeTotal sdk.Dec) { if !ss.liquidity.IsZero() { // We round down here since we want to avoid overdistributing (the "fee charge" refers to // the total fees that will be accrued to the fee accumulator) - feeChargePerUnitOfLiquidity := feeChargeTotal.QuoTruncate(ss.liquidity) - ss.feeGrowthGlobal = ss.feeGrowthGlobal.Add(feeChargePerUnitOfLiquidity) + feesAccruedPerUnitOfLiquidity := feeChargeTotal.QuoTruncate(ss.liquidity) + ss.feeGrowthGlobal = ss.feeGrowthGlobal.Add(feesAccruedPerUnitOfLiquidity) return } } diff --git a/x/pool-incentives/keeper/keeper.go b/x/pool-incentives/keeper/keeper.go index 47074bd4b9e..48ece964e9c 100644 --- a/x/pool-incentives/keeper/keeper.go +++ b/x/pool-incentives/keeper/keeper.go @@ -228,18 +228,18 @@ func (k Keeper) IsPoolIncentivized(ctx sdk.Context, poolId uint64) bool { } isCLPool := pool.GetType() == poolmanagertypes.Concentrated - var gaugeDurations []time.Duration + var lockableDurations []time.Duration if isCLPool { incParams := k.incentivesKeeper.GetEpochInfo(ctx) - gaugeDurations = []time.Duration{incParams.Duration} + lockableDurations = []time.Duration{incParams.Duration} } else { - gaugeDurations = k.GetLockableDurations(ctx) + lockableDurations = k.GetLockableDurations(ctx) } distrInfo := k.GetDistrInfo(ctx) candidateGaugeIds := []uint64{} - for _, gaugeDuration := range gaugeDurations { + for _, gaugeDuration := range lockableDurations { gaugeId, err := k.GetPoolGaugeId(ctx, poolId, gaugeDuration) if err == nil { candidateGaugeIds = append(candidateGaugeIds, gaugeId) diff --git a/x/pool-incentives/keeper/keeper_test.go b/x/pool-incentives/keeper/keeper_test.go index be029d5c1f3..f6e39b32528 100644 --- a/x/pool-incentives/keeper/keeper_test.go +++ b/x/pool-incentives/keeper/keeper_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" "github.com/osmosis-labs/osmosis/v15/app/apptesting" @@ -11,6 +12,7 @@ import ( gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" incentivestypes "github.com/osmosis-labs/osmosis/v15/x/incentives/types" "github.com/osmosis-labs/osmosis/v15/x/pool-incentives/types" + poolincentivestypes "github.com/osmosis-labs/osmosis/v15/x/pool-incentives/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" ) @@ -303,7 +305,6 @@ func (suite *KeeperTestSuite) TestGetLongestLockableDuration() { for _, tc := range testCases { suite.Run(tc.name, func() { - suite.App.PoolIncentivesKeeper.SetLockableDurations(suite.Ctx, tc.lockableDurations) result, err := suite.App.PoolIncentivesKeeper.GetLongestLockableDuration(suite.Ctx) @@ -317,3 +318,43 @@ func (suite *KeeperTestSuite) TestGetLongestLockableDuration() { }) } } + +func (suite *KeeperTestSuite) TestIsPoolIncentivized() { + testCases := []struct { + name string + poolId uint64 + expectedIsIncentivized bool + }{ + { + name: "Incentivized Pool", + poolId: 1, + expectedIsIncentivized: true, + }, + { + name: "Unincentivized Pool", + poolId: 2, + expectedIsIncentivized: false, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.SetupTest() + suite.PrepareConcentratedPool() + + suite.App.PoolIncentivesKeeper.SetDistrInfo(suite.Ctx, poolincentivestypes.DistrInfo{ + TotalWeight: sdk.NewInt(100), + Records: []poolincentivestypes.DistrRecord{ + { + GaugeId: tc.poolId, + Weight: sdk.NewInt(50), + }, + }, + }) + + actualIsIncentivized := suite.App.PoolIncentivesKeeper.IsPoolIncentivized(suite.Ctx, tc.poolId) + suite.Require().Equal(tc.expectedIsIncentivized, actualIsIncentivized) + }) + } + +} diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 7990db8c467..2632d71cfa4 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -613,12 +613,8 @@ func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { suite.Require().Error(err) } else { // calculate the swap as separate swaps with either the reduced swap fee or normal fee - var expectedMultihopTokenOutAmount sdk.Coin - if tc.isConcentrated { - expectedMultihopTokenOutAmount = suite.calcInAmountAsSeparateConcentratedSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) - } else { - expectedMultihopTokenOutAmount = suite.calcInAmountAsSeparateBalancerSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) - } + expectedMultihopTokenOutAmount := suite.calcInAmountAsSeparatePoolSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) + // execute the swap multihopTokenOutAmount, err := poolmanagerKeeper.RouteExactAmountIn(suite.Ctx, suite.TestAccs[0], tc.routes, tc.tokenIn, tc.tokenOutMinAmount) // compare the expected tokenOut to the actual tokenOut @@ -1286,9 +1282,10 @@ func (suite *KeeperTestSuite) calcOutAmountAsSeparateSwaps(osmoFeeReduced bool, } } -// calcInAmountAsSeparateBalancerSwaps calculates the output amount of a series of swaps on Balancer pools while factoring in reduces swap fee changes. -// It uses GAMM functions directly to ensure the poolmanager functions route to the correct modules. -func (suite *KeeperTestSuite) calcInAmountAsSeparateBalancerSwaps(osmoFeeReduced bool, routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { +// calcInAmountAsSeparatePoolSwaps calculates the output amount of a series of swaps on PoolManager pools while factoring in reduces swap fee changes. +// If its GAMM pool functions directly to ensure the poolmanager functions route to the correct modules. It it's CL pool functions directly to ensure the +// poolmanager functions route to the correct modules. +func (suite *KeeperTestSuite) calcInAmountAsSeparatePoolSwaps(osmoFeeReduced bool, routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { cacheCtx, _ := suite.Ctx.CacheContext() if osmoFeeReduced { // extract route from swap @@ -1299,68 +1296,42 @@ func (suite *KeeperTestSuite) calcInAmountAsSeparateBalancerSwaps(osmoFeeReduced routeSwapFee, sumOfSwapFees, err := suite.App.PoolManagerKeeper.GetOsmoRoutedMultihopTotalSwapFee(suite.Ctx, route) suite.Require().NoError(err) nextTokenIn := tokenIn + for _, hop := range routes { - // extract the current pool's swap fee - hopPool, err := suite.App.GAMMKeeper.GetPoolAndPoke(cacheCtx, hop.PoolId) + swapModule, err := suite.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) suite.Require().NoError(err) - currentPoolSwapFee := hopPool.GetSwapFee(cacheCtx) + + pool, err := swapModule.GetPool(suite.Ctx, hop.PoolId) + suite.Require().NoError(err) + // utilize the routeSwapFee, sumOfSwapFees, and current pool swap fee to calculate the new reduced swap fee - swapFee := routeSwapFee.Mul((currentPoolSwapFee.Quo(sumOfSwapFees))) + swapFee := routeSwapFee.Mul(pool.GetSwapFee(cacheCtx).Quo(sumOfSwapFees)) + // we then do individual swaps until we reach the end of the swap route - tokenOut, err := suite.App.GAMMKeeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hopPool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), swapFee) + tokenOut, err := swapModule.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], pool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), swapFee) suite.Require().NoError(err) + nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) } return nextTokenIn } else { nextTokenIn := tokenIn for _, hop := range routes { - hopPool, err := suite.App.GAMMKeeper.GetPoolAndPoke(cacheCtx, hop.PoolId) - suite.Require().NoError(err) - updatedPoolSwapFee := hopPool.GetSwapFee(cacheCtx) - tokenOut, err := suite.App.GAMMKeeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hopPool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), updatedPoolSwapFee) + swapModule, err := suite.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) suite.Require().NoError(err) - nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) - } - return nextTokenIn - } -} -// calcInAmountAsSeparateBalancerSwaps calculates the output amount of a series of swaps on Concentrated pools while factoring in reduces swap fee changes. -// It uses CL functions directly to ensure the poolmanager functions route to the correct modules. -func (suite *KeeperTestSuite) calcInAmountAsSeparateConcentratedSwaps(osmoFeeReduced bool, routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { - cacheCtx, _ := suite.Ctx.CacheContext() - if osmoFeeReduced { - // extract route from swap - route := types.SwapAmountInRoutes(routes) - // utilizing the extracted route, determine the routeSwapFee and sumOfSwapFees - // these two variables are used to calculate the overall swap fee utilizing the following formula - // swapFee = routeSwapFee * ((pool_fee) / (sumOfSwapFees)) - routeSwapFee, sumOfSwapFees, err := suite.App.PoolManagerKeeper.GetOsmoRoutedMultihopTotalSwapFee(suite.Ctx, route) - suite.Require().NoError(err) - nextTokenIn := tokenIn - for _, hop := range routes { - // extract the current pool's swap fee - hopPool, err := suite.App.ConcentratedLiquidityKeeper.GetPool(cacheCtx, hop.PoolId) + pool, err := swapModule.GetPool(suite.Ctx, hop.PoolId) suite.Require().NoError(err) - currentPoolSwapFee := hopPool.GetSwapFee(cacheCtx) + // utilize the routeSwapFee, sumOfSwapFees, and current pool swap fee to calculate the new reduced swap fee - swapFee := routeSwapFee.Mul((currentPoolSwapFee.Quo(sumOfSwapFees))) + swapFee := pool.GetSwapFee(cacheCtx) + // we then do individual swaps until we reach the end of the swap route - tokenOut, err := suite.App.ConcentratedLiquidityKeeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hopPool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), swapFee) - suite.Require().NoError(err) - nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) - } - return nextTokenIn - } else { - nextTokenIn := tokenIn - for _, hop := range routes { - hopPool, err := suite.App.ConcentratedLiquidityKeeper.GetPool(cacheCtx, hop.PoolId) - suite.Require().NoError(err) - updatedPoolSwapFee := hopPool.GetSwapFee(cacheCtx) - tokenOut, err := suite.App.ConcentratedLiquidityKeeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hopPool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), updatedPoolSwapFee) + tokenOut, err := swapModule.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], pool, nextTokenIn, hop.TokenOutDenom, sdk.OneInt(), swapFee) suite.Require().NoError(err) + nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) + } return nextTokenIn }