a909108300920865bb05a1dcee183cc43831c980
510 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kgothatso Ngako
|
a909108300 |
feat: announce which key a room signs with, instead of rederiving it
A signer holds a different secret share under every ceremony it took part in, and signing with the wrong one produces a partial signature that cannot aggregate. Nothing said which was which: FrostSigningManager found a room's key by walking every ceremony this device holds a share for and rederiving each one's room id until one matched. That search can only find rooms derived at the one path the constant names. SharedKeyDerivation.parsePath was written to lift that limit and was never called, so a room derived anywhere else was invisible to signing. So the coordinator now says it. GroupKeyStateEvent (kind 30326) carries the threshold public key, the ceremony that made it and the path the room's id came from, posted into the room as its first application message and filed as a GroupKeyState row. completedKey reads that row first and follows it to the share. Nothing secret travels. Every member of the room can read the event, so a share on it would be each member holding everyone else's -- a 1-of-n key wearing a t-of-n's clothes. The event names the ceremony; the share stays in DkgSession.secretShare on the device that generated it. The coordinator is untrusted, as everywhere else in the ceremony, so a state is verified rather than believed: the room's id *is* the threshold key derived at the path, and one that does not rederive its own room is dropped. That is the same guarantee the rederivation gave, kept rather than traded for a lookup. The old scan stays behind it for rooms that predate the table. Announced after the members are added, which is the only order that works -- adding them commits a new epoch and MLS will not let a member read what was encrypted before the one they joined at. A member invited later still misses it and falls back to the scan, which is where every member was before this existed. Replacement is this app's job. These are rumors inside a Marmot group event, so no relay applies the 3xxxx rule, and the DAO keeps the newest announcement per room so a backfill cannot walk a room backwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
0319f1613b | Merge branch 'mantra' into claude/nostr-event-save-issue-6e9467 | ||
|
Kgothatso Ngako
|
5321e4af72 | Merge branch 'mantra' into claude/distracted-franklin-e95ba4 | ||
|
Kgothatso Ngako
|
fb21678813 |
test: pin where a commit's bytes land when the row recording it is written
The mis-routed `framedCommitBytes` fixed in the previous commit was invisible for
one reason: nothing anywhere covered the persisted row. The bytes that reach a
relay come off the in-memory `CommitResult`, so the wire path stayed correct and
the stored path was wrong, and no test looked at the stored path.
## Why the mapping moved before it could be tested
A test that built `MarmotCommitResult` itself would have been writing its own copy
of the mapping and asserting against that. It would have passed against the buggy
code, because the bug was at the call site the test was not using.
So the mapping is now `MarmotCommitResult.from`, called by
`MarmotOutboundDao.inviteMember` and exercised directly by the test. That also
removes the shape that produced the bug rather than just the instance of it: the
old call site listed its named arguments in an order different from the
declaration, which is what put `preCommitExporterSecret` and `framedCommitBytes`
two lines apart. `from` lists the payload in declaration order, in one place, so
there is no second site to get wrong.
## What is covered
Four tests, each payload given a distinct self-identifying value so that a field
arriving in the wrong column names both halves of the mistake instead of comparing
equal by accident:
- every payload field lands in its own column.
- the framed commit column never holds the exporter secret -- the regression,
stated as an invariant rather than an equality so it keeps holding for a
`CommitResult` this test did not anticipate.
- a `CommitResult` that never framed its commit still stores a commit. quartz
defaults `framedCommitBytes` to `commitBytes` and the entity repeats that
default; the fallback must not quietly become the secret either.
- the bookkeeping `DatabaseNostrRepository` reads back on acknowledgement is
carried through. `id`, `chatRoomId`, `userPublicKey` and
`peerKeyPackageEventId` are all 64-char hex, so two of them swapped in `from`
would typecheck exactly as silently as the original bug.
Checked by reintroducing `framedCommitBytes = commitResult.preCommitExporterSecret`
into `from`: three of the four fail. A green suite that would stay green against
the bug it names is not coverage.
## What is not covered, and why
That the bytes published equal the bytes stored -- the property one level above
this one -- still is not. It needs the DAO, and the DAO needs Room: `commonTest`
carries only `kotlin.test`, the room3 KSP processor is registered for the android
and ios targets alone with `kspJvm` commented out, and `getInMemoryDatabaseBuilder`
wants a `PlatformContext` no unit test has. That is a Robolectric or instrumented
target, which is a larger change than this fix earns and is better decided on its
own merits than smuggled in here.
The ack-triggered rebroadcast that would have turned the bug into a live fault does
not exist yet, so there is nothing to test there either. When it is written, the
invariant it needs is already asserted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
ad3304a665 |
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>
|
||
|
Kgothatso Ngako
|
e1d35bbd6c |
test: pin who can open a gift wrap, and what happens to everyone else's
The Invalid Mac crash had no test standing between it and a repeat, so this adds one that reproduces it. GiftWrapMessageTest builds real NIP-59 wraps with real secp256k1 rather than recorded fixtures. The property under test is the key agreement itself — whether ECDH(ourPriv, ephemeralPub) can stand in for the conversation key the wrap was sealed under — and a fixture would only prove that the fixture still parses. Three cases carry the regression: - someone else's mail comes back null rather than throwing - not even the sender can reopen what they sent - isAddressedTo answers exactly what unsealing would Checked against the reverted fix, those three fail with the production exception verbatim (java.lang.IllegalStateException: Invalid Mac: Calculated bf2e6480…), while the two describing behaviour that never broke — the happy path, and isAddressedTo's reading of the p tag — stay green. A test that cannot fail against the bug it names is not worth the run time, so the split matters. The last of the three is the one guarding the fix's structure rather than its outcome. NostrDao decides whether to index on isAddressedTo, then throws GiftWrapUnsealException if decryptGiftWrapSeal returns null anyway; those two answers have to agree for either path to be correct. If they drift, the DAO either skips mail we can open or resumes rolling back transactions, and neither shows up as a failure anywhere near the change that caused it. commonTest gains kotlinx-coroutines-test for runTest. decryptGiftWrapSeal is suspending, runBlocking does not exist in common code, and every layer worth testing below the ViewModels — DAOs, repositories, the model's crypto — is suspending too, so the dependency pays for more than this file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
42dd38cfc4 |
test: pin the two invariants this session left unguarded
Both are silent when broken, which is why they are worth asserting rather than reasoning about. **The cache's reuse decision.** MlsGroupCache exists because quartz drops a secret tree's skipped-generation keys on save, so rebuilding a group between two messages loses any that arrives late. Its safety argument is one comparison: reuse while the stored state is still what the cache last wrote, rebuild when it is not. Get that wrong in either direction and nothing complains -- reuse too eagerly and a group carries on from a ratchet another writer already moved, which corrupts decryption rather than failing it; reuse too rarely and the cache does nothing and the original bug is back with no symptom. That decision is now a generic LiveInstanceCache with MlsGroupCache as a typed facade over it, so it can be tested without standing up an MLS group. Splitting it also made two behaviours explicit that were previously incidental: a failed build no longer leaves the old instance behind, and an instance whose use threw is deliberately not cached -- it is half-advanced and never persisted, so the next caller has to start from disk. **Rumor and row ids agreeing.** MantraDao writes an entity whose id comes from fromXEventTemplate and separately builds the rumor it submits with rumorOf, which hashes the template itself. Both are meant to produce one id and nothing checked it. Diverging would mean submissions naming an event nobody has, deleteByPayloadEventId silently un-queuing nothing so superseded translations go out anyway, and every receiver creating a second row instead of converging on the sender's -- all of it invisible, since the ids are opaque hex either way. Asserted per kind, plus the whole chain out through the submission envelope. Both suites were mutation-checked rather than trusted: inverting the staleness comparison fails one cache test, recording the pre-block state fails another, and hashing the rumor under a different author fails all six id tests. Still uncovered, and not cheaply fixable: FrostSigningManager's and MantraDao's state machines both need a Room harness, and commonTest has none. The FROST crypto path is covered by FrostSigningRoundTest; the message-driven parts around it are not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
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> |
||
|
Kgothatso Ngako
|
f38a5f12f3 |
fix: ask relays for gift wraps addressed to us, not to our peers
Three kind:1059 sync filters named the wrong pubkey. ChatMessageListViewModel asked for `#p:[peer]` with no author constraint, which subscribes to every wrap anyone has ever sent that peer. None of it is decryptable by us, and it is the direct source of the Invalid Mac saves fixed in the previous commit. It now asks for `#p:[us]` on our own DM relays — the only shape of gift wrap filter that can return something we hold a key for. The peer's relays were the wrong place to look regardless: under NIP-17 a sender publishes to the *recipient's* DM relays, so our mail lands on ours. The two in NostrDao asked for `authors:[userPublicKey]` + `#p:[participant]`, commented "messages from this relay that were sent by us". A gift wrap is signed by the throwaway key from GiftWrapEvent.create, never by the sender's identity key, so no author value we could know will ever match one. These requests were queued once per participant and always reconciled to empty — failing silently rather than loudly, which is why they outlived the bug that made the third filter visible. Both `if (chatMessageRelayListEvent != null)` branches held nothing else, so each is inverted to the `== null` case that does the real work: warn, and queue a profile sync for the participant whose DM relay list we are missing. Nothing is lost; neither filter ever returned an event. Two things worth recording about what a filter can and cannot express here. A wrap discloses only its recipient, so "the messages in this conversation" is not askable — `#p:[us]` pulls the whole inbox and that is the narrowest correct request. That is the privacy property being paid for, not a limitation to work around. Sent-message recovery is likewise not a filter problem. It needs a second wrap addressed to ourselves at send time, which giftWrapAndBroadcast does not yet emit; the `#p:[us]` filters already in place would pick those up with no new subscription. purpose on the chat message request changes from "sent-messages" to "chat", matching the now-identical filter in ChatRoomListViewModel. Since computeId buckets by minute and NegentropySynchronizeRequestDao upserts, the two collapse into a single request rather than racing as separate rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
f57644aa1f |
fix: stop discarding gift wraps addressed to someone else
An inbound kind:1059 whose `p` tag is not our pubkey took down the entire
save transaction:
java.lang.IllegalStateException: Invalid Mac: Calculated f1db537e…, decoded: 45c8c86a…
at com.vitorpamplona.quartz.nip44Encryption.crypto.Hkdf.fastExpand
at com.vitorpamplona.quartz.nip44Encryption.Nip44v2.checkMessageKeys
…
at press.mantra.compose.database.model.GiftWrapMessage.decryptGiftWrapSeal
at press.mantra.compose.database.dao.NostrDao.indexNostrEvent
at press.mantra.compose.database.dao.NostrDao.storeNostrEvent
Two separate things were wrong.
The first is that decryptGiftWrapSeal attempted the decryption at all. When
the recipient did not match our key it logged "We are unwrapping a message we
may have sent" and called
Nip44.decrypt(content, privateKey = ourPrivKey, pubKey = giftWrapEvent.pubKey)
giftWrapEvent.pubKey is the wrap's ephemeral author. NIP-59 encrypts the wrap
under ECDH(ephemeralPriv, recipientPub), and GiftWrapEvent.create mints that
ephemeral key with NostrSignerSync(KeyPair()) and discards it on return.
ECDH(ourPriv, ephemeralPub) is a third, unrelated key, so the MAC check could
never pass. A sender genuinely cannot unwrap their own gift wrap; that is the
point of the construction, not a gap in it. The call threw its result away
anyway (keyPair.privKey?.let { …; null }) and fell through to the trailing
`return null`, so it was a probe whose only possible outcome was an exception.
The second is that a null seal was treated as a failure. indexNostrEvent
throws GiftWrapUnsealException on null, which unwinds out of the Room
transaction in storeNostrEvent and rolls back everything written for the
event: the NostrEvent row, its NostrEventRelay row, and the GiftWrapMessage
upserted moments earlier. The only catch sits in DatabaseNostrRepository,
which logs and continues — and that catch also swallows the
`status = "processed"` upsert on the SynchronizeNostrEventRequest, so the
event was re-fetched and re-failed on every later sync pass.
isAddressedTo now answers the question with no crypto at all, and the indexer
returns early for wraps that are not ours: the event and the wrap row survive,
the remainder of indexNostrEvent still runs, the transaction commits, and the
sync request is marked processed. GiftWrapUnsealException goes back to meaning
what it says — addressed to us, but unsealing failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
3e4166f13d |
feat: sign a dialect into existence instead of submitting one
Adding a dialect no longer creates one. It opens a signing session over a DialectEvent, and the dialect appears -- on every member's device at once, authored by the group's shared key rather than by whoever typed it -- when enough members have signed. That is the difference between the two envelopes. A submission says "I am putting this in front of the group"; the group's only recourse afterwards is social, and the row records the submitter as its author. A signature is the group saying it, it takes a quorum to say, and the author on the row is the group's key. For something as load-bearing as the set of dialects a group translates into, the second is the honest one. **Where the signed event becomes a row.** Every device has the event and the signature once the session completes, so each applies the result itself rather than waiting to be sent something it can already build -- the same reasoning the transcript lines are written on. Nothing goes on the wire for it, and nothing could: the outbound pipeline re-authors rumors as their sender, so a group-signed event pushed through it would come out stripped of the signature and attributed to whoever sent it. Applying reuses the inbound path's dispatch rather than repeating it. applyInnerEvent takes plain ids now instead of a GroupEvent, and both are null here, because there is no group event and no inner event behind a row a device derived for itself. A failure there is logged and the session still completes: the signature is made and valid, and failing the session would tell the group to abandon something that succeeded. **The screen.** One, not three. A ceremony asks three different questions so it gets three approval screens; signing asks one -- sign this or do not -- so a single screen has to carry the whole case: what is being signed, who else has agreed, and what the group is still waiting on. The event is shown as the thing it is, a dialect with its name and country and language, because a member deciding whether to sign is deciding about a dialect and "kind 30304" answers a question nobody asked. Anything unrecognised falls back to the raw kind, which is better than describing it wrongly. The member ladder names people rather than counting them, for the same reason the ceremony's does: "1 of 2" does not tell anyone whose door to knock on. It stays useful after the decision, since a member who has already signed is exactly who needs to see who has not. **Getting there.** Signing lines render in the transcript as system notices like ritual lines -- nobody said them either -- but they lead to the session rather than to the key. A chat row carries no session id and adding a column to the table every message uses would be a poor trade for a lookup, so FrostSigningRoute takes a nullable id and the screen resolves the room's live session. Approving is recorded as answered by the nonce line rather than the partial signature: agreeing is agreeing to take part, and the coordinator may then pick a quorum without you, which should not leave you looking like you never replied. **Proposing needs a key.** The FAB is disabled, and says why, when the room has none -- proposeSigning throws there, and it is not reachable outside the #admins room in the first place. AddDialectViewModel drops MantraRepository, which it no longer uses for anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
63c1879ace |
refactor: carry signing on marmot inner events, not gift wraps
A signing message is now an ordinary Marmot inner event: queued with a
null marmotGroupEventId, picked up by the outbound pipeline, MLS-encrypted
and broadcast as one kind:445 for the room. Inbound it arrives through
ChatMessage.fromGroupEventResult like every other inner event, and is
dispatched from NostrDao rather than from the gift-wrap branch.
The ceremony keeps NIP-17 because it has no choice: its participants are
not yet a Marmot group, and its purpose is to produce the key one would
be keyed on. Signing has that solved for it, so it was paying for
addressing it does not need -- a gift wrap is sealed once per recipient,
so every message cost one wrap per member, and every message had to name
the whole group in p-tags. A group event is encrypted to the group once.
That also removes a small dishonesty. The signer set is supposed to come
from the ceremony; carrying p-tags meant each message also asserted a
membership list, and two sources for one fact is one too many. Now who
can read a message is the MLS tree's business and who may sign is the
ceremony's.
Which room follows from the transport. A ceremony runs in a NIP-17 room
-- every member an equal admin, no MLS tree to be outside of -- and a
group event needs an MLS one, so signing cannot happen where the ceremony
did. It happens in the #admins room, which is the right venue anyway: it
already exists after a ceremony, its membership is exactly the share
holders, and its id *is* the key, derived by
SharedKeyDerivation.marmotGroupId.
So completedKey rederives rather than reading a column: a room cannot be
pointed at a key it was not derived from. Receivers were already
independent of this, naming their key in the proposal's frost_key tag and
looking it up locally.
Mechanical consequences:
- processSigningPayload, acceptProposal, record and isFromCoordinator
take the decrypted Event instead of a GiftWrapPayload.
- replayStoredMessages reads MarmotInnerEvent rows, via a new
getByChatRoomAndKinds, and rebuilds the rumor from the row's own
columns.
- applyInnerEvent returns null for the signing kinds. They are the
manager's, and it writes transcript lines naming who did what, so an
"unsupported" row would be a second and worse account of the same
thing.
- DkgSessionDao gains getKeyHoldingSessions for the derivation match.
The kind comment is rewritten rather than kept. 3032x was chosen to clear
the DKG, which now shares no transport with signing and cannot clash with
it; what it actually has to clear is the nip30303 document kinds, which
run 30300-30312 and are dispatched by the same inbound path. It still
does. The DKG's own overlap with those numbers is noted there as the
routing accident it is, so nothing added later leans on it.
No schema change: both tables and the columns landed in v6 with the
previous commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
b4ac65f5c9 |
feat: sign a nostr event with the group's shared key
A ceremony leaves every member holding a share of a t-of-n key and no way
to use it. This is the other half: a session that turns an unsigned nostr
event into one signed by the group.
The shape is ChillDkgRitualManager's, deliberately. The member who
proposes coordinates, protocol messages travel as gift-wrapped rumors on
the same NIP-17 pipeline chat messages use, each inbound message is
persisted and then the session is asked whether it can move, and every
step is recomputed from stored inputs so a device killed mid-round
resumes on the next message. Anyone who has read that manager can read
this one.
proposer --[ 30320 proposal ]-> everyone the unsigned event
signer --[ 30321 nonce ]-> everyone this device's public nonce
proposer --[ 30322 signer set ]-> everyone who signs, and their aggregated nonce
signer --[ 30323 partial ]-> everyone this device's partial signature
proposer --[ 30324 signature ]-> everyone the finished 64-byte signature
anyone --[ 30325 failure ]-> everyone abandon + blame
Three things are genuinely different, and each is why this is a separate
manager rather than another branch of that one.
**It does not need everybody.** A DKG cannot finish until every member
takes part; that is what makes the key. Signing needs t, and waiting for
n would throw away the property the group ran a ceremony to get. So the
coordinator waits for the threshold to be reachable, picks a set and says
who is in it. Members left out do nothing and stall nothing.
**Restart-safety is forced rather than chosen.** SecretNonce cannot be
serialised and refuses to be used twice, so storing the randomness it
derives from and regenerating on demand is the only way a session
survives the app closing. That is safe for exactly one reason: a session
signs one message and cannot be made to sign another. Two rules hold it
in place and both are load-bearing rather than tidy:
- the event id is written at creation, and a proposal that disagrees
with it is refused rather than applied;
- the aggregated nonce and signer set are write-once. A coordinator
that sends a second, different set is ignored. Obeying it would mean
two partial signatures over one secret nonce against two challenges,
which is precisely how a secret share is extracted. The session
stalls; the share does not.
**One approval, not three.** A DKG asks three times because each step
publishes something different and commits the member to something
different. Here every step serves one decision -- sign this event or do
not -- and the event is fixed before the member is asked, so a second
prompt would be the same question twice. Declining is broadcast rather
than silent: a t-of-n group can sign without you, but only if it knows.
Two things are checked rather than trusted, both because the coordinator
is untrusted by construction: the event id is recomputed from the
proposal's own fields, so a proposer cannot have the group sign one thing
while showing them another; and the finished signature is verified before
the session is called complete, so a bad aggregate is a failure here
rather than a rejection at every relay it reaches.
Signer ids are derived, not stored: a member's FROST id is their index in
the bytewise sort of the ceremony's host keys, the same ordering ChillDKG
hashed into the session identity and the same one the public shares are
in. Deriving means signing cannot disagree with the ceremony that made
the key.
DkgSession gains publicShares, kept because FROST validates each signer's
secret share against its public one. A ceremony finished before this
column reads back null and signing runs without that check rather than
refusing.
The tests run the same calls in the same order against real FROST and
assert the aggregate verifies as a nostr signature. That path was written
from reading the library rather than from a working example, so it is the
part most likely to be subtly wrong -- and wired up wrong it fails
silently, on every device.
Kinds start at 30320 with a gap. The DKG runs 30310-30316 and the
nip30303 document kinds run 30300 up; those two already collide at 30310
and 30311, and SubmissionEvent sits on 30312, which is also the DKG's
round-1 kind. They are kept apart today only by riding different
transports, which is luck. Signing shares a transport and rooms with the
DKG, so it starts clear of both.
No UI yet: this is the session logic, reachable through proposeSigning,
approve and decline.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
fcc28de931 |
Revert "fix: hold a payload whose parent has not arrived instead of losing the event"
This reverts commit
|
||
|
Kgothatso Ngako
|
d7aac49cf1 |
fix: hold a payload whose parent has not arrived instead of losing the event
A receiver hit `FOREIGN KEY constraint failed` on an artifact submission and lost the whole group event. The artifact referenced a dialect the receiver did not have, MantraArtifact.dialectId is a foreign key, and SQLite answers a violated constraint by aborting -- which rolled back the entire transaction the inbound pipeline runs in. Gone with it: the NostrEvent, the MarmotGroupEvent, the submission's MarmotInnerEvent holding the payload verbatim, and the transcript line. Nothing retries, so the artifact stayed lost even once the dialect turned up. Every nip30303 entity is a child of another and the schema enforces all of it -- artifact→dialect, version→artifact, chapter→version, chunk→chapter, translations→both of theirs -- so this was every branch, not one. And submissions make arriving before your parent ordinary rather than exotic. That is the point of them: an admin submits a backlog in whatever order they hold it, and a member who joined last week can be sent what the group was told last month. Both produce payloads whose parents are not here yet, and both were losing data. So check the parents before inserting. A payload that arrives early is held on the submission row -- awaitingEventId names what it waits for -- and applied when that arrives. Releasing one can release another, a version freeing its chapters and those freeing their chunks, so it walks outward until nothing more comes unstuck. A payload with a second parent still missing is re-pointed at that one rather than retried on every arrival. Nothing is written to the transcript while a payload is held. Nobody has said anything yet; the line appears when it is applied, in the position its own timestamp gives it. Two things fall out of the shape: parentRefsOf is pure and separate from the lookups, because the mapping is the part that can silently drift from the schema and there is no database harness in commonTest to catch it. ParentRefsTest pins one case per kind. Which table an id lives in is carried as the kind of event that would have created it, so there is no second enum to keep in step. applyInnerEvent takes ids rather than a GroupEvent, since replay happens long after that object is gone. A released payload is recorded as not ours: we hold the parents of anything we wrote, having written those too. Also reconstructs a held bare nip30303 event from its own columns rather than parsing its content as an event -- only submissions carry an event there, and reading both that way would have stranded every bare one permanently. Verified: the v5→v6 migration runs clean on the receiver's real populated database. The hold path itself still needs a fresh submission from a sender to exercise end to end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
2d0fe6f5fc |
fix: disable Add Artifact until a dialect is picked
Submitting without a dialect was rejected in the view model, which
called onFailure, which navigated to ImplementationPendingRoute("Failed
Artifact") -- a whole screen away from the form, saying nothing about
which field was wrong, and leaving the way back to the only sensible fix
as the back button.
That is a bad way to report any missing field, but the dialect is the
one where it is unrecoverable in place. A blank name or url is answered
by typing; a dialect has to already exist, and since dialects moved to
the group screen there is nothing on this form that can conjure one. So
an unpicked dialect is not a mistake to report after the fact, it is a
state the button should not be pressable in.
Material 3 gives ExtendedFloatingActionButton no `enabled` parameter, so
this paints the disabled colours from ButtonDefaults.buttonColors() --
the same ones every other disabled button in the app resolves from the
theme, rather than an alpha invented here -- and returns early from
onClick.
Also marks it disabled to accessibility services. Colours alone leave a
screen reader announcing a button it is happy to press, and pressing it
does nothing, which is worse than a button that says it is unavailable.
A group with no dialects at all is covered by the same condition, since
there is then nothing to select.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
6c63027912 |
feat: submit nip30303 events to the group instead of authoring them into it
With the receiving side able to open an envelope, start sending one.
Every nip30303 event now leaves as a SubmissionEvent payload and none
leaves on its own: addDialect, addArtifact, addArtifactVersion,
addChapter and each of its chunks, addTranslationArtifactVersion and
each of its translation chapters, and saveTranslation. Because all four
add screens reach the wire through MantraDao, none of them needed
touching.
Two helpers carry it:
rumorOf(template, publicKey) the unsigned event a template describes.
Its id is computed exactly as the
matching Mantra* entity computes its own,
so the row on disk and the payload on the
wire are one event rather than two copies
of one.
submitToGroup(...) wraps a payload, queues the submission as
an unprocessed rumor, and writes the chat
line.
Two things this drags in, neither optional:
The queued MarmotInnerEvent is now the envelope, so its id is the
envelope's and no longer the nip30303 event's. saveTranslation replaces
a chunk whenever its text changes -- the id is derived from the content,
so an edit is a new row -- and un-queued the superseded one by
deleteById(stale.id). That silently stops matching anything once the row
is a submission, leaving the stale translation to be sent anyway. It now
also deletes by what the submission carries, via deleteByPayloadEventId.
The add* methods return the entity rather than the queued rumor. This is
a correctness fix, not tidying: AddArtifactViewModel navigates to
ArtifactDetailRoute on that id, and AddTranslationArtifactVersionViewModel
feeds addDialect's id straight back in as a dialectId. Both used to be
handed a MarmotInnerEvent whose id happened to equal the entity's, and
both would now have been handed a submission id -- one navigating to an
artifact that does not exist, the other tagging a translation with a
dialect that does not. Returning MantraArtifact/MantraDialect/etc. makes
.id mean the entity everywhere and matches saveTranslationChunk, which
already returned its entity.
The sendMarmotInnerEvent overload taking a LocalChatRoom loses its last
caller; submitToGroup names the submitter explicitly, which is the thing
that matters now that it is not necessarily the author.
Outbound still only ever submits payloads authored by the submitter --
nothing in the app originates a foreign event yet. submitToGroup is
where that would attach.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
ce77b77240 |
feat: apply the nip30303 event a submission carries, keeping its author
Teach the receiving side to open an envelope before anything starts
sending one. In that order a client that has this can already handle
submissions from a client that does not yet send them; the reverse would
turn every artifact, dialect and chapter into an "unsupported" row for
anyone who had not updated.
Despite the name, MarmotInboundManager does not dispatch on inner-event
kinds -- it decrypts MLS and hands back a GroupEventResult. The kind
dispatch has always lived in ChatMessage.fromGroupEventResult, so that
is where support for a new kind goes.
The `when (event.kind)` body becomes applyInnerEvent, which takes the
event to apply separately from how it arrived:
event the nip30303 event, written by whoever wrote
it -- possibly nobody in this group
marmotInnerEventId the row the group actually delivered
senderPublicKey the member who delivered it
createdAt when they did
For a plain nip30303 event those all come from the one event, which is
exactly the old behaviour. For a submission they come from the envelope
while `event` is the payload. Entity rows take their author from the
payload via fromXEvent, so the chat line says who added something and
the row says who wrote it -- the point of the envelope, made real at the
only place it can be.
createdAt deliberately follows the envelope rather than the payload: a
submitted archive translation can be years old, and sorting the group's
transcript by when the source was written would file "X added a
translation" somewhere nobody will scroll to.
The stored MarmotInnerEvent stays the outer event -- that is what the
group sent -- and gains payloadEventId naming what it carries. The
payload is not given a row of its own: it is recoverable from the
submission's content, and a second row with a null marmotGroupEventId
would look to the outbound pipeline like something waiting to be sent.
Nullable column, so AutoMigration(4, 5) is all it needs; rumors queued
before this read back null, which is correct, since none of them were
submissions.
Two submissions are stored but not applied, because there is nothing in
them to make a row from: one whose payload will not parse, and one
carrying another submission. Both surface as "unsupported" rather than
disappearing.
The unsupported fallback also stops attributing to groupEvent.pubKey,
which is the ephemeral key every kind:445 is signed with and so names
nobody.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
fa380e94e1 |
feat: add a SubmissionEvent that carries a nip30303 event as its payload
Every nip30303 kind so far describes a thing: an artifact, a dialect, a
chapter, a translated chunk. None of them describes the act of putting
one in front of a group, and until now nothing needed to -- a group
event's sender was the author of the event inside it, so the two
questions had one answer by construction.
That construction is also the limit. It means a group can only ever hold
work written by its own members under their own keys. A translation
lifted from a public archive, a chapter transcribed by an outside
contributor, an artifact somebody published years ago: none of it can go
in without a member re-authoring it and taking the byline.
Kind 30312 is the envelope that separates them. Its content is the
payload event's JSON, whole -- same id, same pubKey, same signature,
nothing rewritten to look like the submitter's work. The submitter signs
for the envelope; the author still signs for the event. Two tags name
what is inside so a client can decide whether it can apply a submission
without parsing the content first:
payloadKind the payload's kind
payloadId the payload's id, with the author slot carrying the
payload's author -- who, unusually for an id tag in
this package, is often not the event's sender
Kinds 30300-30311 are taken (30305 and 30307 by contributor lists), so
30312 is the next free one.
A submission is not an endorsement and grants nothing. Who may submit is
the group's business; this only makes the question expressible.
The test covers the property the whole thing rests on: an event written
by an outsider goes into an envelope, comes out of a JSON round trip
with its id, author and signature intact, and does not acquire the
submitter as its author. It also pins payload() returning null rather
than something empty when the content will not parse -- which needed
android.util.Log stubbing, since quartz logs on that path and unmocked
Log methods throw, failing the test on the log line rather than on what
it came to check.
Nothing sends or reads one yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
e14be2d187 |
refactor: pick an artifact's dialect, do not invent one while adding it
Adding an artifact offered a "New dialect" chip that swapped in three more fields -- name, country, language -- and minted a dialect on the way to creating the artifact. Now that a group defines its dialects on its own screen, that path is a second, worse way to do the same thing: it creates a dialect as a side effect of an unrelated action, in a form where the fields belong to neither entity clearly, and with no sight of what the group has already defined beyond a row of chips. Drop it. The chip row is now exactly the dialects that exist, and selectedDialectId changes meaning from "null = create a new one" to "null = nothing picked yet" -- which the FAB rejects alongside the other required fields, rather than falling through to creating something. A group with no dialects yet gets a line saying so and pointing at the group screen, instead of a lone chip that opens a form. addArtifact loses the three TextFieldStates and existingDialectId for a single dialectId, and with them the branch that called addDialect and threaded its id back in. Validation is now one condition rather than one per path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
6ff9fd2d38 |
feat: let a group define the dialects it translates into
Dialects existed but had nowhere to come from. The only way to create
one was the "New dialect" branch buried inside the add-artifact form,
which meant a dialect could only be born as a side effect of adding the
first artifact written in it. A group that wanted to line up the
languages it works in before any source material arrived had no way to
say so, and a dialect created that way was invisible afterwards -- there
was no screen anywhere that listed what the group had defined.
Give the group detail screen a Dialects section between Library and
Projects: the dialects defined in this room, each showing its name over
"<language> · <country>", and an Add Dialect button. The cards do not
navigate -- there is no dialect detail screen to open, and a card that
goes nowhere is worse than one that plainly does not.
The add screen is the add-artifact form with the artifact half removed:
the same chat-room title bar, the same bottom bar with an extended FAB,
the same three fields (name, country, language) styled the same way.
On success it returns to the group with popUpTo<ChatRoomDetailRoute>
{inclusive = true}, replacing the stale detail screen beneath it so the
new dialect is actually in the list when you land -- these lists load
once, in the view model's initiate().
One deliberate difference from AddArtifactViewModel: it wraps its whole
body in `localChatRoom.chatRoom.toMlsGroup()?.let { ... }` and so does
nothing at all, silently, in a NIP-17 room. Nothing under
MantraRepository.addDialect needs an MLS group, so the gate is left out
rather than copied into a new screen as a button that does nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
248a527267 |
fix: store the framed commit on MarmotCommitResult, not the exporter secret
`MarmotOutboundDao.inviteMember` persisted the commit row with
framedCommitBytes = commitResult.preCommitExporterSecret,
two lines below the argument that value belongs to, which was already assigning it
correctly. It now reads `commitResult.framedCommitBytes`.
The row is written on the deferred branch, after the kind:445 commit has gone out
and while the welcome waits on a relay acknowledgement, so what it holds is meant
to be the record of what was published.
## Why the compiler had nothing to say
`MarmotCommitResult` carries quartz's `CommitResult` payload fields verbatim --
`commitBytes`, `welcomeBytes`, `groupInfoBytes`, `framedCommitBytes`,
`preCommitExporterSecret`, same names, same order, same defaults. Both of the
fields in question are `ByteArray`, so the wrong field of the right object is
indistinguishable from the correct one at the type level.
The call site lists its named arguments in a different order than the declaration,
which is what put `preCommitExporterSecret` and `framedCommitBytes` two lines
apart. The entity also repeats quartz's `framedCommitBytes: ByteArray = commitBytes`
default, so the explicit argument was overriding a fallback that -- while still the
raw commit rather than the framed envelope -- was at least a commit.
## What it cost, and what it would have cost
Nothing so far. `framedCommitBytes` has exactly two references in the tree: this
assignment, and `encryptedCommitEvent` at the top of the same branch, which takes
`commitResult.framedCommitBytes` from the in-memory `CommitResult` rather than from
the row. The bytes that reached the relay were always the right ones; the wrong
ones only ever sat in the column.
They would stop merely sitting there as soon as anything reads the row back.
`DatabaseNostrRepository` already reloads these rows on acknowledgement, at
`getMarmotCommitRequestById`, to pick up `welcomeBytes` and fire `deliveryWelcome`.
An ack-triggered rebroadcast or a replay reaching one field further along would
publish 32 bytes of exporter secret where a
`MlsMessage(PublicMessage(FramedContent(commit)))` envelope was expected: not a
message recipients drop, but a group key on a relay.
The smaller half holds whether or not anything ever reads it. The group's
pre-commit `MLS-Exporter("marmot", "group-event", 32)` output was being written to
a second column that is not intended to hold key material, doubling its footprint
at rest alongside the `preCommitExporterSecret` field that exists for it. Only at
rest -- the ack path logs the row, but the data class has no `toString` override,
so `ByteArray` prints as an identity hash rather than contents.
## Scope
`MarmotCommitResult` has a single construction site in the codebase, the one
changed here, so there is no second copy of this to fix. Worth checking rather than
assuming: the shape that produced it -- adjacent `ByteArray` fields with identical
names on both sides of the copy -- reproduces anywhere the entity is built again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
907dba3c3b |
fix: create the #admins room only once every member can be added
Every member's key package is now resolved before anything is created. If one is missing the room is not created at all, and the coordinator is told which member to go and ask rather than being handed a room quietly short of people. Previously the room was created and then whoever could be added was added, with the rest collected into a list that only reached the log. Two things make that the wrong trade here, and neither applies to ordinary group creation: MarmotGroupData.adminPubkeys is baked into the epoch-0 GroupContext and names every member of the ceremony. A room created without one of them therefore lists an admin who is not in the MLS tree -- a group that disagrees with itself from its first epoch, and MIP-01 leans on that list for most group operations. And the id is derived from the shared key, so there is exactly one room per group at this path. A half-created one occupies that address permanently; unlike a random id there is no second one to retry with. Creating nothing leaves the retry clean. The lookup moves ahead of group creation, which also means the batched add now receives a list it knows is complete -- `addMembers` no longer has to reason about absent key packages on this path. `inviteAdmins` goes with it. Its job was resolving key packages and then adding whoever it could; the first half moved into the precondition and the second is a direct `addMembers` call. The blocked members surface as `DkgRitualUIState.adminGroupBlockedOn`, carrying names rather than public keys -- the action this prompts is asking a particular person to open the app, so a name is what the coordinator needs. Cleared when the button is pressed again, so a retry does not show the previous answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
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> |
||
|
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>
|
||
|
Kgothatso Ngako
|
9f14679aac |
feat: let the coordinator open a #admins room keyed on the shared key
Once a ceremony completes, the shared-key screen offers its coordinator a Marmot
room named "<group> (#admins)" with every member of the ceremony in
MarmotGroupData.adminPubkeys. The room the ceremony ran in is NIP-17, where nobody
administers anything; this gives the same people a room where every one of them
can act, which is the shape a group that has just made a t-of-n key is asking for.
Built directly rather than through MarmotGroupData.bootstrap, which hardcodes a
single admin, and baked into the epoch-0 GroupContext so later invitees receive a
populated group from their welcome instead of chasing a bootstrap commit that
predates their membership.
## The id is derived, not random
Every other Marmot room mints `nostrGroupId` as RandomInstance.bytes(32). This one
derives it from the group's threshold key, settling the
`// TODO: Generate GID through frost...` already sitting in
SelectChatRoomTypeViewModel.
Derivation buys two things random cannot. Every member's device can compute the id
from a ceremony they all took part in, so the room is addressable without being
announced; and two members racing to create it arrive at the same id rather than
two rival rooms -- which is why createAdminGroup returns to the existing room
instead of minting a second one.
## Why the derivation is what it is
SharedKeyDerivation walks the path as successive FROST tweaks, one per index,
returning both the XonlyPublicKey and the TweakCache. The cache is not an
optimisation: a signing session created without the same tweaks aggregates to
signatures that verify against a different key, which is why the id is usable as
an identity later rather than only as a label.
It is not BIP32, and the doc comment argues that at length rather than leaving it
to be rediscovered. A BIP32 node is a key *and* a chain code; ChillDKG produces no
chain code. BIP32 wants one only because it computes the tweak scalar for you, and
a FROST tweak takes that scalar as an input -- so choosing it directly removes the
chain code from the problem rather than requiring one to be invented and agreed
forever. It also removes a trap: with x-only keys there is no single obvious
serP(K_par), and two devices picking different parity conventions would silently
derive different keys rather than fail.
Each scalar commits to the key being tweaked as well as the index, so steps cannot
be reordered or replayed at a different depth. Tests cover that, determinism
across calls, path and key sensitivity, and that the cache and the public key
agree.
Hardened derivation is not available here and never will be: it needs the parent
private key, which in a threshold group nobody has. That leaves the non-hardened
weakness -- k' = k + t with publicly computable t inverts -- so anyone learning one
derived private key recovers the group key and can sign with no quorum at all. The
rule that follows is stated at the top of the file: never reconstruct a derived key
in the clear.
## The path is recorded in the room
MIP-01's group data is a fixed TLS schema with no extension map, so a custom field
would emit bytes other Marmot clients cannot decode. The path rides in the
description instead, on its own line under a marker, so somebody rewriting the
rest of the description does not cost the group the record of how its key was
derived:
Admins of Ubuntu Collective.
Shared key path: m/9420/0/0
Worth storing although the path is currently a constant: it is what rebuilds the
TweakCache a signing session needs, and recomputing from the constant only holds
while the constant never changes. parsePath refuses hardened indices rather than
tolerating them -- such a path cannot have been walked here, so acting on one
would derive something other than what the room claims.
## Known limits
Members without a published MarmotKeyPackage cannot be invited; inviteAdmins
collects them and logs them, and the coordinator is not yet told.
Invites go one at a time, each advancing the MLS epoch, so the room is re-read
between them. That inherits a silent failure mode documented in
docs/marmot-membership.md: the first invite takes the deferred-welcome path even
though the group is still just its creator, and a commit reaching a member before
their welcome is dropped rather than queued. Not introduced here -- group creation
has always done this -- but more visible in a room whose whole membership is known
up front.
Nothing here has run on a device.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
b200916844 |
feat: show the group's key in full, with a copy button
The shared-key screen showed `thresholdPublicKey.take(16)` followed by an ellipsis. A 16-character prefix is enough to recognise a key you already know and not enough for the one thing this key is for. Members compare it out of band to confirm every device finished the ceremony on the same key. That is the check that catches a device which quietly ended up elsewhere -- and it cannot be done against a prefix, or from a screen the value cannot be copied off. Both halves of that were missing. The whole 66-character key now renders, wrapping rather than ellipsised, in a monospaced face so a character-by-character comparison lines up instead of drifting under proportional spacing. A FilledIconButton beside it copies the key via LocalClipboardManager, the same way ShareProfileScreen and the image viewers already do it. The "Key: " prefix became a label above so the key gets the full width. No copied-confirmation toast, matching ShareProfileScreen: Android shows its own clipboard notice on 13+, and a snackbar here would double up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
cae50ce359 |
feat: hold the ritual until its owner approves each step
The ChillDKG ritual ran entirely on its own. `acceptProposal` published this
device's host key the moment a PROPOSAL arrived from a relay, and `advance`
published rounds 1 and 2 as soon as their inputs landed. Receiving a nostr event
was therefore enough to enrol the owner of a phone in a group's permanent signing
quorum, without anything having been shown to them first.
Nothing of this device's own now goes out before its owner says so. Three
approvals, because each publishes something different and commits the member to
something different:
host key joins the ceremony, and fixes n. A member who joins and then stops
answering does not merely fail to help -- the ritual cannot finish
without every member, so they hold it open for everybody.
round 1 contributes to the key itself. The member's own secret material
starts shaping a key they will be expected to help sign with.
round 2 confirms the coordinator's combined result matches what this device
sent. A check rather than a formality: it is what stops a coordinator
substituting a key the members never contributed to.
The coordinator's two aggregations are deliberately not gated. They relay other
members' already-published messages and disclose nothing of the coordinator's own,
so an approval there would stall the whole group on one person's attention without
protecting anybody. The member who opens a ceremony is auto-approved for the host
key alone -- starting one is already the act of agreeing to be in it -- and is
still asked for rounds 1 and 2, which publish key material.
Each gate returns rather than throwing. The ritual is not failing, it is waiting
on a person; everything already received stays stored, so it resumes the moment
they approve. `pendingApproval` mirrors those gates exactly and has to keep doing
so: if the two disagree the screen offers an approval that does nothing, or none
while the ritual sits still.
Schema v2 -> v3 adds four nullable columns to DkgSession -- three approval
timestamps and `approvalRequestedThrough` -- so Room generates the migration. A
ritual already in flight comes back with all three null, which reads as "not
approved yet" and simply asks, rather than silently continuing.
## Being asked
Three screens rather than one parameterised by step, because each is making a
different case and the copy is the substance of the screen, not decoration around
it. They share a scaffold for one reason that is not cosmetic: a screen opened for
one step can go stale -- a redelivery carries the ritual forward, or the member
approves on another device -- so it re-checks the pending step before offering a
button, and `approve` checks again in the manager and ignores a mismatch.
"Not now" does not refuse on the member's behalf. There is no "no" in ChillDKG
short of abandoning the ceremony, and quietly leaving is what a member who is not
ready actually wants; abandoning stays on the ritual screen where the consequence
can be spelled out.
A chat line announces each request, written once per step and guarded by
`approvalRequestedThrough` -- `advance` runs on every arriving message and would
otherwise ask again on each one. It is the one ritual line that asks rather than
reports, so it is the one that is not quiet: primary tint, a Review affordance,
and a tap through to the ritual screen, whose bottom bar routes to the step the
ceremony is actually waiting on.
## Telling the steps apart
The request started as a single message type, which meant one icon for all three
and no way to tell "join the ceremony" from "confirm the key". The type is the
only thing a transcript keeps -- a line drawn days later has no session to ask
what was being requested -- so the step moved into it, one type per step, and
every stage now carries its own icon.
MIGRATION_3_4 rewrites the rows already written. They cannot regenerate: a request
is announced once, so a ceremony already in flight would keep its undifferentiated
icons forever. It changes no schema at all -- the version bump exists only to give
a data rewrite somewhere to run, which is why it is a manual migration on the
builder rather than another AutoMigration. Rows it cannot match keep the old type,
which the renderer still recognises.
An answered request shows a checkmark where Review was. Whether it was answered
comes from the transcript rather than the session: approving is the only thing
that causes the step to be published, and publishing writes an authored line, so a
matching line at or after the request means done. That keeps a room that has run
more than one ceremony correct -- ChatMessage has no session id to disambiguate
with -- and needs no DkgRepository in the message list. The comparison is on
createdAt rather than list position, because the list is ORDER BY createdAt DESC
with reverseLayout, where index arithmetic runs backwards.
Compiles and assembles; the ordering test still passes. No ritual has been run on
a device.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
6a5b6cd6cb | Add ephemeral Relays.kt | ||
|
Kgothatso Ngako
|
d9d27cd0ea |
fix: stop one bad request or one silent relay from stalling all synchronization
Two ways the sync queues could stop draining and never recover.
## A request that throws while loading the local set is never retried, and blocks
## every request behind it
The pending queue is a single row at a time:
SELECT * FROM NegentropySynchronizeRequest WHERE status = 'pending'
ORDER BY createdAt ASC, id ASC LIMIT 1
observed through distinctUntilChanged. The pump advances only when the head row
changes status, and the negentropy request was marked "sent" AFTER the storage
vector was built. StorageVector can throw on the way in -- insert() requires
exactly 64 hex characters, and seal() rejects a duplicate (timestamp, id) with
"duplicate item inserted". guardPump caught the throw and logged it, which kept
the pump alive but left the row at "pending". Nothing else observes that status,
the flow will not re-emit an unchanged row, so the request was neither retried
nor skipped: it sat at the head of the queue and every negentropy request queued
after it waited behind it for the life of the process.
The vector build now filters and de-duplicates on the way in -- a row negentropy
cannot index is one this device cannot reconcile, and dropping it costs one
event's worth of extra transfer where letting it through costs the entire sync --
and the request is claimed either way, so a failure that does get through logs
and lets the queue move on.
## A relay that opens a subscription and then goes quiet parks a slot forever
Both pumps take a permit from subscriptionSlots (4 across all relays) and hold it
for the life of the collection. The collection ends on EOSE, CLOSED or NEG-ERR --
none of which a relay is obliged to send. A negentropy exchange in particular
ends when reconcile() says so; if the relay simply stops answering mid-round,
nothing completes the flow. Four such subscriptions hold every permit and the
queue stops, with no error anywhere: the requests are marked "sent", so the UI's
pending count reads zero while nothing is being fetched.
Both are now bounded by SUBSCRIPTION_TIMEOUT (120s), which covers the collection
itself. The REQ pump is included because it is how negentropy's needIds are
actually fetched -- a wedged REQ slot breaks negentropy sync just as directly as a
wedged NEG one. Generous rather than tight: cutting a slow but live download short
costs a re-fetch next pass, and a REQ can now carry up to 500 ids. The existing
NEG-CLOSE/CLOSE in the finally block already runs under NonCancellable, so a
timed-out subscription still says goodbye to the relay.
Not covered by tests: both failures are timing and Room behaviour on the sync
path, neither of which runs under :composeApp:testDebugUnitTest. Verified by
compilation and by reading the queue's DAO query against the pump's collection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
2d29fc0a37 |
fix: reconcile with a relay to completion instead of stopping after one round
Negentropy is a multi-round protocol. The initiator opens with fingerprints over
its whole set -- 16 buckets, per kmp-negentropy's BUCKETS_IN_MESSAGE -- and the
peer answers each bucket either by agreeing (a skip), by listing the ids in that
range, or, when the range still holds more than 32 items on its side, by
splitting it into 16 finer fingerprints. Only the ranges that come back as id
lists produce have/need ids. Everything still under a fingerprint needs another
NEG-MSG from us, and reconcile() says so by returning a non-null `msg`; it
returns null exactly when there is nothing left to ask about.
This client discarded result.msg and never sent a second NEG-MSG. Worse,
isTerminalFor() listed NegentropyMessage as terminal, so completeOnSubscriptionEnd
ended the flow on the FIRST one -- the collector finished, the finally block sent
NEG-CLOSE, and a reconciliation the relay was still in the middle of was
abandoned. With 16 buckets a single round tells you almost nothing about a set of
any size: for anything past a couple of dozen events the exchange was torn down
before it had located most of the difference, and the ids it did find were
whichever handful happened to resolve at depth one.
The old comment on isTerminalFor described this as a deliberate design ("this
client reconciles in a single round"), which is what kept it in place. It is not
a design one can choose -- the protocol has no single-round mode. What it
produced was a sync that mostly did not sync, hidden behind a diff that was never
empty and a REQ fallback that quietly did the real work.
## The loop
NegentropyMessage is no longer terminal. The collector feeds each NEG-MSG to
reconcile(), accumulates the round's needIds/sendIds, and while `msg` is non-null
sends it straight back on the same subscription via the new
RelayPool.sendNegentropyMessage. When reconcile() returns null the exchange is
over -- a fact only the caller can see, since a relay owes us no EOSE for a NEG
session -- so a `transformWhile` on the flow ends the collection there. The
predicate reads a flag the collector sets, which works because a flow's
downstream collector runs synchronously inside emit().
MAX_NEGENTROPY_ROUNDS caps the ping-pong at 32 in case a peer's ranges never
converge; a healthy exchange settles in far fewer, since each round splits the
disagreeing ranges 16 ways.
## Acting once, at the end
Follow-ups moved out of the per-message branch into applyReconciliation, called
after the exchange. Acting per round would have queued a REQ for ids that later
rounds were still discovering. It runs outside the try and under NonCancellable
so an exchange that is cut short still acts on what it did reconcile rather than
discarding the rounds it paid for.
Two fixes came with the move:
- needIds go out chunked at 500 per REQ. Relays cap the length of a filter's
`ids` array (1000 is common) and a first sync can reconcile thousands; a
single oversized REQ is answered with a CLOSED, or silently truncated, which
loses every id past the cap. Previously all of them went in one filter --
survivable only because one round never found many.
- the "do we actually hold this?" check on sendIds is a Set lookup instead of
`in` on a List, which was a linear scan per id over the whole local set.
Also dropped two logger.d calls that dumped every local event id and every local
timestamp on each NEG-MSG. At one line per message that was tolerable; at one per
round over a real set it is megabytes of logging on the hot path.
Not covered by tests: this is websocket exchange behaviour with a live relay.
Verified by compilation and by tracing kmp-negentropy's Negentropy.reconcile
against quartz's own NegentropySession, whose documented usage is the same loop
("If processMessage returns a non-null NegMsgCmd, send it back / repeat until a
result with a null command").
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
661a5caa17 |
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>
|
||
|
Kgothatso Ngako
|
d8729c5bff |
fix: sync group chat against live messages, not expired ones
getMarmotGroupEvents is the local half of a negentropy exchange for the
mlsMessages purpose: it answers "which kind-445 events for these rooms does this
device already hold", and the answer is compared against the same question asked
of the relay. Its expiry predicate read
(expiresAt IS NULL OR expiresAt < :expiresAt)
with :expiresAt bound to Clock.System.now(). That keeps a row whose expiry is in
the PAST and drops every row still within its lifetime -- the exact inverse of
what a relay serves. NIP-40 says an expiring event is one a relay should stop
returning once its expiration tag has passed, so for every group message with an
expiration the local set handed to negentropy was the complement of the relay's.
The consequence is not a silent no-op. Reconciliation reports the symmetric
difference, so an inverted set turns every live message into an id the relay
believes we are missing (re-downloaded on every pass) and every expired message
into an id we believe the relay is missing (queued for re-broadcast). Group chat
therefore paid full transfer cost on every sync while pushing dead events back at
the relay -- which is also why the bug was invisible: messages still arrived,
just via the diff rather than the fast path.
Flipped to `expiresAt > :now`, and the parameter renamed to `now` since it is the
clock, not a bound on the column.
## Time bounds
NIP-01 `since`/`until` are inclusive: `since <= created_at <= until`. The query
used a strict `createdAt > :since` and had no `until` at all, so an event stamped
exactly on the boundary was in the relay's set and not in ours, and everything
newer than a requested `until` stayed in ours after the relay had excluded it.
Both are now applied inclusively; the one call site passes
Instant.DISTANT_FUTURE when the filter carries no upper bound.
## Ordering
ORDER BY flipped to createdAt DESC. It is irrelevant when the caller asks for the
whole set (negentropy sorts into its own vector regardless), but the parameter is
a LIMIT: a relay answering a limited filter returns the NEWEST matching events,
and ascending order returned the oldest.
Not covered by tests: Room DAO behaviour needs a sqlite driver, which
:composeApp:testDebugUnitTest does not have. Verified by KSP codegen -- the
generated NostrEventDao_Impl carries the corrected predicate -- and compilation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
3576c00ce2 |
feat: put every ritual message in the group's chat, naming who sent it
|
||
|
Kgothatso Ngako
|
74c352ab35 |
fix: show NIP-17 messages that arrive, not just the ones you send
A NIP-17 room only ever displayed your own words. Sending worked end to end -- sendChatMessage queues the gift wrap and writes a local ChatMessage so you see what you typed -- but nothing on the inbound side ever wrote a row for a message that arrived. The kind-14 branch decrypted the payload, stored it, built the chat room from its p-tags, updated the subject, queued profile and relay-list syncs, and stopped. The feed is `SELECT * FROM ChatMessage WHERE chatRoomId = ?`, so with no row written there was nothing to show. Every write of a ChatMessage in the tree confirms it: the sender's own copy in DatabaseChatRepository, three MLS outbound sites, ChatMessage.fromGroupEventResult for inbound MLS group events, one commented out in the welcome branch, and the ritual notices. Nothing for an inbound gift wrap. MLS rooms were never affected, which is why this survived -- CONVENIENT groups render both directions. persistInboundChatMessage files the message once the room is known to exist. ## Two arrivals are deliberately not filed A message already filed. Relays redeliver and negentropy re-syncs the same gift wraps, and the same wrap yields the same payload id every time, so a lookup on giftWrapPayloadId makes a redelivery a no-op. It has to be checked rather than relied on: ChatMessage.id is autogenerated, so a second insert is simply a second line in the conversation. Our own words coming back. sealGiftWrapPayload wraps a copy to every participant of the room including the sender, so a message returns to the device that sent it -- and that device already wrote the row on the way out. Left alone, every message you sent would appear twice. The two copies of your own message cannot be matched on the payload id, which is the interesting part: the outbound row is keyed on EventHasher.hashId over the rumor, while GiftWrapSeal.decryptGiftWrapPayload keys the inbound one on the seal's id. Same message, two ids -- and since every recipient gets their own seal, the same message has a different id on every device that receives it. So the sender is matched instead, which costs multi-device: a second install of the same identity will not pick up messages sent from the first. Keying the inbound payload on the rumor it came from would fix both, and would make payload ids agree across devices, but it changes identity for every gift-wrapped kind rather than just this one and belongs in its own change. ## Timestamps come from the rumor NIP-17 fuzzes the seal and the wrap by up to two days to frustrate correlation, so ordering the feed by either would shuffle the conversation into nonsense. The rumor keeps the real time and that is what the row records. ## Scope Kind 14 only, which is the kind this app sends. A kind 15 file message from another client still falls through to the "Unsupported event" log, as before. Not covered by tests: this is Room writes on the inbound path, which does not run under :composeApp:testDebugUnitTest. Verified by compilation and by tracing the branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
3f4f05162d |
feat: say who opened the ceremony on the shared key screen
The screen showed the quorum, the ladder and now the roster, but never named the member whose ceremony it was. Any member can open one and it settles the group's signing quorum for good, so who opened this one belongs on the screen that describes it — the same reason the chat notice names them. Alice started this ceremony. 2 of 3 members will be needed to sign with this key. It sits above the failure branch so it holds in every state. A ceremony that was abandoned or has already produced a key is still worth attributing: a member arriving at a finished ceremony they do not remember agreeing to should be able to see whose it was, and DkgSession.coordinatorPublicKey is kept for the life of the row either way. ## One naming rule, in one place HexKey.memberName() resolves a member's display name from the profiles joined onto the room, falling back to a shortened key. The roster switched to it, so the opener line and the rows below it cannot disagree about what to call somebody, and the chat notice's copy of the fallback went with it. This replaces a second private SHORTENED_PUBLIC_KEY_LENGTH I had added to ChatMessageListViewModel. A third still lives in SelectChatRoomTypeViewModel at a different value (12) and is deliberately untouched: that is a different choice about a different surface, not a duplicate of this one, and folding them together is a call about that screen rather than about this feature. ## Still reads a raw key on one surface The abandoned card shows DkgSession.failureReason verbatim, which for a ceremony ended by another member begins "Abandoned by 1a2b3c4d:" -- a truncated key where the chat line now shows a name. Naming them there needs the culprit stored beside the reason rather than inside it, which is a column on DkgSession and a schema version, so it is left as it is rather than parsed back out of the string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
3fad331969 |
feat: name every member on the shared key ceremony screen
The ladder said "2 of 3" and stopped there. That is the one thing a stalled
ceremony never needs explaining — you can see it is stuck. What the group has no
way to find out is *who* it is stuck on, and since a ChillDKG ritual cannot
finish until every member's device has taken part, knowing whose door to knock on
is the group's entire recourse.
A roster now sits under the ladder, one row per member, each showing how far they
have got:
✓ You confirmed everyone's part
✓ Alice committed their part
○ Bob not here yet
The wording is deliberately about what a member has *done* rather than a rung
number, since the rungs are named for the group's progress ("Round one") and a
member's own state is a different question.
Each member is named by the furthest round they have published, because that is
the only thing this device knows about them for certain — there is no liveness
signal in ChillDKG, and a member who published a host key an hour ago and then
closed the app is indistinguishable from one still working.
## Members, not counts, in the state
DkgRitualUIState carried three Ints. It now carries the three sets of public keys
they were counting, with the counts derived, so the ladder keeps working
unchanged and the roster has something to name people from.
The roster is drawn from `ritualMembers`: the room's participants deduplicated,
plus anyone who has published a ritual message and is not among them. The union
matters because the two sources can disagree — `n` is fixed from the proposal's
p-tags while the room's rows are local and can drift — and somebody who has
actually taken part is in the ceremony whatever the room's rows say. Showing a
count of 3 above a list of 2 names would be the worst of both.
Members are sorted by public key rather than by progress, so a row does not jump
around under the reader's finger as messages arrive.
## Only while it is running
The roster is skipped once a ceremony is COMPLETE, where the key card says
everything, and it is never reached for a FAILED one, which returns early on the
abandoned card. A half-climbed ladder of names next to "the ceremony was
abandoned" is noise: the ritual is over and who got how far no longer changes
what anyone should do.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
95db7c2f73 |
fix: name the member behind a shared key ceremony notice
The ritual notices landed without an author. Dropping the bubble was right --
"a shared key ceremony started" is not something the coordinator said -- but
dropping the actor with it threw away the part of the line that matters most.
Any member can open a ceremony, and it settles the group's signing quorum for
good, so *who* opened this one is exactly what the group needs to see. Same for
who abandoned one.
System lines carry the actor in the sentence rather than in a header, so the
content of the authored types is now a predicate to be read after a name:
Alice started a shared key ceremony. It will take 2 of 3 members to sign
with the key, and it finishes once everyone has taken part.
Bob abandoned the shared key ceremony. No key was created, and it is safe
to run it again.
✓ The group has a shared key. It takes 2 of 3 members to sign with it.
A finished ceremony keeps no author, which is why DKG_AUTHORED_TYPES is a subset
rather than all three: the group ends up with a key, nobody hands it to them.
## The actor is resolved at render time, not written into the content
The manager could look the name up when it writes the row, and it would be wrong
twice over: the name would be frozen against later renames, and a member first
seen through this very proposal is sitting on the "LOADING..." placeholder that
getOrCreateNip17ChatRoom just inserted for them -- so the line would read
"LOADING... started a shared key ceremony" forever. LocalChatMessage already
joins Profile on senderPublicKey, so the renderer resolves it live, colours it
with ProfileColor like the message bubbles do, and falls back to a short key
when there is no profile yet.
## senderPublicKey now holds who acted
It was the coordinator on all three notices, which was wrong for an abandoned
ceremony: the member who sent FAILURE is the one who ended it. fail() takes a
culprit -- the FAILURE sender, defaulting to this device for a fault raised
locally, which amounts to the same thing from the group's side since hitting one
makes this device broadcast FAILURE in turn. isUserMessage follows from it, so
the line reads "You" for your own actions.
The fault itself is deliberately no longer in the chat line. "Abandoned by
1a2b3c4d: ChillDKG round 2 failed: a participant is faulty (participant 2)" in
the middle of a sentence about who walked away reads badly, and the detail is
already on the ritual screen the notice taps through to. DkgSession.failureReason
keeps it verbatim, so that screen is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
61480dd8b7 |
feat: tell the group in chat when a shared key ceremony happens
A ChillDKG ritual was invisible to everyone it happened to. Kinds 30310-30316
are routed to ChillDkgRitualManager and never become ChatMessage rows, so a
member's device published a host key and joined a ceremony that fixes the
group's signing quorum for good, with nothing appearing anywhere they would
look. The only way to find out was to open the group's details and press Shared
Key on the off chance. Worse in the flow this is reached through: a group whose
first event is the ceremony now materialises as a room with no messages in it at
all and no explanation of why it appeared.
That matters more than it would for a chat feature, because a ritual cannot
finish until every member's device has taken part. The progress ladder on the
ritual screen exists to show that it is waiting on 2 of 3 -- but nothing told
member 3 they were the one being waited on.
Three milestones now land in the transcript: the ceremony starting, the group
getting a key, and the ceremony being abandoned (with the reason, which names
the culprit participant when ChillDKG identified one).
## Derived locally, not sent
Nothing new goes on the wire. Every member already receives the proposal, and
computes the completion and any failure for themselves, so each device writes
its own row from what it already has. That costs no traffic, needs no new event
kind, and -- the reason it is worth doing this way -- makes it impossible for the
transcript to disagree with the ritual it describes. A "ceremony started" message
that was itself sent could arrive without the proposal, or outlive a session that
never existed on that device.
The rows are written where the state changes: announceStarted() at both places a
DkgSession is created (proposeRitual for the member who opens it, acceptProposal
for everyone else), and announce() at the COMPLETE write and in fail().
They are written once by construction rather than by de-duplication, which is
worth spelling out because ChatMessage.id is autogenerated and a second insert
would simply be a second line. A session is created once, since acceptProposal
returns early when the row exists; advance() leaves a COMPLETE ritual alone; and
fail() now re-reads the session and returns if it is already FAILED. That last
one is also a fix in its own right -- a ritual can be failed from two directions,
a FAILURE message from a member and a fault raised locally, and while the second
write was previously harmless it would now have told the group twice.
## Rendered as a system line, not a bubble
ChatMessage.messageType already carries "message", "artifact", "pendingCommit"
and eleven others, so TYPE_DKG_STARTED / _COMPLETE / _FAILED join it with no
schema change. But the list renders every row as a bubble with the sender's name
and a delivery-status icon, and neither fits: "a shared key ceremony started" is
not something the coordinator said, and a row with no gift wrap behind it would
show the KeyOff "unsealed" icon as though it had failed to send.
RitualNotice renders them across the width instead -- icon, text, timestamp, no
author, no side, no delivery state -- and is tappable through to the ritual
screen, since the point of telling the group is to give them somewhere to go. It
branches out of the items() lambda with an early return so the existing bubble
layout is untouched.
The other informational types ("pendingCommit", "processedCommit",
"proposalStaged", "undecryptableOuterLayer") have the same problem and are
deliberately left alone: how MLS commit rows should read is a separate call from
making the key ceremony visible.
## Not covered by tests
The whole change is Room writes and Compose rendering, neither of which runs
under :composeApp:testDebugUnitTest -- there is no sqlite driver on the JVM test
classpath. Verified by compilation only.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
61869f0046 |
test: run a real ChillDKG ceremony through the ritual's ordering rules
ChillDKG has no session-params object the group agrees on out of band: every step
takes the host public keys and the threshold and hashes them into the session
identity itself. A group whose devices order their participants differently
therefore gets no key at all, and nothing in the protocol tells you that is what
went wrong. ChillDkgRitualManager has each device derive that order
independently -- sort the collected host keys, and order each round's messages by
their sender's host key to match -- and until now nothing checked that the two
rules agree, or that they agree with what ChillDKG expects.
Four tests, against the real library rather than a stand-in:
a ritual ordered by host key produces one shared key
A full 2-of-3 run -- step1, coordinatorStep1, step2, coordinatorFinalize,
participantFinalize -- with the participant set built by hostPublicKeys()'s
rule and both rounds ordered by orderedPayloads()' rule. Asserts every
member lands on the same threshold public key and on distinct shares.
sorted host keys give every device the same participant order
The same members in three arrival orders, since relays deliver host keys in
whatever order they please, must sort to one order.
one device ordering participants differently gets no key
The negative that keeps the other two honest: with one member running the
same people in another order, some step has to fault. Without this a broken
ordering rule could pass the happy-path test by being uniformly broken.
host keys are not the nostr keys they come from
deriveHostSecretKey's two obligations: it must not hand ChillDKG the nostr
identity key (a flaw in either protocol would otherwise reach the other),
and it must be deterministic, or a reinstall cannot recover the share.
These live in commonTest and run under `./gradlew :composeApp:testDebugUnitTest`.
The secp256k1 natives do load there: the Android loader fails and falls back to
extracting the JVM platform build, so these are real curve operations, not
mocked ones. Room-backed code still cannot be tested this way, which is why the
manager's database behaviour is not covered here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
90a28e9321 |
fix: let a group actually finish a ChillDKG ritual
Nine defects, one of them enough on its own to stop any ritual from ever getting
past the member who opened it. They are committed together because the fixes
interlock: the coordinator fix rewrites the same guard that derives the
participant count, and the restart-safety rewrite replaces the control flow the
rest of them live in.
## The proposal was dropped by everyone it was sent to
acceptProposal accepted a proposal only from `localChatRoom.chatRoom.userPublicKey`
or from `coordinatorOf(localChatRoom)` -- which returned that same column, so the
two arms of the test were one test. And that column is not the room's creator: it
is the logged-in user, for multi-account support ("Logged in user PublicKey...
should help us have multiple user support"), which is why NostrDao sets it to
`activeKeyPair.pubKey.toHex()` on every room it builds. So the guard read "the
sender must be me", every recipient logged "DKG proposal from non-coordinator"
and dropped it, and the ritual never left the coordinator's device.
The same wrong notion made DkgRitualViewModel.isCoordinator() true on every
device, so every member was shown "Start key ceremony" and could open a competing
ritual.
There is no creator to recover. NIP-17 records none, ChatRoom has no such column,
and initialGiftWrapPayloadId is whichever member's message happened to arrive
first -- not the creator. So the coordinator is now simply whoever opens the
ritual, which is what ChillDKG assumes anyway: the coordinator relays, cannot
learn a secret and cannot bias the key, so being it confers nothing worth
reserving. The UI follows: canStartRitual() replaces isCoordinator(), and the
"Waiting for the group's creator to start it" copy is gone with the notion.
## Devices could disagree on n, which is fatal to a DKG
participantCount came from each device's own `localChatRoom.localParticipants.size`.
ChillDKG has no session-params object to agree on out of band -- every step hashes
the host public keys and the threshold into the session identity -- so two devices
that count n differently do not get a weaker key, they get no key. Local
membership is exactly the thing that drifts between devices.
n now comes from the proposal itself: its p-tags plus its sender, the set every
receiver sees identically. proposeRitual builds the same set the same way
(`memberPublicKeys(room) + userPublicKey`), so both sides count alike even if the
room's own rows have drifted.
Participant rows are also deduplicated by pubkey. Participant is keyed on an
autogenerated id, so upsert can leave the same member in a room twice, which
inflated n and double-p-tagged that member.
## A device killed mid-round stalled the ritual for everyone
advance() branched on DkgSession.stage, and each branch wrote the next stage
before publishing the message that stage stands for. A device that died in
between came back with the stage already advanced, skipped the branch, and never
published its round message -- and a DKG needs every member, so the whole group
waits forever on one that will never speak again.
advance() now asks what is *stored* -- "is my round-1 message out yet?" -- and
takes every step the stored messages allow, in order, stopping at the first one
still waiting on somebody. The stage is demoted to a label for the UI and only
ever moves forward (DkgRitualStage now documents that its declaration order is
the ladder, since the manager compares ordinals). publishOwn broadcasts before it
records, so a crash between the two costs a duplicate broadcast -- which every
receiver folds away on a keyed upsert -- rather than a message the group waits on
forever. The recursion is gone with the stage branching, and participantStep1's
result is reused instead of being recomputed for participantStep2.
## Messages that beat their proposal were thrown away
Gift wraps carry a randomised created_at (TimeUtils.randomWithTwoDays) and relays
hand them back in no particular order, so a round-1 message routinely lands
before the proposal that opens the ritual. Those arrived with no session to file
them under and were dropped, and nothing ever asked for them again.
They were never actually lost: the inbound path upserts every payload it decrypts
before it dispatches on kind. replayStoredMessages reads them back through the
new GiftWrapPayloadDao.getByChatRoomAndKinds as soon as the proposal creates the
session. No schema change -- the payloads were already there.
## Stale copies of the session clobbered each other
Handlers held a DkgSession across several writes and each `copy`d from its own
snapshot, so a later write silently reverted an earlier one -- aggregateRound1
storing cmsg1, then the failure path copying from a session fetched before it.
Every session write now goes through update(), which re-reads the row first, and
fail() with it, so a failed ritual keeps the progress it actually made.
## t = 1 was accepted from the wire
The threshold was taken straight off the proposal's tag. ChillDKG will happily
generate a 1-of-n "threshold" key that any single member can sign with, so the
check has to be ours: a proposal is now refused unless its threshold sits in
ChatRoomType.quorumRange(n), on the proposing side as well as the receiving one.
That also rules out t > n, which would otherwise throw inside ParticipantState1
and fail the session with a local input error.
## Anyone could pass themselves off as the coordinator
cmsg1 and the certificate were stored from whoever sent them. Both come from the
coordinator and only ever once, so a member could stall a ritual by getting a
bogus one in first: the real one would then be ignored as already-set. Both are
now accepted only from session.coordinatorPublicKey.
## Sorting host keys was not case-safe
The participant order is a sort of the host public keys, and it is the *order*
that has to match on every device, not the bytes. Hex from another client could
arrive upper case, parse fine, and sort into a different position -- silently
reordering the participant set and failing the session with no clue why. Both
sort sites now case-fold first.
## Cancelling the sync abandoned the ritual, and told the group to
advance() caught Throwable, which includes CancellationException, so tearing down
a coroutine scope marked the session FAILED and broadcast FAILURE to everyone.
NostrDao already rethrows cancellation for this reason; advance() now does too.
## Also
hostPublicKeys() and orderedPayloads() gated on `size < n` and then used whatever
they had; more host keys than the ritual was opened for now fails loudly instead
of running ChillDKG on a participant set nobody else has. quorumRange() no longer
returns a backwards, empty range for a room below the minimum, which coerceIn
rejects outright, and the ceremony is not offered at all below two members.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
739314ccb4 |
fix: stand up the chat room a ChillDKG proposal arrives for
Creating a NIP-17 group sends nothing to anybody. Membership under NIP-17 *is* the p-tag set on each message, so the group only materialises on the other members' devices when the first gift wrap lands. The chat-message branch of the inbound path knows this and builds the room from the arriving payload's p-tags; the ChillDKG branch did not. It looked the room up, found nothing, logged "DKG payload for unknown chat room" and dropped the message. That is fatal for the flow the feature is reached through. SelectChatRoomTypeViewModel.createNip17ChatRoom writes the room and its participants locally and navigates straight into the chat without publishing anything, and ChatRoomDetailScreen offers "Shared Key" for exactly these rooms (mlsGroupState == null). So "create group -> Shared Key -> Start key ceremony" makes the ritual's own proposal the first event the group is ever heard of, and every recipient dropped it. Nobody joined, and the coordinator sat on one host key -- its own -- forever. getOrCreateNip17ChatRoom builds the room the way the chat branch does: the payload's p-tags plus its sender. That set is the aggregate ChatRoom.id is derived from in the first place, so any payload that routes here already carries the whole membership and there is nothing else to wait for. Placeholder profiles are inserted first because both ChatRoom.userPublicKey and Participant.participantPublicKey are foreign keys onto Profile, and a payload whose membership does not include this device is refused rather than used to build a room we are not a member of. Deliberately not shared with the chat branch: that block also queues relay-list and profile synchronisation per participant, which is best-effort enrichment tangled into the surrounding loop's profilePublicKeysToSync map. Lifting it out is worth doing on its own, not inside a fix whose job is to make the ceremony reachable at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
a611277d5d |
build: drop six unused dependencies and their catalog entries
None of these are imported by any source file in composeApp, generated sources
included, and none is the kind of dependency that gets used without an import.
androidx.paging:paging-common no androidx.paging import anywhere; no DAO
androidx.paging:paging-compose returns PagingSource and nothing calls
collectAsLazyPagingItems, so the Room-Paging
integration that would need it is not in use
androidx.work:work-runtime-ktx no Worker, CoroutineWorker or WorkManager
reference, and no provider or initializer entry
in AndroidManifest.xml
okhttp3:okhttp-coroutines redundant rather than unused: quartz-android
1.14.0 depends on it and at 5.5.0, which was
already upgrading this declaration's 5.4.0. It
stays on the runtime classpath either way
com.ionspin.kotlin:bignum no com.ionspin import. Arrived in
|
||
|
Kgothatso Ngako
|
fa3c00a8ff |
build: drop the secp256k1-frost-kmp submodule
The previous commit moved the ChillDKG ritual onto bitcoin-kmp's native
implementation, which was the only thing in this app using this library. Nothing
imports `ac.cord.auxiliary.frost` or `ac.cord.auxiliary.cryptography` any more, so
the submodule, its composite build and the dependency on it all go.
Removed:
- the `secp256k1-frost-kmp` submodule (deinit, git rm, and .git/modules cleared)
- its entry in .gitmodules
- `includeBuild("secp256k1-frost-kmp")` in settings.gradle.kts, with the
surrounding comment made singular now that one composite build remains
- `implementation("ac.cord.auxiliary:library:1.0.0")` in composeApp
Note that `ac/cord/auxiliary/compose/**` under composeApp/src/jvmMain is this
app's own code in a similarly-named package, unrelated to the library and
untouched.
Two commits made on the submodule to keep it building inside this composite --
gating its ios targets on macos, and moving it to AGP 9.4.0 -- existed only in
this repository's .git/modules copy and are destroyed by the removal. They were
exported as patches first. Neither is a loss worth chasing: both existed purely to
make that project cooperate with a composite build that no longer includes it.
The library is not gone from the world, only from this build. It still holds a
FROST signer, which is the obvious next need now that the ritual produces
threshold key material. But bitcoin-kmp ships `fr.acinq.bitcoin.crypto.frost.Frost`
against the same natives this app now packages, so the signer will almost
certainly come from there rather than from a second copy of secp256k1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
c9b6bd7992 |
feat: run the ritual on bitcoin-kmp's native ChillDKG
Replaces `ac.cord.auxiliary.frost.dkg.chill.ChillDkg` with
`fr.acinq.bitcoin.crypto.dkg.chill.ChillDKG` throughout ChillDkgRitualManager.
The implementation being dropped says what it is in its own header:
WARNING: This code is slow and not hardened against side channel attacks. Do
not use for anything but tests.
It is a 1090-line "Reference port of ChillDKG (chilldkg_ref/chilldkg.py)" doing
BigInteger arithmetic through ac.cord.auxiliary.cryptography's Scalar and
GroupElement, with no call into libsecp256k1 anywhere in the file.
secp256k1-frost-kmp's own KNOWN_ISSUES.md confirms the constant-time
libsecp256k1 backing is used only for hashing. Real key material was being
generated by it.
The replacement is 438 lines in which every operation delegates to
`Secp256k1.chilldkg*` -- the constant-time C from the secp256k1-zkp fork the
submodule chain compiles. This is an improvement, not a clean bill of health: that
module carries its own "experimental and must not be used in production" warning.
It is constant-time C instead of variable-time Kotlin, which is the part that
mattered.
Three things changed shape rather than just types.
SessionParams no longer exists. ChillDKG takes the host public keys and the
threshold at every call and hashes them into the session identity itself, so
`sessionParams()` becomes `hostPublicKeys()` returning List<PublicKey>, and the
threshold rides along on the session row. That removed a parameter from four
signatures.
Faults are values now, not exceptions. ChillDKG reports a faulty participant as a
ChilldkgFault field on each result because it is a normal outcome of a DKG rather
than a bug. This ritual has exactly one response to all of them -- the key is
unusable, so the session dies and the group is told -- so `raiseIfFaulty` turns
them into an exception and lets them join advance()'s existing single failure
path. The gain is in the failure text: the reason shown to the group goes from
whatever `e.message` happened to hold to "ChillDKG round 2 failed: a participant
is faulty (participant 3)". On a failed DKG, which participant to blame is the
only actionable thing there is.
Two recomputes got names. The old code inlined a second participantStep1 call to
rebuild state1 for participantStep2, and rebuilt coordinator state separately in
aggregateRound2. Those are now `participantState1()` and `coordinatorStep1()`, the
latter carrying the fault check so both of its callers get it. The
recompute-rather-than-store design is kept deliberately: ChillDKG's states are
serializable and could be persisted, but they are pure functions of inputs the
DkgSession row already holds, so storing them would mean a schema change and a
Room migration for no behavioural gain. That reasoning is now in the kdoc.
Also drops an unused hostSeckey parameter from aggregateRound2, whose signature
was changing anyway, and updates doc comments in DkgRitualEvents, DkgSession and
DkgThresholdTag that named types which no longer exist -- ChillDkg.ParticipantMsg1
and friends -- to the protocol's own names: pmsg1, cmsg1, CertEq signature,
certificate.
Compiles clean, but no ritual has been run on a device. The natives are in the APK
as of the previous commit; the first real ritual is the actual test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
eee8fa0fd8 |
build: package the android secp256k1 natives, with ChillDKG in them
The android app had no android secp256k1 natives at all, and had not had any for
as long as lightning-kmp-core has been a dependency. `dependencyInsight` on
debugRuntimeClasspath resolved every secp256k1 coordinate to
`:secp256k1-kmp:jni:jvm:{darwin,linux,mingw}` -- desktop .so/.dylib/.dll files,
loaded by extracting them from a jar, which cannot work on a device.
The cause is a chain of individually reasonable decisions. lightning-kmp-core
publishes no android variant, so an android consumer resolves it to the jvm one;
the jvm variant asks for `secp256k1-kmp-jni-jvm`; and nothing anywhere asks for
`secp256k1-kmp-jni-android`. lightning-kmp-app names it only in androidDeviceTest,
so consumers of the published library do not get it. It has to be named here.
Naming it alone would not have been enough. bitcoin-kmp's settings.gradle.kts
substituted five of secp256k1-kmp's coordinates to the fork's projects but not
-jni-android, so it would have resolved from Maven Central to stock 0.24.0 --
built from upstream libsecp256k1, with no ChillDKG module. Because the kotlin API
comes from the substituted root project, that combination type-checks and links
and then fails with UnsatisfiedLinkError at the first native call. The rule is
added in the submodule commits this carries.
Submodule commits carried here:
lightning-kmp-app ce1ed01 -> 6434282 build: carry the jni-android substitution
down from bitcoin-kmp
experimental/lightning-kmp 77be7b78 -> 5103b79e
experimental/bitcoin-kmp 196a479 -> 65c4aab build: substitute
secp256k1-kmp-jni-android to the
included build too
Verified through the artifact rather than the graph: composeApp-debug.apk now
carries lib/{arm64-v8a,armeabi-v7a,x86,x86_64}/libsecp256k1-jni.so, and `nm -D` on
the arm64 one exports all twelve Java_..._chilldkg_... JNI entry points --
hostpubkey_gen, params_hash, participant_step1/step2/finalize,
coordinator_step1/finalize, and the recover and investigate calls.
The three builds in the chain sit on `build/substitute-jni-android` branches
(lightning-kmp-app on `build/agp-9.4.0`, which now carries two commits) and none
are pushed, so a fresh clone cannot resolve these pointers yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
b5158e4e1e |
fix: drop the invalid fr.acinq.bitcoin.crypto package import
`fr.acinq.bitcoin.crypto` is a package, not a declaration -- it is where bitcoin-kmp keeps
Digest, Pack and hmac. Kotlin has no import-a-package form, so this line was never valid:
e: ChillDkgRitualManager.kt:21:25 Packages cannot be imported.
It was also unused. Nothing in the file references anything under that package; the crypto
the file actually uses is fr.acinq.bitcoin.Crypto on the line above, plus quartz's EventHasher.
Worth recording why this only surfaced now, since the line has been there since
|
||
|
Kgothatso Ngako
|
4e939c4570 |
build: bump lightning-kmp-app to bbba08b, building lightning-kmp from source
lightning-kmp-app moves 6745d88 -> bbba08b, four commits: 57353cf Minor updates 9f3cd72 Build lightning-kmp from the experimental submodule via a composite build f22c30a Follow the submodule chain onto Gradle 9.7.1 bbba08b Declare the ios targets only on a mac, so the IDE can import the project 9f3cd72 is the substantive one. lightning-kmp no longer comes from maven central: the submodule now carries its own experimental/lightning-kmp submodule on branch `threshold` -- the branch with the FROST/prefractal signers -- and substitutes fr.acinq.lightning:lightning-kmp-core for that build's project. That build includes bitcoin-kmp, which includes secp256k1-kmp, which compiles the C library from a secp256k1-zkp fork. So this repo's build tree is now four levels deep and compiles native code. Cloning therefore needs `git submodule update --init --recursive`, and because each included build resolves the android SDK from its own local.properties rather than inheriting the root's, the three new nested builds each need a (gitignored) local.properties with sdk.dir. 57353cf changed two signatures that MantraApplication implements -- LightningApplication gained getApplicationContext(), and BusinessManager.initialize now takes the application rather than a Context. Neither needed a source change: MantraApplication extends android.app.Application, which already supplies getApplicationContext(), and it was already passing `this`, which satisfies the narrowed parameter type. The rest of this commit is what the update forces on the outer build. gradle-wrapper.properties, 9.3.1 -> 9.7.1: f22c30a moved every build in the chain onto 9.7.1. An included build does not use its own wrapper -- the root build's gradle version runs the whole tree -- so this repo has to follow for the chain to build at all. gradle.properties, configuration cache off: secp256k1-kmp's `:jni:generateHeaders` and `:native:buildSecp256k1<target>` both hold gradle script object references and cannot be serialized. The configuration cache covers a whole build tree and has no per-build opt-out, so an included build's incompatibility is this build's problem. The comment records how to undo this once those tasks are fixed upstream. composeApp/build.gradle.kts, ios targets gated on the host: secp256k1-kmp declares a libsecp256k1 cinterop, which makes gradle switch off klib cross compilation for apple targets. On linux nothing in the tree then offers an ios variant of fr.acinq.phoenix:lightning-kmp-app, and the ios compilations failed with "No matching variant of project ':lightning-kmp-app:library'" -- not a warning, a build failure. The gate covers the target declarations, the iosMain dependencies (the source set only exists when the targets do) and the kspIos* configurations (likewise). This mirrors bbba08b, which applied the same gate inside the submodule for the same reason. secp256k1-frost-kmp d3b294d applies that gate there too. Substitution rules apply across a whole build tree, so that project's own lightning-kmp-core coordinate started resolving to the source project without it asking, and every apple source set stopped resolving. The android build masked it -- ios compilations are not in its task graph -- but kmpPartiallyResolvedDependenciesChecker reported it and compileAppleMainKotlinMetadata failed outright, which would have broken IDE import. Note that `:composeApp:compileCommonMainKotlinMetadata` no longer exists on a linux host. With ios gated off, androidTarget is the only declared target (jvm() is still commented out), and KMP does not generate a commonMain metadata compilation for a single-target project. `:composeApp:compileDebugKotlinAndroid` is the check now; it passes clean, with none of the resolution errors above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
0d6cefbe21 |
fix: stop the sync pumps opening unbounded relay subscriptions
Relays were answering with "too many concurrent REQs" and the emulator
log was full of it. Two independent defects, compounding.
## The subscription flow never completed
RelayPool.queryAsFlow returned a filtered view of the socket's hot
`incomingMessages`, with the terminating operator commented out:
return this.incomingMessages
.filterBySubscriptionId(id = subscriptionId)
// .transformWhileEventsAreIncoming()
A filter over a hot flow has no terminal event, so every collector
started for a sync request stayed alive for the life of the app --
accumulating one per request ever made, long after EOSE and CLOSE had
been sent. Neither pump's EOSE branch ended collection either;
`return@collect` only ends handling of the message in hand, as the
CLOSED branch's own comment already noted.
That is also why the log *flooded* rather than merely warning.
`filterBySubscriptionId` admits NoticeMessage on every subscription id
(a NOTICE carries none), so a single "too many concurrent REQs" notice
was delivered to every accumulated collector and logged once per
collector. Volume grew as notices x live collectors.
## Nothing bounded how many were open
Both pumps mark the request "sent" and then launch the subscription
detached:
nostrRepository.negentropySynchronizeRequestProcessed(request)
launch(Dispatchers.IO) { relaysSocketManager.negentropySync(...).collect { ... } }
The DAO query is `WHERE status = :status ... LIMIT 1`, so flipping the
row changes the head row, Room re-emits, and the collector body runs for
the next request while the previous subscription is still open. The
mutex covers only the setup block and publishSlots guards publishes, not
REQs, so the number of simultaneously open REQ/NEG-OPEN subscriptions
was bounded only by backlog depth.
## And back-pressure amplified itself
The negentropy ClosedMessage branch -- CLOSED being exactly what a relay
sends when refusing for too many concurrent REQs -- answered by queuing
the request again as a plain REQ. Each refusal therefore produced
another subscription. That branch also skipped the close its EOSE and
NEG-MSG siblings performed, leaking a slot precisely when the slot was
most needed.
## The fix, in the order it has to be applied
1. sockets/NostrIncomingMessageExt.kt gains isTerminalFor() and
completeOnSubscriptionEnd(), which emits the terminal message and
then completes. NOTICE is deliberately not terminal: with no
subscription id it reaches every collector on the socket, so treating
it as terminal would tear down every unrelated subscription at once.
2. RelayPool.queryAsFlow applies it, replacing the commented-out call.
3. SynchronizationViewModel gains subscriptionSlots =
Semaphore(MAX_CONCURRENT_SUBSCRIPTIONS = 4), acquired inside each
pump's launch before the socket call, so the backlog still drains but
queues on the semaphore rather than opening all at once.
4. Both pumps close in a `finally` under NonCancellable, replacing the
hand-rolled closes in the EOSE and NEG-MSG branches, so CLOSED and
NEG-ERR exits close too.
5. The negentropy CLOSED branch consults isBackPressure() -- NIP-01's
`rate-limited:` prefix plus the free-text forms relays actually send
-- and declines to retry, instead of answering back-pressure by
opening another subscription.
Order is load-bearing: capping slots before the flow could complete
would have deadlocked the pump on permits that never came back.
## A negentropy assumption that would have deadlocked it anyway
Capping slots nearly stalled the queue on a wrong assumption about what
ends a negentropy exchange. EOSE does not: the NegentropyMessage branch
reconciles ONCE, queues the ids it needs as a plain REQ, schedules what
the relay is missing, and stops -- this client does single-round
reconciliation. Waiting on an EOSE the exchange need not send would have
held all four slots forever. NegentropyMessage is therefore terminal
too, with the reasoning recorded at isTerminalFor().
Worth knowing separately, and left alone here: single-round
reconciliation may not converge on large sets, since negentropy is
normally iterative. A large divergence is closed by the plain-REQ
fallback rather than by negentropy itself.
Live collectors go from one per sync request ever made to at most four.
MAX_CONCURRENT_SUBSCRIPTIONS is the dial if sync feels slow -- relays
commonly allow around 20 per connection, so there is headroom.
Verified:
./gradlew :composeApp:compileCommonMainKotlinMetadata
./gradlew :composeApp:compileDebugKotlinAndroid
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
e1b79ad842 | Selection container on text |