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

Port #5755 to feature/atree-inlining-cadence-v0.42 #5765

Conversation

turbolent
Copy link
Member

@turbolent turbolent commented Apr 23, 2024

Work towards onflow/cadence#3220

Port #5755 to https://github.com/onflow/flow-go/tree/feature/atree-inlining-cadence-v0.42

Work done by @fxamacker

This PR adds --fix-testnet-slabs-with-broken-references flag to the migration program.

It uses a feature from onflow/atree#388 to fix 10 testnet registers affecting 9 testnet accounts.

The 9 testnet accounts are hardcoded in this commit to prevent accidentally applying this fix to any other accounts.

In testnet, broken references seem to have resulted from a bug that was fixed 2 years ago by onflow/cadence#1565.

A broken reference is a StorageID referencing a non-existent register. So far, only 10 registers in testnet (none on mainnet) were found to contain broken references.

fxamacker and others added 7 commits April 23, 2024 13:28
This commit adds --fix-testnet-slabs-with-broken-references flag
to the migration program.

It uses a feature from onflow/atree#388 to fix 10 testnet registers
affecting 9 testnet accounts.

The 9 testnet accounts are hardcoded in this commit to prevent
accidentally applying this fix to any other accounts.

In testnet, broken references seem to have resulted from a bug
that was fixed 2 years ago by onflow/cadence#1565.

A broken reference is a `StorageID` referencing a non-existent register.
So far, only 10 registers in testnet (none on mainnet) were found to
contain broken references.
There are 9 testnet accounts with known issues that
will be fixed during migration, so we don't need to skip them.

The fixes during migration will only applied to 9 hardcoded
testnet accounts for the identified problem.
@turbolent turbolent requested review from fxamacker and a team April 23, 2024 20:38
@turbolent turbolent self-assigned this Apr 23, 2024
@turbolent
Copy link
Member Author

@fxamacker Integrated the migration in f9798b7

Copy link
Member

@fxamacker fxamacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for porting this over!

I left a comment about integration with pipeline.

Co-authored-by: Faye Amacker <[email protected]>
@turbolent turbolent requested review from fxamacker and a team April 23, 2024 21:58
@turbolent turbolent changed the title Add migration to fix refs to non-existent registers Port #5755 to feature/atree-inlining-cadence-v0.42 Apr 23, 2024
@fxamacker
Copy link
Member

@turbolent There are some compile errors due to breaking compatibility of atree.

Do you mind if I push some commits to fix them?

fxamacker and others added 2 commits April 23, 2024 17:38
This function was added in PR #5613 by Janez for
feature/stable-cadence and was copied into PR #5765.

Co-authored-by: Janez Podhostnik <[email protected]>
@turbolent
Copy link
Member Author

6f7004c and e5c6ec2 look great @fxamacker, thank you for fixing this up!

@turbolent turbolent requested review from a team, fxamacker and SupunS April 23, 2024 22:56
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.39%. Comparing base (7b89544) to head (1f414a9).
Report is 24 commits behind head on feature/atree-inlining-cadence-v0.42.

Additional details and impacted files
@@                          Coverage Diff                          @@
##           feature/atree-inlining-cadence-v0.42    #5765   +/-   ##
=====================================================================
  Coverage                                 53.39%   53.39%           
=====================================================================
  Files                                        16       16           
  Lines                                      2270     2270           
=====================================================================
  Hits                                       1212     1212           
  Misses                                      971      971           
  Partials                                     87       87           
Flag Coverage Δ
unittests 53.39% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@turbolent turbolent merged commit 4836ac7 into feature/atree-inlining-cadence-v0.42 Apr 23, 2024
55 checks passed
@turbolent turbolent deleted the bastian/port-5755-atree-inlining-cadence-v0.42 branch April 23, 2024 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants