diff --git a/__tests__/keysetSelection.test.ts b/__tests__/keysetSelection.test.ts new file mode 100644 index 00000000..e3a4f0d9 --- /dev/null +++ b/__tests__/keysetSelection.test.ts @@ -0,0 +1,123 @@ +/** + * Which keyset a new wallet binds to (`WalletStore.getOptimalKeysetId`). + * + * The wallet used to mirror cashu-ts's KeyChain.getCheapestKeyset by hand: filter to + * active hex-id keysets for the unit, sort by input fee, break ties on id. It now + * delegates to the library, which is a deliberate BEHAVIOUR change, not just dedup — + * the two orderings had diverged: + * + * old (wallet) fee ASC, then id (version) as a tiebreak + * new (cashu-ts) version DESC, then fee ASC, then final_expiry DESC (#836) + * + * They disagree exactly when a mint prices its newer keyset ABOVE an older one. The + * old rule kept minting on the cheaper, older keyset; the new one moves to the + * current keyset and accepts the higher fee. That is the intended direction — a + * keyset the mint has superseded is one it can retire, stranding ecash on it — but + * it does mean a user can pay more per input after this upgrade. + * + * These tests pin that ordering against the real library, so the choice is explicit + * and a future cashu-ts change to it fails here rather than silently altering which + * keyset the wallet mints on. + * + * @jest-environment node + */ +import {KeyChain, deriveKeysetId, getPubKeyFromPrivKey} from '@cashu/cashu-ts' +import type {MintKeys, MintKeyset} from '@cashu/cashu-ts' +import {bytesToHex} from '@noble/curves/utils.js' + +const MINT_URL = 'https://mint.test' +const AMOUNTS = [1, 2, 4, 8, 16, 32] + +/** A keyset whose id genuinely derives from its keys, at a chosen version and fee. */ +const makeKeyset = ( + seedByte: number, + opts: {active: boolean; versionByte: number; fee: number}, +) => { + const keys: Record = {} + for (let i = 0; i < AMOUNTS.length; i++) { + const priv = new Uint8Array(32) + priv[31] = seedByte + priv[30] = i + 1 + keys[String(AMOUNTS[i])] = bytesToHex(getPubKeyFromPrivKey(priv)) + } + const id = deriveKeysetId(keys, { + unit: 'sat', + input_fee_ppk: opts.fee, + versionByte: opts.versionByte, + }) + return { + meta: {id, unit: 'sat', active: opts.active, input_fee_ppk: opts.fee} as MintKeyset, + keys: {id, unit: 'sat', active: opts.active, keys} as MintKeys, + } +} + +/** What WalletStore.getOptimalKeysetId does, minus the AppError wrapping. */ +const choose = (entries: Array<{meta: MintKeyset; keys?: MintKeys}>) => { + const metas = entries.map(e => e.meta) + const keys = entries.flatMap(e => (e.keys ? [e.keys] : [])) + return KeyChain.fromCache( + MINT_URL, + 'sat', + KeyChain.mintToCacheDTO(MINT_URL, metas, keys), + ) + .getCheapestKeyset().id +} + +describe('keyset id VERSION outranks fee — the behaviour change', () => { + // A v2 keyset that charges MORE than an older v0 one. Under the wallet's old + // fee-first rule the v0 keyset won; cashu-ts picks the v2. + const oldCheap = makeKeyset(0x11, {active: true, versionByte: 0, fee: 0}) + const newExpensive = makeKeyset(0x22, {active: true, versionByte: 1, fee: 250}) + + test('the newer keyset is chosen even though it costs more', () => { + expect(choose([oldCheap, newExpensive])).toBe(newExpensive.meta.id) + }) + + test('order of the input does not matter', () => { + expect(choose([newExpensive, oldCheap])).toBe(newExpensive.meta.id) + }) + + test('the old fee-first rule would have chosen differently', () => { + // Pins the divergence itself, so this file explains a real difference rather + // than restating the library. + const byFeeThenId = [oldCheap.meta, newExpensive.meta].sort( + (a, b) => (a.input_fee_ppk ?? 0) - (b.input_fee_ppk ?? 0) || b.id.localeCompare(a.id), + )[0] + + expect(byFeeThenId.id).toBe(oldCheap.meta.id) + expect(choose([oldCheap, newExpensive])).not.toBe(byFeeThenId.id) + }) +}) + +describe('within one version, the cheaper keyset still wins', () => { + const cheap = makeKeyset(0x33, {active: true, versionByte: 0, fee: 0}) + const pricey = makeKeyset(0x44, {active: true, versionByte: 0, fee: 500}) + + test('lowest input fee is chosen', () => { + expect(choose([pricey, cheap])).toBe(cheap.meta.id) + }) +}) + +describe('candidates the selection must exclude', () => { + const active = makeKeyset(0x55, {active: true, versionByte: 0, fee: 100}) + + test('inactive keysets, even when cheaper', () => { + const inactiveCheaper = makeKeyset(0x66, {active: false, versionByte: 1, fee: 0}) + + expect(choose([active, inactiveCheaper])).toBe(active.meta.id) + }) + + test('keysets whose keys are missing — they cannot create outputs', () => { + // The old wallet rule could not express this: it selected on keyset metadata + // and only discovered the absent keys afterwards, as a separate error. + const keyless = makeKeyset(0x77, {active: true, versionByte: 1, fee: 0}) + + expect(choose([active, {meta: keyless.meta}])).toBe(active.meta.id) + }) + + test('throws when nothing is selectable', () => { + const inactive = makeKeyset(0x88, {active: false, versionByte: 0, fee: 0}) + + expect(() => choose([inactive])).toThrow() + }) +}) diff --git a/__tests__/proofSelectionRotating.test.ts b/__tests__/proofSelectionRotating.test.ts index 3b8757d1..185c270a 100644 --- a/__tests__/proofSelectionRotating.test.ts +++ b/__tests__/proofSelectionRotating.test.ts @@ -88,22 +88,42 @@ const staleCount = (ps: Proof[]) => ps.filter(p => p.id === stale.meta.id).lengt describe('when every proof is on the ACTIVE keyset', () => { // The overwhelmingly common case, and the one a device test can reach: there is - // nothing stale to prefer, so the two selectors cannot diverge. + // no stale bucket to prefer, so rotating has nothing to bias toward. + // + // NOTE: these deliberately do NOT assert that rotating and RGLI return the same + // set. RGLI is Randomized Greedy with Local Improvement — two independent calls + // on identical input may legitimately return different, equally valid sets, so + // comparing one run against another is flaky by construction. What is stable, and + // what actually matters, is that rotating adds no stale bias here. const pool = [128, 64, 32, 16, 8, 4].map(a => proof(current.meta.id, a)) - test('RGLI and rotating select the same number of inputs', () => { - const rgli = selectProofsRGLI(pool, 100, keyChain, true, false) - const rotating = selectProofsRotating(pool, 100, keyChain, true, false) + test('rotating selects only current-keyset proofs', () => { + const {send} = selectProofsRotating(pool, 100, keyChain, true, false) - expect(rotating.send.length).toBe(rgli.send.length) - expect(sum(rotating.send as Proof[])).toBe(sum(rgli.send as Proof[])) + expect(staleCount(send as Proof[])).toBe(0) }) - test('so the fee is identical — no upgrade cost for an all-current wallet', () => { - const rgli = selectProofsRGLI(pool, 100, keyChain, true, false) - const rotating = selectProofsRotating(pool, 100, keyChain, true, false) + test('rotating does not inflate the input count — no bucket to force in', () => { + // The stale-bucket behaviour below pulls in 20+ inputs. With nothing stale, + // covering 100 from these denominations never needs more than a handful, + // whichever way the randomisation falls. + for (let i = 0; i < 25; i++) { + const {send} = selectProofsRotating(pool, 100, keyChain, true, false) + expect(send.length).toBeLessThanOrEqual(4) + } + }) - expect(feeFor(rotating.send as Proof[])).toBe(feeFor(rgli.send as Proof[])) + test('and neither selector is systematically dearer than the other', () => { + // Both draw from the same denominations with no bias in play, so their fees + // land in the same small band. Asserted as a bound over repeated draws rather + // than as equality of two single runs. + for (let i = 0; i < 25; i++) { + const rgli = selectProofsRGLI(pool, 100, keyChain, true, false) + const rotating = selectProofsRotating(pool, 100, keyChain, true, false) + + expect(feeFor(rgli.send as Proof[])).toBeLessThanOrEqual(4) + expect(feeFor(rotating.send as Proof[])).toBeLessThanOrEqual(4) + } }) }) @@ -114,13 +134,13 @@ describe('when STALE-keyset proofs are present', () => { ...[64, 32, 16].map(a => proof(current.meta.id, a)), ] - test('RGLI ignores staleness and takes the cheapest set', () => { - const {send} = selectProofsRGLI(pool, 50, keyChain, true, false) - - // One 64 covers 50 + its own 1 sat fee, so RGLI spends a single input and - // leaves all 20 dust proofs stranded exactly where they were. - expect(send.length).toBe(1) - expect(staleCount(send as Proof[])).toBe(0) + test('RGLI ignores staleness and leaves most of the dust stranded', () => { + // Bounded rather than exact: RGLI is randomised, so the precise set varies. The + // stable, meaningful property is that it never force-includes the whole bucket. + for (let i = 0; i < 25; i++) { + const {send} = selectProofsRGLI(pool, 50, keyChain, true, false) + expect(staleCount(send as Proof[])).toBeLessThan(20) + } }) test('rotating force-includes the whole stale bucket', () => { @@ -131,12 +151,18 @@ describe('when STALE-keyset proofs are present', () => { }) test('which costs materially more in input fees — the upgrade consequence', () => { - const rgli = selectProofsRGLI(pool, 50, keyChain, true, false) const rotating = selectProofsRotating(pool, 50, keyChain, true, false) - // 1 sat vs 21. The user buys consolidation of 20 dust proofs for 20 extra sats. - expect(feeFor(rgli.send as Proof[])).toBe(1) + // Rotating spends the whole 20-proof bucket plus a top-up: 21 sats of input fee + // at 1000 ppk. Deterministic, because force-inclusion is not randomised. expect(feeFor(rotating.send as Proof[])).toBe(21) + + // RGLI's exact set varies, but it is always dramatically cheaper — it has no + // reason to touch the dust at all. + for (let i = 0; i < 25; i++) { + const rgli = selectProofsRGLI(pool, 50, keyChain, true, false) + expect(feeFor(rgli.send as Proof[])).toBeLessThan(feeFor(rotating.send as Proof[])) + } }) }) diff --git a/src/models/WalletStore.ts b/src/models/WalletStore.ts index 22cbc3b3..4bd82e77 100644 --- a/src/models/WalletStore.ts +++ b/src/models/WalletStore.ts @@ -19,6 +19,7 @@ import { type ProofState, type OperationCounters, type MeltPreview, + type KeyChainCache, MeltQuoteState, } from '@cashu/cashu-ts' import { JS_BUNDLE_VERSION } from '@env' @@ -122,29 +123,52 @@ export const WalletStoreModel = types const mintsStore = getRootStore(self).mintsStore return mintsStore.findByUrl(mintUrl) as Mint }, - getOptimalKeyset(mintInstance: Mint, unit: MintUnit) { - // Mirrors cashu-ts v4.7 KeyChain.getCheapestKeyset: among active keysets for - // this unit with a valid hex id (v00 `00…` or v2 `01…`; excludes deprecated - // base64 keysets that cannot create outputs), pick the lowest input fee. - const isHexKeysetId = (id: string) => /^[0-9a-f]+$/i.test(id) + /** + * The keyset a new wallet for this unit should bind to, chosen by cashu-ts. + * + * This used to mirror KeyChain.getCheapestKeyset by hand — filter to active + * hex-id keysets for the unit, sort by fee, break ties on id. That copy has + * been replaced by the real thing, for two reasons beyond removing a + * duplicate. + * + * The orderings had diverged. 4.10 (#836) sorts by keyset id VERSION first, + * then fee, then final_expiry; the copy sorted fee first and only used + * version as a tiebreak. They disagree whenever a mint prices its newer v2 + * keyset above an old v0 one, and the wallet would then keep minting on the + * retired keyset. The library's order is the intended one. + * + * It also filters on `hasKeys`, which the copy could not: it selected on + * keyset metadata and left the caller to discover that the matching keys were + * missing. A keyset without keys cannot create outputs, so excluding it up + * front turns a late failure into no selection at all. + * + * `hasHexId` additionally classifies odd-length hex ids as legacy (#840), + * which the old `/^[0-9a-f]+$/i` test accepted. + */ + getOptimalKeysetId(mintInstance: Mint, unit: MintUnit, cache?: KeyChainCache): string { + try { + // getWallet has already built the cache for loadMintFromCache and passes it + // in; other callers just want the answer and let it be built here. + const keychainCache = + cache ?? + CashuKeyChain.mintToCacheDTO( + mintInstance.mintUrl, + [...mintInstance.keysets!], + [...mintInstance.keys!], + ) - const optimalKeyset: MintKeyset | undefined = mintInstance.keysets! - .filter((k: MintKeyset) => k.unit === unit && k.active && isHexKeysetId(k.id)) - .sort((a: MintKeyset, b: MintKeyset) => { - const feeDelta = (a.input_fee_ppk ?? 0) - (b.input_fee_ppk ?? 0) - if (feeDelta !== 0) return feeDelta - // Equal fee: prefer the newer keyset version (v2 `01…` over v0 `00…`) - return b.id.localeCompare(a.id) - })[0] - - if(!optimalKeyset) { - throw new AppError(Err.VALIDATION_ERROR, 'Wallet has not any active keyset for the selected unit.', { - mintUrl: mintInstance.mintUrl, - unit + return CashuKeyChain.fromCache(mintInstance.mintUrl, unit, keychainCache) + .getCheapestKeyset() + .id + } catch (e: any) { + // cashu-ts raises CTSError for "not initialized" and "no active keyset for + // unit"; both mean the same thing to a caller here. + throw new AppError(Err.VALIDATION_ERROR, 'Wallet has no usable active keyset for the selected unit, refresh mint settings.', { + mintUrl: mintInstance.mintUrl, + unit, + message: e.message, }) } - - return optimalKeyset }, })) .actions(self => ({ @@ -189,6 +213,12 @@ export const WalletStoreModel = types // its own KeyChain read. Volatile (never persisted), so it resets on a // fresh cold start, which is exactly when we want a single fresh read. walletKeysInFlight: null as Promise | null, + // Wallet instances used by seed recovery, keyed by mintUrl|unit|keysetId. + // + // Kept OUT of `wallets`/`seedWallets` on purpose: recovery walks inactive + // keysets, and those instances must not become the ones ordinary operations + // pick up. Kept out of the snapshot too — these hold a bip39 seed. + restoreWallets: new Map(), })) .actions(self => ({ getCachedWalletKeys: flow(function* getWalletKeys() { @@ -419,12 +449,20 @@ export const WalletStoreModel = types }) } - // select keys to be used to find or create new cashu-ts wallet instance - let walletKeys: MintKeys + // Built ONCE per call and reused for both the keyset choice and, below, + // loadMintFromCache — mintToCacheDTO walks every keyset and key, so building + // it twice would double that for no reason. + const hasCachedMint = Boolean( + mintInstance.mintInfo && mintInstance.keysets?.length && mintInstance.keys?.length, + ) + const keychainCache: KeyChainCache | undefined = hasCachedMint + ? CashuKeyChain.mintToCacheDTO(mintUrl, [...mintInstance.keysets!], [...mintInstance.keys!]) + : undefined + + // Which keyset this wallet instance binds to. Also the cache key for the + // instance itself, so it has to be settled before the lookup below. + let keysetId: string if(options && options.keysetId) { - - //log.warn(mintInstance.keys) - const requestedKeys = mintInstance.keys!.find((k: MintKeys) => k.id === options.keysetId) if(!requestedKeys) { @@ -442,31 +480,25 @@ export const WalletStoreModel = types }) } - walletKeys = requestedKeys + keysetId = requestedKeys.id } else { - // if not we find active keyset with lowest fees and related keys - const activeKeyset: MintKeyset = self.getOptimalKeyset(mintInstance, unit) // throws - - log.trace('[WalletStore.getWallet] Optimal keyset for this unit', {activeKeyset, unit, mintUrl}) - - const activeKeys = mintInstance.keys!.find((k: MintKeys) => k.id === activeKeyset.id) - - if(!activeKeys) { - throw new AppError(Err.VALIDATION_ERROR, 'Wallet has no active keys for the selected unit, refresh mint settings.', { - mintUrl, + if (!keychainCache) { + throw new AppError(Err.VALIDATION_ERROR, 'Wallet has no usable active keyset for the selected unit, refresh mint settings.', { + mintUrl, unit, - activeKeysetId: activeKeyset.id }) } - - walletKeys = activeKeys + + keysetId = self.getOptimalKeysetId(mintInstance, unit, keychainCache) // throws + + log.trace('[WalletStore.getWallet] Optimal keyset for this unit', {keysetId, unit, mintUrl}) } if (options && options.withSeed) { const seedWallet: CashuWallet | undefined = self.seedWallets.find( w => w.mint.mintUrl === mintUrl && - w.keysetId === walletKeys.id + w.keysetId === keysetId ) if (seedWallet) { @@ -478,17 +510,20 @@ export const WalletStoreModel = types const newSeedWallet = new CashuWallet(cashuMint, { unit, - keysetId: walletKeys.id, + keysetId, bip39seed: seed }) - if (mintInstance.mintInfo && mintInstance.keysets?.length && mintInstance.keys?.length) { - const keychainCache = CashuKeyChain.mintToCacheDTO(mintUrl, [...mintInstance.keysets], [...mintInstance.keys]) - newSeedWallet.loadMintFromCache(mintInstance.mintInfo, keychainCache) + if (keychainCache) { + newSeedWallet.loadMintFromCache(mintInstance.mintInfo!, keychainCache) } else { yield newSeedWallet.loadMint() } + // Write back anything cashu-ts loads or repairs on its own, so the next + // instance built from the Mint model starts with it already present. + newSeedWallet.on.keychainUpdated(({cache}) => persistKeychainUpdates(mintInstance, cache)) + self.seedWallets.push(newSeedWallet) log.trace('[WalletStore.getWallet]', 'Returning NEW cashuWallet instance with seed', {mintUrl}) @@ -498,7 +533,7 @@ export const WalletStoreModel = types const wallet: CashuWallet | undefined = self.wallets.find( w => w.mint.mintUrl === mintUrl && - w.keysetId === walletKeys.id + w.keysetId === keysetId ) if (wallet) { @@ -508,16 +543,17 @@ export const WalletStoreModel = types const newWallet = new CashuWallet(cashuMint, { unit, - keysetId: walletKeys.id, + keysetId, }) - if (mintInstance.mintInfo && mintInstance.keysets?.length && mintInstance.keys?.length) { - const keychainCache = CashuKeyChain.mintToCacheDTO(mintUrl, [...mintInstance.keysets], [...mintInstance.keys]) - newWallet.loadMintFromCache(mintInstance.mintInfo, keychainCache) + if (keychainCache) { + newWallet.loadMintFromCache(mintInstance.mintInfo!, keychainCache) } else { yield newWallet.loadMint() } + newWallet.on.keychainUpdated(({cache}) => persistKeychainUpdates(mintInstance, cache)) + self.wallets.push(newWallet) log.trace('[WalletStore.getWallet]', 'Returning NEW cashuWallet instance', {mintUrl}) @@ -550,6 +586,8 @@ export const WalletStoreModel = types resetWallets() { self.seedWallets.clear() self.wallets.clear() + // Also drops the seeds these hold. + self.restoreWallets.clear() } })) .actions(self => ({ @@ -1616,19 +1654,37 @@ export const WalletStoreModel = types // PERF: Time wallet creation const perfWalletCreate = performance.now() - // Create separate CashuMint and CashuWallet instances for restore operation - // to avoid polluting the main wallet state with inactive keyset data. - // cashu-ts 3.4.1+ supports restore from inactive keysets natively. - const cashuMint = new CashuMint(mintUrl) - const cashuWallet = new CashuWallet(cashuMint, { - unit, - keysetId, - bip39seed: seed - }) + // Separate CashuMint/CashuWallet instances from the ones ordinary + // operations use, so walking inactive keysets during recovery does not + // pollute the main wallet state. cashu-ts 3.4.1+ restores from inactive + // keysets natively. + // + // REUSED across batches. SeedRecoveryScreen calls this once per + // RESTORE_INDEX_INTERVAL (50) counters, so a recovery that scans a few + // hundred indices used to build a wallet and call loadMint() — getInfo, + // getKeysets, getKeys — for every one of them. It also threw away the + // BIP-32 parent-node cache that 4.7.2 (#802) added to the deriver, which + // is per-instance and exists precisely to make repeated derivation on one + // keyset cheap. Recovery is the single most derivation-heavy path in the + // wallet, so it is the one that most wanted that cache kept. + const restoreKey = `${mintUrl}|${unit}|${keysetId}` + let cashuWallet: CashuWallet | undefined = self.restoreWallets.get(restoreKey) - yield cashuWallet.loadMint() + if (cashuWallet) { + log.info('[PERF][WalletStore.restore] CashuWallet REUSED:', { ms: (performance.now() - perfWalletCreate).toFixed(2) }) + } else { + const cashuMint = new CashuMint(mintUrl) + cashuWallet = new CashuWallet(cashuMint, { + unit, + keysetId, + bip39seed: seed + }) - log.info('[PERF][WalletStore.restore] CashuWallet created:', { ms: (performance.now() - perfWalletCreate).toFixed(2) }) + yield cashuWallet.loadMint() + self.restoreWallets.set(restoreKey, cashuWallet) + + log.info('[PERF][WalletStore.restore] CashuWallet created:', { ms: (performance.now() - perfWalletCreate).toFixed(2) }) + } const count = Math.abs(indexTo - indexFrom) log.info('[PERF][WalletStore.restore] About to restore', { indexFrom, count, keysetId }) @@ -1686,6 +1742,42 @@ export const WalletStoreModel = types }) + /** + * Persist keychain data that cashu-ts fetched on its own. + * + * cashu-ts updates its keychain mid-operation — lazily loading keys for a keyset + * whose keys we never had, or repairing an id it did not recognise with a + * loadMint(true). Nothing wrote those results back, so they died with the wallet + * instance and the NEXT wallet built from the Mint model re-fetched exactly the + * same keys. That got more relevant once WalletStore.receive stopped pre-loading + * input keyset keys itself: the lazy fetch is now the only thing that loads them. + * + * Errors are logged and swallowed. This runs INSIDE the caller's operation, and a + * cache write must never break the send or receive that happened to trigger it — + * the same rule refreshMintInfoIfStale follows. initKeyset in particular throws on + * a unit the wallet does not support, which a multi-unit mint will produce. + */ + function persistKeychainUpdates(mintInstance: Mint, cache: KeyChainCache) { + try { + if (!isAlive(mintInstance)) return + + const {keysets, keys} = CashuKeyChain.cacheToMintDTO(cache) + + mintInstance.refreshKeysets!(keysets) + mintInstance.refreshKeys!(keys) + + log.trace('[WalletStore.persistKeychainUpdates]', 'Persisted keychain update', { + mintUrl: mintInstance.mintUrl, + keysets: keysets.length, + }) + } catch (e: any) { + log.warn('[WalletStore.persistKeychainUpdates]', { + mintUrl: mintInstance.mintUrl, + error: e.message, + }) + } + } + function isOnionMint(mintUrl: string) { return new URL(mintUrl).hostname.endsWith('.onion') } diff --git a/src/screens/SeedRecoveryScreen.tsx b/src/screens/SeedRecoveryScreen.tsx index b6590476..01e48a72 100644 --- a/src/screens/SeedRecoveryScreen.tsx +++ b/src/screens/SeedRecoveryScreen.tsx @@ -135,8 +135,11 @@ export const SeedRecoveryScreen = observer(function SeedRecoveryScreen({ route } try { setSelectedMintUrl(mint.mintUrl) const allKeysets = getSnapshot(mint.keysets!) - const defaultKeyset = walletStore.getOptimalKeyset(mint, 'sat') - + // cashu-ts picks it (newest keyset version, then lowest fee); we still + // need the whole keyset here, since the screen shows its unit and id. + const defaultKeysetId = walletStore.getOptimalKeysetId(mint, 'sat') + const defaultKeyset = allKeysets.find(k => k.id === defaultKeysetId) + setSelectedKeyset(defaultKeyset) setSelectedMintKeysets(allKeysets) setStartIndex(0)