From 2b2d2d4e0cb81d4665fd53d761876c1633d757dc Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Thu, 10 Sep 2026 15:33:16 +0200 Subject: [PATCH] fix regressions found reviewing the last two commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveNameMsg read the version off thParams', which on the PFWD path is the proxy's session, not the client's. It takes the version as an argument now, so each call site passes its own — the forwarded one uses fwdVersion. reservedReasonOf matched the reason words before capping, so "internal review" became NRRUnknown "internal", which encodes back as NRRInternal. Capping precedes the match, so what is kept encodes to what it decoded. Four agent tests still pinned NAME NOT_FOUND from a 404 stub, and two spec statements still described the old mapping. A registrar that does not record labels cannot answer a hashed query, which the resolver README now says. --- protocol/simplex-messaging.md | 9 +++--- scripts/resolver/README.md | 7 +++-- src/Simplex/Messaging/Names/Record.hs | 5 ++-- src/Simplex/Messaging/Protocol.hs | 2 +- src/Simplex/Messaging/Server.hs | 10 +++---- tests/AgentTests/ResolveNameTests.hs | 42 ++++++++++++++------------- 6 files changed, 41 insertions(+), 34 deletions(-) diff --git a/protocol/simplex-messaging.md b/protocol/simplex-messaging.md index 577e31acb..abf84a273 100644 --- a/protocol/simplex-messaging.md +++ b/protocol/simplex-messaging.md @@ -1495,9 +1495,10 @@ A hashed query still answers with the name. The registrar records the plaintext label when a name is registered, keyed by the hash of that label, so a router can look up what the hash stands for without ever being told. The router is not trusted for it: a client MUST check that the record names the name it asked -about, and reject the answer otherwise. A name registered without that record -answers `unknown`, which fails that check. An unregistered name has no -recorded label, so it cannot be looked up. +about, and reject the answer otherwise. A registry that does not record the +label cannot answer a hashed query at all, and the router answers `ERR NAME +RESOLVER` rather than a record it knows the client will reject. An unregistered +name has no recorded label, so it cannot be looked up either. **Server-side validation.** The names router parses `domain` as a fully-qualified name (TLD required — bare labels are rejected) and forwards it @@ -1511,7 +1512,7 @@ several configured servers can act on distinctly: | Response | Condition | Client action | |---|---|---| | `RNAME` | the router read the registry | use it | -| `ERR NAME NOT_FOUND` | the router could not read any answer for the name; below v22 also every name that does not resolve | stop, and do not read it as registrable | +| `ERR NAME NOT_FOUND` | below v22 only: every name that does not resolve. From v22 a router never sends it | stop, and do not read it as registrable | | `ERR NAME NO_RESOLVER` | this router has no resolver (names role not enabled) | skip this server, try the next | | `ERR NAME RESOLVER ` | the router cannot state an answer completely: no registrar or price oracle for the TLD, an unreachable chain, a transport failure, a timeout, a registration it could not date or resolve | surface ``; retry only if it reads as transient | diff --git a/scripts/resolver/README.md b/scripts/resolver/README.md index 6436e8004..9678453d3 100644 --- a/scripts/resolver/README.md +++ b/scripts/resolver/README.md @@ -293,8 +293,11 @@ The resolver reads three contracts, each configured per TLD. The **registry** answers who owns a node, and `/resolve` reads the records from it. The **registrar** (ERC-721) holds `nameExpires` and `GRACE_PERIOD`, which -is where every expiry field comes from. With no registrar for a TLD, `/resolve` -still works and reports `"status": "unknown"`. The **controller** holds +is where every expiry field comes from, and `labelOf`, which is how a hashed +query is answered with a name. A name registered without recording its label +cannot answer one, and `/v2/resolve` refuses it rather than answer with a name +the client will reject. With no registrar for a TLD, `/resolve` still works and +reports `"status": "unknown"`. The **controller** holds `reservedNames`, which is where `reasonCode` comes from. With no controller a held-back name reads as not reserved, and no name can be priced. diff --git a/src/Simplex/Messaging/Names/Record.hs b/src/Simplex/Messaging/Names/Record.hs index 4ca7e92f1..05800eb25 100644 --- a/src/Simplex/Messaging/Names/Record.hs +++ b/src/Simplex/Messaging/Names/Record.hs @@ -112,12 +112,13 @@ instance TextEncoding NameReservedReason where textDecode = Just . reservedReasonOf -- | An unknown reason is kept as text, capped: it reaches a client as a word. +-- Capping precedes the match, so what is kept encodes back to what it decoded. reservedReasonOf :: Text -> NameReservedReason -reservedReasonOf = \case +reservedReasonOf t = case T.take 32 $ T.takeWhile (\c -> c > ' ' && c < '\DEL') t of "internal" -> NRRInternal "trademark" -> NRRTrademark "community" -> NRRCommunity - t -> NRRUnknown $ T.take 32 $ T.takeWhile (\c -> c > ' ' && c < '\DEL') t + r -> NRRUnknown r instance ToJSON NameReservedReason where toJSON = textToJSON diff --git a/src/Simplex/Messaging/Protocol.hs b/src/Simplex/Messaging/Protocol.hs index 72ba3ba38..72ddd4aee 100644 --- a/src/Simplex/Messaging/Protocol.hs +++ b/src/Simplex/Messaging/Protocol.hs @@ -1627,7 +1627,7 @@ queryName = \case data NameErrorType = -- | the names role / resolver is not configured on this server NO_RESOLVER - | -- | the name is not registered (resolver returned not-found) + | -- | the name does not resolve; sent only to a session below v22 NOT_FOUND | -- | backing resolver/RPC failure - contains the diagnostic detail RESOLVER {resolverErr :: Text} diff --git a/src/Simplex/Messaging/Server.hs b/src/Simplex/Messaging/Server.hs index 239e52fb3..b6e73d48c 100644 --- a/src/Simplex/Messaging/Server.hs +++ b/src/Simplex/Messaging/Server.hs @@ -1493,8 +1493,8 @@ client Just nenv -> pure (Just nenv) -- Runs on a forked thread so RSLV does not block other commands; -- concurrency is limited by serverResolverConcurrency in forkCmd. - resolveNameMsg :: NamesEnv -> NameQuery -> M s BrokerMsg - resolveNameMsg nenv q = do + resolveNameMsg :: VersionSMP -> NamesEnv -> NameQuery -> M s BrokerMsg + resolveNameMsg v nenv q = do st <- asks (rslvStats . serverStats) (selector, msg) <- liftIO (resolveName nenv q) <&> \case @@ -1505,7 +1505,7 @@ client -- below v22 the encoder answers anything but a record as NAME NOT_FOUND answered = \case NRRegistered {} -> True - _ -> thVersion thParams' >= nameAvailSMPVersion + _ -> v >= nameAvailSMPVersion transportErr :: TransportError -> ErrorType transportErr = PROXY . BROKER . TRANSPORT mkIncProxyStats :: MonadIO m => ProxyStats -> ProxyStats -> OwnServer -> (ProxyStats -> IORef Int) -> m () @@ -1522,7 +1522,7 @@ client Cmd SProxyService (RFWD encBlock) -> (response . (corrId, NoEntity,) =<<) <$> processForwardedCommand encBlock Cmd SResolver (RSLV d) -> rslvNamesEnv >>= \case Nothing -> pure $ response (corrId, NoEntity, ERR (NAME NO_RESOLVER)) - Just nenv -> forkCmd serverResolverConcurrency corrId NoEntity (resolveNameMsg nenv d) + Just nenv -> forkCmd serverResolverConcurrency corrId NoEntity (resolveNameMsg (thVersion thParams') nenv d) Cmd SSenderLink command -> case command of LKEY k -> withQueue $ \q qr -> checkMode QMMessaging qr $ secureQueue_ q k $>> getQueueLink_ q qr LGET -> withQueue $ \q qr -> checkContact qr $ getQueueLink_ q qr @@ -2153,7 +2153,7 @@ client Cmd SResolver (RSLV d) -> lift $ rslvNamesEnv >>= \case Nothing -> pure $ Just (corrId', entId', ERR (NAME NO_RESOLVER)) Just nenv -> forkCmd serverResolverConcurrency corrId NoEntity $ do - msg <- resolveNameMsg nenv d + msg <- resolveNameMsg (thVersion clntTHParams) nenv d either ERR id <$> runExceptT (encodeResp (corrId', entId', msg)) -- INTERNAL because processCommand never returns Nothing for sender commands; -- `fst` drops the empty message only returned for SUB. diff --git a/tests/AgentTests/ResolveNameTests.hs b/tests/AgentTests/ResolveNameTests.hs index 8ade84e41..b6f81a647 100644 --- a/tests/AgentTests/ResolveNameTests.hs +++ b/tests/AgentTests/ResolveNameTests.hs @@ -70,13 +70,13 @@ withNoNameServers k = withAgent 1 agentCfg (oneSrv (proxySrvCfg testSMPServer)) resolveNameTests :: Spec resolveNameTests = do describe "direct path (SPMNever)" $ - it "404 propagates as SMP host (NAME NOT_FOUND)" testDirectNotFound + it "a resolver error propagates as SMP host (NAME RESOLVER)" testDirectResolverErr describe "proxy path (SPMAlways)" $ - it "404 from resolver propagates via proxy as SMP (NAME NOT_FOUND)" testProxyNotFound + it "a resolver error propagates via proxy as SMP (NAME RESOLVER)" testProxyResolverErr describe "TLDTesting path" $ - it "NAME NOT_FOUND for TLDTesting too" testTestingTldNotFound + it "NAME RESOLVER for TLDTesting too" testTestingTldResolverErr describe "TLDWeb path" $ - it "NAME NOT_FOUND for TLDWeb too" testWebTldNotFound + it "NAME RESOLVER for TLDWeb too" testWebTldResolverErr describe "no resolver configured" $ it "answers NAME NO_RESOLVER" testNoResolver describe "no names servers (names role off everywhere)" $ @@ -96,37 +96,39 @@ testAvailSuccess = Right (SMP.NRAvailable {}) -> pure () _ -> expectationFailure $ "expected Right NRAvailable, got: " <> show r -testDirectNotFound :: HasCallStack => IO () -testDirectNotFound = +-- | 404 is a resolver that predates /v2/resolve: no status from that endpoint +-- means "not registered", since an unregistered name answers NRAvailable. +testDirectResolverErr :: HasCallStack => IO () +testDirectResolverErr = withDirectResolver (status404, "{}") $ \c -> do r <- runExceptT $ resolveSimplexName c NRMInteractive 1 (SimplexDomain TLDSimplex "alice" []) case r of - Left (SMP _ (SMP.NAME SMP.NOT_FOUND)) -> pure () - _ -> expectationFailure $ "expected Left (SMP _ (NAME NOT_FOUND)), got: " <> show r + Left (SMP _ (SMP.NAME (SMP.RESOLVER _))) -> pure () + _ -> expectationFailure $ "expected Left (SMP _ (NAME (RESOLVER _))), got: " <> show r -testProxyNotFound :: HasCallStack => IO () -testProxyNotFound = +testProxyResolverErr :: HasCallStack => IO () +testProxyResolverErr = withProxyAndResolver (status404, "{}") $ \c -> do r <- runExceptT $ resolveSimplexName c NRMInteractive 1 (SimplexDomain TLDSimplex "alice" []) case r of - Left (SMP host (SMP.NAME SMP.NOT_FOUND)) | testPort `isInfixOf` host -> pure () - _ -> expectationFailure $ "expected Left (SMP testPort <> "> (NAME NOT_FOUND)), got: " <> show r + Left (SMP host (SMP.NAME (SMP.RESOLVER _))) | testPort `isInfixOf` host -> pure () + _ -> expectationFailure $ "expected Left (SMP testPort <> "> (NAME (RESOLVER _))), got: " <> show r -testTestingTldNotFound :: HasCallStack => IO () -testTestingTldNotFound = +testTestingTldResolverErr :: HasCallStack => IO () +testTestingTldResolverErr = withDirectResolver (status404, "{}") $ \c -> do r <- runExceptT $ resolveSimplexName c NRMInteractive 1 (SimplexDomain TLDTesting "bob" []) case r of - Left (SMP _ (SMP.NAME SMP.NOT_FOUND)) -> pure () - _ -> expectationFailure $ "expected Left (SMP _ (NAME NOT_FOUND)), got: " <> show r + Left (SMP _ (SMP.NAME (SMP.RESOLVER _))) -> pure () + _ -> expectationFailure $ "expected Left (SMP _ (NAME (RESOLVER _))), got: " <> show r -testWebTldNotFound :: HasCallStack => IO () -testWebTldNotFound = +testWebTldResolverErr :: HasCallStack => IO () +testWebTldResolverErr = withDirectResolver (status404, "{}") $ \c -> do r <- runExceptT $ resolveSimplexName c NRMInteractive 1 (SimplexDomain TLDWeb "example.com" []) case r of - Left (SMP _ (SMP.NAME SMP.NOT_FOUND)) -> pure () - _ -> expectationFailure $ "expected Left (SMP _ (NAME NOT_FOUND)), got: " <> show r + Left (SMP _ (SMP.NAME (SMP.RESOLVER _))) -> pure () + _ -> expectationFailure $ "expected Left (SMP _ (NAME (RESOLVER _))), got: " <> show r testNoResolver :: HasCallStack => IO () testNoResolver =