Merge pull request #3110 from onflow/bastian/improve-migration

migrations/capcons/capabilitymigration.go

@@ -86,9 +86,11 @@ func (m *CapabilityValueMigration) Migrate(
 			break
 		}
 
-		newBorrowType, ok := oldCapability.BorrowType.(*interpreter.ReferenceStaticType)
+		oldBorrowType := oldCapability.BorrowType
+
+		newBorrowType, ok := oldBorrowType.(*interpreter.ReferenceStaticType)
 		if !ok {
-			panic(errors.NewUnreachableError())
+			panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", oldBorrowType))
 		}
 
 		// Convert the old AuthAccount type to the new fully-entitled Account type

migrations/capcons/linkmigration.go

@@ -93,9 +93,10 @@ func (m *LinkValueMigration) Migrate(
 
 	case interpreter.PathLinkValue: //nolint:staticcheck
 		var ok bool
-		borrowStaticType, ok = readValue.Type.(*interpreter.ReferenceStaticType)
+		borrowType := readValue.Type
+		borrowStaticType, ok = borrowType.(*interpreter.ReferenceStaticType)
 		if !ok {
-			panic(errors.NewUnreachableError())
+			panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", borrowType))
 		}
 
 	case interpreter.AccountLinkValue: //nolint:staticcheck
@@ -106,12 +107,13 @@ func (m *LinkValueMigration) Migrate(
 		)
 
 	default:
-		panic(errors.NewUnreachableError())
+		panic(errors.NewUnexpectedError("unexpected value type: %T", value))
 	}
 
-	borrowType, ok := inter.MustConvertStaticToSemaType(borrowStaticType).(*sema.ReferenceType)
+	convertedBorrowStaticType := inter.MustConvertStaticToSemaType(borrowStaticType)
+	borrowType, ok := convertedBorrowStaticType.(*sema.ReferenceType)
 	if !ok {
-		panic(errors.NewUnreachableError())
+		panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", borrowType))
 	}
 
 	// Get target
@@ -176,7 +178,7 @@ func (m *LinkValueMigration) Migrate(
 		)
 
 	default:
-		panic(errors.NewUnreachableError())
+		panic(errors.NewUnexpectedError("unexpected target type: %T", target))
 	}
 
 	// Record new capability ID in source path mapping.
@@ -310,9 +312,13 @@ func (m *LinkValueMigration) getPathCapabilityFinalTarget(
 				//    so it's possible that a capability value is encountered when determining the final target,
 				//    when a part of the full link chain was already previously migrated.
 
-				capabilityBorrowType, ok := inter.MustConvertStaticToSemaType(value.BorrowType).(*sema.ReferenceType)
+				convertedBorrowType := inter.MustConvertStaticToSemaType(value.BorrowType)
+				capabilityBorrowType, ok := convertedBorrowType.(*sema.ReferenceType)
 				if !ok {
-					panic(errors.NewUnreachableError())
+					panic(errors.NewUnexpectedError(
+						"unexpected non-reference borrow type: %T",
+						convertedBorrowType,
+					))
 				}
 
 				// Do not borrow final target (i.e. do not require target to exist),
@@ -349,7 +355,7 @@ func (m *LinkValueMigration) getPathCapabilityFinalTarget(
 				}
 
 			default:
-				panic(errors.NewUnreachableError())
+				panic(errors.NewUnexpectedError("unexpected value type: %T", value))
 			}
 		}
 	}

migrations/entitlements/migration_test.go

@@ -909,6 +909,11 @@ func TestConvertToEntitledValue(t *testing.T) {
 			),
 			Name: "[Type]",
 		},
