From 6efc7e5600d3444e44697bc9c022d15990a98c50 Mon Sep 17 00:00:00 2001 From: Florian Dold Date: Fri, 15 May 2020 23:23:49 +0530 Subject: fix issue with wire fee in coin selection that caused wrongly reported fees --- src/headless/taler-wallet-cli.ts | 1 + src/operations/pay.ts | 24 +++++++++++++++--------- src/wallet-test.ts | 12 ++++++------ 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 { - 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(); }); -- cgit v1.2.3