From d957ee8de59d2c6f969fd475eec52bab9c1c0dbd Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 13 Sep 2026 00:20:22 +0200 Subject: [PATCH] feat(identity): a seed's key is a credential, and the seed is the wallet attached to it Phase 1 of docs/multiple-profiles.md. No library change: the file format, the encrypted writer and the manager all exist, and the app already wrote the credentials file from the seed writer -- to delete a public entry a seed superseded. This changes what it writes there, and what the listing believes. Until now a seed's nostr key was never in nostr-credentials.dat. It was derived from the words at listing, to know which npub to show, and from the running node at activation, to know which key to sign with -- so a seed-backed profile existed only as a derivation, and the app had to start a Lightning node to find out who it was. Both sign-in plans made "one key, one file" an invariant, and it was the wrong one: it said a wallet is a profile. Now the credentials file is the list of profiles and a seed is a wallet attached to the entry its key derives. writeMnemonic writes two things, credential first: a Secret for the derived key under its x-only pubkey, replacing a public entry where there is one, and then the seed. Credential first because a crash between the two leaves a bare-key profile the phrase completes, which is a valid thing to hold and says the model out loud -- the profile exists, then a wallet is attached to it. The one refusal it drops is the phrase of a key held as a bare secret, SeedAlreadyExists under the nsec plan: the device did not have that wallet, so this is the profile acquiring the wallet that derives it. The entry stays a secret for the same key, the id becomes the wallet's, and the bare key's preference files go with the old id -- the profile's preferences are the wallet's now, fresh, which is right since there is a new secret to back up. The same seed twice is still refused, by wallet id, and is the only way a profile with a wallet attached is offered its phrase again. SeedCredentials.reconcile is the repair for every seed already on a device, beside migrateFromNostrKeys in listIdentities and shaped like it: a named, idempotent write, one file write for however many seeds are missing, nothing at all on a device with none or one already repaired. A failed write is a result, not a throw, and carries the map that was read: the listing goes on with it and merge derives the key of a seed that has no credential, so a seed this could not repair is still listed. A failed write must never hide a wallet. The plan had the repair running before the credentials file was read; it runs after, and hands the listing what it returns, so the file is decrypted once -- the doc now says so. StoredIdentity.merge inverts: the credentials are the list, and each seed is attached to the Secret its key derives -- listed once, as Mnemonic under the wallet's id, carrying the credential's key -- where before the seeds were the list and a secret for a seed's key was listed twice under two ids. Mnemonic gains privateKey. A Public for a seed's key is skipped with a log line, since the next repair upgrades it; a seed with no credential is listed by derivation, with a log line. IdentityKind.Mnemonic's doc changes to what the kind now means: the name records the attachment, not the source. setActiveWallet takes the StoredIdentity.Mnemonic and builds the identity from the credential's key, with the node's derivation as a cross-check -- a check(), because a node disagreeing with the credentials file is the one corruption worth refusing to run under, and it cannot fail for a file the repair wrote. The node still starts for a profile with a wallet attached: not for the key any more, but for what startNewBusiness does besides -- metadata, preferences, last-used build, and on Android the channel watcher a restored Phoenix phrase may need. Making it lazy is now one branch and is named as its own decision. forgetNostrCredential refuses a second thing: a key a seed derives. The seed would derive it again and the next repair would write it back, so a forget that succeeded would undo itself. NotACredential becomes WalletAttached in the writer's result and ForgetIdentity's outcome, since that is now the only reason a signing profile cannot be forgotten -- a key not in the file at all can, since the repair, only be a seed's key the repair could not write. The two sign-in docs' tables each gain a row pointing here for the invariant this supersedes. Tests: SeedCredentialsJvmTest against a device from before -- seed.dat written directly, no credentials -- writes exactly what is missing in one write; a device already repaired is not written to again, checked by the file's bytes since a rewrite would carry a fresh iv; a public entry for a seed's key is upgraded; bare keys are untouched; and a write that fails, with the key store locked, is reported with the map that was read and the wallet is still listed. IdentityWriterJvmTest drives the callback-shaped seed writer through a CompletableDeferred on a real Main dispatcher, since it reports after a real one-second delay: a phrase writes both files, its nsec and npub are then duplicates and its forget is WalletAttached; the same phrase twice is refused; the phrase of a bare key attaches, under the wallet's id, with the bare key's preferences gone; the phrase of a key held read-only attaches and the entry becomes a secret. StoredIdentityJvmTest lists a secret-plus-seed once with the credential's key, a seed without a credential by derivation, and a public entry for a seed's key as the wallet only. Not under test: the activation's cross-check, because a PhoenixBusiness cannot be built without a node. Co-Authored-By: Claude Opus 5 Pulled-From: curated/curated@3008137e3f5241f1ebd69db1a13f5522e864a55d --- .../mantra/compose/identity/ForgetIdentity.kt | 12 +- .../press/mantra/compose/identity/Identity.kt | 11 +- .../mantra/compose/identity/IdentityWriter.kt | 89 +++++--- .../compose/identity/SeedCredentials.kt | 77 +++++++ .../mantra/compose/identity/StoredIdentity.kt | 85 +++++--- .../SovereignWalletStartupScreen.kt | 6 +- .../ui/view/model/NostrSecretViewModel.kt | 2 +- .../compose/ui/view/model/SignOutViewModel.kt | 2 +- .../ui/view/model/SovereignWalletViewModel.kt | 51 ++++- .../compose/identity/ForgetIdentityJvmTest.kt | 6 +- .../compose/identity/IdentityWriterJvmTest.kt | 159 ++++++++++++++- .../identity/SeedCredentialsJvmTest.kt | 192 ++++++++++++++++++ .../compose/identity/StoredIdentityJvmTest.kt | 51 +++-- docs/multiple-profiles.md | 15 +- docs/npub-sign-in.md | 1 + docs/nsec-sign-in.md | 1 + 16 files changed, 649 insertions(+), 111 deletions(-) create mode 100644 composeApp/src/commonMain/kotlin/press/mantra/compose/identity/SeedCredentials.kt create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/SeedCredentialsJvmTest.kt diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/ForgetIdentity.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/ForgetIdentity.kt index d37c2c03..887b2572 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/ForgetIdentity.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/ForgetIdentity.kt @@ -11,9 +11,9 @@ import press.mantra.compose.repository.NostrRepository * Lifted out of `NostrSecretViewModel.forgetKey`, where it was written for an nsec, so * that the two exits a read-only identity has -- sign out, and *use a different key* on * the not-found screen -- run the same sequence rather than a copy of it. The first step - * is the same call for both credential kinds since the credentials file; only a mnemonic - * identity is refused, because removing a seed is a wallet question this does not - * answer. + * is the same call for both credential kinds since the credentials file; only a profile + * with a wallet attached is refused, because removing a seed is a wallet question this + * does not answer, and because the seed would derive the key again. * * The two writers are injected as functions so the sequence can be pinned in a test * without a key store, as the nsec view model's already is. @@ -24,8 +24,8 @@ object ForgetIdentity { /** The device no longer holds anything for the identity. */ data object Forgotten : Outcome - /** A mnemonic identity: its key is in the seed, and this does not remove seeds. */ - data object NotACredential : Outcome + /** A profile with a wallet attached: the seed derives its key, and this does not remove seeds. */ + data object WalletAttached : Outcome /** The credentials file could not be read; nothing was changed. */ data object CannotForget : Outcome @@ -44,7 +44,7 @@ object ForgetIdentity { nostrRepository.forgetLocalAccount(identity.nostrPublicKey) Outcome.Forgotten } - is IdentityWriter.ForgetNostrCredentialResult.NotACredential -> Outcome.NotACredential + is IdentityWriter.ForgetNostrCredentialResult.WalletAttached -> Outcome.WalletAttached is IdentityWriter.ForgetNostrCredentialResult.CannotLoadKeys -> Outcome.CannotForget } } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/Identity.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/Identity.kt index 7ccd555a..5741fb89 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/Identity.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/Identity.kt @@ -14,16 +14,21 @@ import fr.acinq.phoenix.utils.preferences.InternalPrefs import fr.acinq.phoenix.utils.preferences.UserPrefs /** - * Where an identity's key came from, which decides what else it can have. + * What the device holds beside an identity's key, which decides what else it can have. * * The distinction is not cosmetic: the nostr key is a BIP32 leaf of the seed at * `m/44'/1237'/0'/0/0`, and that derivation runs one way. Twelve words yield the * key and a wallet; a bare key yields nothing further, so an identity made from * one can never grow a node behind it; and a bare public key yields nothing at all, - * not even a signature. See docs/nsec-sign-in.md and docs/npub-sign-in.md. + * not even a signature. See docs/nsec-sign-in.md, docs/npub-sign-in.md and + * docs/multiple-profiles.md. */ enum class IdentityKind { - /** Twelve words. The nostr key is derived from them, and so is a wallet. */ + /** + * A key the device also holds a seed for. The key is a credential like any other + * -- it is read from the credentials file, not derived from the node -- and the seed + * is the wallet attached to it. The name records the attachment, not the source. + */ Mnemonic, /** A bare nostr secret. Nothing else can be derived from it. */ diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/IdentityWriter.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/IdentityWriter.kt index 5fcea706..c784828c 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/IdentityWriter.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/IdentityWriter.kt @@ -43,10 +43,15 @@ import press.mantra.compose.ui.view.model.WritingSeedState * key, the other hash160 of the nostr key -- and the database is keyed by pubkey, so two * identities for one pubkey would share every row and disagree about which is active. * - * With one exception, which is the point of the credentials file: a public key already - * here read-only is not a duplicate of the secret that signs as it. The device does not + * With two exceptions, which are the point of the credentials file. A public key already + * here read-only is not a duplicate of the secret that signs as it: the device does not * have that secret, and refusing it would be false. [writeNostrKey] replaces the entry - * in one write; [writeMnemonic] has to span two files and says which goes first. + * in one write. And a bare secret already here is not a duplicate of the recovery phrase + * that derives it: the device does not have that wallet, and pasting the phrase is the + * profile acquiring the wallet that derives it. [writeMnemonic] writes the credential + * first and then the seed -- see docs/multiple-profiles.md, Phase 1 -- and every seed's + * key is a `Secret` entry in the credentials file, so that the file is the list of + * profiles and the seed is a wallet attached to one of them. */ object IdentityWriter { @@ -130,34 +135,43 @@ object IdentityWriter { return@launch } existingSeeds.containsKey(newWalletId) -> { + // The same seed, and so the same nostr key with a wallet already attached. + // Two different seeds cannot derive one nostr key, so this is the only + // way a profile with a wallet attached is offered its phrase again. log.i("attempting to import a seed that already exists, aborting...") onWritingSeedError.invoke( WritingSeedState.Error.SeedAlreadyExists ) return@launch } - existingCredentials[newNostrPublicKey] is NostrCredential.Secret -> { - // The same npub is already here as a bare key. The id check above cannot - // see that -- different hash, different key -- so it is asked by pubkey. - log.i("attempting to import a seed whose nostr key is already here, aborting...") - onWritingSeedError.invoke( - WritingSeedState.Error.SeedAlreadyExists - ) - return@launch - } else -> { - if (existingCredentials[newNostrPublicKey] is NostrCredential.Public) { - // The npub is here read-only, and the words that sign as it have just - // been pasted: an upgrade, across two files. The credential goes - // first, then the seed. A crash between the two loses the read-only - // identity, which the npub pasted again restores; the reverse order - // would leave one npub listed twice under two ids. - log.i("the seed's nostr key is here read-only; replacing it with the wallet") - NostrCredentialManager.writeToDisk( - phoenixGlobal, - EncryptedNostrCredentials.encrypt(existingCredentials - newNostrPublicKey), - ) + // The credential first, then the seed. What the credentials file holds + // for this key beforehand decides what the write means, not whether it + // happens: nothing, and this is a new profile with a wallet; a public + // entry, and it is the read-only upgrade landing on a wallet; a secret, + // and it is a bare key acquiring the wallet that derives it, which used + // to be refused as a duplicate and is not one -- the device did not have + // the wallet. In every case the entry becomes the derived key. + // + // A crash between the two writes leaves a bare-key profile whose key + // the phrase derives: a valid thing to hold, and pasting the phrase + // again completes it. The reverse order would leave a seed with no + // credential, which the listing derives around and the repair fixes -- + // also recoverable, but this order says the model out loud: the profile + // exists, and then a wallet is attached to it. + val existingEntry = existingCredentials[newNostrPublicKey] + val bareKeyId = if (existingEntry != null) StoredIdentity.nostrPublic(newNostrPublicKey).id else null + when (existingEntry) { + null -> Unit + is NostrCredential.Public -> log.i("the seed's nostr key is here read-only; it becomes a profile with a wallet") + is NostrCredential.Secret -> log.i("the seed's nostr key is here as a bare key; attaching the wallet") } + NostrCredentialManager.writeToDisk( + phoenixGlobal, + EncryptedNostrCredentials.encrypt( + existingCredentials + (newNostrPublicKey to NostrCredential.Secret(keyManager.nostrPrivateKey())) + ), + ) val newSeedMap = existingSeeds + (newWalletId to mnemonics) val encrypted = EncryptedSeed.V2.encrypt(newSeedMap) SeedManager.writeSeedToDisk(phoenixGlobal, encrypted, overwrite = true) @@ -166,6 +180,13 @@ object IdentityWriter { } else { log.i("successfully created wallet=$newWalletId") } + if (bareKeyId != null) { + // The profile's id is its wallet's now, so its preferences are the + // wallet's -- fresh, as a new wallet's are, which is right: there is + // a new secret to back up. The files under the bare key's id would + // otherwise sit on the disk with nothing reading them. + DataStoreManager(phoenixGlobal.ctx, chain = NodeParamsManager.chain).deleteNodeUserPrefs(bareKeyId) + } } } @@ -281,8 +302,14 @@ object IdentityWriter { sealed class ForgetNostrCredentialResult { data object Forgotten : ForgetNostrCredentialResult() - /** The key is not in `nostr-credentials.dat`: a mnemonic identity's key lives in the seed, and removing a seed is a wallet question. */ - data object NotACredential : ForgetNostrCredentialResult() + /** + * A seed derives the key: the profile has a wallet attached, and removing a seed is + * a wallet question this does not answer. Also the answer for a key that is not in + * `nostr-credentials.dat` at all, which since the repair can only be a seed's key + * the repair could not write -- and either way, taking the entry would not take the + * key: the seed would derive it again and the next listing would put it back. + */ + data object WalletAttached : ForgetNostrCredentialResult() data object CannotLoadKeys : ForgetNostrCredentialResult() } @@ -291,7 +318,8 @@ object IdentityWriter { * The inverse of [writeNostrKey] and [writeNostrPublicKey] alike: removes the entry, * whichever kind it is, from `nostr-credentials.dat` and deletes the identity's * preference files. The profile stays on the relays, and the same key can be signed - * in again. Only for a credential -- see [ForgetNostrCredentialResult.NotACredential]. + * in again. Only for a profile with no wallet attached -- see + * [ForgetNostrCredentialResult.WalletAttached]. */ suspend fun forgetNostrCredential( log: Logger, @@ -299,14 +327,19 @@ object IdentityWriter { id: WalletId, nostrPublicKey: HexKey, ): ForgetNostrCredentialResult { + val seeds = seedPublicKeys(phoenixGlobal) val existing = NostrCredentialManager.loadAndDecryptOrNull(phoenixGlobal) - if (existing == null) { + if (seeds == null || existing == null) { log.e("could not load the existing keys, aborting...") return ForgetNostrCredentialResult.CannotLoadKeys } + if (nostrPublicKey in seeds) { + log.i("asked to forget a key a seed derives; the wallet has to go first") + return ForgetNostrCredentialResult.WalletAttached + } if (!existing.containsKey(nostrPublicKey)) { log.i("asked to forget a key that is not a credential on this device") - return ForgetNostrCredentialResult.NotACredential + return ForgetNostrCredentialResult.WalletAttached } NostrCredentialManager.writeToDisk(phoenixGlobal, EncryptedNostrCredentials.encrypt(existing - nostrPublicKey)) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/SeedCredentials.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/SeedCredentials.kt new file mode 100644 index 00000000..11f1af26 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/SeedCredentials.kt @@ -0,0 +1,77 @@ +package press.mantra.compose.identity + +import co.touchlab.kermit.Logger +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import fr.acinq.phoenix.PhoenixGlobal +import fr.acinq.phoenix.data.UserWallet +import fr.acinq.phoenix.data.WalletId +import fr.acinq.phoenix.managers.NostrCredentialManager +import fr.acinq.phoenix.managers.nostrPublicKeyHex +import fr.acinq.phoenix.security.EncryptedNostrCredentials +import fr.acinq.phoenix.security.NostrCredential + +/** + * The credential a seed always implied. + * + * Until docs/multiple-profiles.md, a seed's nostr key was never written to the + * credentials file: it was derived from the words at listing and from the running node + * at activation, and the writers kept one key out of two files. That put the wallet + * where the profile should be. Now the credentials file is the list of profiles and a + * seed is a wallet attached to one, so every seed's key has to be in it -- written by + * `IdentityWriter.writeMnemonic` for a seed created or restored from here on, and by + * this for every seed that was already on the device. + * + * A named, idempotent write that runs from `SovereignWalletViewModel.listIdentities`, + * beside `NostrCredentialManager.migrateFromNostrKeys` and for the same reason: a + * listing that writes is a surprise, so the write is a step with a name, a result and + * a test, rather than a side effect of reading. On a device with no seeds, or one + * already repaired, it writes nothing. + */ +object SeedCredentials { + + sealed interface Result { + /** The credentials the listing should use: repaired where the write succeeded, as read where it did not. */ + val credentials: Map + + /** [count] seeds had no secret credential, and now do. One write. */ + data class Written(val count: Int, override val credentials: Map) : Result + + /** Every seed's key was already a secret credential. Nothing was written. */ + data class NotNeeded(override val credentials: Map) : Result + + /** + * The write failed; the file is as it was and [credentials] is what was read. The + * listing goes on: `StoredIdentity.merge` derives the key of a seed that has no + * credential, so a seed this could not repair is still listed. A failed write must + * never hide a wallet, which is why this is a result and not a throw. + */ + data class Failed(val cause: Throwable, override val credentials: Map) : Result + } + + /** + * Writes a `Secret` for every seed in [wallets] whose derived key is not one in + * [credentials], replacing a `Public` entry for that key where there is one -- the + * read-only upgrade landing on the wallet that was here all along. + */ + fun reconcile( + log: Logger, + phoenixGlobal: PhoenixGlobal, + wallets: Map, + credentials: Map, + ): Result { + val missing = wallets.values + .map { StoredIdentity.nostrPrivateKeyOf(it.words) } + .filter { credentials[it.nostrPublicKeyHex()] !is NostrCredential.Secret } + if (missing.isEmpty()) return Result.NotNeeded(credentials) + + val repaired = credentials + missing.associate { it.nostrPublicKeyHex() to NostrCredential.Secret(it) } + return try { + NostrCredentialManager.writeToDisk(phoenixGlobal, EncryptedNostrCredentials.encrypt(repaired)) + log.i { "wrote the credential for ${missing.size} seed(s) that had none" } + Result.Written(missing.size, repaired) + } catch (e: Exception) { + log.e("could not write the credentials of ${missing.size} seed(s); listing them by derivation", e) + Result.Failed(e, credentials) + } + } +} diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/StoredIdentity.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/StoredIdentity.kt index ec309a7d..9390f081 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/StoredIdentity.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/identity/StoredIdentity.kt @@ -16,31 +16,43 @@ import fr.acinq.phoenix.managers.nostrPublicKeyHex import fr.acinq.phoenix.security.NostrCredential /** - * An identity this device holds something for, as listed before any of them is started. + * A profile this device holds something for, as listed before any of them is started. * - * Two stores feed this -- `seed.dat` for wallets, `nostr-credentials.dat` for bare keys - * and bare public keys -- and the startup screen wants one list. The nostr public key - * is on every kind because it is the one thing they have in common and the one thing a - * duplicate check has to compare: a wallet and an imported key can be the *same* npub - * under different ids. + * The credentials file is the list of profiles: a `Secret` entry for every key the + * device can sign as, a `Public` one for every key it can only look at. `seed.dat` is + * the list of wallets, and a wallet is *attached* to the profile whose key it derives + * -- matched by public key, since a seed's key is written to the credentials file when + * the seed is (`IdentityWriter.writeMnemonic`) and repaired into it for every seed + * already here (`SeedCredentials.reconcile`). See docs/multiple-profiles.md, Phase 1. + * The nostr public key is on every kind because it is the one thing they have in + * common and the one thing that attaches a wallet to a profile or refuses a duplicate. * * On what this holds: `SovereignWalletViewModel.availableWallets` has always carried * decrypted words for the view model's life, because starting the node needs them. - * [NostrSecret] carries its key for the same reason and is no worse; decrypting at - * activation instead would be an improvement for both kinds and belongs to both. + * [NostrSecret] and [Mnemonic] carry their key for the same reason and are no worse; + * decrypting at activation instead would be an improvement for every kind and belongs + * to all of them. */ sealed interface StoredIdentity { val id: WalletId val kind: IdentityKind val nostrPublicKey: HexKey + /** + * A key the device also holds a seed for. The key is the credential's, so that + * activating this profile does not depend on the node to learn who it is; the seed + * is the wallet attached to it, and its id is the wallet's -- `hash160(nodeId)`, as + * it has always been -- because the library keys the node's preferences and + * metadata by that id and nothing has to move. + */ data class Mnemonic( val userWallet: UserWallet, - override val nostrPublicKey: HexKey, + val privateKey: PrivateKey, ) : StoredIdentity { override val id: WalletId get() = userWallet.walletId override val kind: IdentityKind get() = IdentityKind.Mnemonic - override fun toString(): String = "StoredIdentity.Mnemonic(id=$id, npub=$nostrPublicKey)" + override val nostrPublicKey: HexKey = privateKey.nostrPublicKeyHex() + override fun toString(): String = "StoredIdentity.Mnemonic(id=$id, npub=$nostrPublicKey, key=)" } data class NostrSecret( @@ -65,18 +77,23 @@ sealed interface StoredIdentity { companion object { /** - * The nostr public key a wallet's words derive. `SeedManager.loadAndDecrypt` has - * already built exactly this key manager to learn the node id, so this is the second - * derivation of it, not a new cost. + * The nostr key a wallet's words derive, at the NIP-06 path. `SeedManager.loadAndDecrypt` + * has already built exactly this key manager to learn the node id, so this is the + * second derivation of it, not a new cost. It is what attaches a seed to its + * credential, and what the credential is repaired from when a seed has none. */ - fun nostrPublicKeyOf(words: List): HexKey = LocalKeyManager( + fun nostrPrivateKeyOf(words: List): PrivateKey = LocalKeyManager( seed = MnemonicCode.toSeed(words, "").byteVector(), chain = NodeParamsManager.chain, remoteSwapInExtendedPublicKey = NodeParamsManager.remoteSwapInXpub, - ).nostrPrivateKey().nostrPublicKeyHex() + ).nostrPrivateKey() + /** The x-only public key of [nostrPrivateKeyOf]. */ + fun nostrPublicKeyOf(words: List): HexKey = nostrPrivateKeyOf(words).nostrPublicKeyHex() + + /** A wallet whose credential is missing: the key is derived from the words, as it always was. */ fun mnemonic(userWallet: UserWallet): Mnemonic = - Mnemonic(userWallet, nostrPublicKeyOf(userWallet.words)) + Mnemonic(userWallet, nostrPrivateKeyOf(userWallet.words)) fun nostrSecret(privateKey: PrivateKey): NostrSecret { val xOnly = privateKey.publicKey().xOnly() @@ -93,36 +110,38 @@ sealed interface StoredIdentity { ) /** - * One map from the two stores. [credentials] is keyed by x-only public key hex, as + * One map from the two stores: the credentials are the list, and the seeds are + * attached to it. [credentials] is keyed by x-only public key hex, as * `nostr-credentials.dat` is; a secret's id is derived from its own key, so a * stored secret whose map key disagrees with its public key is dropped here too, * though `EncryptedNostrCredentials` refuses such a file before it gets this far. * - * One entry per public key is the credentials file's own invariant, so there is no - * precedence between credentials to decide. There is one between a seed and a - * credential, for the one upgrade that has to span both files -- a recovery phrase - * pasted over a public credential, which `IdentityWriter.writeMnemonic` performs as - * two writes: a `Public` whose public key a seed derives is dropped, with a log line, - * and the next write repairs the file. A *secret* for a seed's key is kept, as it - * always was: two ids for one npub is a duplicate the writers refuse, not a state - * the listing hides. + * A `Secret` whose key a seed derives is that seed's profile, listed once, as a + * [Mnemonic] under the wallet's id and carrying the credential's key. A `Public` + * for such a key is skipped with a log line: `IdentityWriter.writeMnemonic` replaces + * it with a secret before it writes the seed, and `SeedCredentials.reconcile` does + * the same for one left behind, so it is a state the next listing no longer sees. + * + * A seed with no credential at all -- the repair failed to write, or a crash fell + * between the seed writer's two files -- is listed as it always was, with its key + * derived from the words. A failed write must never hide a wallet. */ fun merge( wallets: Map, credentials: Map, ): Map { + val walletsByPublicKey = wallets.values.associateBy { nostrPublicKeyOf(it.words) } val merged = LinkedHashMap() - wallets.values.forEach { userWallet -> merged[userWallet.walletId] = mnemonic(userWallet) } - val seedPublicKeys = merged.values.map { it.nostrPublicKey }.toSet() credentials.forEach { (publicKeyHex, credential) -> when (credential) { is NostrCredential.Secret -> { - val stored = nostrSecret(credential.privateKey) + val wallet = walletsByPublicKey[publicKeyHex] + val stored = if (wallet != null) Mnemonic(wallet, credential.privateKey) else nostrSecret(credential.privateKey) if (stored.nostrPublicKey == publicKeyHex) merged[stored.id] = stored } is NostrCredential.Public -> { - if (publicKeyHex in seedPublicKeys) { - log.w { "public credential for a key a seed already derives; listing the wallet only" } + if (publicKeyHex in walletsByPublicKey) { + log.w { "public credential for a key a seed derives; the next repair upgrades it" } } else { val stored = runCatching { nostrPublic(publicKeyHex) }.getOrNull() if (stored != null) merged[stored.id] = stored @@ -130,6 +149,12 @@ sealed interface StoredIdentity { } } } + walletsByPublicKey.values.forEach { wallet -> + if (wallet.walletId !in merged) { + log.w { "wallet=${wallet.walletId} has no credential for its nostr key; deriving it" } + merged[wallet.walletId] = mnemonic(wallet) + } + } return merged } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt index 32297e2b..39c7db66 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt @@ -177,8 +177,12 @@ fun SovereignWalletStartupScreen( doLoadWallet = { identity -> when (identity) { is StoredIdentity.Mnemonic -> { + // The node starts for the wallet attached to this profile -- + // its preferences, its metadata, the channel watcher -- and no + // longer for the key, which the identity reads from the + // credential the node is handed to cross-check. sovereignWalletStartupViewModel.startupNode(walletId = identity.id, words = identity.userWallet.words, onStartupSuccess = { - sovereignWalletViewModel.setActiveWallet(walletId = identity.id, business = it) + sovereignWalletViewModel.setActiveWallet(stored = identity, business = it) onSuccessfulStartup.invoke() }) } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/NostrSecretViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/NostrSecretViewModel.kt index 88bce8c0..f56a0962 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/NostrSecretViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/NostrSecretViewModel.kt @@ -186,7 +186,7 @@ class NostrSecretViewModel( _forgetting.value = NostrSecretUIState.Forgetting.Idle withContext(Dispatchers.Main) { onForgotten() } } - is ForgetIdentity.Outcome.NotACredential, + is ForgetIdentity.Outcome.WalletAttached, is ForgetIdentity.Outcome.CannotForget -> { _forgetting.value = NostrSecretUIState.Forgetting.Failed } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SignOutViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SignOutViewModel.kt index 29773e92..f9657b88 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SignOutViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SignOutViewModel.kt @@ -76,7 +76,7 @@ class SignOutViewModel( _state.value = State.Idle withContext(Dispatchers.Main) { onSignedOut() } } - is ForgetIdentity.Outcome.NotACredential, + is ForgetIdentity.Outcome.WalletAttached, is ForgetIdentity.Outcome.CannotForget -> { _state.value = State.Failed } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt index d000c5c2..1ab201d7 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt @@ -42,6 +42,7 @@ import kotlinx.coroutines.launch import press.mantra.compose.identity.Identity import press.mantra.compose.identity.IdentityKind import press.mantra.compose.identity.IdentityWriter +import press.mantra.compose.identity.SeedCredentials import press.mantra.compose.identity.StoredIdentity sealed class WritingSeedState { @@ -87,8 +88,9 @@ class SovereignWalletViewModel( val listWalletState = _listWalletState.asStateFlow() /** - * Every identity this device holds a secret for, from both stores -- `seed.dat` and - * `nostr-keys.dat` -- keyed by the id the preferences and metadata use. + * Every profile this device holds something for -- the credentials file, with each + * seed attached to the profile its key derives -- keyed by the id the preferences + * and metadata use. */ private val _availableIdentities = MutableStateFlow>(emptyMap()) val availableIdentities = _availableIdentities.asStateFlow() @@ -145,10 +147,17 @@ class SovereignWalletViewModel( } /** - * Activates the identity a just-started node derives: the mnemonic kind, with the - * node behind it. + * Activates a profile with a wallet attached, once its node has started. + * + * The key is the credential's, not the node's. The node still derives it -- and is + * asked to, as a cross-check -- but the credentials file is what the app signs with + * now, and a node that disagrees with it is the one corruption worth refusing to run + * under: the throw lands in the startup view model's handler and shows as a startup + * error. It cannot happen for a file the repair wrote, since that derived the key + * from the same seed. */ - fun setActiveWallet(walletId: WalletId, business: PhoenixBusiness) { + fun setActiveWallet(stored: StoredIdentity.Mnemonic, business: PhoenixBusiness) { + val walletId = stored.id val keyManager = business.walletManager.keyManager.value if (keyManager == null) { // `startNewBusiness` loads the wallet before it reports success, so a business @@ -157,13 +166,16 @@ class SovereignWalletViewModel( log.e { "business for wallet=$walletId started without a key manager" } return } + check(keyManager.nostrPrivateKey() == stored.privateKey) { + "the node for wallet=$walletId derives a different nostr key than its credential holds" + } val dataStoreManager = DataStoreManager(business) setActiveIdentity( Identity.signing( id = walletId, kind = IdentityKind.Mnemonic, - nostrPrivateKey = keyManager.nostrPrivateKey(), + nostrPrivateKey = stored.privateKey, userPrefs = dataStoreManager.loadUserPrefsForWallet(walletId = walletId), internalPrefs = dataStoreManager.loadInternalPrefsForWallet(walletId = walletId), business = business, @@ -173,12 +185,14 @@ class SovereignWalletViewModel( } /** - * Reads both stores and publishes the merged list, registering metadata for any id - * seen for the first time. + * Reads both stores, repairs the credentials file so that every seed's key is in it, + * and publishes the merged list, registering metadata for any id seen for the first + * time. * * A failure in either file is surfaced, not skipped: a corrupt `nostr-keys.dat` would * otherwise drop every imported identity from the list without a word, and the seed - * file has always been handled this way. + * file has always been handled this way. A failure of the *repair* is the one thing + * logged and not surfaced -- the listing derives around it, see [SeedCredentials]. */ fun listIdentities(onDone: () -> Unit) { viewModelScope.launch(Dispatchers.IO + CoroutineExceptionHandler { _, e -> @@ -237,7 +251,7 @@ class SovereignWalletViewModel( } } - val credentials: Map = when (val result = NostrCredentialManager.loadAndDecrypt(phoenixGlobal)) { + val readCredentials: Map = when (val result = NostrCredentialManager.loadAndDecrypt(phoenixGlobal)) { is DecryptNostrCredentialsResult.Failure.SerializationError -> { log.e { "cannot deserialize nostr credentials file" } _listWalletState.value = ListWalletState.Error.Serialization @@ -262,6 +276,23 @@ class SovereignWalletViewModel( is DecryptNostrCredentialsResult.Success -> result.credentials } + // The second write a listing makes, and for the same reason as the first: a + // seed's key belongs in the credentials file, and every seed from before that + // was so gets its entry here, once. The listing uses what the repair returns -- + // the repaired map, or the one read if the write failed -- so a seed is listed + // either way. + val credentials = when (val repair = SeedCredentials.reconcile(log, phoenixGlobal, wallets, readCredentials)) { + is SeedCredentials.Result.Written -> { + log.i { "wrote the credential of ${repair.count} seed(s) into the credentials file" } + repair.credentials + } + is SeedCredentials.Result.NotNeeded -> repair.credentials + is SeedCredentials.Result.Failed -> { + log.e("the credentials of ${wallets.size} seed(s) could not be repaired; listing by derivation", repair.cause) + repair.credentials + } + } + val identities = StoredIdentity.merge(wallets, credentials) val metadataMap = getAvailableWalletsMeta(phoenixGlobal).first() diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/ForgetIdentityJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/ForgetIdentityJvmTest.kt index e746653e..53651918 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/ForgetIdentityJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/ForgetIdentityJvmTest.kt @@ -74,12 +74,12 @@ class ForgetIdentityJvmTest { } @Test - fun `a mnemonic identity is refused before anything else is touched`() = runBlocking { + fun `a profile with a wallet attached is refused before anything else is touched`() = runBlocking { val recorder = Recorder() - val outcome = forget(recorder, IdentityWriter.ForgetNostrCredentialResult.NotACredential) + val outcome = forget(recorder, IdentityWriter.ForgetNostrCredentialResult.WalletAttached) - assertEquals(ForgetIdentity.Outcome.NotACredential, outcome) + assertEquals(ForgetIdentity.Outcome.WalletAttached, outcome) assertEquals(listOf("forgetNostrCredential"), recorder.effects) } diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/IdentityWriterJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/IdentityWriterJvmTest.kt index 28e2cb61..e19b2f54 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/IdentityWriterJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/IdentityWriterJvmTest.kt @@ -2,19 +2,35 @@ package press.mantra.compose.identity import androidx.datastore.preferences.core.PreferenceDataStoreFactory import co.touchlab.kermit.Logger +import fr.acinq.bitcoin.MnemonicCode import fr.acinq.bitcoin.PrivateKey +import fr.acinq.bitcoin.byteVector import fr.acinq.lightning.Lightning +import fr.acinq.lightning.crypto.LocalKeyManager import fr.acinq.phoenix.PhoenixGlobal +import fr.acinq.phoenix.data.WalletId +import fr.acinq.phoenix.managers.NodeParamsManager import fr.acinq.phoenix.managers.NostrCredentialManager +import fr.acinq.phoenix.managers.SeedManager import fr.acinq.phoenix.managers.computePreferencePath import fr.acinq.phoenix.managers.nostrPublicKeyHex import fr.acinq.phoenix.security.JvmKeyStore import fr.acinq.phoenix.security.NostrCredential +import fr.acinq.phoenix.utils.MnemonicLanguage import fr.acinq.phoenix.utils.PlatformContext import fr.acinq.phoenix.utils.preferences.GlobalPrefs +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout import okio.FileSystem import okio.SYSTEM +import press.mantra.compose.ui.view.model.WritingSeedState import java.io.File import java.nio.file.Files import kotlin.test.AfterTest @@ -23,18 +39,21 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertIs +import kotlin.test.assertNotEquals import kotlin.test.assertTrue /** - * The writers for a bare key and a bare public key, against a real key store and a - * real directory. + * The three writers, against a real key store and a real directory. * - * What is pinned is the duplicate rule, its one exception, and the inverse. A second + * What is pinned is the duplicate rule, its two exceptions, and the inverse. A second * import of the same key is refused by public key, the same npub imported twice being * the one thing the id check cannot see; a public key already here is *not* a duplicate * of the secret that signs as it, and the entry becomes a secret one under the id it - * had; and forgetting takes the entry, whichever kind, out of the file and the identity's - * preference files off the disk, leaving every other entry where it was. + * had; a bare secret already here is *not* a duplicate of the phrase that derives it, + * and the seed attaches to it; a phrase writes its key to the credentials file as well + * as its words to the seed file; and forgetting takes the entry, whichever kind, out of + * the file and the identity's preference files off the disk, leaving every other entry + * where it was -- unless a seed derives the key, in which case nothing is touched. */ class IdentityWriterJvmTest { @@ -51,8 +70,14 @@ class IdentityWriterJvmTest { private val first = PrivateKey(Lightning.randomBytes(32)) private val second = PrivateKey(Lightning.randomBytes(32)) + private val scope = CoroutineScope(Job()) + @BeforeTest fun setUp() { + // The seed writer reports on Main, as the create screen's state machine needs it + // to; a real dispatcher rather than a test one, because it reports after a real + // one-second delay and a virtual clock would never reach it. + Dispatchers.setMain(Dispatchers.Default) storeDir = Files.createTempDirectory("mantra-identity-writer-store").toFile() appDir = Files.createTempDirectory("mantra-identity-writer-app").toFile() JvmKeyStore.lock() @@ -67,11 +92,48 @@ class IdentityWriterJvmTest { @AfterTest fun tearDown() { + scope.cancel() + Dispatchers.resetMain() JvmKeyStore.lock() storeDir.deleteRecursively() appDir.deleteRecursively() } + /** Twelve fresh words, and what they derive. */ + private class Phrase { + val words: List = MnemonicCode.toMnemonics(Lightning.randomBytes(16), MnemonicLanguage.English.wordlist()) + private val keyManager = LocalKeyManager( + seed = MnemonicCode.toSeed(words, "").byteVector(), + chain = NodeParamsManager.chain, + remoteSwapInExtendedPublicKey = NodeParamsManager.remoteSwapInXpub, + ) + val walletId = WalletId(keyManager.nodeKeys.nodeKey.publicKey) + val nostrKey = StoredIdentity.nostrPrivateKeyOf(words) + val nostrPublicKey = nostrKey.nostrPublicKeyHex() + } + + /** The callback-shaped seed writer, awaited: exactly one of its two callbacks fires. */ + private suspend fun writePhrase(phrase: Phrase): Result { + val outcome = CompletableDeferred>() + IdentityWriter.writeMnemonic( + log = log, + phoenixGlobal = phoenixGlobal, + globalPrefs = globalPrefs, + writingState = WritingSeedState.Init, + viewModelScope = scope, + mnemonics = phrase.words, + onWritingSeedError = { outcome.complete(Result.failure(IllegalStateException(it.toString()))) }, + onWritingSeedStateWriting = {}, + isRestoringWallet = false, + isTorEnabled = false, + customElectrumServer = null, + onSeedWritten = { outcome.complete(Result.success(it)) }, + ) + return withTimeout(15_000) { outcome.await() } + } + + private fun seeds() = SeedManager.loadAndDecryptOrNull(phoenixGlobal) + private suspend fun write(key: PrivateKey) = IdentityWriter.writeNostrKey( log = log, phoenixGlobal = phoenixGlobal, @@ -173,15 +235,98 @@ class IdentityWriterJvmTest { assertFalse(FileSystem.SYSTEM.exists(userPrefsFile(readOnly))) } - /** A wallet's nostr key is in the seed, not here; removing a seed is a wallet question. */ + /** + * Not in the file at all. Since the repair that can only be a seed's key the repair + * could not write, and the answer is the same as for one it did: a wallet is attached. + */ @Test fun `a key that is not in the file is not forgotten`() = runBlocking { write(first) val other = StoredIdentity.nostrSecret(second) - assertIs( + assertIs( IdentityWriter.forgetNostrCredential(log, phoenixGlobal, other.id, other.nostrPublicKey) ) assertEquals(1, credentials()?.size) } + + /** + * The credential a seed always implied. A phrase writes two files: its key as a secret + * in the credentials file, and its words in the seed file. The profile's id is the + * wallet's, and the nsec of that key is then a duplicate -- the device holds more than + * the key for it. + */ + @Test + fun `a phrase writes its key to the credentials file as well as its words to the seed file`() = runBlocking { + val phrase = Phrase() + + val id = writePhrase(phrase).getOrThrow() + + assertEquals(phrase.walletId, id) + assertEquals(mapOf(phrase.nostrPublicKey to NostrCredential.Secret(phrase.nostrKey)), credentials()) + assertEquals(setOf(phrase.walletId), seeds()?.keys) + assertTrue(FileSystem.SYSTEM.exists(computePreferencePath(phoenixGlobal.ctx, "userprefs_${id.nodeIdHash}.preferences_pb"))) + + assertIs(write(phrase.nostrKey)) + assertIs(writePublic(phrase.nostrKey)) + assertIs( + IdentityWriter.forgetNostrCredential(log, phoenixGlobal, id, phrase.nostrPublicKey) + ) + assertEquals(1, credentials()?.size, "the forget changed nothing") + } + + /** The same seed twice is the one thing the phrase writer still refuses. */ + @Test + fun `the same phrase twice is refused`() = runBlocking { + val phrase = Phrase() + writePhrase(phrase).getOrThrow() + + val second = writePhrase(phrase) + + assertTrue(second.isFailure) + assertTrue(second.exceptionOrNull()?.message?.contains("SeedAlreadyExists") == true, "$second") + assertEquals(1, seeds()?.size) + } + + /** + * The second exception to the duplicate rule, and the first attachment: the phrase of + * a key held as a bare secret used to be refused, and is the profile acquiring the + * wallet that derives it. The entry stays a secret for the same key; the id becomes + * the wallet's, and the bare key's preference files go with the old id. + */ + @Test + fun `the phrase of a key held as a bare secret attaches its wallet`() = runBlocking { + val phrase = Phrase() + val bare = assertIs(write(phrase.nostrKey)) + val bareUserPrefs = userPrefsFile(StoredIdentity.nostrSecret(phrase.nostrKey)) + assertTrue(FileSystem.SYSTEM.exists(bareUserPrefs)) + + val id = writePhrase(phrase).getOrThrow() + + assertEquals(phrase.walletId, id) + assertNotEquals(bare.id, id, "the profile's id is its wallet's now") + assertEquals(mapOf(phrase.nostrPublicKey to NostrCredential.Secret(phrase.nostrKey)), credentials()) + assertEquals(setOf(phrase.walletId), seeds()?.keys) + assertFalse(FileSystem.SYSTEM.exists(bareUserPrefs), "the bare key's preferences went with its id") + assertTrue(FileSystem.SYSTEM.exists(computePreferencePath(phoenixGlobal.ctx, "userprefs_${id.nodeIdHash}.preferences_pb"))) + } + + /** The read-only upgrade, landing on a wallet: the public entry becomes a secret, and the seed attaches. */ + @Test + fun `the phrase of a key held read-only attaches its wallet and the entry becomes a secret`() = runBlocking { + val phrase = Phrase() + writePublic(phrase.nostrKey) + writePublic(second) + + val id = writePhrase(phrase).getOrThrow() + + assertEquals(phrase.walletId, id) + assertEquals( + mapOf( + phrase.nostrPublicKey to NostrCredential.Secret(phrase.nostrKey), + second.nostrPublicKeyHex() to NostrCredential.Public, + ), + credentials(), + ) + } } diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/SeedCredentialsJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/SeedCredentialsJvmTest.kt new file mode 100644 index 00000000..2cff0a73 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/SeedCredentialsJvmTest.kt @@ -0,0 +1,192 @@ +package press.mantra.compose.identity + +import co.touchlab.kermit.Logger +import fr.acinq.bitcoin.MnemonicCode +import fr.acinq.bitcoin.byteVector +import fr.acinq.lightning.Lightning +import fr.acinq.lightning.crypto.LocalKeyManager +import fr.acinq.phoenix.PhoenixGlobal +import fr.acinq.phoenix.data.WalletId +import fr.acinq.phoenix.managers.NodeParamsManager +import fr.acinq.phoenix.managers.NostrCredentialManager +import fr.acinq.phoenix.managers.SeedManager +import fr.acinq.phoenix.managers.nostrPublicKeyHex +import fr.acinq.phoenix.security.EncryptedNostrCredentials +import fr.acinq.phoenix.security.EncryptedSeed +import fr.acinq.phoenix.security.JvmKeyStore +import fr.acinq.phoenix.security.NostrCredential +import fr.acinq.phoenix.utils.MnemonicLanguage +import fr.acinq.phoenix.utils.PlatformContext +import java.io.File +import java.nio.file.Files +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * The repair that runs at listing, against a device from before the credentials file + * held a seed's key: `seed.dat` written directly, as every wallet on every device was + * until docs/multiple-profiles.md, and no credential for any of them. + * + * What is pinned is that the repair writes exactly what is missing, once; that a + * device already repaired is not written to again -- the file's bytes are compared, + * since a rewrite would carry a fresh iv and could not be told apart by its contents; + * that a public entry for a seed's key is upgraded rather than left beside a secret; + * and that a write that fails is reported with the map that was read, not thrown. + */ +class SeedCredentialsJvmTest { + + private lateinit var storeDir: File + private lateinit var appDir: File + private lateinit var phoenixGlobal: PhoenixGlobal + + private val log = Logger.withTag("SeedCredentialsJvmTest") + + private class Seed(val words: List) { + val keyManager = LocalKeyManager( + seed = MnemonicCode.toSeed(words, "").byteVector(), + chain = NodeParamsManager.chain, + remoteSwapInExtendedPublicKey = NodeParamsManager.remoteSwapInXpub, + ) + val walletId = WalletId(keyManager.nodeKeys.nodeKey.publicKey) + val nostrKey = StoredIdentity.nostrPrivateKeyOf(words) + val nostrPublicKey = nostrKey.nostrPublicKeyHex() + } + + private fun freshSeed() = Seed(MnemonicCode.toMnemonics(Lightning.randomBytes(16), MnemonicLanguage.English.wordlist())) + + private val first = freshSeed() + private val second = freshSeed() + + @BeforeTest + fun setUp() { + storeDir = Files.createTempDirectory("mantra-seed-credentials-store").toFile() + appDir = Files.createTempDirectory("mantra-seed-credentials-app").toFile() + JvmKeyStore.lock() + JvmKeyStore.unlock("correct horse battery staple".toCharArray(), storeDir) + phoenixGlobal = PhoenixGlobal(PlatformContext(applicationDir = appDir)) + } + + @AfterTest + fun tearDown() { + JvmKeyStore.lock() + storeDir.deleteRecursively() + appDir.deleteRecursively() + } + + /** A device from before: the seeds on disk, and nothing said about their keys. */ + private fun writeSeedsOnly(vararg seeds: Seed) { + SeedManager.writeSeedToDisk( + phoenixGlobal, + EncryptedSeed.V2.encrypt(seeds.associate { it.walletId to it.words }), + overwrite = true, + ) + } + + private fun wallets() = SeedManager.loadAndDecryptOrNull(phoenixGlobal) ?: error("seed store unreadable") + private fun credentials() = NostrCredentialManager.loadAndDecryptOrNull(phoenixGlobal) ?: error("credentials unreadable") + private fun credentialsFileBytes(): ByteArray? = + File(SeedManager.getDatadir(phoenixGlobal.ctx).toString(), "nostr-credentials.dat").takeIf { it.exists() }?.readBytes() + + @Test + fun `every seed without a credential gets one, in one write`() { + writeSeedsOnly(first, second) + assertNull(credentialsFileBytes(), "the device from before had no credentials file") + + val result = SeedCredentials.reconcile(log, phoenixGlobal, wallets(), credentials()) + + val written = assertIs(result) + assertEquals(2, written.count) + val expected = mapOf( + first.nostrPublicKey to NostrCredential.Secret(first.nostrKey), + second.nostrPublicKey to NostrCredential.Secret(second.nostrKey), + ) + assertEquals(expected, written.credentials, "the map the listing is handed") + assertEquals(expected, credentials(), "and the file") + } + + @Test + fun `a device already repaired is not written to again`() { + writeSeedsOnly(first) + SeedCredentials.reconcile(log, phoenixGlobal, wallets(), credentials()) + val bytesAfterRepair = credentialsFileBytes() + + val result = SeedCredentials.reconcile(log, phoenixGlobal, wallets(), credentials()) + + val notNeeded = assertIs(result) + assertEquals(mapOf(first.nostrPublicKey to NostrCredential.Secret(first.nostrKey)), notNeeded.credentials) + assertTrue(bytesAfterRepair.contentEquals(credentialsFileBytes()), "a rewrite would carry a fresh iv") + } + + @Test + fun `a device with no seeds needs nothing`() { + val result = SeedCredentials.reconcile(log, phoenixGlobal, emptyMap(), emptyMap()) + + assertIs(result) + assertNull(credentialsFileBytes()) + } + + /** The read-only upgrade the seed writer performs, done for a device where it was left undone. */ + @Test + fun `a public entry for a seed's key becomes a secret`() { + writeSeedsOnly(first) + NostrCredentialManager.writeToDisk( + phoenixGlobal, + EncryptedNostrCredentials.encrypt(mapOf(first.nostrPublicKey to NostrCredential.Public)), + ) + + val result = SeedCredentials.reconcile(log, phoenixGlobal, wallets(), credentials()) + + val written = assertIs(result) + assertEquals(1, written.count) + assertEquals(mapOf(first.nostrPublicKey to NostrCredential.Secret(first.nostrKey)), credentials()) + } + + /** Other people's entries are left exactly where they were. */ + @Test + fun `bare keys already in the file are untouched`() { + writeSeedsOnly(first) + val bare = StoredIdentity.nostrSecret(fr.acinq.bitcoin.PrivateKey(Lightning.randomBytes(32))) + NostrCredentialManager.writeToDisk( + phoenixGlobal, + EncryptedNostrCredentials.encrypt(mapOf(bare.nostrPublicKey to NostrCredential.Secret(bare.privateKey))), + ) + + val written = assertIs(SeedCredentials.reconcile(log, phoenixGlobal, wallets(), credentials())) + + assertEquals(1, written.count) + assertEquals( + mapOf( + bare.nostrPublicKey to NostrCredential.Secret(bare.privateKey), + first.nostrPublicKey to NostrCredential.Secret(first.nostrKey), + ), + credentials(), + ) + } + + /** + * The write fails -- here because the key store is locked, which is how it fails on + * a device whose keystore entry is gone -- and the listing must go on with what it + * read. A failed write must never hide a wallet. + */ + @Test + fun `a write that fails is reported with the map that was read`() { + writeSeedsOnly(first) + val wallets = wallets() + val read = credentials() + JvmKeyStore.lock() + + val result = SeedCredentials.reconcile(log, phoenixGlobal, wallets, read) + + val failed = assertIs(result) + assertEquals(read, failed.credentials) + assertNull(credentialsFileBytes(), "nothing reached the disk") + // And the listing built on it still holds the wallet, by derivation. + val listed = assertIs(StoredIdentity.merge(wallets, failed.credentials)[first.walletId]) + assertEquals(first.nostrKey, listed.privateKey) + } +} diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/StoredIdentityJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/StoredIdentityJvmTest.kt index 5f962472..971b8f26 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/StoredIdentityJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/identity/StoredIdentityJvmTest.kt @@ -15,8 +15,8 @@ import kotlin.test.assertNotEquals * * The vector is NIP-06's own: twelve words, the key at `m/44'/1237'/0'/0/0`, and the * x-only public key. It pins that a wallet's `nostrPublicKey` is the same key an nsec - * import of that wallet's nostr secret would produce -- which is the whole basis of the - * duplicate check in `IdentityWriter`. + * import of that wallet's nostr secret would produce -- which is what attaches a seed + * to its credential, and what the duplicate check in `IdentityWriter` compares. */ class StoredIdentityJvmTest { @@ -43,6 +43,7 @@ class StoredIdentityJvmTest { val merged = StoredIdentity.merge( wallets = mapOf(wallet.walletId to wallet), credentials = mapOf( + nip06PublicKey to NostrCredential.Secret(nip06PrivateKey), bareKey.publicKey().xOnly().value.toHex() to NostrCredential.Secret(bareKey), readOnlyKey to NostrCredential.Public, ), @@ -51,6 +52,7 @@ class StoredIdentityJvmTest { assertEquals(3, merged.size) val mnemonic = assertIs(merged[wallet.walletId]) assertEquals(nip06PublicKey, mnemonic.nostrPublicKey) + assertEquals(nip06PrivateKey, mnemonic.privateKey, "the credential's key, not a derivation") assertEquals(IdentityKind.Mnemonic, mnemonic.kind) val secret = assertIs(merged[bareKey.publicKey().xOnly().toWalletId()]) @@ -73,27 +75,47 @@ class StoredIdentityJvmTest { } /** - * The same npub as a wallet, imported as a bare key, gets a *different* id -- one - * hash is of the node key, the other of the nostr key -- so a map keyed by id holds - * both. That is not a bug in the merge; it is why the writers dedupe by public key. + * The normal state of a wallet since docs/multiple-profiles.md: its key is a secret + * in the credentials file, and the seed is attached to that entry. Listed once, under + * the wallet's id, with the credential's key -- not twice under two ids, which is what + * this map held before the credentials file was the list of profiles. */ @Test - fun `a wallet and its own nostr key as a bare secret do not collide by id`() { + fun `a secret whose key a seed derives is listed once, as the wallet, with the credential's key`() { val merged = StoredIdentity.merge( wallets = mapOf(wallet.walletId to wallet), credentials = mapOf(nip06PublicKey to NostrCredential.Secret(nip06PrivateKey)), ) - assertEquals(2, merged.size) - assertEquals(1, merged.values.map { it.nostrPublicKey }.toSet().size, "one npub, twice") + assertEquals(1, merged.size) + val mnemonic = assertIs(merged[wallet.walletId]) + assertEquals(nip06PrivateKey, mnemonic.privateKey) + assertEquals(wallet, mnemonic.userWallet) } /** - * The one precedence the merge decides. A recovery phrase pasted over a public - * credential is written across two files -- the credential removed first, then the - * seed -- and a crash between the two would leave neither, never both; this is the - * rule for the "both" that is not supposed to happen, so that it lists the wallet - * rather than one npub twice. + * The state a device from before the repair is in, and the one a failed repair + * leaves: the seed is listed anyway, with its key derived from the words, so that a + * missing entry never hides a wallet. + */ + @Test + fun `a seed with no credential is listed with its key derived`() { + val merged = StoredIdentity.merge( + wallets = mapOf(wallet.walletId to wallet), + credentials = emptyMap(), + ) + + assertEquals(1, merged.size) + val mnemonic = assertIs(merged[wallet.walletId]) + assertEquals(nip06PrivateKey, mnemonic.privateKey) + assertEquals(nip06PublicKey, mnemonic.nostrPublicKey) + } + + /** + * A public entry for a key a seed derives is the state the seed writer's two-file + * upgrade leaves if it dies between its writes, and the one the repair upgrades at + * the next listing. Until then the wallet is listed by derivation, and the public + * entry is not listed at all: one npub, once. */ @Test fun `a public key a seed already derives is listed as the wallet only`() { @@ -103,7 +125,8 @@ class StoredIdentityJvmTest { ) assertEquals(1, merged.size) - assertIs(merged[wallet.walletId]) + val mnemonic = assertIs(merged[wallet.walletId]) + assertEquals(nip06PrivateKey, mnemonic.privateKey) } @Test diff --git a/docs/multiple-profiles.md b/docs/multiple-profiles.md index 49cd6ef9..2f6bedef 100644 --- a/docs/multiple-profiles.md +++ b/docs/multiple-profiles.md @@ -337,13 +337,14 @@ object SeedCredentials { } ``` -It runs after the seed file is read and the nostr-keys migration has run, and -before the credentials file is read for the listing — so the listing sees the -repaired file. On a device with no seeds it does nothing; on a device already -repaired it does nothing; on the first launch after this phase it writes once. -`Failed` is logged, not surfaced: this is a repair of something the app can still -work around, and a listing that goes red because a background write failed would -be worse than the derivation it was replacing. +It runs after both files are read and the nostr-keys migration has run, and the +listing is built from what it returns — the repaired map, or the one read if the +write failed — so the listing sees the repaired file without decrypting it twice. +On a device with no seeds it does nothing; on a device already repaired it does +nothing; on the first launch after this phase it writes once. `Failed` is logged, +not surfaced: this is a repair of something the app can still work around, and a +listing that goes red because a background write failed would be worse than the +derivation it was replacing. ### The listing diff --git a/docs/npub-sign-in.md b/docs/npub-sign-in.md index 91eda7e9..13672418 100644 --- a/docs/npub-sign-in.md +++ b/docs/npub-sign-in.md @@ -29,6 +29,7 @@ below says so: | `signInToProfile` "made idempotent by pubkey" | done, and tested by signing in twice and counting one; the sign-in view model's three writers now go through one shared write-and-map | | the round trip "with the network faked at the repository" | with the *database* real: an in-memory Room under the app's own DAOs and a real `NotaryViewModel`, because "nothing was signed" is about what is not in the tables — plus a contrast case the plan did not ask for, the same harness as a signing identity producing a key package, so the assertions are known to bite | | `use_a_different_key` as a new string | it existed: the sign-in screen's own "use a different key" button. One string, two screens, one meaning | +| one entry per pubkey *across both files*, with `merge` listing the seeds and adding the credentials | **superseded** by [multiple-profiles.md](./multiple-profiles.md), Phase 1: the credentials file is the list of profiles and a seed is attached to the entry its key derives, so a seed's key *is* in the file; `merge` lists the credentials and attaches the seeds. The upgrade table gains a row — a phrase over a bare secret attaches — and `NotACredential` becomes `WalletAttached` | One thing found on the way that is in no phase: a compose test that looks for a floating action button's label by text has to search the unmerged tree, because diff --git a/docs/nsec-sign-in.md b/docs/nsec-sign-in.md index 0b02b703..f91e0eb4 100644 --- a/docs/nsec-sign-in.md +++ b/docs/nsec-sign-in.md @@ -30,6 +30,7 @@ it: | the one hop, unspecified where | in the sync pump, after `saveNostrEvent`, with a per-account set so five indexers answering with the same kind 10002 make one hop | | forget: key, prefs, metadata, active identity | plus the account's unsigned rows (`forgetLocalAccount`), which the plan did not list: the kind 0 that made it a local account, and anything queued that can now never be signed. Published events and the profile cache stay | | assert `platformStartupLogic` was not called | `identity.business == null` and the jvm `BusinessManager.businessFlow` empty afterwards — the observable fact rather than the call | +| the dedupe rule: a seed's key is kept out of the key file, and a seed whose key is here as a bare secret is `SeedAlreadyExists` | **superseded** by [multiple-profiles.md](./multiple-profiles.md), Phase 1: a seed's key is a `Secret` credential like any other, written when the seed is and repaired in for every seed already here; a bare secret's phrase *attaches* the wallet rather than being refused; the identity's key is read from the credential, with the node's as a cross-check | Two things found on the way that are not in any phase. `DataStoreManager` caches each id's preferences in a companion object for the life of the process, so a test