Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UnknownId relay hints to Mention::{Profile,Event} #684

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

ksedgwic
Copy link
Contributor

I marked this as WIP because it depends on damus-io/nostrdb-rs#37

It also does not use the relay hints in the unknown_id_send method yet. I'm unsure how much we want to package in this PR; it is ready to go if we want to enhance unknown_id_send in a follow-on PR.

@ksedgwic ksedgwic marked this pull request as ready for review February 3, 2025 16:16
@ksedgwic
Copy link
Contributor Author

ksedgwic commented Feb 3, 2025

Marking this as ready for review, but it needs damus-io/nostrdb-rs#37 to be merged

@jb55
Copy link
Contributor

jb55 commented Feb 3, 2025

looking at this today

@ksedgwic ksedgwic force-pushed the 2025-01-add-relay-hints branch from 6bedd49 to 800bd9c Compare February 4, 2025 19:26
@ksedgwic
Copy link
Contributor Author

ksedgwic commented Feb 4, 2025

updated w/ nostrdb-rs relays_iter interface

@ksedgwic ksedgwic requested a review from jb55 February 4, 2025 19:27
@jb55 jb55 linked an issue Feb 4, 2025 that may be closed by this pull request
@jb55 jb55 changed the title WIP: Add relay hints to Mention::{Profile,Event} Add UnknownId relay hints to Mention::{Profile,Event} Feb 4, 2025
@jb55 jb55 requested review from jb55 and removed request for jb55 February 4, 2025 19:34
@jb55 jb55 linked an issue Feb 4, 2025 that may be closed by this pull request
@jb55 jb55 requested review from jb55 and removed request for jb55 February 4, 2025 19:38
@ksedgwic ksedgwic force-pushed the 2025-01-add-relay-hints branch from 800bd9c to df9ce86 Compare February 4, 2025 20:12
@ksedgwic
Copy link
Contributor Author

ksedgwic commented Feb 4, 2025

damus-io/nostrdb-rs#37 was merged, updated the dependency

Copy link
Contributor

jb55 commented Feb 4, 2025

I think we want a different data structure. we should update ids: HashSet<UnknownId> to HashMap<UnknownId, HashSet<RelayUrl>>

The point of the HashSet<UnknownId> is to collect unique ids. We shouldn't be adding duplicates… ids may have different relays on different notes. HashMap<UnknownId, HashSet<RelayUrl>> will enable us to update the relays for a given unknown id as we collect them.

@ksedgwic ksedgwic force-pushed the 2025-01-add-relay-hints branch from df9ce86 to 7a9ed8a Compare February 5, 2025 00:21
@ksedgwic
Copy link
Contributor Author

ksedgwic commented Feb 5, 2025

Done

Copy link
Contributor

@jb55 jb55 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ksedgwic ksedgwic force-pushed the 2025-01-add-relay-hints branch from 7a9ed8a to 482313f Compare February 6, 2025 18:13
@jb55 jb55 merged commit 482313f into damus-io:master Feb 6, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add relay hints to UnknownIds
2 participants