Merge pull request #3157 from onflow/bastian/skip-values-during-migra…

…tion

Optimize storage migration: Allow skipping of values

migrations/capcons/capabilitymigration.go

@@ -123,3 +123,53 @@ func (m *CapabilityValueMigration) Migrate(
 
 	return nil, nil
 }
+
+func (m *CapabilityValueMigration) CanSkip(valueType interpreter.StaticType) bool {
+	return CanSkipCapabilityValueMigration(valueType)
+}
+
+func CanSkipCapabilityValueMigration(valueType interpreter.StaticType) bool {
+	switch valueType := valueType.(type) {
+	case *interpreter.DictionaryStaticType:
+		return CanSkipCapabilityValueMigration(valueType.KeyType) &&
+			CanSkipCapabilityValueMigration(valueType.ValueType)
+
+	case interpreter.ArrayStaticType:
+		return CanSkipCapabilityValueMigration(valueType.ElementType())
+
+	case *interpreter.OptionalStaticType:
+		return CanSkipCapabilityValueMigration(valueType.Type)
+
+	case *interpreter.CapabilityStaticType:
+		return false
+
+	case interpreter.PrimitiveStaticType:
+
+		switch valueType {
+		case interpreter.PrimitiveStaticTypeCapability:
+			return false
+
+		case interpreter.PrimitiveStaticTypeBool,
+			interpreter.PrimitiveStaticTypeVoid,
+			interpreter.PrimitiveStaticTypeAddress,
+			interpreter.PrimitiveStaticTypeMetaType,
+			interpreter.PrimitiveStaticTypeBlock,
+			interpreter.PrimitiveStaticTypeString,
+			interpreter.PrimitiveStaticTypeCharacter:
+
+			return true
+		}
+
+		if !valueType.IsDeprecated() { //nolint:staticcheck
+			semaType := valueType.SemaType()
+
+			if sema.IsSubType(semaType, sema.NumberType) ||
+				sema.IsSubType(semaType, sema.PathType) {
+
+				return true
+			}
+		}
+	}
+
+	return false
+}

migrations/capcons/linkmigration.go

@@ -51,6 +51,11 @@ func (*LinkValueMigration) Name() string {
 	return "LinkValueMigration"
 }
 
+func (m *LinkValueMigration) CanSkip(valueType interpreter.StaticType) bool {
+	// Link values have a capability static type
+	return CanSkipCapabilityValueMigration(valueType)
+}
+
 func (m *LinkValueMigration) Migrate(
 	storageKey interpreter.StorageKey,
 	storageMapKey interpreter.StorageMapKey,

migrations/capcons/migration_test.go

@@ -2349,3 +2349,116 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) {
 	require.NoError(t, err)
 
 }
