cadence icon indicating copy to clipboard operation
cadence copied to clipboard

Add `borrow!/borrow? ... [as ...]` syntax

Open turbolent opened this issue 5 years ago β€’ 6 comments

Context

It was suggested that in addition/instead of borrow<T>() function calls, the dedicated syntax borrow!/borrow? ... [as ...] might be more readable and explicit.

Definition of Done

  • [ ] Parser: Add rule
  • [ ] Sema: Check
  • [ ] Interpreter: Interpret like borrow call
  • [ ] Tests
  • [ ] Documentation

turbolent avatar Jun 18 '20 22:06 turbolent

From the issue it appears we would like to implement the following syntax to borrow a reference to an object in storage:

borrow!/borrow? <path> [as <type>]

The above seems to imply that the downcast to the <type> is optional. Also note. we currently only have a as? downcast operator, which returns an optional -> downcast variable or nil (if the type could not be cast).

My understanding is that borrow! would force-unwrap the object reference from storage (if not nil, panic otherwise).

Thinking through this, I think it makes sense to have borrow? be just borrow, which returns an optional:

// a is &Any?
let a = borrow /public/a

and the force-unwrapped version:

// a is &Any
let a = borrow! /public/a

Mainly because otherwise, borrow? ssets a precedent of having <fn>? indicate an optional is returned β€” which we don’t really do anywhere else as far as I know.

Now if we want to support the above however, speaking with Bastian, it seems unlikely anyone would want just an &Any and will likely always downcast to a specific type. So perhaps we have the borrow expression always in the form of:

// a is &String
let a = borrow! /public/a as &String

Or perhaps we always force-unwrap the object reference, by definition, and just have the following which is semantically the same:

// a is &String
let a = borrow /public/a as &String

Would love feedback! Trying to determine a rational syntax that is internally consistent with the existing as? operator also if possible.

benjaminkvm avatar Jul 06 '20 21:07 benjaminkvm

Going to put this on pause for a bit as it is a lower prority and Dete is away next week -- would love to get his input on this.

benjaminkvm avatar Jul 07 '20 20:07 benjaminkvm

Assigning this to Tim to re-assign out.

sifmoon avatar Aug 25 '20 01:08 sifmoon

As you're updating borrow could a way to getCapability on multiple paths be added?

To avoid doing this:

    var collectionRef = getAccount(address).getCapability(/public/CollectionOne)!
        .borrow<&{Contract.Collection}>()
        
    if collectionRef == nil {
        collectionRef = getAccount(address).getCapability(/public/CollectionTwo)!
        .borrow<&{Contract.Collection}>()
    }
    
    if collectionRef == nil {
        collectionRef = getAccount(address).getCapability(/public/CollectionThree)!
        .borrow<&{Contract.Collection}>()
    }

We could do this:

  var collectionRef = getAccount(address).getCapability(/public/*)!
        .borrow<&{Contract.Collection}>()

This might be linked to #208

cybercent avatar Sep 30 '20 13:09 cybercent

Capabilities are a security measure so specifying which one is to be used seems like something that should be explicit. And a wildcard should return all matches, not just the first one.

That said, combining Storage Querying API #208 with Typed Paths #369 would allow the program to find if any capabilities of a given type exist under a given path and to use just the first one if desired. I'm wary of recommending a pattern rather than a feature but I think making this explicit in this way might make the program's intent clearer.

rheaplex avatar Sep 30 '20 18:09 rheaplex

Hey @robmyers, sorry for not making this more clear. I meant /public/* to return all matches. In my use-case, I'm looking for a way to fetch info about all NFTs no matter the path.

I expect Dapps to move NFTs around as they see fit and not necessarily put them back to the path from where they took them from.

Example:

  1. As a user I list my NFT for sale on a marketplace, my NFT is moved and instead of /public/kittiesCollection I now need to get info about the NFT from /public/kittiesForSaleCollection.

  2. I delist my sale, the marketplace might put the kitty in another place and now I need to fetch the info from /public/marketplaceKittiesCollection instead of /public/kittiesCollection.

I agree with making things explicit, but from another angle, /public can be used in a script, so there might be no security implications in this case.

To make an analogy, imagine on your Mac you need to search for a PDF file, you don't remember the name of it, so it's just *.pdf and instead of opening Spotlight to search, you would need to go folder by folder and manually look if you have any PDF files in each and every folder of your Mac. This was done like this so that the user can't edit the PDF, but in reality the user only wanted to read it and never intended to modify it.

cybercent avatar Sep 30 '20 23:09 cybercent