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

Merge branch 'mantra' into claude/room-db-testing-setup-b053cd

Browse Source 
Brings the branch up to date with the 40 commits mantra gained while the
jvm target was being built, so that merging the other way is a
fast-forward.

One conflict, in docs/README.md, where both sides added rows to the index
table. Kept both, and gave the jvm-target note a clause in the closing
prose since it is the one document there that is not about the protocol.

One thing the auto-merge could not have caught. `9250991` added
NostrEventDao.getMarmotGroupNostrEventsByChatRoomId as a blocking query,
which android accepts and which Room refuses to generate for any other
target -- so the merged tree failed :composeApp:compileKotlinJvm with the
same "Only suspend functions are allowed in DAOs declared in source sets
targeting non-Android platforms" that phase 4 dealt with 58 times. Made
suspend; its only caller, NostrDao.reindexMarmotGroupEvents, was already
suspend, so again no cascade.

That is now a standing cost of this branch rather than a one-off: any DAO
method added on mantra while this is outstanding will break the jvm build
on merge. It is a one-word fix each time, and the compiler names the line.

Verified on the merged tree: :composeApp:compileKotlinJvm and
:composeApp:compileDebugKotlinAndroid green,
:composeApp:testDebugUnitTest 208 passing, :composeApp:jvmTest 214
passing -- both test tasks re-run from scratch rather than taken from the
cache.

The jvm figure is larger than the android one because jvmTest inherits
commonTest, so declaring the target quietly gained the whole shared suite
a second execution environment. That is worth knowing independently of
whether desktop ever ships: the same tests now run on the host, without an
emulator.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-06 02:01:35 +02:00
parent adf1f03817 607ef72bc3
commit af81933ab4
88 changed files with 19277 additions and 663 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
docs/README.md
View File

@@ -9,9 +9,14 @@ silent, or a decision that looked arbitrary and was not.
| [shared-key-ceremony.md](./shared-key-ceremony.md) | ChillDKG over NIP-17: the rounds, the approval gates, the chat transcript, participant ordering |
| [shared-key-derivation.md](./shared-key-derivation.md) | deriving further keys from the group's threshold key with FROST tweaks — why not BIP32, why no chain code, and the one rule that must not be broken |
| [marmot-membership.md](./marmot-membership.md) | how members join an MLS group, and the epoch race that makes a missing member look like a successful invite |
| [marmot-direct-messages.md](./marmot-direct-messages.md) | a one-to-one message inside a group as a stock NIP-59 gift wrap — what its MIP-03 carve-out costs, why the sender cannot read their own, and the one query that would broadcast it |
| [mls-skipped-keys.md](./mls-skipped-keys.md) | why a group event that arrives a moment late is dropped for good, which flows trigger it, the quartz fix, and the partial mitigation in this app |
| [jvm-target.md](./jvm-target.md) | what desktop support would cost, phased — why the native chain is already done, why an empty source set in our phoenix fork is the real blocker, and why DAO tests do not need any of it |
| [long-running-sync.md](./long-running-sync.md) | the chat subscriptions that stay open instead of pulling once per screen — why the request queue could not simply hold one, and how the group filter follows the room list |
| [dead-code.md](./dead-code.md) | code in the sync and relay stack that nothing calls, why each piece is still there, and which of it is a bug rather than a leftover |
| [jvm-target.md](./jvm-target.md) | what desktop support cost, phased — why the native chain was already done, why an empty source set in our phoenix fork was the real blocker, and why DAO tests need none of it |
Start with the ceremony if you are new to this area; the other two both assume it.
Start with the ceremony if you are new to this area; the Marmot notes all assume it.
Read the skipped-keys note before debugging any "the other device never got it"
report — it is silent, and it looks like every other kind of delivery failure.
report — it is silent, and it looks like every other kind of delivery failure. The
sync note stands alone, and the dead-code inventory reads as a follow-up to it. The
jvm-target note is unrelated to all of them: it is a build and packaging story.

198
docs/dead-code.md Normal file
View File

