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

marshal method names are misleading #1248

Closed
Tracked by #6646
gibson042 opened this issue Aug 9, 2022 · 6 comments · Fixed by #1402
Closed
Tracked by #6646

marshal method names are misleading #1248

gibson042 opened this issue Aug 9, 2022 · 6 comments · Fixed by #1402
Labels

Comments

@gibson042
Copy link
Contributor

gibson042 commented Aug 9, 2022

This ship has probably already sailed, but the names of serialize and unserialize are misleading—they don't actually produce/consume a serialized representation, but rather a CapData structure that in practice is often serialized/deserialized (for real) by means of JSON.{stringify,parse}. They might fairly be called marshal and unmarshal (cf. https://en.wikipedia.org/wiki/Marshalling_(computer_science) , especially its first reference), but something like toCapData and fromCapData would be even more clear.

@turadg
Copy link
Member

turadg commented Aug 16, 2022

I've been mislead in this way. =) I think solving this is important for Mainnet 2.

@turadg
Copy link
Member

turadg commented Aug 16, 2022

I also find myself wanting to express the shape of what's coming out of the CapData. We could solve both ergonomics problems at once with a wrapper that uses @erights 's pattern validation work. E.g.

const capData = JSON.parse(capDataStr);
const shapedData = capPresence(marshaller).from(capData, specShape);

@erights
Copy link
Contributor

erights commented Dec 8, 2022

const capData = JSON.parse(capDataStr);
const shapedData = capPresence(marshaller).from(capData, specShape);

I don't understand. What is this code trying to say? What is it an example of?

@gibson042
Copy link
Contributor Author

gibson042 commented Dec 8, 2022

JSON.parse(capDataStr) is deserialization (it converts an input sequence of UTF-16 code units to an output tree-shaped data structure, for CapData in particular one that is expected to be an object with a string-valued "body" property containing valid JSON text and an array-valued "slots" property), and from(capData, specShape) could fairly be called unmarshalling or decoding (it converts an input {body: string, slots: any[]} CapData structure to a data structure that potentially includes promises, remotables, and/or objects with multiple inbound references).

And the composite process from capDataStr to shapedData is both—the important detail of deserialization is that its input is fully (and almost always primitively) serial, and the important detail of unmarshalling is that its output is the desired typed/shaped/etc. representation.

@turadg
Copy link
Member

turadg commented Dec 8, 2022

What is it an example of?

Consider https://github.com/Agoric/agoric-sdk/blob/d042efce684176b68c4d8055564250c7fabf304c/packages/smart-wallet/src/smartWallet.js#L460-L463

That doesn't validate the shape before casting it. It could with a helper that does the unmarshallng (à la fromCapData née unserialize) that also checks the shape before returning it. And then it wouldn't have to be cast.

In the comment above, capPresence is that utility that takes a remote marshaller to do it.

@erights
Copy link
Contributor

erights commented Dec 11, 2022

I actually still don't understand what the code is trying to say. But I did understand all of #1402 and have approved it. Given that, do I still need to understand the code above? If so, lets schedule a conversation. Thanks.

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

Successfully merging a pull request may close this issue.

3 participants