Running the migrations more than ones on the same state fails

[SupunS]

Current Behavior

Don't know if this is a real problem, but created the issue to not let it fall through cracks. Need to investigate/discuss if this is the expected behavior or not.

Running the cadence migrations twice on the same state (i.e: running migration on an already migrated state) fails with a unreachable error at key removal (doesn't crash, but the value get skipped from migrating).

Error trace:

9:13AM ERR failed to run StorageMigration in account 01cf0e2f2f715450, domain storage, key dictionary_with_restricted_typed_keys: internal error: unreachable
goroutine 58 [running]:
runtime/debug.Stack()
	/opt/homebrew/Cellar/go/1.20.6/libexec/src/runtime/debug/stack.go:24 +0x64
github.com/onflow/cadence/runtime/errors.NewUnexpectedError({0x1018ec6c0?, 0x1400085aba0?}, {0x0?, 0x0?, 0x0?})
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/runtime/errors/errors.go:156 +0x3c
github.com/onflow/cadence/runtime/errors.NewUnreachableError(...)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/runtime/errors/errors.go:30
github.com/onflow/cadence/migrations.(*StorageMigration).MigrateNestedValue(0x14000b32060, {{0x1018e75b1, 0x7}, {0x1, 0xcf, 0xe, 0x2f, 0x2f, 0x71, 0x54, ...}}, ...)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/migration.go:303 +0x8c8
github.com/onflow/cadence/migrations.(*StorageMigration).NewValueMigrationsPathMigrator.func1({{0x1018e75b1, 0x7}, {0x1, 0xcf, 0xe, 0x2f, 0x2f, 0x71, 0x54, 0x50}}, ...)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/migration.go:122 +0x68
github.com/onflow/cadence/migrations.NewValueConverterPathMigrator.func1(0x10207d120?, {{0x1018e75b1, 0x7}, {0x1, 0xcf, 0xe, 0x2f, 0x2f, 0x71, 0x54, ...}}, ...)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/account_storage.go:64 +0x78
github.com/onflow/cadence/migrations.(*AccountStorage).MigrateStorageMap(0x14000791a08?, 0x1009dba30?, {0x1018e75b1, 0x7}, 0x14000b32150, 0x10205aa40)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/account_storage.go:134 +0x244
github.com/onflow/cadence/migrations.(*AccountStorage).MigrateStringKeys(0x14000791a48?, 0x1007cd66c?, {0x1018e75b1?, 0x10?}, 0x10?)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/account_storage.go:82 +0x28
github.com/onflow/cadence/migrations.(*StorageMigration).MigrateAccount(0x14000b32060, {0x1, 0xcf, 0xe, 0x2f, 0x2f, 0x71, 0x54, 0x50}, 0x140008b6e40?)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/migration.go:86 +0x80
github.com/onflow/cadence/migrations.(*StorageMigration).Migrate(0x14000b32060?, {0x10206e068, 0x140003e2b20}, 0x140008b6e00?)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/migrations/migration.go:71 +0x40
github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceBaseMigrator).MigrateAccount(0x140009965a0, {0x1020701b0?, 0x140001cbde0?}, {0x1, 0xcf, 0xe, 0x2f, 0x2f, 0x71, 0x54, ...}, ...)
	/opt/homebrew/opt/[email protected]/bin/pkg/mod/github.com/onflow/[email protected]/cmd/util/ledger/migrations/cadence_values_migration.go:81 +0x294
github.com/onflow/flow-emulator/storage/migration.migrateCadenceValues({0x102068b20?, 0x102d9d8c0?}, {{0x1020701b0, 0x140001cbde0}, 0x1, {0x0, 0x0}, {0x1400093fc00, 0x1, 0x1f4}, ...}, ...)
	/Users/supunsetunga/work/flow-emulator/storage/migration/cadence_values_migration.go:113 +0x158
github.com/onflow/flow-emulator/storage/migration.MigrateCadenceValues(0x1400048bd80, {0x102068b20, 0x102d9d8c0}, {{0x1020701b0, 0x140001cbde0}, 0x1, {0x0, 0x0}, {0x1400093fc00, 0x1, ...}, ...})
	/Users/supunsetunga/work/flow-emulator/storage/migration/cadence_values_migration.go:54 +0x170
github.com/onflow/flow-emulator/storage/migration.TestStateMigration(0x0?)
	/Users/supunsetunga/work/flow-emulator/storage/migration/migration_test.go:69 +0x2f0
testing.tRunner(0x140000c7a00, 0x10205bc18)
	/opt/homebrew/Cellar/go/1.20.6/libexec/src/testing/testing.go:1576 +0x10c
created by testing.(*T).Run
	/opt/homebrew/Cellar/go/1.20.6/libexec/src/testing/testing.go:1629 +0x368
 migration=cadence-value-migration
9:13AM INF Committing changes migration=cadence-value-migration
--- PASS: TestStateMigration (0.19s)

Expected Behavior

TBD

Steps To Reproduce

• Go to the TestStateMigration test in https://github.com/onflow/flow-emulator/pull/557
• Update the the cadence value migrations execution as bellow (i.e: Run the cadence value migrations twice)

  // Then migrate the values.
  rwf := &NOOPReportWriterFactory{}
  err = MigrateCadenceValues(store, rwf, logger)
  require.NoError(t, err)
  
  err = MigrateCadenceValues(store, rwf, logger)
  require.NoError(t, err)
  

• For more visually clear error messages, replace the logger variable in the test-case with logger := NewConsoleLogger()
• Run the test. The last reported error will be the once mentioned in the description (other errors are expected)

Environment

- Cadence version: 1.0
- Network: emulator