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

Changes reactions to not include the entire thread as e-tags. #1221

Merged
merged 2 commits into from
May 6, 2024

Conversation

vitorpamplona
Copy link
Collaborator

@vitorpamplona vitorpamplona commented May 4, 2024

Current guidance recommends including all e tags and p tags, but no other tags, from the liked post in the reaction event. I believe this is a relic of the past and should be deprecated.

There is no need to index the entire thread in every single reaction. The indexing burden in popular threads is real. I would argue reactions should only point to the note receiving the reaction and nothing else.

Reasons for the change:

  1. Indexing burden: Indexing entire threads for every single reaction has quadratic costs on the thread's popularity.
  2. Data consumption: requesting reactions to posts in the feed downloads all reactions to all replies in that thread, which is not necessary.
  3. No consistency: We now use NIP-10 markers instead of positional e tags for the entire thread. This means that some reactions include the e tags for the entire thread while others only include root and reply, and others only include the reply without the marker. Since we also have mention and q tags, mentions are added to the reaction's tag array while q tags are not. Conclusion: there is no consistency for reaction counters that are not the reacted event.
  4. No addresses: The recommendation was from before a tags existed, so a tags are not even included.
  5. It's unnecessary: No one reassembles a thread from a reaction event, so there is really no need to have all the tags there.
  6. Zaps and replies don't follow the same rule: We don't include the entire thread on zaps, replies, and quotes. Not even on reports. This only exists on reactions.

@fabianfabian
Copy link

agree

fabianfabian added a commit to nostur-com/nostur-ios-public that referenced this pull request May 5, 2024
@staab staab merged commit 8073c84 into nostr-protocol:master May 6, 2024
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.

5 participants