Commit Graph

4 Commits

Author SHA1 Message Date
Kgothatso Ngako
178ddd0181 docs: write down how a long-running chat sync would work
Every chat sync today is a pull: a screen queues a request row, a pump drains
it, the relay answers, the subscription is closed. Nothing arrives between
pulls, so a message sent one second after EOSE waits for the next time someone
opens a screen.

This note works out what it takes to hold the two chat subscriptions open for
as long as the app is active — kind 1059 p-tagged to us, and kind 445 h-tagged
with every group we belong to — and, more usefully, what in the current
pipeline quietly assumes a subscription is short:

  - completeOnSubscriptionEnd finishes the flow at EOSE, which is what releases
    the slot and sends the CLOSE,
  - SUBSCRIPTION_TIMEOUT hard-kills anything still open at 120s,
  - subscriptionSlots is a Semaphore(4) shared with the backfill queue, so a
    permanent subscription is a permanently-held permit,
  - both saveNostrEvent overloads need a request row to attach provenance to
    and to flip to "processed",
  - and nothing in the app reconnects a dropped socket at all. That is
    invisible today only because every subscription is short and the next
    queued request re-opens the socket on its way out.

The design keeps the queue and its three pumps exactly as they are: live
subscriptions replace polling, not reconciliation. Negentropy stays the tool
for first login, the catch-up after a background gap, and "load older".

The group filter is derived from chatRepository.observeChatRoomListByPublicKey
rather than wired at each join site, because a group id can appear four ways
and only one of them (creating a group) is somewhere anyone would think to call
a subscribe function — being added arrives as a Welcome processed deep inside
NostrDao.storeNostrEvent. Observing the room list also closes the loop: a
Welcome lands on the live gift wrap subscription, a ChatRoom row is written,
the Flow re-emits, and the group filter widens without anyone opening a chat.

Two findings fell out of checking the details against our own code:

  - `since = now` on kind 1059 would silently drop messages. Gift wraps are
    stamped with TimeUtils.randomWithTwoDays(), so a wrap published now can
    carry a created_at two days in the past. Kind 445 uses TimeUtils.now() and
    can take a watermark — opposite treatment for the two kinds we care about.
  - the "sent-messages" filter (kinds=[1059], authors=[me]) cannot match
    anything, because gift wraps are signed with a fresh throwaway KeyPair().
    It is 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 already picks it up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 16:01:47 +02:00
Kgothatso Ngako
3dea07135c fix: add a group's whole membership in one commit, closing the epoch race
`MarmotOutboundDao.addMembersToChatRoom` stages every member with `proposeAdd` and
issues a single `commit()`. Both callers that know their membership up front now
use it: `SelectChatRoomTypeViewModel.inviteMembers` at room creation, and
`DkgRitualViewModel.inviteAdmins` for the #admins room.

Inviting one at a time created an epoch per member, and each of those commits
raced the previous member's welcome. MarmotInboundManager refuses future-epoch
messages outright, on both wire formats, with no queue and no replay -- so the
member who lost that race was silently stuck an epoch behind while the caller saw
a successful invite. Deriving isOneMemberInitialGroupCreation narrowed that window;
this removes it. No member ever has to process a commit for an epoch they were not
yet in, so there is no longer a race to lose.

One commit yields one welcome: `buildWelcome` emits an EncryptedGroupSecrets per
added member and each joiner finds its own entry by key package reference. The blob
is shared, delivery stays per peer, because each welcome event is tagged with that
peer's key package.

## Why this needed no schema change

Batching at creation time means the single commit happens while the group is still
only its creator, which takes the immediate-welcome branch: nothing is broadcast
and MarmotCommitResult is never written. The bookkeeping that assumes one peer per
commit is simply not on this path.

So the batch is taken only when `members().size == 1`, and anything else falls back
to inviting sequentially -- correct, if not ideal. Batching into an established
group would take the deferred branch, where `peerKeyPackageEventId` is singular and
the ack-triggered delivery in DatabaseNostrRepository expects one welcome; making
that work needs a list there and a fan-out on acknowledgement. Nothing currently
adds several members to an established group, so that is left outstanding and
documented rather than speculatively built.

The group state is persisted after `commit()` and before any welcome goes out, so a
crash between them leaves the group at the epoch the welcomes describe rather than
one behind it.

## Reporting

Members with no published key package still cannot be added -- a Marmot invite
needs one -- and are now returned alongside any that failed to receive their
welcome, rather than the two being conflated. Both still only reach the log; the
coordinator is not yet told.

docs/marmot-membership.md is updated in the same change: batching moves from
outstanding work to described behaviour, with the schema constraint that shapes it
and the remaining fan-out work recorded. The note about sequential invites is
narrowed to where they still happen.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 15:02:06 +02:00
Kgothatso Ngako
8fc1c9e650 fix: stop deferring the first invitee's welcome behind a commit nobody needs
`inviteMember` now works out for itself whether the group it is adding to has
anybody to inform:

    val isOneMemberInitialGroupCreation = mlsGroup.members().size == 1

read before `addMember` advances the tree. The parameter is gone from the
signature and no caller passes it any more.

Callers were the wrong place for this decision and both of them got it wrong.
`inviteMemberToChatRoom` hardcoded `false`, and `ChatRepository.inviteMember` did
not expose it at all, so every invite made through a group -- room creation in
SelectChatRoomTypeViewModel, and the #admins room -- took the deferred-welcome
path. That includes the first invite, when the group is still only its creator, at
which point:

  - the commit has no audience. No other member exists, and nobody outside the
    group can decrypt it, so it is noise on the relay.
  - the welcome is then withheld until a relay acknowledges that noise. If the ack
    never lands, the first invitee receives nothing at all.

Only createMlsDirectMessageChatRoom passed `true`, and only because a DM has
exactly one invite. A group of n has one such invite too -- the first -- and it was
not getting it.

The condition is right at any size, not just for DMs: "the group has nobody to
inform" is true exactly once. Invite two sees one member who must advance, invite
three sees two, and so on. Their commits are encrypted with
`commitResult.preCommitExporterSecret`, the epoch the earlier invitees received in
their own welcome, so they can decrypt and advance. `members()` skips empty leaves,
so this also stays correct for a group that has had members removed.

## What this does and does not fix

It removes a pointless commit and, with DefaultDMRelayList now a single relay, a
single point of failure sitting in front of every group's first member.

It also narrows a silent race rather than closing it. MarmotInboundManager refuses
future-epoch messages outright on both wire formats -- no queue, no replay -- so a
commit arriving before its recipient's welcome is dropped and that member never
advances, while the coordinator sees a successful invite. Previously both commits
went out before either welcome; now welcome 1 is sent before commit 2 exists, so
the first invitee is already at the right epoch. For n >= 3 the window between
welcome 1 and commit 2 remains.

Closing it needs the adds batched into one commit, which is the outstanding work
described in docs/marmot-membership.md. That doc is updated here to describe the
derived flag as current behaviour rather than a proposal, and to keep batching as
the remaining item.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 14:44:28 +02:00
Kgothatso Ngako
b99cb8fcd5 docs: write down the shared-key subsystem and how Marmot membership fails
First docs in the repo -- README.md is still the stock KMP template. Three
documents plus an index, covering the parts whose behaviour is not recoverable by
reading the code: where the reasoning lives in a protocol, where a failure mode is
silent, or where a decision looked arbitrary and was not.

marmot-membership.md is the one that earns its place. Everything about adding a
member compiles, the invite reports success, and a member simply never appears --
and the reason is never in the invite code. It records that
inviteMemberToChatRoom hardcodes isOneMemberInitialGroupCreation = false and that
ChatRepository does not expose it, so every group invite takes the deferred-welcome
path including the first, when the group is still just its creator and the commit
has no audience at all. Then why that is silent rather than noisy:
MarmotInboundManager refuses future-epoch messages outright, on both wire formats,
with no queue and no replay, so a commit arriving before its recipient's welcome
is dropped and that member never advances. EPOCH_RETENTION_WINDOW retains past
epochs and does nothing for messages from ahead. Three options are set out with the
per-invite correctness table, including the honest limit that the recommended one
narrows the race without closing it.

shared-key-derivation.md argues why the paths are not BIP32 -- no chain code
exists, hardened derivation is impossible rather than unimplemented, and a FROST
tweak takes the scalar as input so the chain code leaves the problem entirely. It
records the x-only serialisation trap avoided by choosing the scalar directly, and
states the rule that must not be broken: never reconstruct a derived key in the
clear, because k = k' - t hands over the group key rather than one derived key.

shared-key-ceremony.md covers the seven kinds, the three approval gates and why
the coordinator's aggregations are deliberately not among them, faults as values
rather than exceptions, and the transcript's idempotency-by-construction. It also
writes down the invariant that produces no error when broken: pendingApproval must
mirror the gates in advance, or the screen offers an approval that does nothing --
or none while the ritual sits still.

Every factual claim was checked against the source rather than recalled, which
turned up one correction worth having: there are two future-epoch refusals, for
PrivateMessage and for Commit, so the drop covers both wire formats and not just
one.

Each document leads with the failure mode rather than the architecture, on the
grounds that a failure is what sends somebody to docs in the first place, and each
lists its known gaps -- including that none of this has run on a physical device.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 14:39:19 +02:00