taler-typescript-core

Wallet core logic and WebUIs for various components
Log | Files | Refs | Submodules | README | LICENSE

commit ce62ca1b77fcd168d0262b10752f556227c96a08
parent 3ce5467e4ef93b6fd890cecc560104e0130b994b
Author: Florian Dold <dold@taler.net>
Date:   Mon, 27 Jul 2026 21:23:30 +0200

wallet-core: restore the 2024 coin selection behind TALER_WALLET_COINSEL

Issue: https://bugs.taler.net/n/11272

Diffstat:
Mpackages/taler-harness/src/harness/environments.ts | 6++++++
Mpackages/taler-harness/src/harness/harness.ts | 6++++++
Apackages/taler-harness/src/integrationtests/test-coinsel-legacy-2024.ts | 160+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mpackages/taler-harness/src/integrationtests/testrunner.ts | 2++
Mpackages/taler-util/src/compat.missing.ts | 9+++++++--
Mpackages/taler-util/src/types-taler-wallet.ts | 19+++++++++++++++++++
Mpackages/taler-wallet-core/src/coinSelection.test.ts | 186++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
Mpackages/taler-wallet-core/src/coinSelection.ts | 88+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mpackages/taler-wallet-core/src/wallet.ts | 40++++++++++++++++++++++++++++++++++++++++
9 files changed, 508 insertions(+), 8 deletions(-)

