endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

feat(patterns): export kindOf #1834

Closed erights closed 1 year ago

erights commented 1 year ago

refs: https://github.com/Agoric/agoric-sdk/pull/8459 , #1794 , https://github.com/Agoric/agoric-sdk/pull/8051

Description

At the @endo/patterns level of abstraction, the kindOf function has the same role that passStyleOf has at the lower @endo/pass-style level of abstraction. However, the @endo/patterns package omitted kindOf from its exports, preventing some natural uses, such as those found in https://github.com/Agoric/agoric-sdk/pull/8459

This PR simply repairs that omission, by exporting kindOf so that dependent packages can use it.

Security Considerations

Without kindOf, client code like https://github.com/Agoric/agoric-sdk/pull/8051 might be tempted to just check passStyleOf(x) === 'tagged' && getTag(x) === 'someTagOfInterest'. But this does not verify that x obeys the invariants expected of an object with kind 'someTagOfInterest'. kindOf bundles in this invariant check, protecting client code from assuming invariants that were actually violated.

Scaling Considerations

Like passStyleOf, the kindOf function has an internal memoizing cache, apparently built on a JS WeakMap. However, liveslots replaces the global WeakMap with a virtual one, in order to maintain the virtual object illusion without making gc observable. When used with virtual/durable keys, this replacement WeakMap leaks heap memory. This necessitates mechanism like https://github.com/endojs/endo/pull/1794 and https://github.com/Agoric/agoric-sdk/pull/8051 , so liveSlots can construct its own passStyleOf using the original JS WeakMap and then endow it to compartments running under liveSlots.

Fortunately, the kindOf memo does not have this problem. Because kindOf(x) !== passStyleOf(x) only when passStyleOf(x) === 'tagged', the kindOf memo delegates all other cases to passStyleOf and memoizes only its decision for these tagged objects as keys. These tagged objects are pass-by-copy, and therefore are never virtual or durable objects.

Documentation Considerations

The taxonomy represented by the kindOf function must anyway be documented in any full explanation of the @endo/patterns package. It should be natural to extend such documentation to explain kindOf as producing that taxonomy, just as passStyleOf produces the taxonomy of Passables.

The best explanations of these two levels of abstraction, and how they relate to each other, are

none of which are modified by this PR.

Testing Considerations

We typically do not test that something is exported. Rather, the exporting module typically tests things it exports and assumes that it exports what the programmer thinks it exports. We should follow that practice here. So there should not be any test for the substantive change caused by this PR. But kindOf itself must be adequately tested to be deserving of export. For purposes of this PR, I propose that test-patterns.js already adequate tests kindOf.

Upgrade Considerations

If we change the invariants associated with a tag name, we must ensure that any durably-stored old objects that were already recognized to be of that kind are still recognized to be of that kind. But, for upgrade, it should be harmless for the kind to expand to include objects it previously would have excluded.

However, even this expansion could be problematic for communicating such data, given version skew between vats. If a vat post that does recognize X as a valid 'foo' kind passes X to a vat that does not yet recognize that it is a valid 'foo' kind, they could get confused. But this hazard already exists and is not actually affected by this PR. Further, such recognition mismatches in tagged recognition is already fundamental to the notion of 'tagged' as an extension point in the Passable system.

erights commented 1 year ago

Straightforward. If you want to test the export, you could use import { kindOf } from '../index.js' in one of the tests.

I don't, since we never test elsewhere simply that something is exported. Thanks.