Skip to content

Commit

Permalink
Fix relay URL trailing slash issues
Browse files Browse the repository at this point in the history 
This commit tries to replace all usage of `String` to represent relay
URLs and use `RelayURL` which automatically converts strings to a
canonical relay URL format that is more reliable and avoids issues related to
trailing slashes.

Test 1: Main issue fix
-----------------------

PASS

Device: iPhone 15 Simulator
iOS: 17.4
Damus: This commit
Steps:
1. Delete all connected relays
2. Add `wss://relay.damus.io/` (with the trailing slash) to the relay list
3. Try to post. Post should succeed. PASS
4. Try removing this newly added relay. Relay should be removed successfully. PASS

Test 2: Persistent relay list after upgrade
--------------------------------------------

PASS

Device: iPhone 15 Simulator
iOS: 17.4
Damus: 1.8 (1) `247f313b` + This commit
Steps:
1. Downgrade to old version
2. Add some relays to the list, some without a trailing slash, some with
3. Upgrade to this commit
4. All relays added in step 2 should still be there, and ones with a trailing slash should have been corrected to remove the trailing slash

Test 3: Miscellaneous regression tests
--------------------------------------

Device: iPhone 15 Simulator
iOS: 17.4
Damus: This commit
Coverage:
1. Posting works
2. Search works
3. Relay connection status works
4. Adding relays work
5. Removing relays work
6. Adding relay with trailing slashes works (it fixes itself to remove the trailing slash)
7. Adding relays with different paths works (e.g. wss://yabu.me/v1 and wss://yabu.me/v2)
8. Adding duplicate relay (but with trailing slash) gets rejected as expected
9. Relay details page works. All items on that view loads correctly
10. Relay logs work
11. Getting follower counts and seeing follow lists on profiles still work
12. Relay list changes persist after app restart
13. Notifications view still work
14. Copying the user's pubkey and profile link works
15. Share note + copy link button still works
16. Connecting NWC wallet works
17. One-tap zaps work
18. Onboarding works
19. Unit tests all passing

Closes: #2072
Changelog-Fixed: Fix bug that would cause connection issues with relays defined with a trailing slash URL, and an inability to delete them.
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
  • Loading branch information
@danieldaquino @jb55
danieldaquino authored and jb55 committed Mar 25, 2024
1 parent b189413 commit e951370
Show file tree
Hide file tree
Showing 46 changed files with 386 additions and 311 deletions.
4 changes: 4 additions & 0 deletions damus.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@
D723411A2B6864F200E1E135 /* DamusPurpleEnvironment.swift in Sources */ = {isa = PBXBuildFile; fileRef = D72341182B6864F200E1E135 /* DamusPurpleEnvironment.swift */; };
D723C38E2AB8D83400065664 /* ContentFilters.swift in Sources */ = {isa = PBXBuildFile; fileRef = D723C38D2AB8D83400065664 /* ContentFilters.swift */; };
D724D8272B64B40B00ABE789 /* DamusPurpleAccountView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D724D8262B64B40B00ABE789 /* DamusPurpleAccountView.swift */; };
D72927AD2BAB515C00F93E90 /* RelayURLTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D72927AC2BAB515C00F93E90 /* RelayURLTests.swift */; };
D72A2D022AD9C136002AFF62 /* EventViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D72A2CFF2AD9B66B002AFF62 /* EventViewTests.swift */; };
D72A2D052AD9C1B5002AFF62 /* MockDamusState.swift in Sources */ = {isa = PBXBuildFile; fileRef = D72A2D042AD9C1B5002AFF62 /* MockDamusState.swift */; };
D72A2D072AD9C1FB002AFF62 /* MockProfiles.swift in Sources */ = {isa = PBXBuildFile; fileRef = D72A2D062AD9C1FB002AFF62 /* MockProfiles.swift */; };
Expand Down Expand Up @@ -1375,6 +1376,7 @@
D72341182B6864F200E1E135 /* DamusPurpleEnvironment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurpleEnvironment.swift; sourceTree = "<group>"; };
D723C38D2AB8D83400065664 /* ContentFilters.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentFilters.swift; sourceTree = "<group>"; };
D724D8262B64B40B00ABE789 /* DamusPurpleAccountView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurpleAccountView.swift; sourceTree = "<group>"; };
D72927AC2BAB515C00F93E90 /* RelayURLTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayURLTests.swift; sourceTree = "<group>"; };
D72A2CFF2AD9B66B002AFF62 /* EventViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventViewTests.swift; sourceTree = "<group>"; };
D72A2D042AD9C1B5002AFF62 /* MockDamusState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockDamusState.swift; sourceTree = "<group>"; };
D72A2D062AD9C1FB002AFF62 /* MockProfiles.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProfiles.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2544,6 +2546,7 @@
E0E024102B7C19C20075735D /* TranslationTests.swift */,
E06336A92B75832100A88E6B /* ImageMetadataTest.swift */,
D7CBD1D52B8D509800BFD889 /* DamusPurpleImpendingExpirationTests.swift */,
D72927AC2BAB515C00F93E90 /* RelayURLTests.swift */,
);
path = damusTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -3503,6 +3506,7 @@
3A90B1832A4EA3C600000D94 /* UserSearchCacheTests.swift in Sources */,
4C9B0DEE2A65A75F00CBDA21 /* AttrStringTestExtensions.swift in Sources */,
4C19AE552A5D977400C90DB7 /* HashtagTests.swift in Sources */,
D72927AD2BAB515C00F93E90 /* RelayURLTests.swift in Sources */,
3A3040ED29A5CB86008A0F29 /* ReplyDescriptionTests.swift in Sources */,
D71DC1EC2A9129C3006E207C /* PostViewTests.swift in Sources */,
3AAC7A022A60FE72002B50DF /* LocalizationUtilTests.swift in Sources */,
Expand Down
20 changes: 9 additions & 11 deletions damus/ContentView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,15 +683,13 @@ struct ContentView: View {
UserSettingsStore.pubkey = pubkey
let settings = UserSettingsStore()
UserSettingsStore.shared = settings

let new_relay_filters = load_relay_filters(pubkey) == nil
for relay in bootstrap_relays {
if let url = RelayURL(relay) {
let descriptor = RelayDescriptor(url: url, info: .rw)
add_new_relay(model_cache: model_cache, relay_filters: relay_filters, pool: pool, descriptor: descriptor, new_relay_filters: new_relay_filters, logging_enabled: settings.developer_mode)
}
let descriptor = RelayDescriptor(url: relay, info: .rw)
add_new_relay(model_cache: model_cache, relay_filters: relay_filters, pool: pool, descriptor: descriptor, new_relay_filters: new_relay_filters, logging_enabled: settings.developer_mode)
}

pool.register_handler(sub_id: sub_id, handler: home.handle_event)

if let nwc_str = settings.nostr_wallet_connect,
Expand Down Expand Up @@ -888,13 +886,13 @@ func setup_notifications() {

struct FindEvent {
let type: FindEventType
let find_from: [String]?
static func profile(pubkey: Pubkey, find_from: [String]? = nil) -> FindEvent {
let find_from: [RelayURL]?

static func profile(pubkey: Pubkey, find_from: [RelayURL]? = nil) -> FindEvent {
return FindEvent(type: .profile(pubkey), find_from: find_from)
}
static func event(evid: NoteId, find_from: [String]? = nil) -> FindEvent {

static func event(evid: NoteId, find_from: [RelayURL]? = nil) -> FindEvent {
return FindEvent(type: .event(evid), find_from: find_from)
}
}
Expand Down
6 changes: 1 addition & 5 deletions damus/Models/Contacts+.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ func follow_user_event(our_contacts: NostrEvent?, keypair: FullKeypair, follow:
}


func decode_json_relays(_ content: String) -> [String: RelayInfo]? {
return decode_json(content)
}

func decode_json_relays(_ content: String) -> [RelayURL: RelayInfo]? {
return decode_json(content)
}
Expand Down Expand Up @@ -140,7 +136,7 @@ func make_contact_relays(_ relays: [RelayDescriptor]) -> [RelayURL: RelayInfo] {

func make_relay_metadata(relays: [RelayDescriptor], keypair: FullKeypair) -> NostrEvent? {
let tags = relays.compactMap { r -> [String]? in
var tag = ["r", r.url.id]
var tag = ["r", r.url.absoluteString]
if (r.info.read ?? true) != (r.info.write ?? true) {
tag += r.info.read == true ? ["read"] : ["write"]
}
Expand Down
4 changes: 2 additions & 2 deletions damus/Models/DamusState.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DamusState: HeadlessDamusState {
let events: EventCache
let bookmarks: BookmarksManager
let postbox: PostBox
let bootstrap_relays: [String]
let bootstrap_relays: [RelayURL]
let replies: ReplyCounter
let wallet: WalletModel
let nav: NavigationCoordinator
Expand All @@ -37,7 +37,7 @@ class DamusState: HeadlessDamusState {
let ndb: Ndb
var purple: DamusPurple

init(pool: RelayPool, keypair: Keypair, likes: EventCounter, boosts: EventCounter, contacts: Contacts, mutelist_manager: MutelistManager, profiles: Profiles, dms: DirectMessagesModel, previews: PreviewCache, zaps: Zaps, lnurls: LNUrls, settings: UserSettingsStore, relay_filters: RelayFilters, relay_model_cache: RelayModelCache, drafts: Drafts, events: EventCache, bookmarks: BookmarksManager, postbox: PostBox, bootstrap_relays: [String], replies: ReplyCounter, wallet: WalletModel, nav: NavigationCoordinator, music: MusicController?, video: VideoController, ndb: Ndb, purple: DamusPurple? = nil, quote_reposts: EventCounter) {
init(pool: RelayPool, keypair: Keypair, likes: EventCounter, boosts: EventCounter, contacts: Contacts, mutelist_manager: MutelistManager, profiles: Profiles, dms: DirectMessagesModel, previews: PreviewCache, zaps: Zaps, lnurls: LNUrls, settings: UserSettingsStore, relay_filters: RelayFilters, relay_model_cache: RelayModelCache, drafts: Drafts, events: EventCache, bookmarks: BookmarksManager, postbox: PostBox, bootstrap_relays: [RelayURL], replies: ReplyCounter, wallet: WalletModel, nav: NavigationCoordinator, music: MusicController?, video: VideoController, ndb: Ndb, purple: DamusPurple? = nil, quote_reposts: EventCounter) {
self.pool = pool
self.keypair = keypair
self.likes = likes
Expand Down
8 changes: 4 additions & 4 deletions damus/Models/EventsModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ class EventsModel: ObservableObject {
func unsubscribe() {
state.pool.unsubscribe(sub_id: sub_id)
}
private func handle_event(relay_id: String, ev: NostrEvent) {

private func handle_event(relay_id: RelayURL, ev: NostrEvent) {
if events.insert(ev) {
objectWillChange.send()
}
}
func handle_nostr_event(relay_id: String, ev: NostrConnectionEvent) {

func handle_nostr_event(relay_id: RelayURL, ev: NostrConnectionEvent) {
guard case .nostr_event(let nev) = ev, nev.subid == self.sub_id
else {
return
Expand Down
8 changes: 4 additions & 4 deletions damus/Models/FollowersModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ class FollowersModel: ObservableObject {
contacts?.append(ev.pubkey)
has_contact.insert(ev.pubkey)
}
func load_profiles<Y>(relay_id: String, txn: NdbTxn<Y>) {

func load_profiles<Y>(relay_id: RelayURL, txn: NdbTxn<Y>) {
let authors = find_profiles_to_fetch_from_keys(profiles: damus_state.profiles, pks: contacts ?? [], txn: txn)
if authors.isEmpty {
return
Expand All @@ -63,8 +63,8 @@ class FollowersModel: ObservableObject {
authors: authors)
damus_state.pool.subscribe_to(sub_id: profiles_id, filters: [filter], to: [relay_id], handler: handle_event)
}
func handle_event(relay_id: String, ev: NostrConnectionEvent) {

func handle_event(relay_id: RelayURL, ev: NostrConnectionEvent) {
guard case .nostr_event(let nev) = ev else {
return
}
Expand Down
4 changes: 2 additions & 2 deletions damus/Models/FollowingModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ class FollowingModel {
print("unsubscribing from following \(sub_id)")
self.damus_state.pool.unsubscribe(sub_id: sub_id)
}
func handle_event(relay_id: String, ev: NostrConnectionEvent) {

func handle_event(relay_id: RelayURL, ev: NostrConnectionEvent) {
// don't need to do anything here really
}
}
60 changes: 29 additions & 31 deletions damus/Models/HomeModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class HomeModel {
// NDBTODO: let's get rid of this entirely, let nostrdb handle it
var has_event: [String: Set<NoteId>] = [:]
var deleted_events: Set<NoteId> = Set()
var last_event_of_kind: [String: [UInt32: NostrEvent]] = [:]
var last_event_of_kind: [RelayURL: [UInt32: NostrEvent]] = [:]
var done_init: Bool = false
var incoming_dms: [NostrEvent] = []
let dm_debouncer = Debouncer(interval: 0.5)
Expand Down Expand Up @@ -135,7 +135,7 @@ class HomeModel {
}

@MainActor
func process_event(sub_id: String, relay_id: String, ev: NostrEvent) {
func process_event(sub_id: String, relay_id: RelayURL, ev: NostrEvent) {
if has_sub_id_event(sub_id: sub_id, ev_id: ev.id) {
return
}
Expand Down Expand Up @@ -211,18 +211,18 @@ class HomeModel {
pdata.status.update_status(st)
}

func handle_nwc_response(_ ev: NostrEvent, relay: String) {
func handle_nwc_response(_ ev: NostrEvent, relay: RelayURL) {
Task { @MainActor in
// TODO: Adapt KeychainStorage to StringCodable and instead of parsing to WalletConnectURL every time
guard let nwc_str = damus_state.settings.nostr_wallet_connect,
let nwc = WalletConnectURL(str: nwc_str),
let resp = await FullWalletResponse(from: ev, nwc: nwc) else {
return
}

// since command results are not returned for ephemeral events,
// remove the request from the postbox which is likely failing over and over
if damus_state.postbox.remove_relayer(relay_id: nwc.relay.id, event_id: resp.req_id) {
if damus_state.postbox.remove_relayer(relay_id: nwc.relay, event_id: resp.req_id) {
print("nwc: got response, removed \(resp.req_id) from the postbox [\(relay)]")
} else {
print("nwc: \(resp.req_id) not found in the postbox, nothing to remove [\(relay)]")
Expand Down Expand Up @@ -308,7 +308,7 @@ class HomeModel {
self.deleted_events.insert(ev.id)
}

func handle_contact_event(sub_id: String, relay_id: String, ev: NostrEvent) {
func handle_contact_event(sub_id: String, relay_id: RelayURL, ev: NostrEvent) {
process_contact_event(state: self.damus_state, ev: ev)

if sub_id == init_subid {
Expand Down Expand Up @@ -382,7 +382,7 @@ class HomeModel {
}

@MainActor
func handle_event(relay_id: String, conn_event: NostrConnectionEvent) {
func handle_event(relay_id: RelayURL, conn_event: NostrConnectionEvent) {
switch conn_event {
case .ws_event(let ev):
switch ev {
Expand All @@ -400,7 +400,7 @@ class HomeModel {
let r = pool.get_relay(relay_id),
r.descriptor.variant == .nwc,
let nwc = WalletConnectURL(str: nwc_str),
nwc.relay.id == relay_id
nwc.relay == relay_id
{
subscribe_to_nwc(url: nwc, pool: pool)
}
Expand Down Expand Up @@ -461,14 +461,14 @@ class HomeModel {


/// Send the initial filters, just our contact list mostly
func send_initial_filters(relay_id: String) {
func send_initial_filters(relay_id: RelayURL) {
let filter = NostrFilter(kinds: [.contacts], limit: 1, authors: [damus_state.pubkey])
let subscription = NostrSubscribe(filters: [filter], sub_id: init_subid)
pool.send(.subscribe(subscription), to: [relay_id])
}

/// After initial connection or reconnect, send subscription filters for the home timeline, DMs, and notifications
func send_home_filters(relay_id: String?) {
func send_home_filters(relay_id: RelayURL?) {
// TODO: since times should be based on events from a specific relay
// perhaps we could mark this in the relay pool somehow

Expand Down Expand Up @@ -529,7 +529,7 @@ class HomeModel {
pool.send(.subscribe(.init(filters: dms_filters, sub_id: dms_subid)), to: relay_ids)
}

func get_last_of_kind(relay_id: String?) -> [UInt32: NostrEvent] {
func get_last_of_kind(relay_id: RelayURL?) -> [UInt32: NostrEvent] {
return relay_id.flatMap { last_event_of_kind[$0] } ?? [:]
}

Expand All @@ -543,7 +543,7 @@ class HomeModel {
return Array(friends)
}

func subscribe_to_home_filters(friends fs: [Pubkey]? = nil, relay_id: String? = nil) {
func subscribe_to_home_filters(friends fs: [Pubkey]? = nil, relay_id: RelayURL? = nil) {
// TODO: separate likes?
var home_filter_kinds: [NostrKind] = [
.text, .longform, .boost
Expand Down Expand Up @@ -618,8 +618,8 @@ class HomeModel {

migrate_old_muted_threads_to_new_mutelist(keypair: damus_state.keypair, damus_state: damus_state)
}
func get_last_event_of_kind(relay_id: String, kind: UInt32) -> NostrEvent? {

func get_last_event_of_kind(relay_id: RelayURL, kind: UInt32) -> NostrEvent? {
guard let m = last_event_of_kind[relay_id] else {
last_event_of_kind[relay_id] = [:]
return nil
Expand Down Expand Up @@ -883,23 +883,23 @@ func process_contact_event(state: DamusState, ev: NostrEvent) {
}

func load_our_relays(state: DamusState, m_old_ev: NostrEvent?, ev: NostrEvent) {
let bootstrap_dict: [String: RelayInfo] = [:]
let bootstrap_dict: [RelayURL: RelayInfo] = [:]
let old_decoded = m_old_ev.flatMap { decode_json_relays($0.content) } ?? state.bootstrap_relays.reduce(into: bootstrap_dict) { (d, r) in
d[r] = .rw
}
guard let decoded: [String: RelayInfo] = decode_json_relays(ev.content) else {

guard let decoded: [RelayURL: RelayInfo] = decode_json_relays(ev.content) else {
return
}

var changed = false
var new = Set<String>()

var new = Set<RelayURL>()
for key in decoded.keys {
new.insert(key)
}
var old = Set<String>()

var old = Set<RelayURL>()
for key in old_decoded.keys {
old.insert(key)
}
Expand All @@ -910,10 +910,8 @@ func load_our_relays(state: DamusState, m_old_ev: NostrEvent?, ev: NostrEvent) {
for d in diff {
changed = true
if new.contains(d) {
if let url = RelayURL(d) {
let descriptor = RelayDescriptor(url: url, info: decoded[d] ?? .rw)
add_new_relay(model_cache: state.relay_model_cache, relay_filters: state.relay_filters, pool: state.pool, descriptor: descriptor, new_relay_filters: new_relay_filters, logging_enabled: state.settings.developer_mode)
}
let descriptor = RelayDescriptor(url: d, info: decoded[d] ?? .rw)
add_new_relay(model_cache: state.relay_model_cache, relay_filters: state.relay_filters, pool: state.pool, descriptor: descriptor, new_relay_filters: new_relay_filters, logging_enabled: state.settings.developer_mode)
} else {
state.pool.remove_relay(d)
}
Expand All @@ -929,8 +927,8 @@ func load_our_relays(state: DamusState, m_old_ev: NostrEvent?, ev: NostrEvent) {
func add_new_relay(model_cache: RelayModelCache, relay_filters: RelayFilters, pool: RelayPool, descriptor: RelayDescriptor, new_relay_filters: Bool, logging_enabled: Bool) {
try? pool.add_relay(descriptor)
let url = descriptor.url
let relay_id = url.id

let relay_id = url
guard model_cache.model(withURL: url) == nil else {
return
}
Expand All @@ -956,10 +954,10 @@ func add_new_relay(model_cache: RelayModelCache, relay_filters: RelayFilters, po
}
}

func fetch_relay_metadata(relay_id: String) async throws -> RelayMetadata? {
var urlString = relay_id.replacingOccurrences(of: "wss://", with: "https://")
func fetch_relay_metadata(relay_id: RelayURL) async throws -> RelayMetadata? {
var urlString = relay_id.absoluteString.replacingOccurrences(of: "wss://", with: "https://")
urlString = urlString.replacingOccurrences(of: "ws://", with: "http://")

guard let url = URL(string: urlString) else {
return nil
}
Expand Down
20 changes: 8 additions & 12 deletions damus/Models/ProfileModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import Foundation
class ProfileModel: ObservableObject, Equatable {
@Published var contacts: NostrEvent? = nil
@Published var following: Int = 0
@Published var relays: [String: RelayInfo]? = nil
@Published var relays: [RelayURL: RelayInfo]? = nil
@Published var progress: Int = 0

private let MAX_SHARE_RELAYS = 4

var events: EventHolder
Expand Down Expand Up @@ -108,8 +108,8 @@ class ProfileModel: ObservableObject, Equatable {
}
seen_event.insert(ev.id)
}
private func handle_event(relay_id: String, ev: NostrConnectionEvent) {

private func handle_event(relay_id: RelayURL, ev: NostrConnectionEvent) {
switch ev {
case .ws_event:
return
Expand Down Expand Up @@ -142,8 +142,8 @@ class ProfileModel: ObservableObject, Equatable {
}
}
}
private func findRelaysHandler(relay_id: String, ev: NostrConnectionEvent) {

private func findRelaysHandler(relay_id: RelayURL, ev: NostrConnectionEvent) {
if case .nostr_event(let resp) = ev, case .event(_, let event) = resp, case .contacts = event.known_kind {
self.relays = decode_json_relays(event.content)
}
Expand All @@ -159,13 +159,9 @@ class ProfileModel: ObservableObject, Equatable {
func unsubscribeFindRelays() {
damus.pool.unsubscribe(sub_id: findRelay_subid)
}

func getRelayStrings() -> [String] {
return relays?.keys.map {$0} ?? []
}


func getCappedRelayStrings() -> [String] {
return relays?.keys.prefix(MAX_SHARE_RELAYS).map { $0 } ?? []
return relays?.keys.prefix(MAX_SHARE_RELAYS).map { $0.absoluteString } ?? []
}
}

Expand Down
Loading

0 comments on commit e951370

Please sign in to comment.