From 5623df530c2075917983d57d39e207b4dc300bd8 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 02:50:14 +0200 Subject: [PATCH 01/12] test: execute the nostr filter query against real sqlite NostrEventFilterQueryTest pins the SQL string the builder produces. It never runs that string, and the gap between "the SQL reads correctly" and "sqlite returns the right rows" is where this query's expensive mistakes live. Four classes of bug survive a string assertion intact, and all four are covered here. SQL that is well-formed but not accepted. The clause emitted for a present-but-empty list is the bare literal `0`. Whether sqlite takes that as a false boolean expression rather than rejecting it is not something the builder test can answer; ids, authors, kinds and tags are each asserted to match nothing when handed an empty list. Binding indices. The limit is bound after every tag pattern, so its placeholder is the last in the statement. A drift in that order produces a byte-identical SQL string and different rows, so it is covered by a filter that carries authors, kinds, since, until, search, a tag and a limit at once. LIKE semantics against the column as actually written. The tag pattern is a fragment of the encoded tag -- `["p",""` -- and only real stored JSON can show that it anchors on the tag name (a pubkey in an `e` tag is not a `p` match, which is exactly the regression the substring scan `tags LIKE '%%'` caused), that it tolerates the relay hint and marker that follow a real tag value, and that escapeLike keeps a `%` in a tag value literal instead of widening the match. Timestamp units. This is a cross-file invariant nothing enforces: NostrEventFilterQuery binds since/until as epochSeconds, and MantraConverters.instantToTimestamp writes the createdAt column as epochSeconds. They agree today. Move either to milliseconds and both files still read correctly on their own while the filter silently selects nothing or everything, so the agreement is now asserted directly. Also covered: the NIP-01 inclusive bounds on both ends, using events stamped exactly on since and on until -- the case that tells an inclusive bound from the strict `createdAt > :since` this replaced; tags ORing values within a name and ANDing across names, against tagsAll which ANDs within a name too; newest-first ordering with the `id DESC` tiebreak, and a limit keeping the newest rather than the oldest window the per-shape queries used to return. Three of these were checked by mutation rather than assumed. Reverting the tag pattern to the naive `%value%` substring fails `a tag value is matched in its own position`; relaxing `createdAt >= ?` back to `>` fails both the inclusive bounds test and the units test. The mutations were reverted; no production source is touched by this commit. 14 tests. Co-Authored-By: Claude Opus 5 --- .../NostrEventFilterQueryExecutionTest.kt | 288 ++++++++++++++++++ 1 file changed, 288 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/query/NostrEventFilterQueryExecutionTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/query/NostrEventFilterQueryExecutionTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/query/NostrEventFilterQueryExecutionTest.kt new file mode 100644 index 00000000..fb6e9824 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/query/NostrEventFilterQueryExecutionTest.kt @@ -0,0 +1,288 @@ +package press.mantra.compose.database.query + +import androidx.room3.Room +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.types.SynchronizationFilter +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Instant + +/** + * `NostrEventFilterQueryTest` pins the SQL string this builder produces. It cannot pin what + * SQLite does with that string, and the gap between the two is where the expensive mistakes + * live: SQL that is well-formed but rejected, a bound value whose index is off by one, a LIKE + * whose escaping does not survive contact with the stored column, or a bound timestamp in + * units the column was never written in. All four produce a query that looks right in a + * string assertion. + * + * That last one is worth spelling out, because it spans two files that nothing forces to + * agree. [NostrEventFilterQuery] binds `since`/`until` as `epochSeconds`; + * `MantraConverters.instantToTimestamp` writes the `createdAt` column as `epochSeconds`. Change + * either one to milliseconds and the filter silently selects nothing, or everything. + * + * The stakes are the same as for the builder test: negentropy reconciles our set against the + * relay's set for the *same* filter, so any row this query gets wrong turns into an event + * needlessly re-downloaded or needlessly pushed at a relay that filtered it out on purpose. + */ +class NostrEventFilterQueryExecutionTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val alice = "a".repeat(64) + private val bob = "b".repeat(64) + private val carol = "c".repeat(64) + + private suspend fun store( + id: String, + pubKey: String = alice, + kind: Int = 1, + createdAt: Instant = Instant.fromEpochSeconds(1_000), + content: String = "hello", + tags: Array> = emptyArray(), + ): NostrEvent = NostrEvent( + id = id.padEnd(64, '0'), + pubKey = pubKey, + kind = kind, + tags = tags, + content = content, + sig = "0".repeat(128), + createdAt = createdAt, + ).also { db.nostrEventDao().upsert(it) } + + private suspend fun matching( + filter: SynchronizationFilter, + limit: Int = NostrEventFilterQuery.NO_LIMIT, + ): List = db.nostrEventDao().getNostrEventsMatchingFilter( + NostrEventFilterQuery.build(filter, limit) + ) + + private fun List.ids(): List = map { it.id } + + @Test + fun `an ids filter selects exactly the listed events`() = runBlocking { + val wanted = store("1") + store("2") + + val found = matching(SynchronizationFilter(ids = arrayOf(wanted.id))) + + assertEquals(listOf(wanted.id), found.ids()) + } + + /** + * The clause the builder emits for a present-but-empty list is the literal `0`. A string + * assertion cannot say whether SQLite accepts that as a boolean expression; this can. + */ + @Test + fun `a present but empty list matches nothing rather than everything`() = runBlocking { + store("1") + store("2") + + assertTrue(matching(SynchronizationFilter(ids = emptyArray())).isEmpty()) + assertTrue(matching(SynchronizationFilter(authors = emptyArray())).isEmpty()) + assertTrue(matching(SynchronizationFilter(kinds = emptyArray())).isEmpty()) + assertTrue(matching(SynchronizationFilter(tags = mapOf("p" to emptyList()))).isEmpty()) + } + + @Test + fun `no clauses at all selects every stored event`() = runBlocking { + store("1") + store("2") + + assertEquals(2, matching(SynchronizationFilter()).size) + } + + /** + * NIP-01 bounds are inclusive on both ends, and an event stamped exactly on the boundary + * is the case that tells an inclusive bound from an exclusive one. The queries this + * builder replaced used a strict `createdAt > :since`, which put boundary events in the + * relay's set and not in ours. + */ + @Test + fun `since and until are inclusive on both ends`() = runBlocking { + val before = store("1", createdAt = Instant.fromEpochSeconds(999)) + val onSince = store("2", createdAt = Instant.fromEpochSeconds(1_000)) + val between = store("3", createdAt = Instant.fromEpochSeconds(1_500)) + val onUntil = store("4", createdAt = Instant.fromEpochSeconds(2_000)) + val after = store("5", createdAt = Instant.fromEpochSeconds(2_001)) + + val found = matching( + SynchronizationFilter( + since = Instant.fromEpochSeconds(1_000), + until = Instant.fromEpochSeconds(2_000), + ) + ).ids() + + assertTrue(onSince.id in found, "an event stamped exactly on `since` must be included") + assertTrue(onUntil.id in found, "an event stamped exactly on `until` must be included") + assertTrue(between.id in found) + assertTrue(before.id !in found) + assertTrue(after.id !in found) + } + + /** + * The cross-file invariant. The filter binds seconds; the converter writes seconds. If + * either side moved to milliseconds this would keep compiling and start selecting the + * wrong century. + */ + @Test + fun `the bound timestamps are in the same units the converter writes`() = runBlocking { + val at = Instant.fromEpochSeconds(1_700_000_000) + val event = store("1", createdAt = at) + + assertEquals(listOf(event.id), matching(SynchronizationFilter(since = at)).ids()) + assertEquals(listOf(event.id), matching(SynchronizationFilter(until = at)).ids()) + assertTrue(matching(SynchronizationFilter(since = at.plus(kotlin.time.Duration.parse("1s")))).isEmpty()) + assertTrue(matching(SynchronizationFilter(until = at.minus(kotlin.time.Duration.parse("1s")))).isEmpty()) + } + + @Test + fun `a tag filter ORs the values of one name and ANDs across names`() = runBlocking { + val both = store("1", tags = arrayOf(arrayOf("p", alice), arrayOf("e", "beef".padEnd(64, '0')))) + val onlyP = store("2", tags = arrayOf(arrayOf("p", bob))) + store("3", tags = arrayOf(arrayOf("e", "beef".padEnd(64, '0')))) + + val orOverValues = matching( + SynchronizationFilter(tags = mapOf("p" to listOf(alice, bob))) + ).ids() + assertEquals(setOf(both.id, onlyP.id), orOverValues.toSet()) + + val andOverNames = matching( + SynchronizationFilter( + tags = mapOf("p" to listOf(alice, bob), "e" to listOf("beef".padEnd(64, '0'))) + ) + ).ids() + assertEquals(listOf(both.id), andOverNames) + } + + /** + * The reason [NostrEventFilterQuery] anchors on the tag name as well as the value. The + * substring scan it replaced -- `tags LIKE '%%'` -- matched the same hex sitting + * in any tag position, so a `p` filter pulled in every event that merely referenced that + * key in an `e` tag. + */ + @Test + fun `a tag value is matched in its own position and not anywhere in the row`() = runBlocking { + val taggedAsPerson = store("1", tags = arrayOf(arrayOf("p", carol))) + store("2", tags = arrayOf(arrayOf("e", carol))) + + val found = matching(SynchronizationFilter(tags = mapOf("p" to listOf(carol)))).ids() + + assertEquals(listOf(taggedAsPerson.id), found, "the `e` tagged event is not a `p` match") + } + + /** + * Real tags carry a relay hint and a marker after the value, which is why the pattern + * drops the closing bracket rather than matching the whole encoded tag. + */ + @Test + fun `a tag match tolerates trailing tag elements`() = runBlocking { + val withHint = store( + "1", + tags = arrayOf(arrayOf("p", alice, "wss://relay.example", "mention")), + ) + + val found = matching(SynchronizationFilter(tags = mapOf("p" to listOf(alice)))).ids() + + assertEquals(listOf(withHint.id), found) + } + + /** `escapeLike`: a wildcard inside a tag value must be a literal, not a widening match. */ + @Test + fun `a wildcard inside a tag value does not widen the match`() = runBlocking { + val literal = store("1", tags = arrayOf(arrayOf("d", "100%"))) + store("2", tags = arrayOf(arrayOf("d", "100 percent"))) + + val found = matching(SynchronizationFilter(tags = mapOf("d" to listOf("100%")))).ids() + + assertEquals(listOf(literal.id), found, "the `%` was treated as a wildcard") + } + + @Test + fun `tagsAll requires every value of a name rather than any of them`() = runBlocking { + val hasBoth = store("1", tags = arrayOf(arrayOf("p", alice), arrayOf("p", bob))) + store("2", tags = arrayOf(arrayOf("p", alice))) + + val all = matching(SynchronizationFilter(tagsAll = mapOf("p" to listOf(alice, bob)))).ids() + assertEquals(listOf(hasBoth.id), all) + + val any = matching(SynchronizationFilter(tags = mapOf("p" to listOf(alice, bob)))).ids() + assertEquals(2, any.size, "the OR form should still match both") + } + + /** + * A relay hands back the newest events under a limit. The per-shape queries this replaced + * ordered ascending under the same limit and so returned the oldest, which is the opposite + * window. + */ + @Test + fun `results are newest first and a limit keeps the newest`() = runBlocking { + val oldest = store("1", createdAt = Instant.fromEpochSeconds(1_000)) + val middle = store("2", createdAt = Instant.fromEpochSeconds(2_000)) + val newest = store("3", createdAt = Instant.fromEpochSeconds(3_000)) + + assertEquals(listOf(newest.id, middle.id, oldest.id), matching(SynchronizationFilter()).ids()) + assertEquals(listOf(newest.id, middle.id), matching(SynchronizationFilter(), limit = 2).ids()) + } + + /** The `id DESC` tiebreak, without which a limit over equal timestamps is arbitrary. */ + @Test + fun `events sharing a timestamp are ordered by id descending`() = runBlocking { + val sameMoment = Instant.fromEpochSeconds(1_000) + val lower = store("1", createdAt = sameMoment) + val higher = store("2", createdAt = sameMoment) + + assertEquals(listOf(higher.id, lower.id), matching(SynchronizationFilter()).ids()) + assertEquals(listOf(higher.id), matching(SynchronizationFilter(), limit = 1).ids()) + } + + /** + * The limit is bound *after* every tag pattern, so its placeholder is the last one in the + * statement. Nothing but execution catches a binding index that drifted: the SQL string + * would be identical either way. + */ + @Test + fun `binding order holds when a filter combines clauses with a limit`() = runBlocking { + val wanted = store( + "1", + pubKey = alice, + kind = 1, + createdAt = Instant.fromEpochSeconds(1_500), + content = "find me", + tags = arrayOf(arrayOf("p", bob)), + ) + store("2", pubKey = alice, kind = 1, createdAt = Instant.fromEpochSeconds(1_500), content = "find me", tags = arrayOf(arrayOf("p", carol))) + store("3", pubKey = bob, kind = 1, createdAt = Instant.fromEpochSeconds(1_500), content = "find me", tags = arrayOf(arrayOf("p", bob))) + + val found = matching( + SynchronizationFilter( + authors = arrayOf(alice), + kinds = arrayOf(1), + since = Instant.fromEpochSeconds(1_000), + until = Instant.fromEpochSeconds(2_000), + search = "find me", + tags = mapOf("p" to listOf(bob)), + ), + limit = 10, + ).ids() + + assertEquals(listOf(wanted.id), found) + } + + @Test + fun `a search clause matches on content`() = runBlocking { + val wanted = store("1", content = "the quick brown fox") + store("2", content = "nothing to see") + + assertEquals(listOf(wanted.id), matching(SynchronizationFilter(search = "quick brown")).ids()) + } +} From 20d2547a34e63cf56bf323a9cd263f5e084c797e Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 02:50:32 +0200 Subject: [PATCH 02/12] test: cover the hand-written NostrEventDao queries against sqlite Two of these queries carry a comment describing a bug that already shipped, and neither bug was the kind anything catches by running: a wrong WHERE clause is still a valid query returning a plausible list. Both corrected predicates are now pinned, so the next edit has to argue with a failing test rather than with a comment. getMarmotGroupEvents. The predicate used to read `expiresAt < :now`, which kept exactly the expired events and dropped every live one -- for the whole group-chat sync path the set handed to negentropy was the complement of the relay's. Covered with four rows at once: no expiry at all (always served), an expiry in the future (still served), an expiry in the past (gone), and an expiry landing exactly on `now`, which the strict `>` excludes. Also covered: the inclusive since/until bounds using events stamped on each bound, room membership filtering across two rooms, and newest-first ordering with a limit keeping the newest window. getMarmotGroupNostrEventsByChatRoomId. Ascending order, because a replay has to apply commits in the order they were sent and this is the one query in the DAO that deliberately orders that way. The test that matters most here is that an event which never reached the MarmotGroupEvent table is still returned -- that is the whole reason the query reads NostrEvent instead of joining the index, since an event whose indexing failed part way is precisely what a replay exists to pick up, and a join would skip exactly those rows. Asserted from both sides: the un-indexed event comes back from the replay query and is genuinely absent from getMarmotGroupEvents. The same query's LIKE over-match is pinned deliberately rather than asserted away. The DAO's comment calls it a prefilter and puts the burden on callers to confirm the event's own `h` tag, so a room id sitting in an `e` tag is expected to come back. Recording it in both directions means anyone tightening the query knows a caller may rely on the loose behaviour, and anyone loosening a caller's check knows why it was there. Also covered: getNostrEventByPublicKeyAndKind returning the newest row, which is what makes it correct for replaceable events rather than a coin flip; and the difference between the two write paths, where `insert` with IGNORE keeps the stored event -- correct when re-receiving an immutable event from a second relay -- while `upsert` overwrites it. Last, the paged reads are pinned as treating `since` exclusively, which is what makes them safe to call in a loop with the previous page's last timestamp as the cursor. That sits one query away from the inclusive bound in getMarmotGroupEvents on purpose: the two conventions are genuinely different, and a reader who assumes either holds throughout gets a skipped row or a loop that never advances. The expiry test was checked by mutation rather than assumed: restoring `expiresAt < :now` fails it alone, with "an event expiring in the future is still live". The mutation was reverted; no production source is touched by this commit. 11 tests. composeApp jvmTest is 239 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../database/dao/NostrEventDaoJvmTest.kt | 286 ++++++++++++++++++ 1 file changed, 286 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrEventDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrEventDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrEventDaoJvmTest.kt new file mode 100644 index 00000000..44f60eda --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrEventDaoJvmTest.kt @@ -0,0 +1,286 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.MarmotGroupEvent +import press.mantra.compose.database.model.NostrEvent +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue +import kotlin.time.Instant + +/** + * The hand-written queries on [NostrEventDao], as SQLite actually runs them. + * + * Two of them carry a comment describing a bug that already shipped -- an expiry predicate + * that kept exactly the expired rows and dropped the live ones, and a set of queries that + * returned the oldest events under a limit instead of the newest. Both were reasoned about in + * review and neither was caught by anything that runs, because a wrong `WHERE` clause is + * still a valid query returning a plausible list. These pin the corrected behaviour so the + * next edit to the predicate has to argue with a failing test. + */ +class NostrEventDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val alice = "a".repeat(64) + private val roomOne = "11".repeat(32) + private val roomTwo = "22".repeat(32) + + private val epoch = Instant.fromEpochSeconds(0) + private val distantFuture = Instant.fromEpochSeconds(4_000_000_000) + + private suspend fun storeEvent( + id: String, + kind: Int = MarmotGroupEvent.KIND, + pubKey: String = alice, + createdAt: Instant = Instant.fromEpochSeconds(1_000), + tags: Array> = emptyArray(), + content: String = "{}", + ): NostrEvent = NostrEvent( + id = id.padEnd(64, '0'), + pubKey = pubKey, + kind = kind, + tags = tags, + content = content, + sig = "0".repeat(128), + createdAt = createdAt, + ).also { db.nostrEventDao().upsert(it) } + + /** + * `MarmotGroupEvent.id` is a foreign key onto `NostrEvent.id`, so the parent row has to + * exist first -- the indexed row is a projection of an event that was stored. + */ + private suspend fun storeGroupEvent( + id: String, + chatRoomId: String = roomOne, + createdAt: Instant = Instant.fromEpochSeconds(1_000), + expiresAt: Instant? = null, + ): MarmotGroupEvent { + val event = storeEvent(id, createdAt = createdAt, tags = arrayOf(arrayOf("h", chatRoomId))) + return MarmotGroupEvent( + id = event.id, + userPublicKey = alice, + publicKey = alice, + chatRoomId = chatRoomId, + signature = "0".repeat(128), + encryptedContent = "ciphertext", + expiresAt = expiresAt, + createdAt = createdAt, + ).also { db.marmotGroupEventDao().upsert(it) } + } + + private suspend fun groupEvents( + rooms: Array = arrayOf(roomOne), + since: Instant = epoch, + until: Instant = distantFuture, + limit: Int = 100, + now: Instant, + ) = db.nostrEventDao().getMarmotGroupEvents(rooms, since, until, limit, now) + + /** + * The regression this query's own comment describes. `expiresAt > :now` keeps what a + * relay would still serve; the `expiresAt < :now` it replaced kept precisely the events a + * relay had stopped serving and dropped every live one, so the set handed to negentropy + * was the complement of the relay's for the whole group-chat sync path. + */ + @Test + fun `an expiring group event is served until it expires and not after`() = runBlocking { + val now = Instant.fromEpochSeconds(2_000) + val neverExpires = storeGroupEvent("1", expiresAt = null) + val stillLive = storeGroupEvent("2", expiresAt = Instant.fromEpochSeconds(2_001)) + val expired = storeGroupEvent("3", expiresAt = Instant.fromEpochSeconds(1_999)) + val expiringNow = storeGroupEvent("4", expiresAt = now) + + val found = groupEvents(now = now).map { it.id } + + assertTrue(neverExpires.id in found, "an event with no expiry never stops being served") + assertTrue(stillLive.id in found, "an event expiring in the future is still live") + assertTrue(expired.id !in found, "an expired event must not be served") + assertTrue( + expiringNow.id !in found, + "the bound is strict: an event expiring exactly now has stopped being served", + ) + } + + /** NIP-01 bounds are inclusive, so an event stamped on either bound is inside the window. */ + @Test + fun `the since and until bounds are inclusive`() = runBlocking { + val now = Instant.fromEpochSeconds(10) + val before = storeGroupEvent("1", createdAt = Instant.fromEpochSeconds(999)) + val onSince = storeGroupEvent("2", createdAt = Instant.fromEpochSeconds(1_000)) + val onUntil = storeGroupEvent("3", createdAt = Instant.fromEpochSeconds(2_000)) + val after = storeGroupEvent("4", createdAt = Instant.fromEpochSeconds(2_001)) + + val found = groupEvents( + since = Instant.fromEpochSeconds(1_000), + until = Instant.fromEpochSeconds(2_000), + now = now, + ).map { it.id } + + assertEquals(setOf(onSince.id, onUntil.id), found.toSet()) + assertTrue(before.id !in found) + assertTrue(after.id !in found) + } + + @Test + fun `only the requested rooms come back`() = runBlocking { + val now = Instant.fromEpochSeconds(10) + val mine = storeGroupEvent("1", chatRoomId = roomOne) + val theirs = storeGroupEvent("2", chatRoomId = roomTwo) + + assertEquals(listOf(mine.id), groupEvents(rooms = arrayOf(roomOne), now = now).map { it.id }) + assertEquals( + setOf(mine.id, theirs.id), + groupEvents(rooms = arrayOf(roomOne, roomTwo), now = now).map { it.id }.toSet(), + ) + } + + /** Newest first under a limit, which is the window a relay would hand back. */ + @Test + fun `group events come back newest first and a limit keeps the newest`() = runBlocking { + val now = Instant.fromEpochSeconds(10) + val oldest = storeGroupEvent("1", createdAt = Instant.fromEpochSeconds(1_000)) + val middle = storeGroupEvent("2", createdAt = Instant.fromEpochSeconds(2_000)) + val newest = storeGroupEvent("3", createdAt = Instant.fromEpochSeconds(3_000)) + + assertEquals( + listOf(newest.id, middle.id, oldest.id), + groupEvents(now = now).map { it.id }, + ) + assertEquals(listOf(newest.id, middle.id), groupEvents(limit = 2, now = now).map { it.id }) + } + + /** + * Replay order. A commit stream has to be applied in the order it was sent, so this query + * is the one place in the DAO that deliberately orders ascending. + */ + @Test + fun `a rooms group events replay oldest first`() = runBlocking { + val third = storeEvent("3", createdAt = Instant.fromEpochSeconds(3_000), tags = arrayOf(arrayOf("h", roomOne))) + val first = storeEvent("1", createdAt = Instant.fromEpochSeconds(1_000), tags = arrayOf(arrayOf("h", roomOne))) + val second = storeEvent("2", createdAt = Instant.fromEpochSeconds(2_000), tags = arrayOf(arrayOf("h", roomOne))) + + val found = db.nostrEventDao().getMarmotGroupNostrEventsByChatRoomId(roomOne).map { it.id } + + assertEquals(listOf(first.id, second.id, third.id), found) + } + + /** + * The reason this query reads `NostrEvent` rather than joining `MarmotGroupEvent`: an + * event whose indexing failed part way was stored without ever reaching that table, and + * those are exactly the ones a replay exists to pick up. A join would skip precisely the + * rows worth replaying. + */ + @Test + fun `an event that never reached the index is still replayed`() = runBlocking { + val indexed = storeGroupEvent("1", createdAt = Instant.fromEpochSeconds(1_000)) + val neverIndexed = storeEvent( + "2", + createdAt = Instant.fromEpochSeconds(2_000), + tags = arrayOf(arrayOf("h", roomOne)), + ) + + val found = db.nostrEventDao().getMarmotGroupNostrEventsByChatRoomId(roomOne).map { it.id } + + assertEquals(listOf(indexed.id, neverIndexed.id), found) + assertTrue( + db.nostrEventDao().getMarmotGroupEvents(arrayOf(roomOne), epoch, distantFuture, 100, epoch) + .none { it.id == neverIndexed.id }, + "the un-indexed event is genuinely absent from the indexed table", + ) + } + + @Test + fun `a replay ignores events of other kinds`() = runBlocking { + val groupEvent = storeEvent("1", tags = arrayOf(arrayOf("h", roomOne))) + storeEvent("2", kind = 1, tags = arrayOf(arrayOf("h", roomOne))) + + val found = db.nostrEventDao().getMarmotGroupNostrEventsByChatRoomId(roomOne).map { it.id } + + assertEquals(listOf(groupEvent.id), found) + } + + /** + * Documented contract, not an accident: the `LIKE` is a prefilter over serialised tags, so + * a room id sitting in some other tag position matches too. Callers must confirm the + * event's own `h` tag before treating a row as this room's. Pinned so that anyone + * tightening the query knows a caller may already depend on the loose behaviour, and + * anyone loosening a caller knows why the check is there. + */ + @Test + fun `the replay prefilter can over-match and callers must confirm the h tag`() = runBlocking { + val real = storeEvent("1", tags = arrayOf(arrayOf("h", roomOne))) + val coincidence = storeEvent("2", tags = arrayOf(arrayOf("e", roomOne))) + + val found = db.nostrEventDao().getMarmotGroupNostrEventsByChatRoomId(roomOne).map { it.id } + + assertEquals(setOf(real.id, coincidence.id), found.toSet()) + } + + /** + * `getNostrEventByPublicKeyAndKind` returns a single row from a query ordered newest + * first, which is what makes it correct for replaceable events -- the latest metadata + * event for an author, not whichever one SQLite happened to reach first. + */ + @Test + fun `the newest event wins for an author and kind`() = runBlocking { + storeEvent("1", kind = 0, createdAt = Instant.fromEpochSeconds(1_000), content = "old") + storeEvent("2", kind = 0, createdAt = Instant.fromEpochSeconds(2_000), content = "new") + + val found = db.nostrEventDao().getNostrEventByPublicKeyAndKind(alice, 0) + + assertNotNull(found) + assertEquals("new", found.content) + } + + /** + * The two write paths differ and the difference matters: a nostr event is immutable under + * its id, so `insert` with IGNORE is the right call when re-receiving an event from a + * second relay, while `upsert` overwrites. Using the wrong one silently replaces a stored + * event with a re-received copy that may carry different local columns. + */ + @Test + fun `insert keeps the stored event while upsert replaces it`() = runBlocking { + val original = storeEvent("1", content = "original") + + db.nostrEventDao().insert(original.copy(content = "from another relay")) + assertEquals( + "original", + db.nostrEventDao().getNostrEventById(original.id)?.content, + "insert must ignore a conflicting id rather than overwrite", + ) + + db.nostrEventDao().upsert(original.copy(content = "replaced")) + assertEquals("replaced", db.nostrEventDao().getNostrEventById(original.id)?.content) + } + + /** + * The paged reads use a strict `createdAt > :since`, which is what makes them safe to call + * in a loop with the last row's timestamp as the next cursor. Worth pinning next to the + * inclusive bound in `getMarmotGroupEvents`: the two are deliberately different, and a + * reader who assumes one convention holds everywhere would introduce either a skipped row + * or an endless loop. + */ + @Test + fun `the paged reads treat since as exclusive`() = runBlocking { + val onBoundary = storeEvent("1", kind = 1, createdAt = Instant.fromEpochSeconds(1_000)) + val after = storeEvent("2", kind = 1, createdAt = Instant.fromEpochSeconds(1_001)) + + val found = db.nostrEventDao() + .getNostrEvents(arrayOf(1), Instant.fromEpochSeconds(1_000), 100) + .map { it.id } + + assertEquals(listOf(after.id), found) + assertTrue(onBoundary.id !in found, "`since` is exclusive on this query") + } +} From 36a98c5928144cfda70181f9305a65f31002183c Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:05:17 +0200 Subject: [PATCH 03/12] test: pin the nip30303 store-and-submit invariant in MantraDao Every `add*` on MantraDao does two things in one transaction: writes the entity and queues a SubmissionEvent carrying the same nip30303 event for the group. The part worth asserting is the one `rumorOf` exists for. An entity's id is computed by its `Mantra*.from*EventTemplate` factory. The payload's id is computed separately, in `rumorOf`, from the same template. The two are meant to produce the *same event* -- the row on disk and the payload on the wire, not two copies of one. Nothing enforces that: the factories live in different files, both compile independently, and both produce a plausible 64-character id. A divergence would surface only as a group that receives a submission whose payload matches nothing it can find, which is a long way from the two hash calls that disagreed. Covered, through the seam rather than by recomputing the hash: the submission records `payloadEventId`, and that value has to equal the id of the entity the same call returned. Asserted for a dialect and again for an artifact version, because store-and-submit is the convention every `add*` follows rather than something addDialect does on its own -- and the second one goes through the full foreign key chain, dialect then artifact then version. Also covered: The envelope is not the payload. A submission's own id is the SubmissionEvent's and must differ from the payload's, which is exactly why `deleteByPayloadEventId` exists -- a superseded nip30303 event cannot be un-queued by its own id, and if the two ever collapsed to one value that method would start deleting envelopes by accident. The submission is queued unprocessed, `marmotGroupEventId == null`. That null is what the outbound pipeline selects on to encrypt the row into a kind:445. Filed as processed it would be stored and never sent, and the group would simply never learn about the dialect while the local device showed it as added. A ChatMessage line is written, since the room's feed reads ChatMessage and an added entity that leaves no line is invisible to everyone including its author. Verified by mutation rather than assumed: making `rumorOf` hash a createdAt one second off the template's fails both invariant tests, with the ids compared in the failure output. The mutation was reverted; no production source is touched by this commit. 6 tests. Co-Authored-By: Claude Opus 5 --- .../compose/database/dao/MantraDaoJvmTest.kt | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MantraDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MantraDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MantraDaoJvmTest.kt new file mode 100644 index 00000000..33e7d161 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MantraDaoJvmTest.kt @@ -0,0 +1,206 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.MantraArtifact +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.nostr.nip30303.DialectEvent +import press.mantra.compose.nostr.nip30303.SubmissionEvent +import press.mantra.compose.repository.MantraRepository +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * The nip30303 create-entity flow: every `add*` on [MantraDao] writes the entity and queues a + * [SubmissionEvent] carrying the same event for the group, in one transaction. + * + * The invariant worth a test is the one `rumorOf` exists for. The entity's id is computed by + * the `Mantra*.from*EventTemplate` factory and the payload's id is computed here, from the same + * template -- so the row on disk and the payload on the wire are meant to be *the same event*, + * not two copies of one. Nothing enforces that: both sides compile independently, both produce + * a plausible 64-character id, and a divergence would only show up as a group that can never + * match an arriving submission to the entity it was supposed to create. + */ +class MantraDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val author = "a".repeat(64) + private val roomId = "b".repeat(64) + + /** ChatRoom -> Profile -> NostrEvent, the foreign key chain a room hangs off. */ + private suspend fun seedRoom(): LocalChatRoom { + val nostrEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = author, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = author, userName = "author", nostrEventId = nostrEventId)) + val chatRoom = ChatRoom( + id = roomId, + userPublicKey = author, + subject = "a translation room", + description = null, + mlsGroupState = null, + ) + db.chatRoomDao().upsert(chatRoom) + return LocalChatRoom(chatRoom = chatRoom) + } + + private suspend fun submissions() = db.marmotInnerEventDao() + .getByChatRoomAndKinds(roomId, listOf(SubmissionEvent.KIND)) + + private suspend fun addDialect(name: String = "Sesotho") = db.mantraDao().addDialect( + localChatRoom = seedRoom(), + name = name, + country = "ZA", + language = "st", + userPublicKey = author, + ) + + @Test + fun `a dialect is stored and queued for the group in one call`() = runBlocking { + val dialect = assertNotNull(addDialect(), "addDialect returned null") + + assertEquals("Sesotho", dialect.name) + assertEquals(roomId, dialect.chatRoomId) + assertEquals(author, dialect.publicKey) + assertEquals(1, submissions().size, "the dialect was stored without being submitted") + } + + /** + * The `rumorOf` invariant, asserted across the seam: the submission records the payload's + * id, and that id has to be the entity's own. If the two factories ever compute it + * differently the group receives a submission whose payload matches nothing on disk. + */ + @Test + fun `the stored dialect and the submitted payload are the same event`() = runBlocking { + val dialect = assertNotNull(addDialect()) + + val submission = submissions().single() + + assertEquals( + dialect.id, + submission.payloadEventId, + "the entity id and the submitted payload id have diverged", + ) + } + + /** + * The envelope is not the payload. A submission's own id is the SubmissionEvent's, which is + * why `deleteByPayloadEventId` exists -- a superseded nip30303 event cannot be un-queued by + * its own id. + */ + @Test + fun `the submission is an envelope with its own id`() = runBlocking { + val dialect = assertNotNull(addDialect()) + + val submission = submissions().single() + + assertEquals(SubmissionEvent.KIND, submission.kind) + assertTrue( + submission.id != dialect.id, + "the envelope must not share the payload's id, or it could not be told apart", + ) + assertEquals(author, submission.publicKey) + assertEquals(roomId, submission.chatRoomId) + } + + /** + * `marmotGroupEventId == null` is what makes the row unprocessed, which is the state the + * outbound pipeline selects on to encrypt it into a kind:445. Filed as processed, it would + * be stored and never sent, and the group would never learn about the dialect. + */ + @Test + fun `the submission is queued unprocessed for the outbound pipeline`() = runBlocking { + addDialect() + + val submission = submissions().single() + + assertNull(submission.marmotGroupEventId, "a queued submission must not look processed") + } + + /** The room's feed reads ChatMessage, so an added entity has to leave a line behind. */ + @Test + fun `a chat message line is written so the room shows the change`() = runBlocking { + addDialect(name = "isiZulu") + + val submission = submissions().single() + val chatMessage = db.chatMessageDao().getChatMessagesByMarmotInnerEventId(submission.id) + + assertNotNull(chatMessage, "no chat line was written for the submission") + assertEquals("Added isiZulu as a dialect", chatMessage.content) + assertEquals(roomId, chatMessage.chatRoomId) + assertTrue(chatMessage.isUserMessage) + } + + /** + * A second entity type through the same path, because the store-and-submit shape is the + * convention every `add*` follows rather than something `addDialect` does on its own. + */ + @Test + fun `an artifact version follows the same store-and-submit shape`() = runBlocking { + val localChatRoom = seedRoom() + val dialect = assertNotNull( + db.mantraDao().addDialect( + localChatRoom = localChatRoom, + name = "Setswana", + country = "ZA", + language = "tn", + userPublicKey = author, + ) + ) + val artifactId = "d".repeat(64) + db.mantraArtifactDao().upsert( + MantraArtifact( + id = artifactId, + publicKey = author, + name = "a text", + url = "https://example.invalid/text", + visibility = MantraRepository.DEFAULT_VISIBILITY, + dialectId = dialect.id, + license = MantraRepository.DEFAULT_LICENSE, + chatRoomId = roomId, + signature = "", + ) + ) + + val version = assertNotNull( + db.mantraDao().addArtifactVersion( + localChatRoom = localChatRoom, + artifactId = artifactId, + versionLabel = "first draft", + userPublicKey = author, + ), + "addArtifactVersion returned null", + ) + + val versionSubmission = assertNotNull( + submissions().singleOrNull { it.payloadEventId == version.id }, + "the artifact version was stored without a matching submission", + ) + assertEquals(SubmissionEvent.KIND, versionSubmission.kind) + assertNull(versionSubmission.marmotGroupEventId) + assertEquals(2, submissions().size, "the dialect and the version should each be queued") + } +} From ba0c60dd2e68809dd16ecb425e0d77e4fb66573e Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:05:36 +0200 Subject: [PATCH 04/12] test: cover the NostrDao event funnel and the publish durability split NostrDao is what every event passes through, inbound and outbound, so its two decisions carry everything downstream: which of two copies of an event wins, and what survives when the enrichment after a write fails. Both were described in comments and neither was asserted. Deduplication, at all four positions. A first sighting is stored. A strictly newer copy replaces the stored one. An older copy is ignored. And -- the case that actually distinguishes the implementations -- a redelivery carrying the *same* timestamp is a no-op, because the comparison is a strict `>`. That last one is not hypothetical: relays redeliver and negentropy re-syncs, so the common case is the same event arriving again unchanged, and a `>=` there would rewrite the row on every delivery. The publish durability split, which is where a bug shipped. `commitPublishedNostrEvent` is the durable half -- mark the unsigned row signed, store the event, queue a broadcast per relay -- and indexing is best-effort enrichment that runs in its own transaction. They used to share one, so any throw in indexing rolled back `signedAt` too. Because the notary drains one unsigned row at a time, that row was then re-selected forever and every event queued behind it went unsigned, including the MLS key package that is enqueued last. The test provokes the failure the way the code itself would fail: publishing with no target relays reaches `relayURLs.first()` inside the try and throws. It then asserts `signedAt` and the stored event both survived. The happy path is covered alongside it, asserting a broadcast request per target relay, so the durability test cannot pass by publishing nothing at all. Also covered: an event from an author with no profile leaves a "LOADING..." placeholder stamped GENESIS_AT rather than nothing, since that row is the only record that the pubkey was seen and needs fetching; and rescheduleBroadcastNostrEventRequests re-queueing a broadcast and re-linking it to the chat line when the event is a group message that has one, without inventing a relation when it does not. One test began as a wrong assumption and the schema corrected it. The "no chat line" case was first written against an event id that had never been stored, and failed with SQLite 787: BroadcastNostrEventRequest.nostrEventId is a foreign key onto NostrEvent. So the real invariant is that a broadcast cannot be scheduled for an event the caller has not saved; the test now stores the event and leaves only the chat line missing, and says so in a comment rather than quietly seeding around it. Verified by mutation: relaxing the dedup comparison to `>=` fails the same-timestamp test; removing the try/catch around indexing so the throw propagates fails the durability test. Both mutations were reverted; no production source is touched by this commit. Uses `runBlocking` on the durability test because its last expression is an assertNotNull, and a test method that returns a value is rejected by the JUnit4 runner outright. 9 tests. Co-Authored-By: Claude Opus 5 --- .../compose/database/dao/NostrDaoJvmTest.kt | 321 ++++++++++++++++++ 1 file changed, 321 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrDaoJvmTest.kt new file mode 100644 index 00000000..72915a18 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrDaoJvmTest.kt @@ -0,0 +1,321 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.BroadcastNostrEventRequest +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.UnsignedNostrEvent +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlin.time.Instant + +/** + * [NostrDao] is the funnel every event passes through, inbound and outbound, so its two + * decisions are load-bearing for everything downstream: which of two copies of an event wins, + * and what survives when the enrichment that follows a write fails. + * + * Both were reasoned about in comments rather than asserted. The publish path carries a + * description of a bug that shipped -- indexing sharing the commit's transaction, so any throw + * in it rolled back `signedAt` as well, leaving the notary to re-select the same unsigned row + * forever and never sign anything queued behind it, including the MLS key package that goes + * last. + */ +class NostrDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val keyPair = KeyPair() + private val author = keyPair.pubKey.toHexKey() + private val relay = "wss://relay.example" + + /** A kind nothing dispatches on, so these tests see the funnel and not a kind handler. */ + private val inertKind = 31_337 + + private fun event( + id: String, + createdAt: Instant, + content: String = "original", + kind: Int = inertKind, + pubKey: String = author, + unsignedNostrEventId: Long? = null, + ) = NostrEvent( + id = id.padEnd(64, '0'), + pubKey = pubKey, + kind = kind, + tags = emptyArray(), + content = content, + sig = "0".repeat(128), + createdAt = createdAt, + unsignedNostrEventId = unsignedNostrEventId, + ) + + private suspend fun store(nostrEvent: NostrEvent) = db.nostrDao().storeNostrEvent( + nostrEvent = nostrEvent, + relayURL = relay, + synchronizationRelayURLs = listOf(relay), + level = 0, + activeKeyPair = keyPair, + ) + + @Test + fun `a first sighting of an event is stored`() = runBlocking { + val incoming = event("1", Instant.fromEpochSeconds(1_000)) + + store(incoming) + + assertEquals("original", db.nostrEventDao().getNostrEventById(incoming.id)?.content) + } + + /** + * Same id, later timestamp: the newer copy wins. Relays redeliver and negentropy re-syncs, + * so an event arrives repeatedly and the funnel has to be idempotent in the right + * direction. + */ + @Test + fun `a strictly newer copy of an event replaces the stored one`() = runBlocking { + val first = event("1", Instant.fromEpochSeconds(1_000), content = "original") + store(first) + + store(first.copy(createdAt = Instant.fromEpochSeconds(2_000), content = "newer")) + + assertEquals("newer", db.nostrEventDao().getNostrEventById(first.id)?.content) + } + + @Test + fun `an older copy of an event is ignored`() = runBlocking { + val first = event("1", Instant.fromEpochSeconds(2_000), content = "original") + store(first) + + store(first.copy(createdAt = Instant.fromEpochSeconds(1_000), content = "older")) + + assertEquals("original", db.nostrEventDao().getNostrEventById(first.id)?.content) + } + + /** + * The boundary. The comparison is a strict `>`, so a redelivery of the *same* event -- same + * id, same timestamp, which is what a second relay hands over -- is a no-op rather than a + * rewrite. + */ + @Test + fun `a redelivery at the same timestamp is a no-op`() = runBlocking { + val first = event("1", Instant.fromEpochSeconds(1_000), content = "original") + store(first) + + store(first.copy(content = "from another relay")) + + assertEquals("original", db.nostrEventDao().getNostrEventById(first.id)?.content) + } + + /** + * An event from an author with no profile leaves a placeholder behind rather than nothing. + * The placeholder is stamped GENESIS_AT, which is the marker the sync path looks for -- + * without the row there is no record that this pubkey was ever seen and needs fetching. + */ + @Test + fun `an unknown author gets a placeholder profile to be synced later`() = runBlocking { + val stranger = "f".repeat(64) + + store(event("1", Instant.fromEpochSeconds(1_000), pubKey = stranger)) + + val profile = db.profileDao().getProfileByPublicKey(stranger) + assertNotNull(profile, "no placeholder profile was created for an unseen author") + assertEquals("LOADING...", profile.displayName) + } + + /** + * The documented split. `commitPublishedNostrEvent` is the durable half and indexing is + * best-effort enrichment in its own transaction, so a throw in indexing must leave the + * commit standing. + * + * Indexing is made to fail here the way the code itself would fail it: publishing with no + * target relays reaches `relayURLs.first()` inside the try, which throws. The assertion is + * that everything the durable half wrote is still there afterwards -- above all `signedAt`, + * because the notary drains one unsigned row at a time and a row whose `signedAt` was + * rolled back is re-selected forever, blocking every event queued behind it. + */ + @Test + fun `a failure while indexing does not roll back the published event`() = runBlocking { + val unsignedId = db.unsignedNostrEventDao().upsert( + UnsignedNostrEvent( + pubKey = author, + kind = inertKind, + tags = emptyArray(), + content = "to be published", + ) + ) + val unsigned = db.unsignedNostrEventDao().getAUnsignedNostrEvents().single { it.id == unsignedId } + val signed = event("1", Instant.fromEpochSeconds(1_000), unsignedNostrEventId = unsignedId) + + db.nostrDao().publishNostrEvent( + unsignedNostrEvent = unsigned, + nostrEvent = signed, + relayURLs = emptyList(), + activeKeyPair = keyPair, + ) + + assertNotNull( + db.unsignedNostrEventDao().getAUnsignedNostrEvents().single { it.id == unsignedId }.signedAt, + "signedAt was rolled back, so the notary would re-select this row forever", + ) + assertNotNull( + db.nostrEventDao().getNostrEventById(signed.id), + "the signed event was rolled back with the indexing failure", + ) + } + + /** The happy path of the same split, so the test above is not passing for the wrong reason. */ + @Test + fun `a published event is stored and queued for every target relay`() = runBlocking { + val unsignedId = db.unsignedNostrEventDao().upsert( + UnsignedNostrEvent( + pubKey = author, + kind = inertKind, + tags = emptyArray(), + content = "to be published", + ) + ) + val unsigned = db.unsignedNostrEventDao().getAUnsignedNostrEvents().single { it.id == unsignedId } + val signed = event("1", Instant.fromEpochSeconds(1_000), unsignedNostrEventId = unsignedId) + val relays = listOf("wss://one.example", "wss://two.example") + + db.nostrDao().publishNostrEvent( + unsignedNostrEvent = unsigned, + nostrEvent = signed, + relayURLs = relays, + activeKeyPair = keyPair, + ) + + assertNotNull(db.nostrEventDao().getNostrEventById(signed.id)) + val queued = db.broadcastNostrEventRequestDao().getAllBroadcastNostrEventRequests() + .filter { it.nostrEventId == signed.id } + assertEquals( + relays.toSet(), + queued.map { it.relayURL }.toSet(), + "a broadcast request is queued per target relay", + ) + } + + /** + * Rescheduling re-queues the broadcast and, where the event is a group message that already + * has a chat line, re-links the two. Without the relation the line has no delivery state to + * read and stays looking unsent no matter how the retry goes. + */ + @Test + fun `rescheduling relinks a broadcast to the chat line it belongs to`() = runBlocking { + val groupEventId = "e".repeat(64) + seedRoomWithChatLine(marmotGroupEventId = groupEventId) + + db.nostrDao().rescheduleBroadcastNostrEventRequests( + listOf(BroadcastNostrEventRequest(nostrEventId = groupEventId, relayURL = relay)) + ) + + val request = assertNotNull( + db.broadcastNostrEventRequestDao().getFirstBroadcastNostrEventRequestByNostrEventId(groupEventId), + "the broadcast request was not queued", + ) + val chatMessage = assertNotNull( + db.chatMessageDao().getChatMessagesByMarmotGroupEventId(groupEventId) + ) + val relation = assertNotNull( + db.chatMessageBroadcastNostrEventRequestRelationDao() + .getChatMessageBroadcastNostrEventRequestRelationByBroadcastRequest(request.id), + "the re-queued broadcast was not linked back to its chat line", + ) + assertEquals(chatMessage.id, relation.chatMessageId) + } + + /** + * The relation is conditional; the queueing is not. An event with no chat line -- anything + * that is not a group message -- must still be re-queued for broadcast. + * + * The event itself has to exist: `BroadcastNostrEventRequest.nostrEventId` is a foreign key + * onto NostrEvent, so "no chat line" is the only thing missing here. Writing this test + * against an id that was never stored fails with SQLite 787 instead, which is worth knowing + * -- a caller cannot schedule a broadcast for an event it has not saved. + */ + @Test + fun `rescheduling an event with no chat line still queues the broadcast`() = runBlocking { + val plainEventId = "9".repeat(64) + db.nostrEventDao().upsert(event(plainEventId, Instant.fromEpochSeconds(1_000))) + + db.nostrDao().rescheduleBroadcastNostrEventRequests( + listOf(BroadcastNostrEventRequest(nostrEventId = plainEventId, relayURL = relay)) + ) + + val request = assertNotNull( + db.broadcastNostrEventRequestDao().getFirstBroadcastNostrEventRequestByNostrEventId(plainEventId), + "the broadcast was not queued just because there was no chat line to link", + ) + assertNull(db.chatMessageDao().getChatMessagesByMarmotGroupEventId(plainEventId)) + assertNull( + db.chatMessageBroadcastNostrEventRequestRelationDao() + .getChatMessageBroadcastNostrEventRequestRelationByBroadcastRequest(request.id), + "no chat line means no relation should have been invented", + ) + } + + private suspend fun seedRoomWithChatLine(marmotGroupEventId: String) { + val profileEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = profileEventId, + pubKey = author, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = author, userName = "author", nostrEventId = profileEventId)) + val roomId = "b".repeat(64) + db.chatRoomDao().upsert( + ChatRoom( + id = roomId, + userPublicKey = author, + subject = null, + description = null, + mlsGroupState = null, + ) + ) + db.nostrEventDao().upsert( + event(marmotGroupEventId, Instant.fromEpochSeconds(1_000), kind = 445) + ) + db.marmotGroupEventDao().upsert( + press.mantra.compose.database.model.MarmotGroupEvent( + id = marmotGroupEventId, + userPublicKey = author, + publicKey = author, + chatRoomId = roomId, + signature = "0".repeat(128), + encryptedContent = "ciphertext", + expiresAt = null, + ) + ) + db.chatMessageDao().upsert( + press.mantra.compose.database.model.ChatMessage( + content = "a sent message", + chatRoomId = roomId, + senderPublicKey = author, + isUserMessage = true, + giftWrapPayloadId = null, + marmotGroupEventId = marmotGroupEventId, + marmotInnerEventId = null, + ) + ) + } +} From 02e70d992acd3cd8ec8e34ee8d772ecc783eb89e Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:05:51 +0200 Subject: [PATCH 05/12] test: cover the MarmotOutboundDao membership guards Both entry points that change a group's membership start by restoring the MLS state off the ChatRoom row, and a room restored from an inbound gift wrap has none -- there is nothing to add a member to. The comment on inviteMemberToChatRoom says the throw exists to "say so instead of silently doing nothing and letting the caller report success", which is a claim about behaviour and therefore something a test can hold to. A guard that returned quietly would still compile, still look like it worked, and leave a room whose members believe someone was invited who was not. Covered: inviteMemberToChatRoom and addMembersToChatRoom each raise MarmotMissingChatGroupException against a room whose mlsGroupState is null, which is exactly the shape a gift-wrap-restored room has. Covered separately, because ordering is the substance of it: a refused invite leaves no Participant row behind. The guard has to run before that write, not after. sealGiftWrapPayload walks a room's participants to decide who to wrap a Welcome for, so a participant persisted by a failed invite would make the room look like it has a member no MLS group knows about -- and the next Welcome would be sealed for them. Covered last: the empty-batch guard returns before the MLS state is looked at, so addMembersToChatRoom with no peers must *not* throw on the same stateless room the other two tests reject. Adding nobody is not a failure to add somebody, and pinning that keeps the two guards from being collapsed into one. Deliberately not covered, and the test file says so rather than implying the DAO is done: everything past the guard -- the MLS commit, the Welcome, the epoch advance and persisting it back to the room -- needs a real peer key package, which means an MLS fixture this change does not build. That gap includes the batching rationale on addMembersToChatRoom, which is the more interesting property of the two: one commit and one Welcome so no member ever has to process a commit for an epoch they were not yet in, since MarmotInboundManager refuses future-epoch messages outright with no queue and no replay. Worth covering once there is a fixture to build a key package with. The MarmotKeyPackage these tests pass carries an empty byte array, which is honest: no test here reaches the MLS layer, so the bytes only have to exist. A test that got past the guard could not use it. 4 tests. composeApp jvmTest is 258 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../database/dao/MarmotOutboundDaoJvmTest.kt | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt new file mode 100644 index 00000000..1d0ac995 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt @@ -0,0 +1,154 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.MarmotKeyPackage +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.exceptions.MarmotMissingChatGroupException +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * The membership guards on [MarmotOutboundDao]. + * + * Both entry points that change a group's membership begin by restoring the MLS state off the + * ChatRoom row, and a room restored from an inbound gift wrap has none -- there is nothing to + * add a member to. The comment on `inviteMemberToChatRoom` says the point of the throw is to + * "say so instead of silently doing nothing and letting the caller report success", which is a + * claim about behaviour and therefore testable: a guard that returned quietly would still + * compile, still look like it worked, and leave a room whose members believe someone was + * invited. + * + * These cover the paths that need no MLS material. Everything past the guard -- the commit, the + * Welcome, the epoch advance and its persistence -- needs a real peer key package to exercise, + * which means an MLS fixture this test file deliberately does not build. Those paths are worth + * covering and are not covered here. + */ +class MarmotOutboundDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val user = KeyPair().pubKey.toHexKey() + private val peer = KeyPair().pubKey.toHexKey() + private val roomId = "b".repeat(64) + + /** + * A room with `mlsGroupState = null` -- exactly the shape a room restored from an inbound + * gift wrap has, which is the case the guard exists for. + */ + private suspend fun seedStatelessRoom(): LocalChatRoom { + val nostrEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = user, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = user, userName = "user", nostrEventId = nostrEventId)) + val chatRoom = ChatRoom( + id = roomId, + userPublicKey = user, + subject = "a restored room", + description = null, + mlsGroupState = null, + ) + db.chatRoomDao().upsert(chatRoom) + return LocalChatRoom(chatRoom = chatRoom) + } + + /** + * Never decoded: the guard throws before any of these tests reach the MLS layer, so the + * bytes only have to exist. A test that got past the guard would need a real key package. + */ + private fun keyPackage() = MarmotKeyPackage( + id = "d".repeat(64), + publicKey = peer, + tlsEncodedMarmotKeyPackage = ByteArray(0), + ) + + @Test + fun `inviting into a room with no mls state is refused rather than ignored`() = runBlocking { + val localChatRoom = seedStatelessRoom() + + assertFailsWith { + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = keyPackage(), + ) + } + } + + /** + * The guard has to come before the Participant write, not after. `sealGiftWrapPayload` + * walks a room's participants to decide who to wrap a Welcome for, so a participant row + * left behind by a failed invite would make the room look like it has a member that no MLS + * group knows about. + */ + @Test + fun `a refused invite leaves no participant behind`() = runBlocking { + val localChatRoom = seedStatelessRoom() + + runCatching { + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = keyPackage(), + ) + } + + val participants = db.participantDao().findParticipantsByChatRoomId(roomId) + assertTrue( + participants.none { it.participantPublicKey == peer }, + "the invitee was persisted despite the invite being refused", + ) + } + + @Test + fun `adding members to a room with no mls state is refused`() = runBlocking { + val localChatRoom = seedStatelessRoom() + + assertFailsWith { + db.marmotOutboundDao().addMembersToChatRoom( + localChatRoom = localChatRoom, + peers = listOf(peer to keyPackage()), + ) + } + } + + /** + * The empty-batch guard returns before the MLS state is even looked at, so it must not + * throw on the same stateless room the two tests above reject. Adding nobody is not a + * failure to add somebody. + */ + @Test + fun `adding no members is not an error even without mls state`() = runBlocking { + val localChatRoom = seedStatelessRoom() + + val failed = db.marmotOutboundDao().addMembersToChatRoom( + localChatRoom = localChatRoom, + peers = emptyList(), + ) + + assertEquals(emptyList(), failed) + } +} From d355b683557cf04c83c2727cfe429cc7ecc27943 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:11:33 +0200 Subject: [PATCH 06/12] test: cover the outbound broadcast queue and its stale sweep Nothing else drains BroadcastNostrEventRequest, so a row this DAO fails to hand back is an event that never reaches any relay -- and the failure is silent, because a queue returning nothing is indistinguishable from an empty one. That already happened. The observer's predicate carried a `createdAt > :now` bound whose `now` was evaluated once, when the Flow was built. Instants persist at second resolution, so it hid every broadcast enqueued during the observer's own start second -- the entire profile-creation burst -- plus everything a previous session had left pending. There is a test here for exactly that shape: a row enqueued before the observer existed has to come back. The stale sweep, which is the other half of not losing events. A request is flipped to "processing" before a publish is attempted, and a timeout or a dropped socket leaves it there; nothing observes "processing" or "failed", so those rows are dead weight until the sweep requeues them. Covered: both stale statuses flip to "pending" and are counted; a row already pending is not touched, so the returned count is not inflated by work that was never stale. Covered separately, because it is the reason the sweep is bounded at all: a row newer than the cutoff is left alone. A publish running right now holds its row in "processing", and requeueing that would hand the same event to a second publish while the first is still in flight. The bound is `<=`, so a row stamped exactly on the cutoff second is swept -- asserted, since that is the boundary the second resolution of these timestamps makes common rather than rare. Also covered: the queue drains oldest first; a "processing" row is not handed out as pending work; and getFirstBroadcastNostrEventRequestByNostrEventId returns the oldest of an event's per-relay rows rather than the only one, since an event is queued once per target relay. One test is deliberately kept despite not being able to fail, and says so in its own comment. `requests sharing a timestamp drain in insertion order` pins the observable order of a same-second burst, which is what callers depend on -- but deleting the `, id ASC` tiebreak leaves it passing, because `id` is an autoGenerate primary key and therefore the rowid, so sqlite's unspecified ordering already coincides with it under this plan. That coincidence is the argument for keeping the explicit tiebreak rather than against it: it is not contractual, and an index or a different plan can change it. Recording the limit in the test seemed better than implying a guard that is not there. Verified by mutation: reversing the drain order fails the oldest-first test. Removing only the tiebreak fails nothing, which is how the limitation above was found rather than assumed. Both mutations were reverted; no production source is touched by this commit. 9 tests. Co-Authored-By: Claude Opus 5 --- .../BroadcastNostrEventRequestDaoJvmTest.kt | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/BroadcastNostrEventRequestDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/BroadcastNostrEventRequestDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/BroadcastNostrEventRequestDaoJvmTest.kt new file mode 100644 index 00000000..8e32bbe3 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/BroadcastNostrEventRequestDaoJvmTest.kt @@ -0,0 +1,201 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.BroadcastNostrEventRequest +import press.mantra.compose.database.model.NostrEvent +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.time.Instant + +/** + * The outbound queue. Nothing else drains this table, so a row it fails to hand back is an + * event that is never sent to any relay -- and the failure is silent, because a queue that + * returns nothing looks exactly like a queue that is empty. + * + * That is not hypothetical here. The observer's predicate used to carry a `createdAt > :now` + * bound whose `now` was evaluated once, when the Flow was built. Instants persist at second + * resolution, so it hid every broadcast enqueued during the observer's own start second -- the + * whole profile-creation burst -- along with everything a previous session had left pending. + */ +class BroadcastNostrEventRequestDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val author = "a".repeat(64) + + private suspend fun queue( + eventId: String, + status: String = "pending", + createdAt: Instant = Instant.fromEpochSeconds(1_000), + relayURL: String = "wss://relay.example", + ): Long { + val id = eventId.padEnd(64, '0') + // BroadcastNostrEventRequest.nostrEventId is a foreign key onto NostrEvent: a broadcast + // cannot be queued for an event that was never stored. + if (db.nostrEventDao().getNostrEventById(id) == null) { + db.nostrEventDao().upsert( + NostrEvent( + id = id, + pubKey = author, + kind = 1, + tags = emptyArray(), + content = "outbound", + sig = "0".repeat(128), + ) + ) + } + return db.broadcastNostrEventRequestDao().upsert( + BroadcastNostrEventRequest( + nostrEventId = id, + relayURL = relayURL, + status = status, + createdAt = createdAt, + ) + ) + } + + private suspend fun statusOf(requestId: Long): String = + db.broadcastNostrEventRequestDao().getAllBroadcastNostrEventRequests() + .single { it.id == requestId }.status + + private suspend fun pendingHead() = + db.broadcastNostrEventRequestDao().observeBroadcastNostrEventRequestsByStatus("pending").first() + + /** + * A request is flipped to "processing" before the publish is attempted, and a timeout or a + * dropped socket leaves it there. Nothing observes "processing" or "failed", so without + * this sweep those rows are dead weight and the events never go out. + */ + @Test + fun `an interrupted publish is requeued on startup`() = runBlocking { + val processing = queue("1", status = "processing") + val failed = queue("2", status = "failed") + + val changed = db.broadcastNostrEventRequestDao() + .requeueStaleBroadcastNostrEventRequests(Instant.fromEpochSeconds(2_000)) + + assertEquals(2, changed, "both stale rows should have been requeued") + assertEquals("pending", statusOf(processing)) + assertEquals("pending", statusOf(failed)) + } + + /** A row already pending is not stale; touching it would inflate the reported count. */ + @Test + fun `a pending request is left alone by the sweep`() = runBlocking { + queue("1", status = "pending") + + val changed = db.broadcastNostrEventRequestDao() + .requeueStaleBroadcastNostrEventRequests(Instant.fromEpochSeconds(2_000)) + + assertEquals(0, changed) + } + + /** + * The reason the sweep is bounded at all. A publish running right now holds its row in + * "processing"; requeueing that would hand the same event to a second publish while the + * first is still in flight. + */ + @Test + fun `a request newer than the cutoff is left for the process that owns it`() = runBlocking { + val inFlight = queue("1", status = "processing", createdAt = Instant.fromEpochSeconds(3_000)) + + val changed = db.broadcastNostrEventRequestDao() + .requeueStaleBroadcastNostrEventRequests(Instant.fromEpochSeconds(2_000)) + + assertEquals(0, changed) + assertEquals("processing", statusOf(inFlight)) + } + + /** The bound is `<=`, so a row stamped exactly on the cutoff second is swept. */ + @Test + fun `a request stamped exactly on the cutoff is requeued`() = runBlocking { + val onCutoff = queue("1", status = "processing", createdAt = Instant.fromEpochSeconds(2_000)) + + val changed = db.broadcastNostrEventRequestDao() + .requeueStaleBroadcastNostrEventRequests(Instant.fromEpochSeconds(2_000)) + + assertEquals(1, changed) + assertEquals("pending", statusOf(onCutoff)) + } + + /** + * The regression the observer's comment describes. A row enqueued before the observer was + * built -- a previous session's leftovers -- has to come back. The old bound compared + * against a `now` captured when the Flow was created, so it returned nothing here and the + * queue looked empty forever. + */ + @Test + fun `the queue hands back work enqueued before the observer existed`() = runBlocking { + val old = queue("1", createdAt = Instant.fromEpochSeconds(1_000)) + + val head = assertNotNull(pendingHead(), "a previous session's pending row was not returned") + + assertEquals(old, head.broadcastNostrEventRequest.id) + } + + /** Oldest first: the queue drains in the order things were enqueued. */ + @Test + fun `the queue hands back the oldest pending request first`() = runBlocking { + queue("2", createdAt = Instant.fromEpochSeconds(3_000)) + val oldest = queue("1", createdAt = Instant.fromEpochSeconds(1_000)) + + assertEquals(oldest, assertNotNull(pendingHead()).broadcastNostrEventRequest.id) + } + + /** + * Instants persist at second resolution, so a burst enqueued in one second is all one + * timestamp, and the head of the queue is decided by the `id ASC` tiebreak. + * + * Note what this test does and does not do. It pins the observable drain order, which is + * what a caller depends on. It cannot fail if the tiebreak is deleted: `id` is an + * autoGenerate primary key and therefore the rowid, so sqlite's own unspecified ordering + * already coincides with it under this plan -- removing `, id ASC` leaves every assertion + * here passing. That coincidence is exactly why the explicit tiebreak is worth keeping: + * it is not contractual, and an index or a different query plan can change it. + */ + @Test + fun `requests sharing a timestamp drain in insertion order`() = runBlocking { + val sameSecond = Instant.fromEpochSeconds(1_000) + val first = queue("1", createdAt = sameSecond) + queue("2", createdAt = sameSecond) + queue("3", createdAt = sameSecond) + + assertEquals(first, assertNotNull(pendingHead()).broadcastNostrEventRequest.id) + } + + @Test + fun `the queue is empty when nothing is pending`() = runBlocking { + queue("1", status = "processing") + + assertNull(pendingHead(), "a processing row must not be handed out as pending work") + } + + /** + * One event is queued once per target relay, so this returns the oldest of several rows + * rather than the only one. + */ + @Test + fun `the first request for an event is the oldest of its per-relay rows`() = runBlocking { + val eventId = "1".padEnd(64, '0') + val oldest = queue("1", createdAt = Instant.fromEpochSeconds(1_000), relayURL = "wss://one.example") + queue("1", createdAt = Instant.fromEpochSeconds(2_000), relayURL = "wss://two.example") + + val found = assertNotNull( + db.broadcastNostrEventRequestDao().getFirstBroadcastNostrEventRequestByNostrEventId(eventId) + ) + + assertEquals(oldest, found.id) + } +} From 521a4f5690cf30d5633008301d4f75ef286c4e6c Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:11:58 +0200 Subject: [PATCH 07/12] test: pin the epoch secret retention window A retained epoch secret is what lets a member read a message sent under an epoch the group has since moved past. Both ways of getting the policy wrong are quiet: keep too few and old messages become permanently unreadable, keep too many and secrets that should have been dropped stay on disk. The entire policy is one strict `<` in a query and an IGNORE on an insert. The cutoff is strict, and that matters more than an off-by-one usually does. This query feeds a delete, so an epoch wrongly reported as droppable is not a stale read -- it is the messages of that epoch becoming undecryptable, with nothing to recover them from. Covered with three epochs either side of the boundary: only strictly older ones are droppable, the epoch equal to the cutoff is still inside the window, and a cutoff at or below every retained epoch drops nothing. Room scoping, for the same reason. Rooms advance epochs independently, so a sweep driven by one room's cutoff must never reach another's -- a leak here costs the other room its history. Asserted from both ends: the sweep returns only the sweeping room's rows, and the other room's secret is still there afterwards. Insert is IGNORE over the composite key (chatRoomId, epoch), which is what makes re-processing a commit safe. A redelivery or a replay re-derives the secret, and overwriting the stored one with that re-derivation would replace the value that actually decrypts the messages already on disk. Covered by inserting a second, different secret for the same epoch and asserting the first survives -- and alongside it, that the same epoch number in two different rooms is two rows rather than a conflict, since the composite key is what separates them. Also covered: defenestrate removes exactly the rows the sweep selected and leaves the rest, and a room's retained epochs are all readable back, which is what a rejoin or a full replay reads before deciding what it can still decrypt. Verified by mutation: relaxing the cutoff to `epoch <= :epochCutOffPoint` fails three of these, including the boundary test. The mutation was reverted; no production source is touched by this commit. 7 tests. Co-Authored-By: Claude Opus 5 --- .../MarmotRetainedEpochSecretDaoJvmTest.kt | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotRetainedEpochSecretDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotRetainedEpochSecretDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotRetainedEpochSecretDaoJvmTest.kt new file mode 100644 index 00000000..6ddc8527 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotRetainedEpochSecretDaoJvmTest.kt @@ -0,0 +1,202 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.MarmotRetainedEpochSecret +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Retained epoch secrets are what lets a member read a message sent under an epoch the group + * has since moved past. Both directions of getting this wrong are bad and neither shows up as + * an error: keep too few and old messages become permanently unreadable, keep too many and + * secrets that should have been dropped stay on disk. + * + * The whole policy is one strict `<` in a query and an IGNORE on an insert, and nothing else + * checks either. + */ +class MarmotRetainedEpochSecretDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val user = "a".repeat(64) + private val roomOne = "11".repeat(32) + private val roomTwo = "22".repeat(32) + + private suspend fun seedRooms() { + val nostrEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = user, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = user, userName = "user", nostrEventId = nostrEventId)) + listOf(roomOne, roomTwo).forEach { id -> + db.chatRoomDao().upsert( + ChatRoom( + id = id, + userPublicKey = user, + subject = null, + description = null, + mlsGroupState = null, + ) + ) + } + } + + private suspend fun retain( + epoch: Long, + chatRoomId: String = roomOne, + secret: Byte = 1, + ) = MarmotRetainedEpochSecret( + chatRoomId = chatRoomId, + epoch = epoch, + senderDataSecret = byteArrayOf(secret), + encryptionSecret = byteArrayOf(secret), + leafCount = 2, + ).also { db.marmotRetainedEpochSecretDao().insert(it) } + + private suspend fun defenestratable(cutoff: Long, chatRoomId: String = roomOne) = + db.marmotRetainedEpochSecretDao() + .getDefenestratableMarmotRetainedEpochSecretForChatRoomId(chatRoomId, cutoff) + + /** + * The cutoff is strict. An epoch equal to it is still inside the retention window and + * dropping it makes every message sent under that epoch unreadable -- an off-by-one here + * destroys data rather than merely wasting space. + */ + @Test + fun `the epoch on the cutoff is kept and only older ones can be dropped`() = runBlocking { + seedRooms() + retain(epoch = 3) + retain(epoch = 4) + retain(epoch = 5) + + val droppable = defenestratable(cutoff = 4).map { it.epoch }.toSet() + + assertEquals(setOf(3L), droppable, "only epochs strictly below the cutoff are droppable") + assertTrue(4L !in droppable, "the epoch on the cutoff is still within the window") + assertTrue(5L !in droppable) + } + + @Test + fun `nothing is droppable when the cutoff precedes every retained epoch`() = runBlocking { + seedRooms() + retain(epoch = 7) + + assertTrue(defenestratable(cutoff = 7).isEmpty()) + assertTrue(defenestratable(cutoff = 0).isEmpty()) + } + + /** + * Rooms advance epochs independently, so a sweep driven by one room's cutoff must never + * reach another room's secrets. This query feeds a delete, so a leak here is not a stale + * read -- it is another room losing its history. + */ + @Test + fun `one rooms cutoff never reaches another rooms secrets`() = runBlocking { + seedRooms() + retain(epoch = 1, chatRoomId = roomOne) + retain(epoch = 1, chatRoomId = roomTwo) + + val droppable = defenestratable(cutoff = 9, chatRoomId = roomOne) + + assertEquals(1, droppable.size) + assertTrue(droppable.all { it.chatRoomId == roomOne }) + assertEquals( + 1, + db.marmotRetainedEpochSecretDao().getMarmotRetainedEpochSecretForChatRoomId(roomTwo).size, + "the other room's secret must be untouched", + ) + } + + /** + * Insert is IGNORE over the composite key (chatRoomId, epoch). Re-processing a commit -- + * a redelivery, or a replay -- must not overwrite the retained secret with a + * re-derivation, because the stored one is what actually decrypts the messages already on + * disk. + */ + @Test + fun `re-retaining an epoch keeps the secret already stored`() = runBlocking { + seedRooms() + retain(epoch = 1, secret = 1) + + retain(epoch = 1, secret = 9) + + val stored = db.marmotRetainedEpochSecretDao() + .getMarmotRetainedEpochSecretForChatRoomId(roomOne) + assertEquals(1, stored.size, "the composite key should have kept this to one row") + assertContentEquals( + byteArrayOf(1), + stored.single().encryptionSecret, + "the original secret was overwritten by a re-derivation", + ) + } + + /** The same epoch number in two rooms is two different secrets, not a conflict. */ + @Test + fun `the same epoch in two rooms is retained separately`() = runBlocking { + seedRooms() + retain(epoch = 1, chatRoomId = roomOne, secret = 1) + retain(epoch = 1, chatRoomId = roomTwo, secret = 2) + + assertContentEquals( + byteArrayOf(1), + db.marmotRetainedEpochSecretDao() + .getMarmotRetainedEpochSecretForChatRoomId(roomOne).single().encryptionSecret, + ) + assertContentEquals( + byteArrayOf(2), + db.marmotRetainedEpochSecretDao() + .getMarmotRetainedEpochSecretForChatRoomId(roomTwo).single().encryptionSecret, + ) + } + + /** Defenestration removes what the sweep selected and nothing else. */ + @Test + fun `defenestrating drops only the rows handed to it`() = runBlocking { + seedRooms() + retain(epoch = 1) + retain(epoch = 2) + val kept = retain(epoch = 3) + + db.marmotRetainedEpochSecretDao().defenestrate(defenestratable(cutoff = 3)) + + val remaining = db.marmotRetainedEpochSecretDao() + .getMarmotRetainedEpochSecretForChatRoomId(roomOne) + assertEquals(listOf(kept.epoch), remaining.map { it.epoch }) + } + + /** + * The room's secrets are reachable as a set, which is what a rejoin or a full replay reads + * before deciding what it can still decrypt. + */ + @Test + fun `a rooms retained epochs are all readable`() = runBlocking { + seedRooms() + retain(epoch = 1) + retain(epoch = 2) + + val all = db.marmotRetainedEpochSecretDao().getMarmotRetainedEpochSecretForChatRoomId(roomOne) + + assertEquals(setOf(1L, 2L), all.map { it.epoch }.toSet()) + } +} From baecb76253912aeddc8de3ff177746cb06a35d07 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:11:58 +0200 Subject: [PATCH 08/12] test: cover the query a marmot reindex decides its work from getResolvedMarmotGroupEventIds is what a reindex sweep subtracts from a room's stored group events to decide what to replay, so its answer decides what work the sweep does -- and both ways of being wrong are silent. Report an event as resolved when it is not, and the replay skips the one event that needed it: the message stays missing from the feed with nothing left to trigger another attempt. Report it as unresolved when it is resolved, and every sweep re-decrypts it forever. The whole distinction rests on `messageType NOT IN (:unresolvedTypes)`, where those types are the two placeholder lines that stand in for a message still to come rather than reporting one. Covered: an event with a real line is resolved; an undecryptable outer layer and a pending commit each leave their event unresolved, which is right because those are precisely what a replay exists to retry. Then the subtraction itself, since that is how the caller uses it -- three events, one settled, one holding a placeholder, one with no line at all, and the sweep left with exactly the last two. Covered because the query says so and nothing else would: `marmotGroupEventId IS NOT NULL` keeps out lines that are not about a group event -- a NIP-17 direct message, a locally written line -- which would otherwise carry nulls into a set the sweep subtracts with. And the room scoping, since a sweep runs per room and another room's resolutions must not shorten its work. Covered last, and it is the transition the sweep exists to cause: a placeholder upserted in place into a real line resolves its event, visible through this same query. Two smaller ones alongside: the single-row lookups order newest first, which is what makes them "the line for this event" rather than whichever row sqlite reached first, and the per-sender count is scoped to its room. Verified by mutation: defeating the messageType exclusion so placeholders count as resolved fails five of these, including the subtraction test. The mutation was reverted; no production source is touched by this commit. 8 tests. composeApp jvmTest is 282 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../database/dao/ChatMessageDaoJvmTest.kt | 241 ++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/ChatMessageDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/ChatMessageDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/ChatMessageDaoJvmTest.kt new file mode 100644 index 00000000..7ab681ab --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/ChatMessageDaoJvmTest.kt @@ -0,0 +1,241 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import kotlinx.coroutines.runBlocking +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.MarmotGroupEvent +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue +import kotlin.time.Instant + +/** + * `getResolvedMarmotGroupEventIds` is the query a reindex sweep subtracts from the room's + * stored group events to decide what to replay, so its answer decides what work the sweep does. + * Both ways of being wrong are quiet. Report an event as resolved when it is not and the replay + * skips the one event that needed it -- the message stays missing from the feed with nothing + * left to trigger another attempt. Report it as unresolved when it is resolved and every sweep + * re-decrypts it forever. + * + * The distinction rests entirely on `messageType NOT IN (:unresolvedTypes)`, where the + * unresolved types are the two placeholder lines that stand in for a message still to come + * rather than reporting one. + */ +class ChatMessageDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val user = "a".repeat(64) + private val other = "b".repeat(64) + private val roomOne = "11".repeat(32) + private val roomTwo = "22".repeat(32) + + private suspend fun seedRooms() { + val nostrEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = user, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = user, userName = "user", nostrEventId = nostrEventId)) + listOf(roomOne, roomTwo).forEach { id -> + db.chatRoomDao().upsert( + ChatRoom( + id = id, + userPublicKey = user, + subject = null, + description = null, + mlsGroupState = null, + ) + ) + } + } + + /** A stored kind:445 plus its indexed row, which a chat line's foreign key hangs off. */ + private suspend fun seedGroupEvent(id: String, chatRoomId: String = roomOne): String { + val eventId = id.padEnd(64, '0') + db.nostrEventDao().upsert( + NostrEvent( + id = eventId, + pubKey = user, + kind = MarmotGroupEvent.KIND, + tags = arrayOf(arrayOf("h", chatRoomId)), + content = "ciphertext", + sig = "0".repeat(128), + ) + ) + db.marmotGroupEventDao().upsert( + MarmotGroupEvent( + id = eventId, + userPublicKey = user, + publicKey = user, + chatRoomId = chatRoomId, + signature = "0".repeat(128), + encryptedContent = "ciphertext", + expiresAt = null, + ) + ) + return eventId + } + + private suspend fun line( + groupEventId: String?, + messageType: String = ChatMessage.TYPE_DIRECT_MESSAGE, + chatRoomId: String = roomOne, + content: String = "a line", + sender: String = user, + createdAt: Instant = Instant.fromEpochSeconds(1_000), + ): Long = db.chatMessageDao().upsert( + ChatMessage( + content = content, + chatRoomId = chatRoomId, + senderPublicKey = sender, + isUserMessage = sender == user, + giftWrapPayloadId = null, + marmotGroupEventId = groupEventId, + marmotInnerEventId = null, + messageType = messageType, + createdAt = createdAt, + ) + ) + + private suspend fun resolved(chatRoomId: String = roomOne) = + db.chatMessageDao().getResolvedMarmotGroupEventIds( + chatRoomId = chatRoomId, + unresolvedTypes = ChatMessage.UNRESOLVED_MARMOT_TYPES, + ) + + @Test + fun `an event with a real line is resolved`() = runBlocking { + seedRooms() + val eventId = seedGroupEvent("1") + line(eventId) + + assertEquals(listOf(eventId), resolved()) + } + + /** + * The two placeholder types. An undecryptable outer layer stands in for a message that + * could not be opened yet, and a pending commit for one whose commit has not arrived -- + * both are exactly what a replay exists to retry, so neither may count as resolved. + */ + @Test + fun `a placeholder line leaves its event unresolved`() = runBlocking { + seedRooms() + val undecryptable = seedGroupEvent("1") + val pendingCommit = seedGroupEvent("2") + line(undecryptable, messageType = ChatMessage.TYPE_UNDECRYPTABLE_OUTER_LAYER) + line(pendingCommit, messageType = ChatMessage.TYPE_PENDING_COMMIT) + + val found = resolved() + + assertTrue(undecryptable !in found, "an undecryptable placeholder is not a resolution") + assertTrue(pendingCommit !in found, "a pending commit is not a resolution") + assertTrue(found.isEmpty()) + } + + /** The sweep's subtraction: only the placeholder-backed event is left to replay. */ + @Test + fun `a replay is left with exactly the events that still have nothing to show`() = runBlocking { + seedRooms() + val settled = seedGroupEvent("1") + val stillWaiting = seedGroupEvent("2") + val neverSeen = seedGroupEvent("3") + line(settled) + line(stillWaiting, messageType = ChatMessage.TYPE_PENDING_COMMIT) + + val unresolved = listOf(settled, stillWaiting, neverSeen) - resolved().toSet() + + assertEquals(listOf(stillWaiting, neverSeen), unresolved) + } + + /** + * Lines that are not about a group event -- a NIP-17 direct message, a locally written + * line -- carry a null marmotGroupEventId, and `IS NOT NULL` keeps them out. Without it + * the result would carry nulls into a set the sweep subtracts with. + */ + @Test + fun `lines with no group event are not reported as resolutions`() = runBlocking { + seedRooms() + line(groupEventId = null) + + assertTrue(resolved().isEmpty()) + } + + /** A sweep runs per room, so another room's resolved lines must not shorten its work. */ + @Test + fun `resolutions are scoped to their own room`() = runBlocking { + seedRooms() + val mine = seedGroupEvent("1", chatRoomId = roomOne) + val theirs = seedGroupEvent("2", chatRoomId = roomTwo) + line(mine, chatRoomId = roomOne) + line(theirs, chatRoomId = roomTwo) + + assertEquals(listOf(mine), resolved(roomOne)) + assertEquals(listOf(theirs), resolved(roomTwo)) + } + + /** + * A placeholder replaced by a real line resolves the event. This is the transition the + * sweep is trying to cause, so it has to be visible through this query -- the row is + * upserted in place, keeping its id. + */ + @Test + fun `a placeholder that becomes a real line resolves its event`() = runBlocking { + seedRooms() + val eventId = seedGroupEvent("1") + val lineId = line(eventId, messageType = ChatMessage.TYPE_PENDING_COMMIT) + assertTrue(resolved().isEmpty(), "precondition: the placeholder is unresolved") + + val placeholder = assertNotNull(db.chatMessageDao().getChatMessagesByMarmotGroupEventId(eventId)) + db.chatMessageDao().upsert( + placeholder.copy(id = lineId, messageType = ChatMessage.TYPE_DIRECT_MESSAGE, content = "decrypted") + ) + + assertEquals(listOf(eventId), resolved()) + } + + /** + * The single-row lookups order newest first, which is what makes them a sensible "the line + * for this event" rather than whichever row sqlite reached first. + */ + @Test + fun `the newest line wins for a group event`() = runBlocking { + seedRooms() + val eventId = seedGroupEvent("1") + line(eventId, content = "older", createdAt = Instant.fromEpochSeconds(1_000)) + line(eventId, content = "newer", createdAt = Instant.fromEpochSeconds(2_000)) + + assertEquals("newer", db.chatMessageDao().getChatMessagesByMarmotGroupEventId(eventId)?.content) + } + + @Test + fun `a senders lines are counted per room`() = runBlocking { + seedRooms() + line(groupEventId = null, sender = user, chatRoomId = roomOne) + line(groupEventId = null, sender = user, chatRoomId = roomOne) + line(groupEventId = null, sender = other, chatRoomId = roomOne) + line(groupEventId = null, sender = user, chatRoomId = roomTwo) + + assertEquals(2, db.chatMessageDao().countChatMessagesBySenderPublicKey(roomOne, user)) + assertEquals(1, db.chatMessageDao().countChatMessagesBySenderPublicKey(roomOne, other)) + assertEquals(0, db.chatMessageDao().countChatMessagesBySenderPublicKey(roomTwo, other)) + } +} From 168d16c9330f806ddc49a606407b4d8da5ec5268 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:24:49 +0200 Subject: [PATCH 09/12] test: cover the DKG ritual state a ceremony is resumed from Two properties here decide whether a ceremony can finish, and the compiler sees neither. A participant gets one message per round, and that is enforced by the composite key (sessionId, participantPublicKey, kind) rather than by any code that writes to the table. Rounds advance on countMessagesByKind reaching the participant count, so a redelivered message that added a row instead of replacing one would let the count reach the threshold while a member had still never been heard from -- and the ritual would proceed on a participant set it never assembled. Covered by resending a participant's message with a different payload and asserting the count stays at one and the payload is the newer of the two, and separately by writing the same participant into two different rounds and asserting neither overwrites the other. A key-holding session needs both halves. thresholdPublicKey without secretShare is a ceremony that produced a group key this device cannot sign against; secretShare without thresholdPublicKey is a share with no key to sign for. Either alone is a failed ceremony, and offering it up as a signing key means attempting to sign with half a result. Covered with all four combinations present in the table at once, asserting only the complete one comes back. Also covered: the live ritual for a room is the newest, because a group may have abandoned earlier attempts and a resume that picked up an abandoned one would wait forever on participants who have moved to the newer; rituals belonging to another room are not offered as this room's; key-holding sessions come back newest first; and messages are counted per session and per round rather than across either. And the ordering, which is the one with a reason beyond tidiness: a round's messages come back ordered by participant public key, not by arrival. Every device has to assemble a round in the same order to compute the same thing, and arrival order is per-device. The test writes three participants in an order deliberately unlike the sorted one. Real secp256k1 keys throughout rather than hex filler, since these are the values a canonical ordering is defined over. Verified by mutation: relaxing the key-holding predicate to `OR` returns all three of the incomplete sessions and fails that test; reordering the round query by createdAt fails the canonical-order test. Both mutations were reverted; no production source is touched by this commit. 8 tests. Co-Authored-By: Claude Opus 5 --- .../database/dao/DkgSessionDaoJvmTest.kt | 245 ++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/DkgSessionDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/DkgSessionDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/DkgSessionDaoJvmTest.kt new file mode 100644 index 00000000..8ab8e9b4 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/DkgSessionDaoJvmTest.kt @@ -0,0 +1,245 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgParticipantMessage +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.types.DkgRitualStage +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.time.Instant + +/** + * The state a DKG ritual is resumed from. Two properties here decide whether a ceremony can + * finish, and neither is visible to the compiler. + * + * The first is that a participant gets one message per round, enforced by the composite key + * (sessionId, participantPublicKey, kind) rather than by any code that writes to it. Rounds + * advance on `countMessagesByKind` reaching the participant count, so if a redelivered message + * added a second row instead of replacing the first, the count would reach the threshold with + * fewer real participants than the ritual requires -- and the ritual would proceed on a set it + * never actually assembled. + * + * The second is that a key-holding session needs *both* the threshold public key and the secret + * share. A ceremony that stored one and not the other is a failed ceremony, and offering it as + * a signing key would mean attempting to sign with half a result. + */ +class DkgSessionDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val user = KeyPair().pubKey.toHexKey() + private val alice = KeyPair().pubKey.toHexKey() + private val bob = KeyPair().pubKey.toHexKey() + private val roomOne = "11".repeat(32) + private val roomTwo = "22".repeat(32) + + private val hostKeyKind = 1 + private val round1Kind = 2 + + private suspend fun seedRooms() { + val nostrEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = user, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = user, userName = "user", nostrEventId = nostrEventId)) + listOf(roomOne, roomTwo).forEach { id -> + db.chatRoomDao().upsert( + ChatRoom( + id = id, + userPublicKey = user, + subject = null, + description = null, + mlsGroupState = null, + ) + ) + } + } + + private suspend fun session( + id: String, + chatRoomId: String = roomOne, + createdAt: Instant = Instant.fromEpochSeconds(1_000), + thresholdPublicKey: String? = null, + secretShare: String? = null, + stage: DkgRitualStage = DkgRitualStage.COLLECTING_HOST_KEYS, + ): DkgSession = DkgSession( + id = id, + chatRoomId = chatRoomId, + coordinatorPublicKey = user, + userPublicKey = user, + threshold = 2, + participantCount = 3, + stage = stage, + hostPublicKey = user, + round1Random = "aa".repeat(32), + round2AuxRandom = "bb".repeat(32), + thresholdPublicKey = thresholdPublicKey, + secretShare = secretShare, + createdAt = createdAt, + ).also { db.dkgSessionDao().upsert(it) } + + private suspend fun message( + sessionId: String, + participant: String, + kind: Int = hostKeyKind, + payload: String = "aa".repeat(32), + createdAt: Instant = Instant.fromEpochSeconds(1_000), + ) = db.dkgSessionDao().upsert( + DkgParticipantMessage( + sessionId = sessionId, + participantPublicKey = participant, + kind = kind, + payload = payload, + createdAt = createdAt, + ) + ) + + /** + * "A group may have abandoned earlier attempts; the live one is the most recent." A resume + * that picked up an abandoned ritual would wait forever on participants who have moved on + * to the newer one. + */ + @Test + fun `the live ritual for a room is the newest one`() = runBlocking { + seedRooms() + session("abandoned", createdAt = Instant.fromEpochSeconds(1_000)) + session("live", createdAt = Instant.fromEpochSeconds(2_000)) + + assertEquals("live", db.dkgSessionDao().getLatestSessionForChatRoom(roomOne)?.id) + } + + @Test + fun `rituals in another room are not offered as this rooms`() = runBlocking { + seedRooms() + session("theirs", chatRoomId = roomTwo, createdAt = Instant.fromEpochSeconds(2_000)) + session("mine", chatRoomId = roomOne, createdAt = Instant.fromEpochSeconds(1_000)) + + assertEquals("mine", db.dkgSessionDao().getLatestSessionForChatRoom(roomOne)?.id) + assertNull(db.dkgSessionDao().getLatestSessionForChatRoom("33".repeat(32))) + } + + /** + * Half a ceremony is not a key. Both columns have to be present, because a session holding + * only a threshold public key never derived a share to sign with, and one holding only a + * share has no group key to sign against. + */ + @Test + fun `only a ceremony that produced both halves counts as key holding`() = runBlocking { + seedRooms() + session("complete", thresholdPublicKey = "dd".repeat(32), secretShare = "ee".repeat(32)) + session("keyOnly", thresholdPublicKey = "dd".repeat(32), secretShare = null) + session("shareOnly", thresholdPublicKey = null, secretShare = "ee".repeat(32)) + session("neither") + + val holding = db.dkgSessionDao().getKeyHoldingSessions().map { it.id } + + assertEquals(listOf("complete"), holding) + } + + /** Newest first, so the most recent ceremony's key is the one reached for. */ + @Test + fun `key holding sessions come back newest first`() = runBlocking { + seedRooms() + session("older", createdAt = Instant.fromEpochSeconds(1_000), thresholdPublicKey = "dd".repeat(32), secretShare = "ee".repeat(32)) + session("newer", createdAt = Instant.fromEpochSeconds(2_000), thresholdPublicKey = "dd".repeat(32), secretShare = "ee".repeat(32)) + + assertEquals(listOf("newer", "older"), db.dkgSessionDao().getKeyHoldingSessions().map { it.id }) + } + + /** + * The property the round counter depends on. A relay redelivers, and a participant may + * resend; either way the composite key means the row is replaced, not added. If it were + * added, `countMessagesByKind` would reach the participant count while one member had + * still never been heard from. + */ + @Test + fun `a resent round message replaces the participants earlier one`() = runBlocking { + seedRooms() + session("s1") + message("s1", alice, payload = "aa".repeat(32)) + + message("s1", alice, payload = "bb".repeat(32)) + + assertEquals(1, db.dkgSessionDao().countMessagesByKind("s1", hostKeyKind)) + assertEquals( + "bb".repeat(32), + db.dkgSessionDao().getMessage("s1", hostKeyKind, alice)?.payload, + "the resent payload should have replaced the earlier one", + ) + } + + /** One participant can hold a message in each round without either replacing the other. */ + @Test + fun `the same participant holds one message per round`() = runBlocking { + seedRooms() + session("s1") + message("s1", alice, kind = hostKeyKind, payload = "aa".repeat(32)) + message("s1", alice, kind = round1Kind, payload = "cc".repeat(32)) + + assertEquals(1, db.dkgSessionDao().countMessagesByKind("s1", hostKeyKind)) + assertEquals(1, db.dkgSessionDao().countMessagesByKind("s1", round1Kind)) + assertEquals( + "aa".repeat(32), + db.dkgSessionDao().getMessage("s1", hostKeyKind, alice)?.payload, + "the round-1 message overwrote the host key message", + ) + } + + /** + * Ordered by participant public key, not by arrival. Every device has to assemble a round + * in the same order to compute the same thing, and arrival order differs per device. + */ + @Test + fun `a rounds messages are ordered by participant rather than by arrival`() = runBlocking { + seedRooms() + session("s1") + val ordered = listOf(user, alice, bob).sorted() + // Written in an order deliberately unlike the sorted one. + message("s1", ordered[2], createdAt = Instant.fromEpochSeconds(1_000)) + message("s1", ordered[0], createdAt = Instant.fromEpochSeconds(2_000)) + message("s1", ordered[1], createdAt = Instant.fromEpochSeconds(3_000)) + + val found = db.dkgSessionDao().getMessagesByKind("s1", hostKeyKind).map { it.participantPublicKey } + + assertEquals(ordered, found, "a round must assemble in canonical participant order") + } + + /** Rounds and sessions are counted apart, which is what makes the counter a round gate. */ + @Test + fun `messages are counted per session and per round`() = runBlocking { + seedRooms() + session("s1") + session("s2") + message("s1", alice, kind = hostKeyKind) + message("s1", bob, kind = hostKeyKind) + message("s1", alice, kind = round1Kind) + message("s2", alice, kind = hostKeyKind) + + assertEquals(2, db.dkgSessionDao().countMessagesByKind("s1", hostKeyKind)) + assertEquals(1, db.dkgSessionDao().countMessagesByKind("s1", round1Kind)) + assertEquals(1, db.dkgSessionDao().countMessagesByKind("s2", hostKeyKind)) + assertNull(db.dkgSessionDao().getMessage("s2", round1Kind, alice)) + } +} From 8a6cb81bf919bfc0604dbb8d6f0fd9d0d06596b2 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:25:01 +0200 Subject: [PATCH 10/12] test: cover the FROST signing session state The signing counterpart to the DKG coverage, and it differs in the way the DAO's own comment gives: "unlike a DKG a group signs repeatedly, so there is no single current one to observe". Sessions accumulate rather than replacing each other, which makes room scoping and ordering load-bearing rather than incidental. The duplicate-suppression property is the same and matters for the same reason. The composite key (sessionId, signerPublicKey, kind) is what makes a redelivered nonce or partial signature replace its predecessor rather than add a row, and countMessagesByKind is what decides that enough signers have answered. A second row for one signer lets a session cross its threshold while short a real participant, and the aggregation then runs over a signer set that was never assembled. Covered by resending a nonce with a different payload, and separately by giving one signer both a nonce and a partial signature and asserting the second does not overwrite the first -- the kind in the key is the only thing keeping those apart. Also covered: a session reads back by id with its stage intact and a missing id gives null; a room's sessions accumulate newest first, with the latest reachable on its own; sessions are scoped to their room, which matters because signing happens in the #admins room and a device can be in more than one -- a session leaking across would have a signer answering a request its group never made; counts are per session and per round; and a completed session keeps its signature, which is what a resume reads to avoid signing the same event twice. One test records a difference rather than a guarantee. FROST orders a round's messages by createdAt where the DKG orders the same query by participant public key. Arrival order is per-device, so this ordering is not canonical across the group the way the DKG's is. It is pinned as it stands rather than asserted to be right: whether it is deliberate is not something this change can settle, and a caller that needs a canonical signer order has to impose one itself. Worth looking at separately. 8 tests. Co-Authored-By: Claude Opus 5 --- .../dao/FrostSigningSessionDaoJvmTest.kt | 247 ++++++++++++++++++ 1 file changed, 247 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/FrostSigningSessionDaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/FrostSigningSessionDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/FrostSigningSessionDaoJvmTest.kt new file mode 100644 index 00000000..2e03b6f7 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/FrostSigningSessionDaoJvmTest.kt @@ -0,0 +1,247 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.FrostSignerMessage +import press.mantra.compose.database.model.FrostSigningSession +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.types.FrostSigningStage +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.time.Instant + +/** + * The signing counterpart to [DkgSessionDaoJvmTest], and it differs from the DKG in one way + * that shapes every query here: "unlike a DKG a group signs repeatedly, so there is no single + * current one to observe". Sessions accumulate, which makes room scoping and ordering + * load-bearing rather than incidental. + * + * The duplicate-suppression property is the same and matters for the same reason. The composite + * key (sessionId, signerPublicKey, kind) is what makes a redelivered nonce or partial signature + * replace its predecessor instead of adding a row, and `countMessagesByKind` is what decides + * that enough signers have answered. A second row for one signer would let a session cross its + * threshold while short a real participant. + */ +class FrostSigningSessionDaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val user = KeyPair().pubKey.toHexKey() + private val alice = KeyPair().pubKey.toHexKey() + private val bob = KeyPair().pubKey.toHexKey() + private val roomOne = "11".repeat(32) + private val roomTwo = "22".repeat(32) + + private val nonceKind = 1 + private val partialSignatureKind = 2 + + private suspend fun seedRooms() { + val nostrEventId = "c".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = user, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = user, userName = "user", nostrEventId = nostrEventId)) + listOf(roomOne, roomTwo).forEach { id -> + db.chatRoomDao().upsert( + ChatRoom( + id = id, + userPublicKey = user, + subject = null, + description = null, + mlsGroupState = null, + ) + ) + } + } + + private suspend fun session( + id: String, + chatRoomId: String = roomOne, + createdAt: Instant = Instant.fromEpochSeconds(1_000), + stage: FrostSigningStage = FrostSigningStage.COLLECTING_NONCES, + signature: String? = null, + ): FrostSigningSession = FrostSigningSession( + id = id, + chatRoomId = chatRoomId, + coordinatorPublicKey = user, + userPublicKey = user, + dkgSessionId = "dkg-1", + threshold = 2, + participantCount = 3, + signerId = 1, + stage = stage, + unsignedEventJson = "{}", + eventId = "ff".repeat(32), + nonceRandom = "aa".repeat(32), + signature = signature, + createdAt = createdAt, + ).also { db.frostSigningSessionDao().upsert(it) } + + private suspend fun message( + sessionId: String, + signer: String, + kind: Int = nonceKind, + payload: String = "aa".repeat(32), + createdAt: Instant = Instant.fromEpochSeconds(1_000), + ) = db.frostSigningSessionDao().upsert( + FrostSignerMessage( + sessionId = sessionId, + signerPublicKey = signer, + kind = kind, + payload = payload, + createdAt = createdAt, + ) + ) + + @Test + fun `a signing session reads back by its id`() = runBlocking { + seedRooms() + session("s1", stage = FrostSigningStage.COLLECTING_PARTIAL_SIGNATURES) + + val found = assertNotNull(db.frostSigningSessionDao().getSessionById("s1")) + + assertEquals(FrostSigningStage.COLLECTING_PARTIAL_SIGNATURES, found.stage) + assertNull(db.frostSigningSessionDao().getSessionById("nope")) + } + + /** + * Sessions accumulate rather than replacing each other, so a room keeps a history and the + * newest is the one a resume cares about. + */ + @Test + fun `a rooms signing sessions accumulate newest first`() = runBlocking { + seedRooms() + session("first", createdAt = Instant.fromEpochSeconds(1_000)) + session("second", createdAt = Instant.fromEpochSeconds(2_000)) + session("third", createdAt = Instant.fromEpochSeconds(3_000)) + + assertEquals( + listOf("third", "second", "first"), + db.frostSigningSessionDao().getSessionsForChatRoom(roomOne).map { it.id }, + ) + assertEquals("third", db.frostSigningSessionDao().getLatestSessionForChatRoom(roomOne)?.id) + } + + /** + * Signing happens in the #admins room, and a device can be in more than one. A session from + * another room appearing here would have a signer answering a request its group never made. + */ + @Test + fun `sessions are scoped to their own room`() = runBlocking { + seedRooms() + session("mine", chatRoomId = roomOne) + session("theirs", chatRoomId = roomTwo, createdAt = Instant.fromEpochSeconds(9_000)) + + assertEquals(listOf("mine"), db.frostSigningSessionDao().getSessionsForChatRoom(roomOne).map { it.id }) + assertEquals("mine", db.frostSigningSessionDao().getLatestSessionForChatRoom(roomOne)?.id) + assertEquals(emptyList(), db.frostSigningSessionDao().getSessionsForChatRoom("33".repeat(32))) + } + + /** + * The threshold gate. A redelivered nonce must replace the signer's earlier one rather than + * add a row, or the count crosses the threshold with fewer signers than the session + * requires -- and the aggregation proceeds on a set that was never assembled. + */ + @Test + fun `a resent nonce replaces the signers earlier one`() = runBlocking { + seedRooms() + session("s1") + message("s1", alice, payload = "aa".repeat(32)) + + message("s1", alice, payload = "bb".repeat(32)) + + assertEquals(1, db.frostSigningSessionDao().countMessagesByKind("s1", nonceKind)) + assertEquals( + "bb".repeat(32), + db.frostSigningSessionDao().getMessage("s1", nonceKind, alice)?.payload, + ) + } + + /** + * A signer contributes to both rounds of a session -- a nonce and then a partial signature + * -- and the kind in the composite key is what keeps the second from overwriting the first. + */ + @Test + fun `a signer holds a nonce and a partial signature at once`() = runBlocking { + seedRooms() + session("s1") + message("s1", alice, kind = nonceKind, payload = "aa".repeat(32)) + message("s1", alice, kind = partialSignatureKind, payload = "cc".repeat(32)) + + assertEquals(1, db.frostSigningSessionDao().countMessagesByKind("s1", nonceKind)) + assertEquals(1, db.frostSigningSessionDao().countMessagesByKind("s1", partialSignatureKind)) + assertEquals( + "aa".repeat(32), + db.frostSigningSessionDao().getMessage("s1", nonceKind, alice)?.payload, + "the partial signature overwrote the nonce", + ) + } + + @Test + fun `messages are counted per session and per round`() = runBlocking { + seedRooms() + session("s1") + session("s2") + message("s1", alice, kind = nonceKind) + message("s1", bob, kind = nonceKind) + message("s2", alice, kind = nonceKind) + + assertEquals(2, db.frostSigningSessionDao().countMessagesByKind("s1", nonceKind)) + assertEquals(1, db.frostSigningSessionDao().countMessagesByKind("s2", nonceKind)) + assertEquals(0, db.frostSigningSessionDao().countMessagesByKind("s1", partialSignatureKind)) + assertNull(db.frostSigningSessionDao().getMessage("s1", nonceKind, user)) + } + + /** + * Worth recording because it is the one place these two DAOs disagree: FROST orders a + * round's messages by `createdAt`, where the DKG orders the same query by participant + * public key. Arrival order is per-device, so this ordering is not canonical across the + * group the way the DKG's is. Pinned as it stands rather than assumed to be either + * deliberate or a slip -- a caller that needs a canonical signer order has to impose one. + */ + @Test + fun `a rounds messages are ordered by arrival, unlike the dkg`() = runBlocking { + seedRooms() + session("s1") + val byKey = listOf(alice, bob).sorted() + message("s1", byKey[1], createdAt = Instant.fromEpochSeconds(1_000)) + message("s1", byKey[0], createdAt = Instant.fromEpochSeconds(2_000)) + + val found = db.frostSigningSessionDao().getMessagesByKind("s1", nonceKind).map { it.signerPublicKey } + + assertEquals(listOf(byKey[1], byKey[0]), found, "FROST returns a round in arrival order") + } + + /** A finished session keeps its signature, which is what a resume reads to avoid re-signing. */ + @Test + fun `a completed session keeps its signature`() = runBlocking { + seedRooms() + session("s1", stage = FrostSigningStage.COMPLETE, signature = "ab".repeat(32)) + + val found = assertNotNull(db.frostSigningSessionDao().getSessionById("s1")) + + assertEquals(FrostSigningStage.COMPLETE, found.stage) + assertEquals("ab".repeat(32), found.signature) + } +} From 5fa0d08dfde5feac195e46796d6ea96bbfc292e1 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:25:19 +0200 Subject: [PATCH 11/12] test: cover nip17 room derivation and its preconditions A NIP-17 room has no MLS group, no key packages and no invites -- membership *is* the p-tag set on each message. Two things follow, and both are load-bearing. The room id is deriveChatRoomId over the member set, the same aggregate the inbound path derives from an arriving gift wrap. That is what makes creation idempotent, and idempotence here is not a nicety: two people starting the same conversation have to land on one room, or the thread exists twice with each side writing into its own copy and neither seeing the other. Covered from three angles -- the order members are named in does not change the id, creating the same conversation twice reuses the room as it stands rather than rewriting it, and a different member set derives a different room. The order-independence test is guarding `deriveChatRoomId`'s own `.sorted()`, not the DAO's `.toSet()`, and its comment now says so. That was established by mutation rather than assumed: rebuilding the member set as an order-preserving LinkedHashSet in the DAO changes nothing, because the derivation sorts anyway, while removing the sort fails the test. The distinction matters for anyone reading the DAO and concluding the set is what does the work. And `mlsGroupState = null` is what marks the room NIP-17. sendChatMessage reads exactly that field to choose between a kind:445 group event and per-recipient gift wraps, so a room that acquired MLS state would have its messages routed down a path no recipient is running. Covered alongside: the creator is a participant of their own conversation even when not listed among the participants -- sealGiftWrapPayload walks that list to decide who to wrap for, so omitting the creator would send messages every other member could read and the sender could not -- and naming the creator among the participants does not produce a second row for them. One test records a precondition and an asymmetry. Participant.participantPublicKey is a foreign key onto Profile, so createNip17ChatRoom raises a SQLite constraint failure for a member this device has no profile for, while getOrCreateChatRoom, one method down, answers the same "never seen this user" situation by returning null. A caller treating the two alike gets an unhandled exception out of the first. That was found by writing the tests -- seven of them failed with SQLite 787 before every member was seeded -- and is pinned rather than seeded around silently. The remaining getOrCreate coverage: it returns the room already stored rather than overwriting it with the defaults passed in, stands one up for a user it has a profile for, and writes nothing at all when it does not. Real secp256k1 keys throughout, because deriveChatRoomId does point arithmetic and treats an off-curve value differently from a valid one -- hex filler would exercise a path users never reach. 11 tests. composeApp jvmTest is 309 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../database/dao/NostrNip17DaoJvmTest.kt | 289 ++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrNip17DaoJvmTest.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrNip17DaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrNip17DaoJvmTest.kt new file mode 100644 index 00000000..28a135b8 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/NostrNip17DaoJvmTest.kt @@ -0,0 +1,289 @@ +package press.mantra.compose.database.dao + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * NIP-17 rooms have no MLS group, no key packages and no invites -- membership *is* the p-tag + * set on each message. Two properties follow from that, and both are load-bearing. + * + * The room id is [ChatRoom.deriveChatRoomId] over the member set, the same aggregate the + * inbound path derives from an arriving gift wrap. That is what makes creation idempotent: + * two people starting the same conversation have to land on one room rather than two, or the + * same thread exists twice with each side writing into its own copy. + * + * And `mlsGroupState = null` is not incidental -- `sendChatMessage` reads exactly that to + * decide between a group event and gift wraps. A NIP-17 room that acquired MLS state would + * have its messages routed down a path no recipient is running. + */ +class NostrNip17DaoJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + // Real keys: deriveChatRoomId does secp256k1 point work and treats an off-curve value + // differently from a valid one, so hex filler would exercise a path users never hit. + private val user = KeyPair().pubKey.toHexKey() + private val alice = KeyPair().pubKey.toHexKey() + private val bob = KeyPair().pubKey.toHexKey() + + private suspend fun seedProfile(publicKey: String) { + val nostrEventId = publicKey.take(63) + "f" + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = publicKey, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert( + Profile(publicKey = publicKey, userName = "member", nostrEventId = nostrEventId) + ) + } + + /** + * Every member, not just the creator. `Participant.participantPublicKey` is a foreign key + * onto Profile, so a room cannot be stood up for someone this device has never seen -- see + * `creating a room with an unknown member is refused by the schema` for what that costs. + */ + private suspend fun seedMembers(vararg publicKeys: String) = publicKeys.forEach { seedProfile(it) } + + private suspend fun participantsOf(chatRoomId: String) = + db.participantDao().findParticipantsByChatRoomId(chatRoomId).map { it.participantPublicKey }.toSet() + + @Test + fun `a nip17 room is created with its members as participants`() = runBlocking { + seedMembers(user, alice, bob) + + val room = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom( + userPublicKey = user, + participantPublicKeys = listOf(alice, bob), + subject = "a thread", + ), + "createNip17ChatRoom returned null", + ) + + assertEquals(setOf(user, alice, bob), participantsOf(room.chatRoom.id)) + assertEquals("a thread", room.chatRoom.subject) + } + + /** + * The author is a member of their own conversation. `sealGiftWrapPayload` walks the + * participants to decide who to wrap for, and a room that omitted its creator would send + * messages every other member could read and the sender could not. + */ + @Test + fun `the creator is a participant even when not listed`() = runBlocking { + seedMembers(user, alice) + + val room = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom( + userPublicKey = user, + participantPublicKeys = listOf(alice), + ) + ) + + assertTrue(user in participantsOf(room.chatRoom.id)) + } + + /** Membership is a set, so naming the creator among the participants is not a second member. */ + @Test + fun `listing the creator among the participants does not duplicate them`() = runBlocking { + seedMembers(user, alice) + + val room = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom( + userPublicKey = user, + participantPublicKeys = listOf(user, alice), + ) + ) + + assertEquals(setOf(user, alice), participantsOf(room.chatRoom.id)) + assertEquals(2, db.participantDao().findParticipantsByChatRoomId(room.chatRoom.id).size) + } + + /** + * What actually enforces this is the `.sorted()` inside `deriveChatRoomId` -- the DAO's + * `.toSet()` dedupes but carries an order. Sorting is what lets both ends of a + * conversation derive the same id independently: one from the list a user typed, the other + * from the p-tags on an arriving gift wrap, which will not be in the same order. + */ + @Test + fun `the room id does not depend on the order members are named`() = runBlocking { + seedMembers(user, alice, bob) + + val first = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom( + userPublicKey = user, + participantPublicKeys = listOf(alice, bob), + ) + ) + val second = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom( + userPublicKey = user, + participantPublicKeys = listOf(bob, alice), + ) + ) + + assertEquals(first.chatRoom.id, second.chatRoom.id) + } + + /** + * Idempotence, which is the point of deriving the id rather than generating one. Creating + * the same conversation twice reuses the room instead of standing up a second one that + * would split the thread. + */ + @Test + fun `creating the same conversation twice reuses the room`() = runBlocking { + seedMembers(user, alice, bob) + + val first = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom(user, listOf(alice, bob), subject = "first") + ) + val second = assertNotNull( + db.nostrNip17Dao().createNip17ChatRoom(user, listOf(alice, bob), subject = "ignored") + ) + + assertEquals(first.chatRoom.id, second.chatRoom.id) + assertEquals( + "first", + second.chatRoom.subject, + "the existing room is reused as it stands rather than rewritten", + ) + assertEquals(3, db.participantDao().findParticipantsByChatRoomId(first.chatRoom.id).size) + } + + /** A different member set is a different conversation. */ + @Test + fun `a different member set derives a different room`() = runBlocking { + seedMembers(user, alice, bob) + + val pair = assertNotNull(db.nostrNip17Dao().createNip17ChatRoom(user, listOf(alice))) + val trio = assertNotNull(db.nostrNip17Dao().createNip17ChatRoom(user, listOf(alice, bob))) + + assertTrue(pair.chatRoom.id != trio.chatRoom.id) + } + + /** + * What marks the room NIP-17. `sendChatMessage` branches on this field to choose between a + * kind:445 group event and per-recipient gift wraps, so a non-null value here would route + * direct messages down the MLS path. + */ + @Test + fun `a nip17 room carries no mls state`() = runBlocking { + seedMembers(user, alice) + + val room = assertNotNull(db.nostrNip17Dao().createNip17ChatRoom(user, listOf(alice))) + + assertNull(room.chatRoom.mlsGroupState, "MLS state is what tells the two room kinds apart") + } + + /** + * The precondition, and an asymmetry worth knowing about. A member with no Profile row + * violates Participant's foreign key, so this raises rather than returning null -- while + * `getOrCreateChatRoom`, one method down, answers the same "I have never seen this user" + * situation by returning null. A caller that treats the two alike gets an unhandled + * exception out of the first one. + */ + @Test + fun `creating a room with an unknown member is refused by the schema`() = runBlocking { + seedMembers(user) + + assertFailsWith { + db.nostrNip17Dao().createNip17ChatRoom( + userPublicKey = user, + participantPublicKeys = listOf(alice), + ) + } + } + + /** + * getOrCreateChatRoom is the inbound counterpart and takes the id as given, since it comes + * off an arriving event rather than from a member list. It returns the room already stored + * rather than overwriting it. + */ + @Test + fun `getOrCreate returns the room that already exists`() = runBlocking { + seedProfile(user) + val chatRoomId = "11".repeat(32) + db.chatRoomDao().upsert( + ChatRoom( + id = chatRoomId, + userPublicKey = user, + subject = "already here", + description = null, + mlsGroupState = null, + ) + ) + + val found = assertNotNull( + db.nostrNip17Dao().getOrCreateChatRoom( + chatRoomId = chatRoomId, + activeUserPublicKey = user, + relayHint = null, + defaultSubject = "would be new", + ) + ) + + assertEquals("already here", found.chatRoom.subject) + } + + @Test + fun `getOrCreate stands up a room the active user has a profile for`() = runBlocking { + seedProfile(user) + val chatRoomId = "22".repeat(32) + + val created = assertNotNull( + db.nostrNip17Dao().getOrCreateChatRoom( + chatRoomId = chatRoomId, + activeUserPublicKey = user, + relayHint = "wss://relay.example", + defaultSubject = "new room", + ) + ) + + assertEquals(chatRoomId, created.chatRoom.id) + assertEquals("new room", created.chatRoom.subject) + assertTrue(user in participantsOf(chatRoomId)) + } + + /** + * The guard: with no profile for the active user there is nothing to hang a room off, and + * the DAO returns null rather than writing a room whose owner it cannot name. + */ + @Test + fun `getOrCreate refuses when the active user has no profile`() = runBlocking { + val chatRoomId = "33".repeat(32) + + val created = db.nostrNip17Dao().getOrCreateChatRoom( + chatRoomId = chatRoomId, + activeUserPublicKey = user, + relayHint = null, + ) + + assertNull(created) + assertNull(db.chatRoomDao().findChatRoomById(chatRoomId), "no room should have been written") + } +} From 44127cf5148312df10ba697cce9a880e5580c8a3 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 03:35:27 +0200 Subject: [PATCH 12/12] test: exercise MarmotOutboundDao past the MLS guard 02e70d9 claimed the paths past the membership guard "need a real peer key package to exercise, which means an MLS fixture this test file deliberately does not build", and left them uncovered on that basis. That was wrong, and this corrects it. Nothing about a key package needs a relay. DatabaseMarmotRepository.generateKeyPackage already builds this device's own entirely locally: two X25519 key generations, one Ed25519, a leaf node signed under "LeafNodeTBS" and a key package signed under "KeyPackageTBS". Everything it touches is quartz public API, so MarmotKeyPackageFixture replicates it in about forty lines. The capabilities it advertises are not decoration -- the group's RequiredCapabilities rejects a leaf that does not carry LastResort and NostrGroupData, so a fixture omitting them is refused at addMember rather than at decode, and the comment says so. With that, three properties past the guard are asserted rather than described. The invitee is persisted. sealGiftWrapPayload walks the room's participants to decide who to wrap a Welcome for, so without the row the Welcome produces no gift wraps at all and sits unsealed forever. The advanced epoch reaches the database. addMember moves the in-memory group forward, and the comment on that write explains what happens when it is not saved back: the creator keeps encrypting under the old epoch, which the new member cannot decrypt, and the next invite re-derives from stale state and produces a conflicting commit. The test asserts the stored state changed, that it still restores, and that the restored group has two members -- so it is checking a real advance rather than any write at all. The epoch being left behind is retained, at epoch 0 for a freshly created group. That is the call that actually writes a retained secret, so it belongs here as well as in the retention-window tests that only read them. Verified by mutation: deleting the write that persists the advanced state fails `inviting a member persists the advanced group state` and nothing else. The mutation was reverted; no production source is touched by this commit. The peer needs a Profile row here where the guard tests did not, because Participant.participantPublicKey is a foreign key onto Profile and only a successful invite reaches that write -- the same constraint that shapes the nip17 tests in 5fa0d08. Found the same way, by three of these failing with SQLite 787 first. Still not covered: the Welcome itself, the deferred-welcome path for a group that already has members, and the batching in addMembersToChatRoom. Those need more than a key package -- a second device's view of the group -- and are a separate piece of work. 3 tests added, 7 in the class. composeApp jvmTest is 312 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../database/dao/MarmotKeyPackageFixture.kt | 73 ++++++++++ .../database/dao/MarmotOutboundDaoJvmTest.kt | 126 +++++++++++++++++- 2 files changed, 193 insertions(+), 6 deletions(-) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotKeyPackageFixture.kt diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotKeyPackageFixture.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotKeyPackageFixture.kt new file mode 100644 index 00000000..08a4ef9e --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotKeyPackageFixture.kt @@ -0,0 +1,73 @@ +package press.mantra.compose.database.dao + +import com.vitorpamplona.quartz.marmot.mls.crypto.Ed25519 +import com.vitorpamplona.quartz.marmot.mls.crypto.MlsCryptoProvider +import com.vitorpamplona.quartz.marmot.mls.crypto.X25519 +import com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage +import com.vitorpamplona.quartz.marmot.mls.tree.Capabilities +import com.vitorpamplona.quartz.marmot.mls.tree.Credential +import com.vitorpamplona.quartz.marmot.mls.tree.Extension +import com.vitorpamplona.quartz.marmot.mls.tree.LeafNode +import com.vitorpamplona.quartz.marmot.mls.tree.LeafNodeSource +import com.vitorpamplona.quartz.marmot.mls.tree.Lifetime +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import com.vitorpamplona.quartz.utils.TimeUtils +import press.mantra.compose.database.model.MarmotKeyPackage + +/** + * A real MLS key package for [publicKey], built the same way + * `DatabaseMarmotRepository.generateKeyPackage` builds this device's own. Entirely local: + * three key generations, a signed leaf node and a signed key package. No relay, no network, + * nothing to stub. + */ +internal fun marmotKeyPackageFor(publicKey: HexKey): MarmotKeyPackage { + val initKp = X25519.generateKeyPair() + val encKp = X25519.generateKeyPair() + val sigKp = Ed25519.generateKeyPair() + val now = TimeUtils.now() + + val unsignedLeaf = LeafNode( + encryptionKey = encKp.publicKey, + signatureKey = sigKp.publicKey, + credential = Credential.Basic(publicKey.hexToByteArray()), + capabilities = Capabilities( + // LastResort, then NostrGroupData -- the group's RequiredCapabilities rejects a + // leaf that does not advertise both, so a fixture without them is refused at + // addMember rather than at decode. + extensions = listOf(0x000A, 0xF2EE), + proposals = listOf(0x000A), + ), + leafNodeSource = LeafNodeSource.KEY_PACKAGE, + lifetime = Lifetime(notBefore = now, notAfter = now + 60L * 60L * 24L * 90L), + extensions = emptyList(), + signature = ByteArray(0), + ) + val leafNode = unsignedLeaf.copy( + signature = MlsCryptoProvider.signWithLabel( + sigKp.privateKey, + "LeafNodeTBS", + unsignedLeaf.encodeTbs(groupId = null, leafIndex = null), + ), + ) + + val unsigned = MlsKeyPackage( + initKey = initKp.publicKey, + leafNode = leafNode, + extensions = listOf(Extension(extensionType = 0x000A, extensionData = ByteArray(0))), + signature = ByteArray(0), + ) + val keyPackage = unsigned.copy( + signature = MlsCryptoProvider.signWithLabel( + sigKp.privateKey, + "KeyPackageTBS", + unsigned.encodeTbs(), + ), + ) + + return MarmotKeyPackage( + id = publicKey, + publicKey = publicKey, + tlsEncodedMarmotKeyPackage = keyPackage.toTlsBytes(), + ) +} diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt index 1d0ac995..37693973 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt @@ -11,11 +11,17 @@ import press.mantra.compose.database.model.MarmotKeyPackage import press.mantra.compose.database.model.NostrEvent import press.mantra.compose.database.model.Profile import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.extensions.toHex +import press.mantra.compose.nostr.Relays +import com.vitorpamplona.quartz.marmot.mip01Groups.MarmotGroupData +import com.vitorpamplona.quartz.marmot.mls.group.MlsGroup +import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray import press.mantra.compose.exceptions.MarmotMissingChatGroupException import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertNotNull import kotlin.test.assertTrue /** @@ -29,10 +35,10 @@ import kotlin.test.assertTrue * compile, still look like it worked, and leave a room whose members believe someone was * invited. * - * These cover the paths that need no MLS material. Everything past the guard -- the commit, the - * Welcome, the epoch advance and its persistence -- needs a real peer key package to exercise, - * which means an MLS fixture this test file deliberately does not build. Those paths are worth - * covering and are not covered here. + * Past the guard, the tests build a real MLS group and a real peer key package with + * [marmotKeyPackageFor], so the commit, the epoch advance and its persistence are exercised + * rather than described. Nothing there needs a relay: a key package is three local key + * generations and two signatures. */ class MarmotOutboundDaoJvmTest { @@ -76,8 +82,8 @@ class MarmotOutboundDaoJvmTest { } /** - * Never decoded: the guard throws before any of these tests reach the MLS layer, so the - * bytes only have to exist. A test that got past the guard would need a real key package. + * Never decoded: the guard throws before the tests using it reach the MLS layer, so the + * bytes only have to exist. The tests past the guard use [marmotKeyPackageFor] instead. */ private fun keyPackage() = MarmotKeyPackage( id = "d".repeat(64), @@ -85,6 +91,44 @@ class MarmotOutboundDaoJvmTest { tlsEncodedMarmotKeyPackage = ByteArray(0), ) + /** + * A room holding real MLS state, as a room this device created would. + * + * The peer gets a Profile row because Participant.participantPublicKey is a foreign key + * onto it, and a successful invite writes a Participant. The guard tests above never reach + * that write, which is why only they can get away without one. + */ + private suspend fun seedMlsRoom(): LocalChatRoom { + val stateless = seedStatelessRoom() + val peerEventId = "e".repeat(64) + db.nostrEventDao().upsert( + NostrEvent( + id = peerEventId, + pubKey = peer, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = peer, userName = "peer", nostrEventId = peerEventId)) + val mlsGroup = MlsGroup.create( + identity = user.hexToByteArray(), + initialExtensions = listOf( + MarmotGroupData.bootstrap( + nostrGroupId = roomId, + creatorPubKey = user, + outboxRelays = Relays.DefaultDMRelayList.map { it.url }, + ).toExtension() + ), + ) + val chatRoom = stateless.chatRoom.copy( + mlsGroupState = mlsGroup.saveState().encodeTls().toHex() + ) + db.chatRoomDao().upsert(chatRoom) + return LocalChatRoom(chatRoom = chatRoom) + } + @Test fun `inviting into a room with no mls state is refused rather than ignored`() = runBlocking { val localChatRoom = seedStatelessRoom() @@ -151,4 +195,74 @@ class MarmotOutboundDaoJvmTest { assertEquals(emptyList(), failed) } + + /** + * Past the guard. The invitee is persisted before the Welcome is sealed, because + * sealGiftWrapPayload walks the room's participants to decide who to wrap for -- without + * the row the Welcome produced no gift wraps at all and sat unsealed forever. + */ + @Test + fun `inviting a member into a real group persists the invitee`() = runBlocking { + val localChatRoom = seedMlsRoom() + + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = marmotKeyPackageFor(peer), + ) + + assertTrue( + db.participantDao().findParticipantsByChatRoomId(roomId) + .any { it.participantPublicKey == peer }, + "the invitee was not persisted", + ) + } + + /** + * The epoch advance has to reach the database. `addMember` moves the in-memory group to + * the next epoch; without saving it back the creator keeps encrypting under the old one -- + * which the new member cannot decrypt -- and the next invite re-derives from stale state + * and produces a conflicting commit. + */ + @Test + fun `inviting a member persists the advanced group state`() = runBlocking { + val localChatRoom = seedMlsRoom() + val stateBefore = localChatRoom.chatRoom.mlsGroupState + + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = marmotKeyPackageFor(peer), + ) + + val stateAfter = assertNotNull(db.chatRoomDao().findChatRoomById(roomId)).chatRoom.mlsGroupState + assertNotNull(stateAfter) + assertTrue(stateAfter != stateBefore, "the advanced epoch was never written back") + val group = assertNotNull( + db.chatRoomDao().findChatRoomById(roomId)!!.chatRoom.toMlsGroup(), + "the persisted state no longer restores", + ) + assertEquals(2, group.members().size.toInt(), "the invitee is not in the restored group") + } + + /** + * The epoch the group is leaving is retained on the way past, so messages already sent + * under it stay readable. Asserted here rather than only in the retention-window tests, + * because this is the call that actually writes one. + */ + @Test + fun `inviting a member retains the epoch being left behind`() = runBlocking { + val localChatRoom = seedMlsRoom() + + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = marmotKeyPackageFor(peer), + ) + + val retained = db.marmotRetainedEpochSecretDao() + .getMarmotRetainedEpochSecretForChatRoomId(roomId) + assertEquals(1, retained.size, "the pre-commit epoch was not retained") + assertEquals(0L, retained.single().epoch, "a freshly created group is at epoch 0") + } }