From a3fb63ff06eab61be0cc68e5048bbd5e7aab52ea Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Thu, 10 Sep 2026 14:42:19 +0200 Subject: [PATCH] fix review findings --- protocol/simplex-messaging.md | 16 +++--- scripts/resolver/README.md | 53 +++++++++---------- scripts/resolver/service/snrc-resolve.py | 10 ++-- src/Simplex/Messaging/Agent.hs | 2 +- src/Simplex/Messaging/Agent/Client.hs | 2 +- src/Simplex/Messaging/Client.hs | 8 +-- src/Simplex/Messaging/Names/Record.hs | 6 +-- src/Simplex/Messaging/Protocol.hs | 3 -- .../Messaging/Server/Names/HttpResolver.hs | 17 +----- src/Simplex/Messaging/SimplexName.hs | 16 ------ src/Simplex/Messaging/SystemTime.hs | 3 +- tests/SMPNamesTests.hs | 4 +- 12 files changed, 48 insertions(+), 92 deletions(-) diff --git a/protocol/simplex-messaging.md b/protocol/simplex-messaging.md index c864889fd..dbbfd6000 100644 --- a/protocol/simplex-messaging.md +++ b/protocol/simplex-messaging.md @@ -1496,9 +1496,8 @@ label when a name is registered, keyed by the hash of that label, so a router ca 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. What stays impossible is learning a -name that is *not* registered: there is nothing recorded to look up, so a name -someone is merely considering never becomes known. +answers `unknown`, which fails that check. An unregistered name has no +recorded label, so it cannot be looked up. **Server-side validation.** The names router parses `domain` as a fully-qualified name (TLD required — bare labels are rejected) and forwards it @@ -1582,8 +1581,7 @@ honour. That is why `reserved` has no pricing field. The record is carried while a name is registered and through its grace period, and stops once the name is registrable by anyone. Keeping it through grace lets whoever opens the name tell its owner that it is about to lapse; keeping it -longer would show a record whose owner no longer holds the name. How long a -client goes on opening an expiring name is its own decision. +longer would show a record whose owner no longer holds the name. **Computing the price.** In US cents, for a duration in seconds: @@ -1596,8 +1594,8 @@ The registry's minimum registration is 730 days, a contract constant rather than a per-deployment value, so it is specified here rather than sent. `registrationPrices` omits any length below `minLabelLength`, those being unregistrable. `minLabelLength` is sent because a hashed query carries no -length: the router cannot check it, so the client must, and a price quoted for a -label the registry will refuse is worse than no quote at all. +length: the router cannot check it, so the client must, and a quote for a label +the registry refuses must not be shown. Below v22, `RNAME` carries the bare record and nothing else, and every answer without one is `ERR NAME NOT_FOUND`, as it was before this version. A name in @@ -1614,9 +1612,7 @@ A router that cannot state an answer completely MUST say so as `ERR NAME RESOLVER ` rather than answer partially. That covers a TLD with no registrar or no price oracle configured, an unreachable chain, a timeout, a registration it could not date, a registered name it could not resolve, and any -status word it does not recognise. Neither a registration nor availability may -be guessed: one would assert a registration nobody read, the other would offer a -name that may be held. +status word it does not recognise. A router MUST NOT guess either. A client MUST read a `reason` it does not know as unknown and still treat the name as reserved: a later version may reserve names for reasons this one cannot diff --git a/scripts/resolver/README.md b/scripts/resolver/README.md index 59e650215..b2394fc12 100644 --- a/scripts/resolver/README.md +++ b/scripts/resolver/README.md @@ -19,15 +19,15 @@ against **Ethereum mainnet** (where the `.testing` contracts live): ## 1. Configure -Edit `.env`. The defaults work as they are; change them only if you need to: +Edit `.env` — the defaults work as-is; override only if needed: ```sh NETWORK=mainnet # default TRUSTED_NODE_URL=https://mainnet-checkpoint-sync.attestant.io # default ``` -Everything else (NAT) already has a working default in `docker-compose.yml`. -Uncomment the hints in `.env` only if you need to change one. +Everything else (NAT) has a working default baked into `docker-compose.yml`; +uncomment the hints in `.env` only to override. ## 2. Run @@ -37,7 +37,7 @@ docker compose up -d docker compose logs -f reth resolver ``` -Compose starts the node before the resolver; `depends_on` takes care of that. +`depends_on` handles ordering automatically (start node → start resolver). ## 3. Wait for the node to sync @@ -45,13 +45,12 @@ Compose starts the node before the resolver; `depends_on` takes care of that. docker compose logs --tail=20 reth ``` -This is the slow step: about a day on mainnet. Until reth has synced, the -resolver returns `502`. +This is the long pole (~1 day on mainnet). Until reth is synced the resolver +returns `502`. ## Verify -Run the three checks below once the stack is up. The ones that need chain data -pass only after the node has synced. +Run these once the stack is up (the node-dependent ones pass after sync): **1. reth is reachable and reporting a block:** ```sh @@ -72,7 +71,7 @@ curl -s http://127.0.0.1:8000/resolve/foobar.testing | jq # → {"name":"foobar.testing","nickname":"Foo","simplexContact":["https://smp16.simplex.im/a#…"], … } ``` -**Point your smp-server at it:** in its `[NAMES]` section set +**Wire your smp-server:** in its `[NAMES]` section set `resolver_endpoint: http://127.0.0.1:8000` (no auth needed for loopback). ## Ports (all loopback unless noted) @@ -87,10 +86,9 @@ curl -s http://127.0.0.1:8000/resolve/foobar.testing | jq ## Caveats -- **All images track `:latest`** (reth, nimbus). Each `docker compose pull` - brings upstream fixes, so re-run the checks above afterwards. -- All ports bind to loopback. Expose only what you put behind a TLS reverse - proxy. +- **All images track `:latest`** (reth, nimbus) — you get upstream fixes on each + `docker compose pull`; re-run the verify checks after pulling. +- All ports bind to loopback; expose only what you put behind a TLS reverse proxy. ## Teardown @@ -105,9 +103,8 @@ docker compose down -v # also wipe volumes → full re-sync ## Resolver API reference -You can also run the resolver (`snrc-resolve.py`, host `127.0.0.1:8000`) on its -own for local development, without Docker, using -[`uv`](https://docs.astral.sh/uv/): +The resolver (`snrc-resolve.py`, host `127.0.0.1:8000`) is also runnable +standalone for local dev (no Docker), via [`uv`](https://docs.astral.sh/uv/): ```sh uv run scripts/resolver/service/snrc-resolve.py # defaults to local reth + mainnet .testing @@ -131,12 +128,11 @@ uv run scripts/resolver/service/snrc-resolve.py # defaults to local reth + main } ``` -`simplexContact` and `simplexChannel` are arrays, because a name can advertise -several SMP servers; clients try them in order. On chain each one is a single -text record with the entries joined by `;`. The resolver splits that record, -trims each entry and drops the empty ones. Addresses come back in each chain's -usual format (EIP-55, bech32, SS58, Monero base58). Subnames work the same way -(`bar.foobar.testing`). +`simplexContact`/`simplexChannel` are arrays (a name can advertise multiple SMP +servers; clients try them in order). On-chain they're a single `;`-separated +text record; the resolver splits/trims/drops-empties. Address encodings are +canonical per chain (EIP-55 / bech32 / SS58 / Monero-base58). Subnames work +identically (`bar.foobar.testing`). ### Registration status and expiry @@ -179,9 +175,9 @@ per second, and charges a premium on a lapsed name that it does not expose. A quote from one is therefore only safe for a name that was never registered: an `expired` name gets no price rather than one below what the registrar charges. -Deployment constants - the grace period, the oracle and its curve - are cached -for `CONSTANTS_TTL` (5 minutes), so a retune shows up within that. Per-name -values are read on every query. +The grace period, the oracle and its curve are cached for `CONSTANTS_TTL` +(5 minutes), so a retune shows up within that. Per-name values are read on +every query. **Set `SNRC_CONTROLLER_` wherever `SNRC_REGISTRAR_` is.** Without a controller there is no oracle, so no name can be priced. @@ -219,8 +215,9 @@ returns the same record. The registrar keys `nameExpires` and `reservedNames` on the labelhash too, so the status fields do not need the label either. The resolver learns the name only by guessing the label and hashing it. -Only the second-level label is a registry key, and it is decoded wherever it -sits: `sub.[].testing` reaches the node `sub.name.testing` does. Subname +Only the second-level label is a registry key, and `status` decodes a bracket +there at any depth. The record does not: a bracket is decoded only in a +two-label name, so `sub.[].testing` is not a supported query. Subname labels stay text; a bracket label left of the 2LD is an ordinary label. Routers from v22 send every 2LD this way, so a registrable name normally never reaches this service. @@ -309,4 +306,4 @@ here. To override any of them, set `SNRC_REGISTRY_`, `SNRC_REGISTRAR_` or `SNRC_CONTROLLER_` on the `resolver` service in `docker-compose.yml`, or -as env vars when you run the script directly. \ No newline at end of file +as env vars when you run the script directly. diff --git a/scripts/resolver/service/snrc-resolve.py b/scripts/resolver/service/snrc-resolve.py index e9bafc66d..aef8baf88 100755 --- a/scripts/resolver/service/snrc-resolve.py +++ b/scripts/resolver/service/snrc-resolve.py @@ -190,9 +190,9 @@ def chain_now() -> int: return decode_uint(block["timestamp"]) -# Deployment constants - the grace period, the oracle and its curve - change only -# when the owner retunes a contract, so they are read once per TTL rather than on -# every query. Per-name values and the decaying premium are never cached. +# The grace period, the oracle and its curve change only when a contract is +# retuned, so they are read once per TTL rather than on every query. Per-name +# values are never cached. CONSTANTS_TTL = 300 _constants: dict = {} @@ -331,8 +331,8 @@ def name_status(name: str): } # nameExpires and reservedNames are keyed on uint256(keccak(label)). - # Only the 2LD's label is a registry key, wherever it sits - the same rule - # node_of applies to the node. + # The 2LD's label is that key at any depth. node_of decodes a bracket only + # in a two-label name, so a bracket subname gets a status but no record. token = label_token(labels[-2]) expires = decode_uint( eth_call(registrar, selector("nameExpires(uint256)") + encode_uint(token)) diff --git a/src/Simplex/Messaging/Agent.hs b/src/Simplex/Messaging/Agent.hs index 552996d1b..b1e791d6d 100644 --- a/src/Simplex/Messaging/Agent.hs +++ b/src/Simplex/Messaging/Agent.hs @@ -228,7 +228,7 @@ import Simplex.Messaging.Protocol ErrorType (AUTH), MsgBody, MsgFlags (..), - NameRegistration (..), + NameRegistration, NtfServer, ProtoServerWithAuth (..), ProtocolServer (..), diff --git a/src/Simplex/Messaging/Agent/Client.hs b/src/Simplex/Messaging/Agent/Client.hs index b1ee8c011..d34fb55b9 100644 --- a/src/Simplex/Messaging/Agent/Client.hs +++ b/src/Simplex/Messaging/Agent/Client.hs @@ -272,7 +272,7 @@ import Simplex.Messaging.Protocol NetworkError (..), MsgFlags (..), MsgId, - NameRegistration (..), + NameRegistration, NtfServer, NtfServerWithAuth, ProtoServer, diff --git a/src/Simplex/Messaging/Client.hs b/src/Simplex/Messaging/Client.hs index a76273f47..8c051e3eb 100644 --- a/src/Simplex/Messaging/Client.hs +++ b/src/Simplex/Messaging/Client.hs @@ -1056,14 +1056,12 @@ proxySMPMessage c nm proxiedRelay spKey sId flags msg = proxyOKSMPCommand c nm p -- the client never sends RSLV to a relay that predates names support. proxyResolveName :: SMPClient -> NetworkRequestMode -> ProxiedRelay -> SimplexDomain -> ExceptT SMPClientError IO (Either ProxyClientError NameRegistration) proxyResolveName c nm proxiedRelay name - | v >= namesSMPVersion = + | prVersion proxiedRelay >= namesSMPVersion = proxySMPCommand c nm proxiedRelay Nothing NoEntity (RSLV (NQDomain name)) >>= \case Right (RNAME reg) | resolvedName name reg -> pure $ Right reg Right r -> throwE $ unexpectedResponse r Left e -> pure $ Left e | otherwise = throwE $ PCETransportError TEVersion - where - v = prVersion proxiedRelay -- | Direct (non-PFWD) name resolution. Exposes the client IP to the resolver; -- callers that want anonymity should use `proxyResolveName` via the standard @@ -1072,13 +1070,11 @@ proxyResolveName c nm proxiedRelay name -- the server has no RSLV at all; the encoder gates the query format separately. directResolveName :: SMPClient -> NetworkRequestMode -> SimplexDomain -> ExceptT SMPClientError IO NameRegistration directResolveName c nm name - | v >= namesSMPVersion = + | thVersion (thParams c) >= namesSMPVersion = sendProtocolCommand c nm Nothing NoEntity (Cmd SResolver (RSLV (NQDomain name))) >>= \case RNAME reg | resolvedName name reg -> pure reg r -> throwE $ unexpectedResponse r | otherwise = throwE $ PCETransportError TEVersion - where - v = thVersion (thParams c) -- | The record must name the name that was asked for: a hashed query does not -- tell the router which name it is, so the router is not trusted for it. diff --git a/src/Simplex/Messaging/Names/Record.hs b/src/Simplex/Messaging/Names/Record.hs index 5a76c3445..499038365 100644 --- a/src/Simplex/Messaging/Names/Record.hs +++ b/src/Simplex/Messaging/Names/Record.hs @@ -58,7 +58,7 @@ $( JQ.deriveJSON ''NameRecord ) --- | US cents, rounded up so a quote is never below what is charged. +-- | US cents. newtype USDCents = USDCents Int64 deriving (Eq, Ord, Show) deriving newtype (ToJSON, FromJSON) @@ -110,7 +110,7 @@ instance TextEncoding NameReservedReason where NRRUnknown t -> t textDecode = Just . reservedReasonOf --- | A reason this version has no word for keeps its own. +-- | An unknown reason is kept as text. reservedReasonOf :: Text -> NameReservedReason reservedReasonOf = \case "internal" -> NRRInternal @@ -125,7 +125,7 @@ instance ToJSON NameReservedReason where instance FromJSON NameReservedReason where parseJSON = textParseJSON "NameReservedReason" --- | What a v20/v21 router's answer amounts to. +-- | A v20/v21 router's record as a registration. oldRegistration :: NameRecord -> NameRegistration oldRegistration nameRecord = NRRegistered {expires = Nothing, graceUntil = Nothing, reservedReason_ = Nothing, nameRecord} diff --git a/src/Simplex/Messaging/Protocol.hs b/src/Simplex/Messaging/Protocol.hs index c5b6f9c35..72ba3ba38 100644 --- a/src/Simplex/Messaging/Protocol.hs +++ b/src/Simplex/Messaging/Protocol.hs @@ -255,8 +255,6 @@ import Data.Kind import Data.List (find, foldl') import Data.List.NonEmpty (NonEmpty (..)) import qualified Data.List.NonEmpty as L -import Data.Map.Strict (Map) -import qualified Data.Map.Strict as M import Data.Maybe (isJust, isNothing) import Data.String import Data.Text (Text) @@ -279,7 +277,6 @@ import Simplex.Messaging.Parsers import Simplex.Messaging.Protocol.Types import Simplex.Messaging.Server.QueueStore.QueueInfo import Simplex.Messaging.ServiceScheme -import Simplex.Messaging.SystemTime (SystemSeconds) import Simplex.Messaging.SimplexName (LabelHash, SimplexDomain (..), SimplexTLD (..), boundedNonSpace, fullDomainName, labelHash, labelHashOfText, labelHashText, tldSuffix) import Simplex.Messaging.Transport import Simplex.Messaging.Transport.Client (TransportHost, TransportHosts (..)) diff --git a/src/Simplex/Messaging/Server/Names/HttpResolver.hs b/src/Simplex/Messaging/Server/Names/HttpResolver.hs index 1ed57a6b3..881fe664b 100644 --- a/src/Simplex/Messaging/Server/Names/HttpResolver.hs +++ b/src/Simplex/Messaging/Server/Names/HttpResolver.hs @@ -3,8 +3,6 @@ {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE ScopedTypeVariables #-} {-# LANGUAGE StrictData #-} -{-# LANGUAGE TemplateHaskell #-} -{-# LANGUAGE TupleSections #-} -- | HTTP transport for the public-namespace resolver. -- @@ -37,17 +35,11 @@ where import qualified Control.Exception as E import qualified Data.Aeson as J -import Data.Aeson.Key (Key) -import qualified Data.Aeson.KeyMap as JKM -import qualified Data.Aeson.TH as JQ -import qualified Data.Aeson.Types as JT import Data.Bifunctor (first) import qualified Data.ByteArray.Encoding as BAE import Data.ByteString.Char8 (ByteString) import qualified Data.ByteString.Char8 as B import qualified Data.ByteString.Lazy as BL -import Data.Int (Int64) -import Data.Map.Strict (Map) import Data.Text (Text) import Data.Text.Encoding (encodeUtf8) import Network.HTTP.Client @@ -68,7 +60,6 @@ import Network.HTTP.Client.TLS (tlsManagerSettings) import qualified Network.HTTP.Types as HT import Network.HTTP.Types.URI (urlEncode) import Simplex.Messaging.Names.Record (NameRegistration) -import Simplex.Messaging.Parsers (defaultJSON, dropPrefix) data RpcAuth = AuthBearer Text | AuthBasic Text Text @@ -119,12 +110,8 @@ authHeader = \case let encoded = BAE.convertToBase BAE.Base64 (encodeUtf8 u <> ":" <> encodeUtf8 p) :: ByteString in ("Authorization", "Basic " <> encoded) --- | GET /v2/resolve/, which answers with --- NameRegistration JSON. v1 is /resolve, which answers with a NameRecord and is --- what relays before SMP v22 call; the resolver API is versioned separately from --- the protocol, so it only changes when its own shape does. The query is a name, --- or a bracketed label hash, percent-encoded (every non-unreserved byte per RFC --- 3986) so slashes and punctuation cannot alter the path. +-- | The query is a name or a bracketed label hash, percent-encoded (every +-- non-unreserved byte per RFC 3986) so it cannot alter the path. resolveHttp :: ResolverEnv -> Text -> IO (Either ResolverError NameRegistration) resolveHttp env q = (>>= first InvalidJson . J.eitherDecodeStrict . BL.toStrict) diff --git a/src/Simplex/Messaging/SimplexName.hs b/src/Simplex/Messaging/SimplexName.hs index 84b203004..dcde97463 100644 --- a/src/Simplex/Messaging/SimplexName.hs +++ b/src/Simplex/Messaging/SimplexName.hs @@ -84,10 +84,6 @@ nameLabelP = do newtype LabelHash = LabelHash ByteString deriving (Eq, Show) -instance Encoding LabelHash where - smpEncode (LabelHash h) = h - smpP = LabelHash <$> A.take 32 - -- | keccak-256 of the lowercased label, as the registry keys it. labelHash :: Text -> LabelHash labelHash label = LabelHash $ BA.convert (hash (encodeUtf8 (T.toLower label)) :: Digest Keccak_256) @@ -140,18 +136,6 @@ instance Encoding SimplexDomain where smpEncode = strEncode smpP = strP -instance Encoding SimplexTLD where - smpEncode = \case - TLDSimplex -> "s" - TLDTesting -> "t" - TLDWeb -> "w" - smpP = - A.anyChar >>= \case - 's' -> pure TLDSimplex - 't' -> pure TLDTesting - 'w' -> pure TLDWeb - _ -> fail "bad SimplexTLD" - fullDomainName :: SimplexDomain -> Text fullDomainName SimplexDomain {nameTLD, domain, subDomain} = T.intercalate "." (reverse subDomain ++ [domain] ++ tld') where diff --git a/src/Simplex/Messaging/SystemTime.hs b/src/Simplex/Messaging/SystemTime.hs index 18d5ffb50..d53a60aa1 100644 --- a/src/Simplex/Messaging/SystemTime.hs +++ b/src/Simplex/Messaging/SystemTime.hs @@ -23,12 +23,11 @@ import Data.Time.Clock.System (SystemTime (..), getSystemTime, systemToUTCTime) import Data.Typeable (Proxy (..)) import GHC.TypeLits (KnownNat, Nat, natVal) import Simplex.Messaging.Agent.Store.DB (FromField (..), ToField (..)) -import Simplex.Messaging.Encoding (Encoding) import Simplex.Messaging.Encoding.String newtype RoundedSystemTime (t :: Nat) = RoundedSystemTime {roundedSeconds :: Int64} deriving (Eq, Ord, Show) - deriving newtype (Encoding, FromJSON, ToJSON, FromField, ToField) + deriving newtype (FromJSON, ToJSON, FromField, ToField) type SystemDate = RoundedSystemTime 86400 diff --git a/tests/SMPNamesTests.hs b/tests/SMPNamesTests.hs index 26857ef95..3f0be57b5 100644 --- a/tests/SMPNamesTests.hs +++ b/tests/SMPNamesTests.hs @@ -17,7 +17,7 @@ import Data.Text.Encoding (encodeUtf8) import Network.HTTP.Types (status200, status400, status404, status410, status500, status502) import NamesResolverServer (resolveResp, testNamesConfig, withResolverServer, withResolverServerDelayed) import Simplex.Messaging.Encoding (smpDecode, smpEncode) -import Simplex.Messaging.Encoding.String (strDecode, strEncode) +import Simplex.Messaging.Encoding.String (strDecode) import Simplex.Messaging.Protocol (Command (..), ErrorType (..), NameErrorType (..), NamePricing (..), NameQuery (..), NameRecord (..), NameRegistration (..), NameReservedReason (..), ProtocolEncoding (..), USDCents (..)) import Simplex.Messaging.Server.Main (validateUrl) import Simplex.Messaging.Server.Names @@ -28,7 +28,7 @@ import Simplex.Messaging.Server.Names resolveName, ) import Simplex.Messaging.Server.Names.HttpResolver (ResolverError (..)) -import Simplex.Messaging.SimplexName (SimplexDomain (..), SimplexTLD (..), fullDomainName, labelHash) +import Simplex.Messaging.SimplexName (SimplexDomain (..), SimplexTLD (..), labelHash) import Simplex.Messaging.SystemTime (RoundedSystemTime (..)) import Simplex.Messaging.Transport (nameAvailSMPVersion, serverInfoSMPVersion) import Test.Hspec