feat(zone)!: implement `zone.makeOnce(key, maker)` #7891

michaelfig · 2023-06-06T20:07:47Z

closes: #7663

Description

Implements zone.makeOnce(key, maker) in a similar spirit to #7663, but with verification that it is called only once per key,zone,incarnation.

Also moves the implementation to Zone. The original once implementation was a method on Stores, but they might be detached (i.e. have no backing store) and thereby not need key usage tracking.

Security Considerations

A breaking change in this PR was necessary: heapZone and virtualZone became new vestiges of global mutable state since they each had a new namespace (the used keys) associated with them. This was not Jessie-safe and introduced many name conflicts in actual code (especially in concurrent testing), so I removed them and replaced them with makeHeapZone() and makeVirtualZone() to make them more POLA-friendly and segregate their scope.

Scaling Considerations

Documentation Considerations

Testing Considerations

packages/zone/src/make-once.js

erights · 2023-06-07T02:47:30Z

A breaking change in this PR was necessary

Agreed. But doesn't this mean the title should be feat(zone)! ... ?

michaelfig · 2023-06-07T02:53:21Z

A breaking change in this PR was necessary

Agreed. But doesn't this mean the title should be feat(zone)! ... ?

Yes. When I squash the commits (one of them has the breaking change bang) I'll also update the commit messages.

packages/zone/src/make-once.js

packages/zone/src/virtual.js

packages/zone/src/heap.js

packages/zone/src/durable.js

packages/zone/src/virtual.js

packages/vats/src/vat-agoricNames.js

packages/zone/src/make-once.js

packages/zone/src/durable.js

packages/zone/src/heap.js

erights

More testing? I see only test-make-once.js . IIUC, it tests only makeOnce and isStorable.

I mean, not just in this PR, but in the zone/test directory that would result from this PR. IOW, prior testing situation was thin.

erights · 2023-06-21T03:01:11Z

See endojs/endo#1648 , which I assigned to myself.

I will continue the review under the assumption we expect endojs/endo#1648 to be fixed shortly after this PR. Thus, this PR should behave sensibly both before and afterwards, but be designed for the world in which endojs/endo#1648 has been fixed.

mhofman

I am concerned about the upgradability of this change, but then I don't really know how to solve the problem either.

I think we half painted ourselves in a corner with the prepare and zone abstractions.

At the lowest level, kind handles are the unique identity representing a durable class definition, and it's up to the contract code to stash it in baggage and reconnect the definitions on restart. The code has full control over the process.

What the prepare and zone abstractions do is hide the handles and automatically store them in a location derived from the definition. However this removes the flexibility to restructure the code and update the definition, such as renaming the class, or switching from the provide pattern to the zone mechanism.

We should have a discussion about what transition use case we want to support, and how.

packages/zone/src/durable.js

erights

Found some problems that I think need to be fixed. However, I'm too tired now to try to write them up. It looks like I'll be able to focus on this for most of tomorrow.

Requesting changes for now just as an interlock, so it's not accidentally merged before I write these down.

erights

See my suggestions at #7971

erights

Sorry, somehow I thought I'd already approved this.

LGTM!!

michaelfig · 2023-07-02T20:33:56Z

Putting back into draft until more tests are written.

erights · 2023-07-05T02:26:03Z

#7116 raises an additional question about this PR:

When switching durable state in the obvious manner from a pre-zone baggage-based API to a zone-based one, is the resulting durable state exactly the same? If not, what are the implications for upgrade? Should we have a test for upgrade of code using pre-zone baggage to zone, such as #7116 ? How would we write such a test?

Attn @warner @FUDCo @dckc

