-
Notifications
You must be signed in to change notification settings - Fork 96
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
feat: Allow deserialization of candid values with unknown types #555
Conversation
Open questions / things I'm unsure about:
@nomeata : For some reason, I cannot add you as a reviewer. That's why I'm pinging you this way. |
I’m not a member of this repo, so you can’t add me as a reviewer. But pinging works. My opinion is that the HTTP Gateway Spec is flawed by trying to use “generic Candid” in this ad-hoc and unsupported way. It is actually impossible to implement this correctly in in full generality, in the presence of “future types” as pointed out elsewhere. But as long as the “service specific type” is “simple enough” it may be possible… so with this discouraging rant out of the way, let’s actually answer your questions :-)
Doesn’t that mean it can’t be used for the token, because you need to serialize that again? Or am I misreading that sentence (I must, because you later ask about encoding). So I am a bit confused. |
The idea is to use Take a look at the unit test, they should illustrate how to do this. |
Ah, I see. Values of this type can be used for serialization ( |
Yes, exactly. |
Yeah, then it indeed looks like a reasonable best effort approach. |
Generally LGTM. My sentiment is the same as @nomeata that this can only be a best effort support from Candid side, as this is not properly spec'ed.
As it's not in the spec, you are free to choose the representation.
Please do add a comment that this is for internal use only. Once we have proper support for generics, we want to migrate away from |
writable: true, | ||
enumerable: false, | ||
configurable: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these properties used?
Technically it should be possible to represent a value of And you still have to decose the type table, of course, that'd be the same |
I like this approach better, because we can then still do the subtype check when we know the concrete expected type. Implementation-wise, the current approach is probably easier. |
Description
This change introduces
IDL.Unknown
to the candid library to specify that the type of the value is not known.IDL.Unknown
can be use for deserialisation only.The deserialisation of
IDL.Unknown
will be based on the type table included within the candid encoded value. This means that all field names will only be available in hashed form. The decoded value has a functiontype()
that returns it's actual candid type.How Has This Been Tested?
Unit tests have been added.
Checklist: