fix: giving collateral should not change totalDebt (#9873)

closes: #XXXX
refs: #XXXX

## Description

Updating `totalDebt` for 1 `vault` was doing `totalDebt += vault.getCurrentDebt() - old` where current debt includes compounded interest but old did not.

### Security Considerations

This (in combination with another PR to reset totalDebt to the sum over all vaults) should address a confusion risk for metrics clients.

### Scaling Considerations

The separate work to reset totalDebt to the sum over all vaults may have some, but I don't see any here.

### Testing Considerations

A straightforward unit test is included.

### Upgrade Considerations

This change takes effect with the next upgrade to the `vaultFactory`.

### Documentation Considerations

As part of the governance decision to upgrade `vaultFactory`, metrics consumers should be notified of this fix.

Author: mergify[bot]

packages/inter-protocol/src/interest.js

@@ -197,7 +197,7 @@ export const chargeInterest = (powers, params, prior, accruedUntil) => {
   }
 
   // NB: This method of inferring the compounded rate from the ratio of debts
-  // acrrued suffers slightly from the integer nature of debts. However in
+  // accrued suffers slightly from the integer nature of debts. However in
   // testing with small numbers there's 5 digits of precision, and with large
   // numbers the ratios tend towards ample precision.
   // TODO adopt banker's rounding https://github.com/Agoric/agoric-sdk/issues/4573

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

@@ -52,6 +52,7 @@ import {
   TopicsRecordShape,
 } from '@agoric/zoe/src/contractSupport/index.js';
 import { PriceQuoteShape, SeatShape } from '@agoric/zoe/src/typeGuards.js';
+import { multiplyBy } from '@agoric/zoe/src/contractSupport/ratio.js';
 import {
   checkDebtLimit,
   makeNatAmountShape,
@@ -993,8 +994,9 @@ export const prepareVaultManagerKit = (
           );
           state.totalDebt = AmountMath.subtract(
             AmountMath.add(state.totalDebt, vault.getCurrentDebt()),
-            oldDebtNormalized,
+            multiplyBy(oldDebtNormalized, state.compoundedInterest),
           );
+
           void facets.helper.writeMetrics();
         },
       },

packages/inter-protocol/test/vaultFactory/driver.js

@@ -213,7 +213,7 @@ const getRunFromFaucet = async (t, amt) => {
  * @param {Amount} priceBase
  */
 const setupServices = async (t, initialPrice, priceBase) => {
-  const timer = buildZoeManualTimer(t.log);
+  const timer = buildZoeManualTimer(t.log, 0n, { timeStep: 60n * 60n });
   const { zoe, run, aeth, interestTiming, minInitialDebt, rates } = t.context;
   t.context.timer = timer;
 
@@ -336,6 +336,10 @@ export const makeManagerDriver = async (
     publicTopics.asset.subscriber,
   );
   let managerNotification = await E(managerNotifier).getUpdateSince();
+  const metricsNotifier = await makeNotifierFromSubscriber(
+    publicTopics.metrics.subscriber,
+  );
+  let metricsNotification = await E(metricsNotifier).getUpdateSince();
 
   /** @type {UserSeat} */
   let currentSeat;
@@ -559,6 +563,21 @@ export const makeManagerDriver = async (
       }
       return managerNotification;
     },
+    /**
+     * @param {object} [likeExpected]
+     * @param {AT_NEXT | number} [optSince] AT_NEXT is an alias for updateCount
+     *   of the last update, forcing to wait for another
+     */
+    metricsNotified: async (likeExpected, optSince) => {
+      metricsNotification = await E(metricsNotifier).getUpdateSince(
+        optSince === AT_NEXT ? metricsNotification.updateCount : optSince,
+      );
+      trace(t, 'metrics notifier', metricsNotification);
+      if (likeExpected) {
+        t.like(metricsNotification.value, likeExpected);
+      }
+      return managerNotification;
+    },
     checkReserveAllocation: async stableValue => {
       const { reserveCreatorFacet } = t.context;
       const reserveAllocations = await E(reserveCreatorFacet).getAllocations();

packages/inter-protocol/test/vaultFactory/snapshots/vaultFactory.test.js.md

@@ -204,7 +204,7 @@ Generated by [AVA](https://avajs.dev).
           },
           totalDebt: {
             brand: Object @Alleged: IST brand {},
-            value: 1634n,
+            value: 1608n,
           },
           totalOverageReceived: {
             brand: Object @Alleged: IST brand {},

packages/inter-protocol/test/vaultFactory/snapshots/vaultFactory.test.js.snap

@@ -2,7 +2,8 @@ import { test as unknownTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';
 
 import { makeTracer } from '@agoric/internal';
 import { E } from '@endo/eventual-send';
-import { makeDriverContext, makeManagerDriver } from './driver.js';
+import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
+import { AT_NEXT, makeDriverContext, makeManagerDriver } from './driver.js';
 
 /** @typedef {import('./driver.js').DriverContext & {}} Context */
 /** @type {import('ava').TestFn<Context>} */
@@ -15,6 +16,46 @@ test.before(async t => {
   trace(t, 'CONTEXT');
 });
 
+test('totalDebt calculation includes compoundedInterest', async t => {
+  const { aeth, run } = t.context;
+  const md = await makeManagerDriver(t);
+  const timer = md.timer();
+
+  t.log('charge 2% per day to simulate lower rates over longer times');
+  const aethKey = { collateralBrand: aeth.brand };
+  const twoPctPerDay = run.makeRatio(2n * 360n, 100n);
+  await md.setGovernedParam('InterestRate', twoPctPerDay, { key: aethKey });
+  const plausibleDebtLimit = run.units(500000);
+  await md.setGovernedParam('DebtLimit', plausibleDebtLimit, { key: aethKey });
+
+  t.log('mint 100 IST against 25 AETH');
+  const vd = await md.makeVaultDriver(aeth.units(25), run.units(100));
+  const v1debtAfterMint = await E(vd.vault()).getCurrentDebt();
+  t.deepEqual(v1debtAfterMint, run.units(100 + 5));
+  // totalDebt is debit of this 1 vault
+  await md.metricsNotified({ totalDebt: v1debtAfterMint }, AT_NEXT);
+
+  await E(timer).tickN(40, 'interest accumulates over 40hrs');
+  await eventLoopIteration(); // let all timer-induced promises settle
+  const v1debtAfterDay = await E(vd.vault()).getCurrentDebt();
+  t.deepEqual(v1debtAfterDay, run.make(106_008_000n));
+
+  // Checking that totalDebt here matches v1debtAfterDay would be handy,
+  // but totalDebt isn't updated when calculating interest.
+  const expectedCompoundedInterest = {
+    denominator: { value: 100000000000000000000n },
+    numerator: { value: 100960000000000000000n },
+  };
+  await md.managerNotified({ compoundedInterest: expectedCompoundedInterest });
+
+  t.log('give some collateral (adjustBalances); no change to debt');
+  await E(vd).giveCollateral(50n, aeth);
+  const v1debtAfterGive = await E(vd.vault()).getCurrentDebt();
+  t.deepEqual(v1debtAfterGive, v1debtAfterDay, 'no debt change');
+
+  await md.metricsNotified({ totalDebt: v1debtAfterDay }, AT_NEXT);
+});
+
 test('excessive loan', async t => {
   const { aeth, run } = t.context;
   const md = await makeManagerDriver(t);

packages/inter-protocol/test/vaultFactory/vault-collateralization.test.js

@@ -1838,6 +1838,13 @@ test('manager notifiers, with snapshot', async t => {
     }),
   );
   ({ vault } = await E(vaultSeat).getOfferResult());
+  totalCollateral += ENOUGH;
+  totalDebt += DEBT2;
+  await m.assertChange({
+    numActiveVaults: 4,
+    totalCollateral: { value: totalCollateral },
+    totalDebt: { value: totalDebt },
+  });
 
   trace('6. Loan interest');
   vaultSeat = await E(services.zoe).offer(
@@ -1851,18 +1858,21 @@ test('manager notifiers, with snapshot', async t => {
     }),
   );
   ({ vault } = await E(vaultSeat).getOfferResult());
-  totalCollateral += ENOUGH;
-  totalDebt += DEBT2;
+  totalCollateral += AMPLE;
+  totalDebt += DEBT1;
   await m.assertChange({
-    numActiveVaults: 4,
+    numActiveVaults: 5,
     totalCollateral: { value: totalCollateral },
     totalDebt: { value: totalDebt },
   });
-  m.addDebt(DEBT2);
+
   await manualTimer.tickN(5);
+  // This is interest for a single vault.
   const interestAccrued = (await E(vault).getCurrentDebt()).value - DEBT1;
   m.addDebt(interestAccrued);
-  t.is(interestAccrued, 9n);
+
+  t.is(interestAccrued, 9n); // interest on OPEN1 for 5 periods
+  totalDebt += 30n; // interest on 270_000 for 5 periods
 
   trace('7. make another loan to trigger a publish');
   vaultSeat = await E(services.zoe).offer(
@@ -1876,10 +1886,10 @@ test('manager notifiers, with snapshot', async t => {
     }),
   );
   ({ vault } = await E(vaultSeat).getOfferResult());
-  totalCollateral += AMPLE;
-  totalDebt += DEBT1;
+  totalCollateral += ENOUGH;
+  totalDebt += DEBT2;
   await m.assertChange({
-    numActiveVaults: 5,
+    numActiveVaults: 6,
     totalCollateral: { value: totalCollateral },
     totalDebt: { value: totalDebt },
   });
@@ -1898,11 +1908,10 @@ test('manager notifiers, with snapshot', async t => {
     }),
   );
   ({ vault } = await E(vaultSeat).getOfferResult());
-  totalCollateral += ENOUGH;
-  totalDebt += DEBT2 + 30n; // XXX ??
-  m.addDebt(DEBT2);
+  totalCollateral += AMPLE;
+  totalDebt += DEBT1;
   await m.assertChange({
-    numActiveVaults: 6,
+    numActiveVaults: 7,
     totalCollateral: { value: totalCollateral },
     totalDebt: { value: totalDebt },
   });
@@ -1925,20 +1934,11 @@ test('manager notifiers, with snapshot', async t => {
     }),
   );
   await E(vaultOpSeat).getOfferResult();
-  totalCollateral += AMPLE;
-  totalDebt += DEBT1;
-  await m.assertChange({
-    numActiveVaults: 7,
-    totalDebt: { value: totalDebt },
-    totalCollateral: { value: totalCollateral },
-  });
-
   totalCollateral += given.value;
-  totalDebt += WANT_EXTRA + 29n; // magic number is fees
-
+  totalDebt += WANT_EXTRA + 20n;
   await m.assertChange({
-    totalCollateral: { value: totalCollateral },
     totalDebt: { value: totalDebt },
+    totalCollateral: { value: totalCollateral },
   });
 
   trace('10. Close vault');
@@ -1955,7 +1955,7 @@ test('manager notifiers, with snapshot', async t => {
   await E(vaultOpSeat).getOfferResult();
 
   totalCollateral -= AMPLE + given.value;
-  totalDebt -= DEBT1_EXTRA - 17n; // magic number is fees
+  totalDebt -= DEBT1_EXTRA;
   await m.assertChange({
     numActiveVaults: 6,
     totalCollateral: { value: totalCollateral },