Also migrate path-capability and account-capability storage maps #3411
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit 145dea1 Collapsed results for better readability
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3411 +/- ##
==========================================
- Coverage 80.42% 80.39% -0.03%
==========================================
Files 389 390 +1
Lines 95654 95790 +136
==========================================
+ Hits 76930 77013 +83
- Misses 16006 16058 +52
- Partials 2718 2719 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
turbolent
changed the title
fxamacker
approved these changes
Jun 11, 2024
SupunS
approved these changes
Jun 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to onflow/flow-go#6069
Description
For completeness, also migrate the path-capability and the account-capability storage map.
The path-capability storage maps record all storage capability IDs for a storage path. The storage maps have strings as keys (the identifier of the storage path, e.g.
fooin/storage/foo), and the values are Cadence dictionaries, which are rather sets, where the keys are the capability IDs (UInt64) and the values are always Cadencenilvalues.The account-capability storage maps record all account capability IDs for an account. The storage maps have uint64 keys (the capability IDs), and the values are always Cadence
nilvalues. The storage maps are rather sets.AFAIK we currently do not have migrations required for Cadence 1.0 that migrate such values (dictionaries and
UInt64values), but still migrate these storage maps for completeness / correctness – the assumption we do not have migrations for such values might not hold in the future.Kudos to @fxamacker for noticing this omission. In the Cadence 1.0 migrations it is not important, but in the atree inlining migrations all values need to be migrated, so it is important there. See related issue.
masterbranchFiles changedin the Github PR explorer