Skip to content

fix(nostr): auto-relink local key when backend npub is missing or stale - #720

Open
islandbitcoin wants to merge 12 commits into
mainfrom
fix/nostr-npub-auto-relink
Open

fix(nostr): auto-relink local key when backend npub is missing or stale#720
islandbitcoin wants to merge 12 commits into
mainfrom
fix/nostr-npub-auto-relink

Conversation

@islandbitcoin

@islandbitcoin islandbitcoin commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Ticket: ENG-602

Problem

"Sent but not received" Nostr DMs. Verified on relay.flashapp.me: gift-wraps are published with the correct #p (the backend npubByUsername value), but the recipient device never shows them.

Senders encrypt to the backend-registered npub. The device can only decrypt with the key in its keychain. NostrKeyEnsurer only handled the "no local key" cases, so a device holding a key the backend did not advertise never received anything:

  • backend npub null (the flash account on prod today: npubByUsername("flash") → null) → nobody can address it
  • backend npub ≠ device key (stale key from an earlier install / second device) → DMs go to a key this device does not hold

The only repair was the buried "Reconnect profile" button under Advanced settings.

Fix

  • app/nostr/npub-link.ts: npubLinkState(local, backend) → linked | unregistered | mismatch | conflict | fresh.
  • NostrKeyEnsurer (mounted inside NavigationContainerWrapper, reads the account npub network-only because the persisted Apollo cache would otherwise decide a backend write off last session's value):
    • unregistered → silently registers the local key. A backend refusal is told to the user.
    • mismatch → asks once per backend npub ("Use this device for chat?"). Never rewrites a registered npub without the user choosing it, so two live installs cannot flip the account back and forth. The "asked" marker is written only from a button handler and only when the relink did not fail in transit.
    • Alerts are held until the PIN/biometric gate is passed; the silent write is not.
    • conflict (backend npub, no local key) and fresh are unchanged.
  • saveNewNostrKey: the catch is narrowed to getSigner, so a failed relink can never fall through to generating a new key over an existing one. The existing-key branch registers the local key only for unregistered and only once me is loaded.
  • handleReconnectNostr (Advanced settings): reports a refused relink instead of "success", and catches a rejected mutation.
  • Five new strings, translated in all 23 locales.

Out of scope: shared phones

Two Flash accounts on one phone are not handled here. The keychain survives logout, so account B can start with account A's key, and this PR would register it silently for B if B has no npub. That behaviour is unchanged from what any device with a leftover key does today, and is tracked as multi-account support in ENG-601. The key-ownership piece is in draft #727, stacked on this PR.

Not fixed by this

Wraps already encrypted to a key this device does not hold cannot be decrypted here; they are readable only by whatever install holds that key. After this ships, new DMs land correctly.

Tests (36, all passing)

  • __tests__/nostr/npub-link-state.spec.ts: every row of the state table.
  • __tests__/nostr/reconnect-npub.spec.ts: ok, refused, no-key, propagated failure.
  • __tests__/components/nostr-key-ensurer.spec.tsx: network-only policy, linked, unregistered (silent / refused / rejected), mismatch (prompt only, confirm, decline, dismissed re-asks, failed re-asks, refused remembered, per-npub marker), unreadable local key, locked-app deferral, conflict, fresh.
  • __tests__/hooks/use-nostr-profile-relink.spec.ts: existing key + null backend → register, no keychain write; linked / mismatch / me not loaded → no write; rejected or refused mutation → no key generation, no keychain overwrite.

use-nostr-profile.ts and advanced-settings.tsx have pre-existing eslint errors outside the touched lines; none added.

🤖 Generated with Claude Code

https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX

Dread and others added 8 commits September 15, 2026 10:26
Dread @claude
fix(nostr): auto-relink local key when backend npub is missing or stale
6e8e77e 
Senders resolve a recipient through npubByUsername and encrypt gift-wraps
to that key. The device can only decrypt wraps addressed to the key in
its keychain. NostrKeyEnsurer only handled "no local key" cases, so a
device holding a key the backend did not advertise (npub null, or a
different key registered from an earlier install) published fine but
never received anything: DMs were encrypted to a key nobody held.

- npubLinkState(): explicit linked / unregistered / mismatch / conflict /
  fresh table, shared by the ensurer and saveNewNostrKey.
- NostrKeyEnsurer: when a local key exists and the backend npub is null
  or differs, re-register the local npub and re-init chat. A backend
  refusal (NPUB_NOT_AVAILABLE) is logged and left for the settings
  screen; the conflict branch (backend npub, no local key) is unchanged
  and never auto-generates.
- saveNewNostrKey: the existing-signer early return now relinks too, so
  the username-set path also repairs an unregistered account.

Tests cover every state of the table and the ensurer's behaviour for
each, including refusal and network failure.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): review fixes — never regenerate over an existing key, ask…
f0d5fcd 
… before replacing a registered npub

