refactor: Add mailbox types and differentiate use of maps from map-like stores #10290
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
6822daf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://16c8dd53.agoric-sdk.pages.dev |
| Branch Preview URL: | https://gibson-2024-10-mailbox-types.agoric-sdk.pages.dev |
|
I plan on reviewing this, but I'm surprised to see a "breaking change" |
Yeah, I'm not sure how to accurately account for the details here—the return value of |
IMO, if you are confident this couldn't actually break consumers then it's not necessary to mark. Another reason not to have |
packages/boot/tools/supports.ts
Outdated
| /** @type {KVStore<Mailbox>} */ | ||
| const mailboxStorage = Object.assign(new Map(), { | ||
| getNextKey: previousKey => { | ||
| for (const k of mailboxStorage.keys()) { | ||
| if (k > previousKey) return k; | ||
| } | ||
| }, | ||
| }); |
There was a problem hiding this comment.
yikes, why do we need this ugly inefficient appendage? And why wasn't it needed before? If it's just to satisfy types, can we add a "not implemented" throwing implementation like
agoric-sdk/packages/cosmic-swingset/src/helpers/bufferedStorage.js
Lines 174 to 176 in 55a8847
There was a problem hiding this comment.
Yeah, now simplified to just a ts-expect-error.
My understanding is that it's not actually breaking anyone. Let's drop the |
…ke stores Notably, chain-main.js passes a non-map mailboxStorage to `launch`, which threads it into mailbox code.
gibson042
force-pushed
the
gibson-2024-10-mailbox-types
branch
from
4128aef
to
6822daf
Compare
gibson042
changed the title
(split from #10165)
Description
chain-main.js passes a non-map
mailboxStoragetolaunch, which threads it into mailbox code that returns methods expecting a map-likeentriesmethod andsizeproperty. This PR adds type data to reject such issues and refactors mailbox code accordingly by converting those methods into independent functions that accept appropriate backing maps.Security Considerations
None known.
Scaling Considerations
n/a
Documentation Considerations
n/a
Testing Considerations
Existing tests should continue to pass.
Upgrade Considerations
This code does not affect any vat and is safe to include in the next release.