Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: giving collateral should not change totalDebt #9873

Merged
merged 7 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/inter-protocol/src/interest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion packages/inter-protocol/src/vaultFactory/vaultManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
},
},
Expand Down
21 changes: 20 additions & 1 deletion packages/inter-protocol/test/vaultFactory/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {},
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -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>} */
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const twoPctPerDay = run.makeRatio(2n * 360n, 100n);
const twoPctPerDay = run.makeRatio(2n * 365n, 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 },
turadg marked this conversation as resolved.
Show resolved Hide resolved
};
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);
Expand Down
48 changes: 24 additions & 24 deletions packages/inter-protocol/test/vaultFactory/vaultFactory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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 },
});
Expand All @@ -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 },
});
Expand All @@ -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');
Expand All @@ -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 },
Expand Down
Loading