[PATCH damus v3 0/2] Mute reposts of muted events

0 views
Skip to first unread message

Daniel D’Aquino

unread,
May 24, 2024, 9:28:33 PMMay 24
to pat...@damus.io, Daniel D’Aquino
Hi Will,

I made the changes you requested.

PATCH CHANGELOG:
- V1: Initial implementation using `get_inner_event()`
- V2: Use `ndb` queries to get the inner event for improved efficiency
- V3: Use unowned reference for recursive mute computation to improve efficiency; added depth limit to avoid long computation times

Notes:
- I added a depth limit. Although cryptographically we cannot get a self-referencing NdbNote, we are using an unowned value, and we should avoid long computations as they increase likelihood of a crash
- I retested changes to make sure they still work as expected
- I added this to our CI pipeline and made sure everything is passing. https://appstoreconnect.apple.com/teams/69a6de74-1a7a-47e3-e053-5b8c7c11a4d1/apps/1628663131/ci/groups/69b2d285-3915-4502-8634-56104dc897e2

Please let me know if you have any other feedback.

Thank you,
Daniel

Daniel D’Aquino (2):
Add ndb to MutelistManager
Mute reposts of muted events

.../NotificationExtensionState.swift | 2 +-
damus/ContentView.swift | 2 +-
damus/Models/DamusState.swift | 2 +-
damus/Models/MutelistManager.swift | 13 +++++++++++--
damus/TestData.swift | 2 +-
damusTests/Mocking/MockDamusState.swift | 2 +-
nostrdb/NdbNote.swift | 13 +++++++++++++
7 files changed, 29 insertions(+), 7 deletions(-)


base-commit: 0d9954290a674e1520164c08050bcfb9291fdd05
--
2.44.0


Daniel D’Aquino

