0757e50dc5e8409b711fdac34d4aaade1e25ed96
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kgothatso Ngako
|
0757e50dc5 |
docs: correct the jvm plan against what phase 1 actually did
Phase 1 is implemented and verified in the lightning-kmp-app fork on claude/jvm-target-actuals (27a0054). Four things in the plan were wrong, and doing the work is what surfaced them. **jvm() belongs at the start of phase 1, not phase 4 -- for the library.** The plan said leave it off in both builds until phase 4. That is right for mantra and wrong for the fork: library/src/jvmMain/ is an orphan source set until the library declares the target, so phases 1-3 would all have been written blind. Declared first, `:library:compileKotlinJvm` names the remaining expects, and that list beats grepping for `expect ` -- it shrinks by exactly what you implement and cannot drift from the truth. The build stays red across phases 1-3 by design. That checklist is now recorded as the phase 1 exit condition: exactly eight expects should remain, and exactly which eight. Anything else means something in the phase is wrong. **Phase 3 is two decisions, not four.** gracefulSingleSeedDecryption and gracefulMultiSeedDecryption are pure exception mapping into a DecryptSeedResult, and the exception they branch on is java.security.KeyStoreException -- a plain JCA type that exists on the jvm. Both are near-copies of the android actuals and need nothing settled first, so they move alongside phase 2. Only keyStoreEncryption and keyStoreDecryption are the security decision, and that part of the analysis stands. **The Fibonacci template must not be deleted.** The plan said to drop it "assuming nothing references them". Things do: generateFibi is exercised by template tests in commonTest, androidHostTest, iosTest, jvmTest and linuxX64Test, and JvmFibiTest asserts a value that depends on precisely the two properties fibiprops.jvm.kt defines. That file already satisfies two of the 25 expects, which is why the count was 23 missing rather than 25. Removing the template is five test files plus four fibiprops.* actuals, and it is a separate cleanup. **Phase 1 is fifteen actuals, not fourteen**, and two of them are not copies of android -- platformElectrumRegtestConf (10.0.2.2 is the emulator's alias for the host loopback; a jvm process is already on the host) and phoenixLogWriters (android routes kermit into slf4j because android tooling reads that back). Also recorded, because it cost time: a worktree cannot run gradle at all until the submodules are checked out *and* local.properties exists at five levels. Neither is version controlled, so a fresh worktree has neither, and the failure surfaces four builds down at :...:secp256k1-kmp:jni:android as "SDK location not found" rather than anywhere obviously related. Both builds were run: `:library:compileKotlinJvm` fails only on the known eight, and `:composeApp:compileDebugKotlinAndroid` still passes with the library's jvm target declared -- the check that matters, since a new variant must not change how the android target resolves the library. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
2aaa7b99a6 |
build: phase 0 of the jvm target -- clear the ground, correct the plan
First phase of docs/jvm-target.md. Nothing here turns the target on; it
removes what would break the moment it is turned on, and stages the two
catalog entries that cannot be derived automatically. Two of the four
steps as written in the doc turned out to be wrong, and implementing them
is how that surfaced -- both are corrected in the doc in this commit.
**Deleted the stale jvmMain tree.** Six files under
composeApp/src/jvmMain/kotlin/ac/cord/auxiliary/ survived from the Aux
project this codebase grew out of. They have gone unnoticed because
`jvmMain` is currently an orphan source set -- the accessor creates it,
no target compiles it -- so the wrong package, the Room 2 imports
(androidx.room, not androidx.room3), and the references to a long-gone
AuxDatabase and AuxGlobal have never had to resolve. They would all become
compile errors in phase 4.
They are not lost: they are the closest thing to a skeleton for five of
the six platform actuals phase 4 needs, and main.kt is a reasonable
starting shape for the phase 5 desktop entry point. `git show HEAD~1` has
them.
**Added two catalog entries, not four.** sqlite-bundled-jvm and
sqldelight-sqlite-driver. Both earn their place by being unreachable
otherwise: sqlite-bundled-jvm has to be named explicitly because
variant-aware resolution hands the *android* artifact to anything running
on the host, and sqldelight-sqlite-driver is the jvm counterpart to the
android-driver and native-driver entries already there.
The doc also listed room3-runtime-jvm and sqldelight-jdbc-driver. Neither
is right. Once jvm() exists, commonMain's existing androidx-room3-runtime
resolves to the -jvm variant on its own, so an explicit entry is
redundant and would drift. And the SQLDelight drivers phase 2 needs are
for DbFactory, which lives in lightning-kmp-app -- a separate gradle build
with its own version catalog, where an entry here is simply not visible.
**kspJvm cannot be wired yet, and the build file already said so.** The
doc's phase 0 told you to uncomment
composeApp/build.gradle.kts:194. It contradicted its own phase 4, which is
where jvm() gets turned on. The comment three lines above it states the
rule:
These configurations only exist when the ios targets are declared,
which the kotlin block above does only on a mac.
The same holds for kspJvm -- `dependencies { add("kspJvm", ...) }` throws
UnknownConfigurationException until a jvm() target creates the
configuration. So it moves into phase 4, into the same edit that declares
the target. composeApp/build.gradle.kts is deliberately untouched by this
commit.
**Also documented: gradle does not run in a worktree here at all** until
the submodule is checked out, which worktrees do not do automatically.
`lightning-kmp-app/` is empty and configuration fails with "Project with
path ':library' not found in build ':lightning-kmp-app'". Recorded in the
phase 0 verification section along with the caveat that a linked worktree
shares .git/modules/ with the main checkout, so both trees end up on one
submodule git dir.
**Not verified by a build.** For that reason. The deletion is an orphan
source set and the additions are unreferenced catalog lines, so neither
can change a build's outcome -- but that is an argument, not a green
check, and it is the second commit in a row on this branch that has not
compiled anything. Phase 4 is the first phase that genuinely cannot be
done without a working gradle invocation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
5abc37e463 |
docs: scope the jvm target, and separate it from testing the daos
Two questions arrived together -- whether Room's own testing guidance applies to this project, and what desktop support would cost -- and they turned out to have opposite answers. Both are now in docs/jvm-target.md, phased, with the blocking work separated from the mechanical work. **The expensive part is already done.** The four-deep native chain -- secp256k1 -> bitcoin-kmp -> lightning-kmp -> lightning-kmp-app -- already builds for JVM, on every android build we do. The comment at composeApp/build.gradle.kts:50 records the mechanism without drawing the conclusion: lightning-kmp-core publishes no android variant, so our android target resolves it to the *jvm* one, which pulls secp256k1-kmp-jni-jvm desktop natives, which is exactly why the build has to name the android artifact by hand. Read the other way round, every JVM artifact in the chain is already compiled from source by the composite build. A jvm target adds no cinterop, no C compilation and no new native constraints. That was the part worth being afraid of, and it is finished. **The blocker is one level down, and smaller than it looks.** lightning-kmp-app/library declares 25 expects and implements them across 35 androidMain files. Its jvmMain holds exactly one: fibiprops.jvm.kt, the Kotlin multiplatform library template's Fibonacci boilerplate, satisfying two of the 25 -- both of them the template's own. So 23 actuals are missing, which is why jvm() is commented out there (library/build.gradle.kts:18), which is why it is commented out here (composeApp/build.gradle.kts:46). Mantra cannot declare the target until the fork does. Six phases, ordered by that dependency. 0 build config; 1 the fourteen mechanical phoenix actuals; 2 the three SQLDelight JDBC drivers and NetworkMonitor; 3 key storage; 4 mantra's own sixteen expects; 5 the desktop entry point. 1-3 are independent and parallelisable, 4 is where the compiler finally checks the whole thing. Roughly a week to a launchable build. **Phase 3 has no day estimate, deliberately.** keyStoreEncryption / keyStoreDecryption and their two graceful* wrappers delegate on android to KeystoreHelper.kt -- 116 lines against AndroidKeyStore, StrongBox attempted first and fallen back from, key material never leaving hardware. Desktop JVM has no equivalent, so this is a decision rather than a port, and the doc gives the three real options against what each actually protects. A fixed-key JCEKS file is named there as a liability rather than a stopgap: this is wallet seed material, and it lands on top of the plaintext-key finding already open against this codebase. Recommended sequencing is a passphrase-derived KEK with the desktop build marked unsuitable for real funds, so phases 4 and 5 can proceed without the security question being quietly treated as answered. Two inherited mistakes are called out rather than carried forward. The old Aux jvmMain put the database in java.io.tmpdir behind a TODO -- the doc says not to inherit that in either phase that touches it. And schedulePlatformLogic goes through WorkManager on android with no desktop counterpart, so the doc asks for an explicit choice between a no-op and an in-process coroutine, written down. **The DAO answer is an appendix, because it is the opposite answer.** None of the above is needed to test the DAOs, and burying that would have been misleading. room3-runtime-android:3.0.1 already exposes the no-Context inMemoryDatabaseBuilder(Function0<T>) overload, and MantraDatabaseConstructor already supplies what it needs, so Room's recommended host-machine form compiles in commonTest and runs under testDebugUnitTest today. The one trap is native and is the secp256k1 problem mirrored: sqlite-bundled-android ships only android-ABI .so under jni/, so a local unit test's JVM cannot load it and BundledSQLiteDriver fails at construction; sqlite-bundled-jvm on the androidUnitTest classpath is the fix. Robolectric neither helps nor is needed -- it cannot load android .so on the host either. Everything structural here was checked against the artifacts rather than recalled: the Room builder overloads by javap on room3-runtime-android, the two sqlite-bundled native layouts by unzipping both, and the availability of room3-runtime-jvm, room3-testing, quartz-jvm and the two SQLDelight drivers by request against the repositories this build actually resolves from. The absence of android.* and java.* imports in commonMain, and of any NFC reference from it, was likewise grepped rather than assumed. **Not verified: anything that requires compiling.** No jvm target was turned on, nothing was built, and the day estimates are estimates. Phase 4 is where dependency-substitution surprises would surface if there are any, and it is precisely the phase nothing here exercises. 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
|
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
|
b99cb8fcd5 |
docs: write down the shared-key subsystem and how Marmot membership fails
First docs in the repo -- README.md is still the stock KMP template. Three documents plus an index, covering the parts whose behaviour is not recoverable by reading the code: where the reasoning lives in a protocol, where a failure mode is silent, or where a decision looked arbitrary and was not. marmot-membership.md is the one that earns its place. Everything about adding a member compiles, the invite reports success, and a member simply never appears -- and the reason is never in the invite code. It records that inviteMemberToChatRoom hardcodes isOneMemberInitialGroupCreation = false and that ChatRepository does not expose it, so every group invite takes the deferred-welcome path including the first, when the group is still just its creator and the commit has no audience at all. Then why that is silent rather than noisy: MarmotInboundManager refuses future-epoch messages outright, on both wire formats, with no queue and no replay, so a commit arriving before its recipient's welcome is dropped and that member never advances. EPOCH_RETENTION_WINDOW retains past epochs and does nothing for messages from ahead. Three options are set out with the per-invite correctness table, including the honest limit that the recommended one narrows the race without closing it. shared-key-derivation.md argues why the paths are not BIP32 -- no chain code exists, hardened derivation is impossible rather than unimplemented, and a FROST tweak takes the scalar as input so the chain code leaves the problem entirely. It records the x-only serialisation trap avoided by choosing the scalar directly, and states the rule that must not be broken: never reconstruct a derived key in the clear, because k = k' - t hands over the group key rather than one derived key. shared-key-ceremony.md covers the seven kinds, the three approval gates and why the coordinator's aggregations are deliberately not among them, faults as values rather than exceptions, and the transcript's idempotency-by-construction. It also writes down the invariant that produces no error when broken: pendingApproval must mirror the gates in advance, or the screen offers an approval that does nothing -- or none while the ritual sits still. Every factual claim was checked against the source rather than recalled, which turned up one correction worth having: there are two future-epoch refusals, for PrivateMessage and for Commit, so the drop covers both wire formats and not just one. Each document leads with the failure mode rather than the architecture, on the grounds that a failure is what sends somebody to docs in the first place, and each lists its known gaps -- including that none of this has run on a physical device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |