This commit is contained in:
spaced4ndy
2026-08-11 19:55:39 +04:00
parent b999b585cf
commit 247e327ebe
7 changed files with 61 additions and 11 deletions
+12 -2
View File
@@ -93,7 +93,17 @@ Because the switch is opt-in, a deployment that omits it fails silently — ever
**The address is upgradable in place; the published link does not change.** Requests need address ratchet keys or fail with `ASENotDRAddress` (agent Agent.hs:1739-1740), and `APICreateMyAddress` generates them only when `pqRatchet` is `Just` (Commands.hs:2439-2443), which the bot's start-up path is not. But `setConnShortLink` **creates keys when there are none** (agent Agent.hs:1171-1178), so `APIRotateAddressRatchetKeys` (Commands.hs:2478, `rotateKeys = True`) or `APIAddMyAddressShortLink userId (Just True)` (:2474) upgrades the existing address, reusing the same `shortLinkKey` and `linkId` (agent Agent.hs:1181-1185) so the link string is unchanged. `keepAddressKeys` retains earlier generations (:1057-1060), so rotating does not break in-flight requests. Nothing needs recreating and no published link is invalidated.
**Only the short link carries the keys.** `setMyAddressData` leaves `connFullLink` untouched (Commands.hs:4039) and that link was built with `useDR = False`, while `serviceRequest_` reads the keys off the resolved URI — so any full-link path yields `ASENotDRAddress` permanently. The app MUST use the short link (§3). Note the links shipped today are full links to the older directory address (WhatsNewView.kt:512, :575; WhatsNewView.swift:293, :355), and no directory short-link constant exists in the codebase yet — obtaining and publishing it is a prerequisite, not a code change.
**Only the short link carries the keys.** `setMyAddressData` leaves `connFullLink` untouched (Commands.hs:4039) and that link was built with `useDR = False`, while `serviceRequest_` reads the keys off the resolved URI — so any full-link path yields `ASENotDRAddress` permanently. The app MUST use the short link (§3).
The directory's short link is published in `docs/DIRECTORY.md`:
```
https://smp4.simplex.im/a#lXUjJW5vHYQzoLYgmi8GbxkGP41_kjefFvBrdwg-0Ok
```
`/a#` is the contact-address short-link form (`'A' -> CCTContact`, agent Agent/Protocol.hs:1772). The links shipped in the apps today are unrelated to this: they are full links used only as "learn more" targets on the What's New cards (WhatsNewView.kt:512, :575; WhatsNewView.swift:293, :355, the latter inlined in the localized string and so duplicated across every `.lproj`). They stay as they are.
The remaining deployment step is rotating the live address's ratchet keys so it carries DR keys. That keeps the link above unchanged, so nothing published anywhere needs updating.
Cover the upgrade with a test: `/ad`, rotate, start a responder with `service_requests=on`, expect the exchange of `testServiceRequestResponse` (Direct.hs:1903) instead of `ASENotDRAddress`. Do not extend `testServiceRequestNonDRAddress` (:1966) — it has no responder and can only observe failure.
@@ -101,7 +111,7 @@ Cover the upgrade with a test: `/ad`, rotate, start a responder with `service_re
Kotlin `CC.APISendServiceRequest(userId, target, timeoutSec, request: JsonObject)` with `cmdString` matching the parser (Commands.hs:5523 — `/_service_request <userId> <target>[ timeout=<s>][ sign_key=<k>] <json>`; the request is never signed, see §1), a `CR.ServiceResponse` case, and `apiSearchDirectory(rh, text, cursor)` wrapping the envelope. iOS: the same as a `ChatCommand` case plus a response case in `ChatResponse1` (AppAPITypes.swift:819), which already carries the command-result cases.
The app ships the directory's **short link** as a constant; it cannot cache a resolved target, since `APISendServiceRequest` resolves internally and returns only the response.
`APISendServiceRequest` stays general — it takes the target, and will carry requests to services other than the directory. The directory's **short link** (§2) is therefore a constant in the app, declared once per platform in Kotlin and Swift and referenced only by `apiSearchDirectory`. Two copies is the accepted cost of keeping the command address-agnostic; do not push the address down into the core or the command. The app cannot cache a resolved target either, since `APISendServiceRequest` resolves internally and returns only the response.
The call blocks until reply or timeout — pass `requestTimeout` of **10 s**, and use `withLongRunningApi` (Utils.kt:43), not the single-threaded `withBGApi` (:38). Show progress through the existing `ConnectProgressManager` (ChatModel.kt:55-78, ChatModel.swift:303-329): `startConnectProgress(text, onCancel)` when the request goes out, `stopConnectProgress()` when it returns. It already withholds the spinner for 1 s, and the search bar already renders it (ChatListView.kt:522, ChatListView.swift:670); `onCancel` gives the user a way out of the wait.
+2 -2
View File
@@ -87,10 +87,10 @@ simplexChatCore cfg@ChatConfig {confirmMigrations, testView, chatHooks} opts@Cha
exitFailure
runSimplexChat :: ChatConfig -> ChatOpts -> User -> ChatController -> (User -> ChatController -> IO ()) -> IO ()
runSimplexChat ChatConfig {testView} ChatOpts {coreOptions = CoreChatOpts {chatRelay, chatRelayServer, headless, maintenance}} u cc@ChatController {config = ChatConfig {chatHooks}} chat
runSimplexChat ChatConfig {testView} ChatOpts {coreOptions = CoreChatOpts {chatRelay, chatRelayServer, headless, serviceRequests, maintenance}} u cc@ChatController {config = ChatConfig {chatHooks}} chat
| maintenance = wait =<< async (chat u cc)
| otherwise = do
a1 <- runReaderT (startChatController True True False) cc
a1 <- runReaderT (startChatController True True serviceRequests) cc
when (chatRelay && not testView) $ askCreateRelayAddress cc u chatRelayServer headless
forM_ (postStartHook chatHooks) ($ cc)
a2 <- async $ chat u cc
+1
View File
@@ -265,6 +265,7 @@ mobileChatOpts dbOptions =
chatRelayServer = Nothing,
headless = False,
highlyAvailable = False,
serviceRequests = False,
yesToUpMigrations = False,
migrationBackupPath = Just "",
maintenance = True
+8
View File
@@ -73,6 +73,7 @@ data CoreChatOpts = CoreChatOpts
chatRelayServer :: Maybe SMPServerWithAuth,
headless :: Bool,
highlyAvailable :: Bool,
serviceRequests :: Bool,
yesToUpMigrations :: Bool,
migrationBackupPath :: Maybe FilePath,
maintenance :: Bool
@@ -304,6 +305,12 @@ coreChatOptsP appDir defaultDbName = do
( long "ha"
<> help "Run as a highly available client (this may increase traffic in groups)"
)
-- TODO [directory] default this on for the directory binary, so a deployment cannot omit it
serviceRequests <-
switch
( long "service-requests"
<> help "Process service requests received on the address (requires an address with DR keys)"
)
yesToUpMigrations <-
switch
( long "yes-migrate"
@@ -350,6 +357,7 @@ coreChatOptsP appDir defaultDbName = do
True | not chatRelay -> errorWithoutStackTrace "--headless option requires --relay option"
_ -> headless,
highlyAvailable,
serviceRequests,
yesToUpMigrations,
migrationBackupPath,
maintenance
+2 -1
View File
@@ -115,7 +115,8 @@ mkDirectoryOpts TestParams {tmpPath = ps} superUsers ownersGroup webFolder =
DirectoryOpts
{ coreOptions =
testCoreOpts
{ dbOptions =
{ serviceRequests = True,
dbOptions =
(dbOptions testCoreOpts)
#if defined(dbPostgres)
{dbSchemaPrefix = "client_" <> serviceDbPrefix}
+1
View File
@@ -165,6 +165,7 @@ testCoreOpts =
chatRelayServer = Nothing,
headless = False,
highlyAvailable = False,
serviceRequests = False,
yesToUpMigrations = False,
migrationBackupPath = Nothing,
maintenance = False
+35 -6
View File
@@ -125,6 +125,7 @@ chatDirectTests = do
it "signed service request delivers the verified key" testSignedServiceRequest
it "service request dropped when service processing is off" testServiceRequestDroppedWhenOff
it "service request to a non-DR address fails fast" testServiceRequestNonDRAddress
it "service request to an address upgraded by key rotation" testServiceRequestUpgradedAddress
it "create user with same servers" testCreateUserSameServers
it "delete user" testDeleteUser
it "delete user with chat tags" testDeleteUserChatTags
@@ -1921,9 +1922,6 @@ testServiceRequestResponse =
replyConnId <- serviceReplyConnId alice
alice <## ("service reply sent, connection id: " <> replyConnId)
)
where
serviceRequestId cc = getTermLine cc >>= maybe (serviceRequestId cc) pure . stripPrefix "service request "
serviceReplyConnId cc = getTermLine cc >>= maybe (serviceReplyConnId cc) pure . stripPrefix "service reply accepted, connection id: "
testSignedServiceRequest :: HasCallStack => TestParams -> IO ()
testSignedServiceRequest =
@@ -1951,9 +1949,12 @@ testSignedServiceRequest =
replyConnId <- serviceReplyConnId alice
alice <## ("service reply sent, connection id: " <> replyConnId)
)
where
serviceRequestId cc = getTermLine cc >>= maybe (serviceRequestId cc) pure . stripPrefix "service request "
serviceReplyConnId cc = getTermLine cc >>= maybe (serviceReplyConnId cc) pure . stripPrefix "service reply accepted, connection id: "
serviceRequestId :: HasCallStack => TestCC -> IO String
serviceRequestId cc = getTermLine cc >>= maybe (serviceRequestId cc) pure . stripPrefix "service request "
serviceReplyConnId :: HasCallStack => TestCC -> IO String
serviceReplyConnId cc = getTermLine cc >>= maybe (serviceReplyConnId cc) pure . stripPrefix "service reply accepted, connection id: "
testServiceRequestDroppedWhenOff :: HasCallStack => TestParams -> IO ()
testServiceRequestDroppedWhenOff =
@@ -1971,6 +1972,34 @@ testServiceRequestNonDRAddress =
bob ##> ("/_service_request 1 " <> sLink <> " {\"ping\":1}")
bob <## "smp agent error: AGENT {agentErr = A_SERVICE {serviceError = ASENotDRAddress}}"
-- an address created without DR keys gets them from a key rotation, and keeps the same link,
-- so a published address can be upgraded in place rather than replaced
testServiceRequestUpgradedAddress :: HasCallStack => TestParams -> IO ()
testServiceRequestUpgradedAddress =
testChat2 aliceProfile bobProfile $ \alice bob -> do
alice ##> "/ad"
(sLink, _) <- getContactLinks alice True
alice ##> "/_rotate_address_keys 1"
(sLink', _) <- getContactLinks alice False
alice <## "auto_accept off"
sLink' `shouldBe` sLink
alice ##> "/_stop"
alice <## "chat stopped"
alice ##> "/_start main=on snd_files=on service_requests=on"
alice <## "chat started"
concurrently_
( do
bob ##> ("/_service_request 1 " <> sLink <> " {\"ping\":1}")
bob <## "service response: {\"pong\":2}"
)
( do
reqId <- serviceRequestId alice
alice <## "request: {\"ping\":1}"
alice ##> ("/_service_response 1 " <> reqId <> " {\"pong\":2}")
replyConnId <- serviceReplyConnId alice
alice <## ("service reply sent, connection id: " <> replyConnId)
)
testMultipleUserAddresses :: HasCallStack => TestParams -> IO ()
testMultipleUserAddresses =
testChat3 aliceProfile bobProfile cathProfile $