@@ -0,0 +1,198 @@
# Unreferenced code in the sync and relay stack
An inventory of code nothing calls, found while building the long-running chat
sync (see [long-running-sync.md](./long-running-sync.md)). Some of it was
orphaned by that change; most of it was already dead and only became visible
because the subsystem was being read closely.
None of it is removed yet. It is written down rather than deleted because
several items are one decision away from being wanted, and that decision is not
the sync change's to make.
## How this was checked
Every claim below is "the identifier appears exactly once in
`composeApp/src`, at its own declaration", verified with a script that tokenises
every `.kt` file outside `build/` and counts occurrences:
```bash
grep -rn "\bidentifierName\b" --include=*.kt composeApp/src | grep -v '/build/'
```
Line numbers below were correct at `f5eb744` and drift with every edit to those
files; treat them as a hint and confirm with the grep.
Two things that method cannot see, so each item was also read in context:
- **Room DAOs** are called through generated code as `database.xDao().method()`,
so the method name does appear at the call site and a genuinely unused DAO
query still counts as one occurrence. The DAO section below is therefore the
least certain.
- **Compose** entry points and `@Composable` functions invoked from `setContent`
or navigation graphs may be reached without a textual reference.
## Orphaned by the live-sync change
**`ChatRoomListViewModel.scheduleSynchronization`** —
[ChatRoomListViewModel.kt:80](composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomListViewModel.kt:80)
No longer called by `initiate()`. It was kept as the entry point for a
user-initiated refresh, which is the one thing the live tier does not answer —
but no refresh affordance exists in the UI, so today it is unreferenced.
It also now duplicates `LiveSubscriptionManager.queueCatchUpSynchronization`,
which queues the same two negentropy requests on every foreground, including the
first one after launch. Drift between the two is harmless (a negentropy request
is keyed on a hash of its filter, and both are valid reconciliations), but there
is no reason for two copies.
**Decide one of:** wire a pull-to-refresh to it, or delete it. Deleting it also
orphans `nostrRepository` in that view model, which ripples into its `factory`
and into `HomeScreen` — the reason it was left alone rather than removed in the
same commit.
## Dead on arrival, in the relay stack
Each of these predates the sync work.
| what | where | note |
|---|---|---|
| `RelayPool.removeRelays` | [RelayPool.kt:123](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:123) | never called; `changeRelays` and `closePool` cover every path that removes a relay |
| `RelayPool.hasRelays` | [RelayPool.kt:171](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:171) | never called |
| `RelayPool.transformWhileEventsAreIncoming` | [RelayPool.kt:486](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:486) | private, never called. Superseded by `completeOnSubscriptionEnd`, which ends a flow on EOSE/CLOSED/NEG-ERR rather than on "the last message was not an event" |
| the commented-out publish gate | [RelayPool.kt:547-560](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:547) | the only thing keeping `import kotlinx.coroutines.flow.transform` ([:30](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:30)) alive |
| `RelaysSocketManager.clearRelayPools` | [RelaysSocketManager.kt:100](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:100) | private, never called. Nothing tears the pool down on sign-out |
| `RelaysSocketManager.tryConnectingToAllRelays` | [RelaysSocketManager.kt:131](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:131) | never called; the only caller of `RelayPool.tryConnectingToRelay`, which is otherwise dead too |
| the commented `tryConnectingToUserRelay` | [RelaysSocketManager.kt:139](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:139) | |
| `NostrIncomingMessage?.verifyOrThrow` | [NostrIncomingMessage.kt:59](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrIncomingMessage.kt:59) | never called. It also treats any NOTICE as a failure, which is wrong for the reason `filterByEventId` documents — a NOTICE has no subscription id and reaches every collector |
| `String?.decodeFromJsonStringOrNull` | [CommonJson.kt:37](composeApp/src/commonMain/kotlin/press/mantra/compose/network/serialization/CommonJson.kt:37) | never called |
| `NostrSocketClientImpl.compressMessage` | [NostrSocketClientImpl.kt:368](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:368) | already carries `@Suppress("unused")`. Outgoing compression is not a thing any relay here asks for |
| `DefaultNostrSocketClientFactory.create(httpClient = …)` | [NostrSocketClientFactory.kt](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientFactory.kt) | the overload taking an explicit `HttpClient` has no caller; everything goes through the interface method and `defaultSocketsHttpClient` |
### `RelaysSocketManager.userRelays` is a field that is never written
[RelaysSocketManager.kt:52](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:52)
Worth separating from the table, because it does not read as dead — it reads as
a bug.
```kotlin
private val userRelays = mutableSetOf<RelayDTO>() // nothing ever adds to this
```
The `userRelays` at [:83](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:83)
is a *different, shadowing local* inside `observeRelays`. The field itself is
only ever read, at [:107](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:107):
```kotlin
return if (userRelays.isNotEmpty()) { ...publish to the user's relays... }
else { ...publish to FALLBACK_RELAYS... }
```
So the single-argument `publishEvent` **always** takes the fallback branch, and
the user's own relay list is never used for publishing. Either populate the
field from `updateRelayPools` or delete it and the branch; leaving a permanently
false condition in place is the one option that keeps the bug.
### The unused halves of the socket protocol
`NostrSocketClient` declares five send methods. Only `sendEVENT` and
`sendMESSAGE` are ever called — everything else in the pool serialises its own
command with `OptimizedJsonMapper` and goes through `sendMESSAGE`.
| declaration | impl |
|---|---|
| `sendREQ` [:37](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:37) | [:349](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:333) |
| `sendCLOSE` [:31](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:31) | [:361](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:345) |
| `sendCOUNT` [:33](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:33) | [:354](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:338) |
| `sendAUTH` [:29](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:29) | [:365](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:349) |
Removing them also orphans `buildNostrREQMessage`, `buildNostrCOUNTMessage`,
`buildNostrCLOSEMessage` and `buildNostrAUTHMessage` in
`NostrOutgoingMessageBuilder`.
Two of the four are worth keeping for a named reason rather than out of habit:
- **`sendAUTH` is the NIP-42 hole.** `NostrIncomingMessage.AuthMessage`
([:34](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrIncomingMessage.kt:34))
is parsed and then dropped — nothing anywhere handles an AUTH challenge. A
relay that answers CLOSED with `auth-required` is currently retried with
backoff forever and will never succeed. `sendAUTH` is half of the fix, so
delete it only alongside a decision not to support authenticated relays.
- **`sendCOUNT` and `CountMessage`** ([:38](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrIncomingMessage.kt:38))
are a matched pair with no caller. `CountMessage` is parsed and admitted by
`filterBySubscriptionId`, but since nothing sends a COUNT it can never arrive.
Both go, or neither.
`sendREQ` and `sendCLOSE` have no such argument — the pool does not use them and
would not start.
## Dead all the way down: the publisher chain
**`NostrPublisherRepository`** —
[NostrPublisherRepository.kt:15](composeApp/src/commonMain/kotlin/press/mantra/compose/repository/NostrPublisherRepository.kt:15)
The whole class is unreferenced: nothing constructs it, and neither of its two
methods (`signPublishImportNostrEvent`, `publishRelayList`) is called. Publishing
goes through the broadcast queue in `SynchronizationViewModel` instead.
It matters beyond itself because it is the **only** consumer of
`CachingImportRepository.importEvents`. `RelayPool` and `RelaysSocketManager`
both take a `cachingImportRepository` constructor parameter, store it, and never
dereference it — and `SynchronizationViewModel` satisfies that parameter with
`NO_OP_CACHING_IMPORT_REPOSITORY`. So the entire caching-import seam is a
parameter passed from nowhere to nothing.
Removing `NostrPublisherRepository` and the two unused constructor parameters
would let `CachingImportRepository` go with them.
## Dead columns (need a migration)
`isRecommendedRelay` on both
[SynchronizeNostrEventRequest.kt:40](composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/SynchronizeNostrEventRequest.kt:40)
and
[NegentropySynchronizeRequest.kt:39](composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/NegentropySynchronizeRequest.kt:39)
is never written by any caller (every construction takes the `false` default)
and never read.
A different risk class from everything else here: these are Room columns, so
removing them is a schema change with a migration, not a delete. Cheap to leave;
worth folding into the next migration that touches those tables.
## Wider sweep, needing per-item judgement
The same sweep across all of `composeApp/src` turns up ~99 single-occurrence
function declarations. Most are outside the sync stack and were not read in
context. Two clusters stand out and are recorded here so the next person does
not have to re-derive them:
- **Unused DAO queries** — roughly 30, concentrated in
`BroadcastNostrEventReceiptDao`, `BroadcastNostrEventRequestDao`,
`UnsignedNostrEventDao`, `RepostedRelationDao`, `QuotedRelationDao`,
`InReplyToRelationDao`, `ReactionDao` and `ZapDao`. Mostly
`getAllX()`/`getXById()` shapes that look like they were written to round out
a DAO rather than because something needed them. Cheap to delete and cheap to
re-add, but see the Room caveat above: confirm each against the generated
implementation, not just by grep.
- **Bech32/credential helpers** in
[Credentials.kt](composeApp/src/commonMain/kotlin/press/mantra/compose/extensions/Credentials.kt) —
`assureValidNpub`, `assureValidPubKeyHex`, `hexToNoteHrp`, `urlToLnUrlHrp`,
`bechToBytesOrThrow`. A conversion toolkit where only some conversions are
used. Worth keeping as a set if the intent is a complete encoding surface;
worth deleting if not.
## Suggested order, if this gets done
1. `NostrPublisherRepository` + the two `cachingImportRepository` parameters +
`CachingImportRepository`. Largest reduction, zero behavioural risk, and it
removes a constructor parameter from two classes at the centre of the relay
stack.
2. The relay-stack table above, plus the two commented-out blocks and the
`transform` import they keep alive.
3. `userRelays` — but as a **fix**, not a deletion, unless publishing to the
user's own relay list is deliberately not wanted.
4. `sendREQ`/`sendCLOSE`, keeping `sendAUTH` and `sendCOUNT` pending the NIP-42
and COUNT decisions.
5. `ChatRoomListViewModel.scheduleSynchronization`, once there is a refresh
affordance or a decision that there will not be one.
6. `isRecommendedRelay`, with the next migration that touches those tables.

