onflow / flips

Flow Improvement Proposals
25 stars 22 forks source link

FLIP 95: proposal for entitlements migration #95

Closed dsainati1 closed 10 months ago

dsainati1 commented 1 year ago

This adds a "FLIP" proposing a migration scheme for entitlements to enable the rollout of this feature with the release of Stable Cadence. Note that this is not strictly speaking a protocol or language change, just a proposal for how we will handle the rollout of the feature.

austinkline commented 1 year ago

One question came up in some Hybrid Custody discussions. What happens to dictionaries which are key'd by type when entitlements launch? For some types, that will not matter, but HC has a dictionary key'd on Capability<T> which means entitlements could mess with them quite a bit

dsainati1 commented 1 year ago

Hmm, this is an interesting question. Naively I'd say we would just migrate the type of the key of the dictionary according to this scheme, but the important point to note there is that because entitlements have no runtime counterpart (i.e they only exist statically), this would mean that the keys in the dictionary would only ever have the same entitlements as the key type. This may mean some of the keys inside the dictionary become less powerful than they were before. I don't really see any way around this though; it is not really possible to create a dictionary like this, where the keys within have more entitlements than the general key type.

austinkline commented 1 year ago

I don't really see any way around this though; it is not really possible to create a dictionary like this, where the keys within have more entitlements than the general key type.

Can we drill into this more? It's actually pretty important to have a feature like this because it's what allows HybridCustody to share explicit capabilities, and it's definitely in the realm of possibility that you would want to share two capabilities to the same resource for different purposes. Not only that, but keying on the type is what allows the consumer to actually know what permissions they expect to get. For instance, I might ask to borrow Capability<&NFTStorefrontV2.Storefront{NFTStorefrontV2.StorefrontManager}>, which would only come back to me if that capability is actually accessible. If we remove runtime types, it sounds like we lose that guarantee

Taking this away seems like a pretty big change, why can't entitlements have a runtime counterpart? And does that mean Type<Capability<T>>() is basically not usable?

bluesign commented 1 year ago

but the important point to note there is that because entitlements have no runtime counterpart (i.e they only exist statically)

Ofc capability can hash with entitlements, no? As it is currently hashing with borrowType, even it is not exposed?

dsainati1 commented 1 year ago

Sure. What I mean when I say that there isn't a runtime component to entitlements is this: if I have a program like so:

let x = 3
let y = x as AnyStruct
return y as! Int

This will successfully return an Int value; the value stored in x is upcast statically to an AnyStruct, but retains its Int type at runtime, and can thus be cast back down. The same would be true if you stored x in an AnyStruct array or dictionary, you would be able to regain the Int type later with a cast.

This is not the case with entitlements:

let x: auth(A, B) &T = ...
let y = x as auth(A) &T
return y as!  auth(A, B) &T

This would error at runtime, as upcasting x to a smaller set of entitlements effectively removes the entitlement it has at runtime, and this cannot be regained via downcasting. We did this for security reasons in order to ensure that it was always the case that people could see at a glance (by looking directly at source code) exactly what permissions a value has, and they would not have to worry that a value might have more permissions at runtime than it appears to statically. By making entitlements a purely static construct, we can guarantee to programmers that values can only ever do exactly what they say they do in the source program.

