-
Notifications
You must be signed in to change notification settings - Fork 228
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
fix(swingset): createVatDynamically option to disable metering #1309
Conversation
56e7061
to
fb07411
Compare
3e80484
to
016869c
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.
Looks good, modulo the qualification in my comment below.
* subject to a meter that limits the amount of computation and allocation | ||
* that can occur during any given crank. Stack frames are limited as well. | ||
* The meter is refilled between cranks, but if the meter ever underflows, | ||
* the vat is terminated. If 'false', the vat is unmetered. |
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.
Presumably if the creating vat is itself metered it won't have the power to create new, unmetered vats, so this option is subject to being ignored. If that understanding is correct, then probably you should expand this comment to talk about that.
Or did we decide that a dynamic vat creator could always create an unmetered vat if it chose to and thus the control to be exercised was whether a vat was vested with this power at all? In that case the "you can create a vat but it has to be metered" case would be realized by logic inside the vat admin vat. If so, then probably that should be documented. However, it doesn't look to me like the vat admin vat code that's part of this PR implements such a control mechanism.
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.
Yeah, I'm going with the second option for now (anyone who can create a dynamic vat can also create a metered one, whether they're metered or not). I don't think we have a use-case for dynamic vats creating dynamic vats yet, so I don't think we need to build a facet that enforces a metered: true
option yet. But you're right, I should document that to make it clear.
016869c
to
388af11
Compare
refs #1307