fix coin selection

packages/taler-util/package.json

@@ -6,6 +6,7 @@
     ".": "./lib/index.js"
   },
   "module": "./lib/index.js",
+  "main": "./lib/index.js",
   "types": "./lib/index.d.ts",
   "typesVersions": {
     "*": {

packages/taler-wallet-core/src/operations/pay.ts

@@ -24,12 +24,72 @@
 /**
  * Imports.
  */
-import { AmountJson, Amounts, timestampIsBetween, getTimestampNow, isTimestampExpired, Timestamp, RefreshReason, CoinDepositPermission, NotificationType, TalerErrorDetails, Duration, durationMax, durationMin, durationMul, ContractTerms, codecForProposal, TalerErrorCode, codecForContractTerms, timestampAddDuration, ConfirmPayResult, ConfirmPayResultType, codecForMerchantPayResponse, PreparePayResult, PreparePayResultType, parsePayUri } from "@gnu-taler/taler-util";
+import {
+  AmountJson,
+  Amounts,
+  timestampIsBetween,
+  getTimestampNow,
+  isTimestampExpired,
+  Timestamp,
+  RefreshReason,
+  CoinDepositPermission,
+  NotificationType,
+  TalerErrorDetails,
+  Duration,
+  durationMax,
+  durationMin,
+  durationMul,
+  ContractTerms,
+  codecForProposal,
+  TalerErrorCode,
+  codecForContractTerms,
+  timestampAddDuration,
+  ConfirmPayResult,
+  ConfirmPayResultType,
+  codecForMerchantPayResponse,
+  PreparePayResult,
+  PreparePayResultType,
+  parsePayUri,
+} from "@gnu-taler/taler-util";
 import { encodeCrock, getRandomBytes } from "../crypto/talerCrypto";
-import { AbortStatus, AllowedAuditorInfo, AllowedExchangeInfo, CoinRecord, CoinStatus, DenominationRecord, getHttpResponseErrorDetails, guardOperationException, HttpResponseStatus, Logger, makeErrorDetails, OperationFailedAndReportedError, OperationFailedError, ProposalRecord, ProposalStatus, PurchaseRecord, readSuccessResponseJsonOrErrorCode, readSuccessResponseJsonOrThrow, readTalerErrorResponse, Stores, throwUnexpectedRequestError, TransactionHandle, URL, WalletContractData } from "../index.js";
+import {
-import { PayCoinSelection, CoinCandidateSelection, AvailableCoinInfo, selectPayCoins } from "../util/coinSelection.js";
+  AbortStatus,
+  AllowedAuditorInfo,
+  AllowedExchangeInfo,
+  CoinRecord,
+  CoinStatus,
+  DenominationRecord,
+  getHttpResponseErrorDetails,
+  guardOperationException,
+  HttpResponseStatus,
+  Logger,
+  makeErrorDetails,
+  OperationFailedAndReportedError,
+  OperationFailedError,
+  ProposalRecord,
+  ProposalStatus,
+  PurchaseRecord,
+  readSuccessResponseJsonOrErrorCode,
+  readSuccessResponseJsonOrThrow,
+  readTalerErrorResponse,
+  Stores,
+  throwUnexpectedRequestError,
+  TransactionHandle,
+  URL,
+  WalletContractData,
+} from "../index.js";
+import {
+  PayCoinSelection,
+  CoinCandidateSelection,
+  AvailableCoinInfo,
+  selectPayCoins,
+} from "../util/coinSelection.js";
 import { canonicalJson } from "../util/helpers.js";
-import { initRetryInfo, updateRetryInfoTimeout, getRetryDuration } from "../util/retries.js";
+import {
+  initRetryInfo,
+  updateRetryInfoTimeout,
+  getRetryDuration,
+} from "../util/retries.js";
 import { getTotalRefreshCost, createRefreshGroup } from "./refresh.js";
 import { InternalWalletState, EXCHANGE_COINS_LOCK } from "./state.js";
 
@@ -38,7 +98,6 @@ import { InternalWalletState, EXCHANGE_COINS_LOCK } from "./state.js";
  */
 const logger = new Logger("pay.ts");
 
-
 /**
  * Compute the total cost of a payment to the customer.
  *
@@ -123,7 +182,6 @@ export async function getEffectiveDepositAmount(
   return Amounts.sub(Amounts.sum(amt).amount, Amounts.sum(fees).amount).amount;
 }
 
-
 export function isSpendableCoin(
   coin: CoinRecord,
   denom: DenominationRecord,

packages/taler-wallet-core/src/util/coinSelection.ts

@@ -23,8 +23,11 @@
 /**
  * Imports.
  */
-import { AmountJson, Amounts } from "@gnu-taler/taler-util";
+import { AmountJson, AmountLike, Amounts } from "@gnu-taler/taler-util";
-import { strcmp } from "./helpers";
+import { strcmp } from "./helpers.js";
+import { Logger } from "./logging.js";
+
+const logger = new Logger("coinSelection.ts");
 
 /**
  * Result of selecting coins, contains the exchange, and selected
@@ -107,6 +110,103 @@ export interface SelectPayCoinRequest {
   prevPayCoins?: PreviousPayCoins;
 }
 
+interface CoinSelectionTally {
+  /**
+   * Amount that still needs to be paid.
+   * May increase during the computation when fees need to be covered.
+   */
+  amountPayRemaining: AmountJson;
+
+  /**
+   * Allowance given by the merchant towards wire fees
+   */
+  amountWireFeeLimitRemaining: AmountJson;
+
+  /**
+   * Allowance given by the merchant towards deposit fees
+   * (and wire fees after wire fee limit is exhausted)
+   */
+  amountDepositFeeLimitRemaining: AmountJson;
+
+  customerDepositFees: AmountJson;
+
+  customerWireFees: AmountJson;
+
+  wireFeeCoveredForExchange: Set<string>;
+}
+
+/**
+ * Account for the fees of spending a coin.
+ */
+function tallyFees(
+  tally: CoinSelectionTally,
+  wireFeesPerExchange: Record<string, AmountJson>,
+  wireFeeAmortization: number,
+  exchangeBaseUrl: string,
+  feeDeposit: AmountJson,
+): CoinSelectionTally {
+  const currency = tally.amountPayRemaining.currency;
+  let amountWireFeeLimitRemaining = tally.amountWireFeeLimitRemaining;
+  let amountDepositFeeLimitRemaining = tally.amountDepositFeeLimitRemaining;
+  let customerDepositFees = tally.customerDepositFees;
+  let customerWireFees = tally.customerWireFees;
+  let amountPayRemaining = tally.amountPayRemaining;
+  const wireFeeCoveredForExchange = new Set(tally.wireFeeCoveredForExchange);
+
+  if (!tally.wireFeeCoveredForExchange.has(exchangeBaseUrl)) {
+    const wf =
+      wireFeesPerExchange[exchangeBaseUrl] ?? Amounts.getZero(currency);
+    const wfForgiven = Amounts.min(amountWireFeeLimitRemaining, wf);
+    amountWireFeeLimitRemaining = Amounts.sub(
+      amountWireFeeLimitRemaining,
+      wfForgiven,
+    ).amount;
+    // The remaining, amortized amount needs to be paid by the
+    // wallet or covered by the deposit fee allowance.
+    let wfRemaining = Amounts.divide(
+      Amounts.sub(wf, wfForgiven).amount,
+      wireFeeAmortization,
+    );
+
+    // This is the amount forgiven via the deposit fee allowance.
+    const wfDepositForgiven = Amounts.min(
+      amountDepositFeeLimitRemaining,
+      wfRemaining,
+    );
+    amountDepositFeeLimitRemaining = Amounts.sub(
+      amountDepositFeeLimitRemaining,
+      wfDepositForgiven,
+    ).amount;
+
+    wfRemaining = Amounts.sub(wfRemaining, wfDepositForgiven).amount;
+    customerWireFees = Amounts.add(customerWireFees, wfRemaining).amount;
+    amountPayRemaining = Amounts.add(amountPayRemaining, wfRemaining).amount;
+
+    wireFeeCoveredForExchange.add(exchangeBaseUrl);
+  }
+
+  const dfForgiven = Amounts.min(feeDeposit, amountDepositFeeLimitRemaining);
+
+  amountDepositFeeLimitRemaining = Amounts.sub(
+    amountDepositFeeLimitRemaining,
+    dfForgiven,
+  ).amount;
+
+  // How much does the user spend on deposit fees for this coin?
+  const dfRemaining = Amounts.sub(feeDeposit, dfForgiven).amount;
+  customerDepositFees = Amounts.add(customerDepositFees, dfRemaining).amount;
+  amountPayRemaining = Amounts.add(amountPayRemaining, dfRemaining).amount;
+
+  return {
+    amountDepositFeeLimitRemaining,
+    amountPayRemaining,
+    amountWireFeeLimitRemaining,
+    customerDepositFees,
+    customerWireFees,
+    wireFeeCoveredForExchange,
+  };
+}
+
 /**
  * Given a list of candidate coins, select coins to spend under the merchant's
  * constraints.
@@ -134,75 +234,31 @@ export function selectPayCoins(
   const coinPubs: string[] = [];
   const coinContributions: AmountJson[] = [];
   const currency = contractTermsAmount.currency;
-  // Amount that still needs to be paid.
-  // May increase during the computation when fees need to be covered.
-  let amountPayRemaining = contractTermsAmount;
-  // Allowance given by the merchant towards wire fees
-  let amountWireFeeLimitRemaining = wireFeeLimit;
-  // Allowance given by the merchant towards deposit fees
-  // (and wire fees after wire fee limit is exhausted)
-  let amountDepositFeeLimitRemaining = depositFeeLimit;
-  let customerDepositFees = Amounts.getZero(currency);
-  let customerWireFees = Amounts.getZero(currency);
 
-  const wireFeeCoveredForExchange: Set<string> = new Set();
+  let tally: CoinSelectionTally = {
-
+    amountPayRemaining: contractTermsAmount,
-  /**
+    amountWireFeeLimitRemaining: wireFeeLimit,
-   * Account for the fees of spending a coin.
+    amountDepositFeeLimitRemaining: depositFeeLimit,
-   */
+    customerDepositFees: Amounts.getZero(currency),
-  function tallyFees(exchangeBaseUrl: string, feeDeposit: AmountJson): void {
+    customerWireFees: Amounts.getZero(currency),
-    if (!wireFeeCoveredForExchange.has(exchangeBaseUrl)) {
+    wireFeeCoveredForExchange: new Set(),
-      const wf =
+  };
-        candidates.wireFeesPerExchange[exchangeBaseUrl] ??
-        Amounts.getZero(currency);
-      const wfForgiven = Amounts.min(amountWireFeeLimitRemaining, wf);
-      amountWireFeeLimitRemaining = Amounts.sub(
-        amountWireFeeLimitRemaining,
-        wfForgiven,
-      ).amount;
-      // The remaining, amortized amount needs to be paid by the
-      // wallet or covered by the deposit fee allowance.
-      let wfRemaining = Amounts.divide(
-        Amounts.sub(wf, wfForgiven).amount,
-        wireFeeAmortization,
-      );
-
-      // This is the amount forgiven via the deposit fee allowance.
-      const wfDepositForgiven = Amounts.min(
-        amountDepositFeeLimitRemaining,
-        wfRemaining,
-      );
-      amountDepositFeeLimitRemaining = Amounts.sub(
-        amountDepositFeeLimitRemaining,
-        wfDepositForgiven,
-      ).amount;
-
-      wfRemaining = Amounts.sub(wfRemaining, wfDepositForgiven).amount;
-      customerWireFees = Amounts.add(customerWireFees, wfRemaining).amount;
-      amountPayRemaining = Amounts.add(amountPayRemaining, wfRemaining).amount;
-      wireFeeCoveredForExchange.add(exchangeBaseUrl);
-    }
-
-    const dfForgiven = Amounts.min(feeDeposit, amountDepositFeeLimitRemaining);
-
-    amountDepositFeeLimitRemaining = Amounts.sub(
-      amountDepositFeeLimitRemaining,
-      dfForgiven,
-    ).amount;
-
-    // How much does the user spend on deposit fees for this coin?
-    const dfRemaining = Amounts.sub(feeDeposit, dfForgiven).amount;
-    customerDepositFees = Amounts.add(customerDepositFees, dfRemaining).amount;
-    amountPayRemaining = Amounts.add(amountPayRemaining, dfRemaining).amount;
-  }
 
   const prevPayCoins = req.prevPayCoins ?? [];
 
   // Look at existing pay coin selection and tally up
   for (const prev of prevPayCoins) {
-    tallyFees(prev.exchangeBaseUrl, prev.feeDeposit);
+    tally = tallyFees(
-    amountPayRemaining = Amounts.sub(amountPayRemaining, prev.contribution)
+      tally,
-      .amount;
+      candidates.wireFeesPerExchange,
+      wireFeeAmortization,
+      prev.exchangeBaseUrl,
+      prev.feeDeposit,
+    );
+    tally.amountPayRemaining = Amounts.sub(
+      tally.amountPayRemaining,
+      prev.contribution,
+    ).amount;
 
     coinPubs.push(prev.coinPub);
     coinContributions.push(prev.contribution);
@@ -231,7 +287,7 @@ export function selectPayCoins(
     if (Amounts.cmp(aci.feeDeposit, aci.availableAmount) >= 0) {
       continue;
     }
-    if (Amounts.isZero(amountPayRemaining)) {
+    if (Amounts.isZero(tally.amountPayRemaining)) {
       // We have spent enough!
       break;
     }
@@ -242,21 +298,34 @@ export function selectPayCoins(
       continue;
     }
 
-    tallyFees(aci.exchangeBaseUrl, aci.feeDeposit);
+    tally = tallyFees(
+      tally,
+      candidates.wireFeesPerExchange,
+      wireFeeAmortization,
+      aci.exchangeBaseUrl,
+      aci.feeDeposit,
+    );
 
-    const coinSpend = Amounts.min(amountPayRemaining, aci.availableAmount);
+    let coinSpend = Amounts.max(
-    amountPayRemaining = Amounts.sub(amountPayRemaining, coinSpend).amount;
+      Amounts.min(tally.amountPayRemaining, aci.availableAmount),
+      aci.feeDeposit,
+    );
+
+    tally.amountPayRemaining = Amounts.sub(
+      tally.amountPayRemaining,
+      coinSpend,
+    ).amount;
     coinPubs.push(aci.coinPub);
     coinContributions.push(coinSpend);
   }
 
-  if (Amounts.isZero(amountPayRemaining)) {
+  if (Amounts.isZero(tally.amountPayRemaining)) {
     return {
       paymentAmount: contractTermsAmount,
       coinContributions,
       coinPubs,
-      customerDepositFees,
+      customerDepositFees: tally.customerDepositFees,
-      customerWireFees,
+      customerWireFees: tally.customerWireFees,
     };
   }
   return undefined;