Merge pull request #2958 from onflow/supun/improve-reference-tracking

Improve resource-reference tracking

runtime/environment.go

@@ -168,23 +168,22 @@ func newInterpreterEnvironment(config Config) *interpreterEnvironment {
 
 func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config {
 	return &interpreter.Config{
-		InvalidatedResourceValidationEnabled: true,
-		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),

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

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
@@ -838,14 +838,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)
 }
 
@@ -5260,18 +5258,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{}{}
@@ -5280,7 +5280,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 {
@@ -5304,10 +5304,7 @@ func (interpreter *Interpreter) startResourceTracking(
 	hasPosition ast.HasPosition,
 ) {
 
-	config := interpreter.SharedState.Config
-
-	if !config.InvalidatedResourceValidationEnabled ||
-		identifier == sema.SelfIdentifier {
+	if identifier == sema.SelfIdentifier {
 		return
 	}
 
@@ -5339,10 +5336,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
 	}
 
@@ -5385,12 +5380,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
 	}

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.(ResourceKindedValue)
+	if ok && resourceKindedValue.isInvalidatedResource(interpreter) {
+		panic(InvalidatedResourceError{
+			LocationRange: LocationRange{
+				Location:    interpreter.Location,
+				HasPosition: expression,
+			},
+		})
+	}
+
+	return result
 }
 
 func (interpreter *Interpreter) VisitBinaryExpression(expression *ast.BinaryExpression) Value {
@@ -960,15 +972,6 @@ func (interpreter *Interpreter) visitInvocationExpressionWithImplicitArgument(in
 		panic(errors.NewUnreachableError())
 	}
 
-	// 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)
-		}
-	}
-
 	// NOTE: evaluate all argument expressions in call-site scope, not in function body
 
 	var argumentExpressions []ast.Expression
@@ -1183,8 +1186,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
@@ -1211,7 +1212,6 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as
 			}
 
 			innerValue := result.InnerValue(interpreter, locationRange)
-			interpreter.maybeTrackReferencedResourceKindedValue(innerValue)
 
 			return NewSomeValueNonCopying(
 				interpreter,
@@ -1251,14 +1251,15 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as
 
 	case *sema.ReferenceType:
 		// Case (3): target type is non-optional, actual value is optional.
-		// Unwrap the optional and add it to reference tracking.
+		// 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)
-			interpreter.maybeTrackReferencedResourceKindedValue(innerValue)
+			return NewEphemeralReferenceValue(interpreter, typ.Authorized, innerValue, typ.Type)
 		}
 
 		// Case (4): target type is non-optional, actual value is also non-optional
@@ -1341,7 +1342,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,
@@ -1352,9 +1352,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,

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{}{},