From 18b0dafec60554c3c84f51f370ba26307f874c7f Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Wed, 26 Aug 2026 20:24:08 -0400 Subject: [PATCH] feat(tor): route the app's stand-in relays like the user's own until their lists arrive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A brand-new install routes 100% of its relay traffic over Tor by construction, and that is a chicken-and-egg rather than a preference: `trustedRelays` is empty, so `TorRelayEvaluation` falls through to `newRelaysViaTor` (default true) for every url — and the kind:10002 that would populate it can only be fetched over Tor. Measured on a Samsung SM-T220, same account, same login timing, fresh install each: the first relay socket opened 2.3-2.9s *after* Tor became ready, whenever that happened to be, and Arti's directory download ran 12.6-51.7s. While an account's own lists are unknown, the defaults the app is already dialling are now also classified for Tor purposes — as `assumed` relays, the last branch before `newRelaysViaTor`: first relay socket, vs when Tor became ready (n=3 each, counterbalanced) before: login+5.87s / +7.89s — always 2.3-2.9s AFTER Tor Active after: login+1.21s / +1.24s / +1.29s — independent of Tor entirely events ingested by the 20s census, non-overlapping before: 0 / 892 / 1159 / 2590 after: 3719 / 3997 / 4081 / 5311 / 6051 It resolves to `trustedRelaysViaTor`, not to a hardcoded false: the app's stand-in for a list gets the policy the user chose for their own list, so anyone who set that preference keeps Tor here with nothing new to discover. And it sits below .onion, money-operation and DM in the precedence chain, so those keep their own policy for free — the branch can only capture urls that would have been treated as strangers. The guess ends by itself. `assumedDefaults` keys on the *event* being absent — never on a list being empty, which is a choice we honor — so each list's contribution empties the moment that event lands, with no window, timeout or per-account bookkeeping. Device log: `Guessed relays: 15 -> 10 -> 5 -> 0 (own lists arrived; released to their real Tor policy)`, after which 28 relays re-dialled and their connect latency moved from a median 116ms to 503ms — the handover onto Tor circuits, visible in the timings. Deliberately NOT merged into `TrustedRelayListsState`. That feeds `Account.isInMyRelayList` -> `RelayAuthPermissionLedger` -> `RelayAuthResolver`, i.e. the NIP-42 AUTH decision. Guessed relays must never make the app sign an AUTH challenge as though they were the user's own; that would turn a timing signal into a signed identity assertion. Tor routing is the only consumer. Two supporting changes, both of which pay for themselves here: `RelayClassification` groups the four category sets into one value. The reconnect trigger in `RelayProxyClientConnector` used to compare them field by field, so a new category meant remembering another `||` — and I had forgotten it, which is exactly the silent failure it invites: relays keep a socket on a transport the policy has already moved them off. It is now one structural comparison. That also removes a `Pair` that existed only to squeeze past `combineTransform`'s five-source limit. Regression test covers the case that made the omission reachable: an *empty* arriving list, where `trusted` does not change while `assumed` empties. `AccountsTorStateConnector.unionAcrossAccounts` replaces four ~30-line copies of the same per-account fold. The copies had already drifted — two carried an `if (isEmpty)` guard that could never fire, since `ifEmpty` had just guaranteed otherwise. Verified byte-identical to the build these numbers were measured on, and re-measured after the refactors: first socket 1.24s median vs 1.21s before, fully overlapping. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BKYGEp22uGSzWrBDg8fAQ9 --- ...otstrap-relays-skip-tor-until-user-data.md | 182 ++++++++++++++++ .../vitorpamplona/amethyst/model/Account.kt | 4 + .../indexerRelays/IndexerRelayListState.kt | 14 ++ .../searchRelays/SearchRelayListState.kt | 14 ++ .../nip65RelayList/Nip65RelayListState.kt | 21 ++ .../serverList/AssumedRelayListsState.kt | 68 ++++++ .../torState/AccountsTorStateConnector.kt | 205 ++++++++++-------- .../model/torState/TorRelayEvaluation.kt | 2 + .../amethyst/model/torState/TorRelayState.kt | 75 ++++--- .../relayClient/RelayProxyClientConnector.kt | 26 +-- .../model/torState/TorRelayEvaluationTest.kt | 7 +- .../RelayProxyClientConnectorTest.kt | 46 +++- .../relayGroup/TorClearnetFallbackTest.kt | 8 +- .../commons/tor/TorRelayEvaluation.kt | 39 +++- .../commons/tor/TorRelayEvaluationTest.kt | 65 +++++- .../commons/tor/YggdrasilTorRoutingTest.kt | 7 +- .../vitorpamplona/amethyst/desktop/Main.kt | 6 +- 17 files changed, 627 insertions(+), 162 deletions(-) create mode 100644 amethyst/plans/2026-08-26-bootstrap-relays-skip-tor-until-user-data.md create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/model/serverList/AssumedRelayListsState.kt diff --git a/amethyst/plans/2026-08-26-bootstrap-relays-skip-tor-until-user-data.md b/amethyst/plans/2026-08-26-bootstrap-relays-skip-tor-until-user-data.md new file mode 100644 index 0000000000..67f7e1859f --- /dev/null +++ b/amethyst/plans/2026-08-26-bootstrap-relays-skip-tor-until-user-data.md @@ -0,0 +1,182 @@ +# Defaults stand in for the user's relay lists only while we have no event + +**Status:** proposal — not implemented +**Goal:** first-login startup on a Tor-enabled install +**Related:** `fix/tor-bootstrap-stall-and-ondemand`, `[[fresh-install-routes-everything-via-tor]]` + +## The rule + +Three states, currently collapsed into two: + +| we have | effective list | today | +|---|---|---| +| **no event** for the user | app defaults | defaults ✅ | +| event, **empty** list | **empty** — the user chose nothing | defaults ❌ | +| event with relays | those relays | those relays ✅ | + +Everything below follows from separating "we don't know" from "we know, and it's nothing". + +## Why the first login is slow + +On a fresh install **100% of relay traffic is Tor-routed by construction**. +`TorRelayState.trustedRelays` is empty, so `TorRelayEvaluation.useTor()` falls through to +`newRelaysViaTor` (**default true**) for every URL — and the kind-10002 that would populate it can +only be fetched over Tor. Measured (SM-T220, same account, same ~app+8-10s login, fresh install +each; the Tor-OFF arm sets the pref, force-stops, then starts the timed run so Arti never boots): + +| @20s census | Tor ON | Tor OFF | +|---|---|---| +| feed on screen | login+18s | **login+11s** | +| relays opened | 18/40 | **32/41** | +| relays serving events | 9 | **22** | +| events ingested | 2,830 | **6,134 / 7,641** | + +≈7s of first paint and half the relay coverage. + +## Finding 1 — every `WithBackup` helper keys on emptiness, not absence + +This is a pre-existing bug against the rule above, and it must be fixed first because the whole +feature depends on the distinction being real. + +```kotlin +// AdvertisedRelayListEvent +fun relays() = tags.mapNotNull(AdvertisedRelayInfo::parse) // [] when none +fun readRelaysNorm() = tags.mapNotNull(AdvertisedRelayInfo::parseReadNorm).ifEmpty { null } // null! +fun writeRelaysNorm()= tags.mapNotNull(AdvertisedRelayInfo::parseWriteNorm).ifEmpty { null } // null! +``` + +| helper | fallback fires when | correct | +|---|---|---| +| `normalizeNIP65AllRelayListWithBackup` | event absent only | ✅ (by accident — `relays()` has no `ifEmpty`) | +| `normalizeNIP65Read/WriteRelayListWithBackup` | event absent **or list empty** | ❌ | +| `normalizeIndexerRelayListWithBackup` | `?.ifEmpty { null } ?: DefaultIndexerRelayList` | ❌ | +| `normalizeSearchRelayListWithBackup` | `?.ifEmpty { null } ?: DefaultSearchRelayList` | ❌ | + +Consequence today: **a user who publishes a kind-10002 with only write relays gets +`Constants.bootstrapInbox` silently substituted as their inbox list.** Same for a deliberately empty +search or indexer list. The app overrides an explicit choice. + +The mirror problem sinks the obvious implementation: the `NoDefaults` variants return `emptySet()` +for *both* "no event" and "empty event", so `trustedRelays.isEmpty()` cannot be used as the +"do we have data yet" signal. + +**Fix:** make presence explicit, and never infer it from emptiness. + +```kotlin +// absent -> defaults; present -> whatever it says, including nothing +fun readRelayList(note: Note): Set = + nip65Event(note)?.let { it.readRelaysNorm()?.toSet() ?: emptySet() } ?: Constants.bootstrapInbox +``` + +Same shape for write/all, and drop the `?.ifEmpty { null }` from the indexer and search helpers. +Worth doing on its own merits even if the rest of this plan is dropped. + +**This removes the need for any window or timeout.** The fallback becomes a pure function of "do we +have the event", so it ends the instant one arrives — even an empty one. No per-account bookkeeping, +no 30s backstop, no race to close. + +## Finding 2 — do NOT put defaults into `TrustedRelayListsState` + +Tempting (it already merges all nine lists) but wrong: `account.trustedRelays.flow` feeds +`Account.kt:454` + +```kotlin +isInMyRelayList = { relayUrl -> ... it in trustedRelays.flow.value } +``` + +which feeds `RelayAuthPermissionLedger` -> `RelayAuthResolver` -> **the NIP-42 AUTH decision**. +Adding defaults there would make the app **auto-AUTH to the six hardcoded bootstrap relays as if +they were the user's own** — signing a challenge with the user's key and revealing the pubkey — at +exactly the moment we are also going clearnet. That converts a modest timing leak into a signed +identity assertion. See `[[relay-auth-always-was-gated]]` and `[[inbox-wine-notify-auth-billing]]` +for why AUTH is the sensitive edge. + +(The `saveTrustedRelayList(trustedRelays + relay)` write path in `RelayGroupChannelListScreen:449` +is **not** a hazard — it reads `account.trustedRelayList` (the NIP-51 list), not the merged +`trustedRelays`. Checked.) + +**Instead:** add a separate, purpose-named flow consumed only by Tor evaluation, e.g. +`Account.relaysAssumedWhileUnknown` — the union of the with-defaults views, non-empty only while the +corresponding events are absent. `AccountsTorStateConnector` feeds it into a new +`TorRelayState.assumedRelays`. Nothing else reads it. + +## Where the check goes in `useTor()` + +``` +torType == OFF -> false +isLocalHost -> false +isOverlayNetwork -> false +isOnion -> onionRelaysViaTor +in moneyOpRelayList -> moneyOperationsViaTor +in dmRelayList -> dmRelaysViaTor +in trustedRelayList -> trustedRelaysViaTor +in assumedRelayList -> trustedRelaysViaTor <-- new, immediately above the fallback +else -> newRelaysViaTor +``` + +Landing immediately above the fallback means **.onion, money-operation and DM relays keep their own +policy for free** — the change can only ever affect URLs that would have been treated as "new". + +Resolve to `trustedRelaysViaTor`, **not** a hardcoded `false`: + +- default user (`false`) -> clearnet -> fast start; +- hardened user (`true`) -> stays on Tor, automatically, with no new setting to discover. + +That is the difference between "the app overrides you" and "the app treats its stand-in list the way +you asked your own list to be treated". + +## Privacy, for the PR body + +The window correlates the user's **IP with their pubkey** at ~6 hardcoded relays, because the REQ +asks those relays for that pubkey's events. A first login is the most sensitive moment there is. + +What makes it defensible: **`trustedRelaysViaTor` already defaults to false**, so the moment +kind-10002 lands the user's own relays are dialled over clearnet anyway. This moves an existing +disclosure slightly earlier, to a different well-known set. It is not a new class of exposure for +the default configuration — and it is *not* an AUTH disclosure, provided Finding 2 is respected. + +If `trustedRelaysViaTor` ever becomes default-true, **this feature must be revisited in the same +commit** — its justification disappears. Leave a comment at the default linking the two. + +Residual, worth verifying rather than assuming: `useTor()` is keyed by relay **URL**, and the pool +multiplexes every subscription for a URL over one socket. During the window, anything addressed to a +default relay rides that clearnet socket — including a kind-1059 giftwrap subscription, since the DM +list is also absent. Measure it (below) before deciding it is acceptable. + +## Testing + +Unit — the rule itself, per list type: absent event -> defaults; present-but-empty -> **empty**; +present-with-values -> values. The middle case is the regression guard and the one that fails today. + +Unit (`TorRelayEvaluationTest`): an assumed relay resolves to `trustedRelaysViaTor` (both values); +.onion / money-op / DM keep their own policy while also listed as assumed; a non-assumed "new" relay +still resolves to `newRelaysViaTor`; an empty assumed set is byte-for-byte today's behaviour. + +Unit: `isInMyRelayList` does **not** see assumed relays (guards Finding 2 permanently). + +Device — the number that justifies the change. `relaytiming.sh` + `BootRelayDiag` census, +`VERBOSE_LOGS=true` benchmark build, fresh install each, counterbalanced, n>=3: +- primary: login -> first note; login -> own profile + follow list; +- secondary: relays opened / serving / events at the 20s census; +- guard: grep the verbose log for any request to a default relay during the window that is not for + the account's own pubkey, and for any AUTH sent to one. + +Harness traps (all in `[[fresh-install-routes-everything-via-tor]]`): the tablet raises its lock +screen during long waits (`wm dismiss-keyguard`, not just `KEYCODE_WAKEUP`); the login layout shifts +when the IME opens, so dismiss it before tapping fixed coordinates; `BACK` on the home screen exits +the app; always assert the run left the login screen before trusting its timing. + +## Expected outcome + +Approach the Tor-OFF column: ≈**-7s to first paint, ~2x relay coverage** in the first 20s, with +everything after the first event behaving exactly as today. + +If the gain is materially smaller, the likely cause is that the feed is gated on outbox-discovered +relays (which stay "new", hence Tor) rather than the user's own list — in which case the win is +limited to profile and follows, and may not be worth the privacy cost. Decide on the numbers. + +## Order of work + +1. Fix the absent-vs-empty bug in the four helpers + tests. Independently correct; ship separately. +2. Add `relaysAssumedWhileUnknown` + `TorRelayState.assumedRelays` + the `useTor()` branch. +3. Device A/B. Keep only if it earns its keep. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index bb04ed0b55..5b4404dd6a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -137,6 +137,7 @@ import com.vitorpamplona.amethyst.model.nip78AppSpecific.AppSpecificState import com.vitorpamplona.amethyst.model.nip89AppHandlers.AppRecommendationsState import com.vitorpamplona.amethyst.model.nipA3PaymentTargets.NipA3PaymentTargetsState import com.vitorpamplona.amethyst.model.nipB7Blossom.BlossomServerListState +import com.vitorpamplona.amethyst.model.serverList.AssumedRelayListsState import com.vitorpamplona.amethyst.model.serverList.MergedFollowListsState import com.vitorpamplona.amethyst.model.serverList.MergedFollowPlusMineRelayListsState import com.vitorpamplona.amethyst.model.serverList.MergedFollowPlusMineWithIndexRelayListsState @@ -812,6 +813,9 @@ class Account( val trustedRelays = TrustedRelayListsState(nip65RelayList, privateStorageRelayList, localRelayList, dmRelayList, searchRelayList, indexerRelayList, proxyRelayList, trustedRelayList, broadcastRelayList, scope) + /** Relays guessed on the user's behalf until their own lists arrive. Read only by Tor routing. */ + val assumedRelays = AssumedRelayListsState(nip65RelayList, searchRelayList, indexerRelayList, scope) + // Follows Relays val followOutboxesOrProxy = FollowListOutboxOrProxyRelays(kind3FollowList, blockedRelayList, proxyRelayList, cache, scope) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/indexerRelays/IndexerRelayListState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/indexerRelays/IndexerRelayListState.kt index 050240d188..756ce00e13 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/indexerRelays/IndexerRelayListState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/indexerRelays/IndexerRelayListState.kt @@ -77,6 +77,20 @@ class IndexerRelayListState( */ fun normalizeIndexerRelayListPrecached(note: Note): Set = indexListEvent(note)?.let { decryptionCache.cachedRelays(it) }?.ifEmpty { null } ?: DefaultIndexerRelayList + /** See `Nip65RelayListState.assumedDefaults`. Empty as soon as any kind:10086 exists. */ + fun assumedDefaults(note: Note): Set = if (indexListEvent(note) == null) DefaultIndexerRelayList else emptySet() + + val assumedDefaultsFlow = + getIndexerRelayListFlow() + .map { assumedDefaults(it.note) } + .onStart { emit(assumedDefaults(indexerListNote)) } + .flowOn(Dispatchers.IO) + .stateIn( + scope, + SharingStarted.Eagerly, + assumedDefaults(indexerListNote), + ) + /** * The account's indexer relays. [normalizeIndexerRelayListWithBackup] substitutes * [DefaultIndexerRelayList] when there is no kind:10086 at all — but **not** when the one we diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/searchRelays/SearchRelayListState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/searchRelays/SearchRelayListState.kt index ddffacd0bc..7d990b2a02 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/searchRelays/SearchRelayListState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/searchRelays/SearchRelayListState.kt @@ -78,6 +78,20 @@ class SearchRelayListState( */ fun normalizeSearchRelayListPrecached(note: Note): Set = searchListEvent(note)?.let { decryptionCache.cachedRelays(it) }?.ifEmpty { null } ?: DefaultSearchRelayList + /** See `Nip65RelayListState.assumedDefaults`. Empty as soon as any kind:10007 exists. */ + fun assumedDefaults(note: Note): Set = if (searchListEvent(note) == null) DefaultSearchRelayList else emptySet() + + val assumedDefaultsFlow = + getSearchRelayListFlow() + .map { assumedDefaults(it.note) } + .onStart { emit(assumedDefaults(searchListNote)) } + .flowOn(Dispatchers.IO) + .stateIn( + scope, + SharingStarted.Eagerly, + assumedDefaults(searchListNote), + ) + /** * The account's search relays. [normalizeSearchRelayListWithBackup] substitutes * [DefaultSearchRelayList] when there is no kind:10007 at all — but **not** when the one we diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip65RelayList/Nip65RelayListState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip65RelayList/Nip65RelayListState.kt index 1440fd2ab0..261c6b1d97 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip65RelayList/Nip65RelayListState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip65RelayList/Nip65RelayListState.kt @@ -71,6 +71,27 @@ class Nip65RelayListState( fun normalizeNIP65AllRelayListWithBackupNoDefaults(note: Note): Set = nip65Event(note)?.relays()?.map { it.relayUrl }?.toSet() ?: emptySet() + /** + * The app defaults currently standing in for a user we have no kind:10002 for — empty as soon + * as one exists, including an empty one. + * + * Uses the same `nip65Event(note) == null` predicate the substitution itself uses, so the two + * cannot drift: whatever is listed here is exactly what the app is guessing on the user's + * behalf. See [relayListOrDefaultsWhenUnknown]. + */ + fun assumedDefaults(note: Note): Set = if (nip65Event(note) == null) Constants.bootstrapInbox + Constants.eventFinderRelays else emptySet() + + val assumedDefaultsFlow = + getNIP65RelayListFlow() + .map { assumedDefaults(it.note) } + .onStart { emit(assumedDefaults(nip65ListNote)) } + .flowOn(Dispatchers.IO) + .stateIn( + scope, + SharingStarted.Eagerly, + assumedDefaults(nip65ListNote), + ) + val outboxFlow = getNIP65RelayListFlow() .map { normalizeNIP65WriteRelayListWithBackup(it.note) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/serverList/AssumedRelayListsState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/serverList/AssumedRelayListsState.kt new file mode 100644 index 0000000000..f2e6f30141 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/serverList/AssumedRelayListsState.kt @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.model.serverList + +import com.vitorpamplona.amethyst.model.nip51Lists.indexerRelays.IndexerRelayListState +import com.vitorpamplona.amethyst.model.nip51Lists.searchRelays.SearchRelayListState +import com.vitorpamplona.amethyst.model.nip65RelayList.Nip65RelayListState +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.stateIn + +/** + * The relays the app is **guessing** on the user's behalf because it has not seen their lists yet. + * + * Non-empty only while the corresponding event is absent — never because a list is empty, which is + * a choice we honor (see `relayListOrDefaultsWhenUnknown`). It therefore empties itself, per list, + * the moment the user's own data lands; no window, no timeout, no bookkeeping. + * + * **Deliberately NOT merged into [TrustedRelayListsState].** That one feeds `Account.isInMyRelayList` + * -> `RelayAuthPermissionLedger` -> `RelayAuthResolver`, i.e. the NIP-42 AUTH decision. Guessed + * relays must never make the app sign an AUTH challenge as though they were the user's own — that + * would turn a timing signal into a signed identity assertion. The single consumer of this flow is + * Tor routing. + */ +class AssumedRelayListsState( + val nip65RelayList: Nip65RelayListState, + val searchRelayList: SearchRelayListState, + val indexerRelayList: IndexerRelayListState, + val scope: CoroutineScope, +) { + val flow: StateFlow> = + combine( + nip65RelayList.assumedDefaultsFlow, + searchRelayList.assumedDefaultsFlow, + indexerRelayList.assumedDefaultsFlow, + ) { nip65, search, indexer -> + nip65 + search + indexer + }.flowOn(Dispatchers.IO) + .stateIn( + scope, + kotlinx.coroutines.flow.SharingStarted.Eagerly, + nip65RelayList.assumedDefaultsFlow.value + + searchRelayList.assumedDefaultsFlow.value + + indexerRelayList.assumedDefaultsFlow.value, + ) +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/AccountsTorStateConnector.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/AccountsTorStateConnector.kt index a437179a8e..8f6dc5c9c0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/AccountsTorStateConnector.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/AccountsTorStateConnector.kt @@ -20,14 +20,17 @@ */ package com.vitorpamplona.amethyst.model.torState +import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.accountsCache.AccountCacheState import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.emitAll @@ -35,112 +38,132 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.transformLatest +/** + * Pushes the relay classifications [TorRelayState] needs — which relays are DM, trusted, guessed, or + * money-operation relays — as a union across every logged-in account. + * + * All four are the same fold: pick one set per account, union them, publish. It used to be written + * out four times at ~30 lines each, and the copies had already drifted apart in trivial ways (an + * `if (isEmpty)` guard that could never fire, differently-named accumulators). Sharing one + * implementation is what keeps a fifth classification from being another 30 lines of the same + * thing — and, more importantly, from being 30 lines that quietly forget a step. + */ class AccountsTorStateConnector( accountsCache: AccountCacheState, torEvaluatorFlow: TorRelayState, scope: CoroutineScope, ) { - @OptIn(ExperimentalCoroutinesApi::class, FlowPreview::class) - val allDmRelayFlows: Flow> = - accountsCache.accounts - .debounce(200) - .transformLatest { snapshot -> - val dmFlows = snapshot.map { it.value.dmRelayList.flow } - - val dmFlowReady = - dmFlows.ifEmpty { - listOf(MutableStateFlow(emptySet())) - } - - if (dmFlowReady.isEmpty()) { - emit(emptySet()) - } else { - emitAll( - combine(dmFlowReady) { - val dmRelays = mutableSetOf() - it.forEach { - dmRelays.addAll(it) - } - dmRelays.toSet() - }, - ) - } - }.onEach { - torEvaluatorFlow.dmRelays.tryEmit(it) - }.stateIn( - scope, - SharingStarted.Eagerly, - emptySet(), - ) - + /** + * Union of one relay set across all logged-in accounts, republished into [TorRelayState]. + * + * `debounce(200)` rides out the burst of account churn at login; `transformLatest` drops the + * previous fan-in when the account set changes so a logged-out account cannot keep contributing. + * The seed is `emptySet()` for every classification: before any account exists, nothing is + * classified. + * + * Takes its collaborators as parameters rather than reading constructor properties because the + * call sites are property initializers, where non-`val` constructor parameters are in scope but + * member functions cannot see them. + */ @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) - val allTrustedRelaysFlow: Flow> = + private fun unionAcrossAccounts( + accountsCache: AccountCacheState, + scope: CoroutineScope, + select: (Account) -> Flow>, + publish: (Set) -> Unit, + ): StateFlow> = accountsCache.accounts .debounce(200) .transformLatest { snapshot -> - val trustedRelayFlows = snapshot.map { it.value.trustedRelays.flow } - - val trustedRelayFlowReady = - trustedRelayFlows.ifEmpty { - listOf(MutableStateFlow(emptySet())) - } - - if (trustedRelayFlowReady.isEmpty()) { - emit(emptySet()) - } else { - emitAll( - combine(trustedRelayFlowReady) { - val trustedRelays = mutableSetOf() - it.forEach { - trustedRelays.addAll(it) - } - trustedRelays.toSet() - }, - ) - } - }.onEach { - torEvaluatorFlow.trustedRelays.tryEmit(it) - }.stateIn( - scope, - SharingStarted.Eagerly, - emptySet(), - ) - - // Persistent money-operation relays across all accounts: NIP-47 wallet relays and saved CLINK - // Debits service relays. Feeds TorRelayState.moneyOpRelays so these connections honor the - // money-operations Tor preference instead of being classified as generic "new" relays. - @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) - val allMoneyOpRelaysFlow: Flow> = - accountsCache.accounts - .debounce(200) - .transformLatest { snapshot -> - val perAccountFlows = - snapshot.map { (_, account) -> - combine( - account.settings.nwcWallets, - account.settings.clinkDebitWallets, - ) { nwcWallets, clinkDebitWallets -> - val relays = mutableSetOf() - nwcWallets.forEach { relays.add(it.uri.relayUri) } - clinkDebitWallets.forEach { relays.addAll(it.pointer.relays) } - relays.toSet() - } - } - - val ready = perAccountFlows.ifEmpty { listOf(MutableStateFlow(emptySet())) } + val perAccount = + snapshot + .map { select(it.value) } + .ifEmpty { listOf(MutableStateFlow(emptySet())) } emitAll( - combine(ready) { perAccount -> - val moneyOpRelays = mutableSetOf() - perAccount.forEach { moneyOpRelays.addAll(it) } - moneyOpRelays.toSet() + combine(perAccount) { sets -> + sets.flatMapTo(mutableSetOf()) { it } }, ) - }.onEach { - torEvaluatorFlow.moneyOpRelays.tryEmit(it) - }.stateIn( + }.onEach(publish) + .stateIn( scope, SharingStarted.Eagerly, emptySet(), ) + + /** NIP-17 DM relays: these follow the dedicated DM preference, never the generic "new" one. */ + val allDmRelayFlows: StateFlow> = + unionAcrossAccounts( + accountsCache, + scope, + select = { it.dmRelayList.flow }, + publish = { torEvaluatorFlow.dmRelays.tryEmit(it) }, + ) + + /** Everything the user actually put in one of their own relay lists. */ + val allTrustedRelaysFlow: StateFlow> = + unionAcrossAccounts( + accountsCache, + scope, + select = { it.trustedRelays.flow }, + publish = { torEvaluatorFlow.trustedRelays.tryEmit(it) }, + ) + + /** + * Relays the app is *guessing* while an account's own lists are unknown. Feeds + * [TorRelayState.assumedRelays] and nothing else — see `AssumedRelayListsState` for why these + * must never reach the AUTH decision. + * + * Per account, so a second login cannot re-open the guess for an established one; each + * account's contribution empties itself as soon as that account's own lists land. + */ + val allAssumedRelaysFlow: StateFlow> = + unionAcrossAccounts( + accountsCache, + scope, + select = { it.assumedRelays.flow }, + publish = { + logHandover(it) + torEvaluatorFlow.assumedRelays.tryEmit(it) + }, + ) + + /** + * Persistent money-operation relays: NIP-47 wallet relays and saved CLINK Debits service + * relays, so these connections honor the money-operations preference rather than being + * classified as generic "new" relays. + */ + val allMoneyOpRelaysFlow: StateFlow> = + unionAcrossAccounts( + accountsCache, + scope, + select = { account -> + combine( + account.settings.nwcWallets, + account.settings.clinkDebitWallets, + ) { nwcWallets, clinkDebitWallets -> + val relays = mutableSetOf() + nwcWallets.forEach { relays.add(it.uri.relayUri) } + clinkDebitWallets.forEach { relays.addAll(it.pointer.relays) } + relays.toSet() + } + }, + publish = { torEvaluatorFlow.moneyOpRelays.tryEmit(it) }, + ) + + @Volatile private var lastAssumedCount: Int = -1 + + /** + * The handover is the whole contract of the guessed-relay feature: the moment a user's own + * lists arrive, every relay we were guessing about goes back to the policy they actually asked + * for. Logged at INFO because "did it hand over, and when" is not answerable from any other + * line — the reconnect that follows looks identical to an ordinary one. + */ + private fun logHandover(relays: Set) { + if (relays.size == lastAssumedCount) return + val released = if (relays.isEmpty()) " (own lists arrived; released to their real Tor policy)" else "" + Log.i("AccountsTorState") { "Guessed relays: $lastAssumedCount -> ${relays.size}$released" } + lastAssumedCount = relays.size + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluation.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluation.kt index e7d1bb1ebe..0e6a0d5dcd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluation.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluation.kt @@ -22,3 +22,5 @@ package com.vitorpamplona.amethyst.model.torState // Canonical type now lives in commons typealias TorRelayEvaluation = com.vitorpamplona.amethyst.commons.tor.TorRelayEvaluation + +typealias RelayClassification = com.vitorpamplona.amethyst.commons.tor.RelayClassification diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayState.kt index bd0366a1df..577a4d1ae9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/torState/TorRelayState.kt @@ -46,6 +46,13 @@ class TorRelayState( val dmRelays = MutableStateFlow>(emptySet()) val trustedRelays = MutableStateFlow>(emptySet()) + /** + * Relays guessed on the user's behalf while their own lists are unknown. Fed by + * [AccountsTorStateConnector]; see `AssumedRelayListsState` for why this is separate from + * [trustedRelays] rather than merged into it. + */ + val assumedRelays = MutableStateFlow>(emptySet()) + /** * Relays known to be used for money operations from persistent configuration: NIP-47 wallet * relays and saved CLINK Debits service relays. Fed by [AccountsTorStateConnector] across all @@ -130,47 +137,49 @@ class TorRelayState( currentSettings(), ) - val flow = - combineTransform( - torSettings, + private fun currentClassification() = + RelayClassification( + trusted = trustedRelays.value, + dm = dmRelays.value, + moneyOp = currentMoneyOpRelays(), + assumed = assumedRelays.value, + ) + + /** + * The four category sets as one value. Folding them here also keeps the evaluation flow below + * at two sources instead of six — `combineTransform`'s typed overloads stop at five. + */ + private val classification = + combine( trustedRelays, dmRelays, moneyOpRelays, adHocMoneyOpCounts, - ) { - torSettings: TorRelaySettings, - trustedRelayList: Set, - dmRelayList: Set, - moneyOpRelayList: Set, - adHocMoneyOps: Map, - -> - emit( - TorRelayEvaluation( - torSettings = torSettings, - trustedRelayList = trustedRelayList, - dmRelayList = dmRelayList, - moneyOpRelayList = moneyOpRelayList + adHocMoneyOps.keys, - ), + assumedRelays, + ) { trusted, dm, moneyOp, adHocMoneyOps, assumed -> + RelayClassification( + trusted = trusted, + dm = dm, + moneyOp = moneyOp + adHocMoneyOps.keys, + assumed = assumed, ) + } + + val flow = + combineTransform( + torSettings, + classification, + ) { torSettings: TorRelaySettings, classification: RelayClassification -> + emit(TorRelayEvaluation(torSettings, classification)) }.onStart { emit( - TorRelayEvaluation( - torSettings = torSettings.value, - trustedRelayList = trustedRelays.value, - dmRelayList = dmRelays.value, - moneyOpRelayList = currentMoneyOpRelays(), - ), + TorRelayEvaluation(torSettings.value, currentClassification()), ) }.flowOn(Dispatchers.IO) .stateIn( scope, SharingStarted.Eagerly, - TorRelayEvaluation( - torSettings = torSettings.value, - trustedRelayList = trustedRelays.value, - dmRelayList = dmRelays.value, - moneyOpRelayList = currentMoneyOpRelays(), - ), + TorRelayEvaluation(torSettings.value, currentClassification()), ) /** @@ -178,13 +187,7 @@ class TorRelayState( * snapshot. This makes ad-hoc money-op registration ([registerMoneyOpRelays]) take effect on the * very next connection attempt, with no dependency on the combine pipeline having propagated yet. */ - fun shouldUseTorForRelay(relay: NormalizedRelayUrl) = - TorRelayEvaluation( - torSettings = currentSettings(), - trustedRelayList = trustedRelays.value, - dmRelayList = dmRelays.value, - moneyOpRelayList = currentMoneyOpRelays(), - ).useTor(relay) + fun shouldUseTorForRelay(relay: NormalizedRelayUrl) = TorRelayEvaluation(currentSettings(), currentClassification()).useTor(relay) fun okHttpClientForRelay(url: NormalizedRelayUrl): OkHttpClient = okHttpClient.getHttpClient(shouldUseTorForRelay(url)) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt index d0630e2cab..69e8328591 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt @@ -20,13 +20,13 @@ */ package com.vitorpamplona.amethyst.service.relayClient +import com.vitorpamplona.amethyst.commons.tor.RelayClassification import com.vitorpamplona.amethyst.commons.tor.TorRelaySettings import com.vitorpamplona.amethyst.model.torState.TorRelayEvaluation import com.vitorpamplona.amethyst.service.connectivity.ConnectivityStatus import com.vitorpamplona.amethyst.service.resourceusage.UsageKeys import com.vitorpamplona.amethyst.ui.tor.TorServiceStatus import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient -import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -101,9 +101,7 @@ class RelayProxyClientConnector( // flipped relay would sit out its (now-irrelevant) backoff. We track these so such a relay can // skip its retry delay on the next reconnect — scoped to onlyIfChanged, so only the relays that // actually flipped re-dial and the rest of the pool's backoff is left untouched. - private var lastTrustedRelays: Set? = null - private var lastDmRelays: Set? = null - private var lastMoneyOpRelays: Set? = null + private var lastClassification: RelayClassification? = null @OptIn(FlowPreview::class) val relayServices = @@ -174,9 +172,7 @@ class RelayProxyClientConnector( lastTorSettings = torSettings lastTorConnection = infra.torConnection lastClearConnection = infra.clearConnection - lastTrustedRelays = infra.evaluator.trustedRelayList - lastDmRelays = infra.evaluator.dmRelayList - lastMoneyOpRelays = infra.evaluator.moneyOpRelayList + lastClassification = infra.evaluator.classification } else -> { @@ -202,13 +198,13 @@ class RelayProxyClientConnector( // so let onlyIfChanged pick out the flipped relay(s) and skip THEIR retry delay — // without resetBackoff(), so the rest of the pool's backoff is untouched (these sets // churn while relay lists load, and forgiving the whole pool then would be too much). + // + // One comparison over the whole classification, not one per category: this used to + // be a four-way `||` and adding a category meant remembering to extend it. Missing + // a term fails silently — the affected relays keep a socket on a transport the + // policy has already moved them off. val classificationChanged = - lastTrustedRelays != null && - ( - infra.evaluator.trustedRelayList != lastTrustedRelays || - infra.evaluator.dmRelayList != lastDmRelays || - infra.evaluator.moneyOpRelayList != lastMoneyOpRelays - ) + lastClassification != null && infra.evaluator.classification != lastClassification val previousNetworkId = lastNetworkId @@ -216,9 +212,7 @@ class RelayProxyClientConnector( lastClearConnection = infra.clearConnection lastNetworkId = networkId ?: lastNetworkId lastTorSettings = torSettings - lastTrustedRelays = infra.evaluator.trustedRelayList - lastDmRelays = infra.evaluator.dmRelayList - lastMoneyOpRelays = infra.evaluator.moneyOpRelayList + lastClassification = infra.evaluator.classification if (networkChanged) { Log.d("ManageRelayServices") { diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluationTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluationTest.kt index 3c47751fac..8eae27fb59 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluationTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/torState/TorRelayEvaluationTest.kt @@ -53,8 +53,11 @@ class TorRelayEvaluationTest { newRelaysViaTor = newViaTor, trustedRelaysViaTor = trustedViaTor, ), - trustedRelayList = trustedRelays, - dmRelayList = dmRelays, + classification = + RelayClassification( + trusted = trustedRelays, + dm = dmRelays, + ), ) // --- Tor OFF: always false --- diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnectorTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnectorTest.kt index 1b3f8b9da8..57648ff286 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnectorTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnectorTest.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.service.relayClient +import com.vitorpamplona.amethyst.commons.tor.RelayClassification import com.vitorpamplona.amethyst.commons.tor.TorRelaySettings import com.vitorpamplona.amethyst.commons.tor.TorType import com.vitorpamplona.amethyst.model.torState.TorRelayEvaluation @@ -28,6 +29,7 @@ import com.vitorpamplona.amethyst.service.relayClient.RelayProxyClientConnector. import com.vitorpamplona.amethyst.ui.tor.TorServiceStatus import com.vitorpamplona.quartz.nip01Core.relay.client.EmptyNostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import io.mockk.mockk import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -112,8 +114,11 @@ class RelayProxyClientConnectorTest { private fun evaluation(settings: TorRelaySettings = TorRelaySettings()) = TorRelayEvaluation( torSettings = settings, - trustedRelayList = emptySet(), - dmRelayList = emptySet(), + classification = + RelayClassification( + trusted = emptySet(), + dm = emptySet(), + ), ) private fun infra( @@ -179,6 +184,43 @@ class RelayProxyClientConnectorTest { assertEquals(listOf(true to true), client.reconnects) } + /** + * The handover the guessed-relay feature exists to perform: when a user's own lists arrive, the + * relays we were guessing about must re-dial onto whatever policy the user actually chose. + * + * The case that matters is an **empty** arriving list. Normally `trusted` grows at the same + * moment and would have flagged the change on its own — but an account that publishes an empty + * relay list leaves `trusted` untouched while `assumed` empties, and before the classification + * was compared as one value that combination produced no reconnect at all, stranding those + * relays on clearnet against a policy that had already moved them to Tor. + */ + @Test + fun `guessed relays re-dial when an empty list arrives and trusted does not change`() { + settleOnFirstNetwork() + + val guessing = + TorRelayEvaluation( + torSettings = TorRelaySettings(), + classification = RelayClassification(assumed = setOf(NormalizedRelayUrl("wss://guessed.example/"))), + ) + connector.apply(infra(networkId = 1L, evaluation = guessing)) + client.reconnects.clear() + + // The user's own list arrives and is empty: `assumed` empties, `trusted` stays empty. + val released = + TorRelayEvaluation( + torSettings = TorRelaySettings(), + classification = RelayClassification(), + ) + connector.apply(infra(networkId = 1L, evaluation = released)) + + assertEquals( + "The relays we stopped guessing about must be asked to re-dial onto their real policy", + listOf(true to true), + client.reconnects, + ) + } + @Test fun `unrelated churn on the same network leaves the backoff alone`() { settleOnFirstNetwork() diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/TorClearnetFallbackTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/TorClearnetFallbackTest.kt index 1bbf2fcf93..f39ad62777 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/TorClearnetFallbackTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/TorClearnetFallbackTest.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.relayGroup +import com.vitorpamplona.amethyst.commons.tor.RelayClassification import com.vitorpamplona.amethyst.commons.tor.TorRelayEvaluation import com.vitorpamplona.amethyst.commons.tor.TorRelaySettings import com.vitorpamplona.amethyst.commons.tor.TorSettings @@ -88,8 +89,11 @@ class TorClearnetFallbackTest { trustedRelaysViaTor = preset.trustedRelaysViaTor, moneyOperationsViaTor = preset.moneyOperationsViaTor, ), - trustedRelayList = emptySet(), - dmRelayList = emptySet(), + classification = + RelayClassification( + trusted = emptySet(), + dm = emptySet(), + ), ) @Test diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluation.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluation.kt index 40695348b8..5046ea2d6b 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluation.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluation.kt @@ -25,11 +25,32 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.isLocalHost import com.vitorpamplona.quartz.nip01Core.relay.normalizer.isOnion import com.vitorpamplona.quartz.nip01Core.relay.normalizer.isOverlayNetwork +/** + * Which relays fall into each Tor-routing category, as one value. + * + * Grouped deliberately rather than passed as four loose sets. Consumers that must react when the + * categories change — `RelayProxyClientConnector` re-dials the relays whose transport flipped — + * previously compared the sets field by field, so adding a fifth category meant remembering to add + * a fifth `||`. Forgetting it fails silently: relays keep a socket on a transport the policy has + * already moved them off. Structural equality on one object makes that impossible to forget. + */ +data class RelayClassification( + /** Relays the user actually put in one of their own lists. */ + val trusted: Set = emptySet(), + /** NIP-17 DM relays. */ + val dm: Set = emptySet(), + /** NIP-47 wallet and CLINK debit relays, including ad-hoc registrations. */ + val moneyOp: Set = emptySet(), + /** + * Relays the app is guessing on the user's behalf while their own lists are unknown. Empties + * itself as soon as any of their events arrive; see `AssumedRelayListsState`. + */ + val assumed: Set = emptySet(), +) + class TorRelayEvaluation( val torSettings: TorRelaySettings, - val trustedRelayList: Set, - val dmRelayList: Set, - val moneyOpRelayList: Set = emptySet(), + val classification: RelayClassification = RelayClassification(), ) { fun useTor(relay: NormalizedRelayUrl): Boolean = if (torSettings.torType == TorType.OFF) { @@ -45,15 +66,21 @@ class TorRelayEvaluation( } else if (relay.isOnion()) { // .onion is only reachable over Tor regardless of any other classification. torSettings.onionRelaysViaTor - } else if (relay in moneyOpRelayList) { + } else if (relay in classification.moneyOp) { // Relays used for money operations (NIP-47 wallets, CLINK offer/debit services) // follow the dedicated money-operations preference, taking precedence over the // generic DM/trusted/new classification so a payment never silently inherits a // different Tor policy than the one the user set for money. torSettings.moneyOperationsViaTor - } else if (relay in dmRelayList) { + } else if (relay in classification.dm) { torSettings.dmRelaysViaTor - } else if (relay in trustedRelayList) { + } else if (relay in classification.trusted) { + torSettings.trustedRelaysViaTor + } else if (relay in classification.assumed) { + // Last resort before treating it as a stranger. Sits below every other + // classification on purpose: .onion, money-operation and DM relays keep their own + // policy even while we are guessing, because this branch can only ever capture + // relays that would otherwise have fallen through to `newRelaysViaTor`. torSettings.trustedRelaysViaTor } else { torSettings.newRelaysViaTor diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluationTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluationTest.kt index a6b441a8e4..483faf0fd3 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluationTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/TorRelayEvaluationTest.kt @@ -34,6 +34,7 @@ class TorRelayEvaluationTest { private val dmRelay = NormalizedRelayUrl("wss://dm.relay.com/") private val trustedRelay = NormalizedRelayUrl("wss://trusted.relay.com/") private val moneyRelay = NormalizedRelayUrl("wss://wallet.relay.com/") + private val assumedRelay = NormalizedRelayUrl("wss://assumed.relay.com/") private fun buildEvaluation( torType: TorType = TorType.INTERNAL, @@ -45,6 +46,7 @@ class TorRelayEvaluationTest { dmRelays: Set = setOf(dmRelay), trustedRelays: Set = setOf(trustedRelay), moneyOpRelays: Set = setOf(moneyRelay), + assumedRelays: Set = setOf(assumedRelay), ) = TorRelayEvaluation( torSettings = TorRelaySettings( @@ -55,11 +57,68 @@ class TorRelayEvaluationTest { trustedRelaysViaTor = trustedViaTor, moneyOperationsViaTor = moneyViaTor, ), - trustedRelayList = trustedRelays, - dmRelayList = dmRelays, - moneyOpRelayList = moneyOpRelays, + classification = + RelayClassification( + trusted = trustedRelays, + dm = dmRelays, + moneyOp = moneyOpRelays, + assumed = assumedRelays, + ), ) + // --- assumed relays: the app's stand-in while the user's lists are unknown --- + + /** + * The whole point: a guessed relay inherits the policy the user chose for their *own* lists, so + * the default configuration starts on clearnet and gets a fast first login. + */ + @Test + fun assumedRelay_followsTrustedPreference() { + assertFalse(buildEvaluation(trustedViaTor = false).useTor(assumedRelay)) + assertTrue(buildEvaluation(trustedViaTor = true).useTor(assumedRelay)) + } + + /** Anyone who asked for Tor on their own relays keeps it here, with no separate opt-out. */ + @Test + fun assumedRelay_hardenedUserStillUsesTor() { + assertTrue(buildEvaluation(trustedViaTor = true, newViaTor = true).useTor(assumedRelay)) + } + + /** + * The branch sits below every other classification, so being guessed can never downgrade a + * relay that already had a stricter policy. + */ + @Test + fun assumedRelay_neverOverridesOnionDmOrMoney() { + val eval = + buildEvaluation( + trustedViaTor = false, + onionViaTor = true, + dmViaTor = true, + moneyViaTor = true, + assumedRelays = setOf(assumedRelay, onionRelay, dmRelay, moneyRelay), + ) + assertTrue(eval.useTor(onionRelay)) + assertTrue(eval.useTor(dmRelay)) + assertTrue(eval.useTor(moneyRelay)) + } + + /** A relay we are not guessing about is still a stranger. */ + @Test + fun unknownRelay_stillFollowsNewPreference() { + assertTrue(buildEvaluation(newViaTor = true).useTor(clearnetRelay)) + assertFalse(buildEvaluation(newViaTor = false).useTor(clearnetRelay)) + } + + /** With nothing guessed — every account that has any list — behaviour is exactly as before. */ + @Test + fun emptyAssumedList_isTodaysBehaviour() { + val eval = buildEvaluation(assumedRelays = emptySet(), newViaTor = true, trustedViaTor = false) + assertTrue(eval.useTor(assumedRelay)) + assertTrue(eval.useTor(clearnetRelay)) + assertFalse(eval.useTor(trustedRelay)) + } + // --- Tor OFF --- @Test fun torOff_alwaysFalse() { diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/YggdrasilTorRoutingTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/YggdrasilTorRoutingTest.kt index 9a2dbb577b..86fa2063c5 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/YggdrasilTorRoutingTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/tor/YggdrasilTorRoutingTest.kt @@ -48,8 +48,11 @@ class YggdrasilTorRoutingTest { trustedRelaysViaTor = false, moneyOperationsViaTor = false, ), - trustedRelayList = emptySet(), - dmRelayList = emptySet(), + classification = + RelayClassification( + trusted = emptySet(), + dm = emptySet(), + ), ) @Test diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt index 9d20aebca5..db0bd529f8 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt @@ -994,8 +994,10 @@ private fun AppInner( newRelaysViaTor = torSettings.newRelaysViaTor, trustedRelaysViaTor = torSettings.trustedRelaysViaTor, ), - trustedRelayList = emptySet(), // TODO: populate from account relay lists - dmRelayList = emptySet(), // TODO: populate from account relay lists + // TODO: populate from account relay lists + classification = + com.vitorpamplona.amethyst.commons.tor + .RelayClassification(), ) }