254
docs/long-running-sync.md Normal file
View File

@@ -0,0 +1,254 @@
# The long-running chat sync
Chat messages arrive because a subscription is open, not because a screen asked.
This note is what that means, what it replaced, and the handful of decisions
inside it that look arbitrary and are not.
Two subscriptions carry conversation, and they are held open for as long as the
app is in the foreground:
- **kind 1059**, NIP-59 gift wraps p-tagged to us — DMs, and the Marmot Welcome
events that make us a member of a new group.
- **kind 445**, Marmot group events h-tagged with every group id we belong to.
The code is [`LiveSubscriptionManager`](composeApp/src/commonMain/kotlin/press/mantra/compose/managers/LiveSubscriptionManager.kt),
built and scoped by `SynchronizationViewModel`.
## What this replaced
Every chat sync used to be a *pull*. A screen queued a request row, a pump in
`SynchronizationViewModel` drained it, the relay answered, the subscription was
closed at EOSE. Nothing arrived between pulls, so a message sent one second after
EOSE waited for the next time someone opened a screen.
| screen | queued |
|---|---|
| `ChatRoomListViewModel.scheduleSynchronization` | negentropy over `1059 #p=[me]`, then over `445 #h=[every group id]`, per DM relay |
| `ChatMessageListViewModel.scheduleSynchronization` | for an MLS room, negentropy over `445 #h=[this group]`; for a NIP-17 room, gift wraps p-tagged at the peer |
Neither runs on open any more. The first stays as the entry point for an
explicit user-initiated refresh — the one thing the live tier does not answer,
because it is the user saying they believe something is missing. The second now
does discovery only: if we do not hold a participant's kind-10050 we cannot
address a message to them, and that is worth resolving when a chat is opened
rather than whenever a background pass reaches it.
## Why the old pipeline could not simply stop closing
Five properties, each of which had to be undone deliberately. None was wrong;
all of them assume a subscription is short.
**1. EOSE ends the flow.** `completeOnSubscriptionEnd` is what makes a collector
finish at all — the socket's `incomingMessages` is hot, and a filtered view of it
never completes on its own. `RelayPool.openLiveSubscription` is `query()` without
it: for a live subscription EOSE is only the boundary between stored history and
the live tail, and CLOSED is the only genuinely terminal message.
**2. Every subscription was force-killed at 120 seconds.** `SUBSCRIPTION_TIMEOUT`
exists so a relay that goes quiet cannot park a slot forever. It applies to the
queue, and nothing else.
**3. There are four subscription slots, total, across all relays.**
`subscriptionSlots` is a `Semaphore(4)`. A permanently-open subscription is a
permanently-held permit, so the live tier deliberately does not draw from it. The
two budgets have to be read together against what a relay tolerates per
connection (commonly ~20), not separately.
**4. The queue row was the subscription identity and the provenance record.** Both
original `saveNostrEvent` overloads take a request object, use its `relayURL` and
`level`, and flip its status to "processed". A live subscription has no row and
never finishes, so there is a third overload carrying relay and level itself.
**5. Nothing reconnected.** `NostrSocketClientImpl` caught a failure, called
`close()`, and fired a callback that only flipped a boolean nobody read. That was
survivable *only* because every subscription was short: the next queued request
opened a fresh socket on its way out. See "Reconnecting" below.
There was a sixth, quieter one. `_incomingMessages` was a rendezvous
`MutableSharedFlow` emitted into from a coroutine launched per message, so
messages reached collectors in scheduling order rather than wire order, and an
emit with every collector busy blocked on the slowest. It now has a 256-message
buffer and is emitted into inline, on the reader. That also retired a 75ms sleep
before every EOSE, which existed to *hope* the events preceding it had already
been delivered.
## How it holds together
### Reconnecting
A socket that drops now comes back on its own, with exponential backoff (1s
doubling to 60s, plus up to 25% jitter — every relay drops at once when the
network does, and without jitter they return in lockstep). `autoReconnect` is off
by default and owned by the pool, which turns it on for exactly as long as it is
retaining a subscription for that relay: reconnecting a socket nobody is
subscribed on is battery spent on nothing.
`RelayPool` retains the REQ text per `(relay, subscription id)` and replays it
when that relay's socket is re-established. A relay answers a repeated REQ on the
same id by *replacing* that subscription's filter, so replay is a send rather
than a close-and-reopen — and the collector, attached to the socket **client**
rather than to a session, simply starts receiving again. A live subscription
therefore needs no drop handling of its own.
Negentropy is deliberately not retained. NEG-OPEN carries a fingerprint of the
local set and each round depends on the last, so replaying one mid-exchange would
reconcile against a conversation the relay is no longer having.
### The filters, and their timestamps
The two kinds need opposite treatment, and it is visible in our own outbound code.
Gift wraps are stamped with `TimeUtils.randomWithTwoDays()`
([`DatabaseChatRepository.kt:344`](composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt:344)),
so a wrap published *now* can carry a `created_at` two days old. **A `since`
anywhere near the present would silently drop a large fraction of genuinely new
messages** — and "some DMs just don't arrive" is the worst failure mode to debug.
There is none.
Group events use `TimeUtils.now()`
([`MarmotOutboundDao.kt:578`](composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt:578)),
so a watermark would be safe there. It still is not used: `limit` already bounds
the initial burst, and a watermark would have to be recomputed every time a
chunk's membership changed.
`limit` does the bounding instead, because NIP-01 scopes it to the initial query
— the stored events sent before EOSE — and explicitly not to the stream that
follows. So it caps what a reconnect costs without touching the live tail at all.
100 for gift wraps; 500 for a group chunk, which covers up to 100 conversations.
`#h` is chunked 100 group ids to a subscription. One subscription per group would
be simpler, but every Marmot group here lives on the same DM relay set, so
carrying many ids in one filter costs a fraction of the subscriptions.
### Following group membership
The group filter is derived from `chatRepository.observeChatRoomListByPublicKey`
— the same Flow the chat list screen renders, collected at wallet scope so it
keeps running with no chat screen open. Rooms are filtered to those with MLS
state (a NIP-17 room has none and is served by the gift wrap subscription), minus
any we have left or deleted; ids are sorted, de-duplicated with
`distinctUntilChanged`, and debounced 500ms.
Deriving it from the table rather than from the join sites is the decision that
keeps this correct, because a group id can appear four ways and only one of them
is somewhere anyone would think to call a subscribe function:
- we create a group (`MarmotOutboundDao`),
- we are added to one — the Welcome arrives inside a gift wrap and is processed
deep inside `NostrDao.storeNostrEvent`, on the *inbound* path,
- membership shifts under us via a commit
(`MarmotInboundManager.processGroupMembershipChanges`),
- we leave, or the room is deleted.
It also closes the loop: **a Welcome lands on the gift wrap subscription → a
`ChatRoom` row is written → the Flow re-emits → the group filter widens → the
first message in that new group arrives without anyone opening a chat.**
Reconciling is an update, not a rebuild. A chunk that already has a subscription
gets a repeated REQ under the same id, so adding a group does not interrupt
delivery on the groups already being watched. Chunks that no longer exist are
`cancelAndJoin`ed — the CLOSE goes out from the coroutine's `finally`, and
returning before it lands would let a later reconcile open a subscription on that
id which the old one then closes out from under.
### Lifecycle
`AppLifecycle` is a singleton `StateFlow<Boolean>` fed by a `LifecycleEventObserver`
in `MantraNavHost`. A singleton rather than something threaded through the
composition, because the consumers are application-scoped coroutines that outlive
any screen. It defaults to foreground: on a platform where the observer is not
wired up, "always on" is the behaviour that predates it, and a subscription that
never opens is far worse than one that stays open too long.
`collectLatest` over that flow is the whole mechanism. Backgrounding cancels the
block holding the subscriptions and each `finally` sends its CLOSE.
Returning does two things before anything else:
1. **`RelayPool.reconnectAll`** tears every socket down and immediately rebuilds
it. Trusting the connection is the mistake here — a socket that was open when
the OS suspended the process reports itself connected on the way back while
being functionally dead. Rebuilding eagerly rather than leaving it to the next
send is what makes it a *re*-connection, so retained subscriptions are replayed
and collectors still attached from before the gap resume.
2. **A catch-up reconciliation.** A live subscription answers "what is new since I
connected"; negentropy answers "what do you have that I don't". A background
gap is exactly the question only the second can answer — `limit` on the
re-opened subscriptions is a window, not a guarantee.
### What negentropy is still for
Live subscriptions replaced *polling*, not *reconciliation*. The queue and its
three pumps are unchanged, and negentropy remains the right tool for first
login, the foreground catch-up, "load older messages", and repairing a relay that
was unreachable while we were online.
## Two things found while building this
**The `"sent-messages"` filter could never match anything.** It asked for kind
1059 with `authors=[userPublicKey]`, but gift wraps are signed with a fresh
throwaway `KeyPair()`, so the pubkey is random and never ours. It was also
unnecessary: `createNip17ChatRoom` puts the user in their own participant list, so
we wrap a copy to ourselves and the account-wide `#p=[me]` subscription picks our
own sent messages up on every device. Deleted, in the view model and at both
sites in `NostrDao`.
**The negentropy `saveNostrEvent` wrote outside `storeNostrEventMutex`** while
the other overload held it. `storeNostrEvent` reads an event and then writes it
and its indexes, so two of those interleaving is a lost update — survivable while
a single queue was the only writer, not survivable with a live subscription
writing alongside a backfill.
## What is tested, and what is not
Every failure mode in this subsystem is silent — a filter that drops messages, a
subscription that stops being replayed, a group id that never reaches an `#h`
tag. None of them throws, and all of them present as "some messages didn't
arrive", later, on someone else's phone. So the tests are aimed at the decisions
rather than at the plumbing:
| file | pins |
|---|---|
| `RelayPoolSubscriptionTest` | retain on query, replay on reconnect, release on close, never replay negentropy, per-relay scoping — and that a live subscription keeps delivering after EOSE while a one-shot query still ends at it |
| `LiveSubscriptionReconcileTest` | the group filter following membership: widening in place rather than reopening, dropping a left group, closing when the last one goes, collapsing churn inside the debounce, and the CLOSED/back-pressure reopen behaviour |
| `LiveSubscriptionPlanTest` | the filter shapes, led by `since` being absent from the gift wrap filter |
| `RelayBackPressureTest`, `ReconnectBackoffTest` | the two pure decisions: which CLOSED reasons mean "ease off", and the backoff arithmetic including the exponent clamp |
They were checked by mutation rather than by passing: reintroducing a `since`,
making EOSE terminal, dropping the `leftGroupAt` filter, removing retention from
`query()`, reconciling by close-and-reopen, removing the debounce and removing
the back-pressure branch each produce failures.
Three things this does **not** cover, and could not without a real relay or a
database:
- **`NostrSocketClientImpl` itself.** The reconnect loop, the session-identity
check on teardown, and the ordered inbound emission are exercised only through
their extracted arithmetic. Testing them wants a fake `WebSocketSession`.
- **Anything downstream of `saveNostrEvent`.** Indexing, gift wrap unwrapping,
Welcome handling and MLS decryption are Room-backed, and there is no sqlite
driver on the JVM test classpath (see the note in `build-verification-commands`).
The tests assert the event reaches the repository with the right relay and
level, and stop there.
- **The app on a device.** Nothing here proves a DM lands with the chat list
closed; that still wants the two manual checks — send yourself a message from
another client, and have a second device add you to a group.
## Not done
- **Connectivity changes.** A network switch mid-foreground is only noticed by
the socket's own reconnect loop, which handles the common case but cannot know
the network changed underneath it. Wisp's `RelayLifecycleManager` is the model:
observe connectivity, debounce, and suppress a network-change reconnect shortly
after a resume so the two do not fire twice.
- **NIP-42 AUTH.** A relay that answers CLOSED with `auth-required` is treated as
any other refusal and retried with backoff. It will never succeed. Worth
knowing before pointing this at a relay that requires auth.
- **A router.** The original design called for one permanent collector per socket
dispatching by subscription id prefix. With the inbound flow buffered and only a
handful of live subscriptions per relay, a collector per subscription has the
same properties for less machinery. If that count grows, the router is the next
move.
- **`Relays.DefaultDMRelayList` is one relay.** Every per-relay budget above is
currently a budget of one socket, which makes this cheap and makes a single
relay outage total.

371
docs/marmot-direct-messages.md Normal file
View File

@@ -0,0 +1,371 @@
# Direct messages inside a Marmot group
A one-to-one message carried as an MLS application message: an ordinary NIP-59 gift
wrap, ephemeral-keyed and signed the way NIP-59 says, addressed to one member and
never broadcast to a relay. Every member of the group sees that a private message
was sent and to whom. Only the recipient can read it.
Read [Why a throwaway key, and what it costs](#why-a-throwaway-key-and-what-it-costs)
before changing anything here. Two consequences of that choice reach into every
part of this, and one of them cannot be undone.
## What a direct message is, outside in
```
kind:445 GroupEvent ephemeral signer, h-tag = nostrGroupId → relays
└─ ChaCha20-Poly1305 MLS exporter secret → group
└─ MLS PrivateMessage ContentType.APPLICATION → group
│ sender authenticated by MLS leaf ────────┐
└─ kind:1059 wrap pubkey = throwaway, signed by it │ → group
└─ NIP-44 conversation key: throwaway ↔ recipient │
└─ kind:13 seal signed by the sender ───── bound to ─────┘
└─ NIP-44 → recipient
└─ kind:14 rumor → recipient
```
The group reaches the fourth layer and stops. Everything above it is what a
bystander renders from: a recipient, a time, a wall — and a sender who comes from
the MLS frame rather than from anything written in the wrap.
The two outer layers are unchanged. A direct message is an ordinary kind:445 to
anyone watching a relay and an ordinary application message to `MlsGroup`; the
whole feature is what goes in as the application payload.
`MarmotDirectMessage` builds and opens it, as pure functions of their arguments
with no database and no MLS state — which is what makes it testable, since
Room-backed code cannot be unit-tested in this project.
## Why a throwaway key, and what it costs
The wrap uses a fresh random key and is signed by it, exactly as NIP-59 specifies
and exactly as `GiftWrapEvent.create` builds one. Nothing in the wrap names the
sender.
**What it does not buy: hiding the sender from the group.** MLS authenticates every
application message to a leaf. `mlsGroup.memberIdentityHex(decrypted.senderLeafIndex)`
yields the sender's real pubkey no matter what the inner event claims, and there is
no way to send an application message that does not. The throwaway key removes a
*redundant* copy of an identity the MLS frame already proves — it does not remove
the identity.
**What it does buy:** a wrap lifted out of its MLS frame — a log line, a database
export, a crash dump — is not attributable to anyone. It is also a well-formed
NIP-59 gift wrap rather than a Marmot-shaped variant, which is worth something to
anyone reading the payload with ordinary nostr tooling.
Two costs follow, and both are load-bearing.
### It requires a carve-out in a security check
`MarmotInboundManager.processPrivateMessage` rejects any inner application event
whose `pubKey` is not the MLS sender's credential identity:
```
MIP-03: inner event pubkey (…) does not match MLS sender identity (…)
```
That is MIP-03, not local policy, and it exists so a member cannot mint events
claiming a different author. A throwaway-keyed wrap cannot satisfy a check about
its author, so kind:1059 is exempt from it:
```kotlin
if (innerEvent.kind != GiftWrapEvent.KIND && innerEvent.pubKey != senderIdentity) {
return GroupEventResult.Error(groupId, "MIP-03: inner event pubkey …")
}
```
`senderIdentity` is now *required* rather than merely compared. Under the old check
a null identity failed only because the comparison failed; attribution depends on
it outright, so it fails on its own.
**The check is not weakened, it is relocated and strengthened.** What replaces it
for kind:1059 is `seal.pubKey == senderIdentity` on a seal whose `verify()` passes
— a signature bound to an MLS leaf, rather than a plaintext field compared to one.
The attack it stops is a member re-wrapping a seal they were legitimately sent and
passing it to a third party as its author's; that fails because the MLS frame says
who actually sent *this* one. Forging one outright needs the other member's private
key.
A seal is harder to relabel than it looks, and not because of the signature.
NIP-44 derives the conversation key from the pubkey being claimed, so writing
another member's key over a seal makes it undecryptable by the person it was
encrypted for — it fails at the decryption, before any check runs, and comes out as
a message the recipient simply cannot read. The label is bound to the key rather
than asserted alongside it. `verify()` is what catches the remaining case: a seal
altered after signing in a way that survives decryption. Both have tests, because
the first was asserted the wrong way round until one of them failed.
> **Compatibility.** Every member's client needs this carve-out. A Marmot client
> implementing MIP-03 as written drops these messages as impersonation — silently,
> as a `GroupEventResult.Error` — so a group with one unpatched member has one
> member who never receives a direct message and is never told why. This is a
> divergence from the spec, and a reason to raise it upstream rather than carry it
> indefinitely.
### The sender cannot read their own messages back
The throwaway private key is discarded at send time, so nothing can reopen the wrap
afterwards — not even the person who built it. NIP-17 solves this by sending a
second wrap addressed to yourself; here that would be a second application message,
and so a second *"sent a private message"* line in everyone else's transcript, so it
is not available.
What follows:
- The sending device keeps its own copy, because `sendChatMessage` writes the
plaintext `ChatMessage` row on the way out. Normal use is unaffected.
- A second device, or a reinstall, gets **nothing** — the sender's own half of
every conversation is unreadable to them anywhere it was not typed. The
recipient's half is unaffected.
- On a re-sync the sender's own message comes back as an opaque wrap they cannot
open, indistinguishable from a bystander's view. `ChatMessage.directMessage`
guards that case explicitly and files nothing, because a bystander line would
replace the words on the row `sendChatMessage` wrote — the only copy there is.
`NostrDao.persistInboundChatMessage` carries the same guard, for the same reason,
on the NIP-17 path.
Do not solve this by storing the throwaway private key. A per-message private key at
rest is strictly worse than the identity-keyed wrap it was chosen over, and it
reintroduces the attribution the throwaway key exists to remove. There is a test —
`the sender cannot reopen their own message` — whose only job is to make that
reversal fail loudly rather than ship.
## The one way to broadcast this by accident
Nothing sweeps events to relays on its own. Broadcast is driven by
`BroadcastNostrEventRequest` rows joined to `NostrEvent`, inserted explicitly. So
the requirement "this must never be broadcast" reduces to one query:
```sql
SELECT * FROM GiftWrapPayload WHERE publicKey = :publicKey AND giftWrapSealId IS NULL
```
`GiftWrapPayloadDao.observeUnsealedGiftWrapPayloads`. The notary watches it, and
`DatabaseChatRepository.sealGiftWrapPayload` seals every row it returns and hands it
to `NostrNip17Dao.persistAndBroadcastGiftWrap` — which inserts a
`BroadcastNostrEventRequest` per relay. **Write one `GiftWrapPayload` row authored
by the local user with a null seal id and the direct message leaves the device.**
Hence the rule:
> A Marmot direct message never writes to `GiftWrapPayload` or `GiftWrapMessage`
> at all. Its outbound queue is `MarmotInnerEvent`, and so is its inbound record.
The risk is sharper here than it would be with a Marmot-shaped payload, because what
this builds is a genuine, well-formed, correctly signed NIP-59 gift wrap. It is
indistinguishable from something the NIP-17 path would be right to publish. Nothing
but the tables it is kept out of stops it going to a relay.
`GiftWrapMessage` could not be written anyway without a `NostrEvent` row — its
foreign key — and `NostrEvent` is the broadcast join target. The rule is also
enforced at the other end: `sealGiftWrapPayload` refuses any payload whose room has
a non-null `mlsGroupState`, and logs. An invariant in code is what stops a later
refactor from walking a direct message onto a relay without reading this page first.
**The Welcome is exempt, and the exemption is not optional.** MIP-02 addresses
kind:444 to someone who is not yet in the group and holds no key to read a
kind:445 — a relay-borne gift wrap is the only way to reach them, and
`MarmotOutboundDao.deliveryWelcome` writes exactly such a `GiftWrapPayload` row on
purpose. Every Welcome carries its own room's `nostrGroupId`, so a refusal keyed on
`mlsGroupState` alone matches all of them: while it did, no invite to any Marmot
room was ever delivered, and — since the unsealed queue is a single-row flow — the
first refused Welcome sat at its head and blocked every payload queued behind it.
The check is therefore on kind first, room second. When adding a kind to that
exemption, the question to answer is whether its recipient can read a kind:445; if
they can, it does not belong on this path.
## Attribution comes from MLS, not from the payload
Because nothing in the wrap names the sender, every sender-derived field is read
from the MLS frame. This is not a workaround; it is the correct source, and it is
what makes the carve-out above safe.
`GroupEventResult.ApplicationMessage` carries `senderLeafIndex` but not the resolved
identity, and `ChatMessage.fromGroupEventResult` has no `MlsGroup` to resolve it
with. The obvious fix — adding `senderIdentity` to the result — is not available:
quartz is a **binary dependency** here (`com.vitorpamplona.quartz:quartz:1.14.0`),
and the local checkout at `~/Documents/development/nostr/amethyst/quartz` is a
reference copy, not a build input. Changing the result type would mean publishing a
fork.
So `NostrDao` resolves it at the call site instead and passes it in. It holds the
group, the leaf index is already on the result, and an application message advances
no epoch, so the tree has not moved by the time it reads it. Same value, no fork.
This also settled a bug that predates the feature. All fourteen arms of
`fromGroupEventResult` computed:
```kotlin
isUserMessage = activeKeyPair.pubKey.toHex() == groupEvent.pubKey
```
`groupEvent.pubKey` is the kind:445's signer, and that is a fresh random ephemeral
key on every send — `NostrSignerInternal(KeyPair())` in
`encryptAndSendMarmotInnerEvent` — so it could never equal the active key. It was
false for every Marmot message in every room, which meant every message a member
sent themselves rendered as somebody else's: wrong side of the transcript, wrong
colour, and no delivery status, which is drawn only for our own lines. All fourteen
now read `senderIdentity`. For the four results that carry no leaf index — the
commit and proposal statuses — it is null and yields false, exactly as before.
## The message's identity is the rumor id
The queued `MarmotInnerEvent` for a direct message **is the rumor** — kind:14,
plaintext, p-tagged — so its id is a hash of exactly the fields the recipient will
have after unwrapping.
This matters in two places. It keeps the outbound link intact, which is the
difference between a message being sent and silently not being sent (see
[Two bugs this uncovered](#two-bugs-this-uncovered)). And it gives the recipient a
stable id for dedupe across redelivery and re-sync.
It does **not** give sender and recipient a shared identity for the same message the
way an identity-keyed wrap would, because the sender never re-derives the rumor from
an echo they cannot open. Two devices belonging to the sender do not converge; they
simply do not both have the message.
The wire event — the wrap — has its own id, so a direct message leaves two rows on a
recipient's device: one for the artifact that travelled and one for the message it
carried.
## Timestamps are not fuzzed
NIP-59 randomises the wrap's and the seal's `created_at` by up to two days to
frustrate correlation at a relay, and both `GiftWrapEvent.create` and
`SealedRumorEvent.create` default to `TimeUtils.randomWithTwoDays()`.
The wrap never reaches a relay, and the kind:445 around it already carries the true
time. Fuzzing would do nothing but scatter the *"sent a private message"* line up to
two days out of position in every other member's transcript. `MarmotDirectMessage`
passes the real message time to all three layers — a default to override, not one to
accept, and the easiest thing here to get wrong by omission. There is a test for it.
## Three decisions
| decision | taken | what it costs |
|---|---|---|
| Is the recipient visible to the group? | yes — the `p` tag stays on the wrap | The group learns who messages whom, and how often. Omitting the tag is genuinely cheap here: the recipient decrypts against the wrap's own throwaway pubkey, so they need no tag to find their own mail, and one failed NIP-44 decrypt per member per message is the whole cost. What it takes is the named bystander line — *"sent a private message"* with no recipient — and any ordering by conversation. Worth revisiting; not free, but close. |
| Is the wrap signed? | yes — by the throwaway key, per NIP-59 | Nothing, which is the point. The signer is meaningless and discarded, so the signature attributes nothing, and keeping it means the payload is a real NIP-59 gift wrap that `GiftWrapEvent.create` builds and `unwrapOrNull` opens with no special cases. |
| What kind is the rumor? | `ChatMessageEvent.KIND` (14) | Group messages here are kind:9 (`ChatEvent`) — `sendChatMessage` maps 14 → 9 on the way in for an ordinary message and leaves a direct message at 14. It is a NIP-17 chat message that happens to travel inside a group, and the kind is what tells the two apart on the way back in. Costs one more arm in the inbound switch. |
## The path a message takes
**Out.** `sendChatMessage` resolves the recipient against the room's participants —
refusing a non-member, whose wrap nobody could open, and refusing yourself, whose
wrap could never be read back — and writes two rows: the queued `MarmotInnerEvent`
(the rumor) and the `ChatMessage` holding the plaintext. The notary picks the queued
row up, `MarmotOutboundDao` calls `MarmotDirectMessage.wrap` instead of assembling a
bare rumor, and everything downstream is unchanged. Once sent, the plaintext is
scrubbed from the queued row: it is already on the `ChatMessage`, and a second copy
would be cleartext left in a table that otherwise holds nothing but wire events.
**In.** `MarmotInboundManager` decrypts and resolves the sender's identity, and
`MarmotDirectMessage.classify` decides what the wrap is to this device — ours,
readable, unreadable, or rejected. `ChatMessage.directMessage` turns that into rows.
The split is so the decision can be tested: only the filing needs a database, and
Room-backed code cannot be unit-tested here. The recipient opens the wrap,
validates the seal against `senderIdentity`, stores the rumor as its own
`MarmotInnerEvent` keyed on the rumor's id, and gets the words. A bystander cannot
open it and gets a line with no content. The sender gets nothing, because their
local row is the only copy.
A failed validation drops the message and logs; it does not throw. The caller is
inside `storeNostrEvent`'s transaction, and a forged direct message should cost its
own line, not the whole event.
**On screen.** Tapping somebody else's message in a group offers *Reply privately*,
which arms the composer; sending clears it. An armed composer shows a chip naming
the recipient, a placeholder that says who it is going to, and a tinted field, and
the chip's close button is the one tap back to the room. Three signals rather than
one because the failure this guards against — sending to the room what was meant for
one person, or the reverse — cannot be taken back once it is on the wire.
## Two bugs this uncovered
### The wrap's id is not the rumor's
`encryptAndSendMarmotInnerEvent` found the chat message to link and broadcast with
`getChatMessagesByMarmotInnerEventId(innerEvent.id)` — the *recomputed* id. For a
direct message that is the wrap's id, not the queued rumor's, so the lookup returned
null, the `ChatMessage` was never linked, **and no `BroadcastNostrEventRequest` was
ever inserted**. The message would have been encrypted, persisted, and never sent,
with no error anywhere.
It now keys on `marmotInnerEvent.id` — what `sendChatMessage` wrote into
`ChatMessage.marmotInnerEventId`, and identical to `innerEvent.id` for every
non-direct message, so nothing else moved.
### `isUserMessage` was always false for Marmot messages
Covered under [Attribution comes from MLS](#attribution-comes-from-mls-not-from-the-payload).
It stopped being merely cosmetic here: with nothing in the payload naming the
sender, `senderIdentity` is the only source of attribution there is.
## Where it lives
| file | what |
|---|---|
| `nostr/MarmotDirectMessage.kt` | wrap / open, the pure seam both directions call |
| `commonTest/.../MarmotDirectMessageTest.kt` | the envelope: ten cases, against real secp256k1 and real NIP-44 |
| `commonTest/.../MarmotDirectMessageDeliveryTest.kt` | what a device does with an arriving wrap, including every forgery it must refuse |
| `commonTest/.../MarmotMip03CarveOutTest.kt` | the kind:1059 exemption and its edges |
| `managers/MarmotInboundManager.kt` | the kind:1059 carve-out; requires the sender identity |
| `database/model/ChatMessage.kt` | `TYPE_DIRECT_MESSAGE`; the kind:1059 arm and its three outcomes |
| `database/dao/NostrDao.kt` | resolves `senderIdentity` and passes it in |
| `database/repository/DatabaseChatRepository.kt` | queues the rumor; refuses to seal an MLS room's payload, except a Welcome |
| `database/dao/MarmotOutboundDao.kt` | wraps on the way out; scrubs the plaintext |
| `database/model/MarmotInnerEvent.kt` | `directMessageRecipientPublicKey`, the outbound signal |
| `ui/view/model/ChatMessageListViewModel.kt` | armed state; the two renderings |
| `ui/composable/ChatRoomMessagingScreen.kt` | the recipient chip |
Untouched, deliberately: `NostrNip17Dao`, `GiftWrapMessage`, `GiftWrapSeal`,
`GiftWrapPayload`, and every broadcast path.
## What this does not do
Each of these will be reported as a bug at some point. They are not.
**The group learns that a direct message happened, and to whom.** Only the contents
are private, and the sender is authenticated by MLS whatever the wrap says. That is
the design, and the UI should say so in words somewhere a user meets before their
first private message — it does not yet.
**The sender cannot read their own messages anywhere they were not typed.** The
throwaway key is gone. A reinstall or a second device recovers the recipient's half
of a conversation and none of its own.
**Other Marmot clients drop these messages.** Until the carve-out is upstream, a
group needs every member on a client that carries it, and an unpatched member fails
silently.
**The inner layer is not forward secret.** The kind:445 envelope inherits MLS epoch
forward secrecy; the NIP-44 layers inside do not. A recipient's identity key that
leaks opens every direct message they still hold, including ones sent years earlier.
This is strictly weaker than the group messages sitting beside them in the same
room.
**A removed member keeps what they already have.** Removal advances the epoch; it
does not reach back into their device.
**Disappearing messages apply at the envelope only.** The group's
`disappearingMessageSecs` puts a NIP-40 expiration on the kind:445, which relays
honour. Local rows are unaffected, exactly as for group messages today.
**One recipient per message.** Several would mean several wraps, and so several
bystander lines for one message. Worth doing; worth designing first.
**No reactions, receipts, replies or attachments.** A reply needs an `e` tag inside
the rumor and is a small addition later. A reaction is a design question rather than
a coding one, because the reaction itself would be visible to the group.
**No member picker.** The only way to start a private message is to reply to one the
member already sent, so you cannot open a conversation with somebody who has not
spoken.
**Three things have no automated test**, all of them for want of infrastructure
rather than by choice. The recipient validation in `sendChatMessage` (refusing a
non-member, refusing yourself) and the outbound id lookup both need a database; the
invariant the lookup depends on — that the queued row's id is the rumor's — is
tested in its place. The two transcript renderings need Compose UI testing, which
this project has no dependency on. Anything touching a real `MlsGroup` is likewise
untested: the carve-out is tested through `mip03Rejection`, not through a group.