diff --git a/CHANGELOG.md b/CHANGELOG.md index 695fe066510..ce4f6493ffb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#3966](https://github.com/osmosis-labs/osmosis/pull/3966) Add Redelegate, Withdraw cli commands and sim_msgs * [#4107](https://github.com/osmosis-labs/osmosis/pull/4107) Add superfluid unbond partial amount * [#4207](https://github.com/osmosis-labs/osmosis/pull/4207) Add support for Async Interchain Queries + * [#4248](https://github.com/osmosis-labs/osmosis/pull/4248) Add panic recovery to `MultihopEstimateInGivenExactAmountOut`, `MultihopEstimateOutGivenExactAmountIn` and `RouteExactAmountOut` ## Misc Improvements * [#4131](https://github.com/osmosis-labs/osmosis/pull/4141) Add GatherValuesFromStorePrefixWithKeyParser function to osmoutils. diff --git a/app/apptesting/gamm.go b/app/apptesting/gamm.go index b224783156b..ec009271cce 100644 --- a/app/apptesting/gamm.go +++ b/app/apptesting/gamm.go @@ -13,9 +13,9 @@ import ( var DefaultAcctFunds sdk.Coins = sdk.NewCoins( sdk.NewCoin("uosmo", sdk.NewInt(10000000000)), - sdk.NewCoin("foo", sdk.NewInt(10000000)), - sdk.NewCoin("bar", sdk.NewInt(10000000)), - sdk.NewCoin("baz", sdk.NewInt(10000000)), + sdk.NewCoin("foo", sdk.NewInt(10000000000)), + sdk.NewCoin("bar", sdk.NewInt(10000000000)), + sdk.NewCoin("baz", sdk.NewInt(10000000000)), ) var DefaultPoolAssets = []balancer.PoolAsset{ diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index e7fcbb7a969..f47371774fd 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -141,6 +141,14 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( sumOfSwapFees sdk.Dec ) + // recover from panic + defer func() { + if r := recover(); r != nil { + tokenOutAmount = sdk.Int{} + err = fmt.Errorf("function MultihopEstimateOutGivenExactAmountIn failed due to internal reason: %v", r) + } + }() + route := types.SwapAmountInRoutes(routes) if err := route.Validate(); err != nil { return sdk.Int{}, err @@ -206,6 +214,13 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, return sdk.Int{}, err } + defer func() { + if r := recover(); r != nil { + tokenInAmount = sdk.Int{} + err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) + } + }() + // in this loop, we check if: // - the route is of length 2 // - route 1 and route 2 don't trade via the same pool @@ -295,6 +310,16 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( tokenOut sdk.Coin, ) (tokenInAmount sdk.Int, err error) { isMultiHopRouted, routeSwapFee, sumOfSwapFees := false, sdk.Dec{}, sdk.Dec{} + var insExpected []sdk.Int + + // recover from panic + defer func() { + if r := recover(); r != nil { + insExpected = []sdk.Int{} + err = fmt.Errorf("function MultihopEstimateInGivenExactAmountOut failed due to internal reason: %v", r) + } + }() + route := types.SwapAmountOutRoutes(routes) if err := route.Validate(); err != nil { return sdk.Int{}, err @@ -311,7 +336,6 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( // 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 swap fees - var insExpected []sdk.Int if isMultiHopRouted { insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, routes, tokenOut, routeSwapFee, sumOfSwapFees) } else { diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go index 6733abe0136..a58c0bc523c 100644 --- a/x/poolmanager/router_test.go +++ b/x/poolmanager/router_test.go @@ -594,6 +594,7 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountIn() { param param expectPass bool reducedFeeApplied bool + poolType types.PoolType }{ { name: "Proper swap - foo -> bar(pool 1) - bar(pool 2) -> baz", @@ -652,6 +653,64 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountIn() { reducedFeeApplied: true, expectPass: true, }, + { + name: "Proper swap (stableswap pool) - foo -> bar(pool 1) - bar(pool 2) -> baz", + param: param{ + routes: []types.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + }, + estimateRoutes: []types.SwapAmountInRoute{ + { + PoolId: 3, + TokenOutDenom: bar, + }, + { + PoolId: 4, + TokenOutDenom: baz, + }, + }, + tokenIn: sdk.NewCoin(foo, sdk.NewInt(100000)), + tokenOutMinAmount: sdk.NewInt(1), + }, + expectPass: true, + poolType: types.Stableswap, + }, + { + name: "Asserts panic catching in MultihopEstimateOutGivenExactAmountIn works: tokenOut more than pool reserves", + param: param{ + routes: []types.SwapAmountInRoute{ + { + PoolId: 1, + TokenOutDenom: bar, + }, + { + PoolId: 2, + TokenOutDenom: baz, + }, + }, + estimateRoutes: []types.SwapAmountInRoute{ + { + PoolId: 3, + TokenOutDenom: bar, + }, + { + PoolId: 4, + TokenOutDenom: baz, + }, + }, + tokenIn: sdk.NewCoin(foo, sdk.NewInt(9000000000000000000)), + tokenOutMinAmount: sdk.NewInt(1), + }, + expectPass: false, + poolType: types.Stableswap, + }, } for _, test := range tests { @@ -660,28 +719,8 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountIn() { suite.Run(test.name, func() { poolmanagerKeeper := suite.App.PoolManagerKeeper - poolDefaultSwapFee := sdk.NewDecWithPrec(1, 2) // 1% - - // Prepare 4 pools, - // Two pools for calculating `MultihopSwapExactAmountIn` - // and two pools for calculating `EstimateMultihopSwapExactAmountIn` - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - firstEstimatePoolId := suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - secondEstimatePoolId := suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) + firstEstimatePoolId, secondEstimatePoolId := suite.setupPools(test.poolType, defaultPoolSwapFee) firstEstimatePool, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, firstEstimatePoolId) suite.Require().NoError(err) @@ -689,24 +728,27 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountIn() { suite.Require().NoError(err) // calculate token out amount using `MultihopSwapExactAmountIn` - multihopTokenOutAmount, err := poolmanagerKeeper.RouteExactAmountIn( + multihopTokenOutAmount, errMultihop := poolmanagerKeeper.RouteExactAmountIn( suite.Ctx, suite.TestAccs[0], test.param.routes, test.param.tokenIn, test.param.tokenOutMinAmount) - suite.Require().NoError(err) // calculate token out amount using `EstimateMultihopSwapExactAmountIn` - estimateMultihopTokenOutAmount, err := poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( + estimateMultihopTokenOutAmount, errEstimate := poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn( suite.Ctx, test.param.estimateRoutes, test.param.tokenIn) - suite.Require().NoError(err) - - // ensure that the token out amount is same - suite.Require().Equal(multihopTokenOutAmount, estimateMultihopTokenOutAmount) + if test.expectPass { + suite.Require().NoError(errMultihop, "test: %v", test.name) + suite.Require().NoError(errEstimate, "test: %v", test.name) + suite.Require().Equal(multihopTokenOutAmount, estimateMultihopTokenOutAmount) + } else { + suite.Require().Error(errMultihop, "test: %v", test.name) + suite.Require().Error(errEstimate, "test: %v", test.name) + } // ensure that pool state has not been altered after estimation firstEstimatePoolAfterSwap, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, firstEstimatePoolId) suite.Require().NoError(err) @@ -734,6 +776,7 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountOut() { param param expectPass bool reducedFeeApplied bool + poolType types.PoolType }{ { name: "Proper swap: foo -> bar (pool 1), bar -> baz (pool 2)", @@ -792,6 +835,64 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountOut() { expectPass: true, reducedFeeApplied: true, }, + { + name: "Proper swap, stableswap pool: foo -> bar (pool 1), bar -> baz (pool 2)", + param: param{ + routes: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: foo, + }, + { + PoolId: 2, + TokenInDenom: bar, + }, + }, + estimateRoutes: []types.SwapAmountOutRoute{ + { + PoolId: 3, + TokenInDenom: foo, + }, + { + PoolId: 4, + TokenInDenom: bar, + }, + }, + tokenInMaxAmount: sdk.NewInt(90000000), + tokenOut: sdk.NewCoin(baz, sdk.NewInt(100000)), + }, + expectPass: true, + poolType: types.Stableswap, + }, + { + name: "Asserts panic catching in MultihopEstimateInGivenExactAmountOut works: tokenOut more than pool reserves", + param: param{ + routes: []types.SwapAmountOutRoute{ + { + PoolId: 1, + TokenInDenom: foo, + }, + { + PoolId: 2, + TokenInDenom: bar, + }, + }, + estimateRoutes: []types.SwapAmountOutRoute{ + { + PoolId: 3, + TokenInDenom: foo, + }, + { + PoolId: 4, + TokenInDenom: bar, + }, + }, + tokenInMaxAmount: sdk.NewInt(90000000), + tokenOut: sdk.NewCoin(baz, sdk.NewInt(9000000000000000000)), + }, + expectPass: false, + poolType: types.Stableswap, + }, } for _, test := range tests { @@ -800,47 +901,34 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountOut() { suite.Run(test.name, func() { poolmanagerKeeper := suite.App.PoolManagerKeeper - poolDefaultSwapFee := sdk.NewDecWithPrec(1, 2) // 1% - // Prepare 4 pools, - // Two pools for calculating `MultihopSwapExactAmountOut` - // and two pools for calculating `EstimateMultihopSwapExactAmountOut` - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, // 1% - ExitFee: sdk.NewDec(0), - }) - suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) - firstEstimatePoolId := suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, // 1% - ExitFee: sdk.NewDec(0), - }) - secondEstimatePoolId := suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ - SwapFee: poolDefaultSwapFee, - ExitFee: sdk.NewDec(0), - }) + firstEstimatePoolId, secondEstimatePoolId := suite.setupPools(test.poolType, defaultPoolSwapFee) + firstEstimatePool, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, firstEstimatePoolId) suite.Require().NoError(err) secondEstimatePool, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, secondEstimatePoolId) suite.Require().NoError(err) - multihopTokenInAmount, err := poolmanagerKeeper.RouteExactAmountOut( + multihopTokenInAmount, errMultihop := poolmanagerKeeper.RouteExactAmountOut( suite.Ctx, suite.TestAccs[0], test.param.routes, test.param.tokenInMaxAmount, test.param.tokenOut) - suite.Require().NoError(err, "test: %v", test.name) - estimateMultihopTokenInAmount, err := poolmanagerKeeper.MultihopEstimateInGivenExactAmountOut( + estimateMultihopTokenInAmount, errEstimate := poolmanagerKeeper.MultihopEstimateInGivenExactAmountOut( suite.Ctx, test.param.estimateRoutes, test.param.tokenOut) - suite.Require().NoError(err, "test: %v", test.name) - suite.Require().Equal(multihopTokenInAmount, estimateMultihopTokenInAmount) + if test.expectPass { + suite.Require().NoError(errMultihop, "test: %v", test.name) + suite.Require().NoError(errEstimate, "test: %v", test.name) + suite.Require().Equal(multihopTokenInAmount, estimateMultihopTokenInAmount) + } else { + suite.Require().Error(errMultihop, "test: %v", test.name) + suite.Require().Error(errEstimate, "test: %v", test.name) + } // ensure that pool state has not been altered after estimation firstEstimatePoolAfterSwap, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, firstEstimatePoolId) @@ -1030,3 +1118,43 @@ func (suite *KeeperTestSuite) TestSingleSwapExactAmountIn() { }) } } + +// setupPools creates pools of desired type and returns their IDs +func (suite *KeeperTestSuite) setupPools(poolType types.PoolType, poolDefaultSwapFee sdk.Dec) (firstEstimatePoolId, secondEstimatePoolId uint64) { + switch poolType { + case types.Stableswap: + // Prepare 4 pools, + // Two pools for calculating `MultihopSwapExactAmountOut` + // and two pools for calculating `EstimateMultihopSwapExactAmountOut` + suite.PrepareBasicStableswapPool() + suite.PrepareBasicStableswapPool() + + firstEstimatePoolId = suite.PrepareBasicStableswapPool() + + secondEstimatePoolId = suite.PrepareBasicStableswapPool() + return + default: + // Prepare 4 pools, + // Two pools for calculating `MultihopSwapExactAmountOut` + // and two pools for calculating `EstimateMultihopSwapExactAmountOut` + suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ + SwapFee: poolDefaultSwapFee, // 1% + ExitFee: sdk.NewDec(0), + }) + suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ + SwapFee: poolDefaultSwapFee, + ExitFee: sdk.NewDec(0), + }) + + firstEstimatePoolId = suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ + SwapFee: poolDefaultSwapFee, // 1% + ExitFee: sdk.NewDec(0), + }) + + secondEstimatePoolId = suite.PrepareBalancerPoolWithPoolParams(balancer.PoolParams{ + SwapFee: poolDefaultSwapFee, + ExitFee: sdk.NewDec(0), + }) + return + } +}