From 54fc604b7ee5b275ded834ab211b54dd63d4fe5d Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 10 Nov 2022 00:38:57 +0100 Subject: [PATCH] Add keeper level panic catches (#3312) (#3315) * Add keeper level panic catches * Add changelog * Add panic catch for swap (cherry picked from commit f88577d30e35f3bc0ee7ddc79a727818fc1b50cb) Co-authored-by: Dev Ojha --- CHANGELOG.md | 1 + x/gamm/keeper/pool_service.go | 28 ++++++++++++++++++++++++++-- x/gamm/keeper/swap.go | 15 +++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c9739d788..0da28fc90b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#2914](https://github.com/osmosis-labs/osmosis/pull/2914) Remove out of gas panics from node logs * [#2937](https://github.com/osmosis-labs/osmosis/pull/2937) End block ordering - staking after gov and module sorting. * [#2923](https://github.com/osmosis-labs/osmosis/pull/2923) TWAP calculation now errors if it uses records that have errored previously. +* [#3312](https://github.com/osmosis-labs/osmosis/pull/3312) Add better panic catches within GAMM txs ### Misc Improvements diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 0f630e10a7e..b464dc952c6 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -202,6 +202,14 @@ func (k Keeper) JoinPoolNoSwap( shareOutAmount sdk.Int, tokenInMaxs sdk.Coins, ) (tokenIn sdk.Coins, sharesOut sdk.Int, err error) { + // defer to catch panics, in case something internal overflows. + defer func() { + if r := recover(); r != nil { + tokenIn = sdk.Coins{} + sharesOut = sdk.Int{} + err = fmt.Errorf("function JoinPoolNoSwap failed due to internal reason: %v", r) + } + }() // all pools handled within this method are pointer references, `JoinPool` directly updates the pools pool, err := k.GetPoolAndPoke(ctx, poolId) if err != nil { @@ -281,13 +289,21 @@ func (k Keeper) JoinSwapExactAmountIn( poolId uint64, tokensIn sdk.Coins, shareOutMinAmount sdk.Int, -) (sdk.Int, error) { +) (sharesOut sdk.Int, err error) { + // defer to catch panics, in case something internal overflows. + defer func() { + if r := recover(); r != nil { + sharesOut = sdk.Int{} + err = fmt.Errorf("function JoinSwapExactAmountIn failed due to internal reason: %v", r) + } + }() + pool, err := k.getPoolForSwap(ctx, poolId) if err != nil { return sdk.Int{}, err } - sharesOut, err := pool.JoinPool(ctx, tokensIn, pool.GetSwapFee(ctx)) + sharesOut, err = pool.JoinPool(ctx, tokensIn, pool.GetSwapFee(ctx)) switch { case err != nil: return sdk.ZeroInt(), err @@ -318,6 +334,14 @@ func (k Keeper) JoinSwapShareAmountOut( shareOutAmount sdk.Int, tokenInMaxAmount sdk.Int, ) (tokenInAmount sdk.Int, err error) { + // defer to catch panics, in case something internal overflows. + defer func() { + if r := recover(); r != nil { + tokenInAmount = sdk.Int{} + err = fmt.Errorf("function JoinSwapShareAmountOut failed due to internal reason: %v", r) + } + }() + pool, err := k.getPoolForSwap(ctx, poolId) if err != nil { return sdk.Int{}, err diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index f048784c269..e8486a4a4f0 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -2,6 +2,7 @@ package keeper import ( "errors" + "fmt" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -50,6 +51,13 @@ func (k Keeper) swapExactAmountIn( } tokensIn := sdk.Coins{tokenIn} + defer func() { + if r := recover(); r != nil { + tokenOutAmount = sdk.Int{} + err = fmt.Errorf("function swapExactAmountIn failed due to internal reason: %v", r) + } + }() + // Executes the swap in the pool and stores the output. Updates pool assets but // does not actually transfer any tokens to or from the pool. tokenOutCoin, err := pool.SwapOutAmtGivenIn(ctx, tokensIn, tokenOutDenom, swapFee) @@ -109,6 +117,13 @@ func (k Keeper) swapExactAmountOut( return sdk.Int{}, errors.New("cannot trade same denomination in and out") } + defer func() { + if r := recover(); r != nil { + tokenInAmount = sdk.Int{} + err = fmt.Errorf("function swapExactAmountOut failed due to internal reason: %v", r) + } + }() + poolOutBal := pool.GetTotalPoolLiquidity(ctx).AmountOf(tokenOut.Denom) if tokenOut.Amount.GTE(poolOutBal) { return sdk.Int{}, sdkerrors.Wrapf(types.ErrTooManyTokensOut,