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

Clear up presences vs. remotables #870

Merged
merged 3 commits into from
Apr 26, 2020
Merged

Conversation

michaelfig
Copy link
Member

@michaelfig michaelfig commented Apr 6, 2020

This implements the first part of the #804 plan for explicitly creating remotables via the @agoric/marshal Remotable() function.

Remotable and getInterfaceOf should be shared across marshal systems within the vat that imports them. I explicitly provide them to the liveSlots caller in order to expose to the vat code that uses it, rather than bundling a new copy of @agoric/marshal and getting a different instance of the WeakMap that keeps the interface information.

This is the smallest step forward that properly solves #792 without needing more far-reaching changes.

Here is some annotated REPL output:

// Show how presences print
command[0] home
history[0] {"chainTimerService":[Presence o-50]{},"sharingService":[Presence o-51]{},"contractHost":[Presence o-52]{},"registrar":[Presence o-53]{},"zoe":[Presence o-54]{},"localTimerService":[Presence o-55]{},"uploads":[Presence o-56]{},"spawner":[Presence o-57]{},"wallet":[Presence o-58]{},"http":[Presence o-59]{}}
// Try making a remotable from an array
command[2] a = []
history[2] []
command[3] Remotable('Foo', {abc: 1}, a)
history[3] exception: Error: Arrays cannot be pass-by-presence
// Note that there is no mutation.
command[4] a
history[4] []
// Try adding a property to an empty remotable.
command[5] Remotable('Foo', {abc: 1})
history[5] exception: Error: cannot serialize objects with non-methods like the .abc in [Foo]
// Add a method to an empty remotable.
command[6] Remotable('Foo', {abc() { return 'def' }})
history[6] [Foo]{"abc":[Function abc]}
// Call the method.
command[7] history[6].abc()
history[7] "def"
// Use tildot.
command[8] history[6]~.abc()
history[8] "def"
// Try making a function into a remotable.
command[9] Remotable('Foo', {abc() { return 'def' }}, a => a + 1)
history[9] exception: Error: cannot serialize non-objects like a => a + 1
// Demonstrate that getInterfaceOf shows the iface we attached.
command[11] getInterfaceOf(history[6])
history[11] "Foo"

@michaelfig michaelfig requested review from warner, erights and FUDCo April 6, 2020 22:38
@michaelfig michaelfig self-assigned this Apr 6, 2020
@michaelfig michaelfig added captp package: captp cosmic-swingset package: cosmic-swingset marshal package: marshal SwingSet package: SwingSet labels Apr 6, 2020
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a start on review.

packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
@michaelfig michaelfig requested a review from kriskowal April 10, 2020 21:13
@michaelfig michaelfig force-pushed the mfig/804-clear-up-presences branch from e13ab7d to 0a12b1e Compare April 12, 2020 02:06
@michaelfig michaelfig requested a review from erights April 12, 2020 02:27
@michaelfig
Copy link
Member Author

Please, all, take a look at tc39/proposal-eventual-send#11

@erights
Copy link
Member

erights commented Apr 25, 2020

@michaelfig please let me know when this has been updated to the latest eventual send API and terminology.

@michaelfig michaelfig force-pushed the mfig/804-clear-up-presences branch from 0a12b1e to c0c5c0c Compare April 25, 2020 17:36
@michaelfig
Copy link
Member Author

@michaelfig please let me know when this has been updated to the latest eventual send API and terminology.

@erights I moved to the Presence vs Remotable distinction we now make. However, I will change to Promise.delegate in a PR after this one. I really want to land this one first, and get it in!

So, this is ready for review.

fix: assign a singleton `globalThis.harden`
@michaelfig michaelfig force-pushed the mfig/804-clear-up-presences branch from 3c02bcc to aac1899 Compare April 25, 2020 19:00
@michaelfig michaelfig changed the title Clear up presences Clear up presences vs. remotables Apr 25, 2020
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited to see this, thanks!

packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
@michaelfig michaelfig requested a review from erights April 26, 2020 08:08
packages/marshal/marshal.js Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
packages/marshal/marshal.js Outdated Show resolved Hide resolved
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the copying of errors are fixed.

packages/marshal/marshal.js Show resolved Hide resolved
@michaelfig michaelfig merged commit f3b8449 into master Apr 26, 2020
@michaelfig michaelfig deleted the mfig/804-clear-up-presences branch April 26, 2020 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
captp package: captp cosmic-swingset package: cosmic-swingset marshal package: marshal SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants