From 2d5e0327a874abf27784188ddf2818089f66a2f7 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 17 May 2023 16:44:10 -0700 Subject: [PATCH 1/6] Add FLIP for changing member access semantics --- cadence/20230517-member-access-semnatics.md | 185 ++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 cadence/20230517-member-access-semnatics.md diff --git a/cadence/20230517-member-access-semnatics.md b/cadence/20230517-member-access-semnatics.md new file mode 100644 index 00000000..ebfeff7c --- /dev/null +++ b/cadence/20230517-member-access-semnatics.md @@ -0,0 +1,185 @@ +--- +status: proposed +flip: 89 +authors: Supun Setunga (supun.setunga@dapperlabs.com) +sponsor: Supun Setunga (supun.setunga@dapperlabs.com) +updated: 2023-05-17 +--- + +# Change Member Access Semantics + +## Objective + +This proposal suggest to return a reference when accessing a member of a container (field of a composite value, +key of a map, index of an array), if the parent value is also a reference and the accessed member is a container type. + +## Motivation + +A previous version of Cadence ("Secure Cadence") restricted the potential foot-gun of mutating container-typed +`let` fields/variables via the +[Cadence mutability restrictions FLIP](https://github.com/onflow/flips/blob/main/flips/20211129-cadence-mutability-restrictions.md). +However, this was a partial solution, and was only applicable for arrays and dictionaries, but did not solve it for +user-defined types. + +There are still ways to mutate such fields by: +- Directly mutating a nested composite typed field. +- Mutating a field by calling a mutating function on the field. +- Mutating the field via a reference. + +More details on this problem is described in the [FLIP for improving mutability restrictions](https://github.com/onflow/flips/pull/58). + +This proposal is aimed at moving one step closer to solving this problem by returning references for member-access, +so that [Entitlements](https://github.com/onflow/flips/pull/54) can be used to control who can perform mutating +operations via references. + +## User Benefit + +As mentioned in the previous section, this change enables using Entitlements to prevent accidental mutations via references. + +## Design Proposal + +This proposal suggests field-access (i.e. `foo.bar`) and index-access (`foo[bar]`) to +return a reference to the targeting (field or element) value, instead of the value itself, if: + - The parent (i.e. `foo`) is a reference to a container-typed value. + - The accessed field/element is again container-typed. + +where a "container" is any of: + - Dictionary + - Array + - Composite + - An optional of above + +For example, consider the below `Collection` struct which has two fields: one (`id`) is String-typed, +and the other (`ownedNFTs`) is dictionary-typed. + +```cadence +pub struct Collection { + + // Primitive-typed field + pub var id: String + + // Dictionary typed field + pub var ownedNFTs: {UInt64: NFT} +} +``` + +**Case I: Code owns the container value** + +Assume `collection` is of type `Collection`. i.e. the code owns the value. +Then, there is no change to the current behavior. + +```cadence +var collection: Collection = ... + +// `collection.ownedNFTs` would return the concrete value, which is of type `{UInt64: NFT}`. +// This is same as existing semantics. +var ownedNFTs: {UInt64: NFT} = collection.ownedNFTs + +// `collection.id` would return the concrete string value. +// This is same as existing semantics. +var id: String = collection.id +``` + +**Case II: Code only has a reference to the container value** + +Assume a reference to the collection `collectionRef` is available, and is of type `&Collection`. +i.e. code doesn't own the value, but has only a reference to the value. + +```cadence +var collectionRef: &Collection = ... + +// `collectionRef.ownedNFTs` would now return a reference of type `&{UInt64: NFT}`. +var ownedNFTsRef: &{UInt64: NFT} = collectionRef.ownedNFTs + +// However, `collectionRef.id` would return the value, since it is a primitive type. +// This is same as existing semantics. +var id: String = collectionRef.id +``` + +It is also important to note that, the returned reference has no entitlements assigned to them. +i.e: they are **non-**`auth` references. + +### Example + +Assume a nested resource collection as bellow. + +```cadence +pub resource MasterCollection { + pub var kittyCollection: @Collection + pub var topshotCollection: @Collection +} + + +pub resource Collection { + pub var ownedNFTs: @{UInt64: NonFungibleToken.NFT} + + access(Withdraw) fun withdraw(withdrawID: UInt64): @NonFungibleToken.NFT { + let token <- self.ownedNFTs.remove(key: withdrawID) ?? panic("missing NFT") + emit Withdraw(id: token.id, from: self.owner?.address) + return <-token + } +} +``` + +Also, suppose a reference to the `MasterCollection` without any entitlements (i.e: non-auth) is available/borrowed. + +```cadence +var masterCollectionRef: @MasterCollection = ... +``` + +Previously, it was possible to withdraw from the inner collection, despite the `masterCollectionRef` not having the +`Withdraw` entitlement. + +```cadence +masterCollectionRef.kittyCollection.withdraw(24) // OK +``` + +With the proposed changes, this will be an error, since `masterCollectionRef.kittyCollection` is now return a +non-auth reference, and calling `withdraw` is prohibited because that reference doesn't have the `Withdraw` entitlement. + +```cadence +masterCollectionRef.kittyCollection.withdraw(24) // Static Error +``` + +### Drawbacks + +None + +### Alternatives Considered + +None + +### Performance Implications + +There is no direct performance impact by this change. + +However, since accessing a nested objects through a reference would now be returning reference, +it will avoid the copying of nested objects, which may increase the performance for certain use-cases. + +### Dependencies + +None + +### Engineering Impact + +This change has medium complexity in the implementation. + +### Compatibility + +This is a backward incompatible change. + +### User Impact + +Many deployed contracts might be affected by this change. + +## Related Issues + +None + +## Prior Art + +None + +## Questions and Discussion Topics + +None \ No newline at end of file From 37df62b2e68371bd79a3b1130ea26cf218345847 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 18 May 2023 16:54:59 -0700 Subject: [PATCH 2/6] Fix link Co-authored-by: Jan Bernatik --- cadence/20230517-member-access-semnatics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cadence/20230517-member-access-semnatics.md b/cadence/20230517-member-access-semnatics.md index ebfeff7c..e3e35895 100644 --- a/cadence/20230517-member-access-semnatics.md +++ b/cadence/20230517-member-access-semnatics.md @@ -17,7 +17,7 @@ key of a map, index of an array), if the parent value is also a reference and th A previous version of Cadence ("Secure Cadence") restricted the potential foot-gun of mutating container-typed `let` fields/variables via the -[Cadence mutability restrictions FLIP](https://github.com/onflow/flips/blob/main/flips/20211129-cadence-mutability-restrictions.md). +[Cadence mutability restrictions FLIP](https://github.com/onflow/flips/blob/main/cadence/20211129-cadence-mutability-restrictions.md). However, this was a partial solution, and was only applicable for arrays and dictionaries, but did not solve it for user-defined types. From 459b3b185fdab3fa3b635e5ac6ebf1fdf2acf79d Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 31 May 2023 14:24:33 -0700 Subject: [PATCH 3/6] Update example to use resources --- cadence/20230517-member-access-semnatics.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/cadence/20230517-member-access-semnatics.md b/cadence/20230517-member-access-semnatics.md index ebfeff7c..ffb8d094 100644 --- a/cadence/20230517-member-access-semnatics.md +++ b/cadence/20230517-member-access-semnatics.md @@ -6,7 +6,7 @@ sponsor: Supun Setunga (supun.setunga@dapperlabs.com) updated: 2023-05-17 --- -# Change Member Access Semantics +# FLIP 89: Change Member Access Semantics ## Objective @@ -53,34 +53,35 @@ For example, consider the below `Collection` struct which has two fields: one (` and the other (`ownedNFTs`) is dictionary-typed. ```cadence -pub struct Collection { +pub resource Collection { // Primitive-typed field pub var id: String // Dictionary typed field - pub var ownedNFTs: {UInt64: NFT} + pub var ownedNFTs: @{UInt64: NFT} } ``` -**Case I: Code owns the container value** +#### Case I: Code owns the container value Assume `collection` is of type `Collection`. i.e. the code owns the value. Then, there is no change to the current behavior. ```cadence -var collection: Collection = ... +var collection: @Collection = ... -// `collection.ownedNFTs` would return the concrete value, which is of type `{UInt64: NFT}`. +// `collection.ownedNFTs` would return the concrete value, which is of type `@{UInt64: NFT}`. +// This is an error because it is not possible to move nested resource. // This is same as existing semantics. -var ownedNFTs: {UInt64: NFT} = collection.ownedNFTs +var ownedNFTs: @{UInt64: NFT} <- collection.ownedNFTs // `collection.id` would return the concrete string value. // This is same as existing semantics. var id: String = collection.id ``` -**Case II: Code only has a reference to the container value** +#### Case II: Code only has a reference to the container value** Assume a reference to the collection `collectionRef` is available, and is of type `&Collection`. i.e. code doesn't own the value, but has only a reference to the value. From 179c6b180211d9d8f7049cd44da2fce0641f796b Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 1 Jun 2023 09:07:50 -0700 Subject: [PATCH 4/6] Add example for structs --- cadence/20230517-member-access-semnatics.md | 60 +++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/cadence/20230517-member-access-semnatics.md b/cadence/20230517-member-access-semnatics.md index a38ed8e2..b3212106 100644 --- a/cadence/20230517-member-access-semnatics.md +++ b/cadence/20230517-member-access-semnatics.md @@ -49,7 +49,9 @@ where a "container" is any of: - Composite - An optional of above -For example, consider the below `Collection` struct which has two fields: one (`id`) is String-typed, +### Resource Example: + +For example, consider the below `Collection` resource which has two fields: one (`id`) is String-typed, and the other (`ownedNFTs`) is dictionary-typed. ```cadence @@ -81,7 +83,7 @@ var ownedNFTs: @{UInt64: NFT} <- collection.ownedNFTs var id: String = collection.id ``` -#### Case II: Code only has a reference to the container value** +#### Case II: Code only has a reference to the container value Assume a reference to the collection `collectionRef` is available, and is of type `&Collection`. i.e. code doesn't own the value, but has only a reference to the value. @@ -100,7 +102,7 @@ var id: String = collectionRef.id It is also important to note that, the returned reference has no entitlements assigned to them. i.e: they are **non-**`auth` references. -### Example +#### Nested resources Assume a nested resource collection as bellow. @@ -142,6 +144,58 @@ non-auth reference, and calling `withdraw` is prohibited because that reference masterCollectionRef.kittyCollection.withdraw(24) // Static Error ``` +### Struct Example: + +Similarly, consider the below `NFTView` struct which has two fields: one (`id`) is UInt64-typed, +and the other (`royalties`) is array-typed. + +```cadence +pub struct NFTView { + + // Primitive-typed field + pub var id: UInt64 + + // Array typed field + pub var royalties: [Royalty] +} +``` + +#### Case I: Code owns the container value + +Assume `nftView` is of type `NFTView`. i.e. the code owns the value. +Then, there is no change to the current behavior. + +```cadence +var nftView: NFTView = ... + +// `nftView.royalties` would return the concrete value, which is of type `[Royalty]`. +// This is same as existing semantics. +var royalties: [Royalty] = nftView.royalties + +// `nftView.id` would return the concrete `UInt64` value. +// This is same as existing semantics. +var id: UInt64 = collection.id +``` + +#### Case II: Code only has a reference to the container value + +Assume a reference to the nftView `nftViewRef` is available, and is of type `&NFTView`. +i.e. code doesn't own the value, but has only a reference to the value. + +```cadence +var nftViewRef: &Collection = ... + +// `nftViewRef.royalties` would now return a reference of type `&[Royalty]`. +var royaltiesRef: &[Royalty] = nftViewRef.royalties + +// However, `nftViewRef.id` would return the value, since it is a primitive type. +// This is same as existing semantics. +var id: String = nftViewRef.id +``` + +Similar to resources, the returned reference has no entitlements assigned to them. +i.e: they are **non-**`auth` references. + ### Drawbacks None From 883764e0335c8cb89e1b10de6613d342357a310e Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 5 Jun 2023 10:49:23 -0700 Subject: [PATCH 5/6] Add reference to the 'vision' --- cadence/20230517-member-access-semnatics.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cadence/20230517-member-access-semnatics.md b/cadence/20230517-member-access-semnatics.md index b3212106..8864c3de 100644 --- a/cadence/20230517-member-access-semnatics.md +++ b/cadence/20230517-member-access-semnatics.md @@ -32,6 +32,9 @@ This proposal is aimed at moving one step closer to solving this problem by retu so that [Entitlements](https://github.com/onflow/flips/pull/54) can be used to control who can perform mutating operations via references. +The [Mutability Restrictions Vision](https://github.com/onflow/flips/pull/97) explains how this proposed change +contributes to solving the aforementioned problems, and how the final solution looks like, with examples. + ## User Benefit As mentioned in the previous section, this change enables using Entitlements to prevent accidental mutations via references. From b2f3996451c3ff8caa0b93f3137ff3d014c53d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 5 Jul 2023 13:50:30 -0700 Subject: [PATCH 6/6] accept --- cadence/20230517-member-access-semnatics.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cadence/20230517-member-access-semnatics.md b/cadence/20230517-member-access-semnatics.md index 8864c3de..5bce98b3 100644 --- a/cadence/20230517-member-access-semnatics.md +++ b/cadence/20230517-member-access-semnatics.md @@ -1,9 +1,9 @@ --- -status: proposed +status: accepted flip: 89 authors: Supun Setunga (supun.setunga@dapperlabs.com) sponsor: Supun Setunga (supun.setunga@dapperlabs.com) -updated: 2023-05-17 +updated: 2023-07-05 --- # FLIP 89: Change Member Access Semantics