mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-11 16:57:39 +00:00
fix(nip29): split group-state REQs so relays stop rejecting them
A joined NIP-29 group showed its raw hex id instead of its name — in the
header, the Messages list and the browse directory — offered "Join" to an
account the relay already listed as an ADMIN, and hid the entire admin
surface. Members, Edit group, Invite people, the Threads "+" FAB,
pin/unpin, share and the member count all sit behind the same
`isMember()` gate, so none of them were reachable. Posting was blocked
too.
The cause is not membership modelling, warmup skipping, or the local
kind-10009 list. **The relay rejected the subscription outright.** The app
asked for all five group-state kinds in one filter, captured on the wire:
SEND ["REQ",…,{"kinds":[39000,39001,39002,39003,39005],"#d":[…]}]
RECV ["CLOSED",…,"blocked: it's not allowed to mix metadata kinds
with others"]
The whole REQ is dropped, so zero 39000/39001/39002 reach LocalCache.
Membership is derived from those events, so it fell back to NONE and
every gated control disappeared. Chat messages arrived on a separate `#h`
REQ, which is why the group looked half-alive rather than broken.
relay29/khatru29 evaluates that rule PER FILTER, confirmed by probe:
39000-39003 + 39005 in one filter -> CLOSED, 0 events
the same kinds as two filters -> 4 events, EOSE
39000-39003 alone -> 4 events
So the fix is to split the kinds into two filters in the same REQ — no
extra subscription, no new assembler. This repairs every joined group on
any relay29-family relay, not just the one that surfaced it: the always-on
joined-state subscription was hitting the identical rejection everywhere.
Two hypotheses were tested and disproved rather than assumed. The
join-tap/warmup theory was wrong — instrumentation showed the filter was
built correctly with the right scope and only failed on the wire. The
`since`-on-replaceable theory was also wrong here: `since` logged null on
every call, since the map is per-subscription and reset on disconnect
rather than a persisted floor. Both left unchanged.
Regression tests fail on the old single-filter behaviour and pass on the
new one, verified by reverting the behaviour while keeping the constants
so the tests genuinely run rather than fail to compile.
Verified on device: header now reads "Amethyst QA 1.13" with an Admin
badge and member count 2, composer enabled, overflow menu showing
Members / Edit group / Invite people / Leave — all consistent with the
relay's roster.
Not fixed here: a stale "Requested" state is client-side only and is
never reconciled against an arriving roster. With this fix a genuine
pending→member transition now resolves, but a rejected 9021 still shows
"Requested" until restart. Separate change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
a87049e451
commit
ab2f4d2da8
+13
-24
@@ -22,45 +22,34 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.datas
|
||||
|
||||
import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChannel
|
||||
import com.vitorpamplona.amethyst.service.relays.SincePerRelayMap
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.relayGroup.datasource.RELAY_GROUP_METADATA_KINDS
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.relayGroup.datasource.RELAY_GROUP_PIN_KINDS
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.client.pool.RelayBasedFilter
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupAdminsEvent
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupMembersEvent
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupMetadataEvent
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupPinnedEvent
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.SupportedRolesEvent
|
||||
|
||||
/** Relay-signed group directory kinds: metadata + admins + members + roles + pins. */
|
||||
private val RELAY_GROUP_METADATA_KINDS =
|
||||
listOf(
|
||||
GroupMetadataEvent.KIND,
|
||||
GroupAdminsEvent.KIND,
|
||||
GroupMembersEvent.KIND,
|
||||
SupportedRolesEvent.KIND,
|
||||
GroupPinnedEvent.KIND,
|
||||
)
|
||||
|
||||
/**
|
||||
* The relay-signed metadata for a NIP-29 group (name/picture/about + admin,
|
||||
* member and role lists), addressed by the group id (`d` tag) and pinned to the
|
||||
* group's host relay. The relay signs these with its own key, so a single-relay
|
||||
* query scoped by `#d` returns exactly this group's directory.
|
||||
*
|
||||
* The 39000-39003 metadata block and the 39005 pin list go out as **two separate filters**: relay29-family
|
||||
* relays (0xchat's included) reject a filter that mixes them and drop the whole REQ, which would leave the
|
||||
* group with no name, no roster and no membership. See
|
||||
* [com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.relayGroup.datasource.RELAY_GROUP_PIN_KINDS].
|
||||
*/
|
||||
fun filterRelayGroupState(
|
||||
channel: RelayGroupChannel,
|
||||
since: SincePerRelayMap?,
|
||||
): List<RelayBasedFilter> {
|
||||
val relays = channel.relays().toSet()
|
||||
val scope = mapOf("d" to listOf(channel.groupId.id))
|
||||
val directory =
|
||||
relays.map {
|
||||
RelayBasedFilter(
|
||||
relay = it,
|
||||
filter =
|
||||
Filter(
|
||||
kinds = RELAY_GROUP_METADATA_KINDS,
|
||||
tags = mapOf("d" to listOf(channel.groupId.id)),
|
||||
since = since?.get(it)?.time,
|
||||
),
|
||||
relays.flatMap {
|
||||
val floor = since?.get(it)?.time
|
||||
listOf(
|
||||
RelayBasedFilter(relay = it, filter = Filter(kinds = RELAY_GROUP_METADATA_KINDS, tags = scope, since = floor)),
|
||||
RelayBasedFilter(relay = it, filter = Filter(kinds = RELAY_GROUP_PIN_KINDS, tags = scope, since = floor)),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+41
-21
@@ -45,16 +45,43 @@ import com.vitorpamplona.quartz.nipC7Chats.ChatEvent
|
||||
* See amethyst/plans/2026-07-18-nip29-group-chat-subscriptions.md and the companion test plan.
|
||||
*/
|
||||
|
||||
/** Relay-signed group *state*: metadata + admins + members + roles + pins. Small replaceable events. */
|
||||
val RELAY_GROUP_STATE_KINDS =
|
||||
/**
|
||||
* The relay's **directory** kinds for a group — metadata + admins + members + roles (39000-39003).
|
||||
* These four are what NIP-29 relays treat as a group's "metadata" block, and they must be requested
|
||||
* **alone**: see [RELAY_GROUP_PIN_KINDS].
|
||||
*/
|
||||
val RELAY_GROUP_METADATA_KINDS =
|
||||
listOf(
|
||||
GroupMetadataEvent.KIND,
|
||||
GroupAdminsEvent.KIND,
|
||||
GroupMembersEvent.KIND,
|
||||
SupportedRolesEvent.KIND,
|
||||
GroupPinnedEvent.KIND,
|
||||
)
|
||||
|
||||
/**
|
||||
* The pin list (39005), deliberately kept in its **own** filter rather than merged into
|
||||
* [RELAY_GROUP_METADATA_KINDS].
|
||||
*
|
||||
* NIP-29 relays derived from `relay29`/`khatru29` (0xchat's `groups.0xchat.com` among them) reject a REQ
|
||||
* whose filter mixes the 39000-39003 metadata kinds with any other kind, replying
|
||||
* `CLOSED … "blocked: it's not allowed to mix metadata kinds with others"`. A single filter asking for
|
||||
* 39000-39003 **plus** 39005 is therefore dropped **whole** — the group never resolves its name, roster,
|
||||
* roles or the user's own membership, so it renders as a raw id and offers "Join" to somebody the relay
|
||||
* already lists as an admin.
|
||||
*
|
||||
* Splitting into two filter objects fixes it: those relays evaluate the rule per filter, so the
|
||||
* metadata filter is served normally and the pins filter is served (or harmlessly ignored) on its own.
|
||||
*/
|
||||
val RELAY_GROUP_PIN_KINDS = listOf(GroupPinnedEvent.KIND)
|
||||
|
||||
/**
|
||||
* Every relay-signed group *state* kind: metadata + admins + members + roles + pins. Small replaceable
|
||||
* events. **Never put this list on the wire as one filter** — request [RELAY_GROUP_METADATA_KINDS] and
|
||||
* [RELAY_GROUP_PIN_KINDS] as separate filters instead (see [RELAY_GROUP_PIN_KINDS]). Kept as the
|
||||
* semantic "all state kinds" set for cache/consume-side code.
|
||||
*/
|
||||
val RELAY_GROUP_STATE_KINDS = RELAY_GROUP_METADATA_KINDS + RELAY_GROUP_PIN_KINDS
|
||||
|
||||
/** Timeline kinds shown in a group's chat — chat messages and polls. */
|
||||
val RELAY_GROUP_TIMELINE_KINDS = listOf(ChatEvent.KIND, PollEvent.KIND)
|
||||
|
||||
@@ -69,13 +96,7 @@ val RELAY_GROUP_CARD_WARMUP_KINDS = listOf(ChatEvent.KIND, PollEvent.KIND, Threa
|
||||
* Narrower than [RELAY_GROUP_STATE_KINDS] on purpose: the directory lists groups, it doesn't need each
|
||||
* group's pin list.
|
||||
*/
|
||||
val RELAY_GROUP_DIRECTORY_KINDS =
|
||||
listOf(
|
||||
GroupMetadataEvent.KIND,
|
||||
GroupAdminsEvent.KIND,
|
||||
GroupMembersEvent.KIND,
|
||||
SupportedRolesEvent.KIND,
|
||||
)
|
||||
val RELAY_GROUP_DIRECTORY_KINDS = RELAY_GROUP_METADATA_KINDS
|
||||
|
||||
/** How many directory entries to pull per relay when browsing its whole group list. */
|
||||
const val RELAY_GROUP_DIRECTORY_LIMIT = 500
|
||||
@@ -93,22 +114,21 @@ private fun byHostRelay(joined: Collection<GroupTag>): Map<NormalizedRelayUrl, L
|
||||
}
|
||||
|
||||
/**
|
||||
* State (39000-39005) for every joined group, **one `#d` filter per host relay** carrying that relay's
|
||||
* group ids. `since` is per-relay (replaceable events; a reconnect just re-confirms).
|
||||
* State (39000-39005) for every joined group, **two `#d` filters per host relay** carrying that relay's
|
||||
* group ids: the 39000-39003 metadata block and the 39005 pin list, kept apart because relay29-family
|
||||
* relays refuse a filter that mixes them (see [RELAY_GROUP_PIN_KINDS]). `since` is per-relay (replaceable
|
||||
* events; a reconnect just re-confirms).
|
||||
*/
|
||||
fun buildRelayGroupStateFilters(
|
||||
joined: Collection<GroupTag>,
|
||||
sinceForRelay: (NormalizedRelayUrl) -> Long?,
|
||||
): List<RelayBasedFilter> =
|
||||
byHostRelay(joined).map { (relay, ids) ->
|
||||
RelayBasedFilter(
|
||||
relay = relay,
|
||||
filter =
|
||||
Filter(
|
||||
kinds = RELAY_GROUP_STATE_KINDS,
|
||||
tags = mapOf(D_TAG to ids.distinct()),
|
||||
since = sinceForRelay(relay),
|
||||
),
|
||||
byHostRelay(joined).flatMap { (relay, ids) ->
|
||||
val scope = mapOf(D_TAG to ids.distinct())
|
||||
val since = sinceForRelay(relay)
|
||||
listOf(
|
||||
RelayBasedFilter(relay = relay, filter = Filter(kinds = RELAY_GROUP_METADATA_KINDS, tags = scope, since = since)),
|
||||
RelayBasedFilter(relay = relay, filter = Filter(kinds = RELAY_GROUP_PIN_KINDS, tags = scope, since = since)),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+42
-16
@@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupMetadataEvent
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.GroupPinnedEvent
|
||||
import com.vitorpamplona.quartz.nip29RelayGroups.metadata.SupportedRolesEvent
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
@@ -45,28 +46,51 @@ class FilterRelayGroupStateTest {
|
||||
private val relaySignKey = "b".repeat(64)
|
||||
private val sig = "0".repeat(128)
|
||||
|
||||
private val stateKinds =
|
||||
private val metadataKinds =
|
||||
listOf(
|
||||
GroupMetadataEvent.KIND,
|
||||
GroupAdminsEvent.KIND,
|
||||
GroupMembersEvent.KIND,
|
||||
SupportedRolesEvent.KIND,
|
||||
GroupPinnedEvent.KIND,
|
||||
)
|
||||
private val pinKinds = listOf(GroupPinnedEvent.KIND)
|
||||
|
||||
@Test
|
||||
fun `with no pins it is a single host d-scoped state filter and no message window`() {
|
||||
fun `with no pins it is host d-scoped state filters and no message window`() {
|
||||
val channel = RelayGroupChannel(groupId)
|
||||
|
||||
val filters = filterRelayGroupState(channel, since = null)
|
||||
|
||||
val f = filters.single()
|
||||
assertEquals(relayA, f.relay)
|
||||
assertEquals(stateKinds, f.filter.kinds)
|
||||
assertEquals(listOf("g1"), f.filter.tags!!["d"])
|
||||
assertNull("state is #d-scoped, never #h — the message window is the tail/pager's job", f.filter.tags!!["h"])
|
||||
assertNull("state carries no message-window kinds (9/poll), so no limit either", f.filter.limit)
|
||||
assertNull(f.filter.until)
|
||||
assertEquals(2, filters.size)
|
||||
filters.forEach { f ->
|
||||
assertEquals(relayA, f.relay)
|
||||
assertEquals(listOf("g1"), f.filter.tags!!["d"])
|
||||
assertNull("state is #d-scoped, never #h — the message window is the tail/pager's job", f.filter.tags!!["h"])
|
||||
assertNull("state carries no message-window kinds (9/poll), so no limit either", f.filter.limit)
|
||||
assertNull(f.filter.until)
|
||||
}
|
||||
assertEquals(metadataKinds, filters[0].filter.kinds)
|
||||
assertEquals(pinKinds, filters[1].filter.kinds)
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression: relay29-family relays (0xchat's `groups.0xchat.com`) answer a filter that mixes the
|
||||
* 39000-39003 metadata kinds with any other kind — 39005 pins included — with
|
||||
* `CLOSED … "blocked: it's not allowed to mix metadata kinds with others"`, dropping the WHOLE REQ.
|
||||
* The group then never learns its name, roster or the user's own membership, so it renders as a raw
|
||||
* id and offers "Join" to somebody the relay already lists as an admin. Keep the two apart.
|
||||
*/
|
||||
@Test
|
||||
fun `pins are never mixed into the metadata filter`() {
|
||||
val channel = RelayGroupChannel(groupId)
|
||||
|
||||
filterRelayGroupState(channel, since = null).forEach { f ->
|
||||
val kinds = f.filter.kinds ?: return@forEach
|
||||
assertFalse(
|
||||
"39005 must not share a filter with the 39000-39003 metadata block: $kinds",
|
||||
kinds.contains(GroupPinnedEvent.KIND) && kinds.any { it in metadataKinds },
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -85,7 +109,7 @@ class FilterRelayGroupStateTest {
|
||||
)
|
||||
|
||||
val filters = filterRelayGroupState(channel, since = null)
|
||||
assertEquals(2, filters.size)
|
||||
assertEquals(3, filters.size)
|
||||
|
||||
val pinFilter = filters.first { it.filter.ids != null }
|
||||
assertEquals(relayA, pinFilter.relay)
|
||||
@@ -93,10 +117,12 @@ class FilterRelayGroupStateTest {
|
||||
assertNull("pinned bodies are fetched by id, so no kinds", pinFilter.filter.kinds)
|
||||
assertNull("pinned events are immutable, so no since either", pinFilter.filter.since)
|
||||
|
||||
// The state filter is still present and still carries no #h message window.
|
||||
val stateFilter = filters.first { it.filter.ids == null }
|
||||
assertEquals(stateKinds, stateFilter.filter.kinds)
|
||||
assertTrue(stateFilter.filter.tags!!.containsKey("d"))
|
||||
assertNull(stateFilter.filter.tags!!["h"])
|
||||
// The state filters are still present and still carry no #h message window.
|
||||
val stateFilters = filters.filter { it.filter.ids == null }
|
||||
assertEquals(listOf(metadataKinds, pinKinds), stateFilters.map { it.filter.kinds })
|
||||
stateFilters.forEach {
|
||||
assertTrue(it.filter.tags!!.containsKey("d"))
|
||||
assertNull(it.filter.tags!!["h"])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+30
-10
@@ -65,24 +65,44 @@ class RelayGroupFilterBuildersTest {
|
||||
// --- State (always-on): one #d filter per host relay, batching that relay's group ids ---
|
||||
|
||||
@Test
|
||||
fun `state batches one d-filter per host relay`() {
|
||||
fun `state batches d-filters per host relay, metadata and pins kept apart`() {
|
||||
val filters = buildRelayGroupStateFilters(joined) { null }
|
||||
assertEquals(2, filters.size)
|
||||
// two relays x (metadata + pins)
|
||||
assertEquals(4, filters.size)
|
||||
|
||||
val a = filters.single { it.relay == relayA }
|
||||
assertEquals(RELAY_GROUP_STATE_KINDS, a.filter.kinds)
|
||||
assertEquals(setOf("g1", "g2"), a.filter.tags!!["d"]!!.toSet())
|
||||
assertNull("state is #d-scoped, never #h", a.filter.tags!!["h"])
|
||||
val a = filters.filter { it.relay == relayA }
|
||||
assertEquals(listOf(RELAY_GROUP_METADATA_KINDS, RELAY_GROUP_PIN_KINDS), a.map { it.filter.kinds })
|
||||
a.forEach {
|
||||
assertEquals(setOf("g1", "g2"), it.filter.tags!!["d"]!!.toSet())
|
||||
assertNull("state is #d-scoped, never #h", it.filter.tags!!["h"])
|
||||
}
|
||||
|
||||
val b = filters.single { it.relay == relayB }
|
||||
assertEquals(listOf("g3"), b.filter.tags!!["d"])
|
||||
val b = filters.filter { it.relay == relayB }
|
||||
b.forEach { assertEquals(listOf("g3"), it.filter.tags!!["d"]) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression: relay29-family relays (0xchat's `groups.0xchat.com`) reject a filter mixing the
|
||||
* 39000-39003 metadata kinds with any other kind (39005 pins included) with
|
||||
* `CLOSED … "blocked: it's not allowed to mix metadata kinds with others"` and drop the whole REQ —
|
||||
* so every joined group on such a relay silently loses its name, roster and membership.
|
||||
*/
|
||||
@Test
|
||||
fun `state never mixes pins into the metadata filter`() {
|
||||
buildRelayGroupStateFilters(joined) { null }.forEach { f ->
|
||||
val kinds = f.filter.kinds!!
|
||||
assertFalse(
|
||||
"39005 must not share a filter with the 39000-39003 metadata block: $kinds",
|
||||
kinds.contains(GroupPinnedEvent.KIND) && kinds.any { it in RELAY_GROUP_METADATA_KINDS },
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `state applies the per-relay since`() {
|
||||
val filters = buildRelayGroupStateFilters(joined) { relay -> if (relay == relayA) 111L else null }
|
||||
assertEquals(111L, filters.single { it.relay == relayA }.filter.since)
|
||||
assertNull(filters.single { it.relay == relayB }.filter.since)
|
||||
filters.filter { it.relay == relayA }.forEach { assertEquals(111L, it.filter.since) }
|
||||
filters.filter { it.relay == relayB }.forEach { assertNull(it.filter.since) }
|
||||
}
|
||||
|
||||
// --- Joined chat tail (always-on): batched #h per relay, time floor, NO per-group limit ---
|
||||
|
||||
Reference in New Issue
Block a user