fix: build the local negentropy set from the whole filter, not a guess at its shape
A negentropy exchange compares two sets defined by the SAME filter: the relay
builds its side from the filter carried in NEG-OPEN, and this device builds its
side from getNegentropicNostrFeedIds. Any clause we fail to apply locally makes
our set a superset of the relay's, and each extra row comes back as an id the
relay is "missing" -- which this app then queues as a broadcast. Any clause we
apply more tightly makes it a subset, and the difference comes back as ids to
re-download that we already hold. Neither shows up as an error; both show up as a
sync that never settles.
getNegentropicNostrFeedIds was a `when` over the shape of the filter, dispatching
to one of eight hand-written @Query methods. Each method could only bind the
parameters it happened to declare, so the branches disagreed with the filter they
were serving:
- `until` was expressible by NO branch. It is sent to the relay in NEG-OPEN and
was never applied here, so every local event past the requested window was
reported to the relay as one it lacked.
- `since` was strict (`createdAt > :since`) where NIP-01 is inclusive, so an
event stamped exactly on the boundary was a phantom "need" on every pass.
- `kinds && authors` was tested before any tag branch, so a filter carrying
kinds, authors AND tags silently dropped the tags. `kinds && ids` dropped
authors. Every branch dropped whatever it had no parameter for.
- tags were matched with `tags LIKE '%' || :value || '%'` -- a substring scan of
the serialized tag JSON that matches the value in ANY tag position. A pubkey
referenced in an `e` tag counted as a `p` match. And only `tags[name].first()`
was ever bound, so the second and later values of a tag were dropped.
- the reply branch matched `'%' || :eventId || '%reply%'`, which needs the
literal text "reply" to appear somewhere after the id: it misses
`["e","<id>"]` with no marker and false-positives on any later tag containing
the word.
- the `else` branch ignored the filter's kinds entirely and substituted
`arrayOf(TextNoteEvent.KIND)`. A filter with only authors, or only tags, got a
local set of kind-1 notes -- unrelated to what the relay was reconciling.
- more than one filter returned emptyList() with a "not yet supported" warning.
That is the worst available answer: an empty local set tells the relay we hold
none of these events, so it hands back its entire set as ids to download.
- the limit branches ordered `createdAt ASC LIMIT n`, returning the OLDEST n
where a relay answering a limited filter returns the newest.
## The replacement
NostrEventFilterQuery translates a SynchronizationFilter into one SQL statement
that applies every clause, and NostrEventDao.getNostrEventsMatchingFilter runs it
as a @RawQuery. Raw because a nostr filter is a variable set of constraints over
variable-length lists, which is precisely what @Query cannot express -- and what
drove the per-shape methods that dropped constraints in the first place.
Semantics follow quartz's FilterMatcher, which is what the relays this app talks
to implement: membership for ids/authors/kinds; AND between tag names and OR
between the values of one name for `tags`; AND both ways for `tagsAll`; inclusive
`since`/`until`; and a present-but-empty list matches nothing.
Tags are matched by looking for the `["<name>","<value>"` fragment, built by
encoding through the same serializer that wrote the column so escaping agrees,
with `%`/`_`/`\` escaped and `ESCAPE '\'` on the LIKE so a wildcard inside a value
cannot widen the match. Anchoring on the tag name and on the closing quote of the
value is what keeps a hex string from matching in an unrelated tag position.
Multiple filters are now the union of their matches, de-duplicated by id.
## The Marmot branch is kept, and narrowed
Group messages still answer from MarmotGroupEvent: that table carries the NIP-40
expiry a relay uses to decide whether it still serves an event, and an indexed
chatRoomId instead of a scan of the tags JSON. But the branch now only claims a
filter it can fully honour -- exactly kind 445, an `h` tag, and nothing else --
because it answers from a different table and would otherwise reproduce the same
silently-dropped-constraint bug it is an exception to. It also fills in the `h`
tag and the real signature on the NostrEvent it synthesizes rather than leaving
them empty.
## Tests
NostrEventFilterQueryTest pins the generated SQL and the bound values for each
clause, including tag escaping and the empty-list case. It asserts the
translation rather than eyeballing it, because a dropped clause is not an error
at runtime -- it is reconciliation quietly reporting differences that are not
real.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,191 @@
|
||||
package press.mantra.compose.database.query
|
||||
|
||||
import androidx.sqlite.SQLiteStatement
|
||||
import press.mantra.compose.database.model.types.SynchronizationFilter
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.time.Instant
|
||||
|
||||
/**
|
||||
* Pins the SQL a [SynchronizationFilter] turns into.
|
||||
*
|
||||
* Negentropy is only as good as the agreement between the set we build from this query and the
|
||||
* set the relay builds from the same filter sent in NEG-OPEN. A clause that is dropped or a
|
||||
* bound that is off by one shows up not as an error but as reconciliation reporting differences
|
||||
* that are not real — events re-downloaded, or events pushed at a relay that filtered them out
|
||||
* on purpose. So the translation is asserted rather than eyeballed.
|
||||
*/
|
||||
class NostrEventFilterQueryTest {
|
||||
|
||||
private val alice = "a".repeat(64)
|
||||
private val bob = "b".repeat(64)
|
||||
private val note = "c".repeat(64)
|
||||
|
||||
@Test
|
||||
fun `applies every clause a nostr filter can carry`() {
|
||||
val query = NostrEventFilterQuery.build(
|
||||
SynchronizationFilter(
|
||||
ids = arrayOf(note),
|
||||
authors = arrayOf(alice, bob),
|
||||
kinds = arrayOf(1, 6),
|
||||
since = Instant.fromEpochSeconds(1_000),
|
||||
until = Instant.fromEpochSeconds(2_000),
|
||||
)
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE id IN (?) AND pubKey IN (?, ?) AND kind IN (?, ?) " +
|
||||
"AND createdAt >= ? AND createdAt <= ? ORDER BY createdAt DESC, id DESC",
|
||||
query.sql,
|
||||
)
|
||||
assertEquals(listOf(note, alice, bob, 1L, 6L, 1_000L, 2_000L), query.boundValues())
|
||||
}
|
||||
|
||||
/**
|
||||
* `until` is the clause the per-shape queries this replaced could not express at all: it was
|
||||
* sent to the relay and never applied locally, so every local event past the window came
|
||||
* back as one the relay was "missing".
|
||||
*/
|
||||
@Test
|
||||
fun `time bounds are inclusive on both ends`() {
|
||||
val query = NostrEventFilterQuery.build(
|
||||
SynchronizationFilter(since = Instant.fromEpochSeconds(7), until = Instant.fromEpochSeconds(9))
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE createdAt >= ? AND createdAt <= ? ORDER BY createdAt DESC, id DESC",
|
||||
query.sql,
|
||||
)
|
||||
assertEquals(listOf(7L, 9L), query.boundValues())
|
||||
}
|
||||
|
||||
/** AND between tag names, OR between the values of one name — as `FilterMatcher` does. */
|
||||
@Test
|
||||
fun `tags are matched by name and value, not by substring`() {
|
||||
val query = NostrEventFilterQuery.build(
|
||||
SynchronizationFilter(
|
||||
kinds = arrayOf(1059),
|
||||
tags = mapOf("p" to listOf(alice, bob), "e" to listOf(note)),
|
||||
)
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE kind IN (?) " +
|
||||
"AND (tags LIKE ? ESCAPE '\\' OR tags LIKE ? ESCAPE '\\') " +
|
||||
"AND (tags LIKE ? ESCAPE '\\') ORDER BY createdAt DESC, id DESC",
|
||||
query.sql,
|
||||
)
|
||||
assertEquals(
|
||||
listOf(1059L, "%[\"p\",\"$alice\"%", "%[\"p\",\"$bob\"%", "%[\"e\",\"$note\"%"),
|
||||
query.boundValues(),
|
||||
)
|
||||
}
|
||||
|
||||
/** tagsAll is AND between values too, so each value gets its own clause. */
|
||||
@Test
|
||||
fun `tagsAll requires every value`() {
|
||||
val query = NostrEventFilterQuery.build(
|
||||
SynchronizationFilter(tagsAll = mapOf("t" to listOf("nostr", "kmp")))
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE tags LIKE ? ESCAPE '\\' AND tags LIKE ? ESCAPE '\\' " +
|
||||
"ORDER BY createdAt DESC, id DESC",
|
||||
query.sql,
|
||||
)
|
||||
assertEquals(listOf("%[\"t\",\"nostr\"%", "%[\"t\",\"kmp\"%"), query.boundValues())
|
||||
}
|
||||
|
||||
/** A wildcard in a tag value must not widen the match. */
|
||||
@Test
|
||||
fun `like wildcards inside a tag value are escaped`() {
|
||||
val query = NostrEventFilterQuery.build(
|
||||
SynchronizationFilter(tags = mapOf("d" to listOf("100%_off")))
|
||||
)
|
||||
|
||||
assertEquals(listOf("%[\"d\",\"100\\%\\_off\"%"), query.boundValues())
|
||||
}
|
||||
|
||||
/** A present-but-empty list matches nothing, the way a relay reads `{"kinds":[]}`. */
|
||||
@Test
|
||||
fun `an empty list matches nothing`() {
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE 0 ORDER BY createdAt DESC, id DESC",
|
||||
NostrEventFilterQuery.build(SynchronizationFilter(kinds = emptyArray())).sql,
|
||||
)
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE 0 ORDER BY createdAt DESC, id DESC",
|
||||
NostrEventFilterQuery.build(SynchronizationFilter(tags = mapOf("h" to emptyList()))).sql,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unconstrained filter selects everything`() {
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent ORDER BY createdAt DESC, id DESC",
|
||||
NostrEventFilterQuery.build(SynchronizationFilter()).sql,
|
||||
)
|
||||
}
|
||||
|
||||
/** A limited filter takes the newest rows, which is the window a relay returns. */
|
||||
@Test
|
||||
fun `a limit is bound last and ordered newest first`() {
|
||||
val query = NostrEventFilterQuery.build(
|
||||
SynchronizationFilter(kinds = arrayOf(1)),
|
||||
limit = 50,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE kind IN (?) ORDER BY createdAt DESC, id DESC LIMIT ?",
|
||||
query.sql,
|
||||
)
|
||||
assertEquals(listOf(1L, 50L), query.boundValues())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `search still matches content`() {
|
||||
val query = NostrEventFilterQuery.build(SynchronizationFilter(search = "mantra"))
|
||||
|
||||
assertEquals(
|
||||
"SELECT * FROM NostrEvent WHERE content LIKE '%' || ? || '%' ORDER BY createdAt DESC, id DESC",
|
||||
query.sql,
|
||||
)
|
||||
assertEquals(listOf("mantra"), query.boundValues())
|
||||
}
|
||||
|
||||
private fun androidx.room3.RoomRawQuery.boundValues(): List<Any> =
|
||||
RecordingStatement().also { getBindingFunction().invoke(it) }.bound
|
||||
|
||||
/** Captures what the query binds, in order. Only the `bind*` calls are ever made. */
|
||||
private class RecordingStatement : SQLiteStatement {
|
||||
val bound = mutableListOf<Any>()
|
||||
|
||||
override fun bindText(index: Int, value: String) {
|
||||
check(index == bound.size + 1) { "out of order bind at $index" }
|
||||
bound += value
|
||||
}
|
||||
|
||||
override fun bindLong(index: Int, value: Long) {
|
||||
check(index == bound.size + 1) { "out of order bind at $index" }
|
||||
bound += value
|
||||
}
|
||||
|
||||
override fun bindBlob(index: Int, value: ByteArray) = unsupported()
|
||||
override fun bindDouble(index: Int, value: Double) = unsupported()
|
||||
override fun bindNull(index: Int) = unsupported()
|
||||
override fun getBlob(index: Int): ByteArray = unsupported()
|
||||
override fun getDouble(index: Int): Double = unsupported()
|
||||
override fun getLong(index: Int): Long = unsupported()
|
||||
override fun getText(index: Int): String = unsupported()
|
||||
override fun isNull(index: Int): Boolean = unsupported()
|
||||
override fun getColumnCount(): Int = unsupported()
|
||||
override fun getColumnName(index: Int): String = unsupported()
|
||||
override fun getColumnType(index: Int): Int = unsupported()
|
||||
override fun step(): Boolean = unsupported()
|
||||
override fun reset() = unsupported()
|
||||
override fun clearBindings() = unsupported()
|
||||
override fun close() = unsupported()
|
||||
|
||||
private fun unsupported(): Nothing = error("only bind calls are expected")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user