From b5093a4e039e9d194ca2b683e0bdec9d09dca5d0 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Wed, 1 Nov 2023 16:27:28 -0700 Subject: [PATCH] refactor: add back a try-catch in executeOffer --- packages/smart-wallet/src/smartWallet.js | 123 ++++++++++++-------- packages/smart-wallet/test/test-addAsset.js | 37 ++++-- 2 files changed, 100 insertions(+), 60 deletions(-) diff --git a/packages/smart-wallet/src/smartWallet.js b/packages/smart-wallet/src/smartWallet.js index db57b3955dca..787cf1fdabfb 100644 --- a/packages/smart-wallet/src/smartWallet.js +++ b/packages/smart-wallet/src/smartWallet.js @@ -735,61 +735,88 @@ export const prepareSmartWallet = (baggage, shared) => { facets.helper.assertUniqueOfferId(String(offerSpec.id)); - const invitationFromSpec = makeInvitationsHelper( - zoe, - agoricNames, - invitationBrand, - invitationPurse, - state.offerToInvitationMakers.get, - ); - - console.info( - 'wallet', - address, - 'starting executeOffer', - offerSpec.id, - ); - - // 1. Prepare values and validate synchronously. - const { proposal } = offerSpec; + let seatRef; + let watcher; + try { + const invitationFromSpec = makeInvitationsHelper( + zoe, + agoricNames, + invitationBrand, + invitationPurse, + state.offerToInvitationMakers.get, + ); - const invitation = invitationFromSpec(offerSpec.invitationSpec); + console.info( + 'wallet', + address, + 'starting executeOffer', + offerSpec.id, + ); - const [paymentKeywordRecord, invitationAmount] = await Promise.all([ - proposal?.give && - deeplyFulfilledObject( - facets.payments.withdrawGive(proposal.give), - ), - E(invitationIssuer).getAmountOf(invitation), - ]); + // 1. Prepare values and validate synchronously. + const { proposal } = offerSpec; - // 2. Begin executing offer - // No explicit signal to user that we reached here but if anything above - // failed they'd get an 'error' status update. + const invitation = invitationFromSpec(offerSpec.invitationSpec); - /** @type {UserSeat} */ - const seatRef = await E(zoe).offer( - invitation, - proposal, - paymentKeywordRecord, - offerSpec.offerArgs, - ); + const [paymentKeywordRecord, invitationAmount] = await Promise.all([ + proposal?.give && + deeplyFulfilledObject( + facets.payments.withdrawGive(proposal.give), + ), + E(invitationIssuer).getAmountOf(invitation), + ]); + + // 2. Begin executing offer + // No explicit signal to user that we reached here but if anything above + // failed they'd get an 'error' status update. + + /** @type {UserSeat} */ + seatRef = await E(zoe).offer( + invitation, + proposal, + paymentKeywordRecord, + offerSpec.offerArgs, + ); + console.info('wallet', address, offerSpec.id, 'seated'); - console.info('wallet', address, offerSpec.id, 'seated'); + watcher = makeOfferWatcher( + facets.helper, + facets.deposit, + offerSpec, + address, + invitationAmount, + seatRef, + ); - const watcher = makeOfferWatcher( - facets.helper, - facets.deposit, - offerSpec, - address, - invitationAmount, - seatRef, - ); + watchOfferOutcomes(watcher, seatRef); + state.liveOffers.init(offerSpec.id, offerSpec); + facets.helper.publishCurrentState(); + state.liveOfferSeats.init(offerSpec.id, seatRef); + } catch (err) { + console.error('OFFER ERROR:', err); + // Notify the user + debugger; + if (watcher) { + watcher.helper.updateStatus({ error: err.toString() }); + } else { + facets.helper.updateStatus( + { + error: err.toString(), + ...offerSpec, + }, + address, + ); + } - watchOfferOutcomes(watcher, seatRef); - state.liveOffers.init(offerSpec.id, offerSpec); - facets.helper.publishCurrentState(); - state.liveOfferSeats.init(offerSpec.id, seatRef); + if (seatRef) { + E.when(E(seatRef).hasExited(), hasExited => { + if (!hasExited) { + void E(seatRef).tryExit(); + } + }); + } + throw err; + } }, /** * Take an offer's id, look up its seat, try to exit. diff --git a/packages/smart-wallet/test/test-addAsset.js b/packages/smart-wallet/test/test-addAsset.js index 748a2a806a7b..1ec96d2527f4 100644 --- a/packages/smart-wallet/test/test-addAsset.js +++ b/packages/smart-wallet/test/test-addAsset.js @@ -11,10 +11,10 @@ import bundleSource from '@endo/bundle-source'; import { makeMarshal } from '@endo/marshal'; import { resolve as importMetaResolve } from 'import-meta-resolve'; import { makeDefaultTestContext } from './contexts.js'; -import { ActionType, headValue, makeMockTestSpace } from './supports.js'; +import { ActionType, makeMockTestSpace } from './supports.js'; import { makeImportContext } from '../src/marshal-contexts.js'; -const { Fail } = assert; +const { Fail, quote: q } = assert; const importSpec = spec => importMetaResolve(spec, import.meta.url).then(u => new URL(u).pathname); @@ -376,8 +376,10 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => { ); const wallet = simpleProvideWallet(addr); t.log('deposit', istQty, 'IST into wallet of', addr); - await E(E(wallet).getDepositFacet()).receive(spendingPmt); + + // order of updates changed so offerStatus might not be last const updates = await E(wallet).getUpdatesSubscriber(); + await E(E(wallet).getDepositFacet()).receive(spendingPmt); return updates; }; @@ -418,10 +420,21 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => { ['Boardwalk', 1n], ]; - /** @type {import('../src/smartWallet.js').UpdateRecord} */ - const update = await headValue(updates); - assert(update.updated === 'offerStatus'); - // t.log(update.status); + // status update used to be last, but now we have to look for the right one + let record = await E(updates).subscribeAfter(); + while ( + record.head.value.updated !== 'offerStatus' || + !record.head.value.status.numWantsSatisfied || + !record.head.value.status.payouts?.Places + ) { + record = await E(updates).subscribeAfter(record.publishCount); + } + const update = record.head.value; + + assert( + update.updated === 'offerStatus', + `Should have had "updated":"offerStatus", had "${q(update)}"`, + ); t.like(update, { updated: 'offerStatus', status: { @@ -435,7 +448,6 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => { const { status: { id, result, payouts }, } = update; - // @ts-expect-error cast value to copyBag const names = payouts?.Places.value.payload.map(([name, _qty]) => name); t.log(id, 'result:', result, ', payouts:', names.join(', ')); @@ -495,13 +507,15 @@ test.serial('non-vbank asset: give before deposit', async t => { proposal: { give, want }, }); t.log('goofy client: propose to give', choices.join(', ')); - await E(walletBridge).proposeOffer(ctx.fromBoard.toCapData(offer1)); + await t.throwsAsync( + () => E(walletBridge).proposeOffer(ctx.fromBoard.toCapData(offer1)), + { message: /Withdrawal of .* failed because the purse only contained/ }, + ); }; { const addr2 = 'agoric1player2'; const walletUIbridge = makePromiseKit(); - // await eventLoopIteration(); const { simpleProvideWallet, consume, sendToBridge } = t.context; const wallet = simpleProvideWallet(addr2); @@ -512,8 +526,7 @@ test.serial('non-vbank asset: give before deposit', async t => { const { aPlayer } = makeScenario(t); await aPlayer(addr2, walletUIbridge, mockStorage, sendToBridge, updates); - const c2 = goofyClient(mockStorage, walletUIbridge.promise); - await t.throwsAsync(c2, { message: /Withdrawal of {.*} failed/ }); + await goofyClient(mockStorage, walletUIbridge.promise); await eventLoopIteration(); // wallet balance was also updated