From 695a6a43ea143475b2dddd070a2e16680b2bc9c7 Mon Sep 17 00:00:00 2001
From: Florian Dold <florian@dold.me>
Date: Mon, 4 Mar 2024 23:34:17 +0100
Subject: fix totally broken p2p coin selection

---
 packages/taler-wallet-core/src/coinSelection.ts | 79 ++++++++++++-------------
 1 file changed, 38 insertions(+), 41 deletions(-)

(limited to 'packages/taler-wallet-core/src/coinSelection.ts')

diff --git a/packages/taler-wallet-core/src/coinSelection.ts b/packages/taler-wallet-core/src/coinSelection.ts
index 680e5faa1..3ece5546c 100644
--- a/packages/taler-wallet-core/src/coinSelection.ts
+++ b/packages/taler-wallet-core/src/coinSelection.ts
@@ -32,7 +32,6 @@ import {
   AllowedAuditorInfo,
   AllowedExchangeInfo,
   AmountJson,
-  AmountLike,
   Amounts,
   AmountString,
   checkDbInvariant,
@@ -64,11 +63,7 @@ import { getAutoRefreshExecuteThreshold } from "./common.js";
 import { DenominationRecord, WalletDbReadOnlyTransaction } from "./db.js";
 import { isWithdrawableDenom } from "./denominations.js";
 import { getExchangeWireDetailsInTx } from "./exchanges.js";
-import {
-  getDenomInfo,
-  InternalWalletState,
-  WalletExecutionContext,
-} from "./wallet.js";
+import { getDenomInfo, WalletExecutionContext } from "./wallet.js";
 
 const logger = new Logger("coinSelection.ts");
 
@@ -927,6 +922,9 @@ async function selectPayPeerCandidatesForExchange(
     );
 
   for (const coinAvail of myExchangeCoins) {
+    if (coinAvail.freshCoinCount <= 0) {
+      continue;
+    }
     const denom = await tx.denominations.get([
       coinAvail.exchangeBaseUrl,
       coinAvail.denomPubHash,
@@ -954,8 +952,8 @@ async function selectPayPeerCandidatesForExchange(
   return denoms;
 }
 
-interface PeerCoinSelectionTally {
-  amountAcc: AmountJson;
+export interface PeerCoinSelectionTally {
+  amountRemaining: AmountJson;
   depositFeesAcc: AmountJson;
   lastDepositFee: AmountJson;
 }
@@ -971,40 +969,37 @@ export function testing_greedySelectPeer(
 
 function greedySelectPeer(
   candidates: AvailableDenom[],
-  instructedAmount: AmountLike,
   tally: PeerCoinSelectionTally,
 ): SelResult | undefined {
   const selectedDenom: SelResult = {};
   for (const denom of candidates) {
     const contributions: AmountJson[] = [];
+    const feeDeposit = Amounts.parseOrThrow(denom.feeDeposit);
     for (
       let i = 0;
-      i < denom.numAvailable &&
-      Amounts.cmp(tally.amountAcc, instructedAmount) < 0;
+      i < denom.numAvailable && Amounts.isNonZero(tally.amountRemaining);
       i++
     ) {
-      const amountPayRemaining = Amounts.sub(
-        instructedAmount,
-        tally.amountAcc,
+      tally.depositFeesAcc = Amounts.add(
+        tally.depositFeesAcc,
+        feeDeposit,
+      ).amount;
+      tally.amountRemaining = Amounts.add(
+        tally.amountRemaining,
+        feeDeposit,
       ).amount;
-      // Maximum amount the coin could effectively contribute.
-      const maxCoinContrib = Amounts.sub(denom.value, denom.feeDeposit).amount;
+      tally.lastDepositFee = feeDeposit;
 
-      const coinSpend = Amounts.min(
-        Amounts.add(amountPayRemaining, denom.feeDeposit).amount,
-        maxCoinContrib,
+      const coinSpend = Amounts.max(
+        Amounts.min(tally.amountRemaining, denom.value),
+        denom.feeDeposit,
       );
 
-      tally.amountAcc = Amounts.add(tally.amountAcc, coinSpend).amount;
-      tally.amountAcc = Amounts.sub(tally.amountAcc, denom.feeDeposit).amount;
-
-      tally.depositFeesAcc = Amounts.add(
-        tally.depositFeesAcc,
-        denom.feeDeposit,
+      tally.amountRemaining = Amounts.sub(
+        tally.amountRemaining,
+        coinSpend,
       ).amount;
 
-      tally.lastDepositFee = Amounts.parseOrThrow(denom.feeDeposit);
-
       contributions.push(coinSpend);
     }
     if (contributions.length > 0) {
@@ -1027,14 +1022,12 @@ function greedySelectPeer(
       sd.contributions.push(...contributions);
       selectedDenom[avKey] = sd;
     }
-    if (Amounts.cmp(tally.amountAcc, instructedAmount) >= 0) {
-      break;
-    }
   }
 
-  if (Amounts.cmp(tally.amountAcc, instructedAmount) >= 0) {
+  if (Amounts.isZero(tally.amountRemaining)) {
     return selectedDenom;
   }
+
   return undefined;
 }
 
@@ -1071,8 +1064,11 @@ export async function selectPeerCoins(
           tx,
           exch.baseUrl,
         );
+        if (logger.shouldLogTrace()) {
+          logger.trace(`peer payment candidate coins: ${j2s(candidates)}`);
+        }
         const tally: PeerCoinSelectionTally = {
-          amountAcc: Amounts.zeroOfCurrency(currency),
+          amountRemaining: Amounts.parseOrThrow(instructedAmount),
           depositFeesAcc: Amounts.zeroOfCurrency(currency),
           lastDepositFee: Amounts.zeroOfCurrency(currency),
         };
@@ -1109,8 +1105,8 @@ export async function selectPeerCoins(
             });
             const depositFee = Amounts.parseOrThrow(denom.feeDeposit);
             tally.lastDepositFee = depositFee;
-            tally.amountAcc = Amounts.add(
-              tally.amountAcc,
+            tally.amountRemaining = Amounts.sub(
+              tally.amountRemaining,
               Amounts.sub(contrib, depositFee).amount,
             ).amount;
             tally.depositFeesAcc = Amounts.add(
@@ -1120,11 +1116,13 @@ export async function selectPeerCoins(
           }
         }
 
-        const selectedDenom = greedySelectPeer(
-          candidates,
-          instructedAmount,
-          tally,
-        );
+        if (logger.shouldLogTrace()) {
+          logger.trace(`candidates: ${j2s(candidates)}`);
+          logger.trace(`instructedAmount: ${j2s(instructedAmount)}`);
+          logger.trace(`tally: ${j2s(tally)}`);
+        }
+
+        const selectedDenom = greedySelectPeer(candidates, tally);
 
         if (selectedDenom) {
           let minAutorefreshExecuteThreshold = TalerProtocolTimestamp.never();
@@ -1180,10 +1178,9 @@ export async function selectPeerCoins(
           return { type: "success", result: res };
         }
 
-        const diff = Amounts.sub(instructedAmount, tally.amountAcc).amount;
         exchangeFeeGap[exch.baseUrl] = Amounts.add(
           tally.lastDepositFee,
-          diff,
+          tally.amountRemaining,
         ).amount;
 
         continue;
-- 
cgit v1.2.3