Feature/delete location note#686
Feature/delete location note#686AleksPlekhov wants to merge 3 commits intopermissionlesstech:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a3ca26451
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| val filter = NostrFilter( | ||
| kinds = listOf(NostrKind.TEXT_NOTE, NostrKind.DELETION), | ||
| tagFilters = mapOf("g" to listOf(gh)), | ||
| limit = 200 |
There was a problem hiding this comment.
Stop filtering kind:5 deletions on a geohash tag
This subscription now requests kind 5 events only when they also carry #g=$gh, but NostrProtocol.createDeletionEvent() builds deletion events with just an e tag (app/src/main/java/com/bitchat/android/nostr/NostrProtocol.kt:125-136). Because NostrRelayManager re-applies filter.matches() before dispatching handlers, every deletion event is dropped before handleEvent() can remove anything. In practice, deletes only look successful locally; other devices, and the same device after a refresh, will load the original note again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. NostrFilter.matches() requires all tag filters to pass - a kind:5 event carries e tags, not g tags, so it would always fail the geohash filter and get dropped silently. Fixed by splitting into two subscriptions: kind:1 keeps the #g filter, and kind:5 gets its own subscription with #e = the loaded note IDs. The deletion subscription is opened after the initial batch loads and refreshed (throttled to once per 500ms) as new notes stream in.
| val before = _notes.value | ||
| val removed = before.filter { it.id in targetIds && it.pubkey == event.pubkey } |
There was a problem hiding this comment.
Require a valid signature before honoring a deletion
The new kind-5 path authorizes removal purely by comparing event.pubkey with the stored note author, but it never verifies that the deletion event was actually signed by that pubkey. NostrEvent.isValidSignature() exists, and NostrRelayManager forwards matching events without doing that check first, so any malicious relay or client can forge a kind 5 event with someone else's pubkey and make this handler hide that user's note.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Without this check, any relay could forge {"kind":5, "pubkey":"", "tags":[["e","<note_id>"]]} and silently hide someone else's notes. Fixed by calling event.isValidSignature() at the top of the kind:5
Description
Implements the ability for users to delete their own location notes via a long-press gesture.
Closes: #639
User flow:
User long-presses a note in the Location Notes sheet → haptic feedback fires + a bottom action drawer appears
Only notes authored by the local user show the "delete note" action (ownership check via pubkey)
Tapping "delete note" sends a NIP-09 kind:5 deletion event to geo-specific relays and removes the note from the local list immediately (optimistic UI)
Remote deletion events received from relays are also processed, keeping the list consistent across devices
Changes:
NostrKind.DELETION = 5 constant (NIP-09)
NostrProtocol.createDeletionEvent() — builds and signs a kind:5 event
LocationNotesManager — deleteNote(), localPubkey StateFlow, subscription filter extended to include kind:5 for real-time remote deletion sync
LocationNotesSheet — long-press with detectTapGestures + HapticFeedbackType.LongPress, NoteActionsSheet bottom drawer
LocationNotesSheetPresenter — ownership-gated NoteActionsSheet rendering
Checklist
[x] I have read the contribution guidelines
[x] I have performed a self-review of my code
[x] I have mentioned the corresponding issue and the relevant keyword (e.g., "Closes: #xy") in the description
[x] If it is a core feature, I have added automated tests