feat: NIP-17 private DMs + nostrdb submodule conversion#3667
Draft
alltheseas wants to merge 24 commits intodamus-io:masterfrom
Draft
feat: NIP-17 private DMs + nostrdb submodule conversion#3667alltheseas wants to merge 24 commits intodamus-io:masterfrom
alltheseas wants to merge 24 commits intodamus-io:masterfrom
Conversation
Update nostrdb to include gift wrap (NIP-59) processing support: - Add NIP-44 encryption/decryption (ChaCha20 + HMAC-SHA256) - Add gift wrap unwrapping with key management - Add base64 encoding/decoding utilities Co-Authored-By: Claude Opus 4.5 <[email protected]> nostrdb: Add Swift bindings for gift wrap APIs Add Swift bindings for nostrdb's gift wrap processing: - ndb_add_key(): Register decryption key for gift wrap processing - ndb_process_giftwraps(): Trigger processing of stored gift wraps - NdbNote extensions for rumor detection and metadata Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
Implement NIP-17 Private Direct Messages (rumor→seal→gift_wrap): - createMessage(): Create gift wraps for recipient and sender (self-wrap) - unwrap(): Decrypt gift wrap → seal → rumor with signature verification - parseRumorJson(): Handle rumors without id field (calculate from hash) - createDMRelayList()/parseDMRelayList(): Kind 10050 support - Add kind definitions: dm_chat (14), seal (13), gift_wrap (1059), dm_relay_list (10050) Security: - Verify seal signature to prevent spoofed sender pubkeys - Use ephemeral keypairs for gift wrap signing - Randomize timestamps for temporal privacy (up to 2 days) Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
Register the user's private key with nostrdb on app startup to enable automatic gift wrap decryption: - Add initializeNip17KeysIfNeeded() to DamusState - Call from ContentView after DamusState creation - Trigger processGiftWraps() after adding key - Key initialization is idempotent and safe for multiple accounts Co-Authored-By: Claude Opus 4.5 <[email protected]> feat(nip17): Subscribe to kind 1059 gift wraps and process DM rumors Add NIP-17 event handling in HomeModel: - Subscribe to kind 1059 (gift wrap) where p-tag matches our pubkey - Process incoming gift wraps with Swift-side NIP17.unwrap() - Handle unwrapped kind 14 rumors via handle_dm() - Add debug logging for gift wrap reception and processing - Trigger nostrdb processGiftWraps() when new gift wraps arrive Co-Authored-By: Claude Opus 4.5 <[email protected]> refactor(nip17): Remove Swift-side gift wrap unwrapping Remove redundant Swift NIP17.unwrap() from HomeModel - nostrdb handles gift wrap processing natively: 1. Gift wraps (kind 1059) are ingested by nostrdb 2. nostrdb unwraps to rumors (kind 14) using registered keys 3. Unwrapped rumors are stored in nostrdb and streamed to app 4. App subscribes to kind 14 events which includes nostrdb-unwrapped rumors The processGiftWraps() call in dmsStream triggers nostrdb to process any pending gift wraps when new ones arrive. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
… model Implement NIP-17 message sending with DM outbox model: - Create gift-wrapped messages using NIP17.createMessage() - Fetch recipient's DM relay list (kind 10050) - Connect on-demand to recipient's DM relays (ensureConnected) - Send recipient wrap to their DM relays - Send sender self-wrap to own relays for cross-device recovery - Show warning if recipient has no DM relay list or connection fails - Fall back to NIP-04 if no private key available - Uses ephemeral relays that auto-cleanup (not persisted) Co-Authored-By: Claude Opus 4.5 <[email protected]> fix(nip17): Address code review issues 1. [High] Remove main-thread DB work during app init - Remove synchronous ndb.addKey/processGiftWraps from convenience init - Keep only the background init in initializeNip17KeysIfNeeded() 2. [Medium] Fix ephemeral relay leak / pool growth - Add sendToEphemeralRelays() that handles acquire/release lifecycle - Properly release leases after delayed timeout 3. [Medium] Fix stale relay list fetch - fetchDMRelayList now returns the latest event by created_at - Correctly handles kind 10050 as a replaceable event Co-Authored-By: Claude Opus 4.5 <[email protected]> fix(nip17): Restore warning on connection failure and use PostBox for reliability - Route ephemeral relay sends through PostBox for retry/backoff - Release ephemeral relay leases via OnFlush.once callback after ACK - Return Bool from send_nip17_message to indicate success/failure - Show fallback warning dialog when all DM relay connections fail - Move draft clearing to after successful send Co-Authored-By: Claude Opus 4.5 <[email protected]> fix(nip17): Improve ephemeral relay lease lifecycle and add DEBUG fallback Ephemeral relay lease lifecycle: - Release leases after first ACK only if remaining relays are disconnected - Keep leases alive while PostBox has connected relays to retry - Add 20-second safety timeout to prevent indefinite lease leaks DEBUG safety net: - Add Swift-side unwrap fallback in DEBUG builds only - Helps diagnose nostrdb gift wrap processing issues during development - Production builds rely solely on nostrdb for unwrapping Co-Authored-By: Claude Opus 4.5 <[email protected]> fix(nip17): Release ephemeral leases only after PostBox drains all relays - Switch from OnFlush.once to OnFlush.all for proper completion detection - Release leases only when remaining relays list is empty - Cancel PostBox retries after timeout to prevent "relay not found" errors - Removes unnecessary MainActor.run hop for relay connection checks Co-Authored-By: Claude Opus 4.5 <[email protected]> fix(nip17): Add force_cancel_send to stop PostBox retries after timeout - Add PostBox.force_cancel_send() that removes events regardless of delay - Timeout cleanup now properly stops retries when leases are released - Add debug logging when no pending entry exists to cancel This fixes the issue where cancel_send only worked for delayed events, leaving immediate sends retrying to removed relays indefinitely. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
UI improvements for NIP-17 DMs: - Add encryption protocol indicator in DM toolbar - Shield icon for NIP-17 (private), lock icon for NIP-04 (legacy) - Fix empty DM bubble when render_blocks returns empty - Update Xcode project for new NIP17 files Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]> Changelog-Added: Added NIP-04/NIP-17 encryption indicator in DM UI
Add unit tests for NIP-17 implementation: - testCreateMessageReturnsTwoWraps: Verify recipient + sender wraps - testGiftWrapUsesEphemeralKey: Verify ephemeral key usage - testTimestampRandomization: Verify temporal privacy (0-2 days) - testWrapUnwrapRoundTrip: Full encryption/decryption cycle - testSenderCanUnwrapSelfWrap: Cross-device recovery - testWrongRecipientCannotUnwrap: Security validation - testUnwrapRejectsNonGiftWrap: Input validation - Additional edge case and security tests Co-Authored-By: Claude Opus 4.5 <[email protected]> test(nip17): Add DM relay list and error path tests Add additional tests for NIP-17: - testCreateDMRelayList: Verify kind 10050 event creation - testParseDMRelayList: Verify relay URL extraction - testParseDMRelayListEmpty: Handle empty relay list - testParseDMRelayListWrongKind: Reject wrong event kind - testUnwrapEmptyContentFails: Handle empty gift wrap content - testUnwrapMalformedContentFails: Handle invalid encryption - testCreateMessageEmptyContent: Allow empty message content Note: Integration tests for ensureConnected() require network mocking infrastructure and are documented but not implemented. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
- Move keypair generation off main thread in NIP17.swift using Task.detached - Gate debug prints with #if DEBUG in DamusState.swift, truncate pubkey - Wrap addKey with withNdb for thread safety in Ndb.swift - Update NIP17Tests to async for new createMessage signature Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
P1 Memory Bugs: - nostrdb.c:6357: Check malloc result, free on strdup failure - nostrdb.c:6196: Check malloc result, free on strdup failure P2 Bugs: - nostrdb.c:2416: Fix duplicate quotes check, add total_reactions - metadata.c:99: Fix pointer arithmetic to use bytes not struct units - metadata.c:99: Assign size output parameter - metadata.c:342: Use sizeof(struct) not sizeof(pointer) - threadpool.h:85: Always dispatch to all threads, avoid short-circuit P3 Bugs: - nip44.h: Add missing stddef.h, stdint.h, sys/types.h includes - binmoji_table.h: Add EmojiHashEntry typedef fallback - base64.c: Use unsigned char to avoid sign-extension Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
HIGH PRIORITY: 1. Disable since optimization for giftwrap filters - NIP-17 randomizes timestamps up to 2 days past - Since optimization was filtering out new messages - SubscriptionManager.swift now skips optimization for kind:1059 2. Subscribe to own 10050 relays on login - Added subscribeToOwnDMRelays() in DamusState.swift - Fetches user's kind:10050, connects to those relays - Streams giftwraps indefinitely with no since optimization - Called after network connects in ContentView.swift MEDIUM PRIORITY: 3. Enforce seal/rumor pubkey match (security) - NIP17.swift now rejects messages where rumor.pubkey != seal.pubkey - Prevents sender spoofing attacks 4. Send self-wrap to sender's 10050 relays - DMChatView.swift now sends self-wrap to own DM relays - Enables cross-device message recovery TESTS: - Complete outbound DM flow test - Complete inbound DM flow test - DM relay list roundtrip tests - Unhappy path tests (wrong recipient, replay attacks) - Bidirectional conversation test - Security test for pubkey verification Closes damus-jps Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]> Changelog-Fixed: Fixed inbound NIP-17 DM reception
Technical problem statement for external review of why jb55's DM sent via notedeck isn't being received in Damus iOS. Key findings: - Relay overlap EXISTS (nos.lol, nostr.wine, relay.damus.io) - Other NIP-17 DMs work (0xchat, nospeak) - Most likely cause: `since` optimization + timestamp randomization - AUTH requirements ruled out (nos.lol, relay.damus.io don't require) Includes focused tests and questions for notedeck team. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
Adds a "DM Relays" tab to Settings -> Relays allowing users to: - View their current DM relay list (kind:10050) - Add/remove DM relays - Set up default relays for new users - Publish updated 10050 to network This enables NIP-17 interoperability by letting users configure where they receive encrypted DMs from other NIP-17 compatible apps. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]> Changelog-Added: Added DM Relays settings tab for kind:10050 relay management
Adds isSending state to prevent rapid double-taps during NIP-17 message creation. The send button now: - Shows a spinner while sending - Is disabled during the async operation - Clears draft immediately for responsive feel Note: We don't create a local display event for immediate UI feedback because the self-wrap returns from DM relays quickly with the correct rumor ID. Creating a separate display event caused duplicate messages. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]> Changelog-Fixed: Fixed duplicate DM sends
Adds tests for: - Seal signature validation documentation - Special character content handling (newlines, tabs, quotes, null bytes) - p-tag recipient identification in rumor - Self-to-self message edge case - Cross-client interop test vector template (TODO: add real vectors) Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]>
Signed-off-by: alltheseas <[email protected]>
Signed-off-by: alltheseas <[email protected]>
Signed-off-by: alltheseas <[email protected]> Changelog-Changed: Scaled event fonts for Dynamic Type
Implement NIP-17 Private Direct Messages with end-to-end encryption using gift wrap protocol (NIP-59). Code cleanup: - Wrap all debug prints in #if DEBUG guards - Consolidate duplicate fetchLatestDMRelayListEvent and fetchLatestNIP65RelayListEvent into single fetchLatestEvent helper - Flatten fetchDMRelayList nesting using guard/early returns - Remove redundant kindName variables in production builds Tested successfully with send & receive across: - Damus - Notedeck - 0xchat - Nospeak - Pollerama Closes damus-io#1737 Closes: damus-io#2815 Changelog-Added: NIP-17 Private Direct Messages with end-to-end encryption Changelog-Added: DM Relays settings for managing kind 10050 relay list Signed-off-by: alltheseas <[email protected]> Co-Authored-By: Claude Opus 4.5 <[email protected]>
Per lead dev feedback, nostrdb must use submodule instead of vendored code. Changes: - Remove vendored nostrdb C code (231 files) - Add nostrdb as git submodule from https://github.com/damus-io/nostrdb - Move Swift bindings to damus/Bindings/nostrdb/ - Update Xcode project to reference: - Swift bindings from damus/Bindings/nostrdb/ - C code from nostrdb submodule (using SOURCE_ROOT) This maintains clean separation: Swift wrappers in damus repo, upstream C code in submodule. Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Signed-off-by: alltheseas <[email protected]> Changelog-Changed: Converted nostrdb from vendored code to git submodule Closes: damus-io#3666
The prior commit (98bbeca) converted nostrdb from vendored code to a git submodule but left several path references and API mismatches that prevented compilation. This fixes all of them: - Update pbxproj paths for secp256k1, flatcc, and lmdb files to match the submodule's directory layout (deps/secp256k1/include/, etc.) - Update HEADER_SEARCH_PATHS to mirror nostrdb Makefile's -I flags - Add local copies of NdbProfile.swift and NdbMeta.swift without the import FlatBuffers line (FlatBuffers is compiled as source, not module) - Fix OpaquePointer → UnsafeMutablePointer<ndb_blocks> in NdbBlock.swift - Fix nostr_bech32_t → nostr_bech32 type name in Bech32Object.swift - Remove has_kind/kind field references not present in nostrdb 4a14400 - Remove dead bech32_util.c from build phases (not in nostrdb Makefile) - Restore damus-c/parser.h (cursor parser needed by wasm.c) - Fix #if DEBUG / else preprocessor split in HomeModel.swift Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: alltheseas <[email protected]>
The nostrdb C parser defines TLV_KIND but never parses it — the bech32_nevent and bech32_naddr structs lack a kind field across all commits and branches. This caused decoded nevent/naddr to silently drop the kind, breaking round-trip encoding and naddr resolution (e.g. kind 30023 long-form articles would resolve as kind 0). Add a Swift-side TLV kind extractor that re-decodes the bech32 data and scans for TLV type 3 after the C parse completes. This is a workaround until nostrdb adds kind support upstream. Upstream issue: damus-io/nostrdb#126 Also untrack .beads/metadata.json (local tooling artifact). Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: alltheseas <[email protected]> Changelog-Fixed: Fixed bech32 kind decoding for nevent/naddr entities
The init(block:) path for inline bech32 mentions (used by NoteContent for rendered mentions) was still hardcoding kind to nil/0. This could cause naddr resolution to fail for addressable events like kind 30023 long-form articles. Extract the raw bech32 string from block.str and apply the same Swift-side TLV kind fallback used in parse(). Also add .beads/ to .gitignore to prevent accidental re-tracking of local tooling artifacts. Ref: damus-io/nostrdb#126 Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: alltheseas <[email protected]>
# Conflicts: # damus.xcodeproj/project.pbxproj # damus/Bindings/nostrdb/NdbDatabase+UI.swift # nostrdb/src/nostrdb.c # nostrdb~HEAD
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
alltheseas
force-pushed
the
feat/nip17-private-dms
branch
from
e8e3b6d to
486513f
Compare
…d database compaction Replace ~88 individual C file build entries in the Xcode project with a single SPM local package reference to nostrdb. This simplifies the build configuration and makes it easier to update the nostrdb dependency. Move Swift bindings from damus/Bindings/nostrdb/ to damus/Nostrdb/. Add database compaction feature: Ndb.compact() calls ndb_compact() to create a compacted copy keeping all profiles and the user's own notes. The compacted DB is swapped in atomically at next app launch. UI added in Storage settings. Signed-off-by: alltheseas <[email protected]> Changelog-Changed: Switched nostrdb build from Xcode pbxproj to SPM local package Changelog-Added: Added database compaction UI in storage settings
Nostrdb's nip44.c uses libsodium (crypto_stream_chacha20_ietf_xor_ic). The Clibsodium xcframework should be declared as a dependency of the Nostrdb SPM package rather than linked separately by each app target. Note: The pre-existing modulemap conflict between nostr_sdkFFI.xcframework and Clibsodium.xcframework (both output include/module.modulemap) still prevents xcodebuild test from succeeding. This is an upstream issue requiring unique modulemap paths in one of the xcframeworks. Signed-off-by: alltheseas <[email protected]>
|
Great to see NIP-17 support being worked on! We're building divine.video and planning to focus on NIP-17 for DMs — so we're really hoping to see all the major clients adopt it. The NIP-04 → NIP-17 migration is one of the most important interoperability gaps in Nostr right now. Primal still only supports NIP-04 with no NIP-17 work in progress, and their open issues (android #470, iOS #190) show users hitting DM delivery failures with clients like Amethyst and 0xchat that default to NIP-17. Damus shipping NIP-17 would put more pressure on the remaining holdouts and improve the DM experience across the whole ecosystem. It's also great that Notedeck already has NIP-17 natively — strong signal of where things are headed. A few thoughts on the PR itself:
Looking forward to seeing this ship! 🤙 |
Collaborator
Author
|
@rabble interop DM thread is here: nostrability/nostrability#169 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NIP-17 encrypted private direct messages with NIP-44 encryption and NIP-59 gift wraps, nostrdb submodule conversion using SPM local package (inspired by jb55's
compactbranch), and database compaction UI.NIP-17 DMs:
nostrdb submodule + SPM local package (Closes #3666):
alltheseas/nostrdb@spm(based on nostrdb compact branch tip4e5af99+Package.swift)XCLocalSwiftPackageReferenceto the nostrdb SPM packagePackage.swiftcreated for nostrdb (not yet upstream) — compiles all C sources as one SPM target with proper header search paths and preprocessor definesdamus/Bindings/nostrdb/→damus/Nostrdb/Database compaction (from jb55's
compactbranch):Ndb.compact()callsndb_compact()to create a compacted database keeping all profiles and the user's own notesNdb.try_swap_compacted_db()atomically swaps the compacted DB into place at app startupcompactbranch (origin/compact, commita4afb16), adapted to work with the SPM package approachChecklist
Standard PR Checklist
compactbranch (SPM approach + compaction)Closes:orFixes:tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patchesTest report
Device: iPhone 17 Simulator
iOS: 26.2
Damus:
3f0d9e93(HEAD of this PR)Build verification:
Still needs testing:
Note: Unit tests (
xcodebuild test) are blocked by a pre-existing SPM conflict (duplicatemodule.modulemapoutput fromnostr-sdk-swiftvsswift-sodium). This exists on current master and is unrelated to this PR.Other notes
Package.swiftin nostrdb should be upstreamed todamus-io/nostrdb. Once that happens,.gitmodulesURL can switch back todamus-io/nostrdb.extractTLVKind(fromBech32:)) is documented with references to nostrdb#126 and can be removed once nostrdb addskindtobech32_nevent/bech32_naddrstructs upstream.bech32_util.candnode_id.care excluded from the SPM package — they have unresolvable ccan header dependencies (talstr.h,array_size.h) and are not in nostrdb's Makefile SRCS.error.cis included (needed bywasm.c)..beads/added to.gitignoreandmetadata.jsonuntracked to prevent local tooling artifacts in repo.