-
Notifications
You must be signed in to change notification settings - Fork 138
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
Entitlements Migration #2951
Entitlements Migration #2951
Conversation
…ati/entitlement-sets
…to sainati/entitlement-sets
…into sainati/convert-entitled-value
…:onflow/cadence into sainati/entitlements-migration
…dence into sainati/entitlements-migration
…to sainati/entitlements-migration
This is currently in a draft state, I need to add more tests, especially for the cases where contracts are being loaded from multiple different locations and addresses, and for the case where one or more contracts has not yet been updated and thus fails to typecheck. |
Co-authored-by: Supun Setunga <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just some last suggestions and questions
@SupunS Do we have to use the same "trick" that we used in the type value migration, which forces the re-hashing / removal + reinsertion, in this migration? |
ah, that's a good flag. It is currently handled in the migration framework: cadence/migrations/migration.go Lines 243 to 246 in f5f26f6
But might need to make a small adjustment to return a 'legacy reference type' for the reference type: cadence/migrations/migration.go Lines 353 to 357 in f5f26f6
@dsainati1 A way to test this is to have a type-value as a dictionary key, and the inner type of that type-value should get migrated by the entitlement migrations. |
Added this test, which passes with no code changes. |
Let's double check https://discord.com/channels/613813861610684416/1108479699732152503/1194054315644571750, (sorry for being overly cautious with the migration code) |
I think we can merge this one. Any changes required for re-hashing can/will be added in #3013 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Sorry it took so long to get in
…dence into sainati/entitlements-migration
Co-authored-by: Supun Setunga <[email protected]>
Adds the entitlements migration as detailed in onflow/flips#95.
Part of #2709
master
branchFiles changed
in the Github PR explorer