FLIP: Cadence - Enable new fields on existing resource and struct definitions

[austinkline]

FLIP for enabling the addition of new fields in existing resource and struct definitions

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[vercel]

@austinkline is attempting to deploy a commit to the Flow Team on Vercel.

A member of the Team first needs to authorize it.

[dsainati1]

In the future, if you're submitting a Cadence FLIP, please add some people from the Cadence team (e.g. myself, @turbolent, @SupunS and @robert-e-davidson3) as reviewers; otherwise we don't get notifications about these. I wasn't aware this even existed until someone pointed out to me!

[austinkline]

In the future, if you're submitting a Cadence FLIP, please add some people from the Cadence team (e.g. myself, @turbolent, @SupunS and @robert-e-davidson3) as reviewers; otherwise we don't get notifications about these. I wasn't aware this even existed until someone pointed out to me!

ahhh good to know! Noted for the future 👍

[dsainati1]

Nice. I like the idea here; I agree this would make life easier for Cadence developers. The proposal could use a little more detail in some places though, specifically with regards to how the proposed changes would interact with existing behavior of the language.

[dsainati1]

The design proposal needs more detail about how optional or default initialized fields would interact with the existing initialization requirements. Do these fields need to be handled in the init function, or is it possible to omit them and have these fields be nil or the default value implicitly on initialization?

[turbolent]

The current design of only allowing field initialization in one place, the initializer, instead of the initializer or in the field declaration, was deliberate: There is only one place to look at when reading the code and it avoids many design questions related to expressions in field declarations:

• When is the expression evaluated? Once, "statically" for all instances? For each instance, before the initializer?
• What is the evaluation order? Can a field refer to another?

Is the default value only used when migrating existing values, or is it also used for new values?

[austinkline]

My thought here was the the init function could recognize what's missing and either accept the default field or set the value to nil. It sounds like a default value might be more tough to achieve specially from what @turbolent is calling out. Happy to scope this FLIP down to specifically nullable additional fields if default initial values aren't feasible.

When is the expression evaluated? Once, "statically" for all instances? For each instance, before the initializer?

If we can accept default values it would have to be before the analyzer, right?

What is the evaluation order? Can a field refer to another?

My thought was that fields cannot refer to one another. If they can, this would get much more complex since then we're talking about migrations and the nuances with multiple new fields all relying on each other (not to mention side effects and function calls if it got that deep). Ideally, when the composite type is accessed, these nullable values would be updated at that time so that we can get around the need for full migrations which aren't feasible to do.

[dsainati1]

cc @ramtinms if implemented this might have implications for potential work on composite type inlining

[dsainati1]

We should add a Questions and Discussion section to talk about related topics to this FLIP. In particular, one that comes to mind is how this is going to interact with the potential addition of extensions to the language proposed here: #1101. As that proposal stands now, we'd need some way of handling the case where a user updates their struct or resource with a new field or method that also exists in an extension. It's also worth considering the extent to which these two proposals overlap in their use cases.

[austinkline]

We should add a Questions and Discussion section to talk about related topics to this FLIP.

Will do 👍

In particular, one that comes to mind is how this is going to interact with the potential addition of extensions to the language proposed here: #1101. As that proposal stands now, we'd need some way of handling the case where a user updates their struct or resource with a new field or method that also exists in an extension.

Would the analyzer know about this conflict when an update is attempted or would it only be detected at runtime? This sounds like the Diamond problem (kind of), so I wonder if we can't pull inspiration from how Go does it, or how interpreted languages like Python do it as a means to discuss in general how overlapping types in general should be handled.

Multiple Inheritance Wiki

If we went with the way Go does this, we would reject the update outright, stating that the use of certain fields (or methods) are ambiguous, though I'm not sure if we know what we need to when an update is attempted to make that work. Perhaps if you only allow the extension of Composite types inside of the contract they exist in?

If we went with the way Perl and Python do it, we would take whichever definition comes first. That is, if we have struct T and extensions T2 and T3, the order in which the extensions are specified would dictate what wins out. In that model, any overlapping fields which exist both on T and TN would always fallback to T since it came first.

In the end, I think this problem depends on what the "real" type is that we are dealing with. Definitely worth exploring that more here so I'd love to hear your thoughts!

