core: iterate past RSLV-unsupported name servers

src/Simplex/Chat/Library/Commands.hs

@@ -4633,10 +4633,11 @@ processChatCommand cxt nm = \case
 
 -- | Failure modes for 'resolveOnUserServers' / 'iterateResolvers'.
 data ResolveError
-  = -- | First server returned CMD UNKNOWN / PROHIBITED (does not speak RSLV), or no servers configured.
-    -- We do not try further servers: each candidate-name query broadcasts the name
-    -- to the chosen relay, so iterating across operators on a definite "answered but
-    -- can't help" response would leak the queried name to every operator.
+  = -- | No enabled server can resolve: every candidate answered CMD UNKNOWN /
+    -- PROHIBITED (does not speak RSLV), or none were configured / reachable.
+    -- Iterating across unsupported servers is privacy-safe -- the client degrades
+    -- RSLV to a no-op for relays below namesSMPVersion (see Protocol.hs), so an
+    -- unsupported relay never receives the queried name.
     ResolverUnavailable
   | -- | AUTH from a name-capable server. Every name server reads the same on-chain state, so we trust the first one's no.
     NameNotRegistered
@@ -4656,13 +4657,15 @@ enabledSMPServersForUser user =
       | otherwise = Nothing
 
 -- | Resolve a SimpleX name by trying the user's enabled SMP servers in order.
--- Only transport-level failures (NETWORK, TIMEOUT, host-unreachable) fall
--- through to the next server. Any server that actually answered the request
--- terminates iteration: AUTH is definitive NotFound (every name server reads
--- the same on-chain state); CMD UNKNOWN / PROHIBITED means the server doesn't
--- speak RSLV; any other definite error surfaces as ResolverTransport.
--- Privacy: each query reveals the candidate name to the relay, so we must
--- not broadcast misses to every operator the user has configured.
+-- Transport-level failures (NETWORK, TIMEOUT, host-unreachable) and unsupported
+-- servers (CMD UNKNOWN / PROHIBITED, i.e. relays that don't speak RSLV) both fall
+-- through to the next server. Skipping an unsupported relay discloses nothing:
+-- the client degrades RSLV to a no-op below namesSMPVersion, so the name is never
+-- sent to it. A definitive answer from a name-capable relay terminates iteration:
+-- AUTH is definitive NotFound (every name server reads the same on-chain state);
+-- any other definite error surfaces as ResolverTransport.
+-- Privacy: a name-capable relay does see the queried name, so once one has
+-- answered we do not broadcast the miss to every other operator the user has.
 resolveOnUserServers :: User -> SimplexNameDomain -> CM (Either ResolveError NameRecord)
 resolveOnUserServers user@User {userId} domain = do
   srvs <- enabledSMPServersForUser user
@@ -4842,7 +4845,7 @@ iterateResolvers servers resolve = go servers
         Right nr -> pure $ Right nr
         Left e
           | isNotRegistered e -> pure $ Left NameNotRegistered
-          | isUnsupported e -> pure $ Left ResolverUnavailable
+          | isUnsupported e -> go rest
           | temporaryOrHostError e -> go rest
           | otherwise -> pure $ Left $ ResolverTransport e
     isNotRegistered = \case
@@ -4850,8 +4853,10 @@ iterateResolvers servers resolve = go servers
       _ -> False
     -- A server that doesn't speak RSLV answers CMD UNKNOWN (predates the
     -- command, e.g. the official servers) or CMD PROHIBITED (knows it but gates
-    -- on namesSMPVersion) -- directly, or wrapped by a proxy. All mean "can't
-    -- resolve here", surfaced to the user as CESimplexNameResolverUnavailable.
+    -- on namesSMPVersion) -- directly, or wrapped by a proxy. We skip it and try
+    -- the next server: the client degrades RSLV to a no-op below namesSMPVersion
+    -- (Protocol.hs), so an unsupported relay never received the queried name.
+    -- ResolverUnavailable is returned only when no server can resolve.
     isUnsupported = \case
       SMP _ (SMP.CMD SMP.UNKNOWN) -> True
       SMP _ (SMP.CMD SMP.PROHIBITED) -> True

tests/ResolveNameTests.hs

@@ -23,22 +23,30 @@ import Test.Hspec
 resolveNameTests :: Spec
 resolveNameTests = do
   -- iterateResolvers is the testable core of resolveOnUserServers: it walks
