Merge pull request #5216 from onflow/gregor/evm/runtime-error-handling

[EVM] Handle EVM errors

Makefile

@@ -207,6 +207,7 @@ generate-mocks: install-mock-generators
 	mockery --name '.*' --dir=./cmd/util/ledger/reporters --case=underscore --output="./cmd/util/ledger/reporters/mock" --outpkg="mock"
 	mockery --name 'Storage' --dir=module/executiondatasync/tracker --case=underscore --output="module/executiondatasync/tracker/mock" --outpkg="mocktracker"
 	mockery --name 'ScriptExecutor' --dir=module/execution --case=underscore --output="module/execution/mock" --outpkg="mock"
+	mockery --name 'StorageSnapshot' --dir=fvm/storage/snapshot --case=underscore --output="fvm/storage/snapshot/mock" --outpkg="mock"
 
 	#temporarily make insecure/ a non-module to allow mockery to create mocks
 	mv insecure/go.mod insecure/go2.mod

fvm/errors/errors.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/onflow/cadence/runtime"
+	"github.com/onflow/cadence/runtime/errors"
 )
 
 type Unwrappable interface {
@@ -213,3 +214,29 @@ func NewEventEncodingError(err error) CodedError {
 		ErrCodeEventEncodingError,
 		"error while encoding emitted event: %w ", err)
 }
+
+// EVMError needs to satisfy the user error interface
+// in order for Cadence to correctly handle the error
+var _ errors.UserError = &(EVMError{})
+
+type EVMError struct {
+	CodedError
+}
+
+func (e EVMError) IsUserError() {}
+
+// NewEVMError constructs a new CodedError which captures a
+// collection of errors provided by (non-fatal) evm runtime.
+func NewEVMError(err error) EVMError {
+	return EVMError{
+		WrapCodedError(
+			ErrEVMExecutionError,
+			err,
+			"evm runtime error"),
+	}
+}
+
+// IsEVMError returns true if error is an EVM error
+func IsEVMError(err error) bool {
+	return HasErrorCode(err, ErrEVMExecutionError)
+}

fvm/errors/execution.go

@@ -289,17 +289,3 @@ func NewInvalidInternalStateAccessError(
 		opType,
 		id)
 }
-
-// NewEVMError constructs a new CodedError which captures a
-// collection of errors provided by (non-fatal) evm runtime.
-func NewEVMError(err error) CodedError {
-	return WrapCodedError(
-		ErrEVMExecutionError,
-		err,
-		"evm runtime error")
-}
-
-// IsEVMError returns true if error is an EVM error
-func IsEVMError(err error) bool {
-	return HasErrorCode(err, ErrEVMExecutionError)
-}

fvm/evm/types/errors.go

@@ -16,8 +16,9 @@ var (
 	// left in the context of flow transaction to execute the evm operation.
 	ErrInsufficientComputation = errors.New("insufficient computation")
 
-	// unauthorized method call, usually emited when calls are called on EOA accounts
+	// ErrUnAuthroizedMethodCall method call, usually emited when calls are called on EOA accounts
 	ErrUnAuthroizedMethodCall = errors.New("unauthroized method call")
+
 	// ErrInsufficientTotalSupply is returned when flow token
 	// is withdraw request is there but not enough balance is on EVM vault
 	// this should never happen but its a saftey measure to protect Flow against EVM issues.
@@ -117,7 +118,7 @@ func IsAStateError(err error) bool {
 	return errors.As(err, &StateError{})
 }
 
-// FatalError is user for any error that is not user related and something
+// FatalError is used for any error that is not user related and something
 // unusual has happend. Usually we stop the node when this happens
 // given it might have a non-deterministic root.
 type FatalError struct {

fvm/fvm_test.go

@@ -8,27 +8,29 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/onflow/flow-go/fvm/evm/stdlib"
-
 	"github.com/onflow/cadence"
 	"github.com/onflow/cadence/encoding/ccf"
 	jsoncdc "github.com/onflow/cadence/encoding/json"
 	"github.com/onflow/cadence/runtime"
 	"github.com/onflow/cadence/runtime/common"
+	cadenceErrors "github.com/onflow/cadence/runtime/errors"
 	"github.com/onflow/cadence/runtime/tests/utils"
+	mockery "github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"github.com/onflow/flow-go/crypto"
-
 	"github.com/onflow/flow-go/engine/execution/testutil"
 	exeUtils "github.com/onflow/flow-go/engine/execution/utils"
 	"github.com/onflow/flow-go/fvm"
 	fvmCrypto "github.com/onflow/flow-go/fvm/crypto"
 	"github.com/onflow/flow-go/fvm/environment"
-	errors "github.com/onflow/flow-go/fvm/errors"
+	"github.com/onflow/flow-go/fvm/errors"
+	"github.com/onflow/flow-go/fvm/evm/stdlib"
+	"github.com/onflow/flow-go/fvm/evm/types"
 	"github.com/onflow/flow-go/fvm/meter"
 	reusableRuntime "github.com/onflow/flow-go/fvm/runtime"
 	"github.com/onflow/flow-go/fvm/storage/snapshot"
+	"github.com/onflow/flow-go/fvm/storage/snapshot/mock"
 	"github.com/onflow/flow-go/fvm/storage/testutils"
 	"github.com/onflow/flow-go/fvm/systemcontracts"
 	"github.com/onflow/flow-go/model/flow"
@@ -3013,4 +3015,94 @@ func TestEVM(t *testing.T) {
 			))
 		}),
 	)