Review findings on #720:

- saveNewNostrKey: the relink mutation lived inside the catch documented as
  "no existing key — proceed with generation". A rejected userUpdateNpub on
  a device that HAS a key fell through to generateSecretKey and overwrote
  the keychain, making every DM ever encrypted to the old key unreadable.
  The catch now covers getSigner only; with a signer present the relink
  has its own log-only catch, ensureContactListExists still runs, and
  generation is unreachable.

- NostrKeyEnsurer: only `unregistered` is auto-relinked (nothing on the
  account to steal). `mismatch` means another install holds the registered
  key; two live installs silently relinking on every cold start flipped the
  account back and forth. It now asks once per backend npub (Alert, marker
  `npubMismatchPrompted:<backendNpub>` in AsyncStorage) and never rewrites
  a registered npub without the user choosing it.

- Dropped initializeChat() from the relink branch: the local signer did
  not change and ChatContextProvider already subscribed with it on mount.
  Kept in the fresh/generate branch where the key really is new.

- Tests: new __tests__/hooks/use-nostr-profile-relink.spec.ts covers the
  hook (existing key + backend null, linked, rejected mutation, refusal,
  unreadable pubkey — none may touch the keychain). Ensurer spec keys on
  the mutation rather than initializeChat, asserts mismatch never calls
  userUpdateNpub without confirmation, and the linked/conflict tests now
  flush the microtask chain before their negative assertions.
  npub-link-state.spec.ts moved to __tests__/nostr/ to mirror app/.

- i18n at the source (Nostr.keyMismatch*), 23 locales filled (es
  translated), drift green.

99 suites / 980 tests green; tsc clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): persist mismatch prompt marker only on an answer; never r…
2727782 
…elink over a registered npub from saveNewNostrKey

Review round 2 on #720:

- NostrKeyEnsurer: the "already asked" marker was written before
  Alert.alert. On Android any later alert during cold start
  (DialogModule.showNewAlert → dismissExisting) or a back-button dismiss
  closes the prompt with no button pressed, and the marker for that backend
  npub was already persisted — the device never asked again and stayed
  undeliverable. The marker is now written from both button handlers
  (Cancel and Use this device) only; a prompt dismissed without an answer
  is asked again on the next launch.

- saveNewNostrKey: `needsRelink` still included `mismatch`, so the
  username-set path silently overwrote a registered npub — the exact thing
  the ensurer prompt refuses to do without consent (phone holds K1, tablet
  holds K2, user declines the prompt, then sets a username → backend
  flipped to K2). Also no readiness guard: while the network-only query
  was in flight `npubLinkState(local, undefined)` read as `unregistered`
  and could relink against unknown backend state. Now gated on `me` being
  loaded and relinks only `unregistered`; mismatch stays owned by the
  ensurer prompt and the explicit Reconnect in advanced settings.
  `needsRelink` removed (no other callers).

- Tests: ensurer spec — alert shown, no button pressed → marker absent;
  cancel and confirm → marker written. Relink spec — rejecting-mutation
  case moved to `npub: null` (the only path that fires the mutation now),
  new `mismatch → no mutation` and `me undefined → no mutation` cases;
  query mock can now return undefined data. npub-link-state spec drops
  the `needsRelink` assertions. Each new test fails against the previous
  source.

