From d6640be62c1db59e20e3a7360db5db105603b75b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 14:13:26 +0000 Subject: [PATCH] =?UTF-8?q?refactor(topup/zap):=20address=20audit=20cleanu?= =?UTF-8?q?ps=20=E2=80=94=20pin=20send=20mint,=20dedupe=20scrub,=20share?= =?UTF-8?q?=20clipboard,=20scope=20rebuilds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R2 — sendNutzap gains an optional preferredMintUrl; the Top-up screen passes the just-funded selectedTarget so the nutzap spends from THAT mint instead of whichever shared mint holds the most (which could leave the top-up sitting idle). Falls back to the best-balance pick when the preferred mint isn't a valid shared target. R5 — meltToLightning gains skipScrub; rebalance() (which already scrubbed the source for its coverage check) passes it to drop the redundant second NUT-07 /checkstate round-trip. The Top-up screen's wallet collector now projects to the per-mint balance map + distinctUntilChanged, so unrelated wallet activity (an inbound redeem, a scrub, a token for another mint) no longer re-runs the whole balances/targets/sources rebuild on every global tokenEntries emission. R7 — replace ReloadMintScreen's hand-rolled copyToClipboard with the shared Clipboard.setText helper (drops the android ClipData/ClipboardManager imports); document the keys-only observed values in the reactive railCapability block so a future reader doesn't delete them as "unused" and silently break the live rail loading + relay fetch. https://claude.ai/code/session_01HNE2z7CSYZ2G8KwC5fziJn --- .../model/nip60Cashu/CashuWalletState.kt | 33 ++++++++++++++++--- .../amethyst/ui/note/ReactionsRow.kt | 6 ++++ .../loggedIn/wallet/ReloadMintScreen.kt | 21 ++++-------- .../loggedIn/wallet/ReloadMintViewModel.kt | 25 +++++++++----- 4 files changed, 59 insertions(+), 26 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt index 902ca796d8..8aed359088 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt @@ -763,6 +763,16 @@ class CashuWalletState( */ fun peekNutzapTarget(recipientPubKey: HexKey): NutzapTarget? = peekNutzapFunding(recipientPubKey)?.target + /** True if [mintUrl] is a mint we hold AND the recipient accepts nutzaps on. */ + fun sharedNutzapMint( + recipientPubKey: HexKey, + mintUrl: String, + ): Boolean { + if (mintUrl !in _mints.value) return false + val info = cache.getOrCreateUser(recipientPubKey).nutzapInfo() ?: return false + return info.mints().any { it.mintUrl == mintUrl } + } + /** * Resolve nutzap funding for [recipientPubKey]: the best shared mint to * spend from plus the balance figures the zap picker needs to classify @@ -1078,12 +1088,22 @@ class CashuWalletState( recipientPubKey: HexKey, zappedEvent: EventHintBundle, message: String = "", + preferredMintUrl: String? = null, onProgress: ((Float) -> Unit)? = null, ): NutzapSent { check(started) { "CashuWalletState.start() not called" } - val target = + val resolved = peekNutzapTarget(recipientPubKey) ?: throw IllegalStateException("Recipient does not accept nutzaps from any of our mints") + // Honor an explicit mint when the caller has one in mind (e.g. the Top-up + // screen just funded a specific mint and must spend from THAT one, not + // whichever shared mint happens to hold the most). Only if it's still a + // valid shared target; otherwise fall back to the best-balance pick. + val target = + preferredMintUrl + ?.takeIf { it == resolved.mintUrl || sharedNutzapMint(recipientPubKey, it) } + ?.let { NutzapTarget(mintUrl = it, recipientP2pkPubkeyHex = resolved.recipientP2pkPubkeyHex) } + ?: resolved // NUT-07 check + immediate state cleanup before selecting. // The startup scrub can't catch entries that arrive from relays @@ -1146,11 +1166,14 @@ class CashuWalletState( suspend fun meltToLightning( mintUrl: String, quote: MeltQuoteBolt11ResponseDto, + skipScrub: Boolean = false, ): MeltCompleted { check(started) { "CashuWalletState.start() not called" } if (mintUrl.isBlank()) throw IllegalArgumentException("Pick a mint") - scrubLocallyStaleProofs(mintUrl) + // [rebalance] already scrubbed this mint to compute its coverage check, so + // it passes skipScrub=true to avoid a second NUT-07 /checkstate round-trip. + if (!skipScrub) scrubLocallyStaleProofs(mintUrl) val available = _tokenEntries.value.filter { it.content.mint == mintUrl } if (available.isEmpty()) { @@ -1205,9 +1228,11 @@ class CashuWalletState( throw IllegalStateException("$sourceMintUrl has $sourceBalance sat — needs $required to move $sats") } - // 3. Pay the destination invoice by melting at the source. + // 3. Pay the destination invoice by melting at the source. We already + // scrubbed sourceMintUrl above for the coverage check, so skip the + // redundant second scrub inside meltToLightning. onProgress?.invoke(0.5f) - meltToLightning(sourceMintUrl, meltQuote) + meltToLightning(sourceMintUrl, meltQuote, skipScrub = true) // Funds have now LEFT the source. Signal the caller immediately so a // failure in the steps below (slow confirmation, completeMint error) // never causes a retry to melt a second time — the destination quote is 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 e2e7f89962..2e352ab051 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 @@ -1962,6 +1962,12 @@ fun ZapAmountChoicePopup( // dialog (onchainSupported == false) masks that rail off here. 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. val cashuMints by cashuState.mints.collectAsStateWithLifecycle() val cashuEntries by cashuState.tokenEntries.collectAsStateWithLifecycle() val recipientInfo = author?.let { observeUserInfo(it, accountViewModel).value } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintScreen.kt index 51dc439c52..5aef6bdd64 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintScreen.kt @@ -20,9 +20,6 @@ */ package com.vitorpamplona.amethyst.ui.screen.loggedIn.wallet -import android.content.ClipData -import android.content.ClipboardManager -import android.content.Context import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column @@ -58,10 +55,11 @@ import androidx.compose.runtime.LaunchedEffect 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.platform.LocalContext +import androidx.compose.ui.platform.LocalClipboard import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.style.TextOverflow @@ -74,6 +72,7 @@ import com.vitorpamplona.amethyst.commons.hashtags.CustomHashTagIcons import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.model.Note +import com.vitorpamplona.amethyst.ui.components.util.setText import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.note.UserPicture @@ -81,6 +80,7 @@ import com.vitorpamplona.amethyst.ui.note.showAmount import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.placeholderText +import kotlinx.coroutines.launch import java.util.UUID import androidx.compose.material3.Icon as Material3Icon @@ -122,7 +122,8 @@ fun ReloadMintScreen( LaunchedEffect(requestId) { viewModel.init(accountViewModel, request) } val ui by viewModel.uiState.collectAsStateWithLifecycle() - val context = LocalContext.current + val clipboard = LocalClipboard.current + val scope = rememberCoroutineScope() // Local source-of-truth for the editable top-up field. The send amount is // fixed; the top-up defaults to the shortfall the VM computes once the wallet @@ -320,7 +321,7 @@ fun ReloadMintScreen( Text(stringRes(R.string.reload_mint_awaiting_payment), style = MaterialTheme.typography.bodyMedium) } OutlinedButton( - onClick = { copyToClipboard(context, status.invoice) }, + onClick = { scope.launch { clipboard.setText(status.invoice) } }, modifier = Modifier.fillMaxWidth(), ) { Text(stringRes(R.string.reload_mint_copy_invoice)) @@ -439,11 +440,3 @@ private fun SourceRow( } } } - -private fun copyToClipboard( - context: Context, - text: String, -) { - val clipboard = context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager - clipboard.setPrimaryClip(ClipData.newPlainText("invoice", text)) -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt index 477adc4af4..3fcb017ec3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt @@ -40,6 +40,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch @@ -161,18 +162,22 @@ class ReloadMintViewModel : ViewModel() { // Wallet proofs/mints arrive from relays and can land *after* the screen // opens — a one-time snapshot would show only the first-loaded mint. Keep - // balances/targets in sync as the wallet fills in. + // balances/targets in sync as the wallet fills in, but project to just the + // per-mint balance map and distinctUntilChanged so unrelated wallet churn + // (an inbound nutzap redeem, a scrub, a token for some other mint) doesn't + // re-run the whole rebuild on every global tokenEntries emission. viewModelScope.launch { - combine(st.tokenEntries, st.mints) { _, _ -> Unit }.collect { rebuild() } + combine(st.tokenEntries, st.mints) { _, _ -> st.peekMintBalances() } + .distinctUntilChanged() + .collect { rebuild(it) } } } /** Rebuild balances/targets/sources from the current wallet state, keeping the * user's chosen target, top-up, source pick, and status intact. */ - private fun rebuild() { - val st = state ?: return + private fun rebuild(balances: Map) { val recipient = recipient ?: return - val balances = st.peekMintBalances() + val st = state ?: return val targets = st.recipientSharedMints(recipient).sortedByDescending { balances[it] ?: 0L } _uiState.update { cur -> val target = cur.selectedTarget.takeIf { it in targets } ?: targets.firstOrNull().orEmpty() @@ -285,7 +290,7 @@ class ReloadMintViewModel : ViewModel() { when { // Already topped up (or the target already covers the send) — // never move funds again on retry, just (re)send the zap. - toppedUp || moveSats <= 0L -> sendNutzapAndFinish(note, s.sendSats) + toppedUp || moveSats <= 0L -> sendNutzapAndFinish(note, s.sendSats, s.selectedTarget) source is ReloadSource.Mint -> rebalanceThenZap(source.mintUrl, s.selectedTarget, moveSats, note, s.sendSats) source is ReloadSource.LightningWallet -> reloadFromLightningThenZap(s.selectedTarget, moveSats, walletUriFor(source.walletId), note, s.sendSats) @@ -320,7 +325,7 @@ class ReloadMintViewModel : ViewModel() { onFundsMoved = { toppedUp = true }, ) awaitTargetFunded(targetMint, sendSats) - sendNutzapAndFinish(note, sendSats) + sendNutzapAndFinish(note, sendSats, targetMint) } private suspend fun reloadFromLightningThenZap( @@ -374,7 +379,7 @@ class ReloadMintViewModel : ViewModel() { setStatus(ReloadStatus.Working("Issuing ecash", 0.85f)) ops.completeMintFromLightning(targetMint, flow.quoteEvent, moveSats) awaitTargetFunded(targetMint, sendSats) - sendNutzapAndFinish(note, sendSats) + sendNutzapAndFinish(note, sendSats, targetMint) } /** @@ -406,6 +411,7 @@ class ReloadMintViewModel : ViewModel() { private suspend fun sendNutzapAndFinish( note: Note, sendSats: Long, + targetMint: String, ) { val st = state ?: return val recipient = note.author?.pubkeyHex ?: throw IllegalStateException("Recipient has no pubkey") @@ -416,6 +422,9 @@ class ReloadMintViewModel : ViewModel() { recipientPubKey = recipient, zappedEvent = zappedEvent, message = "", + // Spend from the mint we just topped up, not whichever shared mint + // happens to hold the most — otherwise the top-up sits idle. + preferredMintUrl = targetMint.takeIf { it.isNotBlank() }, ) setStatus(ReloadStatus.Done) }