+
+	// this test makes sure the execution error is correctly handled and returned as a correct type
+	t.Run("execution reverted", newVMTest().
+		withBootstrapProcedureOptions(fvm.WithSetupEVMEnabled(true)).
+		withContextOptions(fvm.WithEVMEnabled(true)).
+		run(func(
+			t *testing.T,
+			vm fvm.VM,
+			chain flow.Chain,
+			ctx fvm.Context,
+			snapshotTree snapshot.SnapshotTree,
+		) {
+			script := fvm.Script([]byte(fmt.Sprintf(`
+				import EVM from %s
+				
+				pub fun main() {
+					let bal = EVM.Balance(flow: 1.0);
+					let acc <- EVM.createBridgedAccount();
+					// withdraw insufficient balance
+					destroy acc.withdraw(balance: bal);
+					destroy acc;
+				}
+			`, chain.ServiceAddress().HexWithPrefix())))
+
+			_, output, err := vm.Run(ctx, script, snapshotTree)
+
+			require.NoError(t, err)
+			require.Error(t, output.Err)
+			require.True(t, errors.IsEVMError(output.Err))
+
+			// make sure error is not treated as internal error by Cadence
+			var internal cadenceErrors.InternalError
+			require.False(t, errors.As(output.Err, &internal))
+		}),
+	)
+
+	// this test makes sure the EVM error is correctly returned as an error and has a correct type
+	// we have implemented a snapshot wrapper to return an error from the EVM
+	t.Run("internal evm error handling", newVMTest().
+		withBootstrapProcedureOptions(fvm.WithSetupEVMEnabled(true)).
+		withContextOptions(fvm.WithEVMEnabled(true)).
+		run(func(
+			t *testing.T,
+			vm fvm.VM,
+			chain flow.Chain,
+			ctx fvm.Context,
+			snapshotTree snapshot.SnapshotTree,
+		) {
+
+			tests := []struct {
+				err        error
+				errChecker func(error) bool
+			}{{
+				types.ErrNotImplemented,
+				types.IsAFatalError,
+			}, {
+				types.NewStateError(fmt.Errorf("test state error")),
+				types.IsAStateError,
+			}}
+
+			for _, e := range tests {
+				// this mock will return an error we provide with the test once it starts to access address allocator registers
+				// that is done to make sure the error is coming out of EVM execution
+				errStorage := &mock.StorageSnapshot{}
+				errStorage.
+					On("Get", mockery.AnythingOfType("flow.RegisterID")).
+					Return(func(id flow.RegisterID) (flow.RegisterValue, error) {
+						if id.Key == "AddressAllocator" {
+							return nil, e.err
+						}
+						return snapshotTree.Get(id)
+					})
+
+				script := fvm.Script([]byte(fmt.Sprintf(`
+					import EVM from %s
+					
+					pub fun main() {
+						destroy <- EVM.createBridgedAccount();
+					}
+				`, chain.ServiceAddress().HexWithPrefix())))
+
+				_, output, err := vm.Run(ctx, script, errStorage)
+
+				require.NoError(t, output.Err)
+				require.Error(t, err)
+				// make sure error it's the right type of error
+				require.True(t, e.errChecker(err), "error is not of the right type")
+			}
+		}),
+	)
 }