diff --git a/packages/taler-harness/src/harness/environments.ts b/packages/taler-harness/src/harness/environments.ts @@ -759,6 +759,11 @@ export interface CreateWalletArgs { overrideDbPath?: string; config?: PartialWalletRunConfig; emitObservabilityEvents?: boolean; + + /** + * Environment variables to add to the ones the wallet process inherits. + */ + extraEnv?: Record<string, string>; } export async function createWalletDaemonWithClient( @@ -769,6 +774,7 @@ export async function createWalletDaemonWithClient( name: args.name, useInMemoryDb: !args.persistent, overrideDbPath: args.overrideDbPath, + extraEnv: args.extraEnv, }); await walletService.start(); await walletService.pingUntilAvailable(); diff --git a/packages/taler-harness/src/harness/harness.ts b/packages/taler-harness/src/harness/harness.ts @@ -2913,6 +2913,11 @@ export interface WalletServiceOptions { */ overrideDbPath?: string; name: string; + + /** + * Environment variables to add to the ones the wallet process inherits. + */ + extraEnv?: Record<string, string>; } /** @@ -2976,6 +2981,7 @@ export class WalletService { "--no-init", ], `wallet-${this.opts.name}`, + { ...process.env, ...(this.opts.extraEnv ?? {}) }, ); logger.info( `hint: connect to wallet using taler-wallet-cli --wallet-connection=${unixPath}`, diff --git a/packages/taler-harness/src/integrationtests/test-coinsel-legacy-2024.ts b/packages/taler-harness/src/integrationtests/test-coinsel-legacy-2024.ts @@ -0,0 +1,160 @@ +/* + This file is part of GNU Taler + (C) 2026 Taler Systems S.A. + + GNU Taler is free software; you can redistribute it and/or modify it under the + terms of the GNU General Public License as published by the Free Software + Foundation; either version 3, or (at your option) any later version. + + GNU Taler is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + A PARTICULAR PURPOSE. See the GNU General Public License for more details. + + You should have received a copy of the GNU General Public License along with + GNU Taler; see the file COPYING. If not, see <http://www.gnu.org/licenses/> + */ + +/** + * Imports. + */ +import { AmountString, j2s, TransactionIdStr } from "@gnu-taler/taler-util"; +import { WalletApiOperation } from "@gnu-taler/taler-wallet-core"; +import { + createSimpleTestkudosEnvironmentV2, + createWalletDaemonWithClient, + makeTestPaymentV2, +} from "../harness/environments.js"; +import { GlobalTestState, WalletClient } from "../harness/harness.js"; + +/** + * A coin the wallet spent, reduced to what the coin selection decided. + */ +interface SpentCoin { + denomValue: string; + contribution: string; +} + +/** + * Collect the coins that a payment spent, from the coin history. + */ +async function getSpentCoins( + walletClient: WalletClient, + transactionId: TransactionIdStr, +): Promise<SpentCoin[]> { + const coinDump = await walletClient.call(WalletApiOperation.DumpCoins, {}); + const spent: SpentCoin[] = []; + for (const coin of coinDump.coins) { + for (const ev of coin.history) { + if (ev.type === "spend" && ev.transactionId === transactionId) { + spent.push({ denomValue: coin.denomValue, contribution: ev.amount }); + } + } + } + // The dump is not in a defined order, so sort to make the result stable. + spent.sort( + (a, b) => + a.denomValue.localeCompare(b.denomValue) || + a.contribution.localeCompare(b.contribution), + ); + return spent; +} + +/** + * Give a wallet one TESTKUDOS:8 coin and four TESTKUDOS:1 coins, and let it + * pay TESTKUDOS:4. With those coins the two algorithms disagree: the 2024 one + * always reaches for the largest coin, while the current one leaves it alone + * and pays with the small coins. + */ +async function withdrawAndPay( + t: GlobalTestState, + args: { + name: string; + extraEnv?: Record<string, string>; + env: Awaited<ReturnType<typeof createSimpleTestkudosEnvironmentV2>>; + }, +): Promise<SpentCoin[]> { + const { bank, exchange, merchant, merchantAdminAccessToken } = args.env; + + const { walletClient } = await createWalletDaemonWithClient(t, { + name: args.name, + persistent: true, + extraEnv: args.extraEnv, + }); + + await walletClient.call(WalletApiOperation.AddExchange, { + exchangeBaseUrl: exchange.baseUrl, + }); + + await walletClient.call(WalletApiOperation.WithdrawTestBalance, { + exchangeBaseUrl: exchange.baseUrl, + amount: "TESTKUDOS:20" as AmountString, + corebankApiBaseUrl: bank.corebankApiBaseUrl, + forcedDenomSel: { + denoms: [ + { value: "TESTKUDOS:8" as AmountString, count: 1 }, + { value: "TESTKUDOS:1" as AmountString, count: 4 }, + ], + }, + }); + + await walletClient.call(WalletApiOperation.TestingWaitTransactionsFinal, {}); + + const coinDump = await walletClient.call(WalletApiOperation.DumpCoins, {}); + t.assertDeepEqual(coinDump.coins.length, 5); + + const { transactionId } = await makeTestPaymentV2(t, { + merchant, + merchantAdminAccessToken, + walletClient, + order: { + summary: "Hello, World", + amount: "TESTKUDOS:4" as AmountString, + }, + }); + + await walletClient.call(WalletApiOperation.TestingWaitTransactionsFinal, {}); + + const spent = await getSpentCoins(walletClient, transactionId); + console.log(`coins spent by the ${args.name} wallet: ${j2s(spent)}`); + return spent; +} + +/** + * Pin down the coin selection that TALER_WALLET_COINSEL=legacy-2024 restores. + * + * The exchange's auditor tests reproduce coin selections from that era, so the + * wallet has to keep producing them on request. The default algorithm is only + * checked for being different here: it is free to change, the legacy one is + * not. + */ +export async function runCoinselLegacy2024Test(t: GlobalTestState) { + const env = await createSimpleTestkudosEnvironmentV2(t); + + const legacySpent = await withdrawAndPay(t, { + name: "legacy", + extraEnv: { TALER_WALLET_COINSEL: "legacy-2024" }, + env, + }); + + // Largest coin first: the TESTKUDOS:8 coin covers the whole payment on its + // own and the four TESTKUDOS:1 coins are left untouched. The merchant's fee + // allowance absorbs the deposit and wire fees, so the contribution is the + // order amount exactly. + t.assertDeepEqual(legacySpent, [ + { + denomValue: "TESTKUDOS:8", + contribution: "TESTKUDOS:4", + }, + ]); + + const defaultSpent = await withdrawAndPay(t, { name: "default", env }); + + // Same coins, same order, different selection. + t.assertTrue(defaultSpent.length > legacySpent.length); + t.assertDeepEqual( + defaultSpent.filter((c) => c.denomValue === "TESTKUDOS:1").length, + 4, + ); +} + +runCoinselLegacy2024Test.suites = ["wallet"]; diff --git a/packages/taler-harness/src/integrationtests/testrunner.ts b/packages/taler-harness/src/integrationtests/testrunner.ts @@ -43,6 +43,7 @@ import { runBankApiTest } from "./test-bank-api.js"; import { runBankWopTest } from "./test-bank-wop.js"; import { runClaimLoopTest } from "./test-claim-loop.js"; import { runClauseSchnorrTest } from "./test-clause-schnorr.js"; +import { runCoinselLegacy2024Test } from "./test-coinsel-legacy-2024.js"; import { runCurrencyScopeSeparationTest } from "./test-currency-scope-separation.js"; import { runCurrencyScopeTest } from "./test-currency-scope.js"; import { runDenomLostComplexTest } from "./test-denom-lost-complex.js"; @@ -276,6 +277,7 @@ const allTests: TestMainFunction[] = [ runExchangeTimetravelTest, runFeeRegressionTest, runForcedSelectionTest, + runCoinselLegacy2024Test, runKycChallengerTest, runExchangePurseTest, runExchangeDepositTest, diff --git a/packages/taler-util/src/compat.missing.ts b/packages/taler-util/src/compat.missing.ts @@ -16,8 +16,12 @@ /** * Fallback for the "#compat-impl" / "./compat" conditions on runtimes that are - * neither node nor qtart. Every entry point throws, so an unsupported host is + * neither node nor qtart. Entry points throw, so an unsupported host is * reported where it is used rather than failing to resolve at import time. + * + * The one exception is getenv: a runtime without a process environment has a + * meaningful answer ("not set"), and callers use it to pick a default rather + * than to perform an operation. */ function notImplemented(name: string): never { @@ -49,7 +53,8 @@ export function setUnhandledRejectionHandler(h: (e: any) => void): void { } export function getenv(name: string): string | undefined { - return notImplemented("getenv"); + // No process environment on this platform, so nothing is ever set. + return undefined; } export function readFile(fileName: string): string { diff --git a/packages/taler-util/src/types-taler-wallet.ts b/packages/taler-util/src/types-taler-wallet.ts @@ -424,6 +424,17 @@ export interface BuiltinExchange { currencyHint: string; } +/** + * Coin selection algorithm the wallet uses when spending. + * + * "legacy-2024" is the algorithm the wallet shipped with in 2024: strictly + * largest-coin-first, without the exact-fit shortcut and the deposit-fee + * allowance correction that were added later. It is kept reachable because + * test suites outside this repository (in particular the exchange's auditor + * tests) pin the coin selections it produces. + */ +export type CoinSelectionAlgorithm = "default" | "legacy-2024"; + export interface PartialWalletRunConfig { testing?: Partial<WalletRunConfig["testing"]>; features?: Partial<WalletRunConfig["features"]>; @@ -442,6 +453,14 @@ export interface WalletRunConfig { preventThrottling: boolean; skipDefaults: boolean; emitObservabilityEvents?: boolean; + + /** + * Coin selection algorithm to use when spending. + * + * Defaults to the TALER_WALLET_COINSEL environment variable, and to + * "default" when that is unset. + */ + coinSelectionAlgorithm: CoinSelectionAlgorithm; }; /** diff --git a/packages/taler-wallet-core/src/coinSelection.test.ts b/packages/taler-wallet-core/src/coinSelection.test.ts @@ -650,9 +650,9 @@ test("peer push debit max ignores coins that cost more to deposit than they are ); }); -function wireInfoWithRestrictions( - restrictions: AccountRestriction[], -): { wireInfo: WireInfo } { +function wireInfoWithRestrictions(restrictions: AccountRestriction[]): { + wireInfo: WireInfo; +} { return { wireInfo: { accounts: [ @@ -689,9 +689,7 @@ test("a wire account without restrictions matches", (t) => { }); test("a rejected wire account reports the restrictions that rejected it", (t) => { - const restrictions: AccountRestriction[] = [ - { type: "deny" }, - ]; + const restrictions: AccountRestriction[] = [{ type: "deny" }]; const res = findMatchingWire( "iban", "payto://iban/CH62414246VCSW2LM4FG0", @@ -705,3 +703,179 @@ test("a rejected wire account reports the restrictions that rejected it", (t) => }); } }); + +// The tests below pin the "legacy-2024" algorithm, which reproduces how the +// wallet selected coins in 2024. They deliberately mirror scenarios that the +// default algorithm now handles differently, so a change to either one shows +// up as a failing assertion rather than as a silent drift. + +test("legacy-2024: takes the largest coin instead of the exact one", (t) => { + const instructedAmount = Amounts.parseOrThrow("LOCAL:2"); + const tally = emptyTallyForPeerPayment({ + instructedAmount, + }); + tally.amountDepositFeeLimitRemaining = Amounts.parseOrThrow("LOCAL:0.5"); + + const coins = testing_selectGreedy( + { + wireFeesPerExchange: {}, + algorithm: "legacy-2024", + }, + createCandidates([ + { + amount: "LOCAL:3" as AmountString, + numAvailable: 1, + depositFee: "LOCAL:0.2" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + { + amount: "LOCAL:2" as AmountString, + numAvailable: 1, + depositFee: "LOCAL:0.2" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + { + amount: "LOCAL:1" as AmountString, + numAvailable: 1, + depositFee: "LOCAL:0.2" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + ]), + tally, + ); + + assert.ok(coins != null); + + // The default algorithm picks the exactly fitting LOCAL:2 coin (hash1). + assert.deepStrictEqual(coins, { + "hash0;32;http://exchange.localhost/": { + exchangeBaseUrl: "http://exchange.localhost/", + denomPubHash: "hash0", + maxAge: 32, + contributions: [Amounts.parseOrThrow("LOCAL:2")], + }, + }); +}); + +test("legacy-2024: overspends the deposit fee that the allowance covers", (t) => { + const instructedAmount = Amounts.parseOrThrow("LOCAL:2.1"); + const tally = emptyTallyForPeerPayment({ + instructedAmount, + }); + tally.amountDepositFeeLimitRemaining = Amounts.parseOrThrow("LOCAL:0.5"); + + const coins = testing_selectGreedy( + { + wireFeesPerExchange: {}, + algorithm: "legacy-2024", + }, + createCandidates([ + { + amount: "LOCAL:2" as AmountString, + numAvailable: 1, + depositFee: "LOCAL:0.2" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + { + amount: "LOCAL:1" as AmountString, + numAvailable: 1, + depositFee: "LOCAL:0.2" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + ]), + tally, + ); + + assert.ok(coins != null); + + // Only LOCAL:0.1 is still owed and the allowance still has LOCAL:0.3 of + // room, but the 2024 algorithm never contributes less than the full deposit + // fee, so the second coin contributes LOCAL:0.2. That overspending is what + // the default algorithm fixed. + assert.deepStrictEqual(coins, { + "hash0;32;http://exchange.localhost/": { + exchangeBaseUrl: "http://exchange.localhost/", + denomPubHash: "hash0", + maxAge: 32, + contributions: [Amounts.parseOrThrow("LOCAL:2")], + }, + "hash1;32;http://exchange.localhost/": { + exchangeBaseUrl: "http://exchange.localhost/", + denomPubHash: "hash1", + maxAge: 32, + contributions: [Amounts.parseOrThrow("LOCAL:0.2")], + }, + }); +}); + +test("legacy-2024: spends the largest coins first", (t) => { + const instructedAmount = Amounts.parseOrThrow("LOCAL:4"); + const tally = emptyTallyForPeerPayment({ + instructedAmount, + }); + + const candidates = () => + createCandidates([ + { + amount: "LOCAL:8" as AmountString, + numAvailable: 1, + depositFee: "LOCAL:0" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + { + amount: "LOCAL:1" as AmountString, + numAvailable: 4, + depositFee: "LOCAL:0" as AmountString, + fromExchange: "http://exchange.localhost/", + fromMasterPub: "123", + }, + ]); + + const legacyCoins = testing_selectGreedy( + { + wireFeesPerExchange: {}, + algorithm: "legacy-2024", + }, + candidates(), + tally, + ); + + assert.deepStrictEqual(legacyCoins, { + "hash0;32;http://exchange.localhost/": { + exchangeBaseUrl: "http://exchange.localhost/", + denomPubHash: "hash0", + maxAge: 32, + contributions: [Amounts.parseOrThrow("LOCAL:4")], + }, + }); + + // For contrast: the default algorithm spends the small coins. + const defaultTally = emptyTallyForPeerPayment({ instructedAmount }); + const defaultCoins = testing_selectGreedy( + { + wireFeesPerExchange: {}, + }, + candidates(), + defaultTally, + ); + + assert.deepStrictEqual(defaultCoins, { + "hash1;32;http://exchange.localhost/": { + exchangeBaseUrl: "http://exchange.localhost/", + denomPubHash: "hash1", + maxAge: 32, + contributions: [ + Amounts.parseOrThrow("LOCAL:1"), + Amounts.parseOrThrow("LOCAL:1"), + Amounts.parseOrThrow("LOCAL:1"), + Amounts.parseOrThrow("LOCAL:1"), + ], + }, + }); +}); diff --git a/packages/taler-wallet-core/src/coinSelection.ts b/packages/taler-wallet-core/src/coinSelection.ts @@ -31,9 +31,11 @@ import { AllowedExchangeInfo, AmountJson, Amounts, + assertUnreachable, checkAccountRestriction, checkDbInvariant, checkLogicInvariant, + CoinSelectionAlgorithm, CoinStatus, DenominationInfo, Exchange, @@ -288,6 +290,7 @@ async function internalSelectPayCoins( selectedDenom = selectGreedy( { wireFeesPerExchange: wireFeesPerExchange, + algorithm: wex.ws.config.testing.coinSelectionAlgorithm, }, candidateDenoms, tally, @@ -671,6 +674,11 @@ export function testing_selectGreedy( export interface SelectGreedyRequest { wireFeesPerExchange: Record<string, AmountJson>; + + /** + * Algorithm to use. Defaults to "default". + */ + algorithm?: CoinSelectionAlgorithm; } function applyContributions( @@ -703,6 +711,85 @@ function selectGreedy( candidateDenoms: AvailableCoinsOfDenom[], tally: CoinSelectionTally, ): SelResult | undefined { + const algorithm = req.algorithm ?? "default"; + switch (algorithm) { + case "default": + return selectGreedyDefault(req, candidateDenoms, tally); + case "legacy-2024": + return selectGreedyLegacy2024(req, candidateDenoms, tally); + default: + assertUnreachable(algorithm); + } +} + +/** + * Coin selection as the wallet did it in 2024. + * + * Kept around so that test suites which pin the coin selections it produces + * (the exchange's auditor tests, most notably) keep seeing the same coins. + * Two later changes are deliberately *not* applied here: the exact-fit + * shortcut with smallest-coin-first fallback, and the correction that stops a + * coin from overspending while the merchant's deposit fee allowance still has + * room. The latter is a genuine bug, so this algorithm is only for + * reproducing old results, never for real spending. + * + * The only intentional deviation from the 2024 source is that it no longer + * records the last deposit fee in the tally: that field fed the insufficient + * balance hint, which has since been rewritten, and never influenced which + * coins were picked. + */ +function selectGreedyLegacy2024( + req: SelectGreedyRequest, + candidateDenoms: AvailableCoinsOfDenom[], + tally: CoinSelectionTally, +): SelResult | undefined { + const selectedDenom: SelResult = {}; + // Candidates are sorted by descending value, so this always spends the + // largest coin that is still available. + for (const denom of candidateDenoms) { + const contributions: AmountJson[] = []; + + // Don't use this coin if depositing it is more expensive than + // the amount it would give the merchant. + if (Amounts.cmp(denom.feeDeposit, denom.value) > 0) { + continue; + } + + for ( + let i = 0; + i < denom.numAvailable && Amounts.isNonZero(tally.amountPayRemaining); + i++ + ) { + tallyFees( + tally, + req.wireFeesPerExchange, + denom.exchangeBaseUrl, + Amounts.parseOrThrow(denom.feeDeposit), + ); + + const coinSpend = Amounts.max( + Amounts.min(tally.amountPayRemaining, denom.value), + denom.feeDeposit, + ); + + tally.amountPayRemaining = Amounts.sub( + tally.amountPayRemaining, + coinSpend, + ).amount; + + contributions.push(coinSpend); + } + + applyContributions(selectedDenom, contributions, denom); + } + return Amounts.isZero(tally.amountPayRemaining) ? selectedDenom : undefined; +} + +function selectGreedyDefault( + req: SelectGreedyRequest, + candidateDenoms: AvailableCoinsOfDenom[], + tally: CoinSelectionTally, +): SelResult | undefined { const selectedDenom: SelResult = {}; const currency = Amounts.currencyOf(tally.amountPayRemaining); // Optimization: If we have a coin that exactly fits, use it. @@ -1274,6 +1361,7 @@ async function internalSelectPeerCoins( const selRes = selectGreedy( { wireFeesPerExchange: {}, + algorithm: wex.ws.config.testing.coinSelectionAlgorithm, }, candidates, tally, diff --git a/packages/taler-wallet-core/src/wallet.ts b/packages/taler-wallet-core/src/wallet.ts @@ -28,6 +28,7 @@ import { AsyncCondition, Cache, CancellationToken, + CoinSelectionAlgorithm, CoreApiResponse, DenominationInfo, Duration, @@ -61,6 +62,7 @@ import { performanceNow, safeStringifyException, } from "@gnu-taler/taler-util"; +import { getenv } from "@gnu-taler/taler-util/compat"; import { type HttpRequestLibrary } from "@gnu-taler/taler-util/http"; import { TalerCryptoInterface } from "./crypto/cryptoImplementation.js"; import { @@ -549,12 +551,49 @@ async function dispatchWalletCoreApiRequest( } } +/** + * Environment variable that selects the coin selection algorithm. + */ +const coinSelectionEnvVar = "TALER_WALLET_COINSEL"; + +/** + * Read the coin selection algorithm from the environment. + * + * Returns undefined on platforms without an environment and when the variable + * is unset, so that the caller falls back to its own default. An unknown + * value is an error: silently spending with a different algorithm than the + * one that was asked for would be very hard to notice. + */ +function coinSelectionAlgorithmFromEnv(): CoinSelectionAlgorithm | undefined { + const val = getenv(coinSelectionEnvVar); + if (val == null || val === "") { + return undefined; + } + switch (val) { + case "default": + case "legacy-2024": + return val; + default: + throw Error( + `invalid value for ${coinSelectionEnvVar}: "${val}" ` + + `(expected "default" or "legacy-2024")`, + ); + } +} + export function applyRunConfigDefaults( wcp?: PartialWalletRunConfig, ): WalletRunConfig { if (wcp?.features?.allowHttp != null) { logger.warn(`allowHttp flag not supported anymore`); } + const coinSelectionAlgorithm = + wcp?.testing?.coinSelectionAlgorithm ?? + coinSelectionAlgorithmFromEnv() ?? + "default"; + if (coinSelectionAlgorithm !== "default") { + logger.warn(`using non-default coin selection: ${coinSelectionAlgorithm}`); + } return { features: { allowHttp: true, @@ -565,6 +604,7 @@ export function applyRunConfigDefaults( preventThrottling: wcp?.testing?.preventThrottling ?? false, skipDefaults: wcp?.testing?.skipDefaults ?? false, emitObservabilityEvents: wcp?.testing?.emitObservabilityEvents ?? false, + coinSelectionAlgorithm, }, lazyTaskLoop: wcp?.lazyTaskLoop ?? false, logLevel: wcp?.logLevel ?? "INFO",