mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(relays): only substitute default relays when we have no event, not when the list is empty
There are three states, and two of them were collapsed:
| we have | effective list |
|------------------------|---------------------------------------|
| no event for the user | app defaults — we do not know |
| an event, empty list | **empty** — they told us: nothing |
| an event with relays | those relays |
Every `WithBackup` helper keyed its fallback on the list being *empty* rather
than the event being *absent*, because `readRelaysNorm()`/`writeRelaysNorm()`
end in `.ifEmpty { null }` and the indexer/search helpers wrote
`?.ifEmpty { null } ?: DEFAULTS` outright. So a user who publishes a kind:10002
carrying only write relays silently acquired `Constants.bootstrapInbox` as their
*inbox* list, and a deliberately empty search or indexer list was replaced by
ours. That is the app overriding an explicit choice.
Only `normalizeNIP65AllRelayListWithBackup` was correct, and only by accident:
`relays()` has no `ifEmpty`, so its `?:` could fire only for a missing event.
The rule is now one named, tested primitive rather than an expression
open-coded at four call sites — three of which got it wrong the same way:
relayListOrDefaultsWhenUnknown(event, defaults) { it.readRelaysNorm()?.toSet() }
`Account.indexRelays()` loses its `.ifEmpty { DefaultIndexerRelayList }` too;
it re-applied the substitution a layer up and would have undone the fix.
Two things deliberately left alone. The `Precached` variants keep substituting
defaults: they read only *already decrypted* tags, so empty there can mean "not
decrypted yet" — an unbounded window for a NIP-46 signer — rather than "the user
chose nothing", and the primitive's KDoc records that as a non-goal. And the
`NoDefaults` flows keep returning `emptySet()` for both cases, since their job is
to show what the user published.
Note for callers: the indexer and search flows previously documented themselves
as **never empty** and that contract is gone. A user who publishes an empty
kind:10007 now gets no search relays, which is what their event says. The same
applies to NIP-65 write relays, where the old fallback meant posts went to six
hardcoded relays; if a safety net is wanted there it belongs at the publish site
as a visible decision, not as a silent list substitution.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BKYGEp22uGSzWrBDg8fAQ9
This commit is contained in:
co-authored by
Claude Opus 5
parent
b266f1c403
commit
e7bcb88d30
@@ -31,7 +31,6 @@ import com.vitorpamplona.amethyst.commons.connectedApps.signers.InMemoryNostrSig
|
||||
import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerPermissionLedger
|
||||
import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerPermissionStore
|
||||
import com.vitorpamplona.amethyst.commons.defaults.Constants
|
||||
import com.vitorpamplona.amethyst.commons.defaults.DefaultIndexerRelayList
|
||||
import com.vitorpamplona.amethyst.commons.marmot.MarmotManager
|
||||
import com.vitorpamplona.amethyst.commons.model.IAccount
|
||||
import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelStars
|
||||
@@ -384,12 +383,16 @@ class Account(
|
||||
// doubles as the attribution pubkey for ExplainedFilter.accountPubKeys.
|
||||
override val userFinderPubkeyHex: HexKey get() = userProfile().pubkeyHex
|
||||
|
||||
override fun indexRelays(): Set<NormalizedRelayUrl> = indexerRelayList.flow.value.ifEmpty { DefaultIndexerRelayList }
|
||||
// No ifEmpty here on purpose: an empty kind:10086 is the user asking for no indexers, and
|
||||
// IndexerRelayListState already substitutes the defaults for the only case we may override —
|
||||
// never having seen the event. Re-substituting here would undo that choice.
|
||||
override fun indexRelays(): Set<NormalizedRelayUrl> = indexerRelayList.flow.value
|
||||
|
||||
override fun outboxHomeRelays(): Set<NormalizedRelayUrl> = nip65RelayList.allFlowNoDefaults.value + privateStorageRelayList.flow.value + localRelayList.flow.value
|
||||
|
||||
// searchRelayList.flow already applies the DefaultSearchRelayList fallback internally
|
||||
// (SearchRelayListState.normalizeSearchRelayListWithBackup), so no ifEmpty needed here.
|
||||
// searchRelayList.flow applies DefaultSearchRelayList internally when no kind:10007 has ever
|
||||
// been seen (SearchRelayListState.normalizeSearchRelayListWithBackup); an empty published list
|
||||
// stays empty. No ifEmpty here either way.
|
||||
override fun searchRelays(): Set<NormalizedRelayUrl> = (trustedRelayList.flow.value + searchRelayList.flow.value).toSet()
|
||||
|
||||
override fun searchOnlyRelays(): Set<NormalizedRelayUrl> = searchRelayList.flow.value
|
||||
|
||||
+10
-6
@@ -58,7 +58,11 @@ class IndexerRelayListState(
|
||||
|
||||
fun indexListEvent(note: Note) = note.event as? IndexerRelayListEvent ?: settings.backupIndexRelayList
|
||||
|
||||
suspend fun normalizeIndexerRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> = indexListEvent(note)?.let { decryptionCache.relays(it) }?.ifEmpty { null } ?: DefaultIndexerRelayList
|
||||
suspend fun normalizeIndexerRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> {
|
||||
val event = indexListEvent(note) ?: return DefaultIndexerRelayList
|
||||
// Fully decrypted here, so empty means the user listed nothing — not "not decrypted yet".
|
||||
return decryptionCache.relays(event)
|
||||
}
|
||||
|
||||
suspend fun normalizeIndexerRelayListWithBackupNoDefaults(note: Note): Set<NormalizedRelayUrl> = indexListEvent(note)?.let { decryptionCache.relays(it) } ?: emptySet()
|
||||
|
||||
@@ -74,11 +78,11 @@ class IndexerRelayListState(
|
||||
fun normalizeIndexerRelayListPrecached(note: Note): Set<NormalizedRelayUrl> = indexListEvent(note)?.let { decryptionCache.cachedRelays(it) }?.ifEmpty { null } ?: DefaultIndexerRelayList
|
||||
|
||||
/**
|
||||
* The account's indexer relays, **never empty** — [normalizeIndexerRelayListWithBackup]
|
||||
* substitutes [DefaultIndexerRelayList] both when there is no kind:10086 and when the
|
||||
* one we have decodes to zero relays. Callers assembling metadata / relay-list REQs read
|
||||
* this and can rely on getting a usable set; use [flowNoDefaults] instead to show or diff
|
||||
* what the user actually configured.
|
||||
* The account's indexer relays. [normalizeIndexerRelayListWithBackup] substitutes
|
||||
* [DefaultIndexerRelayList] when there is no kind:10086 at all — but **not** when the one we
|
||||
* have decodes to zero relays, which is the user saying "no indexers" and is honored. Callers
|
||||
* assembling metadata / relay-list REQs must therefore tolerate an empty set; use
|
||||
* [flowNoDefaults] to show or diff what the user actually configured.
|
||||
*
|
||||
* Seeded via [normalizeIndexerRelayListPrecached] rather than `emptySet()`, for the same
|
||||
* reason as the search list: `flowOn(IO)` makes the first real emission asynchronous, so an
|
||||
|
||||
+10
-5
@@ -58,7 +58,11 @@ class SearchRelayListState(
|
||||
|
||||
fun searchListEvent(note: Note) = note.event as? SearchRelayListEvent ?: settings.backupSearchRelayList
|
||||
|
||||
suspend fun normalizeSearchRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> = searchListEvent(note)?.let { decryptionCache.relays(it) }?.ifEmpty { null } ?: DefaultSearchRelayList
|
||||
suspend fun normalizeSearchRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> {
|
||||
val event = searchListEvent(note) ?: return DefaultSearchRelayList
|
||||
// Fully decrypted here, so empty means the user listed nothing — not "not decrypted yet".
|
||||
return decryptionCache.relays(event)
|
||||
}
|
||||
|
||||
suspend fun normalizeSearchRelayListWithBackupNoDefaults(note: Note): Set<NormalizedRelayUrl> = searchListEvent(note)?.let { decryptionCache.relays(it) } ?: emptySet()
|
||||
|
||||
@@ -75,15 +79,16 @@ class SearchRelayListState(
|
||||
fun normalizeSearchRelayListPrecached(note: Note): Set<NormalizedRelayUrl> = searchListEvent(note)?.let { decryptionCache.cachedRelays(it) }?.ifEmpty { null } ?: DefaultSearchRelayList
|
||||
|
||||
/**
|
||||
* The account's search relays, **never empty** — [normalizeSearchRelayListWithBackup]
|
||||
* substitutes [DefaultSearchRelayList] both when there is no kind:10007 and when the
|
||||
* one we have decodes to zero relays. Callers assembling NIP-50 REQs read this and can
|
||||
* The account's search relays. [normalizeSearchRelayListWithBackup] substitutes
|
||||
* [DefaultSearchRelayList] when there is no kind:10007 at all — but **not** when the one we
|
||||
* have decodes to zero relays, which is the user saying "no search relays" and is honored.
|
||||
* Callers assembling NIP-50 REQs must tolerate an empty set, and can
|
||||
* rely on getting a usable set; use [flowNoDefaults] instead to show or diff what the
|
||||
* user actually configured.
|
||||
*
|
||||
* Seeded via [normalizeSearchRelayListPrecached] rather than `emptySet()`: `flowOn(IO)` means
|
||||
* the first real emission can never be synchronous with `stateIn`, so an `emptySet()` seed
|
||||
* left a window where `.value` contradicted the "never empty" contract above and search
|
||||
* left a window where `.value` reported nothing before the event had been read at all, so search
|
||||
* silently queried nothing. That window is unbounded for a NIP-46 signer whose list has
|
||||
* private entries, since the first emission waits on a remote decrypt.
|
||||
*/
|
||||
|
||||
+3
-2
@@ -21,6 +21,7 @@
|
||||
package com.vitorpamplona.amethyst.model.nip65RelayList
|
||||
|
||||
import com.vitorpamplona.amethyst.commons.defaults.Constants
|
||||
import com.vitorpamplona.amethyst.commons.defaults.relayListOrDefaultsWhenUnknown
|
||||
import com.vitorpamplona.amethyst.model.AccountSettings
|
||||
import com.vitorpamplona.amethyst.model.LocalCache
|
||||
import com.vitorpamplona.amethyst.model.Note
|
||||
@@ -58,9 +59,9 @@ class Nip65RelayListState(
|
||||
|
||||
fun nip65Event(note: Note) = note.event as? AdvertisedRelayListEvent ?: settings.backupNIP65RelayList
|
||||
|
||||
fun normalizeNIP65WriteRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> = nip65Event(note)?.writeRelaysNorm()?.toSet() ?: Constants.eventFinderRelays
|
||||
fun normalizeNIP65WriteRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> = relayListOrDefaultsWhenUnknown(nip65Event(note), Constants.eventFinderRelays) { it.writeRelaysNorm()?.toSet() }
|
||||
|
||||
fun normalizeNIP65ReadRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> = nip65Event(note)?.readRelaysNorm()?.toSet() ?: Constants.bootstrapInbox
|
||||
fun normalizeNIP65ReadRelayListWithBackup(note: Note): Set<NormalizedRelayUrl> = relayListOrDefaultsWhenUnknown(nip65Event(note), Constants.bootstrapInbox) { it.readRelaysNorm()?.toSet() }
|
||||
|
||||
fun normalizeNIP65WriteRelayListNoDefaults(note: Note): Set<NormalizedRelayUrl> = nip65Event(note)?.writeRelaysNorm()?.toSet() ?: emptySet()
|
||||
|
||||
|
||||
+52
@@ -0,0 +1,52 @@
|
||||
/*
|
||||
* 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.commons.defaults
|
||||
|
||||
/**
|
||||
* Substitute app defaults only when we have **never seen** the user's list — never when they
|
||||
* published an empty one.
|
||||
*
|
||||
* There are three states, and collapsing the last two is how the app ends up overriding an explicit
|
||||
* choice:
|
||||
*
|
||||
* | we have | effective list |
|
||||
* |---|---|
|
||||
* | no event | [defaults] — we do not know what they want |
|
||||
* | an event, empty list | **empty** — they told us: nothing |
|
||||
* | an event with relays | those relays |
|
||||
*
|
||||
* Written as one named primitive because the rule was open-coded at four call sites and three of
|
||||
* them got it wrong the same way: `event?.relays()?.ifEmpty { null } ?: DEFAULTS` reads naturally
|
||||
* but folds "published nothing" into "published nothing we know of", so a kind:10002 carrying only
|
||||
* write relays silently acquired a default *inbox* list.
|
||||
*
|
||||
* [read] may return null — several event accessors end in `.ifEmpty { null }` — and null from a
|
||||
* present event means the same thing as an empty set: the user listed nothing.
|
||||
*
|
||||
* **Not for partially-resolved sources.** A reader that can only see *already decrypted* private
|
||||
* tags returns empty both for "the user listed nothing" and for "we have not decrypted it yet",
|
||||
* which this cannot distinguish; those callers legitimately want defaults until the decrypt lands.
|
||||
*/
|
||||
inline fun <E : Any, T> relayListOrDefaultsWhenUnknown(
|
||||
event: E?,
|
||||
defaults: Set<T>,
|
||||
read: (E) -> Set<T>?,
|
||||
): Set<T> = if (event == null) defaults else read(event) ?: emptySet()
|
||||
+69
@@ -0,0 +1,69 @@
|
||||
/*
|
||||
* 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.commons.defaults
|
||||
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
|
||||
class RelayListDefaultsTest {
|
||||
private val defaults = setOf("wss://default.one", "wss://default.two")
|
||||
|
||||
/** No event: we genuinely do not know what the user wants, so the app's defaults stand in. */
|
||||
@Test
|
||||
fun `absent event yields the defaults`() {
|
||||
assertEquals(defaults, relayListOrDefaultsWhenUnknown<String, String>(null, defaults) { setOf("wss://ignored") })
|
||||
}
|
||||
|
||||
/**
|
||||
* The case every open-coded copy of this rule got wrong: a published-but-empty list is the user
|
||||
* saying "nothing", and must not acquire the defaults.
|
||||
*/
|
||||
@Test
|
||||
fun `present event with an empty list stays empty`() {
|
||||
assertEquals(emptySet(), relayListOrDefaultsWhenUnknown("event", defaults) { emptySet<String>() })
|
||||
}
|
||||
|
||||
/**
|
||||
* Same, via null: several event accessors end in `.ifEmpty { null }`, so null from a *present*
|
||||
* event means the user listed nothing — not that the event is missing.
|
||||
*/
|
||||
@Test
|
||||
fun `present event whose reader returns null stays empty`() {
|
||||
assertEquals(emptySet(), relayListOrDefaultsWhenUnknown<String, String>("event", defaults) { null })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present event with relays yields those relays`() {
|
||||
val mine = setOf("wss://mine.example")
|
||||
assertEquals(mine, relayListOrDefaultsWhenUnknown("event", defaults) { mine })
|
||||
}
|
||||
|
||||
/** The reader must not even be consulted when there is no event to read. */
|
||||
@Test
|
||||
fun `reader is not invoked for an absent event`() {
|
||||
var invoked = false
|
||||
relayListOrDefaultsWhenUnknown<String, String>(null, defaults) {
|
||||
invoked = true
|
||||
emptySet()
|
||||
}
|
||||
assertEquals(false, invoked)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user