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

Get the run-time type of an object #195

Closed
kharlov-art opened this issue Jul 2, 2020 · 14 comments · Fixed by #493
Closed

Get the run-time type of an object #195

kharlov-art opened this issue Jul 2, 2020 · 14 comments · Fixed by #493

Comments

@kharlov-art
Copy link

Issue To Be Solved

Using interfaces we can't check what type is really used at run-time, sometimes it's required to check a specific type. Need to know the full name of a type I'm working with.

(Optional): Suggest A Solution

I know you are working to add a new type Type, so maybe you can add something like fun getType(): Type and also a fun toString(): String to the Type if it doesn't exist now.
I'd expect to call it in the following way: obj.getType().toString().
The format of a name of a type can be for instance: T.{account_address}.{contract_name}.{type_name}

@turbolent
Copy link
Member

Thanks for the feature request @kharlov-art!

I think adding fun getType(): Type makes a lot of sense. The only issue is the actual implementation of it, as Type currently is a static type and getType should return the dynamic type of the value.

Type already has a field let identifier: String which is the fully qualified identifier of the type, similar to your format example (the only difference is that there is no T. prefix)

@turbolent
Copy link
Member

For most values it is pretty straightforward to return their static type.

For containers (array, dictionary, optional values, etc.), we currently don’t store the static element type, so I can't see how we could implement this in a backwards-compatible way for those types.

@turbolent turbolent self-assigned this Nov 12, 2020
@turbolent turbolent removed the Good First Issue Good for newcomers label Nov 12, 2020
@rheaplex
Copy link
Contributor

I think we only need this for resources to satisfy the immediate use cases. So this could be a function on AnyResource.

If we force unwrap an optional to get a resource can we get the static Type of the resulting resource in the same straightforward way?

@turbolent
Copy link
Member

Right, we could just start with composite types for now (resources, structs, etc.) and of course the other primitive values (number types, string, bool, path, etc.).

We couldn't have the type function/field on AnyResource/AnyStruct though, because also container types (arrays, dictionaries, optionals) are subtypes of those types.

For optionals, you wouldn't want to force-unwrap, because there could be no value, but yes, you could get the type of the optional's value if there is one.

@dete
Copy link

dete commented Nov 12, 2020

I would suggest that we can make it a run-time error to query the type of a container in the first implementation, or just have built-in Type values representing "array" and "dictionary", with no way to query the type information about the contents of the array (yet!). That's obviously not a great solution for the long term, but the pressing requirements for getType() are mostly about querying NFT and FT implementations to figure out what kind of token we're talking to.

Most critically, I think we need the ability to query a Vault to know which fungible token we've been handed, especially in the FT DEX or NFT Marketplace contexts. Additionally, lots of off-chain NFT code will want the ability to use scripts to get a string version of an NFT's type, so it can show users the detail of NFTs that are stored in contexts where the concrete type isn't available statically.

@rheaplex
Copy link
Contributor

A more future-proof way to handle the array/dictionary cases might be to add the ability to check what kind of resource/struct something is. So Type.kind or Type.isArray(), Type.isResource() etc. That would be somewhat related to #442 .

But we don't need that right now so I think an error might be best, as that makes sure nobody relies on something we will change later.

@rheaplex
Copy link
Contributor

A quick sketch of how this would help with an FT DEX:


pub fun registerSwapPair(
    xVault: @{FungibleToken.Vault},
    yToken: @{FungibleToken.Vault}
) {
    self.xVaultType = xVault.getType()
    self.yVaultType = yVault.getType()

    self.xVault <- xVault
    self.yVault <- yVault
}

pub fun swapXForY(xVault: @{FungibleToken.Vault}) @FungibleToken.Vault {
    pre {
        (!xVault.isInstance(self.xVaultType)): "Vault is not correct FT type"
    }

    let yVaultWithdrawal <- self.yVault.withdraw(amount: xVault.balance)
    self.xVault.deposit(from: xVault)
    return <-yVaultWithdrawal
}

@turbolent
Copy link
Member

turbolent commented Dec 3, 2020

@dete @robmyers What do you think about making this function return an optional, i.e. have it be fun getType(): Type? ?

We would start storing the static type for arrays, dictionaries, and optionals, for new values, that we could return the type for those values; and for backwards-compatiblity, we would just return nil.

@turbolent
Copy link
Member

