refactor(ertp): ertp on zones #7116

erights · 2023-03-05T21:08:31Z

closes: #XXXX
refs: #8021 #8965 #8977 #8779

Description

ERTP predates both durability and zones. Before zones, it was made durable using baggage-based abstractions. We want to start shifting in general from baggage-based durability to zone-based parameterizability, so that the same parameterizable abstraction can be run in different storage regimes (heap, virtual, durable).

However, the public exports of @agoric/ertp are not changed by this PR. These are the issuer making / preparing / upgrading functions to be invoked from outside @agoric/ertp, and remain parameterized by baggage as of this PR. Later PRs may also directly expose functions parameterized by zones, but this PR does not take that step. Thus, this PR by itself does not yet enable users of @agoric/ertp to parameterize issuer creation by zone.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

Aside from upgrade testing, nothing special. Because the publicly exported API of @agoric/ertp has not changed, nothing outside of this package needs new tests. The fact that CI runs without such modification supports that.

Likewise though less obviously for the tests inside @agoric/ertp. The fact that the change to internal APIs did not cause us to revise any tests within the package means that there are no unit tests that are using those internal APIs. A bit of a surprise, but not cause to change that in this PR.

Upgrade Considerations

We believe we are using zones in a sufficiently literal analog to how we used to use baggage that this PR should not cause or need any change to durable state. If true, it should have no effect on upgrade. But this has not yet been tested. I would appreciate help in figuring out how to test that this PR is upgrade neutral.

datadog-full-agoric · 2023-03-05T21:15:29Z

Datadog Report

Branch report: markm-ertp-on-zones
Commit report: c333a19

✅ agoric-sdk: 0 Failed, 0 New Flaky, 3662 Passed, 57 Skipped, 15m 52.47s Wall Time

packages/ERTP/src/issuerKit.js

packages/ERTP/src/paymentLedger.js

datadog-full-agoric · 2023-03-20T01:25:07Z

Datadog Report

Branch report: markm-ertp-on-zones
Commit report: a90cc99

✅ agoric-sdk: 0 Failed, 0 New Flaky, 3757 Passed, 53 Skipped, 16m 39.39s Wall Time

erights · 2024-02-20T20:55:50Z

packages/ERTP/src/payment.js

- */
-/** @typedef {import('@agoric/zone').Zone} Zone */
-
+// TODO Type InterfaceGuard better than InterfaceGuard<any>


Need help. Finally resorted to any after everything else I tried failed. I don't understand why this problem arose in this PR but not prior to this PR.

I got rid of all the @typedefs in case it could help with this, which it did not. But I left the non-@typedef form in anyway because it is now our recommended practice. Preserves type info better in the IDE.

erights · 2024-02-20T20:56:05Z

packages/ERTP/src/purse.js

 const { Fail } = assert;

+// TODO Type InterfaceGuard better than InterfaceGuard<any>


Need help. Finally resorted to any after everything else I tried failed. I don't understand why this problem arose in this PR but not prior to this PR.

I got rid of all the @typedefs in case it could help with this, which it did not. But I left the non-@typedef form in anyway because it is now our recommended practice. Preserves type info better in the IDE.

packages/ERTP/src/types-ambient.js

Chris-Hibbert

LGTM.

one question that's more about zones and doc than about this PR.

Chris-Hibbert · 2024-02-23T23:41:56Z

packages/ERTP/src/purse.js

-      const recoverySet = makeScalarBigSetStore('recovery set', {
-        durable: true,
-      });
+      const recoverySet = issuerZone.detached().setStore('recovery set');


What does detached() do? Is there any documentation?

detached() is how you ask that a new one always be made, even if it is a durable zone. Without detach(), issuerZone.setStore('recovery set') would reuse a 'recovery set' entry in issuerZone if there is one, and register the newly made one there otherwise. IOW, detached() is the new way to say make*SetStore rather than provide*SetStore.

@michaelfig , I leave the documentation question to you. Thanks.

erights requested a review from michaelfig March 5, 2023 21:08

erights self-assigned this Mar 5, 2023

erights force-pushed the markm-ertp-on-zones branch from 8ac1531 to 241ec12 Compare March 6, 2023 02:19

erights changed the base branch from master to mfig-zones March 6, 2023 02:19

erights force-pushed the markm-ertp-on-zones branch 2 times, most recently from 1ec6ade to e1b781a Compare March 6, 2023 18:55

This was referenced Mar 6, 2023

WIP refactor(ertp): use places in ertp #6915

Closed

feat(zone): abstract heap, virtual, and durable object interface #7057

Merged

michaelfig force-pushed the mfig-zones branch 5 times, most recently from ba481f6 to 667bfb0 Compare March 19, 2023 22:20

Base automatically changed from mfig-zones to master March 19, 2023 23:01

erights force-pushed the markm-ertp-on-zones branch from 5e44278 to 25ac290 Compare March 20, 2023 01:17

erights marked this pull request as ready for review March 20, 2023 01:17

erights commented Mar 20, 2023

View reviewed changes

packages/ERTP/src/issuerKit.js Show resolved Hide resolved

