From 9107d1e025ffb82b2b2649f4cd0f499397ab0ef3 Mon Sep 17 00:00:00 2001 From: Roman <roman@osmosis.team> Date: Mon, 24 Oct 2022 18:42:05 -0500 Subject: [PATCH] refactor(CL): move multihop to swap router, wire to concentrated liquidity module (#3128) * feat/refactor(CL): move multihop to swaprouter, connect to concentrated liquidity * comments * godoc for RouteExactAmountOut * link issues * delete multihop_test.go * Update x/swaprouter/router_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update x/swaprouter/router_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update x/concentrated-liquidity/pool.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update x/gamm/keeper/swap.go Co-authored-by: Adam Tucker <adam@osmosis.team> Co-authored-by: Adam Tucker <adam@osmosis.team> --- app/apptesting/test_suite.go | 2 +- app/keepers/keepers.go | 2 +- x/concentrated-liquidity/pool.go | 6 + x/concentrated-liquidity/swaps.go | 22 +- x/gamm/keeper/multihop.go | 142 ------------- x/gamm/keeper/multihop_test.go | 323 ----------------------------- x/gamm/keeper/pool.go | 5 + x/gamm/keeper/pool_service.go | 2 +- x/gamm/keeper/swap.go | 26 ++- x/gamm/keeper/swap_test.go | 20 +- x/swaprouter/keeper_test.go | 11 + x/swaprouter/router.go | 140 ++++++++++++- x/swaprouter/router_test.go | 161 ++++++++++++++ x/swaprouter/types/routes.go | 16 +- x/txfees/keeper/hooks.go | 9 +- x/txfees/keeper/keeper.go | 6 +- x/txfees/types/expected_keepers.go | 14 +- 17 files changed, 386 insertions(+), 521 deletions(-) delete mode 100644 x/gamm/keeper/multihop.go delete mode 100644 x/gamm/keeper/multihop_test.go create mode 100644 x/swaprouter/router_test.go diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index 71446fecf82..66a0d70fce4 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -302,7 +302,7 @@ func (s *KeeperTestHelper) SwapAndSetSpotPrice(poolId uint64, fromAsset sdk.Coin coins := sdk.Coins{sdk.NewInt64Coin(fromAsset.Denom, 100000000000000)} s.FundAcc(acc1, coins) - _, err := s.App.GAMMKeeper.SwapExactAmountOut( + _, err := s.App.GAMMKeeper.SwapExactAmountOutDefaultSwapFee( s.Ctx, acc1, poolId, diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 108ec96cc7f..bfd324ad0c8 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -280,7 +280,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appKeepers.AccountKeeper, appKeepers.BankKeeper, appKeepers.keys[txfeestypes.StoreKey], - appKeepers.GAMMKeeper, + appKeepers.SwapRouterKeeper, appKeepers.GAMMKeeper, txfeestypes.FeeCollectorName, txfeestypes.NonNativeFeeCollectorName, diff --git a/x/concentrated-liquidity/pool.go b/x/concentrated-liquidity/pool.go index b54b4461db4..fb74affb339 100644 --- a/x/concentrated-liquidity/pool.go +++ b/x/concentrated-liquidity/pool.go @@ -1,6 +1,7 @@ package concentrated_liquidity import ( + "errors" fmt "fmt" sdk "github.com/cosmos/cosmos-sdk/types" @@ -30,6 +31,11 @@ func (k Keeper) CreateNewConcentratedLiquidityPool(ctx sdk.Context, poolId uint6 return pool, nil } +// GetPool returns a pool with a given id. +func (k Keeper) GetPool(ctx sdk.Context, poolId uint64) (types.PoolI, error) { + return nil, errors.New("not implemented") +} + func priceToTick(price sdk.Dec) sdk.Int { logOfPrice := osmomath.BigDecFromSDKDec(price).LogBase2() logInt := osmomath.NewDecWithPrec(10001, 4) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 76f82a9e468..26275c47bef 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -3,27 +3,31 @@ package concentrated_liquidity import ( sdk "github.com/cosmos/cosmos-sdk/types" - swaproutertypes "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" + gammtypes "github.com/osmosis-labs/osmosis/v12/x/gamm/types" ) -// TODO: godoc -func (k Keeper) MultihopSwapExactAmountIn( +// TODO: spec here and in gamm +func (k Keeper) SwapExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, - routes []swaproutertypes.SwapAmountInRoute, + pool gammtypes.PoolI, tokenIn sdk.Coin, + tokenOutDenom string, tokenOutMinAmount sdk.Int, + swapFee sdk.Dec, ) (tokenOutAmount sdk.Int, err error) { - return sdk.Int{}, nil + panic("not implemented") } -// TODO: godoc -func (k Keeper) MultihopSwapExactAmountOut( +// TODO: spec here and in gamm +func (k Keeper) SwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, - routes []swaproutertypes.SwapAmountOutRoute, + poolI gammtypes.PoolI, + tokenInDenom string, tokenInMaxAmount sdk.Int, tokenOut sdk.Coin, + swapFee sdk.Dec, ) (tokenInAmount sdk.Int, err error) { - return sdk.Int{}, nil + panic("not implemented") } diff --git a/x/gamm/keeper/multihop.go b/x/gamm/keeper/multihop.go deleted file mode 100644 index 20c2a904403..00000000000 --- a/x/gamm/keeper/multihop.go +++ /dev/null @@ -1,142 +0,0 @@ -package keeper - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/osmosis-labs/osmosis/v12/x/gamm/types" - swaproutertypes "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" -) - -// MultihopSwapExactAmountIn 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. -func (k Keeper) MultihopSwapExactAmountIn( - ctx sdk.Context, - sender sdk.AccAddress, - routes []swaproutertypes.SwapAmountInRoute, - tokenIn sdk.Coin, - tokenOutMinAmount sdk.Int, -) (tokenOutAmount sdk.Int, err error) { - for i, route := range routes { - swapFeeMultiplier := sdk.OneDec() - if swaproutertypes.SwapAmountInRoutes(routes).IsOsmoRoutedMultihop() { - swapFeeMultiplier = types.MultihopSwapFeeMultiplierForOsmoPools.Clone() - } - - // 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 { - _outMinAmount = tokenOutMinAmount - } - - // Execute the expected swap on the current routed pool - pool, poolErr := k.getPoolForSwap(ctx, route.PoolId) - if poolErr != nil { - return sdk.Int{}, poolErr - } - - swapFee := pool.GetSwapFee(ctx).Mul(swapFeeMultiplier) - tokenOutAmount, err = k.swapExactAmountIn(ctx, sender, pool, tokenIn, route.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) - } - return tokenOutAmount, err -} - -// MultihopSwapExactAmountOut defines the output denom and output amount for the last pool. -// Calculation starts by providing the tokenOutAmount of the final pool to calculate the required tokenInAmount -// the calculated tokenInAmount is used as defined tokenOutAmount of the previous pool, calculating in reverse order of the swap -// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined tokenInMaxAmount defined. -func (k Keeper) MultihopSwapExactAmountOut( - ctx sdk.Context, - sender sdk.AccAddress, - routes []swaproutertypes.SwapAmountOutRoute, - tokenInMaxAmount sdk.Int, - tokenOut sdk.Coin, -) (tokenInAmount sdk.Int, err error) { - swapFeeMultiplier := sdk.OneDec() - - if swaproutertypes.SwapAmountOutRoutes(routes).IsOsmoRoutedMultihop() { - swapFeeMultiplier = types.MultihopSwapFeeMultiplierForOsmoPools.Clone() - } - - // Determine what the estimated input would be for each pool along the multihop route - insExpected, err := k.createMultihopExpectedSwapOuts(ctx, routes, tokenOut, swapFeeMultiplier) - if err != nil { - return sdk.Int{}, err - } - if len(insExpected) == 0 { - return sdk.Int{}, nil - } - - insExpected[0] = tokenInMaxAmount - - // Iterates through each routed pool and executes their respective swaps. Note that all of the work to get the return - // 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 { - _tokenOut := tokenOut - - // If there is one pool left in the route, 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]) - } - - // Execute the expected swap on the current routed pool - pool, poolErr := k.getPoolForSwap(ctx, route.PoolId) - if poolErr != nil { - return sdk.Int{}, poolErr - } - swapFee := pool.GetSwapFee(ctx).Mul(swapFeeMultiplier) - _tokenInAmount, swapErr := k.swapExactAmountOut(ctx, sender, pool, route.TokenInDenom, insExpected[i], _tokenOut, swapFee) - if swapErr != nil { - return sdk.Int{}, swapErr - } - - // Sets the final amount of tokens that need to be input into the first pool. Even though this is the final return value for the - // whole method and will not change after the first iteration, we still iterate through the rest of the pools to execute their respective - // swaps. - if i == 0 { - tokenInAmount = _tokenInAmount - } - } - - return tokenInAmount, nil -} - -// 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 -// 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. -func (k Keeper) createMultihopExpectedSwapOuts( - ctx sdk.Context, - routes []swaproutertypes.SwapAmountOutRoute, - tokenOut sdk.Coin, swapFeeMultiplier sdk.Dec, -) ([]sdk.Int, error) { - insExpected := make([]sdk.Int, len(routes)) - for i := len(routes) - 1; i >= 0; i-- { - route := routes[i] - - pool, err := k.getPoolForSwap(ctx, route.PoolId) - if err != nil { - return nil, err - } - - tokenIn, err := pool.CalcInAmtGivenOut(ctx, sdk.NewCoins(tokenOut), route.TokenInDenom, pool.GetSwapFee(ctx).Mul(swapFeeMultiplier)) - if err != nil { - return nil, err - } - - insExpected[i] = tokenIn.Amount - tokenOut = tokenIn - } - - return insExpected, nil -} diff --git a/x/gamm/keeper/multihop_test.go b/x/gamm/keeper/multihop_test.go deleted file mode 100644 index f951ff1ef9e..00000000000 --- a/x/gamm/keeper/multihop_test.go +++ /dev/null @@ -1,323 +0,0 @@ -package keeper_test - -import ( - "errors" - - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/osmosis-labs/osmosis/v12/x/gamm/pool-models/balancer" - swaproutertypes "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" -) - -func (suite *KeeperTestSuite) TestBalancerPoolSimpleMultihopSwapExactAmountIn() { - type param struct { - routes []swaproutertypes.SwapAmountInRoute - tokenIn sdk.Coin - tokenOutMinAmount sdk.Int - } - - tests := []struct { - name string - param param - expectPass bool - reducedFeeApplied bool - }{ - { - name: "Proper swap - foo -> bar(pool 1) - bar(pool 2) -> baz", - param: param{ - routes: []swaproutertypes.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: "bar", - }, - { - PoolId: 2, - TokenOutDenom: "baz", - }, - }, - tokenIn: sdk.NewCoin("foo", sdk.NewInt(100000)), - tokenOutMinAmount: sdk.NewInt(1), - }, - expectPass: true, - }, - { - name: "Swap - foo -> uosmo(pool 1) - uosmo(pool 2) -> baz with a half fee applied", - param: param{ - routes: []swaproutertypes.SwapAmountInRoute{ - { - PoolId: 1, - TokenOutDenom: "uosmo", - }, - { - PoolId: 2, - TokenOutDenom: "baz", - }, - }, - tokenIn: sdk.NewCoin("foo", sdk.NewInt(100000)), - tokenOutMinAmount: sdk.NewInt(1), - }, - reducedFeeApplied: true, - expectPass: true, - }, - } - - for _, test := range tests { - // Init suite for each test. - suite.SetupTest() - - suite.Run(test.name, func() { - keeper := suite.App.GAMMKeeper - poolDefaultSwapFee := sdk.NewDecWithPrec(1, 2) // 1% - poolZeroSwapFee := sdk.ZeroDec() - - // Prepare pools - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - - // Calculate the chained spot price. - calcSpotPrice := func() sdk.Dec { - dec := sdk.NewDec(1) - tokenInDenom := test.param.tokenIn.Denom - for i, route := range test.param.routes { - if i != 0 { - tokenInDenom = test.param.routes[i-1].TokenOutDenom - } - pool, err := keeper.GetPoolAndPoke(suite.Ctx, route.PoolId) - suite.NoError(err, "test: %v", test.name) - - sp, err := pool.SpotPrice(suite.Ctx, tokenInDenom, route.TokenOutDenom) - suite.NoError(err, "test: %v", test.name) - dec = dec.Mul(sp) - } - return dec - } - - calcOutAmountAsSeparateSwaps := func(adjustedPoolSwapFee sdk.Dec) sdk.Coin { - cacheCtx, _ := suite.Ctx.CacheContext() - - if adjustedPoolSwapFee != poolDefaultSwapFee { - for _, hop := range test.param.routes { - err := suite.updatePoolSwapFee(cacheCtx, hop.PoolId, adjustedPoolSwapFee) - suite.NoError(err, "test: %v", test.name) - } - } - - nextTokenIn := test.param.tokenIn - for _, hop := range test.param.routes { - tokenOut, err := keeper.SwapExactAmountIn(cacheCtx, suite.TestAccs[0], hop.PoolId, nextTokenIn, hop.TokenOutDenom, sdk.OneInt()) - suite.Require().NoError(err) - nextTokenIn = sdk.NewCoin(hop.TokenOutDenom, tokenOut) - } - return nextTokenIn - } - - if test.expectPass { - tokenOutCalculatedAsSeparateSwaps := calcOutAmountAsSeparateSwaps(poolDefaultSwapFee) - tokenOutCalculatedAsSeparateSwapsWithoutFee := calcOutAmountAsSeparateSwaps(poolZeroSwapFee) - - spotPriceBefore := calcSpotPrice() - - multihopTokenOutAmount, err := keeper.MultihopSwapExactAmountIn(suite.Ctx, suite.TestAccs[0], test.param.routes, test.param.tokenIn, test.param.tokenOutMinAmount) - suite.NoError(err, "test: %v", test.name) - - spotPriceAfter := calcSpotPrice() - - // Ratio of the token out should be between the before spot price and after spot price. - sp := test.param.tokenIn.Amount.ToDec().Quo(multihopTokenOutAmount.ToDec()) - suite.True(sp.GT(spotPriceBefore) && sp.LT(spotPriceAfter), "test: %v", test.name) - - if test.reducedFeeApplied { - // we have 3 values: - // ---------------- tokenOutCalculatedAsSeparateSwapsWithoutFee - // diffA - // ---------------- multihopTokenOutAmount (with half fees) - // diffB - // ---------------- tokenOutCalculatedAsSeparateSwaps (with full fees) - // here we want to test, that difference between this 3 values around 50% (fee is actually halved) - // we do not have exact 50% reduce due to amm math rounding - // playing with input values for this test can result in different discount % - // so lets check, that we have around 50% +-1% reduction - diffA := tokenOutCalculatedAsSeparateSwapsWithoutFee.Amount.Sub(multihopTokenOutAmount) - diffB := multihopTokenOutAmount.Sub(tokenOutCalculatedAsSeparateSwaps.Amount) - diffDistinctionPercent := diffA.Sub(diffB).Abs().ToDec().Quo(diffA.Add(diffB).ToDec()) - suite.Require().True(diffDistinctionPercent.LT(sdk.MustNewDecFromStr("0.01"))) - } else { - suite.Require().True(multihopTokenOutAmount.Equal(tokenOutCalculatedAsSeparateSwaps.Amount)) - } - } else { - _, err := keeper.MultihopSwapExactAmountIn(suite.Ctx, suite.TestAccs[0], test.param.routes, test.param.tokenIn, test.param.tokenOutMinAmount) - suite.Error(err, "test: %v", test.name) - } - }) - } -} - -func (suite *KeeperTestSuite) TestBalancerPoolSimpleMultihopSwapExactAmountOut() { - type param struct { - routes []swaproutertypes.SwapAmountOutRoute - tokenInMaxAmount sdk.Int - tokenOut sdk.Coin - } - - tests := []struct { - name string - param param - expectPass bool - reducedFeeApplied bool - }{ - { - name: "Proper swap: foo -> bar (pool 1), bar -> baz (pool 2)", - param: param{ - routes: []swaproutertypes.SwapAmountOutRoute{ - { - PoolId: 1, - TokenInDenom: "foo", - }, - { - PoolId: 2, - TokenInDenom: "bar", - }, - }, - tokenInMaxAmount: sdk.NewInt(90000000), - tokenOut: sdk.NewCoin("baz", sdk.NewInt(100000)), - }, - expectPass: true, - }, - { - name: "Swap - foo -> uosmo(pool 1) - uosmo(pool 2) -> baz with a half fee applied", - param: param{ - routes: []swaproutertypes.SwapAmountOutRoute{ - { - PoolId: 1, - TokenInDenom: "foo", - }, - { - PoolId: 2, - TokenInDenom: "uosmo", - }, - }, - tokenInMaxAmount: sdk.NewInt(90000000), - tokenOut: sdk.NewCoin("baz", sdk.NewInt(100000)), - }, - expectPass: true, - reducedFeeApplied: true, - }, - } - - for _, test := range tests { - // Init suite for each test. - suite.SetupTest() - - suite.Run(test.name, func() { - keeper := suite.App.GAMMKeeper - poolDefaultSwapFee := sdk.NewDecWithPrec(1, 2) // 1% - poolZeroSwapFee := sdk.ZeroDec() - - // Prepare pools - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, // 1% - ExitFee: sdk.NewDec(0), - }) - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - - // Calculate the chained spot price. - calcSpotPrice := func() sdk.Dec { - dec := sdk.NewDec(1) - for i, route := range test.param.routes { - tokenOutDenom := test.param.tokenOut.Denom - if i != len(test.param.routes)-1 { - tokenOutDenom = test.param.routes[i+1].TokenInDenom - } - - pool, err := keeper.GetPoolAndPoke(suite.Ctx, route.PoolId) - suite.NoError(err, "test: %v", test.name) - - sp, err := pool.SpotPrice(suite.Ctx, route.TokenInDenom, tokenOutDenom) - suite.NoError(err, "test: %v", test.name) - dec = dec.Mul(sp) - } - return dec - } - - calcInAmountAsSeparateSwaps := func(adjustedPoolSwapFee sdk.Dec) sdk.Coin { - cacheCtx, _ := suite.Ctx.CacheContext() - - if adjustedPoolSwapFee != poolDefaultSwapFee { - for _, hop := range test.param.routes { - err := suite.updatePoolSwapFee(cacheCtx, hop.PoolId, adjustedPoolSwapFee) - suite.NoError(err, "test: %v", test.name) - } - } - - nextTokenOut := test.param.tokenOut - for i := len(test.param.routes) - 1; i >= 0; i-- { - hop := test.param.routes[i] - tokenOut, err := keeper.SwapExactAmountOut(cacheCtx, suite.TestAccs[0], hop.PoolId, hop.TokenInDenom, sdk.NewInt(100000000), nextTokenOut) - suite.Require().NoError(err) - nextTokenOut = sdk.NewCoin(hop.TokenInDenom, tokenOut) - } - return nextTokenOut - } - - if test.expectPass { - tokenInCalculatedAsSeparateSwaps := calcInAmountAsSeparateSwaps(poolDefaultSwapFee) - tokenInCalculatedAsSeparateSwapsWithoutFee := calcInAmountAsSeparateSwaps(poolZeroSwapFee) - - spotPriceBefore := calcSpotPrice() - multihopTokenInAmount, err := keeper.MultihopSwapExactAmountOut(suite.Ctx, suite.TestAccs[0], test.param.routes, test.param.tokenInMaxAmount, test.param.tokenOut) - suite.Require().NoError(err, "test: %v", test.name) - - spotPriceAfter := calcSpotPrice() - // Ratio of the token out should be between the before spot price and after spot price. - // This is because the swap increases the spot price - sp := multihopTokenInAmount.ToDec().Quo(test.param.tokenOut.Amount.ToDec()) - suite.True(sp.GT(spotPriceBefore) && sp.LT(spotPriceAfter), "multi-hop spot price wrong, test: %v", test.name) - - if test.reducedFeeApplied { - // we have 3 values: - // ---------------- tokenInCalculatedAsSeparateSwaps (with full fees) - // diffA - // ---------------- multihopTokenInAmount (with half fees) - // diffB - // ---------------- tokenInCalculatedAsSeparateSwapsWithoutFee - // here we want to test, that difference between this 3 values around 50% (fee is actually halved) - // we do not have exact 50% reduce due to amm math rounding - // playing with input values for this test can result in different discount % - // so lets check, that we have around 50% +-1% reduction - diffA := tokenInCalculatedAsSeparateSwaps.Amount.Sub(multihopTokenInAmount) - diffB := multihopTokenInAmount.Sub(tokenInCalculatedAsSeparateSwapsWithoutFee.Amount) - diffDistinctionPercent := diffA.Sub(diffB).Abs().ToDec().Quo(diffA.Add(diffB).ToDec()) - suite.Require().True(diffDistinctionPercent.LT(sdk.MustNewDecFromStr("0.01"))) - } else { - suite.Require().True(multihopTokenInAmount.Equal(tokenInCalculatedAsSeparateSwaps.Amount)) - } - } else { - _, err := keeper.MultihopSwapExactAmountOut(suite.Ctx, suite.TestAccs[0], test.param.routes, test.param.tokenInMaxAmount, test.param.tokenOut) - suite.Error(err, "test: %v", test.name) - } - }) - } -} - -func (s *KeeperTestSuite) updatePoolSwapFee(ctx sdk.Context, poolId uint64, adjustedPoolSwapFee sdk.Dec) error { - pool, err := s.App.GAMMKeeper.GetPoolAndPoke(ctx, poolId) - if err != nil { - return err - } - - balancerPool, ok := pool.(*balancer.Pool) - if !ok { - return errors.New("can't update swap fee on non-balancer pool") - } - balancerPool.PoolParams.SwapFee = adjustedPoolSwapFee - return s.App.GAMMKeeper.SetPool(ctx, pool) -} diff --git a/x/gamm/keeper/pool.go b/x/gamm/keeper/pool.go index 7a7a4c38f0d..9e8cb1b8cf2 100644 --- a/x/gamm/keeper/pool.go +++ b/x/gamm/keeper/pool.go @@ -22,6 +22,11 @@ func (k Keeper) UnmarshalPool(bz []byte) (types.TraditionalAmmInterface, error) return acc, k.cdc.UnmarshalInterface(bz, &acc) } +// GetPool returns a pool with a given id. +func (k Keeper) GetPool(ctx sdk.Context, poolId uint64) (types.PoolI, error) { + return k.getPoolForSwap(ctx, poolId) +} + // GetPoolAndPoke returns a PoolI based on it's identifier if one exists. If poolId corresponds // to a pool with weights (e.g. balancer), the weights of the pool are updated via PokePool prior to returning. // TODO: Consider rename to GetPool due to downstream API confusion. diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 1c0d24ce00d..2157373c466 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -403,7 +403,7 @@ func (k Keeper) ExitSwapShareAmountIn( if coin.Denom == tokenOutDenom { continue } - swapOut, err := k.SwapExactAmountIn(ctx, sender, poolId, coin, tokenOutDenom, sdk.ZeroInt()) + swapOut, err := k.SwapExactAmountInDefaultSwapFee(ctx, sender, poolId, coin, tokenOutDenom, sdk.ZeroInt()) if err != nil { return sdk.Int{}, err } diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index 7d7b94c3f27..f0d658db05a 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -10,12 +10,14 @@ import ( "github.com/osmosis-labs/osmosis/v12/x/gamm/types" ) -// SwapExactAmountIn attempts to swap one asset, tokenIn, for another asset +// SwapExactAmountInDefaultSwapFee attempts to swap one asset, tokenIn, for another asset // denominated via tokenOutDenom through a pool denoted by poolId specifying that // tokenOutMinAmount must be returned in the resulting asset returning an error // upon failure. Upon success, the resulting tokens swapped for are returned. A // swap fee is applied determined by the pool's parameters. -func (k Keeper) SwapExactAmountIn( +// TODO: remove this method: +// https://github.com/osmosis-labs/osmosis/issues/3129 +func (k Keeper) SwapExactAmountInDefaultSwapFee( ctx sdk.Context, sender sdk.AccAddress, poolId uint64, @@ -29,14 +31,16 @@ func (k Keeper) SwapExactAmountIn( } swapFee := pool.GetSwapFee(ctx) - return k.swapExactAmountIn(ctx, sender, pool, tokenIn, tokenOutDenom, tokenOutMinAmount, swapFee) + return k.SwapExactAmountIn(ctx, sender, pool, tokenIn, tokenOutDenom, tokenOutMinAmount, swapFee) } // swapExactAmountIn is an internal method for swapping an exact amount of tokens // as input to a pool, using the provided swapFee. This is intended to allow // different swap fees as determined by multi-hops, or when recovering from // chain liveness failures. -func (k Keeper) swapExactAmountIn( +// TODO: investigate if swapFee can be unexported +// https://github.com/osmosis-labs/osmosis/issues/3130 +func (k Keeper) SwapExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, pool types.PoolI, @@ -76,7 +80,7 @@ func (k Keeper) swapExactAmountIn( return tokenOutAmount, nil } -func (k Keeper) SwapExactAmountOut( +func (k Keeper) SwapExactAmountOutDefaultSwapFee( ctx sdk.Context, sender sdk.AccAddress, poolId uint64, @@ -89,17 +93,17 @@ func (k Keeper) SwapExactAmountOut( return sdk.Int{}, err } swapFee := pool.GetSwapFee(ctx) - return k.swapExactAmountOut(ctx, sender, pool, tokenInDenom, tokenInMaxAmount, tokenOut, swapFee) + return k.SwapExactAmountOut(ctx, sender, pool, tokenInDenom, tokenInMaxAmount, tokenOut, swapFee) } -// swapExactAmountIn is an internal method for swapping to get an exact number of tokens out of a pool, +// swapExactAmountIn is a method for swapping to get an exact number of tokens out of a pool, // using the provided swapFee. // This is intended to allow different swap fees as determined by multi-hops, // or when recovering from chain liveness failures. -func (k Keeper) swapExactAmountOut( +func (k Keeper) SwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, - pool types.TraditionalAmmInterface, + poolI types.PoolI, tokenInDenom string, tokenInMaxAmount sdk.Int, tokenOut sdk.Coin, @@ -108,6 +112,10 @@ func (k Keeper) swapExactAmountOut( if tokenInDenom == tokenOut.Denom { return sdk.Int{}, errors.New("cannot trade same denomination in and out") } + pool, ok := poolI.(types.TraditionalAmmInterface) + if !ok { + return sdk.Int{}, errors.New("given pool does not implement TraditionalAmmInterface") + } poolOutBal := pool.GetTotalPoolLiquidity(ctx).AmountOf(tokenOut.Denom) if tokenOut.Amount.GTE(poolOutBal) { diff --git a/x/gamm/keeper/swap_test.go b/x/gamm/keeper/swap_test.go index b924d098ced..d0db98164aa 100644 --- a/x/gamm/keeper/swap_test.go +++ b/x/gamm/keeper/swap_test.go @@ -98,7 +98,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() { suite.NoError(err, "test: %v", test.name) prevGasConsumed := suite.Ctx.GasMeter().GasConsumed() - tokenOutAmount, err := keeper.SwapExactAmountIn(ctx, suite.TestAccs[0], poolId, test.param.tokenIn, test.param.tokenOutDenom, test.param.tokenOutMinAmount) + tokenOutAmount, err := keeper.SwapExactAmountInDefaultSwapFee(ctx, suite.TestAccs[0], poolId, test.param.tokenIn, test.param.tokenOutDenom, test.param.tokenOutMinAmount) suite.NoError(err, "test: %v", test.name) suite.True(tokenOutAmount.Equal(test.param.expectedTokenOut), "test: %v", test.name) gasConsumedForSwap := suite.Ctx.GasMeter().GasConsumed() - prevGasConsumed @@ -114,7 +114,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() { tradeAvgPrice := test.param.tokenIn.Amount.ToDec().Quo(tokenOutAmount.ToDec()) suite.True(tradeAvgPrice.GT(spotPriceBefore) && tradeAvgPrice.LT(spotPriceAfter), "test: %v", test.name) } else { - _, err := keeper.SwapExactAmountIn(ctx, suite.TestAccs[0], poolId, test.param.tokenIn, test.param.tokenOutDenom, test.param.tokenOutMinAmount) + _, err := keeper.SwapExactAmountInDefaultSwapFee(ctx, suite.TestAccs[0], poolId, test.param.tokenIn, test.param.tokenOutDenom, test.param.tokenOutMinAmount) suite.Error(err, "test: %v", test.name) } }) @@ -206,7 +206,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountOut() { suite.NoError(err, "test: %v", test.name) prevGasConsumed := suite.Ctx.GasMeter().GasConsumed() - tokenInAmount, err := keeper.SwapExactAmountOut(ctx, suite.TestAccs[0], poolId, test.param.tokenInDenom, test.param.tokenInMaxAmount, test.param.tokenOut) + tokenInAmount, err := keeper.SwapExactAmountOutDefaultSwapFee(ctx, suite.TestAccs[0], poolId, test.param.tokenInDenom, test.param.tokenInMaxAmount, test.param.tokenOut) suite.NoError(err, "test: %v", test.name) suite.True(tokenInAmount.Equal(test.param.expectedTokenInAmount), "test: %v\n expect_eq actual: %s, expected: %s", @@ -224,7 +224,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountOut() { tradeAvgPrice := tokenInAmount.ToDec().Quo(test.param.tokenOut.Amount.ToDec()) suite.True(tradeAvgPrice.GT(spotPriceBefore) && tradeAvgPrice.LT(spotPriceAfter), "test: %v", test.name) } else { - _, err := keeper.SwapExactAmountOut(suite.Ctx, suite.TestAccs[0], poolId, test.param.tokenInDenom, test.param.tokenInMaxAmount, test.param.tokenOut) + _, err := keeper.SwapExactAmountOutDefaultSwapFee(suite.Ctx, suite.TestAccs[0], poolId, test.param.tokenInDenom, test.param.tokenInMaxAmount, test.param.tokenOut) suite.Error(err, "test: %v", test.name) } }) @@ -259,14 +259,14 @@ func (suite *KeeperTestSuite) TestActiveBalancerPoolSwap() { foocoin := sdk.NewCoin("foo", sdk.NewInt(10)) if tc.expectPass { - _, err := suite.App.GAMMKeeper.SwapExactAmountIn(suite.Ctx, suite.TestAccs[0], poolId, foocoin, "bar", sdk.ZeroInt()) + _, err := suite.App.GAMMKeeper.SwapExactAmountInDefaultSwapFee(suite.Ctx, suite.TestAccs[0], poolId, foocoin, "bar", sdk.ZeroInt()) suite.Require().NoError(err) - _, err = suite.App.GAMMKeeper.SwapExactAmountOut(suite.Ctx, suite.TestAccs[0], poolId, "bar", sdk.NewInt(1000000000000000000), foocoin) + _, err = suite.App.GAMMKeeper.SwapExactAmountOutDefaultSwapFee(suite.Ctx, suite.TestAccs[0], poolId, "bar", sdk.NewInt(1000000000000000000), foocoin) suite.Require().NoError(err) } else { - _, err := suite.App.GAMMKeeper.SwapExactAmountIn(suite.Ctx, suite.TestAccs[0], poolId, foocoin, "bar", sdk.ZeroInt()) + _, err := suite.App.GAMMKeeper.SwapExactAmountInDefaultSwapFee(suite.Ctx, suite.TestAccs[0], poolId, foocoin, "bar", sdk.ZeroInt()) suite.Require().Error(err) - _, err = suite.App.GAMMKeeper.SwapExactAmountOut(suite.Ctx, suite.TestAccs[0], poolId, "bar", sdk.NewInt(1000000000000000000), foocoin) + _, err = suite.App.GAMMKeeper.SwapExactAmountOutDefaultSwapFee(suite.Ctx, suite.TestAccs[0], poolId, "bar", sdk.NewInt(1000000000000000000), foocoin) suite.Require().Error(err) } } @@ -313,8 +313,8 @@ func (suite *KeeperTestSuite) TestInactivePoolFreezeSwaps() { for _, test := range testCases { suite.Run(test.name, func() { // Check swaps - _, swapInErr := gammKeeper.SwapExactAmountIn(suite.Ctx, suite.TestAccs[0], test.poolId, testCoin, "bar", sdk.ZeroInt()) - _, swapOutErr := gammKeeper.SwapExactAmountOut(suite.Ctx, suite.TestAccs[0], test.poolId, "bar", sdk.NewInt(1000000000000000000), testCoin) + _, swapInErr := gammKeeper.SwapExactAmountInDefaultSwapFee(suite.Ctx, suite.TestAccs[0], test.poolId, testCoin, "bar", sdk.ZeroInt()) + _, swapOutErr := gammKeeper.SwapExactAmountOutDefaultSwapFee(suite.Ctx, suite.TestAccs[0], test.poolId, "bar", sdk.NewInt(1000000000000000000), testCoin) if test.expectPass { suite.Require().NoError(swapInErr) suite.Require().NoError(swapOutErr) diff --git a/x/swaprouter/keeper_test.go b/x/swaprouter/keeper_test.go index 489fe7e3c3f..bff43a57e0d 100644 --- a/x/swaprouter/keeper_test.go +++ b/x/swaprouter/keeper_test.go @@ -3,6 +3,7 @@ package swaprouter_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" "github.com/osmosis-labs/osmosis/v12/app/apptesting" @@ -24,3 +25,13 @@ func (suite *KeeperTestSuite) SetupTest() { suite.queryClient = types.NewQueryClient(suite.QueryHelper) } + +// CreateBalancerPoolsFromCoins creates balancer pools from given sets of coins. +// 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.PrepareBalancerPoolWithCoins(curPoolCoins...) + } +} diff --git a/x/swaprouter/router.go b/x/swaprouter/router.go index e0273a5c181..9e421063ea4 100644 --- a/x/swaprouter/router.go +++ b/x/swaprouter/router.go @@ -3,36 +3,158 @@ package swaprouter import ( sdk "github.com/cosmos/cosmos-sdk/types" + gammtypes "github.com/osmosis-labs/osmosis/v12/x/gamm/types" "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" ) -// TODO: spec and tests +// 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. func (k Keeper) RouteExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, routes []types.SwapAmountInRoute, tokenIn sdk.Coin, tokenOutMinAmount sdk.Int) (tokenOutAmount sdk.Int, err error) { + // TODO: fix this once proper pool id routing exists + // https: //github.com/osmosis-labs/osmosis/issues/3097 isGamm := true - if isGamm { - return k.gammKeeper.MultihopSwapExactAmountIn(ctx, sender, routes, tokenIn, tokenOutMinAmount) - } + swapModule := k.withSwapModule(isGamm) + + for i, route := range routes { + swapFeeMultiplier := sdk.OneDec() + if types.SwapAmountInRoutes(routes).IsOsmoRoutedMultihop() { + swapFeeMultiplier = gammtypes.MultihopSwapFeeMultiplierForOsmoPools.Clone() + } + + // 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 { + _outMinAmount = tokenOutMinAmount + } - return k.concentratedKeeper.MultihopSwapExactAmountIn(ctx, sender, routes, tokenIn, tokenOutMinAmount) + // Execute the expected swap on the current routed pool + pool, poolErr := swapModule.GetPool(ctx, route.PoolId) + if poolErr != nil { + return sdk.Int{}, poolErr + } + + swapFee := pool.GetSwapFee(ctx).Mul(swapFeeMultiplier) + tokenOutAmount, err = swapModule.SwapExactAmountIn(ctx, sender, pool, tokenIn, route.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) + } + return tokenOutAmount, err } -// TODO: spec and tests +// RouteExactAmountOut defines the output denom and output amount for the last pool. +// Calculation starts by providing the tokenOutAmount of the final pool to calculate the required tokenInAmount +// the calculated tokenInAmount is used as defined tokenOutAmount of the previous pool, calculating in reverse order of the swap +// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined tokenInMaxAmount defined. func (k Keeper) RouteExactAmountOut(ctx sdk.Context, sender sdk.AccAddress, routes []types.SwapAmountOutRoute, tokenInMaxAmount sdk.Int, tokenOut sdk.Coin) (tokenInAmount sdk.Int, err error) { + // TODO: fix this once proper pool id routing exists + // https://github.com/osmosis-labs/osmosis/issues/3097 isGamm := true - if isGamm { - return k.gammKeeper.MultihopSwapExactAmountOut(ctx, sender, routes, tokenInMaxAmount, tokenOut) + swapModule := k.withSwapModule(isGamm) + + swapFeeMultiplier := sdk.OneDec() + + if types.SwapAmountOutRoutes(routes).IsOsmoRoutedMultihop() { + swapFeeMultiplier = gammtypes.MultihopSwapFeeMultiplierForOsmoPools.Clone() + } + + // Determine what the estimated input would be for each pool along the multihop route + insExpected, err := createMultihopExpectedSwapOuts(ctx, swapModule, routes, tokenOut, swapFeeMultiplier) + if err != nil { + return sdk.Int{}, err + } + if len(insExpected) == 0 { + return sdk.Int{}, nil + } + + insExpected[0] = tokenInMaxAmount + + // Iterates through each routed pool and executes their respective swaps. Note that all of the work to get the return + // 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 { + _tokenOut := tokenOut + + // If there is one pool left in the route, 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]) + } + + // Execute the expected swap on the current routed pool + pool, poolErr := swapModule.GetPool(ctx, route.PoolId) + if poolErr != nil { + return sdk.Int{}, poolErr + } + swapFee := pool.GetSwapFee(ctx).Mul(swapFeeMultiplier) + _tokenInAmount, swapErr := swapModule.SwapExactAmountOut(ctx, sender, pool, route.TokenInDenom, insExpected[i], _tokenOut, swapFee) + if swapErr != nil { + return sdk.Int{}, swapErr + } + + // Sets the final amount of tokens that need to be input into the first pool. Even though this is the final return value for the + // whole method and will not change after the first iteration, we still iterate through the rest of the pools to execute their respective + // swaps. + if i == 0 { + tokenInAmount = _tokenInAmount + } + } + + return tokenInAmount, nil +} + +// 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 +// 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. +// TODO: test this. +func createMultihopExpectedSwapOuts( + ctx sdk.Context, + swapModule types.SwapI, + routes []types.SwapAmountOutRoute, + tokenOut sdk.Coin, swapFeeMultiplier sdk.Dec, +) ([]sdk.Int, error) { + insExpected := make([]sdk.Int, len(routes)) + for i := len(routes) - 1; i >= 0; i-- { + route := routes[i] + + pool, err := swapModule.GetPool(ctx, route.PoolId) + if err != nil { + return nil, err + } + + tokenIn, err := pool.CalcInAmtGivenOut(ctx, sdk.NewCoins(tokenOut), route.TokenInDenom, pool.GetSwapFee(ctx).Mul(swapFeeMultiplier)) + if err != nil { + return nil, err + } + + insExpected[i] = tokenIn.Amount + tokenOut = tokenIn } - return k.concentratedKeeper.MultihopSwapExactAmountOut(ctx, sender, routes, tokenInMaxAmount, tokenOut) + return insExpected, nil +} + +func (k Keeper) withSwapModule(isGamm bool) types.SwapI { + if isGamm { + return k.gammKeeper + } + return k.concentratedKeeper } diff --git a/x/swaprouter/router_test.go b/x/swaprouter/router_test.go new file mode 100644 index 00000000000..1222dc6aeb3 --- /dev/null +++ b/x/swaprouter/router_test.go @@ -0,0 +1,161 @@ +package swaprouter_test + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + swaproutertypes "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" +) + +const ( + denomA = "denomA" + denomB = "denomB" + denomC = "denomC" +) + +var ( + defaultInitPoolAmount = sdk.NewInt(1000000000000) + defaultSwapAmount = sdk.NewInt(1000000) +) + +// TestMultihopSwapExactAmountIn tests that the swaps are routed correctly. +// That is: +// - to the correct module (concentrated-liquidity or gamm) +// - over the right routes (hops) +// This test does not actually validate the math behind the swaps. +func (suite *KeeperTestSuite) TestMultihopSwapExactAmountIn() { + type param struct { + } + + tests := []struct { + name string + poolCoins []sdk.Coins + routes []swaproutertypes.SwapAmountInRoute + tokenIn sdk.Coin + tokenOutMinAmount sdk.Int + swapFee sdk.Dec + expectError bool + reducedFeeApplied bool + }{ + { + name: "x/gamm - single route", + poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(denomA, defaultInitPoolAmount), sdk.NewCoin(denomB, defaultInitPoolAmount))}, + routes: []swaproutertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: denomB, + }, + }, + tokenIn: sdk.NewCoin(denomA, sdk.NewInt(100000)), + tokenOutMinAmount: sdk.NewInt(1), + }, + { + name: "x/gamm - two routes", + poolCoins: []sdk.Coins{ + sdk.NewCoins(sdk.NewCoin(denomA, defaultInitPoolAmount), sdk.NewCoin(denomB, defaultInitPoolAmount)), // pool 1. + sdk.NewCoins(sdk.NewCoin(denomB, defaultInitPoolAmount), sdk.NewCoin(denomC, defaultInitPoolAmount)), // pool 2. + }, + routes: []swaproutertypes.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: denomB, + }, + { + PoolId: 2, + TokenOutDenom: denomC, + }, + }, + tokenIn: sdk.NewCoin(denomA, sdk.NewInt(100000)), + tokenOutMinAmount: sdk.NewInt(1), + }, + // TODO: + // tests for concentrated liquidity + // test for multi-hop + // test for multi hop (osmo routes) -> reduces fee, add an assertion + // change values in and out to be different with each swap module type + // create more precise assertions on what the results are + // edge cases: + // * invalid route length + // * pool does not exist + // * swap errors + } + + for _, tc := range tests { + suite.Run(tc.name, func() { + suite.SetupTest() + + swaprouterKeeper := suite.App.SwapRouterKeeper + + suite.CreateBalancerPoolsFromCoins(tc.poolCoins) + + tokenOut, err := swaprouterKeeper.RouteExactAmountIn(suite.Ctx, suite.TestAccs[0], tc.routes, tc.tokenIn, tc.tokenOutMinAmount) + + if tc.expectError { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + suite.Require().True(tokenOut.GTE(tc.tokenOutMinAmount)) + } + }) + } +} + +// TestMultihopSwapExactAmountOut tests that the swaps are routed correctly. +// That is: +// - to the correct module (concentrated-liquidity or gamm) +// - over the right routes (hops) +// This test does not actually validate the math behind the swaps. +func (suite *KeeperTestSuite) TestMultihopSwapExactAmountOut() { + + tests := []struct { + name string + poolCoins []sdk.Coins + routes []swaproutertypes.SwapAmountOutRoute + tokenOut sdk.Coin + tokenInMaxAmount sdk.Int + swapFee sdk.Dec + expectError bool + reducedFeeApplied bool + }{ + { + name: "x/gamm - single route", + poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(denomA, defaultInitPoolAmount), sdk.NewCoin(denomB, defaultInitPoolAmount))}, + routes: []swaproutertypes.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: denomB, + }, + }, + tokenOut: sdk.NewCoin(denomA, defaultSwapAmount), + tokenInMaxAmount: defaultSwapAmount.Mul(sdk.NewInt(2)), // the amount here is arbitrary + }, + // TODO: + // tests for concentrated liquidity + // test for multi-hop + // test for multi hop (osmo routes) -> reduces fee, add an assertion + // change values in and out to be different with each swap module type + // create more precise assertions on what the results are + // edge cases: + // * invalid route length + // * pool does not exist + // * swap errors + } + + for _, tc := range tests { + suite.Run(tc.name, func() { + suite.SetupTest() + + swaprouterKeeper := suite.App.SwapRouterKeeper + + suite.CreateBalancerPoolsFromCoins(tc.poolCoins) + + tokenIn, err := swaprouterKeeper.RouteExactAmountOut(suite.Ctx, suite.TestAccs[0], tc.routes, tc.tokenInMaxAmount, tc.tokenOut) + + if tc.expectError { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + suite.Require().True(tokenIn.LTE(tc.tokenInMaxAmount)) + } + }) + } +} diff --git a/x/swaprouter/types/routes.go b/x/swaprouter/types/routes.go index e5ae3bf277e..939c96a67d5 100644 --- a/x/swaprouter/types/routes.go +++ b/x/swaprouter/types/routes.go @@ -8,20 +8,26 @@ import ( // TODO: godoc type SwapI interface { - MultihopSwapExactAmountIn( + GetPool(ctx sdk.Context, poolId uint64) (gammtypes.PoolI, error) + + SwapExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, - routes []SwapAmountInRoute, + poolId gammtypes.PoolI, tokenIn sdk.Coin, + tokenOutDenom string, tokenOutMinAmount sdk.Int, - ) (tokenOutAmount sdk.Int, err error) + swapFee sdk.Dec, + ) (sdk.Int, error) - MultihopSwapExactAmountOut( + SwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, - routes []SwapAmountOutRoute, + poolId gammtypes.PoolI, + tokenInDenom string, tokenInMaxAmount sdk.Int, tokenOut sdk.Coin, + swapFee sdk.Dec, ) (tokenInAmount sdk.Int, err error) } diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 0ee9be07606..bb6f6b0bc30 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -5,6 +5,7 @@ import ( "github.com/osmosis-labs/osmosis/v12/osmoutils" epochstypes "github.com/osmosis-labs/osmosis/v12/x/epochs/types" + swaproutertypes "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" txfeestypes "github.com/osmosis-labs/osmosis/v12/x/txfees/types" ) @@ -34,7 +35,13 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. minAmountOut := sdk.ZeroInt() - _, err := k.gammKeeper.SwapExactAmountIn(cacheCtx, nonNativeFeeAddr, feetoken.PoolID, coinBalance, baseDenom, minAmountOut) + routes := []swaproutertypes.SwapAmountInRoute{ + { + PoolId: feetoken.PoolID, + TokenOutDenom: baseDenom, + }, + } + _, err := k.gammKeeper.RouteExactAmountIn(cacheCtx, nonNativeFeeAddr, routes, coinBalance, minAmountOut) return err }) } diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index 060946efdd4..75bd1d02750 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -16,7 +16,7 @@ type Keeper struct { accountKeeper types.AccountKeeper bankKeeper types.BankKeeper - gammKeeper types.GammKeeper + gammKeeper types.SwapRouterKeeper spotPriceCalculator types.SpotPriceCalculator feeCollectorName string nonNativeFeeCollectorName string @@ -28,7 +28,7 @@ func NewKeeper( accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, storeKey sdk.StoreKey, - gammKeeper types.GammKeeper, + swaprouterKeeper types.SwapRouterKeeper, spotPriceCalculator types.SpotPriceCalculator, feeCollectorName string, nonNativeFeeCollectorName string, @@ -37,7 +37,7 @@ func NewKeeper( accountKeeper: accountKeeper, bankKeeper: bankKeeper, storeKey: storeKey, - gammKeeper: gammKeeper, + gammKeeper: swaprouterKeeper, spotPriceCalculator: spotPriceCalculator, feeCollectorName: feeCollectorName, nonNativeFeeCollectorName: nonNativeFeeCollectorName, diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index b4f050e75cb..69a4d96f2ec 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -3,6 +3,8 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + + swaproutertypes "github.com/osmosis-labs/osmosis/v12/x/swaprouter/types" ) // SpotPriceCalculator defines the contract that must be fulfilled by a spot price calculator @@ -11,16 +13,14 @@ type SpotPriceCalculator interface { CalculateSpotPrice(ctx sdk.Context, poolId uint64, tokenInDenom, tokenOutDenom string) (sdk.Dec, error) } -// GammKeeper defines the contract needed for AccountKeeper related APIs. -type GammKeeper interface { - SwapExactAmountIn( +// SwapRouterKeeper defines the contract needed for swap related APIs. +type SwapRouterKeeper interface { + RouteExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, - poolId uint64, + routes []swaproutertypes.SwapAmountInRoute, tokenIn sdk.Coin, - tokenOutDenom string, - tokenOutMinAmount sdk.Int, - ) (tokenOutAmount sdk.Int, err error) + tokenOutMinAmount sdk.Int) (tokenOutAmount sdk.Int, err error) } // AccountKeeper defines the contract needed for AccountKeeper related APIs.