turbolent commented Dec 3, 2020

Some notes:

  • We need to change the storage format: Arrays are currently stored as CBOR arrays, we need to switch that to a CBOR map with a field which contains the static type
  • We currently represent both constant sized arrays and variable sized arrays using the same run-time value. When an array literal instantiates an array value, we'd use the variable-sized static type, but need to change it to the constant-sized static type when copying/boxing in the interpreter.

@dete
Copy link

dete commented Dec 7, 2020

I'm curious what a "Type" is. I assume it's a value type, not a resource. Are there any limits on creating a Type object? Could I create a Type value that doesn't reference any actual types defined in the system?

If it's not too difficult, I would be inclined to return a "sentinel" Type value that is inert for now, but will have additional functionality in the future. So, maybe calling getType on an array/dictionary/optional returns the "Internal Type" type, that today is just an opaque empty thing that is no more useful that the nil value you propose above. Later, of course, it will have methods that let you ask it things like "Are you an Optional? If so, what your unwrapped type?" The advantage is that we don't have to force people to deal with unwrapping today, and we don't have to break a bunch of code when getType stops returning an optional.

Of course, if that's too much effort, I do think returning an optional is totally okay for now.

@turbolent
Copy link
Member

I'm curious what a "Type" is. I assume it's a value type, not a resource.

Correct, it's a value type, not a resource.

Are there any limits on creating a Type object? Could I create a Type value that doesn't reference any actual types defined in the system?

Types can only be acquired for static types, so it's not possible to create a type that is not declared somewhere.

I would be inclined to return a "sentinel" Type value that is inert for now, [...]

That's a great idea! I like the idea of not having to introduce the optional / bug developers with handling the "unknown" case. 👍

@rheaplex
Copy link
Contributor

rheaplex commented Dec 7, 2020

it's not possible to create a type that is not declared somewhere

Just thinking this through, this would mean that we cannot construct a Type with an SDK then pass it in to Cadence as a transaction/script argument - the entity that we wish to fetch the Type of must be accessible within the body of the code.

Given this I am wondering about types in events: do they represent useful information, or are they likely to mislead developers who will want to capture them off-chain and then feed them back in to transactions?

That's a great idea! I like the idea of not having to introduce the optional / bug developers with handling the "unknown" case.

Yes I agree with this.

@rheaplex
Copy link
Contributor

rheaplex commented Dec 7, 2020

We need to change the storage format: Arrays are currently stored as CBOR arrays, we need to switch that to a CBOR map with a field which contains the static type

Does this require changing data currently stored by nodes on the live network?

We currently represent both constant sized arrays and variable sized arrays using the same run-time value. When an array literal instantiates an array value, we'd use the variable-sized static type, but need to change it to the constant-sized static type when copying/boxing in the interpreter.

Does this introduce much run-time overhead?

@turbolent
Copy link
Member

it's not possible to create a type that is not declared somewhere

Just thinking this through, this would mean that we cannot construct a Type with an SDK then pass it in to Cadence as a transaction/script argument - the entity that we wish to fetch the Type of must be accessible within the body of the code.

Given this I am wondering about types in events: do they represent useful information, or are they likely to mislead developers who will want to capture them off-chain and then feed them back in to transactions?

Good point, but what I meant was not that we should prohibit importing types in general, just not ones that don't exist / "are not declared in the system".

I looked at the current code and saw that we do not support this yet. I opened #491 to implement it.

Given this I am wondering about types in events: do they represent useful information, or are they likely to mislead developers who will want to capture them off-chain and then feed them back in to transactions?

Good point! I think this is a great idea and would be very useful. Again, I checked the code and saw that we currently don't allow types to be used in events (event parameters cannot have type Type at the moment). I opened #492 to implement it.

We need to change the storage format: Arrays are currently stored as CBOR arrays, we need to switch that to a CBOR map with a field which contains the static type

Does this require changing data currently stored by nodes on the live network?

The problem is that we cannot add type information to existing data stored on networks, we can only add it to new values.

We currently represent both constant sized arrays and variable sized arrays using the same run-time value. When an array literal instantiates an array value, we'd use the variable-sized static type, but need to change it to the constant-sized static type when copying/boxing in the interpreter.

Does this introduce much run-time overhead?

Not much I think. We already copy/box these values, we'd need to look up the type information from the checker's elaboration, which will likely be just a hashmap lookup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants