marshal: Allow promise stand-ins

[gibson042]

The agoric-sdk kernel needs to transform marshaled messages, preferably while respecting their encoding by using standard marshal interfaces. But doing so effectively requires opaque treatment of remotables and promises, which is not currently possible with smallcaps because it maintains an encoding-level distinction between the two classes and requires that the latter correspond with actual ECMAScript Promise instances.

Proposed solution: Update passStyleOf to support ersatz promises like { [PASS_STYLE]: 'promise' }, allowing convertValToSlot and convertSlotToVal functions to utilize such objects in place of native promises. How much state they should be allowed to carry is an open question, but my initial inclination is to either prohibit it entirely or to have it match what can be carried by a native promise (e.g., the { status: 'fulfilled', value: any } | { status: 'rejected', reason: any } objects from Promise.allSettled.

[erights]

or to have it match what can be carried by a native promise (e.g., the { status: 'fulfilled', value: any } | { status: 'rejected', reason: any } objects from Promise.allSettled.

Clever! This points out and solves a related issue: When one of these hits liveSlots, how should liveSlots express interest in its settlement? Add also | { status: 'forwarded', value: any } | { status: 'unresolved' } and we have an interesting bridge to persistent promises.

[mhofman]

How much state they should be allowed to carry is an open question

I actually do not understand this question.

What state are we taking about exactly? Why is the marshalling layer interested in promise state vs simply considering them as some external reference like a remotable?

From what I remember marshall is agnostic of how the "send" of a method onto an object is encoded. Similarly it should then be agnostic of how promise resolution is encoded.

[erights]

marshal is agnostic. Liveslots is not.

[mhofman]

Ok I think I follow now. I'm of the opinion that all promise state should be private for custom promises that liveslots creates, like they are for native promises. The only way to get at that state should be through methods on the promise objects, or through liveslots provided functions that recognize the promise instances.

[mhofman]

Add also | { status: 'forwarded', value: any } | { status: 'unresolved' } and we have an interesting bridge to persistent promises.

FWIW, my user land promise implementation has such internal states.

[erights]

But whatever this escape hatch is, anyone can use it to create a new “promise”. LiveSlots may then see a promise created by something else.

[mhofman]

Correct, and in the same way, user-land can create an object that claims to be a remotable. I believe that currently the LiveSlots behavior is to consider any such remotable object it didn't create as a simple Far object that the vat wants to export. It's unclear what the LiveSlots behavior should be for promise objects it didn't create, especially if we disallow such promise objects to be thenable (if they were, it'd be the natural way for these user-land promise objects to be consumable by LiveSlots).

[erights]

Requirement: E and E.when must work on these somehow.

[gibson042]

Are you saying that an ersatz promise can have a then method, or that it must? I think the latter would add some otherwise unnecessary overhead for @FUDCo's opaque manipulation.

[mhofman]

In the discussion I had with @erights the other day, we decided against an ersatz promise having a then method (but allowed to have any other methods, like remotables), however that would make difficult (impossible without extra hooks?) for LiveSlots to be able to adopt ersatz promise created from thin air by user code.

[FUDCo]

that would make difficult (impossible without extra hooks?) for LiveSlots to be able to adopt ersatz promise created from thin air by user code.

I think that's a feature, not a bug. I don't think we want anyone to try to use one of these ersatz promises as an actual promise. The whole point is to be able to roundtrip from unserialization to serialization without the intermediate entity being usable as anything other than a (mostly) opaque token.

[mhofman]

I understood the ability of being able to use anything that passes passStyle checks as being a requirement from @erights. I believe that's currently the case since any passStyle of remotable is effectively accepted by LiveSlots. If we want to introduce such a restriction for promise, we should make it clear, and define what happens in case where LiveSlots rejects these values.

I guess it comes down to a question of layering, which I'm still a little confused about.

[erights]

Yes. For any x, if passStyleOf(x) === 'promise', then it must work with E, E.when, and liveSlots.

These discussions changed my mind. @FUDCo , in what way is the API at https://github.com/endojs/endo/pull/1313#discussion_r992797479

getSomething(makePromiseWithSomething(something)) === something

inadequate, or even significantly suboptimal, for your current needs? If it is adequate, I think we should hold #1311 until we understand better what we want from virtual and durable promises. For example, it would be natural and expected for them to satisfy the E, E.when, and liveSlots requirements above.

[FUDCo]

My goal is to change:

const promiseRefMap = new WeakMap();

export const kslot = (kref, iface) => {
  if (isPromiseRef(kref)) {
    const p = new Promise(() => undefined);
    promiseRefMap.set(p, kref);
    return harden(p);
  } else {
    return Far(iface, {
      toString: () => `${kref}`,
    });
  }
};

export const krefOf = obj => promiseRefMap.get(obj) || obj.toString();

to something like:

export const kslot = (kref, iface) => {
  const obj = {
    toString: () => `${kref}`,
  };
  if (isPromiseRef(kref)) {
    obj[PASS_STYLE] = 'promise';
    return harden(obj);
  }
  return Far(iface, obj);
};

export const krefOf = obj => obj.toString();

This (a) gets rid of the ugly, time and space wasting WeakMap, (b) makes the code simpler and more orthogonal, and (c) lets me reliably and safely say `${foo}` instead of `${krefOf(foo)}`.

[erights]

The weakmap in question is small and short-lived, right? If so, it should actually be a map. Is this a performance critical piece of code?

If the code is not performance critical and the weakMap can be a small and short-lived map, the differential pain here is tiny compared to the means we're using to try to solve it.

[kriskowal]

What’s the status of this issue, @gibson042? Is this completed and follow-up work tracked elsewhere? Did we shift from this approach to another?