Merge branch 'master' into feature/range-type

migrations/account_type/migration.go

@@ -44,29 +44,29 @@ func (AccountTypeMigration) Migrate(
 	_ interpreter.AddressPath,
 	value interpreter.Value,
 	_ *interpreter.Interpreter,
-) (newValue interpreter.Value) {
+) (newValue interpreter.Value, err error) {
 	switch value := value.(type) {
 	case interpreter.TypeValue:
 		convertedType := maybeConvertAccountType(value.Type)
 		if convertedType == nil {
 			return
 		}
-		return interpreter.NewTypeValue(nil, convertedType)
+		return interpreter.NewTypeValue(nil, convertedType), nil
 
 	case *interpreter.CapabilityValue:
 		convertedBorrowType := maybeConvertAccountType(value.BorrowType)
 		if convertedBorrowType == nil {
 			return
 		}
-		return interpreter.NewUnmeteredCapabilityValue(value.ID, value.Address, convertedBorrowType)
+		return interpreter.NewUnmeteredCapabilityValue(value.ID, value.Address, convertedBorrowType), nil
 
 	case *interpreter.AccountCapabilityControllerValue:
 		convertedBorrowType := maybeConvertAccountType(value.BorrowType)
 		if convertedBorrowType == nil {
 			return
 		}
 		borrowType := convertedBorrowType.(*interpreter.ReferenceStaticType)
-		return interpreter.NewUnmeteredAccountCapabilityControllerValue(borrowType, value.CapabilityID)
+		return interpreter.NewUnmeteredAccountCapabilityControllerValue(borrowType, value.CapabilityID), nil
 
 	case *interpreter.StorageCapabilityControllerValue:
 		// Note: A storage capability with Account type shouldn't be possible theoretically.
@@ -79,7 +79,7 @@ func (AccountTypeMigration) Migrate(
 			borrowType,
 			value.CapabilityID,
 			value.TargetPath,
-		)
+		), nil
 	}
 
 	return

migrations/account_type/migration_test.go

@@ -46,13 +46,20 @@ func newTestReporter() *testReporter {
 	}
 }
 
-func (t *testReporter) Report(
+func (t *testReporter) Migrated(
 	addressPath interpreter.AddressPath,
 	_ string,
 ) {
 	t.migratedPaths[addressPath] = struct{}{}
 }
 
+func (t *testReporter) Error(
+	_ interpreter.AddressPath,
+	_ string,
+	_ error,
+) {
+}
+
 func TestTypeValueMigration(t *testing.T) {
 	t.Parallel()
 
@@ -366,7 +373,8 @@ func TestTypeValueMigration(t *testing.T) {
 		),
 	)
 
-	migration.Commit()
+	err = migration.Commit()
+	require.NoError(t, err)
 
 	// Check reported migrated paths
 	for identifier, test := range testCases {
@@ -689,7 +697,8 @@ func TestNestedTypeValueMigration(t *testing.T) {
 		),
 	)
 
-	migration.Commit()
+	err = migration.Commit()
+	require.NoError(t, err)
 
 	// Assert: Traverse through the storage and see if the values are updated now.
 
@@ -834,7 +843,8 @@ func TestValuesWithStaticTypeMigration(t *testing.T) {
 		),
 	)
 
-	migration.Commit()
+	err = migration.Commit()
+	require.NoError(t, err)
 
 	// Assert: Traverse through the storage and see if the values are updated now.
 

migrations/capcons/capabilitymigration.go

@@ -63,7 +63,7 @@ func (m *CapabilityValueMigration) Migrate(
 	addressPath interpreter.AddressPath,
 	value interpreter.Value,
 	_ *interpreter.Interpreter,
-) interpreter.Value {
+) (interpreter.Value, error) {
 	reporter := m.Reporter
 
 	switch value := value.(type) {
@@ -109,8 +109,8 @@ func (m *CapabilityValueMigration) Migrate(
 			)
 		}
 
-		return newCapability
+		return newCapability, nil
 	}
 
-	return nil
+	return nil, nil
 }

migrations/capcons/linkmigration.go

