Allow removal of enum declarations from contract interfaces for C1.0 upgrade #3197
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 ae127d3 Collapsed results for better readability
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3197 +/- ##
==========================================
+ Coverage 80.66% 80.68% +0.02%
==========================================
Files 380 380
Lines 93155 93179 +24
==========================================
+ Hits 75141 75181 +40
+ Misses 15306 15292 -14
+ Partials 2708 2706 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
turbolent
approved these changes
Mar 26, 2024
SupunS
approved these changes
Mar 26, 2024
runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go
Outdated
Show resolved
Hide resolved
…_test.go Co-authored-by: Supun Setunga <supun.setunga@gmail.com>
Why should this not be possible? This should be allowed even by the regular validator. |
We can only allow the removal if we also forbid the addition, otherwise an incompatible declaration could be reintroduced, circumventing the update checking rules for enums. I should have been maybe more specific, but I meant enum declarations added to interfaces in particular. I guess type checking on its own already rejects concrete type declarations (like e.g. structs, and enums) for in interfaces for all but events? (event declarations in interfaces are a particular exception (https://github.com/onflow/flips/blob/main/cadence/20230711-remove-type-requirements.md#defining-concrete-events-in-interfaces) |
|
Oh, yes in that case the new type checking will prevent enums from being added to interfaces implicitly; those contracts won't type check so there's no need to forbid it in the validator. |
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.
Closes #3182
This allows enum declarations to be removed from contract interfaces as part of the Cadence 1.0 upgrade. While in general this is not a safe operation to permit, analysis of mainnet contracts indicates that there are a small number of existing declarations of this kind, none of which are used in fields that themselves cannot be easily removed.
masterbranchFiles changedin the Github PR explorer