Skip to content

Commit

Permalink
don't reschedule auction's price notifier if we already have one (#10615
Browse files Browse the repository at this point in the history
)

## Description

It was observed, after applying upgrade 18 to EmeryNet, that a whole slew of promises resolved when a price was provided. The current belief is that `observeQuoteNotifier` will be called every auction round until a price is available, and that creates a new observer with each call that wait until a price is published, and then they all continue waiting for each successive update.

This change adds an interlock, so if there's already a notifier waiting, we don't add a new one.

### Security Considerations

No security implication.

### Scaling Considerations

Processing about 19000 actions waiting on a notifier in EmeryNet took several hours. If we're correct that the notifiers will continue to cycle, we expect to see a similar wait for each price update on that currency. That's unsustainable.

The only current theory about dropping all those actions waiting for notifiers is to kill the vat. We can't kill the priceAuthority vats that hold the notifiers, but we might be able to cleanly kill the abandoned auctioneers.

### Documentation Considerations

Not needed.

### Testing Considerations

Tough to test in unit tests. It's conceivable that we could recreate the situation in `a3p-integration`, though it would be hard to observe the results.

### Upgrade Considerations

We probably shouldn't ship upgrade 18 with something to address this problem.
  • Loading branch information
Chris-Hibbert authored Dec 4, 2024
1 parent 8b9c8db commit e596a01
Showing 1 changed file with 49 additions and 25 deletions.
74 changes: 49 additions & 25 deletions packages/inter-protocol/src/auction/auctionBook.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ export const makeOfferSpecShape = (bidBrand, collateralBrand) => {
export const prepareAuctionBook = (baggage, zcf, makeRecorderKit) => {
const makeScaledBidBook = prepareScaledBidBook(baggage);
const makePriceBook = preparePriceBook(baggage);
// Brands that have or are making active quoteNotifier Observers
const observedBrands = new Set();

const AuctionBookStateShape = harden({
collateralBrand: M.any(),
Expand Down Expand Up @@ -454,38 +456,59 @@ export const prepareAuctionBook = (baggage, zcf, makeRecorderKit) => {
});
return state.bookDataKit.recorder.write(bookData);
},
observeQuoteNotifier() {
// Ensure that there is an observer monitoring the quoteNotifier. We
// assume that all failure modes for quoteNotifier eventually lead to
// fail or finish.
ensureQuoteNotifierObserved() {
const { state, facets } = this;
const { collateralBrand, bidBrand, priceAuthority } = state;

if (observedBrands.has(collateralBrand)) {
return;
}
observedBrands.add(collateralBrand);
trace('observing');

const quoteNotifier = E(priceAuthority).makeQuoteNotifier(
const quoteNotifierP = E(priceAuthority).makeQuoteNotifier(
AmountMath.make(collateralBrand, QUOTE_SCALE),
bidBrand,
);
void observeNotifier(quoteNotifier, {
updateState: quote => {
trace(
`BOOK notifier ${priceFrom(quote).numerator.value}/${
priceFrom(quote).denominator.value
}`,
);
state.updatingOracleQuote = priceFrom(quote);
},
fail: reason => {
trace(`Failure from quoteNotifier (${reason}) setting to null`);
// lack of quote will trigger restart
state.updatingOracleQuote = null;
},
finish: done => {
trace(
`quoteNotifier invoked finish(${done}). setting quote to null`,
);
// lack of quote will trigger restart

void E.when(
quoteNotifierP,
quoteNotifier =>
observeNotifier(quoteNotifier, {
updateState: quote => {
trace(
`BOOK notifier ${priceFrom(quote).numerator.value}/${
priceFrom(quote).denominator.value
}`,
);
state.updatingOracleQuote = priceFrom(quote);
},
fail: reason => {
trace(
`Failure from quoteNotifier (${reason}) setting to null`,
);
// lack of quote will trigger restart
state.updatingOracleQuote = null;
observedBrands.delete(collateralBrand);
},
finish: done => {
trace(
`quoteNotifier invoked finish(${done}). setting quote to null`,
);
// lack of quote will trigger restart
state.updatingOracleQuote = null;
observedBrands.delete(collateralBrand);
},
}),
e => {
trace('makeQuoteNotifier failed, resetting', e);
state.updatingOracleQuote = null;
observedBrands.delete(collateralBrand);
},
});
);

void facets.helper.publishBookData();
},
Expand Down Expand Up @@ -645,8 +668,9 @@ export const prepareAuctionBook = (baggage, zcf, makeRecorderKit) => {

trace(`capturing oracle price `, state.updatingOracleQuote);
if (!state.updatingOracleQuote) {
// if the price has feed has died, try restarting it.
facets.helper.observeQuoteNotifier();
// if the price feed has died (or hasn't been started for this
// incarnation), (re)start it.
facets.helper.ensureQuoteNotifierObserved();
return;
}

Expand Down Expand Up @@ -750,7 +774,7 @@ export const prepareAuctionBook = (baggage, zcf, makeRecorderKit) => {
const { collateralBrand, bidBrand, priceAuthority } = state;
assertAllDefined({ collateralBrand, bidBrand, priceAuthority });

facets.helper.observeQuoteNotifier();
facets.helper.ensureQuoteNotifierObserved();
},
stateShape: AuctionBookStateShape,
},
Expand Down

0 comments on commit e596a01

Please sign in to comment.