taler-typescript-core

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

commit db398ca7b88cfd85c103840bc53e8a47cb16ccec
parent 8cfb838934714b3bce5de6938256cbc8990c9e79
Author: Florian Dold <florian@dold.me>
Date:   Tue,  4 Mar 2025 20:43:14 +0100

wallet-core: fix denom validation typo, ensure denoms are really validated before being used

The (lazy) denomination validation still has some issues,
see https://bugs.taler.net/n/9593 for details.

Diffstat:
Mpackages/taler-harness/src/integrationtests/test-withdrawal-bank-integrated.ts | 32++++++++++++++++----------------
Mpackages/taler-harness/src/integrationtests/testrunner.ts | 8++++----
Mpackages/taler-wallet-core/src/dbless.ts | 8+++++---
Mpackages/taler-wallet-core/src/denominations.ts | 76+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Mpackages/taler-wallet-core/src/exchanges.ts | 26++++++++++++++++----------
Mpackages/taler-wallet-core/src/pay-peer-push-debit.ts | 4++++
Mpackages/taler-wallet-core/src/refresh.ts | 6+++---
Mpackages/taler-wallet-core/src/withdraw.ts | 98+++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------
8 files changed, 194 insertions(+), 64 deletions(-)

diff --git a/packages/taler-harness/src/integrationtests/test-withdrawal-bank-integrated.ts b/packages/taler-harness/src/integrationtests/test-withdrawal-bank-integrated.ts @@ -26,8 +26,8 @@ import { j2s, } from "@gnu-taler/taler-util"; import { WalletApiOperation } from "@gnu-taler/taler-wallet-core"; -import { GlobalTestState } from "../harness/harness.js"; import { createSimpleTestkudosEnvironmentV3 } from "../harness/environments.js"; +import { GlobalTestState } from "../harness/harness.js"; /** * Run test for basic, bank-integrated withdrawal. @@ -46,7 +46,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { "TESTKUDOS:10", ); - t.logStep("Hand it to the wallet") + t.logStep("Hand it to the wallet"); const r1 = await walletClient.client.call( WalletApiOperation.GetWithdrawalDetailsForUri, @@ -55,7 +55,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { }, ); - t.logStep("Withdraw") + t.logStep("Withdraw"); const r2 = await walletClient.client.call( WalletApiOperation.AcceptBankIntegratedWithdrawal, @@ -65,7 +65,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { }, ); - t.logStep("wait confirmed") + t.logStep("wait confirmed"); const withdrawalBankConfirmedCond = walletClient.waitForNotificationCond( (x) => { return ( @@ -77,7 +77,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { }, ); - t.logStep("wait finished") + t.logStep("wait finished"); const withdrawalFinishedCond = walletClient.waitForNotificationCond((x) => { return ( x.type === NotificationType.TransactionStateTransition && @@ -86,7 +86,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { ); }); - t.logStep("wait withdraw coins") + t.logStep("wait withdraw coins"); const withdrawalReserveReadyCond = walletClient.waitForNotificationCond( (x) => { return ( @@ -98,7 +98,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { }, ); - t.logStep("Do it twice to check idempotency") + t.logStep("Do it twice to check idempotency"); const r3 = await walletClient.client.call( WalletApiOperation.AcceptBankIntegratedWithdrawal, { @@ -107,10 +107,10 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { }, ); - t.logStep("stop wirewatch") + t.logStep("stop wirewatch"); await exchange.stopWirewatch(); - t.logStep("Check status before withdrawal is confirmed by bank.") + t.logStep("Check status before withdrawal is confirmed by bank."); { const txn = await walletClient.client.call( WalletApiOperation.GetTransactions, @@ -126,7 +126,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { t.assertTrue(tx0.withdrawalDetails.reserveIsReady === false); } - t.logStep("Confirm it") + t.logStep("Confirm it"); await walletClient.call(WalletApiOperation.TestingWaitTransactionState, { transactionId: r3.transactionId, @@ -144,7 +144,7 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { // Check status after withdrawal is confirmed by bank, // but before funds are wired to the exchange. - t.logStep("Check status after withdrawal") + t.logStep("Check status after withdrawal"); { const txn = await walletClient.client.call( WalletApiOperation.GetTransactions, @@ -160,13 +160,13 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { t.assertTrue(tx0.withdrawalDetails.reserveIsReady === false); } - t.logStep("start wirewatch") + t.logStep("start wirewatch"); await exchange.startWirewatch(); - t.logStep("wait reserve") + t.logStep("wait reserve"); await withdrawalReserveReadyCond; - t.logStep("Check status after funds were wired.") + t.logStep("Check status after funds were wired."); { const txn = await walletClient.client.call( WalletApiOperation.GetTransactions, @@ -184,13 +184,13 @@ export async function runWithdrawalBankIntegratedTest(t: GlobalTestState) { await withdrawalFinishedCond; - t.logStep("Check balance") + t.logStep("Check balance"); const balResp = await walletClient.client.call( WalletApiOperation.GetBalances, {}, ); - t.assertAmountEquals("TESTKUDOS:9.72", balResp.balances[0].available); + t.assertAmountEquals(balResp.balances[0].available, "TESTKUDOS:9.72"); const txn = await walletClient.client.call( WalletApiOperation.GetTransactions, diff --git a/packages/taler-harness/src/integrationtests/testrunner.ts b/packages/taler-harness/src/integrationtests/testrunner.ts @@ -176,10 +176,6 @@ interface TestMainFunction { } const allTests: TestMainFunction[] = [ - runAgeRestrictionsMerchantTest, - runAgeRestrictionsMixedMerchantTest, - runAgeRestrictionsPeerTest, - runAgeRestrictionsDepositTest, runBankApiTest, runClaimLoopTest, runClauseSchnorrTest, @@ -308,6 +304,10 @@ const allTests: TestMainFunction[] = [ runKycNewMeasuresProgTest, runKycDecisionAttrTest, runDepositFaultTest, + runAgeRestrictionsMerchantTest, + runAgeRestrictionsMixedMerchantTest, + runAgeRestrictionsPeerTest, + runAgeRestrictionsDepositTest, ]; export interface TestRunSpec { diff --git a/packages/taler-wallet-core/src/dbless.ts b/packages/taler-wallet-core/src/dbless.ts @@ -58,7 +58,7 @@ import { } from "@gnu-taler/taler-util/http"; import { TalerCryptoInterface } from "./crypto/cryptoImplementation.js"; import { DenominationRecord } from "./db.js"; -import { isWithdrawableDenom } from "./denominations.js"; +import { isCandidateWithdrawableDenom } from "./denominations.js"; import { ExchangeInfo, downloadExchangeInfo } from "./exchanges.js"; import { assembleRefreshRevealRequest } from "./refresh.js"; import { getBankStatusUrl, getBankWithdrawalInfo } from "./withdraw.js"; @@ -97,7 +97,9 @@ export interface TopupReserveWithBankArgs { export async function topupReserveWithBank(args: TopupReserveWithBankArgs) { const { http, corebankApiBaseUrl, amount, exchangeInfo, reservePub } = args; - const bankClient = new TalerCorebankApiClient(corebankApiBaseUrl, { httpClient: http }); + const bankClient = new TalerCorebankApiClient(corebankApiBaseUrl, { + httpClient: http, + }); const bankUser = await bankClient.createRandomBankUser(); bankClient.setAuth(bankUser); const wopi = await bankClient.createWithdrawalOperation( @@ -200,7 +202,7 @@ export function findDenomOrThrow( const value: AmountJson = Amounts.parseOrThrow(d.value); if ( Amounts.cmp(value, amount) === 0 && - isWithdrawableDenom(d, denomselAllowLate) + isCandidateWithdrawableDenom(d, denomselAllowLate) ) { return d; } diff --git a/packages/taler-wallet-core/src/denominations.ts b/packages/taler-wallet-core/src/denominations.ts @@ -22,14 +22,25 @@ import { AmountJson, Amounts, AmountString, + assertUnreachable, DenominationInfo, Duration, FeeDescription, FeeDescriptionPair, + Logger, TalerProtocolTimestamp, TimePoint, } from "@gnu-taler/taler-util"; -import { DenominationRecord, timestampProtocolFromDb } from "./db.js"; +import { + DenominationRecord, + DenominationVerificationStatus, + timestampProtocolFromDb, +} from "./db.js"; + +/** + * Logger for this file. + */ +const logger = new Logger("denominations.ts"); /** * Given a list of denominations with the same value and same period of time: @@ -451,6 +462,9 @@ export function createTimeline<Type extends object>( /** * Check if a denom is withdrawable based on the expiration time, * revocation and offered state. + * + * Denominations with an unverified signature + * are not considered withdrawable. */ export function isWithdrawableDenom( d: DenominationRecord, @@ -460,10 +474,70 @@ export function isWithdrawableDenom( const start = AbsoluteTime.fromProtocolTimestamp( timestampProtocolFromDb(d.stampStart), ); + const started = AbsoluteTime.cmp(now, start) >= 0; + switch (d.verificationStatus) { + case DenominationVerificationStatus.Unverified: + // If this happens, either we delayed signature validation + // for denominations very far into the future (good) or forgot + // to validate (bad). + if (started) { + logger.error( + `Skipping unverified denomination ${d.denomPubHash} of ${d.exchangeBaseUrl}`, + ); + } + return false; + case DenominationVerificationStatus.VerifiedBad: + return false; + case DenominationVerificationStatus.VerifiedGood: + break; + default: + assertUnreachable(d.verificationStatus); + } const withdrawExpire = AbsoluteTime.fromProtocolTimestamp( timestampProtocolFromDb(d.stampExpireWithdraw), ); + let lastPossibleWithdraw: AbsoluteTime; + if (denomselAllowLate) { + lastPossibleWithdraw = start; + } else { + lastPossibleWithdraw = AbsoluteTime.subtractDuraction( + withdrawExpire, + Duration.fromSpec({ minutes: 5 }), + ); + } + const remaining = Duration.getRemaining(lastPossibleWithdraw, now); + const stillOkay = remaining.d_ms !== 0; + return started && stillOkay && !d.isRevoked && d.isOffered && !d.isLost; +} + +/** + * Check if a denom is withdrawable based on the expiration time, + * revocation and offered state. + * + * Denominations with an unverified signature + * are considered candidates. + */ +export function isCandidateWithdrawableDenom( + d: DenominationRecord, + denomselAllowLate?: boolean, +): boolean { + const now = AbsoluteTime.now(); + const start = AbsoluteTime.fromProtocolTimestamp( + timestampProtocolFromDb(d.stampStart), + ); const started = AbsoluteTime.cmp(now, start) >= 0; + switch (d.verificationStatus) { + case DenominationVerificationStatus.VerifiedBad: + return false; + case DenominationVerificationStatus.VerifiedGood: + case DenominationVerificationStatus.Unverified: + break; + default: + assertUnreachable(d.verificationStatus); + } + const withdrawExpire = AbsoluteTime.fromProtocolTimestamp( + timestampProtocolFromDb(d.stampExpireWithdraw), + ); let lastPossibleWithdraw: AbsoluteTime; if (denomselAllowLate) { lastPossibleWithdraw = start; diff --git a/packages/taler-wallet-core/src/exchanges.ts b/packages/taler-wallet-core/src/exchanges.ts @@ -163,7 +163,7 @@ import { } from "./db.js"; import { createTimeline, - isWithdrawableDenom, + isCandidateWithdrawableDenom, selectBestForOverlappingDenominations, selectMinimumFee, } from "./denominations.js"; @@ -174,6 +174,8 @@ import { } from "./pay-merchant.js"; import { PeerPullCreditTransactionContext } from "./pay-peer-pull-credit.js"; import { PeerPullDebitTransactionContext } from "./pay-peer-pull-debit.js"; +import { PeerPushCreditTransactionContext } from "./pay-peer-push-credit.js"; +import { PeerPushDebitTransactionContext } from "./pay-peer-push-debit.js"; import { DbReadOnlyTransaction } from "./query.js"; import { RecoupTransactionContext, createRecoupGroup } from "./recoup.js"; import { RefreshTransactionContext, createRefreshGroup } from "./refresh.js"; @@ -185,9 +187,10 @@ import { } from "./transactions.js"; import { WALLET_EXCHANGE_PROTOCOL_VERSION } from "./versions.js"; import { InternalWalletState, WalletExecutionContext } from "./wallet.js"; -import { WithdrawTransactionContext } from "./withdraw.js"; -import { PeerPushCreditTransactionContext } from "./pay-peer-push-credit.js"; -import { PeerPushDebitTransactionContext } from "./pay-peer-push-debit.js"; +import { + WithdrawTransactionContext, + updateWithdrawalDenomsForExchange, +} from "./withdraw.js"; const logger = new Logger("exchanges.ts"); @@ -1162,7 +1165,7 @@ async function checkExchangeEntryOutdated( await tx.denominations.indexes.byExchangeBaseUrl.getAll(exchangeBaseUrl); logger.trace(`exchange entry has ${denoms.length} denominations`); for (const denom of denoms) { - const denomOkay = isWithdrawableDenom( + const denomOkay = isCandidateWithdrawableDenom( denom, wex.ws.config.testing.denomselAllowLate, ); @@ -1709,7 +1712,10 @@ export async function updateExchangeFromUrlHandler( let ageMask = 0; for (const x of keysInfo.currentDenominations) { if ( - isWithdrawableDenom(x, wex.ws.config.testing.denomselAllowLate) && + isCandidateWithdrawableDenom( + x, + wex.ws.config.testing.denomselAllowLate, + ) && x.denomPub.age_mask != 0 ) { ageMask = x.denomPub.age_mask; @@ -1988,6 +1994,9 @@ async function doAutoRefresh( ): Promise<void> { logger.trace(`doing auto-refresh check for '${exchangeBaseUrl}'`); + // FIXME: This should be part of updating the exchange entry. + await updateWithdrawalDenomsForExchange(wex, exchangeBaseUrl); + let minCheckThreshold = AbsoluteTime.addDuration( AbsoluteTime.now(), Duration.fromSpec({ days: 1 }), @@ -3023,10 +3032,7 @@ async function purgeExchange( { await tx.peerPushDebit.iter().forEachAsync(async (rec) => { if (rec.exchangeBaseUrl === exchangeBaseUrl) { - const ctx = new PeerPushDebitTransactionContext( - wex, - rec.pursePub, - ); + const ctx = new PeerPushDebitTransactionContext(wex, rec.pursePub); const res = await ctx.deleteTransactionInTx(tx); notifs.push(...res.notifs); } diff --git a/packages/taler-wallet-core/src/pay-peer-push-debit.ts b/packages/taler-wallet-core/src/pay-peer-push-debit.ts @@ -84,6 +84,7 @@ import { timestampProtocolToDb, } from "./db.js"; import { + fetchFreshExchange, getPreferredExchangeForCurrency, getScopeForAllExchanges, } from "./exchanges.js"; @@ -101,6 +102,7 @@ import { notifyTransition, } from "./transactions.js"; import { WalletExecutionContext } from "./wallet.js"; +import { updateWithdrawalDenomsForCurrency } from "./withdraw.js"; const logger = new Logger("pay-peer-push-debit.ts"); @@ -1192,6 +1194,8 @@ export async function initiatePeerPushDebit( const hContractTerms = ContractTermsUtil.hashContractTerms(contractTerms); + await updateWithdrawalDenomsForCurrency(wex, instructedAmount.currency); + const res = await wex.db.runReadWriteTx( { storeNames: [ diff --git a/packages/taler-wallet-core/src/refresh.ts b/packages/taler-wallet-core/src/refresh.ts @@ -123,7 +123,7 @@ import { getDenomInfo, WalletExecutionContext, } from "./wallet.js"; -import { getCandidateWithdrawalDenomsTx } from "./withdraw.js"; +import { getWithdrawableDenomsTx } from "./withdraw.js"; const logger = new Logger("refresh.ts"); @@ -415,7 +415,7 @@ export async function getTotalRefreshCost( if (cacheRes) { return cacheRes; } - const allDenoms = await getCandidateWithdrawalDenomsTx( + const allDenoms = await getWithdrawableDenomsTx( wex, tx, refreshedDenom.exchangeBaseUrl, @@ -541,7 +541,7 @@ async function initRefreshSession( const currency = refreshGroup.currency; - const availableDenoms = await getCandidateWithdrawalDenomsTx( + const availableDenoms = await getWithdrawableDenomsTx( wex, tx, exchangeBaseUrl, diff --git a/packages/taler-wallet-core/src/withdraw.ts b/packages/taler-wallet-core/src/withdraw.ts @@ -156,7 +156,10 @@ import { selectForcedWithdrawalDenominations, selectWithdrawalDenominations, } from "./denomSelection.js"; -import { isWithdrawableDenom } from "./denominations.js"; +import { + isCandidateWithdrawableDenom, + isWithdrawableDenom, +} from "./denominations.js"; import { BalanceThresholdCheckResult, ExchangeWireDetails, @@ -1285,7 +1288,7 @@ export async function getBankWithdrawalInfo( /** * Return denominations that can potentially used for a withdrawal. */ -async function getCandidateWithdrawalDenoms( +async function getWithdrawableDenoms( wex: WalletExecutionContext, exchangeBaseUrl: string, currency: string, @@ -1293,12 +1296,12 @@ async function getCandidateWithdrawalDenoms( return await wex.db.runReadOnlyTx( { storeNames: ["denominations"] }, async (tx) => { - return getCandidateWithdrawalDenomsTx(wex, tx, exchangeBaseUrl, currency); + return getWithdrawableDenomsTx(wex, tx, exchangeBaseUrl, currency); }, ); } -export async function getCandidateWithdrawalDenomsTx( +export async function getWithdrawableDenomsTx( wex: WalletExecutionContext, tx: WalletDbReadOnlyTransaction<["denominations"]>, exchangeBaseUrl: string, @@ -1822,7 +1825,7 @@ async function isValidDenomRecord( stampExpireDeposit: timestampProtocolFromDb(d.stampExpireDeposit), stampExpireLegal: timestampProtocolFromDb(d.stampExpireLegal), stampExpireWithdraw: timestampProtocolFromDb(d.stampExpireWithdraw), - stampStart: timestampProtocolFromDb(d.stampExpireWithdraw), + stampStart: timestampProtocolFromDb(d.stampStart), value: Amounts.parseOrThrow(d.value), }); return res.valid; @@ -1832,31 +1835,71 @@ async function isValidDenomRecord( * Make sure that denominations that currently can be used for withdrawal * are validated, and the result of validation is stored in the database. */ -export async function updateWithdrawalDenoms( +export async function updateWithdrawalDenomsForCurrency( + wex: WalletExecutionContext, + currency: string, +): Promise<void> { + const res = await wex.db.runReadOnlyTx( + { storeNames: ["exchanges", "exchangeDetails", "denominations"] }, + async (tx) => { + return await tx.exchanges.getAll(); + }, + ); + for (const exch of res) { + if (exch.detailsPointer?.currency === currency) { + await updateWithdrawalDenomsForExchange(wex, exch.baseUrl); + } + } +} + +/** + * Make sure that denominations that currently can be used for withdrawal + * are validated, and the result of validation is stored in the database. + */ +export async function updateWithdrawalDenomsForExchange( wex: WalletExecutionContext, exchangeBaseUrl: string, ): Promise<void> { logger.trace( `updating denominations used for withdrawal for ${exchangeBaseUrl}`, ); - const exchangeDetails = await wex.db.runReadOnlyTx( - { storeNames: ["exchanges", "exchangeDetails"] }, + + const res = await wex.db.runReadOnlyTx( + { storeNames: ["exchanges", "exchangeDetails", "denominations"] }, async (tx) => { - return getExchangeWireDetailsInTx(tx, exchangeBaseUrl); + const exchangeDetails = await getExchangeWireDetailsInTx( + tx, + exchangeBaseUrl, + ); + let denominations: DenominationRecord[] | undefined = []; + if (exchangeDetails) { + // FIXME: Use denom groups to speed this up. + const allDenoms = + await tx.denominations.indexes.byExchangeBaseUrl.getAll( + exchangeBaseUrl, + ); + denominations = allDenoms + .filter((d) => d.currency === exchangeDetails.currency) + .filter((d) => + isCandidateWithdrawableDenom( + d, + wex.ws.config.testing.denomselAllowLate, + ), + ); + } + return { exchangeDetails, denominations }; }, ); + + const exchangeDetails = res.exchangeDetails; + if (!exchangeDetails) { logger.error("exchange details not available"); - throw Error(`exchange ${exchangeBaseUrl} details not available`); + return; } // First do a pass where the validity of candidate denominations // is checked and the result is stored in the database. - logger.trace("getting candidate denominations"); - const denominations = await getCandidateWithdrawalDenoms( - wex, - exchangeBaseUrl, - exchangeDetails.currency, - ); + const denominations = res.denominations; logger.trace(`got ${denominations.length} candidate denominations`); const batchSize = 500; let current = 0; @@ -2032,7 +2075,7 @@ async function processQueryReserve( await storeKnownBankAccount(tx, currency, lastOrigin); } if (amountChanged) { - const candidates = await getCandidateWithdrawalDenomsTx( + const candidates = await getWithdrawableDenomsTx( wex, tx, exchangeBaseUrl, @@ -2192,8 +2235,13 @@ async function processWithdrawalGroupPendingKyc( */ async function redenominateWithdrawal( wex: WalletExecutionContext, + exchangeBaseUrl: string, withdrawalGroupId: string, ): Promise<void> { + await fetchFreshExchange(wex, exchangeBaseUrl, { + forceUpdate: true, + }); + await updateWithdrawalDenomsForExchange(wex, exchangeBaseUrl); logger.trace(`redenominating withdrawal group ${withdrawalGroupId}`); await wex.db.runReadWriteTx( { storeNames: ["withdrawalGroups", "planchets", "denominations"] }, @@ -2213,7 +2261,7 @@ async function redenominateWithdrawal( const currency = Amounts.currencyOf(wg.denomsSel.totalWithdrawCost); const exchangeBaseUrl = wg.exchangeBaseUrl; - const candidates = await getCandidateWithdrawalDenomsTx( + const candidates = await getWithdrawableDenomsTx( wex, tx, exchangeBaseUrl, @@ -2486,11 +2534,7 @@ async function processWithdrawalGroupPendingReady( if (redenomRequired) { logger.warn(`withdrawal ${withdrawalGroupId} requires redenomination`); - await fetchFreshExchange(wex, exchangeBaseUrl, { - forceUpdate: true, - }); - await updateWithdrawalDenoms(wex, exchangeBaseUrl); - await redenominateWithdrawal(wex, withdrawalGroupId); + await redenominateWithdrawal(wex, exchangeBaseUrl, withdrawalGroupId); return TaskRunResult.backoff(); } @@ -2652,12 +2696,12 @@ export async function getExchangeWithdrawalInfo( ); logger.trace("updating withdrawal denoms"); - await updateWithdrawalDenoms(wex, exchangeBaseUrl); + await updateWithdrawalDenomsForExchange(wex, exchangeBaseUrl); wex.cancellationToken.throwIfCancelled(); logger.trace("getting candidate denoms"); - const candidateDenoms = await getCandidateWithdrawalDenoms( + const candidateDenoms = await getWithdrawableDenoms( wex, exchangeBaseUrl, instructedAmount.currency, @@ -3250,8 +3294,8 @@ async function getInitialDenomsSelection( forcedDenoms: ForcedDenomSel | undefined, ): Promise<DenomSelectionState> { const currency = Amounts.currencyOf(amount); - await updateWithdrawalDenoms(wex, exchange); - const denoms = await getCandidateWithdrawalDenoms(wex, exchange, currency); + await updateWithdrawalDenomsForExchange(wex, exchange); + const denoms = await getWithdrawableDenoms(wex, exchange, currency); if (forcedDenoms) { logger.warn("using forced denom selection");