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): revocables #1666

Merged
merged 1 commit into from
Aug 25, 2023
Merged

feat(exo): revocables #1666

merged 1 commit into from
Aug 25, 2023

Conversation

erights
Copy link
Contributor

@erights erights commented Jul 4, 2023

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.

An additional benefit we can obtain from this indirection is revocation. However, creating a revocable exo is the odd case. We don't want the normal case to pay an undue notational cost to accommodate the odd case, even if that raises the notational cost of the odd case a bit. We do so by allowing an additional getRevoker option, which is called with a revoke function to be called with the exo to be revoked as an argument.

This seems unduly higher order. But the getRevoker option is directly analogous to the executor argument of the promise constructor, which is called with a resolve function to be called with a resolution.

This is motivated by the benefits of direct support for revocation for exos at the liveslots level, which needs a parallel implementation of exactly the same options with the same observable semantics. At 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 ( Agoric/agoric-sdk#8020 , Agoric/agoric-sdk#8008 ), then perhaps zones can mirror this universal support. Further, zones themselves could conceivably absorb the revocation support, to provide batch revocation according to creating zone.

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.

Looking good so far.

packages/exo/src/exo-makers.js Outdated Show resolved Hide resolved
@erights
Copy link
Contributor Author

erights commented Jul 5, 2023

Closing in favor of #1668

@erights erights closed this Jul 5, 2023
@erights
Copy link
Contributor Author

erights commented Jul 6, 2023

Reopening. Having tried to use the API of #1668, I now think this one may be the better approach. But also leaving #1668 open while investigating.

@erights erights reopened this Jul 6, 2023
@erights erights marked this pull request as draft July 6, 2023 18:10
@erights erights removed the request for review from michaelfig July 6, 2023 18:10
@erights erights force-pushed the markm-revocables branch 3 times, most recently from 3ec9ea5 to b015165 Compare July 7, 2023 01:43
@erights erights removed the request for review from mhofman July 7, 2023 22:29
@erights erights force-pushed the markm-revocables branch 2 times, most recently from a7e3119 to ee06fe5 Compare July 14, 2023 01:46
@erights erights force-pushed the markm-revocables branch 3 times, most recently from 6853197 to 48c04da Compare July 27, 2023 04:23
@erights erights requested a review from FUDCo July 27, 2023 04:30
@erights erights marked this pull request as ready for review July 27, 2023 04:31
@erights erights requested a review from warner July 27, 2023 04:32
@erights
Copy link
Contributor Author

erights commented Jul 27, 2023

@michaelfig @FUDCo @warner , now that Chip is back, we should talk about this. In particular, the major design choice: should the unit of revocation be the facet or the cohort of facets?

@erights
Copy link
Contributor Author

erights commented Jul 31, 2023

@FUDCo and I discussed the main controversy, and decided that revoking a facet revokes just the facet, not the cohort. This PR does not yet reflect that decision.

@erights erights force-pushed the markm-revocables branch 3 times, most recently from f39f187 to 2636eca Compare August 2, 2023 18:28
@michaelfig michaelfig requested review from michaelfig and removed request for michaelfig August 8, 2023 04:00
@erights
Copy link
Contributor Author

erights commented Aug 9, 2023

@FUDCo and I discussed the main controversy, and decided that revoking a facet revokes just the facet, not the cohort. This PR does not yet reflect that decision.

It does now.

@erights erights force-pushed the markm-revocables branch 3 times, most recently from 323a28f to 0ea5cb5 Compare August 21, 2023 23:23
@erights erights requested a review from kriskowal August 25, 2023 00:44
@erights
Copy link
Contributor Author

erights commented Aug 25, 2023

Reviewers, ping.

@kriskowal is gearing up to make a new endo release. I'd like to get this in there, because agoric-sdk needs to be able to depend on it in order to proceed with the work there to support the same feature for virtual/durable exos. Thanks.

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.

LGTM. Please choose between my two suggestions for more clearly specifying what you mean by object.

packages/exo/src/exo-makers.js Outdated Show resolved Hide resolved
@erights erights enabled auto-merge August 25, 2023 19:55
@erights erights merged commit 47fd2c1 into master Aug 25, 2023
13 checks passed
@erights erights deleted the markm-revocables branch August 25, 2023 20:05
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