From 476e8c884aa8b40200cf3d37cc4f98cf7f75796a Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Wed, 20 Sep 2023 13:03:03 -0600 Subject: [PATCH 1/4] remove osmo multihop discount --- x/poolmanager/export_test.go | 19 - x/poolmanager/router.go | 194 +--------- x/poolmanager/router_test.go | 717 ++++------------------------------- 3 files changed, 79 insertions(+), 851 deletions(-) diff --git a/x/poolmanager/export_test.go b/x/poolmanager/export_test.go index 81d0f3d8d92..497779819e2 100644 --- a/x/poolmanager/export_test.go +++ b/x/poolmanager/export_test.go @@ -13,12 +13,6 @@ func (k Keeper) GetNextPoolIdAndIncrement(ctx sdk.Context) uint64 { return k.getNextPoolIdAndIncrement(ctx) } -func (k Keeper) GetOsmoRoutedMultihopTotalSpreadFactor(ctx sdk.Context, route types.MultihopRoute) ( - totalPathSpreadFactor osmomath.Dec, sumOfSpreadFactors osmomath.Dec, err error, -) { - return k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, route) -} - // SetPoolRoutesUnsafe sets the given routes to the poolmanager keeper // to allow routing from a pool type to a certain swap module. // For example, balancer -> gamm. @@ -43,10 +37,6 @@ func (k Keeper) ValidateCreatedPool(ctx sdk.Context, poolId uint64, pool types.P return k.validateCreatedPool(poolId, pool) } -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, @@ -55,15 +45,6 @@ func (k Keeper) CreateMultihopExpectedSwapOuts( return k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) } -func (k Keeper) CreateOsmoMultihopExpectedSwapOuts( - ctx sdk.Context, - route []types.SwapAmountOutRoute, - tokenOut sdk.Coin, - cumulativeRouteSwapFee, sumOfSwapFees osmomath.Dec, -) ([]osmomath.Int, error) { - return k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, cumulativeRouteSwapFee, sumOfSwapFees) -} - func (k Keeper) CalcTakerFeeExactIn(tokenIn sdk.Coin, takerFee osmomath.Dec) (sdk.Coin, sdk.Coin) { return k.calcTakerFeeExactIn(tokenIn, takerFee) } diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index a9542bcd52e..70032e8e3d8 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -7,8 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" - appparams "github.com/osmosis-labs/osmosis/v19/app/params" - "github.com/osmosis-labs/osmosis/osmomath" "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/v19/x/poolmanager/types" @@ -30,36 +28,12 @@ func (k Keeper) RouteExactAmountIn( tokenIn sdk.Coin, tokenOutMinAmount osmomath.Int, ) (tokenOutAmount osmomath.Int, err error) { - var ( - isMultiHopRouted bool - routeSpreadFactor osmomath.Dec - sumOfSpreadFactors osmomath.Dec - ) - // Ensure that provided route is not empty and has valid denom format. routeStep := types.SwapAmountInRoutes(route) if err := routeStep.Validate(); err != nil { return osmomath.Int{}, err } - // 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_spread_factor = max(spread_factor1, spread_factor2) - // fee_per_pool = total_spread_factor * ((pool_fee) / (spread_factor1 + spread_factor2)) - if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenOutDenom, tokenIn.Denom) { - isMultiHopRouted = true - routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep) - if err != nil { - return osmomath.Int{}, err - } - } - // 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 @@ -88,12 +62,6 @@ func (k Keeper) RouteExactAmountIn( spreadFactor := pool.GetSpreadFactor(ctx) - // If we determined the route is an osmo multi-hop and both routes are incentivized, - // we modify the spread factor accordingly. - if isMultiHopRouted { - spreadFactor = routeSpreadFactor.MulRoundUp((spreadFactor.QuoRoundUp(sumOfSpreadFactors))) - } - tokenInAfterSubTakerFee, err := k.chargeTakerFee(ctx, tokenIn, routeStep.TokenOutDenom, sender, true) if err != nil { return osmomath.Int{}, err @@ -273,12 +241,6 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( route []types.SwapAmountInRoute, tokenIn sdk.Coin, ) (tokenOutAmount osmomath.Int, err error) { - var ( - isMultiHopRouted bool - routeSpreadFactor osmomath.Dec - sumOfSpreadFactors osmomath.Dec - ) - // recover from panic defer func() { if r := recover(); r != nil { @@ -292,14 +254,6 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( return osmomath.Int{}, err } - if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenOutDenom, tokenIn.Denom) { - isMultiHopRouted = true - routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep) - if err != nil { - return osmomath.Int{}, err - } - } - for _, routeStep := range route { swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) if err != nil { @@ -314,12 +268,6 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( spreadFactor := poolI.GetSpreadFactor(ctx) - // If we determined the routeStep is an osmo multi-hop and both route are incentivized, - // we modify the swap fee accordingly. - if isMultiHopRouted { - spreadFactor = routeSpreadFactor.Mul((spreadFactor.Quo(sumOfSpreadFactors))) - } - takerFee, err := k.GetTradingPairTakerFee(ctx, routeStep.TokenOutDenom, tokenIn.Denom) if err != nil { return osmomath.Int{}, err @@ -369,30 +317,8 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, } }() - // 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_spread_factor = total_spread_factor = max(spread_factor1, spread_factor2) - // fee_per_pool = total_spread_factor * ((pool_fee) / (spread_factor1 + spread_factor2)) var insExpected []osmomath.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 - if isMultiHopRouted { - routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep) - if err != nil { - return osmomath.Int{}, err - } - insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSpreadFactor, sumOfSpreadFactors) - } else { - insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) - } + insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) if err != nil { return osmomath.Int{}, err @@ -573,7 +499,6 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( route []types.SwapAmountOutRoute, tokenOut sdk.Coin, ) (tokenInAmount osmomath.Int, err error) { - isMultiHopRouted, routeSpreadFactor, sumOfSpreadFactors := false, osmomath.Dec{}, osmomath.Dec{} var insExpected []osmomath.Int // recover from panic @@ -589,22 +514,8 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( return osmomath.Int{}, err } - if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) { - isMultiHopRouted = true - routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep) - if err != nil { - return osmomath.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, - // we utilize a separate function that calculates the discounted spread factors - if isMultiHopRouted { - insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSpreadFactor, sumOfSpreadFactors) - } else { - insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) - } + insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut) if err != nil { return osmomath.Int{}, err } @@ -652,63 +563,6 @@ 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 - } - intemediateDenoms := route.IntermediateDenoms() - if len(intemediateDenoms) != 1 || intemediateDenoms[0] != appparams.BaseCoinUnit { - return false - } - if inDenom == outDenom { - return false - } - poolIds := route.PoolIds() - if poolIds[0] == poolIds[1] { - return false - } - - route0Incentivized := k.poolIncentivesKeeper.IsPoolIncentivized(ctx, poolIds[0]) - route1Incentivized := k.poolIncentivesKeeper.IsPoolIncentivized(ctx, poolIds[1]) - - return route0Incentivized && route1Incentivized -} - -// getOsmoRoutedMultihopTotalSpreadFactor 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) getOsmoRoutedMultihopTotalSpreadFactor(ctx sdk.Context, route types.MultihopRoute) ( - totalPathSpreadFactor osmomath.Dec, sumOfSpreadFactors osmomath.Dec, err error, -) { - additiveSpreadFactor := osmomath.ZeroDec() - maxSpreadFactor := osmomath.ZeroDec() - - for _, poolId := range route.PoolIds() { - swapModule, err := k.GetPoolModule(ctx, poolId) - if err != nil { - return osmomath.Dec{}, osmomath.Dec{}, err - } - - pool, poolErr := swapModule.GetPool(ctx, poolId) - if poolErr != nil { - return osmomath.Dec{}, osmomath.Dec{}, poolErr - } - spreadFactor := pool.GetSpreadFactor(ctx) - additiveSpreadFactor = additiveSpreadFactor.Add(spreadFactor) - maxSpreadFactor = sdk.MaxDec(maxSpreadFactor, spreadFactor) - } - - // We divide by 2 to get the average since OSMO-routed multihops always have exactly 2 pools. - averageSpreadFactor := additiveSpreadFactor.QuoInt64(2) - - // 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 - routeSpreadFactor := sdk.MaxDec(maxSpreadFactor, averageSpreadFactor) - - return routeSpreadFactor, additiveSpreadFactor, nil -} - // createMultihopExpectedSwapOuts defines the output denom and output amount for the last pool in // 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 @@ -754,50 +608,6 @@ func (k Keeper) createMultihopExpectedSwapOuts( return insExpected, nil } -// createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee. -func (k Keeper) createOsmoMultihopExpectedSwapOuts( - ctx sdk.Context, - route []types.SwapAmountOutRoute, - tokenOut sdk.Coin, - cumulativeRouteSpreadFactor, sumOfSpreadFactors osmomath.Dec, -) ([]osmomath.Int, error) { - insExpected := make([]osmomath.Int, len(route)) - for i := len(route) - 1; i >= 0; i-- { - routeStep := route[i] - - swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId) - if err != nil { - return nil, err - } - - poolI, err := swapModule.GetPool(ctx, routeStep.PoolId) - if err != nil { - return nil, err - } - - spreadFactor := poolI.GetSpreadFactor(ctx) - - takerFee, err := k.GetTradingPairTakerFee(ctx, routeStep.TokenInDenom, tokenOut.Denom) - if err != nil { - return nil, err - } - - osmoDiscountedSpreadFactor := cumulativeRouteSpreadFactor.Mul((spreadFactor.Quo(sumOfSpreadFactors))) - - tokenIn, err := swapModule.CalcInAmtGivenOut(ctx, poolI, tokenOut, routeStep.TokenInDenom, osmoDiscountedSpreadFactor) - if err != nil { - return nil, err - } - - tokenInAfterTakerFee, _ := k.calcTakerFeeExactOut(tokenIn, takerFee) - - insExpected[i] = tokenInAfterTakerFee.Amount - tokenOut = tokenInAfterTakerFee - } - - 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) diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 9dbfd913b74..5aa743d3476 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -433,17 +433,16 @@ func (s *KeeperTestSuite) TestRouteCalculateSpotPrice() { // - fee reduction is applied correctly func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { tests := []struct { - name string - poolCoins []sdk.Coins - poolSpreadFactor []osmomath.Dec - poolType []types.PoolType - routes []types.SwapAmountInRoute - incentivizedGauges []uint64 - tokenIn sdk.Coin - tokenOutMinAmount osmomath.Int - spreadFactor osmomath.Dec - expectError bool - expectReducedFeeApplied bool + name string + poolCoins []sdk.Coins + poolSpreadFactor []osmomath.Dec + poolType []types.PoolType + routes []types.SwapAmountInRoute + incentivizedGauges []uint64 + tokenIn sdk.Coin + tokenOutMinAmount osmomath.Int + spreadFactor osmomath.Dec + expectError bool }{ { name: "One route: Swap - [foo -> bar], 1 percent fee", @@ -481,52 +480,6 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), tokenOutMinAmount: osmomath.NewInt(1), }, - { - name: "Two routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) with a half fee applied, both pools 1 percent fee", - poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(baz, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. - }, - poolType: []types.PoolType{types.Balancer, types.Balancer}, - poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - routes: []types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: baz, - }, - }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenIn: sdk.NewCoin("foo", osmomath.NewInt(100000)), - tokenOutMinAmount: osmomath.NewInt(1), - expectReducedFeeApplied: true, - }, - { - name: "Two routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) with a half fee applied, (pool 1) 1 percent fee, (pool 2) 10 percent fee", - poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(baz, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. - }, - poolType: []types.PoolType{types.Balancer, types.Balancer}, - poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, sdk.NewDecWithPrec(1, 1)}, - routes: []types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: baz, - }, - }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), - tokenOutMinAmount: osmomath.NewInt(1), - expectReducedFeeApplied: true, - }, { name: "Three routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", poolCoins: []sdk.Coins{ @@ -550,10 +503,9 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { TokenOutDenom: bar, }, }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), - tokenOutMinAmount: osmomath.NewInt(1), - expectReducedFeeApplied: false, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), + tokenOutMinAmount: osmomath.NewInt(1), }, { name: "Two routes: Swap between four asset pools - [foo -> bar](pool 1) - [bar -> baz](pool 2), all pools 1 percent fee", @@ -575,35 +527,9 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { TokenOutDenom: baz, }, }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), - tokenOutMinAmount: osmomath.NewInt(1), - expectReducedFeeApplied: false, - }, - { - name: "Two routes: Swap between four asset pools - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2), with a half fee applied, both pools 1 percent fee", - poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount), - sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount), - sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. // pool 3. - }, - poolType: []types.PoolType{types.Balancer, types.Balancer}, - poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - routes: []types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: baz, - }, - }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), - tokenOutMinAmount: osmomath.NewInt(1), - expectReducedFeeApplied: true, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), + tokenOutMinAmount: osmomath.NewInt(1), }, { name: "Three routes: Swap between four asset pools - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", @@ -631,10 +557,9 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { TokenOutDenom: bar, }, }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6, 7, 8, 9}, - tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), - tokenOutMinAmount: osmomath.NewInt(1), - expectReducedFeeApplied: false, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6, 7, 8, 9}, + tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), + tokenOutMinAmount: osmomath.NewInt(1), }, { name: "[Concentrated] One route: Swap - [foo -> bar], 1 percent fee", @@ -746,7 +671,7 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { s.Require().Error(err) } else { // calculate the swap as separate swaps with either the reduced swap fee or normal fee - expectedMultihopTokenOutAmount := s.calcOutGivenInAmountAsSeparatePoolSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenIn) + expectedMultihopTokenOutAmount := s.calcOutGivenInAmountAsSeparatePoolSwaps(tc.routes, tc.tokenIn) // execute the swap multihopTokenOutAmount, err := poolmanagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], tc.routes, tc.tokenIn, tc.tokenOutMinAmount) @@ -765,17 +690,16 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { // - fee reduction is applied correctly func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { tests := []struct { - name string - poolCoins []sdk.Coins - poolSpreadFactor []osmomath.Dec - poolType []types.PoolType - routes []types.SwapAmountOutRoute - incentivizedGauges []uint64 - tokenOut sdk.Coin - tokenInMaxAmount osmomath.Int - spreadFactor osmomath.Dec - expectError bool - expectReducedFeeApplied bool + name string + poolCoins []sdk.Coins + poolSpreadFactor []osmomath.Dec + poolType []types.PoolType + routes []types.SwapAmountOutRoute + incentivizedGauges []uint64 + tokenOut sdk.Coin + tokenInMaxAmount osmomath.Int + spreadFactor osmomath.Dec + expectError bool }{ { name: "One route: Swap - [foo -> bar], 1 percent fee", @@ -814,52 +738,6 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { tokenInMaxAmount: osmomath.NewInt(90000000), tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), }, - { - name: "Two routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) with a half fee applied, both pools 1 percent fee", - poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(baz, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. - }, - poolType: []types.PoolType{types.Balancer, types.Balancer}, - poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - routes: []types.SwapAmountOutRoute{ - { - PoolId: 1, - TokenInDenom: foo, - }, - { - PoolId: 2, - TokenInDenom: uosmo, - }, - }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenInMaxAmount: osmomath.NewInt(90000000), - tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), - expectReducedFeeApplied: true, - }, - { - name: "Two routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) with a half fee applied, (pool 1) 1 percent fee, (pool 2) 10 percent fee", - poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(baz, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. - }, - poolType: []types.PoolType{types.Balancer, types.Balancer}, - poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, sdk.NewDecWithPrec(1, 1)}, - routes: []types.SwapAmountOutRoute{ - { - PoolId: 1, - TokenInDenom: foo, - }, - { - PoolId: 2, - TokenInDenom: uosmo, - }, - }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenInMaxAmount: osmomath.NewInt(90000000), - tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), - expectReducedFeeApplied: true, - }, { name: "Three routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", poolCoins: []sdk.Coins{ @@ -883,10 +761,9 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { TokenInDenom: baz, }, }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenInMaxAmount: osmomath.NewInt(90000000), - tokenOut: sdk.NewCoin(bar, osmomath.NewInt(100000)), - expectReducedFeeApplied: false, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenInMaxAmount: osmomath.NewInt(90000000), + tokenOut: sdk.NewCoin(bar, osmomath.NewInt(100000)), }, { name: "Two routes: Swap between four asset pools - [foo -> bar](pool 1) - [bar -> baz](pool 2), all pools 1 percent fee", @@ -908,35 +785,9 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { TokenInDenom: bar, }, }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), - tokenInMaxAmount: osmomath.NewInt(90000000), - expectReducedFeeApplied: false, - }, - { - name: "Two routes: Swap between four asset pools - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2), with a half fee applied, both pools 1 percent fee", - poolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount), - sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. - sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount), - sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. // pool 3. - }, - poolType: []types.PoolType{types.Balancer, types.Balancer}, - poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - routes: []types.SwapAmountOutRoute{ - { - PoolId: 1, - TokenInDenom: foo, - }, - { - PoolId: 2, - TokenInDenom: uosmo, - }, - }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, - tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), - tokenInMaxAmount: osmomath.NewInt(90000000), - expectReducedFeeApplied: true, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), + tokenInMaxAmount: osmomath.NewInt(90000000), }, { name: "Three routes: Swap between four asset pools - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", @@ -964,10 +815,9 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { TokenInDenom: baz, }, }, - incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6, 7, 8, 9}, - tokenOut: sdk.NewCoin(bar, osmomath.NewInt(100000)), - tokenInMaxAmount: osmomath.NewInt(90000000), - expectReducedFeeApplied: false, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6, 7, 8, 9}, + tokenOut: sdk.NewCoin(bar, osmomath.NewInt(100000)), + tokenInMaxAmount: osmomath.NewInt(90000000), }, { name: "[Cosmwasm] One route: Swap - [foo -> bar], 1 percent fee", @@ -1038,7 +888,7 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { s.Require().Error(err) } else { // calculate the swap as separate swaps with either the reduced swap fee or normal fee - expectedMultihopTokenInAmount := s.calcInGivenOutAmountAsSeparateSwaps(tc.expectReducedFeeApplied, tc.routes, tc.tokenOut) + expectedMultihopTokenInAmount := s.calcInGivenOutAmountAsSeparateSwaps(tc.routes, tc.tokenOut) // execute the swap multihopTokenInAmount, err := poolmanagerKeeper.RouteExactAmountOut(s.Ctx, s.TestAccs[0], tc.routes, tc.tokenInMaxAmount, tc.tokenOut) // compare the expected tokenOut to the actual tokenOut @@ -1425,131 +1275,61 @@ func (s *KeeperTestSuite) makeGaugesIncentivized(incentivizedGauges []uint64) { s.App.PoolIncentivesKeeper.SetDistrInfo(s.Ctx, distInfo) } -func (s *KeeperTestSuite) calcInGivenOutAmountAsSeparateSwaps(osmoFeeReduced bool, routes []types.SwapAmountOutRoute, tokenOut sdk.Coin) sdk.Coin { +func (s *KeeperTestSuite) calcInGivenOutAmountAsSeparateSwaps(routes []types.SwapAmountOutRoute, tokenOut sdk.Coin) sdk.Coin { cacheCtx, _ := s.Ctx.CacheContext() - if osmoFeeReduced { - // extract route from swap - route := types.SwapAmountOutRoutes(routes) - // utilizing the extracted route, determine the routeSpreadFactor and sumOfspreadFactors - // these two variables are used to calculate the overall swap fee utilizing the following formula - // spreadFactor = routeSpreadFactor * ((pool_fee) / (sumOfspreadFactors)) - routeSpreadFactor, sumOfSpreadFactors, err := s.App.PoolManagerKeeper.GetOsmoRoutedMultihopTotalSpreadFactor(s.Ctx, route) + nextTokenOut := tokenOut + for i := len(routes) - 1; i >= 0; i-- { + hop := routes[i] + hopPool, err := s.App.PoolManagerKeeper.GetPool(cacheCtx, hop.PoolId) s.Require().NoError(err) - nextTokenOut := tokenOut - for i := len(routes) - 1; i >= 0; i-- { - hop := routes[i] - // extract the current pool's swap fee - hopPool, err := s.App.GAMMKeeper.GetPoolAndPoke(cacheCtx, hop.PoolId) - s.Require().NoError(err) - currentPoolSpreadFactor := hopPool.GetSpreadFactor(cacheCtx) - // utilize the routeSpreadFactor, sumOfSpreadFactors, and current pool swap fee to calculate the new reduced swap fee - spreadFactor := routeSpreadFactor.Mul((currentPoolSpreadFactor.Quo(sumOfSpreadFactors))) - - takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenInDenom, nextTokenOut.Denom) - s.Require().NoError(err) + updatedPoolSpreadFactor := hopPool.GetSpreadFactor(cacheCtx) - swapModule, err := s.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) - s.Require().NoError(err) - - // we then do individual swaps until we reach the end of the swap route - tokenInAmt, err := swapModule.SwapExactAmountOut(cacheCtx, s.TestAccs[0], hopPool, hop.TokenInDenom, osmomath.NewInt(100000000), nextTokenOut, spreadFactor) - s.Require().NoError(err) - - tokenInCoin := sdk.NewCoin(hop.TokenInDenom, tokenInAmt) - tokenInCoinAfterAddTakerFee, _ := s.App.PoolManagerKeeper.CalcTakerFeeExactOut(tokenInCoin, takerFee) - - nextTokenOut = tokenInCoinAfterAddTakerFee - } - return nextTokenOut - } else { - nextTokenOut := tokenOut - for i := len(routes) - 1; i >= 0; i-- { - hop := routes[i] - hopPool, err := s.App.PoolManagerKeeper.GetPool(cacheCtx, hop.PoolId) - s.Require().NoError(err) - updatedPoolSpreadFactor := hopPool.GetSpreadFactor(cacheCtx) - - takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenInDenom, nextTokenOut.Denom) - s.Require().NoError(err) + takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenInDenom, nextTokenOut.Denom) + s.Require().NoError(err) - swapModule, err := s.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) - s.Require().NoError(err) + swapModule, err := s.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) + s.Require().NoError(err) - tokenInAmt, err := swapModule.SwapExactAmountOut(cacheCtx, s.TestAccs[0], hopPool, hop.TokenInDenom, osmomath.NewInt(100000000), nextTokenOut, updatedPoolSpreadFactor) - s.Require().NoError(err) + tokenInAmt, err := swapModule.SwapExactAmountOut(cacheCtx, s.TestAccs[0], hopPool, hop.TokenInDenom, osmomath.NewInt(100000000), nextTokenOut, updatedPoolSpreadFactor) + s.Require().NoError(err) - tokenInCoin := sdk.NewCoin(hop.TokenInDenom, tokenInAmt) - tokenInCoinAfterAddTakerFee, _ := s.App.PoolManagerKeeper.CalcTakerFeeExactOut(tokenInCoin, takerFee) + tokenInCoin := sdk.NewCoin(hop.TokenInDenom, tokenInAmt) + tokenInCoinAfterAddTakerFee, _ := s.App.PoolManagerKeeper.CalcTakerFeeExactOut(tokenInCoin, takerFee) - nextTokenOut = tokenInCoinAfterAddTakerFee - } - return nextTokenOut + nextTokenOut = tokenInCoinAfterAddTakerFee } + return nextTokenOut } // calcOutGivenInAmountAsSeparatePoolSwaps 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 (s *KeeperTestSuite) calcOutGivenInAmountAsSeparatePoolSwaps(osmoFeeReduced bool, routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { +func (s *KeeperTestSuite) calcOutGivenInAmountAsSeparatePoolSwaps(routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { cacheCtx, _ := s.Ctx.CacheContext() - if osmoFeeReduced { - // extract route from swap - route := types.SwapAmountInRoutes(routes) - // utilizing the extracted route, determine the routeSpreadFactor and sumOfSpreadFactors - // these two variables are used to calculate the overall swap fee utilizing the following formula - // spreadFactor = routeSpreadFactor * ((pool_fee) / (sumOfSpreadFactors)) - routeSpreadFactor, sumOfSpreadFactors, err := s.App.PoolManagerKeeper.GetOsmoRoutedMultihopTotalSpreadFactor(s.Ctx, route) + nextTokenIn := tokenIn + for _, hop := range routes { + swapModule, err := s.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) s.Require().NoError(err) - nextTokenIn := tokenIn - - for _, hop := range routes { - swapModule, err := s.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) - s.Require().NoError(err) - - pool, err := swapModule.GetPool(s.Ctx, hop.PoolId) - s.Require().NoError(err) - - // utilize the routeSpreadFactor, sumOfSpreadFactors, and current pool swap fee to calculate the new reduced swap fee - spreadFactor := routeSpreadFactor.Mul(pool.GetSpreadFactor(cacheCtx).Quo(sumOfSpreadFactors)) - - takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenOutDenom, nextTokenIn.Denom) - s.Require().NoError(err) - nextTokenInAfterSubTakerFee, _ := s.App.PoolManagerKeeper.CalcTakerFeeExactIn(nextTokenIn, takerFee) - - // we then do individual swaps until we reach the end of the swap route - tokenOut, err := swapModule.SwapExactAmountIn(cacheCtx, s.TestAccs[0], pool, nextTokenInAfterSubTakerFee, hop.TokenOutDenom, osmomath.OneInt(), spreadFactor) - s.Require().NoError(err) - - nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) - } - return nextTokenIn - } else { - nextTokenIn := tokenIn - for _, hop := range routes { - swapModule, err := s.App.PoolManagerKeeper.GetPoolModule(cacheCtx, hop.PoolId) - s.Require().NoError(err) - - pool, err := swapModule.GetPool(s.Ctx, hop.PoolId) - s.Require().NoError(err) + pool, err := swapModule.GetPool(s.Ctx, hop.PoolId) + s.Require().NoError(err) - // utilize the routeSpreadFactor, sumOfSpreadFactors, and current pool swap fee to calculate the new reduced swap fee - spreadFactor := pool.GetSpreadFactor(cacheCtx) + // utilize the routeSpreadFactor, sumOfSpreadFactors, and current pool swap fee to calculate the new reduced swap fee + spreadFactor := pool.GetSpreadFactor(cacheCtx) - takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenOutDenom, nextTokenIn.Denom) - s.Require().NoError(err) + takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenOutDenom, nextTokenIn.Denom) + s.Require().NoError(err) - nextTokenInAfterSubTakerFee, _ := s.App.PoolManagerKeeper.CalcTakerFeeExactIn(nextTokenIn, takerFee) + nextTokenInAfterSubTakerFee, _ := s.App.PoolManagerKeeper.CalcTakerFeeExactIn(nextTokenIn, takerFee) - // we then do individual swaps until we reach the end of the swap route - tokenOut, err := swapModule.SwapExactAmountIn(cacheCtx, s.TestAccs[0], pool, nextTokenInAfterSubTakerFee, hop.TokenOutDenom, osmomath.OneInt(), spreadFactor) - s.Require().NoError(err) + // we then do individual swaps until we reach the end of the swap route + tokenOut, err := swapModule.SwapExactAmountIn(cacheCtx, s.TestAccs[0], pool, nextTokenInAfterSubTakerFee, hop.TokenOutDenom, osmomath.OneInt(), spreadFactor) + s.Require().NoError(err) - nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) + nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) - } - return nextTokenIn } + return nextTokenIn } // TODO: abstract SwapAgainstBalancerPool and SwapAgainstConcentratedPool @@ -2750,316 +2530,13 @@ func (s *KeeperTestSuite) TestGetTotalPoolLiquidity() { } } -func (s *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([]types.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([]types.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([]types.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([]types.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([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: bar, - }, - }), - inDenom: foo, - outDenom: bar, - - expectIsRouted: false, - }, - "not osmo routed (two pools)": { - route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: bar, - }, - { - PoolId: 2, - TokenOutDenom: baz, - }, - }), - inDenom: foo, - outDenom: baz, - - expectIsRouted: false, - }, - } - - for name, tc := range tests { - s.Run(name, func() { - s.SetupTest() - poolManagerKeeper := s.App.PoolManagerKeeper - - // Create pools to route through - if tc.concentratedPoolDenoms != nil { - s.CreateConcentratedPoolsAndFullRangePosition(tc.concentratedPoolDenoms) - } - - if tc.balancerPoolCoins != nil { - s.createBalancerPoolsFromCoins(tc.balancerPoolCoins) - } - - // If test specifies incentivized gauges, set them here - if len(tc.incentivizedGauges) > 0 { - s.makeGaugesIncentivized(tc.incentivizedGauges) - } - - // System under test - isRouted := poolManagerKeeper.IsOsmoRoutedMultihop(s.Ctx, tc.route, tc.inDenom, tc.outDenom) - - // Check output - s.Require().Equal(tc.expectIsRouted, isRouted) - }) - } -} - -// TestGetOsmoRoutedMultihopTotalSpreadFactor tests the GetOsmoRoutedMultihopTotalSpreadFactor function -func (s *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSpreadFactor() { +func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { tests := map[string]struct { - route types.MultihopRoute + route []types.SwapAmountOutRoute + tokenOut sdk.Coin balancerPoolCoins []sdk.Coins concentratedPoolDenoms [][]string - poolFees []osmomath.Dec - - expectedRouteFee osmomath.Dec - expectedTotalFee osmomath.Dec - expectedError error - }{ - "happy path: balancer route": { - route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: bar, - }, - }), - poolFees: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - 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: defaultPoolSpreadFactor, - expectedTotalFee: defaultPoolSpreadFactor.Add(defaultPoolSpreadFactor), - }, - "concentrated route": { - route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: bar, - }, - }), - poolFees: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - concentratedPoolDenoms: [][]string{ - {foo, uosmo}, // pool 1. - {uosmo, baz}, // pool 2. - }, - - expectedRouteFee: defaultPoolSpreadFactor, - expectedTotalFee: defaultPoolSpreadFactor.Add(defaultPoolSpreadFactor), - }, - "mixed concentrated and balancer route": { - route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: bar, - }, - }), - poolFees: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - concentratedPoolDenoms: [][]string{ - {foo, uosmo}, // pool 1. - }, - balancerPoolCoins: []sdk.Coins{ - sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 2. - }, - - expectedRouteFee: defaultPoolSpreadFactor, - expectedTotalFee: defaultPoolSpreadFactor.Add(defaultPoolSpreadFactor), - }, - "edge case: average fee is lower than highest pool fee": { - route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: bar, - }, - }), - // Note that pool 2 has 5x the swap fee of pool 1 - poolFees: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor.Mul(osmomath.NewDec(5))}, - concentratedPoolDenoms: [][]string{ - {foo, uosmo}, // pool 1. - {uosmo, baz}, // pool 2. - }, - - expectedRouteFee: defaultPoolSpreadFactor.Mul(osmomath.NewDec(5)), - expectedTotalFee: defaultPoolSpreadFactor.Mul(osmomath.NewDec(6)), - }, - "error: pool does not exist": { - route: types.SwapAmountInRoutes([]types.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: uosmo, - }, - { - PoolId: 2, - TokenOutDenom: bar, - }, - }), - poolFees: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, - - expectedError: types.FailedToFindRouteError{PoolId: 1}, - }, - } - - for name, tc := range tests { - s.Run(name, func() { - s.SetupTest() - poolManagerKeeper := s.App.PoolManagerKeeper - - // Create pools for test route - if tc.concentratedPoolDenoms != nil { - s.CreateConcentratedPoolsAndFullRangePositionWithSpreadFactor(tc.concentratedPoolDenoms, tc.poolFees) - } - - if tc.balancerPoolCoins != nil { - s.createBalancerPoolsFromCoinsWithSpreadFactor(tc.balancerPoolCoins, tc.poolFees) - } - - // System under test - routeFee, totalFee, err := poolManagerKeeper.GetOsmoRoutedMultihopTotalSpreadFactor(s.Ctx, tc.route) - - // Assertions - if tc.expectedError != nil { - s.Require().Error(err) - s.Require().Equal(tc.expectedError.Error(), err.Error()) - s.Require().Equal(osmomath.Dec{}, routeFee) - s.Require().Equal(osmomath.Dec{}, totalFee) - return - } - - s.Require().NoError(err) - s.Require().Equal(tc.expectedRouteFee, routeFee) - s.Require().Equal(tc.expectedTotalFee, totalFee) - }) - } -} - -func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { - tests := map[string]struct { - route []types.SwapAmountOutRoute - tokenOut sdk.Coin - balancerPoolCoins []sdk.Coins - concentratedPoolDenoms [][]string - poolCoins []sdk.Coins - cumulativeRouteSpreadFactor osmomath.Dec - sumOfSpreadFactors osmomath.Dec + poolCoins []sdk.Coins expectedSwapIns []osmomath.Int expectedError bool @@ -3100,42 +2577,6 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { // bar token = 12 * (100 / 88) ~ 14 expectedSwapIns: []osmomath.Int{osmomath.NewInt(14), osmomath.NewInt(12)}, }, - "happy path: one route with swap Fee": { - route: []types.SwapAmountOutRoute{ - { - PoolId: 1, - TokenInDenom: bar, - }, - }, - poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(uosmo, osmomath.NewInt(100)), sdk.NewCoin(bar, osmomath.NewInt(100)))}, - cumulativeRouteSpreadFactor: osmomath.NewDec(100), - sumOfSpreadFactors: osmomath.NewDec(500), - - tokenOut: sdk.NewCoin(uosmo, osmomath.NewInt(10)), - expectedSwapIns: []osmomath.Int{osmomath.NewInt(12)}, - }, - "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, osmomath.NewInt(100)), sdk.NewCoin(bar, osmomath.NewInt(100))), // pool 1. - sdk.NewCoins(sdk.NewCoin(bar, osmomath.NewInt(100)), sdk.NewCoin(uosmo, osmomath.NewInt(100))), // pool 2. - }, - cumulativeRouteSpreadFactor: osmomath.NewDec(100), - sumOfSpreadFactors: osmomath.NewDec(500), - - tokenOut: sdk.NewCoin(uosmo, osmomath.NewInt(10)), - expectedSwapIns: []osmomath.Int{osmomath.NewInt(14), osmomath.NewInt(12)}, - }, "error: Invalid Pool": { route: []types.SwapAmountOutRoute{ { @@ -3176,11 +2617,7 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() { var actualSwapOuts []osmomath.Int var err error - if !tc.sumOfSpreadFactors.IsNil() && !tc.cumulativeRouteSpreadFactor.IsNil() { - actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateOsmoMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut, tc.cumulativeRouteSpreadFactor, tc.sumOfSpreadFactors) - } else { - actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut) - } + actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut) if tc.expectedError { suite.Require().Error(err) } else { From 036f2833671fa4c7bc112bfd21831ba772fb94c6 Mon Sep 17 00:00:00 2001 From: devbot-wizard <141283918+devbot-wizard@users.noreply.github.com> Date: Wed, 20 Sep 2023 19:04:27 +0000 Subject: [PATCH 2/4] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8a4d11cb57..df16c6e1e64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#6427](https://github.com/osmosis-labs/osmosis/pull/6427) sdk.Coins Mul and Quo helpers in osmoutils * [#6437](https://github.com/osmosis-labs/osmosis/pull/6437) mutative version for QuoRoundUp +* [#6468](https://github.com/osmosis-labs/osmosis/pull/6468) chore: remove osmo multihop discount ### Misc Improvements From eeafde4ab6ea0d16c807c738b75985f1daa1ab49 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Wed, 20 Sep 2023 13:25:02 -0600 Subject: [PATCH 3/4] edit comments --- x/poolmanager/README.md | 5 ----- x/poolmanager/router_test.go | 15 ++++++--------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/x/poolmanager/README.md b/x/poolmanager/README.md index cb9007dd1a4..fc9338e3356 100644 --- a/x/poolmanager/README.md +++ b/x/poolmanager/README.md @@ -309,11 +309,6 @@ multiple pools in the process. The most cost-efficient route is determined offline and the list of the pools is provided externally, by user, during the broadcasting of the swapping transaction. At the moment of execution, the provided route may not be the most cost-efficient one anymore. -When a trade consists of just two OSMO-included routes during a single transaction, -the spread factors on each hop would be automatically halved. -Example: for converting `ATOM -> OSMO -> LUNA` using two pools with spread factors `0.3% + 0.2%`, -instead `0.15% + 0.1%` spread factors will be applied. - [Multi-Hop](https://github.com/osmosis-labs/osmosis/blob/f26ceb958adaaf31510e17ed88f5eab47e2bac03/x/poolmanager/router.go#L16) ## Route Splitting diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 5aa743d3476..6ad88dc4913 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -430,7 +430,6 @@ func (s *KeeperTestSuite) TestRouteCalculateSpotPrice() { // That is: // - to the correct module (concentrated-liquidity or gamm) // - over the right routes (hops) -// - fee reduction is applied correctly func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { tests := []struct { name string @@ -670,7 +669,7 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { _, err := poolmanagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], tc.routes, tc.tokenIn, tc.tokenOutMinAmount) s.Require().Error(err) } else { - // calculate the swap as separate swaps with either the reduced swap fee or normal fee + // calculate the swap as separate swaps expectedMultihopTokenOutAmount := s.calcOutGivenInAmountAsSeparatePoolSwaps(tc.routes, tc.tokenIn) // execute the swap @@ -687,7 +686,6 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { // That is: // - to the correct module (concentrated-liquidity or gamm) // - over the right routes (hops) -// - fee reduction is applied correctly func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { tests := []struct { name string @@ -887,7 +885,7 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { _, err := poolmanagerKeeper.RouteExactAmountOut(s.Ctx, s.TestAccs[0], tc.routes, tc.tokenInMaxAmount, tc.tokenOut) s.Require().Error(err) } else { - // calculate the swap as separate swaps with either the reduced swap fee or normal fee + // calculate the swap as separate swaps expectedMultihopTokenInAmount := s.calcInGivenOutAmountAsSeparateSwaps(tc.routes, tc.tokenOut) // execute the swap multihopTokenInAmount, err := poolmanagerKeeper.RouteExactAmountOut(s.Ctx, s.TestAccs[0], tc.routes, tc.tokenInMaxAmount, tc.tokenOut) @@ -1301,7 +1299,7 @@ func (s *KeeperTestSuite) calcInGivenOutAmountAsSeparateSwaps(routes []types.Swa return nextTokenOut } -// calcOutGivenInAmountAsSeparatePoolSwaps calculates the output amount of a series of swaps on PoolManager pools while factoring in reduces swap fee changes. +// calcOutGivenInAmountAsSeparatePoolSwaps calculates the output amount of a series of swaps on PoolManager pools. // 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 (s *KeeperTestSuite) calcOutGivenInAmountAsSeparatePoolSwaps(routes []types.SwapAmountInRoute, tokenIn sdk.Coin) sdk.Coin { @@ -1314,7 +1312,6 @@ func (s *KeeperTestSuite) calcOutGivenInAmountAsSeparatePoolSwaps(routes []types pool, err := swapModule.GetPool(s.Ctx, hop.PoolId) s.Require().NoError(err) - // utilize the routeSpreadFactor, sumOfSpreadFactors, and current pool swap fee to calculate the new reduced swap fee spreadFactor := pool.GetSpreadFactor(cacheCtx) takerFee, err := s.App.PoolManagerKeeper.GetTradingPairTakerFee(cacheCtx, hop.TokenOutDenom, nextTokenIn.Denom) @@ -2085,7 +2082,7 @@ func (s *KeeperTestSuite) TestSplitRouteExactAmountIn() { // Set taker fee for pool/pair k.SetDenomPairTakerFee(s.Ctx, pool.initialLiquidity[0].Denom, pool.initialLiquidity[1].Denom, pool.takerFee) - // Fund sender with initial liqudity + // Fund sender with initial liquidity // If not valid, we don't fund to trigger an error case. if !tc.isInvalidSender { s.FundAcc(sender, pool.initialLiquidity) @@ -2397,7 +2394,7 @@ func (s *KeeperTestSuite) TestSplitRouteExactAmountOut() { // Set taker fee for pool/pair k.SetDenomPairTakerFee(s.Ctx, pool.initialLiquidity[0].Denom, pool.initialLiquidity[1].Denom, pool.takerFee) - // Fund sender with initial liqudity + // Fund sender with initial liquidity // If not valid, we don't fund to trigger an error case. if !tc.isInvalidSender { s.FundAcc(sender, pool.initialLiquidity) @@ -3031,7 +3028,7 @@ func (s *KeeperTestSuite) TestTakerFee() { // the swap we don't expect the price to change significantly. // As a result, we roughly expect the amount out to be the same // as the amount in given in another token. However, the actual - // amount must be stricly less than the given due to price impact. + // amount must be strictly less than the given due to price impact. multiplicativeTolerance := osmomath.OneDec() errTolerance := osmomath.ErrTolerance{ RoundingDir: osmomath.RoundDown, From 7d1c6bf9bd09f9634d2c223f30aca72ceac93524 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Wed, 20 Sep 2023 13:38:51 -0600 Subject: [PATCH 4/4] add sanity check that reduced swap fee is no longer applied --- x/poolmanager/router_test.go | 44 ++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 6ad88dc4913..71ea355945a 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -479,6 +479,28 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountIn() { tokenIn: sdk.NewCoin(foo, osmomath.NewInt(100000)), tokenOutMinAmount: osmomath.NewInt(1), }, + { + name: "Two routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2), both pools 1 percent fee, sanity check no more half fee applied", + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(baz, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. + }, + poolType: []types.PoolType{types.Balancer, types.Balancer}, + poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, + routes: []types.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: uosmo, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + }, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenIn: sdk.NewCoin("foo", osmomath.NewInt(100000)), + tokenOutMinAmount: osmomath.NewInt(1), + }, { name: "Three routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", poolCoins: []sdk.Coins{ @@ -736,6 +758,28 @@ func (s *KeeperTestSuite) TestMultihopSwapExactAmountOut() { tokenInMaxAmount: osmomath.NewInt(90000000), tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), }, + { + name: "Two routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2), both pools 1 percent fee, sanity check no more half fee applied", + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(baz, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2. + }, + poolType: []types.PoolType{types.Balancer, types.Balancer}, + poolSpreadFactor: []osmomath.Dec{defaultPoolSpreadFactor, defaultPoolSpreadFactor}, + routes: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: foo, + }, + { + PoolId: 2, + TokenInDenom: uosmo, + }, + }, + incentivizedGauges: []uint64{1, 2, 3, 4, 5, 6}, + tokenInMaxAmount: osmomath.NewInt(90000000), + tokenOut: sdk.NewCoin(baz, osmomath.NewInt(100000)), + }, { name: "Three routes: Swap - [foo -> uosmo](pool 1) - [uosmo -> baz](pool 2) - [baz -> bar](pool 3), all pools 1 percent fee", poolCoins: []sdk.Coins{