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

refactor: use errors.As to detect out of gas exceptions #3320

Merged
merged 7 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 1 addition & 8 deletions gno.land/cmd/gnoland/testdata/addpkg_outofgas.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,12 @@ gnoland start
gnokey maketx addpkg -pkgdir $WORK/foo -pkgpath gno.land/r/foo -gas-fee 1000000ugnot -gas-wanted 220000 -broadcast -chainid=tendermint_test test1


# add bar package
# out of gas at store.GetPackage() with gas 60000
# add bar package - out of gas at store.GetPackage() with gas 60000

! gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/bar -gas-fee 1000000ugnot -gas-wanted 60000 -broadcast -chainid=tendermint_test test1

# Out of gas error

stderr '--= Error =--'
stderr 'Data: out of gas error'
stderr 'Msg Traces:'
stderr 'out of gas.*?in preprocess'
stderr '--= /Error =--'


Expand All @@ -28,8 +23,6 @@ stderr '--= /Error =--'

stderr '--= Error =--'
stderr 'Data: out of gas error'
stderr 'Msg Traces:'
stderr 'out of gas.*?in preprocess'
stderr '--= /Error =--'


Expand Down
4 changes: 2 additions & 2 deletions gno.land/pkg/sdk/vm/gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestAddPkgDeliverTxInsuffGas(t *testing.T) {
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException:
case store.OutOfGasError:
res.Error = sdk.ABCIError(std.ErrOutOfGas(""))
abort = true
default:
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestAddPkgDeliverTxFailedNoGas(t *testing.T) {
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException:
case store.OutOfGasError:
res.Error = sdk.ABCIError(std.ErrOutOfGas(""))
abort = true
default:
Expand Down
149 changes: 59 additions & 90 deletions gno.land/pkg/sdk/vm/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import (
"bytes"
"context"
goerrors "errors"
"fmt"
"io"
"log/slog"
Expand Down Expand Up @@ -392,18 +393,7 @@
GasMeter: ctx.GasMeter(),
})
defer m2.Release()
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException: // panic in consumeGas()
panic(r)
default:
err = errors.Wrapf(fmt.Errorf("%v", r), "VM addpkg panic: %v\n%s\n",
r, m2.String())
return
}
}
}()
defer doRecover(m2, &err)
m2.RunMemPackage(memPkg, true)

// Log the telemetry
Expand Down Expand Up @@ -495,21 +485,7 @@
})
defer m.Release()
m.SetActivePackage(mpv)
defer func() {
if r := recover(); r != nil {
switch r := r.(type) {
case store.OutOfGasException: // panic in consumeGas()
panic(r)
case gno.UnhandledPanicError:
err = errors.Wrapf(fmt.Errorf("%v", r.Error()), "VM call panic: %s\nStacktrace: %s\n",
r.Error(), m.ExceptionsStacktrace())
default:
err = errors.Wrapf(fmt.Errorf("%v", r), "VM call panic: %v\nMachine State:%s\nStacktrace: %s\n",
r, m.String(), m.Stacktrace().String())
return
}
}
}()
defer doRecover(m, &err)
rtvs := m.Eval(xn)
for i, rtv := range rtvs {
res = res + rtv.String()
Expand All @@ -534,6 +510,33 @@
// TODO pay for gas? TODO see context?
}

func doRecover(m *gno.Machine, e *error) {
if r := recover(); r != nil {
thehowl marked this conversation as resolved.
Show resolved Hide resolved
if err, ok := r.(error); ok {
var oog types.OutOfGasError
if goerrors.As(err, &oog) {
// Re-panic and don't wrap.
panic(oog)
}
var up gno.UnhandledPanicError
if goerrors.As(err, &up) {
// Common unhandled panic error, skip machine state.
*e = errors.Wrapf(
errors.New(up.Descriptor),
"VM panic: %s\nStacktrace: %s\n",
up.Descriptor, m.ExceptionsStacktrace(),
)
return
}
}
*e = errors.Wrapf(
fmt.Errorf("%v", r),
"VM panic: %v\nMachine State:%s\nStacktrace: %s\n",
r, m.String(), m.Stacktrace().String(),
)
}
}

