diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt index 234e93821d..651af1abeb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt @@ -37,6 +37,7 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.server.BunkerRequestProcessor import com.vitorpamplona.quartz.nip46RemoteSigner.server.NostrConnectSignerService import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.RandomInstance +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow @@ -44,6 +45,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -121,6 +123,9 @@ class Nip46SignerState( scope.launch(Dispatchers.IO) { combine(settings.nip46SignerEnabled, listeningRelays) { enabled, relays -> enabled to relays } + // Inbox/relay StateFlows can re-emit an identical set; without this every duplicate + // would tear the subscription down and re-open it on every relay for no reason. + .distinctUntilChanged() .collectLatest { (enabled, relays) -> if (!enabled) return@collectLatest if (!signer.isWriteable()) { @@ -207,6 +212,8 @@ class Nip46SignerState( setEnabled(true) ConnectResult.Connected(offer.clientPubKey, offer.name) + } catch (e: CancellationException) { + throw e } catch (e: Exception) { Log.w("NIP46Signer") { "nostrconnect pairing failed: ${e.message}" } ConnectResult.Failed(e.message ?: "unknown error") diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/signers/Nip46PermissionAuthorizer.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/signers/Nip46PermissionAuthorizer.kt index bf7cc7c6f7..e83e07eb66 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/signers/Nip46PermissionAuthorizer.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/signers/Nip46PermissionAuthorizer.kt @@ -20,6 +20,8 @@ */ package com.vitorpamplona.amethyst.commons.napplet.signers +import com.vitorpamplona.amethyst.commons.util.KmpLock +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestConnect @@ -30,6 +32,7 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip44Encrypt import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestSign import com.vitorpamplona.quartz.nip46RemoteSigner.server.Nip46ConnectDecision import com.vitorpamplona.quartz.nip46RemoteSigner.server.Nip46RequestAuthorizer +import com.vitorpamplona.quartz.utils.TimeUtils /** * Bridges the NIP-46 signer core to Amethyst's shared "Connected Apps" trust @@ -67,9 +70,30 @@ class Nip46PermissionAuthorizer( /** Invoked after a successful connect so the host can persist display metadata (name/url/image). */ val onConnected: (suspend (clientPubKey: HexKey, request: BunkerRequestConnect) -> Unit)? = null, ) : Nip46RequestAuthorizer { + // A high-throughput client can authorize many signs per second; last-used is display-only, + // so coalesce the DataStore write to at most one per client per LAST_USED_THROTTLE_SECS + // instead of writing the whole per-client preferences file on every request. + private val lastUsedThrottle = mutableMapOf() + private val throttleLock = KmpLock() + /** The ledger coordinate for [clientPubKey] under this account. */ fun coordinateFor(clientPubKey: HexKey): String = coordinateFor(signerPubKey, clientPubKey) + private suspend fun touchLastUsed(coordinate: String) { + val now = TimeUtils.now() + val shouldWrite = + throttleLock.withLock { + val previous = lastUsedThrottle[coordinate] ?: 0L + if (now - previous >= LAST_USED_THROTTLE_SECS) { + lastUsedThrottle[coordinate] = now + true + } else { + false + } + } + if (shouldWrite) ledger.updateLastUsed(coordinate, now) + } + override suspend fun onConnect( clientPubKey: HexKey, request: BunkerRequestConnect, @@ -82,7 +106,7 @@ class Nip46PermissionAuthorizer( if (!ledger.hasPolicy(coordinate)) { ledger.setPolicy(coordinate, defaultPolicyOnConnect) } - ledger.updateLastUsed(coordinate) + touchLastUsed(coordinate) onConnected?.invoke(clientPubKey, request) // Echo the offered secret when present (the client validates it); otherwise ack. @@ -100,7 +124,7 @@ class Nip46PermissionAuthorizer( val op = request.toSignerOp() ?: return true val coordinate = coordinateFor(clientPubKey) val allowed = ledger.decide(coordinate, op) == NostrOpDecision.ALLOW - if (allowed) ledger.updateLastUsed(coordinate) + if (allowed) touchLastUsed(coordinate) return allowed } @@ -115,6 +139,9 @@ class Nip46PermissionAuthorizer( private const val ACK = "ack" + /** Minimum seconds between persisted last-used updates for one client (write-coalescing). */ + private const val LAST_USED_THROTTLE_SECS = 60L + /** * The Connected-Apps ledger coordinate for a NIP-46 client, namespaced by * the user's signer so the same client paired with two accounts on one diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt index efb7454bd3..f86e3c984c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt @@ -44,6 +44,7 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponseGetRelays import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponsePong import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponsePublicKey import com.vitorpamplona.quartz.nip46RemoteSigner.ReadWrite +import kotlinx.coroutines.CancellationException /** * The signer/bunker side of NIP-46: turns a decrypted [BunkerRequest] from a @@ -130,6 +131,8 @@ class BunkerRequestProcessor( else -> BunkerResponseError(request.id, "unsupported method: ${request.method}") } } + } catch (e: CancellationException) { + throw e } catch (e: Exception) { BunkerResponseError(request.id, "${e::class.simpleName}: ${e.message}") } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt index 41317dad27..af42c53a26 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt @@ -31,6 +31,7 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponseError import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.Channel.Factory.UNLIMITED @@ -76,8 +77,6 @@ class NostrConnectSignerService( val self = signer.pubKey val events = Channel(UNLIMITED) - // Insertion-ordered so the oldest id can be evicted once the cap is hit. - val seen = LinkedHashSet() val subId = newSubId() val listener = object : SubscriptionListener { @@ -87,23 +86,31 @@ class NostrConnectSignerService( relay: NormalizedRelayUrl, forFilters: List?, ) { - if (event is NostrConnectEvent && event.verifiedRecipientPubKey() == self && seen.add(event.id)) { - if (seen.size > seenCap) { - seen.iterator().let { - it.next() - it.remove() - } - } + // onEvent is invoked CONCURRENTLY from each relay's socket thread, so it must + // touch no shared mutable state here — the (thread-safe) channel send is all it + // does; dedup happens in the single-threaded consumer below. + if (event is NostrConnectEvent && event.verifiedRecipientPubKey() == self) { events.trySend(event) } } } + // Insertion-ordered dedup, confined to this one consumer coroutine (never the relay threads); + // evicts the oldest id past the cap so a long-lived signer can't grow it without bound. + val seen = LinkedHashSet() val filter = Filter(kinds = listOf(NostrConnectEvent.KIND), tags = mapOf("p" to listOf(self))) client.subscribe(subId, relays.associateWith { listOf(filter) }, listener) try { while (true) { - handle(events.receive()) + val event = events.receive() + if (!seen.add(event.id)) continue // same request already handled (arrived on another relay) + if (seen.size > seenCap) { + seen.iterator().let { + it.next() + it.remove() + } + } + handle(event) } } finally { client.unsubscribe(subId) @@ -116,6 +123,8 @@ class NostrConnectSignerService( val request = try { event.decryptMessage(signer) as? BunkerRequest ?: return + } catch (e: CancellationException) { + throw e } catch (e: Exception) { Log.w("NIP46Signer") { "could not decrypt request ${event.id.take(8)}: ${e.message}" } return @@ -128,6 +137,8 @@ class NostrConnectSignerService( try { val reply = NostrConnectEvent.create(response, client, signer) this.client.publish(reply, relays) + } catch (e: CancellationException) { + throw e } catch (e: Exception) { Log.w("NIP46Signer") { "failed to send reply for ${request.method}: ${e.message}" } }