+		{
+			Input:  interpreter.NewTypeValue(inter, nil),
+			Output: interpreter.NewTypeValue(inter, nil),
+			Name:   "Type(nil)",
+		},
 		{
 			Input: interpreter.NewDictionaryValue(
 				inter,
@@ -1041,6 +1046,19 @@ func TestConvertToEntitledValue(t *testing.T) {
 			},
 			Name: "PathCapability<&S>",
 		},
+		{
+			Input: &interpreter.PathCapabilityValue{ //nolint:staticcheck
+				Address:    interpreter.NewAddressValue(inter, testAddress),
+				Path:       testPathValue,
+				BorrowType: nil,
+			},
+			Output: &interpreter.PathCapabilityValue{ //nolint:staticcheck
+				Address:    interpreter.NewAddressValue(inter, testAddress),
+				Path:       testPathValue,
+				BorrowType: nil,
+			},
+			Name: "PathCapability<nil>",
+		},
 		{
 			Input: interpreter.NewCapabilityValue(
 				inter,
@@ -1276,6 +1294,21 @@ func TestConvertToEntitledValue(t *testing.T) {
 			}
 			return true
 
+		case interpreter.TypeValue:
+			// TypeValue considers missing type "unknown"/"invalid",
+			// and "unknown"/"invalid" type values unequal.
+			// However, we want to consider those equal here for testing/asserting purposes
+			other, ok := output.(interpreter.TypeValue)
+			if !ok {
+				return false
+			}
+
+			if other.Type == nil {
+				return v.Type == nil
+			} else {
+				return other.Type.Equal(v.Type)
+			}
+
 		case *interpreter.EphemeralReferenceValue:
 			otherReference, ok := output.(*interpreter.EphemeralReferenceValue)
 			if !ok || !v.Authorization.Equal(otherReference.Authorization) {
@@ -1304,7 +1337,7 @@ func TestConvertToEntitledValue(t *testing.T) {
 			convertedValue := convertEntireTestValue(inter, storage, testAddress, test.Input)
 			switch convertedValue := convertedValue.(type) {
 			case interpreter.EquatableValue:
-				require.True(t, referencePeekingEqual(convertedValue, test.Output))
+				require.True(t, referencePeekingEqual(convertedValue, test.Output), "expected: %s\nactual: %s", test.Output, convertedValue)
 			default:
 				require.Equal(t, convertedValue, test.Output)
 			}

migrations/legacy_primitivestatic_type.go

@@ -33,7 +33,9 @@ type LegacyPrimitiveStaticType struct {
 var _ interpreter.StaticType = LegacyPrimitiveStaticType{}
 
 func (t LegacyPrimitiveStaticType) ID() common.TypeID {
-	switch t.PrimitiveStaticType {
+	primitiveStaticType := t.PrimitiveStaticType
+
+	switch primitiveStaticType {
 	case interpreter.PrimitiveStaticTypeAuthAccount: //nolint:staticcheck
 		return "AuthAccount"
 	case interpreter.PrimitiveStaticTypePublicAccount: //nolint:staticcheck
@@ -59,7 +61,6 @@ func (t LegacyPrimitiveStaticType) ID() common.TypeID {
 	case interpreter.PrimitiveStaticTypeAccountKey: //nolint:staticcheck
 		return "AccountKey"
 	default:
-		panic(errors.NewUnreachableError())
-
+		panic(errors.NewUnexpectedError("unexpected non-legacy primitive static type: %s", primitiveStaticType))
 	}
 }

migrations/migration.go

@@ -306,7 +306,10 @@ func (m *StorageMigration) MigrateNestedValue(
 				)
 
 				if _, ok := oldValue.(*interpreter.SomeValue); !ok {
-					panic(errors.NewUnreachableError())
+					panic(errors.NewUnexpectedError(
+						"failed to remove old value for migrated key: %s",
+						existingKey,
+					))
 				}
 
 				keyToSet = newKey
@@ -372,8 +375,14 @@ func (m *StorageMigration) MigrateNestedValue(
 			// is the same as the owner of the old value
 			if ownedValue, ok := value.(interpreter.OwnedValue); ok {
 				if ownedConvertedValue, ok := convertedValue.(interpreter.OwnedValue); ok {
-					if ownedConvertedValue.GetOwner() != ownedValue.GetOwner() {
-						panic(errors.NewUnreachableError())
+					convertedOwner := ownedConvertedValue.GetOwner()
+					originalOwner := ownedValue.GetOwner()
+					if convertedOwner != originalOwner {
+						panic(errors.NewUnexpectedError(
+							"migrated value has different owner: expected %s, got %s",
+							originalOwner,
+							convertedOwner,
+						))
 					}
 				}
 			}

migrations/statictypes/account_type_migration_test.go

@@ -42,6 +42,7 @@ type testReporter struct {
 		interpreter.StorageKey
 		interpreter.StorageMapKey
 	}]struct{}
+	errors []error
 }
 
 func newTestReporter() *testReporter {
@@ -72,8 +73,9 @@ func (t *testReporter) Error(
 	_ interpreter.StorageKey,
 	_ interpreter.StorageMapKey,
 	_ string,
-	_ error,
+	err error,
 ) {
+	t.errors = append(t.errors, err)
 }
 
 func TestAccountTypeInTypeValueMigration(t *testing.T) {
@@ -424,6 +426,8 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) {
 	err = migration.Commit()
 	require.NoError(t, err)
 
+	require.Empty(t, reporter.errors)
+
 	// Check reported migrated paths
 	for identifier, test := range testCases {
 		key := struct {
@@ -1080,6 +1084,8 @@ func TestAccountTypeRehash(t *testing.T) {
 		err := migration.Commit()
 		require.NoError(t, err)
 
+		require.Empty(t, reporter.errors)
+
 		require.Equal(t,
 			map[struct {
 				interpreter.StorageKey

migrations/statictypes/composite_type_migration_test.go

@@ -172,6 +172,8 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) {
 	err = migration.Commit()
 	require.NoError(t, err)
 
+	require.Empty(t, reporter.errors)
+
 	// Check reported migrated paths
 	for identifier, test := range testCases {
 		key := struct {

migrations/statictypes/intersection_type_migration_test.go

@@ -443,6 +443,8 @@ func TestIntersectionTypeMigration(t *testing.T) {
 	err = migration.Commit()
 	require.NoError(t, err)
 
+	require.Empty(t, reporter.errors)
+
 	// Check reported migrated paths
 	for identifier, test := range testCases {
 		key := struct {
@@ -611,6 +613,8 @@ func TestIntersectionTypeRehash(t *testing.T) {
 		err := migration.Commit()
 		require.NoError(t, err)
 
+		require.Empty(t, reporter.errors)
+
 		require.Equal(t,
 			map[struct {
 				interpreter.StorageKey
@@ -775,6 +779,8 @@ func TestRehashNestedIntersectionType(t *testing.T) {
 			err := migration.Commit()
 			require.NoError(t, err)
 
+			require.Empty(t, reporter.errors)
+
 			require.Equal(t,
 				map[struct {
 					interpreter.StorageKey
@@ -914,6 +920,8 @@ func TestRehashNestedIntersectionType(t *testing.T) {
 			err := migration.Commit()
 			require.NoError(t, err)
 
+			require.Empty(t, reporter.errors)
+
 			require.Equal(t,
 				map[struct {
 					interpreter.StorageKey
@@ -1114,6 +1122,8 @@ func TestIntersectionTypeMigrationWithInterfaceTypeConverter(t *testing.T) {
 			err = migration.Commit()
 			require.NoError(t, err)
 
+			require.Empty(t, reporter.errors)
+
 			expectLegacyTypeConverted := convertCompositeType && legacyType != nil
 			expectInterfaceTypeConverted := convertInterfaceType && len(interfaceTypeQualifiedIdentifiers) > 0
 			expectMigration := len(interfaceTypeQualifiedIdentifiers) >= 2 ||
@@ -1282,6 +1292,8 @@ func TestIntersectionTypeMigrationWithTypeConverters(t *testing.T) {
 		err = migration.Commit()
 		require.NoError(t, err)
 
+		require.Empty(t, reporter.errors)
+
 		key := struct {
 			interpreter.StorageKey
 			interpreter.StorageMapKey

migrations/statictypes/statictype_migration.go

@@ -66,7 +66,12 @@ func (m *StaticTypeMigration) Migrate(
 ) (newValue interpreter.Value, err error) {
 	switch value := value.(type) {
 	case interpreter.TypeValue:
-		convertedType := m.maybeConvertStaticType(value.Type, nil)
+		// Type is optional. nil represents "unknown"/"invalid" type
+		ty := value.Type
+		if ty == nil {
+			return
+		}
+		convertedType := m.maybeConvertStaticType(ty, nil)
 		if convertedType == nil {
 			return
 		}
@@ -80,7 +85,12 @@ func (m *StaticTypeMigration) Migrate(
 		return interpreter.NewUnmeteredCapabilityValue(value.ID, value.Address, convertedBorrowType), nil
 
 	case *interpreter.PathCapabilityValue: //nolint:staticcheck
-		convertedBorrowType := m.maybeConvertStaticType(value.BorrowType, nil)
+		// Type is optional
+		borrowType := value.BorrowType
+		if borrowType == nil {
+			return
+		}
+		convertedBorrowType := m.maybeConvertStaticType(borrowType, nil)
 		if convertedBorrowType == nil {
 			return
 		}
@@ -345,7 +355,7 @@ func (m *StaticTypeMigration) maybeConvertStaticType(staticType, parentType inte
 		}
 
 	default:
-		panic(errors.NewUnreachableError())
+		panic(errors.NewUnexpectedError("unexpected static type: %T", staticType))
 	}
 
 	return nil