mantra/mantra-kmp
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix: a member is not shown the messages sent before they were in the room

Browse Source 
A joiner gets the MLS key schedule from their own epoch forward and nothing
before it. The relay does not know that and hands them the whole room: negentropy
syncs down every kind:445 the group ever published, `indexMarmotGroupEvent` read
each one against the group, the outer layer refused, and every refusal wrote an
`undecryptableOuterLayer` line. So the room a member had just been invited to
opened on a screenful of "Undecryptable Message" above the conversation -- one
per message the group had sent before they arrived, none of them ever readable,
and the count only grows with how long the group had been talking.

**The epoch of a kind:445 is inside the layer that will not decrypt**, so an
event this device cannot read cannot be asked what epoch it is from. "From before
we joined", "from an epoch we have not caught up to" and "from an epoch that fell
out of the retention window" are indistinguishable from the outside, and only the
first is permanent. What separates it is not the ciphertext but the clock: it was
published before the group made the epoch we joined at.

**`ChatRoom.joinedGroupAt` is that moment, written down.** The Welcome's
`created_at`, which the inviter stamps as it mints the Welcome out of the Add
commit that made us a member -- so it is the group's own account of when our
epoch began, not this device's account of when it heard about it. A group this
device created sets it to the room's creation; it was a member from epoch 0 and
there is nothing behind it to hold back.

Stored rather than read off `createdAt`, which today holds the same value in both
paths. `createdAt` is row bookkeeping and this decides which of a group's
messages a member is allowed to see at all; the two being equal is a coincidence
of the current code, and hanging the second off the first makes a future change
to when a room row is written into a change in what gets discarded. `memberSince`
is `joinedGroupAt ?: createdAt`, so a room joined before the column existed gets
the fix too -- and gets it from the value every path that sets the column would
have written anyway.

**`predatesMembership` draws the line strictly before**, and that is a judgement
rather than a fact. Nostr stamps `created_at` in whole seconds, so the second the
Welcome was minted holds both the commit that added us -- the last act of the
epoch before ours, unreadable by construction -- and any message another member
sent the instant they applied it. Only one of the two can be had. An unreadable
event kept costs one refused decrypt; a readable event discarded is a message the
member never sees. So the second is kept, and a room may still show a single
placeholder for the commit that added its newest member.

**Two places gate on it.** `indexMarmotGroupEvent` returns before touching the
MLS group, so nothing is decrypted, no `MarmotGroupEvent` row is filed for
ciphertext whose key this device never had, and no line is written.
`reindexMarmotGroupEvents` partitions them out of the sweep entirely: a replay
can say in advance that no pass will ever read them, so replaying them only
spends a refused decrypt per sweep and reports every one as a failure on a room
where nothing is wrong. `MarmotReindexSweep` is untouched apart from carrying the
new count -- it decides how many times to go round, not what is worth going round
for.

**Schema v15, and the migration is the half that fixes devices already showing
the bug.** Nothing rewrites a chat line that is already in the transcript, so
fixing the write path alone would leave every member who joined a busy room
opening it on the same run of placeholders forever. `MIGRATION_14_15` adds the
column and deletes the lines: only the two types in `UNRESOLVED_MARMOT_TYPES`,
and only where the group event behind them predates the room. Those lines say
nothing by design -- they stand in for an event that was never read -- so
removing one loses nothing, while every other line is the final word on its group
event. The group events themselves stay; this is about what the room shows.

The column is left null rather than backfilled from `createdAt`. Null already
means "ask `createdAt`", and copying the value would turn a fallback into a claim
this migration is in no position to make. It is manual rather than an
`AutoMigration` only because of the delete: `ALTER TABLE ... ADD COLUMN` appends,
which is where Room's own generated migration for a nullable addition puts one,
and Room compares a table's columns by name rather than by position.

**The reindex report stopped being true**, so it carries the number now. With the
backlog held back, `unresolved` falls to zero and the screen said "Nothing to
reindex - 30 event(s) all read" about a room where 27 of them were never this
device's to read. `MarmotReindexReport.predatingMembership` is reported alongside
`stored`, and the detail screen names it: "3 event(s) all read - 27 from before
you joined". A member invited into an old room is the ordinary case, not an
anomaly to bury in a total.

