Migration problems

[turbolent]

See https://discord.com/channels/613813861610684416/1108479699732152503/1241110592153063619

We recently created two migration results:

• migrationtestnet1-may8-inlined-preview20: migrated with an older, unoptimized version of Cadence/flow-go
• migrationtestnet1-may8-inlined-preview23: migrated with the latest, optimized version of Cadence/flow-go

These results were both generated from the same TN snapshot (right, @j1010001?), but produced different state commitments.

As a result, I developed a flow-go util command to compare two states.

The diffing uncovered several reasons for the different state commitments, but also uncovered several problems:

• Some capabilities using LockedTokens.TokenAdmin have different authorizations:

  {
    "Address": "50f4b24a66fc3efb",
    "Domain": "storage",
    "Kind": "cadence_value_diff",
    "Msg": "values differ: *interpreter.PathCapabilityValue vs *interpreter.PathCapabilityValue",
    "Trace": "storage[flowTokenHolder].tokenManager",
    "OldValue": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw) &A.95e019a17d0e23d7.LockedTokens.LockedTokenManager>(address: 0x0a4b7be95de5c4f7, path: /storage/lockedTokenManager)",
    "NewValue": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw, A.95e019a17d0e23d7.LockedTokens.UnlockTokens) &A.95e019a17d0e23d7.LockedTokens.LockedTokenManager>(address: 0x0a4b7be95de5c4f7, path: /storage/lockedTokenManager)",
    "OldValueStaticType": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw) &A.95e019a17d0e23d7.LockedTokens.LockedTokenManager>",
    "NewValueStaticType": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw, A.95e019a17d0e23d7.LockedTokens.UnlockTokens) &A.95e019a17d0e23d7.LockedTokens.LockedTokenManager>"
  }

  This is because a new entitlement was added to the LockedTokenManager contract, see https://github.com/onflow/flow-core-contracts/pull/429/files#diff-a200545fac9017d8d015d11d56aa104f41e0a09088a2fb5f8eabb7006001528dR85

  Nothing wrong here, just explains the reason the results are different.

• Diffing calls interpreter.Value.String() which indirectly might call interpreter.Value.MeteredString, which is not implemented for legacy values, like PathCapabilityValue

  We just need to implement the function. Fixed by Fix string formatting for values #3370

• StorageCapabilityControllerValue.RecursiveString accidentally has the arguments swapped

  We just need to fix the argument order here. Fixed by Fix string formatting for values #3370

• Migrated capability has different borrow type, e.g. FT and NFT:

  {
    "Address": "a8014057de303e14",
    "Domain": "storage",
    "Kind": "cadence_value_diff",
    "Msg": "values differ: *interpreter.IDCapabilityValue vs *interpreter.IDCapabilityValue",
    "Trace": "storage[AssetHandover].providerCap",
    "OldValue": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw) &A.e223d8a629e49c68.FUSD.Vault>(address: 0xa8014057de303e14, id: 3)",
    "NewValue": "Capability<&{A.9a0766d93b6608b7.FungibleToken.Provider}>(address: 0xa8014057de303e14, id: 3)",
    "OldValueStaticType": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw) &A.e223d8a629e49c68.FUSD.Vault>",
    "NewValueStaticType": "Capability<&{A.9a0766d93b6608b7.FungibleToken.Provider}>"
  },
  {
    "Address": "a3328383c8a91659",
    "Domain": "storage",
    "Kind": "cadence_value_diff",
    "Msg": "values differ: *interpreter.IDCapabilityValue vs *interpreter.IDCapabilityValue",
    "Trace": "storage[BYC_Swap].barters[109135547].nftAssetsOffered[0].providerCap",
    "OldValue": "Capability<auth(A.631e88ae7f1d7c20.NonFungibleToken.Withdraw) &{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic}>(address: 0xa3328383c8a91659, id: 5)",
    "NewValue": "Capability<&{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic}>(address: 0xa3328383c8a91659, id: 5)",
    "OldValueStaticType": "Capability<auth(A.631e88ae7f1d7c20.NonFungibleToken.Withdraw) &{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic}>",
    "NewValueStaticType": "Capability<&{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic}>"
  }

  Caused by incorrect caching in entitlements migration. Fixed by Remove incorrect caching from migrations #3375

