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

other ways to provide getInterfaceOf/Remotable to vat code #1816

Closed
warner opened this issue Sep 22, 2020 · 6 comments
Closed

other ways to provide getInterfaceOf/Remotable to vat code #1816

warner opened this issue Sep 22, 2020 · 6 comments
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Sep 22, 2020

What is the Problem Being Solved?

In today's meeting we discussed what would be the most ergnonomic way to provide the getInterfaceOf and Remotable functions into vat code (and makeMarshal). These functions are powerless but not pure: they all close over the same WeakMap. Moreover, they share this weakmap with liveslots. To work properly, the vat code must share a WeakMap with the liveslots which serves it. I think it's ok to share a map with the other vats: since vats can't share objects directly, they can't observe that fact.

I'm using "vat code" to mean the ocap-style code that lives in a namespace which exports a buildRootObject(vatPowers, vatParameters) function. The vat as a whole consists of liveslots plus the vat code. Liveslots gets direct access to the syscall object, and thus is not ocap-confined (in the sense that any code within liveslots gets to send messages to any kernel-level object that the vat as a whole has been granted access to).

Currently, the vat code receives getInterfaceOf/etc as properties of the vatPowers argument. vatPowers is roughly intended to provide non-powerless authorities which the vat code might which to internally partition, so buildRootObject gets it but could choose to withhold it from other objectsthat buildRootObject creates. But vatPowers was also a convenient place to deliver getInterfaceOf/etc even though it's not particularly useful to withhold from anything within the vat code.

The downside of using an argument like vatPowers is that when you do want to share access, you have to pass it around everywhere, using more arguments.

The other two approaches we could take are to provide it as a global endowment, or allow code to import it from a special module.

1: provide it as a global endowment

The benefit of using a global is that there's no visual overhead to using it: no extra import in the file that contains the call site, no extra arguments in the causal path from buildRootObject() down to the call site. The downside of a global endowment is namespace clutter and specification divergence (we tell developers "it's javascript(*)", and then list the differences in the fine print, and this adds to those differences). Someone reading a contract who sees reference to getInterfaceOf has no obvious way to discover what it does, and unlike every other javascript global, they won't find it on MDN or other JS tutorial sites.

It wouldn't be hard to expose these as a global. Currently the vat's globals are shared among all local vats by virtue of a single vatEndowments object being passed to makeLocalVatManagerFactory. A single allVatPowers object is simultaneously passed as well, and currently getInterfaceOf/etc are housed inside allVatPowers. We could move that to vatEndowments instead.

To accomodate non-local workers, the managers for the other kinds of workers would need to be changed similarly. There would be one WeakMap per process/thread, since it cannot be shared outside a single JS engine / heap. We might want to change the way this is set up, making liveslots itself responsible for creating a separate getInterfaceOf/etc set for each vat. If we did this and also wanted to pass them as globals, we would need to rearrange the managers to pass a bundle to liveslots and allow liveslots to perform the call to importBundle, because importBundle needs the globals as an argument.

2: allow code to use import or require to retrieve it whenever/whereever they want

The benefit of an import is an appropriate amount of overhead (one import statement per file that uses it), and it follows a clear+readable+learnable tradition of bringing additional functionality into a namespace. The downside is the hassle of getting the right things made available to the right places.

We used to use import/require to provide Nat, harden, and evaluate. We changed Nat to be just a regular import, and we added harden to the definition of SES so it's now delivered as a global. evaluate got merged into Compartment, which is also a global. Once all three were removed as potential imports, we removed the import mechanism entirely, and now require is not present as a global in the compartments used by vats.

If we were going to bring that back as a way to distribute getInterfaceOf, we'd need to:

  • decide on a module name, preferably one that looks like a built-in and not to be confused with one on NPM
  • add that module name to the list of externals in @agoric/bundle-source, so any import 'foo' will be turned into a require('foo') instead of trying to find a foo package and interpolate its source code
  • reintroduce a require function into the globals of the importBundle() call used to load vat bundles
  • have that require serve up the same getInterfaceOf/etc that liveslots uses

Sooner or later we'll move from using rollup to endo. At that point, we'll replace require with entries in a module map of some sort, which should make this cleaner. We'll still need to teach endo's create-archive code that it shouldn't try to search for the special module name in node_modules/, that instead it will be made available in the final import environment.

Security Considerations

We need to continue to make getInterfaceOf/etc powerless: they close over a WeakMap, but must not allow that to be used as a communication channel between otherwise unconnected objects.

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Sep 22, 2020
@katelynsills
Copy link
Contributor

It sounds like solution no. 1 is significantly easier. I believe we can internalize the usage of getInterfaceOf to a large extent by providing useful helpers as part of ERTP and Zoe. For instance, a helper that asserts that something is an issuer, or that something is a Zoe invitation, would need to use getInterfaceOf internally but the user would not.

@warner
Copy link
Member Author

warner commented Sep 22, 2020

I'm happy to implement either (or stick with the existing vatPowers argument approach). I'd say that developer ergonomics should be the biggest consideration.

@katelynsills
Copy link
Contributor

Sounds good. The existing vatPowers argument approach got shot down by @erights on developer ergonomics grounds (to use the new ERTP functions to test whether something is an issuer, getInterfaceOf had to be passed in to make the functions, and it seems like with the vatPowers argument approach there's not a way around it). So, I think whichever is easiest of these two solutions sounds great.

@erights
Copy link
Member

erights commented Sep 22, 2020 via email

@warner
Copy link
Member Author

warner commented Mar 7, 2021

We resolved this by making getInterfaceOf/Remotable/Far (and the temporary Data) all properly powerless: the interface/data marker is now added to the object in question (in a new prototype, to avoid changing the own-properties of the target). This removes the WeakMap, which means nothing needs to be shared between the vat code that adds the marker and the liveslots code which needs to read it.

So we no longer need to solve this problem. vatPowers.getInterfaceOf is a simple copy of the import that liveslots does, and can+should be replaced in vat code by a direct import. #2590 is about removing vatPowers.getInterfaceOf and friends now that it's no longer needed, after fixing all vat code to use an import instead.

@erights
Copy link
Member

erights commented Jul 29, 2021

See also #3558

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

3 participants