diff --git a/migrations/migration.go b/migrations/migration.go index e06e468706..070e0a747a 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -139,6 +139,31 @@ func (m *StorageMigration) MigrateNestedValue( valueMigrations []ValueMigration, reporter Reporter, ) (newValue interpreter.Value) { + + defer func() { + // Here it catches the panics that may occur at the framework level, + // even before going to each individual migration. e.g: iterating the dictionary for elements. + // + // There is a similar recovery at the `StorageMigration.migrate()` method, + // which handles panics from each individual migrations (e.g: capcon migration, static type migration, etc.). + + if r := recover(); r != nil { + switch r := r.(type) { + case error: + if reporter != nil { + reporter.Error( + storageKey, + storageMapKey, + "StorageMigration", + r, + ) + } + default: + panic(r) + } + } + }() + switch value := value.(type) { case *interpreter.SomeValue: innerValue := value.InnerValue(m.interpreter, emptyLocationRange) @@ -378,6 +403,11 @@ func (m *StorageMigration) migrate( value interpreter.Value, ) (converted interpreter.Value, err error) { + // Handles panics from each individual migrations (e.g: capcon migration, static type migration, etc.). + // So even if one migration panics, others could still run (i.e: panics are caught inside the loop). + // Removing that would cause all migrations to stop for a particular value, if one of them panics. + // NOTE: this won't catch panics occur at the migration framework level. + // They are caught at `StorageMigration.MigrateNestedValue()`. defer func() { if r := recover(); r != nil { switch r := r.(type) { diff --git a/migrations/migration_test.go b/migrations/migration_test.go index 702cd07e3e..db987f8733 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -33,6 +33,7 @@ import ( "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/stdlib" + "github.com/onflow/cadence/runtime/tests/checker" . "github.com/onflow/cadence/runtime/tests/runtime_utils" "github.com/onflow/cadence/runtime/tests/utils" ) @@ -947,3 +948,148 @@ func TestContractMigration(t *testing.T) { value, ) } + +func TestMigrationPanics(t *testing.T) { + + t.Parallel() + + t.Run("composite dictionary", func(t *testing.T) { + t.Parallel() + + ledger := NewTestLedger(nil, nil) + account := common.Address{0x42} + + t.Run("prepare", func(t *testing.T) { + + fooContractLocation := common.NewAddressLocation(nil, account, "Foo") + + storage := runtime.NewStorage(ledger, nil) + locationRange := interpreter.EmptyLocationRange + + contractChecker, err := checker.ParseAndCheckWithOptions(t, ` + access(all) contract Foo { + access(all) struct Bar {} + }`, + checker.ParseAndCheckOptions{ + Location: fooContractLocation, + }, + ) + require.NoError(t, err) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: false, + AtreeStorageValidationEnabled: false, + ImportLocationHandler: func(inter *interpreter.Interpreter, location common.Location) interpreter.Import { + program := interpreter.ProgramFromChecker(contractChecker) + subInterpreter, err := inter.NewSubInterpreter(program, location) + if err != nil { + panic(err) + } + + return interpreter.InterpreterImport{ + Interpreter: subInterpreter, + } + }, + }, + ) + require.NoError(t, err) + + const fooBarQualifiedIdentifier = "Foo.Bar" + + dictionaryAnyStructStaticType := + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.NewCompositeStaticTypeComputeTypeID( + nil, + fooContractLocation, + fooBarQualifiedIdentifier, + ), + ) + + storedValue := interpreter.NewDictionaryValue( + inter, + emptyLocationRange, + dictionaryAnyStructStaticType, + ) + + transferredValue := storedValue.Transfer( + inter, + locationRange, + atree.Address(account), + false, + nil, + nil, + ) + + inter.WriteStored( + account, + common.PathDomainStorage.Identifier(), + interpreter.StringStorageMapKey("dictionary_value"), + transferredValue, + ) + + err = storage.Commit(inter, true) + require.NoError(t, err) + }) + + t.Run("migrate", func(t *testing.T) { + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: false, + AtreeStorageValidationEnabled: false, + ImportLocationHandler: func(_ *interpreter.Interpreter, _ common.Location) interpreter.Import { + // During the migration, treat the imported `Foo` contract as un-migrated. + panic(errors.New("type checking error in Foo")) + }, + }, + ) + require.NoError(t, err) + + migration := NewStorageMigration(inter, storage) + + reporter := newTestReporter() + + migration.Migrate( + &AddressSliceIterator{ + Addresses: []common.Address{ + account, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + testStringMigration{}, + testInt8Migration{}, + testCapMigration{}, + ), + ) + + err = migration.Commit() + require.NoError(t, err) + + expectedErrors := map[struct { + interpreter.StorageKey + interpreter.StorageMapKey + }][]string{ + { + StorageKey: interpreter.StorageKey{ + Key: common.PathDomainStorage.Identifier(), + Address: account, + }, + StorageMapKey: interpreter.StringStorageMapKey("dictionary_value"), + }: {"StorageMigration"}, + } + + assert.Equal(t, expectedErrors, reporter.errored) + }) + }) +}