// Run executes arbitrary Gno code in the context of the caller's realm.
func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) {
caller := msg.Caller
Expand Down Expand Up @@ -583,37 +586,36 @@
Params: NewSDKParams(vm, ctx),
EventLogger: ctx.EventLogger(),
}
// Parse and run the files, construct *PV.

buf := new(bytes.Buffer)
output := io.Writer(buf)
if vm.Output != nil {
output = io.MultiWriter(buf, vm.Output)
}
m := gno.NewMachineWithOptions(
gno.MachineOptions{
PkgPath: "",
Output: output,
Store: gnostore,
Alloc: gnostore.GetAllocator(),
Context: msgCtx,
GasMeter: ctx.GasMeter(),
})
// XXX MsgRun does not have pkgPath. How do we find it on chain?
defer m.Release()
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException: // panic in consumeGas()
panic(r)
default:
err = errors.Wrapf(fmt.Errorf("%v", r), "VM run main addpkg panic: %v\n%s\n",
r, m.String())
return
}

// Run as self-executing closure to have own function for doRecover / m.Release defers.
pv := func() *gno.PackageValue {
thehowl marked this conversation as resolved.
Show resolved Hide resolved
// Parse and run the files, construct *PV.
if vm.Output != nil {
output = io.MultiWriter(buf, vm.Output)

Check warning on line 597 in gno.land/pkg/sdk/vm/keeper.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/sdk/vm/keeper.go#L597

Added line #L597 was not covered by tests
}
}()
m := gno.NewMachineWithOptions(
gno.MachineOptions{
PkgPath: "",
Output: output,
Store: gnostore,
Alloc: gnostore.GetAllocator(),
Context: msgCtx,
GasMeter: ctx.GasMeter(),
})
// XXX MsgRun does not have pkgPath. How do we find it on chain?
defer m.Release()
defer doRecover(m, &err)

_, pv := m.RunMemPackage(memPkg, false)
_, pv := m.RunMemPackage(memPkg, false)
return pv
}()
if err != nil {
// handle any errors happened within pv generation.
return
}

Check warning on line 618 in gno.land/pkg/sdk/vm/keeper.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/sdk/vm/keeper.go#L616-L618

Added lines #L616 - L618 were not covered by tests

m2 := gno.NewMachineWithOptions(
gno.MachineOptions{
Expand All @@ -626,18 +628,7 @@
})
defer m2.Release()
m2.SetActivePackage(pv)
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException: // panic in consumeGas()
panic(r)
default:
err = errors.Wrapf(fmt.Errorf("%v", r), "VM run main call panic: %v\n%s\n",
r, m2.String())
return
}
}
}()
defer doRecover(m2, &err)
m2.RunMain()
res = buf.String()

Expand Down Expand Up @@ -758,18 +749,7 @@
GasMeter: ctx.GasMeter(),
})
defer m.Release()
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException: // panic in consumeGas()
panic(r)
default:
err = errors.Wrapf(fmt.Errorf("%v", r), "VM query eval panic: %v\n%s\n",
r, m.String())
return
}
}
}()
defer doRecover(m, &err)
rtvs := m.Eval(xx)
res = ""
for i, rtv := range rtvs {
Expand Down Expand Up @@ -826,18 +806,7 @@
GasMeter: ctx.GasMeter(),
})
defer m.Release()
defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException: // panic in consumeGas()
panic(r)
default:
err = errors.Wrapf(fmt.Errorf("%v", r), "VM query eval string panic: %v\n%s\n",
r, m.String())
return
}
}
}()
defer doRecover(m, &err)
rtvs := m.Eval(xx)
if len(rtvs) != 1 {
return "", errors.New("expected 1 string result, got %d", len(rtvs))
Expand Down
9 changes: 0 additions & 9 deletions gnovm/pkg/gnolang/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"sync/atomic"

"github.com/gnolang/gno/tm2/pkg/errors"
tmstore "github.com/gnolang/gno/tm2/pkg/store"
)

