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

Allow removing type declarations from contracts #3210

Closed
Tracked by #3595
SupunS opened this issue Apr 1, 2024 · 3 comments · Fixed by #3376, onflow/cadence-lang.org#107 or #3658
Closed
Tracked by #3595

Allow removing type declarations from contracts #3210

SupunS opened this issue Apr 1, 2024 · 3 comments · Fixed by #3376, onflow/cadence-lang.org#107 or #3658

Comments

@SupunS
Copy link
Member

SupunS commented Apr 1, 2024

Issue to be solved

Currently, the contract update validator prevents removing type declarations from a contract, to prevent it from getting re-added with a different structure. However, it would be useful to have a way to remove type definitions and force users of those contracts to start using a new type instead. This could be even more useful with cadence 1.0.

Suggested Solution

Suggestion I:

One possible solution is to introduce a pragma to tombstone a type definition. e.g: #removedType(T).
One can remove a type and add this pragma to indicate that this type was removed. It will prevent re-adding a new type with the same name in future. The pragma itself is not removable once it is aded.

Cons:

  • The problem with just removing a type is that, any stored value belongs to this type would become unusable. This does not cause any harm to the network though (which was the original intention of preventing type removals).
  • How would migrations handle these missing types?

Suggestion II:

Alternatively, it is possible to provide a way to remove and replace a type with another, particularly during the Cadence 1.0 upgrades. e.g: #replacedType(T, R), where R is the new type to replace with. Similar to the suggested removedType pragma, this will prevent any re-additions of a new type with the same name.

  • During the state migrations, all usages of T would be replaced with type R, including static types of values, conformances, etc.
  • This is in a way, providing the user also the ability to define type-change rules similar to what we have done for the system contracts.
  • Migrations will first have to collect all these replacements pairs first, before do the actual migrations

This would be only supported if T and R satisfy particular conditions, such as:

  • T and R has to be of the same kind. i.e: both are resources / both are resource-interfaces / etc.
  • The new type R should have at-most the set of fields as the old type T. It's ok to have less number of fields, but no any additional fields. The filed names and types of the fields should be the same. (This is similar to the existing field update restrictions during contract updates).
  • So this is basically similar to allowing renaming a type (in addition to what is already supported in existing contract updates)
  • Anything else??
@j1010001
Copy link
Member

j1010001 commented Apr 5, 2024

Not breaking, does not make sense to do this post C1.0 migration,
might be needed for some contracts to complete migration to C1.0, not a blocker, nice to have (workaround - keeping old types, NFT standard also keep some old type interfaces, if people start to use the old types, it can cause problems).

@j1010001
Copy link
Member

Implementing Suggestion 1 would unblock staging of the FiatToken contract, which is now a blocker for Crescendo launch. Increasing priority to urgent.

@turbolent
Copy link
Member

@dsainati1 Could you please document this? Both in the language reference, as well as a short update to the forum post https://forum.flow.com/t/update-on-cadence-1-0/5197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment