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

This commit is contained in:
Florian Dold 2020-05-15 23:23:49 +05:30
parent 49f362ba6d
commit 6efc7e5600
No known key found for this signature in database
GPG Key ID: D2E4F00F29D02A4B
3 changed files with 22 additions and 15 deletions

View File

@ -70,6 +70,7 @@ async function doPay(
throw Error("not reached"); throw Error("not reached");
} }
console.log("contract", result.contractTermsRaw); console.log("contract", result.contractTermsRaw);
console.log("total fees:", Amounts.stringify(result.totalFees));
let pay; let pay;
if (options.alwaysYes) { if (options.alwaysYes) {
pay = true; pay = true;

View File

@ -177,7 +177,8 @@ export async function getTotalPaymentCost(
*/ */
export function selectPayCoins( export function selectPayCoins(
acis: AvailableCoinInfo[], acis: AvailableCoinInfo[],
paymentAmount: AmountJson, contractTermsAmount: AmountJson,
customerWireFees: AmountJson,
depositFeeLimit: AmountJson, depositFeeLimit: AmountJson,
): PayCoinSelection | undefined { ): PayCoinSelection | undefined {
if (acis.length === 0) { if (acis.length === 0) {
@ -194,11 +195,10 @@ export function selectPayCoins(
Amounts.cmp(o1.feeDeposit, o2.feeDeposit) || Amounts.cmp(o1.feeDeposit, o2.feeDeposit) ||
strcmp(o1.denomPub, o2.denomPub), strcmp(o1.denomPub, o2.denomPub),
); );
const paymentAmount = Amounts.add(contractTermsAmount, customerWireFees).amount;
const currency = paymentAmount.currency; const currency = paymentAmount.currency;
let totalFees = Amounts.getZero(currency);
let amountPayRemaining = paymentAmount; let amountPayRemaining = paymentAmount;
let amountDepositFeeLimitRemaining = depositFeeLimit; let amountDepositFeeLimitRemaining = depositFeeLimit;
const customerWireFees = Amounts.getZero(currency);
const customerDepositFees = Amounts.getZero(currency); const customerDepositFees = Amounts.getZero(currency);
for (const aci of acis) { for (const aci of acis) {
// Don't use this coin if depositing it is more expensive than // Don't use this coin if depositing it is more expensive than
@ -254,11 +254,10 @@ export function selectPayCoins(
coinPubs.push(aci.coinPub); coinPubs.push(aci.coinPub);
coinContributions.push(coinSpend); coinContributions.push(coinSpend);
totalFees = Amounts.add(totalFees, depositFeeSpend).amount;
} }
if (Amounts.isZero(amountPayRemaining)) { if (Amounts.isZero(amountPayRemaining)) {
return { return {
paymentAmount, paymentAmount: contractTermsAmount,
coinContributions, coinContributions,
coinPubs, coinPubs,
customerDepositFees, customerDepositFees,
@ -278,7 +277,7 @@ async function getCoinsForPayment(
ws: InternalWalletState, ws: InternalWalletState,
contractData: WalletContractData, contractData: WalletContractData,
): Promise<PayCoinSelection | undefined> { ): Promise<PayCoinSelection | undefined> {
let remainingAmount = contractData.amount; const remainingAmount = contractData.amount;
const exchanges = await ws.db.iter(Stores.exchanges).toArray(); 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; let wireFee: AmountJson | undefined;
for (const fee of exchangeFees.feesForType[contractData.wireMethod] || []) { for (const fee of exchangeFees.feesForType[contractData.wireMethod] || []) {
if ( if (
@ -379,21 +377,27 @@ async function getCoinsForPayment(
} }
} }
let customerWireFee: AmountJson;
if (wireFee) { if (wireFee) {
const amortizedWireFee = Amounts.divide( const amortizedWireFee = Amounts.divide(
wireFee, wireFee,
contractData.wireFeeAmortization, contractData.wireFeeAmortization,
); );
if (Amounts.cmp(contractData.maxWireFee, amortizedWireFee) < 0) { if (Amounts.cmp(contractData.maxWireFee, amortizedWireFee) < 0) {
totalFees = Amounts.add(amortizedWireFee, totalFees).amount; customerWireFee = amortizedWireFee;
remainingAmount = Amounts.add(amortizedWireFee, remainingAmount).amount; } else {
customerWireFee = Amounts.getZero(currency);
} }
} else {
customerWireFee = Amounts.getZero(currency);
} }
// Try if paying using this exchange works // Try if paying using this exchange works
const res = selectPayCoins( const res = selectPayCoins(
acis, acis,
remainingAmount, remainingAmount,
customerWireFee,
contractData.maxDepositFee, contractData.maxDepositFee,
); );
if (res) { if (res) {
@ -914,6 +918,8 @@ export async function preparePayForUri(
} }
const costInfo = await getTotalPaymentCost(ws, res); const costInfo = await getTotalPaymentCost(ws, res);
console.log("costInfo", costInfo);
console.log("coinsForPayment", res);
const totalFees = Amounts.sub(costInfo.totalCost, res.paymentAmount).amount; const totalFees = Amounts.sub(costInfo.totalCost, res.paymentAmount).amount;
return { return {

View File

@ -43,7 +43,7 @@ test("coin selection 1", (t) => {
fakeAci("EUR:1.0", "EUR:0.0"), 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) { if (!res) {
t.fail(); t.fail();
return; return;
@ -59,7 +59,7 @@ test("coin selection 2", (t) => {
// Merchant covers the fee, this one shouldn't be used // Merchant covers the fee, this one shouldn't be used
fakeAci("EUR:1.0", "EUR:0.0"), 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) { if (!res) {
t.fail(); t.fail();
return; return;
@ -75,7 +75,7 @@ test("coin selection 3", (t) => {
// this coin should be selected instead of previous one with fee // this coin should be selected instead of previous one with fee
fakeAci("EUR:1.0", "EUR:0.0"), 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) { if (!res) {
t.fail(); t.fail();
return; return;
@ -90,7 +90,7 @@ test("coin selection 4", (t) => {
fakeAci("EUR:1.0", "EUR:0.5"), fakeAci("EUR:1.0", "EUR:0.5"),
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) { if (!res) {
t.fail(); t.fail();
return; return;
@ -105,7 +105,7 @@ test("coin selection 5", (t) => {
fakeAci("EUR:1.0", "EUR:0.5"), fakeAci("EUR:1.0", "EUR:0.5"),
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.true(!res);
t.pass(); 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"),
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.true(!res);
t.pass(); t.pass();
}); });