Merge pull request #2854 from onflow/bastian/sync-stable-cadence-10

runtime/cmd/cmd.go

@@ -366,6 +366,11 @@ func (*StandardLibraryHandler) RecordContractUpdate(_ common.AddressLocation, _
 	// NO-OP
 }
 
+func (h *StandardLibraryHandler) ContractUpdateRecorded(_ common.AddressLocation) bool {
+	// NO-OP
+	return false
+}
+
 func (*StandardLibraryHandler) InterpretContract(
 	_ common.AddressLocation,
 	_ *interpreter.Program,

runtime/contract_test.go

@@ -309,6 +309,8 @@ func TestRuntimeContract(t *testing.T) {
 			)
 			RequireError(t, err)
 
+			require.ErrorContains(t, err, "cannot overwrite existing contract")
+
 			// the deployed code should not have been updated,
 			// and no events should have been emitted,
 			// as the deployment should fail
@@ -450,6 +452,8 @@ func TestRuntimeContract(t *testing.T) {
 			} else {
 				RequireError(t, err)
 
+				require.ErrorContains(t, err, "cannot overwrite existing contract")
+
 				require.Empty(t, deployedCode)
 				require.Empty(t, events)
 				require.Empty(t, loggedMessages)
@@ -475,9 +479,11 @@ func TestRuntimeContract(t *testing.T) {
 					Location:  nextTransactionLocation(),
 				},
 			)
-			require.NoError(t, err)
+			RequireError(t, err)
 
-			require.Equal(t, []byte(tc.code2), deployedCode)
+			require.ErrorContains(t, err, "cannot overwrite existing contract")
+
+			require.Empty(t, deployedCode)
 
 			require.Equal(t,
 				[]string{
@@ -486,20 +492,18 @@ func TestRuntimeContract(t *testing.T) {
 					`"Test"`,
 					codeArrayString,
 					`nil`,
-					`"Test"`,
-					code2ArrayString,
-					`"Test"`,
-					code2ArrayString,
 				},
 				loggedMessages,
 			)
 
-			require.Len(t, events, 2)
-			assert.EqualValues(t, stdlib.AccountContractRemovedEventType.ID(), events[0].Type().ID())
-			assert.EqualValues(t, stdlib.AccountContractAddedEventType.ID(), events[1].Type().ID())
+			require.Len(t, events, 1)
+			assert.EqualValues(t,
+				stdlib.AccountContractRemovedEventType.ID(),
+				events[0].Type().ID(),
+			)
 
 			contractValueExists := getContractValueExists()
-
+			// contract still exists (from previous transaction), if not interface
 			if tc.isInterface {
 				require.False(t, contractValueExists)
 			} else {

runtime/contract_update_test.go

@@ -29,7 +29,7 @@ import (
 	"github.com/onflow/cadence/runtime/common"
 	"github.com/onflow/cadence/runtime/interpreter"
 	. "github.com/onflow/cadence/runtime/tests/runtime_utils"
-	"github.com/onflow/cadence/runtime/tests/utils"
+	. "github.com/onflow/cadence/runtime/tests/utils"
 )
 
 func TestRuntimeContractUpdateWithDependencies(t *testing.T) {
@@ -142,7 +142,7 @@ func TestRuntimeContractUpdateWithDependencies(t *testing.T) {
 
 	err := runtime.ExecuteTransaction(
 		Script{
-			Source: utils.DeploymentTransaction(
+			Source: DeploymentTransaction(
 				"Foo",
 				[]byte(fooContractV1),
 			),
@@ -163,7 +163,7 @@ func TestRuntimeContractUpdateWithDependencies(t *testing.T) {
 
 	err = runtime.ExecuteTransaction(
 		Script{
-			Source: utils.DeploymentTransaction(
+			Source: DeploymentTransaction(
 				"Bar",
 				[]byte(barContractV1),
 			),
@@ -183,7 +183,7 @@ func TestRuntimeContractUpdateWithDependencies(t *testing.T) {
 	signerAccount = common.MustBytesToAddress([]byte{0x1})
 	err = runtime.ExecuteTransaction(
 		Script{
-			Source: utils.UpdateTransaction("Foo", []byte(fooContractV2)),
+			Source: UpdateTransaction("Foo", []byte(fooContractV2)),
 		},
 		Context{
 			Interface: runtimeInterface,
@@ -204,7 +204,7 @@ func TestRuntimeContractUpdateWithDependencies(t *testing.T) {
 
 	err = runtime.ExecuteTransaction(
 		Script{
-			Source: utils.UpdateTransaction("Bar", []byte(barContractV2)),
+			Source: UpdateTransaction("Bar", []byte(barContractV2)),
 		},
 		Context{
 			Interface: runtimeInterface,
@@ -283,7 +283,7 @@ func TestRuntimeContractUpdateWithPrecedingIdentifiers(t *testing.T) {
 
 	err := runtime.ExecuteTransaction(
 		Script{
-			Source: utils.UpdateTransaction("Foo", []byte(fooContractV2)),
+			Source: UpdateTransaction("Foo", []byte(fooContractV2)),
 		},
 		Context{
 			Interface: runtimeInterface,
@@ -293,3 +293,107 @@ func TestRuntimeContractUpdateWithPrecedingIdentifiers(t *testing.T) {
 	require.NoError(t, err)
 
 }
+
+func TestRuntimeInvalidContractRedeploy(t *testing.T) {
+
+	t.Parallel()
+
+	foo1 := []byte(`
+        access(all)
+        contract Foo {
+
+            access(all)
+            resource R {
+
+                access(all)
+                var x: Int
+
+                init() {
+                    self.x = 0
+                }
+            }
+
+            access(all)
+            fun createR(): @R {
+                return <-create R()
+            }
+        }
+    `)
+
+	foo2 := []byte(`
+        access(all)
+        contract Foo {
+
+            access(all)
+            struct R {
+                access(all)
+                var x: Int
+
+                init() {
+                    self.x = 0
+                }
+            }
+        }
+    `)
+
+	tx := []byte(`
+      transaction(foo1: String, foo2: String) {
+          prepare(signer: auth(Contracts) &Account) {
+              signer.contracts.add(name: "Foo", code: foo1.utf8)
+              signer.contracts.add(name: "Foo", code: foo2.utf8)
+          }
+      }
+    `)
+
+	runtime := NewTestInterpreterRuntimeWithConfig(Config{
+		AtreeValidationEnabled: false,
+	})
+
+	address := common.MustBytesToAddress([]byte{0x1})
+
+	var events []cadence.Event
+
+	runtimeInterface := &TestRuntimeInterface{
+		Storage: NewTestLedger(nil, nil),
+		OnGetSigningAccounts: func() ([]Address, error) {
+			return []Address{address}, nil
+		},
+		OnGetAccountContractCode: func(location common.AddressLocation) ([]byte, error) {
+			return nil, nil
+		},
+		OnResolveLocation: NewSingleIdentifierLocationResolver(t),
+		OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error {
+			// "delay"
+			return nil
+		},
+		OnEmitEvent: func(event cadence.Event) error {
+			events = append(events, event)
+			return nil
+		},
+		OnDecodeArgument: func(b []byte, t cadence.Type) (value cadence.Value, err error) {
+			return json.Decode(nil, b)
+		},
+	}
+
+	nextTransactionLocation := NewTransactionLocationGenerator()
+
+	// Deploy
+
+	err := runtime.ExecuteTransaction(
+		Script{
+			Source: tx,
+			Arguments: encodeArgs([]cadence.Value{
+				cadence.String(foo1),
+				cadence.String(foo2),
+			}),
+		},
+		Context{
+			Interface: runtimeInterface,
+			Location:  nextTransactionLocation(),
+		},
+	)
+
+	RequireError(t, err)
+
+	require.ErrorContains(t, err, "cannot overwrite existing contract")
+}

runtime/environment.go

@@ -318,6 +318,10 @@ func (e *interpreterEnvironment) RecordContractUpdate(
 	e.storage.recordContractUpdate(location, contractValue)
 }
 
+func (e *interpreterEnvironment) ContractUpdateRecorded(location common.AddressLocation) bool {
+	return e.storage.contractUpdateRecorded(location)
+}
+
 func (e *interpreterEnvironment) TemporarilyRecordCode(location common.AddressLocation, code []byte) {
 	e.codesAndPrograms.setCode(location, code)
 }

runtime/interpreter/interpreter_expression.go

@@ -136,26 +136,53 @@ func (interpreter *Interpreter) valueIndexExpressionGetterSetter(indexExpression
 		},
 	)
 
+	// Normally, moves of nested resources (e.g `let r <- rs[0]`) are statically rejected.
+	//
+	// However, there are cases in which we do allow moves of nested resources:
+	//
+	// - In a swap statement (e.g. `rs[0] <-> rs[1]`)
+	// - In a variable declaration with two values/assignments (e.g. `let r <- rs["foo"] <- nil`)
+	//
+	// In both cases we know that a move of the nested resource is immediately followed by a replacement.
+	// This notion of an expression that moves a nested resource is tracked in the elaboration.
+	//
+	// When indexing is a move of a nested resource, we need to remove the key/value from the container.
+	// However, for some containers, like arrays, the removal influences other values in the container.
+	// In case of an array, the removal of an element shifts all following elements.
+	//
+	// A removal alone would thus result in subsequent code being executed incorrectly.
+	// For example, in the case where a swap operation through indexing is performed on the same array,
+	// e.g. `rs[0] <-> rs[1]`, once the first removal was performed, the second operates on a modified container.
+	//
+	// Prevent this problem by temporarily writing a placeholder value after the removal.
+	// Only perform the replacement with a placeholder in the case of a nested resource move.
+	// We know that in that case the get operation will be followed by a set operation,
+	// which will replace the temporary placeholder.
+
 	isNestedResourceMove := elaboration.IsNestedResourceMoveExpression(indexExpression)
 
+	var get func(allowMissing bool) Value
+
+	if isNestedResourceMove {
+		get = func(_ bool) Value {
+			value := target.RemoveKey(interpreter, locationRange, transferredIndexingValue)
+			target.InsertKey(interpreter, locationRange, transferredIndexingValue, placeholder)
+			return value
+		}
+	} else {
+		get = func(_ bool) Value {
+			value := target.GetKey(interpreter, locationRange, transferredIndexingValue)
+
+			// If the indexing value is a reference, then return a reference for the resulting value.
+			return interpreter.maybeGetReference(indexExpression, value)
+		}
+	}
+
 	return getterSetter{
 		target: target,
-		get: func(_ bool) Value {
-			if isNestedResourceMove {
-				return target.RemoveKey(interpreter, locationRange, transferredIndexingValue)
-			} else {
-				value := target.GetKey(interpreter, locationRange, transferredIndexingValue)
-
-				// If the indexing value is a reference, then return a reference for the resulting value.
-				return interpreter.maybeGetReference(indexExpression, value)
-			}
-		},
+		get:    get,
 		set: func(value Value) {
-			if isNestedResourceMove {
-				target.InsertKey(interpreter, locationRange, transferredIndexingValue, value)
-			} else {
-				target.SetKey(interpreter, locationRange, transferredIndexingValue, value)
-			}
+			target.SetKey(interpreter, locationRange, transferredIndexingValue, value)
 		},
 	}
 }

runtime/interpreter/interpreter_statement.go

@@ -163,8 +163,10 @@ func (interpreter *Interpreter) visitIfStatementWithVariableDeclaration(
 	// If the resource was not moved ou of the container,
 	// its contents get deleted.
 
+	getterSetter := interpreter.assignmentGetterSetter(declaration.Value)
+
 	const allowMissing = false
-	value := interpreter.assignmentGetterSetter(declaration.Value).get(allowMissing)
+	value := getterSetter.get(allowMissing)
 	if value == nil {
 		panic(errors.NewUnreachableError())
 	}
@@ -527,8 +529,10 @@ func (interpreter *Interpreter) visitVariableDeclaration(
 	// If the resource was not moved ou of the container,
 	// its contents get deleted.
 
+	getterSetter := interpreter.assignmentGetterSetter(declaration.Value)
+
 	const allowMissing = false
-	result := interpreter.assignmentGetterSetter(declaration.Value).get(allowMissing)
+	result := getterSetter.get(allowMissing)
 	if result == nil {
 		panic(errors.NewUnreachableError())
 	}
@@ -581,23 +585,32 @@ func (interpreter *Interpreter) VisitAssignmentStatement(assignment *ast.Assignm
 }
 
 func (interpreter *Interpreter) VisitSwapStatement(swap *ast.SwapStatement) StatementResult {
+
+	// Get type information
+
 	swapStatementTypes := interpreter.Program.Elaboration.SwapStatementTypes(swap)
 	leftType := swapStatementTypes.LeftType
 	rightType := swapStatementTypes.RightType
 
-	const allowMissing = false
+	// Evaluate the left side (target and key)
 
-	// Evaluate the left expression
 	leftGetterSetter := interpreter.assignmentGetterSetter(swap.Left)
+
+	// Evaluate the right side (target and key)
+
+	rightGetterSetter := interpreter.assignmentGetterSetter(swap.Right)
+
+	// Get left and right values
+
+	const allowMissing = false
+
 	leftValue := leftGetterSetter.get(allowMissing)
 	interpreter.checkSwapValue(leftValue, swap.Left)
 
-	// Evaluate the right expression
-	rightGetterSetter := interpreter.assignmentGetterSetter(swap.Right)
 	rightValue := rightGetterSetter.get(allowMissing)
 	interpreter.checkSwapValue(rightValue, swap.Right)
 
-	// Set right value to left target
+	// Set right value to left target,
 	// and left value to right target
 
 	locationRange := LocationRange{

runtime/interpreter/value.go

@@ -18408,6 +18408,9 @@ func (v *DictionaryValue) SetKey(
 	case NilValue:
 		_ = v.Remove(interpreter, locationRange, keyValue)
 
+	case placeholderValue:
+		// NO-OP
+
 	default:
 		panic(errors.NewUnreachableError())
 	}