fix(zoe): fix auction makeBidInvitation #4899
Conversation
| @@ -102,20 +102,20 @@ const start = zcf => { | |||
| return bidSeats.map(seat => seat.getAmountAllocated('Bid', bidBrand)); | |||
| }; | |||
|
|
|||
| const getSessionDetails = () => { | |||
| const getSessionDetails = async () => { | |||
| const sellerProposal = sellSeat.getProposal(); | |||
| return harden({ | |||
There was a problem hiding this comment.
Your use of await nested within an expression violates our await design rule. This code is a good example of why. Looking at this code, is it clear whether sellerProposal.want.Ask and getCurrentBids() are evaluated before or after the await point?
Instead, if you want to use await here, I suggest you rename the variable that might hold a promise for a time authority to something else, and then await it at function top level to obtain the non-promise time authority variable.
| return harden({ | |
| const timeAuthority = await timeAuthorityP; | |
| return harden({ |
| const sellerProposal = sellSeat.getProposal(); | ||
| return harden({ | ||
| auctionedAssets: sellerProposal.give.Asset, | ||
| minimumBid: sellerProposal.want.Ask, | ||
| winnerPriceOption, | ||
| closesAfter, | ||
| bidDuration, | ||
| timeAuthority, | ||
| timeAuthority: await timeAuthority, |
There was a problem hiding this comment.
| timeAuthority: await timeAuthority, | |
| timeAuthority, |
| @@ -133,7 +133,7 @@ const start = zcf => { | |||
| return defaultAcceptanceMsg; | |||
| }; | |||
|
|
|||
| const customProperties = getSessionDetails(); | |||
| const customProperties = await getSessionDetails(); | |||
| @@ -40,7 +40,10 @@ test('zoe - secondPriceAuction w/ 3 bids', async t => { | |||
| Asset: moolaKit.issuer, | |||
| Ask: simoleanKit.issuer, | |||
| }); | |||
| const terms = { timeAuthority: timer, bidDuration: 1n }; | |||
| const terms = { | |||
| timeAuthority: Promise.resolve(timer), | |||
There was a problem hiding this comment.
I don't understand the purpose of this. Why wrap the timer in a promise? I'm equally confused by the other examples below.
|
@erights Thanks for reviewing. Your feedback about the However, I think I'd like to close this PR in favor of Agoric/dapp-card-store#51 since it's simpler. |
fixes: Agoric/dapp-card-store#50