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

vat-safe cosmic-proto top level #9743

Merged
merged 2 commits into from
Jul 22, 2024
Merged

vat-safe cosmic-proto top level #9743

merged 2 commits into from
Jul 22, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Jul 19, 2024

closes: #9578

Description

The reason for #9578 was an import from @agoric/cosmic-proto in a vat instead of @agoric/cosmic-proto/vatsafe. That's quite an easy error to make (a footgun) so this makes the top level module export be the "vat safe" one.

When consumers want the convenience of a barrel export or rpc clients that can resolve anything (#9200), they can use a different subpath.

Security Considerations

none

Scaling Considerations

can reduce bundle size

Documentation Considerations

reduces need to document

Testing Considerations

existing coverage

Upgrade Considerations

affects only new bundles

@turadg turadg requested review from kriskowal and 0xpatrickdev July 19, 2024 22:10
Copy link

cloudflare-workers-and-pages bot commented Jul 19, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7dd34e3
Status: ✅  Deploy successful!
Preview URL: https://26d7b846.agoric-sdk.pages.dev
Branch Preview URL: https://9578-fix-protos-import.agoric-sdk.pages.dev

View logs

@turadg turadg force-pushed the 9578-fix-protos-import branch from 61a50a1 to 4d7e50a Compare July 19, 2024 22:29
@turadg turadg force-pushed the 9578-fix-protos-import branch from 4d7e50a to 7dd34e3 Compare July 22, 2024 15:02
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.37%. Comparing base (e6a0e81) to head (7dd34e3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9743      +/-   ##
==========================================
+ Coverage   55.77%   56.37%   +0.59%     
==========================================
  Files         680      667      -13     
  Lines      213499   207526    -5973     
  Branches      352      339      -13     
==========================================
- Hits       119081   116988    -2093     
+ Misses      94300    90419    -3881     
- Partials      118      119       +1     
Files Coverage Δ
packages/cosmic-proto/src/index.ts 100.00% <100.00%> (ø)
...hestration/src/exos/local-orchestration-account.js 87.78% <100.00%> (ø)

... and 18 files with indirect coverage changes

Copy link
Member

@0xpatrickdev 0xpatrickdev left a comment

Choose a reason for hiding this comment

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

Glad to see the fix was really simple 🚀

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Jul 22, 2024
@mergify mergify bot merged commit ee2126b into master Jul 22, 2024
88 checks passed
@mergify mergify bot deleted the 9578-fix-protos-import branch July 22, 2024 17:18
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

orchestration bundles: remove unused protobuf encoders/decoders in CosmosOrchestrationAccount
2 participants