It's also worth considering the extent to which these two proposals overlap in their use cases.

Makes sense, their purpose at their core seem to be the same. Primarily that currently folks have to over-design for their contracts and take on risk for that over-design in order to get flexibility in return so that future features are possible. Curious peoples' thoughts to whether both have their merits, though. My general thought process to this FLIP was also centered around reducing the amount of extra code a dev needs in order to support new fields. Should this FLIP go through, theoretically new fields would be accessible with no extra work (all handled out of sight of the cadence dev)

[dsainati1]

Would it be beneficial to allow updating contracts with new methods as well? Or just fields?

[austinkline]

Just to clarify, can't functions be updated freely except for some fairly specific circumstances regarding function types?

https://developers.flow.com/cadence/language/contract-updatability#functions

Updating a function definition is always valid, as function definitions are never stored as data. i.e: Function definition is a part of the code, but not data.

• Changing a function signature (parameters, return types) is valid.
• Changing a function body is also valid.
• Changing the access modifier is valid.

However, changing a function type may or may not be valid, depending on where it is used. i.e: If a function type is used in the type annotation of a composite type field (direct or indirect), then changing the function type signature is the same as changing the type annotation of that field (which is again invalid).

[dsainati1]

Users already have the ability to remove fields from existing composites, so giving them the ability to add new fields can implicitly allow changing a field's type if a user removes an existing field (that had type T1) in one update and then adds it back with a different type T2 in another update. We'd need a specific solution for this case. One obvious answer is to simply fail a contract update if existing instances of a struct or resource have conflicting types with the field being added, but there are other potential solutions. Either way we should make it explicit in the proposal.

[austinkline]

Great callout, I hadn't thought about this. My understanding of what happens when a type definition is taken away is that it is simply hidden (the data itself is not destroyed) but I am assuming that is only for existing definitions of those resources/structs which wouldn't cover doing them in separate instances.

Your proposed option is what I would jump to as well, and then perhaps another alternative is to only allow new nullable fields to struct/resource definitions and then to assign those conflicting types which have the old type definition to nil when accessed since they don't match the new definition.

// old definition
struct Foo {
  amount: UInt64
}

//new definition
struct Foo {
  amount: Int64?
}

If I were to change the definition as mentioned above, existing instances of Foo would have their amounts set to nil because their types do not match. This approach could open up the ability to alter structs/resources even more but I worry about the risk of a malicious dev toggling between type definitions in some capacity to force some kind of odd behavior.

[turbolent]

This feature has been a goal ever since the early days of Cadence, thank you for opening a FLIP for it.

The main reason why this feature has not been implemented yet is that it is unclear how it should be implemented. Even though designs/proposals for language features usually do not discuss implementation details, it is an important factor when deciding if the proposal should be accepted.

It would be great if the proposal could describe the behaviour of the necessary migration, in particular: When is the migration performed? Eagerly migrating all existing data at contract update-time is not tractable on-chain. For example, common relational database systems take multiple seconds, minutes, or even hours to migrate tables when adding new columns. The migration should probably occur lazily, when an existing value is accessed.

Another open question is how this feature interacts with the existing feature of removing fields. Currently, this is implemented by only updating the code and not actually removing the data (for the same reasons as mentioned above). Allowing addition would lead to effectively allowing field updates, i.e. changing the type of a field, by first removing the field, then adding it back with a different type.

[turbolent]

The current design of only allowing field initialization in one place, the initializer, instead of the initializer or in the field declaration, was deliberate: There is only one place to look at when reading the code and it avoids many design questions related to expressions in field declarations:

• When is the expression evaluated? Once, "statically" for all instances? For each instance, before the initializer?
• What is the evaluation order? Can a field refer to another?

Is the default value only used when migrating existing values, or is it also used for new values?

[austinkline]

Nice. I like the idea here; I agree this would make life easier for Cadence developers. The proposal could use a little more detail in some places though, specifically with regards to how the proposed changes would interact with existing behavior of the language.

To make sure I can cover what is missing as explicitly as possible, it sounds like two main pieces:

1. How would default initialized values work? Sounds like this approach could go against some core design decisions in cadence so this might be better off getting scoped out.
2. How would cadence handle these new fields on existing instances of composite types?