Seventeen tests. `ChatRoomMembershipWindowTest` holds the boundary, including the
same-second case and both directions of the `createdAt` fallback.
`JoinedGroupAtMigrationJvmTest` runs the migration's own SQL against v14's three
tables and covers what it must not take as carefully as what it must: a
placeholder for an event from *after* the join is left to be recovered, a message
that was read is left alone however old it is, a line with no group event behind
it is out of reach of the rule, and two rooms joined at different times are each
measured against their own join. `MarmotPreJoinIndexingJvmTest` drives the DAO
against a room with no MLS state, which is what separates "left alone because it
predates the join" from "tried and failed".

520 jvm tests and 302 android unit tests pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-06 18:18:16 +02:00
parent 19d57ef3a5
commit 49c012bf8b
14 changed files with 6402 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
composeApp/src/commonMain/kotlin/press/mantra/compose/database/MantraDatabase.kt
View File

@@ -177,7 +177,7 @@ val GENESIS_AT = Instant.fromEpochMilliseconds(1231006505000L)
UnsignedNostrEvent::class,
Zap::class
],
version = 14,
version = 15,
autoMigrations = [
// v2 only adds the DkgSession/DkgParticipantMessage tables, so Room can
// generate the migration itself — nothing existing changes shape.
@@ -255,6 +255,14 @@ val GENESIS_AT = Instant.fromEpochMilliseconds(1231006505000L)
// meanings would have met. Room can rename a column and cannot rewrite the
// rows in the same breath, so this is a manual migration passed to the
// builder rather than an entry here. See MIGRATION_13_14.
//
// v15 adds the nullable ChatRoom.joinedGroupAt, which says when this
// device became a member and so which of the group's messages were never
// its to read. Adding a nullable column is a shape Room migrates itself;
// deleting the placeholder chat lines already written for those messages
// is not, and a member who joined a busy room is looking at a screenful of
// them. Manual for that half, so it too is passed to the builder rather
// than listed here. See MIGRATION_14_15.
]
)
@ColumnTypeConverters(MantraConverters::class)

10
composeApp/src/commonMain/kotlin/press/mantra/compose/database/builder/PlatformDatabaseBuilder.kt
View File

