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(vats): ERTP face on orch assets, take 3 #10456

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

Conversation

erights
Copy link
Member

@erights erights commented Nov 12, 2024

Successor to #10443
Likely to be closed in favor of #10504

closes: #XXXX
refs: #XXXX

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

Copy link

cloudflare-workers-and-pages bot commented Nov 13, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: ef65261
Status: ✅  Deploy successful!
Preview URL: https://db46a9b8.agoric-sdk.pages.dev
Branch Preview URL: https://markm-orch-purse-3.agoric-sdk.pages.dev

View logs

@erights erights force-pushed the markm-orch-purse-3 branch 2 times, most recently from e320322 to 1a36c17 Compare November 13, 2024 19:37
mergify bot pushed a commit that referenced this pull request Nov 14, 2024
closes: #XXXX
refs: #XXXX

## Description

For some reason, vscode does successfully highlight unused jsdoc type imports

![image](https://github.com/user-attachments/assets/8ba7670d-cde2-4458-b5dc-501b4b315414)

but `yarn lint` does not

![image](https://github.com/user-attachments/assets/1f4769f4-2f85-41f2-886b-9a0e6c5a0078)

This PR started as a drive-by extracted from #10456 , where I noticed that ERTP in particular had some of these. Since #10456 experiments with a variation on ERTP, simplifying it first in harmless ways helps a bit.

### Security Considerations

fewer distractions helps

### Scaling Considerations

none

### Documentation Considerations

none

### Testing Considerations

none

### Upgrade Considerations

In the agoric-sdk repo there are no remaining type imports of the legacy type reexport of `Baggage` from ertp. But if there are such imports in other repos, they will need to be fixed. But this is a static-only issue, not a runtime issue, so it's fine for those repos to be fixed only once they depend on the new `@agoric/ertp` without that legacy reexport.
mergify bot pushed a commit that referenced this pull request Nov 15, 2024
closes: #XXXX
refs: #XXXX

## Description

orchestration/src/typeGuards was defining and exporting some patterns as just unhardened object literals. This is locally wrong, since these are not valid patterns (or even passables) until hardened. The only reason these errors were uncaught must be because they were then used in a context which happened to harden them before they were used in a context where they were validated as patterns or passables.

Started as a drive-by of #10456 , where I import some of these patterns for usage elsewhere.

### Security Considerations

defensive hardening is good

### Scaling Considerations

none

### Documentation Considerations

one less thing we would need to explain

### Testing Considerations

In theory, a test could have revealed the bug that these were exported unhardened. But IMO, such tests would only be too much noise. We should just fix these. But this PR only fixes these in this one file. I don't know how many others there are.

### Upgrade Considerations

In theory, it is possible that some importing code somewhere accidentally replies on these being unhardened, perhaps because it mutates one in place before using it, corrupting use by all other importers. If so, this PR will break such uses, which is good, because the silent corruption is a worse surprise than a new noisy error.
@erights erights force-pushed the markm-orch-purse-3 branch 4 times, most recently from b30ce82 to 0b6eecb Compare November 15, 2024 15:09
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.

1 participant