[austinkline]

@turbolent replying inline here but wanted to make sure your points are all addressed to be the best of my ability:

The main reason why this feature has not been implemented yet is that it is unclear how it should be implemented. Even though designs/proposals for language features usually do not discuss implementation details, it is an important factor when deciding if the proposal should be accepted.

Agreed and makes total sense, I will circle back to this FLIP and add more detail so we have more to go on in terms of discussion.

It would be great if the proposal could describe the behaviour of the necessary migration, in particular: When is the migration performed? Eagerly migrating all existing data at contract update-time is not tractable on-chain. For example, common relational database systems take multiple seconds, minutes, or even hours to migrate tables when adding new columns. The migration should probably occur lazily, when an existing value is accessed.

A few folks I've discussed this topic with have brought up migrations as well. It is why this FLIP doesn't allow reliance on other fields which would make evaluating them much more difficult when they don't "exist" yet under the hood. My take here is also to lazily evaluate as they are accessed for the first time. That could be when we access the entire object, or it could be when the new field itself is accessed for the first time

Another open question is how this feature interacts with the existing feature of removing fields. Currently, this is implemented by only updating the code and not actually removing the data (for the same reasons as mentioned above). Allowing addition would lead to effectively allowing field updates, i.e. changing the type of a field, by first removing the field, then adding it back with a different type.

Yes, this was a good callout which I had neglected to consider but is certainly a core part of the problems in allowing updates like this. My current leaning is that since new fields would need to be nullable (or have a default), and conflicting type would need to be returned as null if it is found to be "incorrect" That is, if I have

struct Foo {
  amount: Int
}

and I then remove the field, and try to add a new one with the same name:

struct Foo {
  amount: UInt64?
}

Then existing instances of Foo would return nil when the amount field is accessed. That value would still remain unless explicitly overridden in the scope of a transaction

Alternatively, if we allowed default values, instead of nil we would assign the colliding field to that default value.

struct Foo {
  amount: UInt64 = 100
}

The above would mean that all existing instances of Foo which have the old type for amount would read as 100

[austinkline]

Thanks for the comments, I will take some time this weekend to elaborate on the FLIP per everyone's feedback and go from there

[austinkline]

Just to clarify, can't functions be updated freely except for some fairly specific circumstances regarding function types?

https://developers.flow.com/cadence/language/contract-updatability#functions

Updating a function definition is always valid, as function definitions are never stored as data. i.e: Function definition is a part of the code, but not data.

• Changing a function signature (parameters, return types) is valid.
• Changing a function body is also valid.
• Changing the access modifier is valid.

However, changing a function type may or may not be valid, depending on where it is used. i.e: If a function type is used in the type annotation of a composite type field (direct or indirect), then changing the function type signature is the same as changing the type annotation of that field (which is again invalid).

[austinkline]

My thought here was the the init function could recognize what's missing and either accept the default field or set the value to nil. It sounds like a default value might be more tough to achieve specially from what @turbolent is calling out. Happy to scope this FLIP down to specifically nullable additional fields if default initial values aren't feasible.

When is the expression evaluated? Once, "statically" for all instances? For each instance, before the initializer?

If we can accept default values it would have to be before the analyzer, right?

What is the evaluation order? Can a field refer to another?

My thought was that fields cannot refer to one another. If they can, this would get much more complex since then we're talking about migrations and the nuances with multiple new fields all relying on each other (not to mention side effects and function calls if it got that deep). Ideally, when the composite type is accessed, these nullable values would be updated at that time so that we can get around the need for full migrations which aren't feasible to do.

[austinkline]

Great callout, I hadn't thought about this. My understanding of what happens when a type definition is taken away is that it is simply hidden (the data itself is not destroyed) but I am assuming that is only for existing definitions of those resources/structs which wouldn't cover doing them in separate instances.

Your proposed option is what I would jump to as well, and then perhaps another alternative is to only allow new nullable fields to struct/resource definitions and then to assign those conflicting types which have the old type definition to nil when accessed since they don't match the new definition.

// old definition
struct Foo {
  amount: UInt64
}

//new definition
struct Foo {
  amount: Int64?
}

