Commit Graph

2 Commits

Author SHA1 Message Date
Kgothatso Ngako
0be31803f2 docs: record phase 10, and the deferred decision it carries out
`docs/subgroups.md` was written as ten phases of reasoning kept in the order they
were argued, and phase 4 spent forty lines on why the child's ceremony was *not*
held in the parent's Marmot room -- explicitly so the decision would not be
re-litigated without its price attached. That section is now a shopping list that
has been carried out, so it keeps its argument and gains a pointer forward, and
the three costs it enumerated are checked off one by one in a new phase 10.

The parts of the note that state the old arrangement as present-tense fact are
updated rather than annotated: the three-ceremonies table now reads one room,
three ceremonies, two quorums, and says the thing that needs saying twice -- an
MLS message reaches the whole tree, so a ceremony in the parent's room has to name
who it is with.

Phase 10 itself is written the way the others are, around what fails silently:

- `DkgSession.chatRoomId` stopped identifying a ceremony, and the place that
  matters is `completedKey`'s last fallback, which every member welcomed after a
  group's own ceremony lands on;
- `signingPath` had to admit a Marmot room, which widens the one function whose
  contract is that a path never comes off a proposal;
- the p-tags had to stay on both transports, which is the opposite of what
  `FrostSigningManager` correctly does.

The two sections that argued the old collision -- "The collision this buys" and
"Why a subgroup cannot be the whole group was withdrawn" -- keep their reasoning
and gain the end of it: `(room, parent)` stopped telling two subgroups of one
parent apart, so the lookup moved to `(room, parent, admins)`, and the permanent
half of the refusal disappeared with the derived room. The limitation and the
appendix entry are struck through rather than deleted, since what they were
weighing is why the phase exists.

`docs/shared-key-ceremony.md` no longer says a ceremony runs over a NIP-17 chat.
The participant set is the proposal's p-tags on both transports, and that
distinction is the whole reason it is stated that way rather than as "the group".

`docs/mls-skipped-keys.md` keeps `proposeRitual` in its table of reliable
triggers and now says what changed about it: it reached that table on gift wraps,
where the bug does not apply, and a subgroup's ceremony now rides group events. It
is the entry with the worst consequence -- a ChillDKG cannot finish until every
participant takes part, so one lost round-1 message stalls it permanently for
everybody rather than costing one member a line of chat. That is the thing the
quartz fix in that note is now load-bearing for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-09 16:28:28 +02:00
Kgothatso Ngako
d110737f9a fix: keep a room's MlsGroup alive so a late message can still be read
Two events published in the same second reliably lose one of them. The
receiver stores the kind:445 and produces nothing from it -- no inner
event, no chat line, no error anybody sees, because MarmotGroupEvent is
written before the message is decrypted and so survives while everything
downstream silently does not.

Observed as a FROST signing session that never started on the receiver:
proposeSigning publishes the proposal and then the proposer's own nonce,
the relay handed them back in the other order, and the proposal was
dropped. The nonce is still sitting there filed against a session that
will never exist. The same bug ate a dialect earlier, which then took out
the artifact referencing it via a foreign key.

MLS is specified to tolerate this. RFC 9420 says a receiver that gets
generation N+1 before N keeps the intermediate keys so the older message
can still be read, and quartz's SecretTree does exactly that, in a
private skippedKeys map. What it does not do is persist it:
exportSenderStates() returns the ratchet positions only, so saveState()
drops the cache. NostrDao rebuilt the group from stored state for every
inbound event, so the cache was empty every single time, and generation N
arriving after N+1 failed `require(generation >= applicationGeneration)`
and was swallowed. Terminal -- the key is derived from a ratchet that has
moved past it, and nothing asks the sender to resend.

This keeps the instance alive instead. MlsGroupCache holds one MlsGroup
per room, and the inbound path goes through it, so skippedKeys survives
from one message to the next. That covers the case that actually bites --
a burst arriving in one sync, decrypted one after another against the
same tree -- which is what every bursty flow needs: proposeRitual sends
two, addArtifact sends two, and addChapter sends one per paragraph plus
one, of which only the ones arriving in ascending generation order
survived.

Reuse is conditional on the stored state still being exactly what the
cache last wrote. Sending a message advances the sender ratchet and saves;
so does adding a member. When that happens the cache rebuilds rather than
carrying on from a group that has been overtaken -- which is what keeps
this from being worse than no cache at all: the fallback is always the old
behaviour, never a diverged ratchet.

One lock per room, not one overall, because the group is mutable and
decryption advances it: two events for the same room decrypted at once
would corrupt the tree, and a busy room should not hold up a quiet one.

**This is a mitigation, not the fix.** It does not survive a restart, and
it does not survive another writer, so a long enough reorder still loses
the message. The fix belongs in quartz -- carry skippedKeys through
saveState/restore -- and quartz is a mavenCentral binary, not a fork, so
it cannot be made here. docs/mls-skipped-keys.md has the analysis, the
patch, the migration constraint on the persisted state format, and the
three ways to actually land it.

Not verified end to end: the proposal that exposed this cannot be
recovered, since its generation is already past, so confirming the fix
needs a fresh burst.

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