feat(ertp): upgraded quotes incrementally empty recovery-sets

packages/ERTP/src/issuerKit.js

@@ -1,6 +1,6 @@
 // @jessie-check
 
-import { assert } from '@agoric/assert';
+import { assert, Fail } from '@agoric/assert';
 import { assertPattern } from '@agoric/store';
 import { makeScalarBigMapStore } from '@agoric/vat-data';
 
@@ -28,6 +28,8 @@ import './types-ambient.js';
  *
  * @template {AssetKind} K
  * @param {IssuerRecord<K>} issuerRecord
+ * @param {RecoverySetsOption} recoverySetsState Omitted from issuerRecord
+ *   because it was added in an upgrade.
  * @param {Baggage} issuerBaggage
  * @param {ShutdownWithFailure} [optShutdownWithFailure] If this issuer fails in
  *   the middle of an atomic action (which btw should never happen), it
@@ -40,6 +42,7 @@ import './types-ambient.js';
  */
 const setupIssuerKit = (
   { name, assetKind, displayInfo, elementShape },
+  recoverySetsState,
   issuerBaggage,
   optShutdownWithFailure = undefined,
 ) => {
@@ -65,6 +68,7 @@ const setupIssuerKit = (
     assetKind,
     cleanDisplayInfo,
     elementShape,
+    recoverySetsState,
     optShutdownWithFailure,
   );
 
@@ -80,6 +84,12 @@ harden(setupIssuerKit);
 
 /** The key at which the issuer record is stored. */
 const INSTANCE_KEY = 'issuer';
+/**
+ * The key at which the issuerKit's `RecoverySetsOption` state is stored.
+ * Introduced by an upgrade, so may be absent on a predecessor incarnation. See
+ * `RecoverySetsOption` for defaulting behavior.
+ */
+const RECOVERY_SETS_STATE = 'recoverySetsState';
 
 /**
  * Used _only_ to upgrade a predecessor issuerKit. Use `makeDurableIssuerKit` to
@@ -94,14 +104,32 @@ const INSTANCE_KEY = 'issuer';
  *   unit of computation, like the enclosing vat, can be shutdown before
  *   anything else is corrupted by that corrupted state. See
  *   https://github.com/Agoric/agoric-sdk/issues/3434
+ * @param {RecoverySetsOption} [recoverySetsOption] Added in upgrade, so last
+ *   and optional. See `RecoverySetsOption` for defaulting behavior.
  * @returns {IssuerKit<K>}
  */
 export const upgradeIssuerKit = (
   issuerBaggage,
   optShutdownWithFailure = undefined,
+  recoverySetsOption = undefined,
 ) => {
   const issuerRecord = issuerBaggage.get(INSTANCE_KEY);
-  return setupIssuerKit(issuerRecord, issuerBaggage, optShutdownWithFailure);
+  const oldRecoverySetsState = issuerBaggage.has(RECOVERY_SETS_STATE)
+    ? issuerBaggage.get(RECOVERY_SETS_STATE)
+    : 'hasRecoverySets';
+  if (
+    oldRecoverySetsState === 'noRecoverySets' &&
+    recoverySetsOption === 'hasRecoverySets'
+  ) {
+    Fail`Cannot (yet?) upgrade from 'noRecoverySets' to 'hasRecoverySets'`;
+  }
+  const recoverySetsState = recoverySetsOption || oldRecoverySetsState;
+  return setupIssuerKit(
+    issuerRecord,
+    recoverySetsState,
+    issuerBaggage,
+    optShutdownWithFailure,
+  );
 };
 harden(upgradeIssuerKit);
 
@@ -121,8 +149,14 @@ export const hasIssuer = baggage => baggage.has(INSTANCE_KEY);
  * typically, the amount of an invitation payment is a singleton set. Such a
  * payment is often referred to in the singular as "an invitation".)
  *
