Skip to content

Commit

Permalink
MultihopEstimateInGivenExactAmountOut and `MultihopEstimateOutGiven…
Browse files Browse the repository at this point in the history 
…ExactAmountIn` panic recovering (#4248)

* add panic recovering to multihop estimate functions

* unit test for making sure recovery works as expected

* change error message

* separate tests

* refactoring

* refactor

* add panic catching to RouteExactAmountOut

* add panic catching assertions to existing tests

* delete old unit tests

* remove fmt

* hardcoded stableswap to enum

* getSetupPoolsStrategy as a helper function

* remove callback return

* remove redundant swap fee variable

* .String rm

* docs fix/rename

* typo

* typo2

* changelog entry
  • Loading branch information
@pysel
pysel authored Feb 8, 2023
1 parent d9af48b commit a33893d
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 57 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions app/apptesting/gamm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
26 changes: 25 additions & 1 deletion x/poolmanager/router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
234 changes: 181 additions & 53 deletions x/poolmanager/router_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand All @@ -660,53 +719,36 @@ 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)
secondEstimatePool, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, secondEstimatePoolId)
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)
Expand Down Expand Up @@ -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)",
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
}

0 comments on commit a33893d

Please sign in to comment.