99 suites / 983 tests green; tsc clean; i18n drift clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): decide relink from the live account, and only remember an…
e6708d4 
… answered prompt

The ensurer read the account npub cache-first, but the Apollo cache is
persisted across launches, so a cold start decided a backend write off
last session's value: a device that last saw `npub: null` would register
its key over one another install registered since, with no prompt. Read
network-only; an offline cold start now does nothing.

The mismatch confirm handler also persisted the "asked" marker before the
relink ran, so a mutation that failed in transit left the device
permanently undeliverable and silent. The relink now reports
ok / refused / failed: a transient failure leaves no marker so the
question returns next launch, a refusal is remembered, and both tell the
user their choice did not take effect.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): gate key prompts on app unlock, track key owner, surface …
295014a 
…refused relink

Review fixes for #720:

- Mount NostrKeyEnsurer inside NavigationContainerWrapper and hold every
  user-facing alert (mismatch prompt, foreign-key prompt, refused-relink
  notice) until isAppLocked drops. The silent `unregistered` write still
  runs as soon as live account data arrives. Previously the prompt could
  paint over the PIN/biometric screen and one tap rewrote the account npub
  without unlocking.
- Record which account a local key belongs to (app/nostr/key-owner.ts,
  keyed on the local npub) whenever a key is generated, registered or
  found linked. The keychain survives logout, so on a shared phone a key
  owned by another account is now prompt-class, not silently registered
  — both in the ensurer and in saveNewNostrKey after username set.
- A refused silent `unregistered` relink (NPUB_NOT_AVAILABLE) now shows
  the relink-failed alert instead of only logging.
- Add keyMismatchRelinkFailed to all 23 translation files (Spanish
  translated; the rest carry the English string, matching the sibling
  keys).
- Tests: unreadable-local-key guard, locked/unlocked prompt deferral,
  foreign-owner prompt/accept/decline/no-nag, refused silent relink
  alert, owner recording on link/relink/generate; saveNewNostrKey owner
  gating.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): key the foreign-key prompt on the key, split refused vs f…
66fd950 
…ailed copy, honor refusal in Reconnect

Review fixes for #720:

- foreignKeyPromptedKey is now keyed on (account, local npub). A marker
  written when B declined A's key K1 no longer swallows the prompt when a
  different foreign key K2 later lands in the keychain.
- The foreign-key prompt gets its own copy (keyForeignMessage): the account
  has no key in that state, so "your keys differ" was wrong.
- A deterministic NPUB_NOT_AVAILABLE refusal now shows keyMismatchRelinkRefused
  (delete the chat keys) instead of the "check your connection" text that
  pointed users at a Reconnect which refused the same way. Both strings are
  translated in all 23 locales.
- Settings > Nostr > Advanced > Reconnect profile reads userUpdateNpub.errors
  (via the new reconnectLocalNpub helper) and reports an error instead of
  "Success" when the backend refuses.
- Tests: key-owner set/get/clear, generateAndStoreKey owner stamp, the
  saveNewNostrKey fresh path stamping me.id, deleteNostrKeys clearing owner
  records, the (B,K1)/K2-owned-by-C prompt case, and reconnectLocalNpub.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): check once per account, owner-aware mismatch prompt, catc…
6b0ad17 
…h in Reconnect, translate all prompt strings

- The ensurer's once-guard was mount-scoped, and nothing above it remounts
  on logout, so account B logging in after A in the same process was
  never checked. Keyed on the account id instead.
- The mismatch prompt now consults the key-owner record: a local key that
  another account on this phone generated gets the foreign-key wording,
  and a refusal shows keyForeignRelinkRefused instead of advising the
  user to delete the other account's only copy of a registered key.
- handleReconnectNostr had a finally but no catch, so a rejected mutation
  (offline, the very state the failure alert sends people there in)
  escaped the async onPress. It now reports keyMismatchRelinkFailed.