+ * `recoverySetsOption` added in upgrade. Note that `IssuerOptionsRecord` is
+ * never stored, so we never need to worry about inheriting one from a
+ * predecessor predating the introduction of recovery sets. See
+ * `RecoverySetsOption` for defaulting behavior.
+ *
  * @typedef {Partial<{
  *   elementShape: Pattern;
+ *   recoverySetsOption: RecoverySetsOption;
  * }>} IssuerOptionsRecord
  */
 
@@ -163,11 +197,23 @@ export const makeDurableIssuerKit = (
   assetKind = AssetKind.NAT,
   displayInfo = harden({}),
   optShutdownWithFailure = undefined,
-  { elementShape = undefined } = {},
+  { elementShape = undefined, recoverySetsOption = undefined } = {},
 ) => {
-  const issuerData = harden({ name, assetKind, displayInfo, elementShape });
+  const issuerData = harden({
+    name,
+    assetKind,
+    displayInfo,
+    elementShape,
+  });
   issuerBaggage.init(INSTANCE_KEY, issuerData);
-  return setupIssuerKit(issuerData, issuerBaggage, optShutdownWithFailure);
+  const recoverySetsState = recoverySetsOption || 'hasRecoverySets';
+  issuerBaggage.init(RECOVERY_SETS_STATE, recoverySetsState);
+  return setupIssuerKit(
+    issuerData,
+    recoverySetsState,
+    issuerBaggage,
+    optShutdownWithFailure,
+  );
 };
 harden(makeDurableIssuerKit);
 
