refactor: build the DM inbox filter once, where it can be asserted

The filter fix a commit ago changed a value inline in a ViewModel, which is
not a place a test can reach: ChatMessageListViewModel needs a repository and
a coroutine scope to construct, and NostrDao needs Room. So the filter that
had just been wrong in three call sites went back to having no coverage at
all.

Nip17Filters.inbox is that filter with one definition. ChatMessageListViewModel
and ChatRoomListViewModel now both call it — they had been building it
separately and identically, which is also what made their negentropy requests
collapse into one under computeId, a coincidence better expressed as shared
code than left to hold by luck.

Nip17FiltersTest asserts every clause that was got wrong in production:

  - the p tag names us, not a peer
  - there is no authors clause, because a wrap is signed by the throwaway key
    GiftWrapEvent.create mints and discards, so authors=[anything knowable]
    matches nothing on any relay
  - there is no since cursor, because NIP-59 back-dates a wrap by up to two
    days and a high-water mark taken from the newest wrap we hold skips mail
    stamped behind it — the trap waiting for whoever acts on the TODO in
    NegentropySynchronizeRequest.toSynchronizeNostrEventRequest
  - the wire JSON is pinned, so an added default cannot quietly split the two
    callers back into separate requests
  - the SQL NostrEventFilterQuery builds from it bounds no author either,
    since negentropy is only as good as the agreement between the set we build
    locally and the set the relay builds from the same filter

Neither of the two failure modes this covers was visible from reading the
filter. The authors clause failed silently for as long as it existed, and the
peer p-tag failed loudly but somewhere else entirely — in a Room transaction,
three files away, as a MAC error out of Nip44.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-05 23:22:06 +02:00
parent e1d35bbd6c
commit ad3304a665
4 changed files with 125 additions and 25 deletions

View File

@@ -0,0 +1,37 @@
package press.mantra.compose.nostr
import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
import press.mantra.compose.database.model.types.SynchronizationFilter
/**
* The one filter shape that can return a NIP-17 message we are able to read.
*
* A gift wrap hides everything except who it is for. The author is the throwaway
* key [GiftWrapEvent.create] mints and discards, the content is sealed to the
* recipient, and `created_at` is randomised up to two days into the past. That
* leaves the `p` tag as the only clause worth writing, and it has to name us:
* naming a peer subscribes to mail no key of ours can open, and adding `authors`
* matches nothing on any relay, ever. Both mistakes were live in three separate
* call sites, so the filter is built in one place now and asserted in one place.
*/
object Nip17Filters {
/**
* Everything gift-wrapped to [publicKey], capped at [limit] events.
*
* Deliberately carries no `since`. NIP-59 back-dates a wrap by up to two days,
* so a cursor built from the newest wrap we hold silently skips mail that was
* sent later but stamped earlier.
*/
fun inbox(
publicKey: HexKey,
limit: Int = DEFAULT_LIMIT,
) = SynchronizationFilter(
kinds = arrayOf(GiftWrapEvent.KIND),
tags = mapOf("p" to listOf(publicKey)),
limit = limit,
)
const val DEFAULT_LIMIT = 50
}

View File

