fix(zap): address reload-mint audit findings (double-submit, premature done, fee/poll robustness)

Correctness: - ReloadMintViewModel.confirm() now guards against re-entry (only starts from Configuring/Failed and flips to Working synchronously), so a double-tap or a Failed-state Retry can't launch two pipelines that double-spend the source / double-mint the target. - sendNutzapAndFinish awaits the real CashuWalletState.sendNutzap (throws on failure) and reports Done only on success — a send that fails after a successful reload now surfaces as Failed instead of popping the screen on a premature "done" and silently stranding the moved funds. - The reload pipeline runs on the long-lived AccountViewModel scope; the VM now holds its Job and cancels it in onCleared(), so leaving the screen stops the (up to 3-minute) Lightning poll instead of hammering the mint unobserved. - rebalance() poll budget widened to a steady ~60s so a merely-slow mint no longer strands funds that already left the source. - Mint a small headroom buffer (RELOAD_FEE_BUFFER_SATS) above the bare shortfall so the follow-up nutzap's own swap fee doesn't leave the target a sat short; the source-feasibility gate accounts for it. Regressions from the settings merge: - mergeZapAmounts / the picker no longer .sorted() the amounts — a user's saved preset order is preserved instead of being silently reordered ascending. - The on-chain send dialog falls back to DEFAULT_ONCHAIN_ZAP_SATS when the unified list has nothing above the on-chain minimum, restoring the guaranteed quick-pick preset. - zapClick's one-tap (single-amount) path now checks rail capability and opens the picker when the recipient can't receive Lightning, instead of firing a doomed Lightning zap. Cleanup: - Centralized the mint-quote "settled" predicate as MintQuoteBolt11ResponseDto .isSettled(); rebalance, ReloadMintViewModel and CashuWalletViewModel now share it instead of three copies of paid==true || PAID || ISSUED. https://claude.ai/code/session_01HNE2z7CSYZ2G8KwC5fziJn
2026-08-09 16:14:40 +00:00 · 2026-05-29 20:23:00 +00:00
parent ce32dc4fc3
commit b1961ea19c
8 changed files with 140 additions and 49 deletions
@@ -189,7 +189,7 @@ class AccountZapPreferences(
 * legacy `onchainZapAmountChoices`. Idempotent: [AccountSyncedSettings.toInternal]
 * re-derives the on-chain field as a subset of this list.
 */
-internal fun mergeZapAmounts(zaps: AccountZapPreferencesInternal): List<Long> = (zaps.zapAmountChoices + zaps.onchainZapAmountChoices).distinct().sorted()
+internal fun mergeZapAmounts(zaps: AccountZapPreferencesInternal): List<Long> = (zaps.zapAmountChoices + zaps.onchainZapAmountChoices).distinct()

@Stable
 class AccountLanguagePreferences(
@@ -51,6 +51,13 @@ val DefaultReportWarningThreshold = 5
 */
 const val MIN_ONCHAIN_ZAP_SATS = 1_000L

+/**
+ * Default on-chain quick-pick amount, used when the user's unified zap-amount
+ * list contains nothing at or above [MIN_ONCHAIN_ZAP_SATS]. Matches the legacy
+ * dedicated on-chain default so the send dialog always offers one preset.
+ */
+const val DEFAULT_ONCHAIN_ZAP_SATS = 5_000L
+
@Serializable
 enum class ReactionRowAction {
    Reply,
@@ -1209,16 +1209,19 @@ class CashuWalletState(
        meltToLightning(sourceMintUrl, meltQuote)

        // 4. The melt paid the invoice; confirm + issue proofs at the target.
+        //    The melt settled synchronously, but a healthy mint can still lag a
+        //    while before flipping the quote to PAID, so give it a generous
+        //    ~60s steady budget rather than a tight escalating one — a merely
+        //    slow mint shouldn't strand funds that have already left the source.
        onProgress?.invoke(0.75f)
-        val pollAttempts = 8
-        val pollDelayMs = 1_000L
+        val pollAttempts = 30
+        val pollDelayMs = 2_000L
        var paid = false
        var attempt = 0
        while (!paid && attempt < pollAttempts) {
-            val status = ops.checkMintQuote(targetMintUrl, mintFlow.mintQuote.quote)
-            paid = status.paid == true || status.state == "PAID" || status.state == "ISSUED"
+            paid = ops.checkMintQuote(targetMintUrl, mintFlow.mintQuote.quote).isSettled()
            if (!paid) {
-                delay(pollDelayMs * (attempt + 1))
+                delay(pollDelayMs)
                attempt++
            }
        }
@@ -1387,17 +1387,26 @@ fun zapClick(
        }

        choices.size == 1 -> {
-            onZapStarts()
-            accountViewModel.zap(
-                baseNote,
-                choices.first() * 1000,
-                null,
-                "",
-                context,
-                onError = onError,
-                onProgress = { onZappingProgress(it) },
-                onPayViaIntent = onPayViaIntent,
-            )
+            // 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)
+            if (caps.hasLightning) {
+                onZapStarts()
+                accountViewModel.zap(
+                    baseNote,
+                    choices.first() * 1000,
+                    null,
+                    "",
+                    context,
+                    onError = onError,
+                    onProgress = { onZappingProgress(it) },
+                    onPayViaIntent = onPayViaIntent,
+                )
+            } else {
+                onMultipleChoices()
+            }
        }

        choices.size > 1 -> {
@@ -1948,7 +1957,9 @@ fun ZapAmountChoicePopup(
        }
    val amountChoices =
        remember(zapAmountChoices) {
-            zapAmountChoices.distinct().sorted().toImmutableList()
+            // Keep the user's saved order (already de-duped at the settings
+            // layer); don't re-sort, that would override a deliberate ordering.
+            zapAmountChoices.distinct().toImmutableList()
        }
    val visibilityState = rememberVisibilityState(onDismiss)
    ZapAmountChoicePopup(
@@ -426,7 +426,7 @@ class CashuWalletViewModel : ViewModel() {
        vm.launchSigner {
            try {
                val status = ops.checkMintQuote(current.mintUrl, current.flow.mintQuote.quote)
-                val paid = status.paid == true || status.state == "PAID" || status.state == "ISSUED"
+                val paid = status.isSettled()
                if (!paid) {
                    // Roll back to AwaitingPayment so the polling
                    // LaunchedEffect picks up again on the next tick.
@@ -480,7 +480,7 @@ class CashuWalletViewModel : ViewModel() {
                val amountSats =
                    runCatching { LnInvoiceUtil.getAmountInSats(status.request).toLong() }
                        .getOrElse { 0L }
-                val paid = status.paid == true || status.state == "PAID" || status.state == "ISSUED"
+                val paid = status.isSettled()
                if (paid && amountSats > 0) {
                    ops.completeMintFromLightning(mintUrl, quoteEvent, amountSats)
                    _mintState.value = CashuMintFlowState.Completed(amountSats)
@@ -73,6 +73,7 @@ import com.vitorpamplona.amethyst.commons.onchain.OnchainZapSendResult
 import com.vitorpamplona.amethyst.commons.onchain.OnchainZapSendStage
 import com.vitorpamplona.amethyst.commons.onchain.OnchainZapShare
 import com.vitorpamplona.amethyst.commons.onchain.OnchainZapSplitter
+import com.vitorpamplona.amethyst.model.DEFAULT_ONCHAIN_ZAP_SATS
 import com.vitorpamplona.amethyst.model.LocalCache
 import com.vitorpamplona.amethyst.model.MIN_ONCHAIN_ZAP_SATS
 import com.vitorpamplona.amethyst.model.User
@@ -211,7 +212,13 @@ fun OnchainZapSendDialog(
    // that clear the on-chain minimum as quick presets.
    val zapAmountChoices by accountViewModel.account.settings.syncedSettings.zaps.zapAmountChoices
        .collectAsStateWithLifecycle()
-    val presetAmounts = remember(zapAmountChoices) { zapAmountChoices.filter { it >= MIN_ONCHAIN_ZAP_SATS } }
+    // Presets that clear the on-chain minimum. If the user's unified list has
+    // none (e.g. only small Lightning amounts), fall back to a sensible default
+    // so the dialog still offers a quick-pick chip instead of an empty row.
+    val presetAmounts =
+        remember(zapAmountChoices) {
+            zapAmountChoices.filter { it >= MIN_ONCHAIN_ZAP_SATS }.ifEmpty { listOf(DEFAULT_ONCHAIN_ZAP_SATS) }
+        }

    // Mirror the dropdown's NIP-05 / Namecoin (.bit) resolution so Send can
    // enable as soon as the typed name resolves, without forcing the user to
@@ -26,10 +26,13 @@ import com.vitorpamplona.amethyst.model.Note
 import com.vitorpamplona.amethyst.model.nip60Cashu.CashuWalletState
 import com.vitorpamplona.amethyst.model.nip60Cashu.describeMintError
 import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
+import com.vitorpamplona.quartz.nip01Core.core.Event
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceMethod
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
@@ -105,6 +108,11 @@ class ReloadMintViewModel : ViewModel() {
    private var baseNote: Note? = null
    private val state: CashuWalletState? get() = accountViewModel?.account?.cashuWalletState

+    /** The in-flight reload pipeline, so a second tap can be rejected and the
+     *  whole flow cancelled when the screen leaves (it runs on the long-lived
+     *  AccountViewModel scope, not this VM's). */
+    private var job: Job? = null
+
    private val _uiState = MutableStateFlow(ReloadUiState())
    val uiState: StateFlow<ReloadUiState> = _uiState.asStateFlow()

@@ -146,14 +154,20 @@ class ReloadMintViewModel : ViewModel() {
        val balances = base.balances.associate { it.mintUrl to it.balanceSats }
        val targetBalance = balances[target] ?: 0L
        val shortfall = (base.amountSats - targetBalance).coerceAtLeast(0L)
-        // Rough fee cushion for enabling a source; the melt quote sets the real
-        // fee at execution time. 1% (min 1 sat).
-        val estFee = (shortfall / 100L).coerceAtLeast(1L)
+        // Rough fee cushion for *enabling* a source — the real Lightning
+        // feeReserve is only known once rebalance() fetches the melt quote, so
+        // this is a heuristic (1%, min 2 sat). A source that clears the estimate
+        // but not the real quote fails recoverably (Failed → pick another), it
+        // doesn't move funds.
+        val estFee = (shortfall / 100L).coerceAtLeast(2L)
+        // A source must cover what we actually mint (shortfall + headroom buffer)
+        // plus that melt fee.
+        val needFromSource = shortfall + RELOAD_FEE_BUFFER_SATS + estFee

        val mintSources =
            base.balances
                .filter { it.mintUrl != target }
-                .map { ReloadSource.Mint(it.mintUrl, it.balanceSats, canCover = it.balanceSats >= shortfall + estFee) }
+                .map { ReloadSource.Mint(it.mintUrl, it.balanceSats, canCover = it.balanceSats >= needFromSource) }

        val sources: List<ReloadSource> = mintSources + ReloadSource.Lightning
        // Prefer the richest mint that can cover (no new sats); else Lightning.
@@ -183,28 +197,43 @@ class ReloadMintViewModel : ViewModel() {
        val s = _uiState.value
        val source = s.selectedSource ?: return

-        vm.launchSigner {
-            try {
-                when (source) {
-                    is ReloadSource.Mint -> rebalanceThenZap(source.mintUrl, s.selectedTarget, s.shortfallSats, note, s.amountSats)
-                    ReloadSource.Lightning -> reloadFromLightningThenZap(s.selectedTarget, s.shortfallSats, note, s.amountSats)
+        // In-flight guard: only start from a resting state. Without it a double
+        // tap (or the Failed-state Retry) launches a second pipeline — two mint
+        // quotes at the target and two melts at the source, double-spending /
+        // double-minting. Flip to Working synchronously so the second call bails.
+        if (s.status !is ReloadStatus.Configuring && s.status !is ReloadStatus.Failed) return
+        setStatus(ReloadStatus.Working("Starting", 0.05f))
+
+        // Mint a hair more than the bare shortfall so the follow-up nutzap's own
+        // swap fee doesn't leave the target a sat short (see #RELOAD_FEE_BUFFER).
+        val moveSats = s.shortfallSats + RELOAD_FEE_BUFFER_SATS
+
+        job?.cancel()
+        job =
+            vm.launchSigner {
+                try {
+                    when (source) {
+                        is ReloadSource.Mint -> rebalanceThenZap(source.mintUrl, s.selectedTarget, moveSats, note, s.amountSats)
+                        ReloadSource.Lightning -> reloadFromLightningThenZap(s.selectedTarget, moveSats, note, s.amountSats)
+                    }
+                } catch (e: CancellationException) {
+                    throw e // screen left mid-flow — don't mask as a Failed state
+                } catch (e: Exception) {
+                    setStatus(ReloadStatus.Failed(describeMintError(e)))
                }
-            } catch (e: Exception) {
-                setStatus(ReloadStatus.Failed(describeMintError(e)))
            }
-        }
    }

    private suspend fun rebalanceThenZap(
        sourceMint: String,
        targetMint: String,
-        shortfall: Long,
+        moveSats: Long,
        note: Note,
        amount: Long,
    ) {
        val st = state ?: return
        setStatus(ReloadStatus.Working("Moving funds", 0.1f))
-        st.rebalance(sourceMint, targetMint, shortfall) { p ->
+        st.rebalance(sourceMint, targetMint, moveSats) { p ->
            setStatus(ReloadStatus.Working("Moving funds", p.coerceIn(0.1f, 0.9f)))
        }
        sendNutzapAndFinish(note, amount)
@@ -212,7 +241,7 @@ class ReloadMintViewModel : ViewModel() {

    private suspend fun reloadFromLightningThenZap(
        targetMint: String,
-        shortfall: Long,
+        moveSats: Long,
        note: Note,
        amount: Long,
    ) {
@@ -221,7 +250,7 @@ class ReloadMintViewModel : ViewModel() {
        val ops = st.ops

        setStatus(ReloadStatus.Working("Requesting invoice", 0.1f))
-        val flow = ops.startMintFromLightning(targetMint, shortfall)
+        val flow = ops.startMintFromLightning(targetMint, moveSats)

        val walletUri = vm.account.settings.defaultZapPaymentRequest()
        if (walletUri != null) {
@@ -233,16 +262,18 @@ class ReloadMintViewModel : ViewModel() {
            }
        } else {
            // No NWC — surface the invoice for an external wallet and keep polling.
-            setStatus(ReloadStatus.AwaitingInvoice(flow.invoice, shortfall))
+            setStatus(ReloadStatus.AwaitingInvoice(flow.invoice, moveSats))
        }

+        // External payment can take a while; the poll runs on a job tied to the
+        // screen (cancelled in onCleared), so leaving stops it instead of
+        // hammering the mint for 3 minutes with nobody watching.
        val attempts = 90
        val delayMs = 2_000L
        var paid = false
        var attempt = 0
        while (!paid && attempt < attempts) {
-            val status = ops.checkMintQuote(targetMint, flow.mintQuote.quote)
-            paid = status.paid == true || status.state == "PAID" || status.state == "ISSUED"
+            paid = ops.checkMintQuote(targetMint, flow.mintQuote.quote).isSettled()
            if (!paid) {
                delay(delayMs)
                attempt++
@@ -253,23 +284,30 @@ class ReloadMintViewModel : ViewModel() {
            return
        }
        setStatus(ReloadStatus.Working("Issuing ecash", 0.85f))
-        ops.completeMintFromLightning(targetMint, flow.quoteEvent, shortfall)
+        ops.completeMintFromLightning(targetMint, flow.quoteEvent, moveSats)
        sendNutzapAndFinish(note, amount)
    }

-    private fun sendNutzapAndFinish(
+    /**
+     * Send the nutzap and only report [ReloadStatus.Done] once it actually
+     * succeeds. Suspends on the real send (throwing on failure) instead of the
+     * fire-and-forget AccountViewModel.sendNutzap, so a send that fails after a
+     * successful reload surfaces as Failed here rather than silently stranding
+     * the just-moved funds while the screen pops on a premature "done".
+     */
+    private suspend fun sendNutzapAndFinish(
        note: Note,
        amount: Long,
    ) {
-        val vm = accountViewModel ?: return
+        val st = state ?: return
+        val recipient = note.author?.pubkeyHex ?: throw IllegalStateException("Recipient has no pubkey")
+        val zappedEvent = note.toEventHint<Event>() ?: throw IllegalStateException("Nothing to zap")
        setStatus(ReloadStatus.Working("Sending zap", 0.95f))
-        // The destination is now funded; fire the nutzap (its own error path
-        // toasts) and report Done so the screen can pop.
-        vm.sendNutzap(
-            baseNote = note,
+        st.sendNutzap(
            amountSats = amount,
+            recipientPubKey = recipient,
+            zappedEvent = zappedEvent,
            message = "",
-            onError = { _, msg, _ -> setStatus(ReloadStatus.Failed(msg)) },
        )
        setStatus(ReloadStatus.Done)
    }
@@ -277,4 +315,21 @@ class ReloadMintViewModel : ViewModel() {
    private fun setStatus(status: ReloadStatus) {
        _uiState.update { it.copy(status = status) }
    }
+
+    override fun onCleared() {
+        // The pipeline runs on the AccountViewModel scope, not this VM's, so it
+        // would outlive the screen — cancel it when the screen goes away.
+        job?.cancel()
+        super.onCleared()
+    }
+
+    companion object {
+        /**
+         * Extra sats minted at the target beyond the bare shortfall, so the
+         * follow-up nutzap's own swap fee doesn't leave the mint a hair short.
+         * Most mints charge no input fee; this small cushion covers the ones
+         * that do without meaningfully over-minting.
+         */
+        private const val RELOAD_FEE_BUFFER_SATS = 2L
+    }
 }
@@ -192,7 +192,15 @@ data class MintQuoteBolt11ResponseDto(
    val state: String,
    val expiry: Long? = null,
    val paid: Boolean? = null,
-)
+) {
+    /**
+     * True once the mint considers the quote's invoice settled — either the
+     * legacy `paid` boolean (NUT-04 v0) or the `state` machine reaching PAID
+     * (invoice settled, proofs not yet issued) or ISSUED (proofs minted).
+     * Centralized so every poll loop agrees on what "paid" means.
+     */
+    fun isSettled(): Boolean = paid == true || state == "PAID" || state == "ISSUED"
+}

@Serializable
 data class MintBolt11RequestDto(