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

FLIP: standardize initialization of contract #748

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bjartek
Copy link
Contributor

@bjartek bjartek commented Jan 10, 2022

What happends when a user in BloctoApp clicks "create new versus collection"? As the creator of the versus contract, I do not know.

In the current state of cadence I see a trend where initializing the storage/link a solution/contracts requires is becomming more and more decentralized. The process of doing this is not standardized in any way and the owner of a contrat have no way of knowing if another product initializes the storages correctly.

This FLIP aims to suggest a solution where it would be possible for the owner of a contract to specify how it wants itself to be initialized in a user accounts. And also enable script access to this information.


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
Copy link

vercel bot commented Jan 10, 2022

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

A member of the Team first needs to authorize it.

@siddthesquid
Copy link

siddthesquid commented Feb 1, 2022

Is there a use case that paths enable that cannot be solved with nested resources? If each resource and resource interface was allocated exactly one unique path based on its unique identifier, then instead of asking "Is this account initialized according to a contract's definition of initialized?", we might be able to more generically ask "Does this account have this [Resource] (e.g. FungibleToken.Vault) or [Capability] (e.g. FungibleToken.Receiver)?". You would lose the ability to store multiple Vaults on the top-level, but, following this design, you would instead have a VaultHolder or something that stores multiple Vaults.

^ By the way, I am just thinking out loud here! I aim to learn the Flow codebase a bit more but right now I am not sure what is possible/feasible and in line with Flow's design patterns

@bjartek
Copy link
Contributor Author

bjartek commented Feb 1, 2022

Right now it is possible to link one resource to several paths in order to create private capabilities, I think removing that possibility is not the right way to go about this at all. Personally I have nothing against paths as an implementation detail and maybe even a powertool, but the common use cases should not require that you have to decide on a path or even care about it.

@siddthesquid
Copy link

Oh yes I definitely do not want to remove the ability to create private capabilities - which was why I was suggesting each resource and resource interface have its own qualified-identifier path. Capabilities are exposed through interfaces and an account shouldn't need more than one of the same interface exposed.

Though... As I was writing that, it seems like a possible use case for there to be multiple of the same capability in the same account if the account wanted to be able to revoke certain capabilities - in which case I agree with you

@bluesign
Copy link
Contributor

bluesign commented Feb 1, 2022

@ssingal05 actually we have parallel discussion about this (especially types to query storage) in onflow/cadence#208

I am mostly against usage of paths (as least in common usage).

For capabilities part: they are already fragile and lack basic needs (like revoking) we discussed that long ago on forum: https://forum.onflow.org/t/private-capability-revoke/1997

For private capabilities, I think it should work something like:

You have some 'storage reference'. Let's call it 'sr'. Which you can get by somehow querying your storage by type.

From this reference you can create as many as capabilities you want with 'sr.getCapability()' which will live as a reference also in 'sr.capabilities'. (Which you can revoke later)

Also you may want to publicly share the resource on storage, with some method like 'sr.setPublic(bool)' etc

If we came back into this topic, I think this needs an urgent fix. But also maybe in the mean time it is better if we consider sandboxing and isolating contracts storage a bit.

@siddthesquid
Copy link

@bluesign I liked the ideas you all had - if the capability ID model or something similar were implemented, then I'd stand by my previous suggestion of removing paths altogether. I think that if a Dapp developer wanted a path-like functionality, they can implement that type of functionality as part of the contract.

The storage query API could be useful as an alternative, but it feels easier to work with pre-defined paths based on the resource or resource interface

I was under the assumption (which I now realize is wrong) that capabilities were invalid once the resource was moved but it sounds like that capability is still active while its alive! Seems dangerous (if the resource moved back)...

@turbolent turbolent added the FLIP Flow Improvement Proposal label Feb 8, 2022
@austinkline
Copy link
Contributor

Adding my two cents on this, we have a big challenge with users for Flowty that when a lender on our platform funds a loan, we will need to ensure that the lender is configured appropriately to receive that loan's collateral in the event of a default. To that end, we're trying to just parameterize creation of the Collection resource but there are instances where this can be risky. Doing that wrong could mess up that wallet's ability to use other sites that rely on different things we didn't know about.

To us, the ideal would be having some kind of secure way to ask a contract to set an account up. For instance, if I'm importing TopShot, I'd like to be able to do something like:

import TopShot from 0x1

transaction() {
    prepare(acct: AuthAccount) {
        TopShot.setup(acct)
        
        // OR
        acct.setup(TopShot)   
    }
}

My main concern with a pattern like this is the issue of giving any contract the acct itself. So something would need to be able to ensure that the only thing a contract can setup for that account are the things pertaining to it. We'd love to help push this or some standardization like it for our platform to be more robust/not harm other platforms if we do an improper collection initialization, happy to help out where we can.

@austinkline
Copy link
Contributor

The main complexity we're dealing with right now basically breaks down to:

  1. What paths are we using for this collection
  2. What type(s) should we be linking to the public path such that a platform relying on that capability is not going to break. For instance, Gaia exposes more than just NonFungibleToken.CollectionPublic, it also exposes Gaia.CollectionPublic when you have Blocto set it up.

In @bjartek's example of not knowing how Blocto is setting up a wallet for Versus, flowty similarly runs the risk of not setting up a collection the right way such that Versus won't function as a platform properly for that wallet.

As a secondary, it feels like a risk that another competing platform to any existing project could basically take over the storage path that your contract uses and prevent a wallet from using you (or anything else). Giving contracts the ability to set themselves up in a way that only they can do sounds like a plus to me. Whether that's resources or some kind of path pattern segmented by the contract (/TopShot/public/X or /TopShot/storage/Y) could also be an option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cadence FLIP Flow Improvement Proposal S-Governance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants