diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt index ac39759a4c..eb854749ea 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt @@ -37,7 +37,10 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import com.vitorpamplona.quartz.nip47WalletConnect.Nip47WalletConnect import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorCode +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayMethod import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PaySuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Request @@ -162,6 +165,16 @@ class AccountZapActions( ?.supportsMethod(NwcMethod.PAY) == true } + /** + * True when this account can settle a BOLT12 zap at all: an NWC wallet is + * configured and the default one advertises `pay`. The sender-side half of the + * BOLT12 route; the recipient-side half is a published kind:10058 offer. + */ + fun canZapViaBolt12(): Boolean = + account.settings.nwcWallets.value + .isNotEmpty() && + defaultWalletSupportsBolt12Pay() + /** * Sends a NIP-B1 BOLT12 zap to [recipientPubKey] over the default NWC wallet. * @@ -173,6 +186,14 @@ class AccountZapActions( * still happened; [onError] reports "paid, no receipt"). [zappedEvent] is null for * a profile zap. Requires an NWC wallet (see [hasNwcWallet]); BOLT12 zaps have no * external-wallet or LNURL fallback because only NWC returns the proof. + * + * Outcomes are split by what they say about the money: + * - [onNotPaid]: the wallet answered with an error. The wallet does the offer → + * invoice exchange itself, so a stale or dead offer lands here too. Whether a + * retry is safe depends on the code — `PAYMENT_FAILED` may be a timeout with the + * HTLC still in flight — see `Bolt12LightningFallback`. + * - [onError]: paid but no valid receipt, or nothing conclusive. Never retry. + * - [onTimeout]: the wallet never answered. Unknown state — never retry. */ suspend fun sendBolt12Zap( zappedEvent: Event?, @@ -183,15 +204,21 @@ class AccountZapActions( zapType: LnZapEvent.ZapType, // (messageResId, detail) — the caller localizes; detail carries a wallet error, if any. onError: (Int, String?) -> Unit, + // (code, detail) — the wallet refused or failed the payment; no funds moved. + onNotPaid: suspend (NwcErrorCode?, String?) -> Unit, + onTimeout: () -> Unit, onProcessed: () -> Unit, ) { // NONZAP means "pay, but publish no receipt" — settle the offer without binding // a zap intent or emitting a 9736, matching the privacy of a bolt11 NONZAP. if (zapType == LnZapEvent.ZapType.NONZAP) { - sendNwcRequest(PayMethod.create("bitcoin:?lno=$offer", amountMillisats)) { response -> + sendNwcRequest(PayMethod.create("bitcoin:?lno=$offer", amountMillisats), onTimeout) { response -> account.scope.launch { - if (response is IErrorResponseLike) onError(R.string.bolt12_payment_failed, response.errorMessage()) - onProcessed() + try { + if (response is IErrorResponseLike) onNotPaid(response.nwcErrorCode(), response.errorMessage()) + } finally { + onProcessed() + } } } return @@ -211,7 +238,7 @@ class AccountZapActions( val payerNote = Bolt12ZapBuilder.payerNote(intent) - sendNwcRequest(PayMethod.create("bitcoin:?lno=$offer", amountMillisats, payerNote)) { response -> + sendNwcRequest(PayMethod.create("bitcoin:?lno=$offer", amountMillisats, payerNote), onTimeout) { response -> account.scope.launch { // try/finally so a failure while assembling/publishing the receipt (e.g. a // remote signer error) still steps progress and surfaces an error, instead @@ -234,7 +261,7 @@ class AccountZapActions( } } - is IErrorResponseLike -> onError(R.string.bolt12_payment_failed, response.errorMessage()) + is IErrorResponseLike -> onNotPaid(response.nwcErrorCode(), response.errorMessage()) else -> onError(R.string.bolt12_zap_paid_no_receipt, null) } @@ -374,3 +401,11 @@ class AccountZapActions( return this } } + +/** The NIP-47 error code on a failed reply, whichever error shape the wallet used. */ +private fun Response.nwcErrorCode(): NwcErrorCode? = + when (this) { + is NwcErrorResponse -> error?.code + is PayInvoiceErrorResponse -> error?.code + else -> null + } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/zap/RailCapability.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/zap/RailCapability.kt index f53cc2144a..0c3b1593c8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/zap/RailCapability.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/zap/RailCapability.kt @@ -46,6 +46,12 @@ import com.vitorpamplona.quartz.nip57Zaps.splits.zapSplitSetup * through that rail — matching the existing best-effort behaviour of the * actual send paths (Lightning skips pubkeys with no `lnAddress`; on-chain * separately warns about lnAddress-only splits that can't be paid on-chain). + * + * [hasLightning] is the whole Lightning rail, not just BOLT11: a recipient with + * no `lnAddress` but a published kind:10058 BOLT12 offer counts when our own + * NWC wallet can pay offers, because the zap send path routes them over BOLT12 + * (see `ZapPaymentHandler`). The chip stays one bolt either way — which flavour + * gets used is decided at send time, not in the picker. */ @Immutable data class RailCapability( @@ -143,6 +149,12 @@ object RailCapabilityResolver { baseNote: Note, cashuState: CashuWalletState, payToEnabled: Boolean = false, + /** + * Whether our default NWC wallet can pay BOLT12 offers + * (`AccountZapActions.canZapViaBolt12`). When true, a recipient's published + * offer makes them payable on the Lightning rail even without an lnAddress. + */ + bolt12Payable: Boolean = false, ): RailCapability { val author = baseNote.author?.pubkeyHex val splits = baseNote.event?.zapSplitSetup().orEmpty() @@ -170,7 +182,9 @@ object RailCapabilityResolver { val hasLightning = lnAddressOnlySplits.isNotEmpty() || pubKeyRecipients.any { pk -> - LocalCache.getUserIfExists(pk)?.lnAddress() != null + val user = LocalCache.getUserIfExists(pk) + user?.lnAddress() != null || + (bolt12Payable && user?.bolt12Offers()?.isNotEmpty() == true) } // On-chain pays the pubkey directly; an event with only lnAddress diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/Bolt12LightningFallback.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/Bolt12LightningFallback.kt new file mode 100644 index 0000000000..4f73a3b2c2 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/Bolt12LightningFallback.kt @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service + +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorCode + +/** + * Decides whether a BOLT12 zap the wallet refused should be re-sent as a BOLT11 zap. + * + * Only ever consulted for a NIP-47 *error* reply. An allowlist, because not every + * error means no money moved: NIP-47 defines `PAYMENT_FAILED` as "may be due to a + * timeout, exhausting all routes, insufficient capacity or similar", and a wallet + * that gave up on a payment whose HTLC is still in flight can see it settle later. + * Retrying on that, or on the catch-all `INTERNAL` / `OTHER` / no-code replies, + * could pay the recipient twice. Only refusals the wallet raises *before* it + * attempts a payment qualify — the offer could not be resolved or has expired, the + * request was rejected as malformed, or our wallet does not handle `lno` at all. + * Those are the "recipient's configuration is stale" cases the fallback exists for. + * Refusals about our own wallet (balance, quota, permissions) are out too: BOLT11 + * through the same wallet would fail identically and only add a second error. + */ +object Bolt12LightningFallback { + /** Refusals raised before any payment attempt, about the offer or the instruction. */ + private val offerSideCodes = + setOf( + NwcErrorCode.EXPIRED, + NwcErrorCode.NOT_FOUND, + NwcErrorCode.BAD_REQUEST, + NwcErrorCode.NOT_IMPLEMENTED, + NwcErrorCode.UNSUPPORTED_PAYMENT_INSTRUCTION, + NwcErrorCode.UNSUPPORTED_NETWORK, + ) + + /** True when a refusal with [code] (null when the wallet sent none) should be retried over BOLT11. */ + fun shouldRetry(code: NwcErrorCode?): Boolean = code in offerSideCodes +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt index cd44293231..0d08176f3c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.amethyst.ui.nwc.nwcTimeoutMessage import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.quartz.experimental.clink.pointers.NDebit import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorCode import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcTransactionMetadata import com.vitorpamplona.quartz.nip53LiveActivities.streaming.LiveActivitiesEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent @@ -43,6 +44,7 @@ import com.vitorpamplona.quartz.nip57Zaps.splits.ZapSplitSetupLnAddress import com.vitorpamplona.quartz.nip57Zaps.splits.zapSplitSetup import com.vitorpamplona.quartz.nip57Zaps.validate.LnurlForm import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppDefinitionEvent +import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.mapNotNullAsync import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toImmutableList @@ -84,11 +86,17 @@ class ZapPaymentHandler( val user: User? = null, ) - /** A recipient routed over BOLT12 (NIP-B1): they publish a kind:10058 [offer] and we hold an NWC wallet. */ + /** + * A recipient routed over BOLT12 (NIP-B1): they publish a kind:10058 [offer] and we + * hold an NWC wallet. [lnAddress] is their BOLT11 route, kept so a refused offer can + * fall back to a regular zap (see [payViaBolt12]); null when they publish none. + */ data class Bolt12Recipient( val user: User, val offer: String, val weight: Double = 1.0, + val lnAddress: String? = null, + val relay: NormalizedRelayUrl? = null, ) suspend fun zap( @@ -167,16 +175,13 @@ class ZapPaymentHandler( // BOLT12 when our default NWC wallet advertises the nwc#2 `pay` method (needed for // the payer proof). Otherwise — no wallet, or a wallet without `pay` — the recipient // stays on lightning, so an unsupported wallet degrades gracefully instead of erroring. - val canBolt12 = - account.settings.nwcWallets.value - .isNotEmpty() && - account.zaps.defaultWalletSupportsBolt12Pay() + val canBolt12 = account.zaps.canZapViaBolt12() val bolt12Recipients = unverifiedZapsToSend.mapNotNull { val user = it.user if (canBolt12 && it.bolt12Offer != null && user != null) { - Bolt12Recipient(user, it.bolt12Offer, it.weight) + Bolt12Recipient(user, it.bolt12Offer, it.weight, it.lnAddress, it.relay) } else { null } @@ -233,48 +238,20 @@ class ZapPaymentHandler( // --- Lightning lane ----------------------------------------------------------- if (zapsToSend.isNotEmpty()) { - val splitZapRequests = signAllZapRequests(note, pollOption, message, zapType, zapsToSend, amountMilliSats, totalWeight) - - if (splitZapRequests.isNotEmpty()) { - onProgress(0.05f) - - val payables = - assembleAllInvoices( - requests = splitZapRequests, - totalAmountMilliSats = amountMilliSats, - message = message, - okHttpClient = okHttpClient, - onError = onError, - onProgress = { onProgress(it * 0.7f + 0.05f) }, - context = context, - totalWeight = totalWeight, - ) - - if (payables.isNotEmpty()) { - onProgress(0.75f) - - // Route through the user's selected default payment source. A CLINK debit takes - // precedence over NWC when it is the chosen default; NWC-only users are unaffected - // (defaultPaymentSource() resolves to their NWC wallet). No source -> wallet app. - when (val source = account.settings.defaultPaymentSource()) { - is PaymentSource.ClinkDebit -> { - payViaClinkDebit(payables, source.wallet.pointer, onError = onError, onProgress = { - onProgress(it * 0.25f + 0.75f) - }, context) - } - - is PaymentSource.Nwc -> { - payViaNWC(payables, note, onError = onError, onProgress = { - onProgress(it * 0.25f + 0.75f) // keeps within range. - }, context) - } - - null -> { - onPayViaIntent(payables.toImmutableList()) - } - } - } - } + zapOverLightning( + zapsToSend = zapsToSend, + note = note, + pollOption = pollOption, + message = message, + zapType = zapType, + totalAmountMilliSats = amountMilliSats, + totalWeight = totalWeight, + okHttpClient = okHttpClient, + onError = onError, + onProgress = onProgress, + onPayViaIntent = onPayViaIntent, + context = context, + ) } // --- BOLT12 lane -------------------------------------------------------------- @@ -282,12 +259,15 @@ class ZapPaymentHandler( payViaBolt12( recipients = bolt12Recipients, note = note, + pollOption = pollOption, totalAmountMilliSats = amountMilliSats, totalWeight = totalWeight, message = message, zapType = zapType, + okHttpClient = okHttpClient, onError = onError, onProgress = { onProgress(it * 0.25f + 0.75f) }, + onPayViaIntent = onPayViaIntent, context = context, ) } @@ -295,6 +275,68 @@ class ZapPaymentHandler( onProgress(1f) } + /** + * The BOLT11 lane: signs one kind 9734 per recipient, fetches each invoice from + * the recipient's LNURL, then settles through the default payment source. Used + * for every lnAddress recipient of a zap, and again by [payViaBolt12] for a + * recipient whose offer the wallet refused. [onProgress] spans 0.05..1.0. + */ + private suspend fun zapOverLightning( + zapsToSend: List, + note: Note, + pollOption: Int?, + message: String, + zapType: LnZapEvent.ZapType, + totalAmountMilliSats: Long, + totalWeight: Double, + okHttpClient: (String) -> OkHttpClient, + onError: (String, String, User?) -> Unit, + onProgress: (percent: Float) -> Unit, + onPayViaIntent: (ImmutableList) -> Unit, + context: Context, + ) { + val splitZapRequests = signAllZapRequests(note, pollOption, message, zapType, zapsToSend, totalAmountMilliSats, totalWeight) + if (splitZapRequests.isEmpty()) return + + onProgress(0.05f) + + val payables = + assembleAllInvoices( + requests = splitZapRequests, + totalAmountMilliSats = totalAmountMilliSats, + message = message, + okHttpClient = okHttpClient, + onError = onError, + onProgress = { onProgress(it * 0.7f + 0.05f) }, + context = context, + totalWeight = totalWeight, + ) + if (payables.isEmpty()) return + + onProgress(0.75f) + + // Route through the user's selected default payment source. A CLINK debit takes + // precedence over NWC when it is the chosen default; NWC-only users are unaffected + // (defaultPaymentSource() resolves to their NWC wallet). No source -> wallet app. + when (val source = account.settings.defaultPaymentSource()) { + is PaymentSource.ClinkDebit -> { + payViaClinkDebit(payables, source.wallet.pointer, onError = onError, onProgress = { + onProgress(it * 0.25f + 0.75f) + }, context) + } + + is PaymentSource.Nwc -> { + payViaNWC(payables, note, onError = onError, onProgress = { + onProgress(it * 0.25f + 0.75f) // keeps within range. + }, context) + } + + null -> { + onPayViaIntent(payables.toImmutableList()) + } + } + } + private fun calculateZapValue( amountMilliSats: Long, weight: Double, @@ -463,21 +505,41 @@ class ZapPaymentHandler( * and (if the returned proof validates) publishes a 9736 zap — see * [Account.sendBolt12Zap]. Fire-and-forget like [payViaNWC]: dispatch is optimistic * and settlement/errors surface later through the async NWC response. + * + * When the wallet refuses the offer before attempting a payment — it resolves the + * offer itself, so a stale, expired or unsupported offer fails there — and the + * recipient also publishes a lightning address, the same share is re-sent as a + * regular BOLT11 zap through [zapOverLightning], silently. The BOLT12 error is + * shown when there is no BOLT11 route or when the refusal does not qualify + * ([Bolt12LightningFallback]: a failed payment attempt may still settle, and a + * refusal about our own wallet would repeat on BOLT11). A paid-but-no-receipt + * outcome and a wallet that never answers are never retried either. */ suspend fun payViaBolt12( recipients: List, note: Note, + pollOption: Int?, totalAmountMilliSats: Long, totalWeight: Double, message: String, zapType: LnZapEvent.ZapType, + okHttpClient: (String) -> OkHttpClient, onError: (String, String, User?) -> Unit, onProgress: (percent: Float) -> Unit, + onPayViaIntent: (ImmutableList) -> Unit, context: Context, ) { val progress = PaymentProgress(recipients.size, onProgress) mapNotNullAsync(recipients) { recipient: Bolt12Recipient -> + fun reportBolt12Error( + msgRes: Int, + detail: String?, + ) { + val msg = if (detail != null) stringRes(context, msgRes, detail) else stringRes(context, msgRes) + onError(stringRes(context, R.string.bolt12_zap_error), msg, recipient.user) + } + account.zaps.sendBolt12Zap( zappedEvent = note.event, recipientPubKey = recipient.user.pubkeyHex, @@ -485,9 +547,46 @@ class ZapPaymentHandler( amountMillisats = calculateZapValue(totalAmountMilliSats, recipient.weight, totalWeight), message = message, zapType = zapType, - onError = { msgRes, detail -> - val msg = if (detail != null) stringRes(context, msgRes, detail) else stringRes(context, msgRes) - onError(stringRes(context, R.string.bolt12_zap_error), msg, recipient.user) + onError = ::reportBolt12Error, + onNotPaid = { code, detail -> + val lnAddress = recipient.lnAddress + if (lnAddress != null && Bolt12LightningFallback.shouldRetry(code)) { + Log.i("ZapPaymentHandler") { "BOLT12 offer refused ($code: $detail); re-sending over BOLT11 to $lnAddress" } + try { + zapOverLightning( + zapsToSend = listOf(MyZapSplitSetup(lnAddress, recipient.weight, recipient.relay, recipient.user)), + note = note, + pollOption = pollOption, + message = message, + zapType = zapType, + totalAmountMilliSats = totalAmountMilliSats, + totalWeight = totalWeight, + okHttpClient = okHttpClient, + onError = onError, + // The zap's own progress finished when the BOLT12 request was + // dispatched; the retry settles in the background like NWC does. + onProgress = {}, + onPayViaIntent = onPayViaIntent, + context = context, + ) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // Nothing was paid on either rail. Report it as the lightning failure it + // is, rather than letting [sendBolt12Zap]'s catch call it "paid, no receipt". + Log.w("ZapPaymentHandler", "BOLT11 fallback failed after a refused BOLT12 offer", e) + onError(stringRes(context, R.string.error_dialog_zap_error), e.message ?: e.toString(), recipient.user) + } + } else { + // bolt12_payment_failed always formats a detail; a wallet may send neither + // message nor a recognised code. + reportBolt12Error(R.string.bolt12_payment_failed, detail ?: (code ?: NwcErrorCode.OTHER).name) + } + }, + onTimeout = { + // No response callback will fire, so account for the settlement step here. + reportBolt12Error(R.string.bolt12_payment_failed, nwcTimeoutMessage(context)) + progress.step() }, onProcessed = { progress.step() }, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/bolt12Offers/Bolt12OffersScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/bolt12Offers/Bolt12OffersScreen.kt index 354d0a435e..0a0ef7d197 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/bolt12Offers/Bolt12OffersScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/bolt12Offers/Bolt12OffersScreen.kt @@ -65,6 +65,7 @@ import com.vitorpamplona.amethyst.ui.insets.imePaddingSafe import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.topbars.SavingTopBar import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel +import com.vitorpamplona.amethyst.ui.screen.loggedIn.profile.header.abbreviateBolt12Offer import com.vitorpamplona.amethyst.ui.screen.loggedIn.relays.SettingsCategory import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.ButtonBorder @@ -191,7 +192,7 @@ fun Bolt12OfferEntry( horizontalArrangement = Arrangement.SpaceAround, ) { Text( - text = "${offer.take(14)}…${offer.takeLast(6)}", + text = abbreviateBolt12Offer(offer), style = MaterialTheme.typography.bodyMedium, fontFamily = FontFamily.Monospace, maxLines = 1, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt index a394481d99..b469b4091e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt @@ -216,6 +216,7 @@ import com.vitorpamplona.quartz.nip30CustomEmoji.CustomEmoji import com.vitorpamplona.quartz.nip57Zaps.zapraiser.zapraiserAmount import com.vitorpamplona.quartz.nip61Nutzaps.info.NutzapInfoEvent import com.vitorpamplona.quartz.nipA0VoiceMessages.BaseVoiceEvent +import com.vitorpamplona.quartz.nipB1Bolt12Zaps.offer.Bolt12OfferListEvent import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.ImmutableSet @@ -225,6 +226,7 @@ import kotlinx.collections.immutable.toImmutableList import kotlinx.collections.immutable.toImmutableSet import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.serialization.json.Json @@ -1480,10 +1482,15 @@ fun zapClick( choices.size == 1 -> { // One-tap fast path is Lightning-only. If the recipient can't - // receive Lightning (no lud16/lud06), firing a zap here would just - // fail — open the picker instead so the rail-aware chip can route to - // cashu / on-chain / reload. - val caps = RailCapabilityResolver.peek(baseNote, accountViewModel.account.cashuWalletState) + // receive Lightning (no lud16/lud06, and no BOLT12 offer our wallet + // can pay), firing a zap here would just fail — open the picker + // instead so the rail-aware chip can route to cashu / on-chain / reload. + val caps = + RailCapabilityResolver.peek( + baseNote, + accountViewModel.account.cashuWalletState, + bolt12Payable = accountViewModel.account.zaps.canZapViaBolt12(), + ) if (caps.hasLightning) { onZapStarts() accountViewModel.zap( @@ -2124,16 +2131,26 @@ fun observeZapRailCapability( ): RailCapability { val cashuState = accountViewModel.account.cashuWalletState val author = baseNote.author - // These four are deliberately read only to drive the recompute below — do NOT - // delete them as "unused". Each observe* call ALSO subscribes the relay fetch - // (so a not-yet-seen kind:0 / kind:10019 gets pulled in while the popup is - // open), and each value is a remember() key so railCapability recomputes when - // it arrives. RailCapabilityResolver.peek re-reads everything itself; these - // just say *when* to re-run it. + // Every value below up to `showOnchainWallet` is deliberately read only to drive + // the recompute — do NOT delete them as "unused". Each observe* call ALSO + // subscribes the relay fetch (so a not-yet-seen kind:0 / kind:10019 / kind:10058 + // gets pulled in while the popup is open), and each value is a remember() key so + // railCapability recomputes when it arrives. RailCapabilityResolver.peek re-reads + // everything itself; these just say *when* to re-run it. val cashuMints by cashuState.mints.collectAsStateWithLifecycle() val cashuEntries by cashuState.tokenEntries.collectAsStateWithLifecycle() val recipientInfo = author?.let { observeUserInfo(it, accountViewModel).value } val nutzapInfo = author?.let { observeNoteEvent(it.nutzapInfoNote, accountViewModel).value } + // BOLT12 route inputs, same contract: the recipient's kind:10058 offer list + // (rides in UserMetadataForKeyKinds beside kind:0) and our default NWC wallet, + // whose `pay` support decides whether that offer makes them Lightning-payable. + val bolt12OfferList = author?.let { observeNoteEvent(it.bolt12OfferListNote, accountViewModel).value } + val nip47State = accountViewModel.account.nip47SignerState + val defaultWalletUri by nip47State.defaultWalletUri.collectAsStateWithLifecycle() + // The wallet's kind:13194 info (its `pay` support) is a plain cache read inside + // canZapViaBolt12(); this counter is what recomputes when it lands after opening. + val walletInfoUpdates by remember(nip47State) { nip47State.infoCache?.updates ?: MutableStateFlow(0) } + .collectAsStateWithLifecycle() // Honors the user's "show on-chain wallet" preference: off hides the on-chain // rail from the zap chips too, matching the wallet screen, profile chips, and // Send Payment screen. @@ -2180,11 +2197,20 @@ fun observeZapRailCapability( cashuEntries, recipientInfo, nutzapInfo, + bolt12OfferList, + defaultWalletUri, + walletInfoUpdates, showPayToChip, recipientPayTo, payToApps, ) { - val rc = RailCapabilityResolver.peek(baseNote, cashuState, showPayToChip) + val rc = + RailCapabilityResolver.peek( + baseNote, + cashuState, + showPayToChip, + bolt12Payable = accountViewModel.account.zaps.canZapViaBolt12(), + ) if (onchainEnabled) { rc.copy(onchainMaxSpendableSats = onchainFunds?.maxSpendableSats) } else { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index d19db54b1e..c53ba18afc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1196,7 +1196,7 @@ class AccountViewModel( .isNotEmpty() /** True when a BOLT12 offer can be paid in-app: an NWC wallet is set and advertises `pay` (nwc#2). */ - fun canPayBolt12ViaNwc(): Boolean = hasNwcWallet() && account.zaps.defaultWalletSupportsBolt12Pay() + fun canPayBolt12ViaNwc(): Boolean = account.zaps.canZapViaBolt12() /** * Pays a recipient's BOLT12 [offer] over the default NWC wallet using the nwc#2 diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/Bolt12PayButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/Bolt12OffersDialog.kt similarity index 82% rename from amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/Bolt12PayButton.kt rename to amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/Bolt12OffersDialog.kt index f0a50ef5a5..e377c20473 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/Bolt12PayButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/Bolt12OffersDialog.kt @@ -31,7 +31,6 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.Button -import androidx.compose.material3.FilledTonalButton import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField @@ -55,80 +54,19 @@ import androidx.compose.ui.window.Dialog import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols -import com.vitorpamplona.amethyst.commons.model.User import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.bolt12_pay_with_wallet import com.vitorpamplona.amethyst.commons.resources.bolt12_payment_amount_sats -import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription -import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNoteEvent import com.vitorpamplona.amethyst.ui.components.M3ActionDialog import com.vitorpamplona.amethyst.ui.components.M3ActionSection import com.vitorpamplona.amethyst.ui.components.util.setText -import com.vitorpamplona.amethyst.ui.note.LoadAddressableNote import com.vitorpamplona.amethyst.ui.note.payViaBolt12Intent import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.ButtonBorder import com.vitorpamplona.amethyst.ui.theme.Size20Modifier -import com.vitorpamplona.amethyst.ui.theme.ZeroPadding -import com.vitorpamplona.quartz.nipB1Bolt12Zaps.offer.Bolt12OfferListEvent import kotlinx.coroutines.launch -@Composable -fun Bolt12PayButton( - user: User, - accountViewModel: AccountViewModel, -) { - val address = - remember(user.pubkeyHex) { - Bolt12OfferListEvent.createAddress(user.pubkeyHex) - } - - LoadAddressableNote(address, accountViewModel) { note -> - if (note != null) { - EventFinderFilterAssemblerSubscription(note, accountViewModel) - val event by observeNoteEvent(note, accountViewModel) - val offers = - remember(event) { - event?.offers() ?: emptyList() - } - if (offers.isNotEmpty()) { - Bolt12PayButtonWithOffers(offers, accountViewModel) - } - } - } -} - -@Composable -fun Bolt12PayButtonWithOffers( - offers: List, - accountViewModel: AccountViewModel, -) { - var expanded by remember { mutableStateOf(false) } - - FilledTonalButton( - modifier = - Modifier - .padding(horizontal = 3.dp) - .width(50.dp), - onClick = { expanded = true }, - contentPadding = ZeroPadding, - ) { - Icon( - symbol = MaterialSymbols.Bolt, - contentDescription = stringRes(R.string.bolt12_offers), - ) - } - - if (expanded) { - Bolt12OffersDialog( - offers = offers, - accountViewModel = accountViewModel, - onDismiss = { expanded = false }, - ) - } -} - @Composable fun Bolt12OffersDialog( offers: List, @@ -210,7 +148,7 @@ private fun Bolt12OfferRow( .padding(horizontal = 16.dp, vertical = 10.dp), ) { Text( - text = "${offer.take(14)}…${offer.takeLast(6)}", + text = abbreviateBolt12Offer(offer), style = MaterialTheme.typography.bodyMedium, fontFamily = FontFamily.Monospace, color = MaterialTheme.colorScheme.onSurface, @@ -295,3 +233,10 @@ private fun Bolt12NwcAmountDialog( } } } + +/** + * The short form every BOLT12 surface shows for an `lno1…` offer: enough of the + * head to recognise the prefix, the tail to tell two offers apart. Shared by the + * profile chip, this dialog and the offers settings screen so they agree. + */ +fun abbreviateBolt12Offer(offer: String): String = "${offer.take(14)}\u2026${offer.takeLast(6)}" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/PaymentButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/PaymentTargetsDialog.kt similarity index 76% rename from amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/PaymentButton.kt rename to amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/PaymentTargetsDialog.kt index c35e73e5c4..86bea970f0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/PaymentButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/PaymentTargetsDialog.kt @@ -32,7 +32,6 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.material3.FilledTonalButton import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface @@ -54,98 +53,20 @@ import androidx.compose.ui.window.Dialog import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols -import com.vitorpamplona.amethyst.commons.model.User import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.no_payment_targets_message import com.vitorpamplona.amethyst.commons.resources.show_qr -import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription -import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNoteEvent import com.vitorpamplona.amethyst.ui.components.M3ActionDialog import com.vitorpamplona.amethyst.ui.components.M3ActionRow import com.vitorpamplona.amethyst.ui.components.M3ActionSection import com.vitorpamplona.amethyst.ui.components.util.setText -import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.note.ErrorMessageDialog -import com.vitorpamplona.amethyst.ui.note.LoadAddressableNote -import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.QrCodeDrawer import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.Size20Modifier -import com.vitorpamplona.amethyst.ui.theme.ZeroPadding import com.vitorpamplona.quartz.experimental.nipA3.PaymentTarget -import com.vitorpamplona.quartz.experimental.nipA3.PaymentTargetsEvent import kotlinx.coroutines.launch -@Composable -fun PaymentButton( - user: User, - accountViewModel: AccountViewModel, - nav: INav, -) { - val address = - remember(user.pubkeyHex) { - PaymentTargetsEvent.createAddress(user.pubkeyHex) - } - - LoadAddressableNote(address, accountViewModel) { note -> - if (note != null) { - EventFinderFilterAssemblerSubscription(note, accountViewModel) - val event by observeNoteEvent(note, accountViewModel) - val targets = - remember(event) { - event?.paymentTargets() ?: emptyList() - } - if (targets.isNotEmpty()) { - PaymentButtonWithTargets(user, targets, nav) - } - } - } -} - -@Composable -fun PaymentButtonWithTargets( - user: User, - targets: List, - nav: INav, -) { - var expanded by remember { mutableStateOf(false) } - - FilledTonalButton( - modifier = - Modifier - .padding(horizontal = 3.dp) - .width(50.dp), - onClick = { expanded = true }, - contentPadding = ZeroPadding, - ) { - Icon( - symbol = MaterialSymbols.AccountBalanceWallet, - contentDescription = stringRes(R.string.payment_targets), - ) - } - - if (expanded) { - PaymentTargetsDialog( - targets = targets, - onDismiss = { expanded = false }, - payInApp = { target -> - // Targets one of the user's wallets can pay (lightning, bitcoin) - // go to the Send Payment screen, which collects the amount and - // confirms in place — no extra dialog. Returns false when no - // in-app wallet applies so the dialog falls back to payto://. - val route = inAppPaymentRouteFor(user.pubkeyHex, target) - if (route != null) { - expanded = false - nav.nav(route) - true - } else { - false - } - }, - ) - } -} - @Composable fun PaymentTargetsDialog( targets: List, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfileActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfileActions.kt index 3501530049..5a1a29969b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfileActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfileActions.kt @@ -40,10 +40,6 @@ fun ProfileActions( ) { MessageButton(baseUser, accountViewModel, nav) - PaymentButton(baseUser, accountViewModel, nav) - - Bolt12PayButton(baseUser, accountViewModel) - val isMe by remember(accountViewModel) { derivedStateOf { accountViewModel.userProfile() == baseUser } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfilePaymentRailChips.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfilePaymentRailChips.kt index eeda4bccf3..43e0949b2c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfilePaymentRailChips.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/header/ProfilePaymentRailChips.kt @@ -36,8 +36,10 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -56,6 +58,7 @@ import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.model.User import com.vitorpamplona.amethyst.commons.model.nip01Core.UserInfo import com.vitorpamplona.amethyst.commons.resources.Res +import com.vitorpamplona.amethyst.commons.resources.bolt12_lightning_offer import com.vitorpamplona.amethyst.commons.resources.clink_lightning_offer import com.vitorpamplona.amethyst.commons.resources.send_payment_method_cashu import com.vitorpamplona.amethyst.commons.resources.send_payment_method_lightning @@ -76,6 +79,7 @@ import com.vitorpamplona.amethyst.ui.theme.Size16Modifier import com.vitorpamplona.quartz.experimental.clink.pointers.NOffer import com.vitorpamplona.quartz.experimental.nipA3.PaymentTarget import com.vitorpamplona.quartz.experimental.nipA3.PaymentTargetsEvent +import com.vitorpamplona.quartz.nipB1Bolt12Zaps.offer.Bolt12OfferListEvent import com.vitorpamplona.quartz.nipBCOnchainZaps.taproot.TaprootAddress import kotlinx.coroutines.launch import androidx.compose.material3.Icon as M3Icon @@ -84,9 +88,10 @@ private val CashuPurple = Color(0xFFA855F7) /** * One FlowRow of tappable chips for every way to pay this profile: Lightning - * (lud16, long-press copies the address), the CLINK offer, the NIP-BC on-chain - * wallet, Cashu nutzaps (shown only when the logged-in user's cashu wallet - * shares a mint the recipient accepts), and the NIP-A3 payment-target chips. + * (lud16, long-press copies the address), the CLINK offer, the NIP-B1 BOLT12 + * offers (one chip each), the NIP-BC on-chain wallet, Cashu nutzaps (shown only + * when the logged-in user's cashu wallet shares a mint the recipient accepts), + * and the NIP-A3 payment-target chips. * A single FlowRow so the chips wrap together with uniform spacing instead of * stacking as separately padded rows. */ @@ -113,35 +118,57 @@ fun DisplayPaymentRailChips( .collectAsStateWithLifecycle() val onchainAvailable = showOnchainWallet && LocalCache.onchainBackend != null - val address = + val targetsAddress = remember(baseUser.pubkeyHex) { PaymentTargetsEvent.createAddress(baseUser.pubkeyHex) } + val bolt12Address = + remember(baseUser.pubkeyHex) { + Bolt12OfferListEvent.createAddress(baseUser.pubkeyHex) + } - LoadAddressableNote(address, accountViewModel) { note -> + LoadAddressableNote(targetsAddress, accountViewModel) { targetsNote -> val targets = - if (note != null) { - EventFinderFilterAssemblerSubscription(note, accountViewModel) - val event by observeNoteEvent(note, accountViewModel) + if (targetsNote != null) { + EventFinderFilterAssemblerSubscription(targetsNote, accountViewModel) + val event by observeNoteEvent(targetsNote, accountViewModel) remember(event) { event?.paymentTargets() ?: emptyList() } } else { emptyList() } - if (lud16.isNullOrEmpty() && clinkOffer == null && !onchainAvailable && cashuMintUrl == null && targets.isEmpty()) { - return@LoadAddressableNote - } + LoadAddressableNote(bolt12Address, accountViewModel) { bolt12Note -> + val bolt12Offers = + if (bolt12Note != null) { + EventFinderFilterAssemblerSubscription(bolt12Note, accountViewModel) + val event by observeNoteEvent(bolt12Note, accountViewModel) + remember(event) { event?.offers() ?: emptyList() } + } else { + emptyList() + } - RailAndTargetChips( - baseUser = baseUser, - lud16 = lud16, - clinkOffer = clinkOffer, - onchainAvailable = onchainAvailable, - cashuMintUrl = cashuMintUrl, - targets = targets, - accountViewModel = accountViewModel, - nav = nav, - ) + if (lud16.isNullOrEmpty() && + clinkOffer == null && + bolt12Offers.isEmpty() && + !onchainAvailable && + cashuMintUrl == null && + targets.isEmpty() + ) { + return@LoadAddressableNote + } + + RailAndTargetChips( + baseUser = baseUser, + lud16 = lud16, + clinkOffer = clinkOffer, + bolt12Offers = bolt12Offers, + onchainAvailable = onchainAvailable, + cashuMintUrl = cashuMintUrl, + targets = targets, + accountViewModel = accountViewModel, + nav = nav, + ) + } } } @@ -151,6 +178,7 @@ private fun RailAndTargetChips( baseUser: User, lud16: String?, clinkOffer: NOffer?, + bolt12Offers: List, onchainAvailable: Boolean, cashuMintUrl: String?, targets: List, @@ -161,6 +189,9 @@ private fun RailAndTargetChips( nav.nav(Route.SendPayment(baseUser.pubkeyHex, method.routeKey)) } + // The BOLT12 offer whose pay/copy dialog is open, if any. + var bolt12DialogOffer by remember { mutableStateOf(null) } + FlowRow( horizontalArrangement = Arrangement.spacedBy(6.dp), verticalArrangement = Arrangement.spacedBy(6.dp), @@ -199,6 +230,23 @@ private fun RailAndTargetChips( } } + bolt12Offers.forEach { offer -> + ProfilePaymentChip( + color = BitcoinOrange, + label = stringRes(Res.string.bolt12_lightning_offer), + detail = remember(offer) { abbreviateBolt12Offer(offer) }, + copyValue = offer, + onClick = { bolt12DialogOffer = offer }, + ) { + Icon( + symbol = MaterialSymbols.Bolt, + contentDescription = null, + tint = BitcoinOrange, + modifier = Size16Modifier, + ) + } + } + if (onchainAvailable) { ProfilePaymentChip( color = BitcoinOrange, @@ -239,6 +287,14 @@ private fun RailAndTargetChips( PaymentTargetChip(baseUser, target, accountViewModel, nav) } } + + bolt12DialogOffer?.let { offer -> + Bolt12OffersDialog( + offers = listOf(offer), + accountViewModel = accountViewModel, + onDismiss = { bolt12DialogOffer = null }, + ) + } } /** diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/Bolt12LightningFallbackTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/Bolt12LightningFallbackTest.kt new file mode 100644 index 0000000000..98fe9c90fc --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/Bolt12LightningFallbackTest.kt @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service + +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorCode +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class Bolt12LightningFallbackTest { + @Test + fun offerSideRefusalsRetryOverLightning() { + listOf( + NwcErrorCode.EXPIRED, + NwcErrorCode.NOT_FOUND, + NwcErrorCode.BAD_REQUEST, + NwcErrorCode.NOT_IMPLEMENTED, + NwcErrorCode.UNSUPPORTED_PAYMENT_INSTRUCTION, + NwcErrorCode.UNSUPPORTED_NETWORK, + ).forEach { code -> + assertTrue("$code should fall back to BOLT11", Bolt12LightningFallback.shouldRetry(code)) + } + } + + @Test + fun aFailedOrAmbiguousPaymentNeverRetries() { + // PAYMENT_FAILED "may be due to a timeout" (NIP-47): the HTLC can still settle. + // The catch-alls and a reply with no code say nothing about whether funds moved. + listOf( + NwcErrorCode.PAYMENT_FAILED, + NwcErrorCode.INTERNAL, + NwcErrorCode.OTHER, + null, + ).forEach { code -> + assertFalse("$code may already have paid; a retry could double-spend", Bolt12LightningFallback.shouldRetry(code)) + } + } + + @Test + fun senderSideRefusalsDoNotRetry() { + listOf( + NwcErrorCode.INSUFFICIENT_BALANCE, + NwcErrorCode.QUOTA_EXCEEDED, + NwcErrorCode.RATE_LIMITED, + NwcErrorCode.RESTRICTED, + NwcErrorCode.UNAUTHORIZED, + NwcErrorCode.UNSUPPORTED_ENCRYPTION, + ).forEach { code -> + assertFalse("$code is about our wallet, BOLT11 would fail the same way", Bolt12LightningFallback.shouldRetry(code)) + } + } +} diff --git a/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index 74b279d621..dbb4efb401 100644 --- a/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -107,6 +107,26 @@ actual class SecureKeyStorage private actual constructor() { } } + /** + * Android backend: EncryptedSharedPreferences.contains + getString has no + * ambiguous-error state comparable to macOS Keychain user-cancel/deny, so + * "key not present" and "key present" are the only two null outcomes. + * Any thrown exception is a genuine failure and propagates. + */ + actual suspend fun getPrivateKeyOrThrow(npub: String): String? = + withContext(Dispatchers.IO) { + try { + val key = KEY_PREFIX + npub + if (!encryptedPrefs.contains(key)) { + null + } else { + encryptedPrefs.getString(key, null) + } + } catch (e: Exception) { + throw SecureStorageException("Failed to retrieve private key", e) + } + } + actual suspend fun deletePrivateKey(npub: String): Boolean = withContext(Dispatchers.IO) { try { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index efb9d7342e..2ee7e124cf 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -76,12 +76,34 @@ expect class SecureKeyStorage private constructor() { * **Security Warning:** The returned String cannot be securely zeroed from memory (JVM limitation). * Dereference the returned value immediately after use to minimize exposure time. * + * Callers that MUST distinguish "key does not exist" from "backend refused/locked/failed" + * (for example, before generating a replacement key on disk) should use + * [getPrivateKeyOrThrow] instead. This method returns null on any error and cannot + * safely be used as an "is this the first launch?" probe. + * * @param npub The public key in npub (Bech32) format * @return The private key in hexadecimal format, or null if not found * @throws SecureStorageException if retrieval operation fails */ suspend fun getPrivateKey(npub: String): String? + /** + * Retrieves a private key for the given npub, distinguishing "definitively absent" + * from any other failure mode. + * + * On success, returns the key. When the backend confirms the item does not exist, + * returns null. Any other outcome (backend unavailable, user denied the OS prompt, + * keychain locked, I/O error) throws [SecureStorageException]. This is the safe + * primitive for compare-and-swap style flows where a null must not be interpreted + * as permission to generate and persist a replacement. + * + * @param npub The public key in npub (Bech32) format + * @return The private key in hexadecimal format, or null only when the backend + * confirms the item does not exist + * @throws SecureStorageException on any ambiguous or transient failure + */ + suspend fun getPrivateKeyOrThrow(npub: String): String? + /** * Deletes a private key for the given npub. * diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip47WalletConnect/NwcInfoCache.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip47WalletConnect/NwcInfoCache.kt index 51c1c1e6df..3782bdfe66 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip47WalletConnect/NwcInfoCache.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip47WalletConnect/NwcInfoCache.kt @@ -30,6 +30,10 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch /** @@ -75,6 +79,16 @@ class NwcInfoCache( private val cache = ConcurrentMap() + private val updatesState = MutableStateFlow(0) + + /** + * Bumps every time a fetch stores an entry. [current] is a plain map read, so a + * composable that derives state from it (the zap picker's "can our wallet pay a + * BOLT12 offer" check) has nothing to recompose on when the info arrives after + * it opened; keying on this flow closes that gap. + */ + val updates: StateFlow = updatesState.asStateFlow() + // Fetches in progress, keyed like [cache]. Every fetching path goes through [fetchOnce]. private val inFlight = ConcurrentMap>() @@ -182,6 +196,7 @@ class NwcInfoCache( } cache[uri.pubKeyHex] = Entry(info, now()) + updatesState.update { it + 1 } return info } diff --git a/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt b/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt index d67627da12..d48ac93cfd 100644 --- a/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt +++ b/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt @@ -34,6 +34,8 @@ actual class SecureKeyStorage private actual constructor() { actual suspend fun getPrivateKey(npub: String): String? = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") + actual suspend fun getPrivateKeyOrThrow(npub: String): String? = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") + actual suspend fun deletePrivateKey(npub: String): Boolean = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") actual suspend fun hasPrivateKey(npub: String): Boolean = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") diff --git a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index 5cd5aba9a0..cb3731be4c 100644 --- a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -153,6 +153,75 @@ actual class SecureKeyStorage private actual constructor() { } } + /** + * Strict variant that distinguishes "backend confirms item not found" from every + * other outcome. This matters on macOS: `javakeyring` collapses `errSecItemNotFound` + * (-25300), `errSecAuthFailed` (-25293), `errSecUserCanceled` (-128), and + * `errSecInteractionNotAllowed` (-25308) into the same `PasswordAccessException`. + * A caller that mistook "user clicked Deny" for "first launch, generate a fresh + * key" would silently rotate the metadata AES key and permanently destroy the + * accounts.json.enc it was supposed to unlock. + * + * On macOS this shells out to `/usr/bin/security find-generic-password`, whose + * exit codes are documented and unambiguous (44 = not found, 128 = user cancel / + * dialog dismissed, others = backend failure). On Windows / Linux, javakeyring + * has no such ambiguity for the equivalent flows in practice, but we still treat + * any `PasswordAccessException` here as ambiguous (throw) to keep the contract + * strict on the getOrCreate path. + */ + actual suspend fun getPrivateKeyOrThrow(npub: String): String? = + withContext(Dispatchers.IO) { + try { + if (!keyringAvailable) { + return@withContext getFromFallback(npub) + } + if (isMacOs()) { + return@withContext getFromMacSecurityCli(SERVICE_NAME, npub) + } + try { + keyring().getPassword(SERVICE_NAME, npub) + } catch (e: PasswordAccessException) { + // Non-mac backends: keep the strict contract by refusing to + // treat this as "definitively absent". A caller that needs a + // permissive lookup should use getPrivateKey() instead. + throw SecureStorageException( + "Keyring backend refused access or returned ambiguous not-found", + e, + ) + } + } catch (e: SecureStorageException) { + throw e + } catch (e: BackendNotSupportedException) { + keyringAvailable = false + println("OS keyring not available, using fallback encrypted storage") + getFromFallback(npub) + } catch (e: Exception) { + throw SecureStorageException("Failed to retrieve private key (strict)", e) + } + } + + /** + * Test seam: overridable strategy for the strict macOS lookup. Production wires + * to [defaultMacSecurityLookup] which spawns `/usr/bin/security`. Tests replace + * this with a stub so unit tests run hermetically on any OS. + */ + internal var macSecurityLookup: (String, String) -> MacSecurityResult = + ::defaultMacSecurityLookup + + private fun getFromMacSecurityCli( + service: String, + account: String, + ): String? { + val result = macSecurityLookup(service, account) + return when (result) { + is MacSecurityResult.Found -> result.password + is MacSecurityResult.NotFound -> null + is MacSecurityResult.Ambiguous -> throw SecureStorageException( + "macOS Keychain access failed (${result.reason}, exit=${result.exitCode})", + ) + } + } + actual suspend fun deletePrivateKey(npub: String): Boolean = withContext(Dispatchers.IO) { try { @@ -470,6 +539,98 @@ internal interface KeyringHandle { ) } +/** + * Outcome of a strict macOS `/usr/bin/security find-generic-password` lookup. + * Kept as a sealed hierarchy so [SecureKeyStorage.getPrivateKeyOrThrow] can + * cleanly translate to `null` versus `SecureStorageException`. + */ +internal sealed class MacSecurityResult { + data class Found( + val password: String, + ) : MacSecurityResult() + + object NotFound : MacSecurityResult() + + /** + * Any exit code other than 0 (found) or 44 (item not found). Reason is a short + * human string derived from stderr / documented codes: + * 128 = user cancelled or dismissed the Keychain Access dialog + * -25293 (errSecAuthFailed) surfaces as exit 51 in practice + * -25308 (errSecInteractionNotAllowed) surfaces when Keychain is locked + */ + data class Ambiguous( + val exitCode: Int, + val reason: String, + ) : MacSecurityResult() +} + +/** + * Pure parser split out for testability on non-macOS CI runners. Maps the + * documented exit code contract of `/usr/bin/security find-generic-password` + * to a [MacSecurityResult]. `stdout` is the raw password body (`-w` prints it + * followed by a newline; strip the trailing newline only). `stderr` is used + * as a hint for the ambiguous [MacSecurityResult.Ambiguous.reason] string. + */ +internal fun parseMacSecurityFindResult( + exitCode: Int, + stdout: String, + stderr: String, +): MacSecurityResult = + when (exitCode) { + 0 -> MacSecurityResult.Found(stdout.trimEnd('\n', '\r')) + 44 -> MacSecurityResult.NotFound + else -> { + val reason = + when { + exitCode == 128 -> "user cancelled Keychain dialog" + stderr.contains("-25293") -> "errSecAuthFailed" + stderr.contains("-25308") -> "errSecInteractionNotAllowed" + stderr.contains("-128") -> "user cancelled Keychain dialog" + stderr.isNotBlank() -> + stderr + .lineSequence() + .first() + .trim() + .take(120) + else -> "unknown" + } + MacSecurityResult.Ambiguous(exitCode, reason) + } + } + +private fun isMacOs(): Boolean = System.getProperty("os.name").orEmpty().startsWith("Mac") + +/** + * Production implementation: spawn `/usr/bin/security` and read exit code + streams. + * Kept package-private so tests can also reach it if they want to run the real path + * on a mac host, but production always goes through the [SecureKeyStorage.macSecurityLookup] + * indirection. + */ +internal fun defaultMacSecurityLookup( + service: String, + account: String, +): MacSecurityResult { + val process = + try { + ProcessBuilder( + "/usr/bin/security", + "find-generic-password", + "-s", + service, + "-a", + account, + "-w", + ).redirectErrorStream(false).start() + } catch (e: Exception) { + return MacSecurityResult.Ambiguous(-1, "failed to spawn /usr/bin/security: ${e.message ?: e::class.simpleName ?: "unknown"}") + } + process.outputStream.close() + val stdout = process.inputStream.bufferedReader().use { it.readText() } + val stderr = process.errorStream.bufferedReader().use { it.readText() } + val exitCode = process.waitFor() + return parseMacSecurityFindResult(exitCode, stdout, stderr) +} + internal class RealKeyringHandle( private val keyring: Keyring, ) : KeyringHandle { diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt new file mode 100644 index 0000000000..980d6c718f --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt @@ -0,0 +1,212 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.keystorage + +import com.github.javakeyring.PasswordAccessException +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test + +/** + * Unit tests for the strict `getPrivateKeyOrThrow` lookup and its pure macOS + * `/usr/bin/security` exit-code parser. + * + * These tests must never touch the OS keychain and must run on any host, so: + * - the macOS integration paths are gated behind [MacSecurityResult] stubs + * injected via `SecureKeyStorage.macSecurityLookup`; + * - the parser test operates on captured stdout/stderr/exitCode triples; + * - non-mac backends are exercised via the `KeyringHandle` test seam already + * used by [SecureKeyStorageKeyringCacheTest]. + */ +class SecureKeyStorageOrThrowTest { + private class ExplodingKeyring( + private val onGet: () -> Nothing, + ) : KeyringHandle { + override fun getPassword( + service: String, + account: String, + ): String = onGet() + + override fun setPassword( + service: String, + account: String, + password: String, + ) { + // unused in these tests + } + + override fun deletePassword( + service: String, + account: String, + ) { + // unused in these tests + } + } + + private class StaticKeyring( + private val map: Map, String>, + ) : KeyringHandle { + override fun getPassword( + service: String, + account: String, + ): String = map[service to account] ?: throw PasswordAccessException("no entry") + + override fun setPassword( + service: String, + account: String, + password: String, + ) {} + + override fun deletePassword( + service: String, + account: String, + ) {} + } + + // --- macOS security(1) parser --- + + @Test + fun `parser exit 0 returns Found with trimmed password`() { + val result = parseMacSecurityFindResult(0, "hunter2\n", "") + assertTrue(result is MacSecurityResult.Found) + assertEquals("hunter2", (result as MacSecurityResult.Found).password) + } + + @Test + fun `parser exit 0 preserves internal newlines and only strips trailing`() { + val result = parseMacSecurityFindResult(0, "line1\nline2\n", "") + assertEquals("line1\nline2", (result as MacSecurityResult.Found).password) + } + + @Test + fun `parser exit 44 returns NotFound`() { + val result = + parseMacSecurityFindResult( + 44, + "", + "security: SecKeychainSearchCopyNext: The specified item could not be found in the keychain.\n", + ) + assertTrue(result is MacSecurityResult.NotFound) + } + + @Test + fun `parser exit 128 flagged as user-cancelled`() { + val result = parseMacSecurityFindResult(128, "", "security: dismissed\n") + assertTrue(result is MacSecurityResult.Ambiguous) + val ambig = result as MacSecurityResult.Ambiguous + assertEquals(128, ambig.exitCode) + assertTrue(ambig.reason.contains("cancel")) + } + + @Test + fun `parser stderr -25293 mapped to errSecAuthFailed`() { + val result = parseMacSecurityFindResult(51, "", "security: SecKeychainItemCopyContent (-25293)\n") + val ambig = result as MacSecurityResult.Ambiguous + assertEquals("errSecAuthFailed", ambig.reason) + } + + @Test + fun `parser unknown exit falls back to first stderr line`() { + val result = parseMacSecurityFindResult(9999, "", "security: mystery: line 1\nline 2\n") + val ambig = result as MacSecurityResult.Ambiguous + assertEquals("security: mystery: line 1", ambig.reason) + } + + // --- getPrivateKeyOrThrow: strict semantics via injected macOS lookup --- + // (Enabled unconditionally: the macSecurityLookup indirection is exercised + // via a stub, so no `security` binary is invoked. The `isMacOs()` check + // means this test only takes the mac path on macOS runners; on Linux it + // takes the javakeyring path, which we validate separately below.) + + private fun newStorage(): SecureKeyStorage = SecureKeyStorage.create() + + @Test + fun `mac lookup Found returns password without ambiguity`() = + runBlocking { + if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.macSecurityLookup = { _, _ -> MacSecurityResult.Found("secretval") } + assertEquals("secretval", storage.getPrivateKeyOrThrow("account-metadata-key")) + } + + @Test + fun `mac lookup NotFound returns null`() = + runBlocking { + if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.macSecurityLookup = { _, _ -> MacSecurityResult.NotFound } + assertNull(storage.getPrivateKeyOrThrow("account-metadata-key")) + } + + @Test + fun `mac lookup Ambiguous throws SecureStorageException with reason`() = + runBlocking { + if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.macSecurityLookup = { _, _ -> + MacSecurityResult.Ambiguous(128, "user cancelled Keychain dialog") + } + try { + storage.getPrivateKeyOrThrow("account-metadata-key") + fail("Expected SecureStorageException") + } catch (e: SecureStorageException) { + assertTrue(e.message?.contains("user cancelled") == true) + assertTrue(e.message?.contains("128") == true) + } + } + + // --- non-mac backend: PasswordAccessException must throw, never null --- + + @Test + fun `non-mac keyring PasswordAccessException throws not returns null`() = + runBlocking { + if (System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.keyringFactory = { ExplodingKeyring { throw PasswordAccessException("locked") } } + try { + storage.getPrivateKeyOrThrow("account-metadata-key") + fail("Expected SecureStorageException") + } catch (e: SecureStorageException) { + assertTrue( + e.message?.contains("ambiguous", ignoreCase = true) == true || + e.message?.contains("refused", ignoreCase = true) == true, + ) + } + } + + @Test + fun `non-mac keyring hit returns password`() = + runBlocking { + if (System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.keyringFactory = { + StaticKeyring( + mapOf( + ("amethyst-desktop" to "account-metadata-key") to "abc123", + ), + ) + } + assertEquals("abc123", storage.getPrivateKeyOrThrow("account-metadata-key")) + } +} diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index 6cee50140c..cdb9318316 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -258,6 +258,7 @@ Lightning Invoice Expired CLINK Offer + BOLT12 Offer To Confirm payment Amount (sats) diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt index 3f6ff53ec2..e1813ff197 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt @@ -23,12 +23,16 @@ package com.vitorpamplona.amethyst.desktop.account import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.fasterxml.jackson.module.kotlin.readValue import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage +import com.vitorpamplona.amethyst.commons.keystorage.SecureStorageException import com.vitorpamplona.amethyst.commons.model.account.AccountInfo import com.vitorpamplona.amethyst.commons.model.account.AccountStorage import com.vitorpamplona.amethyst.commons.model.account.SignerType import com.vitorpamplona.amethyst.commons.util.deleteOrWarn import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.io.File +import java.io.RandomAccessFile import java.nio.file.Files import java.nio.file.StandardCopyOption import java.nio.file.attribute.PosixFilePermission @@ -58,6 +62,16 @@ sealed class StorageCorruption( class JsonMalformed( backupPath: String?, ) : StorageCorruption(backupPath) + + /** + * A transient failure surfaced from the read path (I/O error, keychain refused + * or otherwise ambiguous access, OOM, etc). No backup was written and the + * on-disk file is untouched. Callers should retry or surface an error UI rather + * than treating this as data loss. See [DesktopAccountStorage.readMetadataFromDisk]. + */ + class TransientError( + val cause: Throwable, + ) : StorageCorruption(backupPath = null) } class DesktopAccountStorage( @@ -68,6 +82,7 @@ class DesktopAccountStorage( companion object { private const val METADATA_KEY_ALIAS = "account-metadata-key" private const val ACCOUNTS_FILE = "accounts.json.enc" + private const val ACCOUNTS_LOCK_FILE = "accounts.json.enc.lock" private const val AES_KEY_SIZE = 32 // 256 bits private const val GCM_IV_SIZE = 12 private const val GCM_TAG_BITS = 128 @@ -76,38 +91,51 @@ class DesktopAccountStorage( private val mapper = jacksonObjectMapper() private val amethystDir by lazy { File(homeDir, ".amethyst") } - // In-memory cache — read from disk once, then serve from memory + // In-memory cache: read from disk once, then serve from memory private var cachedMetadata: AccountMetadata? = null + // In-process mutex around the cross-process file lock. Two callers inside + // the same JVM would otherwise fail with OverlappingFileLockException from + // FileChannel.lock(), since JVM file locks are per-JVM not per-thread. + private val fileLockMutex = Mutex() + + // Guards read-modify-write cycles on [cachedMetadata]. Distinct from + // [fileLockMutex] so we can hold it across a full read + mutate + write + // sequence (the file lock is taken and released inside each disk op). + private val stateMutex = Mutex() + // --- AccountStorage interface --- override suspend fun loadAccounts(): List = getCachedMetadata().accounts.map { it.toAccountInfo() } - override suspend fun saveAccount(info: AccountInfo) { - val metadata = getCachedMetadata() - val dto = AccountInfoDto.from(info) - val updated = metadata.accounts.filter { it.npub != info.npub } + dto - writeCachedMetadata(metadata.copy(accounts = updated)) - } + override suspend fun saveAccount(info: AccountInfo) = + stateMutex.withLock { + val metadata = getCachedMetadata() + val dto = AccountInfoDto.from(info) + val updated = metadata.accounts.filter { it.npub != info.npub } + dto + writeCachedMetadata(metadata.copy(accounts = updated)) + } - override suspend fun deleteAccount(npub: String) { - val metadata = getCachedMetadata() - val updated = metadata.accounts.filter { it.npub != npub } - val newActive = - if (metadata.activeNpub == npub) { - updated.firstOrNull()?.npub - } else { - metadata.activeNpub - } - writeCachedMetadata(metadata.copy(accounts = updated, activeNpub = newActive)) - } + override suspend fun deleteAccount(npub: String) = + stateMutex.withLock { + val metadata = getCachedMetadata() + val updated = metadata.accounts.filter { it.npub != npub } + val newActive = + if (metadata.activeNpub == npub) { + updated.firstOrNull()?.npub + } else { + metadata.activeNpub + } + writeCachedMetadata(metadata.copy(accounts = updated, activeNpub = newActive)) + } override suspend fun currentAccount(): String? = getCachedMetadata().activeNpub - override suspend fun setCurrentAccount(npub: String) { - val metadata = getCachedMetadata() - writeCachedMetadata(metadata.copy(activeNpub = npub)) - } + override suspend fun setCurrentAccount(npub: String) = + stateMutex.withLock { + val metadata = getCachedMetadata() + writeCachedMetadata(metadata.copy(activeNpub = npub)) + } // --- Cached I/O --- @@ -118,9 +146,17 @@ class DesktopAccountStorage( return loaded } + /** + * Persists first, caches second. + * + * If the disk write fails (keychain refused, I/O error, disk full) the in-memory + * cache must NOT be left claiming a state that was never written: the rest of the + * session would serve accounts that vanish on the next launch, and the user would + * see a successful save that silently did nothing. + */ private suspend fun writeCachedMetadata(metadata: AccountMetadata) { - cachedMetadata = metadata writeMetadataToDisk(metadata) + cachedMetadata = metadata } // --- Encrypted file I/O --- @@ -129,9 +165,17 @@ class DesktopAccountStorage( val file = getAccountsFile() if (!file.exists()) return AccountMetadata() + ensureDir() + return withAccountsFileLock { + readMetadataFromDiskLocked(file) + } + } + + private suspend fun readMetadataFromDiskLocked(file: File): AccountMetadata { val encrypted = file.readBytes() if (encrypted.size < GCM_IV_SIZE) { - val backup = backupCorruptFile(file) + // Genuinely unusable: not enough bytes for the IV. Back up and reset. + val backup = backupCorruptFile(file, ".corrupt") onCorruption(StorageCorruption.FileCorrupted(backup)) return AccountMetadata() } @@ -140,31 +184,43 @@ class DesktopAccountStorage( val decrypted = decrypt(encrypted) mapper.readValue(decrypted) } catch (e: javax.crypto.AEADBadTagException) { - Log.e("DesktopAccountStorage", "GCM auth tag mismatch — file corrupted or key lost", e) - val backup = backupCorruptFile(file) + // Genuine ciphertext corruption or lost/rotated AES key. + Log.e("DesktopAccountStorage", "GCM auth tag mismatch, file corrupted or key lost", e) + val backup = backupCorruptFile(file, ".corrupt") onCorruption(StorageCorruption.FileCorrupted(backup)) AccountMetadata() } catch (e: javax.crypto.BadPaddingException) { - Log.e("DesktopAccountStorage", "Decryption failed — file corrupted", e) - val backup = backupCorruptFile(file) + // Genuine ciphertext corruption. + Log.e("DesktopAccountStorage", "Decryption failed, file corrupted", e) + val backup = backupCorruptFile(file, ".corrupt") onCorruption(StorageCorruption.FileCorrupted(backup)) AccountMetadata() } catch (e: com.fasterxml.jackson.core.JacksonException) { + // Schema mismatch: decrypted cleanly but the JSON does not fit our shape. + // Distinct suffix so operators can tell it apart from ciphertext corruption. Log.e("DesktopAccountStorage", "JSON malformed after decryption", e) - val backup = backupCorruptFile(file) + val backup = backupCorruptFile(file, ".jsonerror") onCorruption(StorageCorruption.JsonMalformed(backup)) AccountMetadata() + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e } catch (e: Exception) { - Log.e("DesktopAccountStorage", "Failed to read accounts metadata", e) - val backup = backupCorruptFile(file) - onCorruption(StorageCorruption.FileCorrupted(backup)) - AccountMetadata() + // Transient failure: I/O error, keychain refused / ambiguous, OOM, etc. + // DO NOT rename the on-disk file; the ciphertext is intact and the next + // launch may succeed (for example after the user re-approves the + // Keychain Access prompt). Surface up for the caller to decide. + Log.e("DesktopAccountStorage", "Transient error reading accounts metadata; file preserved", e) + onCorruption(StorageCorruption.TransientError(e)) + throw e } } - private fun backupCorruptFile(file: File): String? = + private fun backupCorruptFile( + file: File, + suffix: String, + ): String? = try { - val backup = File(file.parent, "accounts.json.enc.corrupt.${System.currentTimeMillis()}") + val backup = File(file.parent, "${file.name}$suffix.${System.currentTimeMillis()}") java.nio.file.Files .copy(file.toPath(), backup.toPath()) file.deleteOrWarn("DesktopAccountStorage", "corrupt accounts file") @@ -178,31 +234,103 @@ class DesktopAccountStorage( val json = mapper.writeValueAsBytes(metadata) val encrypted = encrypt(json) - // Atomic write via temp file val file = getAccountsFile() - val temp = File(amethystDir, "${ACCOUNTS_FILE}.tmp") - temp.writeBytes(encrypted) - Files.move(temp.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING) - - setFilePermissions(file) + withAccountsFileLock { + // Atomic write via temp file, under the cross-process lock so two + // Amethyst instances (Homebrew upgrade race, accidental double-launch) + // cannot interleave writes and truncate the file. + val temp = File(amethystDir, "$ACCOUNTS_FILE.tmp") + temp.writeBytes(encrypted) + Files.move( + temp.toPath(), + file.toPath(), + StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE, + ) + setFilePermissions(file) + } } + /** + * Cross-process advisory lock + in-process mutex around the accounts.json.enc + * read/write critical section. The mutex is required because JVM + * `FileChannel.lock()` is a per-JVM lock and would throw + * `OverlappingFileLockException` on the second acquire from the same JVM. + * The channel lock is required to keep two Amethyst processes serial (upgrade + * race, accidental double-launch, cron-style relaunch). + * + * Mirrors the pattern used in SecureKeyStorage.withFileLock; kept private + * to this class so the two lock lifecycles stay independent. + */ + private suspend inline fun withAccountsFileLock(crossinline block: suspend () -> T): T = + fileLockMutex.withLock { + val lockFile = File(amethystDir, ACCOUNTS_LOCK_FILE) + if (!lockFile.exists()) { + lockFile.createNewFile() + setFilePermissions(lockFile) + } + RandomAccessFile(lockFile, "rw").use { raf -> + raf.channel.lock().use { _ -> + block() + } + } + } + private fun getAccountsFile() = File(amethystDir, ACCOUNTS_FILE) // --- AES-256-GCM encryption --- private var cachedKey: ByteArray? = null + /** + * Reads (or creates on first launch) the metadata AES key. + * + * Distinguishes: + * - key exists in keychain: use it + * - keychain confirms definitively absent: generate + persist a fresh key + * - any other outcome (user cancelled/denied prompt, keychain locked, + * backend transient error): propagate the exception, do NOT rotate -- + * unless there is no accounts.json.enc yet, in which case there is no + * ciphertext to orphan and we bootstrap a fresh key (see below). + * + * Rotating the AES key on an ambiguous miss silently destroys the ability + * to decrypt the existing accounts.json.enc, wiping the logged-in accounts + * on next launch. That is the bug this method exists to prevent. + */ private suspend fun getOrCreateKey(): ByteArray { cachedKey?.let { return it } - val existing = secureStorage.getPrivateKey(METADATA_KEY_ALIAS) + val existing = + try { + secureStorage.getPrivateKeyOrThrow(METADATA_KEY_ALIAS) + } catch (e: SecureStorageException) { + // Bootstrap escape. Every non-macOS backend java-keyring ships + // (Windows Credential Store, Freedesktop Secret Service, KWallet) + // throws PasswordAccessException for a *genuinely absent* credential, + // so the strict lookup structurally cannot report "definitively + // absent" there. Without this branch a fresh Linux/Windows install + // could never mint the key and could never persist an account. + // + // Minting is only safe while there is no accounts.json.enc: with no + // ciphertext on disk there is nothing a new key can orphan. Once the + // file exists the strict contract applies and we propagate. + if (getAccountsFile().exists()) throw e + Log.w( + "DesktopAccountStorage", + "Keychain lookup failed and no accounts file exists; bootstrapping a fresh metadata key", + e, + ) + null + } + if (existing != null) { val key = Base64.getDecoder().decode(existing) cachedKey = key return key } + // Definitively absent (or bootstrapping with nothing on disk): safe to + // create and persist a fresh key. val key = ByteArray(AES_KEY_SIZE).also { SecureRandom().nextBytes(it) } secureStorage.savePrivateKey(METADATA_KEY_ALIAS, Base64.getEncoder().encodeToString(key)) cachedKey = key diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt index 4d511c57f7..6059f961fe 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt @@ -58,6 +58,7 @@ class AccountManagerKeyLoginTest { val keySlot = slot() val valueSlot = slot() coEvery { storage.getPrivateKey(capture(keySlot)) } answers { keyStore[keySlot.captured] } + coEvery { storage.getPrivateKeyOrThrow(capture(keySlot)) } answers { keyStore[keySlot.captured] } coEvery { storage.savePrivateKey(capture(keySlot), capture(valueSlot)) } answers { keyStore[keySlot.captured] = valueSlot.captured } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt index 8b01b71677..5aad247a29 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt @@ -49,6 +49,7 @@ class AccountManagerLoadAccountTest { storage = mockk(relaxed = true) // Return null so DesktopAccountStorage generates a fresh AES key coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-load-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt index f98a4c3407..602016ed5f 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt @@ -63,6 +63,7 @@ class AccountManagerLoadStateTransitionsTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-load-state").toFile() File(tempDir, ".amethyst").mkdirs() manager = AccountManager(storage, tempDir) diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt index 2f74a42390..741b79044d 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt @@ -46,6 +46,7 @@ class AccountManagerLogoutTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-logout-test").toFile() manager = AccountManager(storage, tempDir) } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt index f2688a2819..6cb2a59e15 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt @@ -56,6 +56,7 @@ class AccountManagerNip46IsolationTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-nip46-iso-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt index 6734d49793..62e5eb08c0 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt @@ -56,6 +56,7 @@ class AccountManagerStateTransitionTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-state-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt index 9ffd9ac1b3..a4d2e9a260 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt @@ -21,18 +21,28 @@ package com.vitorpamplona.amethyst.desktop.account import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage +import com.vitorpamplona.amethyst.commons.keystorage.SecureStorageException import com.vitorpamplona.amethyst.commons.model.account.AccountInfo import com.vitorpamplona.amethyst.commons.model.account.SignerType import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk import io.mockk.slot +import kotlinx.coroutines.asCoroutineDispatcher +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext import java.io.File +import java.util.concurrent.Executors import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFails +import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -56,6 +66,9 @@ class DesktopAccountStorageTest { coEvery { secureStorage.getPrivateKey(capture(keySlot)) } answers { keyStore[keySlot.captured] } + coEvery { secureStorage.getPrivateKeyOrThrow(capture(keySlot)) } answers { + keyStore[keySlot.captured] + } coEvery { secureStorage.savePrivateKey(capture(keySlot), capture(valueSlot)) } answers { keyStore[keySlot.captured] = valueSlot.captured } @@ -197,4 +210,269 @@ class DesktopAccountStorageTest { // Encrypted content should NOT contain the npub in plaintext assertTrue(!content.contains("npub1secret")) } + + // --- Bug 1: silent AES key rotation --- + + @Test + fun `getOrCreateKey keyring throws ambiguous error does not rotate key or touch file`() = + runTest { + // First launch: seed a real metadata key + an existing accounts.json.enc + storage.saveAccount(AccountInfo("npub1existing", SignerType.Internal)) + val file = File(File(tempDir, ".amethyst"), "accounts.json.enc") + val originalBytes = file.readBytes() + val originalMetadataKey = keyStore["account-metadata-key"] + assertNotNull(originalMetadataKey) + + // Fresh storage instance simulating a relaunch: the keychain now + // returns an ambiguous error (user cancelled the Keychain dialog). + val throwingStorage: SecureKeyStorage = mockk() + coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws + SecureStorageException("user cancelled Keychain dialog") + // Legacy permissive read should still return the key; production + // must not fall back to it on the getOrCreate path. + coEvery { throwingStorage.getPrivateKey(any()) } answers { keyStore[firstArg()] } + coEvery { throwingStorage.hasPrivateKey(any()) } answers { keyStore.containsKey(firstArg()) } + + val relaunched = DesktopAccountStorage(throwingStorage, tempDir) + + // Any operation that needs the metadata key must fail loudly, not + // rotate the key or write a fresh empty file. + assertFails { runBlocking { relaunched.loadAccounts() } } + + // Bug 1 invariant: no new savePrivateKey call for the metadata key. + coVerify(exactly = 0) { + throwingStorage.savePrivateKey("account-metadata-key", any()) + } + // Bug 1 + Bug 2 invariant: on-disk ciphertext untouched. + assertTrue(file.exists()) + assertContentEquals(originalBytes, file.readBytes()) + // Bug 1 invariant: keyStore metadata key unchanged. + assertEquals(originalMetadataKey, keyStore["account-metadata-key"]) + } + + @Test + fun `getOrCreateKey keyring returns definitive not-found creates and persists new key`() = + runTest { + // Happy path first launch: getPrivateKeyOrThrow returns null, + // storage generates + persists a fresh AES key exactly once. + assertNull(keyStore["account-metadata-key"]) + + storage.saveAccount(AccountInfo("npub1first", SignerType.Internal)) + + assertNotNull(keyStore["account-metadata-key"]) + coVerify(exactly = 1) { + secureStorage.savePrivateKey("account-metadata-key", any()) + } + } + + @Test + fun `getOrCreateKey ambiguous error with no accounts file bootstraps a fresh key`() = + runTest { + // Every non-macOS backend java-keyring ships (Windows Credential Store, + // Freedesktop Secret Service, KWallet) throws PasswordAccessException for a + // *genuinely absent* credential, which the strict lookup surfaces as + // SecureStorageException. With no accounts.json.enc there is no ciphertext + // a new key could orphan, so a fresh install must still be able to mint one + // -- otherwise Linux/Windows can never persist an account at all. + val saved = mutableMapOf() + val throwingStorage: SecureKeyStorage = mockk() + coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws + SecureStorageException("Keyring backend refused access or returned ambiguous not-found") + coEvery { throwingStorage.savePrivateKey(any(), any()) } answers { + saved[firstArg()] = secondArg() + } + coEvery { throwingStorage.getPrivateKey(any()) } answers { saved[firstArg()] } + coEvery { throwingStorage.hasPrivateKey(any()) } answers { saved.containsKey(firstArg()) } + + val file = File(File(tempDir, ".amethyst"), "accounts.json.enc") + assertFalse(file.exists()) + + val fresh = DesktopAccountStorage(throwingStorage, tempDir) + fresh.saveAccount(AccountInfo("npub1freshinstall", SignerType.Internal)) + + assertNotNull(saved["account-metadata-key"]) + assertTrue(file.exists()) + assertEquals(listOf("npub1freshinstall"), fresh.loadAccounts().map { it.npub }) + // The escape is bootstrap-only: once the file exists the strict contract + // applies again -- pinned by `getOrCreateKey keyring throws ambiguous error + // does not rotate key or touch file` above. + } + + // --- Cache must never claim a state that was not persisted --- + + @Test + fun `failed disk write does not poison the in-memory cache`() = + runTest { + storage.saveAccount(AccountInfo("npub1persisted", SignerType.Internal)) + assertEquals(listOf("npub1persisted"), storage.loadAccounts().map { it.npub }) + + // Block the atomic-write temp path so writeMetadataToDisk fails. + val temp = File(File(tempDir, ".amethyst"), "accounts.json.enc.tmp") + assertTrue(temp.mkdirs()) + + assertFails { + runBlocking { + storage.saveAccount(AccountInfo("npub1phantom", SignerType.Internal)) + } + } + + // Same instance: the cache must still reflect only what reached the disk, + // not the account the failed save handed it. + assertEquals(listOf("npub1persisted"), storage.loadAccounts().map { it.npub }) + + // And the on-disk file agrees. + temp.delete() + val relaunched = DesktopAccountStorage(secureStorage, tempDir) + assertEquals(listOf("npub1persisted"), relaunched.loadAccounts().map { it.npub }) + } + + // --- Bug 2: read failure must not silently reset the file --- + + @Test + fun `readMetadataFromDisk transient IO error does not backup file`() = + runTest { + // Seed a real file we can inspect. + storage.saveAccount(AccountInfo("npub1existing", SignerType.Internal)) + val amethystDir = File(tempDir, ".amethyst") + val file = File(amethystDir, "accounts.json.enc") + val originalBytes = file.readBytes() + val originalName = file.name + + // Fresh storage that surfaces a transient error from the keychain. + val throwingStorage: SecureKeyStorage = mockk() + coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws + SecureStorageException("transient keychain error") + coEvery { throwingStorage.getPrivateKey(any()) } returns null + coEvery { throwingStorage.hasPrivateKey(any()) } returns false + + val corruptions = mutableListOf() + val relaunched = + DesktopAccountStorage(throwingStorage, tempDir, onCorruption = { corruptions += it }) + + assertFails { runBlocking { relaunched.loadAccounts() } } + + // File preserved, no .corrupt.* or .jsonerror.* sibling created. + assertTrue(file.exists()) + assertContentEquals(originalBytes, file.readBytes()) + val siblings = amethystDir.listFiles().orEmpty().map { it.name } + assertFalse(siblings.any { it != originalName && it.startsWith("accounts.json.enc") && (it.contains(".corrupt.") || it.contains(".jsonerror.")) }) + + // The callback fired with the transient subtype so the app can retry. + assertTrue(corruptions.any { it is StorageCorruption.TransientError }) + } + + @Test + fun `readMetadataFromDisk gcm tag mismatch backs up and resets`() = + runTest { + // Seed a valid file so we have a real metadata key in the mock keystore. + storage.saveAccount(AccountInfo("npub1a", SignerType.Internal)) + val amethystDir = File(tempDir, ".amethyst") + val file = File(amethystDir, "accounts.json.enc") + assertTrue(file.exists()) + + // Overwrite with random bytes that pass the length check but fail + // GCM auth tag verification. Prefix with a fresh IV, then garbage. + val garbage = ByteArray(64) { it.toByte() } + file.writeBytes(garbage) + + val corruptions = mutableListOf() + val relaunched = + DesktopAccountStorage(secureStorage, tempDir, onCorruption = { corruptions += it }) + + val loaded = relaunched.loadAccounts() + assertTrue(loaded.isEmpty()) + + // Backup exists with the .corrupt. suffix; original file was + // removed (and will be re-created on next save). + val siblings = amethystDir.listFiles().orEmpty().map { it.name } + assertTrue(siblings.any { it.startsWith("accounts.json.enc.corrupt.") }) + assertTrue(corruptions.any { it is StorageCorruption.FileCorrupted }) + } + + @Test + fun `readMetadataFromDisk json malformed uses jsonerror suffix`() = + runTest { + // Build an accounts.json.enc whose plaintext decrypts fine but is + // not the expected AccountMetadata shape. Easiest path: reuse the + // production encrypt via a lightweight helper storage that lets us + // control the plaintext. + storage.saveAccount(AccountInfo("npub1a", SignerType.Internal)) + val amethystDir = File(tempDir, ".amethyst") + val file = File(amethystDir, "accounts.json.enc") + + // Encrypt an unrelated JSON payload with the same AES key the mock + // keystore holds so decryption succeeds but Jackson rejects the shape. + val key = + java.util.Base64 + .getDecoder() + .decode(keyStore["account-metadata-key"]!!) + val iv = ByteArray(12) { 7 } + val cipher = javax.crypto.Cipher.getInstance("AES/GCM/NoPadding") + cipher.init( + javax.crypto.Cipher.ENCRYPT_MODE, + javax.crypto.spec.SecretKeySpec(key, "AES"), + javax.crypto.spec.GCMParameterSpec(128, iv), + ) + val badPayload = cipher.doFinal("\"not an object\"".toByteArray()) + file.writeBytes(iv + badPayload) + + val corruptions = mutableListOf() + val relaunched = + DesktopAccountStorage(secureStorage, tempDir, onCorruption = { corruptions += it }) + + val loaded = relaunched.loadAccounts() + assertTrue(loaded.isEmpty()) + + val siblings = amethystDir.listFiles().orEmpty().map { it.name } + assertTrue(siblings.any { it.startsWith("accounts.json.enc.jsonerror.") }) + assertTrue(corruptions.any { it is StorageCorruption.JsonMalformed }) + } + + // --- Bug 3: cross-process file lock --- + + @Test + fun `writeMetadataToDisk concurrent saves serialize under file lock`() { + val executor = Executors.newFixedThreadPool(4) + try { + runBlocking { + withContext(executor.asCoroutineDispatcher()) { + val jobs = + (1..8).map { idx -> + async { + storage.saveAccount( + AccountInfo( + npub = "npub1parallel$idx", + signerType = SignerType.Internal, + ), + ) + } + } + jobs.awaitAll() + } + } + // All eight accounts present, file not truncated. + val loaded = runBlocking { storage.loadAccounts() } + assertEquals(8, loaded.size) + val npubs = loaded.map { it.npub }.toSet() + assertEquals((1..8).map { "npub1parallel$it" }.toSet(), npubs) + + // Lock sidecar exists and is respected. + val lockFile = File(File(tempDir, ".amethyst"), "accounts.json.enc.lock") + assertTrue(lockFile.exists()) + } finally { + executor.shutdownNow() + } + } + + private fun assertContentEquals( + expected: ByteArray, + actual: ByteArray, + ) { + assertEquals(expected.size, actual.size, "byte size mismatch") + for (i in expected.indices) { + if (expected[i] != actual[i]) { + throw AssertionError("byte differs at index $i: expected=${expected[i]} actual=${actual[i]}") + } + } + } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt index 7edddfa185..3b017fff94 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt @@ -95,6 +95,7 @@ object LaunchScenario { val tempHome = createTempDirectory("launch-scenario").toFile() val storage = mockk(relaxed = true) coEvery { storage.getPrivateKey(any()) } returns null + coEvery { storage.getPrivateKeyOrThrow(any()) } returns null File(tempHome, ".amethyst").mkdirs() val account = AccountManager(storage, tempHome) diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt index e645ca4cfc..cd98c7ea9c 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt @@ -89,6 +89,7 @@ class AppStateMachineTest { File(tempDir, ".amethyst").mkdirs() storage = mockk(relaxed = true) coEvery { storage.getPrivateKey(any()) } returns null + coEvery { storage.getPrivateKeyOrThrow(any()) } returns null harnessScope = CoroutineScope(Dispatchers.Default + SupervisorJob()) relay = LaunchFixtureRelay.open(LaunchFixture.build(noteCount = 0).events) }