From a69d80d38f8ad76834c3fe9909acedbe4cd48acb Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 15:51:16 +0200 Subject: [PATCH] feat: archive the events the group signed, not rebuilds of its rows `assemble` read the archive out of `Mantra*` rows, rebuilding each payload with `toXEvent()` and standing or falling on that rebuild being byte-identical to what was signed. It had to: nothing kept the events. `GroupSignedEvent` keeps them now, so `signedEventsOf` reads the record first and rebuilds only what the record does not hold. **The rebuild stays, as the fallback, keyed by id.** A room whose work predates v13 has no events on file, and dropping the walk would silently empty its archive -- the failure mode being that a member asks for the history, a member answers, and nobody notices the answer was blank. So both sources are read and unioned by event id, which is also what a half-upgraded room needs: older work only the rows remember, newer work on file, and neither half complete on its own. The fallback can go once no install still carries pre-v13 work, and `ArchiveRoundTripTest` is what holds it up until then. **The allowlist does real work on the way out now, and this is the part that would have bitten.** The rebuild could only ever produce document kinds, because those are the only rows it walks. The record holds every kind the group has ever signed -- and every room signs a `GroupKeyStateEvent` as its first act, so one is on file in every room that has signed anything at all. `ArchiveEvent.build` refuses a non-archivable kind with `require`, so an unfiltered read does not quietly ship a key state: it throws, and the room's entire archive fails on the one event every room has. `signedEventsOf` therefore filters on `isArchivable` before anything else, which is the same rule `applyPage` applies on the way in. Removing that one line fails two tests with exactly that exception, which is how I know they are load-bearing rather than passing for the reason I expected. **An artifact whose initial version row is missing now archives.** The rebuild has to recover the version label from that row -- `fromArtifactEvent` drops it, so it is not on the artifact -- and logs and gives up without it, which is a hole in the archive for any device that applied half a batch. Read from the record there is nothing to recover: the label never left the event. That is the case that makes the record the better source rather than merely the faster one, and it has a test of its own. **One verify filter over both sources**, because the rule is per event and not per source: nothing leaves that the recipient could not check for themselves. A drop still means different things on each side -- a member's own rumor sitting in the same table as the group's work, versus a row that has drifted from the event it recorded -- and the comment now says so, since the log line cannot. **Ordering is unchanged where it matters and looser where it does not.** `inApplyOrder` is a stable sort by dependency rank, so the union only affects order *within* a rank: a room holding some work both ways can order two chapters differently from a member holding one way only. Pages are idempotent and applied payload by payload, and two members already differed by the order their rows were written in, so this costs nothing. `rebuiltEventsOf` still runs on every archive even where it contributes nothing, because there is no way to tell a complete record from a partial one without doing the walk, and it is a handful of indexed queries against a room's own rows. 495 jvm tests and 297 android unit tests pass. Five new cases in `ArchiveAssemblyJvmTest`, which seeds through the real inbound path and now records the same batch the way `FrostSigningManager.complete` does: payloads compared byte-for-byte against what was signed, work held both ways travelling exactly once, a genuinely room-signed key state left behind, a signed kind the archive has no arm for left behind, and the artifact the rebuild has to leave out archiving from the record. The existing assembly and end-to-end tests seed without recording, so they go on covering the rebuild fallback unchanged -- which is why they all still pass, and why that is evidence rather than luck. Co-Authored-By: Claude Opus 5 --- .../mantra/compose/managers/ArchiveManager.kt | 121 +++++++--- .../managers/ArchiveAssemblyJvmTest.kt | 211 ++++++++++++++++++ docs/member-archive.md | 21 +- 3 files changed, 322 insertions(+), 31 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt index 4b8eefb5..a3659d44 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt @@ -27,22 +27,31 @@ import press.mantra.compose.nostr.frost.GroupKeyStateEvent * locally by each device that took part and never goes on the wire. This is how * it gets to them. * - * ### The rows are the archive + * ### The events are the archive, and the rows are the fallback * - * Every payload here is rebuilt from a `Mantra*` row with `toXEvent()`, and - * stands or falls on that rebuild being byte-identical to what was signed. - * `ArchiveRoundTripTest` is what says it is, per kind, against a real quorum -- - * and it found two faults in the artifact's rebuild the first time it ran, both - * of which would have shipped payloads that every receiver drops as forgeries - * without a word. + * `GroupSignedEvent` holds what the group signed, as it signed it, so + * [signedEventsOf] reads it first: no rebuild, no round-trip risk, and nothing + * that depends on a row having kept every field of the event it came from. The + * artifact's version label is the standing example -- it is not on the artifact + * row at all, and [rebuiltEventsOf] has to go and find it on the initial version + * or leave the artifact out. * - * That was once the only way: a signed event was applied and what survived was - * the row. `GroupSignedEvent` now keeps the event too, and [applyPage] files one - * for every payload it accepts -- which is what lets a member who was handed - * their history hand it on. Assembly still walks the rows, because a room whose - * work predates that table has no events on file and rebuilding is the only way - * to reach it. Reading assembled events from the table instead is worth doing - * once the fallback can be dropped. + * A room whose work predates that table has no events on file, so the rebuild + * stays as the fallback for exactly what the table is missing, keyed by id. + * Every payload it produces stands or falls on being byte-identical to what was + * signed; `ArchiveRoundTripTest` is what says it is, per kind, against a real + * quorum, and it found two faults in the artifact's rebuild the first time it + * ran -- both of which would have shipped payloads that every receiver drops as + * forgeries without a word. The fallback can go once no install still holds + * pre-v13 work. + * + * **The allowlist does real work on the way out now.** The rebuild could only + * ever produce document kinds; the table holds everything the group has ever + * signed, `GroupKeyStateEvent` included -- and every room signs one of those as + * its first act. So [signedEventsOf] filters on [ArchiveEvent.isArchivable] + * before anything else, which is the same rule [applyPage] applies on the way + * in. Without it `ArchiveEvent.build` would refuse the page, and a room's whole + * archive would fail on the one event every room has. * * ### Nothing unverifiable leaves * @@ -75,17 +84,19 @@ object ArchiveManager { archiveId: String = RandomInstance.bytes(32).toHex(), createdAt: Long = TimeUtils.now(), ): List> { - val rebuilt = signedEventsOf(database, chatRoomId) + val held = signedEventsOf(database, chatRoomId) - val verified = rebuilt.filter { GroupKeyStateEvent.isSignedByRoom(it, chatRoomId) } - if (verified.size != rebuilt.size) { - // Expected rather than alarming: an artifact version is derived - // rather than signed, and a translation is its author's rumor. What - // would be worth looking at is this dropping something the group - // really did sign, which reads as a broken `toXEvent` rather than as - // a missing signature. + // One rule over both sources: nothing leaves that the recipient could + // not check for themselves. What a drop means depends on where it came + // from, and the two are worth telling apart when reading this log -- + // from the rebuild it is the ordinary case of a member's own rumor + // sitting in the same table as the group's work, and from the record it + // is a row that has drifted from the event it recorded, which nothing + // in this app does on purpose. + val verified = held.filter { GroupKeyStateEvent.isSignedByRoom(it, chatRoomId) } + if (verified.size != held.size) { logger.d( - "Leaving ${rebuilt.size - verified.size} of ${rebuilt.size} row(s) out of " + + "Leaving ${held.size - verified.size} of ${held.size} event(s) out of " + "$chatRoomId's archive: nothing verifiably signed by the room" ) } @@ -583,6 +594,55 @@ object ArchiveManager { return outcome } + /** + * Every archivable event this device holds for the room: the ones the group + * signed here or sent here, plus anything only the rows still remember. + * + * The record comes first because it is the event rather than a reconstruction + * of one, and the rebuild fills the gap behind it -- keyed by id, so an event + * held both ways travels once. A room that upgraded mid-life has both, and + * neither half is complete on its own. + * + * The two disagree only in one direction worth naming. An id the rebuild + * produces that the record does not hold is either work from before the + * table existed, which is the point of the fallback, or a rebuild that has + * gone wrong -- and a wrong rebuild hashes to an id whose signature does not + * verify, so the filter in [assemble] drops it either way rather than + * shipping a payload every receiver reads as a forgery. + * + * Order does not matter at this point; [ArchiveEvent.inApplyOrder] settles it + * afterwards. It is stable within a rank, so a room holding some of its work + * both ways can order two chapters differently from a member holding one way + * only. That costs nothing: pages are idempotent and applied payload by + * payload, and two members already differed by the order their rows were + * written in. + */ + private suspend fun signedEventsOf( + database: MantraDatabase, + chatRoomId: String, + ): List { + val recorded = database.groupSignedEventDao() + .getByChatRoomId(chatRoomId) + // The allowlist, applied to the source that can actually trip it -- + // see the class comment. A key state on file is the room's own, and + // sending it would be handing every member a validly signed + // statement about what the room signs with, replayable forever. + .filter { ArchiveEvent.isArchivable(it.kind) } + .map { it.toEvent() } + + val onFile = recorded.mapTo(mutableSetOf()) { it.id } + val rebuilt = rebuiltEventsOf(database, chatRoomId).filterNot { it.id in onFile } + + if (rebuilt.isNotEmpty()) { + logger.d( + "Archiving $chatRoomId: ${recorded.size} event(s) as the group signed them, " + + "${rebuilt.size} rebuilt from rows that predate the record" + ) + } + + return recorded + rebuilt + } + /** * The room's rows, rebuilt into the events they came from. * @@ -592,11 +652,18 @@ object ArchiveManager { * artifact row -- see `MantraArtifact.toArtifactEvent` -- and the version it * went into is one step away here. * - * Order does not matter at this point; [ArchiveEvent.inApplyOrder] settles it - * afterwards. What matters is that nothing is missed, so this returns - * everything and the verify filter above decides what can travel. + * Still walked on every archive, and by now it contributes nothing in most + * rooms: everything signed or applied since `GroupSignedEvent` existed is on + * file as an event, and [signedEventsOf] discards whatever this rebuilds of + * it. The walk is a handful of indexed queries against a room's own rows, + * and it is what makes a half-upgraded room whole, so it runs rather than + * being skipped when the record looks complete -- there is no way to tell a + * complete record from a partial one without doing it. + * + * It returns everything it can rebuild and lets [signedEventsOf] and the + * verify filter decide what can travel. */ - private suspend fun signedEventsOf( + private suspend fun rebuiltEventsOf( database: MantraDatabase, chatRoomId: String, ): List = buildList { diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ArchiveAssemblyJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ArchiveAssemblyJvmTest.kt index 6997d868..77e8e103 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ArchiveAssemblyJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ArchiveAssemblyJvmTest.kt @@ -24,12 +24,14 @@ import press.mantra.compose.database.MantraDatabase import press.mantra.compose.database.builder.getRoomDatabase import press.mantra.compose.database.model.ChatMessage import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.GroupSignedEvent import press.mantra.compose.database.model.MantraDialect import press.mantra.compose.database.model.NostrEvent import press.mantra.compose.database.model.Profile import press.mantra.compose.extensions.toHex import press.mantra.compose.nostr.archive.ArchiveEvent import press.mantra.compose.nostr.frost.GroupKeyStateEvent +import press.mantra.compose.nostr.nip30303.SubmissionEvent import press.mantra.compose.nostr.nip30303.ArtifactEvent import press.mantra.compose.nostr.nip30303.ArtifactVersionEvent import press.mantra.compose.nostr.nip30303.ChapterEvent @@ -132,6 +134,16 @@ class ArchiveAssemblyJvmTest { ) } + /** + * Every event `apply` has been given, in the order it was given them. + * + * A test that wants the room's work on file as events too replays this + * through [record], which is the pair `FrostSigningManager.complete` writes + * in one pass. Kept here rather than returned by `seedSignedWork` so the + * seeding reads the same in both worlds. + */ + private val applied = mutableListOf() + /** What `FrostSigningManager.applySignedEvent` does, through the same call. */ private suspend fun apply(template: EventTemplate<*>): Event { val event = signed(template) @@ -147,9 +159,28 @@ class ArchiveAssemblyJvmTest { createdAt = Instant.fromEpochSeconds(event.createdAt) ) + applied += event + return event } + /** What `FrostSigningManager.recordSignedEvents` does, through the same table. */ + private suspend fun record(vararg events: Event) { + db.groupSignedEventDao().recordAll( + events.map { + GroupSignedEvent.fromEvent( + event = it, + chatRoomId = chatRoomId, + derivationPath = SharedKeyDerivation.formatPath(), + frostSigningSessionId = "session-1", + ) + } + ) + } + + /** The whole seeded batch on file as events, as a device that signed it holds it. */ + private suspend fun recordEverythingApplied() = record(*applied.toTypedArray()) + /** ChatRoom -> Profile -> NostrEvent, the foreign key chain a room hangs off. */ private suspend fun seedRoom() { val nostrEventId = "c".repeat(64) @@ -376,6 +407,186 @@ class ArchiveAssemblyJvmTest { assertNotNull(page.archiveId()) } + // ---- Reading from the record rather than rebuilding ------------------- + + /** + * The events, not a reconstruction of them. + * + * Byte-for-byte against what was signed, which is the assertion the whole + * rebuild machinery exists to approximate. Here it is exact by construction: + * nothing is rebuilt, so there is no tag order to get wrong and no field a + * row could have failed to keep. + */ + @Test + fun `an archive carries the group's events as the group signed them`() = runBlocking { + seedSignedWork() + recordEverythingApplied() + + val payloads = archivedPayloads().associateBy { it.id } + + assertEquals( + applied.map { it.id }.toSet(), + payloads.keys, + "everything the group signed here should be in the archive" + ) + applied.forEach { event -> + assertEquals( + event.toJson(), + assertNotNull(payloads[event.id]).toJson(), + "kind ${event.kind} did not travel as the group signed it" + ) + } + } + + /** + * Both sources, one archive, and nothing counted twice. + * + * The state a device upgrading mid-life is in: older work only the rows + * remember, newer work on file as events. Neither half is the archive on its + * own, and an event held both ways is still one event. + */ + @Test + fun `work held both ways travels exactly once`() = runBlocking { + seedSignedWork() + + // Half the batch on file, the other half only as rows -- which is what + // a room that was upgraded partway through its life looks like. + record(*applied.take(applied.size / 2).toTypedArray()) + + val payloads = archivedPayloads() + + assertEquals( + applied.map { it.id }.toSet(), + payloads.map { it.id }.toSet(), + "the two sources together should still be the whole record" + ) + assertEquals( + payloads.size, + payloads.map { it.id }.toSet().size, + "an event on file and rebuildable is one event, not two" + ) + } + + /** + * The failure this source made possible, and the allowlist that stops it. + * + * A key state is group-signed and verifies perfectly, and every room signs + * one as its first act -- so it is on file in every room that has ever + * signed anything. The rebuild could not reach one because no `Mantra*` row + * holds it; the record holds every kind. Without the filter on the way out + * `ArchiveEvent.build` refuses the page and the room's whole archive fails + * on the one event every room has. + */ + @Test + fun `a key state the room really signed is never archived`() = runBlocking { + seedSignedWork() + recordEverythingApplied() + + val keyState = signed( + EventTemplate( + createdAt = 1_700_000_070L, + kind = GroupKeyStateEvent.KIND, + tags = GroupKeyStateEvent.assembleTags( + chatRoomId = chatRoomId, + dkgSessionId = "the-ceremony", + ), + content = keyMaterial.thresholdPublicKey.value.toHex(), + ) + ) + record(keyState) + assertTrue( + GroupKeyStateEvent.isSignedByRoom(keyState, chatRoomId), + "the point of this test is that verification passes" + ) + + val payloads = archivedPayloads() + + assertTrue(payloads.isNotEmpty(), "the rest of the room's work still archives") + assertEquals( + ArchiveEvent.ARCHIVABLE_KINDS, + payloads.map { it.kind }.toSet(), + "an archive carries the document kinds and nothing else" + ) + assertTrue(payloads.none { it.id == keyState.id }) + } + + /** The same rule, for a kind that is merely not on the list rather than dangerous. */ + @Test + fun `a signed kind the archive has no arm for is left behind`() = runBlocking { + seedSignedWork() + recordEverythingApplied() + + record( + signed( + EventTemplate( + createdAt = 1_700_000_080L, + kind = SubmissionEvent.KIND, + tags = emptyArray(), + content = "{}", + ) + ) + ) + + assertEquals( + ArchiveEvent.ARCHIVABLE_KINDS, + archivedPayloads().map { it.kind }.toSet() + ) + } + + /** + * The case the rebuild cannot reach at all, which is why the record is the + * source and not merely a faster one. + * + * `MantraArtifact` does not hold the version label it was signed with -- + * `fromArtifactEvent` drops it -- so `toArtifactEvent` has to go and find it + * on the initial version, and an artifact whose version row is missing is one + * the rebuild logs and leaves out. On file as an event there is nothing to + * find: the label never left. + */ + @Test + fun `an artifact the rebuild has to leave out still archives from the record`() = runBlocking { + seedRoom() + + val dialect = apply( + DialectEvent.build(name = "isiZulu", country = "ZA", language = "zu", createdAt = 1_700_000_000L) + ) + + // Applied without the second item of its batch, so no version row carries + // the label back. A device that was offline for half a batch, or one that + // failed to apply the version, is in exactly this state. + val artifact = apply( + ArtifactEvent.build( + name = "In Detention", + url = "https://example.com/in-detention", + visibility = "private", + license = "cc", + dialectId = dialect.id, + versionLabel = "1.0", + createdAt = 1_700_000_010L + ) + ) + + assertEquals( + emptyList(), + db.mantraArtifactVersionDao().getArtifactVersionsByArtifactId(artifact.id), + "this test is only about an artifact whose initial version is missing" + ) + + // Rows only: the rebuild reaches the dialect and gives up on the artifact. + assertEquals( + listOf(dialect.id), + archivedPayloads().map { it.id }, + ) + + record(dialect, artifact) + + assertEquals( + setOf(dialect.id, artifact.id), + archivedPayloads().map { it.id }.toSet(), + "the record holds the whole event, version label and all" + ) + } + @Test fun `two archives of the same rows do not share an id`() = runBlocking { seedSignedWork() diff --git a/docs/member-archive.md b/docs/member-archive.md index b15c3ac3..5eb50732 100644 --- a/docs/member-archive.md +++ b/docs/member-archive.md @@ -296,10 +296,23 @@ keeps the decision about transcript lines next to the decision about triggers. > **Since the `GroupSignedEvent` table landed**, a signed event *is* stored as an > event -- `FrostSigningManager` files one per batch it completes, and > `ArchiveManager.applyPage` files one per payload it accepts, each with the -> derivation path its author was reached at. Assembly still rebuilds from rows, -> because a room whose work predates the table has nothing else to rebuild from. -> The rest of this section is the reasoning as it stood then, and the round-trip -> gate it describes is still what holds those rooms up. +> derivation path its author was reached at. `assemble` reads that table first +> and rebuilds only what it does not hold, which is work signed before the table +> existed. So the rest of this section describes the *fallback*: the round-trip +> gate it argues for is what holds those older rooms up, and it can go once no +> install still carries pre-v13 work. +> +> Two things changed with the source, both worth knowing before reading on: +> +> - **The allowlist now does real work on the way out.** The rebuild could only +> ever produce document kinds; the table holds everything the group has signed, +> and every room signs a `GroupKeyStateEvent` as its first act. `assemble` +> filters on `ArchiveEvent.isArchivable` before anything else -- without it +> `ArchiveEvent.build` refuses the page and a room's whole archive fails on the +> one event every room has. +> - **An artifact whose initial version row is missing now archives.** The +> rebuild has to recover the version label from that row and logs and gives up +> without it; on file as an event, the label never left. Signed events are not stored as events; they are stored as rows. So the archive is rebuilt from `Mantra*` rows via each entity's `toXEvent()`, which is exactly