If I were to change the definition as mentioned above, existing instances of Foo would have their amounts set to nil because their types do not match. This approach could open up the ability to alter structs/resources even more but I worry about the risk of a malicious dev toggling between type definitions in some capacity to force some kind of odd behavior.

[austinkline]

We should add a Questions and Discussion section to talk about related topics to this FLIP.

Will do 👍

In particular, one that comes to mind is how this is going to interact with the potential addition of extensions to the language proposed here: #1101. As that proposal stands now, we'd need some way of handling the case where a user updates their struct or resource with a new field or method that also exists in an extension.

Would the analyzer know about this conflict when an update is attempted or would it only be detected at runtime? This sounds like the Diamond problem (kind of), so I wonder if we can't pull inspiration from how Go does it, or how interpreted languages like Python do it as a means to discuss in general how overlapping types in general should be handled.

Multiple Inheritance Wiki

If we went with the way Go does this, we would reject the update outright, stating that the use of certain fields (or methods) are ambiguous, though I'm not sure if we know what we need to when an update is attempted to make that work. Perhaps if you only allow the extension of Composite types inside of the contract they exist in?

If we went with the way Perl and Python do it, we would take whichever definition comes first. That is, if we have struct T and extensions T2 and T3, the order in which the extensions are specified would dictate what wins out. In that model, any overlapping fields which exist both on T and TN would always fallback to T since it came first.

In the end, I think this problem depends on what the "real" type is that we are dealing with. Definitely worth exploring that more here so I'd love to hear your thoughts!

It's also worth considering the extent to which these two proposals overlap in their use cases.

Makes sense, their purpose at their core seem to be the same. Primarily that currently folks have to over-design for their contracts and take on risk for that over-design in order to get flexibility in return so that future features are possible. Curious peoples' thoughts to whether both have their merits, though. My general thought process to this FLIP was also centered around reducing the amount of extra code a dev needs in order to support new fields. Should this FLIP go through, theoretically new fields would be accessible with no extra work (all handled out of sight of the cadence dev)

[bluesign]

In the future, if you're submitting a Cadence FLIP, please add some people from the Cadence team (e.g. myself, @turbolent, @SupunS and @robert-e-davidson3) as reviewers; otherwise we don't get notifications about these. I wasn't aware this even existed until someone pointed out to me!

Yeah this is big problem usually, though I think everyone from Dapper should follow @onflow/flow repository at least ( I am following almost all Flow & Dapper repositories, it is taking just 20 mins with my morning coffee, 80% of activity is being flow-go )

[bluesign]

Allowing addition would lead to effectively allowing field updates, i.e. changing the type of a field, by first removing the field, then adding it back with a different type.

I think forbidding removal and allowing addition can be net improvement

One problem I see from developer perspective, is adding fields as optional. I think it is a bit changing the meaning of Optional. Maybe we can allow adding synthetics fields ? They can lead to lazy migration. But also they have the problem of evaluation order.

[austinkline]

Allowing addition would lead to effectively allowing field updates, i.e. changing the type of a field, by first removing the field, then adding it back with a different type.

I think forbidding removal and allowing addition can be net improvement

One problem I see from developer perspective, is adding fields as optional. I think it is a bit changing the meaning of Optional. Maybe we can allow adding synthetics fields ? They can lead to lazy migration. But also they have the problem of evaluation order.

Not sure I follow how this would change the meaning of optionals? Is it really any different to a cadence dev whether the field used to be nil or is nil, now? Or do you mean to flow itself and how it treats them?

Is synthetic right here, though? Once a new field is set it would be as real as anything else. Forcing them to be optional just helps ensure we don't need a migration strategy for existing data which I would say is a core requirement to this FLIP

[bluesign]

For me, also I think in the design of Cadence, optional variable means more than variable can be nil. ( though I can be wrong, maybe @turbolent can clarify more )

But with my limited knowledge of optionals, actually they are property of the object also. ( some property of an object being optional, is also a property of the object )

so when we add a new property, it is being optional should depend on the object, not our technical limitations. Ofc we can decide this is a valid trade off, and add new fields as optionals.

For me migration is not that scary to be honest, if we forbid removal.

[robert-e-davidson3]

lgtm though if this cannot be resolved with the extensions FLIP, I prefer that FLIP to this one