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(exo): internal revocables #1668

Closed
wants to merge 1 commit into from
Closed

Conversation

erights
Copy link
Contributor

@erights erights commented Jul 5, 2023

Alternative to #1666

Background: Exos already have a level of indirection between the exposed method with the protective method guard vs the raw method. In order to enable prototype-based method sharing of the protective methods, they lookup the context from their this, which is the exo object itself. The context is then passed as the this to the raw method. Having paid for this level of indirection, we already use it for many benefits, like enforcing that the exposed methods cannot be applied to unrelated instances.

We do this by having all the context (this) objects for a given class (or classKit) inherit a common revoke(obj) method. Thus, revoke(obj) is available from inside instances of a class (or classKit), but not available outside. However, in the current design, this is a per-class (or classKit) power, not a per-instance power. Any instance of a class (or classKit) can revoke any other that it has a reference to. Revoking any facet of a kit revokes all facets of that kit.

This is motivated by the benefits of direct support for revocation for exos at the liveslots level, which needs a parallel implementation with the same observable semantics. As the liveslots level, the benefits would be

  • A step towards dropping exports of used-up exo objects such as payments, relieving distributed gc pressure.
  • A step towards Zoe2 use objects that can be turned (back) into transferable invitations.

Once this support is uniform across heap exos (from this PR) and virtual and durable exos (a future agoric-sdk PR), then zones can mirror this universal support. Further, zones themselves could conceivably absorb the revocation support, to provide batch revocation according to creating zone.

@erights
Copy link
Contributor Author

erights commented Jul 5, 2023

The savings of sharing the inherited revoke method is not worth the complexity it causes. TODO switch to a per-context revoke() closure.

@michaelfig
Copy link
Member

The savings of sharing the inherited revoke method is not worth the complexity it causes. TODO switch to a per-context revoke() closure.

I agree. But this PR seems like a step in the right direction. Please @mention me when you are satisfied with a specific implementation, and I'll review it.

@erights erights removed the request for review from michaelfig July 6, 2023 02:06
@erights erights force-pushed the markm-revocables-internal branch 2 times, most recently from d16165a to 5ede1c8 Compare July 6, 2023 02:57
@erights erights requested a review from FUDCo July 6, 2023 04:12
@erights erights force-pushed the markm-revocables-internal branch from 5ede1c8 to 8219460 Compare July 6, 2023 07:11
@erights erights marked this pull request as draft July 6, 2023 15:38

// Be careful not to freeze the state record
/** @type {ClassContext<ReturnType<I>,M>} */
const context = freeze({ state, self });
// @ts-ignore Doesn't understand __proto__
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove. No more proto to ignore

@FUDCo
Copy link
Contributor

FUDCo commented Jul 6, 2023

It feels unintuitive to me that revoking a facet also revokes all the other facets. In particular, naively I would think it would be a plausible use case to have an object with one facet whose very purpose is to be given out (and thus wants to be revokable) while another facet is retained internally for one's own use. I understand this might involve a potentially unreasonable amount of bookkeeping, so rejecting for that reason might be the right choice, but if so this reasoning ought to be explicit. Or is there some different use model in mind here that motivates this design choice?

@erights
Copy link
Contributor Author

erights commented Jul 6, 2023

It was the bookkeeping. Suggestions for better bookkeeping for virtual objects?

Btw, I turned this back to draft for several reasons. But one is: I think my mechanism here is only deleting the representative from the cache, but not persistently killing the baseRef. But it needs to persistently kill the baseRef or vref, as well as, ideally, notifying liveslots so liveslots can drop the exports. Suggestions?

@erights erights force-pushed the markm-revocables-internal branch from 8219460 to 8d266ab Compare July 7, 2023 00:16
@erights
Copy link
Contributor Author

erights commented Jul 7, 2023

Reversed decision. Now closing in favor of #1666

@erights erights closed this Jul 7, 2023
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.

3 participants