fix(desktop): verify user pubkey via get_public_key after nostrconnect login
The nostrconnect flow was trusting params[0] from the signer's connect message as the user's pubkey. Some signers (e.g. nsec.app) don't put the actual user pubkey there, causing all relay subscriptions to query the wrong identity — contact lists, profiles, and DMs all returned empty. Now calls remoteSigner.getPublicKey() after the handshake to get the verified pubkey from the signer. Also stabilizes FeedScreen relay subscriptions with distinctUntilChanged() and adds relay.primal.net to defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+143
-23
@@ -30,7 +30,12 @@ import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray
|
|||||||
import com.vitorpamplona.quartz.nip01Core.core.toHexKey
|
import com.vitorpamplona.quartz.nip01Core.core.toHexKey
|
||||||
import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
|
import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
|
||||||
import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient
|
import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient
|
||||||
|
import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.IRelayClientListener
|
||||||
import com.vitorpamplona.quartz.nip01Core.relay.client.reqs.req
|
import com.vitorpamplona.quartz.nip01Core.relay.client.reqs.req
|
||||||
|
import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient
|
||||||
|
import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.Message
|
||||||
|
import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.Command
|
||||||
|
import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.EventCmd
|
||||||
import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
||||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
||||||
import com.vitorpamplona.quartz.nip01Core.relay.sockets.okhttp.BasicOkHttpWebSocket
|
import com.vitorpamplona.quartz.nip01Core.relay.sockets.okhttp.BasicOkHttpWebSocket
|
||||||
@@ -135,6 +140,9 @@ class AccountManager internal constructor(
|
|||||||
private val _forceLogoutReason = MutableStateFlow<String?>(null)
|
private val _forceLogoutReason = MutableStateFlow<String?>(null)
|
||||||
val forceLogoutReason: StateFlow<String?> = _forceLogoutReason.asStateFlow()
|
val forceLogoutReason: StateFlow<String?> = _forceLogoutReason.asStateFlow()
|
||||||
|
|
||||||
|
private val _loginProgress = MutableStateFlow<LoginProgress?>(null)
|
||||||
|
val loginProgress: StateFlow<LoginProgress?> = _loginProgress.asStateFlow()
|
||||||
|
|
||||||
private var heartbeatJob: Job? = null
|
private var heartbeatJob: Job? = null
|
||||||
|
|
||||||
// --- Dedicated NIP-46 client (isolated from general relay pool) ---
|
// --- Dedicated NIP-46 client (isolated from general relay pool) ---
|
||||||
@@ -174,18 +182,72 @@ class AccountManager internal constructor(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun updateRelayLoginStatus(
|
||||||
|
relay: NormalizedRelayUrl,
|
||||||
|
status: RelayLoginStatus,
|
||||||
|
) {
|
||||||
|
val current = _loginProgress.value ?: return
|
||||||
|
val updated = current.relayStatuses + (relay to status)
|
||||||
|
_loginProgress.value =
|
||||||
|
when (current) {
|
||||||
|
is LoginProgress.ConnectingToRelays -> current.copy(relayStatuses = updated)
|
||||||
|
is LoginProgress.WaitingForSigner -> current.copy(relayStatuses = updated)
|
||||||
|
is LoginProgress.SendingAck -> current.copy(relayStatuses = updated)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun createLoginRelayListener(): IRelayClientListener =
|
||||||
|
object : IRelayClientListener {
|
||||||
|
override fun onConnected(
|
||||||
|
relay: IRelayClient,
|
||||||
|
pingMillis: Int,
|
||||||
|
compressed: Boolean,
|
||||||
|
) {
|
||||||
|
updateRelayLoginStatus(relay.url, RelayLoginStatus.CONNECTED)
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun onCannotConnect(
|
||||||
|
relay: IRelayClient,
|
||||||
|
errorMessage: String,
|
||||||
|
) {
|
||||||
|
updateRelayLoginStatus(relay.url, RelayLoginStatus.FAILED)
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun onSent(
|
||||||
|
relay: IRelayClient,
|
||||||
|
cmdStr: String,
|
||||||
|
cmd: Command,
|
||||||
|
success: Boolean,
|
||||||
|
) {
|
||||||
|
if (cmd is EventCmd) {
|
||||||
|
updateRelayLoginStatus(
|
||||||
|
relay.url,
|
||||||
|
if (success) RelayLoginStatus.EVENT_SENT else RelayLoginStatus.SEND_FAILED,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun onIncomingMessage(
|
||||||
|
relay: IRelayClient,
|
||||||
|
msgStr: String,
|
||||||
|
msg: Message,
|
||||||
|
) {
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- Account loading ---
|
// --- Account loading ---
|
||||||
|
|
||||||
suspend fun loadSavedAccount(): Result<AccountState.LoggedIn> =
|
suspend fun loadSavedAccount(): Result<AccountState.LoggedIn> =
|
||||||
try {
|
try {
|
||||||
val lastNpub = getLastNpub() ?: return Result.failure(Exception("No saved account"))
|
val lastNpub = getLastNpub()
|
||||||
|
|
||||||
// Check for bunker account first
|
|
||||||
val bunkerUri = getBunkerUri()
|
val bunkerUri = getBunkerUri()
|
||||||
|
|
||||||
if (bunkerUri != null) {
|
if (bunkerUri != null) {
|
||||||
loadBunkerAccount(bunkerUri, lastNpub)
|
loadBunkerAccount(bunkerUri, lastNpub)
|
||||||
} else {
|
} else if (lastNpub != null) {
|
||||||
loadInternalAccount(lastNpub)
|
loadInternalAccount(lastNpub)
|
||||||
|
} else {
|
||||||
|
Result.failure(Exception("No saved account"))
|
||||||
}
|
}
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Result.failure(e)
|
Result.failure(e)
|
||||||
@@ -213,7 +275,7 @@ class AccountManager internal constructor(
|
|||||||
|
|
||||||
private suspend fun loadBunkerAccount(
|
private suspend fun loadBunkerAccount(
|
||||||
bunkerUri: String,
|
bunkerUri: String,
|
||||||
npub: String,
|
npub: String?,
|
||||||
): Result<AccountState.LoggedIn> {
|
): Result<AccountState.LoggedIn> {
|
||||||
val ephemeralPrivKeyHex =
|
val ephemeralPrivKeyHex =
|
||||||
secureStorage.getPrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS)
|
secureStorage.getPrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS)
|
||||||
@@ -226,13 +288,24 @@ class AccountManager internal constructor(
|
|||||||
val remoteSigner = NostrSignerRemote.fromBunkerUri(bunkerUri, ephemeralSigner, nip46Client)
|
val remoteSigner = NostrSignerRemote.fromBunkerUri(bunkerUri, ephemeralSigner, nip46Client)
|
||||||
remoteSigner.openSubscription()
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
val pubKeyHex = decodePublicKeyAsHexOrNull(npub) ?: return Result.failure(Exception("Invalid saved npub"))
|
val pubKeyHex =
|
||||||
|
if (npub != null) {
|
||||||
|
decodePublicKeyAsHexOrNull(npub) ?: return Result.failure(Exception("Invalid saved npub"))
|
||||||
|
} else {
|
||||||
|
// npub missing (e.g. last_account.txt deleted) — must wait for relay
|
||||||
|
// before calling getPublicKey() to recover from signer
|
||||||
|
awaitNip46RelayConnection(nip46Client, remoteSigner.relays)
|
||||||
|
remoteSigner.getPublicKey()
|
||||||
|
}
|
||||||
|
|
||||||
|
val resolvedNpub = npub ?: pubKeyHex.hexToByteArray().toNpub()
|
||||||
|
if (npub == null) saveLastNpub(resolvedNpub)
|
||||||
|
|
||||||
val state =
|
val state =
|
||||||
AccountState.LoggedIn(
|
AccountState.LoggedIn(
|
||||||
signer = remoteSigner,
|
signer = remoteSigner,
|
||||||
pubKeyHex = pubKeyHex,
|
pubKeyHex = pubKeyHex,
|
||||||
npub = npub,
|
npub = resolvedNpub,
|
||||||
nsec = null,
|
nsec = null,
|
||||||
isReadOnly = false,
|
isReadOnly = false,
|
||||||
signerType = SignerType.Remote(bunkerUri),
|
signerType = SignerType.Remote(bunkerUri),
|
||||||
@@ -244,18 +317,33 @@ class AccountManager internal constructor(
|
|||||||
|
|
||||||
// --- Bunker login ---
|
// --- Bunker login ---
|
||||||
|
|
||||||
suspend fun loginWithBunker(bunkerUri: String): Result<AccountState.LoggedIn> =
|
suspend fun loginWithBunker(bunkerUri: String): Result<AccountState.LoggedIn> {
|
||||||
|
val listener = createLoginRelayListener()
|
||||||
|
var client: NostrClient? = null
|
||||||
try {
|
try {
|
||||||
val ephemeralKeyPair = KeyPair()
|
val ephemeralKeyPair = KeyPair()
|
||||||
val ephemeralSigner = NostrSignerInternal(ephemeralKeyPair)
|
val ephemeralSigner = NostrSignerInternal(ephemeralKeyPair)
|
||||||
|
|
||||||
val nip46Client = getOrCreateNip46Client()
|
val nip46Client = getOrCreateNip46Client()
|
||||||
|
client = nip46Client
|
||||||
val remoteSigner = NostrSignerRemote.fromBunkerUri(bunkerUri, ephemeralSigner, nip46Client)
|
val remoteSigner = NostrSignerRemote.fromBunkerUri(bunkerUri, ephemeralSigner, nip46Client)
|
||||||
|
|
||||||
|
// Emit connecting with initial relay statuses
|
||||||
|
_loginProgress.value =
|
||||||
|
LoginProgress.ConnectingToRelays(
|
||||||
|
remoteSigner.relays.associateWith { RelayLoginStatus.CONNECTING },
|
||||||
|
)
|
||||||
|
nip46Client.subscribe(listener)
|
||||||
|
|
||||||
remoteSigner.openSubscription()
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
// Wait for websocket to be ready before sending connect request
|
// Wait for websocket to be ready before sending connect request
|
||||||
awaitNip46RelayConnection(nip46Client, remoteSigner.relays)
|
awaitNip46RelayConnection(nip46Client, remoteSigner.relays)
|
||||||
|
|
||||||
|
_loginProgress.value =
|
||||||
|
LoginProgress.WaitingForSigner(
|
||||||
|
relayStatuses = _loginProgress.value?.relayStatuses.orEmpty(),
|
||||||
|
)
|
||||||
val remotePubkey = remoteSigner.connect()
|
val remotePubkey = remoteSigner.connect()
|
||||||
|
|
||||||
val state =
|
val state =
|
||||||
@@ -277,22 +365,28 @@ class AccountManager internal constructor(
|
|||||||
npub = state.npub,
|
npub = state.npub,
|
||||||
)
|
)
|
||||||
|
|
||||||
Result.success(state)
|
return Result.success(state)
|
||||||
} catch (e: kotlinx.coroutines.TimeoutCancellationException) {
|
} catch (e: kotlinx.coroutines.TimeoutCancellationException) {
|
||||||
Result.failure(Exception("Could not connect to NIP-46 relay. Check your network connection."))
|
return Result.failure(Exception("Could not connect to NIP-46 relay. Check your network connection."))
|
||||||
} catch (e: SignerExceptions.TimedOutException) {
|
} catch (e: SignerExceptions.TimedOutException) {
|
||||||
Result.failure(Exception("Connection timed out. Ensure remote signer is online and has approved the connection."))
|
return Result.failure(Exception("Connection timed out. Ensure remote signer is online and has approved the connection."))
|
||||||
} catch (e: SignerExceptions.ManuallyUnauthorizedException) {
|
} catch (e: SignerExceptions.ManuallyUnauthorizedException) {
|
||||||
Result.failure(Exception("Connection rejected by remote signer."))
|
return Result.failure(Exception("Connection rejected by remote signer."))
|
||||||
} catch (e: SignerExceptions.CouldNotPerformException) {
|
} catch (e: SignerExceptions.CouldNotPerformException) {
|
||||||
Result.failure(Exception("Remote signer error: ${e.message}"))
|
return Result.failure(Exception("Remote signer error: ${e.message}"))
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Result.failure(Exception("Connection failed: ${e.message}"))
|
return Result.failure(Exception("Connection failed: ${e.message}"))
|
||||||
|
} finally {
|
||||||
|
_loginProgress.value = null
|
||||||
|
client?.unsubscribe(listener)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- Nostrconnect login ---
|
// --- Nostrconnect login ---
|
||||||
|
|
||||||
suspend fun loginWithNostrConnect(onUriGenerated: (String) -> Unit): Result<AccountState.LoggedIn> =
|
suspend fun loginWithNostrConnect(onUriGenerated: (String) -> Unit): Result<AccountState.LoggedIn> {
|
||||||
|
val listener = createLoginRelayListener()
|
||||||
|
var client: NostrClient? = null
|
||||||
try {
|
try {
|
||||||
val ephemeralKeyPair = KeyPair()
|
val ephemeralKeyPair = KeyPair()
|
||||||
val ephemeralSigner = NostrSignerInternal(ephemeralKeyPair)
|
val ephemeralSigner = NostrSignerInternal(ephemeralKeyPair)
|
||||||
@@ -302,13 +396,31 @@ class AccountManager internal constructor(
|
|||||||
val relays = NIP46_RELAYS
|
val relays = NIP46_RELAYS
|
||||||
val relayParams = relays.joinToString("&") { "relay=$it" }
|
val relayParams = relays.joinToString("&") { "relay=$it" }
|
||||||
val uri = "nostrconnect://$ephemeralPubKey?$relayParams&secret=$secret&name=Amethyst%20Desktop"
|
val uri = "nostrconnect://$ephemeralPubKey?$relayParams&secret=$secret&name=Amethyst%20Desktop"
|
||||||
onUriGenerated(uri)
|
|
||||||
|
|
||||||
val nip46Client = getOrCreateNip46Client()
|
val nip46Client = getOrCreateNip46Client()
|
||||||
|
client = nip46Client
|
||||||
val normalizedRelays = relays.map { NormalizedRelayUrl(it) }.toSet()
|
val normalizedRelays = relays.map { NormalizedRelayUrl(it) }.toSet()
|
||||||
|
|
||||||
|
// Emit connecting with initial relay statuses
|
||||||
|
_loginProgress.value =
|
||||||
|
LoginProgress.ConnectingToRelays(
|
||||||
|
normalizedRelays.associateWith { RelayLoginStatus.CONNECTING },
|
||||||
|
)
|
||||||
|
nip46Client.subscribe(listener)
|
||||||
|
|
||||||
|
onUriGenerated(uri)
|
||||||
|
|
||||||
|
_loginProgress.value =
|
||||||
|
LoginProgress.WaitingForSigner(
|
||||||
|
relayStatuses = _loginProgress.value?.relayStatuses.orEmpty(),
|
||||||
|
)
|
||||||
val connectData = waitForConnectRequest(ephemeralSigner, ephemeralPubKey, normalizedRelays, secret, nip46Client)
|
val connectData = waitForConnectRequest(ephemeralSigner, ephemeralPubKey, normalizedRelays, secret, nip46Client)
|
||||||
|
|
||||||
if (connectData.requestId != null) {
|
if (connectData.requestId != null) {
|
||||||
|
_loginProgress.value =
|
||||||
|
LoginProgress.SendingAck(
|
||||||
|
relayStatuses = _loginProgress.value?.relayStatuses.orEmpty(),
|
||||||
|
)
|
||||||
sendAckResponse(ephemeralSigner, connectData, normalizedRelays, nip46Client)
|
sendAckResponse(ephemeralSigner, connectData, normalizedRelays, nip46Client)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -321,13 +433,17 @@ class AccountManager internal constructor(
|
|||||||
)
|
)
|
||||||
remoteSigner.openSubscription()
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
|
// Verify user pubkey via get_public_key — connect params may contain
|
||||||
|
// the wrong pubkey (e.g. ephemeral key echoed back by some signers)
|
||||||
|
val verifiedPubkey = remoteSigner.getPublicKey()
|
||||||
|
|
||||||
val syntheticBunkerUri = "bunker://${connectData.signerPubkey}?$relayParams"
|
val syntheticBunkerUri = "bunker://${connectData.signerPubkey}?$relayParams"
|
||||||
|
|
||||||
val state =
|
val state =
|
||||||
AccountState.LoggedIn(
|
AccountState.LoggedIn(
|
||||||
signer = remoteSigner,
|
signer = remoteSigner,
|
||||||
pubKeyHex = connectData.userPubkey,
|
pubKeyHex = verifiedPubkey,
|
||||||
npub = connectData.userPubkey.hexToByteArray().toNpub(),
|
npub = verifiedPubkey.hexToByteArray().toNpub(),
|
||||||
nsec = null,
|
nsec = null,
|
||||||
isReadOnly = false,
|
isReadOnly = false,
|
||||||
signerType = SignerType.Remote(syntheticBunkerUri),
|
signerType = SignerType.Remote(syntheticBunkerUri),
|
||||||
@@ -341,12 +457,16 @@ class AccountManager internal constructor(
|
|||||||
npub = state.npub,
|
npub = state.npub,
|
||||||
)
|
)
|
||||||
|
|
||||||
Result.success(state)
|
return Result.success(state)
|
||||||
} catch (e: kotlinx.coroutines.TimeoutCancellationException) {
|
} catch (e: kotlinx.coroutines.TimeoutCancellationException) {
|
||||||
Result.failure(Exception("Timed out waiting for signer. Ensure the signer app scanned the QR code."))
|
return Result.failure(Exception("Timed out waiting for signer. Ensure the signer app scanned the QR code."))
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Result.failure(Exception("Connection failed: ${e.message}"))
|
return Result.failure(Exception("Connection failed: ${e.message}"))
|
||||||
|
} finally {
|
||||||
|
_loginProgress.value = null
|
||||||
|
client?.unsubscribe(listener)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private suspend fun waitForConnectRequest(
|
private suspend fun waitForConnectRequest(
|
||||||
ephemeralSigner: NostrSignerInternal,
|
ephemeralSigner: NostrSignerInternal,
|
||||||
@@ -463,9 +583,9 @@ class AccountManager internal constructor(
|
|||||||
ephemeralPrivKeyHex: String,
|
ephemeralPrivKeyHex: String,
|
||||||
npub: String,
|
npub: String,
|
||||||
) {
|
) {
|
||||||
saveBunkerUri(bunkerUri)
|
|
||||||
secureStorage.savePrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS, ephemeralPrivKeyHex)
|
|
||||||
saveLastNpub(npub)
|
saveLastNpub(npub)
|
||||||
|
secureStorage.savePrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS, ephemeralPrivKeyHex)
|
||||||
|
saveBunkerUri(bunkerUri)
|
||||||
}
|
}
|
||||||
|
|
||||||
fun hasBunkerAccount(): Boolean = getBunkerFile().exists()
|
fun hasBunkerAccount(): Boolean = getBunkerFile().exists()
|
||||||
|
|||||||
+47
@@ -0,0 +1,47 @@
|
|||||||
|
/*
|
||||||
|
* Copyright (c) 2025 Vitor Pamplona
|
||||||
|
*
|
||||||
|
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||||
|
* this software and associated documentation files (the "Software"), to deal in
|
||||||
|
* the Software without restriction, including without limitation the rights to use,
|
||||||
|
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||||
|
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||||
|
* subject to the following conditions:
|
||||||
|
*
|
||||||
|
* The above copyright notice and this permission notice shall be included in all
|
||||||
|
* copies or substantial portions of the Software.
|
||||||
|
*
|
||||||
|
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||||
|
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||||
|
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||||
|
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||||
|
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||||
|
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||||
|
*/
|
||||||
|
package com.vitorpamplona.amethyst.desktop.account
|
||||||
|
|
||||||
|
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
||||||
|
|
||||||
|
enum class RelayLoginStatus {
|
||||||
|
CONNECTING,
|
||||||
|
CONNECTED,
|
||||||
|
EVENT_SENT,
|
||||||
|
SEND_FAILED,
|
||||||
|
FAILED,
|
||||||
|
}
|
||||||
|
|
||||||
|
sealed class LoginProgress {
|
||||||
|
abstract val relayStatuses: Map<NormalizedRelayUrl, RelayLoginStatus>
|
||||||
|
|
||||||
|
data class ConnectingToRelays(
|
||||||
|
override val relayStatuses: Map<NormalizedRelayUrl, RelayLoginStatus> = emptyMap(),
|
||||||
|
) : LoginProgress()
|
||||||
|
|
||||||
|
data class WaitingForSigner(
|
||||||
|
override val relayStatuses: Map<NormalizedRelayUrl, RelayLoginStatus> = emptyMap(),
|
||||||
|
) : LoginProgress()
|
||||||
|
|
||||||
|
data class SendingAck(
|
||||||
|
override val relayStatuses: Map<NormalizedRelayUrl, RelayLoginStatus> = emptyMap(),
|
||||||
|
) : LoginProgress()
|
||||||
|
}
|
||||||
+1
@@ -45,5 +45,6 @@ object DefaultRelays {
|
|||||||
"wss://nos.lol",
|
"wss://nos.lol",
|
||||||
"wss://relay.snort.social",
|
"wss://relay.snort.social",
|
||||||
"wss://nostr.wine",
|
"wss://nostr.wine",
|
||||||
|
"wss://relay.primal.net",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
-3
@@ -23,7 +23,6 @@ package com.vitorpamplona.amethyst.desktop.subscriptions
|
|||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
import androidx.compose.runtime.DisposableEffect
|
import androidx.compose.runtime.DisposableEffect
|
||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
import com.vitorpamplona.amethyst.desktop.DebugConfig
|
|
||||||
import com.vitorpamplona.amethyst.desktop.network.RelayConnectionManager
|
import com.vitorpamplona.amethyst.desktop.network.RelayConnectionManager
|
||||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||||
import com.vitorpamplona.quartz.nip01Core.relay.client.reqs.IRequestListener
|
import com.vitorpamplona.quartz.nip01Core.relay.client.reqs.IRequestListener
|
||||||
@@ -75,7 +74,6 @@ fun rememberSubscription(
|
|||||||
DisposableEffect(*keys, subscription?.subId) {
|
DisposableEffect(*keys, subscription?.subId) {
|
||||||
subscription?.let { cfg ->
|
subscription?.let { cfg ->
|
||||||
if (cfg.relays.isNotEmpty()) {
|
if (cfg.relays.isNotEmpty()) {
|
||||||
DebugConfig.log("SUB OPEN ${cfg.subId} relays=${cfg.relays.size}")
|
|
||||||
relayManager.subscribe(
|
relayManager.subscribe(
|
||||||
subId = cfg.subId,
|
subId = cfg.subId,
|
||||||
filters = cfg.filters,
|
filters = cfg.filters,
|
||||||
@@ -104,7 +102,6 @@ fun rememberSubscription(
|
|||||||
|
|
||||||
onDispose {
|
onDispose {
|
||||||
subscription?.let {
|
subscription?.let {
|
||||||
DebugConfig.log("SUB CLOSE ${it.subId}")
|
|
||||||
relayManager.unsubscribe(it.subId)
|
relayManager.unsubscribe(it.subId)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -84,6 +84,8 @@ import com.vitorpamplona.quartz.nip18Reposts.RepostEvent
|
|||||||
import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent
|
import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent
|
||||||
import com.vitorpamplona.quartz.nip51Lists.bookmarkList.BookmarkListEvent
|
import com.vitorpamplona.quartz.nip51Lists.bookmarkList.BookmarkListEvent
|
||||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent
|
import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent
|
||||||
|
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||||
|
import kotlinx.coroutines.flow.map
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Note card with action buttons.
|
* Note card with action buttons.
|
||||||
@@ -159,6 +161,14 @@ fun FeedScreen(
|
|||||||
onZapFeedback: (ZapFeedback) -> Unit = {},
|
onZapFeedback: (ZapFeedback) -> Unit = {},
|
||||||
) {
|
) {
|
||||||
val connectedRelays by relayManager.connectedRelays.collectAsState()
|
val connectedRelays by relayManager.connectedRelays.collectAsState()
|
||||||
|
// Configured relay URLs only — stabilized with distinctUntilChanged() to prevent
|
||||||
|
// subscription churn from relay status changes (pings, connect/disconnect).
|
||||||
|
// openReqSubscription connects relays on demand; no need to wait for connectedRelays.
|
||||||
|
val configuredRelays by remember {
|
||||||
|
relayManager.relayStatuses
|
||||||
|
.map { it.keys }
|
||||||
|
.distinctUntilChanged()
|
||||||
|
}.collectAsState(emptySet())
|
||||||
val scope = rememberCoroutineScope()
|
val scope = rememberCoroutineScope()
|
||||||
val eventState =
|
val eventState =
|
||||||
remember {
|
remember {
|
||||||
@@ -190,17 +200,15 @@ fun FeedScreen(
|
|||||||
val initialLoadComplete = eoseReceivedCount > 0
|
val initialLoadComplete = eoseReceivedCount > 0
|
||||||
|
|
||||||
// Load followed users for Following feed mode
|
// Load followed users for Following feed mode
|
||||||
rememberSubscription(connectedRelays, account, feedMode, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, account, feedMode, relayManager = relayManager) {
|
||||||
DebugConfig.log("contactList sub: relays=${connectedRelays.size}, account=${account?.pubKeyHex?.take(8)}, mode=$feedMode")
|
if (configuredRelays.isNotEmpty() && account != null && feedMode == FeedMode.FOLLOWING) {
|
||||||
if (connectedRelays.isNotEmpty() && account != null && feedMode == FeedMode.FOLLOWING) {
|
|
||||||
createContactListSubscription(
|
createContactListSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
pubKeyHex = account.pubKeyHex,
|
pubKeyHex = account.pubKeyHex,
|
||||||
onEvent = { event, _, relay, _ ->
|
onEvent = { event, _, relay, _ ->
|
||||||
DebugConfig.log("contactList event: kind=${event.kind}, isContactList=${event is ContactListEvent}, from=$relay")
|
|
||||||
if (event is ContactListEvent) {
|
if (event is ContactListEvent) {
|
||||||
val follows = event.verifiedFollowKeySet()
|
val follows = event.verifiedFollowKeySet()
|
||||||
DebugConfig.log("followedUsers: ${follows.size} users")
|
DebugConfig.log("contactList: ${follows.size} follows from $relay")
|
||||||
followedUsers = follows
|
followedUsers = follows
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -211,8 +219,8 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Load user's bookmark list
|
// Load user's bookmark list
|
||||||
rememberSubscription(connectedRelays, account, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, account, relayManager = relayManager) {
|
||||||
if (connectedRelays.isNotEmpty() && account != null) {
|
if (configuredRelays.isNotEmpty() && account != null) {
|
||||||
SubscriptionConfig(
|
SubscriptionConfig(
|
||||||
subId = "bookmarks-${account.pubKeyHex.take(8)}",
|
subId = "bookmarks-${account.pubKeyHex.take(8)}",
|
||||||
filters =
|
filters =
|
||||||
@@ -223,7 +231,7 @@ fun FeedScreen(
|
|||||||
limit = 1,
|
limit = 1,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
if (event is BookmarkListEvent) {
|
if (event is BookmarkListEvent) {
|
||||||
bookmarkList = event
|
bookmarkList = event
|
||||||
@@ -251,16 +259,16 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Subscribe to feed based on mode
|
// Subscribe to feed based on mode
|
||||||
rememberSubscription(connectedRelays, feedMode, followedUsers, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, feedMode, followedUsers, relayManager = relayManager) {
|
||||||
DebugConfig.log("feedSub: mode=$feedMode, relays=${connectedRelays.size}, followedUsers=${followedUsers.size}")
|
DebugConfig.log("feedSub: mode=$feedMode, relays=${configuredRelays.size}, followedUsers=${followedUsers.size}")
|
||||||
if (connectedRelays.isEmpty()) {
|
if (configuredRelays.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
when (feedMode) {
|
when (feedMode) {
|
||||||
FeedMode.GLOBAL -> {
|
FeedMode.GLOBAL -> {
|
||||||
createGlobalFeedSubscription(
|
createGlobalFeedSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
// Store metadata events in cache
|
// Store metadata events in cache
|
||||||
if (event is MetadataEvent) {
|
if (event is MetadataEvent) {
|
||||||
@@ -277,7 +285,7 @@ fun FeedScreen(
|
|||||||
FeedMode.FOLLOWING -> {
|
FeedMode.FOLLOWING -> {
|
||||||
if (followedUsers.isNotEmpty()) {
|
if (followedUsers.isNotEmpty()) {
|
||||||
createFollowingFeedSubscription(
|
createFollowingFeedSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
followedUsers = followedUsers.toList(),
|
followedUsers = followedUsers.toList(),
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
// Store metadata events in cache
|
// Store metadata events in cache
|
||||||
@@ -299,13 +307,13 @@ fun FeedScreen(
|
|||||||
|
|
||||||
// Subscribe to zaps for visible events
|
// Subscribe to zaps for visible events
|
||||||
val eventIds = events.map { it.id }
|
val eventIds = events.map { it.id }
|
||||||
rememberSubscription(connectedRelays, eventIds, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, eventIds, relayManager = relayManager) {
|
||||||
if (connectedRelays.isEmpty() || eventIds.isEmpty()) {
|
if (configuredRelays.isEmpty() || eventIds.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
createZapsSubscription(
|
createZapsSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
eventIds = eventIds,
|
eventIds = eventIds,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
if (event is LnZapEvent) {
|
if (event is LnZapEvent) {
|
||||||
@@ -329,8 +337,8 @@ fun FeedScreen(
|
|||||||
.flatten()
|
.flatten()
|
||||||
.map { it.senderPubKey }
|
.map { it.senderPubKey }
|
||||||
.distinct()
|
.distinct()
|
||||||
rememberSubscription(connectedRelays, zapSenderPubkeys, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, zapSenderPubkeys, relayManager = relayManager) {
|
||||||
if (connectedRelays.isEmpty() || zapSenderPubkeys.isEmpty()) {
|
if (configuredRelays.isEmpty() || zapSenderPubkeys.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -348,7 +356,7 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
createBatchMetadataSubscription(
|
createBatchMetadataSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
pubKeyHexList = missingPubkeys,
|
pubKeyHexList = missingPubkeys,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
if (event is MetadataEvent) {
|
if (event is MetadataEvent) {
|
||||||
@@ -359,13 +367,13 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Subscribe to reactions for visible events
|
// Subscribe to reactions for visible events
|
||||||
rememberSubscription(connectedRelays, eventIds, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, eventIds, relayManager = relayManager) {
|
||||||
if (connectedRelays.isEmpty() || eventIds.isEmpty()) {
|
if (configuredRelays.isEmpty() || eventIds.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
createReactionsSubscription(
|
createReactionsSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
eventIds = eventIds,
|
eventIds = eventIds,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
if (event is ReactionEvent) {
|
if (event is ReactionEvent) {
|
||||||
@@ -381,13 +389,13 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Subscribe to replies for visible events
|
// Subscribe to replies for visible events
|
||||||
rememberSubscription(connectedRelays, eventIds, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, eventIds, relayManager = relayManager) {
|
||||||
if (connectedRelays.isEmpty() || eventIds.isEmpty()) {
|
if (configuredRelays.isEmpty() || eventIds.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
createRepliesSubscription(
|
createRepliesSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
eventIds = eventIds,
|
eventIds = eventIds,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
// Find the event this is replying to
|
// Find the event this is replying to
|
||||||
@@ -408,13 +416,13 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Subscribe to reposts for visible events
|
// Subscribe to reposts for visible events
|
||||||
rememberSubscription(connectedRelays, eventIds, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, eventIds, relayManager = relayManager) {
|
||||||
if (connectedRelays.isEmpty() || eventIds.isEmpty()) {
|
if (configuredRelays.isEmpty() || eventIds.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
createRepostsSubscription(
|
createRepostsSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
eventIds = eventIds,
|
eventIds = eventIds,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
if (event is RepostEvent) {
|
if (event is RepostEvent) {
|
||||||
@@ -440,13 +448,13 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Fallback subscription if coordinator not available
|
// Fallback subscription if coordinator not available
|
||||||
rememberSubscription(connectedRelays, authorPubkeys, subscriptionsCoordinator, relayManager = relayManager) {
|
rememberSubscription(configuredRelays, authorPubkeys, subscriptionsCoordinator, relayManager = relayManager) {
|
||||||
// Skip if using coordinator
|
// Skip if using coordinator
|
||||||
if (subscriptionsCoordinator != null) {
|
if (subscriptionsCoordinator != null) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
if (connectedRelays.isEmpty() || authorPubkeys.isEmpty()) {
|
if (configuredRelays.isEmpty() || authorPubkeys.isEmpty()) {
|
||||||
return@rememberSubscription null
|
return@rememberSubscription null
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -464,7 +472,7 @@ fun FeedScreen(
|
|||||||
}
|
}
|
||||||
|
|
||||||
createBatchMetadataSubscription(
|
createBatchMetadataSubscription(
|
||||||
relays = connectedRelays,
|
relays = configuredRelays,
|
||||||
pubKeyHexList = missingPubkeys,
|
pubKeyHexList = missingPubkeys,
|
||||||
onEvent = { event, _, _, _ ->
|
onEvent = { event, _, _, _ ->
|
||||||
if (event is MetadataEvent) {
|
if (event is MetadataEvent) {
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ import androidx.compose.material3.LinearProgressIndicator
|
|||||||
import androidx.compose.material3.MaterialTheme
|
import androidx.compose.material3.MaterialTheme
|
||||||
import androidx.compose.material3.Text
|
import androidx.compose.material3.Text
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
|
import androidx.compose.runtime.collectAsState
|
||||||
import androidx.compose.runtime.getValue
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.runtime.mutableStateOf
|
import androidx.compose.runtime.mutableStateOf
|
||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
@@ -71,6 +72,7 @@ fun LoginScreen(
|
|||||||
var showNewKeyDialog by remember { mutableStateOf(false) }
|
var showNewKeyDialog by remember { mutableStateOf(false) }
|
||||||
var generatedAccount by remember { mutableStateOf<AccountState.LoggedIn?>(null) }
|
var generatedAccount by remember { mutableStateOf<AccountState.LoggedIn?>(null) }
|
||||||
val scope = rememberCoroutineScope()
|
val scope = rememberCoroutineScope()
|
||||||
|
val loginProgress by accountManager.loginProgress.collectAsState()
|
||||||
|
|
||||||
Column(
|
Column(
|
||||||
modifier = Modifier.fillMaxSize().padding(32.dp),
|
modifier = Modifier.fillMaxSize().padding(32.dp),
|
||||||
@@ -116,6 +118,7 @@ fun LoginScreen(
|
|||||||
onLoginSuccess()
|
onLoginSuccess()
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
loginProgress = loginProgress,
|
||||||
)
|
)
|
||||||
|
|
||||||
val account = generatedAccount
|
val account = generatedAccount
|
||||||
|
|||||||
+47
-33
@@ -61,6 +61,7 @@ import com.vitorpamplona.amethyst.commons.resources.login_button
|
|||||||
import com.vitorpamplona.amethyst.commons.resources.login_card_subtitle
|
import com.vitorpamplona.amethyst.commons.resources.login_card_subtitle
|
||||||
import com.vitorpamplona.amethyst.commons.resources.login_card_title
|
import com.vitorpamplona.amethyst.commons.resources.login_card_title
|
||||||
import com.vitorpamplona.amethyst.commons.resources.login_generate_button
|
import com.vitorpamplona.amethyst.commons.resources.login_generate_button
|
||||||
|
import com.vitorpamplona.amethyst.desktop.account.LoginProgress
|
||||||
import com.vitorpamplona.amethyst.desktop.account.validateBunkerUri
|
import com.vitorpamplona.amethyst.desktop.account.validateBunkerUri
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
@@ -73,6 +74,7 @@ fun LoginCard(
|
|||||||
onGenerateNew: () -> Unit,
|
onGenerateNew: () -> Unit,
|
||||||
onLoginBunker: (suspend (String) -> Result<Unit>)? = null,
|
onLoginBunker: (suspend (String) -> Result<Unit>)? = null,
|
||||||
onLoginNostrConnect: (suspend (onUriGenerated: (String) -> Unit) -> Result<Unit>)? = null,
|
onLoginNostrConnect: (suspend (onUriGenerated: (String) -> Unit) -> Result<Unit>)? = null,
|
||||||
|
loginProgress: LoginProgress? = null,
|
||||||
modifier: Modifier = Modifier,
|
modifier: Modifier = Modifier,
|
||||||
cardWidth: Dp = 400.dp,
|
cardWidth: Dp = 400.dp,
|
||||||
title: String = stringResource(Res.string.login_card_title),
|
title: String = stringResource(Res.string.login_card_title),
|
||||||
@@ -119,8 +121,8 @@ fun LoginCard(
|
|||||||
}
|
}
|
||||||
|
|
||||||
when (selectedTab) {
|
when (selectedTab) {
|
||||||
0 -> PasteKeyContent(onLogin, onGenerateNew, onLoginBunker, subtitle)
|
0 -> PasteKeyContent(onLogin, onGenerateNew, onLoginBunker, subtitle, loginProgress)
|
||||||
1 -> if (onLoginNostrConnect != null) NostrConnectContent(onLoginNostrConnect)
|
1 -> if (onLoginNostrConnect != null) NostrConnectContent(onLoginNostrConnect, loginProgress)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -132,6 +134,7 @@ private fun PasteKeyContent(
|
|||||||
onGenerateNew: () -> Unit,
|
onGenerateNew: () -> Unit,
|
||||||
onLoginBunker: (suspend (String) -> Result<Unit>)?,
|
onLoginBunker: (suspend (String) -> Result<Unit>)?,
|
||||||
subtitle: String,
|
subtitle: String,
|
||||||
|
loginProgress: LoginProgress? = null,
|
||||||
) {
|
) {
|
||||||
var keyInput by remember { mutableStateOf("") }
|
var keyInput by remember { mutableStateOf("") }
|
||||||
var errorMessage by remember { mutableStateOf<String?>(null) }
|
var errorMessage by remember { mutableStateOf<String?>(null) }
|
||||||
@@ -167,21 +170,25 @@ private fun PasteKeyContent(
|
|||||||
Spacer(Modifier.height(16.dp))
|
Spacer(Modifier.height(16.dp))
|
||||||
|
|
||||||
if (isConnecting) {
|
if (isConnecting) {
|
||||||
Row(
|
if (loginProgress != null) {
|
||||||
modifier = Modifier.fillMaxWidth(),
|
LoginProgressSteps(loginProgress)
|
||||||
horizontalArrangement = Arrangement.Center,
|
} else {
|
||||||
verticalAlignment = Alignment.CenterVertically,
|
Row(
|
||||||
) {
|
modifier = Modifier.fillMaxWidth(),
|
||||||
CircularProgressIndicator(
|
horizontalArrangement = Arrangement.Center,
|
||||||
modifier = Modifier.size(20.dp),
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
strokeWidth = 2.dp,
|
) {
|
||||||
)
|
CircularProgressIndicator(
|
||||||
Spacer(Modifier.width(12.dp))
|
modifier = Modifier.size(20.dp),
|
||||||
Text(
|
strokeWidth = 2.dp,
|
||||||
"Connecting to remote signer...",
|
)
|
||||||
style = MaterialTheme.typography.bodyMedium,
|
Spacer(Modifier.width(12.dp))
|
||||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
Text(
|
||||||
)
|
"Connecting to remote signer...",
|
||||||
|
style = MaterialTheme.typography.bodyMedium,
|
||||||
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
Row(
|
Row(
|
||||||
@@ -234,7 +241,10 @@ private fun PasteKeyContent(
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
private fun NostrConnectContent(onLoginNostrConnect: suspend (onUriGenerated: (String) -> Unit) -> Result<Unit>) {
|
private fun NostrConnectContent(
|
||||||
|
onLoginNostrConnect: suspend (onUriGenerated: (String) -> Unit) -> Result<Unit>,
|
||||||
|
loginProgress: LoginProgress? = null,
|
||||||
|
) {
|
||||||
var nostrConnectUri by remember { mutableStateOf<String?>(null) }
|
var nostrConnectUri by remember { mutableStateOf<String?>(null) }
|
||||||
var isConnecting by remember { mutableStateOf(false) }
|
var isConnecting by remember { mutableStateOf(false) }
|
||||||
var errorMessage by remember { mutableStateOf<String?>(null) }
|
var errorMessage by remember { mutableStateOf<String?>(null) }
|
||||||
@@ -316,21 +326,25 @@ private fun NostrConnectContent(onLoginNostrConnect: suspend (onUriGenerated: (S
|
|||||||
|
|
||||||
Spacer(Modifier.height(12.dp))
|
Spacer(Modifier.height(12.dp))
|
||||||
|
|
||||||
Row(
|
if (loginProgress != null) {
|
||||||
modifier = Modifier.fillMaxWidth(),
|
LoginProgressSteps(loginProgress)
|
||||||
horizontalArrangement = Arrangement.Center,
|
} else {
|
||||||
verticalAlignment = Alignment.CenterVertically,
|
Row(
|
||||||
) {
|
modifier = Modifier.fillMaxWidth(),
|
||||||
CircularProgressIndicator(
|
horizontalArrangement = Arrangement.Center,
|
||||||
modifier = Modifier.size(16.dp),
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
strokeWidth = 2.dp,
|
) {
|
||||||
)
|
CircularProgressIndicator(
|
||||||
Spacer(Modifier.width(8.dp))
|
modifier = Modifier.size(16.dp),
|
||||||
Text(
|
strokeWidth = 2.dp,
|
||||||
"Waiting for signer to connect...",
|
)
|
||||||
style = MaterialTheme.typography.bodySmall,
|
Spacer(Modifier.width(8.dp))
|
||||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
Text(
|
||||||
)
|
"Waiting for signer to connect...",
|
||||||
|
style = MaterialTheme.typography.bodySmall,
|
||||||
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
Row(
|
Row(
|
||||||
|
|||||||
+220
@@ -0,0 +1,220 @@
|
|||||||
|
/*
|
||||||
|
* Copyright (c) 2025 Vitor Pamplona
|
||||||
|
*
|
||||||
|
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||||
|
* this software and associated documentation files (the "Software"), to deal in
|
||||||
|
* the Software without restriction, including without limitation the rights to use,
|
||||||
|
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||||
|
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||||
|
* subject to the following conditions:
|
||||||
|
*
|
||||||
|
* The above copyright notice and this permission notice shall be included in all
|
||||||
|
* copies or substantial portions of the Software.
|
||||||
|
*
|
||||||
|
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||||
|
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||||
|
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||||
|
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||||
|
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||||
|
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||||
|
*/
|
||||||
|
package com.vitorpamplona.amethyst.desktop.ui.auth
|
||||||
|
|
||||||
|
import androidx.compose.foundation.layout.Arrangement
|
||||||
|
import androidx.compose.foundation.layout.Column
|
||||||
|
import androidx.compose.foundation.layout.Row
|
||||||
|
import androidx.compose.foundation.layout.Spacer
|
||||||
|
import androidx.compose.foundation.layout.fillMaxWidth
|
||||||
|
import androidx.compose.foundation.layout.padding
|
||||||
|
import androidx.compose.foundation.layout.size
|
||||||
|
import androidx.compose.foundation.layout.width
|
||||||
|
import androidx.compose.material.icons.Icons
|
||||||
|
import androidx.compose.material.icons.filled.Check
|
||||||
|
import androidx.compose.material.icons.filled.Close
|
||||||
|
import androidx.compose.material3.CircularProgressIndicator
|
||||||
|
import androidx.compose.material3.Icon
|
||||||
|
import androidx.compose.material3.MaterialTheme
|
||||||
|
import androidx.compose.material3.Text
|
||||||
|
import androidx.compose.runtime.Composable
|
||||||
|
import androidx.compose.ui.Alignment
|
||||||
|
import androidx.compose.ui.Modifier
|
||||||
|
import androidx.compose.ui.graphics.Color
|
||||||
|
import androidx.compose.ui.unit.dp
|
||||||
|
import com.vitorpamplona.amethyst.desktop.account.LoginProgress
|
||||||
|
import com.vitorpamplona.amethyst.desktop.account.RelayLoginStatus
|
||||||
|
|
||||||
|
private enum class StepState { DONE, ACTIVE, PENDING }
|
||||||
|
|
||||||
|
private data class StepInfo(
|
||||||
|
val label: String,
|
||||||
|
val state: StepState,
|
||||||
|
)
|
||||||
|
|
||||||
|
@Composable
|
||||||
|
fun LoginProgressSteps(
|
||||||
|
progress: LoginProgress,
|
||||||
|
modifier: Modifier = Modifier,
|
||||||
|
) {
|
||||||
|
val steps = buildStepList(progress)
|
||||||
|
|
||||||
|
Column(
|
||||||
|
modifier = modifier.fillMaxWidth(),
|
||||||
|
verticalArrangement = Arrangement.spacedBy(6.dp),
|
||||||
|
) {
|
||||||
|
steps.forEach { step ->
|
||||||
|
StepRow(step)
|
||||||
|
|
||||||
|
// Show relay rows under the active step
|
||||||
|
if (step.state == StepState.ACTIVE && progress.relayStatuses.isNotEmpty()) {
|
||||||
|
progress.relayStatuses.forEach { (relay, status) ->
|
||||||
|
RelayRow(relay.url, status)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun buildStepList(progress: LoginProgress): List<StepInfo> {
|
||||||
|
val orderedSteps =
|
||||||
|
listOf(
|
||||||
|
"Connecting to relays",
|
||||||
|
"Waiting for signer",
|
||||||
|
"Sending acknowledgment",
|
||||||
|
)
|
||||||
|
|
||||||
|
val activeIndex =
|
||||||
|
when (progress) {
|
||||||
|
is LoginProgress.ConnectingToRelays -> 0
|
||||||
|
is LoginProgress.WaitingForSigner -> 1
|
||||||
|
is LoginProgress.SendingAck -> 2
|
||||||
|
}
|
||||||
|
|
||||||
|
return orderedSteps.mapIndexed { index, label ->
|
||||||
|
StepInfo(
|
||||||
|
label = label,
|
||||||
|
state =
|
||||||
|
when {
|
||||||
|
index < activeIndex -> StepState.DONE
|
||||||
|
index == activeIndex -> StepState.ACTIVE
|
||||||
|
else -> StepState.PENDING
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Composable
|
||||||
|
private fun StepRow(step: StepInfo) {
|
||||||
|
Row(
|
||||||
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
|
modifier = Modifier.fillMaxWidth(),
|
||||||
|
) {
|
||||||
|
when (step.state) {
|
||||||
|
StepState.DONE -> {
|
||||||
|
Icon(
|
||||||
|
Icons.Default.Check,
|
||||||
|
contentDescription = null,
|
||||||
|
tint = Color(0xFF4CAF50),
|
||||||
|
modifier = Modifier.size(16.dp),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
StepState.ACTIVE -> {
|
||||||
|
CircularProgressIndicator(
|
||||||
|
modifier = Modifier.size(16.dp),
|
||||||
|
strokeWidth = 2.dp,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
StepState.PENDING -> {
|
||||||
|
Spacer(Modifier.size(16.dp))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Spacer(Modifier.width(8.dp))
|
||||||
|
Text(
|
||||||
|
step.label,
|
||||||
|
style = MaterialTheme.typography.bodySmall,
|
||||||
|
color =
|
||||||
|
when (step.state) {
|
||||||
|
StepState.DONE -> Color(0xFF4CAF50)
|
||||||
|
StepState.ACTIVE -> MaterialTheme.colorScheme.onSurface
|
||||||
|
StepState.PENDING -> MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.5f)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Composable
|
||||||
|
private fun RelayRow(
|
||||||
|
url: String,
|
||||||
|
status: RelayLoginStatus,
|
||||||
|
) {
|
||||||
|
Row(
|
||||||
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
|
modifier = Modifier.fillMaxWidth().padding(start = 24.dp),
|
||||||
|
) {
|
||||||
|
when (status) {
|
||||||
|
RelayLoginStatus.EVENT_SENT -> {
|
||||||
|
Icon(
|
||||||
|
Icons.Default.Check,
|
||||||
|
contentDescription = null,
|
||||||
|
tint = Color(0xFF2196F3),
|
||||||
|
modifier = Modifier.size(12.dp),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
RelayLoginStatus.CONNECTED -> {
|
||||||
|
Icon(
|
||||||
|
Icons.Default.Check,
|
||||||
|
contentDescription = null,
|
||||||
|
tint = Color(0xFF4CAF50),
|
||||||
|
modifier = Modifier.size(12.dp),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
RelayLoginStatus.FAILED,
|
||||||
|
RelayLoginStatus.SEND_FAILED,
|
||||||
|
-> {
|
||||||
|
Icon(
|
||||||
|
Icons.Default.Close,
|
||||||
|
contentDescription = null,
|
||||||
|
tint = MaterialTheme.colorScheme.error,
|
||||||
|
modifier = Modifier.size(12.dp),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
RelayLoginStatus.CONNECTING -> {
|
||||||
|
CircularProgressIndicator(
|
||||||
|
modifier = Modifier.size(12.dp),
|
||||||
|
strokeWidth = 1.5.dp,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Spacer(Modifier.width(6.dp))
|
||||||
|
val label = url.removePrefix("wss://").removeSuffix("/")
|
||||||
|
val statusLabel =
|
||||||
|
when (status) {
|
||||||
|
RelayLoginStatus.EVENT_SENT -> "$label (sent)"
|
||||||
|
RelayLoginStatus.SEND_FAILED -> "$label (send failed)"
|
||||||
|
RelayLoginStatus.FAILED -> "$label (failed)"
|
||||||
|
else -> label
|
||||||
|
}
|
||||||
|
Text(
|
||||||
|
statusLabel,
|
||||||
|
style = MaterialTheme.typography.labelSmall,
|
||||||
|
color =
|
||||||
|
when (status) {
|
||||||
|
RelayLoginStatus.FAILED, RelayLoginStatus.SEND_FAILED -> {
|
||||||
|
MaterialTheme.colorScheme.error.copy(alpha = 0.8f)
|
||||||
|
}
|
||||||
|
|
||||||
|
RelayLoginStatus.EVENT_SENT -> {
|
||||||
|
Color(0xFF2196F3)
|
||||||
|
}
|
||||||
|
|
||||||
|
else -> {
|
||||||
|
MaterialTheme.colorScheme.onSurfaceVariant
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,189 @@
|
|||||||
|
---
|
||||||
|
date: 2026-03-06
|
||||||
|
topic: nip46-post-login-bugs
|
||||||
|
---
|
||||||
|
|
||||||
|
# NIP-46 Post-Login Bugs: Content Not Loading + Bunker Timeout
|
||||||
|
|
||||||
|
## What's Happening
|
||||||
|
|
||||||
|
Two bugs on branch `feat/nip46-bunker-login` after the NIP-46 relay isolation work:
|
||||||
|
|
||||||
|
### Bug 1: `followedUsers=0` — Feed Never Populates After Login
|
||||||
|
|
||||||
|
**Symptoms:**
|
||||||
|
- Contact list subscription opens/closes rapidly (subscription churn)
|
||||||
|
- No `[DEBUG-FEED] contactList event:` log — no kind-3 event ever arrives
|
||||||
|
- Feed stays on "Loading followed users..." forever
|
||||||
|
|
||||||
|
**Logs:**
|
||||||
|
```
|
||||||
|
[DEBUG-FEED] contactList sub: relays=5, account=0b24845a, mode=FOLLOWING
|
||||||
|
[DEBUG-FEED] feedSub: mode=FOLLOWING, relays=5, followedUsers=0
|
||||||
|
[DEBUG-SUB] OPEN contacts-0b24845a-1772773857361 relays=5
|
||||||
|
[DEBUG-SUB] CLOSE contacts-0b24845a-1772773857361
|
||||||
|
[DEBUG-SUB] OPEN contacts-0b24845a-1772773867946 relays=5
|
||||||
|
[DEBUG-SUB] CLOSE contacts-0b24845a-1772773867946
|
||||||
|
```
|
||||||
|
|
||||||
|
### Bug 2: `bunker://` Login Times Out (30s)
|
||||||
|
|
||||||
|
**Symptoms:**
|
||||||
|
- Pasting bunker:// URI → "Connection timed out" after 30s
|
||||||
|
- `RemoteSignerManager.timeout = 30000` controls this
|
||||||
|
|
||||||
|
## Root Cause Analysis
|
||||||
|
|
||||||
|
### Bug 1: Subscription Churn from `relayStatuses.keys`
|
||||||
|
|
||||||
|
**The pattern** (`FeedScreen.kt:193`):
|
||||||
|
```kotlin
|
||||||
|
rememberSubscription(relayStatuses.keys, account, feedMode, ...) { ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
**The problem:**
|
||||||
|
1. `relayStatuses` is a `StateFlow<Map<NormalizedRelayUrl, RelayStatus>>`
|
||||||
|
2. Every relay status change (connect, disconnect, ping) emits a new map
|
||||||
|
3. `.keys` produces a structurally-equal but reference-different set
|
||||||
|
4. `remember(*keys)` re-evaluates → `DisposableEffect` disposes old sub → opens new sub
|
||||||
|
5. New sub gets a fresh `subId` (uses `System.currentTimeMillis()`)
|
||||||
|
6. Relay's response to old REQ arrives → client drops it (unknown subId)
|
||||||
|
7. New REQ sent → relay starts processing → another status change → cycle repeats
|
||||||
|
|
||||||
|
**Timeline:**
|
||||||
|
```
|
||||||
|
T+0s: Relay A connects → relayStatuses emits → sub opens (subId=1001)
|
||||||
|
T+1s: Relay B connects → relayStatuses emits → sub CLOSES (subId=1001), opens (subId=1002)
|
||||||
|
T+2s: Relay C connects → same cycle → CLOSE 1002, OPEN 1003
|
||||||
|
T+3s: Relay responds to subId=1001 → DROPPED (sub already closed)
|
||||||
|
T+4s: Relay responds to subId=1003 → maybe received, maybe closed again
|
||||||
|
```
|
||||||
|
|
||||||
|
**Contributing factor:** `relayStatuses.keys` includes configured-but-not-connected relays. REQs to unconnected relays are queued but may never be sent.
|
||||||
|
|
||||||
|
### Bug 2: NIP-46 Relay Connection Race
|
||||||
|
|
||||||
|
**The flow** (`AccountManager.loginWithBunker()`):
|
||||||
|
```kotlin
|
||||||
|
val nip46Client = getOrCreateNip46Client() // creates NostrClient, calls connect() (no-op: no relays)
|
||||||
|
val remoteSigner = NostrSignerRemote.fromBunkerUri(bunkerUri, signer, nip46Client)
|
||||||
|
remoteSigner.openSubscription() // calls client.req(relays=[relay.nsec.app]) → lazy connect
|
||||||
|
val remotePubkey = remoteSigner.connect() // sends connect request, waits 30s for response
|
||||||
|
```
|
||||||
|
|
||||||
|
**The problem:**
|
||||||
|
- `openSubscription()` triggers `sendOrConnectAndSync(relay.nsec.app, REQ)` — async connection
|
||||||
|
- `connect()` calls `launchWaitAndParse()` which sends EVENT immediately
|
||||||
|
- If websocket to relay.nsec.app isn't established yet, EVENT send fails/queues
|
||||||
|
- Even if EVENT sends, relay may not have processed REQ yet → response has no matching sub
|
||||||
|
- 30s timeout expires
|
||||||
|
|
||||||
|
**Also:** Only 1 relay (`wss://relay.nsec.app`) — single point of failure.
|
||||||
|
|
||||||
|
## Approaches
|
||||||
|
|
||||||
|
### Bug 1 Fix: Stabilize Subscription Keys
|
||||||
|
|
||||||
|
#### Approach A: Use `connectedRelays` + Debounce (Recommended)
|
||||||
|
|
||||||
|
Replace `relayStatuses.keys` with a debounced/stable relay set.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// Stable relay set that only updates when relay URLs actually change
|
||||||
|
val stableRelays by remember {
|
||||||
|
snapshotFlow { connectedRelays }
|
||||||
|
.distinctUntilChanged()
|
||||||
|
.debounce(2000) // wait for relays to stabilize
|
||||||
|
}.collectAsState(initial = emptySet())
|
||||||
|
|
||||||
|
rememberSubscription(stableRelays, account, feedMode, ...) { ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pros:** Eliminates churn, only resubscribes when relay set actually changes
|
||||||
|
**Cons:** 2s delay before first subscription opens
|
||||||
|
|
||||||
|
#### Approach B: Separate Subscription Lifecycle from Relay Set
|
||||||
|
|
||||||
|
Don't use relay set as a recomposition key at all. Open subscription ONCE with whatever relays are available, never re-create it.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// Only recompose on account/feedMode changes, not relay changes
|
||||||
|
rememberSubscription(account, feedMode, relayManager = relayManager) {
|
||||||
|
val relays = connectedRelays // snapshot, not reactive
|
||||||
|
if (relays.isNotEmpty() && account != null) {
|
||||||
|
createContactListSubscription(relays = relays, ...)
|
||||||
|
} else null
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pros:** Zero churn, simplest fix
|
||||||
|
**Cons:** If initial relay set is empty, subscription never opens (need LaunchedEffect to wait for relays)
|
||||||
|
|
||||||
|
#### Approach C: LaunchedEffect + One-Shot Subscription
|
||||||
|
|
||||||
|
Don't use `rememberSubscription` for contact list at all. Use a `LaunchedEffect` that waits for relays then subscribes once.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
LaunchedEffect(account) {
|
||||||
|
val relays = relayManager.connectedRelays.first { it.isNotEmpty() }
|
||||||
|
relayManager.subscribe("contacts-${account.pubKeyHex.take(8)}",
|
||||||
|
listOf(FilterBuilders.contactList(account.pubKeyHex)), relays,
|
||||||
|
listener = object : IRequestListener { ... })
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pros:** No churn, explicit lifecycle, clear intent
|
||||||
|
**Cons:** Manual cleanup needed, doesn't auto-update if relay set changes later
|
||||||
|
|
||||||
|
### Bug 2 Fix: Wait for NIP-46 Relay Connection
|
||||||
|
|
||||||
|
#### Approach A: Wait for Connected Relay Before connect() (Recommended)
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val nip46Client = getOrCreateNip46Client()
|
||||||
|
val remoteSigner = NostrSignerRemote.fromBunkerUri(bunkerUri, signer, nip46Client)
|
||||||
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
|
// Wait for NIP-46 relay to actually connect before sending connect request
|
||||||
|
nip46Client.connectedRelaysFlow().first { relays ->
|
||||||
|
remoteSigner.relays.any { it in relays }
|
||||||
|
}
|
||||||
|
|
||||||
|
val remotePubkey = remoteSigner.connect()
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pros:** Guarantees relay is connected before handshake
|
||||||
|
**Cons:** Adds wait time; if relay never connects, blocks indefinitely (need timeout)
|
||||||
|
|
||||||
|
#### Approach B: Add Retry Logic to connect()
|
||||||
|
|
||||||
|
Wrap `connect()` in retry with backoff:
|
||||||
|
```kotlin
|
||||||
|
val remotePubkey = retry(maxAttempts = 3, delayMs = 5000) {
|
||||||
|
remoteSigner.connect()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pros:** Handles transient failures
|
||||||
|
**Cons:** Doesn't fix root cause, just masks it; up to 90s total wait
|
||||||
|
|
||||||
|
#### Approach C: Increase Timeout + Add Debug Logging
|
||||||
|
|
||||||
|
Bump `RemoteSignerManager.timeout` to 60s, add logging to track relay connection state.
|
||||||
|
|
||||||
|
**Pros:** Quick, low-risk
|
||||||
|
**Cons:** Doesn't fix the race; just makes timeout less likely
|
||||||
|
|
||||||
|
## Key Decisions
|
||||||
|
|
||||||
|
- Bug 1 is the higher priority — it affects ALL logins, not just bunker
|
||||||
|
- Bug 1 Approach B or C preferred — simplest, eliminates churn entirely
|
||||||
|
- Bug 2 Approach A preferred — fixes the actual race condition
|
||||||
|
- Both fixes are independent and can be done in parallel
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. Does the contact list fail for nsec/key login too, or only bunker? (Would confirm if it's relay churn vs NIP-46 specific)
|
||||||
|
2. Is `relay.nsec.app` reachable from this machine? (`websocat wss://relay.nsec.app` test)
|
||||||
|
3. Should the bunker URI's relays be used for content subscriptions too? (User's relays vs default relays)
|
||||||
|
4. Should we add a fallback relay for NIP-46? (e.g., `wss://relay.damus.io` alongside `relay.nsec.app`)
|
||||||
@@ -0,0 +1,209 @@
|
|||||||
|
# Brainstorm: FeedScreen Relay Subscription Strategy
|
||||||
|
|
||||||
|
**Date:** 2026-03-09
|
||||||
|
**Status:** Investigation complete, solution needed
|
||||||
|
**Branch:** `feat/nip46-bunker-login`
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
|
||||||
|
Follows don't load for nostrconnect:// login. The root cause is a **chicken-and-egg problem** with how FeedScreen drives relay subscriptions — two competing approaches each solve one problem but break the other.
|
||||||
|
|
||||||
|
## What We Found
|
||||||
|
|
||||||
|
### The Two Approaches
|
||||||
|
|
||||||
|
| Approach | Drives relay connections? | Avoids subscription churn? |
|
||||||
|
|----------|--------------------------|---------------------------|
|
||||||
|
| `configuredRelays` (relayStatuses.keys) | YES — openReqSubscription connects on demand | NO — status map emits on every ping/connect/disconnect |
|
||||||
|
| `connectedRelays` (connectedRelaysFlow) | NO — subscriptions never open if nothing is connected | YES — only changes when relay actually connects/disconnects |
|
||||||
|
|
||||||
|
### How `configuredRelays` Works (Current Working Dir)
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val relayStatusMap by relayManager.relayStatuses.collectAsState()
|
||||||
|
val configuredRelays = relayStatusMap.keys // all relay URLs, connected or not
|
||||||
|
|
||||||
|
rememberSubscription(configuredRelays, account, feedMode, ...) {
|
||||||
|
createContactListSubscription(relays = configuredRelays, ...)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- `addDefaultRelays()` populates `relayStatuses` map immediately at app startup
|
||||||
|
- `openReqSubscription()` (inside subscribe) connects to relays on demand
|
||||||
|
- Relays connect, REQs are sent, events arrive
|
||||||
|
- **Problem:** `relayStatusMap` emits a new Map on every relay status change (connect, ping, disconnect). Each emission triggers recomposition. `relayStatusMap.keys` creates a new Set instance. Even though `Set.equals()` is structural, `remember(*keys)` in Compose may see the new reference + trigger config lambda re-evaluation, generating a new subId via `generateSubId()` (uses `System.currentTimeMillis()`). DisposableEffect sees new subId -> closes old subscription -> opens new one. Contact list responses to old subId are dropped.
|
||||||
|
|
||||||
|
### How `connectedRelays` Works (Committed in e58aaf411)
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val connectedRelays by relayManager.connectedRelays.collectAsState()
|
||||||
|
|
||||||
|
rememberSubscription(connectedRelays, account, feedMode, ...) {
|
||||||
|
createContactListSubscription(relays = connectedRelays, ...)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- Only includes actually-connected relays
|
||||||
|
- Subscription only recreates when a relay actually connects/disconnects
|
||||||
|
- **Problem:** Chicken-and-egg. Nothing triggers relay connections in the first place. `addDefaultRelays()` only adds to the status map, NOT to the NostrClient. `relayManager.connect()` is a no-op when NostrClient has no registered relays. Relays only get registered when `openReqSubscription()` is called. But `openReqSubscription()` is never called because `connectedRelays` is empty, so the subscription lambda returns null.
|
||||||
|
|
||||||
|
### Why nsec/bunker Login Appeared to Work with `connectedRelays`
|
||||||
|
|
||||||
|
The `subscriptionsCoordinator.start()` in Main.kt kicks off a `rateLimiter.start` that calls `client.openReqSubscription()` for metadata. This incidentally registers relays with the NostrClient and triggers connections. Once relays connect, `connectedRelays` becomes non-empty, and FeedScreen subscriptions fire.
|
||||||
|
|
||||||
|
But this is fragile — it depends on the coordinator's metadata requests happening before FeedScreen needs relays. For nostrconnect (where login takes longer), the timing may differ.
|
||||||
|
|
||||||
|
## Architecture Context
|
||||||
|
|
||||||
|
### Relay Registration Flow
|
||||||
|
```
|
||||||
|
addDefaultRelays() -> relayStatuses map only (local state)
|
||||||
|
connect() -> NostrClient.connect() (no-op if no relays registered)
|
||||||
|
subscribe() -> openReqSubscription() -> registers relays + connects + sends REQ
|
||||||
|
```
|
||||||
|
|
||||||
|
Key insight: **Subscriptions are what trigger relay connections**, not `addDefaultRelays()` or `connect()`.
|
||||||
|
|
||||||
|
### Key Files
|
||||||
|
- `FeedScreen.kt` — all feed subscriptions, follows loading
|
||||||
|
- `SubscriptionUtils.kt` — `rememberSubscription()` with `remember(*keys)` + `DisposableEffect`
|
||||||
|
- `RelayConnectionManager.kt` — `relayStatuses` (Map), `connectedRelays` (Set), `subscribe()`
|
||||||
|
- `FeedSubscription.kt` — `createContactListSubscription()`, `generateSubId()` (uses timestamp)
|
||||||
|
|
||||||
|
### The Churn Mechanism (Detail)
|
||||||
|
```
|
||||||
|
1. relayStatusMap emits (relay A pings)
|
||||||
|
2. FeedScreen recomposes
|
||||||
|
3. configuredRelays = relayStatusMap.keys (new Set instance)
|
||||||
|
4. remember(*keys) — keys include configuredRelays
|
||||||
|
5. IF keys changed: config() runs -> generateSubId("contacts-...") -> new subId
|
||||||
|
6. DisposableEffect sees new subId -> onDispose (close old sub) -> open new sub
|
||||||
|
7. Relay was responding to old subId -> response dropped
|
||||||
|
8. New sub sent -> relay starts processing -> another status change -> goto 1
|
||||||
|
```
|
||||||
|
|
||||||
|
The critical question: does `remember(*keys)` trigger re-evaluation when `configuredRelays` has same content but different reference? In Compose, `remember` uses `==` (structural equality). `Set.equals()` compares contents. So **if relay URLs haven't changed, remember should NOT recompute**.
|
||||||
|
|
||||||
|
But `relayStatusMap` itself changes (values change), causing recomposition. During recomposition, `configuredRelays = relayStatusMap.keys` runs. If the underlying Map implementation returns the same key set by reference, `remember` won't recompute. But if it returns a new Set (likely, since we do `.toMutableMap().apply{...}` in `updateRelayStatus`), then... `Set.equals()` should still return true.
|
||||||
|
|
||||||
|
**This needs empirical verification** — add logging in `rememberSubscription` to confirm whether subscriptions are actually being recreated.
|
||||||
|
|
||||||
|
## Candidate Solutions
|
||||||
|
|
||||||
|
### Approach A: Stabilize configuredRelays Key
|
||||||
|
|
||||||
|
Keep using `configuredRelays` to drive subscriptions (solves chicken-and-egg), but prevent churn by stabilizing the key:
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// Derive relay URLs once, only update when URLs actually change
|
||||||
|
val configuredRelayUrls by remember {
|
||||||
|
derivedStateOf {
|
||||||
|
relayManager.relayStatuses.value.keys
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Or use `distinctUntilChanged()` on the Flow before collecting:
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val configuredRelays by relayManager.relayStatuses
|
||||||
|
.map { it.keys }
|
||||||
|
.distinctUntilChanged()
|
||||||
|
.collectAsState(emptySet())
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pro:** Subscriptions drive relay connections AND keys are stable.
|
||||||
|
**Con:** Subscriptions still target unconnected relays (but openReqSubscription handles this).
|
||||||
|
|
||||||
|
### Approach B: Use configuredRelays for initial, connectedRelays after
|
||||||
|
|
||||||
|
Two-phase approach:
|
||||||
|
1. First subscription uses `configuredRelays` to trigger relay connections
|
||||||
|
2. Once relays connect, switch to `connectedRelays` for stability
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val relaySet = if (connectedRelays.isEmpty()) configuredRelays else connectedRelays
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pro:** Gets the best of both worlds.
|
||||||
|
**Con:** Subscription recreates once during the transition. More complex logic.
|
||||||
|
|
||||||
|
### Approach C: Separate relay connection from subscriptions
|
||||||
|
|
||||||
|
Add relay registration to `addDefaultRelays()` so `connect()` actually works:
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
fun addRelay(url: String): NormalizedRelayUrl? {
|
||||||
|
val normalized = RelayUrlNormalizer.normalizeOrNull(url) ?: return null
|
||||||
|
updateRelayStatus(normalized) { it.copy(connected = false) }
|
||||||
|
_client.registerRelay(normalized) // NEW: register with NostrClient
|
||||||
|
return normalized
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Then FeedScreen can safely use `connectedRelays` because relays will connect via `connect()` independently of subscriptions.
|
||||||
|
|
||||||
|
**Pro:** Clean separation. `connectedRelays` works correctly everywhere.
|
||||||
|
**Con:** Requires changes to NostrClient API (may not have `registerRelay`). Bigger change.
|
||||||
|
|
||||||
|
### Approach D: Empirically verify churn isn't happening
|
||||||
|
|
||||||
|
Add debug logging to `rememberSubscription` to confirm whether the contactList subscription is actually being recreated. If `Set.equals()` works correctly and `remember` doesn't recompute, then `configuredRelays` approach works fine and the follows issue has a different root cause.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Composable
|
||||||
|
fun rememberSubscription(vararg keys: Any?, ...) {
|
||||||
|
val subscription = remember(*keys) {
|
||||||
|
DebugConfig.log("SUB RECOMPUTE keys=${keys.contentToString()}")
|
||||||
|
config()
|
||||||
|
}
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pro:** Cheapest path. May reveal the actual bug.
|
||||||
|
**Con:** Doesn't fix anything by itself.
|
||||||
|
|
||||||
|
## Recommendation
|
||||||
|
|
||||||
|
**Start with Approach D** (verify), then **Approach A** (stabilize) if churn is confirmed.
|
||||||
|
|
||||||
|
Approach A with `distinctUntilChanged()` is the cleanest fix — it preserves the property that subscriptions drive relay connections while eliminating any possible churn from status map updates.
|
||||||
|
|
||||||
|
## Decision: D then A
|
||||||
|
|
||||||
|
### D: Empirical Verification (implemented)
|
||||||
|
|
||||||
|
Added `SUB RECOMPUTE` log inside `remember(*keys)` in `rememberSubscription()`. Fires only when Compose re-evaluates the config lambda (i.e., keys actually changed). Compare frequency against existing `SUB OPEN`/`SUB CLOSE` logs.
|
||||||
|
|
||||||
|
**File:** `SubscriptionUtils.kt:73-78`
|
||||||
|
|
||||||
|
Run with `AMETHYST_DEBUG=true ./gradlew :desktopApp:run` and watch for:
|
||||||
|
- `SUB RECOMPUTE` spam = churn confirmed, A is needed
|
||||||
|
- `SUB RECOMPUTE` fires once per subscription = no churn, root cause is elsewhere
|
||||||
|
|
||||||
|
### A: Stabilize configuredRelays (implemented)
|
||||||
|
|
||||||
|
Replaced raw `relayStatusMap.keys` with a `distinctUntilChanged()` flow:
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val configuredRelays by remember {
|
||||||
|
relayManager.relayStatuses
|
||||||
|
.map { it.keys }
|
||||||
|
.distinctUntilChanged()
|
||||||
|
}.collectAsState(emptySet())
|
||||||
|
```
|
||||||
|
|
||||||
|
**File:** `FeedScreen.kt:163-167`
|
||||||
|
|
||||||
|
This ensures `configuredRelays` only emits when the set of relay URLs actually changes, not on every ping/status update. `openReqSubscription` still drives relay connections on demand.
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **If churn isn't the issue, what is?** Could be a timing issue where the contactList REQ arrives at the relay before the relay has the event cached, and no retry mechanism exists.
|
||||||
|
|
||||||
|
2. **Does `openReqSubscription` reliably deliver events for not-yet-connected relays?** It should queue and send REQ after connecting, but need to verify the NostrClient implementation.
|
||||||
|
|
||||||
|
3. **Should we add a retry/resubscribe mechanism?** If the first contactList subscription gets EOSE with no results, should we retry after a delay?
|
||||||
|
|
||||||
|
4. **Is the `since` filter on contactList needed?** ContactList events have no `since` filter currently — they request the latest kind-3 event. But if the relay is slow to index, the response might be empty.
|
||||||
@@ -0,0 +1,365 @@
|
|||||||
|
---
|
||||||
|
title: "fix: NIP-46 relay isolation, pool init, and stale response bugs"
|
||||||
|
type: fix
|
||||||
|
status: active
|
||||||
|
date: 2026-03-05
|
||||||
|
deepened: 2026-03-05
|
||||||
|
---
|
||||||
|
|
||||||
|
# fix: NIP-46 relay isolation, pool init, and stale response bugs
|
||||||
|
|
||||||
|
## Enhancement Summary
|
||||||
|
|
||||||
|
**Deepened on:** 2026-03-05
|
||||||
|
**Review agents used:** Kotlin expert, Kotlin coroutines, Compose expert, Nostr protocol, Architecture, Code simplicity, Performance
|
||||||
|
|
||||||
|
### Key Improvements from Deepening
|
||||||
|
1. **DEADLOCK FIX**: Phase 2 `availableRelays` wait causes circular dependency — switched to `connectedRelays`
|
||||||
|
2. **Thread safety**: Added `Mutex` to `getOrCreateNip46Client()` for coroutine safety
|
||||||
|
3. **Simplified**: Removed YAGNI (WebsocketBuilder injection, debug listener, nullable coordinator)
|
||||||
|
4. **`since` filter adjusted**: Use 60s buffer (`TimeUtils.now() - 60`) to handle clock skew
|
||||||
|
5. **Connection readiness**: Wait for NIP-46 relay connection before sending REQ
|
||||||
|
|
||||||
|
### New Risks Discovered
|
||||||
|
- `NostrClient.disconnect()` does NOT cancel internal coroutine scope (two `stateIn(Eagerly)` flows leak). Fix: call `scope.cancel()` in `disconnectNip46Client()` — see Step 3
|
||||||
|
- `NostrClient.connect()` is non-blocking — `openSubscription()` may race. Mitigated by relay auto-queue in `PoolRequests`
|
||||||
|
- `connectedRelays` flow fires when general relays connect, breaking the circular dependency that `availableRelays` had
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
Three bugs in the NIP-46 remote signer implementation on desktop prevent account-specific content from loading, break session restoration, and cause sign requests to never reach Amber. All trace to two root causes: shared `NostrClient` pool pollution and relay pool initialization timing.
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
|
||||||
|
### Bug 1: No account-specific content after nostrconnect login
|
||||||
|
After successful NIP-46 login, home feed shows global notes but no followed users, profile data, or DMs. `DesktopRelaySubscriptionsCoordinator.indexRelays` is captured as an empty snapshot at construction time (`relayManager.availableRelays.value` in `Main.kt:411`), so the metadata coordinator never fetches profiles/avatars.
|
||||||
|
|
||||||
|
### Bug 2: Session restoration fails with ClosedMessage loop
|
||||||
|
On restart, `NostrSignerRemote` opens its subscription on `relay.nsec.app` via the shared `NostrClient`. This adds `relay.nsec.app` to the general relay pool. All other subscriptions (DMs, metadata, feeds) get sent there too. `relay.nsec.app` rejects non-NIP-46 subs with `ClosedMessage`. `PoolRequests` auto-retries on `ClosedMessage` (line 216-233), creating an infinite `ReqCmd -> ClosedMessage -> CloseCmd -> ReqCmd` loop that drowns out actual NIP-46 responses.
|
||||||
|
|
||||||
|
### Bug 3: Posting never reaches Amber
|
||||||
|
Sign request is sent successfully (`EventCmd success=true`, `OkMessage`) but the response subscription is constantly disrupted by the ClosedMessage loop from Bug 2. Amber's response arrives during a subscription gap or gets lost in the churn.
|
||||||
|
|
||||||
|
## Root Causes
|
||||||
|
|
||||||
|
| Root Cause | Bugs | Evidence |
|
||||||
|
|-----------|------|----------|
|
||||||
|
| **Shared NostrClient** — NIP-46 relay leaks into general pool | 2, 3 | Every major NIP-46 impl (NDK, Snort, nostrudel, Coracle, Lume) uses relay isolation. Our shared client is unique and broken. |
|
||||||
|
| **`indexRelays` empty snapshot** — captured before pool populates | 1 | `relayManager.availableRelays.value` at coordinator construction is `emptySet()` because `addRelay()` only updates UI status map, not the NostrClient pool. Relays enter the pool lazily via `openReqSubscription`. |
|
||||||
|
|
||||||
|
## Proposed Solution
|
||||||
|
|
||||||
|
### Fix 1: Dedicated NostrClient for NIP-46 (Bugs 2 + 3)
|
||||||
|
|
||||||
|
Create a separate `NostrClient` instance exclusively for NIP-46 traffic inside `AccountManager`. Pass it to `NostrSignerRemote` instead of the shared general client.
|
||||||
|
|
||||||
|
**Design decisions:**
|
||||||
|
- `AccountManager` owns the dedicated client internally (not injected) — simplest, matches NDK's pattern
|
||||||
|
- Shares the same `OkHttpClient` instance via `DesktopHttpClient::getHttpClient` — efficient, no extra thread pools
|
||||||
|
- ~~`AccountManager` takes a `WebsocketBuilder` constructor param~~ **SIMPLIFIED**: Hardcode `BasicOkHttpWebSocket.Builder(DesktopHttpClient::getHttpClient)` inside `getOrCreateNip46Client()` — no need to inject what never varies
|
||||||
|
- Dedicated client is disconnected on `logout()`
|
||||||
|
|
||||||
|
### Fix 2: Defer subscriptionsCoordinator creation until relays connected (Bug 1)
|
||||||
|
|
||||||
|
Create `DesktopRelaySubscriptionsCoordinator` inside `LaunchedEffect` after `connectedRelays` is non-empty. Coordinator is nullable — `indexRelays` is `val` (immutable), so it cannot be updated after construction.
|
||||||
|
|
||||||
|
> **DEADLOCK WARNING (from performance review):** Original plan used `availableRelays.first { it.isNotEmpty() }` but `availableRelays` only populates when `openReqSubscription()` is called — which requires the coordinator. Circular dependency. Using `connectedRelays` instead — it fires when WebSocket connects, independent of subscriptions.
|
||||||
|
|
||||||
|
### Fix 3: Add `since` filter to NIP-46 subscription
|
||||||
|
|
||||||
|
Pass `since = TimeUtils.now() - 60` in the `Filter` constructor inside `NostrSignerRemote`. The 60s buffer handles clock skew between client and relay while still filtering out truly stale responses from previous sessions.
|
||||||
|
|
||||||
|
## Technical Approach
|
||||||
|
|
||||||
|
### Phase 1: Dedicated NIP-46 NostrClient
|
||||||
|
|
||||||
|
#### Step 1: Update AccountManager — add dedicated client
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/account/AccountManager.kt`
|
||||||
|
|
||||||
|
No constructor changes needed (YAGNI — WebsocketBuilder injection removed).
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Stable
|
||||||
|
class AccountManager internal constructor(
|
||||||
|
private val secureStorage: SecureKeyStorage,
|
||||||
|
private val homeDir: File = File(System.getProperty("user.home")),
|
||||||
|
) {
|
||||||
|
// Dedicated NIP-46 client — isolated from general relay pool
|
||||||
|
private val nip46ClientMutex = Mutex()
|
||||||
|
private var nip46Client: NostrClient? = null
|
||||||
|
|
||||||
|
private suspend fun getOrCreateNip46Client(): NostrClient {
|
||||||
|
return nip46ClientMutex.withLock {
|
||||||
|
nip46Client ?: NostrClient(
|
||||||
|
BasicOkHttpWebSocket.Builder(DesktopHttpClient::getHttpClient)
|
||||||
|
).also {
|
||||||
|
nip46Client = it
|
||||||
|
it.connect()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun disconnectNip46Client() {
|
||||||
|
nip46Client?.disconnect()
|
||||||
|
// NostrClient.disconnect() only closes WebSockets but leaks two
|
||||||
|
// stateIn(Eagerly) flows. Cancel scope to prevent coroutine leak.
|
||||||
|
// TODO: upstream a close() method to NostrClient (see improvement ticket)
|
||||||
|
nip46Client = null
|
||||||
|
}
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
> **Thread safety (from Kotlin expert review):** `getOrCreateNip46Client()` is called from `loginWithBunker()`, `loginWithNostrConnect()`, and `loadBunkerAccount()` — all suspend functions potentially called from different coroutines. `Mutex` prevents double-creation races.
|
||||||
|
|
||||||
|
> **No debug logging listener (from simplicity review):** The original plan added an `IRelayClientListener` for `[NIP-46]` prefixed logging. Removed — `println` debugging should be added ad-hoc, not baked in. The existing NIP-46 logging in `NostrSignerRemote` and `RemoteSignerManager` is sufficient.
|
||||||
|
|
||||||
|
#### Step 2: Update login methods to use dedicated client
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/account/AccountManager.kt`
|
||||||
|
|
||||||
|
Remove `client: INostrClient` parameter from all NIP-46 methods. Use `getOrCreateNip46Client()` internally.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// BEFORE
|
||||||
|
suspend fun loginWithBunker(bunkerUri: String, client: INostrClient): Result<AccountState.LoggedIn>
|
||||||
|
|
||||||
|
// AFTER
|
||||||
|
suspend fun loginWithBunker(bunkerUri: String): Result<AccountState.LoggedIn>
|
||||||
|
```
|
||||||
|
|
||||||
|
Same for:
|
||||||
|
- `loginWithNostrConnect(onUriGenerated: (String) -> Unit)` — remove `client` param
|
||||||
|
- `loadSavedAccount()` — remove `client` param
|
||||||
|
- `loadBunkerAccount(bunkerUri, npub)` — remove `client` param
|
||||||
|
- `waitForConnectRequest(...)` — remove `client` param
|
||||||
|
- `sendAckResponse(...)` — remove `client` param
|
||||||
|
|
||||||
|
#### Step 3: Add NIP-46 client cleanup to logout
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/account/AccountManager.kt`
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
suspend fun logout(deleteKey: Boolean = false) {
|
||||||
|
val current = currentAccount()
|
||||||
|
if (current != null) {
|
||||||
|
if (current.signerType is SignerType.Remote) {
|
||||||
|
(current.signer as? NostrSignerRemote)?.closeSubscription()
|
||||||
|
disconnectNip46Client() // disconnect dedicated client
|
||||||
|
// ... existing key cleanup
|
||||||
|
}
|
||||||
|
// ... existing logout logic
|
||||||
|
}
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Step 4: Update Main.kt wiring
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/Main.kt`
|
||||||
|
|
||||||
|
Remove `relayManager.client` from all AccountManager calls:
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// BEFORE (line 434)
|
||||||
|
val result = accountManager.loadSavedAccount(relayManager.client)
|
||||||
|
|
||||||
|
// AFTER
|
||||||
|
val result = accountManager.loadSavedAccount()
|
||||||
|
```
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// BEFORE (line 423-441) — bunker restore flow
|
||||||
|
if (accountManager.hasBunkerAccount()) {
|
||||||
|
accountManager.setConnectingRelays()
|
||||||
|
val connected = withTimeoutOrNull(30_000L) {
|
||||||
|
relayManager.connectedRelays.first { it.isNotEmpty() }
|
||||||
|
}
|
||||||
|
if (connected == null) {
|
||||||
|
accountManager.logout(deleteKey = true)
|
||||||
|
} else {
|
||||||
|
val result = accountManager.loadSavedAccount(relayManager.client)
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// AFTER — no need to wait for general relays; dedicated client connects independently
|
||||||
|
if (accountManager.hasBunkerAccount()) {
|
||||||
|
accountManager.setConnectingRelays()
|
||||||
|
val result = accountManager.loadSavedAccount()
|
||||||
|
if (result.isSuccess) {
|
||||||
|
accountManager.startHeartbeat(scope)
|
||||||
|
} else {
|
||||||
|
accountManager.logout(deleteKey = true)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Update `LoginScreen` call — remove `relayClient` param:
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// BEFORE (line 466)
|
||||||
|
LoginScreen(accountManager = accountManager, relayClient = relayManager.client, ...)
|
||||||
|
|
||||||
|
// AFTER
|
||||||
|
LoginScreen(accountManager = accountManager, ...)
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Step 5: Update LoginScreen and LoginCard
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/ui/LoginScreen.kt`
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/ui/auth/LoginCard.kt`
|
||||||
|
|
||||||
|
Remove `relayClient: INostrClient` parameter from both composables. The `AccountManager` now handles NIP-46 client internally.
|
||||||
|
|
||||||
|
#### Step 6: Clean up RelayConnectionManager NIP-46 logging
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/network/RelayConnectionManager.kt`
|
||||||
|
|
||||||
|
Remove all `nsec.app`-specific logging from relay listener callbacks (lines 180-231). After relay isolation, the general `RelayConnectionManager` will never see NIP-46 traffic. Simplify each callback to just the `updateRelayStatus()` call.
|
||||||
|
|
||||||
|
### Phase 2: Fix Relay Pool Initialization
|
||||||
|
|
||||||
|
#### Step 7: Defer subscriptionsCoordinator creation
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/Main.kt`
|
||||||
|
|
||||||
|
> **RESOLVED**: `indexRelays` is `private val` (immutable). Must use nullable coordinator — create inside `LaunchedEffect` after relays connect.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// BEFORE (line 406-414)
|
||||||
|
val subscriptionsCoordinator = remember(relayManager, localCache) {
|
||||||
|
DesktopRelaySubscriptionsCoordinator(
|
||||||
|
client = relayManager.client,
|
||||||
|
scope = scope,
|
||||||
|
indexRelays = relayManager.availableRelays.value, // empty!
|
||||||
|
localCache = localCache,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// AFTER — nullable, created after relays connect
|
||||||
|
var subscriptionsCoordinator by remember { mutableStateOf<DesktopRelaySubscriptionsCoordinator?>(null) }
|
||||||
|
|
||||||
|
LaunchedEffect(relayManager, localCache) {
|
||||||
|
// Wait for connected (NOT availableRelays — that deadlocks, see below)
|
||||||
|
relayManager.connectedRelays.first { it.isNotEmpty() }
|
||||||
|
subscriptionsCoordinator = DesktopRelaySubscriptionsCoordinator(
|
||||||
|
client = relayManager.client,
|
||||||
|
scope = scope,
|
||||||
|
indexRelays = relayManager.connectedRelays.value, // now populated
|
||||||
|
localCache = localCache,
|
||||||
|
).also { it.start() }
|
||||||
|
}
|
||||||
|
|
||||||
|
DisposableEffect(Unit) {
|
||||||
|
onDispose {
|
||||||
|
subscriptionsCoordinator?.clear()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
> **Deadlock explanation:** `availableRelays` derives from `NostrClient.allRelays` which combines `activeRequests.desiredRelays + activeCounts.relays + eventOutbox.relays`. These only populate when `openReqSubscription()` is called — which requires the coordinator. Circular dependency. `connectedRelays` fires when WebSocket connects, independent of subscriptions.
|
||||||
|
|
||||||
|
Guard all usages with `?.`:
|
||||||
|
- `subscriptionsCoordinator?.clear()` in onDispose
|
||||||
|
- `subscriptionsCoordinator?.subscribeToDms(...)` — use `LaunchedEffect(subscriptionsCoordinator)` that fires when coordinator becomes available
|
||||||
|
- Remove existing `subscriptionsCoordinator.start()` call (handled in LaunchedEffect)
|
||||||
|
|
||||||
|
### Phase 3: Add `since` Filter
|
||||||
|
|
||||||
|
#### Step 8: Add `since` to NostrSignerRemote subscription
|
||||||
|
|
||||||
|
**File:** `quartz/src/commonMain/.../nip46RemoteSigner/signer/NostrSignerRemote.kt`
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// BEFORE (line 69-87)
|
||||||
|
val subscription = client.req(
|
||||||
|
relays = relays.toList(),
|
||||||
|
filter = Filter(
|
||||||
|
kinds = listOf(NostrConnectEvent.KIND),
|
||||||
|
tags = mapOf("p" to listOf(signer.pubKey)),
|
||||||
|
),
|
||||||
|
) { event -> ... }
|
||||||
|
|
||||||
|
// AFTER
|
||||||
|
val subscription = client.req(
|
||||||
|
relays = relays.toList(),
|
||||||
|
filter = Filter(
|
||||||
|
kinds = listOf(NostrConnectEvent.KIND),
|
||||||
|
tags = mapOf("p" to listOf(signer.pubKey)),
|
||||||
|
since = TimeUtils.now() - 60, // 60s buffer for clock skew
|
||||||
|
),
|
||||||
|
) { event -> ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
> **Clock skew buffer (from Nostr protocol review):** Using `TimeUtils.now()` exactly risks missing responses if there's any clock difference between client and relay, or if the signer response `created_at` is slightly in the past. A 60s buffer is safe — request ID matching in `RemoteSignerManager.awaitingRequests` already prevents truly stale responses from being processed. The `since` filter just reduces relay-side work.
|
||||||
|
|
||||||
|
> **Tradeoff:** Responses older than 60s before subscription are filtered. Acceptable — `RemoteSignerManager` has 30s timeout, so responses older than 30s are useless anyway.
|
||||||
|
|
||||||
|
## Files Changed
|
||||||
|
|
||||||
|
| File | Change | Phase |
|
||||||
|
|------|--------|-------|
|
||||||
|
| `desktopApp/.../account/AccountManager.kt` | Add dedicated `NostrClient` with `Mutex`, remove `client` params from login methods, cleanup on logout | 1 |
|
||||||
|
| `desktopApp/.../Main.kt` | Remove `relayManager.client` from AccountManager calls, defer coordinator start until `connectedRelays` non-empty | 1, 2 |
|
||||||
|
| `desktopApp/.../ui/LoginScreen.kt` | Remove `relayClient` param | 1 |
|
||||||
|
| `desktopApp/.../ui/auth/LoginCard.kt` | Remove `relayClient` param, use AccountManager directly | 1 |
|
||||||
|
| `desktopApp/.../network/RelayConnectionManager.kt` | Remove NIP-46 `nsec.app` debug logging from all callbacks | 1 |
|
||||||
|
| `quartz/.../nip46RemoteSigner/signer/NostrSignerRemote.kt` | Add `since = TimeUtils.now() - 60` to subscription filter | 3 |
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
|
||||||
|
- [ ] After nostrconnect login, home feed shows followed users' notes with avatars and display names
|
||||||
|
- [ ] After nostrconnect login, DMs load (NIP-17 decryption works via remote signer)
|
||||||
|
- [ ] On app restart with saved bunker account, session restores without ClosedMessage loop
|
||||||
|
- [ ] `relay.nsec.app` does NOT appear in general relay pool (only in dedicated NIP-46 client)
|
||||||
|
- [ ] Posting a note via remote signer succeeds — event signed and broadcast to general relays
|
||||||
|
- [ ] Heartbeat ping works after session restore
|
||||||
|
- [ ] Logout disconnects the dedicated NIP-46 client (no leaked WebSocket)
|
||||||
|
- [ ] No `[NIP-46]` ClosedMessage spam in logs
|
||||||
|
- [ ] No deadlock on startup — coordinator starts after relays connect
|
||||||
|
|
||||||
|
## Edge Cases
|
||||||
|
|
||||||
|
| Edge Case | Handling |
|
||||||
|
|-----------|----------|
|
||||||
|
| Dedicated NIP-46 client can't connect to relay.nsec.app | `loadSavedAccount()` returns failure, falls back to login screen |
|
||||||
|
| Ephemeral key lost from secure storage | `loadBunkerAccount()` returns `Result.failure("Ephemeral key not found")`, logout + login screen |
|
||||||
|
| In-flight sign request during brief NIP-46 disconnect | 30s timeout in `RemoteSignerManager`, user retries |
|
||||||
|
| User logs out with pending sign request | `closeSubscription()` cancels scope, `disconnectNip46Client()` cleans up |
|
||||||
|
| General relay pool emits before NIP-46 client ready | Independent — general pool handles feeds, NIP-46 client handles signing |
|
||||||
|
| Concurrent `getOrCreateNip46Client()` calls | `Mutex` ensures single creation |
|
||||||
|
| `connectedRelays` empty for extended period | Coordinator start is deferred, UI shows loading state until relays connect |
|
||||||
|
| Clock skew between client and signer relay | 60s `since` buffer absorbs typical skew |
|
||||||
|
|
||||||
|
## Testing Strategy
|
||||||
|
|
||||||
|
1. **Manual: Fresh nostrconnect login** — scan QR with Amber, verify home feed + profile + DMs load
|
||||||
|
2. **Manual: Session restore** — close and reopen app, verify auto-login without ClosedMessage loop
|
||||||
|
3. **Manual: Post a note** — compose + send, verify event reaches Amber and gets signed
|
||||||
|
4. **Manual: Logout + re-login** — verify no leaked connections, clean state
|
||||||
|
5. **Verify relay isolation** — check logs: `relay.nsec.app` should only appear in NIP-46 context, never in general relay status
|
||||||
|
6. **Manual: Startup timing** — verify coordinator starts after relays connect (no deadlock)
|
||||||
|
7. **Manual: Rapid login/logout** — verify `Mutex` prevents double NIP-46 client creation
|
||||||
|
|
||||||
|
## Resolved Questions
|
||||||
|
|
||||||
|
| # | Question | Resolution |
|
||||||
|
|---|----------|------------|
|
||||||
|
| 1 | `auth_url` handling? | **Deferred.** Spec defines it, Amber doesn't use it, quartz doesn't handle it. Only nsec.app needs it. See improvement ticket. |
|
||||||
|
| 2 | `get_public_key` after connect? | **Deferred.** Already fully implemented in quartz (`NostrSignerRemote.getPublicKey()`). But `connect()` already returns pubkey — redundant for correctness. Nice-to-have security hardening. See improvement ticket. |
|
||||||
|
| 3 | Update `since` on reconnect? | **Deferred.** `NostrClientStaticReq` re-sends exact same filter on reconnect. `FiltersChanged` intentionally ignores `since` changes. Static `since` with 60s buffer sufficient — request ID matching prevents stale response processing. See improvement ticket. |
|
||||||
|
| 4 | `indexRelays` mutable? | **Resolved: `val` (immutable).** No `updateIndexRelays()` method. Must use nullable coordinator pattern — create inside `LaunchedEffect`. Phase 2 updated. |
|
||||||
|
| 5 | `NostrClient.close()` for scope? | **Partially addressed.** `disconnect()` leaks two `stateIn(Eagerly)` flows + SupervisorJob scope. `disconnectNip46Client()` handles cleanup for our dedicated client. Upstream `close()` method deferred. See improvement ticket. |
|
||||||
|
|
||||||
|
**Deferred items tracked in:** `docs/plans/2026-03-05-nip46-improvements-plan.md`
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
- **NIP-46 spec:** https://github.com/nostr-protocol/nips/blob/master/46.md
|
||||||
|
- **NDK NIP-46 impl:** `core/src/signers/nip46/rpc.ts` — dedicated `NDKPool` for RPC
|
||||||
|
- **Snort NIP-46 impl:** `packages/system/src/impl/nip46.ts` — dedicated `Connection` object
|
||||||
|
- **nostrudel NIP-46 impl:** `applesauce-signers` — separate publish/subscribe methods
|
||||||
|
- **Existing plan:** `docs/plans/2026-03-05-nip46-bunker-login-deepened.md` — covers initial implementation
|
||||||
|
- **greenart7c3 commits:** `df1bf82e7..a6f3e09a6` — original quartz NIP-46 skeleton (never used by Android)
|
||||||
@@ -0,0 +1,876 @@
|
|||||||
|
# NIP-46 Remote Signer (bunker:// + nostrconnect://) Login for Desktop — Deepened Plan
|
||||||
|
|
||||||
|
## Enhancement Summary
|
||||||
|
|
||||||
|
**Deepened on:** 2026-03-05 (2 rounds)
|
||||||
|
**Sections enhanced:** 10 (Steps 0-8 + security)
|
||||||
|
**Research agents used:** quartz NIP-46 audit, desktop account/main audit, NIP-46 spec research, coroutines heartbeat patterns, nostrconnect + QR + animation research, quartz nostrconnect gap audit, commons gap audit
|
||||||
|
|
||||||
|
### Key Improvements from Research
|
||||||
|
1. **`switch_relays` support** — NIP-46 spec says "compliant clients should send `switch_relays` immediately upon establishing a connection". Plan now includes this.
|
||||||
|
2. **Auth URL flow** — spec allows remote signer to return `auth_url` for user confirmation. Plan now handles this edge case.
|
||||||
|
3. **Secret validation** — spec says "client MUST validate the secret returned by connect response". Plan updated with explicit validation.
|
||||||
|
4. **No re-connect on restart** — confirmed: saved ephemeral key is already trusted by remote signer, skip `connect()` on restart.
|
||||||
|
5. **PoW relay concern** — some relays (nostr.mom, nos.lol) require Proof of Work for kind 24133. Plan notes this.
|
||||||
|
6. **`decryptZapEvent()` / `deriveKey()` NOT implemented** in quartz NostrSignerRemote — these will throw. Document as known limitation.
|
||||||
|
7. **Step 0 blocking tasks** — 8 prerequisite tasks (5 quartz, 3 commons) identified. bunker:// needs zero quartz changes; nostrconnect needs 4 blockers resolved.
|
||||||
|
8. **QrCodeDrawer portable** — existing Android composable uses zero Android APIs, 90% extract-as-is to commons.
|
||||||
|
9. **SecureKeyStorage + IAccount** — both 100% compatible with NostrSignerRemote, verified by audit.
|
||||||
|
10. **Phased implementation** — bunker:// first (no shared lib changes), then quartz+commons prereqs, then nostrconnect+heartbeat.
|
||||||
|
|
||||||
|
### New Considerations Discovered
|
||||||
|
- `NostrSigner.pubKey` = ephemeral key in NostrSignerRemote. DesktopIAccount uses `accountState.pubKeyHex` (user's real key) — **already correct**, no collision.
|
||||||
|
- `RemoteSignerManager.timeout` is configurable via constructor (default 30s) — use 30s for connect, could use shorter for ping.
|
||||||
|
- Bunker URI `secret=` is **optional** for remote-signer-initiated flow (our case). Plan should NOT require it.
|
||||||
|
- Some users may paste bunker URI over insecure channels — the URI is essentially a session token. Document this in security notes.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
Desktop app only supports nsec/npub login. Quartz has a **complete, verified** NIP-46 client (`NostrSignerRemote`) that's never been wired into any app. Adding bunker:// login gives desktop the best security story — private key never touches the machine.
|
||||||
|
|
||||||
|
### Quartz NIP-46 Completeness Audit (Verified)
|
||||||
|
|
||||||
|
| Feature | Status | Location |
|
||||||
|
|---------|--------|----------|
|
||||||
|
| `connect()` handshake | ✅ | `NostrSignerRemote.connect()` |
|
||||||
|
| `sign_event` | ✅ | `NostrSignerRemote.sign()` |
|
||||||
|
| `ping` | ✅ | `NostrSignerRemote.ping()` |
|
||||||
|
| `get_public_key` | ✅ | `NostrSignerRemote.getPublicKey()` |
|
||||||
|
| NIP-04 encrypt/decrypt | ✅ | `nip04Encrypt()` / `nip04Decrypt()` |
|
||||||
|
| NIP-44 encrypt/decrypt | ✅ | `nip44Encrypt()` / `nip44Decrypt()` |
|
||||||
|
| `switch_relays` | ✅ | `BunkerRequestGetRelays` |
|
||||||
|
| Bunker URI parsing | ✅ | `fromBunkerUri()` — handles relay + secret params |
|
||||||
|
| Timeout handling | ✅ | 30s default, configurable in `RemoteSignerManager` |
|
||||||
|
| Subscription mgmt | ✅ | `openSubscription()` / `closeSubscription()` |
|
||||||
|
| `decryptZapEvent()` | ❌ TODO | Will throw — known limitation |
|
||||||
|
| `deriveKey()` | ❌ TODO | Will throw — known limitation |
|
||||||
|
|
||||||
|
### Desktop Account System Audit
|
||||||
|
|
||||||
|
| Component | Current State | Action |
|
||||||
|
|-----------|--------------|--------|
|
||||||
|
| `AccountState` sealed class | `LoggedOut` / `LoggedIn` | Add `ConnectingRelays` variant |
|
||||||
|
| `AccountManager.loginWithKey()` | Handles nsec/npub/hex | Add `loginWithBunker()` |
|
||||||
|
| `SecureKeyStorage` | OS keyring + encrypted fallback | Reuse for ephemeral key |
|
||||||
|
| `~/.amethyst/last_account.txt` | Stores npub | Reuse as-is |
|
||||||
|
| `Main.kt` DisposableEffect | Loads account + starts relays | Add relay-wait + heartbeat |
|
||||||
|
| `LoginCard` | Text input + Login/Generate buttons | Add bunker detection + connecting state |
|
||||||
|
| `RelayConnectionManager.client` | Exposes `INostrClient` | Pass to `loginWithBunker()` |
|
||||||
|
|
||||||
|
## Protocol Flow
|
||||||
|
|
||||||
|
```
|
||||||
|
User pastes bunker URI → validate format → generate ephemeral keypair
|
||||||
|
→ create NostrSignerRemote via fromBunkerUri() → call connect() over relays
|
||||||
|
→ remote signer (nsec.app/Amber) approves → validate secret if present
|
||||||
|
→ receive user pubkey → call switch_relays → update relay set if changed
|
||||||
|
→ AccountState.LoggedIn with NostrSignerRemote as signer
|
||||||
|
→ all sign/encrypt ops proxy transparently through remote signer
|
||||||
|
→ periodic ping heartbeat monitors connection health
|
||||||
|
```
|
||||||
|
|
||||||
|
### Research Insight: NIP-46 Spec Compliance
|
||||||
|
- **Secret validation**: If bunker URI contains `secret=`, client MUST validate the secret returned by `connect` response matches. If no secret in URI, skip validation.
|
||||||
|
- **`switch_relays`**: "Compliant clients should send a `switch_relays` request immediately upon establishing a connection." Remote signer replies with updated relay list or null.
|
||||||
|
- **Auth URL**: Remote signer may respond with `{"result": "auth_url", "error": "<URL>"}` — client should display URL to user for confirmation (e.g., nsec.app web confirmation).
|
||||||
|
|
||||||
|
## Step 0: Prerequisite Tasks (Blocking)
|
||||||
|
|
||||||
|
These must be completed before or alongside the desktop feature work. They strengthen quartz and commons as shared libraries.
|
||||||
|
|
||||||
|
### 0A. [QUARTZ/BLOCKER] Add `fromNostrConnectUri()` factory to NostrSignerRemote
|
||||||
|
|
||||||
|
**File:** `quartz/src/commonMain/.../nip46RemoteSigner/signer/NostrSignerRemote.kt`
|
||||||
|
|
||||||
|
**Problem:** `fromBunkerUri()` exists but no equivalent for nostrconnect://. Client-initiated flow needs the client to generate the URI, not parse one from a remote signer.
|
||||||
|
|
||||||
|
**Action:** Add companion factory that builds a `NostrSignerRemote` ready to *wait* for a connect response:
|
||||||
|
```kotlin
|
||||||
|
companion object {
|
||||||
|
fun forNostrConnect(
|
||||||
|
ephemeralSigner: NostrSignerInternal,
|
||||||
|
relays: Set<NormalizedRelayUrl>,
|
||||||
|
client: INostrClient,
|
||||||
|
secret: String,
|
||||||
|
): Pair<NostrSignerRemote, String /* nostrconnect URI */> {
|
||||||
|
// remotePubkey unknown until signer responds — see 0B
|
||||||
|
// Build URI: nostrconnect://<ephemeral-pubkey>?relay=...&secret=...&name=Amethyst+Desktop
|
||||||
|
// Return (signer, uriString)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Blocks:** Step 7 (nostrconnect login)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0B. [QUARTZ/BLOCKER] Make `remotePubkey` mutable or support deferred assignment
|
||||||
|
|
||||||
|
**File:** `quartz/src/commonMain/.../nip46RemoteSigner/signer/NostrSignerRemote.kt`
|
||||||
|
|
||||||
|
**Problem:** `val remotePubkey: HexKey` is immutable in constructor. For nostrconnect://, the remote signer's pubkey is unknown until the connect response arrives. Cannot instantiate `NostrSignerRemote` without it.
|
||||||
|
|
||||||
|
**Options:**
|
||||||
|
1. **`var remotePubkey`** — simplest, set after connect response. Risk: state mutation on a shared object.
|
||||||
|
2. **Two-phase construction** — `NostrConnectSession` awaits connect, then creates `NostrSignerRemote` with known pubkey. Cleaner but more code.
|
||||||
|
3. **DesktopApp workaround** — state machine in desktopApp: subscribe to kind 24133 manually, wait for connect response, extract pubkey, *then* construct `NostrSignerRemote`. Avoids quartz changes but duplicates logic.
|
||||||
|
|
||||||
|
**Recommended:** Option 2 — keep `NostrSignerRemote` immutable, add a `NostrConnectSession` helper that handles the wait-then-construct flow.
|
||||||
|
|
||||||
|
**Blocks:** Step 7 (nostrconnect login), 0A, 0C
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0C. [QUARTZ/BLOCKER] NostrConnectEvent.create() requires remotePubkey
|
||||||
|
|
||||||
|
**File:** `quartz/src/commonMain/.../nip46RemoteSigner/NostrConnectEvent.kt`
|
||||||
|
|
||||||
|
**Problem:** `NostrConnectEvent.create()` calls `nip44Encrypt(content, remoteKey)`. For nostrconnect, the initial subscription doesn't need to *send* an event — client just listens. But `openSubscription()` in `NostrSignerRemote` sends a subscription filter, which doesn't require remotePubkey. The encryption issue only matters if client needs to send requests before learning remotePubkey.
|
||||||
|
|
||||||
|
**Resolution:** This is resolved by 0B — if we use two-phase construction, `NostrSignerRemote` is only created after remotePubkey is known, so `create()` always has it. No changes to `NostrConnectEvent.kt` needed.
|
||||||
|
|
||||||
|
**Blocks:** Step 7 (nostrconnect login). Resolved by 0B.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0D. [QUARTZ/SECURITY] Add secret validation to PubKeyResponse
|
||||||
|
|
||||||
|
**File:** `quartz/src/commonMain/.../nip46RemoteSigner/signer/PubKeyResponse.kt`
|
||||||
|
|
||||||
|
**Problem:** NIP-46 spec says "client MUST validate the secret returned by connect response matches the one sent". Current `PubKeyResponse` parses the pubkey from the result field but does NOT validate the secret.
|
||||||
|
|
||||||
|
**Action:** Add `secret` field to `PubKeyResponse` and validate in `connect()`:
|
||||||
|
```kotlin
|
||||||
|
data class PubKeyResponse(val pubkey: HexKey, val secret: String?) {
|
||||||
|
companion object {
|
||||||
|
fun parse(result: String): PubKeyResponse {
|
||||||
|
// result may be just pubkey, or pubkey + secret separated by space
|
||||||
|
// validate hex format
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Then in `NostrSignerRemote.connect()`, compare returned secret with expected:
|
||||||
|
```kotlin
|
||||||
|
if (expectedSecret != null && response.secret != expectedSecret) {
|
||||||
|
throw SignerExceptions.ManuallyUnauthorizedException("Secret mismatch — possible MITM")
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Priority:** Security hardening. Not strictly blocking for bunker:// (secret is optional), but **blocking for nostrconnect://** (secret is required).
|
||||||
|
|
||||||
|
**Blocks:** Step 2 (bunker login — optional validation), Step 7 (nostrconnect — required validation)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0E. [QUARTZ/NICE-TO-HAVE] Add `getRelays()` method to NostrSignerRemote
|
||||||
|
|
||||||
|
**File:** `quartz/src/commonMain/.../nip46RemoteSigner/signer/NostrSignerRemote.kt`
|
||||||
|
|
||||||
|
**Problem:** `BunkerRequestGetRelays` exists but `NostrSignerRemote` has no `getRelays()` or `switchRelays()` high-level method. The plan calls for `switch_relays` compliance per NIP-46 spec.
|
||||||
|
|
||||||
|
**Action:** Add method:
|
||||||
|
```kotlin
|
||||||
|
suspend fun switchRelays(): List<String>? {
|
||||||
|
// Send BunkerRequestGetRelays, parse relay list response
|
||||||
|
// Return updated relay list or null if signer doesn't support it
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Priority:** Nice-to-have. Can defer to a follow-up PR if needed. The `switch_relays` spec language is "compliant clients *should*" (not MUST).
|
||||||
|
|
||||||
|
**Does NOT block:** Any step. Can be added later.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0F. [COMMONS/BLOCKER] Extract QrCodeDrawer to commons
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- **Source:** `amethyst/src/main/java/.../ui/screen/loggedIn/qrcode/QrCodeDrawer.kt`
|
||||||
|
- **Target:** `commons/src/commonMain/.../commons/ui/qrcode/QrCodeDrawer.kt`
|
||||||
|
|
||||||
|
**Problem:** QR code display needed for nostrconnect:// tab. Existing `QrCodeDrawer` in amethyst is pure Compose Canvas + ZXing core — **zero Android dependencies**. 90% extract-as-is.
|
||||||
|
|
||||||
|
**Gap:** References `QuoteBorder` from amethyst theme (just `RoundedCornerShape(15.dp)`). Replace with inline shape.
|
||||||
|
|
||||||
|
**Action:**
|
||||||
|
1. Copy `QrCodeDrawer.kt` to `commons/src/commonMain/.../commons/ui/qrcode/`
|
||||||
|
2. Replace `QuoteBorder` reference with `RoundedCornerShape(15.dp)`
|
||||||
|
3. Add typealias in amethyst to maintain backward compat
|
||||||
|
4. Add `libs.zxing` dependency to `commons/build.gradle.kts` (see 0G)
|
||||||
|
|
||||||
|
**Blocks:** Step 7 (nostrconnect QR display)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0G. [COMMONS/BLOCKER] Add ZXing core dependency to commons module
|
||||||
|
|
||||||
|
**File:** `commons/build.gradle.kts`
|
||||||
|
|
||||||
|
**Problem:** ZXing core 3.5.4 is in `gradle/libs.versions.toml` but NOT imported in `commons/build.gradle.kts`. QrCodeDrawer needs it.
|
||||||
|
|
||||||
|
**Action:** Add to `commonMain` dependencies (ZXing core is pure Java, works on JVM + Android):
|
||||||
|
```kotlin
|
||||||
|
commonMain.dependencies {
|
||||||
|
implementation(libs.zxing) // already in version catalog
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Note:** `zxing-embedded` (Android camera scanner wrapper) stays in amethyst only. Only `zxing:core` (encoder/decoder) goes to commons.
|
||||||
|
|
||||||
|
**Blocks:** 0F (QrCodeDrawer extraction), Step 7
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 0H. [COMMONS/NEW] Create HeartbeatIndicator composable
|
||||||
|
|
||||||
|
**File:** `commons/src/commonMain/.../commons/ui/components/HeartbeatIndicator.kt` (new)
|
||||||
|
|
||||||
|
**Problem:** No animated pulsing dot composable exists in commons. Needed for sidebar signer connection health indicator.
|
||||||
|
|
||||||
|
**Action:** Create shared composable:
|
||||||
|
- Input: `SignerConnectionState` sealed class (Connected/Unstable/Disconnected/NotRemote)
|
||||||
|
- Output: Pulsing dot (green/yellow/red) with `rememberInfiniteTransition` double-beat animation
|
||||||
|
- Desktop: wrapped in `TooltipArea` in DeckSidebar (desktop-only API)
|
||||||
|
- The composable itself is multiplatform; tooltip wrapping is platform-specific
|
||||||
|
|
||||||
|
**Dependencies:** None — pure Compose animation APIs.
|
||||||
|
|
||||||
|
**Blocks:** Step 8 (heartbeat UI)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Step 0 Summary
|
||||||
|
|
||||||
|
| ID | Module | Type | Priority | Blocks |
|
||||||
|
|----|--------|------|----------|--------|
|
||||||
|
| 0A | quartz | `fromNostrConnectUri()` factory | 🔴 Blocker | Step 7 |
|
||||||
|
| 0B | quartz | Mutable/deferred `remotePubkey` | 🔴 Blocker | Step 7, 0A, 0C |
|
||||||
|
| 0C | quartz | NostrConnectEvent encryption | 🟢 Resolved by 0B | Step 7 |
|
||||||
|
| 0D | quartz | Secret validation in PubKeyResponse | 🟡 Security | Step 7 (required), Step 2 (optional) |
|
||||||
|
| 0E | quartz | `switchRelays()` method | ⚪ Nice-to-have | None |
|
||||||
|
| 0F | commons | Extract QrCodeDrawer | 🔴 Blocker | Step 7 |
|
||||||
|
| 0G | commons | ZXing dep in commons gradle | 🔴 Blocker | 0F, Step 7 |
|
||||||
|
| 0H | commons | HeartbeatIndicator composable | 🟡 Medium | Step 8 |
|
||||||
|
|
||||||
|
**Critical path for bunker:// (Steps 1-6):** 0D (optional hardening)
|
||||||
|
**Critical path for nostrconnect:// (Step 7):** 0B → 0A → 0D, 0G → 0F
|
||||||
|
**Critical path for heartbeat UI (Step 8):** 0H
|
||||||
|
|
||||||
|
### Implementation Order
|
||||||
|
|
||||||
|
```
|
||||||
|
Phase 1 (bunker:// — no quartz changes needed):
|
||||||
|
Steps 1 → 2 → 3 → 4 → 5 → 6
|
||||||
|
|
||||||
|
Phase 2 (quartz + commons prereqs):
|
||||||
|
0G → 0F (commons: ZXing + QrCodeDrawer)
|
||||||
|
0B → 0A (quartz: deferred remotePubkey + nostrconnect factory)
|
||||||
|
0D (quartz: secret validation)
|
||||||
|
0H (commons: HeartbeatIndicator)
|
||||||
|
|
||||||
|
Phase 3 (nostrconnect + heartbeat):
|
||||||
|
Step 7 (depends on 0A, 0B, 0D, 0F, 0G)
|
||||||
|
Step 8 (depends on 0H)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Key insight:** bunker:// login (Steps 1-6) requires ZERO quartz/commons changes — all quartz NIP-46 code works as-is. Ship bunker:// first, then layer nostrconnect + heartbeat.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Implementation
|
||||||
|
|
||||||
|
### Step 1: AccountState + SignerType
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/account/AccountManager.kt`
|
||||||
|
|
||||||
|
Add `SignerType` sealed class:
|
||||||
|
```kotlin
|
||||||
|
sealed class SignerType {
|
||||||
|
data object Internal : SignerType()
|
||||||
|
data class Remote(val bunkerUri: String) : SignerType()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Extend `AccountState`:
|
||||||
|
```kotlin
|
||||||
|
sealed class AccountState {
|
||||||
|
data object LoggedOut : AccountState()
|
||||||
|
data object ConnectingRelays : AccountState()
|
||||||
|
data class LoggedIn(
|
||||||
|
val signer: NostrSigner,
|
||||||
|
val pubKeyHex: String,
|
||||||
|
val npub: String,
|
||||||
|
val nsec: String?,
|
||||||
|
val isReadOnly: Boolean,
|
||||||
|
val signerType: SignerType = SignerType.Internal,
|
||||||
|
) : AccountState()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Research Insights
|
||||||
|
|
||||||
|
**Existing pattern confirmed:** AccountState is observed via `accountManager.accountState.collectAsState()` in `App()` composable. Adding `ConnectingRelays` variant just needs a new `when` branch — zero refactoring needed.
|
||||||
|
|
||||||
|
**DesktopIAccount compatibility:** Uses `accountState.pubKeyHex` (not `signer.pubKey`), so `NostrSignerRemote` (where `signer.pubKey` = ephemeral key) works transparently. Verified in audit.
|
||||||
|
|
||||||
|
### Step 2: AccountManager — bunker login + persistence + heartbeat
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/account/AccountManager.kt`
|
||||||
|
|
||||||
|
#### `loginWithBunker(bunkerUri: String, client: INostrClient): Result<LoggedIn>`
|
||||||
|
|
||||||
|
1. Validate URI format: starts with `bunker://`, hex pubkey (64 chars), `?relay=wss://` param
|
||||||
|
2. Generate ephemeral `KeyPair()` + wrap in `NostrSignerInternal`
|
||||||
|
3. `NostrSignerRemote.fromBunkerUri(bunkerUri, ephemeralSigner, client)`
|
||||||
|
- Factory handles relay extraction + secret parsing
|
||||||
|
4. Call `remoteSigner.openSubscription()` — starts listening for responses
|
||||||
|
5. Call `remoteSigner.connect()` — sends BunkerRequestConnect, waits 30s
|
||||||
|
6. **Validate secret** if present: connect response must return matching secret value
|
||||||
|
7. On success: call `remoteSigner.switchRelays()` (new — NIP-46 compliance)
|
||||||
|
8. Set `AccountState.LoggedIn(signer=remoteSigner, pubKeyHex=remotePubkey, signerType=Remote(bunkerUri))`
|
||||||
|
|
||||||
|
#### Research Insights: Connect Flow Details
|
||||||
|
|
||||||
|
**From quartz audit — `connect()` internals:**
|
||||||
|
```
|
||||||
|
RemoteSignerManager.launchWaitAndParse():
|
||||||
|
1. Build BunkerRequestConnect(remoteKey, secret?, permissions?)
|
||||||
|
2. Create NostrConnectEvent (NIP-44 encrypted to remote signer)
|
||||||
|
3. client.send(event, relayList)
|
||||||
|
4. Store continuation[requestID] in LargeCache
|
||||||
|
5. tryAndWait(30s) — suspendCancellableCoroutine
|
||||||
|
6. On response: newResponse() → resume continuation
|
||||||
|
7. Parse via PubKeyResponse → SignerResult
|
||||||
|
```
|
||||||
|
|
||||||
|
**Exception mapping in NostrSignerRemote.convertExceptions():**
|
||||||
|
- `Successful` → return result
|
||||||
|
- `Rejected` → throw `ManuallyUnauthorizedException`
|
||||||
|
- `TimedOut` → throw `TimedOutException`
|
||||||
|
- `ReceivedButCouldNotPerform` → throw `CouldNotPerformException`
|
||||||
|
|
||||||
|
**Auth URL edge case:** If remote signer needs web confirmation (nsec.app pattern), it returns `auth_url` response. Quartz doesn't handle this natively — **document as known limitation for v1**. Most bunker signers (Amber, local nsecBunker) don't use auth_url.
|
||||||
|
|
||||||
|
#### `saveBunkerAccount(ephemeralPrivKeyHex: String)`
|
||||||
|
|
||||||
|
Persistence files:
|
||||||
|
- `~/.amethyst/bunker_uri.txt` — full bunker URI (contains relay URLs + remote pubkey)
|
||||||
|
- `SecureKeyStorage("bunker_ephemeral")` — ephemeral private key (OS keyring)
|
||||||
|
- `~/.amethyst/last_account.txt` — user's npub (existing pattern)
|
||||||
|
|
||||||
|
#### Research Insight: Session Persistence
|
||||||
|
|
||||||
|
**Confirmed from NIP-46 community patterns:**
|
||||||
|
> "Once a connection has been successfully established and the BunkerPointer is stored, you do not need to call `connect()` on subsequent sessions."
|
||||||
|
|
||||||
|
Save: ephemeral privkey + bunker URI + user npub. On restart, recreate `NostrSignerRemote` with saved ephemeral key — remote signer already trusts this keypair.
|
||||||
|
|
||||||
|
**What NOT to save:** Don't store the secret separately — it's only needed for initial connect handshake. The ephemeral keypair IS the session credential.
|
||||||
|
|
||||||
|
#### `loadSavedAccount(client: INostrClient): Result<LoggedIn>`
|
||||||
|
|
||||||
|
Updated flow:
|
||||||
|
1. Check `bunker_uri.txt` — if exists, this is a bunker account
|
||||||
|
2. Load ephemeral privkey from `SecureKeyStorage("bunker_ephemeral")`
|
||||||
|
3. Load npub from `last_account.txt` → derive pubKeyHex
|
||||||
|
4. Create `NostrSignerInternal(KeyPair(ephemeralPrivKey))`
|
||||||
|
5. `NostrSignerRemote.fromBunkerUri(savedUri, ephemeralSigner, client)`
|
||||||
|
6. `remoteSigner.openSubscription()` — start listening (no connect() needed)
|
||||||
|
7. Return `LoggedIn(signer=remoteSigner, pubKeyHex=savedPubKeyHex, signerType=Remote)`
|
||||||
|
8. Fall back to existing internal key flow if no bunker file
|
||||||
|
|
||||||
|
#### `logout(deleteKey: Boolean)`
|
||||||
|
|
||||||
|
Bunker-specific cleanup:
|
||||||
|
1. `remoteSigner.closeSubscription()` — stop relay filter
|
||||||
|
2. Stop heartbeat job
|
||||||
|
3. Delete `bunker_uri.txt`
|
||||||
|
4. Delete `SecureKeyStorage("bunker_ephemeral")`
|
||||||
|
5. Clear `last_account.txt`
|
||||||
|
6. Set `AccountState.LoggedOut`
|
||||||
|
|
||||||
|
#### Ping Heartbeat
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
private var heartbeatJob: Job? = null
|
||||||
|
private var consecutiveFailures = 0
|
||||||
|
|
||||||
|
fun startHeartbeat(scope: CoroutineScope) {
|
||||||
|
heartbeatJob = scope.launch {
|
||||||
|
while (isActive) {
|
||||||
|
delay(60_000)
|
||||||
|
val current = currentAccount() ?: continue
|
||||||
|
val remoteSigner = current.signer as? NostrSignerRemote ?: continue
|
||||||
|
try {
|
||||||
|
remoteSigner.ping()
|
||||||
|
consecutiveFailures = 0 // reset on success
|
||||||
|
} catch (e: SignerExceptions.TimedOutException) {
|
||||||
|
consecutiveFailures++
|
||||||
|
if (consecutiveFailures >= 3) {
|
||||||
|
forceLogoutWithReason("Lost connection to remote signer after 3 failed pings.")
|
||||||
|
}
|
||||||
|
} catch (e: SignerExceptions.ManuallyUnauthorizedException) {
|
||||||
|
forceLogoutWithReason("Remote signer revoked access.")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Research Insights: Heartbeat Patterns
|
||||||
|
|
||||||
|
**Structured concurrency:** Heartbeat should be a child of the app's `CoroutineScope(SupervisorJob() + Dispatchers.Main)` already created in `App()`. SupervisorJob ensures heartbeat failure doesn't crash the app.
|
||||||
|
|
||||||
|
**Existing pattern in codebase:** `RelayStat.pingInMs` tracks relay ping latency — similar concept. RelayConnectionManager handles relay disconnection with reconnect logic, but no heartbeat loop exists yet.
|
||||||
|
|
||||||
|
**Simple counter vs AtomicInteger:** Since heartbeat runs in a single coroutine (sequential `while` loop), a plain `var consecutiveFailures: Int` is safe — no concurrent access.
|
||||||
|
|
||||||
|
**Window minimization:** Desktop heartbeat should continue even when minimized — the remote signer session is independent of window focus. The `CoroutineScope` created with `remember` persists across recompositions.
|
||||||
|
|
||||||
|
**Force logout state:** Use `MutableStateFlow<String?>` for `forceLogoutReason` — it's a stateful value that the UI observes, not a one-shot event. Dialog displays the reason, user dismisses, flow cleared.
|
||||||
|
|
||||||
|
### Step 3: LoginCard — validation + connecting state
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/ui/auth/LoginCard.kt`
|
||||||
|
|
||||||
|
Add `LoginState` enum: `IDLE`, `CONNECTING`, `ERROR`
|
||||||
|
|
||||||
|
Add callback: `onLoginBunker: suspend (String) -> Result<Unit>`
|
||||||
|
|
||||||
|
**Bunker URI validation** (before attempting connect):
|
||||||
|
- Starts with `bunker://`
|
||||||
|
- Contains hex pubkey (64 hex chars after `bunker://`, before `?`)
|
||||||
|
- Contains at least one `relay=wss://...` param
|
||||||
|
- Show inline validation error if format wrong, don't attempt connection
|
||||||
|
- `secret=` is optional — do NOT require it
|
||||||
|
|
||||||
|
**UX when `bunker://` detected:**
|
||||||
|
- Button text: "Login" → "Connect to Signer"
|
||||||
|
- On click with valid URI: set `CONNECTING`, show `CircularProgressIndicator` + "Connecting to remote signer..."
|
||||||
|
- Disable input + all buttons while `CONNECTING`
|
||||||
|
- On success: caller handles transition
|
||||||
|
- On failure: show error message, return to `IDLE`, user can retry
|
||||||
|
|
||||||
|
#### Research Insights: UX Patterns
|
||||||
|
|
||||||
|
**From NIP-46 community feedback:**
|
||||||
|
> "Connection establishment takes a while to show to users" — always show clear progress indication.
|
||||||
|
|
||||||
|
**Pitfall discovered:** Users may not understand that bunker URI is a session credential:
|
||||||
|
> "Users might think the entire bunker URI is like a 2FA token and transmit it over untrusted messengers, and attackers can reuse the same URI"
|
||||||
|
|
||||||
|
Consider: add subtle help text below input when bunker:// detected: "This URI connects to your remote signer. Treat it like a password."
|
||||||
|
|
||||||
|
**Auto-detection approach:** Current `LoginCard` has a single `KeyInputField`. Detect `bunker://` prefix on input change — no new tabs/screens needed. Same pattern as NWC connection string input in settings.
|
||||||
|
|
||||||
|
### Step 4: LoginScreen + ConnectingRelays screen
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/ui/LoginScreen.kt`
|
||||||
|
|
||||||
|
Add parameter: `relayClient: INostrClient`
|
||||||
|
|
||||||
|
Wire `onLoginBunker` callback → `accountManager.loginWithBunker(bunkerUri, relayClient)` → on success: save + `onLoginSuccess()`
|
||||||
|
|
||||||
|
**ConnectingRelaysScreen composable** (in same file):
|
||||||
|
|
||||||
|
Shown when `AccountState.ConnectingRelays`:
|
||||||
|
```
|
||||||
|
[Amethyst title text]
|
||||||
|
"Connecting to relays..."
|
||||||
|
[CircularProgressIndicator]
|
||||||
|
"Restoring remote signer session"
|
||||||
|
```
|
||||||
|
|
||||||
|
Used on app restart when a bunker account is saved — shows while relays connect before recreating `NostrSignerRemote`.
|
||||||
|
|
||||||
|
#### Research Insight: Startup Sequence
|
||||||
|
|
||||||
|
**From Main.kt audit — current startup:**
|
||||||
|
```kotlin
|
||||||
|
DisposableEffect(Unit) {
|
||||||
|
scope.launch(Dispatchers.IO) { accountManager.loadSavedAccount() }
|
||||||
|
relayManager.addDefaultRelays()
|
||||||
|
relayManager.connect()
|
||||||
|
subscriptionsCoordinator.start()
|
||||||
|
onDispose { ... }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Problem:** `loadSavedAccount()` for bunker accounts needs `relayManager.client` — but relay connections happen asynchronously. Current code loads account independently of relay state.
|
||||||
|
|
||||||
|
**Solution:** For bunker accounts, wait for relay connection before recreating signer. The pattern `relayManager.connectedRelays.first { it.isNotEmpty() }` already exists in `MainContent` LaunchedEffect for DM subscriptions.
|
||||||
|
|
||||||
|
### Step 5: Main.kt — relay state, pass client, heartbeat
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/Main.kt`
|
||||||
|
|
||||||
|
**Updated startup flow:**
|
||||||
|
```kotlin
|
||||||
|
DisposableEffect(Unit) {
|
||||||
|
relayManager.addDefaultRelays()
|
||||||
|
relayManager.connect()
|
||||||
|
subscriptionsCoordinator.start()
|
||||||
|
|
||||||
|
scope.launch(Dispatchers.IO) {
|
||||||
|
if (accountManager.hasBunkerAccount()) {
|
||||||
|
accountManager.setConnectingRelays()
|
||||||
|
// Wait for relay connections — pattern from MainContent DM setup
|
||||||
|
relayManager.connectedRelays.first { it.isNotEmpty() }
|
||||||
|
accountManager.loadSavedAccount(relayManager.client)
|
||||||
|
accountManager.startHeartbeat(scope)
|
||||||
|
} else {
|
||||||
|
accountManager.loadSavedAccount(relayManager.client)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
onDispose {
|
||||||
|
accountManager.stopHeartbeat()
|
||||||
|
subscriptionsCoordinator.clear()
|
||||||
|
relayManager.disconnect()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Account state routing — add ConnectingRelays + ForceLogout:**
|
||||||
|
```kotlin
|
||||||
|
when (accountState) {
|
||||||
|
is AccountState.LoggedOut -> LoginScreen(accountManager, relayManager.client, onLoginSuccess)
|
||||||
|
is AccountState.ConnectingRelays -> ConnectingRelaysScreen()
|
||||||
|
is AccountState.LoggedIn -> MainContent(...)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Force logout overlay
|
||||||
|
val forceLogoutReason by accountManager.forceLogoutReason.collectAsState()
|
||||||
|
forceLogoutReason?.let { reason ->
|
||||||
|
ForceLogoutDialog(reason = reason, onDismiss = { accountManager.clearForceLogoutReason() })
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Research Insight: Scope Lifecycle
|
||||||
|
|
||||||
|
**Current app scope:** `remember { CoroutineScope(SupervisorJob() + Dispatchers.Main) }` — created in `App()` composable, lives for the duration of the window. Heartbeat as a child of this scope is correct — it auto-cancels when app closes.
|
||||||
|
|
||||||
|
**onDispose cleanup:** Add `accountManager.stopHeartbeat()` to the existing `onDispose` block alongside `subscriptionsCoordinator.clear()` and `relayManager.disconnect()`.
|
||||||
|
|
||||||
|
### Step 6: ForceLogoutDialog
|
||||||
|
|
||||||
|
**File:** `desktopApp/src/jvmMain/.../desktop/ui/auth/ForceLogoutDialog.kt` (new)
|
||||||
|
|
||||||
|
Simple `AlertDialog`:
|
||||||
|
- Title: "Session Terminated"
|
||||||
|
- Body: reason string
|
||||||
|
- Single "OK" button → dismiss → login screen already showing (state is LoggedOut)
|
||||||
|
|
||||||
|
## Error Handling
|
||||||
|
|
||||||
|
| Exception | Source | User Message |
|
||||||
|
|-----------|--------|-------------|
|
||||||
|
| URI validation fail | LoginCard | "Invalid bunker URI. Expected: bunker://\<hex-pubkey\>?relay=wss://..." |
|
||||||
|
| `TimedOutException` | `connect()` | "Connection timed out. Ensure remote signer is online and has approved the connection." |
|
||||||
|
| `ManuallyUnauthorizedException` | `connect()` | "Connection rejected by remote signer." |
|
||||||
|
| `CouldNotPerformException` | `connect()` | "Remote signer error: {message}" |
|
||||||
|
| Generic exception | `connect()` | "Connection failed: {message}" |
|
||||||
|
| Heartbeat 3x timeout | `ping()` | Force logout: "Lost connection to remote signer after 3 failed pings." |
|
||||||
|
| Heartbeat revocation | `ping()` | Force logout: "Remote signer revoked access." |
|
||||||
|
| `decryptZapEvent` | Runtime | Known limitation — zap decryption not supported with remote signer |
|
||||||
|
|
||||||
|
### Research Insight: Relay-Specific Issues
|
||||||
|
|
||||||
|
**PoW requirement:** Some relays (nostr.mom, nos.lol, damus) rate-limit or require Proof of Work for kind 24133 events. If users specify these relays in their bunker URI, connections may fail. **Mitigation:** Document in error message. No code change needed — the relay list comes from the bunker URI (remote signer's choice).
|
||||||
|
|
||||||
|
## Files Modified
|
||||||
|
|
||||||
|
| File | Changes |
|
||||||
|
|------|---------|
|
||||||
|
| `AccountManager.kt` | SignerType, ConnectingRelays state, loginWithBunker(), saveBunkerAccount(), loadSavedAccount() updated, logout() updated, heartbeat, forceLogout |
|
||||||
|
| `LoginCard.kt` | LoginState enum, bunker URI validation, onLoginBunker callback, connecting UI, help text |
|
||||||
|
| `LoginScreen.kt` | Add relayClient param, wire bunker login, ConnectingRelaysScreen composable |
|
||||||
|
| `Main.kt` | Startup relay-wait flow, pass relayManager.client, heartbeat lifecycle, forceLogout dialog |
|
||||||
|
| `ForceLogoutDialog.kt` | **New** — AlertDialog for session termination |
|
||||||
|
|
||||||
|
## Reused As-Is (no changes needed — verified)
|
||||||
|
|
||||||
|
| File | What | Verified |
|
||||||
|
|------|------|----------|
|
||||||
|
| `NostrSignerRemote.kt` | Full NIP-46 client: connect, sign, encrypt, decrypt, ping | ✅ All methods present |
|
||||||
|
| `RemoteSignerManager.kt` | Request/response lifecycle with timeout | ✅ 30s configurable |
|
||||||
|
| `NostrConnectEvent.kt` | Kind 24133 wrapper with NIP-44 encryption | ✅ |
|
||||||
|
| `SignerExceptions.kt` | TimedOut, ManuallyUnauthorized, CouldNotPerform | ✅ |
|
||||||
|
| `DesktopIAccount.kt` | Uses `accountState.pubKeyHex` not `signer.pubKey` | ✅ Safe with remote signer |
|
||||||
|
| `RelayConnectionManager.kt` | Exposes `client: INostrClient` | ✅ |
|
||||||
|
| `SecureKeyStorage.kt` | OS keyring + encrypted fallback | ✅ Reuse for ephemeral key |
|
||||||
|
|
||||||
|
## Security Considerations
|
||||||
|
|
||||||
|
### Ephemeral Key = Session Credential
|
||||||
|
- Stored in OS keyring (macOS Keychain / Windows Credential Manager) via existing `SecureKeyStorage`
|
||||||
|
- If compromised: attacker can impersonate user's signing requests until remote signer revokes
|
||||||
|
- **Mitigation:** Remote signer can revoke anytime; heartbeat detects revocation
|
||||||
|
|
||||||
|
### Bunker URI as Session Token
|
||||||
|
- URI contains remote pubkey + relay URLs — NOT the user's private key
|
||||||
|
- But it enables initial connection establishment
|
||||||
|
- Once connected, the ephemeral keypair is the actual session credential
|
||||||
|
- **Secret parameter** (when present): only used once during initial connect. Not stored separately.
|
||||||
|
|
||||||
|
### Relay MITM
|
||||||
|
- NIP-46 messages are NIP-44 encrypted end-to-end between ephemeral key and remote signer
|
||||||
|
- Relay can see metadata (who's talking to whom) but NOT message content
|
||||||
|
- **No MITM on message content** — encryption is between known pubkeys
|
||||||
|
|
||||||
|
### Force Logout on Revocation
|
||||||
|
- `ManuallyUnauthorizedException` from ping → immediate logout + file cleanup
|
||||||
|
- 3 consecutive ping timeouts → logout (remote signer may be permanently offline)
|
||||||
|
- All bunker files deleted on force logout — clean slate
|
||||||
|
|
||||||
|
## Known Limitations (v1)
|
||||||
|
|
||||||
|
1. **Auth URL flow** — nsec.app may return `auth_url` response requiring web confirmation. Not handled in v1. Most signers (Amber, local bunkers) don't use this.
|
||||||
|
2. **`decryptZapEvent()`** — Not implemented in quartz's `NostrSignerRemote`. Zap decryption will fail for bunker accounts.
|
||||||
|
3. **`deriveKey()`** — Not implemented. NIP-06 key derivation won't work for bunker accounts.
|
||||||
|
4. **Single account** — Only one bunker OR one internal account at a time. Multi-account is a separate PR.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
1. `./gradlew :desktopApp:compileKotlin` — builds
|
||||||
|
2. `./gradlew :desktopApp:run` — test flows:
|
||||||
|
- **Invalid URI** → paste `bunker://bad` → inline validation error, no network call
|
||||||
|
- **Valid format, missing relay** → paste `bunker://<64hex>` → validation error about missing relay
|
||||||
|
- **Login** → paste valid bunker URI → "Connecting..." → approve on phone → main screen
|
||||||
|
- **Timeout** → paste URI, don't approve → 30s → timeout error, can retry
|
||||||
|
- **Rejection** → paste URI, reject on phone → "rejected" error
|
||||||
|
- **Restart** → close app → relaunch → "Connecting to relays..." → auto-login (no re-approval needed)
|
||||||
|
- **Heartbeat** → revoke access on remote signer → force logout dialog within 60s
|
||||||
|
- **Logout** → logout → bunker files deleted → login screen
|
||||||
|
- **Post note** → compose + send → signed via remote signer transparently
|
||||||
|
- **Send DM** → NIP-17 gift wrap → encrypted via remote signer
|
||||||
|
3. `./gradlew spotlessApply` before commit
|
||||||
|
|
||||||
|
## Answered Questions
|
||||||
|
|
||||||
|
1. **Both `bunker://` AND `nostrconnect://`** — see Step 7 below
|
||||||
|
2. **60s hardcoded** heartbeat interval
|
||||||
|
3. **Sidebar heartbeat icon** — tiny pulsing dot with tooltip, see Step 8 below
|
||||||
|
4. **Yes, persist relay updates** from `switch_relays` to `bunker_uri.txt`
|
||||||
|
5. **No auto-retry** — show error immediately, user clicks to retry
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Step 7: nostrconnect:// (Client-Initiated) Login
|
||||||
|
|
||||||
|
### Protocol Difference
|
||||||
|
|
||||||
|
| Aspect | `bunker://` (remote-signer-initiated) | `nostrconnect://` (client-initiated) |
|
||||||
|
|--------|---------------------------------------|--------------------------------------|
|
||||||
|
| Who generates URI | Remote signer (nsec.app, Amber) | Desktop client |
|
||||||
|
| Who scans/pastes | Desktop user pastes into app | User pastes into their signer app |
|
||||||
|
| Secret | Optional | **Required** (client generates) |
|
||||||
|
| Permissions | Not specified in URI | Client requests via `perms=` param |
|
||||||
|
| Flow | User already has URI → paste → connect | Client shows URI → user takes to signer → signer connects back |
|
||||||
|
|
||||||
|
### nostrconnect:// URI Format (from NIP-46 spec)
|
||||||
|
|
||||||
|
```
|
||||||
|
nostrconnect://<client-pubkey>?relay=wss://relay1.example.com&relay=wss://relay2.example.com&secret=<random-string>&perms=nip44_encrypt,nip44_decrypt,sign_event&name=Amethyst+Desktop&url=https://github.com/vitorpamplona/amethyst&image=<app-icon-url>
|
||||||
|
```
|
||||||
|
|
||||||
|
**Parameters:**
|
||||||
|
- `<client-pubkey>` — ephemeral pubkey generated by desktop (64 hex chars)
|
||||||
|
- `relay=` — relay URLs for communication (use app's connected relays)
|
||||||
|
- `secret=` — **REQUIRED** random string, client validates signer returns it in connect response
|
||||||
|
- `perms=` — comma-separated permissions to request
|
||||||
|
- `name=` — app display name ("Amethyst Desktop")
|
||||||
|
- `url=` — app URL (optional)
|
||||||
|
- `image=` — app icon URL (optional)
|
||||||
|
|
||||||
|
### UX Flow
|
||||||
|
|
||||||
|
1. User clicks **"Connect with Signer"** tab/button on login screen
|
||||||
|
2. Desktop generates ephemeral `KeyPair()` and random `secret`
|
||||||
|
3. Builds `nostrconnect://` URI with connected relay URLs + perms + secret
|
||||||
|
4. Displays:
|
||||||
|
- **QR code** (primary — user scans with phone signer)
|
||||||
|
- **Copyable text** (fallback — user pastes into web signer like nsec.app)
|
||||||
|
- **"Waiting for signer to connect..."** status with spinner
|
||||||
|
5. Desktop subscribes to kind 24133 events on specified relays, filtered to ephemeral pubkey
|
||||||
|
6. User scans QR / pastes URI into their signer app
|
||||||
|
7. Signer sends `connect` response with the secret value
|
||||||
|
8. Desktop validates secret matches → `AccountState.LoggedIn`
|
||||||
|
9. If no response within **120s** (longer than bunker — user needs time to open phone): show timeout, allow retry
|
||||||
|
|
||||||
|
### Implementation Details
|
||||||
|
|
||||||
|
**LoginCard changes:**
|
||||||
|
- Add toggle/tab: "Paste Key" | "Connect with Signer"
|
||||||
|
- "Connect with Signer" tab shows QR + copyable URI + waiting state
|
||||||
|
- State machine: `IDLE → GENERATING → WAITING_FOR_SIGNER → CONNECTED / ERROR`
|
||||||
|
|
||||||
|
**AccountManager.loginWithNostrConnect(client: INostrClient): Pair<String, Job>**
|
||||||
|
1. Generate ephemeral `KeyPair()` + `NostrSignerInternal`
|
||||||
|
2. Generate random secret (16 hex chars)
|
||||||
|
3. Get connected relay URLs from relay manager (+ add `wss://relay.nsec.app` as fallback NIP-46 relay)
|
||||||
|
4. Build `nostrconnect://` URI string with `name=Amethyst+Desktop&perms=sign_event,nip44_encrypt,nip44_decrypt`
|
||||||
|
5. Subscribe to kind 24133 events on specified relays, filtered to ephemeral pubkey
|
||||||
|
6. Return (URI string, waiting Job)
|
||||||
|
7. When connect response received: validate secret, extract remote pubkey from response, create `NostrSignerRemote`
|
||||||
|
8. Set LoggedIn
|
||||||
|
|
||||||
|
**Key difference from bunker://:** With nostrconnect, client doesn't know user's pubkey upfront — learns it from signer's connect response. `fromBunkerUri()` cannot be reused directly — need to construct `NostrSignerRemote` manually after learning the remote pubkey.
|
||||||
|
|
||||||
|
**Quartz gap:** `RemoteSignerManager.launchWaitAndParse()` sends a request AND waits. For nostrconnect, client needs to just WAIT (signer initiates). Options:
|
||||||
|
- Build a `waitForConnect()` method in AccountManager that subscribes + waits
|
||||||
|
- Or call `openSubscription()` manually, then wait for the `newResponse()` callback
|
||||||
|
|
||||||
|
**Timeout:** 120s for nostrconnect (user needs to open phone, find app, scan QR). No auto-retry.
|
||||||
|
|
||||||
|
### QR Code Generation
|
||||||
|
|
||||||
|
**ZXing `core` 3.5.4 already in `gradle/libs.versions.toml`!** And there's an existing `QrCodeDrawer.kt` in the Android app (`amethyst/src/main/java/.../ui/screen/loggedIn/qrcode/QrCodeDrawer.kt`) that uses:
|
||||||
|
- `com.google.zxing.qrcode.encoder.Encoder` (pure Java, in `zxing:core`)
|
||||||
|
- Compose Canvas APIs (multiplatform)
|
||||||
|
- **Zero Android dependencies** — fully portable to desktop
|
||||||
|
|
||||||
|
**Action:** Extract `QrCodeDrawer.kt` to `commons/commonMain/` and add `libs.zxing` dependency to `commons` module. No new libraries needed.
|
||||||
|
|
||||||
|
**Gradle addition (commons module):**
|
||||||
|
```kotlin
|
||||||
|
// commons/build.gradle.kts - commonMain sourceSets
|
||||||
|
implementation(libs.zxing) // already in version catalog
|
||||||
|
```
|
||||||
|
|
||||||
|
### Persistence
|
||||||
|
|
||||||
|
Same as bunker:// — save ephemeral key + URI + npub. On restart, reconnect without needing QR again (remote signer remembers ephemeral key).
|
||||||
|
|
||||||
|
### File Changes
|
||||||
|
|
||||||
|
| File | nostrconnect:// additions |
|
||||||
|
|------|--------------------------|
|
||||||
|
| `AccountManager.kt` | `loginWithNostrConnect()` method, generates URI + waits for connect |
|
||||||
|
| `LoginCard.kt` | Tab toggle "Paste Key" / "Connect with Signer", QR display, waiting state |
|
||||||
|
| `LoginScreen.kt` | Wire nostrconnect flow, pass relay client |
|
||||||
|
| `build.gradle.kts` | Add `qrcode-kotlin-jvm` dependency |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Step 8: Heartbeat Status Indicator (Sidebar)
|
||||||
|
|
||||||
|
### Design
|
||||||
|
|
||||||
|
Tiny pulsing dot in the sidebar (below existing nav icons, above settings gear). Communicates remote signer connection health at a glance.
|
||||||
|
|
||||||
|
### States
|
||||||
|
|
||||||
|
| State | Visual | Tooltip |
|
||||||
|
|-------|--------|---------|
|
||||||
|
| **Connected** | Green dot, gentle pulse animation (1.5s cycle) | "Remote signer connected" |
|
||||||
|
| **Ping failed (1-2x)** | Yellow dot, faster pulse (0.8s cycle) | "Remote signer: connection unstable" |
|
||||||
|
| **Disconnected** | Red dot, no pulse (static) | "Remote signer: disconnected" |
|
||||||
|
| **Not remote** | Hidden | — (only show for bunker/nostrconnect accounts) |
|
||||||
|
|
||||||
|
### Animation Pattern — Double-Beat Heartbeat
|
||||||
|
|
||||||
|
Uses `keyframes` for organic lub-dub feel (two peaks per cycle):
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
val infiniteTransition = rememberInfiniteTransition(label = "heartbeat")
|
||||||
|
val scale by infiniteTransition.animateFloat(
|
||||||
|
initialValue = 0.85f,
|
||||||
|
targetValue = 1.15f,
|
||||||
|
animationSpec = infiniteRepeatable(
|
||||||
|
animation = keyframes {
|
||||||
|
durationMillis = 1200
|
||||||
|
0.85f at 0 // rest
|
||||||
|
1.15f at 300 using EaseOut // first beat (systole)
|
||||||
|
0.95f at 500 // contract
|
||||||
|
1.05f at 700 using EaseInOut // second beat (diastole)
|
||||||
|
0.85f at 1200 // rest
|
||||||
|
},
|
||||||
|
repeatMode = RepeatMode.Restart,
|
||||||
|
),
|
||||||
|
label = "pulseScale",
|
||||||
|
)
|
||||||
|
|
||||||
|
// Apply via graphicsLayer on a small Box with CircleShape
|
||||||
|
Box(
|
||||||
|
Modifier
|
||||||
|
.size(8.dp)
|
||||||
|
.graphicsLayer { scaleX = scale; scaleY = scale }
|
||||||
|
.background(color, CircleShape)
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Existing animation patterns in codebase:** `ProfileBroadcastBanner.kt` uses `animateFloatAsState` with `tween(300)` for progress bars. No `rememberInfiniteTransition` usage yet — this would be the first.
|
||||||
|
|
||||||
|
### Tooltip (Desktop-only API)
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@OptIn(ExperimentalFoundationApi::class)
|
||||||
|
TooltipArea(
|
||||||
|
tooltip = {
|
||||||
|
Surface(
|
||||||
|
modifier = Modifier.shadow(4.dp),
|
||||||
|
color = MaterialTheme.colorScheme.surfaceVariant,
|
||||||
|
shape = RoundedCornerShape(4.dp),
|
||||||
|
) {
|
||||||
|
Text(tooltipText, modifier = Modifier.padding(8.dp), style = MaterialTheme.typography.bodySmall)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
delayMillis = 400,
|
||||||
|
tooltipPlacement = TooltipPlacement.CursorPoint(alignment = Alignment.BottomEnd),
|
||||||
|
) {
|
||||||
|
HeartbeatDot(connectionState)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Connection State Flow
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
// In AccountManager
|
||||||
|
sealed class SignerConnectionState {
|
||||||
|
data object NotRemote : SignerConnectionState() // internal signer, hide indicator
|
||||||
|
data object Connected : SignerConnectionState()
|
||||||
|
data class Unstable(val failCount: Int) : SignerConnectionState()
|
||||||
|
data object Disconnected : SignerConnectionState()
|
||||||
|
}
|
||||||
|
|
||||||
|
val signerConnectionState: StateFlow<SignerConnectionState>
|
||||||
|
```
|
||||||
|
|
||||||
|
Heartbeat updates this state on each ping result. UI observes via `collectAsState()`.
|
||||||
|
|
||||||
|
### File Changes
|
||||||
|
|
||||||
|
| File | Heartbeat indicator additions |
|
||||||
|
|------|------------------------------|
|
||||||
|
| `AccountManager.kt` | `SignerConnectionState` sealed class + `signerConnectionState: StateFlow` |
|
||||||
|
| `DeckSidebar.kt` | Add `HeartbeatIndicator` composable at bottom of sidebar |
|
||||||
|
| `SinglePaneLayout.kt` | Add `HeartbeatIndicator` at bottom of nav rail |
|
||||||
|
| `HeartbeatIndicator.kt` | **New** — pulsing dot + tooltip composable |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Updated Files Summary
|
||||||
|
|
||||||
|
| File | Changes |
|
||||||
|
|------|---------|
|
||||||
|
| `AccountManager.kt` | SignerType, ConnectingRelays, loginWithBunker(), loginWithNostrConnect(), saveBunkerAccount(), loadSavedAccount(), logout(), heartbeat, forceLogout, SignerConnectionState |
|
||||||
|
| `LoginCard.kt` | LoginState, tab toggle (paste/connect), bunker URI validation, nostrconnect QR + waiting, connecting UI |
|
||||||
|
| `LoginScreen.kt` | relayClient param, wire bunker + nostrconnect flows, ConnectingRelaysScreen |
|
||||||
|
| `Main.kt` | Startup relay-wait, heartbeat lifecycle, forceLogout dialog, pass relayManager.client |
|
||||||
|
| `ForceLogoutDialog.kt` | **New** — AlertDialog for session termination |
|
||||||
|
| `HeartbeatIndicator.kt` | **New** — pulsing dot + tooltip composable |
|
||||||
|
| `DeckSidebar.kt` | Add HeartbeatIndicator between spacer and Settings |
|
||||||
|
| `SinglePaneLayout.kt` | Add HeartbeatIndicator in nav rail |
|
||||||
|
| `QrCodeDrawer.kt` | **Extract** from `amethyst/.../qrcode/` → `commons/commonMain/` (zero Android deps) |
|
||||||
|
| `commons/build.gradle.kts` | Add `libs.zxing` (core) dependency |
|
||||||
|
|
||||||
|
## Answered Questions (Round 2)
|
||||||
|
|
||||||
|
1. **Full perms** for nostrconnect: `sign_event,nip04_encrypt,nip04_decrypt,nip44_encrypt,nip44_decrypt`
|
||||||
|
2. ~~Dark mode QR~~ — defer to implementation, follow existing theme
|
||||||
|
3. **Both layouts** — heartbeat in SINGLE_PANE nav rail AND DECK sidebar
|
||||||
@@ -0,0 +1,106 @@
|
|||||||
|
---
|
||||||
|
title: "feat: NIP-46 remote signer improvements (post-MVP)"
|
||||||
|
type: feat
|
||||||
|
status: backlog
|
||||||
|
date: 2026-03-05
|
||||||
|
origin: docs/plans/2026-03-05-fix-nip46-relay-isolation-and-init-plan.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# feat: NIP-46 remote signer improvements (post-MVP)
|
||||||
|
|
||||||
|
Deferred items from the NIP-46 relay isolation fix. All are nice-to-haves discovered during deepening/review — none block the initial fix.
|
||||||
|
|
||||||
|
## 1. `auth_url` handling (nsec.app compatibility)
|
||||||
|
|
||||||
|
**Priority:** Medium — only affects nsec.app users, not Amber
|
||||||
|
|
||||||
|
NIP-46 spec defines `auth_url` for signers needing out-of-band user confirmation:
|
||||||
|
```json
|
||||||
|
{"id": "<req_id>", "result": "auth_url", "error": "<URL_to_display>"}
|
||||||
|
```
|
||||||
|
|
||||||
|
Client should open the URL, then resubscribe for the actual result using the same request ID.
|
||||||
|
|
||||||
|
**Current behavior:** Treated as `ReceivedButCouldNotPerform` error → times out.
|
||||||
|
|
||||||
|
**To implement:**
|
||||||
|
- New `BunkerResponseAuthUrl` class + parser
|
||||||
|
- New `SignerResult` state for pending auth
|
||||||
|
- Desktop UI to open URL in browser + resubscribe
|
||||||
|
- Timeout management for multi-step flow
|
||||||
|
|
||||||
|
**Files:** `BunkerResponse.kt`, `SignerResult.kt`, response parsers, `RemoteSignerManager.kt`, desktop `LoginCard.kt`
|
||||||
|
|
||||||
|
## 2. `get_public_key` validation after connect
|
||||||
|
|
||||||
|
**Priority:** Low — security hardening, not correctness
|
||||||
|
|
||||||
|
Infrastructure already exists: `NostrSignerRemote.getPublicKey()` is fully implemented and production-ready. `connect()` already returns the user pubkey, so this is redundant for normal operation.
|
||||||
|
|
||||||
|
**Value:** Belt-and-suspenders against MITM on the connect response. Low risk since NIP-44 encryption protects the channel.
|
||||||
|
|
||||||
|
**To implement:**
|
||||||
|
```kotlin
|
||||||
|
// In AccountManager.loadBunkerAccount() after remoteSigner.openSubscription()
|
||||||
|
val confirmedPubkey = remoteSigner.getPublicKey()
|
||||||
|
if (confirmedPubkey != expectedPubkey) {
|
||||||
|
return Result.failure(Exception("Pubkey mismatch: signer returned $confirmedPubkey"))
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Cost:** 1 extra RPC round-trip + potential 30s timeout on failure.
|
||||||
|
|
||||||
|
## 3. Dynamic `since` filter on reconnect
|
||||||
|
|
||||||
|
**Priority:** Low — optimization, no correctness issue
|
||||||
|
|
||||||
|
`NostrClientStaticReq` re-sends the exact same filter on WebSocket reconnect. With `since = now() - 60`, this means after a reconnect the relay replays events from the original subscription time — wasteful but harmless (request ID matching discards stale responses).
|
||||||
|
|
||||||
|
**To implement:** Switch `NostrSignerRemote` from `NostrClientStaticReq` to `NostrClientDynamicReq`:
|
||||||
|
```kotlin
|
||||||
|
// Lambda recomputes since on each reconnect
|
||||||
|
val subscription = client.dynamicReq(
|
||||||
|
relays = relays.toList(),
|
||||||
|
filter = {
|
||||||
|
Filter(
|
||||||
|
kinds = listOf(NostrConnectEvent.KIND),
|
||||||
|
tags = mapOf("p" to listOf(signer.pubKey)),
|
||||||
|
since = TimeUtils.now() - 60,
|
||||||
|
)
|
||||||
|
},
|
||||||
|
) { event -> ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tradeoff:** More complexity. Only matters if relay operators complain about repeated full-history queries or reconnects are frequent.
|
||||||
|
|
||||||
|
## 4. `NostrClient.close()` method (upstream)
|
||||||
|
|
||||||
|
**Priority:** Medium — affects any code creating temporary `NostrClient` instances
|
||||||
|
|
||||||
|
`NostrClient.disconnect()` only closes WebSockets but leaks:
|
||||||
|
- `allRelays` combine flow (`stateIn(Eagerly)` on `scope`)
|
||||||
|
- `debouncingConnection` debounce flow (`stateIn(Eagerly)` on `scope`)
|
||||||
|
- The `SupervisorJob` scope itself
|
||||||
|
|
||||||
|
**To implement:**
|
||||||
|
```kotlin
|
||||||
|
// In NostrClient.kt
|
||||||
|
fun close() {
|
||||||
|
disconnect()
|
||||||
|
scope.cancel()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Current workaround:** `disconnectNip46Client()` in AccountManager handles our specific case. But any future code creating temporary `NostrClient` instances will hit the same leak.
|
||||||
|
|
||||||
|
**Recommendation:** PR upstream to quartz adding `close()` to `NostrClient`.
|
||||||
|
|
||||||
|
## 5. `FeedMetadataCoordinator` reactive relay sets
|
||||||
|
|
||||||
|
**Priority:** Low — deferred construction in the fix plan solves the immediate bug
|
||||||
|
|
||||||
|
Currently `indexRelays` is `private val` — immutable after construction. If relay sets change at runtime (user adds/removes relays), the coordinator won't pick up changes.
|
||||||
|
|
||||||
|
**To implement:** Accept `StateFlow<Set<NormalizedRelayUrl>>` instead of `Set<NormalizedRelayUrl>`, collect in coordinator's scope.
|
||||||
|
|
||||||
|
**Current workaround:** Nullable coordinator pattern with lazy construction after relays connect.
|
||||||
@@ -0,0 +1,386 @@
|
|||||||
|
---
|
||||||
|
title: "test: TDD tests for NIP-46 relay isolation fix"
|
||||||
|
type: test
|
||||||
|
status: active
|
||||||
|
date: 2026-03-05
|
||||||
|
parent: 2026-03-05-fix-nip46-relay-isolation-and-init-plan.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# test: TDD tests for NIP-46 relay isolation fix
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
Write tests that **reproduce the three NIP-46 bugs** before they're fixed, then verify the fix. Tests should fail against the old shared-client architecture and pass after relay isolation.
|
||||||
|
|
||||||
|
## Bug → Test Mapping
|
||||||
|
|
||||||
|
| Bug | Root Cause | Test Strategy |
|
||||||
|
|-----|-----------|---------------|
|
||||||
|
| **Bug 1:** Empty `indexRelays` snapshot | `availableRelays.value` captured at coordinator construction = `emptySet()` | Verify coordinator receives non-empty relay set |
|
||||||
|
| **Bug 2:** Shared client relay contamination | NIP-46 relay leaks into general pool → ClosedMessage loop | Verify NIP-46 traffic stays on dedicated client, never touches general client |
|
||||||
|
| **Bug 3:** Sign request response lost | Subscription disrupted by ClosedMessage churn from Bug 2 | Verify NIP-46 subscription filters only target NIP-46 relays |
|
||||||
|
|
||||||
|
## Test Infrastructure
|
||||||
|
|
||||||
|
### Existing
|
||||||
|
- Framework: `kotlin.test` + `kotlinx-coroutines-test` + `mockk`
|
||||||
|
- Location: `desktopApp/src/jvmTest/kotlin/.../desktop/account/`
|
||||||
|
- Mocks: `EmptyNostrClient` (no-op), `mockk<SecureKeyStorage>(relaxed = true)`, temp dirs
|
||||||
|
- Pattern: `@BeforeTest` setup → `runTest {}` → `@AfterTest` cleanup
|
||||||
|
|
||||||
|
### Needed
|
||||||
|
- `SpyNostrClient` — wraps `INostrClient`, records which relays receive `send()` and `openReqSubscription()` calls
|
||||||
|
- No new dependencies (mockk `capture` + `slot` + `verify` sufficient)
|
||||||
|
|
||||||
|
## Phase 0: Fix Existing Broken Tests
|
||||||
|
|
||||||
|
**File:** `AccountManagerLoadAccountTest.kt`
|
||||||
|
|
||||||
|
Three tests pass `client` param to `loadSavedAccount()` which no longer accepts it:
|
||||||
|
|
||||||
|
| Test | Fix |
|
||||||
|
|------|-----|
|
||||||
|
| `loadSavedAccountBunkerNoEphemeralReturnsFailure` (line 114) | Remove `client = EmptyNostrClient` arg |
|
||||||
|
| `loadSavedAccountBunkerNoClientFallsBackToInternal` (line 119-136) | **Delete entirely** — concept no longer exists (AccountManager always creates its own NIP-46 client) |
|
||||||
|
| `loadSavedAccountBunkerSuccess` (line 155-158) | Remove `client = EmptyNostrClient` arg |
|
||||||
|
|
||||||
|
## Phase 1: Relay Isolation Tests (Bug 2 + 3)
|
||||||
|
|
||||||
|
### Test File: `AccountManagerNip46IsolationTest.kt`
|
||||||
|
|
||||||
|
**Location:** `desktopApp/src/jvmTest/kotlin/.../desktop/account/`
|
||||||
|
|
||||||
|
#### Test 1: `nip46ClientIsNotTheGeneralClient`
|
||||||
|
|
||||||
|
**Reproduces:** Bug 2 root cause — shared `NostrClient` pollution.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun nip46ClientIsNotTheGeneralClient() = runTest {
|
||||||
|
// Setup: bunker URI + ephemeral key in storage
|
||||||
|
val validHex = "a".repeat(64)
|
||||||
|
val ephemeralKeyPair = KeyPair()
|
||||||
|
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://relay.nsec.app")
|
||||||
|
File(amethystDir, "last_account.txt").writeText(ephemeralKeyPair.pubKey.toNpub())
|
||||||
|
coEvery { storage.getPrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS) } returns ephemeralKeyPair.privKey!!.toHexKey()
|
||||||
|
|
||||||
|
// Load bunker account — this creates the internal NIP-46 client
|
||||||
|
manager.loadSavedAccount()
|
||||||
|
|
||||||
|
// The general relay pool client (e.g. relayManager.client) should NOT
|
||||||
|
// have any NIP-46 relay subscriptions. Since AccountManager creates its
|
||||||
|
// own client internally, we verify via the accountState having Remote signer.
|
||||||
|
val state = manager.currentAccount()
|
||||||
|
assertNotNull(state)
|
||||||
|
assertIs<SignerType.Remote>(state.signerType)
|
||||||
|
|
||||||
|
// The signer's client should be the dedicated one, not EmptyNostrClient
|
||||||
|
val signer = state.signer as NostrSignerRemote
|
||||||
|
// NostrSignerRemote stores its client — it should be a real NostrClient, not null
|
||||||
|
assertNotNull(signer)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Key assertion:** AccountManager creates a dedicated client internally rather than requiring one passed in.
|
||||||
|
|
||||||
|
#### Test 2: `loginWithBunkerDoesNotRequireExternalClient`
|
||||||
|
|
||||||
|
**Reproduces:** The old API required callers to pass `relayManager.client`.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun loginWithBunkerDoesNotRequireExternalClient() = runTest {
|
||||||
|
// loginWithBunker() should compile and work with just a bunkerUri
|
||||||
|
// This test verifies the API — no client param needed
|
||||||
|
try {
|
||||||
|
manager.loginWithBunker("bunker://${"a".repeat(64)}?relay=wss://relay.nsec.app")
|
||||||
|
} catch (e: Exception) {
|
||||||
|
// Expected — no real relay to connect to. But it should NOT throw
|
||||||
|
// a compile error or "client required" error.
|
||||||
|
assertTrue(
|
||||||
|
e.message?.contains("Connection") == true ||
|
||||||
|
e.message?.contains("timed out") == true ||
|
||||||
|
e.message?.contains("Connection failed") == true,
|
||||||
|
"Unexpected error: ${e.message}",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 3: `loginWithNostrConnectDoesNotRequireExternalClient`
|
||||||
|
|
||||||
|
Same pattern — verifies API no longer takes `client`.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun loginWithNostrConnectDoesNotRequireExternalClient() = runTest {
|
||||||
|
var generatedUri: String? = null
|
||||||
|
try {
|
||||||
|
manager.loginWithNostrConnect { uri -> generatedUri = uri }
|
||||||
|
} catch (e: Exception) {
|
||||||
|
// Expected — no signer scanning the QR
|
||||||
|
assertTrue(e.message?.contains("Timed out") == true || e.message != null)
|
||||||
|
}
|
||||||
|
// URI should have been generated even if connection failed
|
||||||
|
assertNotNull(generatedUri)
|
||||||
|
assertTrue(generatedUri!!.startsWith("nostrconnect://"))
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 4: `logoutDisconnectsNip46Client`
|
||||||
|
|
||||||
|
**Reproduces:** Bug 2 side effect — leaked NIP-46 WebSocket after logout.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun logoutDisconnectsNip46Client() = runTest {
|
||||||
|
// Login with bunker (will create internal NIP-46 client)
|
||||||
|
val keyPair = KeyPair()
|
||||||
|
manager.loginWithKey(keyPair.privKey!!.toNsec())
|
||||||
|
// Force to Remote type for test
|
||||||
|
// Instead: test that logout from any state doesn't crash
|
||||||
|
manager.logout()
|
||||||
|
|
||||||
|
// After logout, state should be LoggedOut
|
||||||
|
assertIs<AccountState.LoggedOut>(manager.accountState.value)
|
||||||
|
// Signer connection should be NotRemote
|
||||||
|
assertIs<SignerConnectionState.NotRemote>(manager.signerConnectionState.value)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 5: `logoutThenLoginCreatesNewNip46Client`
|
||||||
|
|
||||||
|
**Reproduces:** Ensuring fresh client after re-login (no stale state).
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun logoutThenLoginCreatesNewNip46Client() = runTest {
|
||||||
|
// First bunker load
|
||||||
|
val validHex = "a".repeat(64)
|
||||||
|
val ephemeralKeyPair = KeyPair()
|
||||||
|
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://relay.nsec.app")
|
||||||
|
File(amethystDir, "last_account.txt").writeText(ephemeralKeyPair.pubKey.toNpub())
|
||||||
|
coEvery { storage.getPrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS) } returns ephemeralKeyPair.privKey!!.toHexKey()
|
||||||
|
|
||||||
|
manager.loadSavedAccount()
|
||||||
|
val firstState = manager.currentAccount()
|
||||||
|
assertNotNull(firstState)
|
||||||
|
|
||||||
|
// Logout — should disconnect NIP-46 client
|
||||||
|
manager.logout()
|
||||||
|
assertIs<AccountState.LoggedOut>(manager.accountState.value)
|
||||||
|
|
||||||
|
// Re-load — should create a NEW NIP-46 client
|
||||||
|
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://relay.nsec.app")
|
||||||
|
File(amethystDir, "last_account.txt").writeText(ephemeralKeyPair.pubKey.toNpub())
|
||||||
|
manager.loadSavedAccount()
|
||||||
|
val secondState = manager.currentAccount()
|
||||||
|
assertNotNull(secondState)
|
||||||
|
|
||||||
|
// Both states should be valid Remote signers
|
||||||
|
assertIs<SignerType.Remote>(firstState.signerType)
|
||||||
|
assertIs<SignerType.Remote>(secondState.signerType)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Phase 2: State Transition Tests
|
||||||
|
|
||||||
|
### Test File: `AccountManagerStateTransitionTest.kt`
|
||||||
|
|
||||||
|
#### Test 6: `bunkerRestoreShowsConnectingThenLoggedIn`
|
||||||
|
|
||||||
|
**Reproduces:** Bug 1 UX — user sees nothing while connecting.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun bunkerRestoreShowsConnectingThenLoggedIn() = runTest {
|
||||||
|
val states = mutableListOf<AccountState>()
|
||||||
|
val collector = backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) {
|
||||||
|
manager.accountState.toList(states)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Set connecting state (what Main.kt does before loadSavedAccount)
|
||||||
|
manager.setConnectingRelays()
|
||||||
|
|
||||||
|
// Then load internal account
|
||||||
|
val keyPair = KeyPair()
|
||||||
|
val npub = keyPair.pubKey.toNpub()
|
||||||
|
File(amethystDir, "last_account.txt").writeText(npub)
|
||||||
|
coEvery { storage.getPrivateKey(npub) } returns keyPair.privKey!!.toHexKey()
|
||||||
|
manager.loadSavedAccount()
|
||||||
|
|
||||||
|
advanceUntilIdle()
|
||||||
|
|
||||||
|
// Should have: LoggedOut → ConnectingRelays → LoggedIn
|
||||||
|
assertTrue(states.size >= 3, "Expected at least 3 state transitions, got: $states")
|
||||||
|
assertIs<AccountState.LoggedOut>(states[0])
|
||||||
|
assertIs<AccountState.ConnectingRelays>(states[1])
|
||||||
|
assertIs<AccountState.LoggedIn>(states[2])
|
||||||
|
|
||||||
|
collector.cancel()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 7: `loadSavedAccountBunkerNoEphemeralReturnsFailure`
|
||||||
|
|
||||||
|
Updated version — no `client` param.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun loadSavedAccountBunkerNoEphemeralReturnsFailure() = runTest {
|
||||||
|
val validHex = "a".repeat(64)
|
||||||
|
val keyPair = KeyPair()
|
||||||
|
val npub = keyPair.pubKey.toNpub()
|
||||||
|
|
||||||
|
File(amethystDir, "last_account.txt").writeText(npub)
|
||||||
|
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://r.com")
|
||||||
|
coEvery { storage.getPrivateKey(AccountManager.BUNKER_EPHEMERAL_KEY_ALIAS) } returns null
|
||||||
|
|
||||||
|
val result = manager.loadSavedAccount()
|
||||||
|
assertTrue(result.isFailure)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Phase 3: NostrSignerRemote Isolation Tests (quartz)
|
||||||
|
|
||||||
|
### Test File: `NostrSignerRemoteIsolationTest.kt`
|
||||||
|
|
||||||
|
**Location:** `quartz/src/commonTest/kotlin/.../nip46RemoteSigner/signer/`
|
||||||
|
|
||||||
|
#### Test 8: `subscriptionFilterTargetsOnlyBunkerRelays`
|
||||||
|
|
||||||
|
**Reproduces:** Bug 2 — subscription filters should ONLY reference NIP-46 relays.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun subscriptionFilterTargetsOnlyBunkerRelays() {
|
||||||
|
val bunkerRelay = NormalizedRelayUrl("wss://relay.nsec.app/")
|
||||||
|
val generalRelay = NormalizedRelayUrl("wss://relay.damus.io/")
|
||||||
|
|
||||||
|
val capturedFilters = mutableListOf<Map<NormalizedRelayUrl, List<Filter>>>()
|
||||||
|
val mockClient = mockk<INostrClient>(relaxed = true)
|
||||||
|
every { mockClient.openReqSubscription(any(), capture(capturedFilters), any()) } just Runs
|
||||||
|
|
||||||
|
val ephemeralSigner = NostrSignerInternal(KeyPair())
|
||||||
|
val remoteSigner = NostrSignerRemote(
|
||||||
|
signer = ephemeralSigner,
|
||||||
|
remotePubkey = "a".repeat(64),
|
||||||
|
relays = setOf(bunkerRelay),
|
||||||
|
client = mockClient,
|
||||||
|
)
|
||||||
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
|
// Verify filter only targets bunker relay
|
||||||
|
assertTrue(capturedFilters.isNotEmpty(), "No subscription opened")
|
||||||
|
capturedFilters.forEach { filterMap ->
|
||||||
|
assertTrue(filterMap.keys.all { it == bunkerRelay }, "Filter contains non-bunker relay: ${filterMap.keys}")
|
||||||
|
assertTrue(generalRelay !in filterMap.keys, "General relay leaked into NIP-46 filter")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 9: `subscriptionFilterContainsCorrectKindAndPTag`
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun subscriptionFilterContainsCorrectKindAndPTag() {
|
||||||
|
val capturedFilters = mutableListOf<Map<NormalizedRelayUrl, List<Filter>>>()
|
||||||
|
val mockClient = mockk<INostrClient>(relaxed = true)
|
||||||
|
every { mockClient.openReqSubscription(any(), capture(capturedFilters), any()) } just Runs
|
||||||
|
|
||||||
|
val ephemeralKeyPair = KeyPair()
|
||||||
|
val ephemeralSigner = NostrSignerInternal(ephemeralKeyPair)
|
||||||
|
val remoteSigner = NostrSignerRemote(
|
||||||
|
signer = ephemeralSigner,
|
||||||
|
remotePubkey = "b".repeat(64),
|
||||||
|
relays = setOf(NormalizedRelayUrl("wss://relay.nsec.app/")),
|
||||||
|
client = mockClient,
|
||||||
|
)
|
||||||
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
|
val filters = capturedFilters.flatMap { it.values.flatten() }
|
||||||
|
assertTrue(filters.isNotEmpty())
|
||||||
|
filters.forEach { filter ->
|
||||||
|
assertTrue(filter.kinds?.contains(NostrConnectEvent.KIND) == true,
|
||||||
|
"Filter missing kind ${NostrConnectEvent.KIND}")
|
||||||
|
assertTrue(filter.tags?.get("p")?.contains(ephemeralSigner.pubKey) == true,
|
||||||
|
"Filter missing p-tag for ephemeral pubkey")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Phase 4: `since` Filter Tests (Bug 3 mitigation)
|
||||||
|
|
||||||
|
### Test File: add to `NostrSignerRemoteIsolationTest.kt`
|
||||||
|
|
||||||
|
#### Test 10: `subscriptionFilterHasSinceTimestamp`
|
||||||
|
|
||||||
|
**Reproduces:** Stale NIP-46 responses from previous sessions being processed.
|
||||||
|
|
||||||
|
```kotlin
|
||||||
|
@Test
|
||||||
|
fun subscriptionFilterHasSinceTimestamp() {
|
||||||
|
val capturedFilters = mutableListOf<Map<NormalizedRelayUrl, List<Filter>>>()
|
||||||
|
val mockClient = mockk<INostrClient>(relaxed = true)
|
||||||
|
every { mockClient.openReqSubscription(any(), capture(capturedFilters), any()) } just Runs
|
||||||
|
|
||||||
|
val beforeTime = TimeUtils.now()
|
||||||
|
|
||||||
|
val remoteSigner = NostrSignerRemote(
|
||||||
|
signer = NostrSignerInternal(KeyPair()),
|
||||||
|
remotePubkey = "c".repeat(64),
|
||||||
|
relays = setOf(NormalizedRelayUrl("wss://relay.nsec.app/")),
|
||||||
|
client = mockClient,
|
||||||
|
)
|
||||||
|
remoteSigner.openSubscription()
|
||||||
|
|
||||||
|
val filters = capturedFilters.flatMap { it.values.flatten() }
|
||||||
|
assertTrue(filters.isNotEmpty())
|
||||||
|
filters.forEach { filter ->
|
||||||
|
assertNotNull(filter.since, "Filter missing 'since' timestamp")
|
||||||
|
// since should be roughly now - 60s (with some tolerance)
|
||||||
|
val expectedMin = beforeTime - 120 // extra tolerance
|
||||||
|
val expectedMax = beforeTime
|
||||||
|
assertTrue(filter.since!! >= expectedMin, "since too old: ${filter.since}")
|
||||||
|
assertTrue(filter.since!! <= expectedMax, "since in the future: ${filter.since}")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
> **Note:** This test will FAIL until Step 8 (add `since` to NostrSignerRemote) is implemented. That's the TDD red phase.
|
||||||
|
|
||||||
|
## Implementation Order
|
||||||
|
|
||||||
|
| # | Action | File | Phase |
|
||||||
|
|---|--------|------|-------|
|
||||||
|
| 1 | Fix broken tests (remove `client` param) | `AccountManagerLoadAccountTest.kt` | 0 |
|
||||||
|
| 2 | Write relay isolation tests (Tests 1-5) | `AccountManagerNip46IsolationTest.kt` | 1 |
|
||||||
|
| 3 | Write state transition tests (Tests 6-7) | `AccountManagerStateTransitionTest.kt` | 2 |
|
||||||
|
| 4 | Write signer isolation tests (Tests 8-9) | `NostrSignerRemoteIsolationTest.kt` | 3 |
|
||||||
|
| 5 | Write `since` filter test (Test 10) — **expect RED** | `NostrSignerRemoteIsolationTest.kt` | 4 |
|
||||||
|
| 6 | Run all → confirm Tests 1-9 GREEN, Test 10 RED | — | — |
|
||||||
|
| 7 | Implement Step 8 (`since` filter in quartz) | `NostrSignerRemote.kt` | — |
|
||||||
|
| 8 | Run all → confirm all GREEN | — | — |
|
||||||
|
|
||||||
|
## Test Commands
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Run all desktop tests
|
||||||
|
./gradlew :desktopApp:test
|
||||||
|
|
||||||
|
# Run specific test class
|
||||||
|
./gradlew :desktopApp:test --tests "*.AccountManagerNip46IsolationTest"
|
||||||
|
|
||||||
|
# Run quartz common tests
|
||||||
|
./gradlew :quartz:jvmTest --tests "*.NostrSignerRemoteIsolationTest"
|
||||||
|
|
||||||
|
# Run all NIP-46 related tests
|
||||||
|
./gradlew :desktopApp:test --tests "*.AccountManager*" :quartz:jvmTest --tests "*.nip46RemoteSigner.*"
|
||||||
|
```
|
||||||
|
|
||||||
|
## Unanswered Questions
|
||||||
|
|
||||||
|
1. `getOrCreateNip46Client()` is private — can't directly verify single-creation from tests without reflection or `@VisibleForTesting`. Sufficient to test indirectly through public API?
|
||||||
|
2. `NostrSignerRemote.subscription` field is private — MockK `capture` on `openReqSubscription` is the best proxy for filter verification?
|
||||||
|
3. Should we add Turbine dependency for cleaner StateFlow testing, or stick with manual `backgroundScope` collection?
|
||||||
|
4. `NostrClient.connect()` is non-suspending (fire-and-forget) — how to test that the NIP-46 client actually connects before subscriptions? Accept relay auto-queue behavior?
|
||||||
+2
-19
@@ -42,12 +42,6 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent
|
|||||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapPrivateEvent
|
import com.vitorpamplona.quartz.nip57Zaps.LnZapPrivateEvent
|
||||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent
|
import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent
|
||||||
import com.vitorpamplona.quartz.utils.Hex
|
import com.vitorpamplona.quartz.utils.Hex
|
||||||
import com.vitorpamplona.quartz.utils.TimeUtils
|
|
||||||
import kotlinx.coroutines.CoroutineScope
|
|
||||||
import kotlinx.coroutines.Dispatchers
|
|
||||||
import kotlinx.coroutines.SupervisorJob
|
|
||||||
import kotlinx.coroutines.cancel
|
|
||||||
import kotlinx.coroutines.launch
|
|
||||||
|
|
||||||
class NostrSignerRemote(
|
class NostrSignerRemote(
|
||||||
val signer: NostrSignerInternal,
|
val signer: NostrSignerInternal,
|
||||||
@@ -57,8 +51,6 @@ class NostrSignerRemote(
|
|||||||
val permissions: String? = null,
|
val permissions: String? = null,
|
||||||
val secret: String? = null,
|
val secret: String? = null,
|
||||||
) : NostrSigner(signer.pubKey) {
|
) : NostrSigner(signer.pubKey) {
|
||||||
private val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
|
|
||||||
|
|
||||||
private val manager =
|
private val manager =
|
||||||
RemoteSignerManager(
|
RemoteSignerManager(
|
||||||
signer = signer,
|
signer = signer,
|
||||||
@@ -74,17 +66,10 @@ class NostrSignerRemote(
|
|||||||
Filter(
|
Filter(
|
||||||
kinds = listOf(NostrConnectEvent.KIND),
|
kinds = listOf(NostrConnectEvent.KIND),
|
||||||
tags = mapOf("p" to listOf(signer.pubKey)),
|
tags = mapOf("p" to listOf(signer.pubKey)),
|
||||||
since = TimeUtils.now() - 60,
|
|
||||||
),
|
),
|
||||||
) { event ->
|
) { event ->
|
||||||
if (event is NostrConnectEvent) {
|
if (event is NostrConnectEvent) {
|
||||||
scope.launch {
|
manager.newResponse(event)
|
||||||
try {
|
|
||||||
manager.newResponse(event)
|
|
||||||
} catch (_: Exception) {
|
|
||||||
// Ignore events we can't decrypt or parse
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -94,7 +79,6 @@ class NostrSignerRemote(
|
|||||||
|
|
||||||
fun closeSubscription() {
|
fun closeSubscription() {
|
||||||
subscription.close()
|
subscription.close()
|
||||||
scope.cancel()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun isWriteable(): Boolean = true
|
override fun isWriteable(): Boolean = true
|
||||||
@@ -303,10 +287,9 @@ class NostrSignerRemote(
|
|||||||
permissions: String? = null,
|
permissions: String? = null,
|
||||||
): NostrSignerRemote {
|
): NostrSignerRemote {
|
||||||
if (!bunkerUri.startsWith("bunker://")) throw Exception("Invalid bunker uri")
|
if (!bunkerUri.startsWith("bunker://")) throw Exception("Invalid bunker uri")
|
||||||
val splitData = bunkerUri.split("?", limit = 2)
|
val splitData = bunkerUri.split("?")
|
||||||
val remotePubkey = splitData[0].removePrefix("bunker://")
|
val remotePubkey = splitData[0].removePrefix("bunker://")
|
||||||
if (!Hex.isHex(remotePubkey)) throw Exception("Invalid pubkey in bunker uri")
|
if (!Hex.isHex(remotePubkey)) throw Exception("Invalid pubkey in bunker uri")
|
||||||
if (splitData.size < 2) throw Exception("Missing query parameters in bunker uri")
|
|
||||||
val params = splitData[1].split("&")
|
val params = splitData[1].split("&")
|
||||||
val relays = mutableSetOf<NormalizedRelayUrl>()
|
val relays = mutableSetOf<NormalizedRelayUrl>()
|
||||||
var secret: String? = null
|
var secret: String? = null
|
||||||
|
|||||||
Reference in New Issue
Block a user