Merge pull request #2719 from nrobi144/fix/bunker-timeout-and-decrypt

fix(nip46): fix bunker decrypt/encrypt parsing and increase timeout

desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/chats/ChatPane.kt

@@ -455,7 +455,7 @@ private fun MessageWithReactions(
                     try {
                         event.decryptContent(account.signer)
                     } catch (_: Exception) {
-                        event.content
+                        null
                     }
                 }
 
@@ -597,11 +597,22 @@ private fun MessageWithReactions(
 
                     else -> {
                         SelectionContainer {
-                            Text(
+                            if (decryptedContent != null) {
-                                text = decryptedContent ?: "",
+                                Text(
-                                style = MaterialTheme.typography.bodyMedium,
+                                    text = decryptedContent!!,
-                                color = MaterialTheme.colorScheme.onSurface,
+                                    style = MaterialTheme.typography.bodyMedium,
-                            )
+                                    color = MaterialTheme.colorScheme.onSurface,
+                                )
+                            } else {
+                                Text(
+                                    text = "Could not decrypt the message",
+                                    style =
+                                        MaterialTheme.typography.bodyMedium.copy(
+                                            fontStyle = androidx.compose.ui.text.font.FontStyle.Italic,
+                                        ),
+                                    color = MaterialTheme.colorScheme.onSurfaceVariant,
+                                )
+                            }
                         }
                     }
                 }

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/Nip04DecryptResponse.kt

@@ -37,7 +37,9 @@ class Nip04DecryptResponse {
                 }
 
                 else -> {
-                    SignerResult.RequestAddressed.ReceivedButCouldNotPerform()
+                    response.result?.let {
+                        SignerResult.RequestAddressed.Successful(DecryptionResult(it))
+                    } ?: SignerResult.RequestAddressed.ReceivedButCouldNotPerform("No result in response")
                 }
             }
     }

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/Nip04EncryptResponse.kt

@@ -37,7 +37,9 @@ class Nip04EncryptResponse {
                 }
 
                 else -> {
-                    SignerResult.RequestAddressed.ReceivedButCouldNotPerform()
+                    response.result?.let {
+                        SignerResult.RequestAddressed.Successful(EncryptionResult(it))
+                    } ?: SignerResult.RequestAddressed.ReceivedButCouldNotPerform("No result in response")
                 }
             }
     }

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/Nip44DecryptResponse.kt

@@ -37,7 +37,9 @@ class Nip44DecryptResponse {
                 }
 
                 else -> {
-                    SignerResult.RequestAddressed.ReceivedButCouldNotPerform()
+                    response.result?.let {
+                        SignerResult.RequestAddressed.Successful(DecryptionResult(it))
+                    } ?: SignerResult.RequestAddressed.ReceivedButCouldNotPerform("No result in response")
                 }
             }
     }

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/Nip44EncryptResponse.kt

@@ -37,7 +37,9 @@ class Nip44EncryptResponse {
                 }
 
                 else -> {
-                    SignerResult.RequestAddressed.ReceivedButCouldNotPerform()
+                    response.result?.let {
+                        SignerResult.RequestAddressed.Successful(EncryptionResult(it))
+                    } ?: SignerResult.RequestAddressed.ReceivedButCouldNotPerform("No result in response")
                 }
             }
     }

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManager.kt

@@ -29,15 +29,17 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponse
 import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent
 import com.vitorpamplona.quartz.utils.cache.LargeCache
 import com.vitorpamplona.quartz.utils.tryAndWait