unread,
May 24, 2024, 9:28:37 PMMay 24
to pat...@damus.io, Daniel D’Aquino
Signed-off-by: Daniel D’Aquino <dan...@daquino.me>
---
DamusNotificationService/NotificationExtensionState.swift | 2 +-
damus/ContentView.swift | 2 +-
damus/Models/DamusState.swift | 2 +-
damus/Models/MutelistManager.swift | 4 +++-
damus/TestData.swift | 2 +-
damusTests/Mocking/MockDamusState.swift | 2 +-
6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/DamusNotificationService/NotificationExtensionState.swift b/DamusNotificationService/NotificationExtensionState.swift
index 0cfb8e6a..f7daa5b5 100644
--- a/DamusNotificationService/NotificationExtensionState.swift
+++ b/DamusNotificationService/NotificationExtensionState.swift
@@ -28,7 +28,7 @@ struct NotificationExtensionState: HeadlessDamusState {
self.settings = UserSettingsStore()

self.contacts = Contacts(our_pubkey: keypair.pubkey)
- self.mutelist_manager = MutelistManager(user_keypair: keypair)
+ self.mutelist_manager = MutelistManager(user_keypair: keypair, ndb: self.ndb)
self.keypair = keypair
self.profiles = Profiles(ndb: ndb)
self.zaps = Zaps(our_pubkey: keypair.pubkey)
diff --git a/damus/ContentView.swift b/damus/ContentView.swift
index b3846a57..a70bf421 100644
--- a/damus/ContentView.swift
+++ b/damus/ContentView.swift
@@ -699,7 +699,7 @@ struct ContentView: View {
likes: EventCounter(our_pubkey: pubkey),
boosts: EventCounter(our_pubkey: pubkey),
contacts: Contacts(our_pubkey: pubkey),
- mutelist_manager: MutelistManager(user_keypair: keypair),
+ mutelist_manager: MutelistManager(user_keypair: keypair, ndb: ndb),
profiles: Profiles(ndb: ndb),
dms: home.dms,
previews: PreviewCache(),
diff --git a/damus/Models/DamusState.swift b/damus/Models/DamusState.swift
index 13679e45..4cc7de80 100644
--- a/damus/Models/DamusState.swift
+++ b/damus/Models/DamusState.swift
@@ -112,7 +112,7 @@ class DamusState: HeadlessDamusState {
likes: EventCounter(our_pubkey: empty_pub),
boosts: EventCounter(our_pubkey: empty_pub),
contacts: Contacts(our_pubkey: empty_pub),
- mutelist_manager: MutelistManager(user_keypair: kp),
+ mutelist_manager: MutelistManager(user_keypair: kp, ndb: .empty),
profiles: Profiles(ndb: .empty),
dms: DirectMessagesModel(our_pubkey: empty_pub),
previews: PreviewCache(),
diff --git a/damus/Models/MutelistManager.swift b/damus/Models/MutelistManager.swift
index 1096bdf9..8a444b0a 100644
--- a/damus/Models/MutelistManager.swift
+++ b/damus/Models/MutelistManager.swift
@@ -9,6 +9,7 @@ import Foundation

class MutelistManager {
let user_keypair: Keypair
+ let ndb: Ndb
private(set) var event: NostrEvent? = nil

var users: Set<MuteItem> = [] {
@@ -26,8 +27,9 @@ class MutelistManager {

var muted_notes_cache: [NoteId: EventMuteStatus] = [:]

- init(user_keypair: Keypair) {
+ init(user_keypair: Keypair, ndb: Ndb) {
self.user_keypair = user_keypair
+ self.ndb = ndb
}

func refresh_sets() {
diff --git a/damus/TestData.swift b/damus/TestData.swift
index d7e9c977..8b931ef2 100644
--- a/damus/TestData.swift
+++ b/damus/TestData.swift
@@ -73,7 +73,7 @@ var test_damus_state: DamusState = ({
likes: .init(our_pubkey: our_pubkey),
boosts: .init(our_pubkey: our_pubkey),
contacts: .init(our_pubkey: our_pubkey),
- mutelist_manager: MutelistManager(user_keypair: test_keypair),
+ mutelist_manager: MutelistManager(user_keypair: test_keypair, ndb: ndb),
profiles: .init(ndb: ndb),
dms: .init(our_pubkey: our_pubkey),
previews: .init(),
diff --git a/damusTests/Mocking/MockDamusState.swift b/damusTests/Mocking/MockDamusState.swift
index e554f2fe..fe1094dd 100644
--- a/damusTests/Mocking/MockDamusState.swift
+++ b/damusTests/Mocking/MockDamusState.swift
@@ -25,7 +25,7 @@ func generate_test_damus_state(
return profiles
}()

- let mutelist_manager = MutelistManager(user_keypair: test_keypair)
+ let mutelist_manager = MutelistManager(user_keypair: test_keypair, ndb: ndb)
let damus = DamusState(pool: pool,
keypair: test_keypair,
likes: .init(our_pubkey: our_pubkey),
--
2.44.0


Daniel D’Aquino

unread,
May 24, 2024, 9:28:47 PMMay 24
to pat...@damus.io, Daniel D’Aquino
Testing
--------

PASS

Device: iPhone 15 simulator
iOS: 17.4
Damus: This commit
Steps:
1. Mute a specific user "A" from account "B"
2. Make account "B" follow account "C"
3. Using a separate account "C", repost a note from account "A"
4. Make sure that the reposted note from step 3 does not appear on account "B"'s timeline
5. Make sure other reposts of other users still show normally

Other testing: Ran all automated tests, and they are passing.

Closes: https://github.com/damus-io/damus/issues/1934
Changelog-Changed: Do not show reposts of muted events
Signed-off-by: Daniel D’Aquino <dan...@daquino.me>
---
damus/Models/MutelistManager.swift | 9 ++++++++-
nostrdb/NdbNote.swift | 13 +++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/damus/Models/MutelistManager.swift b/damus/Models/MutelistManager.swift
index 8a444b0a..9fb1aa19 100644
--- a/damus/Models/MutelistManager.swift
+++ b/damus/Models/MutelistManager.swift
@@ -152,8 +152,9 @@ class MutelistManager {
/// Check if an event is muted given a collection of ``MutedItem``.
///
/// - Parameter ev: The ``NostrEvent`` that you want to check the muted reason for.
+ /// - Parameter max_recursive_depth: (Optional) The maximum depth for any recursive checks. We use unowned values in computation, so low values are recommended to avoid stability issues.
/// - Returns: The ``MuteItem`` that matched the event. Or `nil` if the event is not muted.
- func compute_event_muted_reason(_ ev: NostrEvent) -> MuteItem? {
+ func compute_event_muted_reason(_ ev: NostrEvent, max_recursive_depth: Int = 1) -> MuteItem? {
// Events from the current user should not be muted.
guard self.user_keypair.pubkey != ev.pubkey else { return nil }

@@ -189,6 +190,12 @@ class MutelistManager {
}
}
}
+
+ // Check if event is a repost of a muted event
+ if max_recursive_depth > 0, let inner_event = ev.get_inner_event(ndb: self.ndb) {
+ // We are only using the note for a short period to perform a calculation, so it is likely ok to use unowned value
+ return self.compute_event_muted_reason(inner_event.unsafeUnownedValue, max_recursive_depth: max_recursive_depth - 1)
+ }

return nil
}
diff --git a/nostrdb/NdbNote.swift b/nostrdb/NdbNote.swift
index ce2f358f..87431d76 100644
--- a/nostrdb/NdbNote.swift
+++ b/nostrdb/NdbNote.swift
@@ -272,6 +272,19 @@ class NdbNote: Encodable, Equatable, Hashable {
func get_inner_event() -> NdbNote? {
return self.inner_event
}
+
+ func get_inner_event(ndb: Ndb) -> NdbTxn<NdbNote>? {
+ guard self.known_kind == .boost else {
+ return nil
+ }
+
+ if let id = self.referenced_ids.first {
+ guard let note_key = ndb.lookup_note_key(id) else { return nil }
+ return ndb.lookup_note_by_key(note_key)?.collect()
+ }
+
+ return nil
+ }
}

// Extension to make NdbNote compatible with NostrEvent's original API
--
2.44.0


William Casarin

unread,
May 27, 2024, 1:37:58 PMMay 27
to Daniel D’Aquino, pat...@damus.io
On Sat, May 25, 2024 at 01:28:38AM GMT, Daniel D’Aquino wrote:
>Testing
>--------
>
>PASS
>
>Device: iPhone 15 simulator
>iOS: 17.4
>Damus: This commit
>Steps:
>1. Mute a specific user "A" from account "B"
>2. Make account "B" follow account "C"
>3. Using a separate account "C", repost a note from account "A"
>4. Make sure that the reposted note from step 3 does not appear on account "B"'s timeline
>5. Make sure other reposts of other users still show normally
>
>Other testing: Ran all automated tests, and they are passing.
>
>Closes: https://github.com/damus-io/damus/issues/1934
>Changelog-Changed: Do not show reposts of muted events
>Signed-off-by: Daniel D’Aquino <dan...@daquino.me>
>---
> [.. ]
> damus/Models/MutelistManager.swift | 9 +
>diff --git a/nostrdb/NdbNote.swift b/nostrdb/NdbNote.swift
>index ce2f358f..87431d76 100644
>--- a/nostrdb/NdbNote.swift
>+++ b/nostrdb/NdbNote.swift
>@@ -272,6 +272,19 @@ class NdbNote: Encodable, Equatable, Hashable {
> func get_inner_event() -> NdbNote? {
> return self.inner_event
> }
>+
>+ func get_inner_event(ndb: Ndb) -> NdbTxn<NdbNote>? {
>+ guard self.known_kind == .boost else {
>+ return nil
>+ }
>+
>+ if let id = self.referenced_ids.first {
>+ guard let note_key = ndb.lookup_note_key(id) else { return nil }
>+ return ndb.lookup_note_by_key(note_key)?.collect()
>+ }
>+
>+ return nil
>+ }

Inner event means something specific: is there a json note in the
content. This function doesn't seem to be looking there at all. The name
of the function is incorrect. It is not getting the innner event, it is
seeing if the inner event is in the local cache.

This is why I originally called this function get_cached_inner_event or
something like that, other than that this patch looks right!

Reviewed-by: William Casarin <jb...@jb55.com>
Reply all
Reply to author
Forward
0 new messages