Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultihopEstimateInGivenExactAmountOut and MultihopEstimateOutGivenExactAmountIn panic recovering #4248

Merged
merged 19 commits into from
Feb 8, 2023
Merged
6 changes: 3 additions & 3 deletions app/apptesting/gamm.go
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
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
241 changes: 190 additions & 51 deletions x/poolmanager/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,53 @@ var (
concentratedKeeperType = reflect.TypeOf(&cl.Keeper{})
)

// prepare pool function
type preparePool func() (firstEstimatePoolId, secondEstimatePoolId uint64)

// getPoolStrategy returns a function that will setup pools based on the poolType provided
var getSetupPoolsStrategy = func(suite *KeeperTestSuite, poolType string, poolDefaultSwapFee sdk.Dec) preparePool {
pysel marked this conversation as resolved.
Show resolved Hide resolved
switch poolType {
case "stableswap":
pysel marked this conversation as resolved.
Show resolved Hide resolved
return func() (firstEstimatePoolId, secondEstimatePoolId uint64) {
// 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:
return func() (firstEstimatePoolId, secondEstimatePoolId uint64) {
// 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
}
}
}

// TestGetPoolModule tests that the correct pool module is returned for a given pool id.
// Additionally, validates that the expected errors are produced when expected.
func (suite *KeeperTestSuite) TestGetPoolModule() {
Expand Down Expand Up @@ -594,6 +641,7 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountIn() {
param param
expectPass bool
reducedFeeApplied bool
poolType string
}{
{
name: "Proper swap - foo -> bar(pool 1) - bar(pool 2) -> baz",
Expand Down Expand Up @@ -652,6 +700,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: "stableswap",
pysel marked this conversation as resolved.
Show resolved Hide resolved
},
{
name: "Asserts panic catching in MultihopEstimateInGivenExactAmountOut 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: "stableswap",
},
}

for _, test := range tests {
Expand All @@ -662,51 +768,36 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountIn() {
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),
})
setupPoolsStrategy := getSetupPoolsStrategy(suite, test.poolType, poolDefaultSwapFee)
firstEstimatePoolId, secondEstimatePoolId := setupPoolsStrategy()
pysel marked this conversation as resolved.
Show resolved Hide resolved

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 +825,7 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountOut() {
param param
expectPass bool
reducedFeeApplied bool
poolType string
}{
{
name: "Proper swap: foo -> bar (pool 1), bar -> baz (pool 2)",
Expand Down Expand Up @@ -792,6 +884,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: "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: "stableswap",
},
}

for _, test := range tests {
Expand All @@ -802,45 +952,34 @@ func (suite *KeeperTestSuite) TestEstimateMultihopSwapExactAmountOut() {
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),
})
setupPoolsStrategy := getSetupPoolsStrategy(suite, test.poolType, poolDefaultSwapFee)
firstEstimatePoolId, secondEstimatePoolId := setupPoolsStrategy()

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