diff --git a/runtime/interpreter/errors.go b/runtime/interpreter/errors.go index 6ff6d71395..e3514b464f 100644 --- a/runtime/interpreter/errors.go +++ b/runtime/interpreter/errors.go @@ -1119,3 +1119,16 @@ func (InvalidCapabilityIDError) IsInternalError() {} func (e InvalidCapabilityIDError) Error() string { return "capability created with invalid ID" } + +// ReferencedValueChangedError +type ReferencedValueChangedError struct { + LocationRange +} + +var _ errors.UserError = ReferencedValueChangedError{} + +func (ReferencedValueChangedError) IsUserError() {} + +func (e ReferencedValueChangedError) Error() string { + return "referenced value has been changed after taking the reference" +} diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 55dff46d99..a8e30237aa 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -20657,8 +20657,22 @@ func (v *StorageReferenceValue) GetMember( locationRange LocationRange, name string, ) Value { - self := v.mustReferencedValue(interpreter, locationRange) - return interpreter.getMember(self, locationRange, name) + referencedValue := v.mustReferencedValue(interpreter, locationRange) + + member := interpreter.getMember(referencedValue, locationRange, name) + + // If the member is a function, it is always a bound-function. + // By default, bound functions create and hold an ephemeral reference (`selfRef`). + // For storage references, replace this default one with the actual storage reference. + // It is not possible (or a lot of work), to create the bound function with the storage reference + // when it was created originally, because `getMember(referencedValue, ...)` doesn't know + // whether the member was accessed directly, or via a reference. + if boundFunction, isBoundFunction := member.(BoundFunctionValue); isBoundFunction { + boundFunction.selfRef = v + return boundFunction + } + + return member } func (v *StorageReferenceValue) RemoveMember( diff --git a/runtime/interpreter/value_function.go b/runtime/interpreter/value_function.go index af4a717793..70a6218296 100644 --- a/runtime/interpreter/value_function.go +++ b/runtime/interpreter/value_function.go @@ -330,7 +330,7 @@ type BoundFunctionValue struct { Base *EphemeralReferenceValue Self *Value BoundAuthorization Authorization - selfRef *EphemeralReferenceValue + selfRef ReferenceValue } var _ Value = BoundFunctionValue{} @@ -353,8 +353,8 @@ func NewBoundFunctionValue( // Since 'self' work as an implicit reference, create an explicit one and hold it. // This reference is later used to check the validity of the referenced value/resource. - var selfRef *EphemeralReferenceValue - if reference, isReference := (*self).(*EphemeralReferenceValue); isReference { + var selfRef ReferenceValue + if reference, isReference := (*self).(ReferenceValue); isReference { // For attachments, 'self' is already a reference. // So no need to create a reference again. selfRef = reference @@ -413,12 +413,42 @@ func (f BoundFunctionValue) invoke(invocation Invocation) Value { invocation.Base = f.Base invocation.BoundAuthorization = f.BoundAuthorization + locationRange := invocation.LocationRange + inter := invocation.Interpreter + // Check if the 'self' is not invalidated. - invocation.Interpreter.checkInvalidatedResourceOrResourceReference(f.selfRef, invocation.LocationRange) + if storageRef, isStorageRef := f.selfRef.(*StorageReferenceValue); isStorageRef { + inter.checkInvalidatedStorageReference(storageRef, locationRange) + } else { + inter.checkInvalidatedResourceOrResourceReference(f.selfRef, locationRange) + } return f.Function.invoke(invocation) } +// checkInvalidatedStorageReference checks whether a storage reference is valid, by +// comparing the referenced-value against the cached-referenced-value. +// A storage reference can be invalid for both resources and non-resource values. +func (interpreter *Interpreter) checkInvalidatedStorageReference( + storageRef *StorageReferenceValue, + locationRange LocationRange, +) { + + referencedValue := storageRef.ReferencedValue( + interpreter, + locationRange, + true, + ) + + // `storageRef.ReferencedValue` above already checks for the type validity, if it's not nil. + // If nil, that means the value has been moved out of storage. + if referencedValue == nil { + panic(ReferencedValueChangedError{ + LocationRange: locationRange, + }) + } +} + func (f BoundFunctionValue) ConformsToStaticType( interpreter *Interpreter, locationRange LocationRange, diff --git a/runtime/storage_test.go b/runtime/storage_test.go index 98cf8597b7..7fa5516298 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -5725,103 +5725,266 @@ func TestRuntimeStorageReferenceBoundFunction(t *testing.T) { t.Parallel() - runtime := NewTestInterpreterRuntime() + t.Run("resource", func(t *testing.T) { - signerAddress := common.MustBytesToAddress([]byte{0x42}) + runtime := NewTestInterpreterRuntime() - deployTx := DeploymentTransaction("Test", []byte(` - access(all) contract Test { + signerAddress := common.MustBytesToAddress([]byte{0x42}) - access(all) resource R { - access(all) fun foo() {} - } + deployTx := DeploymentTransaction("Test", []byte(` + access(all) contract Test { - access(all) fun createR(): @R { - return <-create R() - } - } - `)) + access(all) resource R { + access(all) fun foo() {} + } - accountCodes := map[Location][]byte{} - var events []cadence.Event - var loggedMessages []string + access(all) fun createR(): @R { + return <-create R() + } + } + `)) - runtimeInterface := &TestRuntimeInterface{ - Storage: NewTestLedger(nil, nil), - OnGetSigningAccounts: func() ([]Address, error) { - return []Address{signerAddress}, nil - }, - OnResolveLocation: NewSingleIdentifierLocationResolver(t), - OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error { - accountCodes[location] = code - return nil - }, - OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { - code = accountCodes[location] - return code, nil - }, - OnEmitEvent: func(event cadence.Event) error { - events = append(events, event) - return nil - }, - OnProgramLog: func(message string) { - loggedMessages = append(loggedMessages, message) - }, - } + accountCodes := map[Location][]byte{} + var events []cadence.Event + var loggedMessages []string - nextTransactionLocation := NewTransactionLocationGenerator() + runtimeInterface := &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]Address, error) { + return []Address{signerAddress}, nil + }, + OnResolveLocation: NewSingleIdentifierLocationResolver(t), + OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + code = accountCodes[location] + return code, nil + }, + OnEmitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + OnProgramLog: func(message string) { + loggedMessages = append(loggedMessages, message) + }, + } - // Deploy contract + nextTransactionLocation := NewTransactionLocationGenerator() - err := runtime.ExecuteTransaction( - Script{ - Source: deployTx, - }, - Context{ - Interface: runtimeInterface, - Location: nextTransactionLocation(), - }, - ) - require.NoError(t, err) + // Deploy contract - // Run test transaction + err := runtime.ExecuteTransaction( + Script{ + Source: deployTx, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) - const testTx = ` - import Test from 0x42 + // Run test transaction - transaction { - prepare(signer: auth(Storage) &Account) { - signer.storage.save(<-Test.createR(), to: /storage/r) + const testTx = ` + import Test from 0x42 - let ref = signer.storage.borrow<&Test.R>(from: /storage/r)! + transaction { + prepare(signer: auth(Storage) &Account) { + signer.storage.save(<-Test.createR(), to: /storage/r) - var func = ref.foo + let ref = signer.storage.borrow<&Test.R>(from: /storage/r)! - let r <- signer.storage.load<@Test.R>(from: /storage/r)! + var func = ref.foo - // Should be OK - func() + let r <- signer.storage.load<@Test.R>(from: /storage/r)! - destroy r + // Should fail: Underlying value was removed from storage + func() - // Should fail! - func() - } - } - ` + destroy r + } + } + ` - err = runtime.ExecuteTransaction( - Script{ - Source: []byte(testTx), - }, - Context{ - Interface: runtimeInterface, - Location: nextTransactionLocation(), - }, - ) + err = runtime.ExecuteTransaction( + Script{ + Source: []byte(testTx), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.ReferencedValueChangedError{}) + }) + + t.Run("struct", func(t *testing.T) { + t.Parallel() + + runtime := NewTestInterpreterRuntimeWithAttachments() + + tx := []byte(` + transaction { + + prepare(signer: auth(Storage, Capabilities) &Account) { + + signer.storage.save([] as [AnyStruct], to: /storage/zombieArray) + var borrowed = signer.storage.borrow(from: /storage/zombieArray)! + + var x: [Int] = [] + + var appendFunc = borrowed.append + + // If we were to call appendFunc() here, we wouldn't see a big effect as the + // next load() call will remove the array from storage + var throwaway = signer.storage.load<[AnyStruct]>(from: /storage/zombieArray) + + // Should be an error, since the value was moved out. + appendFunc(x) + } + } + `) + + signer := common.MustBytesToAddress([]byte{0x1}) + + runtimeInterface := &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]Address, error) { + return []Address{signer}, nil + }, + OnResolveLocation: NewSingleIdentifierLocationResolver(t), + } + + nextTransactionLocation := NewTransactionLocationGenerator() + + err := runtime.ExecuteTransaction( + Script{ + Source: tx, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.ReferencedValueChangedError{}) + }) + + t.Run("replace resource", func(t *testing.T) { + + runtime := NewTestInterpreterRuntime() + + signerAddress := common.MustBytesToAddress([]byte{0x42}) + + deployTx := DeploymentTransaction("Test", []byte(` + access(all) contract Test { + + access(all) resource Foo { + access(all) fun hello() {} + } + + access(all) fun createFoo(): @Foo { + return <-create Foo() + } + + access(all) resource Bar { + access(all) fun hello() {} + } + + access(all) fun createBar(): @Bar { + return <-create Bar() + } + } + `)) + + accountCodes := map[Location][]byte{} + var events []cadence.Event + var loggedMessages []string + + runtimeInterface := &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]Address, error) { + return []Address{signerAddress}, nil + }, + OnResolveLocation: NewSingleIdentifierLocationResolver(t), + OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + code = accountCodes[location] + return code, nil + }, + OnEmitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + OnProgramLog: func(message string) { + loggedMessages = append(loggedMessages, message) + }, + } + + nextTransactionLocation := NewTransactionLocationGenerator() + + // Deploy contract + + err := runtime.ExecuteTransaction( + Script{ + Source: deployTx, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Run test transaction + + const testTx = ` + import Test from 0x42 + + transaction { + prepare(signer: auth(Storage) &Account) { + signer.storage.save(<-Test.createFoo(), to: /storage/xyz) + let ref = signer.storage.borrow<&Test.Foo>(from: /storage/xyz)! + + // Take a reference to 'Foo.hello' + var hello = ref.hello + + // Remove 'Foo' + let foo <- signer.storage.load<@Test.Foo>(from: /storage/xyz)! + + // Replace it with 'Bar' value + signer.storage.save(<-Test.createBar(), to: /storage/xyz) + + // Should be an error + hello() + + destroy foo + } + } + ` + + err = runtime.ExecuteTransaction( + Script{ + Source: []byte(testTx), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.DereferenceError{}) + }) - RequireError(t, err) - require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{}) } func TestRuntimeStorageReferenceAccess(t *testing.T) {