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(swingset): add controller.terminateVat(vatID, reason) #9253

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

warner
Copy link
Member

@warner warner commented Apr 18, 2024

This new API allows the host application to terminate any vat for which is knows the VatID (which must be gleaned manually from logs or the database). This might be useful if the normal vat code is unable or unwilling to terminate the vat, or if you need to trigger termination at some specific point in time.

closes #8687

Copy link

cloudflare-workers-and-pages bot commented Apr 18, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 693b367
Status: ✅  Deploy successful!
Preview URL: https://43cb910d.agoric-sdk.pages.dev
Branch Preview URL: https://warner-8687-controller-termi.agoric-sdk.pages.dev

View logs

@warner
Copy link
Member Author

warner commented Apr 18, 2024

Security Considerations

The new API is powerful, and uses a trivially-forgeable vatID string, but is only available to the host application.

Scaling Considerations

As noted in the docstring, unless a runPolicy is used to rate-limit cleanups, all vat state will be terminated during the first controller.run() call after this new controller.terminateVat() is invoked. That state might be large, and deleting it all at once could be a problem. This PR is targeted to land on top of the #8928 rate-limited cleanup branch, to enable host applications to avoid this problem.

Documentation Considerations

Nothing here should be visible to userspace developers. The primary audience of this new feature is a specialized cosmic-swingset upgrade handler, which can call it during some future chain upgrade, to initiate deletion of the then-unused large price-feed vats, after they have been replaced by others. It's entirely possible that userspace will have a way to delete these vats by then, and we won't need the host-app to do it.

Testing Considerations

If/when we write that future upgrade handler to invoke this, we will need some kind of main-fork test to make sure it can initiate termination correctly. That will be part of the PR which modifies cosmic-swingset.

Upgrade Considerations

none

@warner warner added the SwingSet package: SwingSet label Apr 18, 2024
@warner warner requested a review from mhofman April 18, 2024 18:02
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 2d621ee to 98c3e10 Compare April 23, 2024 19:03
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from c3299e5 to a31549a Compare April 23, 2024 19:03
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

This all seems reasonable, even though I'm not sure in which (non-test) circumstance we'd want to use this mechanism as I'd expect us to use bootstrap powers instead to terminate a vat during some core proposal.

@mhofman mhofman assigned warner and unassigned mhofman May 21, 2024
@warner
Copy link
Member Author

warner commented Jun 10, 2024

This all seems reasonable, even though I'm not sure in which (non-test) circumstance we'd want to use this mechanism as I'd expect us to use bootstrap powers instead to terminate a vat during some core proposal.

Yeah, it wasn't 100% clear to me that the mainnet bootstrap vat has access to the necessary adminNode facets.. for contract vats, they might be held inside Zoe and not exposed to bootstrap. So this is a just-in-case. But it would also useful for non-critical static vats. In conjunction with a clear-criticalVat feature that we should probably add, it'd be the only way to terminate the bootstrap vat (unless vatPowers is available to core-eval proposals).

