From 31acb5037bdb550dca54c862c4d6a03e9598388a Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Thu, 23 Jul 2026 17:25:52 -0400 Subject: [PATCH] fix(concord): decode the community list even when an entry has a keyless channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A public channel carries no delivered key, so a writer lists it under an entry's `channels` with only {id, epoch, name}. `WireChannel.key` was a required field, so kotlinx.serialization threw MissingFieldException — and `decodeDocument`'s catch-all turned that one bad channel into an empty list, silently dropping EVERY joined community from the kind-13302 list (communities "won't load" at all). - Default `WireChannel.key = ""` so a keyless (public) channel no longer throws. - Decode entries one at a time and keep any we still can't parse verbatim in `ConcordListResidue.unparsedEntries`, re-emitted on write — so one malformed entry can never wipe the whole list, and a read-modify-write never deletes a membership this version can't model. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../cord02Community/ConcordCommunityList.kt | 97 +++++++++++++---- .../ConcordCommunityListTest.kt | 102 ++++++++++++++++++ 2 files changed, 176 insertions(+), 23 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityList.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityList.kt index 871666677b..e77709ecdf 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityList.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityList.kt @@ -29,6 +29,7 @@ import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import kotlinx.serialization.Transient import kotlinx.serialization.descriptors.elementNames +import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive @@ -98,10 +99,16 @@ class ConcordEntryResidue( * - [tombstones] — the document's tombstones, verbatim. We derive liveness from them but * never author one, so dropping them on write would both lose another client's unknown * keys and resurrect communities that client deliberately removed. + * - [unparsedEntries] — entries we could not decode into the typed model, kept verbatim. A + * single entry carrying a wire field this version still requires but a peer omitted would + * otherwise throw and empty the *entire* list; instead we skip just that entry on read and + * re-emit it untouched on write, so a read-modify-write never deletes a membership this + * version merely failed to understand — the same guarantee [tombstones]/[extras] give. */ class ConcordListResidue( val extras: JsonObject = NoExtras, val tombstones: List = emptyList(), + val unparsedEntries: List = emptyList(), ) { companion object { val EMPTY = ConcordListResidue() @@ -229,7 +236,11 @@ object ConcordCommunityList { @Serializable private class WireChannel( val id: String, - val key: String, + // A public channel carries no delivered key, so a writer lists it with only {id, epoch, + // name}. `key` MUST default rather than be required: a single keyless channel would + // otherwise throw MissingFieldException and make the whole document decode to empty, + // silently dropping *every* joined community from the list. + val key: String = "", val epoch: Long, val name: String = "", @SerialName(EXTRAS) val extras: JsonObject = NoExtras, @@ -373,7 +384,15 @@ object ConcordCommunityList { tombstones = residue.tombstones, extras = residue.extras, ) - return ConcordJson.instance.encodeToString(CommunityListDocSerializer, doc) + if (residue.unparsedEntries.isEmpty()) { + return ConcordJson.instance.encodeToString(CommunityListDocSerializer, doc) + } + // Re-attach the entries we couldn't parse, verbatim, so a read-modify-write never deletes + // a membership this version failed to understand. They ride alongside the typed entries. + val encoded = ConcordJson.instance.encodeToJsonElement(CommunityListDocSerializer, doc).jsonObject + val allEntries = ((encoded["entries"] as? JsonArray)?.toList() ?: emptyList()) + residue.unparsedEntries + val merged = JsonObject(encoded + ("entries" to JsonArray(allEntries))) + return ConcordJson.instance.encodeToString(JsonObject.serializer(), merged) } /** @@ -385,38 +404,70 @@ object ConcordCommunityList { /** * Parses the decrypted plaintext JSON document into its live entries plus the - * document-level residue (unknown keys and tombstones) that [encode] must hand back. - * Returns an empty document on failure. + * document-level residue (unknown keys, tombstones, and any entries we couldn't decode) + * that [encode] must hand back. + * + * Entries are decoded **one at a time**: an entry this version can't parse (a wire field we + * still require that a peer omitted) is skipped and kept verbatim in + * [ConcordListResidue.unparsedEntries], never allowed to fail the whole document and empty + * the user's entire community list. Returns an empty document only when the outer structure + * itself is unreadable. */ fun decodeDocument(json: String): ConcordCommunityListDocument = try { - val doc = ConcordJson.instance.decodeFromString(CommunityListDocSerializer, json) + val root = ConcordJson.instance.parseToJsonElement(json).jsonObject + val tombstones = (root["tombstones"] as? JsonArray)?.mapNotNull { it as? JsonObject } ?: emptyList() + val latestRemoval = HashMap() - for (t in doc.tombstones) { + for (t in tombstones) { val id = (t["community_id"] as? JsonPrimitive)?.contentOrNull ?: continue val removedAt = (t["removed_at"] as? JsonPrimitive)?.longOrNull ?: 0L val prev = latestRemoval[id] if (prev == null || removedAt > prev) latestRemoval[id] = removedAt } - val entries = - doc.entries.mapNotNull { e -> - val removedAt = latestRemoval[e.communityId] - if (removedAt != null && e.addedAt <= removedAt) return@mapNotNull null - val current = e.current - val seed = e.seed?.let { ConcordJson.instance.decodeFromJsonElement(JoinMaterialWireSerializer, it) } - // Hydrating from `seed` mints a fresh `current`; its unknown keys stay safe in - // the verbatim seed, so they are not copied into the new snapshot. - val residue = - ConcordEntryResidue( - entryExtras = e.extras, - seed = e.seed, - currentExtras = if (current != null) current.extras else NoExtras, - ) - (current ?: seed)?.toEntry(e.addedAt, e.inviteRef, e.excludedAtEpoch, residue) - } + + val entries = ArrayList() + val unparsed = ArrayList() + for (element in (root["entries"] as? JsonArray ?: emptyList())) { + val wireObj = element as? JsonObject ?: continue + val e = + try { + ConcordJson.instance.decodeFromJsonElement(CommunityListEntryWireSerializer, wireObj) + } catch (_: Exception) { + // Skip just this entry, but keep it verbatim so a later write re-emits it + // instead of deleting a membership this version failed to understand. + unparsed.add(wireObj) + continue + } + val removedAt = latestRemoval[e.communityId] + if (removedAt != null && e.addedAt <= removedAt) continue + val current = e.current + // Hydrating from `seed` mints a fresh `current`; its unknown keys stay safe in the + // verbatim seed, so they are not copied into the new snapshot. `seed` decode is + // guarded too: it only hydrates when `current` is absent, and survives in residue. + val seed = + e.seed?.let { + try { + ConcordJson.instance.decodeFromJsonElement(JoinMaterialWireSerializer, it) + } catch (_: Exception) { + null + } + } + val residue = + ConcordEntryResidue( + entryExtras = e.extras, + seed = e.seed, + currentExtras = if (current != null) current.extras else NoExtras, + ) + (current ?: seed)?.toEntry(e.addedAt, e.inviteRef, e.excludedAtEpoch, residue)?.let { entries.add(it) } + } + + // Doc-level unknown keys: everything at the root that is not a field we model. + val docExtras = JsonObject(root.filterKeys { it != "entries" && it != "tombstones" }) + ConcordCommunityListDocument( entries = entries, - residue = ConcordListResidue(extras = doc.extras, tombstones = doc.tombstones), + residue = ConcordListResidue(extras = docExtras, tombstones = tombstones, unparsedEntries = unparsed), ) } catch (_: Exception) { ConcordCommunityListDocument(emptyList()) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityListTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityListTest.kt index ff379f5d19..b6ccaa8f81 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityListTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord02Community/ConcordCommunityListTest.kt @@ -133,6 +133,108 @@ class ConcordCommunityListTest { assertEquals(1L, e.heldRoots[0].epoch) } + @Test + fun aKeylessChannelDoesNotNukeTheWholeList() { + // A public channel carries no delivered key, so a writer lists it under `channels` + // with only {id, epoch, name} and no `key`. The whole document must still decode — + // a single keyless channel entry must not throw MissingFieldException and take every + // joined community down with it (the "Concord communities won't load" regression). + val json = + """ + { + "entries": [ + { + "community_id": "${"11".repeat(32)}", + "current": { + "community_id": "${"11".repeat(32)}", + "owner": "${"0f".repeat(32)}", + "owner_salt": "${"aa".repeat(32)}", + "community_root": "${"cc".repeat(32)}", + "root_epoch": 0, + "channels": [ + { "id": "${"ee".repeat(32)}", "epoch": 0, "name": "general" } + ], + "relays": ["wss://relay.example"], + "name": "Has A Public Channel" + }, + "added_at": 1700000000000 + } + ], + "tombstones": [] + } + """.trimIndent() + + val entries = ConcordCommunityList.decode(json) + assertEquals(1, entries.size, "a keyless (public) channel must not drop the entry") + assertEquals("Has A Public Channel", entries[0].name) + assertEquals(1, entries[0].privateChannels.size) + assertEquals("", entries[0].privateChannels[0].key) // absent key ⇒ empty, not a throw + } + + // A good entry, and one whose `current` is missing the required `owner` field so it cannot be + // decoded into the typed model at all. + private fun docWithOneUnparseableEntry() = + """ + { + "entries": [ + { + "community_id": "${"11".repeat(32)}", + "current": { + "community_id": "${"11".repeat(32)}", + "owner": "${"0f".repeat(32)}", + "owner_salt": "${"aa".repeat(32)}", + "community_root": "${"cc".repeat(32)}", + "root_epoch": 0, + "relays": ["wss://relay.example"], + "name": "Healthy" + }, + "added_at": 1700000000000 + }, + { + "community_id": "${"22".repeat(32)}", + "current": { + "community_id": "${"22".repeat(32)}", + "owner_salt": "${"bb".repeat(32)}", + "community_root": "${"dd".repeat(32)}", + "root_epoch": 0, + "name": "Missing Owner" + }, + "added_at": 1700000000001 + } + ], + "tombstones": [] + } + """.trimIndent() + + @Test + fun oneUnparseableEntryDoesNotDropItsGoodNeighbors() { + val doc = ConcordCommunityList.decodeDocument(docWithOneUnparseableEntry()) + // The healthy entry survives; the broken one is set aside, not fatal to the whole list. + assertEquals(1, doc.entries.size) + assertEquals("Healthy", doc.entries[0].name) + assertEquals("11".repeat(32), doc.entries[0].id) + assertEquals(1, doc.residue.unparsedEntries.size) + assertEquals( + "22".repeat(32), + doc.residue.unparsedEntries[0]["community_id"]!! + .jsonPrimitive.content, + ) + } + + @Test + fun anUnparseableEntrySurvivesAWriteRoundTrip() { + // A write (follow/unfollow) must re-emit the entry we couldn't parse, verbatim — dropping + // it would delete another client's membership just because this version can't model it. + val doc = ConcordCommunityList.decodeDocument(docWithOneUnparseableEntry()) + val out = ConcordCommunityList.encode(doc.entries, doc.residue).asJson() + val ids = + out["entries"]!!.jsonArray.map { it.jsonObject["community_id"]!!.jsonPrimitive.content }.toSet() + assertTrue(ids.contains("11".repeat(32)), "the healthy entry is written back") + assertTrue(ids.contains("22".repeat(32)), "the unparseable entry is preserved verbatim") + // And a second decode still surfaces exactly the one healthy entry (idempotent). + assertEquals(1, ConcordCommunityList.decode(out.toString()).size) + } + @Test fun tombstoneAfterAddDropsEntry() { val jm = """{"community_id":"${"11".repeat(32)}","owner":"${"0f".repeat(32)}","owner_salt":"${"aa".repeat(32)}","community_root":"${"bb".repeat(32)}","root_epoch":0,"channels":[],"relays":[],"name":"Gone"}"""