@@ -5,6 +5,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver
import press.mantra.compose.database.migrations.MIGRATION_3_4
import press.mantra.compose.database.migrations.MIGRATION_9_10
import press.mantra.compose.database.migrations.MIGRATION_13_14
import press.mantra.compose.database.migrations.MIGRATION_14_15
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.IO
@@ -18,12 +19,13 @@ fun getRoomDatabase(
builder: RoomDatabase.Builder<press.mantra.compose.database.MantraDatabase>
): press.mantra.compose.database.MantraDatabase {
return builder
// Everything else Room generates itself. These three move data rather than
// Everything else Room generates itself. These four move data rather than
// only changing shape, which an AutoMigration cannot express: 3->4 rewrites
// chat rows, 9->10 copies a session's per-event columns onto the items
// table before dropping them, and 13->14 renames a column and rewrites the
// chat rows that named it the old way.
.addMigrations(MIGRATION_3_4, MIGRATION_9_10, MIGRATION_13_14)
// table before dropping them, 13->14 renames a column and rewrites the
// chat rows that named it the old way, and 14->15 adds a column and deletes
// the chat rows written for messages from before this device joined.
.addMigrations(MIGRATION_3_4, MIGRATION_9_10, MIGRATION_13_14, MIGRATION_14_15)
.setDriver(BundledSQLiteDriver())
.setQueryCoroutineContext(Dispatchers.IO)
.build()

9
composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt
View File

@@ -81,12 +81,19 @@ abstract class MarmotOutboundDao(
)
// Save Chat Room
//
// Member since the group existed, because this device is what created it:
// there is no epoch of this group that predates us, and so nothing in it
// for ChatRoom.predatesMembership to hold back.
val createdAt = Clock.System.now()
val chatRoom = ChatRoom(
id = nostrGroupId,
userPublicKey = userPublicKey,
mlsGroupState = mlsGroup.saveState().encodeTls().toHex(),
subject = name,
description = description
description = description,
joinedGroupAt = createdAt,
createdAt = createdAt,
)
database.chatRoomDao().upsert(
chatRoom

50
composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/NostrDao.kt
View File

@@ -706,6 +706,14 @@ abstract class NostrDao(
description = group.currentMarmotData()?.description?.ifBlank { null },
initialGiftWrapPayloadId = decryptedGiftWrapPayload.id,
createdAt = decryptedGiftWrapPayload.createdAt,
// The Welcome's own `created_at`, which the
// inviter stamps as it mints the Welcome out of
// the Add commit that made us a member. That
// commit is what created the epoch we are joining
// at, so it is the group's clock on when this
// room's history stops being ours to read. See
// ChatRoom.predatesMembership.
joinedGroupAt = decryptedGiftWrapPayload.createdAt,
mlsGroupState = group.saveState().encodeTls().toHex(),
)
)
@@ -1145,6 +1153,13 @@ abstract class NostrDao(
* see [reindexMarmotGroupEvents]. Throws for a room this device cannot process
* the event against at all, which the caller decides what to do about: a first
* delivery lets it roll back its transaction, a replay logs it and moves on.
*
* An event from before this device joined is not read and not filed -- see
* [ChatRoom.predatesMembership]. Nothing about it is this device's: not the
* epoch key it was encrypted under, and so not the message either. Reading it
* anyway is how a room a member was invited to yesterday opened on a screenful
* of "Undecryptable Message" above the conversation, one line for every
* message the group had sent before they arrived.
*/
private suspend fun indexMarmotGroupEvent(
groupEvent: GroupEvent,
@@ -1154,6 +1169,11 @@ abstract class NostrDao(
val localChatRoom = database.chatRoomDao().findChatRoomById(chatRoomId)
?: throw MarmotMissingChatGroupException("Couldn't find chatRoom for ${groupEvent.id}")
if (localChatRoom.chatRoom.predatesMembership(Instant.fromEpochSeconds(groupEvent.createdAt))) {
logger.d("${groupEvent.id} predates this device joining $chatRoomId, nothing to index")
return
}
// Through the cache rather than rebuilt here, so the secret
// tree's skipped-generation keys survive from one message to
// the next. Two events published in the same instant arrive in
@@ -1314,16 +1334,22 @@ abstract class NostrDao(
* recovered by one pass can be what lets the next read the messages that were
* waiting on it.
*
* Events from before this device joined are left out of the sweep entirely --
* see [ChatRoom.predatesMembership]. They are the one part of the backlog a
* replay can say something about in advance: no pass will ever read them, so
* replaying them only spends a refused decrypt each time and reports every one
* of them as a failure, on a room where nothing is wrong.
*
* What this cannot do is recover a message whose key is gone: an application
* message the ratchet has already advanced past, or one from an epoch that
* predates this device joining. Those stay unreadable however often they are
* replayed.
* message the ratchet has already advanced past. Those stay unreadable however
* often they are replayed.
*/
open suspend fun reindexMarmotGroupEvents(
chatRoomId: String,
activeKeyPair: KeyPair,
): MarmotReindexReport {
val userPublicKey = activeKeyPair.pubKey.toHex()
val chatRoom = database.chatRoomDao().findChatRoomById(chatRoomId)?.chatRoom
// The query's LIKE only narrows; the h tag is what decides the room. Sorted
// by CommitOrdering's own comparator rather than left in createdAt order, so
@@ -1338,7 +1364,20 @@ abstract class NostrDao(
}
.sortedWith(CommitOrdering.comparator)
logger.i("Reindex $chatRoomId: ${groupEvents.size} stored group event(s)")
// Held back rather than dropped from the report: they are counted in
// `stored` and again on their own, so the screen can say a room is mostly
// older than the member reading it instead of calling those events read.
//
// A room with no row to ask keeps every event, so a replay against one
// does what it always did rather than quietly finding nothing to do.
val (readable, predatingMembership) = groupEvents.partition { groupEvent ->
chatRoom?.predatesMembership(Instant.fromEpochSeconds(groupEvent.createdAt)) != true
}
logger.i(
"Reindex $chatRoomId: ${groupEvents.size} stored group event(s), " +
"${predatingMembership.size} from before this device joined"
)
// Held commits are what a replay is most often for, and they are only ever
// cleared by one applying -- see MarmotInboundManager.forgetPendingCommits.
@@ -1351,7 +1390,8 @@ abstract class NostrDao(
return MarmotReindexSweep.run(
stored = groupEvents.size,
unresolved = groupEvents.filterNot { it.id in resolved },
predatingMembership = predatingMembership.size,
unresolved = readable.filterNot { it.id in resolved },
replay = { groupEvent ->
indexMarmotGroupEvent(
groupEvent = groupEvent,

58
composeApp/src/commonMain/kotlin/press/mantra/compose/database/migrations/JoinedGroupAtMigration.kt Normal file
View File

@@ -0,0 +1,58 @@
package press.mantra.compose.database.migrations
import androidx.room3.migration.Migration
import androidx.sqlite.SQLiteConnection
import androidx.sqlite.execSQL
import press.mantra.compose.database.model.ChatMessage
/**
* Records when this device joined each room, and clears the lines it wrote for
* messages it was never able to read.
*
* A member added to a group is given the key schedule from their own epoch
* forward and nothing before it. Every kind:445 the group published earlier is
* still on the relays, still syncs down, and is still unreadable -- so the room
* they opened for the first time led with a run of "Undecryptable Message",
* one per message sent before they arrived, above the conversation they were
* actually invited to.
*
* Two changes, one shape and one data, which is why this is a manual migration
* rather than an `AutoMigration` Room could generate:
*
* - `ChatRoom.joinedGroupAt` says when this device became a member, which is
* what `ChatRoom.predatesMembership` reads to leave those events alone. It
* is left null here rather than backfilled from `createdAt`: null already
* means "ask `createdAt`" -- see `ChatRoom.memberSince` -- and copying the
* value would turn a fallback into a claim this migration is in no position
* to make.
* - The placeholder lines already written for those events are deleted. Fixing
* the write path only stops the next one; nothing rewrites a line that is
* already in the transcript, so a member who joined last week would go on
* seeing their run of them forever.
*
* Only the two types in [ChatMessage.UNRESOLVED_MARMOT_TYPES] are deleted, and
* only where the group event behind them predates the room. Those lines say
* nothing by design -- they stand in for an event that was never read -- so
* removing one loses nothing, while every other line is the final word on its
* group event and is left alone. The group events themselves stay: this is
* about what the room shows, not about forgetting what arrived.
*
* `createdAt` is compared rather than `joinedGroupAt` because the column was
* added in this same migration and is null for every row in the database being
* migrated. It is the same comparison `memberSince` falls back to.
*/
val MIGRATION_14_15 = object : Migration(14, 15) {
override suspend fun migrate(connection: SQLiteConnection) {
connection.execSQL("ALTER TABLE `ChatRoom` ADD COLUMN `joinedGroupAt` INTEGER")
val placeholderTypes = ChatMessage.UNRESOLVED_MARMOT_TYPES.joinToString(", ") { "'$it'" }
connection.execSQL(
"DELETE FROM `ChatMessage` WHERE `messageType` IN ($placeholderTypes) " +
"AND `marmotGroupEventId` IN (" +
"SELECT `MarmotGroupEvent`.`id` FROM `MarmotGroupEvent` " +
"JOIN `ChatRoom` ON `ChatRoom`.`id` = `MarmotGroupEvent`.`chatRoomId` " +
"WHERE `MarmotGroupEvent`.`createdAt` < `ChatRoom`.`createdAt`)"
)
}
}

55
composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatRoom.kt
View File

@@ -79,6 +79,24 @@ data class ChatRoom(
val leftGroupAt: Instant? = null,
/**
* When this device became a member of the group, or null for a room that
* predates the column.
*
* The pair to [leftGroupAt], and the thing [memberSince] is really asking
* for. Written from the moment the group's own clock says our epoch began:
* the Welcome's `created_at`, which the inviter stamps as it mints the
* Welcome out of the Add commit that made us a member, or the room's own
* creation for a group this device started at epoch 0.
*
* Stored rather than read off [createdAt] because the two are only
* incidentally equal. [createdAt] is row bookkeeping -- when this device
* first wrote the row down -- and the question asked here decides which of
* the group's messages are ours to read at all. That is not a fact to leave
* hanging off a timestamp somebody could reasonably repurpose.
*/
val joinedGroupAt: Instant? = null,
/**
* When this device last asked the group for its signed history, or null if
* it never has or the answer has since arrived.
@@ -144,6 +162,43 @@ data class ChatRoom(
}
/**
* The moment this device joined, falling back to when it wrote the room down.
*
* A room joined before [joinedGroupAt] existed has no recorded answer, and
* [createdAt] is both the best one available and the one every path that
* writes [joinedGroupAt] would have written anyway: a joiner's row is created
* from the Welcome, and a creator's when it creates the group.
*/
val memberSince: Instant get() = joinedGroupAt ?: createdAt
/**
* Whether something the group published at [publishedAt] belongs to an epoch
* this device was never in.
*
* MLS gives a joiner the key schedule from their own epoch forward and
* nothing before it, so a kind:445 older than [memberSince] cannot be read
* now and cannot be read later -- not by waiting, and not by replaying it.
* The point of asking is to leave those alone rather than file a line saying
* a message arrived that nobody can show.
*
* Time is the only thing to ask it of. The epoch a kind:445 was encrypted
* under is inside the outer layer, so an event this device cannot decrypt
* cannot be asked what epoch it is from, and "before we joined", "from an
* epoch we have not caught up to" and "from an epoch that fell out of the
* retention window" all look identical from the outside. What separates the
* first from the other two is that it was published before the group made
* the epoch we joined at.
*
* Strictly before, so an event stamped in the same second as our Welcome is
* still read. The error worth avoiding runs one way: an unreadable event
* costs a wasted decrypt, and a discarded readable one is a message the
* member never sees. The commit that added us sits exactly on that boundary
* and is unreadable by construction -- it is the last act of the epoch
* before ours -- so a room may still show one placeholder for it.
*/
fun predatesMembership(publishedAt: Instant): Boolean = publishedAt < memberSince
fun toMlsGroup(): MlsGroup? {
return mlsGroupState?.let {
return MlsGroup.restore(

16
composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/types/MarmotReindexReport.kt
View File

@@ -7,17 +7,23 @@ package press.mantra.compose.database.model.types
* can say what happened rather than leaving the user to guess from the message
* list whether anything moved.
*
* @param stored every kind:445 held locally for the room.
* @param unresolved how many of those had nothing to show for them, or only a
* @param stored every kind:445 held locally for the room, [predatingMembership]
* included. They are held, so they are counted.
* @param predatingMembership how many of [stored] the group published before this
* device joined, which a replay does not touch -- see
* `ChatRoom.predatesMembership`. Reported rather than folded into [stored]
* silently, because "20 events, all read" is not true of a room where 15 of them
* were never this device's to read, and a member who was invited into an old
* room deserves the difference said out loud rather than left as a discrepancy.
* @param unresolved how many of the rest had nothing to show for them, or only a
* placeholder line -- the ones a replay was allowed to touch.
* @param recovered how many of [unresolved] came out with something to show:
* a message, an applied commit, an entity added to the group's library.
* @param failed how many threw while being replayed. Expected to be non-zero
* on a room with events from before this device joined, whose epoch secrets it
* never held and never will.
* @param failed how many threw while being replayed.
*/
data class MarmotReindexReport(
val stored: Int = 0,
val predatingMembership: Int = 0,
val unresolved: Int = 0,
val recovered: Int = 0,
val failed: Int = 0,

23
composeApp/src/commonMain/kotlin/press/mantra/compose/managers/MarmotInboundManager.kt
View File

@@ -230,10 +230,17 @@ object MarmotInboundManager {
)
if (mlsBytes == null) {
// Expected when this kind:445 was encrypted with an epoch
// key that predates our join (classical MLS forward
// secrecy), or when the sender's epoch has drifted. Not
// an error — callers should log at DEBUG.
// Expected when the sender's epoch has drifted, or when a
// key that predates our join is what this was encrypted
// with (classical MLS forward secrecy). Not an error —
// callers should log at DEBUG.
//
// The second of those is now rare rather than routine:
// `NostrDao.indexMarmotGroupEvent` holds back anything the
// group published before this device joined, so what
// reaches here from before our epoch is only what sits on
// the boundary — see ChatRoom.predatesMembership. A room
// full of these is a sign that gate is not being applied.
GroupEventResult.UndecryptableOuterLayer(
localChatRoom.chatRoom.id,
retainedEpochCount = retainedExporterSecrets(localChatRoom.chatRoom.id).size,
@@ -656,9 +663,11 @@ object MarmotInboundManager {
*
* Returns null when neither the current epoch key nor any retained key
* decrypts. This happens normally for commits/application messages from
* epochs that predate our join (we never held those keys), so callers
* should treat null as an expected "nothing to do here" outcome and log
* at DEBUG, not as an error.
* epochs we never held the keys for, so callers should treat null as an
* expected "nothing to do here" outcome and log at DEBUG, not as an error.
* Most of that class of event no longer gets this far: what predates this
* device's join is held back before any of it is attempted -- see
* `ChatRoom.predatesMembership`.
*/
private fun tryDecryptOuterLayer(
mlsGroup: MlsGroup,

7
composeApp/src/commonMain/kotlin/press/mantra/compose/managers/MarmotReindexSweep.kt
View File

@@ -36,6 +36,11 @@ object MarmotReindexSweep {
/**
* @param stored how many group events the room holds in total, for the report.
* @param predatingMembership how many of [stored] the caller held back because
* they were published before this device joined, for the report. Carried
* through rather than worked out here for the same reason [stored] is: the
* sweep decides how many times to go round, not what is worth going round
* for.
* @param unresolved those with nothing to show for them, in the order to replay
* them. Anything already read must be left out: a replay is only ever allowed
* to touch events it cannot make worse.
@@ -48,6 +53,7 @@ object MarmotReindexSweep {
*/
suspend fun <T> run(
stored: Int,
predatingMembership: Int = 0,
unresolved: List<T>,
maxPasses: Int = DEFAULT_MAX_PASSES,
replay: suspend (T) -> Unit,
@@ -85,6 +91,7 @@ object MarmotReindexSweep {
return MarmotReindexReport(
stored = stored,
predatingMembership = predatingMembership,
unresolved = unresolved.size,
recovered = recovered,
failed = remaining.size,

22
composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt
View File

@@ -632,13 +632,27 @@ private fun ReindexMarmotGroupEventsButton(
when (reindexState) {
is ChatRoomDetailViewModel.ReindexState.Done -> {
val report = reindexState.report
// What was published before this member joined is named rather
// than counted as read. Their epoch keys were never on this
// device, so "all read" would be a claim about messages nobody
// here can open -- and a room that is mostly older than the
// member is the ordinary case for anyone invited into one.
val beforeJoining =
if (report.predatingMembership > 0) {
" · ${report.predatingMembership} from before you joined"
} else {
""
}
Text(
text = when {
report.isNoOp -> "Nothing to reindex · ${report.stored} event(s) all read"
report.isNoOp ->
"Nothing to reindex · ${report.stored - report.predatingMembership} " +
"event(s) all read$beforeJoining"
report.recovered > 0 && report.failed > 0 ->
"Recovered ${report.recovered} of ${report.unresolved} · ${report.failed} still unreadable"
report.recovered > 0 -> "Recovered ${report.recovered} of ${report.unresolved} event(s)"
else -> "${report.failed} event(s) still unreadable"
"Recovered ${report.recovered} of ${report.unresolved} · ${report.failed} still unreadable$beforeJoining"
report.recovered > 0 ->
"Recovered ${report.recovered} of ${report.unresolved} event(s)$beforeJoining"
else -> "${report.failed} event(s) still unreadable$beforeJoining"
},
style = MaterialTheme.typography.bodySmall,
textAlign = TextAlign.Center