nips icon indicating copy to clipboard operation
nips copied to clipboard

NIP-22 - Comment

Open arthurfranca opened this issue 9 months ago • 112 comments

Please read here

arthurfranca avatar May 12 '24 15:05 arthurfranca

+1.

Thread roots and replies should always be different kinds. This "Comment" kind could be for replies only. The thread root is usually the main content kind in each NIP.

Then it would be similar to reactions. They can react to anything but they never start anything by themselves.

I also think we should migrate Kind 1 to use this kind as reply in the future. Kind 1 should be for root posts only.

vitorpamplona avatar May 12 '24 16:05 vitorpamplona

I also think we should migrate Kind 1 to use this kind as reply in the future. Kind 1 should be for root posts only.

It would be great if clients start posting kind:1111s along with kind:1 replies when publishing new replies.

arthurfranca avatar May 13 '24 18:05 arthurfranca

Juggernauts, mavericks, all of you. This is a seriously breaking change breaking tons of existing software.

mikedilger avatar May 15 '24 23:05 mikedilger

Juggernauts, mavericks, all of you. This is a seriously breaking change breaking tons of existing software.

But is it better or worse?

vitorpamplona avatar May 15 '24 23:05 vitorpamplona

This is a seriously breaking change breaking tons of existing software.

Strong words for something that existing clients can just completely ignore. If devs feel like their clients don't show enough content they can opt-in to querying this kind, like they do with every other event kind.

We also have to be realistic. Kind 1 sucks because it is overloaded. There have been multiple PRs that address this issue. At some point client devs will just use a different kind for replies, because it's easier to work with and is more bandwidth efficient, which is an important point since the majority of potential nostr users will use mobile clients that have bandwidth constraints. Having these replies standardized in this repo does not matter, but would be preferable. If the overlords here disagree with a sensible proposal clients will just define their own standards like jb55 did.

dluvian avatar May 16 '24 10:05 dluvian

@dluvian the the bar for adding something to the nips repo has never been "will the overlords allow it", but "do two clients implement it". So if you can point to two clients that have added this, we can merge it. But it's also fine to have the conversation here and debate whether it's a good idea. I'm against it until we have a system for managing breaking changes. I think there are things we can do, but let's solve that before we break the experience in all existing clients.

staab avatar May 16 '24 15:05 staab

I made a mistake. This it isn't what I initially thought. Reading it again after a good nights sleep and I understand better what this is.

If you need to comment on a patch, or on a bid, or on a relay list, this event makes it possible.

My initial read was that all kind-1 replies would be replaced with these comments, and only kind-1 thread roots would remain kind-1, which is why I made the previous comment. But that's not the case.

EDIT: And I got that impression from comments earlier in this thread, which were I'd say "aspirational" and not reflecting the NIP as it is proposed.

mikedilger avatar May 16 '24 19:05 mikedilger

But please explain why kind-1 notes cannot already be used to do this, and how a new kind improves things. I fail to see how it improves things versus kind-1.

mikedilger avatar May 16 '24 19:05 mikedilger

But please explain why kind-1 notes cannot already be used to do this, and how a new kind improves things. I fail to see how it improves things versus kind-1.

It is currently impossible to make a filter that only brings kind1 root posts. You have to download all new posts and replies and then filter the replies out. It's extremely wasteful.

vitorpamplona avatar May 16 '24 19:05 vitorpamplona

@mikedilger kind:1 could in theory retrofit into a generic comment. One caveat is that to download root posts only, we'de have to convince existing clients to add something hacky like an empty k tag to be able to filter by { #k: [""], kinds: [1] } (and set k=referenced-event-kind for non-root ones). Separating comments from root posts felt more appropriate than the empty string.

Also, this thread you are in is a good example of the current situation: diverse apps are indiscriminately using kind:1 to comment on things that twitter-like clients don't recognize. Kind:1 clients currently have no way to opt-out.

If we do nothing, more and more bogus events will get downloaded only to be discarded right away. A bandwidth waste to wsay the least.

arthurfranca avatar May 16 '24 21:05 arthurfranca

I agree root posts being of a different kind would be better. But it seems like a change that is too late to make.

I'm adding it to https://github.com/mikedilger/nostr-next/issues/31

mikedilger avatar May 16 '24 22:05 mikedilger

But it seems like a change that is too late to make.

But why? Not improving the protocol in fear of a migration process feels wrong. Especially since nostr is still relatively small, we should just bite the bullet if it's worth it.

Consider that right now we can't:

  • Query only roots
  • Query only replies without setting ids
  • Lazy load a thread starting from the root (because of nip10 root e-tags)
  • Know if a reply is to another kind 1 or some other event

dluvian avatar May 17 '24 04:05 dluvian

Talking about the specifics here, I am failing to see how the new role-based tag structures (o and r) are an improvement over NIP-10 markers. The parser of the first value is wild. If we keep adding other types to it (hashtag, geolocation roots?), the parser itself could become an issue.

Is this,

  • ["o", "i:<event-kind>:<event-pubkey>:<event-id>", "<relay-url, optional>"]
  • ["o", "a:<event-kind>:<event-pubkey>:<d-tag, optional>", "<relay-url, optional>"]
  • ["o", "https://abc.com/articles/1/"]
  • ["r", "i:<event-kind>:<event-pubkey>:<event-id>", "<relay-url, optional>"]
  • ["r", "a:<event-kind>:<event-pubkey>:<d-tag, optional>", "<relay-url, optional>"]
  • ["r", "https://abc.com/articles/1/"]

better than this?

  • ["e", "<event-id>", "<relay-url, optional>", "root"]
  • ["a", "<event-kind>:<event-pubkey>:<d-tag, optional>", "<relay-url, optional>", "root"]
  • ["r", "https://abc.com/articles/1/", "root"]
  • ["e", "<event-id>", "<relay-url, optional>", "reply"]
  • ["a", "<event-kind>:<event-pubkey>:<d-tag, optional>", "<relay-url, optional>", "reply"]
  • ["r", "https://abc.com/articles/1/", "reply"]

o and r tags feel worse because now we have a little evolving (do we need versions for this?) algorithm to parse the first value. Historically the algorithm has been defined by each tag and because it had a fixed scope, it didn't need to change over time. Putting the marker as the first tag means that you have to evolve the algo to support any new owners and reply structures that might arise.

vitorpamplona avatar May 17 '24 12:05 vitorpamplona

Maybe what's missing is the clarification of the "schema" in value 1. We could specify the chars before the first : define a schema for the rest of the value.

  • ["o", "i:<event-kind>:<event-pubkey>:<event-id>", "<relay-url, optional>"]
  • ["o", "a:<event-kind>:<event-pubkey>:<d-tag, optional>", "<relay-url, optional>"]
  • ["o", "r:https://abc.com/articles/1/"]
  • ["o", "t:<hashtag>"]
  • ["o", "g:<geohash>"]

Parse the schema first and then send the rest of the value to the appropriate decoder if you support that.

Do the same structure for replies.

Then over time these letters can be expanded to words to represent more objects that can be root or reply.

vitorpamplona avatar May 17 '24 12:05 vitorpamplona

failing to see how the new role-based tag structures (o and r) are an improvement over NIP-10 markers

As you know, relays won't filter by marker. That's why the marker meaning became the indexable tag. "root" became "o", "reply" became "r" (mention became "q" - "The q tag ensures quote reposts are not pulled and included as replies in threads"). There's further explantion in the NIP at the threads section (there are two "This tag is useful to...")

We could specify the chars before the first : define a schema for the rest of the value

I'm not sure about t: and g:. It is unusual treating the hashtag as the subject (OP) considering this can already be achived by picking kind:1 and adding one t tag.

r: prefix is a good idea, good to have prefixes if we need to extend them (though I think "i:", "a:" for nostr - also "s:" for event sets - and "r:" for all old web things would cover everything). People could add r tag too (like t and g), but twitter-like clients would be polluted by kind:1's used as blog comments, followers don't expect to see random blog comments of their follows.

arthurfranca avatar May 17 '24 15:05 arthurfranca

I'm not sure about t: and g:. It is unusual treating the hashtag as the subject (OP) considering this can already be achived by picking kind:1 and adding one t tag.

The hashtag root comment will only show in UIs that follow the hashtag itself. Meaning, you can post into #bitcoin without notifying your followers on the regular kind 1 feed. Only people following #bitcoin would see your post. That is a great new feature to boost the use of hashtag-exclusive content.

Same for geolocation as in #1170

r: prefix is a good idea

