Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cadence 1.0 state migration fixes and improvements #3162

Closed
1 of 4 tasks
turbolent opened this issue Mar 10, 2024 · 2 comments
Closed
1 of 4 tasks

Cadence 1.0 state migration fixes and improvements #3162

turbolent opened this issue Mar 10, 2024 · 2 comments
Assignees

Comments

@turbolent
Copy link
Member

turbolent commented Mar 10, 2024

  • Problem: During the entitlements migration, types from core contracts, e.g. FungibleToken.Vault and MetadataViews.ResolverCollection, cannot be loaded

    ERR failed to run EntitlementsMigration in account f23ef98357f4d5ea, domain public, key cle2lhbpt0000l70hh3mgsbd1_FlowSports_nft_collection: failed to load type: A.631e88ae7f1d7c20.MetadataViews.ResolverCollection
    
    ERR failed to run EntitlementsMigration in account fff1826394fa441f, domain public, key moxyTokenPlayAndEarnBalance: failed to load type: A.9a0766d93b6608b7.FungibleToken.Vault
    

    Solution: Fix intersection type migration to handle restricted type (legacy type in intersection type) getting converted to intersection type itself. See problem below. The type loading fails, because the unconverted nested type requirement keeps being loaded as a composite type, but the type is now an interface.

    Fix: Handle legacy type getting converted to intersection type #3164


  • Problem: The static type migration encounters errors, because FungibleToken.Vault is used in a restricted type (legacy type in an intersection type), and is a type requirement (!), but is converted to an intersection type itself:

    ERR failed to run StaticTypeMigration in account f23e934a74fbf163, domain public, key moxyTokenPlayAndEarnBalance: invalid non-composite/primitive replacement for legacy type in intersection type {A.9a0766d93b6608b7.FungibleToken.Balance}: A.9a0766d93b6608b7.FungibleToken.Vault replaced by {A.9a0766d93b6608b7.FungibleToken.Vault}
    

    Example: FungibleToken.Vault{FungibleToken.Balance} (note the restricted type is a nested type requirement).
    FungibleToken.Vault is now an interface, so gets converted to {FungibleToken.Vault}.

    Solution: Merge intersections

    Fix: Handle legacy type getting converted to intersection type #3164


  • Problem: CadenceBaseMigrator is instantiated for each account, and its InitMigration finds all contracts in all payloads. This is prohibitively expensive for large number of payloads.

    Solution: After performing the contract updates (NewCadence1ContractsMigrations), compute the contract map once, and use it for all value migrations (NewCadence1ValueMigrations)

    • Can this itself be parallelized / run as an "account-based migration"?

    Fix: [Cadence 1.0] Improve migrations flow-go#5533


  • Problem: The migration takes a very long time (several hours) for some accounts:

    5:53PM ERR failed to run EntitlementsMigration in account 34f3140b7f54c743, domain public, key FazeUtilityBonusReceiver: error getting program migration=cadence-value-migration migration_index=0
    9:37PM INF processing account group progress 4874390/4874393 (100.0%) elapsed: 20h40m21s, eta 0s
    10:14PM INF processing account group progress 4874391/4874393 (100.0%) elapsed: 21h16m35s, eta 0s
    10:48PM INF processing account group progress 4874392/4874393 (100.0%) elapsed: 21h50m30s, eta 0s
    

    Solution:

    • The migration of the largest accounts should be started first, schedule account groups in decreasing account size. The large accounts should be in separate groups, to ensure they are migrated concurrently.
    • Add cutoff
    • Merge static type migration with entitlements migration?

    Fixes: [Cadence 1.0] Improve migrations flow-go#5533


  • Problem: The migration requires the result of the published value migration to have an ID capability value. However, it isn't necessarily true that a migration returns a published value with an ID capability value, it may still be a path capability.

    Solution: Allow any capability value inside of a published value. Do not require the capability value to be an ID capability

    Fix: Improve migration #3169


@turbolent
Copy link
Member Author

I investigated the first issue, the loading of core types failing, e.g. failed to load type: A.9a0766d93b6608b7.FungibleToken.Vault.

In particular, the error indicates that the composite type A.9a0766d93b6608b7.FungibleToken.Vault cannot be found.

The error is actually correct: FungibleToken.Vault is not a composite, it is an interface.

So why is it being loaded as a composite type?

Well, FungibleToken.Vault used to be a "composite type", it was a nested type requirement.

So why is the composite type not rewritten by the composite type rewrite rules we specifically added for e.g. FungibleToken.Vault => {FungibleToken.Vault}?

It turns out this is caused by the missing handling of restricted types (intersection types with legacy types) being rewritten into intersection types themselves, i.e. the second error, invalid non-composite/primitive replacement for legacy type in intersection type.

Thus, #3164 actually fixes the first two problems.

@turbolent
Copy link
Member Author

All of the important fixes and improvements have been merged now.

There are two ideas that we still might want to consider, if needed:

  • The migration of the largest accounts should be started first, schedule account groups in decreasing account size. The large accounts should be in separate groups, to ensure they are migrated concurrently
  • Merge entitlements migration into static type migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant