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

refactor: get passStyleOf from VataData if there #1676

Merged
merged 1 commit into from
Jul 15, 2023

Conversation

erights
Copy link
Contributor

@erights erights commented Jul 14, 2023

See Agoric/agoric-sdk#8051

If there is already a VataData global containing a function named passStyleOf, then presumably it was endowed for us by liveslots, so we should use and export that one instead.

@erights erights requested review from warner and mhofman July 14, 2023 23:46
@erights erights self-assigned this Jul 14, 2023
@kriskowal
Copy link
Member

I think I would only suggest considering whether VatData is the right namespace. I assume that it will be equally compelling to endow passStyleOf into caplets.

@kriskowal
Copy link
Member

kriskowal commented Jul 15, 2023

This might be an opportunity to christen the Ocap namespace as suggested by @weizman, with the caveat that would imply the existence of different %Ocap% and %SharedOcap% namespace objects, since Ocap.lockdown() would not be visible in compartments.

@mhofman
Copy link
Contributor

mhofman commented Jul 15, 2023

Haven't reviewed yet. I know that VatData is used in this fashion already, but I believe only within agoric-sdk. I'm actually wondering if we need to namespace passStyleOf for now, or can get away without until we figure out the broader Endo / SES namespacing issue.

if (typeof globalThis?.VatData?.passStyleOf === 'function') {
return globalThis.VatData.passStyleOf;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Why put this check inside the maker instead of the actual passStyleOf exported below? I know it'd become a less readable ternary, but it just feels strange to have a maker return a preexisting value, ignoring it's parameter.

Copy link
Member

Choose a reason for hiding this comment

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

I will not be bothered if this becomes a cascade of the two namespaces whatever they will be in time. It would be a very small scar.

Copy link
Contributor Author

@erights erights Jul 15, 2023

Choose a reason for hiding this comment

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

I know it'd become a less readable ternary,

It wasn't bad. Done.

@erights
Copy link
Contributor Author

erights commented Jul 15, 2023

I assume that it will be equally compelling to endow passStyleOf into caplets.

Not really. The strong motivation comes from running user code in an on-chain virtualized environment, where the user's WeakMap would have been replaced with one that prevents the sensing of garbage collection on behalf of virtual objects.

This make the motivating use so narrow that I'm gonna go ahead with the current name, for the sake of lower friction integration with agoric-adk.

@erights erights enabled auto-merge July 15, 2023 03:04
@erights erights merged commit 4291993 into master Jul 15, 2023
13 checks passed
@erights erights deleted the markm-vatdata-pass-style branch July 15, 2023 03:11
* @type {PassStyleOf}
*/
export const passStyleOf =
globalThis?.VatData?.passStyleOf ||
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT, this is the only mention of VatData in all of endo, and one of only two non-spurious references to liveslots (the others looking like migration cruft). If fundamental modules are going to defer to their environment, it would be good to establish an endo convention supporting that while keeping it independent rather than inverting the usual dependency direction with agoric-sdk. My first thought would be Symbol-keyed properties, e.g.

Suggested change
globalThis?.VatData?.passStyleOf ||
globalThis?.[Symbol.for('@endo passStyleOf')] ||

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.

4 participants