@@ -55,13 +55,13 @@ func (m *LinkValueMigration) Migrate(
 	addressPath interpreter.AddressPath,
 	value interpreter.Value,
 	inter *interpreter.Interpreter,
-) interpreter.Value {
+) (interpreter.Value, error) {
 
 	pathDomain := addressPath.Path.Domain
 	if pathDomain != common.PathDomainPublic &&
 		pathDomain != common.PathDomainPrivate {
 
-		return nil
+		return nil, nil
 	}
 
 	accountAddress := addressPath.Address
@@ -77,7 +77,7 @@ func (m *LinkValueMigration) Migrate(
 	switch readValue := value.(type) {
 	case *interpreter.CapabilityValue:
 		// Already migrated
-		return nil
+		return nil, nil
 
 	case interpreter.PathLinkValue: //nolint:staticcheck
 		var ok bool
@@ -122,10 +122,10 @@ func (m *LinkValueMigration) Migrate(
 			}
 
 			// TODO: really leave as-is? or still convert?
-			return nil
+			return nil, nil
 		}
 
-		panic(err)
+		return nil, err
 	}
 
 	// Issue appropriate capability controller
@@ -139,7 +139,7 @@ func (m *LinkValueMigration) Migrate(
 		}
 
 		// TODO: really leave as-is? or still convert?
-		return nil
+		return nil, nil
 
 	case pathCapabilityTarget:
 
@@ -184,7 +184,7 @@ func (m *LinkValueMigration) Migrate(
 		capabilityID,
 		addressValue,
 		borrowStaticType,
-	)
+	), nil
 }
 
 var authAccountReferenceStaticType = interpreter.NewReferenceStaticType(

migrations/capcons/migration_test.go

@@ -462,7 +462,8 @@ func testPathCapabilityValueMigration(
 		),
 	)
 
-	migration.Commit()
+	err = migration.Commit()
+	require.NoError(t, err)
 
 	// Check migrated capabilities
 
@@ -1179,7 +1180,8 @@ func testLinkMigration(
 		),
 	)
 
-	migration.Commit()
+	err = migration.Commit()
+	require.NoError(t, err)
 
 	// Assert
 
@@ -1683,7 +1685,8 @@ func TestClearPrivateDomain(t *testing.T) {
 		ClearPrivateDomain,
 	)
 
-	migration.Commit()
+	err = migration.Commit()
+	require.NoError(t, err)
 
 	// Assert
 

migrations/migration.go

@@ -31,7 +31,7 @@ type ValueMigration interface {
 		addressPath interpreter.AddressPath,
 		value interpreter.Value,
 		interpreter *interpreter.Interpreter,
-	) (newValue interpreter.Value)
+	) (newValue interpreter.Value, err error)
 }
 
 type DomainMigration interface {
@@ -70,11 +70,8 @@ func (m *StorageMigration) Migrate(
 	}
 }
 
-func (m *StorageMigration) Commit() {
-	err := m.storage.Commit(m.interpreter, false)
-	if err != nil {
-		panic(err)
-	}
+func (m *StorageMigration) Commit() error {
+	return m.storage.Commit(m.interpreter, false)
 }
 
 func (m *StorageMigration) MigrateAccount(
@@ -282,7 +279,14 @@ func (m *StorageMigration) migrateNestedValue(
 	default:
 		// Assumption: all migrations only migrate non-container typed values.
 		for _, migration := range valueMigrations {
-			converted := migration.Migrate(addressPath, value, m.interpreter)
+			converted, err := m.migrate(migration, addressPath, value)
+
+			if err != nil {
+				if reporter != nil {
+					reporter.Error(addressPath, migration.Name(), err)
+				}
+				continue
+			}
 
 			if converted != nil {
 				// Chain the migrations.
@@ -294,7 +298,7 @@ func (m *StorageMigration) migrateNestedValue(
 				newValue = converted
 
 				if reporter != nil {
-					reporter.Report(addressPath, migration.Name())
+					reporter.Migrated(addressPath, migration.Name())
 				}
 			}
 		}
@@ -303,6 +307,26 @@ func (m *StorageMigration) migrateNestedValue(
 	}
 }
 
+func (m *StorageMigration) migrate(
+	migration ValueMigration,
+	addressPath interpreter.AddressPath,
+	value interpreter.Value,
+) (converted interpreter.Value, err error) {
+
+	defer func() {
+		if r := recover(); r != nil {
+			switch r := r.(type) {
+			case error:
+				err = r
+			default:
+				panic(r)
+			}
+		}
+	}()
+
+	return migration.Migrate(addressPath, value, m.interpreter)
+}
+
 // legacyKey return the same type with the "old" hash/ID generation algo.
 func legacyKey(key interpreter.Value) interpreter.Value {
 	typeValue, isTypeValue := key.(interpreter.TypeValue)

migrations/migration_reporter.go

@@ -21,5 +21,6 @@ package migrations
 import "github.com/onflow/cadence/runtime/interpreter"
 
 type Reporter interface {
-	Report(addressPath interpreter.AddressPath, migration string)
+	Migrated(addressPath interpreter.AddressPath, migration string)
+	Error(addressPath interpreter.AddressPath, migration string, err error)
 }