fixed greedy calculation

2 files changed, 118 insertions, 26 deletions

diff --git a/packages/taler-wallet-core/src/util/coinSelection.test.ts b/packages/taler-wallet-core/src/util/coinSelection.test.ts
index f678e75e7..2a322c4a9 100644
--- a/packages/taler-wallet-core/src/util/coinSelection.test.ts
+++ b/packages/taler-wallet-core/src/util/coinSelection.test.ts
@@ -17,13 +17,14 @@ import {
   AbsoluteTime,
   AgeRestriction,
   AmountJson,
+  AmountString,
   Amounts,
   DenomKeyType,
   Duration,
   TransactionAmountMode,
 } from "@gnu-taler/taler-util";
 import test, { ExecutionContext } from "ava";
-import { testing_greedySelectPeer } from "./coinSelection.js"
+import { AvailableDenom, testing_greedySelectPeer } from "./coinSelection.js"
 
 type Tester<T> = {
   deep: {
@@ -47,6 +48,7 @@ const inTheDistantFuture = AbsoluteTime.toProtocolTimestamp(
 const inThePast = AbsoluteTime.toProtocolTimestamp(
   AbsoluteTime.subtractDuraction(AbsoluteTime.now(), Duration.fromSpec({ hours: 1 }))
 )
+
 test("should select the coin", (t) => {
   const instructedAmount = Amounts.parseOrThrow("LOCAL:2")
   const tally = {
@@ -55,16 +57,114 @@ test("should select the coin", (t) => {
     lastDepositFee: Amounts.zeroOfCurrency(instructedAmount.currency),
   };
   const coins = testing_greedySelectPeer(
-    // candidates available
-    [{
+    createCandidates([{
+      amount: "LOCAL:10",
+      numAvailable: 5,
+      depositFee: "LOCAL:0.1",
+      fromExchange: "http://exchange.localhost/",
+    }]),
+    instructedAmount, 
+    tally
+  );
+
+  expect(t, coins).deep.equal({
+    "hash0;32;http://exchange.localhost/": {
+      exchangeBaseUrl: "http://exchange.localhost/",
+      denomPubHash: "hash0",
+      maxAge: 32,
+      contributions: [Amounts.parseOrThrow("LOCAL:2")],
+    }
+  });
+
+  expect(t, tally).deep.equal({
+    amountAcc: Amounts.parseOrThrow("LOCAL:2"),
+    depositFeesAcc: Amounts.parseOrThrow("LOCAL:0.1"),
+    lastDepositFee: Amounts.parseOrThrow("LOCAL:0.1"),
+  });
+
+});
+
+test("should select 3 coins", (t) => {
+  const instructedAmount = Amounts.parseOrThrow("LOCAL:20")
+  const tally = {
+    amountAcc: Amounts.zeroOfCurrency(instructedAmount.currency),
+    depositFeesAcc: Amounts.zeroOfCurrency(instructedAmount.currency),
+    lastDepositFee: Amounts.zeroOfCurrency(instructedAmount.currency),
+  };
+  const coins = testing_greedySelectPeer(
+    createCandidates([{
+      amount: "LOCAL:10",
+      numAvailable: 5,
+      depositFee: "LOCAL:0.1",
+      fromExchange: "http://exchange.localhost/",
+    }]),
+    instructedAmount, 
+    tally
+  );
+
+  expect(t, coins).deep.equal({
+    "hash0;32;http://exchange.localhost/": {
+      exchangeBaseUrl: "http://exchange.localhost/",
+      denomPubHash: "hash0",
+      maxAge: 32,
+      contributions: [
+        Amounts.parseOrThrow("LOCAL:9.9"),
+        Amounts.parseOrThrow("LOCAL:9.9"),
+        Amounts.parseOrThrow("LOCAL:0.2")
+      ],
+    }
+  });
+
+  expect(t, tally).deep.equal({
+    amountAcc: Amounts.parseOrThrow("LOCAL:20"),
+    depositFeesAcc: Amounts.parseOrThrow("LOCAL:0.3"),
+    lastDepositFee: Amounts.parseOrThrow("LOCAL:0.1"),
+  });
+
+});
+
+test("can't select since the instructed amount is too high", (t) => {
+  const instructedAmount = Amounts.parseOrThrow("LOCAL:60")
+  const tally = {
+    amountAcc: Amounts.zeroOfCurrency(instructedAmount.currency),
+    depositFeesAcc: Amounts.zeroOfCurrency(instructedAmount.currency),
+    lastDepositFee: Amounts.zeroOfCurrency(instructedAmount.currency),
+  };
+  const coins = testing_greedySelectPeer(
+    createCandidates([{
+      amount: "LOCAL:10",
+      numAvailable: 5,
+      depositFee: "LOCAL:0.1",
+      fromExchange: "http://exchange.localhost/",
+    }]),
+    instructedAmount, 
+    tally
+  );
+
+  expect(t, coins).deep.equal(undefined);
+
+  expect(t, tally).deep.equal({
+    amountAcc: Amounts.parseOrThrow("LOCAL:49.5"),
+    depositFeesAcc: Amounts.parseOrThrow("LOCAL:0.5"),
+    lastDepositFee: Amounts.parseOrThrow("LOCAL:0.1"),
+  });
+
+});
+
+
+
+
+function createCandidates(ar: {amount: AmountString, depositFee: AmountString, numAvailable: number, fromExchange: string}[]): AvailableDenom[] {
+  return ar.map((r,idx) => {
+    return {
       "denomPub": {
         "age_mask": 0,
         "cipher": DenomKeyType.Rsa,
         "rsa_public_key": "PPP"
       },
-      "denomPubHash": "XXX",
-      "value": "LOCAL:10",
-      "feeDeposit": "LOCAL:0.1",
+      "denomPubHash": `hash${idx}`,
+      "value": r.amount,
+      "feeDeposit": r.depositFee,
       "feeRefresh": "LOCAL:0",
       "feeRefund": "LOCAL:0",
       "feeWithdraw": "LOCAL:0",
@@ -72,18 +172,10 @@ test("should select the coin", (t) => {
       "stampExpireLegal": inTheDistantFuture,
       "stampExpireWithdraw": inTheDistantFuture,
       "stampStart": inThePast,
-      "exchangeBaseUrl": "http://exchange.localhost/",
-      "numAvailable": 5,
-      "maxAge": 32
-    }],
-    instructedAmount, tally);
+      "exchangeBaseUrl": r.fromExchange,
+      "numAvailable": r.numAvailable,
+      "maxAge": 32,
 
-  expect(t, coins).deep.equal({
-    "XXX;32;http://exchange.localhost/": {
-      exchangeBaseUrl: "http://exchange.localhost/",
-      denomPubHash: "XXX",
-      maxAge: 32,
-      contributions: [Amounts.parseOrThrow("LOCAL:2")],
     }
-  });
-});
\ No newline at end of file
+  })
+}
diff --git a/packages/taler-wallet-core/src/util/coinSelection.ts b/packages/taler-wallet-core/src/util/coinSelection.ts
index b8ce5e0f2..0885215dd 100644
--- a/packages/taler-wallet-core/src/util/coinSelection.ts
+++ b/packages/taler-wallet-core/src/util/coinSelection.ts
@@ -918,19 +918,19 @@ function greedySelectPeer(
         instructedAmount,
         tally.amountAcc,
       ).amount;
-      const coinSpend = Amounts.max(
-        Amounts.min(amountPayRemaining, denom.value),
-        denom.feeDeposit,
-      );
+      const coinContrib = Amounts.sub(denom.value, denom.feeDeposit).amount
+
+      const coinSpend = Amounts.min(amountPayRemaining, coinContrib)
+      
       tally.amountAcc = Amounts.add(tally.amountAcc, coinSpend).amount;
-      // Since this is a peer payment, there is no merchant to
-      // potentially cover the deposit fees.
-      tally.amountAcc = Amounts.sub(tally.amountAcc, denom.feeDeposit).amount;
+
       tally.depositFeesAcc = Amounts.add(
         tally.depositFeesAcc,
         denom.feeDeposit,
       ).amount;
+
       tally.lastDepositFee = Amounts.parseOrThrow(denom.feeDeposit);
+      
       contributions.push(coinSpend);
     }
     if (contributions.length > 0) {