diff --git a/CHANGELOG.md b/CHANGELOG.md index 6173c2dbdc2..8ce8ac1f5ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fix * [#3715](https://github.com/osmosis-labs/osmosis/pull/3715) Fix x/gamm (golang API) CalculateSpotPrice, balancer.SpotPrice and Stableswap.SpotPrice base and quote asset. +* [#3746](https://github.com/osmosis-labs/osmosis/pull/3746) Make ApplyFuncIfNoErr logic preserve panics for OutOfGas behavior. + + ## v14 @@ -113,7 +116,6 @@ Additionally, the swagger files for v13 have been updated to improve compatibili * [#3711](https://github.com/osmosis-labs/osmosis/pull/3711) Use Dec instead of Int for additive `ErrTolerace` in `osmoutils`. * [3647](https://github.com/osmosis-labs/osmosis/pull/3647), [3942](https://github.com/osmosis-labs/osmosis/pull/3942) (CLI) re-order the command line arguments for `osmosisd tx gamm join-swap-share-amount-out` - ## v13.0.0 This release includes stableswap, and expands the IBC safety & composability functionality of Osmosis. The primary features are: diff --git a/osmoutils/cache_ctx.go b/osmoutils/cache_ctx.go index 6e398cdbc4d..0d4cbe6802b 100644 --- a/osmoutils/cache_ctx.go +++ b/osmoutils/cache_ctx.go @@ -14,12 +14,20 @@ import ( // drop the state machine change and log the error. // If there is no error, proceeds as normal (but with some slowdown due to SDK store weirdness) // Try to avoid usage of iterators in f. +// +// If its an out of gas panic, this function will also panic like in normal tx execution flow. +// This is still safe for beginblock / endblock code though, as they do not have out of gas panics. func ApplyFuncIfNoError(ctx sdk.Context, f func(ctx sdk.Context) error) (err error) { // Add a panic safeguard defer func() { if recoveryError := recover(); recoveryError != nil { - PrintPanicRecoveryError(ctx, recoveryError) - err = errors.New("panic occurred during execution") + if isErr, _ := IsOutOfGasError(recoveryError); isErr { + // We panic with the same error, to replicate the normal tx execution flow. + panic(recoveryError) + } else { + PrintPanicRecoveryError(ctx, recoveryError) + err = errors.New("panic occurred during execution") + } } }() // makes a new cache context, which all state changes get wrapped inside of. @@ -35,6 +43,19 @@ func ApplyFuncIfNoError(ctx sdk.Context, f func(ctx sdk.Context) error) (err err return err } +// Frustratingly, this has to return the error descriptor, not an actual error itself +// because the SDK errors here are not actually errors. (They don't implement error interface) +func IsOutOfGasError(err any) (bool, string) { + switch e := err.(type) { + case types.ErrorOutOfGas: + return true, e.Descriptor + case types.ErrorGasOverflow: + return true, e.Descriptor + default: + return false, "" + } +} + // PrintPanicRecoveryError error logs the recoveryError, along with the stacktrace, if it can be parsed. // If not emits them to stdout. func PrintPanicRecoveryError(ctx sdk.Context, recoveryError interface{}) { diff --git a/osmoutils/cache_ctx_test.go b/osmoutils/cache_ctx_test.go new file mode 100644 index 00000000000..a8058ea0d71 --- /dev/null +++ b/osmoutils/cache_ctx_test.go @@ -0,0 +1,59 @@ +package osmoutils_test + +import ( + "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/osmosis-labs/osmosis/v13/osmoutils" +) + +var expectedOutOfGasError = types.ErrorOutOfGas{Descriptor: "my func"} + +func consumeGas(ctx sdk.Context, gas uint64, numTimes int) error { + for i := 0; i < numTimes; i++ { + ctx.GasMeter().ConsumeGas(gas, "my func") + } + return nil +} + +func (s *TestSuite) TestCacheCtxConsumeGas() { + // every test case adds 1k gas 10 times + testcases := map[string]struct { + gasLimit uint64 + gasUsedPreCtx uint64 + gasUsedPostCtx uint64 + expectPanic bool + }{ + "no gas limit hit": { + gasLimit: 15_000, + gasUsedPreCtx: 111, + gasUsedPostCtx: 111 + 10_000, + expectPanic: false, + }, + "gas limit hit": { + gasLimit: 10_000, + gasUsedPreCtx: 111, + gasUsedPostCtx: 111 + 10_000, + expectPanic: true, + }, + } + for name, tc := range testcases { + s.Run(name, func() { + ctx := s.Ctx.WithGasMeter(sdk.NewGasMeter(tc.gasLimit)) + ctx.GasMeter().ConsumeGas(tc.gasUsedPreCtx, "pre ctx") + var err error + f := func() { + osmoutils.ApplyFuncIfNoError(ctx, func(c sdk.Context) error { + return consumeGas(c, 1000, 10) + }) + } + if tc.expectPanic { + s.PanicsWithValue(expectedOutOfGasError, f) + } else { + f() + s.Require().NoError(err) + } + s.Equal(tc.gasUsedPostCtx, ctx.GasMeter().GasConsumed()) + }) + } +}