feat(zoe): add E(userSeat).wasWantSatisfied() #5905
Conversation
| wasWantSatisfied: async () => { | ||
| return E.when(payoutPromiseKit.promise, () => | ||
| Object.keys(proposal.want).every(kwd => | ||
| AmountMath.isGTE(currentAllocation[kwd], proposal.want[kwd]), | ||
| ), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Id prefer that you use satisfiesWant, which you'll need to export from offerSafety.js. That way you'll already be prepared for multiples, which does export it defined as
export const satisfiesWant = (proposal, allocation) =>
satisfiesInternal(proposal.want, allocation, 1n) >= 1n;
harden(satisfiesWant);This also makes clear that this PR is necessary! If clients had tested this themselves, they also would have created an incompatibility with multiples!
There was a problem hiding this comment.
I see that it was also exported on master, so no worries about that.
There was a problem hiding this comment.
Actually, I remember we had a discussion about this, and agreed it was better for the public method to return 0 or 1 for now, rather than false and true, so that it could be more informative once we have multiples. How many times was the want satisfied?
There was a problem hiding this comment.
I've updated offerSafety to return 0 or 1, and added tests for satisfiesWant.
| wasWantSatisfied: async () => { | ||
| return E.when(payoutPromiseKit.promise, () => | ||
| Object.keys(proposal.want).every(kwd => | ||
| AmountMath.isGTE(currentAllocation[kwd], proposal.want[kwd]), | ||
| ), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
I've updated offerSafety to return 0 or 1, and added tests for satisfiesWant.
| * @param {AmountKeywordRecord} giveOrWant | ||
| * @param {AmountKeywordRecord} allocation | ||
| * @returns {0|1} |
There was a problem hiding this comment.
Sometimes Typescript amazes me ;)
No change suggested.
| satisfiesGive(proposal, allocation) > 0 || | ||
| satisfiesWant(proposal, allocation) > 0 |
There was a problem hiding this comment.
Thank you for not relying on 0 being falsy ;)
No change suggested.
| harden(isOfferSafe); | ||
| harden(satisfiesWant); |
There was a problem hiding this comment.
Wow, we were missing hardens before. Good catch!
While not suggesting a change, I will note that my style is to export each in place, and follow each immediately by a harden. This is safe against the unhardened state being observable only if the exported declarations are not 'function' function declarations. Converting to arrow functions needs to happen sometime, but need not happen for old functions in the PR.
packages/zoe/src/zoeService/types.js
Outdated
| @@ -182,6 +182,7 @@ | |||
| * @property {() => Promise<OR>} getOfferResult | |||
| * @property {() => void=} tryExit | |||
| * @property {() => Promise<boolean>} hasExited | |||
| * @property {() => Promise<0|1>} wasWantSatisfied | |||
There was a problem hiding this comment.
The name wasWantSatisfied definitely suggests a yes or no answer.
What name would work better with returning a count?
There was a problem hiding this comment.
numWantsSatisfied or simply wantsSatisfied since singular is by far more common than multi
There was a problem hiding this comment.
I like @turadg's suggestion of numWantsSatisfied. (It beats out 'wantsSatisfied' because the latter can be read as boolean.
There was a problem hiding this comment.
because the latter can be read as boolean.
fwiw, that's what I proposed as its virtue. when the caller doesn't know or care about multiples they don't have to. If they try to do anything with it that works for a boolean and not 0 | 1 the type checker will inform them.
They need to return 0 or 1, so they can return a count with Multiples. Added tests for satisfiesWant, since it's exported.
Chris-Hibbert
force-pushed
the
5818-wasWantSatisfied
branch
from
84486bf
to
6f154c3
Compare
Chris-Hibbert
changed the title
closes: #5818
closes: #3056
refs: #4360
Description
Add a method on Zoe's userSeats that reveals whether the seat exited with its wants satisfied.
Security Considerations
This doesn't reveal interim state of a seat. In fact, it doesn't reveal anything the userSeat didn't already have access to, since it only needed the payouts and the proposal. But this is a more reliable way for dApps to find out than doing the calculation themselves.
Documentation Considerations
Needs to be added to the Zoe docs.
Testing Considerations
Tests simple cases of satisfying and not satisfying the wants. Verifies that the promise is available before the seat exits.