I think these prefixes that define how the rest of the value is encoded are required.

vitorpamplona avatar May 17 '24 15:05 vitorpamplona

Makes total sense. Some time ago before this PR I also thought of location-based public chats.

arthurfranca avatar May 17 '24 15:05 arthurfranca

I think this is probably a good idea.

fiatjaf avatar May 18 '24 15:05 fiatjaf

But please explain why kind-1 notes cannot already be used to do this, and how a new kind improves things. I fail to see how it improves things versus kind-1.

@mikedilger multiple NIPs have already been defining special kinds for contextual replies because people are very wary of using kind 1 -- as they should be, because kind 1 will show up in all microblogging clients without any context.

So having a single generic comment kind solves that.

Or we could create dozens of different comment kinds, all with the same format, but using a kind number for each situation.

I think there are pros and cons on both sides.

fiatjaf avatar May 18 '24 15:05 fiatjaf

@pablof7z should we use this kind to reply Wikifreedia posts? :)

vitorpamplona avatar May 23 '24 20:05 vitorpamplona

Is anyone using this NIP currently?

MerryOscar avatar Jul 22 '24 09:07 MerryOscar

Probably no one uses it yet.

AsaiToshiya avatar Jul 22 '24 09:07 AsaiToshiya

I am considering using this proposal for podcast comments using the NIP-73 External Content IDs i tags as the kind within the o / r tag.

Here's an example episode comment:

{
  kind: 1111,
  content: 'This was a great episode!',
  tags: [
    // referencing the original post (the episode)
    ["o", "i:podcast:item:guid:d98d189b-dc7b-45b1-8720-d4b98690f31f", "https://fountain.fm/episode/z1y9TMQRuqXl2awyrQxg"],
    
    // top-level comments have the same o and r tags
    ["r",  "i:podcast:item:guid:d98d189b-dc7b-45b1-8720-d4b98690f31f", "https://fountain.fm/episode/z1y9TMQRuqXl2awyrQxg"],
    
    // the original post "kind" - for external content ids it is the guid scheme without the id
    ["k", "i:podcast:item:guid"]
  ]
  // other fields
}

@arthurfranca @vitorpamplona @staab @fiatjaf if you have some time would you mind going over this proposal again and commenting whether you think it's in a good state or whether it might need some changes?

One thing specifically I wasn't sure about was the k tag "original post kind" - my assumption was that this is used to find all the root comments associated with a specific kind - so I put in the external content id guid scheme without the actual id. This way clients could query all the recent root episode comments for any podcast.

MerryOscar avatar Aug 15 '24 14:08 MerryOscar

@MerryOscar I think the way you're using this makes sense.

To clarify my previous comment, mostly I don't like the key/value delimited by colons when more conventional tags would do. But o for root and r for parent make sense, and i namespaces in k tags when anchoring to something outside of nostr also make sense. That might be a good way to introduce this NIP too, since it gets it into use without requiring clients to update reply fetching behavior or be at risk of missing comments.

staab avatar Aug 15 '24 17:08 staab

@MerryOscar The PR is solid. It wasn't merged yet due to lack of available implementations.

I will add the i prefix related to NIP-73 for o/r tags to the NIP. Meanwhile, check these examples:

A podcast comment example:

{
  id: "80c48d992a38f9c445b943a9c9f1010b396676013443765750431a9004bdac05",
  pubkey: "f7234bd4c1394dda46d09f35bd384dd30cc552ad5541990f98844fb06676e9ca",
  kind: 1111,
  content: "This was a great episode!",
  tags: [
    [
      "o",
      "i:podcast:item:guid:d98d189b-dc7b-45b1-8720-d4b98690f31f",
      // note the "r" followed by a space then the url hint
      "r https://fountain.fm/episode/z1y9TMQRuqXl2awyrQxg"
    ],
    // same value as "o" tag above, because it is a top-level comment (not a reply to a comment)
    [
      "r",
      "i:podcast:item:guid:d98d189b-dc7b-45b1-8720-d4b98690f31f",
      "r https://fountain.fm/episode/z1y9TMQRuqXl2awyrQxg"
    ],
    
    ["K", "i:podcast:item"], // uppercase K, this is the OP (original post)'s kind, that is, a podcast item
    ["k", "i:podcast:item"] // lowercase k; in the case of a top-level comment, it is the same as the uppercase K
  ],
  created_at: 1723748973
}