- The four round-1 prompt strings were still English in 22 locales while
  the later ones were translated; all six plus the new key are now
  translated everywhere.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
refactor(nostr): move shared-phone key ownership out of the DM relink…
6e3b1d2 
… fix

Two Flash accounts sharing one phone is a feature in its own right
(multi-account support), not part of fixing undeliverable DMs. This
removes it from this PR so it can be reviewed on its own:

- app/nostr/key-owner.ts and the owner stamp in generateAndStoreKey
- the foreign-key prompt and its per-(account, key) marker
- owner-aware mismatch wording and refusal copy
- the per-account once-guard (back to once per mount)
- keyForeignMessage / keyForeignRelinkRefused in all locales
- the tests for all of the above

What stays: network-only account read, the narrowed catch that makes
regeneration unreachable when a key exists, silent relink for
unregistered, ask-once for mismatch, prompts held until the app unlocks,
and the Reconnect fixes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
@islandbitcoin islandbitcoin mentioned this pull request Sep 15, 2026
Draft
@linear

linear Bot commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown

ENG-601

ENG-602

Dread and others added 2 commits September 15, 2026 13:00
Dread @claude
fix(nostr): report reconnect result before refresh; drop held prompt …
2330175 
…on logout

- advanced-settings: decide the alert from the mutation result, then refresh
  fire-and-forget so a rejected refetch can't turn a done relink into an error.
  Adds component tests for ok, refused, no-key, rejected mutation and a
  rejected refresh after success.
- nostr-key-ensurer: docstring now says the check runs once per app process
  (hasRun intentionally not reset, ENG-601); a pending prompt is cleared on
  logout and never shown while logged out. Tests cover both orderings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
fix(nostr): drop check results that outlive the auth session
020d52d 
The key check's async work (relink mutation, keychain/storage reads) could
resolve after a logout and hold a prompt for whoever signed in next, or let
'Use this device' relink against a previous account's backend state. Every
auth transition now bumps a session counter; the check captures it and
ignores held prompts, button presses and the auto-generate write once it no
longer matches. Tests cover a deferred refused relink across logout (and
logout+login), a deferred mismatch storage read, and a button pressed after
logout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
islandbitcoin pushed a commit that referenced this pull request Sep 15, 2026
Dread @claude
Merge #720 review fixes into multi-account key ownership
c644bcb 
Both branches independently guarded against async check results that
outlive the session that started them: #720 with a render-time session
counter, this branch with a per-account run token. Kept this branch's
guard and folded the counter into it, so isCurrent() now also rejects a
continuation that resolves between the logout render and the logout
effect.

- Ensurer docstring: "once per app process" from #720 replaced with the
  per-account behaviour this branch implements.
- Reconnect: #720's report-before-refresh ordering with this branch's
  owner-aware refusal copy.
- #720's logout tests re-sign-in the same account. Here that account is
  re-checked on purpose, so each test now signs back in to a linked state
  and any alert or write can only come from the stale check. Disabling
  isCurrent() fails five of them.
- Reconnect screen spec: mock the account query and clear owner records
  between tests; refusal with no owner record expects the neutral copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
Dread @claude
style(nostr): fix lint on reformatted lines in use-nostr-profile
cd906fd 
The earlier prettier pass moved pre-existing lint errors onto changed
lines, which CI's changed-lines lint rejects: drop the unused SimplePool
import, use property shorthand, and mark flash_username (a Nostr profile
field other clients read) as an intentional camelcase exception.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
islandbitcoin pushed a commit that referenced this pull request Sep 15, 2026
Dread @claude
fix(nostr): restore key-owner tracking in use-nostr-profile lost in m…
1706d17 
…erge

The previous merge resolved a one-comment conflict by taking #720's whole
copy of use-nostr-profile.ts, which dropped this branch's owner stamp,
foreign-key guard and clearNostrKeyOwners. Restores this branch's file
with #720's comment wording on the flash_username lint exception.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
@bobodread876 bobodread876 mentioned this pull request Sep 16, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@islandbitcoin