From f05788f59d2872b52410e1689b2c7e896541dff9 Mon Sep 17 00:00:00 2001 From: Florian Dold Date: Tue, 10 Dec 2019 12:22:29 +0100 Subject: fix session IDs, fix proposal download concurrency bug --- src/dbTypes.ts | 9 +++++ src/wallet-impl/pay.ts | 88 +++++++++++++++++++++++++++++++++++----------- src/wallet-impl/pending.ts | 2 +- 3 files changed, 78 insertions(+), 21 deletions(-) diff --git a/src/dbTypes.ts b/src/dbTypes.ts index e0abf1362..28c1ee2e3 100644 --- a/src/dbTypes.ts +++ b/src/dbTypes.ts @@ -1052,6 +1052,15 @@ export interface PurchaseRecord { */ lastSessionId: string | undefined; + /** + * Set for the first payment, or on re-plays. + */ + paymentSubmitPending: boolean; + + /** + * Do we need to query the merchant for the refund status + * of the payment? + */ refundStatusRequested: boolean; /** diff --git a/src/wallet-impl/pay.ts b/src/wallet-impl/pay.ts index 89b124553..af9d44066 100644 --- a/src/wallet-impl/pay.ts +++ b/src/wallet-impl/pay.ts @@ -333,11 +333,19 @@ async function recordConfirmPay( proposal: ProposalRecord, payCoinInfo: PayCoinInfo, chosenExchange: string, + sessionIdOverride: string | undefined, ): Promise { const d = proposal.download; if (!d) { throw Error("proposal is in invalid state"); } + let sessionId; + if (sessionIdOverride) { + sessionId = sessionIdOverride; + } else { + sessionId = proposal.downloadSessionId; + } + logger.trace(`recording payment with session ID ${sessionId}`); const payReq: PayReq = { coins: payCoinInfo.sigs, merchant_pub: d.contractTerms.merchant_pub, @@ -349,7 +357,7 @@ async function recordConfirmPay( abortRequested: false, contractTerms: d.contractTerms, contractTermsHash: d.contractTermsHash, - lastSessionId: undefined, + lastSessionId: sessionId, merchantSig: d.merchantSig, payReq, refundsDone: {}, @@ -366,6 +374,7 @@ async function recordConfirmPay( refundApplyRetryInfo: initRetryInfo(), firstSuccessfulPayTimestamp: undefined, autoRefundDeadline: undefined, + paymentSubmitPending: true, }; await runWithWriteTransaction( @@ -562,13 +571,12 @@ async function resetDownloadProposalRetry( ws: InternalWalletState, proposalId: string, ) { - await oneShotMutate(ws.db, Stores.proposals, proposalId, (x) => { + await oneShotMutate(ws.db, Stores.proposals, proposalId, x => { if (x.retryInfo.active) { x.retryInfo = initRetryInfo(); } return x; }); - } async function processDownloadProposalImpl( @@ -667,7 +675,7 @@ async function startDownloadProposal( ws: InternalWalletState, merchantBaseUrl: string, orderId: string, - sessionId?: string, + sessionId: string | undefined, ): Promise { const oldProposal = await oneShotGetIndexed( ws.db, @@ -694,9 +702,21 @@ async function startDownloadProposal( repurchaseProposalId: undefined, retryInfo: initRetryInfo(), lastError: undefined, + downloadSessionId: sessionId, }; - await oneShotPut(ws.db, Stores.proposals, proposalRecord); + await runWithWriteTransaction(ws.db, [Stores.proposals], async (tx) => { + const existingRecord = await tx.getIndexed(Stores.proposals.urlAndOrderIdIndex, [ + merchantBaseUrl, + orderId, + ]); + if (existingRecord) { + // Created concurrently + return; + } + await tx.put(Stores.proposals, proposalRecord); + }); + await processDownloadProposal(ws, proposalId); return proposalId; } @@ -704,7 +724,6 @@ async function startDownloadProposal( export async function submitPay( ws: InternalWalletState, proposalId: string, - sessionId: string | undefined, ): Promise { const purchase = await oneShotGet(ws.db, Stores.purchases, proposalId); if (!purchase) { @@ -713,9 +732,12 @@ export async function submitPay( if (purchase.abortRequested) { throw Error("not submitting payment for aborted purchase"); } + const sessionId = purchase.lastSessionId; let resp; const payReq = { ...purchase.payReq, session_id: sessionId }; + console.log("paying with session ID", sessionId); + const payUrl = new URL("pay", purchase.contractTerms.merchant_base_url).href; try { @@ -729,7 +751,7 @@ export async function submitPay( throw Error(`unexpected status (${resp.status}) for /pay`); } const merchantResp = await resp.json(); - console.log("got success from pay URL"); + console.log("got success from pay URL", merchantResp); const merchantPub = purchase.contractTerms.merchant_pub; const valid: boolean = await ws.cryptoApi.isValidPaymentSignature( @@ -744,6 +766,7 @@ export async function submitPay( } const isFirst = purchase.firstSuccessfulPayTimestamp === undefined; purchase.firstSuccessfulPayTimestamp = getTimestampNow(); + purchase.paymentSubmitPending = false; purchase.lastPayError = undefined; purchase.payRetryInfo = initRetryInfo(false); if (isFirst) { @@ -916,7 +939,7 @@ export async function preparePay( } if (uriResult.sessionId) { - await submitPay(ws, proposalId, uriResult.sessionId); + await submitPay(ws, proposalId); } return { @@ -985,14 +1008,26 @@ export async function confirmPay( throw Error("proposal is in invalid state"); } - const sessionId = sessionIdOverride || proposal.downloadSessionId; - let purchase = await oneShotGet(ws.db, Stores.purchases, d.contractTermsHash); if (purchase) { - return submitPay(ws, proposalId, sessionId); + if ( + sessionIdOverride !== undefined && + sessionIdOverride != purchase.lastSessionId + ) { + logger.trace(`changing session ID to ${sessionIdOverride}`); + await oneShotMutate(ws.db, Stores.purchases, purchase.proposalId, x => { + x.lastSessionId = sessionIdOverride; + x.paymentSubmitPending = true; + return x; + }); + } + logger.trace("confirmPay: submitting payment for existing purchase"); + return submitPay(ws, proposalId); } + logger.trace("confirmPay: purchase record does not exist yet"); + const contractAmount = Amounts.parseOrThrow(d.contractTerms.amount); let wireFeeLimit; @@ -1029,17 +1064,25 @@ export async function confirmPay( cds, totalAmount, ); - purchase = await recordConfirmPay(ws, proposal, payCoinInfo, exchangeUrl); + purchase = await recordConfirmPay( + ws, + proposal, + payCoinInfo, + exchangeUrl, + sessionIdOverride, + ); } else { purchase = await recordConfirmPay( ws, sd.proposal, sd.payCoinInfo, sd.exchangeUrl, + sessionIdOverride, ); } - return submitPay(ws, proposalId, sessionId); + logger.trace("confirmPay: submitting payment after creating purchase record"); + return submitPay(ws, proposalId); } export async function getFullRefundFees( @@ -1249,7 +1292,7 @@ async function resetPurchasePayRetry( ws: InternalWalletState, proposalId: string, ) { - await oneShotMutate(ws.db, Stores.purchases, proposalId, (x) => { + await oneShotMutate(ws.db, Stores.purchases, proposalId, x => { if (x.payRetryInfo.active) { x.payRetryInfo = initRetryInfo(); } @@ -1269,11 +1312,11 @@ async function processPurchasePayImpl( if (!purchase) { return; } - logger.trace(`processing purchase pay ${proposalId}`); - if (purchase.firstSuccessfulPayTimestamp) { + if (!purchase.paymentSubmitPending) { return; } - await submitPay(ws, proposalId, purchase.lastSessionId); + logger.trace(`processing purchase pay ${proposalId}`); + await submitPay(ws, proposalId); } export async function processPurchaseQueryRefund( @@ -1289,8 +1332,10 @@ export async function processPurchaseQueryRefund( ); } - -async function resetPurchaseQueryRefundRetry(ws: InternalWalletState, proposalId: string) { +async function resetPurchaseQueryRefundRetry( + ws: InternalWalletState, + proposalId: string, +) { await oneShotMutate(ws.db, Stores.purchases, proposalId, x => { if (x.refundStatusRetryInfo.active) { x.refundStatusRetryInfo = initRetryInfo(); @@ -1349,7 +1394,10 @@ export async function processPurchaseApplyRefund( ); } -async function resetPurchaseApplyRefundRetry(ws: InternalWalletState, proposalId: string) { +async function resetPurchaseApplyRefundRetry( + ws: InternalWalletState, + proposalId: string, +) { await oneShotMutate(ws.db, Stores.purchases, proposalId, x => { if (x.refundApplyRetryInfo.active) { x.refundApplyRetryInfo = initRetryInfo(); diff --git a/src/wallet-impl/pending.ts b/src/wallet-impl/pending.ts index 022895e95..7079fa5ff 100644 --- a/src/wallet-impl/pending.ts +++ b/src/wallet-impl/pending.ts @@ -360,7 +360,7 @@ async function gatherPurchasePending( onlyDue: boolean = false, ): Promise { await tx.iter(Stores.purchases).forEach(pr => { - if (!pr.firstSuccessfulPayTimestamp) { + if (pr.paymentSubmitPending) { resp.nextRetryDelay = updateRetryDelay( resp.nextRetryDelay, now, -- cgit v1.2.3