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.

Sign up for GitHub

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

Jump to bottom

Inconsistently, heap exo object state can store non-Passables. #1648

Open
erights opened this issue Jun 21, 2023 · 1 comment
Open

Inconsistently, heap exo object state can store non-Passables. #1648

erights opened this issue Jun 21, 2023 · 1 comment
Assignees
@erights
Labels
kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024

Comments

@erights
Copy link
Contributor

erights commented Jun 21, 2023

I noticed while reviewing Agoric/agoric-sdk#7891 and thinking about zone.isStorable.

The premise of isStorable should be that if a value is storable for a given zone, then it can also be used as values in mapStores made by that zone, and be held in exo instance state variables. The current implementation of heap mapStores in @agoric/store does successfully enforce that it only stores Passable values. However, the heap exo class maker and class kit maker does not check that the values of the state variables are Passable. Thus

  • the heapZone has no one consistent answer to use for isStorable
  • refactoring between heap exos and virtual exos can be more surprising than refactoring from heap mapStores to virtual mapStores.

Further, while heap mapStores understand and enforce keyShape and valueShape, heap exos silently ignore stateShape. This again makes refactoring from, for example, heap to virtual more surprising for exos than for mapStores.
@erights erights self-assigned this Jun 21, 2023
This was referenced Jun 21, 2023
Merged
Merged
@kriskowal kriskowal added the kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 label Jan 8, 2024
@mhofman mhofman mentioned this issue Sep 30, 2024
Draft
@mhofman
Copy link
Contributor

mhofman commented Sep 30, 2024

First stab at this in Agoric/agoric-sdk#10170

@mhofman mhofman mentioned this issue Oct 5, 2024
Merged
mergify bot added a commit to Agoric/agoric-sdk that referenced this issue Oct 5, 2024
@mergify
Passable exo state (#10228)
7d1dfc0 
refs: #10170 
refs: endojs/endo#1648

best reviewed by ignoring white space

## Description
After implementing a change to enforce that heap exo are passable, I found that some things currently stored in heap exos, mostly in tests, are not actually passable. Almost all of them are remotable like objects not marked as `Far`, usually related to storage nodes.

While I plan on moving the implementation of the checks into endo instead of landing it in agoric-sdk first, I figured we should already fix the issues found.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
See #10170 for passing tests with checks enabled

### Upgrade Considerations
As it's a heap zone change, cross upgrade behavior is not involved. In the future any code using a stricter heap zone would need to comply. However heaps zones are used as packages bundled by the contract, and are not provided by liveslots or zcf, so there is no compatibility risk.
@mhofman mhofman mentioned this issue Oct 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@erights erights

Labels
kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kriskowal @mhofman @erights