• Some links are no longer migrated?

  {
    "Address": "9b20b2141c54f108",
    "Domain": "private",
    "Kind": "cadence_value_diff",
    "Msg": "values differ: *interpreter.IDCapabilityValue vs interpreter.PathLinkValue",
    "Trace": "private[TicketTokenProvider]",
    "OldValue": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw) &A.e223d8a629e49c68.FUSD.Vault>(address: 0x9b20b2141c54f108, id: 1)",
    "NewValue": "PathLink<&{A.9a0766d93b6608b7.FungibleToken.Provider}>(/storage/TicketTokenVault)",
    "OldValueStaticType": "Capability<auth(A.9a0766d93b6608b7.FungibleToken.Withdraw) &A.e223d8a629e49c68.FUSD.Vault>",
    "NewValueStaticType": "Capability<&{A.9a0766d93b6608b7.FungibleToken.Provider}>"
  },
  {
    "Address": "691ae94d127969db",
    "Domain": "private",
    "Kind": "cadence_value_diff",
    "Msg": "values differ: *interpreter.IDCapabilityValue vs interpreter.PathLinkValue",
    "Trace": "private[doodleNames]",
    "OldValue": "Capability<auth(A.631e88ae7f1d7c20.NonFungibleToken.Withdraw) &{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic, A.631e88ae7f1d7c20.NonFungibleToken.Receiver, A.631e88ae7f1d7c20.ViewResolver.ResolverCollection}>(address: 0x691ae94d127969db, id: 3)",
    "NewValue": "PathLink<&{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic, A.631e88ae7f1d7c20.NonFungibleToken.Receiver, A.631e88ae7f1d7c20.ViewResolver.ResolverCollection}>(/storage/doodleNames)",
    "OldValueStaticType": "Capability<auth(A.631e88ae7f1d7c20.NonFungibleToken.Withdraw) &{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic, A.631e88ae7f1d7c20.NonFungibleToken.Receiver, A.631e88ae7f1d7c20.ViewResolver.ResolverCollection}>",
    "NewValueStaticType": "Capability<&{A.631e88ae7f1d7c20.NonFungibleToken.Provider, A.631e88ae7f1d7c20.NonFungibleToken.CollectionPublic, A.631e88ae7f1d7c20.NonFungibleToken.Receiver, A.631e88ae7f1d7c20.ViewResolver.ResolverCollection}>"
  }

  Likely caused by incorrect caching in entitlements migration (see above). Should be fixed by Remove incorrect caching from migrations #3375

• Capability controller / ID creation might have different order, migrated capability values might get different IDs

  Maybe look into making this deterministic, if possible.

  As some links fail to be migrated (see above), the resulting IDs are offset. Should be fixed by Remove incorrect caching from migrations #3375

