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.

Sign up for GitHub

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

Jump to bottom

MultihopEstimateInGivenExactAmountOut and MultihopEstimateOutGivenExactAmountIn panic recovering #4248

Merged
merged 19 commits into from
Feb 8, 2023
Merged

MultihopEstimateInGivenExactAmountOut and MultihopEstimateOutGivenExactAmountIn panic recovering #4248

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3c4871b
add panic recovering to multihop estimate functions
pysel Feb 7, 2023
89076b6
unit test for making sure recovery works as expected
pysel Feb 7, 2023
faf7b61
change error message
pysel Feb 7, 2023
3846ec9
separate tests
pysel Feb 7, 2023
fa3ed28
refactoring
pysel Feb 7, 2023
04be5ff
refactor
pysel Feb 7, 2023
071d8df
add panic catching to RouteExactAmountOut
pysel Feb 7, 2023
4f3d63c
add panic catching assertions to existing tests
pysel Feb 7, 2023
c0b0de0
delete old unit tests
pysel Feb 7, 2023
ccb0250
remove fmt
pysel Feb 7, 2023
47d39fc
hardcoded stableswap to enum
pysel Feb 7, 2023
e06e28b
getSetupPoolsStrategy as a helper function
pysel Feb 7, 2023
6a04dfa
remove callback return
pysel Feb 7, 2023
703698a
remove redundant swap fee variable
pysel Feb 7, 2023
1127cdf
.String rm
pysel Feb 7, 2023
48e3f0d
docs fix/rename
pysel Feb 7, 2023
65e8d1a
typo
pysel Feb 7, 2023
5b6ef14
typo2
pysel Feb 7, 2023
d39171c
changelog entry
pysel Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/apptesting/gamm.go
View file
Open in desktop
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
Open in desktop
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
View file
Open in desktop
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