-  -- a list of candidate SMP servers, querying a resolver per server. Only
-  -- transport-level failures (NETWORK / TIMEOUT / host-unreachable) fall through
-  -- to the next server. Any "the server answered" outcome — hit, AUTH, CMD
-  -- PROHIBITED, or any other definite error — stops iteration so the candidate
-  -- name is not broadcast to every operator the user has configured.
+  -- a list of candidate SMP servers, querying a resolver per server. Transport
+  -- failures (NETWORK / TIMEOUT / host-unreachable) and unsupported servers
+  -- (CMD UNKNOWN / PROHIBITED — relays that don't speak RSLV) both fall through
+  -- to the next server; an unsupported relay never received the name (the client
+  -- degrades RSLV below namesSMPVersion), so skipping it discloses nothing. A
+  -- definitive answer from a name-capable relay — hit, AUTH, or any other definite
+  -- error — stops iteration so the name is not broadcast to every operator.
   describe "iterateResolvers" $ do
     it "returns the first server's NameRecord on hit" $ do
       let r = runIdentity $ iterateResolvers [srv1, srv2] $ \_ -> pure $ Right sampleRecord
       r `shouldBe` Right sampleRecord
-    it "stops on CMD PROHIBITED and returns ResolverUnavailable" $ do
+    it "skips an unsupported (CMD PROHIBITED) server and uses the next server's hit" $ do
+      callsRef <- newIORef []
+      r <- iterateResolvers [srv1, srv2] (recording callsRef stubProhibitedThenHit)
+      r `shouldBe` Right sampleRecord
+      -- both servers consulted: srv1 doesn't speak RSLV, so we fall through to
+      -- srv2. srv1 never received the name (RSLV degrades below namesSMPVersion).
+      readIORef callsRef `shouldReturn` [srv1, srv2]
+    it "returns ResolverUnavailable when every server is unsupported" $ do
       callsRef <- newIORef []
       r <- iterateResolvers [srv1, srv2] (recording callsRef (\_ -> pure $ Left prohibitedErr))
       r `shouldBe` Left ResolverUnavailable
-      -- second server must NOT be consulted: PROHIBITED is a server response,
-      -- iterating would broadcast the queried name to every operator.
-      readIORef callsRef `shouldReturn` [srv1]
+      -- all servers consulted, none could resolve.
+      readIORef callsRef `shouldReturn` [srv1, srv2]
     it "treats AUTH as definitive NameNotRegistered and stops iteration" $ do
       callsRef <- newIORef []
       r <- iterateResolvers [srv1, srv2] (recording callsRef stubAuthThenHit)
@@ -90,26 +98,33 @@ resolveNameTests = do
   -- identical to a local-store miss.
   describe "firstNameLink" $ do
     it "NTContact path picks simplexContact" $
-      case firstNameLink NTContact channelLink contactLink aliceNi of
+      case firstNameLink NTContact [channelLink] [contactLink] aliceNi of
         Right lnk -> lnk `shouldBe` contactLink
         Left e -> expectationFailure $ "expected Right, got " <> show e
     it "NTPublicGroup path picks simplexChannel" $
-      case firstNameLink NTPublicGroup channelLink contactLink groupNi of
+      case firstNameLink NTPublicGroup [channelLink] [contactLink] groupNi of
         Right lnk -> lnk `shouldBe` channelLink
         Left e -> expectationFailure $ "expected Right, got " <> show e
-    it "empty Text returns NotFound" $
-      case firstNameLink NTContact "" "" aliceNi of
+    -- Each per-type field is a list of links (primary first); the first non-empty
+    -- entry is used. firstNameLink filters out empty strings, so a list of only
+    -- empty links collapses to NotFound — same UX as a local-store miss.
+    it "picks the first non-empty link, skipping empty entries" $
+      case firstNameLink NTContact [] ["", contactLink] aliceNi of
+        Right lnk -> lnk `shouldBe` contactLink
+        Left e -> expectationFailure $ "expected Right, got " <> show e
+    it "empty link list returns NotFound" $
+      case firstNameLink NTContact [] [] aliceNi of
         Left (ChatError (CESimplexNameNotFound ni)) -> ni `shouldBe` aliceNi
         other -> expectationFailure $ "expected CESimplexNameNotFound, got " <> show other
     -- Each name advertises a per-type link; cross-type fallback would silently
     -- connect to the wrong target, so a populated off-type slot must not satisfy
     -- the queried type.
     it "cross-type contact link with NTPublicGroup returns NotFound" $
-      case firstNameLink NTPublicGroup "" contactLink groupNi of
+      case firstNameLink NTPublicGroup [] [contactLink] groupNi of
         Left (ChatError (CESimplexNameNotFound ni)) -> ni `shouldBe` groupNi
         other -> expectationFailure $ "expected CESimplexNameNotFound, got " <> show other
     it "cross-type channel link with NTContact returns NotFound" $
-      case firstNameLink NTContact channelLink "" aliceNi of
+      case firstNameLink NTContact [channelLink] [] aliceNi of
         Left (ChatError (CESimplexNameNotFound ni)) -> ni `shouldBe` aliceNi
         other -> expectationFailure $ "expected CESimplexNameNotFound, got " <> show other
   -- linksMatch is the byte-equal-after-normalize comparator that gates
@@ -166,6 +181,9 @@ recording ref f srv = modifyIORef' ref (<> [srv]) >> f srv
 stubAuthThenHit :: SMPServer -> IO (Either AgentErrorType NameRecord)
 stubAuthThenHit srv = pure $ M.findWithDefault (Right sampleRecord) srv $ M.fromList [(srv1, Left authErr)]
 
+stubProhibitedThenHit :: SMPServer -> IO (Either AgentErrorType NameRecord)
+stubProhibitedThenHit srv = pure $ M.findWithDefault (Right sampleRecord) srv $ M.fromList [(srv1, Left prohibitedErr)]
+
 stubTransportThenHit :: SMPServer -> IO (Either AgentErrorType NameRecord)
 stubTransportThenHit srv = pure $ M.findWithDefault (Right sampleRecord) srv $ M.fromList [(srv1, Left networkErr)]
 
@@ -196,8 +214,8 @@ sampleRecord =
       nrNickname = "",
       nrWebsite = "",
       nrLocation = "",
-      nrSimplexContact = "",
-      nrSimplexChannel = "",
+      nrSimplexContact = [],
+      nrSimplexChannel = [],
       nrEth = Nothing,
       nrBtc = Nothing,
       nrXmr = Nothing,