@@ -211,12 +257,19 @@ export const prepareIssuerKit = (
   options = {},
 ) => {
   if (hasIssuer(issuerBaggage)) {
-    const { elementShape: _ = undefined } = options;
-    const issuerKit = upgradeIssuerKit(issuerBaggage, optShutdownWithFailure);
+    const { elementShape: _ = undefined, recoverySetsOption = undefined } =
+      options;
+    const issuerKit = upgradeIssuerKit(
+      issuerBaggage,
+      optShutdownWithFailure,
+      recoverySetsOption,
+    );
 
     // TODO check consistency with name, assetKind, displayInfo, elementShape.
     // Consistency either means that these are the same, or that they differ
-    // in a direction we are prepared to upgrade.
+    // in a direction we are prepared to upgrade. Note that it is the
+    // responsibility of `upgradeIssuerKit` to check consistency of
+    // `recoverySetsOption`, so continue to not do that here.
 
     // @ts-expect-error Type parameter confusion.
     return issuerKit;
@@ -274,14 +327,14 @@ export const makeIssuerKit = (
   assetKind = AssetKind.NAT,
   displayInfo = harden({}),
   optShutdownWithFailure = undefined,
-  { elementShape = undefined } = {},
+  { elementShape = undefined, recoverySetsOption = undefined } = {},
 ) =>
   makeDurableIssuerKit(
     makeScalarBigMapStore('dropped issuer kit', { durable: true }),
     name,
     assetKind,
     displayInfo,
     optShutdownWithFailure,
-    { elementShape },
+    { elementShape, recoverySetsOption },
   );
 harden(makeIssuerKit);

packages/ERTP/src/paymentLedger.js

@@ -2,7 +2,7 @@
 
 /* eslint-disable no-use-before-define */
 import { isPromise } from '@endo/promise-kit';
-import { mustMatch, M, keyEQ } from '@agoric/store';
+import { mustMatch, M, keyEQ } from '@endo/patterns';
 import {
   provideDurableWeakMapStore,
   prepareExo,
@@ -79,6 +79,7 @@ const amountShapeFromElementShape = (brand, assetKind, elementShape) => {
  * @param {K} assetKind
  * @param {DisplayInfo<K>} displayInfo
  * @param {Pattern} elementShape
+ * @param {RecoverySetsOption} recoverySetsState
  * @param {ShutdownWithFailure} [optShutdownWithFailure]
  * @returns {PaymentLedger<K>}
  */
@@ -88,6 +89,7 @@ export const preparePaymentLedger = (
   assetKind,
   displayInfo,
   elementShape,
+  recoverySetsState,
   optShutdownWithFailure = undefined,
 ) => {
   /** @type {Brand<K>} */
@@ -162,6 +164,12 @@ export const preparePaymentLedger = (
    * - A purse's recovery set only contains payments withdrawn from that purse and
    *   not yet consumed.
    *
+   * If `recoverySetsState === 'noRecoverySets'`, then nothing should ever be
+   * added to this WeakStore. If upgrading from a previous state with recovery
+   * sets, whether implicitly or explicitly, then this WeakStore should
+   * eventually become empty. But because this store is weak, the responsibility
+   * for emptying it out lies elsewhere (purse.js).
+   *
    * @type {WeakMapStore<Payment, SetStore<Payment>>}
    */
   const paymentRecoverySets = provideDurableWeakMapStore(
@@ -178,7 +186,10 @@ export const preparePaymentLedger = (
    * @param {SetStore<Payment>} [optRecoverySet]
    */
   const initPayment = (payment, amount, optRecoverySet = undefined) => {
-    if (optRecoverySet !== undefined) {
+    if (recoverySetsState === 'noRecoverySets') {
+      assert(optRecoverySet === undefined);
+    }
+    if (optRecoverySet !== undefined && !AmountMath.isEmpty(amount)) {
       optRecoverySet.add(payment);
       paymentRecoverySets.init(payment, optRecoverySet);
     }
@@ -283,14 +294,14 @@ export const preparePaymentLedger = (
    * @param {(newPurseBalance: Amount) => void} updatePurseBalance - commit the
    *   purse balance
    * @param {Amount} amount - the amount to be withdrawn
-   * @param {SetStore<Payment>} recoverySet
+   * @param {SetStore<Payment>} [recoverySet]
    * @returns {Payment}
    */
   const withdrawInternal = (
     currentBalance,
     updatePurseBalance,
     amount,
-    recoverySet,
+    recoverySet = undefined,
   ) => {
     amount = coerce(amount);
     AmountMath.isGTE(currentBalance, amount) ||
@@ -322,6 +333,8 @@ export const preparePaymentLedger = (
       depositInternal,
       withdrawInternal,
     }),
+    recoverySetsState,
+    paymentRecoverySets,
   );
 
   /** @type {Issuer<K>} */

packages/ERTP/src/purse.js

@@ -1,4 +1,4 @@
-import { M } from '@agoric/store';
+import { M, makeCopySet } from '@endo/patterns';
 import { prepareExoClassKit, makeScalarBigSetStore } from '@agoric/vat-data';
 import { AmountMath } from './amountMath.js';
 import { makeTransientNotifierKit } from './transientNotifier.js';
@@ -9,6 +9,8 @@ import { makeTransientNotifierKit } from './transientNotifier.js';
 
 const { Fail } = assert;
 
+const EMPTY_COPY_SET = makeCopySet([]);
+
 /**
  * @param {Baggage} issuerBaggage
  * @param {string} name
@@ -22,6 +24,8 @@ const { Fail } = assert;
  *   depositInternal: any;
  *   withdrawInternal: any;
  * }} purseMethods
+ * @param {RecoverySetsOption} recoverySetsState
+ * @param {WeakMapStore<Payment, SetStore<Payment>>} paymentRecoverySets
  */
 export const preparePurseKind = (
   issuerBaggage,
@@ -30,6 +34,8 @@ export const preparePurseKind = (
   brand,
   PurseIKit,
   purseMethods,
+  recoverySetsState,
+  paymentRecoverySets,
 ) => {
   const amountShape = brand.getAmountShape();
 
@@ -42,6 +48,59 @@ export const preparePurseKind = (
     updateBalance(purse, purse.getCurrentAmount());
   };
 
+  /**
+   * How many payments to clean out of the recoverySet on each call to
+   * `cleanerRecoverySet`.
+   */
+  const CLEANING_BUDGET = 10;
+
+  /**
+   * If `recoverySetsState === 'hasRecoverySets'` (the normal state), then just
+   * return `state.recoverySet`.
+   *
+   * If `recoverySetsState === 'noRecoverySets'`, then first delete up to
+   * `CLEANING_BUDGET` payments from `state.recoverySet`, to eventually clean it
+   * out. Then return `undefined`. Callers must be aware that the `undefined`
+   * return happens iff `recoverySetsState === 'noRecoverySets'`, and to avoid
+   * storing or retrieving anything from the actual recovery set.
+   *
+   * @param {{ recoverySet: SetStore<Payment> }} state
+   * @returns {SetStore<Payment> | undefined}
+   */
+  const cleanerRecoverySet = state => {
+    const { recoverySet } = state;
+    if (recoverySetsState === 'hasRecoverySets') {
+      return recoverySet;
+    } else {
+      assert(recoverySetsState === 'noRecoverySets');
+      assert(paymentRecoverySets !== undefined);
+      let i = 0;
+      for (const payment of recoverySet.keys()) {
+        if (i >= CLEANING_BUDGET) {
+          break;
+        }
+        i += 1;
+        // The stateShape constraint and the current lack of support for schema
+        // upgrade means that we cannot upgrade `state.recoverySet` to
+        // `undefined` or any non-remotable.
+        //
+        // At the time of this writing, SwingSet's liveSlots package does not
+        // yet incrementalize the gc work of virtual and durable objects.
+        // To avoid depending on that, this code does the incremental removal
+        // of payments from recovery sets here. Doing so means that the cleanup
+        // only happens when touched, which would be a potential problem if
+        // an idle purse's recovery set held onto a lot of unneeded payments.
+        // However, we currently only have this problem for quote issuers,
+        // which we know store minted payments only in the mintRecoveryPurse's
+        // recovery purse, which we also know to be perpetually active.
+        assert(paymentRecoverySets.get(payment) === recoverySet);
+        paymentRecoverySets.delete(payment);
+        recoverySet.delete(payment);
+      }
+      return undefined;
+    }
+  };
+
   // - This kind is a pair of purse and depositFacet that have a 1:1
   //   correspondence.
   // - They are virtualized together to share a single state record.
@@ -83,13 +142,14 @@ export const preparePurseKind = (
         },
         withdraw(amount) {
           const { state } = this;
+          const optRecoverySet = cleanerRecoverySet(state);
           // Note COMMIT POINT within withdraw.
           return withdrawInternal(
             state.currentBalance,
             newPurseBalance =>
               updatePurseBalance(state, newPurseBalance, this.facets.purse),
             amount,
-            state.recoverySet,
+            optRecoverySet,
           );
         },
         getCurrentAmount() {
@@ -107,18 +167,29 @@ export const preparePurseKind = (
         },
 
         getRecoverySet() {
-          return this.state.recoverySet.snapshot();
+          const { state } = this;
+          const optRecoverySet = cleanerRecoverySet(state);
+          if (optRecoverySet === undefined) {
+            return EMPTY_COPY_SET;
+          }
+          return optRecoverySet.snapshot();
         },
         recoverAll() {
           const { state, facets } = this;
           let amount = AmountMath.makeEmpty(brand, assetKind);
-          for (const payment of state.recoverySet.keys()) {
+          const optRecoverySet = cleanerRecoverySet(state);
+          if (optRecoverySet === undefined) {
+            // Note that even this case does only the gc work implied by the
+            // call to `cleanerRecoverySet` above.
+            return amount; // empty at this time
+          }
+          for (const payment of optRecoverySet.keys()) {
             // This does cause deletions from the set while iterating,
             // but this special case is allowed.
             const delta = facets.purse.deposit(payment);
             amount = AmountMath.add(amount, delta, brand);
           }
-          state.recoverySet.getSize() === 0 ||
+          optRecoverySet.getSize() === 0 ||
             Fail`internal: Remaining unrecovered payments: ${facets.purse.getRecoverySet()}`;
           return amount;
         },