A reply to a podcast comment example:

{
  kind: 1111,
  content: "I'm replying to the above comment.",
  tags: [
    [
      "o",
      "i:podcast:item:guid:d98d189b-dc7b-45b1-8720-d4b98690f31f",
      "r https://fountain.fm/episode/z1y9TMQRuqXl2awyrQxg"
    ],
    // the "r" tag is a reference to the above comment
    [
      "r",
      // "e:"<id-of-the-above-comment>
      "e:80c48d992a38f9c445b943a9c9f1010b396676013443765750431a9004bdac05",
      // "p"<space><pubkey-of-the-above-comment-author>
      "p f7234bd4c1394dda46d09f35bd384dd30cc552ad5541990f98844fb06676e9ca",
      // "r"<space><url-of-the-relay-where-above-comment-was-fetched-from>
      "r wss://example.relay"
    ],
    
    ["K", "i:podcast:item"], // uppercase K, this is the OP (original post)'s kind, that is, a podcast item
    ["k", "n:1111"] // lowercase k; this is the above comment's kind
  ]
  // other fields
}

arthurfranca avatar Aug 15 '24 19:08 arthurfranca

@arthurfranca can you explain why can't we use e and a tags when referencing Nostr entities? It makes for better and simpler indexes if their values are just 64-char hex entities.

fiatjaf avatar Aug 15 '24 20:08 fiatjaf

@fiatjaf First thing this NIP does is kill NIP-10 markers. Each marker was a different context. That's why we can't use e tag for "root" context and also the same e tag name for "reply" context.

So let's say we use e for "root" and E for "reply". While e is probably used just for event ids, no matter at what event kind, we can't guarantee E will always have 64 chars when used elsewhere.

Then for replaceable events we would need a for "root" and A for "reply", with no said index benefit.

Yet, things other than events (urls, hashtags, geohashes and NIP-73 external content) can be referenced as "root" and "reply". That would be two one-letter tags for each of these things. Or, we pick just two tags and prefix these other things when setting the value.

But if we are already prefixing tag values, I decided to use just o for any "root" reference and r for any "reply" reference, no matter if a nostr event or not or if replaceable or not.

arthurfranca avatar Aug 15 '24 22:08 arthurfranca

we can't guarantee E will always have 64 chars when used elsewhere.

yes, we can't, but on the relay side we can check and ignore the tag if it doesn't -- or reject the event altogether to eliminate bad practices.

That would be two one-letter tags for each of these things.

so what? sounds like a good idea to me, we have enough letters in the alphabet. we only need to define the tag meaning for this event kind in particular, it doesn't have to be the same in other NIPs.

fiatjaf avatar Aug 16 '24 10:08 fiatjaf

But I also see why you would want to just have clear tag names for root and reply, it is better in some ways.

In that case my next suggestion will be to remove the value prefixes and add a market denoting what the value is -- in case it isn't super clear already.

["o", "29578a0507c6be981192cd39996b19c79fa6af33185515d692923b87c65a06e3", "id"]

["r", "12345:1c2655bc1cc6de321865a9cbde9cc509ae0269245def2f6c042532724fa2abeb:banana", "address"]

["o","https://whatever.com","url"]

Actually this is not even necessary as I believe it will be super clear in 100% of the cases, but also doesn't hurt. Getting rid of the prefix in tag values is strictly a good thing though.

fiatjaf avatar Aug 16 '24 10:08 fiatjaf

@fiatjaf Look, I don't have strong opinions here. The prefix thing evolved from this discussion with @vitorpamplona.

I was convinced as it felt appropriate cause the o/r tag can reference many types of things.

Prefixing does help with stuff like this:o/r tags can be hashtag references. A hashtag can be any text. When fetching events by o/r tag, hashtag references may collide with any other reference if we don't prefix it. Parsing is also predictable.

We could do ["o","https://whatever.com","url"], as your example, but is it really saving relay DB space compared to ["o","r:https://whatever.com"]?

And won't ["o","https://whatever.com"] example be worse just to avoid 2 chars addition?

I get that nostr stuff should be first-class citizens but an exception for this case where o/r/k/K tags can be related to distinct kinds of things seems reasonable.

It is just not clear to me why the prefix would be bad.

arthurfranca avatar Aug 16 '24 13:08 arthurfranca