@@ -66,6 +66,7 @@ import press.mantra.compose.database.model.intermdiate.LocalChatRoom
import press.mantra.compose.database.model.types.SynchronizationFilter
import press.mantra.compose.extensions.shortened
import press.mantra.compose.extensions.toFormattedTimeAndDateString
import press.mantra.compose.nostr.Nip17Filters
import press.mantra.compose.nostr.Relays
import press.mantra.compose.repository.ChatRepository
import press.mantra.compose.repository.NostrRepository
@@ -74,7 +75,6 @@ import press.mantra.compose.ui.view.state.ChatMessageListUIState
import co.touchlab.kermit.Logger
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent
import com.vitorpamplona.quartz.nip17Dm.settings.ChatMessageRelayListEvent
import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.IO
import kotlinx.coroutines.flow.distinctUntilChanged
@@ -123,22 +123,13 @@ class ChatMessageListViewModel(
val chatMessageRelayListEvent = chatRepository.getChatMessageRelayForPublicKey(recipients.participant.participantPublicKey)
val relayAndSynchronizationFilter = if (chatMessageRelayListEvent != null) {
// Refresh our own inbox. A wrap names only its recipient, so
// "the messages in this conversation" is not something a filter
// can ask for, and the recipient's relays hold their mail, not
// ours. p-tagging the peer here fetched other people's wraps,
// which no key of ours can open.
// Opening a conversation refreshes our own inbox: this used to
// p-tag the peer and read their relays, which is where their mail
// is kept, not ours. See Nip17Filters for why a per-conversation
// filter is not a thing that can be written.
Pair(
Relays.DefaultDMRelayList,
SynchronizationFilter(
kinds = arrayOf(
GiftWrapEvent.KIND,
),
tags = mapOf(
Pair("p", listOf(localChatRoom.chatRoom.userPublicKey))
),
limit = 50
)
Nip17Filters.inbox(localChatRoom.chatRoom.userPublicKey),
)
} else {

View File

@@ -27,13 +27,13 @@ import androidx.lifecycle.viewmodel.initializer
import androidx.lifecycle.viewmodel.viewModelFactory
import press.mantra.compose.database.model.NegentropySynchronizeRequest
import press.mantra.compose.database.model.types.SynchronizationFilter
import press.mantra.compose.nostr.Nip17Filters
import press.mantra.compose.nostr.Relays
import press.mantra.compose.repository.ChatRepository
import press.mantra.compose.repository.NostrRepository
import press.mantra.compose.ui.view.state.ChatRoomListUIState
import co.touchlab.kermit.Logger
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent
import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.IO
import kotlinx.coroutines.launch
@@ -72,15 +72,7 @@ class ChatRoomListViewModel(
logger.d("scheduleSynchronization")
viewModelScope.launch(Dispatchers.IO) {
// Sync Notifications... might want to also run this in the background
val chatRequestFilter = SynchronizationFilter(
kinds = arrayOf(
GiftWrapEvent.KIND,
),
tags = mapOf(
Pair("p", listOf(publicKey))
),
limit = 50
)
val chatRequestFilter = Nip17Filters.inbox(publicKey)
nostrRepository.queueNegentropySynchronizeRequest(
Relays.DefaultDMRelayList.shuffled().map { normalizedRelayUrl ->
NegentropySynchronizeRequest(

View File

@@ -0,0 +1,80 @@
package press.mantra.compose.nostr
import press.mantra.compose.database.query.NostrEventFilterQuery
import press.mantra.compose.network.serialization.encodeToJsonString
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull
/**
* Pins the only gift wrap filter that can come back with something we can read.
*
* Every clause here is one that was got wrong in production. Two call sites asked
* for `authors = [our pubkey]`, which cannot match a wrap signed by a throwaway
* key and so returned nothing at all, silently, for as long as it existed. A third
* asked for `p = [the peer]`, which returned other people's mail and crashed the
* save that tried to unseal it. Neither failure was visible from reading the
* filter, so the shape is asserted instead of trusted.
*/
class Nip17FiltersTest {
private val us = "a".repeat(64)
@Test
fun `it asks for wraps addressed to us`() {
assertEquals(mapOf("p" to listOf(us)), Nip17Filters.inbox(us).tags)
}
@Test
fun `it constrains no authors`() {
// GiftWrapEvent.create signs with a key it generates and drops, so the author
// of a wrap is a value nobody can predict -- least of all the sender's own
// pubkey. Any authors clause here silently matches zero events on every relay.
assertNull(Nip17Filters.inbox(us).authors)
}
@Test
fun `it carries no since cursor`() {
// A wrap is stamped up to two days earlier than it was sent, so a high-water
// mark taken from the newest wrap we hold skips mail that arrives behind it.
// Anything reintroducing `since` has to back-date by at least two days first.
assertNull(Nip17Filters.inbox(us).since)
assertNull(Nip17Filters.inbox(us).until)
}
@Test
fun `it asks for gift wraps and nothing else`() {
assertEquals(listOf(1059), Nip17Filters.inbox(us).kinds?.toList())
}
@Test
fun `two callers asking for the same inbox make one request`() {
// computeId hashes the encoded filter, so the chat room list and the chat
// message screen collapse into a single negentropy request only while both
// encode identically. Building the filter once is what holds that true; the
// wire shape is asserted so an added default cannot quietly split them.
assertEquals(Nip17Filters.inbox(us), Nip17Filters.inbox(us))
assertEquals(
"""{"kinds":[1059],"tags":{"p":["$us"]},"limit":50}""",
Nip17Filters.inbox(us).encodeToJsonString(),
)
}
@Test
fun `the local set it builds is the same set the relay is asked for`() {
// Negentropy reconciles our local set against the relay's: this filter goes out
// in NEG-OPEN, and the local side is built by running the same filter through
// NostrEventFilterQuery. A clause that survives one trip and not the other
// reports differences that are not real -- events re-downloaded forever, or
// pushed at a relay that excluded them on purpose. What matters here is that
// the local query reads the p tag and, like the wire filter, bounds no author:
// an authors clause would show up as `pubKey IN (?)`.
val query = NostrEventFilterQuery.build(Nip17Filters.inbox(us))
assertEquals(
"SELECT * FROM NostrEvent WHERE kind IN (?) AND (tags LIKE ? ESCAPE '\\') " +
"ORDER BY createdAt DESC, id DESC",
query.sql,
)
}
}