erights commented Mar 20, 2023

View reviewed changes

packages/ERTP/src/issuerKit.js Show resolved Hide resolved

erights commented Mar 20, 2023

View reviewed changes

packages/ERTP/src/paymentLedger.js Show resolved Hide resolved

erights commented Mar 20, 2023

View reviewed changes

packages/ERTP/src/paymentLedger.js Outdated Show resolved Hide resolved

erights changed the title ~~WIP refactor(ertp): ertp on zones~~ refactor(ertp): ertp on zones Mar 20, 2023

erights force-pushed the markm-ertp-on-zones branch from 25ac290 to c5b54db Compare May 10, 2023 05:12

erights changed the base branch from master to markm-zone-once May 10, 2023 05:12

erights force-pushed the markm-ertp-on-zones branch from a0ef385 to 2c74cfc Compare May 10, 2023 05:27

erights requested a review from mhofman May 10, 2023 05:29

erights added ERTP package: ERTP stores labels May 12, 2023

erights force-pushed the markm-zone-once branch from fb0def0 to ce9d1b8 Compare May 20, 2023 22:22

erights force-pushed the markm-ertp-on-zones branch 2 times, most recently from eeae1bd to 46ca164 Compare February 20, 2024 20:51

erights commented Feb 20, 2024

View reviewed changes

packages/ERTP/src/types-ambient.js Show resolved Hide resolved

erights force-pushed the markm-ertp-on-zones branch 2 times, most recently from f7246b4 to cf5b231 Compare February 22, 2024 23:03

erights changed the base branch from master to markm-move-revocability-to-base-zone February 22, 2024 23:03

erights requested review from dckc and Chris-Hibbert February 22, 2024 23:27

dckc removed their request for review February 23, 2024 15:43

erights force-pushed the markm-move-revocability-to-base-zone branch from af26446 to b252dcb Compare February 23, 2024 20:36

erights force-pushed the markm-ertp-on-zones branch from cf5b231 to bb984c9 Compare February 23, 2024 20:38

erights changed the base branch from markm-move-revocability-to-base-zone to markm-virtual-exo-meta-ops February 23, 2024 20:57

erights force-pushed the markm-ertp-on-zones branch from bb984c9 to 8b6fd8b Compare February 23, 2024 22:46

erights changed the base branch from markm-virtual-exo-meta-ops to markm-move-revocability-to-base-zone-2 February 23, 2024 22:47

Chris-Hibbert approved these changes Feb 23, 2024

View reviewed changes

erights force-pushed the markm-ertp-on-zones branch from 8b6fd8b to a7e3695 Compare February 24, 2024 00:11

erights changed the base branch from markm-move-revocability-to-base-zone-2 to master February 24, 2024 00:12

erights added automerge:squash Automatically squash merge and removed automerge:squash Automatically squash merge labels Feb 24, 2024

erights mentioned this pull request Feb 24, 2024

refactor(zoe,base-zone): move prepare-revocable to base-zone helper lib #8965

Merged

erights force-pushed the markm-ertp-on-zones branch from a7e3695 to 6558152 Compare February 24, 2024 02:01

erights added 3 commits February 23, 2024 18:01

refactor(ertp): ertp on zones

3e43dee

fixup! better and worse typing

0e10dfa

fixup! format

85e997d

erights force-pushed the markm-ertp-on-zones branch from 6558152 to 85e997d Compare February 24, 2024 02:02

erights added the automerge:squash Automatically squash merge label Feb 24, 2024

mergify bot merged commit be6d970 into master Feb 24, 2024
76 checks passed

mergify bot deleted the markm-ertp-on-zones branch February 24, 2024 02:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ertp): ertp on zones #7116

refactor(ertp): ertp on zones #7116

erights commented Mar 5, 2023 •

edited

Loading

datadog-full-agoric bot commented Mar 5, 2023 •

edited

Loading

datadog-full-agoric bot commented Mar 20, 2023 •

edited

Loading

erights Feb 20, 2024

erights Feb 20, 2024

Chris-Hibbert left a comment

Chris-Hibbert Feb 23, 2024

erights Feb 23, 2024

		const { Fail } = assert;

		// TODO Type InterfaceGuard better than InterfaceGuard<any>

refactor(ertp): ertp on zones #7116

refactor(ertp): ertp on zones #7116

Conversation

erights commented Mar 5, 2023 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

datadog-full-agoric bot commented Mar 5, 2023 • edited Loading

Datadog Report

datadog-full-agoric bot commented Mar 20, 2023 • edited Loading

Datadog Report

erights Feb 20, 2024

Choose a reason for hiding this comment

erights Feb 20, 2024

Choose a reason for hiding this comment

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Chris-Hibbert Feb 23, 2024

Choose a reason for hiding this comment

erights Feb 23, 2024

Choose a reason for hiding this comment

erights commented Mar 5, 2023 •

edited

Loading

datadog-full-agoric bot commented Mar 5, 2023 •

edited

Loading

datadog-full-agoric bot commented Mar 20, 2023 •

edited

Loading