+
+func TestCanSkipCapabilityValueMigration(t *testing.T) {
+
+	t.Parallel()
+
+	testCases := map[interpreter.StaticType]bool{
+
+		// Primitive types, like Bool and Address
+
+		interpreter.PrimitiveStaticTypeBool:    true,
+		interpreter.PrimitiveStaticTypeAddress: true,
+
+		// Number and Path types, like UInt8 and StoragePath
+
+		interpreter.PrimitiveStaticTypeUInt8:       true,
+		interpreter.PrimitiveStaticTypeStoragePath: true,
+
+		// Capability types
+
+		interpreter.PrimitiveStaticTypeCapability: false,
+		&interpreter.CapabilityStaticType{
+			BorrowType: interpreter.PrimitiveStaticTypeString,
+		}: false,
+		&interpreter.CapabilityStaticType{
+			BorrowType: interpreter.PrimitiveStaticTypeCharacter,
+		}: false,
+
+		// Existential types, like AnyStruct and AnyResource
+
+		interpreter.PrimitiveStaticTypeAnyStruct:   false,
+		interpreter.PrimitiveStaticTypeAnyResource: false,
+	}
+
+	test := func(ty interpreter.StaticType, expected bool) {
+
+		t.Run(ty.String(), func(t *testing.T) {
+
+			t.Parallel()
+
+			t.Run("base", func(t *testing.T) {
+
+				t.Parallel()
+
+				actual := CanSkipCapabilityValueMigration(ty)
+				assert.Equal(t, expected, actual)
+
+			})
+
+			t.Run("optional", func(t *testing.T) {
+
+				t.Parallel()
+
+				optionalType := interpreter.NewOptionalStaticType(nil, ty)
+
+				actual := CanSkipCapabilityValueMigration(optionalType)
+				assert.Equal(t, expected, actual)
+			})
+
+			t.Run("variable-sized", func(t *testing.T) {
+
+				t.Parallel()
+
+				arrayType := interpreter.NewVariableSizedStaticType(nil, ty)
+
+				actual := CanSkipCapabilityValueMigration(arrayType)
+				assert.Equal(t, expected, actual)
+			})
+
+			t.Run("constant-sized", func(t *testing.T) {
+
+				t.Parallel()
+
+				arrayType := interpreter.NewConstantSizedStaticType(nil, ty, 2)
+
+				actual := CanSkipCapabilityValueMigration(arrayType)
+				assert.Equal(t, expected, actual)
+			})
+
+			t.Run("dictionary key", func(t *testing.T) {
+
+				t.Parallel()
+
+				dictionaryType := interpreter.NewDictionaryStaticType(
+					nil,
+					ty,
+					interpreter.PrimitiveStaticTypeInt,
+				)
+
+				actual := CanSkipCapabilityValueMigration(dictionaryType)
+				assert.Equal(t, expected, actual)
+
+			})
+
+			t.Run("dictionary value", func(t *testing.T) {
+
+				t.Parallel()
+
+				dictionaryType := interpreter.NewDictionaryStaticType(
+					nil,
+					interpreter.PrimitiveStaticTypeInt,
+					ty,
+				)
+
+				actual := CanSkipCapabilityValueMigration(dictionaryType)
+				assert.Equal(t, expected, actual)
+			})
+		})
+	}
+
+	for ty, expected := range testCases {
+		test(ty, expected)
+	}
+}

migrations/entitlements/migration.go

@@ -369,3 +369,7 @@ func (mig EntitlementsMigration) Migrate(
 ) {
 	return ConvertValueToEntitlements(mig.Interpreter, value)
 }
+
+func (mig EntitlementsMigration) CanSkip(valueType interpreter.StaticType) bool {
+	return statictypes.CanSkipStaticTypeMigration(valueType)
+}

migrations/entitlements/migration_test.go

@@ -695,6 +695,10 @@ func (m testEntitlementsMigration) Migrate(
 	return ConvertValueToEntitlements(m.inter, value)
 }
 
+func (m testEntitlementsMigration) CanSkip(_ interpreter.StaticType) bool {
+	return false
+}
+
 func convertEntireTestValue(
 	t *testing.T,
 	inter *interpreter.Interpreter,

migrations/migration.go

@@ -37,6 +37,7 @@ type ValueMigration interface {
 		value interpreter.Value,
 		interpreter *interpreter.Interpreter,
 	) (newValue interpreter.Value, err error)
+	CanSkip(valueType interpreter.StaticType) bool
 }
 
 type DomainMigration interface {
@@ -170,11 +171,29 @@ func (m *StorageMigration) MigrateNestedValue(
 		}
 	}()
 
