From ee418bdd0a555c26f96159d552bab5f24a77ec36 Mon Sep 17 00:00:00 2001 From: M Date: Fri, 6 Mar 2026 11:35:27 +1100 Subject: [PATCH] fix: address race conditions and improve Namecoin NIP-05 resolution Fixes discovered while porting Namecoin NIP-05 to Primal Android (PrimalHQ/primal-android-app#934): 1. Race condition: stale intermediate lookups overwrite final result - Changed map to mapLatest in SearchBarViewModel so that previous in-flight ElectrumX lookups are cancelled when the query changes - Without this, typing 'd/testls' character by character causes stale results from 'd/test', 'd/testl' etc. to arrive after the correct 'd/testls' result and overwrite it with null 2. CancellationException swallowed in catch block - mapLatest cancels previous coroutines via CancellationException - The generic catch was catching it and emitting null, wiping the valid result. Now re-throws CancellationException 3. Root lookup fails when names object has no underscore key - extractFromDomainValue tried names[localPart] then names[underscore] which is the same thing for root lookups - Now falls back to first available entry for root lookups when no underscore key exists - Non-root lookups still fail correctly (no false matches) 4. Global Mutex serializes all lookups - Single Mutex in ElectrumxClient blocked unrelated lookups to different servers. A 25s timeout on one server stalled everything - Now uses per-server mutexes via ConcurrentHashMap 5. customServers set but never read - NamecoinNameService.setCustomServers() stored the list but the resolver was constructed with the default serverListProvider - Now wires customServers through to the resolver 6. resolveLive uses orphaned CoroutineScope - Now accepts an optional external scope parameter so callers can tie resolution to their own lifecycle Adds 2 new tests: root fallback to first entry, non-root no-fallback. --- .../com/vitorpamplona/amethyst/AppModules.kt | 3 + .../service/namecoin/NamecoinNameService.kt | 32 +++++++-- .../loggedIn/search/SearchBarViewModel.kt | 6 +- .../namecoin/NamecoinNameResolver.kt | 35 ++++++++-- .../namecoin/ElectrumXClient.kt | 4 +- .../namecoin/NamecoinNameResolverTest.kt | 69 +++++++++++++++++-- 6 files changed, 134 insertions(+), 15 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index c05fa392c..7def92c09 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -161,6 +161,9 @@ class AppModules( } }, ) + val namecoinNameService = + com.vitorpamplona.amethyst.service.namecoin.NamecoinNameService + .init(namecoinElectrumxClient) val nip05Client = Nip05Client(nip05Fetcher, namecoinResolver) // Application-wide block height request cache diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/namecoin/NamecoinNameService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/namecoin/NamecoinNameService.kt index 7f4bf2c37..348521b12 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/namecoin/NamecoinNameService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/namecoin/NamecoinNameService.kt @@ -43,11 +43,32 @@ class NamecoinNameService( ) { private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) - private val resolver = NamecoinNameResolver(electrumxClient) + // Custom server list (user-configurable) + @Volatile + private var customServers: List = emptyList() + + private val resolver = + NamecoinNameResolver( + electrumxClient = electrumxClient, + serverListProvider = { customServers.ifEmpty { ElectrumxClient.DEFAULT_SERVERS } }, + ) private val cache = NamecoinLookupCache() - // Custom server list (user-configurable) - private var customServers: List = emptyList() + companion object { + @Volatile + private var instance: NamecoinNameService? = null + + fun getInstance(): NamecoinNameService = + instance ?: throw IllegalStateException( + "NamecoinNameService not initialized. Call init() first.", + ) + + fun init(electrumxClient: ElectrumxClient): NamecoinNameService = + synchronized(this) { + instance?.let { return it } + NamecoinNameService(electrumxClient).also { instance = it } + } + } // ── Public API ───────────────────────────────────────────────────── @@ -97,7 +118,10 @@ class NamecoinNameService( * * Useful for composable UIs that observe resolution state. */ - fun resolveLive(identifier: String): StateFlow { + fun resolveLive( + identifier: String, + scope: CoroutineScope = this.scope, + ): StateFlow { val state = MutableStateFlow(NamecoinResolveState.Loading) scope.launch { try { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt index 8855dd149..280411842 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt @@ -48,7 +48,7 @@ import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.flowOn -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -83,7 +83,7 @@ class SearchBarViewModel( .debounce(400) .distinctUntilChanged() .filter { NamecoinNameResolver.isNamecoinIdentifier(it) } - .map { term -> + .mapLatest { term -> try { val result = Amethyst.instance.namecoinResolver.resolve(term) if (result != null) { @@ -91,6 +91,8 @@ class SearchBarViewModel( } else { null } + } catch (e: kotlinx.coroutines.CancellationException) { + throw e } catch (_: Exception) { null } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt index b63735330..e9f4e07d0 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt @@ -206,16 +206,43 @@ class NamecoinNameResolver( // Extended form: "nostr": { "names": {...}, "relays": {...} } if (nostrField is JsonObject) { val names = nostrField["names"]?.jsonObject ?: return null - val pubkeyElem = names[parsed.localPart] ?: names["_"] // fall back to root - val pubkey = (pubkeyElem as? JsonPrimitive)?.content ?: return null - if (!isValidPubkey(pubkey)) return null + + // Resolve: exact match → "_" root → first entry (root lookups only) + val resolvedLocalPart: String + val pubkey: String + + val exactMatch = names[parsed.localPart] + val rootMatch = names["_"] + val firstEntry = if (parsed.localPart == "_") names.entries.firstOrNull() else null + + when { + exactMatch is JsonPrimitive && isValidPubkey(exactMatch.content) -> { + resolvedLocalPart = parsed.localPart + pubkey = exactMatch.content + } + + rootMatch is JsonPrimitive && isValidPubkey(rootMatch.content) -> { + resolvedLocalPart = "_" + pubkey = rootMatch.content + } + + firstEntry != null && firstEntry.value is JsonPrimitive && + isValidPubkey((firstEntry.value as JsonPrimitive).content) -> { + resolvedLocalPart = firstEntry.key + pubkey = (firstEntry.value as JsonPrimitive).content + } + + else -> { + return null + } + } val relays = extractRelays(nostrField, pubkey) return NamecoinNostrResult( pubkey = pubkey.lowercase(), relays = relays, namecoinName = parsed.namecoinName, - localPart = parsed.localPart, + localPart = resolvedLocalPart, ) } diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/ElectrumXClient.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/ElectrumXClient.kt index 9043a1d8f..477b93733 100644 --- a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/ElectrumXClient.kt +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/ElectrumXClient.kt @@ -44,6 +44,7 @@ import java.net.Socket import java.security.MessageDigest import java.security.SecureRandom import java.security.cert.X509Certificate +import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicInteger import javax.net.SocketFactory import javax.net.ssl.SSLContext @@ -83,7 +84,7 @@ class ElectrumXClient( isLenient = true } private val requestId = AtomicInteger(0) - private val mutex = Mutex() + private val serverMutexes = ConcurrentHashMap() companion object { private const val PROTOCOL_VERSION = "1.4" @@ -121,6 +122,7 @@ class ElectrumXClient( server: ElectrumxServer = DEFAULT_ELECTRUMX_SERVERS.first(), ): NameShowResult? = withContext(Dispatchers.IO) { + val mutex = serverMutexes.getOrPut("${server.host}:${server.port}") { Mutex() } mutex.withLock { try { connectAndQuery(identifier, server) diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverTest.kt index 42488cbcb..52822c49c 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverTest.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverTest.kt @@ -115,6 +115,36 @@ class NamecoinNameResolverTest { assertEquals("aaaa000000000000000000000000000000000000000000000000000000000001", result!!.pubkey) } + @Test + fun `root lookup falls back to first entry when no underscore key`() { + val value = """{ + "nostr": { + "names": { + "m": "6cdebccabda1dfa058ab85352a79509b592b2bdfa0370325e28ec1cb4f18667d" + } + } + }""" + + val result = extractNostrFromValue(value, "d/testls", "_") + assertNotNull(result) + assertEquals("6cdebccabda1dfa058ab85352a79509b592b2bdfa0370325e28ec1cb4f18667d", result!!.pubkey) + assertEquals("m", result.localPart) + } + + @Test + fun `non-root lookup does NOT fall back to first entry`() { + val value = """{ + "nostr": { + "names": { + "m": "6cdebccabda1dfa058ab85352a79509b592b2bdfa0370325e28ec1cb4f18667d" + } + } + }""" + + val result = extractNostrFromValue(value, "d/testls", "alice") + assertNull(result) + } + // ── Value format: id/ namespace ──────────────────────────────────── @Test @@ -209,9 +239,40 @@ class NamecoinNameResolverTest { // Extended form if (nostrField is kotlinx.serialization.json.JsonObject) { val names = nostrField["names"]?.jsonObject ?: return null - val pubkeyElem = names[localPart] ?: names["_"] ?: return null - val pubkey = (pubkeyElem as? kotlinx.serialization.json.JsonPrimitive)?.content ?: return null - if (!pubkey.matches(Regex("^[0-9a-fA-F]{64}$"))) return null + + // Resolve: exact match → "_" root → first entry (root lookups only) + val resolvedLocalPart: String + val pubkey: String + + val exactMatch = names[localPart] + val rootMatch = names["_"] + val firstEntry = if (localPart == "_") names.entries.firstOrNull() else null + + when { + exactMatch is kotlinx.serialization.json.JsonPrimitive && + exactMatch.content.matches(Regex("^[0-9a-fA-F]{64}$")) -> { + resolvedLocalPart = localPart + pubkey = exactMatch.content + } + + rootMatch is kotlinx.serialization.json.JsonPrimitive && + rootMatch.content.matches(Regex("^[0-9a-fA-F]{64}$")) -> { + resolvedLocalPart = "_" + pubkey = rootMatch.content + } + + firstEntry != null && firstEntry.value is kotlinx.serialization.json.JsonPrimitive && + (firstEntry.value as kotlinx.serialization.json.JsonPrimitive) + .content + .matches(Regex("^[0-9a-fA-F]{64}$")) -> { + resolvedLocalPart = firstEntry.key + pubkey = (firstEntry.value as kotlinx.serialization.json.JsonPrimitive).content + } + + else -> { + return null + } + } val relays = try { @@ -227,7 +288,7 @@ class NamecoinNameResolverTest { pubkey = pubkey.lowercase(), relays = relays, namecoinName = namecoinName, - localPart = localPart, + localPart = resolvedLocalPart, ) } return null