• Iteration fails to load key:

  panic: key (Type<&{A.13f9d5b87b69bbfc.NonFungibleToken.Provider}>()) not found
  
  goroutine 1 [running]:
  github.com/onflow/cadence/runtime/interpreter.(*DictionaryValue).iterateKeys.func1()
  	/Users/bastian/Documents/work/cadence/runtime/interpreter/value.go:18977 +0xbd
  github.com/onflow/cadence/runtime/interpreter.(*Interpreter).withMutationPrevention(0xc141b58240, {0xe1, 0x5c, 0x9d, 0x5f, 0x4b, 0xe1, 0x4d, 0x8, 0x0, ...}, ...)
  	/Users/bastian/Documents/work/cadence/runtime/interpreter/interpreter.go:5639 +0x71
  github.com/onflow/cadence/runtime/interpreter.(*DictionaryValue).iterateKeys(0x10?, 0xc141b58240, 0x1?, 0x7f53fe972c70?)
  	/Users/bastian/Documents/work/cadence/runtime/interpreter/value.go:18981 +0x66
  github.com/onflow/cadence/runtime/interpreter.(*DictionaryValue).IterateKeys(0xc141b903c0, 0xc141b58240, {{0x0?, 0x0?}, {0x0?, 0x0?}}, 0xc141b7dd70?)
  	/Users/bastian/Documents/work/cadence/runtime/interpreter/value.go:18957 +0x1ae
  github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceValueDiffReporter).diffCadenceDictionaryValue(0xd7484079e0, 0x30?, 0xc141b903c0, 0xd7484069c8?, {0xb29008?, 0xc141b903f0?}, {0x6bd1b4, 0x7}, 0xfe323a12c0)
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/migrations/cadence_value_diff.go:680 +0x408
  github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceValueDiffReporter).diffValues(0xd7484079e0, 0xd748406cc0?, {0xb29008?, 0xc141b903c0?}, 0x2?, {0xb29008, 0xc141b903f0}, {0x6bd1b4, 0x7}, 0xfe323a12c0)
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/migrations/cadence_value_diff.go:336 +0x105
  github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceValueDiffReporter).diffCadenceCompositeValue(0xd7484079e0, 0xd748407010?, 0xc141b670e0, 0x30?, {0xb28dc8?, 0xc141b67170?}, {0x6bd1b4, 0x7}, 0xfe323a11d0)
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/migrations/cadence_value_diff.go:626 +0x1085
  github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceValueDiffReporter).diffValues(0xd7484079e0, 0x0?, {0xb28dc8?, 0xc141b670e0?}, 0x3?, {0xb28dc8, 0xc141b67170}, {0x6bd1b4, 0x7}, 0xfe323a11d0)
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/migrations/cadence_value_diff.go:333 +0xcd
  github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceValueDiffReporter).diffStorageDomain(0xd7484079e0, 0xc141b5eae0, 0xc141b5fb90, {0x6bd1b4, 0x7})
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/migrations/cadence_value_diff.go:291 +0xe56
  github.com/onflow/flow-go/cmd/util/ledger/migrations.(*CadenceValueDiffReporter).DiffStates(0xd7484079e0, {0xb12e48, 0xfd3c835e90}, {0xb12e48, 0xfd3b96e630}, {0x2fa82e0, 0x6, 0xc0007db920?})
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/migrations/cadence_value_diff.go:167 +0x8e5
  github.com/onflow/flow-go/cmd/util/cmd/diff-states.diff.func1(0xfd3c835e90)
  	/Users/bastian/Documents/work/flow-go/cmd/util/cmd/diff-states/cmd.go:345 +0x471
  github.com/onflow/flow-go/cmd/util/ledger/util/registers.(*ByAccount).ForEachAccount(...)
  	/Users/bastian/Documents/work/flow-go/cmd/util/ledger/util/registers/registers.go:140
  github.com/onflow/flow-go/cmd/util/cmd/diff-states.diff(0xc000710068, 0xc08acd2180, {0x7fffad10c6b4, 0xc}, {0xb077c8?, 0xc0007b4960})
  	/Users/bastian/Documents/work/flow-go/cmd/util/cmd/diff-states/cmd.go:283 +0x159
  github.com/onflow/flow-go/cmd/util/cmd/diff-states.run(0xc000291800, {0x6b9901, 0x4, 0x6b9905})
  	/Users/bastian/Documents/work/flow-go/cmd/util/cmd/diff-states/cmd.go:186 +0x43b
  github.com/spf13/cobra.(*Command).execute(0x2fb8e80, {0xc0007b3450, 0xd, 0xd})
  	/Users/bastian/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:987 +0xaa3
  github.com/spf13/cobra.(*Command).ExecuteC(0x2fb7d40)
  	/Users/bastian/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
  github.com/spf13/cobra.(*Command).Execute(...)
  	/Users/bastian/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
  github.com/onflow/flow-go/cmd/util/cmd.Execute()
  	/Users/bastian/Documents/work/flow-go/cmd/util/cmd/root.go:62 +0x1a
  main.main()
  	/Users/bastian/Documents/work/flow-go/cmd/util/main.go:8 +0xf
  

  This is a bug we need to fix. It is unlikely a result of the optimization of the migration code, but rather uncovered by walking over the whole migrated state.

  We should maybe extend the tests in the Cadence migration to walk the migrated state.

  We previously had a problem in the migration when using a mutating iterator on old values on the atree inlining branch.
  The error here seems similar – but why would we still have an old unmigrated key?

[j1010001]

Hey @turbolent yes, the two migrations were run on the same TN state taken May 8 (devnet49-execution-snapshot-for-migration-6-may-8 snapshot - note needs DL GCP account to access).

[bluesign]

• The error here seems similar – but why would we still have an old unmigrated key?

I think key here is not migrated, because contract is not staged.

Some links are no longer migrated?
Migrated capability has different borrow type, e.g. FT and NFT:

Those are strangely wrong migrated in the old version somehow, maybe some caching bug there.

[turbolent]

@bluesign

• The error here seems similar – but why would we still have an old unmigrated key?

I think key here is not migrated, because contract is not staged.

You mean 0x13f9d5b87b69bbfc.NonFungibleToken right? I was at first confused, but this is a self-deployed version of NonFungibleToken, the official NFT contract is on 0x631e88ae7f1d7c20 right?

[j1010001]

validate that we can traverse the migrated TN state from June 5 migration. bucket: gs://flow_hosting_exec_state_storage/crescendo-migration-testnet-jun5-preview26/

[turbolent]

@j1010001 That state is inlined, right?

[j1010001]

@j1010001 That state is inlined, right?

@turbolent yes, I re-run C1.0 migration with preview 26 build, using the existing payloads from the atree inlining done May 29.

[j1010001]

@turbolent I opened #3407 to track the last issue - I think this can be closed.