mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 16:14:40 +00:00
fix: route stray NIP-29 group content to the host, not a phantom channel
A group-scoped content event (kind-9 chat, poll, kind-11 thread) is keyed to its RelayGroupChannel by the relay that served it, because a NIP-29 event doesn't carry its host relay. That's correct for the group's own host-pinned subscriptions, but a message resolved from a NON-host relay -- e.g. a quoted kind-9 fetched by id during missing-event resolution -- was filed under GroupId(groupId, strangerRelay), a channel the group's screens never read, so the message silently vanished (the serving-relay hazard). LocalCache.attachToRelayGroupIfScoped / attachThreadToRelayGroupIfScoped now, when no channel is keyed to the serving relay, redirect the stray to the group's single confirmed host channel via redirectStrayRelayGroupContent, keyed off RelayGroupChannel.hasRelaySignedState(). A phantom channel never has relay-signed state, so the redirect can only ever land on a real host, never on another phantom -- the fix is strictly safe and the common host-pinned arrival stays an untouched O(1) fast path (the scan runs only on the rare no-channel-for-serving-relay miss). Also add the cache-prune gap-fill can't-miss test: drive the production RelayLoadingCursors down a real relay, rewindTo below the window, and confirm the pruned band re-loads with no gap. Tests: RelayGroupContentRoutingTest (pure router + the channel signal); RelayGroupHistoryPagingRelayTest gains the rewind reload case. The LocalCache wiring is unit-covered at the router level but still device- untested end-to-end (flagged in the test plan). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDK63toGbE7DQxKxrQnhMU
This commit is contained in:
@@ -169,7 +169,7 @@ is buggy** — both are needed before trusting "loads correctly" in the wild.
|
||||
- **No double-download on reconnect** (C9/D8).
|
||||
- **Retirement left no gap:** with `RelayGroupMyJoinedGroups` deleted and `ChannelPublic`/`ChannelFromUser` relay-group content removed, D1/D2/D3 still load — proving the tail+pager replaced them.
|
||||
- **CardWarmup joined-skip** (Tier B #6 / D5): a joined card issues no warmup REQ.
|
||||
- **Serving-relay hazard (known open item):** quote a group message fetched from a **non-host** relay (`filterMissingEvents`) and confirm whether it appears in the group (`GroupId(G, hostR)`) or is filed under `GroupId(G, otherR)` and lost. Documents the plan's §2 follow-up; expected to **fail** until that fix lands.
|
||||
- **Serving-relay hazard (FIXED):** a group message fetched from a **non-host** relay (e.g. `filterMissingEvents` quote resolution) used to be filed under `GroupId(G, otherR)` and lost. `LocalCache.attachToRelayGroupIfScoped`/`attachThreadToRelayGroupIfScoped` now redirect a stray (no channel for the serving relay) to the group's single confirmed **host** channel via `redirectStrayRelayGroupContent`, keyed off `RelayGroupChannel.hasRelaySignedState()` (a phantom never has relay-signed state, so the redirect only ever lands on a real host — strictly safe, the common host-pinned path is an untouched O(1) fast path). Covered by `RelayGroupContentRoutingTest`. **Device-untested:** the pure router + channel signal are unit-tested; the LocalCache wiring is a guarded fast-path/slow-path swap that still needs a device pass (Tier D) to confirm end-to-end.
|
||||
|
||||
## Exit criteria
|
||||
- Tier A green; Tier B all assertions pass; Tier C1–C9 pass (esp. **C3**).
|
||||
|
||||
@@ -2006,9 +2006,20 @@ object LocalCache : ILocalCache, ICacheProvider {
|
||||
if (note.event == null) return
|
||||
|
||||
if (relay != null) {
|
||||
// Normal arrival: the group only exists on its host relay and the
|
||||
// filters are host-pinned, so the serving relay is the group's key.
|
||||
getOrCreateRelayGroupChannel(GroupId(groupId, relay)).addNote(note, relay)
|
||||
val exact = GroupId(groupId, relay)
|
||||
val existing = getRelayGroupChannelIfExists(exact)
|
||||
if (existing != null) {
|
||||
// Normal arrival: the group's host-pinned filters served it, so the serving relay IS the
|
||||
// group's key and its channel already exists. Fast O(1) path — no scan.
|
||||
existing.addNote(note, relay)
|
||||
} else {
|
||||
// No channel keyed to the serving relay: this may be a stray from a NON-host relay (e.g. a
|
||||
// quoted kind-9 resolved by id). Redirect it to the group's single confirmed host rather
|
||||
// than mint a phantom channel the group's screens never read (the serving-relay hazard);
|
||||
// fall back to the serving-relay key when there is no single host (new/ambiguous group).
|
||||
val target = redirectStrayRelayGroupContent(relayGroupCandidatesFor(groupId)) ?: exact
|
||||
getOrCreateRelayGroupChannel(target).addNote(note, relay)
|
||||
}
|
||||
} else {
|
||||
// Our own optimistic send has no provenance relay, so we can't build the (groupId,
|
||||
// relay) key. Attach only when a SINGLE open channel has this group id (the room being
|
||||
@@ -2022,6 +2033,12 @@ object LocalCache : ILocalCache, ICacheProvider {
|
||||
}
|
||||
}
|
||||
|
||||
/** Candidate group channels for the [redirectStrayRelayGroupContent] slow path — one scan by group id. */
|
||||
private fun relayGroupCandidatesFor(groupId: String): List<RelayGroupTargetCandidate> =
|
||||
relayGroupChannels
|
||||
.filter { key, _ -> key.id == groupId }
|
||||
.map { RelayGroupTargetCandidate(it.groupId, it.hasRelaySignedState()) }
|
||||
|
||||
/**
|
||||
* Same routing as [attachToRelayGroupIfScoped] but for kind-11 threads, which
|
||||
* are kept in a separate collection from the chat timeline so the two content
|
||||
@@ -2036,7 +2053,15 @@ object LocalCache : ILocalCache, ICacheProvider {
|
||||
if (note.event == null) return
|
||||
|
||||
if (relay != null) {
|
||||
getOrCreateRelayGroupChannel(GroupId(groupId, relay)).addThread(note)
|
||||
val exact = GroupId(groupId, relay)
|
||||
val existing = getRelayGroupChannelIfExists(exact)
|
||||
if (existing != null) {
|
||||
existing.addThread(note)
|
||||
} else {
|
||||
// Same serving-relay hazard as the chat path: prefer the single confirmed host over a phantom.
|
||||
val target = redirectStrayRelayGroupContent(relayGroupCandidatesFor(groupId)) ?: exact
|
||||
getOrCreateRelayGroupChannel(target).addThread(note)
|
||||
}
|
||||
} else {
|
||||
// See attachToRelayGroupIfScoped: only attach when the group id is unambiguous.
|
||||
relayGroupChannels
|
||||
|
||||
@@ -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.model
|
||||
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.GroupId
|
||||
|
||||
/**
|
||||
* A candidate group channel when routing a stray group-scoped content event: its [key] and whether it is a
|
||||
* confirmed host (has received relay-signed state). See [redirectStrayRelayGroupContent].
|
||||
*/
|
||||
data class RelayGroupTargetCandidate(
|
||||
val key: GroupId,
|
||||
val hasRelaySignedState: Boolean,
|
||||
)
|
||||
|
||||
/**
|
||||
* Resolves the **serving-relay hazard**. A group-scoped content event (kind-9 chat, poll, kind-11 thread…)
|
||||
* is keyed to its group channel by the relay that served it, because a NIP-29 event doesn't carry its host
|
||||
* relay. That is correct for the group's own host-pinned subscriptions, but a message resolved from a
|
||||
* **non-host** relay — e.g. a quoted kind-9 fetched by id during missing-event resolution — would be filed
|
||||
* under a channel keyed to that stranger relay, one the group's own screens never read, so the message
|
||||
* silently vanishes.
|
||||
*
|
||||
* Called only when there is **no** channel keyed to the serving relay for this group id (the fast, common
|
||||
* path attaches directly and never gets here). It picks the group's single confirmed **host** channel — one
|
||||
* that has received relay-signed state — to attach the stray to instead. Returns that host key, or null when
|
||||
* there is no single confirmed host (a genuinely new group on the serving relay, or an id ambiguous across
|
||||
* several hosts), in which case the caller keeps the serving-relay key as today's best effort.
|
||||
*
|
||||
* A phantom channel (one minted from an earlier stray) never has relay-signed state, so it can never be
|
||||
* chosen here — the redirect only ever lands on a real host, never on another phantom. This makes the fix
|
||||
* strictly safe: it can redirect a stray to a known host, but never divert a message away from one.
|
||||
*/
|
||||
fun redirectStrayRelayGroupContent(candidates: List<RelayGroupTargetCandidate>): GroupId? = candidates.filter { it.hasRelaySignedState }.singleOrNull()?.key
|
||||
+107
@@ -0,0 +1,107 @@
|
||||
/*
|
||||
* 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
|
||||
|
||||
import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChannel
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.GroupId
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupPinnedEvent
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* The serving-relay hazard router: a stray group-scoped content event (no channel keyed to its serving
|
||||
* relay) must be redirected to the group's single confirmed **host** channel, never to another phantom, and
|
||||
* never when the host is ambiguous. Also pins that a phantom channel — one that only ever received content —
|
||||
* reports no relay-signed state, which is what keeps [redirectStrayRelayGroupContent] from ever picking one.
|
||||
*/
|
||||
class RelayGroupContentRoutingTest {
|
||||
private val relayA = RelayUrlNormalizer.normalizeOrNull("wss://relay-a.example/")!!
|
||||
private val relayB = RelayUrlNormalizer.normalizeOrNull("wss://relay-b.example/")!!
|
||||
private val relayC = RelayUrlNormalizer.normalizeOrNull("wss://relay-c.example/")!!
|
||||
|
||||
private fun host(
|
||||
id: String,
|
||||
relay: NormalizedRelayUrl,
|
||||
) = RelayGroupTargetCandidate(GroupId(id, relay), hasRelaySignedState = true)
|
||||
|
||||
private fun phantom(
|
||||
id: String,
|
||||
relay: NormalizedRelayUrl,
|
||||
) = RelayGroupTargetCandidate(GroupId(id, relay), hasRelaySignedState = false)
|
||||
|
||||
@Test
|
||||
fun `no candidates leaves the routing to the caller`() {
|
||||
// A genuinely new group on the serving relay: caller keeps the serving-relay key.
|
||||
assertNull(redirectStrayRelayGroupContent(emptyList()))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a single confirmed host is chosen`() {
|
||||
assertEquals(GroupId("g1", relayA), redirectStrayRelayGroupContent(listOf(host("g1", relayA))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a phantom-only candidate is never chosen`() {
|
||||
// Redirecting a stray onto another phantom would just move it into a second unread channel.
|
||||
assertNull(redirectStrayRelayGroupContent(listOf(phantom("g1", relayB))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the confirmed host wins over a phantom for the same id`() {
|
||||
assertEquals(
|
||||
GroupId("g1", relayA),
|
||||
redirectStrayRelayGroupContent(listOf(phantom("g1", relayB), host("g1", relayA))),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an id claimed by two confirmed hosts is ambiguous and left to the caller`() {
|
||||
assertNull(redirectStrayRelayGroupContent(listOf(host("g1", relayA), host("g1", relayC))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `several phantoms and no host returns null`() {
|
||||
assertNull(redirectStrayRelayGroupContent(listOf(phantom("g1", relayA), phantom("g1", relayB))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a fresh channel has no relay-signed state until a relay-signed event flips it`() {
|
||||
val channel = RelayGroupChannel(GroupId("g1", relayA))
|
||||
assertFalse("a channel with no relay-signed events is a phantom, not a host", channel.hasRelaySignedState())
|
||||
|
||||
channel.updatePinned(
|
||||
GroupPinnedEvent(
|
||||
id = "d".repeat(64),
|
||||
pubKey = "b".repeat(64),
|
||||
createdAt = 1L,
|
||||
tags = arrayOf(arrayOf("d", "g1"), arrayOf("e", "a".repeat(64))),
|
||||
content = "",
|
||||
sig = "0".repeat(128),
|
||||
),
|
||||
)
|
||||
assertTrue("a relay-signed pin list makes it a confirmed host", channel.hasRelaySignedState())
|
||||
}
|
||||
}
|
||||
+14
@@ -139,6 +139,20 @@ class RelayGroupChannel(
|
||||
|
||||
fun threadCount(): Int = threadNotes.size()
|
||||
|
||||
/**
|
||||
* Whether this channel has received any relay-signed state — metadata, roster, roles or pins. Only the
|
||||
* group's **host** relay signs those (via the `isRelaySignedGroupEvent`-gated consume paths), so this is
|
||||
* true only for a confirmed host channel and never for a "phantom" one minted from a stray content event
|
||||
* that arrived from a non-host relay. Used to redirect such strays back to the real host (see
|
||||
* `LocalCache.attachToRelayGroupIfScoped` / the serving-relay hazard).
|
||||
*/
|
||||
fun hasRelaySignedState(): Boolean =
|
||||
event != null ||
|
||||
members.isNotEmpty() ||
|
||||
admins.isNotEmpty() ||
|
||||
supportedRoles.isNotEmpty() ||
|
||||
pinnedEventIds.isNotEmpty()
|
||||
|
||||
/** A relay group lives on exactly one relay: its host. */
|
||||
override fun relays() = setOf(groupId.relayUrl)
|
||||
|
||||
|
||||
+38
-7
@@ -152,16 +152,15 @@ class RelayGroupHistoryPagingRelayTest : RelayClientTest() {
|
||||
// tests that feed it synthetic callbacks (RelayLoadingCursorsTest / BackwardRelayPagerTest).
|
||||
|
||||
/**
|
||||
* Steps the production [RelayLoadingCursors] backward over one group's `#h` chat until it reports done,
|
||||
* exactly as the history assembler does: advance → REQ at the requested `until` → feed each event and
|
||||
* the EOSE back in → advance again. Returns every id delivered.
|
||||
* One backward drain of the production [cursors] over [groupId] until it reports done — exactly as the
|
||||
* history assembler steps it: advance → REQ at the requested `until` → feed each event and the EOSE back
|
||||
* in → advance again. Returns every id delivered.
|
||||
*/
|
||||
private suspend fun driveCursorsToBottom(
|
||||
private suspend fun drainFrom(
|
||||
cursors: RelayLoadingCursors,
|
||||
groupId: String,
|
||||
now: Long,
|
||||
): Set<String> {
|
||||
val cursors = RelayLoadingCursors()
|
||||
cursors.floor = now
|
||||
val relay = defaultRelayUrl
|
||||
val seen = mutableSetOf<String>()
|
||||
cursors.advance(relay, start = now)
|
||||
@@ -175,10 +174,42 @@ class RelayGroupHistoryPagingRelayTest : RelayClientTest() {
|
||||
if (eose) cursors.onEose(relay)
|
||||
cursors.advance(relay, start = now)
|
||||
}
|
||||
assertTrue(cursors.isDone(relay), "the production cursors must reach the bottom (empty page + EOSE)")
|
||||
return seen
|
||||
}
|
||||
|
||||
private suspend fun driveCursorsToBottom(
|
||||
groupId: String,
|
||||
now: Long,
|
||||
): Set<String> {
|
||||
val cursors = RelayLoadingCursors().apply { floor = now }
|
||||
val seen = drainFrom(cursors, groupId, now)
|
||||
assertTrue(cursors.isDone(defaultRelayUrl), "the production cursors must reach the bottom (empty page + EOSE)")
|
||||
return seen
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aCachePruneBelowTheWindowReloadsTheDroppedBandWithNoGap() =
|
||||
runBlocking {
|
||||
// groupChat seeds createdAt == id seed, so hexId(k) is the event at createdAt k.
|
||||
defaultRelay.preload(groupChat(idBase = 1, count = TOTAL, groupId = "g1"))
|
||||
val now = 10_000L
|
||||
val cursors = RelayLoadingCursors().apply { floor = now }
|
||||
|
||||
val firstPass = drainFrom(cursors, "g1", now)
|
||||
assertEquals(TOTAL, firstPass.size)
|
||||
assertTrue(cursors.isDone(defaultRelayUrl), "precondition: the first walk reached the bottom")
|
||||
|
||||
// The cache evicts everything at or below createdAt 150 out of the window. rewindTo pulls the
|
||||
// reached cursor just above the newest pruned event so the next advance re-requests that band —
|
||||
// without it, a done relay whose cursor already sits below the hole would skip it forever.
|
||||
cursors.rewindTo(mapOf(defaultRelayUrl to 150L))
|
||||
|
||||
val reload = drainFrom(cursors, "g1", now)
|
||||
// The re-walk must re-fetch the entire pruned band (createdAt 1..150) — no gap, no skipped id.
|
||||
assertEquals((1..150).map { SyntheticEvents.hexId(it) }.toSet(), reload)
|
||||
assertTrue(cursors.isDone(defaultRelayUrl), "the re-walk terminates on an empty page")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun productionCursorsWalkOneGroupToTheBottomOverTheWire() =
|
||||
runBlocking {
|
||||
|
||||
Reference in New Issue
Block a user