From 0a320ba4686140f54a3fb2e52e1585ece1503841 Mon Sep 17 00:00:00 2001
From: Turadg Aleahmad <turadg@agoric.com>
Date: Wed, 2 Mar 2022 10:09:57 -0800
Subject: [PATCH] refactor(run-protocol): store liquidation cohort before
 liquidating (#4715)

* rename prioritizedVaults to illiquidVaults

* vaultsToLiquidate queue and attendant methods

* inline liquidateAndRemove

* back to prioritizedVaults

* code review suggestions

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 .../src/vaultFactory/vaultManager.js          | 89 ++++++++++++-------
 1 file changed, 55 insertions(+), 34 deletions(-)

diff --git a/packages/run-protocol/src/vaultFactory/vaultManager.js b/packages/run-protocol/src/vaultFactory/vaultManager.js
index ab5a3af51d6..760a9fedddf 100644
--- a/packages/run-protocol/src/vaultFactory/vaultManager.js
+++ b/packages/run-protocol/src/vaultFactory/vaultManager.js
@@ -16,6 +16,7 @@ import { makeNotifierKit, observeNotifier } from '@agoric/notifier';
 import { AmountMath } from '@agoric/ertp';
 import { Far } from '@endo/marshal';
 
+import { makeScalarBigMapStore } from '@agoric/swingset-vat/src/storeModule';
 import { makeInnerVault } from './vault.js';
 import { makePrioritizedVaults } from './prioritizedVaults.js';
 import { liquidate } from './liquidation.js';
@@ -101,13 +102,21 @@ export const makeVaultManager = (
 
   let vaultCounter = 0;
 
-  // A store for vaultKits prioritized by their collaterization ratio.
-  //
-  // It should be set only once but it's a `let` because it can't be set until after the
-  // definition of reschedulePriceCheck, which refers to sortedVaultKits
+  /**
+   * A store for vaultKits prioritized by their collaterization ratio.
+   *
+   * It should be set only once but it's a `let` because it can't be set until after the
+   * definition of reschedulePriceCheck, which refers to sortedVaultKits
+   *
+   * @type {ReturnType<typeof makePrioritizedVaults>=}
+   */
   // XXX misleading mutability and confusing flow control; could be refactored with a listener
-  /** @type {ReturnType<typeof makePrioritizedVaults>=} */
   let prioritizedVaults;
+
+  // Progress towards durability https://github.com/Agoric/agoric-sdk/issues/4568#issuecomment-1042346271
+  /** @type {MapStore<string, InnerVault>} */
+  const vaultsToLiquidate = makeScalarBigMapStore('vaultsToLiquidate');
+
   /** @type {MutableQuote=} */
   let outstandingQuote;
   /** @type {Amount<NatValue>} */
@@ -115,8 +124,10 @@ export const makeVaultManager = (
   /** @type {Ratio}} */
   let compoundedInterest = makeRatio(100n, runBrand); // starts at 1.0, no interest
 
-  // timestamp of most recent update to interest
-  /** @type {bigint} */
+  /**
+   * timestamp of most recent update to interest
+   * @type {bigint}
+   */
   let latestInterestUpdate = startTimeStamp;
 
   const { updater: assetUpdater, notifier: assetNotifer } = makeNotifierKit(
@@ -129,30 +140,43 @@ export const makeVaultManager = (
   );
 
   /**
-   *
-   * @param {[key: string, vaultKit: InnerVault]} record
+   * @param {Iterable<[key: string, vaultKit: InnerVault]>} vaultEntries
    */
-  const liquidateAndRemove = async ([key, vault]) => {
+  const enqueueToLiquidate = vaultEntries => {
     assert(prioritizedVaults);
-    trace('liquidating', vault.getVaultSeat().getProposal());
-
-    try {
-      // Start liquidation (vaultState: LIQUIDATING)
-      await liquidate(
-        zcf,
-        vault,
-        runMint.burnLosses,
-        liquidationStrategy,
-        collateralBrand,
-      );
-
-      await prioritizedVaults.removeVault(key);
-    } catch (e) {
-      // XXX should notify interested parties
-      console.error('liquidateAndRemove failed with', e);
+    for (const [k, v] of vaultEntries) {
+      vaultsToLiquidate.init(k, v);
+      prioritizedVaults.removeVault(k);
     }
   };
 
+  const executeLiquidation = async () => {
+    // Start all promises in parallel
+    // XXX we should have a direct method to map over entries
+    const liquidations = Array.from(vaultsToLiquidate.entries()).map(
+      async ([key, vault]) => {
+        trace('liquidating', vault.getVaultSeat().getProposal());
+
+        try {
+          // Start liquidation (vaultState: LIQUIDATING)
+          await liquidate(
+            zcf,
+            vault,
+            runMint.burnLosses,
+            liquidationStrategy,
+            collateralBrand,
+          );
+
+          vaultsToLiquidate.delete(key);
+        } catch (e) {
+          // XXX should notify interested parties
+          console.error('liquidateAndRemove failed with', e);
+        }
+      },
+    );
+    return Promise.all(liquidations);
+  };
+
   // When any Vault's debt ratio is higher than the current high-water level,
   // call reschedulePriceCheck() to request a fresh notification from the
   // priceAuthority. There will be extra outstanding requests since we can't
@@ -211,14 +235,13 @@ export const makeVaultManager = (
       getAmountIn(quote),
     );
 
-    /** @type {Array<Promise<void>>} */
-    const toLiquidate = Array.from(
+    enqueueToLiquidate(
       prioritizedVaults.entriesPrioritizedGTE(quoteRatioPlusMargin),
-    ).map(liquidateAndRemove);
+    );
 
     outstandingQuote = undefined;
     // Ensure all vaults complete
-    await Promise.all(toLiquidate);
+    await executeLiquidation();
 
     reschedulePriceCheck();
   };
@@ -227,10 +250,8 @@ export const makeVaultManager = (
   // In extreme situations, system health may require liquidating all vaults.
   const liquidateAll = async () => {
     assert(prioritizedVaults);
-    const toLiquidate = Array.from(prioritizedVaults.entries()).map(
-      liquidateAndRemove,
-    );
-    await Promise.all(toLiquidate);
+    enqueueToLiquidate(prioritizedVaults.entries());
+    await executeLiquidation();
   };
 
   /**