[PATCH damus v1] Mute reposts of muted events

[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

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 | 5 +++++
nostrdb/NdbNote+.swift | 9 ++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/damus/Models/MutelistManager.swift b/damus/Models/MutelistManager.swift
index 1096bdf9..bbc91bcc 100644
--- a/damus/Models/MutelistManager.swift
+++ b/damus/Models/MutelistManager.swift
@@ -187,6 +187,11 @@ class MutelistManager {
}
}
}
+
+ // Check if event is a repost of a muted event
+ if let inner_event = ev.get_inner_event() {
+ return self.compute_event_muted_reason(inner_event)
+ }

return nil
}
diff --git a/nostrdb/NdbNote+.swift b/nostrdb/NdbNote+.swift
index 79b2d4c7..8ea84b15 100644
--- a/nostrdb/NdbNote+.swift
+++ b/nostrdb/NdbNote+.swift
@@ -25,7 +25,14 @@ extension NdbNote {

return nil
}
-
+
+ /// Gets the inner event if this event is a repost (boost)
+ ///
+ /// DEPRECATED! please use `get_inner_event() instead`
+ ///
+ /// - Parameter cache: The Event cache to use
+ /// - Returns: The inner event, if available
+ @available(*, deprecated)
func get_inner_event(cache: EventCache) -> NdbNote? {
if let ev = get_cached_inner_event(cache: cache) {
return ev

base-commit: 46185c55d1dcc376842ca26e6ba47caa99a55b66
--
2.44.0

[William Casarin]

get_inner_event is very slow, and this is already a potentially very
slow function when you have lots of muted words. we should be using the
cached version:

ev.get_inner_event(event_cache)

This function probably shouldn't even exist tbh since it's such a
performance footgun, we should be processing inner events ahead of time
in nostrdb.

This also opens us up to a potentially DoS attack with many inner events
since its recursive. Ideally we do it to a depth of 1 with the cached
variant.

>
> return nil
> }
>diff --git a/nostrdb/NdbNote+.swift b/nostrdb/NdbNote+.swift
>index 79b2d4c7..8ea84b15 100644
>--- a/nostrdb/NdbNote+.swift
>+++ b/nostrdb/NdbNote+.swift
>@@ -25,7 +25,14 @@ extension NdbNote {
>
> return nil
> }
>-
>+
>+ /// Gets the inner event if this event is a repost (boost)
>+ ///
>+ /// DEPRECATED! please use `get_inner_event() instead`
>+ ///
>+ /// - Parameter cache: The Event cache to use
>+ /// - Returns: The inner event, if available
>+ @available(*, deprecated)

we should not be deprecating this function. we should be deprecating the
other function ...

This was added because the entire app was freezing due to the mute
changes.

[Daniel D'Aquino]

I see, thank you for pointing it out! I thought the one with the event_cache was the old one because of this TODO comment:

class EventCache {

    // TODO: remove me and change code to use ndb directly

    private let ndb: Ndb

    private var events: [NoteId: NostrEvent] = [:]

I thought that would be deprecated to use Ndb so I initially wrote a get_inner_event function that uses Ndb for lookup. Then I noticed the `get_inner_event()` and thought that was the standard function to be used.

Which approach would you prefer:

1. Recover that new `get_inner_event(ndb: Ndb)` function and mark `ev.get_inner_event(event_cache)` as deprecated? Or,
2. Use `ev.get_inner_event(event_cache)` on MutelistManager as-is?

This also opens us up to a potentially DoS attack with many inner events
since its recursive. Ideally we do it to a depth of 1 with the cached
variant.

Good point, I will implement that depth limit on the function

       return nil
   }
diff --git a/nostrdb/NdbNote+.swift b/nostrdb/NdbNote+.swift
index 79b2d4c7..8ea84b15 100644
--- a/nostrdb/NdbNote+.swift
+++ b/nostrdb/NdbNote+.swift
@@ -25,7 +25,14 @@ extension NdbNote {

       return nil
   }
-
+
+    /// Gets the inner event if this event is a repost (boost)
+    ///
+    /// DEPRECATED! please use `get_inner_event() instead`
+    ///
+    /// - Parameter cache: The Event cache to use
+    /// - Returns: The inner event, if available
+    @available(*, deprecated)

we should not be deprecating this function. we should be deprecating the
other function ...

Sounds good, I will mark the other function as deprecated

[Daniel D'Aquino]

On May 15, 2024, at 12:21, Daniel D'Aquino <dan...@daquino.me> wrote:

On May 14, 2024, at 08:05, William Casarin <jb...@jb55.com> wrote:

I see, thank you for pointing it out! I thought the one with the event_cache was the old one because of this TODO comment:

class EventCache {

    // TODO: remove me and change code to use ndb directly

    private let ndb: Ndb

    private var events: [NoteId: NostrEvent] = [:]

I thought that would be deprecated to use Ndb so I initially wrote a get_inner_event function that uses Ndb for lookup. Then I noticed the `get_inner_event()` and thought that was the standard function to be used.

Which approach would you prefer:

1. Recover that new `get_inner_event(ndb: Ndb)` function and mark `ev.get_inner_event(event_cache)` as deprecated? Or,
2. Use `ev.get_inner_event(event_cache)` on MutelistManager as-is?

I just noticed we cannot use event_cache in MutelistManager without substantial refactoring. The push notification extension target depends on MutelistManager. If we add the EventCache dependency, we will need to do substantial refactoring to make sure it is included on the Push notification extension target. I remember that was a rabbit hole.

I will try approach 1

This also opens us up to a potentially DoS attack with many inner events
since its recursive. Ideally we do it to a depth of 1 with the cached
variant.

Good point, I will implement that depth limit on the function

       return nil
   }
diff --git a/nostrdb/NdbNote+.swift b/nostrdb/NdbNote+.swift
index 79b2d4c7..8ea84b15 100644
--- a/nostrdb/NdbNote+.swift
+++ b/nostrdb/NdbNote+.swift
@@ -25,7 +25,14 @@ extension NdbNote {

       return nil
   }
-
+
+    /// Gets the inner event if this event is a repost (boost)
+    ///
+    /// DEPRECATED! please use `get_inner_event() instead`
+    ///
+    /// - Parameter cache: The Event cache to use
+    /// - Returns: The inner event, if available
+    @available(*, deprecated)

we should not be deprecating this function. we should be deprecating the
other function ...

Sounds good, I will mark the other function as deprecated

[Daniel D'Aquino]

On May 24, 2024, at 15:00, Daniel D'Aquino <dan...@daquino.me> wrote:

On May 15, 2024, at 12:21, Daniel D'Aquino <dan...@daquino.me> wrote:

On May 14, 2024, at 08:05, William Casarin <jb...@jb55.com> wrote:

I see, thank you for pointing it out! I thought the one with the event_cache was the old one because of this TODO comment:

class EventCache {

    // TODO: remove me and change code to use ndb directly

    private let ndb: Ndb

    private var events: [NoteId: NostrEvent] = [:]

I thought that would be deprecated to use Ndb so I initially wrote a get_inner_event function that uses Ndb for lookup. Then I noticed the `get_inner_event()` and thought that was the standard function to be used.

Which approach would you prefer:

1. Recover that new `get_inner_event(ndb: Ndb)` function and mark `ev.get_inner_event(event_cache)` as deprecated? Or,
2. Use `ev.get_inner_event(event_cache)` on MutelistManager as-is?

I just noticed we cannot use event_cache in MutelistManager without substantial refactoring. The push notification extension target depends on MutelistManager. If we add the EventCache dependency, we will need to do substantial refactoring to make sure it is included on the Push notification extension target. I remember that was a rabbit hole.

I will try approach 1

This also opens us up to a potentially DoS attack with many inner events
since its recursive. Ideally we do it to a depth of 1 with the cached
variant.

Good point, I will implement that depth limit on the function

       return nil
   }
diff --git a/nostrdb/NdbNote+.swift b/nostrdb/NdbNote+.swift
index 79b2d4c7..8ea84b15 100644
--- a/nostrdb/NdbNote+.swift
+++ b/nostrdb/NdbNote+.swift
@@ -25,7 +25,14 @@ extension NdbNote {

       return nil
   }
-
+
+    /// Gets the inner event if this event is a repost (boost)
+    ///
+    /// DEPRECATED! please use `get_inner_event() instead`
+    ///
+    /// - Parameter cache: The Event cache to use
+    /// - Returns: The inner event, if available
+    @available(*, deprecated)

we should not be deprecating this function. we should be deprecating the
other function ...

Sounds good, I will mark the other function as deprecated

This was added because the entire app was freezing due to the mute
changes.

   func get_inner_event(cache: EventCache) -> NdbNote? {
       if let ev = get_cached_inner_event(cache: cache) {
           return ev

base-commit: 46185c55d1dcc376842ca26e6ba47caa99a55b66
--
2.44.0

Sent updated patch!