@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from a31549a to 13ea1dc Compare June 10, 2024 17:35
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 98c3e10 to c17ff90 Compare June 10, 2024 17:35
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from 13ea1dc to bb79f9e Compare June 12, 2024 02:15
@warner warner force-pushed the warner/8687-controller-terminateVat branch from c17ff90 to 0f847c7 Compare June 12, 2024 02:15
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from bb79f9e to c9bd283 Compare June 13, 2024 18:12
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 0f847c7 to 07b3d93 Compare June 13, 2024 18:12
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from c9bd283 to cbbc252 Compare June 14, 2024 00:18
@warner warner force-pushed the warner/8687-controller-terminateVat branch 2 times, most recently from c2d78a5 to e45f10a Compare June 14, 2024 22:52
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch 2 times, most recently from dab29e7 to b422019 Compare June 14, 2024 23:07
@warner warner force-pushed the warner/8687-controller-terminateVat branch from e45f10a to 89a6312 Compare June 14, 2024 23:07
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 89a6312 to 27286db Compare June 24, 2024 16:29
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch 2 times, most recently from b07a058 to a2f0a21 Compare June 26, 2024 19:20
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 27286db to 1a38cf2 Compare June 26, 2024 19:20
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from a2f0a21 to fa87abe Compare June 27, 2024 21:39
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 1a38cf2 to e05394d Compare June 27, 2024 21:39
@warner warner force-pushed the warner/8687-controller-terminateVat branch from e05394d to 4fdb16e Compare July 2, 2024 06:03
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from fa87abe to 4ad9689 Compare July 2, 2024 06:03
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 4fdb16e to 6c53a99 Compare July 10, 2024 17:05
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch 2 times, most recently from 0140249 to 71ede77 Compare July 11, 2024 04:38
@warner warner force-pushed the warner/8687-controller-terminateVat branch 2 times, most recently from 794a9c2 to 2990dfb Compare July 11, 2024 23:51
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from 71ede77 to 06ccc1f Compare July 11, 2024 23:51
Copy link
Member

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

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

Nice and compact!—although I do wish the testing was more self-contained; I'll have to see about DX improvements for bootstrap-relay.js.

packages/SwingSet/src/controller/controller.js Outdated Show resolved Hide resolved
packages/SwingSet/src/controller/controller.js Outdated Show resolved Hide resolved

terminateVat(vatID, reasonCD) {
insistCapData(reasonCD);
assert(reasonCD.slots.length === 0, 'no slots allowed in reason');
Copy link
Member

Choose a reason for hiding this comment

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

I don't know how strict we should be about assert patterns, but this is the convention AIUI:

Suggested change
assert(reasonCD.slots.length === 0, 'no slots allowed in reason');
reasonCD.slots.length === 0 || Fail`no slots allowed in reason`;

@warner warner force-pushed the warner/8687-controller-terminateVat branch from 86b55f5 to f76a341 Compare August 10, 2024 23:57
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from af1555e to 164ed8b Compare August 10, 2024 23:57
@warner warner force-pushed the warner/8687-controller-terminateVat branch from f76a341 to b3c1d8c Compare August 11, 2024 16:38
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch 2 times, most recently from 47fe6e9 to cbbb00d Compare August 11, 2024 17:26
@warner warner force-pushed the warner/8687-controller-terminateVat branch 2 times, most recently from 53da414 to 52e0840 Compare August 11, 2024 18:05
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from cbbb00d to fc2718e Compare August 11, 2024 18:05
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 52e0840 to c9b014a Compare August 12, 2024 22:01
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from fc2718e to 3648e55 Compare August 12, 2024 22:01
@warner warner force-pushed the warner/8687-controller-terminateVat branch from c9b014a to 5b67afe Compare August 12, 2024 22:54
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from 3648e55 to 1dcb1d2 Compare August 12, 2024 22:54
@warner warner force-pushed the warner/8687-controller-terminateVat branch from 5b67afe to fba6e3a Compare August 12, 2024 23:51
@warner warner force-pushed the warner/8928-terminate-vats-slowly branch from 1dcb1d2 to 9ac2ef0 Compare August 12, 2024 23:51
Base automatically changed from warner/8928-terminate-vats-slowly to master August 13, 2024 00:48
This new API allows the host application to terminate any vat for
which is knows the VatID (which must be gleaned manually from logs or
the database). This might be useful if the normal vat code is unable
or unwilling to terminate the vat, or if you need to trigger
termination at some specific point in time.

closes #8687
@warner warner force-pushed the warner/8687-controller-terminateVat branch from fba6e3a to 693b367 Compare August 13, 2024 00:50
@warner warner added the automerge:rebase Automatically rebase updates, then merge label Aug 13, 2024
@mergify mergify bot merged commit d32d6ce into master Aug 13, 2024
90 checks passed
@mergify mergify bot deleted the warner/8687-controller-terminateVat branch August 13, 2024 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add controller.terminateVat()
3 participants