+	inter := m.interpreter
+
+	// skip the migration of the value,
+	// if all value migrations agree
+
+	canSkip := true
+	staticType := value.StaticType(inter)
+	for _, migration := range valueMigrations {
+		if !migration.CanSkip(staticType) {
+			canSkip = false
+			break
+		}
+	}
+
+	if canSkip {
+		return
+	}
+
 	// Visit the children first, and migrate them.
 	// i.e: depth-first traversal
 	switch typedValue := value.(type) {
 	case *interpreter.SomeValue:
-		innerValue := typedValue.InnerValue(m.interpreter, emptyLocationRange)
+		innerValue := typedValue.InnerValue(inter, emptyLocationRange)
 		newInnerValue := m.MigrateNestedValue(
 			storageKey,
 			storageMapKey,
@@ -183,7 +202,7 @@ func (m *StorageMigration) MigrateNestedValue(
 			reporter,
 		)
 		if newInnerValue != nil {
-			migratedValue = interpreter.NewSomeValueNonCopying(m.interpreter, newInnerValue)
+			migratedValue = interpreter.NewSomeValueNonCopying(inter, newInnerValue)
 
 			// chain the migrations
 			value = migratedValue
@@ -196,7 +215,7 @@ func (m *StorageMigration) MigrateNestedValue(
 		count := array.Count()
 		for index := 0; index < count; index++ {
 
-			element := array.Get(m.interpreter, emptyLocationRange, index)
+			element := array.Get(inter, emptyLocationRange, index)
 
 			newElement := m.MigrateNestedValue(
 				storageKey,
@@ -211,17 +230,17 @@ func (m *StorageMigration) MigrateNestedValue(
 			}
 
 			existingStorable := array.RemoveWithoutTransfer(
-				m.interpreter,
+				inter,
 				emptyLocationRange,
 				index,
 			)
 
-			interpreter.StoredValue(m.interpreter, existingStorable, m.storage).
-				DeepRemove(m.interpreter)
-			m.interpreter.RemoveReferencedSlab(existingStorable)
+			interpreter.StoredValue(inter, existingStorable, m.storage).
+				DeepRemove(inter)
+			inter.RemoveReferencedSlab(existingStorable)
 
 			array.InsertWithoutTransfer(
-				m.interpreter,
+				inter,
 				emptyLocationRange,
 				index,
 				newElement,
@@ -241,7 +260,7 @@ func (m *StorageMigration) MigrateNestedValue(
 
 		for _, fieldName := range fieldNames {
 			existingValue := composite.GetField(
-				m.interpreter,
+				inter,
 				emptyLocationRange,
 				fieldName,
 			)
@@ -259,7 +278,7 @@ func (m *StorageMigration) MigrateNestedValue(
 			}
 
 			composite.SetMemberWithoutTransfer(
-				m.interpreter,
+				inter,
 				emptyLocationRange,
 				fieldName,
 				migratedValue,
@@ -329,7 +348,7 @@ func (m *StorageMigration) MigrateNestedValue(
 
 			existingKey = legacyKey(existingKey)
 			existingKeyStorable, existingValueStorable := dictionary.RemoveWithoutTransfer(
-				m.interpreter,
+				inter,
 				emptyLocationRange,
 				existingKey,
 			)
@@ -347,13 +366,13 @@ func (m *StorageMigration) MigrateNestedValue(
 				// Value was migrated
 				valueToSet = newValue
 
-				interpreter.StoredValue(m.interpreter, existingValueStorable, m.storage).
-					DeepRemove(m.interpreter)
-				m.interpreter.RemoveReferencedSlab(existingValueStorable)
+				interpreter.StoredValue(inter, existingValueStorable, m.storage).
+					DeepRemove(inter)
+				inter.RemoveReferencedSlab(existingValueStorable)
 			}
 
 			dictionary.InsertWithoutTransfer(
-				m.interpreter,
+				inter,
 				emptyLocationRange,
 				keyToSet,
 				valueToSet,
@@ -372,7 +391,7 @@ func (m *StorageMigration) MigrateNestedValue(
 		if newInnerValue != nil {
 			newInnerCapability := newInnerValue.(*interpreter.IDCapabilityValue)
 			migratedValue = interpreter.NewPublishedValue(
-				m.interpreter,
+				inter,
 				publishedValue.Recipient,
 				newInnerCapability,
 			)