commit 4f243f609d883c38c7fade3486cad5543f5be904 parent 47c23e8aa1d7615a0b8d83d46211d0b28b8bf675 Author: Florian Dold <dold@taler.net> Date: Fri, 7 Aug 2026 02:53:40 +0200 wallet-core: handle a change of the exchange's master public key The wallet refused any /keys whose master public key or currency differed from the stored one, which parked the entry in UnavailableUpdate for good and took the coins already withdrawn with it. It now adopts the new key set so the entry keeps working, records the one it replaced, and withholds only withdrawing -- the account a withdrawal pays into is signed by that key, so a URL that changed hands could otherwise redirect the transfer. Coins under the superseded key keep their own scope and are frozen unless the exchange still offers their denominations. Denominations, coin availability and coins are identified by the key that signed them rather than by the exchange's base URL, which is what makes two key sets able to coexist at one URL. Issue: https://bugs.taler.net/n/8576 Diffstat:
36 files changed, 1844 insertions(+), 527 deletions(-)
diff --git a/packages/taler-harness/src/integrationtests/test-exchange-master-pub-change.ts b/packages/taler-harness/src/integrationtests/test-exchange-master-pub-change.ts @@ -19,7 +19,9 @@ */ import { ExchangeUpdateStatus, + ScopeType, TalerErrorCode, + TransactionType, j2s, } from "@gnu-taler/taler-util"; import { WalletApiOperation } from "@gnu-taler/taler-wallet-core"; @@ -85,19 +87,25 @@ export async function runExchangeMasterPubChangeTest( t.logStep("exchange-restarted"); + const balanceBefore = await walletClient.call( + WalletApiOperation.GetBalances, + {}, + ); + await walletClient.call(WalletApiOperation.UpdateExchangeEntry, { exchangeBaseUrl: exchange.baseUrl, force: true, }); - const err = await t.assertThrowsTalerErrorAsync(async () => { - await walletClient.call(WalletApiOperation.TestingWaitExchangeReady, { - exchangeBaseUrl: exchange.baseUrl, - forceUpdate: true, - }); + // The entry must not wedge. Before bug 8576 was fixed this threw + // WALLET_EXCHANGE_UNAVAILABLE and the entry stayed in UnavailableUpdate + // forever, taking the funds already withdrawn with it. + await walletClient.call(WalletApiOperation.TestingWaitExchangeReady, { + exchangeBaseUrl: exchange.baseUrl, + forceUpdate: true, }); - console.log("updateExchangeEntry err:", j2s(err)); + t.logStep("exchange-still-ready"); const exchangesList = await walletClient.call( WalletApiOperation.ListExchanges, @@ -106,14 +114,121 @@ export async function runExchangeMasterPubChangeTest( console.log(j2s(exchangesList)); + const entry = exchangesList.exchanges.find( + (e) => e.exchangeBaseUrl === exchange.baseUrl, + ); + t.assertTrue(entry != null); + t.assertDeepEqual(entry.exchangeUpdateStatus, ExchangeUpdateStatus.Ready); + + // The new key set is adopted, so the entry keeps working -- but the change + // is recorded until the user confirms it. + const change = entry.unconfirmedKeyChange; + t.assertTrue(change != null); + t.assertDeepEqual(entry.masterPub, change.currentMasterPub); + t.assertTrue(change.currentMasterPub !== change.supersededMasterPub); + // A brand new exchange issues brand new denominations, so it does not offer + // to settle the coins already held. + t.assertDeepEqual(change.sharesDenominations, false); + + t.logStep("key-change-reported"); + + // The coins withdrawn under the old key are still there. The sweep that + // marks denominations as no longer offered, and the denomination-loss + // handling behind it, must not have judged them against the new key set. + const balanceAfter = await walletClient.call( + WalletApiOperation.GetBalances, + {}, + ); + // The funds are still there, but in a bucket of their own: they were issued + // under a key this exchange has replaced, so pooling them with what it + // issues now would show one number the user cannot wholly spend. + const before = balanceBefore.balances.find( + (b) => b.scopeInfo.type === ScopeType.Exchange, + ); + const legacy = balanceAfter.balances.find( + (b) => b.scopeInfo.type === ScopeType.ExchangeLegacyKeys, + ); + t.assertTrue(before != null && legacy != null); + t.assertAmountEquals(legacy.available, before.available); + t.assertDeepEqual( + legacy.scopeInfo.type === ScopeType.ExchangeLegacyKeys + ? legacy.scopeInfo.masterPub + : undefined, + change.supersededMasterPub, + ); + + const txs = await walletClient.call(WalletApiOperation.GetTransactions, {}); + const denomLoss = txs.transactions.filter( + (tx) => tx.type === TransactionType.DenomLoss, + ); + t.assertDeepEqual(denomLoss.length, 0); + + t.logStep("old-funds-intact"); + + // ...but they are not spendable: this exchange issues different + // denominations now and would refuse to settle the old coins, so selecting + // them would build a payment that fails after the user committed to it. + const detail = await walletClient.call(WalletApiOperation.GetBalanceDetail, { + currency: "TESTKUDOS", + }); + t.assertAmountEquals(detail.balanceAvailable, "TESTKUDOS:0"); + + t.logStep("old-funds-frozen"); + + // Withdrawing is what sends money to the exchange, so it is what stays shut + // until the change is confirmed. + const err = await t.assertThrowsTalerErrorAsync(async () => { + await walletClient.call(WalletApiOperation.AcceptManualWithdrawal, { + exchangeBaseUrl: exchange.baseUrl, + amount: "TESTKUDOS:5", + }); + }); t.assertDeepEqual( - exchangesList.exchanges[0].exchangeUpdateStatus, - ExchangeUpdateStatus.UnavailableUpdate, + err.errorDetail.code, + TalerErrorCode.WALLET_EXCHANGE_KEYS_NOT_ACCEPTED, ); + + t.logStep("withdrawal-refused"); + + // Confirming the wrong key must not release it: that is the stale-UI guard. + const mismatch = await t.assertThrowsTalerErrorAsync(async () => { + await walletClient.call(WalletApiOperation.ConfirmExchangeKeyChange, { + exchangeBaseUrl: exchange.baseUrl, + currentMasterPub: change.supersededMasterPub, + }); + }); t.assertDeepEqual( - exchangesList.exchanges[0].lastUpdateErrorInfo?.error.code, - TalerErrorCode.WALLET_EXCHANGE_ENTRY_UPDATE_CONFLICT, + mismatch.errorDetail.code, + TalerErrorCode.WALLET_EXCHANGE_KEY_CHANGE_MISMATCH, + ); + + await walletClient.call(WalletApiOperation.ConfirmExchangeKeyChange, { + exchangeBaseUrl: exchange.baseUrl, + currentMasterPub: change.currentMasterPub, + }); + + const afterConfirm = await walletClient.call( + WalletApiOperation.ListExchanges, + {}, + ); + const confirmedEntry = afterConfirm.exchanges.find( + (e) => e.exchangeBaseUrl === exchange.baseUrl, ); + t.assertTrue(confirmedEntry?.unconfirmedKeyChange === undefined); + + // And now a withdrawal gets as far as the exchange rather than being + // refused locally. + await walletClient.call(WalletApiOperation.AcceptManualWithdrawal, { + exchangeBaseUrl: exchange.baseUrl, + amount: "TESTKUDOS:5", + }); + + t.logStep("withdrawal-released"); + + // Leave the port free: this test runs a second exchange on the same port as + // the first, and a lingering one poisons whichever test runs next -- its + // taler-exchange-offline reports "exchange uses different master key". + await exchange2.stop(); } runExchangeMasterPubChangeTest.suites = ["wallet", "exchange"]; diff --git a/packages/taler-harness/src/integrationtests/test-wallet-exchange-update.ts b/packages/taler-harness/src/integrationtests/test-wallet-exchange-update.ts @@ -19,6 +19,7 @@ */ import { AmountString, + Amounts, BasicAuth, ExchangeUpdateStatus, NotificationType, @@ -159,17 +160,14 @@ export async function runWalletExchangeUpdateTest( console.log("waiting for exchange to be ready"); - // Since the second exchange has the same base URL but - // a different public key, we expect the exchange - // entry to end up in an error state. - // Note that this might change in the future - // when we handle the case more gracefully. + // The second exchange has the same base URL but a different master public + // key. The wallet adopts the new key set rather than wedging the entry -- + // the coins withdrawn under the old one have to stay usable -- and records + // the change until the user confirms it. - await t.assertThrowsAsync(async () => { - await walletClient.call(WalletApiOperation.TestingWaitExchangeReady, { - exchangeBaseUrl: exchangeOne.baseUrl, - forceUpdate: true, - }); + await walletClient.call(WalletApiOperation.TestingWaitExchangeReady, { + exchangeBaseUrl: exchangeOne.baseUrl, + forceUpdate: true, }); const exchangeEntry = await walletClient.call( @@ -181,10 +179,24 @@ export async function runWalletExchangeUpdateTest( console.log(`exchange entry: ${j2s(exchangeEntry)}`); - await t.assertThrowsAsync(async () => { - await walletClient.call(WalletApiOperation.GetWithdrawalDetailsForAmount, { + t.assertTrue(exchangeEntry.unconfirmedKeyChange != null); + + // Asking what a withdrawal would look like still works, and carries the + // warning: this is the screen the user sees before committing. + const details = await walletClient.call( + WalletApiOperation.GetWithdrawalDetailsForAmount, + { amount: "TESTKUDOS:10" as AmountString, exchangeBaseUrl: exchangeOne.baseUrl, + }, + ); + t.assertTrue(details.unconfirmedKeyChange != null); + + // Committing to it is what gets refused. + await t.assertThrowsAsync(async () => { + await walletClient.call(WalletApiOperation.AcceptManualWithdrawal, { + exchangeBaseUrl: exchangeOne.baseUrl, + amount: "TESTKUDOS:10" as AmountString, }); }); @@ -201,7 +213,31 @@ export async function runWalletExchangeUpdateTest( console.log("starting first exchange"); await exchangeOne.start(); + // The entry is healthy under the key it adopted, so it is not being + // retried every few seconds any more. A client that wants to know now + // asks for an update, which is what a UI showing the warning would do. + await walletClient.call(WalletApiOperation.UpdateExchangeEntry, { + exchangeBaseUrl: exchangeOne.baseUrl, + force: true, + }); + await exchangeAvailableCond; + + // Going back to the key the coins were issued under is not a new change to + // confirm: the wallet is where it started, so the warning clears and the + // funds frozen in the meantime are spendable again. + const restored = await walletClient.call( + WalletApiOperation.GetExchangeEntryByUrl, + { + exchangeBaseUrl: exchangeOne.baseUrl, + }, + ); + t.assertTrue(restored.unconfirmedKeyChange === undefined); + + const balance = await walletClient.call(WalletApiOperation.GetBalanceDetail, { + currency: "TESTKUDOS", + }); + t.assertTrue(Amounts.isNonZero(balance.balanceAvailable)); } runWalletExchangeUpdateTest.suites = ["wallet"]; diff --git a/packages/taler-util/src/errors.ts b/packages/taler-util/src/errors.ts @@ -224,6 +224,27 @@ export interface DetailsMap { [TalerErrorCode.WALLET_EXCHANGE_ENTRY_UPDATE_CONFLICT]: { detail?: string; }; + [TalerErrorCode.WALLET_EXCHANGE_NO_KEY_CHANGE_PENDING]: { + exchangeBaseUrl: string; + }; + [TalerErrorCode.WALLET_EXCHANGE_KEY_CHANGE_MISMATCH]: { + exchangeBaseUrl: string; + currentMasterPub: string | undefined; + confirmedMasterPub: string; + }; + [TalerErrorCode.WALLET_EXCHANGE_KEYS_NOT_ACCEPTED]: { + exchangeBaseUrl: string; + currentMasterPub: string; + supersededMasterPub: string; + }; + [TalerErrorCode.WALLET_EXCHANGE_ENTRY_NOT_FOUND]: { + exchangeBaseUrl?: string; + /** + * Set when the entry was looked up by the key that signed a denomination + * rather than by URL. + */ + masterPub?: string; + }; [TalerErrorCode.WALLET_REQUEST_TRANSACTION_STATE_UNSUPPORTED]: { message?: string; txState: TransactionState; diff --git a/packages/taler-util/src/types-taler-wallet.ts b/packages/taler-util/src/types-taler-wallet.ts @@ -66,6 +66,7 @@ import { codecForAbsoluteTime, codecForDuration, codecForTimestamp, + codecForPreciseTimestamp, } from "./time.js"; import { BlindedDonationReceiptKeyPair } from "./types-donau.js"; import { WithdrawalOperationStatusFlag } from "./types-taler-bank-integration.js"; @@ -200,6 +201,16 @@ export enum ScopeType { Global = "global", Exchange = "exchange", Auditor = "auditor", + /** + * Funds issued under a master public key the exchange has since replaced. + * + * A distinct type rather than an optional field on {@link ScopeInfoExchange} + * on purpose: these funds must never be pooled with, or selected alongside, + * funds under the key the exchange currently uses, and a scope that merely + * carried an extra field would render as a second bucket with the same + * label and would still be matched by an existing exchange-scoped filter. + */ + ExchangeLegacyKeys = "exchange-legacy-keys", } export type ScopeInfoGlobal = { type: ScopeType.Global; currency: string }; @@ -216,7 +227,19 @@ export type ScopeInfoAuditor = { url: string; }; -export type ScopeInfo = ScopeInfoGlobal | ScopeInfoExchange | ScopeInfoAuditor; +export type ScopeInfoExchangeLegacyKeys = { + type: ScopeType.ExchangeLegacyKeys; + currency: string; + url: string; + /** The superseded key the funds were issued under. */ + masterPub: string; +}; + +export type ScopeInfo = + | ScopeInfoGlobal + | ScopeInfoExchange + | ScopeInfoAuditor + | ScopeInfoExchangeLegacyKeys; export const codecForScopeInfo = (): Codec<ScopeInfo> => buildCodecForUnion<ScopeInfo>() @@ -224,6 +247,10 @@ export const codecForScopeInfo = (): Codec<ScopeInfo> => .alternative(ScopeType.Global, codecForScopeInfoGlobal()) .alternative(ScopeType.Exchange, codecForScopeInfoExchange()) .alternative(ScopeType.Auditor, codecForScopeInfoAuditor()) + .alternative( + ScopeType.ExchangeLegacyKeys, + codecForScopeInfoExchangeLegacyKeys(), + ) .build("ScopeInfo"); /** @@ -406,6 +433,15 @@ export const codecForScopeInfoAuditor = (): Codec<ScopeInfoAuditor> => .property("url", codecForString()) .build("ScopeInfoAuditor"); +export const codecForScopeInfoExchangeLegacyKeys = + (): Codec<ScopeInfoExchangeLegacyKeys> => + buildCodecForObject<ScopeInfoExchangeLegacyKeys>() + .property("currency", codecForString()) + .property("type", codecForConstString(ScopeType.ExchangeLegacyKeys)) + .property("url", codecForString()) + .property("masterPub", codecForString()) + .build("ScopeInfoExchangeLegacyKeys"); + export interface GetCurrencySpecificationRequest { scope: ScopeInfo; } @@ -526,6 +562,10 @@ export function stringifyScopeInfoShort(si: ScopeInfo): string { return `${si.currency}/${encodeURIComponent(si.url)}`; case ScopeType.Auditor: return `${si.currency}:${encodeURIComponent(si.url)}`; + case ScopeType.ExchangeLegacyKeys: + // The URL is percent-encoded and so contains no slash of its own, + // which is what lets a third field be appended unambiguously. + return `${si.currency}/${encodeURIComponent(si.url)}/${si.masterPub}`; } } export function parseScopeInfoShort(si: string): ScopeInfo | undefined { @@ -545,10 +585,21 @@ export function parseScopeInfoShort(si: string): ScopeInfo | undefined { }; } if (indexOfSlash > 0) { + const currency = si.substring(0, indexOfSlash); + const rest = si.substring(indexOfSlash + 1); + const sep = rest.indexOf("/"); + if (sep > 0) { + return { + type: ScopeType.ExchangeLegacyKeys, + currency, + url: decodeURIComponent(rest.substring(0, sep)), + masterPub: rest.substring(sep + 1), + }; + } return { type: ScopeType.Exchange, - currency: si.substring(0, indexOfSlash), - url: decodeURIComponent(si.substring(indexOfSlash + 1)), + currency, + url: decodeURIComponent(rest), }; } return undefined; @@ -567,6 +618,13 @@ export function stringifyScopeInfo(si: ScopeInfo): string { return `taler-si:auditor/${si.currency}/${encodeURIComponent(si.url)}`; case ScopeType.Exchange: return `taler-si:exchange/${si.currency}/${encodeURIComponent(si.url)}`; + case ScopeType.ExchangeLegacyKeys: + // A prefix of its own, so that every scope string written before this + // type existed still encodes to exactly the bytes it did before and no + // stored record has to be rewritten. + return `taler-si:exchange-legacy/${si.currency}/${encodeURIComponent( + si.url, + )}/${si.masterPub}`; } } @@ -1708,9 +1766,52 @@ export interface ShortExchangeListItem { /** * Info about an exchange entry in the wallet. */ +/** + * An exchange that changed its key set, pending the user's confirmation. + * + * The wallet has already adopted the new key set, so the entry works and the + * older coins stay spendable. What is withheld until this is confirmed is + * withdrawing: the bank details a withdrawal pays into are signed by the + * master key, so adopting a new one silently would let a URL that changed + * hands redirect the next transfer. + */ +export interface ExchangeKeyChangeInfo { + /** Master public key the exchange now uses, and the wallet now trusts. */ + currentMasterPub: string; + currentCurrency: string; + /** Master public key the wallet's older funds were issued under. */ + supersededMasterPub: string; + supersededCurrency: string; + /** + * Whether the new key set still advertises denominations the wallet holds + * coins of. + * + * False means the exchange does not offer to settle the older coins at all. + * True is the exchange's claim that it does, not proof of continuity: + * denomination public keys are public and anyone can re-publish them. + */ + sharesDenominations: boolean; + firstSeen: TalerPreciseTimestamp; +} + +export const codecForExchangeKeyChangeInfo = (): Codec<ExchangeKeyChangeInfo> => + buildCodecForObject<ExchangeKeyChangeInfo>() + .property("currentMasterPub", codecForString()) + .property("currentCurrency", codecForString()) + .property("supersededMasterPub", codecForString()) + .property("supersededCurrency", codecForString()) + .property("sharesDenominations", codecForBoolean()) + .property("firstSeen", codecForPreciseTimestamp) + .build("ExchangeKeyChangeInfo"); + export interface ExchangeListItem { exchangeBaseUrl: string; masterPub: string | undefined; + /** + * Set when the exchange changed its key set and the user has not confirmed + * the change yet. Withdrawals are refused while this is present. + */ + unconfirmedKeyChange?: ExchangeKeyChangeInfo; currency: string; paytoUris: string[]; tosStatus: ExchangeTosStatus; @@ -1922,6 +2023,13 @@ export interface WithdrawalDetailsForAmount { scopeInfo: ScopeInfo; /** + * Set when the exchange changed its key set and the user has not confirmed + * the change. Accepting the withdrawal will be refused until they do, so + * this is the point at which to warn them. + */ + unconfirmedKeyChange?: ExchangeKeyChangeInfo; + + /** * KYC soft limit. * * Withdrawals over that amount will require KYC. @@ -1976,6 +2084,13 @@ export interface ExchangeWithdrawalDetails { exchangePaytoUris: string[]; /** + * Set when the exchange changed its key set and the user has not confirmed + * the change. Copied onto the responses the UIs show before the user + * commits to a withdrawal. + */ + unconfirmedKeyChange?: ExchangeKeyChangeInfo; + + /** * Filtered wire info to send to the bank. */ exchangeWireAccounts: string[]; @@ -2375,6 +2490,24 @@ export const codecForAcceptExchangeTosRequest = .property("exchangeBaseUrl", codecForCanonBaseUrl()) .build("AcceptExchangeTosRequest"); +export interface ConfirmExchangeKeyChangeRequest { + exchangeBaseUrl: string; + /** + * Master public key the exchange now uses. + * + * Required, so that a UI showing a stale key change cannot confirm a + * different one than the user was looking at. + */ + currentMasterPub: string; +} + +export const codecForConfirmExchangeKeyChangeRequest = + (): Codec<ConfirmExchangeKeyChangeRequest> => + buildCodecForObject<ConfirmExchangeKeyChangeRequest>() + .property("exchangeBaseUrl", codecForCanonBaseUrl()) + .property("currentMasterPub", codecForString()) + .build("ConfirmExchangeKeyChangeRequest"); + export interface ForgetExchangeTosRequest { exchangeBaseUrl: string; } @@ -3288,12 +3421,16 @@ export interface SelectedCoin { coinPub: string; contribution: AmountString; exchangeBaseUrl: string; + /** Master public key that signed the denomination. */ + exchangeMasterPub: string; } export interface SelectedProspectiveCoin { denomPubHash: string; contribution: AmountString; exchangeBaseUrl: string; + /** Master public key that signed the denomination. */ + exchangeMasterPub: string; } /** diff --git a/packages/taler-wallet-core/src/balance.ts b/packages/taler-wallet-core/src/balance.ts @@ -93,13 +93,9 @@ import { WithdrawalRecordType, WalletDonationSummary, } from "./db-common.js"; -import { -} from "./db-indexeddb.js"; +import {} from "./db-indexeddb.js"; import { WalletDbTransaction } from "./dbtx.js"; -import { - getDenomInfo, - WalletExecutionContext, -} from "./wallet.js"; +import { getDenomInfo, WalletExecutionContext } from "./wallet.js"; /** * Logger. @@ -153,6 +149,10 @@ function getBalanceKey(scopeInfo: ScopeInfo): string { return `${scopeInfo.type};${scopeInfo.currency};${scopeInfo.url}`; case ScopeType.Global: return `${scopeInfo.type};${scopeInfo.currency}`; + case ScopeType.ExchangeLegacyKeys: + // The key is part of the bucket identity: one exchange can have funds + // under more than one superseded key. + return `${scopeInfo.type};${scopeInfo.currency};${scopeInfo.url};${scopeInfo.masterPub}`; } } @@ -164,6 +164,10 @@ function getScopeSortingOrder(scopeInfo: ScopeInfo): number { return 1; case ScopeType.Exchange: return 2; + // Sorted last: these funds are a leftover, not something to spend from + // by default. + case ScopeType.ExchangeLegacyKeys: + return 3; default: assertUnreachable(scopeInfo); } @@ -188,15 +192,33 @@ class BalancesStore { * Add amount to a balance field, both for * the slicing by exchange and currency. */ + /** + * @param exchangeMasterPub the key that signed the funds, when they are + * coins. Amounts that are not yet coins -- a pending withdrawal, a + * refresh output -- always belong to the key set in force now and pass + * nothing. + */ private async initBalance( currency: string, exchangeBaseUrl: string, + exchangeMasterPub?: string, ): Promise<WalletBalance> { - let scopeInfo: ScopeInfo | undefined = - this.exchangeScopeCache[exchangeBaseUrl]; + // The currency and the key are part of the cache key, not just the URL: + // one exchange can hold funds under a key or a currency it has since + // replaced, and those are different buckets. Keyed on the URL alone, + // the first scope resolved would be reused for all of them and amounts + // in different currencies would be added together. + const cacheKey = `${exchangeBaseUrl}\u0000${currency}\u0000${ + exchangeMasterPub ?? "" + }`; + let scopeInfo: ScopeInfo | undefined = this.exchangeScopeCache[cacheKey]; if (!scopeInfo) { - scopeInfo = await this.tx.getExchangeScopeInfo(exchangeBaseUrl, currency); - this.exchangeScopeCache[exchangeBaseUrl] = scopeInfo; + scopeInfo = await this.resolveScope( + exchangeBaseUrl, + currency, + exchangeMasterPub, + ); + this.exchangeScopeCache[cacheKey] = scopeInfo; } const balanceKey = getBalanceKey(scopeInfo); let b = this.balanceStore[balanceKey]; @@ -220,8 +242,41 @@ class BalancesStore { return this.balanceStore[balanceKey]; } - async addZero(currency: string, exchangeBaseUrl: string): Promise<void> { - await this.initBalance(currency, exchangeBaseUrl); + /** + * Scope for funds at an exchange, telling superseded key sets apart. + * + * Coins issued under a master key the exchange has replaced, or in a + * currency it no longer uses, are not interchangeable with what it issues + * now, so they get a scope of their own rather than being pooled. + */ + private async resolveScope( + exchangeBaseUrl: string, + currency: string, + exchangeMasterPub?: string, + ): Promise<ScopeInfo> { + const det = await this.tx.getExchangeDetails(exchangeBaseUrl); + if ( + det && + (det.currency !== currency || + (exchangeMasterPub != null && + det.masterPublicKey !== exchangeMasterPub)) + ) { + return { + type: ScopeType.ExchangeLegacyKeys, + currency, + url: exchangeBaseUrl, + masterPub: exchangeMasterPub ?? det.masterPublicKey, + }; + } + return await this.tx.getExchangeScopeInfo(exchangeBaseUrl, currency); + } + + async addZero( + currency: string, + exchangeBaseUrl: string, + exchangeMasterPub?: string, + ): Promise<void> { + await this.initBalance(currency, exchangeBaseUrl, exchangeMasterPub); } async setPeerPaymentsDisabled( @@ -253,8 +308,13 @@ class BalancesStore { currency: string, exchangeBaseUrl: string, amount: AmountLike, + exchangeMasterPub?: string, ): Promise<void> { - const b = await this.initBalance(currency, exchangeBaseUrl); + const b = await this.initBalance( + currency, + exchangeBaseUrl, + exchangeMasterPub, + ); b.available = Amounts.add(b.available, amount).amount; } @@ -434,12 +494,19 @@ export async function getBalancesInsideTransaction( const coinAvailability = await tx.getCoinAvailabilities(); for (const ca of coinAvailability) { const count = ca.visibleCoinCount ?? 0; - await balanceStore.addZero(ca.currency, ca.exchangeBaseUrl); + // The denomination is authoritative for which key set the coins belong + // to: an exchange update re-attributes the denominations it still + // offers, while the availability row keeps the key recorded when the + // coin was made available. + const denom = await tx.getDenomination(ca); + const masterPub = denom?.exchangeMasterPub ?? ca.exchangeMasterPub; + await balanceStore.addZero(ca.currency, ca.exchangeBaseUrl, masterPub); if (count > 0) { await balanceStore.addAvailable( ca.currency, ca.exchangeBaseUrl, Amounts.mult(ca.value, count).amount, + masterPub, ); } } @@ -880,12 +947,7 @@ export async function getPaymentBalanceDetailsInTx( continue; } - const denom = await getDenomInfo( - wex, - tx, - ca.exchangeBaseUrl, - ca.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, ca); if (!denom) { continue; } @@ -906,6 +968,14 @@ export async function getPaymentBalanceDetailsInTx( continue; } + // Signed by a master key the exchange has replaced and did not + // re-advertise: coin selection will not pick these, so counting them as + // spendable here would make the balance disagree with what a payment can + // actually do. They keep their value and stay in getBalances. + if (denom.exchangeMasterPub !== wireDetails.masterPublicKey) { + continue; + } + const singleCoinAmount: AmountJson = Amounts.parseOrThrow(ca.value); const coinAmount: AmountJson = Amounts.mult( singleCoinAmount, diff --git a/packages/taler-wallet-core/src/coinSelection.test.ts b/packages/taler-wallet-core/src/coinSelection.test.ts @@ -79,6 +79,7 @@ test("p2p: should select the coin", (t) => { assert.deepStrictEqual(coins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:2.1")], @@ -110,6 +111,7 @@ test("p2p: should select 3 coins", (t) => { assert.deepStrictEqual(coins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [ @@ -176,6 +178,7 @@ test("pay: select one coin to pay with fee", (t) => { assert.deepStrictEqual(coins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:2.2")], @@ -505,12 +508,14 @@ test("overpay when remaining < depositFee", (t) => { assert.deepStrictEqual(coins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:1.1")], }, "hash1;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash1", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:1")], @@ -560,6 +565,7 @@ test("prefer exact denom", (t) => { assert.deepStrictEqual(coins, { "hash1;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash1", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:2")], @@ -753,6 +759,7 @@ test("legacy-2024: takes the largest coin instead of the exact one", (t) => { assert.deepStrictEqual(coins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:2")], @@ -800,12 +807,14 @@ test("legacy-2024: overspends the deposit fee that the allowance covers", (t) => assert.deepStrictEqual(coins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:2")], }, "hash1;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash1", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:0.2")], @@ -849,6 +858,7 @@ test("legacy-2024: spends the largest coins first", (t) => { assert.deepStrictEqual(legacyCoins, { "hash0;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash0", maxAge: 32, contributions: [Amounts.parseOrThrow("LOCAL:4")], @@ -868,6 +878,7 @@ test("legacy-2024: spends the largest coins first", (t) => { assert.deepStrictEqual(defaultCoins, { "hash1;32;http://exchange.localhost/": { exchangeBaseUrl: "http://exchange.localhost/", + exchangeMasterPub: "123", denomPubHash: "hash1", maxAge: 32, contributions: [ diff --git a/packages/taler-wallet-core/src/coinSelection.ts b/packages/taler-wallet-core/src/coinSelection.ts @@ -392,12 +392,7 @@ async function maybeRepairCoinSelection( if (!coin) { continue; } - const denom = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, coin); if (!denom) { continue; } @@ -414,6 +409,7 @@ async function maybeRepairCoinSelection( coinRes.push({ exchangeBaseUrl: coin.exchangeBaseUrl, + exchangeMasterPub: coin.exchangeMasterPub, denomPubHash: coin.denomPubHash, coinPub: prev.coinPub, contribution: Amounts.stringify(prev.contribution), @@ -434,12 +430,7 @@ async function assembleSelectPayCoinsSuccessResult( for (const dph of Object.keys(finalSel)) { const selInfo = finalSel[dph]; const numRequested = selInfo.contributions.length; - const coins = await tx.getFreshCoinsByDenomAndAge( - selInfo.exchangeBaseUrl, - selInfo.denomPubHash, - selInfo.maxAge, - numRequested, - ); + const coins = await tx.getFreshCoinsByDenomAndAge(selInfo, numRequested); if (coins.length != numRequested) { throw Error( `coin selection failed (not available anymore, got only ${coins.length}/${numRequested})`, @@ -452,6 +443,7 @@ async function assembleSelectPayCoinsSuccessResult( coinPub: coins[i].coinPub, contribution: Amounts.stringify(selInfo.contributions[i]), exchangeBaseUrl: coins[i].exchangeBaseUrl, + exchangeMasterPub: coins[i].exchangeMasterPub, }); } } @@ -655,6 +647,7 @@ interface SelResult { */ [avKey: string]: { exchangeBaseUrl: string; + exchangeMasterPub: string; denomPubHash: string; maxAge: number; contributions: AmountJson[]; @@ -693,6 +686,7 @@ function applyContributions( contributions: [], denomPubHash: denom.denomPubHash, exchangeBaseUrl: denom.exchangeBaseUrl, + exchangeMasterPub: denom.exchangeMasterPub, maxAge: denom.maxAge, }; } @@ -902,6 +896,7 @@ function selectForced( contributions: [], denomPubHash: aci.denomPubHash, exchangeBaseUrl: aci.exchangeBaseUrl, + exchangeMasterPub: aci.exchangeMasterPub, maxAge: aci.maxAge, }; } @@ -1149,13 +1144,8 @@ async function selectPayCandidates( // Save denoms with how many coins are available // FIXME: Check that the individual denomination is audited! - // FIXME: Should we exclude denominations that are - // not spendable anymore? for (const coinAvail of myExchangeCoins) { - const denom = await tx.getDenomination( - coinAvail.exchangeBaseUrl, - coinAvail.denomPubHash, - ); + const denom = await tx.getDenomination(coinAvail); checkDbInvariant( !!denom, `denomination of a coin is missing hash: ${coinAvail.denomPubHash}`, @@ -1168,6 +1158,19 @@ async function selectPayCandidates( logger.trace("denom is unoffered"); continue; } + // Signed by a master key the exchange has replaced, and not + // re-advertised under the new one -- an exchange update re-attributes + // the denominations it still offers, so one left on the old key is one + // this exchange has stopped standing behind. The coins keep their + // value in the database and stay visible, but selecting them would + // build a payment the exchange refuses to settle, after the user has + // committed to it. + if (denom.exchangeMasterPub !== exchangeDetails.masterPublicKey) { + logger.trace( + `denom ${denom.denomPubHash} is signed by a superseded master key`, + ); + continue; + } numUsable++; let numAvailable = coinAvail.freshCoinCount ?? 0; if (req.includePendingCoins) { @@ -1258,12 +1261,7 @@ export async function computeCoinSelMaxExpirationDate( let minAutorefreshExecuteThreshold = TalerProtocolTimestamp.never(); for (const dph of Object.keys(selectedDenom)) { const selInfo = selectedDenom[dph]; - const denom = await getDenomInfo( - wex, - tx, - selInfo.exchangeBaseUrl, - selInfo.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, selInfo); if (!denom) { continue; } diff --git a/packages/taler-wallet-core/src/common.ts b/packages/taler-wallet-core/src/common.ts @@ -115,12 +115,7 @@ export async function makeCoinsVisible( if (!coinRecord.visible) { coinRecord.visible = 1; await tx.upsertCoin(coinRecord); - const ageRestriction = coinRecord.maxAge; - const car = await tx.getCoinAvailability( - coinRecord.exchangeBaseUrl, - coinRecord.denomPubHash, - ageRestriction, - ); + const car = await tx.getCoinAvailability(coinRecord); if (!car) { logger.error("missing coin availability record"); continue; @@ -142,20 +137,13 @@ export async function makeCoinAvailable( if (existingCoin) { return; } - const denom = await tx.getDenomination( - coinRecord.exchangeBaseUrl, - coinRecord.denomPubHash, - ); + const denom = await tx.getDenomination(coinRecord); checkDbInvariant( !!denom, `denomination of a coin is missing hash: ${coinRecord.denomPubHash}`, ); const ageRestriction = coinRecord.maxAge; - let car = await tx.getCoinAvailability( - coinRecord.exchangeBaseUrl, - coinRecord.denomPubHash, - ageRestriction, - ); + let car = await tx.getCoinAvailability(coinRecord); if (!car) { car = { maxAge: ageRestriction, @@ -194,21 +182,12 @@ export async function spendCoins( if (!coin) { throw Error("coin allocated for payment doesn't exist anymore"); } - const denom = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, coin); checkDbInvariant( !!denom, `denomination of a coin is missing hash: ${coin.denomPubHash}`, ); - const coinAvailability = await tx.getCoinAvailability( - coin.exchangeBaseUrl, - coin.denomPubHash, - coin.maxAge, - ); + const coinAvailability = await tx.getCoinAvailability(coin); checkDbInvariant( !!coinAvailability, `age denom info is missing for ${coin.maxAge}`, @@ -1054,6 +1033,37 @@ export function requireExchangeTosAcceptedOrThrow( } /** + * Refuse an operation that sends money to an exchange whose key set changed + * and has not been confirmed. + * + * Only the money-in direction is gated. The wallet has already adopted the + * new key set, so spending, depositing and refreshing the coins it already + * holds keep working -- those settle against denominations the exchange + * signed earlier and are not affected by which key it uses now. A + * withdrawal is different: it pays into bank details signed by the current + * master key, so an exchange URL that changed hands could otherwise redirect + * the transfer. + */ +export function requireExchangeKeysConfirmedOrThrow( + wex: WalletExecutionContext, + exchange: ReadyExchangeSummary, +): void { + const change = exchange.unconfirmedKeyChange; + if (!change) { + return; + } + throw TalerError.fromDetail( + TalerErrorCode.WALLET_EXCHANGE_KEYS_NOT_ACCEPTED, + { + exchangeBaseUrl: exchange.exchangeBaseUrl, + currentMasterPub: change.currentMasterPub, + supersededMasterPub: change.supersededMasterPub, + }, + "the exchange changed its master public key and the change has not been confirmed", + ); +} + +/** * Fetch with cancellation token */ export async function cancelableFetch( diff --git a/packages/taler-wallet-core/src/db-common.ts b/packages/taler-wallet-core/src/db-common.ts @@ -1151,6 +1151,26 @@ export interface WalletExchangeDetailsPointer { /** * Exchange record as stored in the wallet's database. */ +/** + * A key set an exchange replaced, pending the user's confirmation. + */ +export interface WalletSupersededKeySet { + masterPublicKey: string; + currency: string; + /** When the change was first observed. */ + firstSeen: DbPreciseTimestamp; + /** + * Whether the new key set re-advertises denominations the wallet holds + * coins of. + * + * A claim, not proof: denomination public keys are public, so anyone can + * re-publish them under a new master key. It says whether the exchange + * offers to settle the older coins at all, which is what decides if they + * are worth selecting. + */ + sharesDenominations: boolean; +} + export interface WalletExchangeEntry { /** * Base url of the exchange. @@ -1193,6 +1213,20 @@ export interface WalletExchangeEntry { */ detailsPointer: WalletExchangeDetailsPointer | undefined; + /** + * The key set this exchange used before it changed keys, kept until the + * user confirms the change. + * + * The new key set is adopted immediately -- {@link detailsPointer} moves -- + * so the entry keeps working and the coins already held stay spendable. + * What is withheld is only the part that sends money to the exchange: the + * wire details a withdrawal pays into are signed by the master key, so a + * URL taken over by someone else would otherwise redirect the next + * withdrawal. Absent once the change has been confirmed, and never set on + * first contact, which is not a change. + */ + supersededKeySet?: WalletSupersededKeySet; + entryStatus: ExchangeEntryDbRecordStatus; updateStatus: ExchangeEntryDbUpdateStatus; @@ -1513,6 +1547,16 @@ export interface WalletCoin { exchangeBaseUrl: string; /** + * Master public key that signed the denomination this coin was issued + * under. + * + * This, not the base URL, is what ties a coin to the keys that can settle + * it: the URL is where the exchange currently answers, and it can change + * without the coin changing. + */ + exchangeMasterPub: string; + + /** * Blinding key used when withdrawing the coin. * Potentionally used again during payback. */ @@ -1560,7 +1604,14 @@ export interface WalletCoinAvailability { value: AmountString; denomPubHash: string; exchangeBaseUrl: string; - exchangeMasterPub?: string; + /** + * Master public key that signed the denomination. + * + * Required: together with the hash it names the denomination these coins + * belong to. Rows written before it was recorded are backfilled from that + * denomination; the empty string means it could no longer be found. + */ + exchangeMasterPub: string; /** * Age restriction on the coin, or 0 for no age restriction (or diff --git a/packages/taler-wallet-core/src/db-indexeddb.ts b/packages/taler-wallet-core/src/db-indexeddb.ts @@ -263,7 +263,7 @@ export const CURRENT_DB_CONFIG_KEY = "currentMainDbName"; * backwards-compatible way or object stores and indices * are added. */ -export const WALLET_DB_MINOR_VERSION = 29; +export const WALLET_DB_MINOR_VERSION = 31; // FIXME: Should these be numeric codes? export type KycUserType = "individual" | "business"; @@ -690,6 +690,29 @@ export const WalletIndexedDbStoresV1 = { ), }, }), + // Keyed by the master public key for the same reason as denominationsV2: + // the coins of one denomination hash under two different keys are not the + // same coins, and must not share a count. + coinAvailabilityV2: describeStore( + "coinAvailabilityV2", + describeContents<WalletCoinAvailability>({ + keyPath: ["exchangeMasterPub", "denomPubHash", "maxAge"], + versionAdded: 31, + }), + { + byExchangeAgeAvailability: describeIndex( + "byExchangeAgeAvailability", + ["exchangeBaseUrl", "maxAge", "freshCoinCount"], + { versionAdded: 31 }, + ), + byExchangeBaseUrl: describeIndex("byExchangeBaseUrl", "exchangeBaseUrl", { + versionAdded: 31, + }), + }, + ), + // The pre-re-key store. Keeps its map key equal to its store name: the + // transaction client exposes accessors by store name, so an `_obsolete_` + // alias would typecheck and then be undefined at runtime. coinAvailability: describeStore( "coinAvailability", describeContents<WalletCoinAvailability>({ @@ -720,6 +743,13 @@ export const WalletIndexedDbStoresV1 = { { byBaseUrl: describeIndex("byBaseUrl", "exchangeBaseUrl"), byDenomPubHash: describeIndex("byDenomPubHash", "denomPubHash"), + byMasterPubDenomPubHashAndAgeAndStatus: describeIndex( + "byMasterPubDenomPubHashAndAgeAndStatus", + ["exchangeMasterPub", "denomPubHash", "maxAge", "status"], + { + versionAdded: 31, + }, + ), byExchangeDenomPubHashAndAgeAndStatus: describeIndex( "byExchangeDenomPubHashAndAgeAndStatus", ["exchangeBaseUrl", "denomPubHash", "maxAge", "status"], @@ -805,6 +835,44 @@ export const WalletIndexedDbStoresV1 = { describeContents<ConfigRecord>({ keyPath: "key" }), {}, ), + // Keyed by the master public key that signed the denomination, not by the + // exchange's URL: the URL is where the exchange currently answers and can + // change, while the key is what decides whether a coin can be settled. A + // new store rather than a re-keyed one because the IndexedDB upgrade path + // can only add stores and indices, never change a keyPath. + denominationsV2: describeStore( + "denominationsV2", + describeContents<WalletDenomination>({ + keyPath: ["exchangeMasterPub", "denomPubHash"], + versionAdded: 31, + }), + { + byExchangeBaseUrl: describeIndex("byExchangeBaseUrl", "exchangeBaseUrl", { + versionAdded: 31, + }), + byExchangeMasterPub: describeIndex( + "byExchangeMasterPub", + "exchangeMasterPub", + { + versionAdded: 31, + }, + ), + byVerificationStatus: describeIndex( + "byVerificationStatus", + "verificationStatus", + { + versionAdded: 31, + }, + ), + byDenominationFamilySerialAndStampExpireWithdraw: describeIndex( + "byDenominationFamilySerialAndStampExpireWithdraw", + ["denominationFamilySerial", "stampExpireWithdraw"], + { + versionAdded: 31, + }, + ), + }, + ), denominations: describeStore( "denominations", describeContents<WalletDenomination>({ @@ -886,6 +954,11 @@ export const WalletIndexedDbStoresV1 = { unique: true, }, ), + // Not unique: the same exchange can be known under two base URLs + // while a migration between them is still in progress. + byMasterPublicKey: describeIndex("byMasterPublicKey", "masterPublicKey", { + versionAdded: 30, + }), }, ), exchangeSignKeys: describeStore( @@ -1533,8 +1606,160 @@ export const walletDbFixups: FixupDescription[] = [ fn: fixup20260718StatusEnumDigits, name: "fixup20260718StatusEnumDigits", }, + // Denominations move to a store keyed by the master public key that signed + // them. Runs after the family migration, which assigns the family serial + // the copied rows carry. + { + fn: fixup20260807DenominationsByMasterPub, + name: "fixup20260807DenominationsByMasterPub", + }, + // Coin availability moves to the same key as the denominations it counts. + { + fn: fixup20260807CoinAvailabilityByMasterPub, + name: "fixup20260807CoinAvailabilityByMasterPub", + }, + // Coins record the key that signed their denomination, so that they are + // tied to the keys that can settle them rather than to the URL the + // exchange happens to answer on. + { + fn: fixup20260807CoinExchangeMasterPub, + name: "fixup20260807CoinExchangeMasterPub", + }, ]; +/** + * Copy coin availability into the store keyed by master public key. + * + * The key comes from the row itself where it was recorded, and otherwise from + * the denomination it counts. A row that resolves to neither is left behind + * rather than filed under a guess: it would misreport what is spendable. + */ +async function fixup20260807CoinAvailabilityByMasterPub( + tx: WalletIndexedDbTransaction, +): Promise<void> { + const batchSize = 500; + let range: IDBKeyRange | undefined = undefined; + while (1) { + const batch = await tx.coinAvailability.getAll(range, batchSize); + if (batch.length === 0) { + break; + } + const last = batch[batch.length - 1]; + range = GlobalIDB.KeyRange.lowerBound( + [last.exchangeBaseUrl, last.denomPubHash, last.maxAge], + true, + ); + for (const av of batch) { + let masterPub: string | undefined = av.exchangeMasterPub; + if (!masterPub) { + const denom = await tx.denominations.get([ + av.exchangeBaseUrl, + av.denomPubHash, + ]); + masterPub = denom?.exchangeMasterPub; + } + if (!masterPub) { + logger.warn( + `coin availability for ${av.denomPubHash} has no master public key, not copying`, + ); + continue; + } + const existing = await tx.coinAvailabilityV2.get([ + masterPub, + av.denomPubHash, + av.maxAge, + ]); + if (existing) { + continue; + } + await tx.coinAvailabilityV2.put({ ...av, exchangeMasterPub: masterPub }); + } + } +} + +/** + * Copy denominations into the store keyed by master public key. + * + * The old store is left populated: it is the only source for this copy, so + * clearing it would make the fixup unrepeatable, and a fixup can abort and be + * retried on the next open. Two base URLs that served the same key set + * collapse onto one row here, which is the point -- they were never two + * denominations. + */ +async function fixup20260807DenominationsByMasterPub( + tx: WalletIndexedDbTransaction, +): Promise<void> { + const batchSize = 500; + let range: IDBKeyRange | undefined = undefined; + while (1) { + const batch = await tx.denominations.getAll(range, batchSize); + if (batch.length === 0) { + break; + } + const last = batch[batch.length - 1]; + range = GlobalIDB.KeyRange.lowerBound( + [last.exchangeBaseUrl, last.denomPubHash], + true, + ); + for (const denom of batch) { + if (!denom.exchangeMasterPub) { + logger.warn( + `denomination ${denom.denomPubHash} has no master public key, not copying`, + ); + continue; + } + const existing = await tx.denominationsV2.get([ + denom.exchangeMasterPub, + denom.denomPubHash, + ]); + if (existing) { + continue; + } + await tx.denominationsV2.put(denom); + } + } +} + +/** + * Backfill {@link WalletCoin.exchangeMasterPub} from the coin's denomination. + * + * The denomination has carried the master public key all along, so nothing + * has to be guessed. A coin whose denomination is gone is left alone rather + * than deleted: a fixup must never destroy coins, and an empty key reads as + * "not known" everywhere it is used. + */ +async function fixup20260807CoinExchangeMasterPub( + tx: WalletIndexedDbTransaction, +): Promise<void> { + const batchSize = 500; + let range: IDBKeyRange | undefined = undefined; + while (1) { + const batch = await tx.coins.getAll(range, batchSize); + if (batch.length === 0) { + break; + } + const last = batch[batch.length - 1]; + range = GlobalIDB.KeyRange.lowerBound(last.coinPub, true); + for (const coin of batch) { + if (coin.exchangeMasterPub) { + continue; + } + const denom = await tx.denominations.get([ + coin.exchangeBaseUrl, + coin.denomPubHash, + ]); + if (!denom) { + logger.warn( + `coin ${coin.coinPub} has no denomination, leaving its master public key unset`, + ); + continue; + } + coin.exchangeMasterPub = denom.exchangeMasterPub; + await tx.coins.put(coin); + } + } +} + async function fixup20260718StatusEnumDigits( tx: WalletIndexedDbTransaction, ): Promise<void> { diff --git a/packages/taler-wallet-core/src/db-sqlite-schema.ts b/packages/taler-wallet-core/src/db-sqlite-schema.ts @@ -189,6 +189,7 @@ export const BLOB_COLUMNS: Readonly<Record<string, readonly string[]>> = { "coin_priv", "coin_pub", "denom_pub_hash", + "exchange_master_pub", ], denomination_families: ["exchange_master_pub"], denominations: ["denom_pub_hash", "exchange_master_pub", "master_sig"], @@ -443,8 +444,15 @@ CREATE TABLE IF NOT EXISTS denominations ( is_lost INTEGER CHECK (is_lost IN (0, 1)), master_sig BLOB NOT NULL, verification_status INTEGER NOT NULL, - PRIMARY KEY (exchange_base_url, denom_pub_hash) + -- Keyed by the master public key that signed the denomination, not by the + -- exchange's URL: the URL is where the exchange currently answers and can + -- change, while the key is what decides whether a coin can be settled. + PRIMARY KEY (exchange_master_pub, denom_pub_hash) ); +-- Only for the queries that mean every key set a URL has served; the +-- denomination's identity is the key. +CREATE INDEX IF NOT EXISTS denominations_by_exchange_base_url + ON denominations (exchange_base_url); CREATE INDEX IF NOT EXISTS denominations_by_verification_status ON denominations (verification_status); -- Serves findDenominationByFamilyFromExpiry. denom_pub_hash is part of the @@ -977,6 +985,13 @@ CREATE TABLE IF NOT EXISTS exchanges ( peer_payments_disabled INTEGER CHECK (peer_payments_disabled IN (0, 1)), direct_deposit_disabled INTEGER CHECK (direct_deposit_disabled IN (0, 1)), no_fees INTEGER CHECK (no_fees IN (0, 1)), + -- Key set this exchange used before it changed keys, kept until the user + -- confirms the change. Flattened like details_pointer. + superseded_master_pub BLOB, + superseded_currency TEXT, + superseded_first_seen INTEGER, + superseded_shares_denoms INTEGER + CHECK (superseded_shares_denoms IN (0, 1)), -- The three details_pointer columns are one value. The mapper checks only -- the master pub and then reads the other two unguarded, so a partially -- set pointer would yield null typed as string. @@ -984,6 +999,10 @@ CREATE TABLE IF NOT EXISTS exchanges ( (details_pointer_master_pub IS NULL) = (details_pointer_currency IS NULL) AND (details_pointer_master_pub IS NULL) = (details_pointer_update_clock IS NULL) + ), + CHECK ( + (superseded_master_pub IS NULL) = (superseded_currency IS NULL) + AND (superseded_master_pub IS NULL) = (superseded_first_seen IS NULL) ) ); @@ -1017,6 +1036,10 @@ CREATE TABLE IF NOT EXISTS exchange_details ( -- The pointer identifies at most one details row. CREATE UNIQUE INDEX IF NOT EXISTS exchange_details_by_pointer ON exchange_details (exchange_base_url, currency, master_public_key); +-- Not unique: the same exchange can be known under two base URLs while a +-- migration between them is still in progress. +CREATE INDEX IF NOT EXISTS exchange_details_by_master_pub + ON exchange_details (master_public_key); CREATE TABLE IF NOT EXISTS exchange_sign_keys ( exchange_details_row_id INTEGER NOT NULL @@ -1153,6 +1176,9 @@ CREATE TABLE IF NOT EXISTS coins ( coin_pub BLOB PRIMARY KEY, coin_priv BLOB NOT NULL, exchange_base_url TEXT NOT NULL, + -- Nullable: a coin whose denomination was already gone when the field was + -- introduced has no key recorded, and the mapper reads that as unknown. + exchange_master_pub BLOB, denom_pub_hash BLOB NOT NULL, -- JSON: UnblindedDenominationSignature denom_sig TEXT NOT NULL, @@ -1179,8 +1205,8 @@ CREATE INDEX IF NOT EXISTS coins_by_coin_ev_hash CREATE INDEX IF NOT EXISTS coins_by_source_transaction_id ON coins (source_transaction_id); -- Serves getFreshCoinsByDenomAndAge, which looks up an exact four-tuple. -CREATE INDEX IF NOT EXISTS coins_by_exchange_denom_age_status - ON coins (exchange_base_url, denom_pub_hash, max_age, status); +CREATE INDEX IF NOT EXISTS coins_by_master_pub_denom_age_status + ON coins (exchange_master_pub, denom_pub_hash, max_age, status); CREATE TABLE IF NOT EXISTS coin_availability ( exchange_base_url TEXT NOT NULL, @@ -1188,7 +1214,7 @@ CREATE TABLE IF NOT EXISTS coin_availability ( max_age INTEGER NOT NULL, currency TEXT NOT NULL, value TEXT NOT NULL, - exchange_master_pub BLOB, + exchange_master_pub BLOB NOT NULL, -- Counts, not flags. A negative value means a decrement ran without a -- matching increment, which is a bug worth failing on rather than -- storing: the coin selector reads these to decide what is spendable. @@ -1196,7 +1222,7 @@ CREATE TABLE IF NOT EXISTS coin_availability ( visible_coin_count INTEGER NOT NULL CHECK (visible_coin_count >= 0), pending_refresh_output_count INTEGER CHECK (pending_refresh_output_count >= 0), - PRIMARY KEY (exchange_base_url, denom_pub_hash, max_age) + PRIMARY KEY (exchange_master_pub, denom_pub_hash, max_age) ); -- Column order matches the IndexedDB byExchangeAgeAvailability index, because -- getCoinAvailabilityByExchangeAndAgeRange depends on the *tuple* ordering @@ -1258,8 +1284,15 @@ CREATE UNIQUE INDEX IF NOT EXISTS refund_items_by_coin_and_rtxid /** * Migrations applied on top of the baseline. * - * Empty for now: a fresh native database starts at the baseline. Every later - * change to the schema appends an entry here and bumps - * {@link SQLITE_SCHEMA_VERSION}. + * Empty: no native database exists yet that has to survive a schema change, + * so the baseline is still edited directly. + * + * That stops being true the moment one does. The baseline is all + * CREATE ... IF NOT EXISTS and is re-executed on every open, so an existing + * table keeps the definition it was created with, and a column added only to + * the baseline would be missing from every database created before the edit. + * From then on, every change appends an entry here and bumps + * {@link SQLITE_SCHEMA_VERSION} -- and goes in one place only, since a fresh + * database runs the baseline *and* the migrations. */ export const schemaMigrations: SchemaMigration[] = []; diff --git a/packages/taler-wallet-core/src/dbless.ts b/packages/taler-wallet-core/src/dbless.ts @@ -470,10 +470,8 @@ export async function createTestingReserve(args: { const components = pt.fullPath.split("/"); const creditorAcct = components[components.length - 1]; const wireGatewayClient = new TalerWireGatewayHttpClient( - new URL( - `accounts/${creditorAcct}/taler-wire-gateway/`, - corebankApiBaseUrl, - ).href, + new URL(`accounts/${creditorAcct}/taler-wire-gateway/`, corebankApiBaseUrl) + .href, { httpClient: http }, ); succeedOrThrow( diff --git a/packages/taler-wallet-core/src/dbtx-bench.ts b/packages/taler-wallet-core/src/dbtx-bench.ts @@ -172,6 +172,7 @@ async function populate( await tx.upsertDenomination(denom); const avail: WalletCoinAvailability = { exchangeBaseUrl: ex, + exchangeMasterPub: key(`master-${d % opts.numExchanges}`), denomPubHash: dph, maxAge: d % 2 === 0 ? 0 : 21, currency: "TESTKUDOS", @@ -193,6 +194,7 @@ async function populate( coinPub: key(`coin-${i}`), coinPriv: key(`coinpriv-${i}`), exchangeBaseUrl: exchangeUrl(d % opts.numExchanges), + exchangeMasterPub: key(`mpk-${d % opts.numExchanges}`), denomPubHash: hash(`denom-${d}`), denomSig: { cipher: DenomKeyType.Rsa, rsa_signature: `sig-${i}` }, blindingKey: key(`bk-${i}`), @@ -272,7 +274,9 @@ async function measure( ); await time("countCoinsByExchange", async () => - runner.runReadWriteTx(async (tx) => tx.countCoinsByExchange(exchangeUrl(0))), + runner.runReadWriteTx(async (tx) => + tx.countCoinsByExchange(exchangeUrl(0)), + ), ); await time("getCoinsByDenomPubHash", async () => @@ -287,9 +291,11 @@ async function measure( async (tx) => ( await tx.getFreshCoinsByDenomAndAge( - exchangeUrl(0), - hash("denom-0"), - 0, + { + exchangeMasterPub: key("master-0"), + denomPubHash: hash("denom-0"), + maxAge: 0, + }, 10, ) ).length, @@ -322,10 +328,10 @@ async function measure( }), ); - await time("getDenominationsByExchange", async () => + await time("getDenominationsByMasterPub", async () => runner.runReadWriteTx( async (tx) => - (await tx.getDenominationsByExchange(exchangeUrl(0))).length, + (await tx.getDenominationsByMasterPub(key("master-0"))).length, ), ); @@ -335,7 +341,9 @@ async function measure( ); await time("getCoinAvailabilities (full scan)", async () => - runner.runReadWriteTx(async (tx) => (await tx.getCoinAvailabilities()).length), + runner.runReadWriteTx( + async (tx) => (await tx.getCoinAvailabilities()).length, + ), ); // A write-heavy transaction, to keep an eye on commit cost. diff --git a/packages/taler-wallet-core/src/dbtx-cache-invalidation.test.ts b/packages/taler-wallet-core/src/dbtx-cache-invalidation.test.ts @@ -48,7 +48,9 @@ import { const CACHE_BACKING_STORES = [ "exchanges", "exchangeDetails", - "denominations", + // The live store; "denominations" is the pre-re-key one, written only by + // the fixup that copies out of it. + "denominationsV2", "globalCurrencyAuditors", "globalCurrencyExchanges", ]; diff --git a/packages/taler-wallet-core/src/dbtx-conformance-cases.ts b/packages/taler-wallet-core/src/dbtx-conformance-cases.ts @@ -210,6 +210,7 @@ function makeCoin(coinPub: string): WalletCoin { coinPub: ck(coinPub), coinPriv: ck(`priv-${coinPub}`), exchangeBaseUrl: "https://exchange.test/", + exchangeMasterPub: ck("master-pub"), denomPubHash: ckh("dph-default"), denomSig: { cipher: DenomKeyType.Rsa, @@ -237,6 +238,7 @@ function makeAvail( ): WalletCoinAvailability { const rec: WalletCoinAvailability = { exchangeBaseUrl, + exchangeMasterPub: ck("master-pub"), denomPubHash: ckh(denomPubHash), maxAge, currency: "TESTKUDOS", @@ -769,21 +771,37 @@ export const conformanceCases: ConformanceCase[] = [ // ------------------------------------------------- compound / array keys { - name: "denomination: compound primary key (exchange, denomPubHash)", + name: "denomination: compound primary key (masterPub, denomPubHash)", async run(t, runner) { await runner.runReadWriteTx(async (tx) => { await seedDenomFamily(tx, "https://fam1/", 1); - await tx.upsertDenomination(makeDenomination("https://e1/", "dph-a")); - await tx.upsertDenomination(makeDenomination("https://e2/", "dph-a")); + // One key set reached through two URLs is one denomination, not two: + // the URL is where the exchange answers, not what signed the coin. + const viaOneUrl = makeDenomination("https://e1/", "dph-a"); + const viaAnother = makeDenomination("https://e2/", "dph-a"); + await tx.upsertDenomination(viaOneUrl); + await tx.upsertDenomination(viaAnother); + // A different key signing the same hash *is* a second denomination. + const otherKey = makeDenomination("https://e1/", "dph-a"); + otherKey.exchangeMasterPub = ck("master-other"); + await tx.upsertDenomination(otherKey); }); - const [d1, d2] = await runner.runReadWriteTx(async (tx) => [ - await tx.getDenomination("https://e1/", ckh("dph-a")), - await tx.getDenomination("https://e2/", ckh("dph-a")), + const [shared, other] = await runner.runReadWriteTx(async (tx) => [ + await tx.getDenominationsByMasterPub(ck("master-pub")), + await tx.getDenominationsByMasterPub(ck("master-other")), ]); - t.ok(d1, "same hash under a different exchange must be a distinct row"); - t.ok(d2); - t.equal(d1?.exchangeBaseUrl, "https://e1/"); - t.equal(d2?.exchangeBaseUrl, "https://e2/"); + t.equal( + shared.length, + 1, + "two URLs serving one key set must collapse onto one row", + ); + t.equal(shared[0].denomPubHash, ckh("dph-a")); + t.equal( + other.length, + 1, + "the same hash under another key must be its own row", + ); + t.equal(other[0].exchangeMasterPub, ck("master-other")); }, }, @@ -1104,7 +1122,10 @@ export const conformanceCases: ConformanceCase[] = [ async run(t, runner) { await runner.runReadWriteTx(async (tx) => { await tx.deletePurchase("no-such-proposal"); - await tx.deleteDenomination("https://nope/", ckh("no-such-hash")); + await tx.deleteDenomination({ + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("no-such-hash"), + }); await tx.deleteRefundGroup("no-such-group"); }); t.ok(true, "deleting a missing row must not throw"); @@ -1146,7 +1167,7 @@ export const conformanceCases: ConformanceCase[] = [ ); }); const all = await runner.runReadWriteTx((tx) => - tx.getDenominationsByExchange("https://e-up/"), + tx.getDenominationsByMasterPub(ck("master-pub")), ); t.equal(all.length, 1, "the second upsert must replace, not append"); t.equal(all[0].stampExpireWithdraw, ts(222)); @@ -1449,22 +1470,14 @@ export const conformanceCases: ConformanceCase[] = [ ); }); await runner.runReadWriteTx((tx) => tx.deleteDenominationFamily(41)); - t.equal( - ( - await runner.runReadWriteTx((tx) => - tx.getDenominationsByExchange("https://fam-a/"), - ) - ).length, - 0, + const left = await runner.runReadWriteTx((tx) => + tx.getDenominationsByMasterPub(ck("master-pub")), ); + t.equal(left.length, 1, "denominations of another family must survive"); t.equal( - ( - await runner.runReadWriteTx((tx) => - tx.getDenominationsByExchange("https://fam-b/"), - ) - ).length, - 1, - "denominations of another family must survive", + left[0].denomPubHash, + ckh("dfb-1"), + "the deleted family's denomination must be the one that went", ); }, }, @@ -1883,11 +1896,25 @@ export const conformanceCases: ConformanceCase[] = [ } }); const all = await runner.runReadWriteTx((tx) => - tx.getFreshCoinsByDenomAndAge("https://ex-f/", ckh("df-1"), 21, 100), + tx.getFreshCoinsByDenomAndAge( + { + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("df-1"), + maxAge: 21, + }, + 100, + ), ); t.equal(all.length, 3, "the dormant coin must be excluded"); const limited = await runner.runReadWriteTx((tx) => - tx.getFreshCoinsByDenomAndAge("https://ex-f/", ckh("df-1"), 21, 2), + tx.getFreshCoinsByDenomAndAge( + { + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("df-1"), + maxAge: 21, + }, + 2, + ), ); t.equal(limited.length, 2, "the limit must be applied"); }, @@ -1977,10 +2004,18 @@ export const conformanceCases: ConformanceCase[] = [ await tx.upsertCoinAvailability(makeAvail("https://ea/", "da", 21)); }); const a = await runner.runReadWriteTx((tx) => - tx.getCoinAvailability("https://ea/", ckh("da"), 0), + tx.getCoinAvailability({ + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("da"), + maxAge: 0, + }), ); const b = await runner.runReadWriteTx((tx) => - tx.getCoinAvailability("https://ea/", ckh("da"), 21), + tx.getCoinAvailability({ + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("da"), + maxAge: 21, + }), ); t.ok(a && b, "differing maxAge must be distinct rows"); t.equal(a?.maxAge, 0); @@ -1998,7 +2033,11 @@ export const conformanceCases: ConformanceCase[] = [ rec.pendingRefreshOutputCount = 2; await runner.runReadWriteTx((tx) => tx.upsertCoinAvailability(rec)); const got = await runner.runReadWriteTx((tx) => - tx.getCoinAvailability("https://eu/", ckh("du"), 0), + tx.getCoinAvailability({ + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("du"), + maxAge: 0, + }), ); t.equal(got?.freshCoinCount, 9); t.equal(got?.visibleCoinCount, 4); @@ -2051,7 +2090,11 @@ export const conformanceCases: ConformanceCase[] = [ await tx.upsertCoinAvailability(makeAvail("https://ed/", "dd", 21)); }); await runner.runReadWriteTx((tx) => - tx.deleteCoinAvailability("https://ed/", ckh("dd"), 0), + tx.deleteCoinAvailability({ + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("dd"), + maxAge: 0, + }), ); const left = await runner.runReadWriteTx((tx) => tx.getCoinAvailabilityByExchange("https://ed/"), @@ -2144,6 +2187,70 @@ export const conformanceCases: ConformanceCase[] = [ // ----------------------------------------------------- exchange details { + name: "exchange: a superseded key set round trips", + async run(t, runner) { + const ex = makeExchange("https://superseded/"); + ex.detailsPointer = { + masterPublicKey: ck("mpk-current"), + currency: "TESTKUDOS", + updateClock: tsPrecise(1), + }; + ex.supersededKeySet = { + masterPublicKey: ck("mpk-old"), + currency: "TESTKUDOS", + firstSeen: tsPrecise(3), + sharesDenominations: false, + }; + await runner.runReadWriteTx((tx) => tx.upsertExchange(ex)); + const got = await runner.runReadWriteTx((tx) => + tx.getExchange("https://superseded/"), + ); + t.deepEqual(got?.detailsPointer, ex.detailsPointer); + t.deepEqual(got?.supersededKeySet, ex.supersededKeySet); + t.equal( + got?.supersededKeySet?.sharesDenominations, + false, + "an explicit false must not become undefined", + ); + }, + }, + + { + name: "exchange: no superseded key set stays absent", + async run(t, runner) { + await runner.runReadWriteTx((tx) => + tx.upsertExchange(makeExchange("https://no-superseded/")), + ); + const got = await runner.runReadWriteTx((tx) => + tx.getExchange("https://no-superseded/"), + ); + t.equal(got?.supersededKeySet, undefined); + }, + }, + + { + name: "exchange: clearing a superseded key set persists", + async run(t, runner) { + const ex = makeExchange("https://confirmed/"); + ex.supersededKeySet = { + masterPublicKey: ck("mpk-gone"), + currency: "TESTKUDOS", + firstSeen: tsPrecise(5), + sharesDenominations: true, + }; + await runner.runReadWriteTx((tx) => tx.upsertExchange(ex)); + // Confirming the change clears it; the columns must go back to NULL + // rather than keeping the previous value. + delete ex.supersededKeySet; + await runner.runReadWriteTx((tx) => tx.upsertExchange(ex)); + const got = await runner.runReadWriteTx((tx) => + tx.getExchange("https://confirmed/"), + ); + t.equal(got?.supersededKeySet, undefined); + }, + }, + + { name: "exchange details: upsert returns a row id and round trips", async run(t, runner) { const det = makeExchangeDetails("https://ed-1/", "mpk-a"); @@ -2235,6 +2342,66 @@ export const conformanceCases: ConformanceCase[] = [ }, }, + { + name: "exchange details: one master public key, two base URLs", + async run(t, runner) { + await runner.runReadWriteTx(async (tx) => { + await tx.upsertExchangeDetails( + makeExchangeDetails("https://mp-a/", "mpk-shared"), + ); + await tx.upsertExchangeDetails( + makeExchangeDetails("https://mp-b/", "mpk-shared"), + ); + }); + const got = await runner.runReadWriteTx((tx) => + tx.listExchangeDetailsByMasterPub(ck("mpk-shared")), + ); + t.equal(got.length, 2, "a master public key may span base URLs"); + t.deepEqual(got.map((d) => d.exchangeBaseUrl).sort(), [ + "https://mp-a/", + "https://mp-b/", + ]); + }, + }, + + { + name: "exchange details: one base URL, two master public keys", + async run(t, runner) { + await runner.runReadWriteTx(async (tx) => { + await tx.upsertExchangeDetails( + makeExchangeDetails("https://mp-two/", "mpk-old"), + ); + await tx.upsertExchangeDetails( + makeExchangeDetails("https://mp-two/", "mpk-new"), + ); + }); + const old = await runner.runReadWriteTx((tx) => + tx.listExchangeDetailsByMasterPub(ck("mpk-old")), + ); + const fresh = await runner.runReadWriteTx((tx) => + tx.listExchangeDetailsByMasterPub(ck("mpk-new")), + ); + // Superseded keys keep their own row: the coins withdrawn under them + // are only interpretable through it. + t.equal(old.length, 1); + t.equal(fresh.length, 1); + t.ok( + old[0].rowId !== fresh[0].rowId, + "each key set must keep its own details row", + ); + }, + }, + + { + name: "exchange details: unknown master public key yields nothing", + async run(t, runner) { + const got = await runner.runReadWriteTx((tx) => + tx.listExchangeDetailsByMasterPub(ck("mpk-never")), + ); + t.equal(got.length, 0); + }, + }, + // --------------------------------------------------- exchange sign keys { @@ -2418,18 +2585,24 @@ export const conformanceCases: ConformanceCase[] = [ }, { - name: "denominations: listed by exchange", + name: "denominations: listed by master public key", async run(t, runner) { await runner.runReadWriteTx(async (tx) => { await seedDenomFamily(tx, "https://fam1/", 1); await tx.upsertDenomination(makeDenomination("https://dl1/", "d-1")); await tx.upsertDenomination(makeDenomination("https://dl1/", "d-2")); - await tx.upsertDenomination(makeDenomination("https://dl2/", "d-3")); + const other = makeDenomination("https://dl2/", "d-3"); + other.exchangeMasterPub = ck("master-other"); + await tx.upsertDenomination(other); }); const got = await runner.runReadWriteTx((tx) => - tx.getDenominationsByExchange("https://dl1/"), + tx.getDenominationsByMasterPub(ck("master-pub")), ); - t.equal(got.length, 2); + t.equal(got.length, 2, "only the denominations of that key set"); + const other = await runner.runReadWriteTx((tx) => + tx.getDenominationsByMasterPub(ck("master-other")), + ); + t.equal(other.length, 1); }, }, // -------------------------------------------------- withdrawal groups @@ -3558,27 +3731,39 @@ export const conformanceCases: ConformanceCase[] = [ }, }, { - name: "coin availability: exchange master pub round trips", + name: "coin availability: the master public key is part of the identity", async run(t, runner) { - // Exercises a column that no other case populates; the sqlite - // storage-class test can only verify columns that have rows. - const rec = makeAvail("https://emp/", "d-emp", 0); - rec.exchangeMasterPub = ck("master-emp"); - await runner.runReadWriteTx((tx) => tx.upsertCoinAvailability(rec)); - const got = await runner.runReadWriteTx((tx) => - tx.getCoinAvailability("https://emp/", ckh("d-emp"), 0), - ); - t.equal(got?.exchangeMasterPub, ck("master-emp")); - const unset = makeAvail("https://emp2/", "d-emp2", 0); - await runner.runReadWriteTx((tx) => tx.upsertCoinAvailability(unset)); - const got2 = await runner.runReadWriteTx((tx) => - tx.getCoinAvailability("https://emp2/", ckh("d-emp2"), 0), + // The same denomination hash under two master public keys is two + // different denominations, so two different availability rows. Sharing + // one would pool coins the exchange settles with coins it does not. + const a = makeAvail("https://emp/", "d-emp", 0); + a.exchangeMasterPub = ck("master-a"); + a.freshCoinCount = 3; + const b = makeAvail("https://emp/", "d-emp", 0); + b.exchangeMasterPub = ck("master-b"); + b.freshCoinCount = 7; + await runner.runReadWriteTx(async (tx) => { + await tx.upsertCoinAvailability(a); + await tx.upsertCoinAvailability(b); + }); + const gotA = await runner.runReadWriteTx((tx) => + tx.getCoinAvailability({ + exchangeMasterPub: ck("master-a"), + denomPubHash: ckh("d-emp"), + maxAge: 0, + }), ); - t.equal( - got2?.exchangeMasterPub, - undefined, - "an unset optional key must stay unset", + const gotB = await runner.runReadWriteTx((tx) => + tx.getCoinAvailability({ + exchangeMasterPub: ck("master-b"), + denomPubHash: ckh("d-emp"), + maxAge: 0, + }), ); + t.equal(gotA?.exchangeMasterPub, ck("master-a")); + t.equal(gotB?.exchangeMasterPub, ck("master-b")); + t.equal(gotA?.freshCoinCount, 3); + t.equal(gotB?.freshCoinCount, 7); }, }, { @@ -3628,7 +3813,14 @@ export const conformanceCases: ConformanceCase[] = [ tx.getCoin(ck("scan-7")), ); await measure("getFreshCoinsByDenomAndAge with limit 5", 8, (tx) => - tx.getFreshCoinsByDenomAndAge("https://scan/", ckh("scan-denom"), 0, 5), + tx.getFreshCoinsByDenomAndAge( + { + exchangeMasterPub: ck("master-pub"), + denomPubHash: ckh("scan-denom"), + maxAge: 0, + }, + 5, + ), ); await measure("listTransactionMetaByTimestamp with limit 5", 8, (tx) => tx.listTransactionMetaByTimestamp({ limit: 5 }), diff --git a/packages/taler-wallet-core/src/dbtx-indexeddb.ts b/packages/taler-wallet-core/src/dbtx-indexeddb.ts @@ -93,7 +93,9 @@ import type { WalletDbRecordCounts, GetCurrencyInfoDbResult, StoreCurrencyInfoDbRequest, + WalletCoinAvailabilityRef, WalletDbTransaction, + WalletDenomRef, } from "./dbtx.js"; function getActiveKeyRange() { @@ -484,14 +486,12 @@ export class IdbWalletTransaction implements WalletDbTransaction { } async getFreshCoinsByDenomAndAge( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, + ref: WalletCoinAvailabilityRef, limit: number, ): Promise<WalletCoin[]> { const tx = this.tx; - return await tx.coins.indexes.byExchangeDenomPubHashAndAgeAndStatus.getAll( - [exchangeBaseUrl, denomPubHash, maxAge, CoinStatus.Fresh], + return await tx.coins.indexes.byMasterPubDenomPubHashAndAgeAndStatus.getAll( + [ref.exchangeMasterPub, ref.denomPubHash, ref.maxAge, CoinStatus.Fresh], limit, ); } @@ -504,7 +504,7 @@ export class IdbWalletTransaction implements WalletDbTransaction { const tx = this.tx; // Lower bound of 1 on freshCoinCount: only denominations that actually // have a fresh coin available. - return await tx.coinAvailability.indexes.byExchangeAgeAvailability.getAll( + return await tx.coinAvailabilityV2.indexes.byExchangeAgeAvailability.getAll( GlobalIDB.KeyRange.bound( [exchangeBaseUrl, ageLower, 1], [exchangeBaseUrl, ageUpper, Number.MAX_SAFE_INTEGER], @@ -540,8 +540,8 @@ export class IdbWalletTransaction implements WalletDbTransaction { const tx = this.tx; return { coins: await tx.coins.count(), - coinAvailability: await tx.coinAvailability.count(), - denominations: await tx.denominations.count(), + coinAvailability: await tx.coinAvailabilityV2.count(), + denominations: await tx.denominationsV2.count(), denominationFamilies: await tx.denominationFamilies.count(), exchanges: await tx.exchanges.count(), exchangeDetails: await tx.exchangeDetails.count(), @@ -586,18 +586,18 @@ export class IdbWalletTransaction implements WalletDbTransaction { exchangeBaseUrl: string, ): Promise<WalletCoinAvailability[]> { const tx = this.tx; - return await tx.coinAvailability.indexes.byExchangeBaseUrl.getAll( + return await tx.coinAvailabilityV2.indexes.byExchangeBaseUrl.getAll( exchangeBaseUrl, ); } - async deleteCoinAvailability( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, - ): Promise<void> { + async deleteCoinAvailability(ref: WalletCoinAvailabilityRef): Promise<void> { const tx = this.tx; - await tx.coinAvailability.delete([exchangeBaseUrl, denomPubHash, maxAge]); + await tx.coinAvailabilityV2.delete([ + ref.exchangeMasterPub, + ref.denomPubHash, + ref.maxAge, + ]); } async getRecoupGroupsByExchange( @@ -698,12 +698,12 @@ export class IdbWalletTransaction implements WalletDbTransaction { // "denominations by family" on its own, so this walks the index whose // first component is the family serial. const all = - await tx.denominations.indexes.byDenominationFamilySerialAndStampExpireWithdraw.getAll(); + await tx.denominationsV2.indexes.byDenominationFamilySerialAndStampExpireWithdraw.getAll(); const doomed = all.filter( (d) => d.denominationFamilySerial === denominationFamilySerial, ); for (const d of doomed) { - await tx.denominations.delete([d.exchangeBaseUrl, d.denomPubHash]); + await tx.denominationsV2.delete([d.exchangeMasterPub, d.denomPubHash]); } await tx.denominationFamilies.delete(denominationFamilySerial); } @@ -781,6 +781,15 @@ export class IdbWalletTransaction implements WalletDbTransaction { ); } + async listExchangeDetailsByMasterPub( + masterPublicKey: string, + ): Promise<WalletExchangeDetails[]> { + const tx = this.tx; + return await tx.exchangeDetails.indexes.byMasterPublicKey.getAll( + masterPublicKey, + ); + } + async listAllExchangeDetails(): Promise<WalletExchangeDetails[]> { const tx = this.tx; return await tx.exchangeDetails.indexes.byExchangeBaseUrl.getAll(); @@ -1034,7 +1043,7 @@ export class IdbWalletTransaction implements WalletDbTransaction { } async listAllDenominations(): Promise<WalletDenomination[]> { - return await this.tx.denominations.getAll(); + return await this.tx.denominationsV2.getAll(); } async listAllContractTerms(): Promise<WalletContractTerms[]> { @@ -1337,21 +1346,19 @@ export class IdbWalletTransaction implements WalletDbTransaction { } async getCoinAvailability( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, + ref: WalletCoinAvailabilityRef, ): Promise<WalletCoinAvailability | undefined> { const tx = this.tx; - return await tx.coinAvailability.get([ - exchangeBaseUrl, - denomPubHash, - maxAge, + return await tx.coinAvailabilityV2.get([ + ref.exchangeMasterPub, + ref.denomPubHash, + ref.maxAge, ]); } async upsertCoinAvailability(rec: WalletCoinAvailability): Promise<void> { const tx = this.tx; - await tx.coinAvailability.put(rec); + await tx.coinAvailabilityV2.put(rec); } async getCoinHistory( @@ -1671,15 +1678,17 @@ export class IdbWalletTransaction implements WalletDbTransaction { async upsertDenomination(rec: WalletDenomination): Promise<void> { const tx = this.tx; - await tx.denominations.put(rec); + await tx.denominationsV2.put(rec); } async getDenomination( - exchangeBaseUrl: string, - denomPubHash: string, + ref: WalletDenomRef, ): Promise<WalletDenomination | undefined> { const tx = this.tx; - return await tx.denominations.get([exchangeBaseUrl, denomPubHash]); + return await tx.denominationsV2.get([ + ref.exchangeMasterPub, + ref.denomPubHash, + ]); } async findDenominationByFamilyFromExpiry( @@ -1689,7 +1698,7 @@ export class IdbWalletTransaction implements WalletDbTransaction { ): Promise<WalletDenomination | undefined> { const tx = this.tx; const cursor = - tx.denominations.indexes.byDenominationFamilySerialAndStampExpireWithdraw.iter(); + tx.denominationsV2.indexes.byDenominationFamilySerialAndStampExpireWithdraw.iter(); // The cursor has to be positioned before it can be moved. const first = await cursor.current(); if (!first.hasValue) { @@ -1721,28 +1730,25 @@ export class IdbWalletTransaction implements WalletDbTransaction { } } - async getDenominationsByExchange( - exchangeBaseUrl: string, + async getDenominationsByMasterPub( + exchangeMasterPub: string, ): Promise<WalletDenomination[]> { const tx = this.tx; - return await tx.denominations.indexes.byExchangeBaseUrl.getAll( - exchangeBaseUrl, + return await tx.denominationsV2.indexes.byExchangeMasterPub.getAll( + exchangeMasterPub, ); } - async deleteDenomination( - exchangeBaseUrl: string, - denomPubHash: string, - ): Promise<void> { + async deleteDenomination(ref: WalletDenomRef): Promise<void> { const tx = this.tx; - await tx.denominations.delete([exchangeBaseUrl, denomPubHash]); + await tx.denominationsV2.delete([ref.exchangeMasterPub, ref.denomPubHash]); } async getDenominationsByVerificationStatus( verificationStatus: DenominationVerificationStatus, ): Promise<WalletDenomination[]> { const tx = this.tx; - return await tx.denominations.indexes.byVerificationStatus.getAll( + return await tx.denominationsV2.indexes.byVerificationStatus.getAll( verificationStatus, ); } @@ -1756,7 +1762,7 @@ export class IdbWalletTransaction implements WalletDbTransaction { } async getCoinAvailabilities(): Promise<WalletCoinAvailability[]> { - return await this.tx.coinAvailability.getAll(); + return await this.tx.coinAvailabilityV2.getAll(); } async getActiveRefreshGroups(): Promise<WalletRefreshGroup[]> { @@ -1867,6 +1873,10 @@ export class IdbWalletTransaction implements WalletDbTransaction { } case ScopeType.Auditor: throw Error("auditor scope not supported yet"); + case ScopeType.ExchangeLegacyKeys: + // See checkExchangeInScopeGeneric: an entry stands for its current + // key set, which is never a superseded one. + return false; default: assertUnreachable(scope); } diff --git a/packages/taler-wallet-core/src/dbtx-runners.ts b/packages/taler-wallet-core/src/dbtx-runners.ts @@ -46,8 +46,9 @@ export async function makeIdbRunner( backend.trackStats = true; BridgeIDBFactory.enableTracing = false; const idbFactory = new BridgeIDBFactory(backend); - const handle = new IdbWalletDbHandle(idbFactory as any, () => - backend.accessStats, + const handle = new IdbWalletDbHandle( + idbFactory as any, + () => backend.accessStats, ); await handle.ensureOpen(); return handle; diff --git a/packages/taler-wallet-core/src/dbtx-shared.ts b/packages/taler-wallet-core/src/dbtx-shared.ts @@ -51,6 +51,11 @@ export async function checkExchangeInScopeGeneric( } case ScopeType.Auditor: throw Error("auditor scope not supported yet"); + case ScopeType.ExchangeLegacyKeys: + // Asked of an exchange entry, which always stands for the key set it + // currently uses. That is by definition not a superseded one, so the + // answer is no even when the URLs agree. + return false; default: assertUnreachable(scope); } diff --git a/packages/taler-wallet-core/src/dbtx-sqlite.test.ts b/packages/taler-wallet-core/src/dbtx-sqlite.test.ts @@ -152,9 +152,10 @@ test("sqlite: schema constraints reject invalid rows", async () => { // floor, so a negative value is a bug rather than a state to store. assert.match( await run( - "INSERT INTO coin_availability (exchange_base_url, denom_pub_hash," + - " max_age, currency, value, fresh_coin_count, visible_coin_count)" + - " VALUES ('https://e/', x'00', 0, 'C', 'C:1', -1, 0)", + "INSERT INTO coin_availability (exchange_base_url, exchange_master_pub," + + " denom_pub_hash, max_age, currency, value, fresh_coin_count," + + " visible_coin_count)" + + " VALUES ('https://e/', x'01', x'00', 0, 'C', 'C:1', -1, 0)", ), /CHECK constraint failed/, "a negative coin count must be rejected", diff --git a/packages/taler-wallet-core/src/dbtx-sqlite.ts b/packages/taler-wallet-core/src/dbtx-sqlite.ts @@ -55,8 +55,10 @@ import { import { GetCurrencyInfoDbResult, StoreCurrencyInfoDbRequest, + WalletCoinAvailabilityRef, WalletDbRecordCounts, WalletDbTransaction, + WalletDenomRef, WalletCurrencyInfoEntry, } from "./dbtx.js"; import { @@ -747,7 +749,7 @@ export class SqliteWalletTransaction implements WalletDbTransaction { $is_offered, $is_revoked, $is_lost, $master_sig, $verification_status ) - ON CONFLICT(exchange_base_url, denom_pub_hash) DO UPDATE SET + ON CONFLICT(exchange_master_pub, denom_pub_hash) DO UPDATE SET denom_pub = excluded.denom_pub, exchange_master_pub = excluded.exchange_master_pub, currency = excluded.currency, @@ -792,23 +794,25 @@ export class SqliteWalletTransaction implements WalletDbTransaction { } async getDenomination( - exchangeBaseUrl: string, - denomPubHash: string, + ref: WalletDenomRef, ): Promise<WalletDenomination | undefined> { const row = await this.first( "SELECT * FROM denominations" + - " WHERE exchange_base_url = $url AND denom_pub_hash = $hash", - { url: exchangeBaseUrl, hash: crockToDb(denomPubHash) }, + " WHERE exchange_master_pub = $mpk AND denom_pub_hash = $hash", + { + mpk: crockToDb(ref.exchangeMasterPub), + hash: crockToDb(ref.denomPubHash), + }, ); return row ? this.rowToDenomination(row) : undefined; } - async getDenominationsByExchange( - exchangeBaseUrl: string, + async getDenominationsByMasterPub( + exchangeMasterPub: string, ): Promise<WalletDenomination[]> { const rows = await this.all( - "SELECT * FROM denominations WHERE exchange_base_url = $url", - { url: exchangeBaseUrl }, + "SELECT * FROM denominations WHERE exchange_master_pub = $mpk", + { mpk: crockToDb(exchangeMasterPub) }, ); return rows.map((r) => this.rowToDenomination(r)); } @@ -823,14 +827,14 @@ export class SqliteWalletTransaction implements WalletDbTransaction { return rows.map((r) => this.rowToDenomination(r)); } - async deleteDenomination( - exchangeBaseUrl: string, - denomPubHash: string, - ): Promise<void> { + async deleteDenomination(ref: WalletDenomRef): Promise<void> { await this.run( "DELETE FROM denominations" + - " WHERE exchange_base_url = $url AND denom_pub_hash = $hash", - { url: exchangeBaseUrl, hash: crockToDb(denomPubHash) }, + " WHERE exchange_master_pub = $mpk AND denom_pub_hash = $hash", + { + mpk: crockToDb(ref.exchangeMasterPub), + hash: crockToDb(ref.denomPubHash), + }, ); } @@ -1119,6 +1123,13 @@ export class SqliteWalletTransaction implements WalletDbTransaction { coinPub: dbToCrock(row.coin_pub), coinPriv: dbToCrock(row.coin_priv), exchangeBaseUrl: str(row.exchange_base_url), + // Coins written before the column existed and whose denomination had + // already been deleted have no key recorded. Empty rather than absent: + // the field is required on the record. + exchangeMasterPub: + row.exchange_master_pub == null + ? "" + : dbToCrock(row.exchange_master_pub), denomPubHash: dbToCrock(row.denom_pub_hash), denomSig: dbToJson(row.denom_sig), blindingKey: dbToCrock(row.blinding_key), @@ -1146,16 +1157,18 @@ export class SqliteWalletTransaction implements WalletDbTransaction { async upsertCoin(coin: WalletCoin): Promise<void> { await this.run( `INSERT INTO coins ( - coin_pub, coin_priv, exchange_base_url, denom_pub_hash, denom_sig, + coin_pub, coin_priv, exchange_base_url, exchange_master_pub, + denom_pub_hash, denom_sig, blinding_key, coin_ev_hash, status, visible, max_age, age_commitment_proof, coin_source, source_transaction_id ) VALUES ( - $pub, $priv, $url, $dph, $sig, $bk, $ceh, $status, $visible, $age, - $acp, $source, $stid + $pub, $priv, $url, $emp, $dph, $sig, $bk, $ceh, $status, $visible, + $age, $acp, $source, $stid ) ON CONFLICT(coin_pub) DO UPDATE SET coin_priv = excluded.coin_priv, exchange_base_url = excluded.exchange_base_url, + exchange_master_pub = excluded.exchange_master_pub, denom_pub_hash = excluded.denom_pub_hash, denom_sig = excluded.denom_sig, blinding_key = excluded.blinding_key, @@ -1170,6 +1183,7 @@ export class SqliteWalletTransaction implements WalletDbTransaction { pub: crockToDb(coin.coinPub), priv: crockToDb(coin.coinPriv), url: coin.exchangeBaseUrl, + emp: optCrockToDb(coin.exchangeMasterPub) ?? null, dph: crockToDb(coin.denomPubHash), sig: jsonToDb(coin.denomSig), bk: crockToDb(coin.blindingKey), @@ -1270,21 +1284,19 @@ export class SqliteWalletTransaction implements WalletDbTransaction { } async getFreshCoinsByDenomAndAge( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, + ref: WalletCoinAvailabilityRef, limit: number, ): Promise<WalletCoin[]> { const rows = await this.all( "SELECT * FROM coins" + - " WHERE exchange_base_url = $url AND denom_pub_hash = $dph" + + " WHERE exchange_master_pub = $mpk AND denom_pub_hash = $dph" + " AND max_age = $age AND status = $status" + " ORDER BY coin_pub" + " LIMIT $limit", { - url: exchangeBaseUrl, - dph: crockToDb(denomPubHash), - age: maxAge, + mpk: crockToDb(ref.exchangeMasterPub), + dph: crockToDb(ref.denomPubHash), + age: ref.maxAge, status: CoinStatus.Fresh, limit, }, @@ -1341,9 +1353,7 @@ export class SqliteWalletTransaction implements WalletDbTransaction { value: dbAmount(row.value), freshCoinCount: num(row.fresh_coin_count), visibleCoinCount: num(row.visible_coin_count), - ...(row.exchange_master_pub != null - ? { exchangeMasterPub: dbToCrock(row.exchange_master_pub) } - : undefined), + exchangeMasterPub: dbToCrock(row.exchange_master_pub), ...(row.pending_refresh_output_count != null ? { pendingRefreshOutputCount: num(row.pending_refresh_output_count) } : undefined), @@ -1351,15 +1361,17 @@ export class SqliteWalletTransaction implements WalletDbTransaction { } async getCoinAvailability( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, + ref: WalletCoinAvailabilityRef, ): Promise<WalletCoinAvailability | undefined> { const row = await this.first( "SELECT * FROM coin_availability" + - " WHERE exchange_base_url = $url AND denom_pub_hash = $dph" + + " WHERE exchange_master_pub = $mpk AND denom_pub_hash = $dph" + " AND max_age = $age", - { url: exchangeBaseUrl, dph: crockToDb(denomPubHash), age: maxAge }, + { + mpk: crockToDb(ref.exchangeMasterPub), + dph: crockToDb(ref.denomPubHash), + age: ref.maxAge, + }, ); return row ? this.rowToCoinAvailability(row) : undefined; } @@ -1371,7 +1383,7 @@ export class SqliteWalletTransaction implements WalletDbTransaction { exchange_master_pub, fresh_coin_count, visible_coin_count, pending_refresh_output_count ) VALUES ($url, $dph, $age, $cur, $val, $emp, $fresh, $vis, $pend) - ON CONFLICT(exchange_base_url, denom_pub_hash, max_age) DO UPDATE SET + ON CONFLICT(exchange_master_pub, denom_pub_hash, max_age) DO UPDATE SET currency = excluded.currency, value = excluded.value, exchange_master_pub = excluded.exchange_master_pub, @@ -1434,16 +1446,16 @@ export class SqliteWalletTransaction implements WalletDbTransaction { return rows.map((r) => this.rowToCoinAvailability(r)); } - async deleteCoinAvailability( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, - ): Promise<void> { + async deleteCoinAvailability(ref: WalletCoinAvailabilityRef): Promise<void> { await this.run( "DELETE FROM coin_availability" + - " WHERE exchange_base_url = $url AND denom_pub_hash = $dph" + + " WHERE exchange_master_pub = $mpk AND denom_pub_hash = $dph" + " AND max_age = $age", - { url: exchangeBaseUrl, dph: crockToDb(denomPubHash), age: maxAge }, + { + mpk: crockToDb(ref.exchangeMasterPub), + dph: crockToDb(ref.denomPubHash), + age: ref.maxAge, + }, ); } @@ -1508,6 +1520,16 @@ export class SqliteWalletTransaction implements WalletDbTransaction { ? { directDepositDisabled: dbToBool(row.direct_deposit_disabled) } : undefined), ...(row.no_fees != null ? { noFees: dbToBool(row.no_fees) } : undefined), + ...(row.superseded_master_pub != null + ? { + supersededKeySet: { + masterPublicKey: dbToCrock(row.superseded_master_pub), + currency: str(row.superseded_currency), + firstSeen: dbTimestamp(row.superseded_first_seen), + sharesDenominations: dbToBool(row.superseded_shares_denoms), + }, + } + : undefined), }; } @@ -1536,11 +1558,13 @@ export class SqliteWalletTransaction implements WalletDbTransaction { last_keys_etag, next_refresh_check_stamp, current_merge_reserve_row_id, current_account_priv, current_account_pub, peer_payments_disabled, - direct_deposit_disabled, no_fees + direct_deposit_disabled, no_fees, + superseded_master_pub, superseded_currency, + superseded_first_seen, superseded_shares_denoms ) VALUES ( $url, $pch, $pcs, $pt, $lw, $dpmp, $dpc, $dpuc, $es, $us, $ur, $cnu, $tce, $tae, $tat, $lu, $nus, $lke, $nrcs, $cmrri, $cap, - $capub, $ppd, $ddd, $nf + $capub, $ppd, $ddd, $nf, $smp, $sc, $sfs, $ssd ) ON CONFLICT(base_url) DO UPDATE SET preset_currency_hint = excluded.preset_currency_hint, @@ -1568,7 +1592,11 @@ export class SqliteWalletTransaction implements WalletDbTransaction { current_account_pub = excluded.current_account_pub, peer_payments_disabled = excluded.peer_payments_disabled, direct_deposit_disabled = excluded.direct_deposit_disabled, - no_fees = excluded.no_fees`, + no_fees = excluded.no_fees, + superseded_master_pub = excluded.superseded_master_pub, + superseded_currency = excluded.superseded_currency, + superseded_first_seen = excluded.superseded_first_seen, + superseded_shares_denoms = excluded.superseded_shares_denoms`, { url: rec.baseUrl, pch: rec.presetCurrencyHint ?? null, @@ -1581,6 +1609,13 @@ export class SqliteWalletTransaction implements WalletDbTransaction { dpmp: optCrockToDb(rec.detailsPointer?.masterPublicKey), dpc: rec.detailsPointer?.currency ?? null, dpuc: rec.detailsPointer?.updateClock ?? null, + smp: optCrockToDb(rec.supersededKeySet?.masterPublicKey), + sc: rec.supersededKeySet?.currency ?? null, + sfs: rec.supersededKeySet?.firstSeen ?? null, + ssd: + rec.supersededKeySet === undefined + ? null + : boolToDb(rec.supersededKeySet.sharesDenominations), es: rec.entryStatus, us: rec.updateStatus, ur: @@ -1748,6 +1783,16 @@ export class SqliteWalletTransaction implements WalletDbTransaction { return rows.map((r) => this.rowToExchangeDetails(r)); } + async listExchangeDetailsByMasterPub( + masterPublicKey: string, + ): Promise<WalletExchangeDetails[]> { + const rows = await this.all( + "SELECT * FROM exchange_details WHERE master_public_key = $pub", + { pub: crockToDb(masterPublicKey) }, + ); + return rows.map((r) => this.rowToExchangeDetails(r)); + } + async listAllExchangeDetails(): Promise<WalletExchangeDetails[]> { const rows = await this.all("SELECT * FROM exchange_details"); return rows.map((r) => this.rowToExchangeDetails(r)); diff --git a/packages/taler-wallet-core/src/dbtx.ts b/packages/taler-wallet-core/src/dbtx.ts @@ -133,6 +133,31 @@ export interface WalletDbRecordCounts { exchangeSignKeys: number; } +/** + * What identifies one denomination to the wallet. + * + * Passed as an object rather than as positional strings on purpose. The + * exchange base URL and the denomination hash are both plain strings, so + * every signature that took them in a row accepted them in either order and + * accepted any other string besides -- a wrong argument was a runtime bug + * that looked like a lookup miss. Naming the fields makes it a compile + * error, which is what made moving the identifying field from the exchange's + * URL to the key that signed the denomination a mechanical change. + * + * `WalletCoin`, `WalletCoinAvailability` and `WalletDenomination` all satisfy + * this structurally, so a caller that holds one of those records passes it + * directly. + */ +export interface WalletDenomRef { + exchangeMasterPub: string; + denomPubHash: string; +} + +/** A denomination together with an age restriction, keying availability. */ +export interface WalletCoinAvailabilityRef extends WalletDenomRef { + maxAge: number; +} + export interface WalletDbTransaction { /** Get the currency specification for a scope, if one is stored. */ getCurrencyInfo( @@ -372,9 +397,7 @@ export interface WalletDbTransaction { * Get up to `limit` fresh coins of a given denomination and age restriction. */ getFreshCoinsByDenomAndAge( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, + ref: WalletCoinAvailabilityRef, limit: number, ): Promise<WalletCoin[]>; @@ -432,11 +455,7 @@ export interface WalletDbTransaction { ): Promise<WalletCoinAvailability[]>; /** Delete a coin availability record. */ - deleteCoinAvailability( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, - ): Promise<void>; + deleteCoinAvailability(ref: WalletCoinAvailabilityRef): Promise<void>; /** Get the recoup groups against an exchange. */ getRecoupGroupsByExchange( @@ -543,6 +562,16 @@ export interface WalletDbTransaction { exchangeBaseUrl: string, ): Promise<WalletExchangeDetails[]>; + /** + * Get every exchange details record signed by a master public key. + * + * More than one is possible: the same exchange can be known under two base + * URLs while a migration between them is still in progress. + */ + listExchangeDetailsByMasterPub( + masterPublicKey: string, + ): Promise<WalletExchangeDetails[]>; + /** List every exchange details record, for all exchanges. */ listAllExchangeDetails(): Promise<WalletExchangeDetails[]>; @@ -856,9 +885,7 @@ export interface WalletDbTransaction { /** Get the availability record for a denomination and age restriction. */ getCoinAvailability( - exchangeBaseUrl: string, - denomPubHash: string, - maxAge: number, + ref: WalletCoinAvailabilityRef, ): Promise<WalletCoinAvailability | undefined>; /** Create or update a coin availability record. */ @@ -983,11 +1010,8 @@ export interface WalletDbTransaction { /** Create or update a denomination. */ upsertDenomination(rec: WalletDenomination): Promise<void>; - /** Get a denomination by exchange base URL and public key hash. */ - getDenomination( - exchangeBaseUrl: string, - denomPubHash: string, - ): Promise<WalletDenomination | undefined>; + /** Get a denomination by its reference. */ + getDenomination(ref: WalletDenomRef): Promise<WalletDenomination | undefined>; /** * Find the first denomination of a family, scanning in withdraw-expiry order @@ -1003,16 +1027,13 @@ export interface WalletDbTransaction { match: (d: WalletDenomination) => boolean, ): Promise<WalletDenomination | undefined>; - /** Get all denominations offered by an exchange. */ - getDenominationsByExchange( - exchangeBaseUrl: string, + /** Get every denomination signed by a master public key. */ + getDenominationsByMasterPub( + exchangeMasterPub: string, ): Promise<WalletDenomination[]>; - /** Delete a denomination by exchange base URL and public key hash. */ - deleteDenomination( - exchangeBaseUrl: string, - denomPubHash: string, - ): Promise<void>; + /** Delete a denomination by its reference. */ + deleteDenomination(ref: WalletDenomRef): Promise<void>; /** Get denominations awaiting or failing signature verification. */ getDenominationsByVerificationStatus( diff --git a/packages/taler-wallet-core/src/deposits.ts b/packages/taler-wallet-core/src/deposits.ts @@ -2382,12 +2382,7 @@ async function getCounterpartyEffectiveDepositAmount( await wex.runWalletDbTx(async (tx) => { for (let i = 0; i < pcs.length; i++) { - const denom = await getDenomInfo( - wex, - tx, - pcs[i].exchangeBaseUrl, - pcs[i].denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, pcs[i]); if (!denom) { throw Error("can't find denomination to calculate deposit amount"); } @@ -2443,12 +2438,7 @@ async function getTotalFeesForDepositAmount( await wex.runWalletDbTx(async (tx) => { for (let i = 0; i < pcs.length; i++) { - const denom = await getDenomInfo( - wex, - tx, - pcs[i].exchangeBaseUrl, - pcs[i].denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, pcs[i]); if (!denom) { throw Error("can't find denomination to calculate deposit amount"); } diff --git a/packages/taler-wallet-core/src/exchanges.ts b/packages/taler-wallet-core/src/exchanges.ts @@ -48,6 +48,8 @@ import { ExchangeEntryState, ExchangeGlobalFees, ExchangeKeysResponse, + ConfirmExchangeKeyChangeRequest, + ExchangeKeyChangeInfo, ExchangeListItem, ExchangeSignKeyJson, ExchangeTosStatus, @@ -139,6 +141,7 @@ import { WalletDenomination, WalletDenominationFamily, WalletExchangeDetails, + WalletExchangeDetailsPointer, WalletExchangeEntry, WalletReserve, timestampAbsoluteFromDb, @@ -250,6 +253,95 @@ async function getExchangeRecordsInternal( return details; } +/** + * Base URL to talk to for an exchange master public key. + * + * A master public key can map to more than one details row while a base-URL + * migration is in progress, so the choice is made explicit rather than left to + * whichever row the index happens to yield first: the row that its own + * exchange entry currently points at wins, then one whose currency matches, + * then the lowest base URL so that the answer is at least stable. + */ +export async function getExchangeBaseUrlForMasterPub( + tx: WalletDbTransaction, + masterPub: string, + options: { currency?: string } = {}, +): Promise<string | undefined> { + const candidates = await tx.listExchangeDetailsByMasterPub(masterPub); + if (candidates.length === 0) { + return undefined; + } + if (candidates.length === 1) { + return candidates[0].exchangeBaseUrl; + } + for (const det of candidates) { + const exch = await tx.getExchange(det.exchangeBaseUrl); + if ( + exch?.detailsPointer?.masterPublicKey === masterPub && + exch.detailsPointer.currency === det.currency + ) { + return det.exchangeBaseUrl; + } + } + const byCurrency = options.currency + ? candidates.filter((d) => d.currency === options.currency) + : candidates; + const pool = byCurrency.length > 0 ? byCurrency : candidates; + return pool.map((d) => d.exchangeBaseUrl).reduce((a, b) => (a < b ? a : b)); +} + +/** + * Like {@link getExchangeBaseUrlForMasterPub}, for callers that cannot carry + * on without an answer. + * + * Failing loudly matters here: a coin that silently resolves to nothing drops + * out of balances and scopes with no trace, which looks like the funds are + * gone rather than like a lookup failed. + */ +export async function getExchangeBaseUrlForMasterPubOrThrow( + tx: WalletDbTransaction, + masterPub: string, + options: { currency?: string } = {}, +): Promise<string> { + const url = await getExchangeBaseUrlForMasterPub(tx, masterPub, options); + if (url == null) { + throw TalerError.fromDetail( + TalerErrorCode.WALLET_EXCHANGE_ENTRY_NOT_FOUND, + { masterPub }, + "no exchange entry for the master public key that signed this denomination", + ); + } + return url; +} + +/** + * Every denomination the wallet has for an exchange, across all of its key + * sets. + * + * Denominations are stored under the key that signed them, so "all + * denominations at this URL" is the union over the key sets the URL has + * served. Only callers that really mean every key set want this -- purging + * an entry, or judging a response against what is already stored. Anything + * about what can be withdrawn or cherry-picked wants the accepted key set + * alone and should call getDenominationsByMasterPub directly. + */ +export async function getAllDenominationsForExchange( + tx: WalletDbTransaction, + exchangeBaseUrl: string, +): Promise<WalletDenomination[]> { + const details = await tx.listExchangeDetailsByBaseUrl(exchangeBaseUrl); + const seen = new Set<string>(); + const out: WalletDenomination[] = []; + for (const det of details) { + if (seen.has(det.masterPublicKey)) { + continue; + } + seen.add(det.masterPublicKey); + out.push(...(await tx.getDenominationsByMasterPub(det.masterPublicKey))); + } + return out; +} + export async function getScopeForAllCoins( tx: WalletDbTransaction, coinPubs: string[], @@ -415,9 +507,22 @@ async function makeExchangeListItem( noFees = true; } + let unconfirmedKeyChange: ExchangeKeyChangeInfo | undefined = undefined; + if (r.supersededKeySet && exchangeDetails) { + unconfirmedKeyChange = { + currentMasterPub: exchangeDetails.masterPublicKey, + currentCurrency: exchangeDetails.currency, + supersededMasterPub: r.supersededKeySet.masterPublicKey, + supersededCurrency: r.supersededKeySet.currency, + sharesDenominations: r.supersededKeySet.sharesDenominations, + firstSeen: timestampPreciseFromDb(r.supersededKeySet.firstSeen), + }; + } + const listItem: ExchangeListItem = { exchangeBaseUrl: r.baseUrl, masterPub: exchangeDetails?.masterPublicKey, + ...(unconfirmedKeyChange ? { unconfirmedKeyChange } : undefined), noFees, peerPaymentsDisabled: r.peerPaymentsDisabled ?? false, directDepositsDisabled: r.directDepositDisabled ?? false, @@ -534,6 +639,62 @@ export async function lookupExchangeByUri( /** * Mark the current ToS version as accepted by the user. */ +/** + * Confirm that a changed key set is legitimate. + * + * The change was adopted when it was observed; this only clears the record + * that gates withdrawing. Nothing about the key material is re-checked here, + * because there is nothing to check against: the protocol has no signature + * linking a new master key to the one it replaces, which is exactly why the + * decision is the user's. + */ +export async function confirmExchangeKeyChange( + wex: WalletExecutionContext, + req: ConfirmExchangeKeyChangeRequest, +): Promise<void> { + await wex.runWalletDbTx(async (tx) => { + const r = await tx.getExchange(req.exchangeBaseUrl); + if (!r) { + throw TalerError.fromDetail( + TalerErrorCode.WALLET_EXCHANGE_ENTRY_NOT_FOUND, + { exchangeBaseUrl: req.exchangeBaseUrl }, + "no exchange entry for that base URL", + ); + } + if (!r.supersededKeySet) { + throw TalerError.fromDetail( + TalerErrorCode.WALLET_EXCHANGE_NO_KEY_CHANGE_PENDING, + { exchangeBaseUrl: req.exchangeBaseUrl }, + "the exchange has no unconfirmed key change", + ); + } + // Guards a stale UI: the key on screen when the user decided must be the + // one in force now, or they confirmed something else. + if (r.detailsPointer?.masterPublicKey !== req.currentMasterPub) { + throw TalerError.fromDetail( + TalerErrorCode.WALLET_EXCHANGE_KEY_CHANGE_MISMATCH, + { + exchangeBaseUrl: req.exchangeBaseUrl, + currentMasterPub: r.detailsPointer?.masterPublicKey, + confirmedMasterPub: req.currentMasterPub, + }, + "the confirmed master public key is not the one the exchange uses", + ); + } + const oldExchangeState = getExchangeState(r); + delete r.supersededKeySet; + await tx.upsertExchange(r); + tx.notify({ + type: NotificationType.ExchangeStateTransition, + exchangeBaseUrl: req.exchangeBaseUrl, + causeHint: "key-change-confirmed", + newExchangeState: getExchangeState(r), + oldExchangeState, + }); + }); + wex.ws.exchangeCache.clear(); +} + export async function acceptExchangeTermsOfService( wex: WalletExecutionContext, exchangeBaseUrl: string, @@ -933,13 +1094,19 @@ async function checkExchangeEntryOutdated( wex: WalletExecutionContext, tx: WalletDbTransaction, exchangeBaseUrl: string, + exchangeMasterPub: string | undefined, ): Promise<boolean> { // We currently consider the exchange outdated when no // denominations can be used for withdrawal. logger.trace(`checking if exchange entry for ${exchangeBaseUrl} is outdated`); let numOkay = 0; - let denoms = await tx.getDenominationsByExchange(exchangeBaseUrl); + // Only the accepted key set can be withdrawn from, so only it can make the + // entry look current; a superseded one is still spendable. + let denoms = + exchangeMasterPub != null + ? await tx.getDenominationsByMasterPub(exchangeMasterPub) + : await getAllDenominationsForExchange(tx, exchangeBaseUrl); logger.trace(`exchange entry has ${denoms.length} denominations`); for (const denom of denoms) { const denomOkay = isCandidateWithdrawableDenomRec(denom); @@ -1001,9 +1168,13 @@ function isDenomWorthStoring(d: DenominationInfo): boolean { async function getKeysCherryPickDate( tx: WalletDbTransaction, exchangeBaseUrl: string, + exchangeMasterPub: string, ): Promise<number | undefined> { let maxStampStart: DbProtocolTimestamp | undefined; - const denoms = await tx.getDenominationsByExchange(exchangeBaseUrl); + // Only the accepted key set: denominations of a superseded one are in no + // /keys the current key signs, so their start date names no cherry-picking + // point either. + const denoms = await tx.getDenominationsByMasterPub(exchangeMasterPub); for (const denom of denoms) { // Only denominations the exchange should still be listing are candidates. // A revoked or retired one is gone from /keys, so its start date names no @@ -1122,6 +1293,7 @@ export async function startUpdateExchangeEntry( wex, tx, exchangeBaseUrl, + r.detailsPointer?.masterPublicKey, ); if (outdated) { r.updateStatus = ExchangeEntryDbUpdateStatus.OutdatedUpdate; @@ -1152,6 +1324,7 @@ export async function startUpdateExchangeEntry( wex, tx, exchangeBaseUrl, + r.detailsPointer?.masterPublicKey, ); if (outdated) { r.updateStatus = ExchangeEntryDbUpdateStatus.OutdatedUpdate; @@ -1217,6 +1390,12 @@ export interface ReadyExchangeSummary { tinyAmount: AmountString; zeroLimits: ZeroLimitedOperation[]; hardLimits: AccountLimit[]; + /** + * Set when the exchange changed its key set and the user has not confirmed + * the change. Present here rather than looked up separately because every + * operation that could act on it already holds a summary. + */ + unconfirmedKeyChange?: ExchangeKeyChangeInfo; } /** @@ -1255,7 +1434,12 @@ export async function requireExchangeReadyTx( // This is fatal, outer transaction will not be retried. throw Error("exchange does not exist in database"); } - const isOutdated = await checkExchangeEntryOutdated(wex, tx, exchangeBaseUrl); + const isOutdated = await checkExchangeEntryOutdated( + wex, + tx, + exchangeBaseUrl, + exchange.detailsPointer?.masterPublicKey, + ); if (isOutdated) { logger.warn(`exchange ${exchangeBaseUrl} outdated`); throw new OutdatedExchangeError("exchange entry outdated", exchangeBaseUrl); @@ -1594,6 +1778,21 @@ function constructReadyExchangeSummary( walletBalanceLimitWithoutKyc: exchangeDetails.walletBalanceLimits, hardLimits: exchangeDetails.hardLimits ?? [], zeroLimits: exchangeDetails.zeroLimits ?? [], + ...(exchangeRec.supersededKeySet + ? { + unconfirmedKeyChange: { + currentMasterPub: exchangeDetails.masterPublicKey, + currentCurrency: exchangeDetails.currency, + supersededMasterPub: exchangeRec.supersededKeySet.masterPublicKey, + supersededCurrency: exchangeRec.supersededKeySet.currency, + sharesDenominations: + exchangeRec.supersededKeySet.sharesDenominations, + firstSeen: timestampPreciseFromDb( + exchangeRec.supersededKeySet.firstSeen, + ), + }, + } + : undefined), }; } @@ -1789,12 +1988,19 @@ export async function updateExchangeFromUrlHandler( // the wallet's picture of the denominations that is in doubt, so those // updates ask for the whole response instead. let cherryPickDate: number | undefined = undefined; + const acceptedMasterPub = oldExchangeRec.detailsPointer?.masterPublicKey; switch (oldExchangeRec.updateStatus) { case ExchangeEntryDbUpdateStatus.Ready: case ExchangeEntryDbUpdateStatus.ReadyUpdate: - cherryPickDate = await wex.runWalletDbTx(async (tx) => { - return await getKeysCherryPickDate(tx, exchangeBaseUrl); - }); + if (acceptedMasterPub != null) { + cherryPickDate = await wex.runWalletDbTx(async (tx) => { + return await getKeysCherryPickDate( + tx, + exchangeBaseUrl, + acceptedMasterPub, + ); + }); + } break; } @@ -2052,32 +2258,61 @@ export async function updateExchangeFromUrlHandler( // FIXME: We need to do some more consistency checks! } if (detailsIncompatible) { + // The exchange presents a different signing authority. The new key set + // is adopted right away, so the entry keeps working and the coins + // already held stay spendable -- what is withheld is the part that + // sends money the other way. Until the user confirms the change, the + // wire details a withdrawal would pay into are not trusted, because a + // URL taken over by someone else would otherwise redirect it. logger.warn( - `exchange ${r.baseUrl} has incompatible data in /keys, not updating`, + `exchange ${r.baseUrl} changed its key set (${conflictHint})`, ); - // We don't support this gracefully right now. - // See https://bugs.taler.net/n/8576 - r.updateStatus = ExchangeEntryDbUpdateStatus.UnavailableUpdate; - r.unavailableReason = makeTalerErrorDetail( - TalerErrorCode.WALLET_EXCHANGE_ENTRY_UPDATE_CONFLICT, - { - detail: conflictHint, - }, + checkLogicInvariant(!!existingDetails); + + // Whether the new key set still advertises denominations the wallet + // holds coins of. A claim rather than proof -- denomination public + // keys are public -- but an exchange that does not list them will + // certainly not settle them. + const heldDenoms = await tx.getDenominationsByMasterPub( + existingDetails.masterPublicKey, ); - r.nextRefreshCheckStamp = timestampPreciseToDb( - AbsoluteTime.toPreciseTimestamp(AbsoluteTime.never()), + const sharesDenominations = heldDenoms.some((d) => + currentDenomSet.has(d.denomPubHash), ); - r.cachebreakNextUpdate = true; - await tx.upsertExchange(r); - tx.notify({ - type: NotificationType.ExchangeStateTransition, - exchangeBaseUrl, - causeHint: "details-incompatible", - newExchangeState: getExchangeState(r), - oldExchangeState, - }); - return TaskRunResult.backoff(); + + // Going back to the key set that was in force before the last change + // is not a new change to confirm -- the wallet is where it started, and + // the funds frozen in the meantime become spendable again by + // themselves. This is the ordinary outcome of a misconfiguration or a + // failover that gets corrected. + const revertedToPrevious = + r.supersededKeySet != null && + r.supersededKeySet.masterPublicKey === keysInfo.master_public_key && + r.supersededKeySet.currency === keysInfo.currency; + if (revertedToPrevious) { + logger.info(`exchange ${r.baseUrl} reverted to its previous key set`); + delete r.supersededKeySet; + } else { + // Keep the first sighting if the same change is seen again, so the + // record says when the exchange changed, not when it was last polled. + const alreadyKnown = + r.supersededKeySet?.masterPublicKey === + existingDetails.masterPublicKey && + r.supersededKeySet?.currency === existingDetails.currency; + r.supersededKeySet = { + masterPublicKey: existingDetails.masterPublicKey, + currency: existingDetails.currency, + firstSeen: alreadyKnown + ? r.supersededKeySet!.firstSeen + : timestampPreciseToDb(TalerPreciseTimestamp.now()), + sharesDenominations, + }; + } + // Falls through: the update proceeds normally from here, and + // detailsPointerChanged is already set, so the new key set becomes the + // current one. } + delete r.unavailableReason; const newDetails: WalletExchangeDetails = { auditors: keysInfo.auditors, @@ -2126,7 +2361,14 @@ export async function updateExchangeFromUrlHandler( // FIXME! AbsoluteTime.addDuration( AbsoluteTime.now(), - Duration.fromSpec({ hours: 2 }), + // While a key change is unconfirmed the situation is unresolved and + // some of the user's funds are frozen, so check back sooner: the + // change may be a misconfiguration or a failover that gets + // corrected, and reverting unfreezes them without the user doing + // anything. + r.supersededKeySet + ? Duration.fromSpec({ minutes: 5 }) + : Duration.fromSpec({ hours: 2 }), ), ), ); @@ -2172,11 +2414,27 @@ export async function updateExchangeFromUrlHandler( } // In the future: Filter out old denominations by index - const allOldDenoms = await tx.getDenominationsByExchange(exchangeBaseUrl); + const allOldDenoms = await getAllDenominationsForExchange( + tx, + exchangeBaseUrl, + ); + // Every denomination stored for this URL, whichever key signed it. An + // operator that rotated its master key re-advertises the same + // denominations under the new one, and recognising them here is what + // re-attributes them to the new key instead of storing a second copy -- + // or, worse, skipping them as not worth storing and leaving the coins + // pointing at a key the exchange no longer uses. const oldDenomByDph = new Map<string, WalletDenomination>(); for (const denom of allOldDenoms) { oldDenomByDph.set(denom.denomPubHash, denom); } + // The retirement sweep below, by contrast, may only judge denominations + // of the key set this response speaks for: those of a superseded key are + // absent from every /keys the new key signs, and reading that absence as + // retirement would write the old balance off as a denomination loss. + const sweepableDenoms = allOldDenoms.filter( + (d) => d.exchangeMasterPub === keysInfo.master_public_key, + ); logger.trace("updating denominations in database"); @@ -2325,7 +2583,7 @@ export async function updateExchangeFromUrlHandler( const coveredFromStampStart = !cherryPicked ? undefined : timestampProtocolToDb(keysInfo.list_issue_date); - for (const x of allOldDenoms) { + for (const x of sweepableDenoms) { if ( coveredFromStampStart != null && x.stampStart < coveredFromStampStart @@ -2357,10 +2615,22 @@ export async function updateExchangeFromUrlHandler( // denominations list as well, so handleDenomLoss can only tell a // revocation apart from a plain retirement once isRevoked is set. if (keysInfo.recoup != null) { - await handleRecoup(wex, tx, exchangeBaseUrl, keysInfo.recoup); + await handleRecoup( + wex, + tx, + exchangeBaseUrl, + keysInfo.master_public_key, + keysInfo.recoup, + ); } - await handleDenomLoss(wex, tx, newDetails.currency, exchangeBaseUrl); + await handleDenomLoss( + wex, + tx, + newDetails.currency, + exchangeBaseUrl, + newDetails.masterPublicKey, + ); const newExchangeState = getExchangeState(r); @@ -2413,10 +2683,7 @@ async function doExchangeAutoRefresh( if (coin.status !== CoinStatus.Fresh) { continue; } - const denom = await tx.getDenomination( - exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await tx.getDenomination(coin); if (!denom) { logger.warn("denomination not in database"); continue; @@ -2513,14 +2780,30 @@ export async function processTaskExchangeAutoRefresh( return TaskRunResult.progress(); } +/** + * Write off the coins of denominations the exchange no longer honours. + * + * Scoped to one master public key: coins issued under a key the exchange has + * since replaced are not lost, they are simply not described by the current + * /keys, and judging them against it would destroy the balance the wallet is + * meant to be preserving. + */ async function handleDenomLoss( wex: WalletExecutionContext, tx: WalletDbTransaction, currency: string, exchangeBaseUrl: string, + exchangeMasterPub: string, ): Promise<void> { - const coinAvailabilityRecs = - await tx.getCoinAvailabilityByExchange(exchangeBaseUrl); + const coinAvailabilityRecs = ( + await tx.getCoinAvailabilityByExchange(exchangeBaseUrl) + ).filter( + // Availability rows written before the master pub was recorded are + // attributed to the current key set, which is where they came from. + (ca) => + ca.exchangeMasterPub == null || + ca.exchangeMasterPub === exchangeMasterPub, + ); const denomsVanished: string[] = []; const denomsRevoked: string[] = []; const denomsUnoffered: string[] = []; @@ -2535,10 +2818,7 @@ async function handleDenomLoss( continue; } const n = coinAv.freshCoinCount; - const denom = await tx.getDenomination( - coinAv.exchangeBaseUrl, - coinAv.denomPubHash, - ); + const denom = await tx.getDenomination(coinAv); const timestampExpireDeposit = !denom ? undefined : timestampAbsoluteFromDb(denom.stampExpireDeposit); @@ -2849,6 +3129,7 @@ async function handleRecoup( wex: WalletExecutionContext, tx: WalletDbTransaction, exchangeBaseUrl: string, + exchangeMasterPub: string, recoup: Recoup[], ): Promise<void> { // Handle recoup @@ -2856,10 +3137,12 @@ async function handleRecoup( const newlyRevokedCoinPubs: string[] = []; logger.trace("recoup list from exchange", recoupDenomList); for (const recoupInfo of recoupDenomList) { - const oldDenom = await tx.getDenomination( - exchangeBaseUrl, - recoupInfo.h_denom_pub, - ); + // A revocation names a denomination of the key set currently in force: + // it arrives in that key set's own /keys response. + const oldDenom = await tx.getDenomination({ + exchangeMasterPub: exchangeMasterPub, + denomPubHash: recoupInfo.h_denom_pub, + }); if (!oldDenom) { // We never even knew about the revoked denomination, all good. continue; @@ -3173,7 +3456,10 @@ export async function getExchangeDetailedInfo( if (!exchangeDetails) { return; } - const denominationRecords = await tx.getDenominationsByExchange(ex.baseUrl); + const denominationRecords = await getAllDenominationsForExchange( + tx, + ex.baseUrl, + ); if (!denominationRecords) { return; @@ -3370,11 +3656,7 @@ async function purgeExchange( const coinAvailabilityRecs = await tx.getCoinAvailabilityByExchange(exchangeBaseUrl); for (const rec of coinAvailabilityRecs) { - await tx.deleteCoinAvailability( - exchangeBaseUrl, - rec.denomPubHash, - rec.maxAge, - ); + await tx.deleteCoinAvailability(rec); } } @@ -3387,9 +3669,9 @@ async function purgeExchange( } { - const denomRecs = await tx.getDenominationsByExchange(exchangeBaseUrl); + const denomRecs = await getAllDenominationsForExchange(tx, exchangeBaseUrl); for (const rec of denomRecs) { - await tx.deleteDenomination(rec.exchangeBaseUrl, rec.denomPubHash); + await tx.deleteDenomination(rec); } } @@ -4288,6 +4570,10 @@ export async function checkExchangeInScopeTx( { parameter: "scopeInfo" }, "the auditor scope is not supported yet", ); + case ScopeType.ExchangeLegacyKeys: + // See checkExchangeInScopeGeneric: an entry stands for its current key + // set, which is never a superseded one. + return false; } } @@ -4402,10 +4688,14 @@ export async function migrateExchange( }); { - const denoms = await tx.getDenominationsByExchange( + // Denominations are stored under the key that signed them, which a + // change of base URL does not touch, so their identity needs no + // rewriting. The URL they carry is a routing hint kept in step here so + // that the by-URL lookups still find them. + const denoms = await getAllDenominationsForExchange( + tx, req.oldExchangeBaseUrl, ); - for (const rec of denoms) { rec.exchangeBaseUrl = req.newExchangeBaseUrl; await tx.upsertDenomination(rec); @@ -4472,11 +4762,7 @@ export async function migrateExchange( req.oldExchangeBaseUrl, ); for (const rec of recs) { - await tx.deleteCoinAvailability( - rec.exchangeBaseUrl, - rec.denomPubHash, - rec.maxAge, - ); + await tx.deleteCoinAvailability(rec); rec.exchangeBaseUrl = req.newExchangeBaseUrl; await tx.upsertCoinAvailability(rec); } diff --git a/packages/taler-wallet-core/src/instructedAmountConversion.ts b/packages/taler-wallet-core/src/instructedAmountConversion.ts @@ -31,7 +31,10 @@ import { } from "@gnu-taler/taler-util"; import { timestampProtocolFromDb } from "./db-common.js"; import { WalletDenomination } from "./db-common.js"; -import { getExchangeDetailsInTx } from "./exchanges.js"; +import { + getAllDenominationsForExchange, + getExchangeDetailsInTx, +} from "./exchanges.js"; import { WalletExecutionContext } from "./wallet.js"; export interface CoinInfo { @@ -209,7 +212,7 @@ async function getAvailableCoins( //4.- filter coins restricted by age if (operationType === OperationType.Credit) { // FIXME: Use denom groups instead of querying all denominations! - const ds = await tx.getDenominationsByExchange(exchangeBaseUrl); + const ds = await getAllDenominationsForExchange(tx, exchangeBaseUrl); for (const denom of ds) { const expiresWithdraw = AbsoluteTime.fromProtocolTimestamp( timestampProtocolFromDb(denom.stampExpireWithdraw), @@ -244,10 +247,7 @@ async function getAvailableCoins( // FIXME: Should we exclude denominations that are // not spendable anymore? for (const coinAvail of myExchangeCoins) { - const denom = await tx.getDenomination( - coinAvail.exchangeBaseUrl, - coinAvail.denomPubHash, - ); + const denom = await tx.getDenomination(coinAvail); checkDbInvariant( !!denom, `denomination of a coin is missing hash: ${coinAvail.denomPubHash}`, diff --git a/packages/taler-wallet-core/src/pay-merchant.ts b/packages/taler-wallet-core/src/pay-merchant.ts @@ -845,10 +845,7 @@ export async function getTotalPaymentCostInTx( ): Promise<AmountJson> { const costs: AmountJson[] = []; for (let i = 0; i < pcs.length; i++) { - const denom = await tx.getDenomination( - pcs[i].exchangeBaseUrl, - pcs[i].denomPubHash, - ); + const denom = await tx.getDenomination(pcs[i]); if (!denom) { throw Error( "can't calculate payment cost, denomination for coin not found", @@ -2092,10 +2089,7 @@ export async function generateDepositPermissions( if (!coin) { throw Error("can't pay, allocated coin not found anymore"); } - const denom = await tx.getDenomination( - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await tx.getDenomination(coin); if (!denom) { throw Error( "can't pay, denomination of allocated coin not found anymore", @@ -4586,12 +4580,7 @@ async function computeRefreshRequest( if (!coin) { throw Error("coin not found"); } - const denomInfo = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, coin); if (!denomInfo) { throw Error("denom not found"); } diff --git a/packages/taler-wallet-core/src/pay-peer-common.ts b/packages/taler-wallet-core/src/pay-peer-common.ts @@ -47,12 +47,7 @@ export async function queryCoinInfosForSelection( if (!coin) { throw Error("coin not found anymore"); } - const denom = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, coin); if (!denom) { throw Error("denom for coin not found anymore"); } @@ -81,12 +76,7 @@ export async function getTotalPeerPaymentCostInTx( } const costs: AmountJson[] = []; for (let i = 0; i < pcs.length; i++) { - const denomInfo = await getDenomInfo( - wex, - tx, - pcs[i].exchangeBaseUrl, - pcs[i].denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, pcs[i]); if (!denomInfo) { throw Error( "can't calculate payment cost, denomination for coin not found", diff --git a/packages/taler-wallet-core/src/pay-peer-pull-credit.ts b/packages/taler-wallet-core/src/pay-peer-pull-credit.ts @@ -61,6 +61,7 @@ import { constructTaskIdentifier, genericWaitForStateVal, getGenericRecordHandle, + requireExchangeKeysConfirmedOrThrow, requireExchangeTosAcceptedOrThrow, reservePaytoFromExchange, } from "./common.js"; @@ -1187,6 +1188,7 @@ async function internalInitiatePeerPullPayment( const exchangeBaseUrl = maybeExchangeBaseUrl; const exchange = await fetchFreshExchangeWithRetryNow(wex, exchangeBaseUrl); + requireExchangeKeysConfirmedOrThrow(wex, exchange); requireExchangeTosAcceptedOrThrow(wex, exchange); if ( diff --git a/packages/taler-wallet-core/src/pay-peer-push-credit.ts b/packages/taler-wallet-core/src/pay-peer-push-credit.ts @@ -66,6 +66,7 @@ import { constructTaskIdentifier, genericWaitForStateVal, getGenericRecordHandle, + requireExchangeKeysConfirmedOrThrow, requireExchangeTosAcceptedOrThrow, reservePaytoFromExchange, } from "./common.js"; @@ -1373,6 +1374,7 @@ async function internalConfirmPeerPushCredit( wex, peerInc.exchangeBaseUrl, ); + requireExchangeKeysConfirmedOrThrow(wex, exchange); requireExchangeTosAcceptedOrThrow(wex, exchange); if (checkPeerCreditHardLimitExceeded(exchange, res.contractTerms.amount)) { diff --git a/packages/taler-wallet-core/src/recoup.ts b/packages/taler-wallet-core/src/recoup.ts @@ -123,12 +123,7 @@ async function recoupRefreshCoin( cs: WalletRefreshCoinSource, ): Promise<void> { const d = await wex.runWalletDbTx(async (tx) => { - const denomInfo = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, coin); if (!denomInfo) { return; } @@ -178,18 +173,8 @@ async function recoupRefreshCoin( logger.warn("refresh old coin for recoup not found"); return; } - const oldCoinDenom = await getDenomInfo( - wex, - tx, - oldCoin.exchangeBaseUrl, - oldCoin.denomPubHash, - ); - const revokedCoinDenom = await getDenomInfo( - wex, - tx, - revokedCoin.exchangeBaseUrl, - revokedCoin.denomPubHash, - ); + const oldCoinDenom = await getDenomInfo(wex, tx, oldCoin); + const revokedCoinDenom = await getDenomInfo(wex, tx, revokedCoin); checkDbInvariant( !!oldCoinDenom, `no denom for coin, hash ${oldCoin.denomPubHash}`, @@ -219,12 +204,7 @@ export async function recoupWithdrawCoin( ): Promise<void> { const reservePub = cs.reservePub; const denomInfo = await wex.runWalletDbTx(async (tx) => { - const denomInfo = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, coin); return denomInfo; }); if (!denomInfo) { diff --git a/packages/taler-wallet-core/src/refresh.ts b/packages/taler-wallet-core/src/refresh.ts @@ -419,11 +419,11 @@ async function getCoinAvailabilityForDenom( denom: DenominationInfo, ageRestriction: number, ): Promise<WalletCoinAvailability> { - let car = await tx.getCoinAvailability( - denom.exchangeBaseUrl, - denom.denomPubHash, - ageRestriction, - ); + let car = await tx.getCoinAvailability({ + exchangeMasterPub: denom.exchangeMasterPub, + denomPubHash: denom.denomPubHash, + maxAge: ageRestriction, + }); if (!car) { car = { maxAge: ageRestriction, @@ -460,12 +460,10 @@ async function initRefreshSession( const exchangeBaseUrl = oldCoin.exchangeBaseUrl; - const oldDenom = await getDenomInfo( - wex, - tx, - exchangeBaseUrl, - oldCoin.denomPubHash, - ); + const oldDenom = await getDenomInfo(wex, tx, { + exchangeMasterPub: oldCoin.exchangeMasterPub, + denomPubHash: oldCoin.denomPubHash, + }); if (!oldDenom) { throw Error("db inconsistent: denomination for coin not found"); @@ -505,7 +503,10 @@ async function initRefreshSession( for (let i = 0; i < newCoinDenoms.selectedDenoms.length; i++) { const dph = newCoinDenoms.selectedDenoms[i].denomPubHash; - const denom = await getDenomInfo(wex, tx, oldDenom.exchangeBaseUrl, dph); + const denom = await getDenomInfo(wex, tx, { + exchangeMasterPub: oldDenom.exchangeMasterPub, + denomPubHash: dph, + }); if (!denom) { logger.error(`denom ${dph} not in DB`); continue; @@ -554,7 +555,10 @@ async function destroyRefreshSession( continue; } const dph = refreshSession.newDenoms[i].denomPubHash; - const denom = await getDenomInfo(wex, tx, oldCoin.exchangeBaseUrl, dph); + const denom = await getDenomInfo(wex, tx, { + exchangeMasterPub: oldCoin.exchangeMasterPub, + denomPubHash: dph, + }); if (!denom) { logger.error(`denom ${dph} not in DB`); continue; @@ -733,23 +737,16 @@ async function refreshMelt( const oldCoin = await tx.getCoin(refreshGroup.oldCoinPubs[coinIndex]); checkDbInvariant(!!oldCoin, "melt coin doesn't exist"); - const oldDenom = await getDenomInfo( - wex, - tx, - oldCoin.exchangeBaseUrl, - oldCoin.denomPubHash, - ); + const oldDenom = await getDenomInfo(wex, tx, oldCoin); checkDbInvariant(!!oldDenom, "denomination for melted coin doesn't exist"); const newCoinDenoms: RefreshNewDenomInfo[] = []; for (const dh of refreshSession.newDenoms) { - const newDenom = await getDenomInfo( - wex, - tx, - oldCoin.exchangeBaseUrl, - dh.denomPubHash, - ); + const newDenom = await getDenomInfo(wex, tx, { + exchangeMasterPub: oldCoin.exchangeMasterPub, + denomPubHash: dh.denomPubHash, + }); checkDbInvariant( !!newDenom, "new denomination for refresh not in database", @@ -1142,27 +1139,27 @@ async function refreshReveal( const oldCoin = await tx.getCoin(refreshGroup.oldCoinPubs[coinIndex]); checkDbInvariant(!!oldCoin, "melt coin doesn't exist"); - const oldDenom = await getDenomInfo( - wex, - tx, - oldCoin.exchangeBaseUrl, - oldCoin.denomPubHash, - ); + const oldDenom = await getDenomInfo(wex, tx, oldCoin); checkDbInvariant(!!oldDenom, "denomination for melted coin doesn't exist"); const newCoinDenoms: RefreshNewDenomInfo[] = []; + // The key that signed the outputs. RefreshNewDenomInfo is a crypto-layer + // type and deliberately carries no exchange identity, so it is taken from + // the denomination here; every output of one refresh comes from the same + // key set. + let newDenomMasterPub: string | undefined = undefined; + for (const dh of refreshSession.newDenoms) { - const newDenom = await getDenomInfo( - wex, - tx, - oldCoin.exchangeBaseUrl, - dh.denomPubHash, - ); + const newDenom = await getDenomInfo(wex, tx, { + exchangeMasterPub: oldCoin.exchangeMasterPub, + denomPubHash: dh.denomPubHash, + }); checkDbInvariant( !!newDenom, "new denomination for refresh not in database", ); + newDenomMasterPub = newDenom.exchangeMasterPub; newCoinDenoms.push({ count: dh.count, denomPub: newDenom.denomPub, @@ -1175,6 +1172,7 @@ async function refreshReveal( oldCoin, oldDenom, newCoinDenoms, + newDenomMasterPub, refreshSession, refreshGroup, norevealIndex, @@ -1189,10 +1187,12 @@ async function refreshReveal( oldCoin, oldDenom, newCoinDenoms, + newDenomMasterPub, refreshSession, refreshGroup, norevealIndex, } = d; + checkLogicInvariant(newDenomMasterPub != null); // Blinded signatures, either from the old or the new reveal protocol. let resEvSigs: BlindedDenominationSignature[]; @@ -1265,6 +1265,7 @@ async function refreshReveal( denomPubHash: ncd.denomPubHash, denomSig, exchangeBaseUrl: oldCoin.exchangeBaseUrl, + exchangeMasterPub: newDenomMasterPub, status: CoinStatus.Fresh, coinSource: { type: CoinSourceType.Refresh, @@ -1301,12 +1302,7 @@ async function refreshReveal( continue; } await tx.upsertCoin(coin); - const denomInfo = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, coin); checkDbInvariant(!!denomInfo, `no denom with hash ${coin.denomPubHash}`); const car = await getCoinAvailabilityForDenom( wex, @@ -1588,12 +1584,7 @@ export async function calculateRefreshOutput( for (const ocp of oldCoinPubs) { const coin = await tx.getCoin(ocp.coinPub); checkDbInvariant(!!coin, "coin must be in database"); - const denom = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, coin); checkDbInvariant( !!denom, "denomination for existing coin must be in database", @@ -1633,12 +1624,7 @@ async function applyRefreshToOldCoins( for (const ocp of oldCoinPubs) { const coin = await tx.getCoin(ocp.coinPub); checkDbInvariant(!!coin, "coin must be in database"); - const denom = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, coin); checkDbInvariant( !!denom, "denomination for existing coin must be in database", @@ -1648,11 +1634,7 @@ async function applyRefreshToOldCoins( break; case CoinStatus.Fresh: { coin.status = CoinStatus.Dormant; - const coinAv = await tx.getCoinAvailability( - coin.exchangeBaseUrl, - coin.denomPubHash, - coin.maxAge, - ); + const coinAv = await tx.getCoinAvailability(coin); checkDbInvariant( !!coinAv, `no denom info for ${coin.denomPubHash} age ${coin.maxAge}`, @@ -1988,12 +1970,7 @@ export async function forceRefresh( if (!coin) { throw Error(`coin (pubkey ${c}) not found`); } - const denom = await getDenomInfo( - wex, - tx, - coin.exchangeBaseUrl, - coin.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, coin); checkDbInvariant(!!denom, `no denom hash: ${coin.denomPubHash}`); coinPubs.push({ coinPub: c.coinPub, diff --git a/packages/taler-wallet-core/src/requests.ts b/packages/taler-wallet-core/src/requests.ts @@ -150,6 +150,7 @@ import { codecForAbortTransaction, codecForAcceptBankIntegratedWithdrawalRequest, codecForAcceptExchangeTosRequest, + codecForConfirmExchangeKeyChangeRequest, codecForAcceptManualWithdrawalRequest, codecForAcceptPeerPullPaymentRequest, codecForAddBankAccountRequest, @@ -306,6 +307,8 @@ import { } from "./exchange-base-url.js"; import { acceptExchangeTermsOfService, + getAllDenominationsForExchange, + confirmExchangeKeyChange, deleteEphemeralExchanges, deleteExchange, fetchFreshExchange, @@ -533,11 +536,7 @@ async function setCoinSuspended( logger.warn(`coin ${coinPub} not found, won't suspend`); return; } - const coinAvailability = await tx.getCoinAvailability( - c.exchangeBaseUrl, - c.denomPubHash, - c.maxAge, - ); + const coinAvailability = await tx.getCoinAvailability(c); checkDbInvariant( !!coinAvailability, `no denom info for ${c.denomPubHash} age ${c.maxAge}`, @@ -574,7 +573,7 @@ async function dumpCoins(wex: WalletExecutionContext): Promise<CoinDumpJson> { await wex.runWalletDbTx(async (tx) => { const coins = await tx.listAllCoins(); for (const c of coins) { - const denom = await tx.getDenomination(c.exchangeBaseUrl, c.denomPubHash); + const denom = await tx.getDenomination(c); if (!denom) { logger.warn("no denom found for coin"); continue; @@ -588,12 +587,7 @@ async function dumpCoins(wex: WalletExecutionContext): Promise<CoinDumpJson> { if (cs.type == CoinSourceType.Withdraw) { withdrawalReservePub = cs.reservePub; } - const denomInfo = await getDenomInfo( - wex, - tx, - c.exchangeBaseUrl, - c.denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, c); if (!denomInfo) { logger.warn("no denomination found for coin"); continue; @@ -1255,7 +1249,10 @@ async function handleTestingGetDenomStats( numOffered: 0, }; await wex.runWalletDbTx(async (tx) => { - const denoms = await tx.getDenominationsByExchange(req.exchangeBaseUrl); + const denoms = await getAllDenominationsForExchange( + tx, + req.exchangeBaseUrl, + ); for (const d of denoms) { denomStats.numKnown++; if (d.isOffered) { @@ -2296,7 +2293,7 @@ export async function handleGetDiagnostics( cnt["exchangeSignKeys"] = counts.exchangeSignKeys; cnt["exchanges"] = counts.exchanges; for (const exch of await tx.getExchanges()) { - const denoms = await tx.getDenominationsByExchange(exch.baseUrl); + const denoms = await getAllDenominationsForExchange(tx, exch.baseUrl); let numWithdrawableDenoms = 0; let numCandidateWithdrawableDenoms = 0; for (let i = 0; i < denoms.length; i++) { @@ -2634,6 +2631,13 @@ const handlers: { [T in WalletApiOperation]: HandlerWithValidator<T> } = { codec: codecForGetBalanceDetailRequest(), handler: getBalanceDetail, }, + [WalletApiOperation.ConfirmExchangeKeyChange]: { + codec: codecForConfirmExchangeKeyChangeRequest(), + handler: async (wex, req) => { + await confirmExchangeKeyChange(wex, req); + return {}; + }, + }, [WalletApiOperation.SetExchangeTosAccepted]: { codec: codecForAcceptExchangeTosRequest(), handler: async (wex, req) => { diff --git a/packages/taler-wallet-core/src/transactions.ts b/packages/taler-wallet-core/src/transactions.ts @@ -106,6 +106,18 @@ function shouldSkipCurrency( "filtering transactions by auditor scope is not implemented", ); } + case ScopeType.ExchangeLegacyKeys: { + // A transaction records the exchanges it involved by URL, which does + // not say which key set signed the coins, so this filter cannot be + // answered accurately. Refusing beats quietly returning the + // transactions of the exchange's current key set as if they were the + // superseded ones. + throw TalerError.fromDetail( + TalerErrorCode.WALLET_CORE_API_BAD_REQUEST, + { parameter: "scopeInfo" }, + "filtering transactions by a superseded exchange key set is not implemented", + ); + } default: assertUnreachable(transactionsRequest.scopeInfo); } diff --git a/packages/taler-wallet-core/src/wallet-api-types.ts b/packages/taler-wallet-core/src/wallet-api-types.ts @@ -27,6 +27,7 @@ import { AbortTransactionRequest, AcceptBankIntegratedWithdrawalRequest, AcceptExchangeTosRequest, + ConfirmExchangeKeyChangeRequest, AcceptManualWithdrawalRequest, AcceptManualWithdrawalResult, AcceptPeerPullPaymentResponse, @@ -277,6 +278,7 @@ export enum WalletApiOperation { GetExchangeResources = "getExchangeResources", CompleteExchangeBaseUrl = "completeExchangeBaseUrl", DeleteExchange = "deleteExchange", + ConfirmExchangeKeyChange = "confirmExchangeKeyChange", SetExchangeTosAccepted = "setExchangeTosAccepted", SetExchangeTosForgotten = "setExchangeTosForgotten", GetExchangeTos = "getExchangeTos", @@ -1090,6 +1092,22 @@ export type ForgetBankAccountsOp = { }; /** + * Confirm that the exchange's changed key set is legitimate. + * + * The wallet has already adopted it; this releases the operations that send + * money to the exchange, which are withheld until the user has had a chance + * to notice that the key changed. + */ +export type ConfirmExchangeKeyChangeOp = { + op: WalletApiOperation.ConfirmExchangeKeyChange; + request: ConfirmExchangeKeyChangeRequest; + response: EmptyObject; + errors: + | TalerErrorCode.WALLET_EXCHANGE_NO_KEY_CHANGE_PENDING + | TalerErrorCode.WALLET_EXCHANGE_KEY_CHANGE_MISMATCH; +}; + +/** * Accept a particular version of the exchange terms of service. */ export type SetExchangeTosAcceptedOp = { @@ -1987,6 +2005,7 @@ export type WalletOperations = { [WalletApiOperation.AddBankAccount]: AddBankAccountsOp; [WalletApiOperation.ForgetBankAccount]: ForgetBankAccountsOp; [WalletApiOperation.GetBankAccountById]: GetBankAccountByIdOp; + [WalletApiOperation.ConfirmExchangeKeyChange]: ConfirmExchangeKeyChangeOp; [WalletApiOperation.SetExchangeTosAccepted]: SetExchangeTosAcceptedOp; [WalletApiOperation.SetExchangeTosForgotten]: SetExchangeTosForgottenOp; [WalletApiOperation.GetExchangeTos]: GetExchangeTosOp; diff --git a/packages/taler-wallet-core/src/wallet.ts b/packages/taler-wallet-core/src/wallet.ts @@ -73,7 +73,7 @@ import { ConfigRecordKey, WalletDenomination } from "./db-common.js"; import { IdbWalletDbHandle } from "./dbtx-handle-impl.js"; import { WalletDbHandle } from "./dbtx-handle.js"; import { watchForCacheInvalidation } from "./dbtx-shared.js"; -import { WalletDbTransaction } from "./dbtx.js"; +import { WalletDbTransaction, WalletDenomRef } from "./dbtx.js"; import { UnverifiedDenomError } from "./denomSelection.js"; import { DevExperimentHttpLib, DevExperimentState } from "./dev-experiments.js"; import { @@ -217,12 +217,11 @@ export async function migrateMaterializedTransactions( export async function getDenomInfo( wex: WalletExecutionContext, tx: WalletDbTransaction, - exchangeBaseUrl: string, - denomPubHash: string, + ref: WalletDenomRef, ): Promise<DenominationInfo | undefined> { - const key = `${exchangeBaseUrl}:${denomPubHash}`; + const key = `${ref.exchangeMasterPub}:${ref.denomPubHash}`; return wex.ws.denomInfoCache.getOrPut(key, async () => { - const d = await tx.getDenomination(exchangeBaseUrl, denomPubHash); + const d = await tx.getDenomination(ref); if (d != null) { return WalletDenomination.toDenomInfo(d); } else { @@ -750,6 +749,17 @@ export class InternalWalletState { ); /** + * Base URL to talk to for a given exchange master public key. + * + * Coins name the key that signed their denomination, not a URL, so every + * request made on their behalf resolves through here. + */ + exchangeBaseUrlCache: Cache<string> = new Cache( + 1000, + Duration.fromSpec({ minutes: 1 }), + ); + + /** * Promises that are waiting for a particular resource. */ private resourceWaiters: Record<string, OpenedPromise<void>[]> = {}; @@ -862,6 +872,7 @@ export class InternalWalletState { this.exchangeCache.clear(); this.denomInfoCache.clear(); this.refreshCostCache.clear(); + this.exchangeBaseUrlCache.clear(); } initWithConfig(newConfig: WalletRunConfig): void { diff --git a/packages/taler-wallet-core/src/withdraw.ts b/packages/taler-wallet-core/src/withdraw.ts @@ -126,6 +126,7 @@ import { makeCoinAvailable, makeCoinsVisible, prepareTransferOptionsRaw, + requireExchangeKeysConfirmedOrThrow, requireExchangeTosAcceptedOrThrow, } from "./common.js"; import { EddsaKeyPairStrings } from "./crypto/cryptoImplementation.js"; @@ -1268,6 +1269,21 @@ async function getWithdrawableDenoms( * Throws if a candidate withdrawal denomination * isn't validated yet. */ +/** + * The master public key a withdrawal from this exchange issues coins under. + * + * A withdrawal always uses the key set the entry currently points at, so the + * details row is the authority; the withdrawal group only records the URL. + */ +async function getWithdrawalMasterPub( + tx: WalletDbTransaction, + exchangeBaseUrl: string, +): Promise<string> { + const det = await tx.getExchangeDetails(exchangeBaseUrl); + checkDbInvariant(!!det, `no exchange details for ${exchangeBaseUrl}`); + return det.masterPublicKey; +} + export async function getWithdrawableDenomsTx( _wex: WalletExecutionContext, tx: WalletDbTransaction, @@ -1276,8 +1292,19 @@ export async function getWithdrawableDenomsTx( maxAmount?: AmountLike, ): Promise<WalletDenomination[]> { const dbNow = timestampProtocolToDb(TalerProtocolTimestamp.now()); - const allFamilies = - await tx.getDenominationFamiliesByExchange(exchangeBaseUrl); + // Only the key set the user has accepted can be withdrawn from. Families + // of a superseded master public key stay in the database for the coins + // already held under them, and offering them here would plan a withdrawal + // against denominations the exchange no longer issues. + const acceptedMasterPub = (await tx.getExchange(exchangeBaseUrl)) + ?.detailsPointer?.masterPublicKey; + const allFamilies = ( + await tx.getDenominationFamiliesByExchange(exchangeBaseUrl) + ).filter( + (fam) => + acceptedMasterPub == null || + fam.familyParams.exchangeMasterPub === acceptedMasterPub, + ); if (logger.shouldLogTrace()) { const maxStr = maxAmount ? Amounts.stringify(maxAmount) : "<unknown>"; logger.trace( @@ -1318,9 +1345,19 @@ export async function getWithdrawableDenomsTx( } relevantDenoms.sort((d1, d2) => Amounts.cmp(d2.value, d1.value)); for (const denom of relevantDenoms) { - if (denom.exchangeBaseUrl != exchangeBaseUrl) { + // Checked against the key set, not the base URL. A denomination belongs + // to whatever signed it, and one key set can be reached through more than + // one URL -- after a base-URL migration onto an exchange the wallet + // already knew, the two collapse onto one row that carries one of the + // URLs. The key is what decides whether these coins can be withdrawn. + if ( + acceptedMasterPub != null && + denom.exchangeMasterPub != acceptedMasterPub + ) { throw Error( - "invariant violation in withdrawal denom selection (exchangeBaseUrl)", + "invariant violation in withdrawal denom selection: denomination" + + ` ${denom.denomPubHash} is signed by ${denom.exchangeMasterPub},` + + ` but was reached through a family of ${acceptedMasterPub}`, ); } } @@ -1380,7 +1417,10 @@ async function processPlanchetGenerate( const denomPubHash = maybeDenomPubHash; const denom = await wex.runWalletDbTx(async (tx) => { - return getDenomInfo(wex, tx, exchangeBaseUrl, denomPubHash); + return getDenomInfo(wex, tx, { + exchangeMasterPub: await getWithdrawalMasterPub(tx, exchangeBaseUrl), + denomPubHash: denomPubHash, + }); }); if (!denom) { // We handle this gracefully, to fix previous bugs that made it into production. @@ -1526,12 +1566,10 @@ async function processPlanchetExchangeLegacyBatchRequest( if (planchet.planchetStatus === PlanchetStatus.AbortedReplaced) { continue; } - const denom = await getDenomInfo( - wex, - tx, - exchangeBaseUrl, - planchet.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, { + exchangeMasterPub: await getWithdrawalMasterPub(tx, exchangeBaseUrl), + denomPubHash: planchet.denomPubHash, + }); if (!denom) { logger.error("db inconsistent: denom for planchet not found"); @@ -1686,12 +1724,10 @@ async function processPlanchetExchangeBatchRequest( if (planchet.planchetStatus === PlanchetStatus.AbortedReplaced) { continue; } - const denom = await getDenomInfo( - wex, - tx, - exchangeBaseUrl, - planchet.denomPubHash, - ); + const denom = await getDenomInfo(wex, tx, { + exchangeMasterPub: await getWithdrawalMasterPub(tx, exchangeBaseUrl), + denomPubHash: planchet.denomPubHash, + }); if (!denom) { logger.error("db inconsistent: denom for planchet not found"); @@ -1831,12 +1867,10 @@ async function processPlanchetVerifyAndStoreCoin( logger.warn("processPlanchet: planchet already withdrawn"); return; } - const denomInfo = await getDenomInfo( - wex, - tx, - exchangeBaseUrl, - planchet.denomPubHash, - ); + const denomInfo = await getDenomInfo(wex, tx, { + exchangeMasterPub: await getWithdrawalMasterPub(tx, exchangeBaseUrl), + denomPubHash: planchet.denomPubHash, + }); if (!denomInfo) { return; } @@ -1917,6 +1951,7 @@ async function processPlanchetVerifyAndStoreCoin( denomSig, coinEvHash: planchet.coinEvHash, exchangeBaseUrl: d.exchangeBaseUrl, + exchangeMasterPub: d.denomInfo.exchangeMasterPub, status: CoinStatus.Fresh, coinSource: { type: CoinSourceType.Withdraw, @@ -1978,8 +2013,18 @@ export async function updateWithdrawalDenomsForExchange( const dbNow = timestampProtocolToDb(TalerProtocolTimestamp.now()); const denoms = await wex.runWalletDbTx(async (tx) => { - const allFamilies = - await tx.getDenominationFamiliesByExchange(exchangeBaseUrl); + // Same restriction as getWithdrawableDenomsTx: validating denominations + // of a superseded key set costs signature checks for coins that can never + // be withdrawn. + const acceptedMasterPub = (await tx.getExchange(exchangeBaseUrl)) + ?.detailsPointer?.masterPublicKey; + const allFamilies = ( + await tx.getDenominationFamiliesByExchange(exchangeBaseUrl) + ).filter( + (fam) => + acceptedMasterPub == null || + fam.familyParams.exchangeMasterPub === acceptedMasterPub, + ); const denominations: WalletDenomination[] | undefined = []; for (const fam of allFamilies) { const fpSerial = fam.denominationFamilySerial; @@ -2356,7 +2401,10 @@ async function redenominateWithdrawal( let coinIndex = 0; for (let i = 0; i < oldSel.selectedDenoms.length; i++) { const sel = wg.denomsSel.selectedDenoms[i]; - const denom = await tx.getDenomination(exchangeBaseUrl, sel.denomPubHash); + const denom = await tx.getDenomination({ + exchangeMasterPub: await getWithdrawalMasterPub(tx, exchangeBaseUrl), + denomPubHash: sel.denomPubHash, + }); let denomOkay: boolean = false; @@ -2895,6 +2943,9 @@ export async function getExchangeWithdrawalInfo( ? AGE_MASK_GROUPS : undefined, scopeInfo: exchange.scopeInfo, + ...(exchange.unconfirmedKeyChange + ? { unconfirmedKeyChange: exchange.unconfirmedKeyChange } + : undefined), ...getWithdrawalLimitInfo(exchange, instructedAmount), }; return ret; @@ -3821,6 +3872,7 @@ export async function confirmWithdrawal( } const exchange = await fetchFreshExchangeWithRetryNow(wex, selectedExchange); + requireExchangeKeysConfirmedOrThrow(wex, exchange); requireExchangeTosAcceptedOrThrow(wex, exchange); if (req.amount && checkWithdrawalHardLimitExceeded(exchange, req.amount)) { @@ -4286,6 +4338,11 @@ export async function createManualWithdrawal( const amount = Amounts.parseOrThrow(req.amount); const exchange = await fetchFreshExchangeWithRetryNow(wex, exchangeBaseUrl); + // A manual withdrawal has no details step where a warning could be shown, + // so refusing here is the only place the user can learn that the exchange + // changed the key that signs the account they are about to pay into. + requireExchangeKeysConfirmedOrThrow(wex, exchange); + if (exchange.currency != amount.currency) { throw TalerError.fromDetail( TalerErrorCode.GENERIC_CURRENCY_MISMATCH, @@ -4450,6 +4507,9 @@ export async function internalGetWithdrawalDetailsForAmount( withdrawalAccountsList: wi.exchangeCreditAccountDetails, numCoins, scopeInfo: wi.scopeInfo, + ...(wi.unconfirmedKeyChange + ? { unconfirmedKeyChange: wi.unconfirmedKeyChange } + : undefined), kycHardLimit: wi.kycHardLimit, kycSoftLimit: wi.kycSoftLimit, }; diff --git a/packages/taler-wallet-webextension/src/wallet/DestinationSelection/state.ts b/packages/taler-wallet-webextension/src/wallet/DestinationSelection/state.ts @@ -110,6 +110,11 @@ export function useComponentState(props: Props): RecursiveState<State> { `${b.scopeInfo.currency} ${b.scopeInfo.url}`; break; } + case ScopeType.ExchangeLegacyKeys: { + // Funds under a key the exchange has replaced cannot be withdrawn + // into, so this is not a destination to offer. + break; + } default: { assertUnreachable(b.scopeInfo); }