mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
8313f0cf12
commit
7bd5fb3122
+163
@@ -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 <T> 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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+1
@@ -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,
|
||||
|
||||
+61
-16
@@ -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 ->
|
||||
|
||||
@@ -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<Note>()
|
||||
}
|
||||
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<HexKey>, 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<NormalizedRelayUrl>()
|
||||
val allEose = CompletableDeferred<Unit>()
|
||||
|
||||
val listener = object : SubscriptionListener {
|
||||
override fun onEvent(event: Event, isLive: Boolean, relay: NormalizedRelayUrl, forFilters: List<Filter>?) {
|
||||
onEvent?.invoke(event, relay)
|
||||
}
|
||||
override fun onEose(relay: NormalizedRelayUrl, forFilters: List<Filter>?) {
|
||||
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`
|
||||
@@ -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<DecryptionResult> =
|
||||
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<T : IResult>(
|
||||
val message: String? = null,
|
||||
) : RequestAddressed<T>
|
||||
```
|
||||
|
||||
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<NormalizedRelayUrl>,
|
||||
) {
|
||||
private val awaitingRequests = LargeCache<String, Continuation<BunkerResponse>>()
|
||||
|
||||
suspend fun newResponse(responseEvent: NostrConnectEvent) {
|
||||
val decryptedJson = signer.decrypt(responseEvent.content, remoteKey)
|
||||
val bunkerResponse = OptimizedJsonMapper.fromJsonTo<BunkerResponse>(decryptedJson)
|
||||
awaitingRequests.get(bunkerResponse.id)?.resume(bunkerResponse)
|
||||
}
|
||||
|
||||
suspend fun <T : IResult> launchWaitAndParse(
|
||||
bunkerRequestBuilder: () -> BunkerRequest,
|
||||
parser: (response: BunkerResponse) -> SignerResult.RequestAddressed<T>,
|
||||
maxRetries: Int = 1,
|
||||
): SignerResult.RequestAddressed<T> {
|
||||
// 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 |
|
||||
Reference in New Issue
Block a user