From abb74027b8101036cc750c2e5ae12a6958953ea2 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 26 Jul 2023 15:56:58 -0700 Subject: [PATCH 1/2] Wrap host env errors with external errors --- runtime/environment.go | 20 +++- runtime/events.go | 2 +- runtime/interpreter/errors.go | 20 ++++ runtime/runtime.go | 1 + runtime/runtime_test.go | 160 ++++++++++++++++++++++++++++++++ runtime/stdlib/account.go | 42 ++++----- runtime/stdlib/block.go | 4 +- runtime/stdlib/hashalgorithm.go | 2 +- runtime/stdlib/log.go | 2 +- runtime/stdlib/publickey.go | 7 +- runtime/stdlib/random.go | 2 +- runtime/storage.go | 4 +- 12 files changed, 234 insertions(+), 32 deletions(-) diff --git a/runtime/environment.go b/runtime/environment.go index 631d8456db..24ce27a85d 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -431,6 +431,10 @@ func (e *interpreterEnvironment) newLocationHandler() sema.LocationHandlerFunc { errors.WrapPanic(func() { res, err = e.runtimeInterface.ResolveLocation(identifiers, location) }) + + if err != nil { + err = interpreter.WrappedExternalError(err) + } return } } @@ -560,6 +564,11 @@ func (e *interpreterEnvironment) getProgram( if panicErr != nil { return nil, panicErr } + + if err != nil { + err = interpreter.WrappedExternalError(err) + } + return }) }) @@ -577,6 +586,11 @@ func (e *interpreterEnvironment) getCode(location common.Location) (code []byte, code, err = e.runtimeInterface.GetCode(location) }) } + + if err != nil { + err = interpreter.WrappedExternalError(err) + } + return } @@ -745,6 +759,10 @@ func (e *interpreterEnvironment) newUUIDHandler() interpreter.UUIDHandlerFunc { errors.WrapPanic(func() { uuid, err = e.runtimeInterface.GenerateUUID() }) + + if err != nil { + err = interpreter.WrappedExternalError(err) + } return } } @@ -941,7 +959,7 @@ func (e *interpreterEnvironment) newOnMeterComputation() interpreter.OnMeterComp err = e.runtimeInterface.MeterComputation(compKind, intensity) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } } } diff --git a/runtime/events.go b/runtime/events.go index 86e01ebe37..4738902bc8 100644 --- a/runtime/events.go +++ b/runtime/events.go @@ -81,6 +81,6 @@ func emitEventFields( err = emitEvent(exportedEvent) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } } diff --git a/runtime/interpreter/errors.go b/runtime/interpreter/errors.go index 1444ef36ae..5b57ea8fe6 100644 --- a/runtime/interpreter/errors.go +++ b/runtime/interpreter/errors.go @@ -20,6 +20,7 @@ package interpreter import ( "fmt" + "runtime" "strings" "github.com/onflow/cadence/runtime/ast" @@ -988,3 +989,22 @@ func (RecursiveTransferError) IsUserError() {} func (RecursiveTransferError) Error() string { return "recursive transfer of value" } + +func WrappedExternalError(err error) error { + switch err := err.(type) { + case + // If the error is a go-runtime error, don't wrap. + // These are crashers. + runtime.Error, + + // If the error is already a cadence error, then avoid redundant wrapping. + errors.InternalError, + errors.UserError, + errors.ExternalError, + Error: + return err + + default: + return errors.NewExternalError(err) + } +} diff --git a/runtime/runtime.go b/runtime/runtime.go index 1ed4f9d6d7..d34514f59f 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -263,6 +263,7 @@ func getWrappedError(recovered any, location Location, codesAndPrograms codesAnd return newError(err, location, codesAndPrograms) } } + func (r *interpreterRuntime) NewScriptExecutor( script Script, context Context, diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 1f6da1c264..81825326cc 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9000,3 +9000,163 @@ func TestRuntimeReturnDestroyedOptional(t *testing.T) { require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) } + +func TestRuntimeComputationMeteringError(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + t.Run("regular error returned", func(t *testing.T) { + t.Parallel() + + script := []byte(` + access(all) fun foo() {} + + pub fun main() { + foo() + } + `) + + runtimeInterface := &testRuntimeInterface{ + storage: newTestLedger(nil, nil), + meterComputation: func(compKind common.ComputationKind, intensity uint) error { + return fmt.Errorf("computation limit exceeded") + }, + } + + _, err := runtime.ExecuteScript( + Script{ + Source: script, + }, + Context{ + Interface: runtimeInterface, + Location: common.ScriptLocation{}, + }, + ) + + require.Error(t, err) + + // Returned error MUST be an external error. + // It can NOT be an internal error. + assertRuntimeErrorIsExternalError(t, err) + }) + + t.Run("regular error panicked", func(t *testing.T) { + t.Parallel() + + script := []byte(` + access(all) fun foo() {} + + pub fun main() { + foo() + } + `) + + runtimeInterface := &testRuntimeInterface{ + storage: newTestLedger(nil, nil), + meterComputation: func(compKind common.ComputationKind, intensity uint) error { + panic(fmt.Errorf("computation limit exceeded")) + }, + } + + _, err := runtime.ExecuteScript( + Script{ + Source: script, + }, + Context{ + Interface: runtimeInterface, + Location: common.ScriptLocation{}, + }, + ) + + require.Error(t, err) + + // Returned error MUST be an external error. + // It can NOT be an internal error. + assertRuntimeErrorIsExternalError(t, err) + }) + + t.Run("go runtime error panicked", func(t *testing.T) { + t.Parallel() + + script := []byte(` + access(all) fun foo() {} + + pub fun main() { + foo() + } + `) + + runtimeInterface := &testRuntimeInterface{ + storage: newTestLedger(nil, nil), + meterComputation: func(compKind common.ComputationKind, intensity uint) error { + // Cause a runtime error + var x any = "hello" + _ = x.(int) + return nil + }, + } + + _, err := runtime.ExecuteScript( + Script{ + Source: script, + }, + Context{ + Interface: runtimeInterface, + Location: common.ScriptLocation{}, + }, + ) + + require.Error(t, err) + + // Returned error MUST be an internal error. + assertRuntimeErrorIsInternalError(t, err) + }) + + t.Run("go runtime error returned", func(t *testing.T) { + t.Parallel() + + script := []byte(` + access(all) fun foo() {} + + pub fun main() { + foo() + } + `) + + runtimeInterface := &testRuntimeInterface{ + storage: newTestLedger(nil, nil), + meterComputation: func(compKind common.ComputationKind, intensity uint) (err error) { + // Cause a runtime error. Catch it and return. + var x any = "hello" + defer func() { + if r := recover(); r != nil { + if r, ok := r.(error); ok { + err = r + } + } + }() + + _ = x.(int) + + return + }, + } + + _, err := runtime.ExecuteScript( + Script{ + Source: script, + }, + Context{ + Interface: runtimeInterface, + Location: common.ScriptLocation{}, + }, + ) + + require.Error(t, err) + + // Returned error MUST be an internal error. + assertRuntimeErrorIsInternalError(t, err) + }) +} diff --git a/runtime/stdlib/account.go b/runtime/stdlib/account.go index cc01bb4c23..55825d75ad 100644 --- a/runtime/stdlib/account.go +++ b/runtime/stdlib/account.go @@ -128,7 +128,7 @@ func NewAuthAccountConstructor(creator AccountCreator) StandardLibraryValue { address, err = creator.CreateAccount(payerAddress) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return @@ -345,7 +345,7 @@ func newAccountBalanceGetFunction( balance, err = provider.GetAccountBalance(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return @@ -374,7 +374,7 @@ func newAccountAvailableBalanceGetFunction( balance, err = provider.GetAccountAvailableBalance(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return @@ -413,7 +413,7 @@ func newStorageUsedGetFunction( capacity, err = provider.GetStorageUsed(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return capacity }, @@ -452,7 +452,7 @@ func newStorageCapacityGetFunction( capacity, err = provider.GetStorageCapacity(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return capacity }, @@ -498,7 +498,7 @@ func newAddPublicKeyFunction( err = handler.AddEncodedAccountKey(address, publicKey) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } handler.EmitEvent( @@ -546,7 +546,7 @@ func newRemovePublicKeyFunction( publicKey, err = handler.RevokeEncodedAccountKey(address, index.ToInt(invocation.LocationRange)) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } inter := invocation.Interpreter @@ -620,7 +620,7 @@ func newAccountKeysAddFunction( accountKey, err = handler.AddAccountKey(address, publicKey, hashAlgo, weight) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } handler.EmitEvent( @@ -691,7 +691,7 @@ func newAccountKeysGetFunction( }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } // Here it is expected the host function to return a nil key, if a key is not found at the given index. @@ -774,7 +774,7 @@ func newAccountKeysForEachFunction( }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } var accountKey *AccountKey @@ -784,7 +784,7 @@ func newAccountKeysForEachFunction( accountKey, err = provider.GetAccountKey(address, int(index)) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } // Here it is expected the host function to return a nil key, if a key is not found at the given index. @@ -838,7 +838,7 @@ func newAccountKeysCountGetter( if err != nil { // The provider might not be able to fetch the number of account keys // e.g. when the account does not exist - panic(err) + panic(interpreter.WrappedExternalError(err)) } return count @@ -882,7 +882,7 @@ func newAccountKeysRevokeFunction( accountKey, err = handler.RevokeAccountKey(address, index) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } // Here it is expected the host function to return a nil key, if a key is not found at the given index. @@ -1250,7 +1250,7 @@ func newAccountContractsGetNamesFunction( names, err = provider.GetAccountContractNames(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } values := make([]interpreter.Value, len(names)) @@ -1315,7 +1315,7 @@ func newAccountContractsGetFunction( code, err = provider.GetAccountContractCode(location) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } if len(code) > 0 { @@ -1381,7 +1381,7 @@ func newAccountContractsBorrowFunction( code, err = handler.GetAccountContractCode(location) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } if len(code) == 0 { return interpreter.Nil @@ -1821,7 +1821,7 @@ func updateAccountContractCode( err = handler.UpdateAccountContractCode(location, code) }) if err != nil { - return err + return interpreter.WrappedExternalError(err) } if createContract { @@ -1978,7 +1978,7 @@ func newAuthAccountContractsRemoveFunction( code, err = handler.GetAccountContractCode(location) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } // Only remove the contract code, remove the contract value, and emit an event, @@ -2006,7 +2006,7 @@ func newAuthAccountContractsRemoveFunction( err = handler.RemoveAccountContractCode(location) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } // NOTE: the contract recording function delays the write @@ -2534,7 +2534,7 @@ func issueStorageCapabilityController( capabilityID, err = idGenerator.GenerateAccountID(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } if capabilityID == 0 { panic(errors.NewUnexpectedError("invalid zero account ID")) @@ -2616,7 +2616,7 @@ func issueAccountCapabilityController( capabilityID, err = idGenerator.GenerateAccountID(address) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } if capabilityID == 0 { panic(errors.NewUnexpectedError("invalid zero account ID")) diff --git a/runtime/stdlib/block.go b/runtime/stdlib/block.go index 19c9a5cab6..c2d77ef022 100644 --- a/runtime/stdlib/block.go +++ b/runtime/stdlib/block.go @@ -180,7 +180,7 @@ func getBlockAtHeight( block, exists, err = provider.GetBlockAtHeight(height) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return block, exists @@ -205,7 +205,7 @@ func NewGetCurrentBlockFunction(provider CurrentBlockProvider) StandardLibraryVa height, err = provider.GetCurrentBlockHeight() }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } block, exists := getBlockAtHeight( diff --git a/runtime/stdlib/hashalgorithm.go b/runtime/stdlib/hashalgorithm.go index 0e36b20b4b..74a428123f 100644 --- a/runtime/stdlib/hashalgorithm.go +++ b/runtime/stdlib/hashalgorithm.go @@ -155,7 +155,7 @@ func hash( result, err = hasher.Hash(data, tag, hashAlgorithm) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return interpreter.ByteSliceToByteArrayValue(inter, result) } diff --git a/runtime/stdlib/log.go b/runtime/stdlib/log.go index c794c2e07d..3e5a92ad2c 100644 --- a/runtime/stdlib/log.go +++ b/runtime/stdlib/log.go @@ -64,7 +64,7 @@ func NewLogFunction(logger Logger) StandardLibraryValue { err = logger.ProgramLog(message) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return interpreter.Void diff --git a/runtime/stdlib/publickey.go b/runtime/stdlib/publickey.go index 6ea9ae53d4..29d4bad749 100644 --- a/runtime/stdlib/publickey.go +++ b/runtime/stdlib/publickey.go @@ -67,6 +67,9 @@ func newPublicKeyValidationHandler(validator PublicKeyValidator) interpreter.Pub errors.WrapPanic(func() { err = validator.ValidatePublicKey(publicKey) }) + if err != nil { + err = interpreter.WrappedExternalError(err) + } return err } } @@ -290,7 +293,7 @@ func newPublicKeyVerifySignatureFunction( }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return interpreter.AsBoolValue(valid) @@ -343,7 +346,7 @@ func newPublicKeyVerifyPoPFunction( valid, err = verifier.BLSVerifyPOP(publicKey, signature) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return interpreter.AsBoolValue(valid) }, diff --git a/runtime/stdlib/random.go b/runtime/stdlib/random.go index 7967fe28c4..c37b0abf44 100644 --- a/runtime/stdlib/random.go +++ b/runtime/stdlib/random.go @@ -59,7 +59,7 @@ func NewUnsafeRandomFunction(generator UnsafeRandomGenerator) StandardLibraryVal rand, err = generator.UnsafeRandom() }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } return rand }, diff --git a/runtime/storage.go b/runtime/storage.go index 53ee07c2e4..dd4618a747 100644 --- a/runtime/storage.go +++ b/runtime/storage.go @@ -102,7 +102,7 @@ func (s *Storage) GetStorageMap( data, err = s.Ledger.GetValue(key.Address[:], []byte(key.Key)) }) if err != nil { - panic(err) + panic(interpreter.WrappedExternalError(err)) } dataLength := len(data) @@ -258,7 +258,7 @@ func (s *Storage) commitNewStorageMaps() error { ) }) if err != nil { - return err + return interpreter.WrappedExternalError(err) } } From fe2ba6312d4434c2e45b0118202c8a17b50a230d Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 27 Jul 2023 09:15:35 -0700 Subject: [PATCH 2/2] Update go.cap --- go.cap | 1 + 1 file changed, 1 insertion(+) diff --git a/go.cap b/go.cap index 34dba49742..bb341c9fa6 100644 --- a/go.cap +++ b/go.cap @@ -10,3 +10,4 @@ github.com/stretchr/testify/require (network) github.com/texttheater/golang-levenshtein/levenshtein (file) github.com/zeebo/blake3/internal/consts (file) golang.org/x/xerrors (runtime) +github.com/onflow/cadence/runtime/interpreter (runtime)