(Side note: this is somewhat of an oversimplification: of course you can save a Capability value with entitlements to storage, and then reload it with those same entitlements. This requires some form of runtime typing, but it is very limited to just support this particular use case. For this reason it might be more accurate to say that entitlements do have runtime types, but they are restricted such that they always exactly match the value's static type).

What this all means is that if I have a dictionary that is typed as {Capability<&T>: V} where V and T are some types, if you were to store a value of Capability<auth(X) &T> as one of the keys of this dictionary, this entitlement would essentially be erased by being stored in the dictionary, and cannot be regained. This is intentional, we want people to be able to look at the type of a dictionary and be able to reason locally and precisely about what permissions its values have. This is a case where we have chosen to trade expressivity for security. You can read more about this decision here: https://github.com/onflow/flips/pull/54#issuecomment-1522423184 (and in subsequent comments).

That said, I agree it is still valuable to be able to have multiple capabilities on the same resource that have different permission sets. Would it be possible to distinct dictionaries for each capability "type" that can be granted? I want to be clear that the behavior of Capabilities with regard to their borrow types are the same, you can only borrow a Capability with a type if that Capability is actually accessible with that type.

bluesign commented 1 year ago

What this all means is that if I have a dictionary that is typed as {Capability<&T>: V} where V and T are some types, if you were to store a value of Capability<auth(X) &T> as one of the keys of this dictionary, this entitlement would essentially be erased by being stored in the dictionary, and cannot be regained.

Ok but if my dictionary was {Capability: V} there is no problem right?

dsainati1 commented 1 year ago

Ok but if my dictionary was {Capability: V} there is no problem right?

cc @turbolent I believe we were planning to remove the untyped Capability right?

austinkline commented 1 year ago

cc @turbolent I believe we were planning to remove the untyped Capability right?

If you do this you would break Hybrid Custody entirely, do not remove Capability

austinkline commented 1 year ago

Would it be possible to distinct dictionaries for each capability "type" that can be granted?

I'm not sure I follow on this question, but I don't think that's possible, no. But here is the contract which manages this: https://github.com/Flowtyio/restricted-child-account/blob/main/contracts/CapabilityProxy.cdc#L46

Basically, you add a capability to the CapabilityProxy.Proxy resource and it gets stored in one of these two dictionaries:

access(self) let privateCapabilities: {Type: Capability}
access(self) let publicCapabilities: {Type: Capability}

The type is the type of the Capability itself (Type<Capability<T>>())

I want to be clear that the behavior of Capabilities with regard to their borrow types are the same, you can only borrow a Capability with a type if that Capability is actually accessible with that type.

What would the borrow type be in this case? Does this mean we just need to rework the key to be the borrowed type instead?

dsainati1 commented 1 year ago

I believe that for generic Capabilities (i.e. those without type arguments like in this example), this should actually work. There is no static entitlement present to conform the runtime type to, so in this case the runtime type would just stay the same as whatever it used to be.

austinkline commented 1 year ago

I believe that for generic Capabilities (i.e. those without type arguments like in this example), this should actually work. There is no static entitlement present to conform the runtime type to, so in this case the runtime type would just stay the same as whatever it used to be.

But under the hood it is a typed capability, it's just that the thing storing it can store any capability which is why it isn't typed on the dictionary itself. At the end of the day, we will need some way to go from Capability<&T{...}> to something that can be used as a key in a dictionary that is unique to the actual conformance expressed by the capability itself. It can be the borrowed type as well, but currently if you call borrow and then getType, you get the actual concrete type, I believe. Because of that, the borrowed type won't actually help us either

Am I missing something here?

dsainati1 commented 1 year ago

getType will include the entitlements present on the runtime value, the behavior is just that those entitlements will be the same as those present on the static type. I think the "weird" behavior is that because Capability (as a static type) doesn't specify a set of entitlements, it doesn't coerce its runtime entitlements to match.

austinkline commented 1 year ago

getType will include the entitlements present on the runtime value, the behavior is just that those entitlements will be the same as those present on the static type. I think the "weird" behavior is that because Capability (as a static type) doesn't specify a set of entitlements, it doesn't coerce its runtime entitlements to match.

So this behavior is also changing, then?

The following always panics:

import FungibleToken from 0x9a0766d93b6608b7

// This is the most basic script you can execute on Flow Network
pub fun main(addr: Address):String {
  let t = getAccount(addr).getCapability<&{FungibleToken.Balance}>(/public/flowTokenBalance).borrow()!.getType()

  assert(t == Type<&{FungibleToken.Balance}>(), message: "types are not equal")

  return "done"
}

calling getType as of now would return the actual underlying type which is:

import FungibleToken from 0x9a0766d93b6608b7

// This is the most basic script you can execute on Flow Network
pub fun main(addr: Address):String {
  let t = getAccount(addr).getCapability<&{FungibleToken.Balance}>(/public/flowTokenBalance).borrow()!.getType()
  return t.identifier
}

// return A.7e60df042a9c0868.FlowToken.Vault

A lot of things are built on this assumption, I didn't realize entitlements were set to redefine not just access control but how we ask for and evaluate types as well, I think we need a call about this, it sounds bigger than I thought.

dsainati1 commented 1 year ago

Sorry, I think I may have miscommunicated this, the behavior we are going to have is this:

let cap: Capability<auth(X) &T> = ...
let runtimeType = cap.getType() // runtimeType is Type<Capability<auth(X) &T>>
let upcastCap = cap as Capability<&T> 
let upcastRuntimeType = upcastCap.getType() ) // runtimeType is Type<Capability<&T>>
return runtimeType == upcastRuntimeType // false

