From 93128f935817c86172406c9de41677db125d0273 Mon Sep 17 00:00:00 2001 From: Florian Dold Date: Sat, 27 Mar 2021 19:35:44 +0100 Subject: [PATCH] fix coin selection --- packages/taler-util/package.json | 1 + .../taler-wallet-core/src/operations/pay.ts | 70 +++++- .../src/util/coinSelection.ts | 213 ++++++++++++------ 3 files changed, 206 insertions(+), 78 deletions(-) diff --git a/packages/taler-util/package.json b/packages/taler-util/package.json index aacabd21a..6b71c429a 100644 --- a/packages/taler-util/package.json +++ b/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": { "*": { diff --git a/packages/taler-wallet-core/src/operations/pay.ts b/packages/taler-wallet-core/src/operations/pay.ts index 168a0f965..da3980565 100644 --- a/packages/taler-wallet-core/src/operations/pay.ts +++ b/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 { PayCoinSelection, CoinCandidateSelection, AvailableCoinInfo, selectPayCoins } from "../util/coinSelection.js"; +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 { + 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, diff --git a/packages/taler-wallet-core/src/util/coinSelection.ts b/packages/taler-wallet-core/src/util/coinSelection.ts index e9cec6144..a840993c4 100644 --- a/packages/taler-wallet-core/src/util/coinSelection.ts +++ b/packages/taler-wallet-core/src/util/coinSelection.ts @@ -23,8 +23,11 @@ /** * Imports. */ -import { AmountJson, Amounts } from "@gnu-taler/taler-util"; -import { strcmp } from "./helpers"; +import { AmountJson, AmountLike, Amounts } from "@gnu-taler/taler-util"; +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; +} + +/** + * Account for the fees of spending a coin. + */ +function tallyFees( + tally: CoinSelectionTally, + wireFeesPerExchange: Record, + 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 = new Set(); - - /** - * Account for the fees of spending a coin. - */ - function tallyFees(exchangeBaseUrl: string, feeDeposit: AmountJson): void { - if (!wireFeeCoveredForExchange.has(exchangeBaseUrl)) { - 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; - } + let tally: CoinSelectionTally = { + amountPayRemaining: contractTermsAmount, + amountWireFeeLimitRemaining: wireFeeLimit, + amountDepositFeeLimitRemaining: depositFeeLimit, + customerDepositFees: Amounts.getZero(currency), + customerWireFees: Amounts.getZero(currency), + wireFeeCoveredForExchange: new Set(), + }; const prevPayCoins = req.prevPayCoins ?? []; // Look at existing pay coin selection and tally up for (const prev of prevPayCoins) { - tallyFees(prev.exchangeBaseUrl, prev.feeDeposit); - amountPayRemaining = Amounts.sub(amountPayRemaining, prev.contribution) - .amount; + tally = tallyFees( + tally, + 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); - amountPayRemaining = Amounts.sub(amountPayRemaining, coinSpend).amount; + let coinSpend = Amounts.max( + 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, - customerWireFees, + customerDepositFees: tally.customerDepositFees, + customerWireFees: tally.customerWireFees, }; } return undefined;