onflow / flips

Flow Improvement Proposals
25 stars 22 forks source link

Account type #92

Closed turbolent closed 1 year ago

turbolent commented 1 year ago

The goal of this proposal is to improve how Cadence provides access to accounts.

The proposed API aims to make account access simpler and safer, by enabling and encouraging the principle of least authority (PoLA) for account access.

This FLIP is based on previous discussion in https://forum.onflow.org/t/super-user-account/4088

bjartek commented 1 year ago

What type will an account we in a prepare statement have?

turbolent commented 1 year ago

@bjartek As described in the "Expose Account Access Through References" section, account values will be only exposed through references, i.e. &Account. The Examples section shows an example.

austinkline commented 1 year ago

Thanks for putting this together @turbolent! My main concern as of now is how an AuthAccount stored in a struct/resource would be migrated. For instance, hybrid custody will need to store an AuthAccount capability in a resource which helps manage access for a parent to get to a child account. Since we're having to add this to a type definition, current type restrictions on contract upgrades would prevent us from migrating to this a new set of interfaces

Any ideas how we can get around that issue?

turbolent commented 1 year ago

@austinkline I clarified in the last commit, 002924e, how a &AuthAccount static type (e.g. used in a Capability), would be migrated.

austinkline commented 1 year ago

Adding another thought as I work through some other hybrid custody related items, what happens if someone has already named an interface or struct of theirs Account in an existing contract?

turbolent commented 1 year ago

@austinkline

What happens if someone has already named an interface or struct of theirs Account in an existing contract

That would lead to an type-checking error where the Account type is declared.

A quick cursory search in https://github.com/bluesign/flow-mainnet-contracts shows that there are currently no such types on Mainnet (yet).

austinkline commented 1 year ago

@austinkline

What happens if someone has already named an interface or struct of theirs Account in an existing contract

That would lead to an type-checking error where the Account type is declared.

A quick cursory search in https://github.com/bluesign/flow-mainnet-contracts shows that there are currently no such types on Mainnet (yet).

Got it, checking for what's out there was my next question. Sounds like only HybridCustody is an issue (we have an Account type in testnet). I'll go ahead and change it tomorrow just to be safe.

bjartek commented 1 year ago

I think the change is good, but i fear the cost of migration here aswell. Is it even possible to do in all cases if we cannot change the type of a field?

turbolent commented 1 year ago

I just realized that we'll also need to rethink getAuthAccount.

Maybe add a type parameter, so any entitled &Account reference can be "summoned"? For example, getAuthAccount<auth(Storage) &Account>(0x1).

We could also "merge" getAuthAccount into getAccount, and have a different type parameter depending on environment, e.g. "hardcode" to &Account in transactions and allow any &Account subtype in scripts.

turbolent commented 1 year ago

@bjartek we'll likely need to somehow allow code to be upgraded in ways that the checker currently rejects, but are actually safe, e.g. here, the name just changes, the underlying value will still be the same (and is actually not storable)

turbolent commented 1 year ago

BTW, there has been an ongoing discussion about naming of entitlements in another FLIP introducing built-in entitlements: https://github.com/onflow/flips/pull/86#discussion_r1192703132

turbolent commented 1 year ago

Some issues I ran into while starting the implementation of this FLIP:

dsainati1 commented 1 year ago

All contracts have a a predeclared field account, which currently has the type AuthAccount. How should we replace this?

IMO it makes the most sense to just add the fully entitled type here auth(Storage, Contracts, Keys, Inbox, Capabilities) &Account. It's verbose, but users will rarely have to write the full type, and I don't think we should add anything new to the language just for this case.

The fine-grained BorrowValue entitlement would probably also suffice, so maybe the entitlement set should be BorrowValue | Storage?

This makes sense to me.

turbolent commented 1 year ago

Approved in the Cadence Language Design Meeting 🎉