From 1a818a0d4a2bb36cfbc2d160dcb5b849c960bcd2 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 21 Apr 2023 11:32:00 -0700 Subject: [PATCH 1/6] Invalidate nested resources upon the move of the outer resource --- runtime/attachments_test.go | 2 +- runtime/interpreter/interpreter.go | 64 +++++-- runtime/interpreter/value.go | 117 ++---------- runtime/tests/interpreter/attachments_test.go | 4 +- runtime/tests/interpreter/reference_test.go | 174 ++++++++++++++++++ 5 files changed, 240 insertions(+), 121 deletions(-) diff --git a/runtime/attachments_test.go b/runtime/attachments_test.go index 2b807cfc0e..4d8dd0be8b 100644 --- a/runtime/attachments_test.go +++ b/runtime/attachments_test.go @@ -223,7 +223,7 @@ func TestAccountAttachmentExportFailure(t *testing.T) { }, ) require.Error(t, err) - require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) } func TestAccountAttachmentExport(t *testing.T) { diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 0b0c7be0dc..c76059924f 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -4657,25 +4657,63 @@ func (interpreter *Interpreter) trackReferencedResourceKindedValue( values[value] = struct{}{} } -func (interpreter *Interpreter) updateReferencedResource( - currentStorageID atree.StorageID, - newStorageID atree.StorageID, - updateFunc func(value ReferenceTrackedResourceKindedValue), -) { - values := interpreter.SharedState.referencedResourceKindedValues[currentStorageID] +func (interpreter *Interpreter) invalidateReferencedResources(value Value) { + // skip non-resource typed values + if !value.IsResourceKinded(interpreter) { + return + } + + var storageID atree.StorageID + + switch value := value.(type) { + case *CompositeValue: + value.ForEachField(interpreter, func(_ string, fieldValue Value) { + interpreter.invalidateReferencedResources(fieldValue) + }) + storageID = value.StorageID() + case *DictionaryValue: + value.Iterate(interpreter, func(_, value Value) (resume bool) { + interpreter.invalidateReferencedResources(value) + return true + }) + storageID = value.StorageID() + case *ArrayValue: + value.Iterate(interpreter, func(element Value) (resume bool) { + interpreter.invalidateReferencedResources(element) + return true + }) + storageID = value.StorageID() + case *SomeValue: + interpreter.invalidateReferencedResources(value.value) + return + default: + // skip non-container typed values. + return + } + + values := interpreter.SharedState.referencedResourceKindedValues[storageID] if values == nil { return } + for value := range values { //nolint:maprange - updateFunc(value) + switch value := value.(type) { + case *CompositeValue: + value.dictionary = nil + case *DictionaryValue: + value.dictionary = nil + case *ArrayValue: + value.array = nil + default: + panic(errors.NewUnreachableError()) + } } - // If the move is to a new location, then the resources are already cleared via the update function above. - // So no need to track those stale resources anymore. - if newStorageID != currentStorageID { - interpreter.SharedState.referencedResourceKindedValues[newStorageID] = values - interpreter.SharedState.referencedResourceKindedValues[currentStorageID] = nil - } + // The old resource instances are already cleared/invalidated above. + // So no need to track those stale resources anymore. We will not need to update/clear them again. + // Therefore, remove them from the mapping. + // This is only to allow GC. No impact to the behavior. + delete(interpreter.SharedState.referencedResourceKindedValues, storageID) } // startResourceTracking starts tracking the life-span of a resource. diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 6f0931fbb9..82176aa2d5 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -1608,8 +1608,6 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan v.checkInvalidatedResourceUse(interpreter, locationRange) } - storageID := v.StorageID() - if config.TracingEnabled { startTime := time.Now() @@ -1631,26 +1629,11 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan v.isDestroyed = true + interpreter.invalidateReferencedResources(v) + if config.InvalidatedResourceValidationEnabled { v.array = nil } - - interpreter.updateReferencedResource( - storageID, - storageID, - func(value ReferenceTrackedResourceKindedValue) { - arrayValue, ok := value.(*ArrayValue) - if !ok { - panic(errors.NewUnreachableError()) - } - - arrayValue.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - arrayValue.array = nil - } - }, - ) } func (v *ArrayValue) IsDestroyed() bool { @@ -2474,28 +2457,14 @@ func (v *ArrayValue) Transfer( // This allows raising an error when the resource array is attempted // to be transferred/moved again (see beginning of this function) + interpreter.invalidateReferencedResources(v) + if config.InvalidatedResourceValidationEnabled { v.array = nil } else { v.array = array res = v } - - newStorageID := array.StorageID() - - interpreter.updateReferencedResource( - currentStorageID, - newStorageID, - func(value ReferenceTrackedResourceKindedValue) { - arrayValue, ok := value.(*ArrayValue) - if !ok { - panic(errors.NewUnreachableError()) - } - - // Any kind of move would invalidate the references. - arrayValue.array = nil - }, - ) } if res == nil { @@ -14046,8 +14015,6 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio v.checkInvalidatedResourceUse(interpreter, locationRange) } - storageID := v.StorageID() - if config.TracingEnabled { startTime := time.Now() @@ -14108,26 +14075,11 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio v.isDestroyed = true + interpreter.invalidateReferencedResources(v) + if config.InvalidatedResourceValidationEnabled { v.dictionary = nil } - - interpreter.updateReferencedResource( - storageID, - storageID, - func(value ReferenceTrackedResourceKindedValue) { - compositeValue, ok := value.(*CompositeValue) - if !ok { - panic(errors.NewUnreachableError()) - } - - compositeValue.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - compositeValue.dictionary = nil - } - }, - ) } func (v *CompositeValue) getBuiltinMember(interpreter *Interpreter, locationRange LocationRange, name string) Value { @@ -14856,28 +14808,14 @@ func (v *CompositeValue) Transfer( // This allows raising an error when the resource is attempted // to be transferred/moved again (see beginning of this function) + interpreter.invalidateReferencedResources(v) + if config.InvalidatedResourceValidationEnabled { v.dictionary = nil } else { v.dictionary = dictionary res = v } - - newStorageID := dictionary.StorageID() - - interpreter.updateReferencedResource( - currentStorageID, - newStorageID, - func(value ReferenceTrackedResourceKindedValue) { - compositeValue, ok := value.(*CompositeValue) - if !ok { - panic(errors.NewUnreachableError()) - } - - // Any kind of move would invalidate the references. - compositeValue.dictionary = nil - }, - ) } if res == nil { @@ -15486,8 +15424,6 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati v.checkInvalidatedResourceUse(interpreter, locationRange) } - storageID := v.StorageID() - if config.TracingEnabled { startTime := time.Now() @@ -15512,26 +15448,11 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati v.isDestroyed = true + interpreter.invalidateReferencedResources(v) + if config.InvalidatedResourceValidationEnabled { v.dictionary = nil } - - interpreter.updateReferencedResource( - storageID, - storageID, - func(value ReferenceTrackedResourceKindedValue) { - dictionaryValue, ok := value.(*DictionaryValue) - if !ok { - panic(errors.NewUnreachableError()) - } - - dictionaryValue.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - dictionaryValue.dictionary = nil - } - }, - ) } func (v *DictionaryValue) ForEachKey( @@ -16308,28 +16229,14 @@ func (v *DictionaryValue) Transfer( // This allows raising an error when the resource array is attempted // to be transferred/moved again (see beginning of this function) + interpreter.invalidateReferencedResources(v) + if config.InvalidatedResourceValidationEnabled { v.dictionary = nil } else { v.dictionary = dictionary res = v } - - newStorageID := dictionary.StorageID() - - interpreter.updateReferencedResource( - currentStorageID, - newStorageID, - func(value ReferenceTrackedResourceKindedValue) { - dictionaryValue, ok := value.(*DictionaryValue) - if !ok { - panic(errors.NewUnreachableError()) - } - - // Any kind of move would invalidate the references. - dictionaryValue.dictionary = nil - }, - ) } if res == nil { diff --git a/runtime/tests/interpreter/attachments_test.go b/runtime/tests/interpreter/attachments_test.go index 216afb1ddb..1f4892eacd 100644 --- a/runtime/tests/interpreter/attachments_test.go +++ b/runtime/tests/interpreter/attachments_test.go @@ -1624,7 +1624,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { ) _, err := inter.Invoke("test") - require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) }) t.Run("nested", func(t *testing.T) { @@ -1748,7 +1748,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { ) _, err := inter.Invoke("test") - require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) }) t.Run("self reference", func(t *testing.T) { diff --git a/runtime/tests/interpreter/reference_test.go b/runtime/tests/interpreter/reference_test.go index f13ab2f01b..222948bc60 100644 --- a/runtime/tests/interpreter/reference_test.go +++ b/runtime/tests/interpreter/reference_test.go @@ -1147,6 +1147,180 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { RequireError(t, err) require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) }) + + t.Run("nested resource in composite", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + resource Foo { + let id: UInt8 // non resource typed field + let bar: @Bar // resource typed field + init() { + self.id = 1 + self.bar <-create Bar() + } + destroy() { + destroy self.bar + } + } + + resource Bar { + let baz: @Baz + init() { + self.baz <-create Baz() + } + destroy() { + destroy self.baz + } + } + + resource Baz { + let id: UInt8 + init() { + self.id = 1 + } + } + + pub fun main() { + var foo <- create Foo() + + // Get a reference to the inner resource. + // Function call is just to trick the checker. + var bazRef = getRef(&foo.bar.baz as &Baz) + + // Move the outer resource + var foo2 <- foo + + // Access the moved resource + bazRef.id + + destroy foo2 + } + + pub fun getRef(_ ref: &Baz): &Baz { + return ref + } + `, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + }) + + t.Run("nested resource in dictionary", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + resource Foo {} + + pub fun main() { + var dict <- {"levelOne": <- {"levelTwo": <- create Foo()}} + + // Get a reference to the inner resource. + // Function call is just to trick the checker. + var dictRef = getRef(&dict["levelOne"] as &{String: Foo}?)! + + // Move the outer resource + var dict2 <- dict + + // Access the inner moved resource + var fooRef = &dictRef["levelTwo"] as &Foo? + + destroy dict2 + } + + pub fun getRef(_ ref: &{String: Foo}?): &{String: Foo}? { + return ref + } + `, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + }) + + t.Run("nested resource in array", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + resource Foo {} + + pub fun main() { + var array <- [<-[<- create Foo()]] + + // Get a reference to the inner resource. + // Function call is just to trick the checker. + var arrayRef = getRef(&array[0] as &[Foo]) + + // Move the outer resource + var array2 <- array + + // Access the inner moved resource + var fooRef = &arrayRef[0] as &Foo + + destroy array2 + } + + pub fun getRef(_ ref: &[Foo]): &[Foo] { + return ref + } + `, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + }) + + t.Run("nested optional resource", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + resource Foo { + let optionalBar: @Bar? + init() { + self.optionalBar <-create Bar() + } + destroy() { + destroy self.optionalBar + } + } + + resource Bar { + let id: UInt8 + init() { + self.id = 1 + } + } + + pub fun main() { + var foo <- create Foo() + + // Get a reference to the inner resource. + // Function call is just to trick the checker. + var barRef = getRef(&foo.optionalBar as &Bar?) + + // Move the outer resource + var foo2 <- foo + + // Access the moved resource + barRef!.id + + destroy foo2 + } + + pub fun getRef(_ ref: &Bar?): &Bar? { + return ref + } + `, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + }) } func TestInterpretResourceReferenceInvalidationOnDestroy(t *testing.T) { From 1099a478791fd0f5ba19ee86e8a264f8bed34ec1 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 24 Jul 2023 12:43:10 -0700 Subject: [PATCH 2/6] Update tests --- runtime/tests/interpreter/reference_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/runtime/tests/interpreter/reference_test.go b/runtime/tests/interpreter/reference_test.go index a28fb1d2c4..ee9559c14c 100644 --- a/runtime/tests/interpreter/reference_test.go +++ b/runtime/tests/interpreter/reference_test.go @@ -1203,7 +1203,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { } } - pub fun main() { + fun main() { var foo <- create Foo() // Get a reference to the inner resource. @@ -1219,7 +1219,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { destroy foo2 } - pub fun getRef(_ ref: &Baz): &Baz { + fun getRef(_ ref: &Baz): &Baz { return ref } `, @@ -1236,7 +1236,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { inter := parseCheckAndInterpret(t, ` resource Foo {} - pub fun main() { + fun main() { var dict <- {"levelOne": <- {"levelTwo": <- create Foo()}} // Get a reference to the inner resource. @@ -1252,7 +1252,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { destroy dict2 } - pub fun getRef(_ ref: &{String: Foo}?): &{String: Foo}? { + fun getRef(_ ref: &{String: Foo}?): &{String: Foo}? { return ref } `, @@ -1269,7 +1269,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { inter := parseCheckAndInterpret(t, ` resource Foo {} - pub fun main() { + fun main() { var array <- [<-[<- create Foo()]] // Get a reference to the inner resource. @@ -1285,7 +1285,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { destroy array2 } - pub fun getRef(_ ref: &[Foo]): &[Foo] { + fun getRef(_ ref: &[Foo]): &[Foo] { return ref } `, @@ -1317,7 +1317,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { } } - pub fun main() { + fun main() { var foo <- create Foo() // Get a reference to the inner resource. @@ -1333,7 +1333,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { destroy foo2 } - pub fun getRef(_ ref: &Bar?): &Bar? { + fun getRef(_ ref: &Bar?): &Bar? { return ref } `, From b93a113e62b02c850e33b2d268f0988a79731fc7 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 24 Jul 2023 14:31:49 -0700 Subject: [PATCH 3/6] Mark resources as destroyed during invalidation --- runtime/attachments_test.go | 2 +- runtime/interpreter/interpreter.go | 13 ++++++++----- runtime/interpreter/value.go | 12 ++++++------ runtime/interpreter/value_function.go | 7 ++----- runtime/runtime_test.go | 3 +-- runtime/tests/interpreter/attachments_test.go | 4 ++-- runtime/tests/interpreter/resources_test.go | 2 +- 7 files changed, 21 insertions(+), 22 deletions(-) diff --git a/runtime/attachments_test.go b/runtime/attachments_test.go index adb5bb64eb..7a05f902df 100644 --- a/runtime/attachments_test.go +++ b/runtime/attachments_test.go @@ -223,7 +223,7 @@ func TestAccountAttachmentExportFailure(t *testing.T) { }, ) require.Error(t, err) - require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) } func TestAccountAttachmentExport(t *testing.T) { diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 7cc6f4df60..ded8e32524 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5544,7 +5544,7 @@ func (interpreter *Interpreter) trackReferencedResourceKindedValue( values[value] = struct{}{} } -func (interpreter *Interpreter) invalidateReferencedResources(value Value) { +func (interpreter *Interpreter) invalidateReferencedResources(value Value, destroyed bool) { // skip non-resource typed values if !value.IsResourceKinded(interpreter) { return @@ -5555,23 +5555,23 @@ func (interpreter *Interpreter) invalidateReferencedResources(value Value) { switch value := value.(type) { case *CompositeValue: value.ForEachField(interpreter, func(_ string, fieldValue Value) { - interpreter.invalidateReferencedResources(fieldValue) + interpreter.invalidateReferencedResources(fieldValue, destroyed) }) storageID = value.StorageID() case *DictionaryValue: value.Iterate(interpreter, func(_, value Value) (resume bool) { - interpreter.invalidateReferencedResources(value) + interpreter.invalidateReferencedResources(value, destroyed) return true }) storageID = value.StorageID() case *ArrayValue: value.Iterate(interpreter, func(element Value) (resume bool) { - interpreter.invalidateReferencedResources(element) + interpreter.invalidateReferencedResources(element, destroyed) return true }) storageID = value.StorageID() case *SomeValue: - interpreter.invalidateReferencedResources(value.value) + interpreter.invalidateReferencedResources(value.value, destroyed) return default: // skip non-container typed values. @@ -5587,10 +5587,13 @@ func (interpreter *Interpreter) invalidateReferencedResources(value Value) { switch value := value.(type) { case *CompositeValue: value.dictionary = nil + value.isDestroyed = destroyed case *DictionaryValue: value.dictionary = nil + value.isDestroyed = destroyed case *ArrayValue: value.array = nil + value.isDestroyed = destroyed default: panic(errors.NewUnreachableError()) } diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 2ed5389ed2..3fe1707105 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -1810,7 +1810,7 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan v.isDestroyed = true - interpreter.invalidateReferencedResources(v) + interpreter.invalidateReferencedResources(v, true) if config.InvalidatedResourceValidationEnabled { v.array = nil @@ -2681,7 +2681,7 @@ func (v *ArrayValue) Transfer( // This allows raising an error when the resource array is attempted // to be transferred/moved again (see beginning of this function) - interpreter.invalidateReferencedResources(v) + interpreter.invalidateReferencedResources(v, false) if config.InvalidatedResourceValidationEnabled { v.array = nil @@ -15487,7 +15487,7 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio v.isDestroyed = true - interpreter.invalidateReferencedResources(v) + interpreter.invalidateReferencedResources(v, true) if config.InvalidatedResourceValidationEnabled { v.dictionary = nil @@ -16252,7 +16252,7 @@ func (v *CompositeValue) Transfer( // This allows raising an error when the resource is attempted // to be transferred/moved again (see beginning of this function) - interpreter.invalidateReferencedResources(v) + interpreter.invalidateReferencedResources(v, false) if config.InvalidatedResourceValidationEnabled { v.dictionary = nil @@ -17104,7 +17104,7 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati v.isDestroyed = true - interpreter.invalidateReferencedResources(v) + interpreter.invalidateReferencedResources(v, true) if config.InvalidatedResourceValidationEnabled { v.dictionary = nil @@ -17929,7 +17929,7 @@ func (v *DictionaryValue) Transfer( // This allows raising an error when the resource array is attempted // to be transferred/moved again (see beginning of this function) - interpreter.invalidateReferencedResources(v) + interpreter.invalidateReferencedResources(v, false) if config.InvalidatedResourceValidationEnabled { v.dictionary = nil diff --git a/runtime/interpreter/value_function.go b/runtime/interpreter/value_function.go index d104e48d64..6df190ad5e 100644 --- a/runtime/interpreter/value_function.go +++ b/runtime/interpreter/value_function.go @@ -391,12 +391,9 @@ func (f BoundFunctionValue) invoke(invocation Invocation) Value { self := f.Self invocation.Self = self if self != nil { - if resource, ok := (*self).(ResourceKindedValue); ok && resource.IsDestroyed() { - panic(DestroyedResourceError{ - LocationRange: invocation.LocationRange, - }) - } + invocation.Interpreter.checkReferencedResourceNotMovedOrDestroyed(*self, invocation.LocationRange) } + invocation.Base = f.Base invocation.BoundAuthorization = f.BoundAuthorization return f.Function.invoke(invocation) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 8f3f81657e..64b870952b 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -8464,8 +8464,7 @@ func TestInvalidatedResourceUse(t *testing.T) { ) RequireError(t, err) - var destroyedResourceErr interpreter.DestroyedResourceError - require.ErrorAs(t, err, &destroyedResourceErr) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) } diff --git a/runtime/tests/interpreter/attachments_test.go b/runtime/tests/interpreter/attachments_test.go index 7cfc572332..bc9cde98d1 100644 --- a/runtime/tests/interpreter/attachments_test.go +++ b/runtime/tests/interpreter/attachments_test.go @@ -1629,7 +1629,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { ) _, err := inter.Invoke("test") - require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) }) t.Run("nested", func(t *testing.T) { @@ -1763,7 +1763,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { ) _, err := inter.Invoke("test") - require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) }) t.Run("self reference", func(t *testing.T) { diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 63a52b5b75..86d5a62747 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2662,7 +2662,7 @@ func TestInterpretResourceFunctionInvocationAfterDestruction(t *testing.T) { _, err := inter.Invoke("main") RequireError(t, err) - require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) } func TestInterpretResourceFunctionReferenceValidity(t *testing.T) { From 986d25bd5dd1c9898ac6704c33b70da4830ca377 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 24 Jul 2023 14:58:16 -0700 Subject: [PATCH 4/6] Bump atree version --- go.mod | 4 ++-- go.sum | 13 ++++--------- runtime/cmd/decode-state-values/main.go | 4 ++++ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index 52669fba70..6d5b94c9a4 100644 --- a/go.mod +++ b/go.mod @@ -9,10 +9,10 @@ require ( github.com/go-test/deep v1.0.5 github.com/google/go-cmp v0.5.9 // indirect github.com/leanovate/gopter v0.2.9 - github.com/onflow/atree v0.5.0 + github.com/onflow/atree v0.6.1-0.20230711151834-86040b30171f github.com/rivo/uniseg v0.2.1-0.20211004051800-57c86be7915a github.com/schollz/progressbar/v3 v3.8.3 - github.com/stretchr/testify v1.8.1 + github.com/stretchr/testify v1.8.4 github.com/tidwall/pretty v1.2.1 github.com/turbolent/prettier v0.0.0-20220320183459-661cc755135d go.opentelemetry.io/otel v1.8.0 diff --git a/go.sum b/go.sum index 5472e78ef4..e9a6d1ec46 100644 --- a/go.sum +++ b/go.sum @@ -57,8 +57,8 @@ github.com/mattn/go-tty v0.0.3/go.mod h1:ihxohKRERHTVzN+aSVRwACLCeqIoZAWpoICkkvr github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db h1:62I3jR2EmQ4l5rM/4FEfDWcRD+abF5XlKShorW5LRoQ= github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db/go.mod h1:l0dey0ia/Uv7NcFFVbCLtqEBQbrT4OCwCSKTEv6enCw= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= -github.com/onflow/atree v0.5.0 h1:y3lh8hY2fUo8KVE2ALVcz0EiNTq0tXJ6YTXKYVDA+3E= -github.com/onflow/atree v0.5.0/go.mod h1:gBHU0M05qCbv9NN0kijLWMgC47gHVNBIp4KmsVFi0tc= +github.com/onflow/atree v0.6.1-0.20230711151834-86040b30171f h1:Z8/PgTqOgOg02MTRpTBYO2k16FE6z4wEOtaC2WBR9Xo= +github.com/onflow/atree v0.6.1-0.20230711151834-86040b30171f/go.mod h1:xvP61FoOs95K7IYdIYRnNcYQGf4nbF/uuJ0tHf4DRuM= github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -70,15 +70,11 @@ github.com/schollz/progressbar/v3 v3.8.3 h1:FnLGl3ewlDUP+YdSwveXBaXs053Mem/du+wr github.com/schollz/progressbar/v3 v3.8.3/go.mod h1:pWnVCjSBZsT2X3nx9HfRdnCDrpbevliMeoEVhStwHko= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= -github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c h1:HelZ2kAFadG0La9d+4htN4HzQ68Bm2iM9qKMSMES6xg= github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c/go.mod h1:JlzghshsemAMDGZLytTFY8C1JQxQPhnatWqNwUXjggo= github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= @@ -157,7 +153,6 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= lukechampine.com/blake3 v1.1.7 h1:GgRMhmdsuK8+ii6UZFDL8Nb+VyMwadAgcJyfYHxG6n0= diff --git a/runtime/cmd/decode-state-values/main.go b/runtime/cmd/decode-state-values/main.go index 76b7a92994..259f852a0c 100644 --- a/runtime/cmd/decode-state-values/main.go +++ b/runtime/cmd/decode-state-values/main.go @@ -212,6 +212,10 @@ func (s *slabStorage) Count() int { return len(storage) } +func (s *slabStorage) RetrieveIfLoaded(id atree.StorageID) atree.Slab { + panic("unexpected RetrieveIfLoaded call") +} + // interpreterStorage type interpreterStorage struct { From f21739f5930bf5b85e2201bad0941ce6f8f887d5 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 24 Jul 2023 14:58:36 -0700 Subject: [PATCH 5/6] Use loaded values iterators --- runtime/interpreter/interpreter.go | 6 ++--- runtime/interpreter/value.go | 41 +++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index ded8e32524..f3f7e592cb 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5554,18 +5554,18 @@ func (interpreter *Interpreter) invalidateReferencedResources(value Value, destr switch value := value.(type) { case *CompositeValue: - value.ForEachField(interpreter, func(_ string, fieldValue Value) { + value.ForEachLoadedField(interpreter, func(_ string, fieldValue Value) { interpreter.invalidateReferencedResources(fieldValue, destroyed) }) storageID = value.StorageID() case *DictionaryValue: - value.Iterate(interpreter, func(_, value Value) (resume bool) { + value.IterateLoaded(interpreter, func(_, value Value) (resume bool) { interpreter.invalidateReferencedResources(value, destroyed) return true }) storageID = value.StorageID() case *ArrayValue: - value.Iterate(interpreter, func(element Value) (resume bool) { + value.IterateLoaded(interpreter, func(element Value) (resume bool) { interpreter.invalidateReferencedResources(element, destroyed) return true }) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 3fe1707105..7636da240b 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -1710,8 +1710,20 @@ func (v *ArrayValue) Accept(interpreter *Interpreter, visitor Visitor) { } func (v *ArrayValue) Iterate(interpreter *Interpreter, f func(element Value) (resume bool)) { + v.iterate(interpreter, v.array.Iterate, f) +} + +func (v *ArrayValue) IterateLoaded(interpreter *Interpreter, f func(element Value) (resume bool)) { + v.iterate(interpreter, v.array.IterateLoadedValues, f) +} + +func (v *ArrayValue) iterate( + interpreter *Interpreter, + atreeIterate func(fn atree.ArrayIterationFunc) error, + f func(element Value) (resume bool), +) { iterate := func() { - err := v.array.Iterate(func(element atree.Value) (resume bool, err error) { + err := atreeIterate(func(element atree.Value) (resume bool, err error) { // atree.Array iteration provides low-level atree.Value, // convert to high-level interpreter.Value @@ -16415,8 +16427,19 @@ func (v *CompositeValue) GetOwner() common.Address { // ForEachField iterates over all field-name field-value pairs of the composite value. // It does NOT iterate over computed fields and functions! func (v *CompositeValue) ForEachField(gauge common.MemoryGauge, f func(fieldName string, fieldValue Value)) { + v.forEachField(gauge, v.dictionary.Iterate, f) +} + +func (v *CompositeValue) ForEachLoadedField(gauge common.MemoryGauge, f func(fieldName string, fieldValue Value)) { + v.forEachField(gauge, v.dictionary.IterateLoadedValues, f) +} - err := v.dictionary.Iterate(func(key atree.Value, value atree.Value) (resume bool, err error) { +func (v *CompositeValue) forEachField( + gauge common.MemoryGauge, + atreeIterate func(fn atree.MapEntryIterationFunc) error, + f func(fieldName string, fieldValue Value), +) { + err := atreeIterate(func(key atree.Value, value atree.Value) (resume bool, err error) { f( string(key.(StringAtreeValue)), MustConvertStoredValue(gauge, value), @@ -16967,8 +16990,20 @@ func (v *DictionaryValue) Accept(interpreter *Interpreter, visitor Visitor) { } func (v *DictionaryValue) Iterate(interpreter *Interpreter, f func(key, value Value) (resume bool)) { + v.iterate(interpreter, v.dictionary.Iterate, f) +} + +func (v *DictionaryValue) IterateLoaded(interpreter *Interpreter, f func(key, value Value) (resume bool)) { + v.iterate(interpreter, v.dictionary.IterateLoadedValues, f) +} + +func (v *DictionaryValue) iterate( + interpreter *Interpreter, + atreeIterate func(fn atree.MapEntryIterationFunc) error, + f func(key Value, value Value) (resume bool), +) { iterate := func() { - err := v.dictionary.Iterate(func(key, value atree.Value) (resume bool, err error) { + err := atreeIterate(func(key, value atree.Value) (resume bool, err error) { // atree.OrderedMap iteration provides low-level atree.Value, // convert to high-level interpreter.Value From e90ddca948729bcfdd14e339d91e6fcae3f975a3 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 8 Aug 2023 17:05:29 -0700 Subject: [PATCH 6/6] Add benchmark for runtime resource tracking --- runtime/runtime_test.go | 121 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 84e08b91ea..66cdb3d2ed 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9254,3 +9254,124 @@ func TestRuntimeWrappedErrorHandling(t *testing.T) { // It can NOT be an internal error. assertRuntimeErrorIsUserError(t, err) } + +func BenchmarkRuntimeResourceTracking(b *testing.B) { + + runtime := newTestInterpreterRuntime() + + contractsAddress := common.MustBytesToAddress([]byte{0x1}) + + accountCodes := map[Location][]byte{} + + signerAccount := contractsAddress + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return []Address{signerAccount}, nil + }, + resolveLocation: singleIdentifierLocationResolver(b), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + return nil + }, + } + runtimeInterface.decodeArgument = func(b []byte, t cadence.Type) (value cadence.Value, err error) { + return json.Decode(runtimeInterface, b) + } + + environment := NewBaseInterpreterEnvironment(Config{}) + + nextTransactionLocation := newTransactionLocationGenerator() + + // Deploy contract + + err := runtime.ExecuteTransaction( + Script{ + Source: DeploymentTransaction( + "Foo", + []byte(` + access(all) contract Foo { + access(all) resource R {} + + access(all) fun getResourceArray(): @[R] { + var resourceArray: @[R] <- [] + var i = 0 + while i < 1000 { + resourceArray.append(<- create R()) + i = i + 1 + } + return <- resourceArray + } + } + `), + ), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + Environment: environment, + }, + ) + require.NoError(b, err) + + err = runtime.ExecuteTransaction( + Script{ + Source: []byte(` + import Foo from 0x1 + + transaction { + prepare(signer: AuthAccount) { + signer.save(<- Foo.getResourceArray(), to: /storage/r) + } + } + `), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + Environment: environment, + }, + ) + require.NoError(b, err) + + b.ReportAllocs() + b.ResetTimer() + + err = runtime.ExecuteTransaction( + Script{ + Source: []byte(` + import Foo from 0x1 + + transaction { + prepare(signer: AuthAccount) { + // When the array is loaded from storage, all elements are also loaded. + // So all moves of this resource will check for tracking of all elements aas well. + + var array1 <- signer.load<@[Foo.R]>(from: /storage/r)! + var array2 <- array1 + var array3 <- array2 + var array4 <- array3 + var array5 <- array4 + destroy array5 + } + } + `), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + Environment: environment, + }, + ) + require.NoError(b, err) +}