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

feat(patterns): export kindOf #1834

Merged
merged 1 commit into from
Oct 29, 2023
Merged

feat(patterns): export kindOf #1834

merged 1 commit into from
Oct 29, 2023

Conversation

erights
Copy link
Contributor

@erights erights commented Oct 15, 2023

refs: Agoric/agoric-sdk#8459 , #1794 , Agoric/agoric-sdk#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 Agoric/agoric-sdk#8459

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

Security Considerations

Without kindOf, client code like Agoric/agoric-sdk#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 #1794 and Agoric/agoric-sdk#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 erights self-assigned this Oct 15, 2023
@erights erights force-pushed the markm-frugal-split-patterns branch 3 times, most recently from b14beb3 to 3045779 Compare October 28, 2023 03:46
@erights erights marked this pull request as ready for review October 28, 2023 03:47
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

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

@erights erights force-pushed the markm-frugal-split-patterns branch from 3045779 to c6c0f67 Compare October 29, 2023 20:39
@erights
Copy link
Contributor Author

erights commented Oct 29, 2023

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.

@erights erights enabled auto-merge (squash) October 29, 2023 20:41
@erights erights merged commit f746e99 into master Oct 29, 2023
14 checks passed
@erights erights deleted the markm-frugal-split-patterns branch October 29, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants