From 8954d9908b290a8636b8728b8fe73cfa04baa08b Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Fri, 10 Jun 2022 15:23:17 -0700 Subject: [PATCH 1/3] refactor(vaultFactory): update economy after vault liquidated --- .../run-protocol/src/vaultFactory/liquidation.js | 13 +++---------- .../run-protocol/src/vaultFactory/vaultManager.js | 7 ++++--- .../test/vaultFactory/test-vaultFactory.js | 11 ++++++----- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/run-protocol/src/vaultFactory/liquidation.js b/packages/run-protocol/src/vaultFactory/liquidation.js index 508f8d3edb9..b5d85b04933 100644 --- a/packages/run-protocol/src/vaultFactory/liquidation.js +++ b/packages/run-protocol/src/vaultFactory/liquidation.js @@ -18,9 +18,6 @@ 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 @@ -28,7 +25,6 @@ const trace = makeTracer('LIQ', false); 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 => diff --git a/packages/run-protocol/src/vaultFactory/vaultManager.js b/packages/run-protocol/src/vaultFactory/vaultManager.js index 66c7a2044f5..661efacb849 100644 --- a/packages/run-protocol/src/vaultFactory/vaultManager.js +++ b/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, diff --git a/packages/run-protocol/test/vaultFactory/test-vaultFactory.js b/packages/run-protocol/test/vaultFactory/test-vaultFactory.js index 081eef95283..114abe8b871 100644 --- a/packages/run-protocol/test/vaultFactory/test-vaultFactory.js +++ b/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; From 5b431c19fe56854ad71c0fd4d4b2f3a6890bd5bd Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Fri, 10 Jun 2022 16:10:48 -0700 Subject: [PATCH 2/3] refactor(vaults): store only vaults with collateral --- .../src/vaultFactory/prioritizedVaults.js | 19 +++---- .../run-protocol/src/vaultFactory/vault.js | 14 +++-- .../src/vaultFactory/vaultManager.js | 53 ++++++++++++++----- 3 files changed, 58 insertions(+), 28 deletions(-) diff --git a/packages/run-protocol/src/vaultFactory/prioritizedVaults.js b/packages/run-protocol/src/vaultFactory/prioritizedVaults.js index 99a24a0fc29..c6ea030f278 100644 --- a/packages/run-protocol/src/vaultFactory/prioritizedVaults.js +++ b/packages/run-protocol/src/vaultFactory/prioritizedVaults.js @@ -85,12 +85,10 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => { } // 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,11 +138,10 @@ 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; diff --git a/packages/run-protocol/src/vaultFactory/vault.js b/packages/run-protocol/src/vaultFactory/vault.js index 5f26cf8d8c6..440e6a9be92 100644 --- a/packages/run-protocol/src/vaultFactory/vault.js +++ b/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, diff --git a/packages/run-protocol/src/vaultFactory/vaultManager.js b/packages/run-protocol/src/vaultFactory/vaultManager.js index 661efacb849..76ce7ff2cd0 100644 --- a/packages/run-protocol/src/vaultFactory/vaultManager.js +++ b/packages/run-protocol/src/vaultFactory/vaultManager.js @@ -565,6 +565,7 @@ 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 }, @@ -572,6 +573,7 @@ const managerBehavior = { oldCollateral, vaultId, vaultPhase, + vault, ) => { const { prioritizedVaults } = state; @@ -591,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. @@ -661,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; } }, From b7df02465cf2eac528aae28bd33a02369bd0f97d Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Fri, 10 Jun 2022 16:23:48 -0700 Subject: [PATCH 3/3] refactor(prioritizedVaults): separate of concerns --- .../src/vaultFactory/prioritizedVaults.js | 20 +++++++++---------- .../src/vaultFactory/vaultManager.js | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/run-protocol/src/vaultFactory/prioritizedVaults.js b/packages/run-protocol/src/vaultFactory/prioritizedVaults.js index c6ea030f278..e3c1cb68827 100644 --- a/packages/run-protocol/src/vaultFactory/prioritizedVaults.js +++ b/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,7 +79,7 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => { * * @returns {Ratio=} actual debt over collateral */ - const firstDebtRatio = () => { + const highestRatio = () => { if (vaults.getSize() === 0) { return undefined; } @@ -145,7 +145,7 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => { trace('addVault', key, 'when first:', firstKey); if (!firstKey || keyLT(key, firstKey)) { firstKey = key; - reschedulePriceCheck(); + highestChanged(); } return key; }; @@ -183,9 +183,9 @@ export const makePrioritizedVaults = (reschedulePriceCheck = () => {}) => { entriesPrioritizedGTE, getCount: vaults.getSize, hasVaultByAttributes, - highestRatio: firstDebtRatio, + highestRatio, removeVault, removeVaultByAttributes, - setRescheduler, + onHighestRatioChanged, }); }; diff --git a/packages/run-protocol/src/vaultFactory/vaultManager.js b/packages/run-protocol/src/vaultFactory/vaultManager.js index 76ce7ff2cd0..08c8c1df36f 100644 --- a/packages/run-protocol/src/vaultFactory/vaultManager.js +++ b/packages/run-protocol/src/vaultFactory/vaultManager.js @@ -770,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();