-
Notifications
You must be signed in to change notification settings - Fork 74
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): lightweight inter-facet rights amplification #1902
Conversation
d6e9229
to
cd9ae8c
Compare
cd9ae8c
to
ee02840
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rely on @mhofman’s review since he has a better grasp of the acceptance criteria. I read this closely enough to grasp what it does and I don’t see defects.
We generally assume the number of facet names for an exo class kit is very
small. This is proportional only to that. Does this seem right?
Cheers,
--MarkM
…On Tue, Dec 19, 2023 at 6:13 PM Dan Connolly ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/exo/src/exo-makers.js
<#1902 (comment)>:
> @@ -228,6 +254,21 @@ export const defineExoClassKit = (
receiveRevoker(revoke);
}
+ if (receiveAmplifier) {
+ const amplify = (aFacet, facetName) => {
+ for (const contextMap of values(contextMapKit)) {
Can this amplify function be considered approximately O(1) for the
purpose of limiting work by message size
<https://github.com/Agoric/agoric-sdk/wiki/Limit-work-by-message-size>?
If not, please document that LOUDLY. Is contextMapKit typically
high-cardinality? What causes it to grow?
In the crabble review <Agoric/agoric-sdk#8465>,
we looked for loops and .map() and such, but we didn't consider the
possibility that functions in the Agoric SDK (including endo) might have
running time that grows much faster than their arguments.
—
Reply to this email directly, view it on GitHub
<#1902 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACC3TGYTK4I2QB2DHQPD3DYKJCWLAVCNFSM6AAAAABA2GVNRGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOJQGA2TSOJZGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
c630268
to
bf29a3a
Compare
In the kernel meeting a couple weeks ago, we discussed having the shape of this rights amplification function be a simple function that accepts a facet, and returns the cohort. I think I preferred that shape vs taking a facet name, but I do realize I didn't capture that feedback in this pull request. |
Quite right. See #1924 |
towards: Agoric/agoric-sdk#8664
refs: #1666
Description
Adds a
receiveAmplifier
option todefinedExoClassKit
to parallel the existingreceiveRevoker
. The amplifier function you receive is a two argument function ofaFacet
and afacetName
. IfaFacet
is an unrevoked facet instance of a cohort of that exo class kit, andfacetName
is the name of a facet column of that exo class kit, then the named element ofaFacet
's cohort is returned.See Agoric/agoric-sdk#8664 for more explanation.
This PR does not by itself finish Agoric/agoric-sdk#8664 . Rather, we need to wait for an endo release incorporating this PR followed by an agoric-sdk-endo sync before we can get started implementing this same feature for virtual and durable class kits.
Security Considerations
As explained at Agoric/agoric-sdk#8664 , this makes one particular pattern of rights amplification easier to express, and more auditable when expressed in this manner.
This is in contrast to systems like JS's or Joe-E class-private instance variables, where inter-instance intra-class amplification is implicit and easy to overlook in a review.
Scaling Considerations
As explained at Agoric/agoric-sdk#8664 , this avoids the need for the additional weak store that would otherwise be needed to express such rights amplification, without losing the explicitness of the separate weak store.
Documentation Considerations
Yet another thing to document. But should probably wait until Agoric/agoric-sdk#8664 is closed, i.e., until the feature is available for virtual and durable exo kits. Probably should wait until revocability is as well, and then both should be documented together.
Testing Considerations
Should enable more secure "jig" testing, where an exo class kit also defines a facet to be used for privileged testing/debugging, and the testing framework is, somehow, endowed with the needed amplifier. We would still need to figure out how to wire that up so the amplifier cannot otherwise be obtained or used.
Upgrade Considerations
Assuming that implementing this feature for virtual and durable exos is similar and as straightforward, this PR should not cause any change to virtual or durable state, enabling exo class kit instance state defined without amplifiers (likewise revokers) to be upgraded into an exo class kit which does.