const (
Expand Down Expand Up @@ -366,12 +365,6 @@ func initStaticBlocks(store Store, ctx BlockNode, bn BlockNode) {

func doRecover(stack []BlockNode, n Node) {
if r := recover(); r != nil {
// Catch the out-of-gas exception and throw it
if exp, ok := r.(tmstore.OutOfGasException); ok {
exp.Descriptor = fmt.Sprintf("in preprocess: %v", r)
panic(exp)
}

if _, ok := r.(*PreprocessError); ok {
// re-panic directly if this is a PreprocessError already.
panic(r)
Expand All @@ -388,10 +381,8 @@ func doRecover(stack []BlockNode, n Node) {
var err error
rerr, ok := r.(error)
if ok {
// NOTE: gotuna/gorilla expects error exceptions.
err = errors.Wrap(rerr, loc.String())
} else {
// NOTE: gotuna/gorilla expects error exceptions.
err = fmt.Errorf("%s: %v", loc.String(), r)
}

Expand Down
2 changes: 1 addition & 1 deletion tm2/pkg/sdk/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature
defer func() {
if r := recover(); r != nil {
switch ex := r.(type) {
case store.OutOfGasException:
case store.OutOfGasError:
log := fmt.Sprintf(
"out of gas in location: %v; gasWanted: %d, gasUsed: %d",
ex.Descriptor, tx.Fee.GasWanted, newCtx.GasMeter().GasConsumed(),
Expand Down
2 changes: 1 addition & 1 deletion tm2/pkg/sdk/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ func (app *BaseApp) runTx(ctx Context, tx Tx) (result Result) {
defer func() {
if r := recover(); r != nil {
switch ex := r.(type) {
case store.OutOfGasException:
case store.OutOfGasError:
log := fmt.Sprintf(
"out of gas, gasWanted: %d, gasUsed: %d location: %v",
gasWanted,
Expand Down
4 changes: 2 additions & 2 deletions tm2/pkg/store/exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ type (
Gas = types.Gas
GasMeter = types.GasMeter
GasConfig = types.GasConfig
OutOfGasException = types.OutOfGasException
GasOverflowException = types.GasOverflowException
OutOfGasError = types.OutOfGasError
GasOverflowError = types.GasOverflowError
)

var (
Expand Down
22 changes: 15 additions & 7 deletions tm2/pkg/store/types/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,25 @@
// Gas measured by the SDK
type Gas = int64

// OutOfGasException defines an error thrown when an action results in out of gas.
type OutOfGasException struct {
// OutOfGasError defines an error thrown when an action results in out of gas.
type OutOfGasError struct {
Descriptor string
}

// GasOverflowException defines an error thrown when an action results gas consumption
func (oog OutOfGasError) Error() string {
return "out of gas in location: " + oog.Descriptor

Check warning on line 30 in tm2/pkg/store/types/gas.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/store/types/gas.go#L29-L30

Added lines #L29 - L30 were not covered by tests
}

// GasOverflowError defines an error thrown when an action results gas consumption
// unsigned integer overflow.
type GasOverflowException struct {
type GasOverflowError struct {
Descriptor string
}

func (oog GasOverflowError) Error() string {
return "gas overflow in location: " + oog.Descriptor

Check warning on line 40 in tm2/pkg/store/types/gas.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/store/types/gas.go#L39-L40

Added lines #L39 - L40 were not covered by tests
}

// GasMeter interface to track gas consumption
type GasMeter interface {
GasConsumed() Gas
Expand Down Expand Up @@ -88,13 +96,13 @@
}
consumed, ok := overflow.Add64(g.consumed, amount)
if !ok {
panic(GasOverflowException{descriptor})
panic(GasOverflowError{descriptor})

Check warning on line 99 in tm2/pkg/store/types/gas.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/store/types/gas.go#L99

Added line #L99 was not covered by tests
}
// consume gas even if out of gas.
// corollary, call (Did)ConsumeGas after consumption.
g.consumed = consumed
if consumed > g.limit {
panic(OutOfGasException{descriptor})
panic(OutOfGasError{descriptor})
}
}

Expand Down Expand Up @@ -139,7 +147,7 @@
func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) {
consumed, ok := overflow.Add64(g.consumed, amount)
if !ok {
panic(GasOverflowException{descriptor})
panic(GasOverflowError{descriptor})

Check warning on line 150 in tm2/pkg/store/types/gas.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/store/types/gas.go#L150

Added line #L150 was not covered by tests
}
g.consumed = consumed
}
Expand Down
Loading