vault manager refactorings

packages/run-protocol/src/vaultFactory/liquidation.js

@@ -18,17 +18,13 @@ const trace = makeTracer('LIQ', false);
  *
  * @param {ZCF} zcf
  * @param {Vault} vault
- * @param {(losses: Amount,
- *             zcfSeat: ZCFSeat
- *            ) => void} burnLosses
  * @param {Liquidator}  liquidator
  * @param {Brand} collateralBrand
  * @param {Ratio} penaltyRate
  */
 const liquidate = async (
   zcf,
   vault,
-  burnLosses,
   liquidator,
   collateralBrand,
   penaltyRate,
@@ -80,16 +76,13 @@ const liquidate = async (
   const runToBurn = AmountMath.min(proceeds.RUN, debt);
   // debt is fully settled, with runToBurn and shortfall
   assert(AmountMath.isEqual(debt, AmountMath.add(runToBurn, shortfall)));
-  trace('before burn', { debt, proceeds, overage, shortfall, runToBurn });
-  // TODO why grant this power; we can burn it from the caller
-  burnLosses(runToBurn, vaultZcfSeat);
 
-  // Accounting complete. Update the vault state.
+  // Manager accounting changes determined. Update the vault state.
   vault.liquidated();
   // remaining funds are left on the vault for the user to close and claim
 
-  // for accounting
-  return { proceeds: proceeds.RUN, overage, shortfall };
+  // for manager's accounting
+  return { proceeds: proceeds.RUN, overage, runToBurn, shortfall };
 };
 
 const liquidationDetailTerms = debtBrand =>

packages/run-protocol/src/vaultFactory/prioritizedVaults.js

@@ -46,19 +46,19 @@ export const currentDebtToCollateral = vault =>
  * Vaults, ordered by their liquidation ratio so that all the
  * vaults below a threshold can be quickly found and liquidated.
  *
- * @param {() => void} reschedulePriceCheck called when there is a new
- * least-collateralized vault
+ * @param {() => void} highestChanged called when there is a new
+ * `highestRatio` (least-collateralized vault)
  */
-export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => {
+export const makePrioritizedVaults = (highestChanged = () => {}) => {
   const vaults = makeOrderedVaultStore();
 
   /**
    * Set the callback for when there is a new least-collateralized vault
    *
-   * @param {() => void} rescheduleFn
+   * @param {() => void} callback
    */
-  const setRescheduler = rescheduleFn => {
-    reschedulePriceCheck = rescheduleFn;
+  const onHighestRatioChanged = callback => {
+    highestChanged = callback;
   };
 
   // To deal with fluctuating prices and varying collateralization, we schedule a
@@ -79,18 +79,16 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => {
    *
    * @returns {Ratio=} actual debt over collateral
    */
-  const firstDebtRatio = () => {
+  const highestRatio = () => {
     if (vaults.getSize() === 0) {
       return undefined;
     }
     // Get the first vault.
     const [vault] = vaults.values();
-    const collateralAmount = vault.getCollateralAmount();
-    if (AmountMath.isEmpty(collateralAmount)) {
-      // This should only happen when the vault has been added but not funded yet
-      // TODO remove exceptional case for new vaults; if it's in the store it must be liquidatable
-      return undefined;
-    }
+    assert(
+      !AmountMath.isEmpty(vault.getCollateralAmount()),
+      'First vault had no collateral',
+    );
     return currentDebtToCollateral(vault);
   };
 
@@ -140,15 +138,14 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => {
    */
   const addVault = (vaultId, vault) => {
     const key = vaults.addVault(vaultId, vault);
-    // TODO refactor to fulfill this invariant
-    // assert(
-    //   !AmountMath.isEmpty(vault.getCollateralAmount()),
-    //   'Tracked vaults must have collateral (be liquidatable',
-    // );
+    assert(
+      !AmountMath.isEmpty(vault.getCollateralAmount()),
+      'Tracked vaults must have collateral (be liquidatable)',
+    );
     trace('addVault', key, 'when first:', firstKey);
     if (!firstKey || keyLT(key, firstKey)) {
       firstKey = key;
-      reschedulePriceCheck();
+      highestChanged();
     }
     return key;
   };
@@ -186,9 +183,9 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => {
     entriesPrioritizedGTE,
     getCount: vaults.getSize,
     hasVaultByAttributes,
-    highestRatio: firstDebtRatio,
+    highestRatio,
     removeVault,
     removeVaultByAttributes,
-    setRescheduler,
+    onHighestRatioChanged,
   });
 };

packages/run-protocol/src/vaultFactory/vault.js

@@ -86,7 +86,7 @@ const validTransitions = {
  * @property {MintAndReallocate} mintAndReallocate
  * @property {(amount: Amount, seat: ZCFSeat) => void} burnAndRecord
  * @property {() => Ratio} getCompoundedInterest
- * @property {(oldDebt: import('./storeUtils.js').NormalizedDebt, oldCollateral: Amount<'nat'>, vaultId: VaultId, vaultPhase: VaultPhase) => void} handleBalanceChange
+ * @property {(oldDebt: import('./storeUtils.js').NormalizedDebt, oldCollateral: Amount<'nat'>, vaultId: VaultId, vaultPhase: VaultPhase, vault: Vault) => void} handleBalanceChange
  * @property {() => import('./vaultManager.js').GovernedParamGetters} getGovernedParams
  */
 
@@ -260,6 +260,7 @@ const helperBehavior = {
       oldCollateral,
       state.idInManager,
       state.phase,
+      facets.self,
     );
   },
 
@@ -411,6 +412,7 @@ const helperBehavior = {
       oldCollateral,
       state.idInManager,
       state.phase,
+      facets.self,
     );
 
     return 'your loan is closed, thank you for your business';
@@ -545,13 +547,15 @@ const selfBehavior = {
    */
   initVaultKit: async ({ state, facets }, seat) => {
     const { self, helper } = facets;
+
+    const normalizedDebtPre = self.getNormalizedDebt();
+    const actualDebtPre = self.getCurrentDebt();
     assert(
-      AmountMath.isEmpty(state.debtSnapshot),
+      AmountMath.isEmpty(normalizedDebtPre) &&
+        AmountMath.isEmpty(actualDebtPre),
       X`vault must be empty initially`,
     );
-    // TODO should this be simplified to know that the oldDebt mut be empty?
-    const normalizedDebtPre = self.getNormalizedDebt();
-    const actualDebtPre = self.getCurrentDebt();
+
     const collateralPre = self.getCollateralAmount();
     trace('initVaultKit start: collateral', state.idInManager, {
       actualDebtPre,

packages/run-protocol/src/vaultFactory/vaultManager.js

@@ -427,23 +427,24 @@ const helperBehavior = {
    */
   liquidateAndRemove: ({ state, facets }, [key, vault]) => {
     const { factoryPowers, prioritizedVaults, zcf } = state;
-    trace('liquidating', vault.getVaultSeat().getProposal());
+    const vaultSeat = vault.getVaultSeat();
+    trace('liquidating', vaultSeat.getProposal());
 
     const collateralPre = vault.getCollateralAmount();
 
     // Start liquidation (vaultState: LIQUIDATING)
     const liquidator = state.liquidator;
     assert(liquidator);
-    trace('liquidating 2', vault.getVaultSeat().getProposal());
     return liquidate(
       zcf,
       vault,
-      (amount, seat) => facets.manager.burnAndRecord(amount, seat),
       liquidator,
       state.collateralBrand,
       factoryPowers.getGovernedParams().getLiquidationPenalty(),
     )
       .then(accounting => {
+        facets.manager.burnAndRecord(accounting.runToBurn, vaultSeat);
+
         // current values
         state.totalCollateral = AmountMath.subtract(
           state.totalCollateral,
@@ -564,13 +565,15 @@ const managerBehavior = {
    * @param {Amount<'nat'>} oldCollateral
    * @param {VaultId} vaultId
    * @param {import('./vault.js').VaultPhase} vaultPhase at the end of whatever change updatied balances
+   * @param {Vault} vault
    */
   handleBalanceChange: (
     { state, facets },
     oldDebtNormalized,
     oldCollateral,
     vaultId,
     vaultPhase,
+    vault,
   ) => {
     const { prioritizedVaults } = state;
 
@@ -590,12 +593,20 @@ const managerBehavior = {
         'Settled vaults must not be retained in storage',
       );
     } else {
-      // its position in the queue is no longer valid
-      const vault = prioritizedVaults.removeVaultByAttributes(
-        oldDebtNormalized,
-        oldCollateral,
-        vaultId,
-      );
+      const isNew = AmountMath.isEmpty(oldDebtNormalized);
+      if (!isNew) {
+        // its position in the queue is no longer valid
+
+        const vaultInStore = prioritizedVaults.removeVaultByAttributes(
+          oldDebtNormalized,
+          oldCollateral,
+          vaultId,
+        );
+        assert(
+          vault === vaultInStore,
+          'handleBalanceChange for two different vaults',
+        );
+      }
 
       // replace in queue, but only if it can accrue interest or be liquidated (i.e. has debt).
       // getCurrentDebt() would also work (0x = 0) but require more computation.
@@ -660,19 +671,38 @@ const selfBehavior = {
 
     const vault = makeVault(zcf, manager, vaultId);
 
-    // TODO Don't record the vault until it gets opened
-    const addedVaultKey = prioritizedVaults.addVault(vaultId, vault);
-
     try {
       // TODO `await` is allowed until the above ordering is fixed
       // eslint-disable-next-line @jessie.js/no-nested-await
       const vaultKit = await vault.initVaultKit(seat);
+      // initVaultKit calls back to handleBalanceChange() which will add the
+      // vault to prioritizedVaults
       seat.exit();
       return vaultKit;
     } catch (err) {
-      // remove it from prioritizedVaults
-      // XXX openLoan shouldn't assume it's already in the prioritizedVaults
-      prioritizedVaults.removeVault(addedVaultKey);
+      // ??? do we still need this cleanup? it won't get into the store unless it has collateral,
+      // which should qualify it to be in the store. If we drop this catch then the nested await
+      // for `vault.initVaultKit()` goes away.
+
+      // remove it from the store if it got in
+      /** @type {NormalizedDebt} */
+      // @ts-expect-error cast
+      const normalizedDebt = AmountMath.makeEmpty(state.debtBrand);
+      const collateralPre = seat.getCurrentAllocation().Collateral;
+      try {
+        prioritizedVaults.removeVaultByAttributes(
+          normalizedDebt,
+          collateralPre,
+          vaultId,
+        );
+        console.error('removed vault', vaultId, 'after initVaultKit failure');
+      } catch {
+        console.error(
+          'vault',
+          vaultId,
+          'never stored during initVaultKit failure',
+        );
+      }
       throw err;
     }
   },
@@ -740,7 +770,7 @@ const selfBehavior = {
 
 /** @param {MethodContext} context */
 const finish = ({ state, facets: { helper } }) => {
-  state.prioritizedVaults.setRescheduler(helper.reschedulePriceCheck);
+  state.prioritizedVaults.onHighestRatioChanged(helper.reschedulePriceCheck);
 
   // push initial state of metrics
   helper.updateMetrics();

packages/run-protocol/test/vaultFactory/test-vaultFactory.js

@@ -2515,17 +2515,17 @@ test('manager notifiers', async t => {
   t.is((await E(vault).getCurrentDebt()).value, DEBT1);
 
   trace('2. Remove collateral');
-  let taken = aeth.make(50_000n);
+  const COLL_REMOVED = 50_000n;
   const takeCollateralSeat = await E(services.zoe).offer(
     await E(vault).makeAdjustBalancesInvitation(),
     harden({
       give: {},
-      want: { Collateral: taken },
+      want: { Collateral: aeth.make(COLL_REMOVED) },
     }),
   );
   await E(takeCollateralSeat).getOfferResult();
   await m.assertChange({
-    totalCollateral: { value: AMPLE - taken.value },
+    totalCollateral: { value: AMPLE - COLL_REMOVED },
   });
 
   trace('3. Liquidate all (1 loan)');
@@ -2590,7 +2590,7 @@ test('manager notifiers', async t => {
     numLiquidationsCompleted: 2,
     numVaults: 1,
     totalCollateral: { value: AMPLE },
-    totalDebt: { value: 0n },
+    totalDebt: { value: DEBT1 },
     totalOverageReceived: { value: totalOverageReceived },
     totalProceedsReceived: { value: totalProceedsReceived },
   });
@@ -2599,6 +2599,7 @@ test('manager notifiers', async t => {
     numLiquidationsCompleted: 3,
     numVaults: 0,
     totalCollateral: { value: 0n },
+    totalDebt: { value: 0n },
     totalProceedsReceived: { value: totalProceedsReceived },
   });
   m.assertFullyLiquidated();
@@ -2693,7 +2694,7 @@ test('manager notifiers', async t => {
     numLiquidationsCompleted: 5,
     numVaults: 1,
     totalCollateral: { value: AMPLE },
-    totalDebt: { value: DEBT1 + DEBT2 + interestAccrued - nextProceeds - 296n }, // debt changed already with proceeds from next notification
+    totalDebt: { value: DEBT1 + DEBT2 + interestAccrued - nextProceeds },
     totalProceedsReceived: { value: totalProceedsReceived },
   });
   nextProceeds = 296n;