Skip to content

Commit

Permalink
Don't reschedule vault's priceNotifier if one exists (#10668)
Browse files Browse the repository at this point in the history
refs: #10660

## Description

in testing upgrade 18 candidates, we realized that vaultManager was subject to the same error as auctions fixed in #10615.

### Security Considerations

No Security implications.

### Scaling Considerations

Not fixing this would add a new notifier to the vaultManager's quote watcher for every hour that elapsed between upgrade and the oracles starting to provide price updates.

### Documentation Considerations

Unnecessary.

### Testing Considerations

This was detected by trawling slogs. It's not clear that we can do better than that to verify the fix.

### Upgrade Considerations

It would probably be a mistake to upgrade vaults without this fix.
  • Loading branch information
mergify[bot] authored Dec 11, 2024
2 parents c1ae023 + a74161c commit 34f026b
Showing 1 changed file with 52 additions and 34 deletions.
86 changes: 52 additions & 34 deletions packages/inter-protocol/src/vaultFactory/vaultManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ export const prepareVaultManagerKit = (
{ zcf, marshaller, makeRecorderKit, factoryPowers },
) => {
const { priceAuthority, timerService, reservePublicFacet } = zcf.getTerms();
const watchedBrands = new Set();

const makeVault = prepareVault(baggage, makeRecorderKit, zcf);

Expand Down Expand Up @@ -424,52 +425,69 @@ export const prepareVaultManagerKit = (
},
});

void facets.helper.observeQuoteNotifier();
void facets.helper.ensureQuoteNotifierWatched();

trace('helper.start() done');
},
observeQuoteNotifier() {
ensureQuoteNotifierWatched() {
const { state } = this;

const { collateralBrand, collateralUnit, debtBrand, storageNode } =
state;
if (watchedBrands.has(collateralBrand)) {
return;
}
watchedBrands.add(collateralBrand);

const ephemera = collateralEphemera(collateralBrand);

const quoteNotifier = E(priceAuthority).makeQuoteNotifier(
const quoteNotifierP = E(priceAuthority).makeQuoteNotifier(
collateralUnit,
debtBrand,
);
// @ts-expect-error XXX quotes
ephemera.storedQuotesNotifier = makeStoredNotifier(
// @ts-expect-error XXX quotes
quoteNotifier,
E(storageNode).makeChildNode('quotes'),
marshaller,
);
trace(
'helper.start() awaiting observe storedQuotesNotifier',
collateralBrand,
);
// NB: upon restart, there may not be a price for a while. If manager
// operations are permitted, ones that depend on price information
// will throw. See https://github.com/Agoric/agoric-sdk/issues/4317
const quoteWatcher = harden({
onFulfilled(value) {
trace('watcher updated price', value);
ephemera.storedCollateralQuote = value;
},
onRejected() {
// NOTE: drastic action, if the quoteNotifier fails, we don't know
// the value of the asset, nor do we know how long we'll be in
// ignorance. Best choice is to disable actions that require
// prices and restart when we have a new price. If we restart the
// notifier immediately, we'll trigger an infinite loop, so try
// to restart each time we get a request.

void E.when(
quoteNotifierP,
quoteNotifier => {
// @ts-expect-error XXX quotes
ephemera.storedQuotesNotifier = makeStoredNotifier(
// @ts-expect-error XXX quotes
quoteNotifier,
E(storageNode).makeChildNode('quotes'),
marshaller,
);
trace(
'helper.start() awaiting observe storedQuotesNotifier',
collateralBrand,
);
// NB: upon restart, there may not be a price for a while. If manager
// operations are permitted, ones that depend on price information
// will throw. See https://github.com/Agoric/agoric-sdk/issues/4317
const quoteWatcher = harden({
onFulfilled(value) {
trace('watcher updated price', value);
ephemera.storedCollateralQuote = value;
},
onRejected() {
// NOTE: drastic action, if the quoteNotifier fails, we don't know
// the value of the asset, nor do we know how long we'll be in
// ignorance. Best choice is to disable actions that require
// prices and restart when we have a new price. If we restart the
// notifier immediately, we'll trigger an infinite loop, so try
// to restart each time we get a request.

ephemera.storedCollateralQuote = null;
watchedBrands.delete(collateralBrand);
},
});
void watchQuoteNotifier(quoteNotifier, quoteWatcher);
},
e => {
trace('makeQuoteNotifier failed, resetting', e);
ephemera.storedCollateralQuote = null;
watchedBrands.delete(collateralBrand);
},
});
void watchQuoteNotifier(quoteNotifier, quoteWatcher);
);
},
/** @param {Timestamp} updateTime */
async chargeAllVaults(updateTime) {
Expand Down Expand Up @@ -841,7 +859,7 @@ export const prepareVaultManagerKit = (
const { collateralBrand } = state;
const { storedCollateralQuote } = collateralEphemera(collateralBrand);
if (!storedCollateralQuote) {
facets.helper.observeQuoteNotifier();
facets.helper.ensureQuoteNotifierWatched();

// it might take an arbitrary amount of time to get a new quote
throw Fail`maxDebtFor called before a collateral quote was available for ${collateralBrand}`;
Expand Down Expand Up @@ -1088,7 +1106,7 @@ export const prepareVaultManagerKit = (
state.collateralBrand,
);
if (!storedCollateralQuote) {
facets.helper.observeQuoteNotifier();
facets.helper.ensureQuoteNotifierWatched();

// it might take an arbitrary amount of time to get a new quote
throw Fail`getCollateralQuote called before a collateral quote was available`;
Expand All @@ -1107,7 +1125,7 @@ export const prepareVaultManagerKit = (
state.collateralBrand,
);
if (!storedCollateralQuote) {
facets.helper.observeQuoteNotifier();
facets.helper.ensureQuoteNotifierWatched();

// it might take an arbitrary amount of time to get a new quote
throw Fail`lockOraclePrices called before a collateral quote was available for ${state.collateralBrand}`;
Expand Down

0 comments on commit 34f026b

Please sign in to comment.