fix issue with wire fee in coin selection that caused wrongly reported fees

3 files changed, 22 insertions, 15 deletions

diff --git a/src/headless/taler-wallet-cli.ts b/src/headless/taler-wallet-cli.ts
index bceb82950..72d00a065 100644
--- a/src/headless/taler-wallet-cli.ts
+++ b/src/headless/taler-wallet-cli.ts
@@ -70,6 +70,7 @@ async function doPay(
     throw Error("not reached");
   }
   console.log("contract", result.contractTermsRaw);
+  console.log("total fees:", Amounts.stringify(result.totalFees));
   let pay;
   if (options.alwaysYes) {
     pay = true;
diff --git a/src/operations/pay.ts b/src/operations/pay.ts
index 372d012d8..b2627f74d 100644
--- a/src/operations/pay.ts
+++ b/src/operations/pay.ts
@@ -177,7 +177,8 @@ export async function getTotalPaymentCost(
  */
 export function selectPayCoins(
   acis: AvailableCoinInfo[],
-  paymentAmount: AmountJson,
+  contractTermsAmount: AmountJson,
+  customerWireFees: AmountJson,
   depositFeeLimit: AmountJson,
 ): PayCoinSelection | undefined {
   if (acis.length === 0) {
@@ -194,11 +195,10 @@ export function selectPayCoins(
       Amounts.cmp(o1.feeDeposit, o2.feeDeposit) ||
       strcmp(o1.denomPub, o2.denomPub),
   );
+  const paymentAmount = Amounts.add(contractTermsAmount, customerWireFees).amount;
   const currency = paymentAmount.currency;
-  let totalFees = Amounts.getZero(currency);
   let amountPayRemaining = paymentAmount;
   let amountDepositFeeLimitRemaining = depositFeeLimit;
-  const customerWireFees = Amounts.getZero(currency);
   const customerDepositFees = Amounts.getZero(currency);
   for (const aci of acis) {
     // Don't use this coin if depositing it is more expensive than
@@ -254,11 +254,10 @@ export function selectPayCoins(
 
     coinPubs.push(aci.coinPub);
     coinContributions.push(coinSpend);
-    totalFees = Amounts.add(totalFees, depositFeeSpend).amount;
   }
   if (Amounts.isZero(amountPayRemaining)) {
     return {
-      paymentAmount,
+      paymentAmount: contractTermsAmount,
       coinContributions,
       coinPubs,
       customerDepositFees,
@@ -278,7 +277,7 @@ async function getCoinsForPayment(
   ws: InternalWalletState,
   contractData: WalletContractData,
 ): Promise<PayCoinSelection | undefined> {
-  let remainingAmount = contractData.amount;
+  const remainingAmount = contractData.amount;
 
   const exchanges = await ws.db.iter(Stores.exchanges).toArray();
 
@@ -367,7 +366,6 @@ async function getCoinsForPayment(
       });
     }
 
-    let totalFees = Amounts.getZero(currency);
     let wireFee: AmountJson | undefined;
     for (const fee of exchangeFees.feesForType[contractData.wireMethod] || []) {
       if (
@@ -379,21 +377,27 @@ async function getCoinsForPayment(
       }
     }
 
+    let customerWireFee: AmountJson;
+
     if (wireFee) {
       const amortizedWireFee = Amounts.divide(
         wireFee,
         contractData.wireFeeAmortization,
       );
       if (Amounts.cmp(contractData.maxWireFee, amortizedWireFee) < 0) {
-        totalFees = Amounts.add(amortizedWireFee, totalFees).amount;
-        remainingAmount = Amounts.add(amortizedWireFee, remainingAmount).amount;
+        customerWireFee = amortizedWireFee;
+      } else {
+        customerWireFee = Amounts.getZero(currency);
       }
+    } else {
+      customerWireFee = Amounts.getZero(currency);
     }
 
     // Try if paying using this exchange works
     const res = selectPayCoins(
       acis,
       remainingAmount,
+      customerWireFee,
       contractData.maxDepositFee,
     );
     if (res) {
@@ -914,6 +918,8 @@ export async function preparePayForUri(
     }
 
     const costInfo = await getTotalPaymentCost(ws, res);
+    console.log("costInfo", costInfo);
+    console.log("coinsForPayment", res);
     const totalFees = Amounts.sub(costInfo.totalCost, res.paymentAmount).amount;
 
     return {
diff --git a/src/wallet-test.ts b/src/wallet-test.ts
index 422523665..4b06accf2 100644
--- a/src/wallet-test.ts
+++ b/src/wallet-test.ts
@@ -43,7 +43,7 @@ test("coin selection 1", (t) => {
     fakeAci("EUR:1.0", "EUR:0.0"),
   ];
 
-  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0.1"));
+  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0"), a("EUR:0.1"));
   if (!res) {
     t.fail();
     return;
@@ -59,7 +59,7 @@ test("coin selection 2", (t) => {
     // Merchant covers the fee, this one shouldn't be used
     fakeAci("EUR:1.0", "EUR:0.0"),
   ];
-  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0.5"));
+  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0"), a("EUR:0.5"));
   if (!res) {
     t.fail();
     return;
@@ -75,7 +75,7 @@ test("coin selection 3", (t) => {
     // this coin should be selected instead of previous one with fee
     fakeAci("EUR:1.0", "EUR:0.0"),
   ];
-  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0.5"));
+  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0"), a("EUR:0.5"));
   if (!res) {
     t.fail();
     return;
@@ -90,7 +90,7 @@ test("coin selection 4", (t) => {
     fakeAci("EUR:1.0", "EUR:0.5"),
     fakeAci("EUR:1.0", "EUR:0.5"),
   ];
-  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0.5"));
+  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0"), a("EUR:0.5"));
   if (!res) {
     t.fail();
     return;
@@ -105,7 +105,7 @@ test("coin selection 5", (t) => {
     fakeAci("EUR:1.0", "EUR:0.5"),
     fakeAci("EUR:1.0", "EUR:0.5"),
   ];
-  const res = selectPayCoins(acis, a("EUR:4.0"), a("EUR:0.2"));
+  const res = selectPayCoins(acis, a("EUR:4.0"), a("EUR:0"), a("EUR:0.2"));
   t.true(!res);
   t.pass();
 });
@@ -115,7 +115,7 @@ test("coin selection 6", (t) => {
     fakeAci("EUR:1.0", "EUR:0.5"),
     fakeAci("EUR:1.0", "EUR:0.5"),
   ];
-  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0.2"));
+  const res = selectPayCoins(acis, a("EUR:2.0"), a("EUR:0"), a("EUR:0.2"));
   t.true(!res);
   t.pass();
 });