search

damus-io / damus

iOS nostr client
GNU General Public License v3.0
1.95k stars 290 forks source link

Bug: cannot remove damus relay #2186

Closed alltheseas closed 2 weeks ago

alltheseas commented 3 weeks ago

What happens When I change to all tor relays, I cannot remove damus relay.

1.9 testflight

Needs recreation

https://damus.io/nevent1qqsq2lrv7cm347aejx36p2eusye0kqntm8fxx3krzerzr87tcz6ej8qpz9mhxue69uhkummnw3ezumr49e4k2qg5waehxw309ahx7um5wghrzvpswqhx7un8qy28wumn8ghj7un9d3shjtn90p5hgtnsw43qzyrhwden5te0xy6rqtnxxaazu6t0ldrazj

cc @danieldaquino

alltheseas commented 3 weeks ago

potentially related to https://github.com/nostrability/nostrability/issues/35

alltheseas commented 3 weeks ago

@jb55 advised

Ok looking at the code it does seem to always use it, but will promptly disconnect once you have your relay list.

We don’t even really need to do this anymore. We should be only connecting to the relays in the relays list in nostrdb. We should fix this

jb55 commented 3 weeks ago

confirmed this is an issue: it will always include the default bootstrap relay list in addition to the relays on your current list. This is the "safest" thing we can do, in case the relays on a users list are all broken.

This shouldn't really be a problem though once we're saving and loading the relay list from nostrdb.

So again, nostrdb fixes this. I am working on the nostrdb branch as we speak.

danieldaquino commented 2 weeks ago

So again, nostrdb fixes this. I am working on the nostrdb branch as we speak.

@jb55, @alltheseas , if this fix will depends on the NostrDB local relay model changes, would it be a good idea to re-target this ticket to 1.9+, (since those changes would be too risky to push to the App Store at the moment)?

alltheseas commented 2 weeks ago

So again, nostrdb fixes this. I am working on the nostrdb branch as we speak.

@jb55, @alltheseas , if this fix will depends on the NostrDB local relay model changes, would it be a good idea to re-target this ticket to 1.9+, (since those changes would be too risky to push to the App Store at the moment)?

This sounds like the right approach. Our nostrdb expert @jb55 to confirm 🤠

jb55 commented 2 weeks ago

On Mon, Apr 29, 2024 at 05:21:08PM -0700, Daniel D’Aquino wrote:

So again, nostrdb fixes this. I am working on the nostrdb branch as we speak.

@jb55, @alltheseas , if this fix will depends on the NostrDB local relay model changes, would it be a good idea to re-target this ticket to 1.9+, (since those changes would be too risky to push to the App Store at the moment)?

I don't think that's the case. as far as I understand it the main issue here is that the default bootstrap relays are always added to the bootstrap relay set.

danieldaquino commented 2 weeks ago

I don't think that's the case. as far as I understand it the main issue here is that the default bootstrap relays are always added to the bootstrap relay set.

Sounds good, thanks for confirming @jb55! I will start working on a fix for this right away in preparation for the v1.8 release

danieldaquino commented 2 weeks ago

@jb55 @alltheseas Sent the fix via email to patches@damus.io!

(https://groups.google.com/a/damus.io/g/patches/c/Y-iM18U3M6I)

Please let me know if there are any questions or concerns. Thank you!