This "change" (it's not really a change since this didn't exist previously) is only for looking at which entitlements are present on the type. If the borrow type is upcast, getType will still produce the actual runtime type that is present, so both of your examples above would still behave exactly the same way. It is only the set of entitlements present on the reference/capability type that is static.

austinkline commented 1 year ago

Up-casted types are totally fine to stay their ambiguous types (I think), I guess my confusion is how we ensure migration can happen correctly such that we still accurately keep these types we key on since they will be a big part of hybrid custody. I want to be as clear as possible here since there seems to be confusion, let's say I do the following:

let cap = ...
let dict: {Type: Capability} = {}
let t = Type<Capability<...>> // note that this type will match the type of cap!
dict[t] = cap

When I try to obtain that capability again, would I be able to reconstruct t and get the capability back? Or would I be losing that granularity? And does that change at all if I put a function on-top of it like so:

let dict: {Type: Capability} = {}

pub fun addCap(_ cap: Capability) {
  dict[cap.getType()] = cap
}

let cap = ...
addCap(cap)
dsainati1 commented 1 year ago

Because you’re using the generic Capability type these examples should both be fine I believe. If the dictionary type used a non-generic Capability then this would not work when the capabilities are entitled.

bluesign commented 1 year ago

I think gist of this thread is we need to guarantee with contract update logic.

If you were storing let dict: {Type: Capability<&T>} it should only allow to change it to let dict: {Type: Capability<auth(X, Y, Z ...) &T>}, or better some shorthand reserved entitlement name like: let dict: {Type: Capability<auth(all) &T>}

as our old &T now maps to auth(all) &T

austinkline commented 1 year ago

Thanks for the details and clarification @dsainati1!

Because you’re using the generic Capability type these examples should both be fine I believe. If the dictionary type used a non-generic Capability then this would not work when the capabilities are entitled.

I think the last piece here, then, is how we can ensure that the type stored as a key in a dictionary like this one is the same as the type you can actually get to. It sounds like this is fine, but I just want to confirm that things will work or that a migration guaranteeing this kind of outcome is possible. So if I have the following:

let caps: {Type: Capability} = {}

// The interface NFTStorefrontV2.Remover doesn't exist, it's just an example
let cap = acct.getCapability<&NFTStorefrontV2.Storefront{NFTStorefrontV2.Remover}>(...)
caps[cap.getType()] = cap

and we migrate to entitlements, would something like this be able to read the dictionary back? Assume I make an equivalent entitlement called "remover"

let type = Type<Capability<auth(remover) &NFTStorefrontV2.Storefront>>()
let cap = caps[type]! as! Capability<auth(remover) &NFTStorefrontV2.Storefront>

assert(cap.check(), ...)
dsainati1 commented 1 year ago

Ah I see what you are saying. Yeah those would be two separate runtime types. The one stored there pre-Stable Cadence would have a &NFTStorefrontV2.Storefront{NFTStorefrontV2.Remover} type and the one created in your post-entitlements example would have a auth(remover) &NFTStorefrontV2.Storefront runtime type.

@turbolent would it be feasible to also migrate existing reference and capability runtime types as part of this migration to handle this case?

j1010001 commented 10 months ago

This flip does not require a review and approval by wider community, as it makes no impact on the Cadence developers, it only impacts the migration process of entitlements during Flow network upgrade to Cadence 1.0. The Flip was created for informing the community and documenting the migration proposal in public.