+import kotlinx.coroutines.delay
 import kotlin.coroutines.Continuation
 import kotlin.coroutines.resume
 
 class RemoteSignerManager(
-    val timeout: Long = 30000,
+    val timeout: Long = 65_000,
     val client: INostrClient,
     val signer: NostrSignerInternal,
     val remoteKey: String,
     val relayList: Set<NormalizedRelayUrl>,
+    val maxRetries: Int = 1,
 ) {
     private val awaitingRequests = LargeCache<String, Continuation<BunkerResponse>>()
 
@@ -48,15 +50,14 @@ class RemoteSignerManager(
     }
 
     /**
-     * Launches the signer, waits and parses the result
+     * Launches the signer, waits and parses the result.
+     *
+     * Builds the request once and republishes the same event on retry to ensure
+     * the bunker's response (keyed by request ID) can always be matched.
      *
      * @param bunkerRequestBuilder The BunkerRequest to be sent.
      * @param parser A function that parses the BunkerResponse into a [SignerResult.RequestAddressed<T>].
      * @return The result after parsing the BunkerResponse using the provided parser.
-     *
-     * This function uses the [tryAndWait] utility to implement a timeout on the Bunker Request approval.
-     * It assigns a unique ID to the request and keeps a continuation to resume once the result is received.
-     * If the timeout occurs or the continuation is cancelled, the request ID is cleaned up from [awaitingRequests].
      */
     suspend fun <T : IResult> launchWaitAndParse(
         bunkerRequestBuilder: () -> BunkerRequest,
@@ -69,20 +70,34 @@ class RemoteSignerManager(
                 remoteKey = remoteKey,
                 signer = signer,
             )
-        val result =
+
-            tryAndWait(timeout) { continuation ->
+        var attempt = 0
-                continuation.invokeOnCancellation {
+        while (true) {
-                    awaitingRequests.remove(request.id)
+            val result =
+                tryAndWait(timeout) { continuation ->
+                    continuation.invokeOnCancellation {
+                        awaitingRequests.remove(request.id)
+                    }
+
+                    awaitingRequests.put(request.id, continuation)
+
+                    client.publish(event, relayList = relayList)
                 }
 
-                awaitingRequests.put(request.id, continuation)
+            when {
+                result != null -> {
+                    return parser(result)
+                }
 
-                client.publish(event, relayList = relayList)
+                attempt >= maxRetries -> {
+                    return SignerResult.RequestAddressed.TimedOut()
+                }
+
+                else -> {
+                    attempt++
+                    delay(2_000L)
+                }
             }
-
-        return when (result) {
-            null -> SignerResult.RequestAddressed.TimedOut()
-            else -> parser(result)
         }
     }
 }

quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManagerRetryTest.kt

@@ -0,0 +1,225 @@
+/*
+ * 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.quartz.nip46RemoteSigner.signer
+
+import com.vitorpamplona.quartz.nip01Core.core.Event
+import com.vitorpamplona.quartz.nip01Core.core.HexKey
+import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
+import com.vitorpamplona.quartz.nip01Core.relay.client.EmptyNostrClient
+import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient
+import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener
+import com.vitorpamplona.quartz.nip01Core.relay.client.reqs.SubscriptionListener
+import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient
+import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
+import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestPing
+import com.vitorpamplona.quartz.utils.Hex
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.test.runTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertIs
+
+class RemoteSignerManagerRetryTest {
+    private val signer = NostrSignerInternal(KeyPair())
+    private val remoteKeyPair = KeyPair()
+    private val remoteKey = Hex.encode(remoteKeyPair.pubKey)
+    private val relay = NormalizedRelayUrl("wss://relay.test")
+
+    @Test
+    fun timeoutReturnsTimedOutAfterMaxRetries() =
+        runTest {
+            val manager =
+                RemoteSignerManager(
+                    timeout = 100, // Very short for test
+                    client = EmptyNostrClient(),
+                    signer = signer,
+                    remoteKey = remoteKey,
+                    relayList = setOf(relay),
+                    maxRetries = 0, // No retries
+                )
+
+            val result =
+                manager.launchWaitAndParse(
+                    bunkerRequestBuilder = { BunkerRequestPing() },
+                    parser = PingResponse::parse,
+                )
+
+            assertIs<SignerResult.RequestAddressed.TimedOut<PingResult>>(result)
+        }
+
+    @Test
+    fun timeoutWithRetryStillTimesOut() =
+        runTest {
+            val manager =
+                RemoteSignerManager(
+                    timeout = 100,
+                    client = EmptyNostrClient(),
+                    signer = signer,
+                    remoteKey = remoteKey,
+                    relayList = setOf(relay),
+                    maxRetries = 1, // 1 retry = 2 total attempts
+                )
+
+            val result =
+                manager.launchWaitAndParse(
+                    bunkerRequestBuilder = { BunkerRequestPing() },
+                    parser = PingResponse::parse,
+                )
+
+            assertIs<SignerResult.RequestAddressed.TimedOut<PingResult>>(result)
+        }
+
+    @Test
+    fun publishCalledOnEachAttempt() =
+        runTest {
+            var publishCount = 0
+            val countingClient = CountingNostrClient { publishCount++ }
+
+            val manager =
+                RemoteSignerManager(
+                    timeout = 100,
+                    client = countingClient,
+                    signer = signer,
+                    remoteKey = remoteKey,
+                    relayList = setOf(relay),
+                    maxRetries = 2, // 2 retries = 3 total attempts
+                )
+
+            manager.launchWaitAndParse(
+                bunkerRequestBuilder = { BunkerRequestPing() },
+                parser = PingResponse::parse,
+            )
+
+            assertEquals(3, publishCount)
+        }
+
+    @Test
+    fun requestBuilderCalledOnlyOnce() =
+        runTest {
+            var builderCount = 0
+
+            val manager =
+                RemoteSignerManager(
+                    timeout = 100,
+                    client = EmptyNostrClient(),
+                    signer = signer,
+                    remoteKey = remoteKey,
+                    relayList = setOf(relay),
+                    maxRetries = 2,
+                )
+
+            manager.launchWaitAndParse(
+                bunkerRequestBuilder = {
+                    builderCount++
+                    BunkerRequestPing()
+                },
+                parser = PingResponse::parse,
+            )
+
+            assertEquals(1, builderCount)
+        }
+
+    @Test
+    fun defaultTimeoutIs65Seconds() {
+        val manager =
+            RemoteSignerManager(
+                client = EmptyNostrClient(),
+                signer = signer,
+                remoteKey = remoteKey,
+                relayList = setOf(relay),
+            )
+
+        assertEquals(65_000L, manager.timeout)
+    }
+
+    @Test
+    fun defaultMaxRetriesIs1() {
+        val manager =
+            RemoteSignerManager(
+                client = EmptyNostrClient(),
+                signer = signer,
+                remoteKey = remoteKey,
+                relayList = setOf(relay),
+            )
+
+        assertEquals(1, manager.maxRetries)
+    }
+}
+
+private class CountingNostrClient(
+    private val onPublish: () -> Unit,
+) : INostrClient {
+    override fun connectedRelaysFlow(): StateFlow<Set<NormalizedRelayUrl>> = MutableStateFlow(emptySet())
+
+    override fun availableRelaysFlow(): StateFlow<Set<NormalizedRelayUrl>> = MutableStateFlow(emptySet())
+
+    override fun connect() {}
+
+    override fun disconnect() {}
+
+    override fun reconnect(
+        onlyIfChanged: Boolean,
+        ignoreRetryDelays: Boolean,
+    ) {}
+
+    override fun isActive(): Boolean = false
+
+    override fun syncFilters(relay: IRelayClient) {}
+
+    override fun subscribe(
+        subId: String,
+        filters: Map<NormalizedRelayUrl, List<Filter>>,
+        listener: SubscriptionListener?,
+    ) {}
+
+    override fun count(
+        subId: String,
+        filters: Map<NormalizedRelayUrl, List<Filter>>,
+    ) {}
+
+    override fun unsubscribe(subId: String) {}
+
+    override fun publish(
+        event: Event,
+        relayList: Set<NormalizedRelayUrl>,
+    ) {
+        onPublish()
+    }
+
+    override fun addConnectionListener(listener: RelayConnectionListener) {}
+
+    override fun removeConnectionListener(listener: RelayConnectionListener) {}
+
+    override fun getReqFiltersOrNull(subId: String): Map<NormalizedRelayUrl, List<Filter>>? = null
+
+    override fun getCountFiltersOrNull(subId: String): Map<NormalizedRelayUrl, List<Filter>>? = null
+
+    override fun activeRequests(url: NormalizedRelayUrl): Map<String, List<Filter>> = emptyMap()
+
+    override fun activeCounts(url: NormalizedRelayUrl): Map<String, List<Filter>> = emptyMap()
+
+    override fun activeOutboxCache(url: NormalizedRelayUrl): Set<HexKey> = emptySet()
+
+    override fun close() {}
+}

quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/ResponseParserTest.kt

@@ -204,12 +204,21 @@ class ResponseParserTest {
     }
 
     @Test
-    fun nip04EncryptParseUnexpected() {
+    fun nip04EncryptParseUnexpectedNullResult() {
-        val response = BunkerResponsePong("req-4")
+        val response = BunkerResponse("req-4", null, null)
         val result = Nip04EncryptResponse.parse(response)
         assertIs<SignerResult.RequestAddressed.ReceivedButCouldNotPerform<EncryptionResult>>(result)
     }
 
+    @Test
+    fun nip04EncryptParseGenericBunkerResponse() {
+        // Production case: deserializer produces generic BunkerResponse with ciphertext
+        val response = BunkerResponse("req-4", "encrypted-ciphertext-data", null)
+        val result = Nip04EncryptResponse.parse(response)
+        assertIs<SignerResult.RequestAddressed.Successful<EncryptionResult>>(result)
+        assertEquals("encrypted-ciphertext-data", result.result.ciphertext)
+    }
+
     // --- Nip04DecryptResponse ---
 
     @Test
@@ -228,12 +237,21 @@ class ResponseParserTest {
     }
 
     @Test
-    fun nip04DecryptParseUnexpected() {
+    fun nip04DecryptParseUnexpectedNullResult() {
-        val response = BunkerResponsePong("req-5")
+        val response = BunkerResponse("req-5", null, null)
         val result = Nip04DecryptResponse.parse(response)
         assertIs<SignerResult.RequestAddressed.ReceivedButCouldNotPerform<DecryptionResult>>(result)
     }
 
+    @Test
+    fun nip04DecryptParseGenericBunkerResponse() {
+        // Production case: deserializer produces generic BunkerResponse with plaintext
+        val response = BunkerResponse("req-5", "Hello, this is the decrypted message!", null)
+        val result = Nip04DecryptResponse.parse(response)
+        assertIs<SignerResult.RequestAddressed.Successful<DecryptionResult>>(result)
+        assertEquals("Hello, this is the decrypted message!", result.result.plaintext)
+    }
+
     // --- Nip44EncryptResponse ---
 
     @Test
@@ -252,12 +270,21 @@ class ResponseParserTest {
     }
 
     @Test
-    fun nip44EncryptParseUnexpected() {
+    fun nip44EncryptParseUnexpectedNullResult() {
-        val response = BunkerResponseAck("req-6")
+        val response = BunkerResponse("req-6", null, null)
         val result = Nip44EncryptResponse.parse(response)
         assertIs<SignerResult.RequestAddressed.ReceivedButCouldNotPerform<EncryptionResult>>(result)
     }
 
+    @Test
+    fun nip44EncryptParseGenericBunkerResponse() {
+        // Production case: deserializer produces generic BunkerResponse with ciphertext
+        val response = BunkerResponse("req-6", "nip44-encrypted-payload", null)
+        val result = Nip44EncryptResponse.parse(response)
+        assertIs<SignerResult.RequestAddressed.Successful<EncryptionResult>>(result)
+        assertEquals("nip44-encrypted-payload", result.result.ciphertext)
+    }
+
     // --- Nip44DecryptResponse ---
 
     @Test
@@ -276,9 +303,18 @@ class ResponseParserTest {
     }
 
     @Test
-    fun nip44DecryptParseUnexpected() {
+    fun nip44DecryptParseUnexpectedNullResult() {
-        val response = BunkerResponseAck("req-7")
+        val response = BunkerResponse("req-7", null, null)
         val result = Nip44DecryptResponse.parse(response)
         assertIs<SignerResult.RequestAddressed.ReceivedButCouldNotPerform<DecryptionResult>>(result)
     }
+
+    @Test
+    fun nip44DecryptParseGenericBunkerResponse() {
+        // Production case: deserializer produces generic BunkerResponse with plaintext
+        val response = BunkerResponse("req-7", "Decrypted NIP-44 content here", null)
+        val result = Nip44DecryptResponse.parse(response)
+        assertIs<SignerResult.RequestAddressed.Successful<DecryptionResult>>(result)
+        assertEquals("Decrypted NIP-44 content here", result.result.plaintext)
+    }
 }