…port.js`

erights

LGTM, thanks!

Looking forward to using this a lot!

erights · 2023-07-25T21:49:28Z

packages/zone/src/make-once.js

+   * Ensure the wrapped function is only called once per incarnation.  It is
+   * expected to update the backing store directly.


also, per wrapper, where each call to wrapProvider creates a fresh wrapper. I want to avoid the misunderstanding that the "ensure" is based on the identity of the provide function.

Good point. I'll make note of that next time I'm in the vicinity.

feat(zone)!: implement `zone.makeOnce(key, maker)`

michaelfig self-assigned this Jun 6, 2023

michaelfig added the contract-upgrade label Jun 6, 2023

mhofman reviewed Jun 6, 2023

View reviewed changes

packages/zone/src/make-once.js Outdated Show resolved Hide resolved

michaelfig marked this pull request as ready for review June 7, 2023 02:16

michaelfig requested review from mhofman and erights June 7, 2023 02:16

michaelfig changed the title ~~feat(zone): implement zone.makeOnce(key, maker)~~ feat(zone)!: implement zone.makeOnce(key, maker) Jun 7, 2023

erights reviewed Jun 7, 2023

View reviewed changes

mhofman reviewed Jun 8, 2023

View reviewed changes

packages/vats/src/vat-agoricNames.js Outdated Show resolved Hide resolved

packages/zone/src/make-once.js Outdated Show resolved Hide resolved

packages/zone/src/durable.js Show resolved Hide resolved

packages/zone/src/heap.js Outdated Show resolved Hide resolved

This was referenced Jun 8, 2023

feat(zone): once is provide for zones #7663

Closed

refactor(ertp): ertp on zones #7116

Merged

fix(vat-data)!: diagnose provide collisions #5875

Closed

michaelfig requested review from mhofman and erights June 16, 2023 01:59

erights reviewed Jun 20, 2023

View reviewed changes

erights mentioned this pull request Jun 21, 2023

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

Open

mhofman reviewed Jun 21, 2023

View reviewed changes

packages/zone/src/durable.js Show resolved Hide resolved

erights requested changes Jun 22, 2023

View reviewed changes

erights added a commit that referenced this pull request Jun 22, 2023

fix(zone): suggestions for #7891

e2cdd5c

erights mentioned this pull request Jun 22, 2023

fix(zone): suggestions for #7891 zone makeOnce etc #7971

Merged

erights requested changes Jun 22, 2023

View reviewed changes

michaelfig pushed a commit that referenced this pull request Jun 23, 2023

fix(zone): suggestions for #7891

00171d0

michaelfig requested a review from erights June 27, 2023 21:40

erights approved these changes Jun 30, 2023

View reviewed changes

michaelfig marked this pull request as draft July 2, 2023 20:33

michaelfig pushed a commit that referenced this pull request Jul 19, 2023

fix(zone): suggestions for #7891

2cb8913

michaelfig force-pushed the mfig-zone-make-once branch 3 times, most recently from 990098b to 0e6ce59 Compare July 19, 2023 22:28

michaelfig mentioned this pull request Jul 19, 2023

Refactor liveslots vatstore usage #8066

Merged

Base automatically changed from 8012-refactor-liveslots-vatstore-usage to master July 19, 2023 23:50

michaelfig and others added 14 commits July 25, 2023 13:28

feat(zone): implement zone.makeOnce(key, maker)

d3be4c0

feat(zone)!: use fresh heap and virtual zones

7a1a411

fix(zone): add wrapProvider and manage backingStore

5e3f6a6

fix(zone): track baggage keys as used by @agoric/vat-data

ac92686

feat(zone): implement isStorable for virtual zones

20feefb

fix(zone): suggestions for #7891

e9e0e21

fix(zone): fixups before merging to 7891

9bbb393

fix(zone): no longer getting M from @agoric/zone

d68bc84

fix: review suggestions

ea28367

fix(zone): review suggestion

2ca7943

test(zone): add more tests

989becb

test(setup-vat-data): move to SwingSet/tools

fe72e6b

test(swingset-liveslots): properly registerEntry in `fakeVirtualSup…

670f0d7

…port.js`

test(swingset-liveslots): better capture WeakMap and WeakSet

dc8eb4a

michaelfig force-pushed the mfig-zone-make-once branch from 0e6ce59 to dc8eb4a Compare July 25, 2023 19:39

erights approved these changes Jul 25, 2023

View reviewed changes

michaelfig added this pull request to the merge queue Jul 25, 2023

Merged via the queue into master with commit 8baf0aa Jul 25, 2023
59 checks passed

michaelfig deleted the mfig-zone-make-once branch July 25, 2023 22:36

mhofman pushed a commit that referenced this pull request Aug 7, 2023

fix(zone): suggestions for #7891

f4ef40b

mhofman pushed a commit that referenced this pull request Aug 7, 2023

Merge pull request #7891 from Agoric/mfig-zone-make-once

fbbe5cb

feat(zone)!: implement `zone.makeOnce(key, maker)`

anilhelvaci pushed a commit to anilhelvaci/agoric-sdk that referenced this pull request Aug 16, 2023

fix(zone): suggestions for Agoric#7891

613013b

anilhelvaci pushed a commit to Jorge-Lopes/agoric-sdk that referenced this pull request Feb 16, 2024

fix(zone): suggestions for Agoric#7891

7c268d3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(zone)!: implement `zone.makeOnce(key, maker)` #7891

feat(zone)!: implement `zone.makeOnce(key, maker)` #7891

michaelfig commented Jun 6, 2023 •

edited

Loading

erights commented Jun 7, 2023

michaelfig commented Jun 7, 2023

erights left a comment •

edited

Loading

erights commented Jun 21, 2023

mhofman left a comment

erights left a comment •

edited

Loading

erights left a comment

erights left a comment

michaelfig commented Jul 2, 2023

erights commented Jul 5, 2023

erights left a comment

erights Jul 25, 2023

michaelfig Jul 25, 2023

		* Ensure the wrapped function is only called once per incarnation. It is
		* expected to update the backing store directly.

feat(zone)!: implement zone.makeOnce(key, maker) #7891

feat(zone)!: implement zone.makeOnce(key, maker) #7891

Conversation

michaelfig commented Jun 6, 2023 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

erights commented Jun 7, 2023

michaelfig commented Jun 7, 2023

erights left a comment • edited Loading

Choose a reason for hiding this comment

erights commented Jun 21, 2023

mhofman left a comment

Choose a reason for hiding this comment

erights left a comment • edited Loading

Choose a reason for hiding this comment

erights left a comment

Choose a reason for hiding this comment

erights left a comment

Choose a reason for hiding this comment

michaelfig commented Jul 2, 2023

erights commented Jul 5, 2023

erights left a comment

Choose a reason for hiding this comment

erights Jul 25, 2023

Choose a reason for hiding this comment

michaelfig Jul 25, 2023

Choose a reason for hiding this comment

feat(zone)!: implement `zone.makeOnce(key, maker)` #7891

feat(zone)!: implement `zone.makeOnce(key, maker)` #7891

michaelfig commented Jun 6, 2023 •

edited

Loading

erights left a comment •

edited

Loading

erights left a comment •

edited

Loading