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

Add storage migration to inject static types to cadence values #1264

Merged
merged 80 commits into from
Sep 8, 2021

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Sep 7, 2021

Description:

This migration adds static types info to cadence values with missing type info. This is required for onflow/cadence#870

Depends on: #1265

Steps to run:

  • Build the binary with:
    GOOS=linux GOARCH=amd64 go build ./cmd/util
    
  • Run with the following command. Here <checkpoint-dir> is the directory where the checkpoint file is located. <output-dir> is the directory where outputs will be generated (assumes <output-dir> exists).
    ./util execution-state-extract --execution-state-dir <checkpoint-dir> --output-dir <output-dir>
    
  • To remove broken contracts and clean up the storage, add --cleanup-storage flag. e.g:
    ./util execution-state-extract --execution-state-dir <checkpoint-dir> --output-dir <output-dir> --cleanup-storage
    

@ramtinms
Copy link
Contributor

ramtinms commented Sep 8, 2021

Looks good to me. as soon as we fix the issue with storage_used this can be used

@SupunS
Copy link
Member Author

SupunS commented Sep 8, 2021

Fixed storage_used issue in 72fd6fc. Manually tested with the state dumps, no issues now. I'll add some unit tests as well in the meantime.

@SupunS SupunS force-pushed the bastian/cadence-storage-v5 branch from e77a19f to e01d6a9 Compare September 8, 2021 03:12
@SupunS SupunS force-pushed the bastian/cadence-storage-v5 branch from e01d6a9 to 2d86829 Compare September 8, 2021 03:20
@SupunS SupunS requested a review from Kay-Zee September 8, 2021 04:57
@codecov-commenter
Copy link

Codecov Report

Merging #1264 (40a45b4) into master (464258f) will decrease coverage by 1.43%.
The diff coverage is 16.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1264      +/-   ##
==========================================
- Coverage   56.03%   54.59%   -1.44%     
==========================================
  Files         497      498       +1     
  Lines       30386    31533    +1147     
==========================================
+ Hits        17027    17217     +190     
- Misses      11049    11968     +919     
- Partials     2310     2348      +38     
Flag Coverage Δ
unittests 54.59% <16.83%> (-1.44%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
ledger/complete/mtrie/forest.go 57.77% <0.00%> (ø)
...execution-state-extract/execution_state_extract.go 59.37% <5.88%> (-17.10%) ⬇️
cmd/util/ledger/migrations/storage_v4.go 4.84% <12.50%> (-0.03%) ⬇️
cmd/util/ledger/migrations/storage_v5.go 16.00% <16.00%> (ø)
cmd/util/cmd/execution-state-extract/cmd.go 56.25% <50.00%> (-4.22%) ⬇️
ledger/complete/ledger.go 61.72% <60.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 464258f...40a45b4. Read the comment docs.

@Kay-Zee
Copy link
Member

Kay-Zee commented Sep 8, 2021

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 8, 2021

@bors bors bot merged commit 06aa464 into master Sep 8, 2021
@bors bors bot deleted the bastian/cadence-storage-v5 branch September 8, 2021 07:27
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

Successfully merging this pull request may close these issues.

5 participants