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

mechanism to marshal immutable Set/Map #16

Closed
warner opened this issue Oct 25, 2019 · 5 comments
Closed

mechanism to marshal immutable Set/Map #16

warner opened this issue Oct 25, 2019 · 5 comments
Assignees
Labels
marshal package: marshal

Comments

@warner
Copy link
Member

warner commented Oct 25, 2019

We commonly use objects as maps, but of course this suffers from confusion between "own properties" and inherited ones (i.e. don't use a key named toString). In the current code, if you send a Map or a Set (after calling harden() on it, of course, which freezes the API but not the actual data), it will probably be recognized as a pass-by-presence object, and the remote end will wind up with a Presence on which they could call m~.get(key) or m~.set(key, value), but they wouldn't be able to do immediate/near/synchronous operations.

I think it'd be useful to have a way to pass-by-copy a frozen Map or Set, so the recipient gets a data structure that they can query locally. We should be able to do this without rendering the source object immutable, so the sender is really providing a snapshot of their Map to the far end. JavaScript doesn't currently have immutable collections (but we have a proposal to add them), but maybe marshal should provide a utility function that takes a regular mutable Set/Map and produce an object which will be recognized as pass-by-copy by the marshaller. On the receiving end, the deserializer creates a regular Map/Set, populates it with the inbound data, then wraps it with a facet that only provides read-only methods.

const m = new Map();
function method1(args) {
  ...
  return harden({ ret1: marshal.MapSnapshot(m)});
}

and:

const inbound_m = await remote~.method1().ret1;
console.log(inbound_m.get('key')); // ok
console.log(inbound_m.set('key')); // throws

I can immediately think of a couple of problems:

  • When exactly does the data get sampled? We might hope to reduce the number of copies that must temporarily exist, but if the MapSnapshot appears earlier than the very end of the function, other code might have a chance to modify the mutable Map before the serializer does it's thing, which would be confusing
  • Roundtrips: when the immutable Map snapshot that comes out of the deserializer is sent back to the original side, should it get the same MapSnapshot object? Or a different one that behaves the same way?

Obviously we want this to align with the Readonly Collections proposal, but I guess I'm wondering if we should have a mechanism that can be used without shimming both sides to understand the new types and snapshot()/diverge() methods which the proposal adds to the builtin collection types.

@warner warner transferred this issue from Agoric/marshal Dec 1, 2019
@warner warner added the marshal package: marshal label Dec 1, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
Not yet complete, but good enough to merge.

refs Agoric#16
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
Make Cosmic-Swingset use ERTP instead of copied Mint
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
…gration-test/lodash-4.17.14

build(deps): bump lodash from 4.17.11 to 4.17.14 in /integration-test
@katelynsills
Copy link
Contributor

👍 Just came across an example in the wallet and autoswap backend code in cosmic-swingset where this would be very helpful.

@katelynsills
Copy link
Contributor

Just came across another case in Zoe where a frozen Map would be incredibly useful. I'm adding "roles" to Zoe, which are user-defined unique identifiers for assays/units/payments. For instance, to create an auction, the user should define the terms, including the roles and assays. Maybe:

{ forSale: paintingAssay }, { bid: moolaAssay }]

But, we can't do that, because on the Zoe side, we would have to use computed property names in order to easily access the assays. Computed property names are notoriously unsafe.

So maybe instead we remove the user defined keys and use pre-determined ones:

[{role: 'forSale', assay: paintingAssay }, { role: 'bid', assay: moolaAssay }]

This is pretty lengthy and unwieldy.

Maybe instead what we want is to pass a frozen map. JavaScript maps can already be transformed to and from 2D Arrays. We could make helper functions to hydrate/dehydrate these 2D Arrays into and out of Map form.

But, it would be so much easier to do this in marshal.

@katelynsills
Copy link
Contributor

The wallet example was: It'd be nice to be able to supply a wallet with assays and their suggested petnames:

makeWallet(suggestedPetNamesToAssaysMap);

or:

bobWallet.addAssays(suggestedPetnamesToAssayMap);

@erights erights self-assigned this Apr 8, 2020
@erights
Copy link
Member

erights commented Apr 19, 2020

See #838

@erights
Copy link
Member

erights commented Feb 19, 2021

As I said in closing #838

The need for passable collections is already described or started in other issues and PRs. The so-called "passable collections" would include passable sets and passable maps. However, that's as far as this support should go. Marshal should not serialize standard sets and maps. Since the others remain open I'm closing this.

So closing this in favor of passable collections.

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

Successfully merging a pull request may close this issue.

3 participants