commit d40f79731bd2d3541d42a698225ae0da6981d7aa
parent ad46372fee8088d94f74b502c083989117695056
Author: Florian Dold <dold@taler.net>
Date: Mon, 27 Jul 2026 18:19:29 +0200
wallet: discard a payment claim that can never succeed
Issue: https://bugs.taler.net/n/10698
Diffstat:
6 files changed, 281 insertions(+), 87 deletions(-)
diff --git a/packages/taler-harness/src/integrationtests/test-payment-claim-already-claimed.ts b/packages/taler-harness/src/integrationtests/test-payment-claim-already-claimed.ts
@@ -0,0 +1,186 @@
+/*
+ This file is part of GNU Taler
+ (C) 2026 Taler Systems S.A.
+
+ GNU Taler is free software; you can redistribute it and/or modify it under the
+ terms of the GNU General Public License as published by the Free Software
+ Foundation; either version 3, or (at your option) any later version.
+
+ GNU Taler is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along with
+ GNU Taler; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+ */
+
+/**
+ * Imports.
+ */
+import {
+ NotificationType,
+ TalerErrorCode,
+ TalerMerchantApi,
+ TalerMerchantInstanceHttpClient,
+ TransactionIdStr,
+ TransactionMajorState,
+ TransactionMinorState,
+ TransactionStateTransitionNotification,
+ succeedOrThrow,
+} from "@gnu-taler/taler-util";
+import { WalletApiOperation } from "@gnu-taler/taler-wallet-core";
+import {
+ createSimpleTestkudosEnvironmentV3,
+ createWalletDaemonWithClient,
+} from "../harness/environments.js";
+import { GlobalTestState, WalletClient } from "../harness/harness.js";
+
+/**
+ * Scanning a payment URI for an order that another wallet has already claimed
+ * must report the conflict and must not leave a transaction behind.
+ *
+ * The user never saw a contract for such a scan, so there is nothing to show
+ * them and nothing for them to act on. The claim runs in the background, so
+ * the error is reported in the state transition that removes the transaction.
+ */
+export async function runPaymentClaimAlreadyClaimedTest(t: GlobalTestState) {
+ const { walletClient, merchant, merchantAdminAccessToken } =
+ await createSimpleTestkudosEnvironmentV3(t);
+
+ const merchantClient = new TalerMerchantInstanceHttpClient(
+ merchant.makeInstanceBaseUrl(),
+ );
+
+ const orderResp = succeedOrThrow(
+ await merchantClient.createOrder(merchantAdminAccessToken, {
+ order: {
+ summary: "Buy me!",
+ amount: "TESTKUDOS:5",
+ fulfillment_url: "taler://fulfillment-success/thx",
+ } satisfies TalerMerchantApi.Order,
+ }),
+ );
+ const orderStatus = succeedOrThrow(
+ await merchantClient.getOrderDetails(
+ merchantAdminAccessToken,
+ orderResp.order_id,
+ ),
+ );
+ t.assertTrue(orderStatus.order_status === "unpaid");
+ const talerPayUri = orderStatus.taler_pay_uri;
+
+ // The first wallet claims the order, so that the merchant answers any
+ // further claim with a conflict.
+ const firstPrep = await walletClient.call(
+ WalletApiOperation.PreparePayForUriV2,
+ { talerPayUri },
+ );
+ await walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
+ transactionId: firstPrep.transactionId,
+ txState: {
+ major: TransactionMajorState.Dialog,
+ minor: TransactionMinorState.Proposed,
+ },
+ });
+
+ let w2 = await createWalletDaemonWithClient(t, {
+ name: "w2",
+ persistent: true,
+ });
+
+ async function restartWallet(): Promise<void> {
+ w2.walletClient.remoteWallet?.close();
+ await w2.walletService.stop();
+ w2 = await createWalletDaemonWithClient(t, {
+ name: "w2",
+ persistent: true,
+ });
+ }
+
+ async function hasTransaction(
+ walletClient: WalletClient,
+ transactionId: TransactionIdStr,
+ ): Promise<boolean> {
+ try {
+ await walletClient.call(WalletApiOperation.GetTransactionById, {
+ transactionId,
+ });
+ return true;
+ } catch (e) {
+ return false;
+ }
+ }
+
+ /**
+ * Wait for a claim to be given up on because the order is already claimed.
+ *
+ * Must be set up before the scan that triggers it: the waiter only sees
+ * notifications that arrive after the condition is registered, and the
+ * claim fails on the task scheduler's schedule.
+ */
+ function claimDiscardedCond(
+ walletClient: WalletClient,
+ ): Promise<TransactionStateTransitionNotification> {
+ return walletClient.waitForNotificationCond((n) =>
+ n.type === NotificationType.TransactionStateTransition &&
+ n.newTxState.major === TransactionMajorState.Deleted &&
+ n.errorInfo?.code === TalerErrorCode.WALLET_ORDER_ALREADY_CLAIMED
+ ? n
+ : false,
+ );
+ }
+
+ // The second wallet scans the URI of the order it cannot have.
+
+ await t.runSpanAsync("scan reports the conflict", async () => {
+ const discarded = claimDiscardedCond(w2.walletClient);
+ const scan = await w2.walletClient.call(
+ WalletApiOperation.PreparePayForUriV2,
+ { talerPayUri },
+ );
+
+ const notif = await discarded;
+ t.assertDeepEqual(notif.transactionId, scan.transactionId);
+ t.assertDeepEqual(
+ notif.oldTxState.minor,
+ TransactionMinorState.ClaimProposal,
+ );
+
+ // Nothing is left behind, neither as a hidden record nor in the list.
+ t.assertTrue(!(await hasTransaction(w2.walletClient, scan.transactionId)));
+ const txs = await w2.walletClient.call(
+ WalletApiOperation.GetTransactionsV2,
+ { includeAll: true },
+ );
+ t.assertTrue(
+ !txs.transactions.some((x) => x.transactionId === scan.transactionId),
+ );
+
+ await restartWallet();
+ t.assertTrue(!(await hasTransaction(w2.walletClient, scan.transactionId)));
+
+ // Scanning again claims anew instead of handing back the gone
+ // transaction, and gives up on that claim the same way.
+ const discardedAgain = claimDiscardedCond(w2.walletClient);
+ const rescan = await w2.walletClient.call(
+ WalletApiOperation.PreparePayForUriV2,
+ { talerPayUri },
+ );
+ t.assertTrue(rescan.transactionId !== scan.transactionId);
+ const notifAgain = await discardedAgain;
+ t.assertDeepEqual(notifAgain.transactionId, rescan.transactionId);
+ t.assertTrue(
+ !(await hasTransaction(w2.walletClient, rescan.transactionId)),
+ );
+ });
+
+ // The wallet that does hold the claim keeps its offer.
+ await t.runSpanAsync("claim holder is unaffected", async () => {
+ const tx = await walletClient.call(WalletApiOperation.GetTransactionById, {
+ transactionId: firstPrep.transactionId,
+ });
+ t.assertDeepEqual(tx.txState.major, TransactionMajorState.Dialog);
+ });
+}
+
+runPaymentClaimAlreadyClaimedTest.suites = ["wallet"];
diff --git a/packages/taler-harness/src/integrationtests/test-payment-claim.ts b/packages/taler-harness/src/integrationtests/test-payment-claim.ts
@@ -18,12 +18,13 @@
* Imports.
*/
import {
- j2s,
+ NotificationType,
succeedOrThrow,
TalerErrorCode,
TalerMerchantInstanceHttpClient,
TransactionMajorState,
TransactionMinorState,
+ TransactionStateTransitionNotification,
} from "@gnu-taler/taler-util";
import { WalletApiOperation } from "@gnu-taler/taler-wallet-core";
import {
@@ -107,7 +108,25 @@ export async function runPaymentClaimTest(t: GlobalTestState) {
},
});
- // The second wallet can't claim the order anymore.
+ /**
+ * Wait for a claim to be given up on because the order is already claimed.
+ *
+ * Must be set up before the scan that triggers it: the waiter only sees
+ * notifications that arrive after the condition is registered.
+ */
+ function claimDiscardedCond(): Promise<TransactionStateTransitionNotification> {
+ return w2.walletClient.waitForNotificationCond((n) =>
+ n.type === NotificationType.TransactionStateTransition &&
+ n.newTxState.major === TransactionMajorState.Deleted &&
+ n.errorInfo?.code === TalerErrorCode.WALLET_ORDER_ALREADY_CLAIMED
+ ? n
+ : false,
+ );
+ }
+
+ // The second wallet can't claim the order anymore. The claim is given up on
+ // and its transaction removed, with the reason in the transition.
+ const claimOneDiscarded = claimDiscardedCond();
const claimOne = await w2.walletClient.call(
WalletApiOperation.PreparePayForUriV2,
{
@@ -115,22 +134,8 @@ export async function runPaymentClaimTest(t: GlobalTestState) {
},
);
- await w2.walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
- transactionId: claimOne.transactionId,
- txState: {
- major: TransactionMajorState.Failed,
- minor: TransactionMinorState.ClaimProposal,
- },
- });
-
- const claimOneTx = await w2.walletClient.call(
- WalletApiOperation.GetTransactionById,
- {
- transactionId: claimOne.transactionId,
- },
- );
-
- console.log(j2s(claimOneTx.failReason));
+ const claimOneNotif = await claimOneDiscarded;
+ t.assertDeepEqual(claimOneNotif.transactionId, claimOne.transactionId);
await walletClient.call(WalletApiOperation.ConfirmPay, {
transactionId: preparePayResult.transactionId,
@@ -150,6 +155,7 @@ export async function runPaymentClaimTest(t: GlobalTestState) {
await w2.walletClient.call(WalletApiOperation.ClearDb, {});
+ const claimTwoDiscarded = claimDiscardedCond();
const claimTwo = await w2.walletClient.call(
WalletApiOperation.PreparePayForUriV2,
{
@@ -157,31 +163,19 @@ export async function runPaymentClaimTest(t: GlobalTestState) {
},
);
- await w2.walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
- transactionId: claimTwo.transactionId,
- txState: {
- major: TransactionMajorState.Failed,
- minor: TransactionMinorState.ClaimProposal,
- },
- });
-
- const claimTwoTx = await w2.walletClient.call(
- WalletApiOperation.GetTransactionById,
- {
- transactionId: claimTwo.transactionId,
- },
- );
-
+ const claimTwoNotif = await claimTwoDiscarded;
+ t.assertDeepEqual(claimTwoNotif.transactionId, claimTwo.transactionId);
t.assertDeepEqual(
- claimTwoTx.failReason?.code,
+ claimTwoNotif.errorInfo?.code,
TalerErrorCode.WALLET_ORDER_ALREADY_CLAIMED,
);
+ // Neither failed claim left anything behind.
const txn = await w2.walletClient.call(WalletApiOperation.GetTransactionsV2, {
includeAll: true,
});
- console.log(j2s(txn));
+ t.assertDeepEqual(txn.transactions.length, 0);
}
runPaymentClaimTest.suites = ["wallet"];
diff --git a/packages/taler-harness/src/integrationtests/testrunner.ts b/packages/taler-harness/src/integrationtests/testrunner.ts
@@ -135,6 +135,7 @@ import { runPaivanaRepurchaseTest } from "./test-paivana-repurchase.js";
import { runPaivanaTest } from "./test-paivana.js";
import { runPayPaidTest } from "./test-pay-paid.js";
import { runPaymentAbortTest } from "./test-payment-abort.js";
+import { runPaymentClaimAlreadyClaimedTest } from "./test-payment-claim-already-claimed.js";
import { runPaymentClaimCleanupTest } from "./test-payment-claim-cleanup.js";
import { runPaymentClaimTest } from "./test-payment-claim.js";
import { runPaymentDeletedTest } from "./test-payment-deleted.js";
@@ -285,6 +286,7 @@ const allTests: TestMainFunction[] = [
runMerchantSpecPublicOrdersTest,
runPaymentClaimTest,
runPaymentClaimCleanupTest,
+ runPaymentClaimAlreadyClaimedTest,
runPaymentForgettableTest,
runPaymentIdempotencyTest,
runPaymentMultipleTest,
diff --git a/packages/taler-wallet-core/src/common.ts b/packages/taler-wallet-core/src/common.ts
@@ -25,6 +25,7 @@ import {
CoinStatus,
Duration,
DurationUnitSpec,
+ ErrorInfoSummary,
ExchangeEntryState,
ExchangeEntryStatus,
ExchangeTosStatus,
@@ -1075,6 +1076,7 @@ export interface RecordHandle<T> {
newRec: T | undefined,
causeHint: string,
eff?: BalanceEffect,
+ errorInfo?: ErrorInfoSummary,
): Promise<void>;
}
@@ -1105,6 +1107,7 @@ export async function getGenericRecordHandle<T>(
newRec: T | undefined,
causeHint: string,
eff?: BalanceEffect,
+ errorInfo?: ErrorInfoSummary,
) => {
let newTxState: TransactionState;
let newStId: number;
@@ -1126,6 +1129,7 @@ export async function getGenericRecordHandle<T>(
newTxState,
balanceEffect: eff ?? BalanceEffect.Any,
causeHint,
+ errorInfo,
oldStId,
newStId,
});
diff --git a/packages/taler-wallet-core/src/pay-merchant.ts b/packages/taler-wallet-core/src/pay-merchant.ts
@@ -47,6 +47,7 @@ import {
DownloadedContractData,
Duration,
encodeCrock,
+ ErrorInfoSummary,
ForcedCoinSel,
GetChoicesForPaymentResult,
getRandomBytes,
@@ -57,7 +58,6 @@ import {
j2s,
Logger,
makeErrorDetail,
- makePendingOperationFailedError,
makeTalerErrorDetail,
MerchantCoinRefundStatus,
MerchantContractInputType,
@@ -415,7 +415,11 @@ export class PayMerchantTransactionContext implements TransactionContext {
async deleteTransactionInTx(
tx: WalletDbTransaction,
- opts: { keepRelated?: boolean } = {},
+ opts: {
+ keepRelated?: boolean;
+ causeHint?: string;
+ errorInfo?: ErrorInfoSummary;
+ } = {},
): Promise<void> {
const [rec, h] = await this.getRecordHandle(tx);
if (!rec) {
@@ -448,7 +452,12 @@ export class PayMerchantTransactionContext implements TransactionContext {
);
await otherCtx.deleteTransactionInTx(tx, { keepRelated: true });
}
- await h.update(undefined, "delete");
+ await h.update(
+ undefined,
+ opts.causeHint ?? "delete",
+ undefined,
+ opts.errorInfo,
+ );
}
async userSuspendTransaction(): Promise<void> {
@@ -903,11 +912,12 @@ async function retryLeftoverClaim(
logger.info(
`discarding leftover claim ${ctx.transactionId}: ${safeStringifyException(e)}`,
);
- await discardLeftoverClaim(wex, proposalId);
+ await discardClaim(wex, proposalId);
return TaskRunResult.finished();
}
- // A claim that fails permanently does not throw, it parks the transaction
- // in a failed state instead.
+ // A claim that fails permanently discards the purchase itself, but one that
+ // merely did not get anywhere leaves it behind for us to clean up. The
+ // failed state only occurs for records written before claims were discarded.
const purchase = await wex.runWalletDbTx(async (tx) => {
return tx.getPurchase(proposalId);
});
@@ -915,40 +925,38 @@ async function retryLeftoverClaim(
case PurchaseStatus.PendingDownloadingProposal:
case PurchaseStatus.FailedClaim:
logger.info(`discarding leftover claim ${ctx.transactionId}`);
- await discardLeftoverClaim(wex, proposalId);
+ await discardClaim(wex, proposalId);
return TaskRunResult.finished();
}
return res;
}
-async function discardLeftoverClaim(
+/**
+ * Discard a purchase whose claim did not produce a contract.
+ *
+ * Such a purchase has no contract terms, so there is nothing to show the user
+ * and nothing for them to act on. Keeping it would leave a transaction that
+ * the list cannot render and that every later scan of the same order would
+ * reuse instead of claiming again.
+ *
+ * When a reason is given, it travels with the deletion notification, since
+ * that transition is the only thing a client still sees of the transaction.
+ */
+async function discardClaim(
wex: WalletExecutionContext,
proposalId: string,
+ err?: TalerErrorDetail,
): Promise<void> {
const ctx = new PayMerchantTransactionContext(wex, proposalId);
await wex.runWalletDbTx(async (tx) => {
- await ctx.deleteTransactionInTx(tx);
+ await ctx.deleteTransactionInTx(tx, {
+ causeHint: err ? "claim-failed" : undefined,
+ errorInfo: err ? { code: err.code as number, hint: err.hint } : undefined,
+ });
});
wex.taskScheduler.stopShepherdTask(ctx.taskId);
}
-async function failProposalClaimPermanently(
- wex: WalletExecutionContext,
- proposalId: string,
- err: TalerErrorDetail,
-): Promise<void> {
- const ctx = new PayMerchantTransactionContext(wex, proposalId);
- await wex.runWalletDbTx(async (tx) => {
- const [p, h] = await ctx.getRecordHandle(tx);
- if (!p) {
- return;
- }
- p.purchaseStatus = PurchaseStatus.FailedClaim;
- p.failReason = err;
- await h.update(p, "claim-failed");
- });
-}
-
/**
* Long-poll timeout (in milliseconds) for merchant order-status requests.
*/
@@ -1061,7 +1069,7 @@ async function processDownloadProposal(
case "ok":
break;
case HttpStatusCode.Conflict:
- await failProposalClaimPermanently(
+ await discardClaim(
wex,
proposalId,
makeTalerErrorDetail(
@@ -1075,7 +1083,7 @@ async function processDownloadProposal(
);
return TaskRunResult.finished();
case TalerErrorCode.MERCHANT_POST_ORDERS_ID_CLAIM_NOT_FOUND: {
- await failProposalClaimPermanently(
+ await discardClaim(
wex,
proposalId,
makeTalerErrorDetail(
@@ -1114,12 +1122,8 @@ async function processDownloadProposal(
{},
"validation for well-formedness failed",
);
- await failProposalClaimPermanently(wex, proposalId, err);
- throw makePendingOperationFailedError(
- err,
- TransactionType.Payment,
- proposalId,
- );
+ await discardClaim(wex, proposalId, err);
+ return TaskRunResult.finished();
}
const contractTermsHash = ContractTermsUtil.hashContractTerms(
@@ -1138,12 +1142,8 @@ async function processDownloadProposal(
{},
`schema validation failed: ${e}`,
);
- await failProposalClaimPermanently(wex, proposalId, err);
- throw makePendingOperationFailedError(
- err,
- TransactionType.Payment,
- proposalId,
- );
+ await discardClaim(wex, proposalId, err);
+ return TaskRunResult.finished();
}
isWellFormed = ContractTermsUtil.validateParsed(parsedContractTerms);
@@ -1157,12 +1157,8 @@ async function processDownloadProposal(
{},
"validation for well-formedness failed (validateParsed)",
);
- await failProposalClaimPermanently(wex, proposalId, err);
- throw makePendingOperationFailedError(
- err,
- TransactionType.Payment,
- proposalId,
- );
+ await discardClaim(wex, proposalId, err);
+ return TaskRunResult.finished();
}
const sigValid = await wex.cryptoApi.isValidContractTermsSignature({
@@ -1180,12 +1176,8 @@ async function processDownloadProposal(
},
"merchant's signature on contract terms is invalid",
);
- await failProposalClaimPermanently(wex, proposalId, err);
- throw makePendingOperationFailedError(
- err,
- TransactionType.Payment,
- proposalId,
- );
+ await discardClaim(wex, proposalId, err);
+ return TaskRunResult.finished();
}
const fulfillmentUrl = parsedContractTerms.fulfillment_url;
@@ -1461,6 +1453,13 @@ async function createOrReusePurchase(
// Should never happen, except for backwards compat.
oldProposal = oldProposals[0];
}
+ if (oldProposal?.purchaseStatus === PurchaseStatus.FailedClaim) {
+ // Written by an older version of the wallet, which kept a record for a
+ // claim that never produced a contract. Reusing it would hand back a
+ // transaction the user cannot see instead of claiming again.
+ await discardClaim(wex, oldProposal.proposalId);
+ oldProposal = undefined;
+ }
// If we have already claimed this proposal with the same
// nonce and claim token, reuse it.
if (oldProposal) {
diff --git a/packages/taler-wallet-core/src/transactions.ts b/packages/taler-wallet-core/src/transactions.ts
@@ -21,6 +21,7 @@ import {
AbsoluteTime,
Amounts,
assertUnreachable,
+ ErrorInfoSummary,
GetTransactionsV2Request,
j2s,
Logger,
@@ -905,6 +906,11 @@ export interface TransitionInfo {
*/
causeHint: string | undefined;
+ /**
+ * Reason for a transition into an error state, reported to clients.
+ */
+ errorInfo?: ErrorInfoSummary;
+
newStId: number;
oldStId: number;
}
@@ -971,6 +977,9 @@ export function applyNotifyTransition(
transactionId,
causeHint: transitionInfo.causeHint,
newStId: transitionInfo.newStId,
+ ...(transitionInfo.errorInfo
+ ? { errorInfo: transitionInfo.errorInfo }
+ : {}),
});
applyNotifyBalanceEffect(
