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: prepare for use of non-trapping integrity trait #10795

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erights
Copy link
Member

@erights erights commented Jan 4, 2025

closes: #XXXX
refs: endojs/endo#2679

Description

Does for agoric-sdk what endojs/endo#2679 does for endo

Prepare for anticipated introduction and use of the non-trapping integrity trait as explained at https://github.com/endojs/endo/blob/b12eb434b6672f0ceae41be55aac7f24c4562b7b/packages/ses/docs/preparing-for-stabilize.md

These preparations must work now, before these traits are introduced, and should continue to work after these traits are introduced and used.

Security Considerations

Some things that had been deeply frozen automatically by harden are now manually frozen by explicit calls to freeze. We need to review these carefully to ensure that nothing has inadvertently be left unfrozen as a result of the changes in this PR.

Some proxies will become unhardenable, but they will still be hardenable as of now, so mistaken hardenings will not be detected.

Scaling Considerations

For this PR by itself, none. Using the shim-based implementation of the non-trapping trait will have scaling consequences: endojs/endo#2675

Documentation Considerations

https://github.com/endojs/endo/blob/b12eb434b6672f0ceae41be55aac7f24c4562b7b/packages/ses/docs/preparing-for-stabilize.md will need to be reflected in developer docs.

Testing Considerations

Since this PR by itself should be a pure refactor with no observable changes, there is nothing to test at this stage. The testing burden will come with #10796 to see how adequate these preparations were.

Compatibility Considerations

The point. This changes to coding patterns that should be compat both with the current status quo and with #10796

Upgrade Considerations

As a pure refactor, none.

@erights erights self-assigned this Jan 4, 2025
Copy link

cloudflare-workers-and-pages bot commented Jan 4, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4d00b72
Status: ✅  Deploy successful!
Preview URL: https://7372ea51.agoric-sdk.pages.dev
Branch Preview URL: https://markm-prepare-for-non-trappi.agoric-sdk.pages.dev

View logs

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 made me realize a broader upgrade hazard which I've described in endojs/endo#2675 (review)

Copy link
Member

Choose a reason for hiding this comment

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

The change in this file points to a potential future upgrade hazard. It means we cannot simply use the currently deployed code and re-run its transcript on top of a lockdown + liveslots (or an XS engine) that introduces non-trapping behavior. Such a "replay" is one of the option we're still keeping under consideration, especially for the bootstrap vat which cannot be upgraded.

@erights erights force-pushed the markm-prepare-for-non-trapping branch from 458c2f6 to f43c9e5 Compare January 6, 2025 20:04
@erights erights force-pushed the markm-prepare-for-non-trapping branch from f43c9e5 to 4d00b72 Compare January 6, 2025 22:22
erights added a commit that referenced this pull request Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants