From 48c4514817b3655b09c4795a2b6ab2baa4469e2e Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Fri, 22 Dec 2023 15:50:27 +0100 Subject: [PATCH 1/4] better errors on out of gas --- x/gamm/keeper/pool_service.go | 10 ++++++++++ x/gamm/keeper/swap.go | 7 +++++++ x/poolmanager/router.go | 9 +++++++++ x/protorev/keeper/posthandler.go | 6 +++++- 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 8d6b01269b8..6df996b5f77 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + "github.com/osmosis-labs/osmosis/osmoutils" errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" @@ -142,6 +143,9 @@ func (k Keeper) JoinPoolNoSwap( if r := recover(); r != nil { tokenIn = sdk.Coins{} sharesOut = osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function JoinPoolNoSwap failed due to lack of gas: %v", d) + } err = fmt.Errorf("function JoinPoolNoSwap failed due to internal reason: %v", r) } }() @@ -234,6 +238,9 @@ func (k Keeper) JoinSwapExactAmountIn( defer func() { if r := recover(); r != nil { sharesOut = osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function JoinSwapExactAmountIn failed due to lack of gas: %v", d) + } err = fmt.Errorf("function JoinSwapExactAmountIn failed due to internal reason: %v", r) } }() @@ -278,6 +285,9 @@ func (k Keeper) JoinSwapShareAmountOut( defer func() { if r := recover(); r != nil { tokenInAmount = osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function JoinSwapShareAmountOut failed due to lack of gas: %v", d) + } err = fmt.Errorf("function JoinSwapShareAmountOut failed due to internal reason: %v", r) } }() diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index bebce7995fe..539ec50d59b 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -3,6 +3,7 @@ package keeper import ( "errors" "fmt" + "github.com/osmosis-labs/osmosis/osmoutils" errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" @@ -39,6 +40,9 @@ func (k Keeper) SwapExactAmountIn( defer func() { if r := recover(); r != nil { + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function swapExactAmountIn failed due to lack of gas: %v", d) + } tokenOutAmount = osmomath.Int{} err = fmt.Errorf("function swapExactAmountIn failed due to internal reason: %v", r) } @@ -94,6 +98,9 @@ func (k Keeper) SwapExactAmountOut( defer func() { if r := recover(); r != nil { tokenInAmount = osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function swapExactAmountOut failed due to lack of gas: %v", d) + } err = fmt.Errorf("function swapExactAmountOut failed due to internal reason: %v", r) } }() diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index bd286b08939..9b22f4f68f3 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -235,6 +235,9 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( defer func() { if r := recover(); r != nil { tokenOutAmount = osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function MultihopEstimateOutGivenExactAmountIn failed due to lack of gas: %v", d) + } err = fmt.Errorf("function MultihopEstimateOutGivenExactAmountIn failed due to internal reason: %v", r) } }() @@ -301,6 +304,9 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, defer func() { if r := recover(); r != nil { tokenInAmount = osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function RouteExactAmountOut failed due to lack of gas: %v", d) + } err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) } }() @@ -493,6 +499,9 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( defer func() { if r := recover(); r != nil { insExpected = []osmomath.Int{} + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("function MultihopEstimateInGivenExactAmountOut failed due to lack of gas: %v", d) + } err = fmt.Errorf("function MultihopEstimateInGivenExactAmountOut failed due to internal reason: %v", r) } }() diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 90898b3afcd..96c982156c4 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + "github.com/osmosis-labs/osmosis/osmoutils" sdk "github.com/cosmos/cosmos-sdk/types" @@ -113,7 +114,10 @@ func (k Keeper) ProtoRevTrade(ctx sdk.Context, swappedPools []SwapToBackrun) (er // recover from panic defer func() { if r := recover(); r != nil { - err = fmt.Errorf("Protorev failed due to internal reason: %v", r) + if isErr, d := osmoutils.IsOutOfGasError(r); isErr { + err = fmt.Errorf("protorev failed due to lack of gas: %v", d) + } + err = fmt.Errorf("protorev failed due to internal reason: %v", r) } }() From eb4d5b960e3fcb0a13ece1fd5180cb8d7b0430c7 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Fri, 22 Dec 2023 16:05:58 +0100 Subject: [PATCH 2/4] added tests and proper if/else --- x/gamm/keeper/pool_service.go | 9 ++++++--- x/gamm/keeper/swap.go | 8 +++++--- x/gamm/keeper/swap_test.go | 14 ++++++++++++++ x/poolmanager/router.go | 9 ++++++--- x/protorev/keeper/posthandler.go | 3 ++- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 6df996b5f77..60d93e334b6 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -145,8 +145,9 @@ func (k Keeper) JoinPoolNoSwap( sharesOut = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function JoinPoolNoSwap failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function JoinPoolNoSwap failed due to internal reason: %v", r) } - 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 @@ -240,8 +241,9 @@ func (k Keeper) JoinSwapExactAmountIn( sharesOut = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function JoinSwapExactAmountIn failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function JoinSwapExactAmountIn failed due to internal reason: %v", r) } - err = fmt.Errorf("function JoinSwapExactAmountIn failed due to internal reason: %v", r) } }() @@ -287,8 +289,9 @@ func (k Keeper) JoinSwapShareAmountOut( tokenInAmount = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function JoinSwapShareAmountOut failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function JoinSwapShareAmountOut failed due to internal reason: %v", r) } - err = fmt.Errorf("function JoinSwapShareAmountOut failed due to internal reason: %v", r) } }() diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index 539ec50d59b..abd3e417b7c 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -40,11 +40,12 @@ func (k Keeper) SwapExactAmountIn( defer func() { if r := recover(); r != nil { + tokenOutAmount = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function swapExactAmountIn failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function swapExactAmountIn failed due to internal reason: %v", r) } - tokenOutAmount = osmomath.Int{} - err = fmt.Errorf("function swapExactAmountIn failed due to internal reason: %v", r) } }() @@ -100,8 +101,9 @@ func (k Keeper) SwapExactAmountOut( tokenInAmount = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function swapExactAmountOut failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function swapExactAmountOut failed due to internal reason: %v", r) } - err = fmt.Errorf("function swapExactAmountOut failed due to internal reason: %v", r) } }() diff --git a/x/gamm/keeper/swap_test.go b/x/gamm/keeper/swap_test.go index 1f6923b3177..d596e20a7c9 100644 --- a/x/gamm/keeper/swap_test.go +++ b/x/gamm/keeper/swap_test.go @@ -466,6 +466,20 @@ func (s *KeeperTestSuite) TestActiveBalancerPoolSwap() { } } +func (s *KeeperTestSuite) TestOutOfGasError() { + s.SetupTest() + poolId := s.PrepareBalancerPool() + + pool, err := s.App.GAMMKeeper.GetPool(s.Ctx, poolId) + s.Require().NoError(err) + foocoin := sdk.NewCoin("foo", osmomath.NewInt(10)) + spreadFactor := pool.GetSpreadFactor(s.Ctx) + ctx := s.Ctx.WithGasMeter(sdk.NewGasMeter(10)) + _, err = s.App.GAMMKeeper.SwapExactAmountIn(ctx, s.TestAccs[0], pool, foocoin, "bar", osmomath.ZeroInt(), spreadFactor) + s.Require().Error(err) + s.Require().Contains(err.Error(), "lack of gas") +} + // UNFORKINGNOTE: This test really wasn't testing anything important // With the unfork, we can no longer utilize mocks when calling SetPools, since // the interface needs to be registered with codec, and the mocks aren't wired to do that. diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go index 9b22f4f68f3..7309259b2f9 100644 --- a/x/poolmanager/router.go +++ b/x/poolmanager/router.go @@ -237,8 +237,9 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn( tokenOutAmount = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function MultihopEstimateOutGivenExactAmountIn failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function MultihopEstimateOutGivenExactAmountIn failed due to internal reason: %v", r) } - err = fmt.Errorf("function MultihopEstimateOutGivenExactAmountIn failed due to internal reason: %v", r) } }() @@ -306,8 +307,9 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context, tokenInAmount = osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function RouteExactAmountOut failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) } - err = fmt.Errorf("function RouteExactAmountOut failed due to internal reason: %v", r) } }() @@ -501,8 +503,9 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut( insExpected = []osmomath.Int{} if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("function MultihopEstimateInGivenExactAmountOut failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("function MultihopEstimateInGivenExactAmountOut failed due to internal reason: %v", r) } - err = fmt.Errorf("function MultihopEstimateInGivenExactAmountOut failed due to internal reason: %v", r) } }() diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 96c982156c4..0f59b9effb7 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -116,8 +116,9 @@ func (k Keeper) ProtoRevTrade(ctx sdk.Context, swappedPools []SwapToBackrun) (er if r := recover(); r != nil { if isErr, d := osmoutils.IsOutOfGasError(r); isErr { err = fmt.Errorf("protorev failed due to lack of gas: %v", d) + } else { + err = fmt.Errorf("protorev failed due to internal reason: %v", r) } - err = fmt.Errorf("protorev failed due to internal reason: %v", r) } }() From d3e38ae0653acff3ef739a137375db3ddf0bc52d Mon Sep 17 00:00:00 2001 From: mattverse Date: Tue, 2 Jan 2024 18:35:43 +0900 Subject: [PATCH 3/4] Make lint happy --- x/gamm/keeper/pool_service.go | 1 + x/gamm/keeper/swap.go | 1 + x/protorev/keeper/posthandler.go | 1 + 3 files changed, 3 insertions(+) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 60d93e334b6..2d427803adb 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + "github.com/osmosis-labs/osmosis/osmoutils" errorsmod "cosmossdk.io/errors" diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index abd3e417b7c..47bb6bb3a7e 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -3,6 +3,7 @@ package keeper import ( "errors" "fmt" + "github.com/osmosis-labs/osmosis/osmoutils" errorsmod "cosmossdk.io/errors" diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 0f59b9effb7..42acef2a727 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + "github.com/osmosis-labs/osmosis/osmoutils" sdk "github.com/cosmos/cosmos-sdk/types" From b25d25d180375bfa13ff5f62b00b8b40f25e06f2 Mon Sep 17 00:00:00 2001 From: mattverse Date: Tue, 2 Jan 2024 18:37:07 +0900 Subject: [PATCH 4/4] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe7d4602743..f9d65e5b578 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Misc Improvements * [#7106](https://github.com/osmosis-labs/osmosis/pull/7106) Halve the time of log2 calculation (speeds up TWAP code) * [#7093](https://github.com/osmosis-labs/osmosis/pull/7093),[#7100](https://github.com/osmosis-labs/osmosis/pull/7100),[#7172](https://github.com/osmosis-labs/osmosis/pull/7093) Lower CPU overheads of the Osmosis epoch. +* [#7181](https://github.com/osmosis-labs/osmosis/pull/7181) Improve errors for out of gas ## v21.0.0