[taler-wallet-core] branch master updated: wallet-core: fix bugs.taler.net/n/7836

[gnunet]

This is an automated email from the git hooks/post-receive script.

dold pushed a commit to branch master
in repository wallet-core.

The following commit(s) were added to refs/heads/master by this push:
     new ef4cc1a1c wallet-core: fix bugs.taler.net/n/7836
ef4cc1a1c is described below

commit ef4cc1a1ca568cf9e43964ca20bfc0b8c07a41e5
Author: Florian Dold <florian@dold.me>
AuthorDate: Fri Jan 19 14:35:00 2024 +0100

    wallet-core: fix bugs.taler.net/n/7836
    
    The test case for expired refunds now does more checks.
    
    A DB query bug for refund items was also fixed.
---
 .../src/integrationtests/test-refund-gone.ts       |  12 +-
 packages/taler-wallet-core/src/db.ts               |  10 ++
 .../taler-wallet-core/src/operations/exchanges.ts  |   3 +-
 .../src/operations/pay-merchant.ts                 | 172 ++++++++++++---------
 .../taler-wallet-core/src/operations/refresh.ts    |   5 +-
 .../taler-wallet-core/src/util/coinSelection.ts    |   3 +-
 6 files changed, 124 insertions(+), 81 deletions(-)

diff --git a/packages/taler-harness/src/integrationtests/test-refund-gone.ts 
b/packages/taler-harness/src/integrationtests/test-refund-gone.ts
index d50919934..60098cc9b 100644
--- a/packages/taler-harness/src/integrationtests/test-refund-gone.ts
+++ b/packages/taler-harness/src/integrationtests/test-refund-gone.ts
@@ -21,6 +21,8 @@ import {
   AbsoluteTime,
   Duration,
   MerchantApiClient,
+  TransactionMajorState,
+  TransactionType,
   durationFromSpec,
 } from "@gnu-taler/taler-util";
 import { WalletApiOperation } from "@gnu-taler/taler-wallet-core";
@@ -32,7 +34,8 @@ import {
 } from "../harness/helpers.js";
 
 /**
- * Run test for basic, bank-integrated withdrawal.
+ * Test wallet behavior when a refund expires before the wallet
+ * can claim it.
  */
 export async function runRefundGoneTest(t: GlobalTestState) {
   // Set up test environment
@@ -102,7 +105,7 @@ export async function runRefundGoneTest(t: GlobalTestState) 
{
 
   await applyTimeTravelV2(
     Duration.toMilliseconds(Duration.fromSpec({ hours: 1 })),
-    { exchange, walletClient: walletClient },
+    { exchange, merchant, walletClient: walletClient },
   );
 
   await exchange.runAggregatorOnce();
@@ -128,7 +131,10 @@ export async function runRefundGoneTest(t: 
GlobalTestState) {
   const r3 = await walletClient.call(WalletApiOperation.GetTransactions, {});
   console.log(JSON.stringify(r3, undefined, 2));
 
-  await t.shutdown();
+  const refundTx = r3.transactions[2];
+
+  t.assertDeepEqual(refundTx.type, TransactionType.Refund);
+  t.assertDeepEqual(refundTx.txState.major, TransactionMajorState.Failed);
 }
 
 runRefundGoneTest.suites = ["wallet"];
diff --git a/packages/taler-wallet-core/src/db.ts 
b/packages/taler-wallet-core/src/db.ts
index 73739c19c..f16600f5d 100644
--- a/packages/taler-wallet-core/src/db.ts
+++ b/packages/taler-wallet-core/src/db.ts
@@ -2217,6 +2217,7 @@ export enum RefundGroupStatus {
   Done = 0x0500_0000,
   Failed = 0x0501_0000,
   Aborted = 0x0503_0000,
+  Expired = 0x0502_0000,
 }
 
 /**
@@ -2641,6 +2642,7 @@ export const WalletStoresV1 = {
         "coinPub",
         "rtxid",
       ]),
+      // FIXME: Why is this a list of index keys? Confusing!
       byRefundGroupId: describeIndex("byRefundGroupId", ["refundGroupId"]),
     },
   ),
@@ -2663,6 +2665,14 @@ export type WalletDbReadWriteTransaction<
   Stores extends StoreNames<typeof WalletStoresV1> & string,
 > = DbReadWriteTransaction<typeof WalletStoresV1, Stores>;
 
+export type WalletDbReadWriteTransactionArr<
+  StoresArr extends Array<StoreNames<typeof WalletStoresV1>>,
+> = DbReadWriteTransactionArr<typeof WalletStoresV1, StoresArr>;
+
+export type WalletDbReadOnlyTransactionArr<
+  StoresArr extends Array<StoreNames<typeof WalletStoresV1>>,
+> = DbReadOnlyTransactionArr<typeof WalletStoresV1, StoresArr>;
+
 /**
  * An applied migration.
  */
diff --git a/packages/taler-wallet-core/src/operations/exchanges.ts 
b/packages/taler-wallet-core/src/operations/exchanges.ts
index f983a7c4d..67404665c 100644
--- a/packages/taler-wallet-core/src/operations/exchanges.ts
+++ b/packages/taler-wallet-core/src/operations/exchanges.ts
@@ -89,6 +89,7 @@ import {
   ExchangeEntryDbUpdateStatus,
   PendingTaskType,
   WalletDbReadWriteTransaction,
+  WalletDbReadWriteTransactionArr,
   createTimeline,
   isWithdrawableDenom,
   selectBestForOverlappingDenominations,
@@ -421,7 +422,7 @@ async function validateGlobalFees(
  * if the DB transaction succeeds.
  */
 export async function addPresetExchangeEntry(
-  tx: WalletDbReadWriteTransaction<"exchanges">,
+  tx: WalletDbReadWriteTransactionArr<["exchanges"]>,
   exchangeBaseUrl: string,
   currencyHint?: string,
 ): Promise<{ notification?: WalletNotification }> {
diff --git a/packages/taler-wallet-core/src/operations/pay-merchant.ts 
b/packages/taler-wallet-core/src/operations/pay-merchant.ts
index 50b73acb7..f6bbe5b9f 100644
--- a/packages/taler-wallet-core/src/operations/pay-merchant.ts
+++ b/packages/taler-wallet-core/src/operations/pay-merchant.ts
@@ -111,6 +111,7 @@ import {
   timestampPreciseToDb,
   timestampProtocolFromDb,
   timestampProtocolToDb,
+  WalletDbReadWriteTransactionArr,
 } from "../index.js";
 import {
   EXCHANGE_COINS_LOCK,
@@ -2006,8 +2007,8 @@ async function processPurchasePay(
       ) {
         // Do this in the background, as it might take some time
         handleInsufficientFunds(ws, proposalId, err).catch(async (e) => {
-          console.log("handling insufficient funds failed");
-          console.log(`${e.toString()}`);
+          logger.error("handling insufficient funds failed");
+          logger.error(`${e.toString()}`);
         });
 
         // FIXME: Should we really consider this to be pending?
@@ -2853,6 +2854,55 @@ export async function startQueryRefund(
   ws.workAvailable.trigger();
 }
 
+async function computeRefreshRequest(
+  ws: InternalWalletState,
+  tx: WalletDbReadWriteTransactionArr<["coins", "denominations"]>,
+  items: RefundItemRecord[],
+): Promise<CoinRefreshRequest[]> {
+  const refreshCoins: CoinRefreshRequest[] = [];
+  for (const item of items) {
+    const coin = await tx.coins.get(item.coinPub);
+    if (!coin) {
+      throw Error("coin not found");
+    }
+    const denomInfo = await ws.getDenomInfo(
+      ws,
+      tx,
+      coin.exchangeBaseUrl,
+      coin.denomPubHash,
+    );
+    if (!denomInfo) {
+      throw Error("denom not found");
+    }
+    if (item.status === RefundItemStatus.Done) {
+      const refundedAmount = Amounts.sub(
+        item.refundAmount,
+        denomInfo.feeRefund,
+      ).amount;
+      refreshCoins.push({
+        amount: Amounts.stringify(refundedAmount),
+        coinPub: item.coinPub,
+      });
+    }
+  }
+  return refreshCoins;
+}
+
+/**
+ * Compute the refund item status based on the merchant's response.
+ */
+function getItemStatus(rf: MerchantCoinRefundStatus): RefundItemStatus {
+  if (rf.type === "success") {
+    return RefundItemStatus.Done;
+  } else {
+    if (rf.exchange_status >= 500 && rf.exchange_status <= 599) {
+      return RefundItemStatus.Pending;
+    } else {
+      return RefundItemStatus.Failed;
+    }
+  }
+}
+
 /**
  * Store refunds, possibly creating a new refund group.
  */
@@ -2875,59 +2925,19 @@ async function storeRefunds(
   const download = await expectProposalDownload(ws, purchase);
   const currency = Amounts.currencyOf(download.contractData.amount);
 
-  const getItemStatus = (rf: MerchantCoinRefundStatus) => {
-    if (rf.type === "success") {
-      return RefundItemStatus.Done;
-    } else {
-      if (rf.exchange_status >= 500 && rf.exchange_status <= 599) {
-        return RefundItemStatus.Pending;
-      } else {
-        return RefundItemStatus.Failed;
-      }
-    }
-  };
-
-  const result = await ws.db
-    .mktx((x) => [
-      x.purchases,
-      x.refundGroups,
-      x.refundItems,
-      x.coins,
-      x.denominations,
-      x.coinAvailability,
-      x.refreshGroups,
-    ])
-    .runReadWrite(async (tx) => {
-      const computeRefreshRequest = async (items: RefundItemRecord[]) => {
-        const refreshCoins: CoinRefreshRequest[] = [];
-        for (const item of items) {
-          const coin = await tx.coins.get(item.coinPub);
-          if (!coin) {
-            throw Error("coin not found");
-          }
-          const denomInfo = await ws.getDenomInfo(
-            ws,
-            tx,
-            coin.exchangeBaseUrl,
-            coin.denomPubHash,
-          );
-          if (!denomInfo) {
-            throw Error("denom not found");
-          }
-          if (item.status === RefundItemStatus.Done) {
-            const refundedAmount = Amounts.sub(
-              item.refundAmount,
-              denomInfo.feeRefund,
-            ).amount;
-            refreshCoins.push({
-              amount: Amounts.stringify(refundedAmount),
-              coinPub: item.coinPub,
-            });
-          }
-        }
-        return refreshCoins;
-      };
-
+  const result = await ws.db.runReadWriteTx(
+    [
+      "coins",
+      "denominations",
+      "purchases",
+      "refundItems",
+      "refundGroups",
+      "denominations",
+      "coins",
+      "coinAvailability",
+      "refreshGroups",
+    ],
+    async (tx) => {
       const myPurchase = await tx.purchases.get(purchase.proposalId);
       if (!myPurchase) {
         logger.warn("purchase group not found anymore");
@@ -3008,7 +3018,11 @@ async function storeRefunds(
       // we can compute the raw/effective amounts.
       if (newGroup) {
         const amountsRaw = newGroupRefunds.map((x) => x.refundAmount);
-        const refreshCoins = await computeRefreshRequest(newGroupRefunds);
+        const refreshCoins = await computeRefreshRequest(
+          ws,
+          tx,
+          newGroupRefunds,
+        );
         const outInfo = await calculateRefreshOutput(
           ws,
           tx,
@@ -3028,35 +3042,40 @@ async function storeRefunds(
         myPurchase.proposalId,
       );
 
-      logger.info(
-        `refund groups for proposal ${myPurchase.proposalId}: ${j2s(
-          refundGroups,
-        )}`,
-      );
-
       for (const refundGroup of refundGroups) {
-        if (refundGroup.status === RefundGroupStatus.Aborted) {
-          continue;
-        }
-        if (refundGroup.status === RefundGroupStatus.Done) {
-          continue;
+        switch (refundGroup.status) {
+          case RefundGroupStatus.Aborted:
+          case RefundGroupStatus.Expired:
+          case RefundGroupStatus.Failed:
+          case RefundGroupStatus.Done:
+            continue;
+          case RefundGroupStatus.Pending:
+            break;
+          default:
+            assertUnreachable(refundGroup.status);
         }
-        const items = await tx.refundItems.indexes.byRefundGroupId.getAll(
+        const items = await tx.refundItems.indexes.byRefundGroupId.getAll([
           refundGroup.refundGroupId,
-        );
+        ]);
         let numPending = 0;
+        let numFailed = 0;
         for (const item of items) {
           if (item.status === RefundItemStatus.Pending) {
             numPending++;
           }
+          if (item.status === RefundItemStatus.Failed) {
+            numFailed++;
+          }
         }
-        logger.info(`refund items pending for refund group: ${numPending}`);
         if (numPending === 0) {
-          logger.info("refund group is done!");
           // We're done for this refund group!
-          refundGroup.status = RefundGroupStatus.Done;
+          if (numFailed === 0) {
+            refundGroup.status = RefundGroupStatus.Done;
+          } else {
+            refundGroup.status = RefundGroupStatus.Failed;
+          }
           await tx.refundGroups.put(refundGroup);
-          const refreshCoins = await computeRefreshRequest(items);
+          const refreshCoins = await computeRefreshRequest(ws, tx, items);
           await createRefreshGroup(
             ws,
             tx,
@@ -3085,7 +3104,8 @@ async function storeRefunds(
           newTxState,
         },
       };
-    });
+    },
+  );
 
   if (!result) {
     return TaskRunResult.finished();
@@ -3120,5 +3140,9 @@ export function computeRefundTransactionState(
       return {
         major: TransactionMajorState.Pending,
       };
+    case RefundGroupStatus.Expired:
+      return {
+        major: TransactionMajorState.Expired,
+      };
   }
 }
diff --git a/packages/taler-wallet-core/src/operations/refresh.ts 
b/packages/taler-wallet-core/src/operations/refresh.ts
index fc2508cd3..d49c9a1cf 100644
--- a/packages/taler-wallet-core/src/operations/refresh.ts
+++ b/packages/taler-wallet-core/src/operations/refresh.ts
@@ -84,6 +84,7 @@ import {
   timestampPreciseToDb,
   timestampProtocolFromDb,
   WalletDbReadWriteTransaction,
+  WalletDbReadWriteTransactionArr,
 } from "../index.js";
 import {
   EXCHANGE_COINS_LOCK,
@@ -1244,8 +1245,8 @@ async function applyRefresh(
  */
 export async function createRefreshGroup(
   ws: InternalWalletState,
-  tx: WalletDbReadWriteTransaction<
-    "denominations" | "coins" | "refreshGroups" | "coinAvailability"
+  tx: WalletDbReadWriteTransactionArr<
+    ["denominations", "coins", "refreshGroups", "coinAvailability"]
   >,
   currency: string,
   oldCoinPubs: CoinRefreshRequest[],
diff --git a/packages/taler-wallet-core/src/util/coinSelection.ts 
b/packages/taler-wallet-core/src/util/coinSelection.ts
index 9b29cee26..d75450a64 100644
--- a/packages/taler-wallet-core/src/util/coinSelection.ts
+++ b/packages/taler-wallet-core/src/util/coinSelection.ts
@@ -60,6 +60,7 @@ import {
   getExchangeWireDetailsInTx,
   isWithdrawableDenom,
   WalletDbReadOnlyTransaction,
+  WalletDbReadOnlyTransactionArr,
 } from "../index.js";
 import { InternalWalletState } from "../internal-wallet-state.js";
 import {
@@ -914,7 +915,7 @@ export interface PeerCoinSelectionRequest {
  */
 async function selectPayPeerCandidatesForExchange(
   ws: InternalWalletState,
-  tx: WalletDbReadOnlyTransaction<"coinAvailability" | "denominations">,
+  tx: WalletDbReadOnlyTransactionArr<["coinAvailability", "denominations"]>,
   exchangeBaseUrl: string,
 ): Promise<AvailableDenom[]> {
   const denoms: AvailableDenom[] = [];

-- 
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.