6771 pricefeed errors #6776
6771 pricefeed errors #6776
Conversation
turadg
force-pushed
the
6771-pricefeed-errors
branch
from
8b681f3 to
ac4d9b4
Compare
turadg
force-pushed
the
6771-pricefeed-errors
branch
from
ac4d9b4 to
c5275fd
Compare
packages/smart-wallet/src/offers.js
Outdated
| @@ -112,6 +112,16 @@ export const makeOfferExecutor = ({ | |||
| ); | |||
| logger.info(id, 'seated'); | |||
|
|
|||
| // NB: the following four promises all resolve exactly when the seat exits. | |||
There was a problem hiding this comment.
@Chris-Hibbert is this true in general? @dckc has a counter example #6777 (comment)
There was a problem hiding this comment.
The way I read @dckc's comment, it was saying that if the seat doesn't exit, then various things don't work. That's consistent with my expectation that numWantsSatisfied(), getFinalAllocation(), and getPayouts() resolve at the same time that the exitSubscriber reports that it's done. The comment above seems to include getOfferResult(), which @erights pointed out often resolves at a different time.
turadg
force-pushed
the
6771-pricefeed-errors
branch
from
c5275fd to
3ca9045
Compare
| /** @param {ZCFSeat} cSeat */ | ||
| async cSeat => { | ||
| cSeat.exit(); | ||
| await admin.pushPrice(result); |
There was a problem hiding this comment.
when this throws, getOfferResult() rejects
There was a problem hiding this comment.
that seems right. (Sorry if I misled you previously.)
getOfferResult() is supposed to give you the return value of the offerHandler, so if it throws, getOfferResult() has nothing else to return. The exitSubscriber should still report that the seat has exited, and numWantsSatisfied should be 1.
| @@ -127,7 +186,7 @@ test('admin price', async t => { | |||
| 'priceAggregator', | |||
| ); | |||
|
|
|||
| // The purse has the invitation to get the makers /////////// | |||
| // The purse has the invitation to get the makers | |||
There was a problem hiding this comment.
Does the reason you removed that apply to line 227 as well?
There was a problem hiding this comment.
Just cleanliness. Not worth another push.
turadg
force-pushed
the
6771-pricefeed-errors
branch
from
3ca9045 to
73f24e8
Compare
closes: #6771
Description
When a
PushPriceinvitation executed, errors in thepushPricecall weren't caught or reported.The reason is
pushPricewas called as a floating promise. We have a way to lint floating promises but it's too slow for CI. Still this error should serve as a motivation to resolve them before the next release, to detect any other surprises.Security Considerations
No new boundaries.
Documentation Considerations
Conforms better to expectation. Nothing new to document.
Testing Considerations
New tests.