From 7bd5fb31225e11512400b90b68fbb0761f57a1bd Mon Sep 17 00:00:00 2001 From: nrobi144 Date: Tue, 12 May 2026 14:27:39 +0300 Subject: [PATCH] feat(desktop): wire NWC wallet operations and AccountManager integration - Wire AccountManager.setNwcConnection() and clearNwcConnection() into wallet column for persistent connect/disconnect - Implement NwcPaymentHandler.getBalance() via NIP-47 get_balance RPC - Implement NwcPaymentHandler.makeInvoice() via NIP-47 make_invoice RPC - Add generic waitForGenericResponse() helper for NWC RPC operations - Auto-fetch balance on wallet column load via LaunchedEffect - Wire receive screen to generate real invoices via NWC - All wallet column features now functional (no blocking TODOs) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../amethyst/desktop/nwc/NwcPaymentHandler.kt | 163 ++++++++ .../desktop/ui/deck/DeckColumnContainer.kt | 1 + .../desktop/ui/wallet/WalletColumnScreen.kt | 77 +++- ...rf-viewport-aware-metadata-loading-plan.md | 187 +++++++++ ...fix-bunker-timeouts-and-decryption-plan.md | 370 ++++++++++++++++++ 5 files changed, 782 insertions(+), 16 deletions(-) create mode 100644 docs/plans/2026-04-29-perf-viewport-aware-metadata-loading-plan.md create mode 100644 docs/plans/2026-05-04-fix-bunker-timeouts-and-decryption-plan.md diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt index 3f34c86263..dddd28667f 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt @@ -27,9 +27,13 @@ import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip47WalletConnect.Nip47Client import com.vitorpamplona.quartz.nip47WalletConnect.Nip47WalletConnect import com.vitorpamplona.quartz.nip47WalletConnect.events.LnZapPaymentRequestEvent import com.vitorpamplona.quartz.nip47WalletConnect.events.LnZapPaymentResponseEvent +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetBalanceSuccessResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.MakeInvoiceSuccessResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Response @@ -185,4 +189,163 @@ class NwcPaymentHandler( PaymentResult.Error("Unexpected response type: ${response.resultType}") } } + + // -- NWC RPC: get_balance -- + + sealed class BalanceResult { + data class Success( + val balanceMsats: Long, + ) : BalanceResult() + + data class Error( + val message: String, + ) : BalanceResult() + + data object Timeout : BalanceResult() + } + + suspend fun getBalance( + nwcConnection: Nip47WalletConnect.Nip47URINorm, + timeoutMs: Long = 30_000, + ): BalanceResult { + val secret = nwcConnection.secret ?: return BalanceResult.Error("NWC connection has no secret") + val nwcSigner = NostrSignerInternal(KeyPair(secret.hexToByteArray())) + val client = Nip47Client.fromNip47URI(nwcConnection) + val requestEvent = client.getBalance() + + relayManager.publishToRelay(nwcConnection.relayUri, requestEvent) + + return withTimeoutOrNull(timeoutMs) { + waitForGenericResponse(requestEvent.id, nwcConnection, nwcSigner) { response -> + when (response) { + is GetBalanceSuccessResponse -> { + val msats = response.result?.balance ?: 0L + BalanceResult.Success(msats) + } + + is NwcErrorResponse -> { + BalanceResult.Error(response.error?.message ?: "Unknown error") + } + + else -> { + BalanceResult.Error("Unexpected response: ${response.resultType}") + } + } + } + } ?: BalanceResult.Timeout + } + + // -- NWC RPC: make_invoice -- + + sealed class InvoiceResult { + data class Success( + val invoice: String, + val paymentHash: String?, + ) : InvoiceResult() + + data class Error( + val message: String, + ) : InvoiceResult() + + data object Timeout : InvoiceResult() + } + + suspend fun makeInvoice( + nwcConnection: Nip47WalletConnect.Nip47URINorm, + amountMsats: Long, + description: String? = null, + timeoutMs: Long = 30_000, + ): InvoiceResult { + val secret = nwcConnection.secret ?: return InvoiceResult.Error("NWC connection has no secret") + val nwcSigner = NostrSignerInternal(KeyPair(secret.hexToByteArray())) + val client = Nip47Client.fromNip47URI(nwcConnection) + val requestEvent = client.makeInvoice(amountMsats, description) + + relayManager.publishToRelay(nwcConnection.relayUri, requestEvent) + + return withTimeoutOrNull(timeoutMs) { + waitForGenericResponse(requestEvent.id, nwcConnection, nwcSigner) { response -> + when (response) { + is MakeInvoiceSuccessResponse -> { + val invoice = response.result?.invoice + if (invoice != null) { + InvoiceResult.Success(invoice, response.result?.payment_hash) + } else { + InvoiceResult.Error("Wallet returned no invoice") + } + } + + is NwcErrorResponse -> { + InvoiceResult.Error(response.error?.message ?: "Unknown error") + } + + else -> { + InvoiceResult.Error("Unexpected response: ${response.resultType}") + } + } + } + } ?: InvoiceResult.Timeout + } + + // -- Generic NWC response listener -- + + private suspend fun waitForGenericResponse( + requestId: String, + nwcConnection: Nip47WalletConnect.Nip47URINorm, + nwcSigner: NostrSignerInternal, + processResponse: (Response) -> T, + ): T = + suspendCancellableCoroutine { continuation -> + val filter = + Filter( + kinds = listOf(LnZapPaymentResponseEvent.KIND), + authors = listOf(nwcConnection.pubKeyHex), + tags = mapOf("e" to listOf(requestId)), + ) + + val subId = "nwc-rpc-${requestId.take(8)}" + + relayManager.subscribeOnRelay( + relay = nwcConnection.relayUri, + subId = subId, + filters = listOf(filter), + onEvent = { event, _ -> + if (event is LnZapPaymentResponseEvent && event.requestId() == requestId) { + @OptIn(kotlinx.coroutines.DelicateCoroutinesApi::class) + kotlinx.coroutines.GlobalScope.launch(kotlinx.coroutines.Dispatchers.IO) { + if (!localCache.justVerify(event)) return@launch + + relayManager.closeSubscription(nwcConnection.relayUri, subId) + + try { + val response = event.decrypt(nwcSigner) + val result = processResponse(response) + if (continuation.isActive) { + continuation.resume(result) + } + } catch (e: Exception) { + if (e is kotlinx.coroutines.CancellationException) throw e + if (continuation.isActive) { + continuation.resume( + processResponse( + NwcErrorResponse( + resultType = "error", + error = + com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcError( + message = "Decrypt failed: ${e.message}", + ), + ), + ), + ) + } + } + } + } + }, + ) + + continuation.invokeOnCancellation { + relayManager.closeSubscription(nwcConnection.relayUri, subId) + } + } } diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt index 08678668c4..86d23cf80f 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt @@ -359,6 +359,7 @@ internal fun RootContent( DeckColumnType.Wallet -> { com.vitorpamplona.amethyst.desktop.ui.wallet.WalletColumnScreen( account = account, + accountManager = accountManager, relayManager = relayManager, localCache = localCache, nwcConnection = nwcConnection, diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt index d3ab7f0f9c..2de8f2fcc4 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt @@ -45,6 +45,7 @@ import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -57,12 +58,12 @@ import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols +import com.vitorpamplona.amethyst.desktop.account.AccountManager import com.vitorpamplona.amethyst.desktop.account.AccountState import com.vitorpamplona.amethyst.desktop.cache.DesktopLocalCache import com.vitorpamplona.amethyst.desktop.network.DesktopRelayConnectionManager import com.vitorpamplona.amethyst.desktop.nwc.NwcPaymentHandler import com.vitorpamplona.amethyst.desktop.ui.ZapFeedback -import com.vitorpamplona.quartz.nip47WalletConnect.Nip47WalletConnect import com.vitorpamplona.quartz.nip47WalletConnect.Nip47WalletConnect.Nip47URINorm import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -82,6 +83,7 @@ enum class WalletScreen { @Composable fun WalletColumnScreen( account: AccountState.LoggedIn, + accountManager: AccountManager, relayManager: DesktopRelayConnectionManager, localCache: DesktopLocalCache, nwcConnection: Nip47URINorm?, @@ -117,6 +119,21 @@ fun WalletColumnScreen( NwcPaymentHandler(relayManager, localCache) } + // Auto-fetch balance when wallet connects + LaunchedEffect(nwcConnection) { + if (nwcConnection != null && balanceSats == null) { + isLoadingBalance = true + when (val result = paymentHandler.getBalance(nwcConnection)) { + is NwcPaymentHandler.BalanceResult.Success -> { + balanceSats = result.balanceMsats / 1000 + } + + else -> { /* silently fail on auto-fetch */ } + } + isLoadingBalance = false + } + } + Column(modifier = Modifier.fillMaxSize()) { when (currentScreen) { WalletScreen.HOME -> { @@ -131,15 +148,27 @@ fun WalletColumnScreen( if (nwcConnection != null) { isLoadingBalance = true scope.launch { - // TODO: implement NWC get_balance RPC + when (val result = paymentHandler.getBalance(nwcConnection)) { + is NwcPaymentHandler.BalanceResult.Success -> { + balanceSats = result.balanceMsats / 1000 + } + + is NwcPaymentHandler.BalanceResult.Error -> { + snackbarHostState.showSnackbar("Balance error: ${result.message}") + } + + is NwcPaymentHandler.BalanceResult.Timeout -> { + snackbarHostState.showSnackbar("Balance request timed out") + } + } isLoadingBalance = false } } }, onDisconnect = { - // TODO: clear NWC from account settings + accountManager.clearNwcConnection() scope.launch { - snackbarHostState.showSnackbar("Disconnect from account settings") + snackbarHostState.showSnackbar("Wallet disconnected") } }, ) @@ -167,14 +196,12 @@ fun WalletColumnScreen( } }, onConnect = { - val parsed = Nip47WalletConnect.parse(nwcUri) - if (parsed != null) { - // TODO: save to account settings - isConnecting = true + val result = accountManager.setNwcConnection(nwcUri) + if (result.isSuccess) { + nwcUri = "" + currentScreen = WalletScreen.HOME scope.launch { - snackbarHostState.showSnackbar("Wallet connected! Restart to apply.") - isConnecting = false - currentScreen = WalletScreen.HOME + snackbarHostState.showSnackbar("Wallet connected!") } } else { connectionError = "Invalid NWC URI. Expected: nostr+walletconnect://..." @@ -245,11 +272,29 @@ fun WalletColumnScreen( onAmountChanged = { receiveAmount = it }, onDescriptionChanged = { receiveDescription = it }, onGenerate = { - // TODO: implement NWC make_invoice RPC - scope.launch { - isGenerating = true - snackbarHostState.showSnackbar("make_invoice not yet implemented") - isGenerating = false + if (nwcConnection != null) { + val amountSats = receiveAmount.toLongOrNull() ?: 0L + if (amountSats > 0) { + isGenerating = true + scope.launch { + val amountMsats = amountSats * 1000 + val desc = receiveDescription.ifBlank { null } + when (val result = paymentHandler.makeInvoice(nwcConnection, amountMsats, desc)) { + is NwcPaymentHandler.InvoiceResult.Success -> { + generatedInvoice = result.invoice + } + + is NwcPaymentHandler.InvoiceResult.Error -> { + snackbarHostState.showSnackbar("Invoice error: ${result.message}") + } + + is NwcPaymentHandler.InvoiceResult.Timeout -> { + snackbarHostState.showSnackbar("Invoice request timed out") + } + } + isGenerating = false + } + } } }, onCopyInvoice = { invoice -> diff --git a/docs/plans/2026-04-29-perf-viewport-aware-metadata-loading-plan.md b/docs/plans/2026-04-29-perf-viewport-aware-metadata-loading-plan.md new file mode 100644 index 0000000000..9ff4aec9eb --- /dev/null +++ b/docs/plans/2026-04-29-perf-viewport-aware-metadata-loading-plan.md @@ -0,0 +1,187 @@ +--- +title: "perf: Viewport-aware feed metadata loading" +type: perf +status: active +date: 2026-04-29 +origin: docs/brainstorms/2026-04-29-feed-metadata-loading-optimization-brainstorm.md +--- + +# perf: Viewport-aware feed metadata loading + +## Enhancement Summary + +**Deepened on:** 2026-04-29 +**Research agents:** compose-expert, kotlin-coroutines, nostr-expert, relay-client + +### Key Improvements +1. Concrete `snapshotFlow` + `debounce` pattern for viewport detection (zero recomposition) +2. `loadMetadataBatched()` implementation with EOSE close (follows Chess helper pattern) +3. Nostr filter sizing: 100 authors max per filter, CLOSE after EOSE +4. `collectLatest` for cancelling stale fetches on scroll change + +--- + +## Overview + +Feed metadata (display names, avatars) takes 5+ seconds because the pipeline loads metadata for ALL notes (100+), rate-limits at 20/sec, and creates individual subscriptions per author. Fix with viewport-aware loading + batched author filter. + +## Problem Statement + +Pre-existing on `main`. Pipeline: +1. `visibleNotes()` returns ALL notes (not viewport-filtered) +2. `MetadataRateLimiter`: 20 pubkeys/sec with 1-sec batch delays +3. Each author gets individual `client.subscribe()` call +4. All subscriptions broadcast to 7 relays + +(see brainstorm: `docs/brainstorms/2026-04-29-feed-metadata-loading-optimization-brainstorm.md`) + +## Technical Approach + +### Phase 1: Viewport-Aware Note Selection + +**Goal:** Only fetch metadata for visible notes + 10-item buffer. + +**Tasks:** +- [ ] Replace the existing `LaunchedEffect(feedState, subscriptionsCoordinator)` in `FeedScreen.kt:354` with a viewport-aware version using `snapshotFlow` +- [ ] Use `lazyListState.layoutInfo.visibleItemsInfo` inside `snapshotFlow` (NOT in composition — avoids per-frame recomposition) +- [ ] Buffer ±10 items with `coerceIn(0, list.lastIndex)` to prevent IndexOutOfBounds +- [ ] Debounce at 500ms via `.debounce(500)` +- [ ] Use `collectLatest` to cancel stale fetches when scroll position changes + +**Implementation pattern (compose-expert + kotlin-coroutines):** + +```kotlin +// In FeedScreen, sibling to LazyColumn (not inside it) +LaunchedEffect(lazyListState, feedNotes) { + snapshotFlow { + val info = lazyListState.layoutInfo + if (info.visibleItemsInfo.isEmpty() || feedNotes.isEmpty()) { + return@snapshotFlow emptyList() + } + val first = (info.visibleItemsInfo.first().index - 10).coerceAtLeast(0) + val last = (info.visibleItemsInfo.last().index + 10).coerceAtMost(feedNotes.lastIndex) + feedNotes.subList(first, last + 1) + } + .distinctUntilChanged() + .debounce(500) + .collectLatest { viewportNotes -> + if (viewportNotes.isNotEmpty()) { + // Fast path: batched metadata for visible authors + val authors = viewportNotes.mapNotNull { it.author?.pubkeyHex }.distinct() + subscriptionsCoordinator.loadMetadataBatched(authors) + // Also load reactions for visible notes + subscriptionsCoordinator.loadMetadataForNotes(viewportNotes) + } + } +} +``` + +**Key insights:** +- `snapshotFlow` reads layout info outside composition → zero recomposition cost +- `collectLatest` cancels in-flight `loadMetadataBatched` when scroll changes +- `distinctUntilChanged` skips if same indices visible after debounce +- `feedNotes` captured in `LaunchedEffect` key ensures re-launch when feed data changes + +**Files:** +- `desktopApp/.../ui/FeedScreen.kt` — replace metadata LaunchedEffect + +### Phase 2: Batched Author Subscription + +**Goal:** One relay subscription per batch instead of N individual ones. + +**Tasks:** +- [ ] Add `loadMetadataBatched(pubkeys)` to `FeedMetadataCoordinator` +- [ ] Bypass rate limiter — subscribe directly via `client.subscribe()` +- [ ] Single `Filter(kind:0, authors:pubkeys, limit:pubkeys.size)` sent to index relays +- [ ] Close subscription after EOSE from all relays (one-shot fetch) +- [ ] 5-second timeout to prevent hanging on slow relays +- [ ] Deduplicate against `queuedPubkeys` to avoid double-fetch with background path + +**Implementation pattern (relay-client + nostr-expert):** + +```kotlin +// In FeedMetadataCoordinator — follows ChessRelayFetchHelper pattern +fun loadMetadataBatched(pubkeys: List, timeoutMs: Long = 5_000L) { + val newPubkeys = pubkeys.filter { it !in queuedPubkeys }.distinct() + if (newPubkeys.isEmpty()) return + queuedPubkeys.addAll(newPubkeys) + + scope.launch { + val filter = Filter( + kinds = listOf(MetadataEvent.KIND), + authors = newPubkeys.take(100), // max 100 per filter + limit = newPubkeys.size, + ) + val filterMap = indexRelays.associateWith { listOf(filter) } + val subId = newSubId() + val eoseReceived = mutableSetOf() + val allEose = CompletableDeferred() + + val listener = object : SubscriptionListener { + override fun onEvent(event: Event, isLive: Boolean, relay: NormalizedRelayUrl, forFilters: List?) { + onEvent?.invoke(event, relay) + } + override fun onEose(relay: NormalizedRelayUrl, forFilters: List?) { + eoseReceived.add(relay) + if (eoseReceived.size >= indexRelays.size) allEose.complete(Unit) + } + } + + client.subscribe(subId, filterMap, listener) + withTimeoutOrNull(timeoutMs) { allEose.await() } + client.unsubscribe(subId) + } +} +``` + +**Key insights (nostr-expert):** +- Kind 0 is replaceable — relay returns one event per pubkey, bulk lookup is efficient +- 100 authors per filter is safe for index relays (purplepag.es, profiles.nostr1.com) +- CLOSE after EOSE — transient viewport set doesn't need live updates +- No `since` on cold load — relay returns latest replaceable event regardless +- Pattern matches existing `ChessRelayFetchHelper.fetchEvents()` idiom in codebase + +**Files:** +- `commons/.../relayClient/assemblers/FeedMetadataCoordinator.kt` — add `loadMetadataBatched()` + +### Phase 3: Progressive Background Loading + +**Goal:** Pre-warm cache for off-screen notes. + +**Tasks:** +- [ ] After viewport metadata loads via batched path, queue remaining feed authors through existing `loadMetadataForNotes()` (rate-limited, low priority) +- [ ] This runs in background — no UI impact + +**Files:** +- `desktopApp/.../ui/FeedScreen.kt` — secondary background pass after viewport pass + +## Acceptance Criteria + +- [ ] Visible note metadata loads within 1-2 seconds of feed render +- [ ] Scrolling to new notes triggers metadata fetch within 500ms +- [ ] No per-frame recomposition from scroll observation (verify with Layout Inspector) +- [ ] No regression for off-screen notes (still loads via background path) +- [ ] `./gradlew :desktopApp:compileKotlin` succeeds +- [ ] `./gradlew :commons:compileKotlinJvm` succeeds +- [ ] `./gradlew spotlessApply` passes +- [ ] Existing tests pass + +## Key Files + +| File | Purpose | +|------|---------| +| `desktopApp/.../ui/FeedScreen.kt:354` | LaunchedEffect trigger (replace) | +| `commons/.../relayClient/assemblers/FeedMetadataCoordinator.kt` | Add `loadMetadataBatched()` | +| `commons/.../relayClient/preload/MetadataRateLimiter.kt` | Bypassed for viewport path | +| `commons/.../chess/ChessRelayFetchHelper.kt:82` | Reference pattern for one-shot fetch | + +## Sources & References + +### Origin +- **Brainstorm:** [docs/brainstorms/2026-04-29-feed-metadata-loading-optimization-brainstorm.md](docs/brainstorms/2026-04-29-feed-metadata-loading-optimization-brainstorm.md) + +### Internal References +- ChessRelayFetchHelper (one-shot pattern): `commons/.../chess/ChessRelayFetchHelper.kt:82-131` +- MetadataFilterAssembler (persistent sub): `commons/.../relayClient/assemblers/MetadataFilterAssembler.kt` +- FeedContentState.visibleNotes: `commons/.../ui/feeds/FeedContentState.kt:77` +- DefaultIndexerRelayList: `amethyst/.../model/Constants.kt` diff --git a/docs/plans/2026-05-04-fix-bunker-timeouts-and-decryption-plan.md b/docs/plans/2026-05-04-fix-bunker-timeouts-and-decryption-plan.md new file mode 100644 index 0000000000..5e13f70555 --- /dev/null +++ b/docs/plans/2026-05-04-fix-bunker-timeouts-and-decryption-plan.md @@ -0,0 +1,370 @@ +# Fix: Bunker Timeouts & Broken Decryption + +**Branch**: `fix/bunker-timeout-and-decrypt` +**Date**: 2026-05-04 +**Deepened**: 2026-05-04 + +## Enhancement Summary + +**Research agents used:** 8 (timeout mechanics, decrypt UI trace, retry edge cases, SignerResult structure, NIP-46 protocol, test coverage, auth-signers skill, coroutine testing patterns) + +### Key Improvements from Research +1. **Retry is unsafe as originally planned** — fresh UUIDs per attempt mean old responses are silently dropped. Changed to: republish same request (same ID) + extend timeout. +2. **Desktop shows raw ciphertext on failure** (confirmed at `ChatPane.kt:447`) — need error placeholder like Android's `R.string.could_not_decrypt_the_message`. +3. **DecryptCache marks `CouldNotPerformException` as `DontTryAgain`** — the parser bug causes PERMANENT failure caching. Fixing the parser also fixes the cache poisoning. +4. **Zero logging in NIP-46 module** — add structured logging for debugging. + +### New Considerations Discovered +- `client.publish()` is fire-and-forget with no error feedback +- Late responses after timeout are silently discarded (continuation already removed) +- `DecryptCache` won't retry `CouldNotPerformException` — existing cached failures need invalidation after fix + +--- + +## Problem Summary + +| Issue | Symptom | Root Cause | +|-------|---------|-----------| +| Timeout | Amber shows "1m event timeout" | Amethyst 30s timeout < Amber 60s timeout | +| Decryption | Weird strings shown instead of messages | `BunkerResponseDeserializer` never creates `BunkerResponseDecrypt`; parsers reject generic `BunkerResponse` | + +--- + +## Fix 1: Response Parsers (Decryption) + +### Root Cause + +`BunkerResponseDeserializer` (jvmAndroid) is context-free — it can't distinguish decrypt results (plaintext) from encrypt results (ciphertext). When result is a plain string that's not a pubkey/JSON/ack/pong, it falls through to `BunkerResponse(id, result, error)` base type. + +The 4 response parsers only match specific subtypes, so all hit `else -> ReceivedButCouldNotPerform()`. + +### Research Insight: Cache Poisoning + +`DecryptCache` (at `quartz/.../signers/caches/DecryptCache.kt`) handles exceptions: +- `CouldNotPerformException` → **`DontTryAgain`** (permanent failure, never retried) +- `TimedOutException` → `CanTryAgain` (retries after 10s) + +This means the parser bug causes **permanent cache poisoning**: once a message fails to decrypt due to the wrong response type, it's cached as permanently undecryptable until app restart. + +### Fix (Option A) — Make parsers handle generic `BunkerResponse` + +**Files to modify:** +- `quartz/src/commonMain/.../nip46RemoteSigner/signer/Nip04DecryptResponse.kt` +- `quartz/src/commonMain/.../nip46RemoteSigner/signer/Nip44DecryptResponse.kt` +- `quartz/src/commonMain/.../nip46RemoteSigner/signer/Nip04EncryptResponse.kt` +- `quartz/src/commonMain/.../nip46RemoteSigner/signer/Nip44EncryptResponse.kt` + +**Pattern (decrypt parsers):** +```kotlin +class Nip04DecryptResponse { + companion object { + fun parse(response: BunkerResponse): SignerResult.RequestAddressed = + when (response) { + is BunkerResponseDecrypt -> { + SignerResult.RequestAddressed.Successful(DecryptionResult(response.plaintext)) + } + is BunkerResponseError -> { + SignerResult.RequestAddressed.Rejected() + } + else -> { + // Deserializer can't distinguish decrypt results from other strings. + // If we got a generic BunkerResponse with a non-null result, treat as plaintext. + response.result?.let { + SignerResult.RequestAddressed.Successful(DecryptionResult(it)) + } ?: SignerResult.RequestAddressed.ReceivedButCouldNotPerform("No result in response") + } + } + } +} +``` + +**Pattern (encrypt parsers):** +```kotlin +else -> { + response.result?.let { + SignerResult.RequestAddressed.Successful(EncryptionResult(it)) + } ?: SignerResult.RequestAddressed.ReceivedButCouldNotPerform("No result in response") +} +``` + +### Research Insight: `ReceivedButCouldNotPerform` already has `message: String?` + +Confirmed at `SignerResult.kt:35-37`: +```kotlin +class ReceivedButCouldNotPerform( + val message: String? = null, +) : RequestAddressed +``` + +And `convertExceptions()` at `NostrSignerRemote.kt:291`: +```kotlin +is SignerResult.RequestAddressed.ReceivedButCouldNotPerform<*> -> + SignerExceptions.CouldNotPerformException("$title: ${result.message}") +``` + +No changes needed to SignerResult — just pass meaningful messages. + +--- + +## Fix 2: Timeout + Retry + +### Root Cause + +`RemoteSignerManager.timeout = 30_000L` is too short. Amber allows 60s for user approval. After relay latency, Amethyst gives up before Amber responds. + +### Research Insight: Retry with Fresh UUIDs is UNSAFE + +Each `bunkerRequestBuilder()` generates a fresh UUID: +```kotlin +class BunkerRequestSign( + id: String = Uuid.random().toString(), // NEW UUID every call + ... +) +``` + +**Problem with naive retry:** +1. Attempt 1: publishes request with UUID-A, times out +2. `invokeOnCancellation` removes UUID-A from `awaitingRequests` +3. Bunker responds to UUID-A → `awaitingRequests.get("UUID-A")` → null → silently dropped +4. Attempt 2: publishes with UUID-B, but bunker already processed UUID-A (may not respond to UUID-B) +5. Result: permanent failure + +### Revised Fix: Republish Same Request + Extended Timeout + +**Strategy:** Build the request ONCE (single UUID), then retry = republish the same event. + +**File:** `quartz/src/commonMain/.../nip46RemoteSigner/signer/RemoteSignerManager.kt` + +```kotlin +class RemoteSignerManager( + val timeout: Long = 65_000, // Match Amber's 60s + 5s relay buffer + val client: INostrClient, + val signer: NostrSignerInternal, + val remoteKey: String, + val relayList: Set, +) { + private val awaitingRequests = LargeCache>() + + suspend fun newResponse(responseEvent: NostrConnectEvent) { + val decryptedJson = signer.decrypt(responseEvent.content, remoteKey) + val bunkerResponse = OptimizedJsonMapper.fromJsonTo(decryptedJson) + awaitingRequests.get(bunkerResponse.id)?.resume(bunkerResponse) + } + + suspend fun launchWaitAndParse( + bunkerRequestBuilder: () -> BunkerRequest, + parser: (response: BunkerResponse) -> SignerResult.RequestAddressed, + maxRetries: Int = 1, + ): SignerResult.RequestAddressed { + // Build request ONCE — same UUID for all attempts + val request = bunkerRequestBuilder() + val event = NostrConnectEvent.create( + message = request, + remoteKey = remoteKey, + signer = signer, + ) + + var attempt = 0 + while (true) { + val result = tryAndWait(timeout) { continuation -> + continuation.invokeOnCancellation { + awaitingRequests.remove(request.id) + } + awaitingRequests.put(request.id, continuation) + client.publish(event, relayList = relayList) + } + + when { + result != null -> return parser(result) + attempt >= maxRetries -> return SignerResult.RequestAddressed.TimedOut() + else -> { + attempt++ + delay(2_000L) // Brief pause before republish + } + } + } + } +} +``` + +**Key differences from original plan:** +1. `bunkerRequestBuilder()` called ONCE (same UUID across retries) +2. `NostrConnectEvent.create()` called ONCE (same encrypted event republished) +3. `maxRetries = 1` (conservative: 1 retry = 2 total attempts = ~130s max) +4. `delay(2_000L)` flat (no exponential — relay reconnection is the issue, not load) + +### Research Insight: `client.publish()` is fire-and-forget + +`NostrClient.publish()` queues to outbox and returns immediately — no success/failure feedback. Republishing the same event to relays is idempotent (relays deduplicate by event ID). + +### Research Insight: No `since` filter on subscription + +`StaticSubscription` uses: +```kotlin +Filter(kinds = listOf(24133), tags = mapOf("p" to listOf(signer.pubKey))) +``` + +No `since` → relay reconnection replays old events. This is GOOD for our retry: if relay reconnects and replays the bunker's response, we'll catch it on the second attempt. + +--- + +## Fix 3: Desktop UI Error Display + +### Research Insight: Desktop shows raw ciphertext + +**File:** `desktopApp/src/jvmMain/.../desktop/ui/chats/ChatPane.kt` (lines 447-466) + +Current code: +```kotlin +decryptedContent = when (event) { + is PrivateDmEvent -> { + try { + event.decryptContent(account.signer) + } catch (_: Exception) { + event.content // BUG: Shows raw ciphertext (base64 gibberish) + } + } + else -> event?.content +} +``` + +Android equivalent uses `LoadDecryptedContentOrNull` with: +```kotlin +if (eventContent != null) { + TranslatableRichTextViewer(content = eventContent, ...) +} else { + TranslatableRichTextViewer(content = stringRes(R.string.could_not_decrypt_the_message), ...) +} +``` + +### Fix: Show error placeholder on Desktop + +```kotlin +decryptedContent = when (event) { + is PrivateDmEvent -> { + try { + event.decryptContent(account.signer) + } catch (_: Exception) { + null // Changed: signal failure instead of showing raw ciphertext + } + } + else -> event?.content +} + +// In display: +Text( + text = decryptedContent ?: "Could not decrypt the message", + style = if (decryptedContent == null) + MaterialTheme.typography.bodyMedium.copy(fontStyle = FontStyle.Italic) + else + MaterialTheme.typography.bodyMedium, + color = if (decryptedContent == null) + MaterialTheme.colorScheme.onSurfaceVariant + else + MaterialTheme.colorScheme.onSurface, +) +``` + +### Research Insight: Cache invalidation after fix + +After deploying Fix 1, previously-failed messages are stuck in `DontTryAgain` cache state. Options: +- **App restart clears cache** (DecryptCache is in-memory only) — simplest +- Document that users should restart after update +- No code change needed for this + +--- + +## Phase Plan + +### Phase 1: Fix decrypt/encrypt response parsing (P0) +1. Update `Nip04DecryptResponse.parse()` — handle generic `BunkerResponse` with `response.result` +2. Update `Nip44DecryptResponse.parse()` — same pattern +3. Update `Nip04EncryptResponse.parse()` — same pattern for encrypt +4. Update `Nip44EncryptResponse.parse()` — same pattern +5. Write tests: generic `BunkerResponse(id, "Hello world", null)` → `Successful(DecryptionResult("Hello world"))` +6. Write tests: generic `BunkerResponse(id, null, null)` → `ReceivedButCouldNotPerform` + +### Phase 2: Fix timeout + safe retry (P0) +1. Increase `RemoteSignerManager.timeout` to `65_000` +2. Refactor `launchWaitAndParse`: build request once, retry = republish same event +3. Add `import kotlinx.coroutines.delay` +4. Write test: first `tryAndWait` times out, second succeeds (same request ID) +5. Write test: all attempts timeout → `TimedOut()` result +6. Write test: verify request built only once (same UUID across retries) + +### Phase 3: Desktop UI error display (P1) +1. In `ChatPane.kt`, change catch fallback from `event.content` to `null` +2. Display italic error placeholder when `decryptedContent == null` +3. Match Android's UX pattern (clear error state, not raw ciphertext) + +### Phase 4: Logging (P2) +1. Add `Log.d` in `newResponse()` when response arrives but no continuation found (late response) +2. Add `Log.w` in `launchWaitAndParse()` when timeout occurs (with request method) +3. Add `Log.d` on retry attempt + +### Phase 5: Test coverage +New tests: +- `ResponseParserDecryptFallbackTest` — generic BunkerResponse → Successful(DecryptionResult) +- `ResponseParserEncryptFallbackTest` — generic BunkerResponse → Successful(EncryptionResult) +- `ResponseParserNullResultTest` — BunkerResponse with null result → ReceivedButCouldNotPerform +- `RemoteSignerManagerRetryTest` — timeout then success (uses `runTest` + virtual time) +- `RemoteSignerManagerSameIdTest` — verify UUID stability across retries +- `RemoteSignerManagerMaxRetriesTest` — all timeout → TimedOut + +**Test patterns to use** (from kotlin-coroutines skill): +```kotlin +@Test +fun `retry succeeds on second attempt`() = runTest { + val fakeClient = EmptyNostrClient() + val manager = RemoteSignerManager( + timeout = 1000, // Short for tests + client = fakeClient, + signer = testSigner, + remoteKey = testRemoteKey, + relayList = setOf(testRelay), + ) + // ... simulate late response arrival +} +``` + +--- + +## Manual Testing Checklist + +- [ ] Login with bunker:// URI on Desktop +- [ ] Send a DM (requires sign + encrypt) +- [ ] Receive and read a DM (requires decrypt) +- [ ] Post a note (requires sign) +- [ ] Verify no "weird strings" in message views +- [ ] Verify error placeholder shown when bunker offline +- [ ] Verify timeout doesn't fire before 60s +- [ ] Restart app after fix to clear poisoned DecryptCache entries + +--- + +## File Change Summary + +| File | Change | Phase | +|------|--------|-------| +| `quartz/.../signer/Nip04DecryptResponse.kt` | Fallback to `response.result` | 1 | +| `quartz/.../signer/Nip44DecryptResponse.kt` | Fallback to `response.result` | 1 | +| `quartz/.../signer/Nip04EncryptResponse.kt` | Fallback to `response.result` | 1 | +| `quartz/.../signer/Nip44EncryptResponse.kt` | Fallback to `response.result` | 1 | +| `quartz/.../signer/RemoteSignerManager.kt` | Timeout 65s, retry with same request | 2 | +| `desktopApp/.../ui/chats/ChatPane.kt` | Error placeholder instead of raw ciphertext | 3 | +| `quartz/src/commonTest/.../ResponseParserFallbackTest.kt` | New test file | 5 | +| `quartz/src/commonTest/.../RemoteSignerManagerRetryTest.kt` | New test file | 5 | + +--- + +## Questions (Resolved) + +| Q | A | +|---|---| +| Android or Desktop? | Desktop (Jackson deserializer path) | +| Amber or Amethyst timeout msg? | Amber UI | +| Global or per-operation timeout? | Global, sync with Amber | +| Retry or just longer timeout? | Both: longer timeout + safe republish retry | +| Option A or B for decrypt? | A (parser handles generic BunkerResponse) | +| UUID per retry? | NO — build request once, reuse across attempts | +| Cache invalidation? | Not needed — app restart clears in-memory DecryptCache |