From 1a80a486eac33dca26109e77b1e9ad125cafa6b9 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 15 Nov 2023 10:45:20 -0800 Subject: [PATCH 01/10] Add test case --- runtime/runtime_test.go | 222 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 46aa3f8a65..7346c5260e 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9756,3 +9756,225 @@ func TestRuntimeNestedResourceMoveInTransaction(t *testing.T) { require.NoError(t, err) } + +func TestRuntimePreconditionDuplication(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + signerAccount := common.MustBytesToAddress([]byte{0x1}) + + signers := []Address{signerAccount} + + accountCodes := map[Location][]byte{} + + 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 { + return nil + }, + log: func(s string) { + fmt.Println(s) + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + attacker := []byte(fmt.Sprintf(` + import Bar from %[1]s + + access(all) contract Foo { + pub var temp: @Bar.Vault? + + init() { + self.temp <- nil + } + + pub fun doubler(_ vault: @Bar.Vault): @Bar.Vault { + var r <- create R(<-vault) + r.doMagic() + destroy r + var doubled <- self.temp <- nil + return <- doubled! + } + + pub fun preconditionFunction(_ unusedref: &Bar.Vault, _ v : @Bar.Vault, _ ref: &R): Bool { + // This gets called twice: once from the interface's precondition and the second + // time from the function's own precondition. We save both copies of the vault + // to the R object + if ref.counter == 0 { + ref.victim1 <-! v + ref.counter = 1 + } else { + ref.victim2 <-! v + } + return true + } + + pub resource interface RInterface{ + pub fun funcFromIface( _ v: @Bar.Vault, _ ref: &R): Void { + pre { + Foo.preconditionFunction(&v as &Bar.Vault, <- v, ref) + } + } + } + + pub resource R: RInterface { + pub var bounty: @Bar.Vault? + pub(set) var victim1: @Bar.Vault? + pub(set) var victim2: @Bar.Vault? + pub(set) var counter: Int + + init(_ v: @Bar.Vault) { + self.counter = 0 + self.bounty <- v + self.victim1 <- nil + self.victim2 <- nil + } + + pub fun funcFromIface(_ v: @Bar.Vault, _ ref: &R): Void { + pre { + Foo.preconditionFunction(&v as &Bar.Vault, <- v, ref) + } + } + + pub fun doMagic(): Void { + var origVault <- self.bounty <- nil + self.funcFromIface(<- origVault!, &self as &R) + + var v1 <- self.victim1 <- nil + var v2 <- self.victim2 <- nil + + // Following moves copied from Supun's test + var r1 = &v2 as &Bar.Vault? + Foo.account.save(<-v1!, to: /storage/v1) + Foo.account.save(<-v2!, to: /storage/v2) + var v1Reloaded <- Foo.account.load<@Bar.Vault>(from: /storage/v1)! + var v2Reloaded <- Foo.account.load<@Bar.Vault>(from: /storage/v2)! + + v1Reloaded.deposit(from:<-v2Reloaded) + Foo.temp <-! v1Reloaded + } + + destroy() { + destroy self.bounty + destroy self.victim1 + destroy self.victim2 + } + } + }`, + signerAccount.HexWithPrefix(), + )) + + bar := []byte(` + pub contract Bar { + 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 createEmptyVault(): @Bar.Vault { + return <- create Bar.Vault(balance: 0.0) + } + + pub fun createVault(balance: UFix64): @Bar.Vault { + return <- create Bar.Vault(balance: balance) + } + } + `) + + // Deploy Bar + + deployVault := DeploymentTransaction("Bar", bar) + err := runtime.ExecuteTransaction( + Script{ + Source: deployVault, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy Attacker + + deployAttacker := DeploymentTransaction("Foo", attacker) + + err = runtime.ExecuteTransaction( + Script{ + Source: deployAttacker, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Attack + + attackTransaction := []byte(fmt.Sprintf(` + import Foo from %[1]s + import Bar from %[1]s + + transaction { + prepare(acc: AuthAccount) { + acc.save(<- Bar.createVault(balance: 100.0), to: /storage/vault)! + var vault = acc.borrow<&Bar.Vault>(from: /storage/vault)! + var flow <- vault.withdraw(amount: 42.0) + + var doubled <- Foo.doubler(<-flow) + log(doubled.balance) + + destroy doubled + } + }`, + signerAccount.HexWithPrefix(), + )) + + err = runtime.ExecuteTransaction( + Script{ + Source: attackTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) +} From 3ab40b4223fc743c0a7fe2644183cb8ad5c2779e Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 15 Nov 2023 16:18:36 -0800 Subject: [PATCH 02/10] Track the reference in bookkeeping instead of the referenced-value --- runtime/interpreter/interpreter.go | 16 +-- runtime/interpreter/interpreter_expression.go | 34 ++--- runtime/interpreter/sharedstate.go | 2 +- runtime/interpreter/value.go | 136 +++++++----------- 4 files changed, 72 insertions(+), 116 deletions(-) diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 9875f5646b..0d153d3c3b 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -246,7 +246,7 @@ type Storage interface { CheckHealth() error } -type ReferencedResourceKindedValues map[atree.StorageID]map[ReferenceTrackedResourceKindedValue]struct{} +type ReferencedResourceKindedValues map[atree.StorageID]map[*EphemeralReferenceValue]struct{} type Interpreter struct { Location common.Location @@ -841,14 +841,12 @@ func (interpreter *Interpreter) resultValue(returnValue Value, returnType sema.T optionalType.Type, ) - interpreter.maybeTrackReferencedResourceKindedValue(returnValue.value) return NewSomeValueNonCopying(interpreter, innerValue) case NilValue: return NilValue{} } } - interpreter.maybeTrackReferencedResourceKindedValue(returnValue) return NewEphemeralReferenceValue(interpreter, false, returnValue, returnType) } @@ -5266,18 +5264,20 @@ func (interpreter *Interpreter) ValidateAtreeValue(value atree.Value) { } func (interpreter *Interpreter) maybeTrackReferencedResourceKindedValue(value Value) { - if value, ok := value.(ReferenceTrackedResourceKindedValue); ok { - interpreter.trackReferencedResourceKindedValue(value.StorageID(), value) + if referenceValue, ok := value.(*EphemeralReferenceValue); ok { + if value, ok := referenceValue.Value.(ReferenceTrackedResourceKindedValue); ok { + interpreter.trackReferencedResourceKindedValue(value.StorageID(), referenceValue) + } } } func (interpreter *Interpreter) trackReferencedResourceKindedValue( id atree.StorageID, - value ReferenceTrackedResourceKindedValue, + value *EphemeralReferenceValue, ) { values := interpreter.SharedState.referencedResourceKindedValues[id] if values == nil { - values = map[ReferenceTrackedResourceKindedValue]struct{}{} + values = map[*EphemeralReferenceValue]struct{}{} interpreter.SharedState.referencedResourceKindedValues[id] = values } values[value] = struct{}{} @@ -5286,7 +5286,7 @@ func (interpreter *Interpreter) trackReferencedResourceKindedValue( func (interpreter *Interpreter) updateReferencedResource( currentStorageID atree.StorageID, newStorageID atree.StorageID, - updateFunc func(value ReferenceTrackedResourceKindedValue), + updateFunc func(value *EphemeralReferenceValue), ) { values := interpreter.SharedState.referencedResourceKindedValues[currentStorageID] if values == nil { diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 96420aa4cd..b7fdfe0a0d 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -962,11 +962,12 @@ func (interpreter *Interpreter) visitInvocationExpressionWithImplicitArgument(in // Bound functions if boundFunction, ok := function.(BoundFunctionValue); ok && boundFunction.Self != nil { - self := *boundFunction.Self - if resource, ok := self.(ReferenceTrackedResourceKindedValue); ok { - storageID := resource.StorageID() - interpreter.trackReferencedResourceKindedValue(storageID, resource) - } + // TODO: + //self := *boundFunction.Self + //if resource, ok := self.(ReferenceTrackedResourceKindedValue); ok { + // storageID := resource.StorageID() + // interpreter.trackReferencedResourceKindedValue(storageID, resource) + //} } // NOTE: evaluate all argument expressions in call-site scope, not in function body @@ -1180,8 +1181,6 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as result := interpreter.evalExpression(referenceExpression.Expression) - interpreter.maybeTrackReferencedResourceKindedValue(result) - // There are four potential cases: // 1) Target type is optional, actual value is also optional (nil/SomeValue) // 2) Target type is optional, actual value is non-optional @@ -1247,16 +1246,17 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as } case *sema.ReferenceType: + // TODO: // Case (3): target type is non-optional, actual value is optional. // Unwrap the optional and add it to reference tracking. - if someValue, ok := result.(*SomeValue); ok { - locationRange := LocationRange{ - Location: interpreter.Location, - HasPosition: referenceExpression.Expression, - } - innerValue := someValue.InnerValue(interpreter, locationRange) - interpreter.maybeTrackReferencedResourceKindedValue(innerValue) - } + //if someValue, ok := referencedValue.(*SomeValue); ok { + // locationRange := LocationRange{ + // Location: interpreter.Location, + // HasPosition: referenceExpression.Expression, + // } + // innerValue := someValue.InnerValue(interpreter, locationRange) + // interpreter.maybeTrackReferencedResourceKindedValue(innerValue) + //} // Case (4): target type is non-optional, actual value is also non-optional return NewEphemeralReferenceValue(interpreter, typ.Authorized, result, typ.Type) @@ -1338,7 +1338,6 @@ func (interpreter *Interpreter) VisitAttachExpression(attachExpression *ast.Atta base, interpreter.MustSemaTypeOfValue(base).(*sema.CompositeType), ) - interpreter.trackReferencedResourceKindedValue(base.StorageID(), base) attachment, ok := interpreter.visitInvocationExpressionWithImplicitArgument( attachExpression.Attachment, @@ -1349,9 +1348,6 @@ func (interpreter *Interpreter) VisitAttachExpression(attachExpression *ast.Atta panic(errors.NewUnreachableError()) } - // Because `self` in attachments is a reference, we need to track the attachment if it's a resource - interpreter.trackReferencedResourceKindedValue(attachment.StorageID(), attachment) - base = base.Transfer( interpreter, locationRange, diff --git a/runtime/interpreter/sharedstate.go b/runtime/interpreter/sharedstate.go index f250ce3f58..71fc3b6674 100644 --- a/runtime/interpreter/sharedstate.go +++ b/runtime/interpreter/sharedstate.go @@ -59,7 +59,7 @@ func NewSharedState(config *Config) *SharedState { }, inStorageIteration: false, storageMutatedDuringIteration: false, - referencedResourceKindedValues: map[atree.StorageID]map[ReferenceTrackedResourceKindedValue]struct{}{}, + referencedResourceKindedValues: map[atree.StorageID]map[*EphemeralReferenceValue]struct{}{}, resourceVariables: map[ResourceKindedValue]*Variable{}, CapabilityControllerIterations: map[AddressPath]int{}, containerValueIteration: map[atree.StorageID]struct{}{}, diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 508515d4d3..796c4962d7 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -1825,8 +1825,8 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan interpreter.updateReferencedResource( storageID, storageID, - func(value ReferenceTrackedResourceKindedValue) { - arrayValue, ok := value.(*ArrayValue) + func(value *EphemeralReferenceValue) { + arrayValue, ok := value.Value.(*ArrayValue) if !ok { panic(errors.NewUnreachableError()) } @@ -2744,7 +2744,11 @@ func (v *ArrayValue) Transfer( } } - var res *ArrayValue + res := newArrayValueFromAtreeValue(array, v.Type) + res.elementSize = v.elementSize + res.semaType = v.semaType + res.isResourceKinded = v.isResourceKinded + res.isDestroyed = v.isDestroyed if isResourceKinded { // Update the resource in-place, @@ -2756,36 +2760,19 @@ 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) - if config.InvalidatedResourceValidationEnabled { - v.array = nil - } else { - v.array = array - res = v - } + v.array = nil newStorageID := array.StorageID() interpreter.updateReferencedResource( currentStorageID, newStorageID, - func(value ReferenceTrackedResourceKindedValue) { - arrayValue, ok := value.(*ArrayValue) - if !ok { - panic(errors.NewUnreachableError()) - } - arrayValue.array = array + func(value *EphemeralReferenceValue) { + value.Value = res }, ) } - if res == nil { - res = newArrayValueFromAtreeValue(array, v.Type) - res.elementSize = v.elementSize - res.semaType = v.semaType - res.isResourceKinded = v.isResourceKinded - res.isDestroyed = v.isDestroyed - } - return res } @@ -16550,8 +16537,8 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio interpreter.updateReferencedResource( storageID, storageID, - func(value ReferenceTrackedResourceKindedValue) { - compositeValue, ok := value.(*CompositeValue) + func(value *EphemeralReferenceValue) { + compositeValue, ok := value.Value.(*CompositeValue) if !ok { panic(errors.NewUnreachableError()) } @@ -17317,7 +17304,24 @@ func (v *CompositeValue) Transfer( } } - var res *CompositeValue + info := NewCompositeTypeInfo( + interpreter, + v.Location, + v.QualifiedIdentifier, + v.Kind, + ) + + res := newCompositeValueFromOrderedMap(dictionary, info) + res.injectedFields = v.injectedFields + res.computedFields = v.computedFields + res.NestedVariables = v.NestedVariables + res.Functions = v.Functions + res.Destructor = v.Destructor + res.Stringer = v.Stringer + res.isDestroyed = v.isDestroyed + res.typeID = v.typeID + res.staticType = v.staticType + res.base = v.base if isResourceKinded { // Update the resource in-place, @@ -17329,48 +17333,19 @@ func (v *CompositeValue) Transfer( // This allows raising an error when the resource is attempted // to be transferred/moved again (see beginning of this function) - if config.InvalidatedResourceValidationEnabled { - v.dictionary = nil - } else { - v.dictionary = dictionary - res = v - } + v.dictionary = nil newStorageID := dictionary.StorageID() interpreter.updateReferencedResource( currentStorageID, newStorageID, - func(value ReferenceTrackedResourceKindedValue) { - compositeValue, ok := value.(*CompositeValue) - if !ok { - panic(errors.NewUnreachableError()) - } - compositeValue.dictionary = dictionary + func(value *EphemeralReferenceValue) { + value.Value = res }, ) } - if res == nil { - info := NewCompositeTypeInfo( - interpreter, - v.Location, - v.QualifiedIdentifier, - v.Kind, - ) - res = newCompositeValueFromOrderedMap(dictionary, info) - res.injectedFields = v.injectedFields - res.computedFields = v.computedFields - res.NestedVariables = v.NestedVariables - res.Functions = v.Functions - res.Destructor = v.Destructor - res.Stringer = v.Stringer - res.isDestroyed = v.isDestroyed - res.typeID = v.typeID - res.staticType = v.staticType - res.base = v.base - } - onResourceOwnerChange := config.OnResourceOwnerChange if needsStoreTo && @@ -17614,8 +17589,6 @@ func (v *CompositeValue) setBaseValue(interpreter *Interpreter, base *CompositeV // the base reference can only be borrowed with the declared type of the attachment's base v.base = NewEphemeralReferenceValue(interpreter, false, base, baseType) - - interpreter.trackReferencedResourceKindedValue(base.StorageID(), base) } func attachmentMemberName(ty sema.Type) string { @@ -17644,7 +17617,6 @@ func attachmentBaseAndSelfValues( base = v.getBaseValue() // in attachment functions, self is a reference value self = NewEphemeralReferenceValue(interpreter, false, v, interpreter.MustSemaTypeOfValue(v)) - interpreter.trackReferencedResourceKindedValue(v.StorageID(), v) return } @@ -17695,7 +17667,6 @@ func (v *CompositeValue) GetTypeKey( attachment.setBaseValue(interpreter, v) attachmentRef := NewEphemeralReferenceValue(interpreter, false, attachment, ty) - interpreter.trackReferencedResourceKindedValue(attachment.StorageID(), attachment) return NewSomeValueNonCopying(interpreter, attachmentRef) } @@ -18027,8 +17998,8 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati interpreter.updateReferencedResource( storageID, storageID, - func(value ReferenceTrackedResourceKindedValue) { - dictionaryValue, ok := value.(*DictionaryValue) + func(value *EphemeralReferenceValue) { + dictionaryValue, ok := value.Value.(*DictionaryValue) if !ok { panic(errors.NewUnreachableError()) } @@ -18850,7 +18821,11 @@ func (v *DictionaryValue) Transfer( } } - var res *DictionaryValue + res := newDictionaryValueFromOrderedMap(dictionary, v.Type) + res.elementSize = v.elementSize + res.semaType = v.semaType + res.isResourceKinded = v.isResourceKinded + res.isDestroyed = v.isDestroyed if isResourceKinded { // Update the resource in-place, @@ -18862,36 +18837,19 @@ 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) - if config.InvalidatedResourceValidationEnabled { - v.dictionary = nil - } else { - v.dictionary = dictionary - res = v - } + v.dictionary = nil newStorageID := dictionary.StorageID() interpreter.updateReferencedResource( currentStorageID, newStorageID, - func(value ReferenceTrackedResourceKindedValue) { - dictionaryValue, ok := value.(*DictionaryValue) - if !ok { - panic(errors.NewUnreachableError()) - } - dictionaryValue.dictionary = dictionary + func(value *EphemeralReferenceValue) { + value.Value = res }, ) } - if res == nil { - res = newDictionaryValueFromOrderedMap(dictionary, v.Type) - res.elementSize = v.elementSize - res.semaType = v.semaType - res.isResourceKinded = v.isResourceKinded - res.isDestroyed = v.isDestroyed - } - return res } @@ -19945,13 +19903,15 @@ func NewUnmeteredEphemeralReferenceValue( } func NewEphemeralReferenceValue( - gauge common.MemoryGauge, + interpreter *Interpreter, authorized bool, value Value, borrowedType sema.Type, ) *EphemeralReferenceValue { - common.UseMemory(gauge, common.EphemeralReferenceValueMemoryUsage) - return NewUnmeteredEphemeralReferenceValue(authorized, value, borrowedType) + common.UseMemory(interpreter, common.EphemeralReferenceValueMemoryUsage) + ref := NewUnmeteredEphemeralReferenceValue(authorized, value, borrowedType) + interpreter.maybeTrackReferencedResourceKindedValue(ref) + return ref } func (*EphemeralReferenceValue) isValue() {} From c0dce99860b183d4f501f4aa7de279df363e5388 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 16 Nov 2023 11:42:09 -0800 Subject: [PATCH 03/10] Create an explicit reference to bound-function receiver --- runtime/interpreter/interpreter_expression.go | 1 - runtime/interpreter/value_function.go | 28 +++++++++++++------ runtime/interpreter/value_function_test.go | 8 ++++++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index b7fdfe0a0d..9491444c3d 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -1207,7 +1207,6 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as } innerValue := result.InnerValue(interpreter, locationRange) - interpreter.maybeTrackReferencedResourceKindedValue(innerValue) return NewSomeValueNonCopying( interpreter, diff --git a/runtime/interpreter/value_function.go b/runtime/interpreter/value_function.go index 1734d966dd..c7028f49e3 100644 --- a/runtime/interpreter/value_function.go +++ b/runtime/interpreter/value_function.go @@ -327,6 +327,7 @@ type BoundFunctionValue struct { Function FunctionValue Base *EphemeralReferenceValue Self *MemberAccessibleValue + selfRef *EphemeralReferenceValue } var _ Value = BoundFunctionValue{} @@ -341,9 +342,22 @@ func NewBoundFunctionValue( common.UseMemory(interpreter, common.BoundFunctionValueMemoryUsage) + // 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 { + // For attachments, 'self' is already a reference. + // So no need to create a reference again. + selfRef = reference + } else { + semaType := interpreter.MustSemaTypeOfValue(*self) + selfRef = NewEphemeralReferenceValue(interpreter, true, *self, semaType) + } + return BoundFunctionValue{ Function: function, Self: self, + selfRef: selfRef, Base: base, } } @@ -385,16 +399,12 @@ func (f BoundFunctionValue) FunctionType() *sema.FunctionType { } 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.Self = f.Self invocation.Base = f.Base + + // Check if the 'self' is not invalidated. + _ = f.selfRef.MustReferencedValue(invocation.Interpreter, invocation.LocationRange) + return f.Function.invoke(invocation) } diff --git a/runtime/interpreter/value_function_test.go b/runtime/interpreter/value_function_test.go index 724229ad77..e7a2f39a99 100644 --- a/runtime/interpreter/value_function_test.go +++ b/runtime/interpreter/value_function_test.go @@ -63,6 +63,14 @@ func TestFunctionStaticType(t *testing.T) { inter := newTestInterpreter(t) + inter.SharedState.Config.CompositeTypeHandler = func(location common.Location, typeID TypeID) *sema.CompositeType { + return &sema.CompositeType{ + Location: utils.TestLocation, + Identifier: "foo", + Kind: common.CompositeKindStructure, + } + } + hostFunction := func(_ Invocation) Value { return TrueValue } From 0ffaf7b4a121557855992613156157a7f48487ec Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 16 Nov 2023 15:40:08 -0800 Subject: [PATCH 04/10] Add runtime check for invalidated resource use --- runtime/interpreter/interpreter_expression.go | 14 +- runtime/interpreter/value.go | 33 ++- runtime/runtime_test.go | 2 +- runtime/tests/interpreter/resources_test.go | 261 +++++++++++++++++- 4 files changed, 284 insertions(+), 26 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 9491444c3d..076013328d 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -303,7 +303,19 @@ func (interpreter *Interpreter) VisitIdentifierExpression(expression *ast.Identi } func (interpreter *Interpreter) evalExpression(expression ast.Expression) Value { - return ast.AcceptExpression[Value](expression, interpreter) + result := ast.AcceptExpression[Value](expression, interpreter) + + resourceKindedValue, ok := result.(ReferenceTrackedResourceKindedValue) + if ok && resourceKindedValue.isInvalidatedResource(interpreter) { + panic(InvalidatedResourceError{ + LocationRange: LocationRange{ + Location: interpreter.Location, + HasPosition: expression, + }, + }) + } + + return result } func (interpreter *Interpreter) VisitBinaryExpression(expression *ast.BinaryExpression) Value { diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 796c4962d7..8c49354254 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -219,6 +219,7 @@ type ReferenceTrackedResourceKindedValue interface { ResourceKindedValue IsReferenceTrackedResourceKindedValue() StorageID() atree.StorageID + isInvalidatedResource(*Interpreter) bool } // ContractValue is the value of a contract. @@ -1771,8 +1772,12 @@ func (v *ArrayValue) IsImportable(inter *Interpreter) bool { return importable } +func (v *ArrayValue) isInvalidatedResource(interpreter *Interpreter) bool { + return v.isDestroyed || (v.array == nil && v.IsResourceKinded(interpreter)) +} + func (v *ArrayValue) checkInvalidatedResourceUse(interpreter *Interpreter, locationRange LocationRange) { - if v.isDestroyed || (v.array == nil && v.IsResourceKinded(interpreter)) { + if v.isInvalidatedResource(interpreter) { panic(InvalidatedResourceError{ LocationRange: locationRange, }) @@ -16459,7 +16464,7 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio config := interpreter.SharedState.Config if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) + v.checkInvalidatedResourceUse(interpreter, locationRange) } if config.TracingEnabled { @@ -16569,7 +16574,7 @@ func (v *CompositeValue) GetMember(interpreter *Interpreter, locationRange Locat config := interpreter.SharedState.Config if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) + v.checkInvalidatedResourceUse(interpreter, locationRange) } if config.TracingEnabled { @@ -16631,8 +16636,12 @@ func (v *CompositeValue) GetMember(interpreter *Interpreter, locationRange Locat return nil } -func (v *CompositeValue) checkInvalidatedResourceUse(locationRange LocationRange) { - if v.isDestroyed || (v.dictionary == nil && v.Kind == common.CompositeKindResource) { +func (v *CompositeValue) isInvalidatedResource(_ *Interpreter) bool { + return v.isDestroyed || (v.dictionary == nil && v.Kind == common.CompositeKindResource) +} + +func (v *CompositeValue) checkInvalidatedResourceUse(interpreter *Interpreter, locationRange LocationRange) { + if v.isInvalidatedResource(interpreter) { panic(InvalidatedResourceError{ LocationRange: locationRange, }) @@ -16728,7 +16737,7 @@ func (v *CompositeValue) RemoveMember( config := interpreter.SharedState.Config if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) + v.checkInvalidatedResourceUse(interpreter, locationRange) } if config.TracingEnabled { @@ -16795,7 +16804,7 @@ func (v *CompositeValue) SetMember( config := interpreter.SharedState.Config if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) + v.checkInvalidatedResourceUse(interpreter, locationRange) } if config.TracingEnabled { @@ -16931,7 +16940,7 @@ func (v *CompositeValue) GetField(interpreter *Interpreter, locationRange Locati config := interpreter.SharedState.Config if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) + v.checkInvalidatedResourceUse(interpreter, locationRange) } storable, err := v.dictionary.Get( @@ -17183,7 +17192,7 @@ func (v *CompositeValue) Transfer( // Should be checked before accessing `v.dictionary`. if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) + v.checkInvalidatedResourceUse(interpreter, locationRange) } baseUse, elementOverhead, dataUse, metaDataUse := common.NewCompositeMemoryUsages(v.dictionary.Count(), 0) @@ -17940,8 +17949,12 @@ func (v *DictionaryValue) IsDestroyed() bool { return v.isDestroyed } +func (v *DictionaryValue) isInvalidatedResource(interpreter *Interpreter) bool { + return v.isDestroyed || (v.dictionary == nil && v.IsResourceKinded(interpreter)) +} + func (v *DictionaryValue) checkInvalidatedResourceUse(interpreter *Interpreter, locationRange LocationRange) { - if v.isDestroyed || (v.dictionary == nil && v.IsResourceKinded(interpreter)) { + if v.isInvalidatedResource(interpreter) { panic(InvalidatedResourceError{ LocationRange: locationRange, }) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 7346c5260e..ce7e63c0af 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9976,5 +9976,5 @@ func TestRuntimePreconditionDuplication(t *testing.T) { ) RequireError(t, err) - require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) } diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 4d810ef4e5..69e2b416a1 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2161,7 +2161,7 @@ func TestInterpreterResourcePreCondition(t *testing.T) { struct interface Receiver { pub fun deposit(from: @S) { - post { + pre { from != nil: "" } } @@ -2192,7 +2192,7 @@ func TestInterpreterResourcePostCondition(t *testing.T) { struct interface Receiver { pub fun deposit(from: @S) { post { - from != nil: "" + from != nil: "" // This be an error. Resource is destroyed at this point } } } @@ -2209,7 +2209,8 @@ func TestInterpreterResourcePostCondition(t *testing.T) { `) _, err := inter.Invoke("test") - require.NoError(t, err) + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) } func TestInterpreterResourcePreAndPostCondition(t *testing.T) { @@ -2222,10 +2223,10 @@ func TestInterpreterResourcePreAndPostCondition(t *testing.T) { struct interface Receiver { pub fun deposit(from: @S) { pre { - from != nil: "" + from != nil: "" // This is OK } post { - from != nil: "" + from != nil: "" // This is an error: Resource is destroyed at this point } } } @@ -2248,7 +2249,8 @@ func TestInterpreterResourcePreAndPostCondition(t *testing.T) { `) _, err := inter.Invoke("test") - require.NoError(t, err) + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) } func TestInterpreterResourceConditionAdditionalParam(t *testing.T) { @@ -2290,7 +2292,7 @@ func TestInterpreterResourceConditionAdditionalParam(t *testing.T) { require.NoError(t, err) } -func TestInterpreterResourceDoubleWrappedCondition(t *testing.T) { +func TestInterpreterResourceDoubleWrappedPreCondition(t *testing.T) { t.Parallel() @@ -2302,28 +2304,60 @@ func TestInterpreterResourceDoubleWrappedCondition(t *testing.T) { pre { from != nil: "" } - post { + } + } + + struct interface B { + pub fun deposit(from: @S) { + pre { from != nil: "" } } } - struct interface B { + struct Vault: A, B { pub fun deposit(from: @S) { pre { from != nil: "" } + destroy from + } + } + + fun test() { + Vault().deposit(from: <-create S()) + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) +} + +func TestInterpreterResourceDoubleWrappedPostCondition(t *testing.T) { + + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + resource S {} + + struct interface A { + pub fun deposit(from: @S) { post { from != nil: "" } } } - struct Vault: A, B { + struct interface B { pub fun deposit(from: @S) { - pre { + post { from != nil: "" } + } + } + + struct Vault: A, B { + pub fun deposit(from: @S) { post { 1 > 0: "" } @@ -2337,7 +2371,8 @@ func TestInterpreterResourceDoubleWrappedCondition(t *testing.T) { `) _, err := inter.Invoke("test") - require.NoError(t, err) + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) } func TestInterpretOptionalResourceReference(t *testing.T) { @@ -2943,7 +2978,10 @@ func TestInterpretInnerResourceMove(t *testing.T) { t.Parallel() - inter := parseCheckAndInterpret(t, ` + t.Run("assignment", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` pub resource OuterResource { pub var a: @InnerResource pub var b: @InnerResource @@ -2973,11 +3011,206 @@ func TestInterpretInnerResourceMove(t *testing.T) { pub fun main() { let a <- create OuterResource() + destroy a + }`, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + }) + + t.Run("second transfer", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + pub resource OuterResource { + pub var a: @InnerResource + pub var b: @InnerResource + + init() { + self.a <- create InnerResource() + self.b <- create InnerResource() + } + + pub fun swap() { + self.a <-> self.b + } + + destroy() { + var a <- create InnerResource() + + // Nested resource is moved here once + var temp <- a <- self.a + + // Nested resource is again moved here. This one should fail. + self.swap() + + destroy a + destroy temp + destroy self.b + } + } + + pub resource InnerResource {} + + pub fun main() { + let a <- create OuterResource() + destroy a + }`, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + }) +} + +func TestInterpretSetMemberResourceLoss(t *testing.T) { + + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + access(all) resource R { + access(all) let id: String + init(id: String) { + self.id = id + } + } + access(all) fun putBack(_ rl: &ResourceLoser, _ r: @R) { + rl.inner <-! r + } + access(all) resource ResourceLoser { + pub(set) var inner: @R?; + init(toLose: @R) { + self.inner <- toLose + } + destroy() { + var ref = &self as &ResourceLoser; + // Let's move self.inner out + var a <- self.inner + // And force-write it back + putBack(ref, <- a!) + } + } + access(all) fun main(): Void { + var resource <- create R(id: "abc"); + var rl <- create ResourceLoser(toLose: <- resource); + destroy rl + }`, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) +} + +func TestInterpretValueTransferResourceLoss(t *testing.T) { + + t.Parallel() + + inter, getLogs, _ := parseCheckAndInterpretWithLogs(t, ` + access(all) resource R { + access(all) let id: String + init(_ id: String) { + log("Creating ".concat(id)) + self.id = id + } + destroy() { + log("Destroying ".concat(self.id)) + } + } + + access(all) struct IndexSwitcher { + access(self) var counter: Int + init() { + self.counter = 0 + } + access(all) fun callback(): Int { + self.counter = self.counter + 1 + if self.counter == 1 { + // Which key we want to be read? + // Let's point it to a non-existent key + return 123 + } else { + // Which key we want to be assigned to? + // We point it to 0 to overwrite the victim value + return 0 + } + } + } + + access(all) fun loseResource(victim: @R) { + var in <- create R("dummy resource") + var dict: @{Int: R} <- { 0: <- victim } + var indexSwitcher = IndexSwitcher() + + // this callback should only be evaluated once, rather than twice + var out <- dict[indexSwitcher.callback()] <- in + destroy out + destroy dict + } + + access(all) fun main(): Void { + var victim <- create R("victim resource") + loseResource(victim: <- victim) + } + `, + ) + + _, err := inter.Invoke("main") + require.NoError(t, err) + + assert.Equal(t, + []string{ + `"Creating victim resource"`, + `"Creating dummy resource"`, + `"Destroying dummy resource"`, + `"Destroying victim resource"`, + }, + getLogs(), + ) +} + +func TestInterpretPreConditionResourceMove(t *testing.T) { + + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + pub resource Vault { } + + pub resource interface Interface { + pub fun foo(_ r: @AnyResource) { + pre { + consume(&r as &AnyResource, <- r) + } + } + } + + pub resource Implementation: Interface { + pub fun foo(_ r: @AnyResource) { + pre { + consume(&r as &AnyResource, <- r) + } + } + } + + pub fun consume(_ unusedRef: &AnyResource?, _ r: @AnyResource): Bool { + destroy r + return true + } + + pub fun main() { + let a <- create Implementation() + let b <- create Vault() + + a.foo(<-b) + destroy a }`, ) _, err := inter.Invoke("main") RequireError(t, err) - require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) } From 77b6ccd68f281f936e2d7151cf31fdfbfb93ba5c Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 16 Nov 2023 16:09:56 -0800 Subject: [PATCH 05/10] Refactor and cleanup --- runtime/interpreter/interpreter_expression.go | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 076013328d..830321d92b 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -972,16 +972,6 @@ func (interpreter *Interpreter) visitInvocationExpressionWithImplicitArgument(in panic(errors.NewUnreachableError()) } - // Bound functions - if boundFunction, ok := function.(BoundFunctionValue); ok && boundFunction.Self != nil { - // TODO: - //self := *boundFunction.Self - //if resource, ok := self.(ReferenceTrackedResourceKindedValue); ok { - // storageID := resource.StorageID() - // interpreter.trackReferencedResourceKindedValue(storageID, resource) - //} - } - // NOTE: evaluate all argument expressions in call-site scope, not in function body var argumentExpressions []ast.Expression @@ -1257,17 +1247,17 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as } case *sema.ReferenceType: - // TODO: // Case (3): target type is non-optional, actual value is optional. - // Unwrap the optional and add it to reference tracking. - //if someValue, ok := referencedValue.(*SomeValue); ok { - // locationRange := LocationRange{ - // Location: interpreter.Location, - // HasPosition: referenceExpression.Expression, - // } - // innerValue := someValue.InnerValue(interpreter, locationRange) - // interpreter.maybeTrackReferencedResourceKindedValue(innerValue) - //} + // This path shouldn't be reachable. This is only a defensive step + // to ensure references are properly created/tracked. + if someValue, ok := result.(*SomeValue); ok { + locationRange := LocationRange{ + Location: interpreter.Location, + HasPosition: referenceExpression.Expression, + } + innerValue := someValue.InnerValue(interpreter, locationRange) + return NewEphemeralReferenceValue(interpreter, typ.Authorized, innerValue, typ.Type) + } // Case (4): target type is non-optional, actual value is also non-optional return NewEphemeralReferenceValue(interpreter, typ.Authorized, result, typ.Type) From 3454b154b8936d483224c22f5c4f85a829e0eb63 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 17 Nov 2023 12:04:54 -0800 Subject: [PATCH 06/10] Remove redundant invalidated-resource checks --- runtime/environment.go | 1 - runtime/interpreter/config.go | 2 - runtime/interpreter/interpreter.go | 15 +- runtime/interpreter/value.go | 232 +----------------- runtime/tests/interpreter/interpreter_test.go | 1 - 5 files changed, 10 insertions(+), 241 deletions(-) diff --git a/runtime/environment.go b/runtime/environment.go index 561dd448f0..1e10a7af49 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -168,7 +168,6 @@ func newInterpreterEnvironment(config Config) *interpreterEnvironment { func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config { return &interpreter.Config{ - InvalidatedResourceValidationEnabled: true, MemoryGauge: e, BaseActivationHandler: e.getBaseActivation, OnEventEmitted: e.newOnEventEmittedHandler(), diff --git a/runtime/interpreter/config.go b/runtime/interpreter/config.go index 90bd3c8981..f2535dbe18 100644 --- a/runtime/interpreter/config.go +++ b/runtime/interpreter/config.go @@ -59,8 +59,6 @@ type Config struct { OnStatement OnStatementFunc // OnLoopIteration is triggered when a loop iteration is about to be executed OnLoopIteration OnLoopIterationFunc - // InvalidatedResourceValidationEnabled determines if the validation of invalidated resources is enabled - InvalidatedResourceValidationEnabled bool // TracingEnabled determines if tracing is enabled. // Tracing reports certain operations, e.g. composite value transfers TracingEnabled bool diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 0d153d3c3b..b618061eb5 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -5310,10 +5310,7 @@ func (interpreter *Interpreter) startResourceTracking( hasPosition ast.HasPosition, ) { - config := interpreter.SharedState.Config - - if !config.InvalidatedResourceValidationEnabled || - identifier == sema.SelfIdentifier { + if identifier == sema.SelfIdentifier { return } @@ -5345,10 +5342,8 @@ func (interpreter *Interpreter) checkInvalidatedResourceUse( identifier string, hasPosition ast.HasPosition, ) { - config := interpreter.SharedState.Config - if !config.InvalidatedResourceValidationEnabled || - identifier == sema.SelfIdentifier { + if identifier == sema.SelfIdentifier { return } @@ -5391,12 +5386,6 @@ func (interpreter *Interpreter) resourceForValidation(value Value) ResourceKinde } func (interpreter *Interpreter) invalidateResource(value Value) { - config := interpreter.SharedState.Config - - if !config.InvalidatedResourceValidationEnabled { - return - } - if value == nil || !value.IsResourceKinded(interpreter) { return } diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 8c49354254..322cca11ae 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -1776,24 +1776,12 @@ func (v *ArrayValue) isInvalidatedResource(interpreter *Interpreter) bool { return v.isDestroyed || (v.array == nil && v.IsResourceKinded(interpreter)) } -func (v *ArrayValue) checkInvalidatedResourceUse(interpreter *Interpreter, locationRange LocationRange) { - if v.isInvalidatedResource(interpreter) { - panic(InvalidatedResourceError{ - LocationRange: locationRange, - }) - } -} - func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRange) { interpreter.ReportComputation(common.ComputationKindDestroyArrayValue, 1) config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -1822,10 +1810,7 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan ) v.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - v.array = nil - } + v.array = nil interpreter.updateReferencedResource( storageID, @@ -1837,10 +1822,7 @@ func (v *ArrayValue) Destroy(interpreter *Interpreter, locationRange LocationRan } arrayValue.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - arrayValue.array = nil - } + arrayValue.array = nil }, ) } @@ -1917,12 +1899,6 @@ func (v *ArrayValue) Concat(interpreter *Interpreter, locationRange LocationRang } func (v *ArrayValue) GetKey(interpreter *Interpreter, locationRange LocationRange, key Value) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - index := key.(NumberValue).ToInt(locationRange) return v.Get(interpreter, locationRange, index) } @@ -1962,12 +1938,6 @@ func (v *ArrayValue) Get(interpreter *Interpreter, locationRange LocationRange, } func (v *ArrayValue) SetKey(interpreter *Interpreter, locationRange LocationRange, key Value, value Value) { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - index := key.(NumberValue).ToInt(locationRange) v.Set(interpreter, locationRange, index, value) } @@ -2090,12 +2060,6 @@ func (v *ArrayValue) AppendAll(interpreter *Interpreter, locationRange LocationR } func (v *ArrayValue) InsertKey(interpreter *Interpreter, locationRange LocationRange, key Value, value Value) { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - index := key.(NumberValue).ToInt(locationRange) v.Insert(interpreter, locationRange, index, value) } @@ -2148,12 +2112,6 @@ func (v *ArrayValue) Insert(interpreter *Interpreter, locationRange LocationRang } func (v *ArrayValue) RemoveKey(interpreter *Interpreter, locationRange LocationRange, key Value) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - index := key.(NumberValue).ToInt(locationRange) return v.Remove(interpreter, locationRange, index) } @@ -2254,11 +2212,6 @@ func (v *ArrayValue) Contains( } func (v *ArrayValue) GetMember(interpreter *Interpreter, locationRange LocationRange, name string) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } switch name { case "length": return NewIntValueFromInt64(interpreter, int64(v.Count())) @@ -2519,23 +2472,11 @@ func (v *ArrayValue) GetMember(interpreter *Interpreter, locationRange LocationR } func (v *ArrayValue) RemoveMember(interpreter *Interpreter, locationRange LocationRange, _ string) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - // Arrays have no removable members (fields / functions) panic(errors.NewUnreachableError()) } func (v *ArrayValue) SetMember(interpreter *Interpreter, locationRange LocationRange, _ string, _ Value) bool { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - // Arrays have no settable members (fields / functions) panic(errors.NewUnreachableError()) } @@ -2668,10 +2609,6 @@ func (v *ArrayValue) Transfer( config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - interpreter.ReportComputation(common.ComputationKindTransferArrayValue, uint(v.Count())) if config.TracingEnabled { @@ -16463,10 +16400,6 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -16534,10 +16467,7 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio ) v.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - v.dictionary = nil - } + v.dictionary = nil interpreter.updateReferencedResource( storageID, @@ -16549,10 +16479,7 @@ func (v *CompositeValue) Destroy(interpreter *Interpreter, locationRange Locatio } compositeValue.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - compositeValue.dictionary = nil - } + compositeValue.dictionary = nil }, ) @@ -16573,10 +16500,6 @@ func (v *CompositeValue) getBuiltinMember(interpreter *Interpreter, locationRang func (v *CompositeValue) GetMember(interpreter *Interpreter, locationRange LocationRange, name string) Value { config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -16640,14 +16563,6 @@ func (v *CompositeValue) isInvalidatedResource(_ *Interpreter) bool { return v.isDestroyed || (v.dictionary == nil && v.Kind == common.CompositeKindResource) } -func (v *CompositeValue) checkInvalidatedResourceUse(interpreter *Interpreter, locationRange LocationRange) { - if v.isInvalidatedResource(interpreter) { - panic(InvalidatedResourceError{ - LocationRange: locationRange, - }) - } -} - func (v *CompositeValue) getInterpreter(interpreter *Interpreter) *Interpreter { // Get the correct interpreter. The program code might need to be loaded. @@ -16736,10 +16651,6 @@ func (v *CompositeValue) RemoveMember( config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -16803,10 +16714,6 @@ func (v *CompositeValue) SetMember( ) bool { config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -16937,12 +16844,6 @@ func formatComposite(memoryGauge common.MemoryGauge, typeId string, fields []Com } func (v *CompositeValue) GetField(interpreter *Interpreter, locationRange LocationRange, name string) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - storable, err := v.dictionary.Get( StringAtreeValueComparator, StringAtreeValueHashInput, @@ -17190,11 +17091,6 @@ func (v *CompositeValue) Transfer( config := interpreter.SharedState.Config - // Should be checked before accessing `v.dictionary`. - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - baseUse, elementOverhead, dataUse, metaDataUse := common.NewCompositeMemoryUsages(v.dictionary.Count(), 0) common.UseMemory(interpreter, baseUse) common.UseMemory(interpreter, elementOverhead) @@ -17953,24 +17849,12 @@ func (v *DictionaryValue) isInvalidatedResource(interpreter *Interpreter) bool { return v.isDestroyed || (v.dictionary == nil && v.IsResourceKinded(interpreter)) } -func (v *DictionaryValue) checkInvalidatedResourceUse(interpreter *Interpreter, locationRange LocationRange) { - if v.isInvalidatedResource(interpreter) { - panic(InvalidatedResourceError{ - LocationRange: locationRange, - }) - } -} - func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange LocationRange) { interpreter.ReportComputation(common.ComputationKindDestroyDictionaryValue, 1) config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -18004,10 +17888,6 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati v.isDestroyed = true - if config.InvalidatedResourceValidationEnabled { - v.dictionary = nil - } - interpreter.updateReferencedResource( storageID, storageID, @@ -18018,10 +17898,7 @@ func (v *DictionaryValue) Destroy(interpreter *Interpreter, locationRange Locati } dictionaryValue.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - dictionaryValue.dictionary = nil - } + dictionaryValue.dictionary = nil }, ) } @@ -18119,12 +17996,6 @@ func (v *DictionaryValue) Get( } func (v *DictionaryValue) GetKey(interpreter *Interpreter, locationRange LocationRange, keyValue Value) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - value, ok := v.Get(interpreter, locationRange, keyValue) if ok { return NewSomeValueNonCopying(interpreter, value) @@ -18141,12 +18012,6 @@ func (v *DictionaryValue) SetKey( ) { interpreter.validateMutation(v.StorageID(), locationRange) - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - interpreter.checkContainerMutation(v.Type.KeyType, keyValue, locationRange) interpreter.checkContainerMutation( OptionalStaticType{ // intentionally unmetered @@ -18225,10 +18090,6 @@ func (v *DictionaryValue) GetMember( ) Value { config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -18394,24 +18255,12 @@ func (v *DictionaryValue) GetMember( return nil } -func (v *DictionaryValue) RemoveMember(interpreter *Interpreter, locationRange LocationRange, _ string) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - +func (v *DictionaryValue) RemoveMember(_ *Interpreter, _ LocationRange, _ string) Value { // Dictionaries have no removable members (fields / functions) panic(errors.NewUnreachableError()) } -func (v *DictionaryValue) SetMember(interpreter *Interpreter, locationRange LocationRange, _ string, _ Value) bool { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - +func (v *DictionaryValue) SetMember(_ *Interpreter, _ LocationRange, _ string, _ Value) bool { // Dictionaries have no settable members (fields / functions) panic(errors.NewUnreachableError()) } @@ -18425,12 +18274,6 @@ func (v *DictionaryValue) RemoveKey( locationRange LocationRange, key Value, ) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - return v.Remove(interpreter, locationRange, key) } @@ -18740,10 +18583,6 @@ func (v *DictionaryValue) Transfer( config := interpreter.SharedState.Config - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(interpreter, locationRange) - } - if config.TracingEnabled { startTime := time.Now() @@ -19234,20 +19073,10 @@ func (v *SomeValue) IsDestroyed() bool { } func (v *SomeValue) Destroy(interpreter *Interpreter, locationRange LocationRange) { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) - } - innerValue := v.InnerValue(interpreter, locationRange) maybeDestroy(interpreter, locationRange, innerValue) v.isDestroyed = true - - if config.InvalidatedResourceValidationEnabled { - v.value = nil - } } func (v *SomeValue) String() string { @@ -19263,11 +19092,6 @@ func (v *SomeValue) MeteredString(memoryGauge common.MemoryGauge, seenReferences } func (v *SomeValue) GetMember(interpreter *Interpreter, locationRange LocationRange, name string) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) - } switch name { case sema.OptionalTypeMapFunctionName: return NewHostFunctionValue( @@ -19313,22 +19137,10 @@ func (v *SomeValue) GetMember(interpreter *Interpreter, locationRange LocationRa } func (v *SomeValue) RemoveMember(interpreter *Interpreter, locationRange LocationRange, _ string) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) - } - panic(errors.NewUnreachableError()) } func (v *SomeValue) SetMember(interpreter *Interpreter, locationRange LocationRange, _ string, _ Value) bool { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) - } - panic(errors.NewUnreachableError()) } @@ -19403,14 +19215,6 @@ func (v *SomeValue) IsResourceKinded(interpreter *Interpreter) bool { return v.value.IsResourceKinded(interpreter) } -func (v *SomeValue) checkInvalidatedResourceUse(locationRange LocationRange) { - if v.isDestroyed || v.value == nil { - panic(InvalidatedResourceError{ - LocationRange: locationRange, - }) - } -} - func (v *SomeValue) Transfer( interpreter *Interpreter, locationRange LocationRange, @@ -19419,12 +19223,6 @@ func (v *SomeValue) Transfer( storable atree.Storable, preventTransfer map[atree.StorageID]struct{}, ) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) - } - innerValue := v.value needsStoreTo := v.NeedsStoreTo(address) @@ -19458,15 +19256,7 @@ func (v *SomeValue) Transfer( // then mark the resource array as invalidated, by unsetting the backing array. // This allows raising an error when the resource array is attempted // to be transferred/moved again (see beginning of this function) - - if config.InvalidatedResourceValidationEnabled { - v.value = nil - } else { - v.value = innerValue - v.valueStorable = nil - res = v - } - + v.value = nil } if res == nil { @@ -19491,12 +19281,6 @@ func (v *SomeValue) DeepRemove(interpreter *Interpreter) { } func (v *SomeValue) InnerValue(interpreter *Interpreter, locationRange LocationRange) Value { - config := interpreter.SharedState.Config - - if config.InvalidatedResourceValidationEnabled { - v.checkInvalidatedResourceUse(locationRange) - } - return v.value } diff --git a/runtime/tests/interpreter/interpreter_test.go b/runtime/tests/interpreter/interpreter_test.go index 80f8e022e4..74b1b75caa 100644 --- a/runtime/tests/interpreter/interpreter_test.go +++ b/runtime/tests/interpreter/interpreter_test.go @@ -199,7 +199,6 @@ func parseCheckAndInterpretWithOptionsAndMemoryMetering( if options.Config != nil { config = *options.Config } - config.InvalidatedResourceValidationEnabled = true config.AtreeValueValidationEnabled = true config.AtreeStorageValidationEnabled = true if config.UUIDHandler == nil { From 82a4c525a0d5294e726df45c5fdbc59706c9efcf Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 30 Nov 2023 12:12:05 -0800 Subject: [PATCH 07/10] Resolve conflicts. Handle SomeValue --- runtime/interpreter/interpreter_expression.go | 2 +- runtime/interpreter/value.go | 12 +- runtime/runtime_test.go | 222 ------------------ runtime/tests/interpreter/resources_test.go | 44 ++++ 4 files changed, 55 insertions(+), 225 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 4ac9d65273..387e54b63b 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -305,7 +305,7 @@ func (interpreter *Interpreter) VisitIdentifierExpression(expression *ast.Identi func (interpreter *Interpreter) evalExpression(expression ast.Expression) Value { result := ast.AcceptExpression[Value](expression, interpreter) - resourceKindedValue, ok := result.(ReferenceTrackedResourceKindedValue) + resourceKindedValue, ok := result.(ResourceKindedValue) if ok && resourceKindedValue.isInvalidatedResource(interpreter) { panic(InvalidatedResourceError{ LocationRange: LocationRange{ diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index f74e7c0452..0cba8a3c61 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -202,6 +202,7 @@ type ResourceKindedValue interface { Value Destroy(interpreter *Interpreter, locationRange LocationRange) IsDestroyed() bool + isInvalidatedResource(*Interpreter) bool } func maybeDestroy(interpreter *Interpreter, locationRange LocationRange, value Value) { @@ -219,7 +220,6 @@ type ReferenceTrackedResourceKindedValue interface { ResourceKindedValue IsReferenceTrackedResourceKindedValue() StorageID() atree.StorageID - isInvalidatedResource(*Interpreter) bool } // ContractValue is the value of a contract. @@ -19000,6 +19000,10 @@ func (NilValue) ChildStorables() []atree.Storable { return nil } +func (NilValue) isInvalidatedResource(_ *Interpreter) bool { + return false +} + // SomeValue type SomeValue struct { @@ -19287,10 +19291,14 @@ func (v *SomeValue) DeepRemove(interpreter *Interpreter) { } } -func (v *SomeValue) InnerValue(interpreter *Interpreter, locationRange LocationRange) Value { +func (v *SomeValue) InnerValue(_ *Interpreter, _ LocationRange) Value { return v.value } +func (v *SomeValue) isInvalidatedResource(_ *Interpreter) bool { + return v.value == nil || v.IsDestroyed() +} + type SomeStorable struct { gauge common.MemoryGauge Storable atree.Storable diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 19b5648b7e..f641c76023 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -10315,225 +10315,3 @@ func TestRuntimeIfLetElseBranchConfusion(t *testing.T) { RequireError(t, err) require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) } - -func TestRuntimePreconditionDuplication(t *testing.T) { - - t.Parallel() - - runtime := newTestInterpreterRuntime() - - signerAccount := common.MustBytesToAddress([]byte{0x1}) - - signers := []Address{signerAccount} - - accountCodes := map[Location][]byte{} - - 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 { - return nil - }, - log: func(s string) { - fmt.Println(s) - }, - } - - nextTransactionLocation := newTransactionLocationGenerator() - - attacker := []byte(fmt.Sprintf(` - import Bar from %[1]s - - access(all) contract Foo { - pub var temp: @Bar.Vault? - - init() { - self.temp <- nil - } - - pub fun doubler(_ vault: @Bar.Vault): @Bar.Vault { - var r <- create R(<-vault) - r.doMagic() - destroy r - var doubled <- self.temp <- nil - return <- doubled! - } - - pub fun preconditionFunction(_ unusedref: &Bar.Vault, _ v : @Bar.Vault, _ ref: &R): Bool { - // This gets called twice: once from the interface's precondition and the second - // time from the function's own precondition. We save both copies of the vault - // to the R object - if ref.counter == 0 { - ref.victim1 <-! v - ref.counter = 1 - } else { - ref.victim2 <-! v - } - return true - } - - pub resource interface RInterface{ - pub fun funcFromIface( _ v: @Bar.Vault, _ ref: &R): Void { - pre { - Foo.preconditionFunction(&v as &Bar.Vault, <- v, ref) - } - } - } - - pub resource R: RInterface { - pub var bounty: @Bar.Vault? - pub(set) var victim1: @Bar.Vault? - pub(set) var victim2: @Bar.Vault? - pub(set) var counter: Int - - init(_ v: @Bar.Vault) { - self.counter = 0 - self.bounty <- v - self.victim1 <- nil - self.victim2 <- nil - } - - pub fun funcFromIface(_ v: @Bar.Vault, _ ref: &R): Void { - pre { - Foo.preconditionFunction(&v as &Bar.Vault, <- v, ref) - } - } - - pub fun doMagic(): Void { - var origVault <- self.bounty <- nil - self.funcFromIface(<- origVault!, &self as &R) - - var v1 <- self.victim1 <- nil - var v2 <- self.victim2 <- nil - - // Following moves copied from Supun's test - var r1 = &v2 as &Bar.Vault? - Foo.account.save(<-v1!, to: /storage/v1) - Foo.account.save(<-v2!, to: /storage/v2) - var v1Reloaded <- Foo.account.load<@Bar.Vault>(from: /storage/v1)! - var v2Reloaded <- Foo.account.load<@Bar.Vault>(from: /storage/v2)! - - v1Reloaded.deposit(from:<-v2Reloaded) - Foo.temp <-! v1Reloaded - } - - destroy() { - destroy self.bounty - destroy self.victim1 - destroy self.victim2 - } - } - }`, - signerAccount.HexWithPrefix(), - )) - - bar := []byte(` - pub contract Bar { - 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 createEmptyVault(): @Bar.Vault { - return <- create Bar.Vault(balance: 0.0) - } - - pub fun createVault(balance: UFix64): @Bar.Vault { - return <- create Bar.Vault(balance: balance) - } - } - `) - - // Deploy Bar - - deployVault := DeploymentTransaction("Bar", bar) - err := runtime.ExecuteTransaction( - Script{ - Source: deployVault, - }, - Context{ - Interface: runtimeInterface, - Location: nextTransactionLocation(), - }, - ) - require.NoError(t, err) - - // Deploy Attacker - - deployAttacker := DeploymentTransaction("Foo", attacker) - - err = runtime.ExecuteTransaction( - Script{ - Source: deployAttacker, - }, - Context{ - Interface: runtimeInterface, - Location: nextTransactionLocation(), - }, - ) - require.NoError(t, err) - - // Attack - - attackTransaction := []byte(fmt.Sprintf(` - import Foo from %[1]s - import Bar from %[1]s - - transaction { - prepare(acc: AuthAccount) { - acc.save(<- Bar.createVault(balance: 100.0), to: /storage/vault)! - var vault = acc.borrow<&Bar.Vault>(from: /storage/vault)! - var flow <- vault.withdraw(amount: 42.0) - - var doubled <- Foo.doubler(<-flow) - log(doubled.balance) - - destroy doubled - } - }`, - signerAccount.HexWithPrefix(), - )) - - err = runtime.ExecuteTransaction( - Script{ - Source: attackTransaction, - }, - Context{ - Interface: runtimeInterface, - Location: nextTransactionLocation(), - }, - ) - - RequireError(t, err) - require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) -} diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 5d3d80b6c1..0c925f8f9d 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -3404,3 +3404,47 @@ func TestInterpretOptionalBindingElseBranch(t *testing.T) { // Error must be thrown at `<-r2` in the array literal assert.Equal(t, 18, invalidatedResourceErr.StartPosition().Line) } + +func TestInterpretPreConditionResourceMove(t *testing.T) { + + t.Parallel() + + inter, err := parseCheckAndInterpretWithOptions(t, ` + pub resource Vault { } + pub resource interface Interface { + pub fun foo(_ r: @AnyResource) { + pre { + consume(&r as &AnyResource, <- r) + } + } + } + pub resource Implementation: Interface { + pub fun foo(_ r: @AnyResource) { + pre { + consume(&r as &AnyResource, <- r) + } + } + } + pub fun consume(_ unusedRef: &AnyResource?, _ r: @AnyResource): Bool { + destroy r + return true + } + pub fun main() { + let a <- create Implementation() + let b <- create Vault() + a.foo(<-b) + destroy a + }`, + ParseCheckAndInterpretOptions{ + HandleCheckerError: func(err error) { + checkerErrors := checker.RequireCheckerErrors(t, err, 1) + require.IsType(t, &sema.InvalidInterfaceConditionResourceInvalidationError{}, checkerErrors[0]) + }, + }, + ) + require.NoError(t, err) + + _, err = inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) +} From 969c20b435dfebe226a8f6c08cbc196a6c19c1f4 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 30 Nov 2023 13:12:59 -0800 Subject: [PATCH 08/10] Lint --- runtime/environment.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/runtime/environment.go b/runtime/environment.go index 1e10a7af49..e02f26024d 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -168,22 +168,22 @@ func newInterpreterEnvironment(config Config) *interpreterEnvironment { func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config { return &interpreter.Config{ - MemoryGauge: e, - BaseActivationHandler: e.getBaseActivation, - OnEventEmitted: e.newOnEventEmittedHandler(), - OnAccountLinked: e.newOnAccountLinkedHandler(), - InjectedCompositeFieldsHandler: e.newInjectedCompositeFieldsHandler(), - UUIDHandler: e.newUUIDHandler(), - ContractValueHandler: e.newContractValueHandler(), - ImportLocationHandler: e.newImportLocationHandler(), - PublicAccountHandler: e.newPublicAccountHandler(), - AuthAccountHandler: e.newAuthAccountHandler(), - OnRecordTrace: e.newOnRecordTraceHandler(), - OnResourceOwnerChange: e.newResourceOwnerChangedHandler(), - CompositeTypeHandler: e.newCompositeTypeHandler(), - CompositeValueFunctionsHandler: e.newCompositeValueFunctionsHandler(), - TracingEnabled: e.config.TracingEnabled, - AtreeValueValidationEnabled: e.config.AtreeValidationEnabled, + MemoryGauge: e, + BaseActivationHandler: e.getBaseActivation, + OnEventEmitted: e.newOnEventEmittedHandler(), + OnAccountLinked: e.newOnAccountLinkedHandler(), + InjectedCompositeFieldsHandler: e.newInjectedCompositeFieldsHandler(), + UUIDHandler: e.newUUIDHandler(), + ContractValueHandler: e.newContractValueHandler(), + ImportLocationHandler: e.newImportLocationHandler(), + PublicAccountHandler: e.newPublicAccountHandler(), + AuthAccountHandler: e.newAuthAccountHandler(), + OnRecordTrace: e.newOnRecordTraceHandler(), + OnResourceOwnerChange: e.newResourceOwnerChangedHandler(), + CompositeTypeHandler: e.newCompositeTypeHandler(), + CompositeValueFunctionsHandler: e.newCompositeValueFunctionsHandler(), + TracingEnabled: e.config.TracingEnabled, + AtreeValueValidationEnabled: e.config.AtreeValidationEnabled, // NOTE: ignore e.config.AtreeValidationEnabled here, // and disable storage validation after each value modification. // Instead, storage is validated after commits (if validation is enabled), From 0c99453f588ea16f2ced8e732f9d5aae8b8c75c7 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 5 Dec 2023 09:38:35 -0800 Subject: [PATCH 09/10] Move tracking to the unmetered constructor --- runtime/interpreter/value.go | 15 +++++++++------ runtime/interpreter/value_test.go | 6 ++++-- runtime/tests/interpreter/member_test.go | 8 ++++---- runtime/tests/interpreter/resources_test.go | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 0cba8a3c61..f542c620f2 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -19703,15 +19703,20 @@ var _ MemberAccessibleValue = &EphemeralReferenceValue{} var _ ReferenceValue = &EphemeralReferenceValue{} func NewUnmeteredEphemeralReferenceValue( + interpreter *Interpreter, authorized bool, value Value, borrowedType sema.Type, ) *EphemeralReferenceValue { - return &EphemeralReferenceValue{ + ref := &EphemeralReferenceValue{ Authorized: authorized, Value: value, BorrowedType: borrowedType, } + + interpreter.maybeTrackReferencedResourceKindedValue(ref) + + return ref } func NewEphemeralReferenceValue( @@ -19721,9 +19726,7 @@ func NewEphemeralReferenceValue( borrowedType sema.Type, ) *EphemeralReferenceValue { common.UseMemory(interpreter, common.EphemeralReferenceValueMemoryUsage) - ref := NewUnmeteredEphemeralReferenceValue(authorized, value, borrowedType) - interpreter.maybeTrackReferencedResourceKindedValue(ref) - return ref + return NewUnmeteredEphemeralReferenceValue(interpreter, authorized, value, borrowedType) } func (*EphemeralReferenceValue) isValue() {} @@ -20008,8 +20011,8 @@ func (v *EphemeralReferenceValue) Transfer( return v } -func (v *EphemeralReferenceValue) Clone(_ *Interpreter) Value { - return NewUnmeteredEphemeralReferenceValue(v.Authorized, v.Value, v.BorrowedType) +func (v *EphemeralReferenceValue) Clone(inter *Interpreter) Value { + return NewUnmeteredEphemeralReferenceValue(inter, v.Authorized, v.Value, v.BorrowedType) } func (*EphemeralReferenceValue) DeepRemove(_ *Interpreter) { diff --git a/runtime/interpreter/value_test.go b/runtime/interpreter/value_test.go index db7d030122..b4413b0048 100644 --- a/runtime/interpreter/value_test.go +++ b/runtime/interpreter/value_test.go @@ -4164,8 +4164,9 @@ func TestValue_ConformsToStaticType(t *testing.T) { t.Parallel() test( - func(_ *Interpreter) Value { + func(inter *Interpreter) Value { return NewUnmeteredEphemeralReferenceValue( + inter, false, TrueValue, sema.BoolType, @@ -4175,8 +4176,9 @@ func TestValue_ConformsToStaticType(t *testing.T) { ) test( - func(_ *Interpreter) Value { + func(inter *Interpreter) Value { return NewUnmeteredEphemeralReferenceValue( + inter, false, TrueValue, sema.StringType, diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index 7a3c03e269..0ade907bfa 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -408,7 +408,7 @@ func TestInterpretMemberAccessType(t *testing.T) { sType := checker.RequireGlobalType(t, inter.Program.Elaboration, "S") - ref := interpreter.NewUnmeteredEphemeralReferenceValue(false, value, sType) + ref := interpreter.NewUnmeteredEphemeralReferenceValue(inter, false, value, sType) _, err = inter.Invoke("get", ref) require.NoError(t, err) @@ -455,7 +455,7 @@ func TestInterpretMemberAccessType(t *testing.T) { sType := checker.RequireGlobalType(t, inter.Program.Elaboration, "S") - ref := interpreter.NewUnmeteredEphemeralReferenceValue(false, value, sType) + ref := interpreter.NewUnmeteredEphemeralReferenceValue(inter, false, value, sType) _, err = inter.Invoke("get", ref) RequireError(t, err) @@ -497,7 +497,7 @@ func TestInterpretMemberAccessType(t *testing.T) { sType := checker.RequireGlobalType(t, inter.Program.Elaboration, "S") - ref := interpreter.NewUnmeteredEphemeralReferenceValue(false, value, sType) + ref := interpreter.NewUnmeteredEphemeralReferenceValue(inter, false, value, sType) _, err = inter.Invoke( "get", @@ -542,7 +542,7 @@ func TestInterpretMemberAccessType(t *testing.T) { sType := checker.RequireGlobalType(t, inter.Program.Elaboration, "S") - ref := interpreter.NewUnmeteredEphemeralReferenceValue(false, value, sType) + ref := interpreter.NewUnmeteredEphemeralReferenceValue(inter, false, value, sType) _, err = inter.Invoke( "get", diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 0c925f8f9d..e62f97d8f4 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2192,7 +2192,7 @@ func TestInterpreterResourcePostCondition(t *testing.T) { struct interface Receiver { pub fun deposit(from: @S) { post { - from != nil: "" // This be an error. Resource is destroyed at this point + from != nil: "" // This is an error. Resource is destroyed at this point } } } From bfdd84a72a72cea758cf310be50069e3de562e60 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 5 Dec 2023 11:51:33 -0800 Subject: [PATCH 10/10] Add test for storage reference bound function --- runtime/storage_test.go | 108 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/runtime/storage_test.go b/runtime/storage_test.go index b90f76d35e..2da5838a41 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -4192,3 +4192,111 @@ func TestRuntimeStorageIteration(t *testing.T) { }) }) } + +func TestRuntimeStorageReferenceBoundFunction(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + signerAddress := common.MustBytesToAddress([]byte{0x42}) + + deployTx := DeploymentTransaction("Test", []byte(` + access(all) contract Test { + + access(all) resource R { + access(all) fun foo() {} + } + + access(all) fun createR(): @R { + return <-create R() + } + } + `)) + + accountCodes := map[Location][]byte{} + var events []cadence.Event + var loggedMessages []string + + runtimeInterface := &testRuntimeInterface{ + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return []Address{signerAddress}, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + updateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + code = accountCodes[location] + return code, nil + }, + emitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + log: 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: AuthAccount) { + signer.save(<-Test.createR(), to: /storage/r) + + signer.link<&Test.R>( + /public/r, + target: /storage/r + ) + + let ref = signer.getCapability<&Test.R>(/public/r).borrow()! + + var func = ref.foo + + let r <- signer.load<@Test.R>(from: /storage/r)! + + // Should be OK + func() + + destroy r + + // Should fail! + func() + } + } + ` + + err = runtime.ExecuteTransaction( + Script{ + Source: []byte(testTx), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) +}