Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some JSDoc updates for Zoe #9001

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/zoe/src/contractFacet/types-ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ type ZcfSeatKit = {
zcfSeat: ZCFSeat;
userSeat: ERef<UserSeat>;
};
type HandleOffer<OR extends unknown, OA> = (
seat: ZCFSeat,
offerArgs?: OA,
) => OR;
type HandleOffer<OR extends unknown, OA> = (seat: ZCFSeat, offerArgs: OA) => OR;
type OfferHandler<OR extends unknown = unknown, OA = never> =
| HandleOffer<OR, OA>
| {
Expand Down
2 changes: 2 additions & 0 deletions packages/zoe/src/internal-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@
* @property {import('@agoric/swingset-vat').ShutdownWithFailure} fail called with the reason
* for calling fail on this seat, where reason is normally an instanceof Error.
* @property {() => Subscriber<AmountKeywordRecord>} getExitSubscriber
* @property {(result: HandleOfferResult) => void} resolveExitAndResult
* @property {(payments: PaymentPKeywordRecord) => Promise<void>} finalPayouts
*/

/**
Expand Down
84 changes: 57 additions & 27 deletions packages/zoe/src/zoeService/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,33 +193,63 @@
* @see {@link https://docs.agoric.com/zoe/api/zoe.html#userseat-object}}
* @template {object} [OR=unknown]
* @typedef {object} UserSeat
* @property {() => Promise<ProposalRecord>} getProposal
* @property {() => Promise<PaymentPKeywordRecord>} getPayouts
* returns a promise for a KeywordPaymentRecord containing all the payouts from
* this seat. The promise will resolve after the seat has exited.
* @property {(keyword: Keyword) => Promise<Payment<any>>} getPayout
* returns a promise for the Payment corresponding to the indicated keyword.
* The promise will resolve after the seat has exited.
* @property {() => Promise<OR>} getOfferResult
* @property {() => void} [tryExit]
* Note: Only works if the seat's `proposal` has an `OnDemand` `exit` clause. Zoe's
* offer-safety guarantee applies no matter how a seat's interaction with a
* contract ends. Under normal circumstances, the participant might be able to
* call `tryExit()`, or the contract might do something explicitly. On exiting,
* the seat holder gets its current `allocation` and the `seat` can no longer
* interact with the contract.
* @property {() => Promise<boolean>} hasExited
* Returns true if the seat has exited, false if it is still active.
* @property {() => Promise<0|1>} numWantsSatisfied returns 1 if the proposal's
* want clause was satisfied by the final allocation, otherwise 0. This is
* numeric to support a planned enhancement called "multiples" which will allow
* the return value to be any non-negative number. The promise will resolve
* after the seat has exited.
* @property {() => Promise<Allocation>} getFinalAllocation
* return a promise for the final allocation. The promise will resolve after the
* seat has exited.
* @property {() => Subscriber<Completion>} getExitSubscriber returns a subscriber that
* will be notified when the seat has exited or failed.
* @property {() => Promise<ProposalRecord>} getProposal A _Proposal_ is
* represented by a _ProposalRecord_. It is the rules accompanying the escrow of
* Payments dictating what the user expects to get back from Zoe. It has keys
* _give_, _want_, and _exit_. _give_ and _want_ are records with
* {@link Keyword} as keys and {@link Amount} as values. If it is compatible
* with the contract, the contract tries to satisfy it. If not, the contract
* kicks the _seat_ out.
*
* Offer safety is always enforced; if kicked out, the user gets back what they
* put in. If the contract attempts to satisfy it, they either get what they
* asked for or Zoe ensures they get back their deposit.
*
* Example:
Copy link
Member

@dckc dckc Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Example:
@example

cf @example tag

*
* const { want, give, exit } = sellerSeat.getProposal();
*
* @property {() => Promise<PaymentPKeywordRecord>} getPayouts Returns a Promise
* for a KeywordRecord containing Promises for all the Payouts associated with
* the seat's offers. A Payout is a {@link Payment} that goes to a party in a
* successful transaction, redirecting escrowed assets in accordance with the
* result of the transaction.
*
* The promise will be resolved promptly once the seat exits.
*
* @property {(keyword: Keyword) => Promise<Payment<any>>} getPayout returns a
* promise for the Payment corresponding to the indicated keyword. The promise
* will resolve after the seat has exited.
Comment on lines +221 to +222
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use consistent terminology about the timing of the response.

Suggested change
* promise for the Payment corresponding to the indicated keyword. The promise
* will resolve after the seat has exited.
* promise for the Payment corresponding to the indicated keyword. The promise
* will be resolved promptly once the seat exits.

* @property {() => Promise<OR>} getOfferResult Returns a Promise for an
* OfferResult. The OfferResult can be any Passable. For example, in the
* Automatic Refund example, it's the string "The offer was accepted". In the
* Covered Call example, it's a call option, which is an assayable Invitation to
* buy the underlying asset. Strings and invitations are the most common things
* returned. The value is the result returned by the offerHandler function
* passed in the first argument to zcf.makeInvitation(...).
* Since the contract can return whatever it wants as an offer result, there is no guarantee that the promise will resolve promptly.
* @property {() => void} tryExit Note: Only works if the seat's `proposal`
* has an `OnDemand` `exit` clause. Zoe's offer-safety guarantee applies no
* matter how a seat's interaction with a contract ends. Under normal
* circumstances, the participant might be able to call `tryExit()`, or the
* contract might do something explicitly. On exiting, the seat holder gets its
* current `allocation` and the `seat` can no longer interact with the contract.
* @property {() => Promise<boolean>} hasExited Returns true if the seat has
* exited, false if it is still active. Returns promptly.
* @property {() => Promise<0|1>} numWantsSatisfied Returns a Promise for a
* number which indicates the result of the exited Proposal, as described below:
*
* - 0: The user didn't get what they wanted from the Proposal, so their offer was refunded.
* - 1: The user got what they wanted from the Proposal, so their offer is spent & gone.
*
* This promise will be resolved promptly once the seat exits.
*
* This is numeric to support a planned enhancement called "multiples" which
* will allow the return value to be any non-negative number.
* @property {() => Promise<Allocation>} getFinalAllocation return a promise for
* the final allocation. The promise will be resolved promptly once the seat exits.
* @property {() => Subscriber<Completion>} getExitSubscriber returns a
* subscriber that will be notified when the seat has exited or failed.
*/

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/zoe/src/zoeService/zoeSeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export const makeZoeSeatAdminFactory = baggage => {
!hasExited1 || assert(!hasExited1, msg);
},
},
/** @type {ZoeSeatAdmin} */
zoeSeatAdmin: {
replaceAllocation(replacementAllocation) {
const { state, facets } = this;
Expand All @@ -228,7 +229,7 @@ export const makeZoeSeatAdminFactory = baggage => {
);

state.exiting = true;
E.when(
void E.when(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, this solves the problem with the linter, but leaves error messages in production logs that are being noticed. I think we should take care of both with a noop catch clause.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhofman you tend to be most affected by unhandled promise rejections in the logs. Do you prefer a rejection here to be silent?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I have enough background on the behavior. what are the consequences of doExit failing? It seems to not be reported anywhere except in console output. I thought voiding a promise was meant to say "I've handled the rejection, trust me".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought voiding a promise was meant to say "I've handled the rejection, trust me".

Close. It's saying "I'm intentionally not handling this rejection". It tells the linter (and the reader) that the absence of handling is intentional.

what are the consequences of doExit failing?

@Chris-Hibbert ? Is it a "this could happen and we don't care" or "this should never happen so if it does we should find out" ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand what the complaints in the log are unhappy about. This is already in an E.when() with an onRejected branch.


If the E.when() itself fails, and the onRejected clauses don't do their job, aren't we in a world of hurt?

@turadg and I talked about this out-of-band, and we concluded that E.when() may need a void in order to make the linter happy, but a noop catch clause would never prevent messages to the logs. In the future, I'll ask for noop catches only on other kinds of bare promises.

doExit(
facets.zoeSeatAdmin,
state.currentAllocation,
Expand Down Expand Up @@ -355,6 +356,7 @@ export const makeZoeSeatAdminFactory = baggage => {
};
},
{
/** @type {UserSeat} */
userSeat: {
async getProposal() {
return this.state.userSeatAccess.getProposal();
Expand Down
Loading