From c6624d2b86e2eb18160797ff8a92635a08af83be Mon Sep 17 00:00:00 2001 From: Ardit Marku Date: Wed, 19 Jul 2023 11:49:00 +0300 Subject: [PATCH 1/3] ScriptExecutor should always populate env values even when the script errors --- fvm/script.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fvm/script.go b/fvm/script.go index 10bd5d68717..4b457a66e21 100644 --- a/fvm/script.go +++ b/fvm/script.go @@ -198,11 +198,13 @@ func (executor *scriptExecutor) executeScript() error { Source: executor.proc.Script, Arguments: executor.proc.Arguments, }, - common.ScriptLocation(executor.proc.ID)) + common.ScriptLocation(executor.proc.ID), + ) + populateErr := executor.output.PopulateEnvironmentValues(executor.env) if err != nil { return err } executor.output.Value = value - return executor.output.PopulateEnvironmentValues(executor.env) + return populateErr } From 574fb22f64c8dd2e60dc957bb3f8883f89195833 Mon Sep 17 00:00:00 2001 From: Ardit Marku Date: Wed, 19 Jul 2023 18:19:46 +0300 Subject: [PATCH 2/3] Append both errors in executeScript() with multierror --- fvm/script.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fvm/script.go b/fvm/script.go index 4b457a66e21..977ba7c0a42 100644 --- a/fvm/script.go +++ b/fvm/script.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/hashicorp/go-multierror" "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" @@ -206,5 +207,5 @@ func (executor *scriptExecutor) executeScript() error { } executor.output.Value = value - return populateErr + return multierror.Append(err, populateErr) } From 1aa6e3dc006a989c623a429439a73a857e0ac140 Mon Sep 17 00:00:00 2001 From: Ardit Marku Date: Thu, 20 Jul 2023 10:56:38 +0300 Subject: [PATCH 3/3] Add test cases for asserting the population of ProcedureOutput from script execution --- fvm/fvm_test.go | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ fvm/script.go | 4 +-- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 587df638ee9..e93c19c575a 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -2023,6 +2023,87 @@ func TestScriptAccountKeyMutationsFailure(t *testing.T) { ) } +func TestScriptExecutionLimit(t *testing.T) { + + t.Parallel() + + script := fvm.Script([]byte(` + pub fun main() { + var s: Int256 = 1024102410241024 + var i: Int256 = 0 + var a: Int256 = 7 + var b: Int256 = 5 + var c: Int256 = 2 + + while i < 150000 { + s = s * a + s = s / b + s = s / c + i = i + 1 + } + } + `)) + + bootstrapProcedureOptions := []fvm.BootstrapProcedureOption{ + fvm.WithTransactionFee(fvm.DefaultTransactionFees), + fvm.WithExecutionMemoryLimit(math.MaxUint32), + fvm.WithExecutionEffortWeights(map[common.ComputationKind]uint64{ + common.ComputationKindStatement: 1569, + common.ComputationKindLoop: 1569, + common.ComputationKindFunctionInvocation: 1569, + environment.ComputationKindGetValue: 808, + environment.ComputationKindCreateAccount: 2837670, + environment.ComputationKindSetValue: 765, + }), + fvm.WithExecutionMemoryWeights(meter.DefaultMemoryWeights), + fvm.WithMinimumStorageReservation(fvm.DefaultMinimumStorageReservation), + fvm.WithAccountCreationFee(fvm.DefaultAccountCreationFee), + fvm.WithStorageMBPerFLOW(fvm.DefaultStorageMBPerFLOW), + } + + t.Run("Exceeding computation limit", + newVMTest().withBootstrapProcedureOptions( + bootstrapProcedureOptions..., + ).withContextOptions( + fvm.WithTransactionFeesEnabled(true), + fvm.WithAccountStorageLimit(true), + fvm.WithComputationLimit(10000), + ).run( + func(t *testing.T, vm fvm.VM, chain flow.Chain, ctx fvm.Context, snapshotTree snapshot.SnapshotTree) { + scriptCtx := fvm.NewContextFromParent(ctx) + + _, output, err := vm.Run(scriptCtx, script, snapshotTree) + require.NoError(t, err) + require.Error(t, output.Err) + require.True(t, errors.IsComputationLimitExceededError(output.Err)) + require.ErrorContains(t, output.Err, "computation exceeds limit (10000)") + require.GreaterOrEqual(t, output.ComputationUsed, uint64(10000)) + require.GreaterOrEqual(t, output.MemoryEstimate, uint64(548020260)) + }, + ), + ) + + t.Run("Sufficient computation limit", + newVMTest().withBootstrapProcedureOptions( + bootstrapProcedureOptions..., + ).withContextOptions( + fvm.WithTransactionFeesEnabled(true), + fvm.WithAccountStorageLimit(true), + fvm.WithComputationLimit(20000), + ).run( + func(t *testing.T, vm fvm.VM, chain flow.Chain, ctx fvm.Context, snapshotTree snapshot.SnapshotTree) { + scriptCtx := fvm.NewContextFromParent(ctx) + + _, output, err := vm.Run(scriptCtx, script, snapshotTree) + require.NoError(t, err) + require.NoError(t, output.Err) + require.GreaterOrEqual(t, output.ComputationUsed, uint64(17955)) + require.GreaterOrEqual(t, output.MemoryEstimate, uint64(984017413)) + }, + ), + ) +} + func TestInteractionLimit(t *testing.T) { type testCase struct { name string diff --git a/fvm/script.go b/fvm/script.go index 977ba7c0a42..c979fb309f5 100644 --- a/fvm/script.go +++ b/fvm/script.go @@ -203,9 +203,9 @@ func (executor *scriptExecutor) executeScript() error { ) populateErr := executor.output.PopulateEnvironmentValues(executor.env) if err != nil { - return err + return multierror.Append(err, populateErr) } executor.output.Value = value - return multierror.Append(err, populateErr) + return populateErr }