MostroP2P / mostro

Lightning Network peer-to-peer exchange platform on Nostr
https://mostro.network
MIT License
184 stars 30 forks source link

Remove pubkeys from rating events #280

Closed grunch closed 1 week ago

grunch commented 6 months ago

Currently we are publishing the user rates events linked to the real user pubkey, this can be used to collect pubkeys, we should avoid showing that kind of data.

Here the event:

[
  "EVENT",
  "RAND",
  {
    "id": "80909a120d17632f99995f92caff4801f25e9e523d7643bf8acb0166bd0932a6",
    "pubkey": "dbe0b1be7aafd3cfba92d7463edbd4e33b2969f61bd554d37ac56f032e13355a",
    "created_at": 1702637077,
    "kind": 38383,
    "tags": [
      ["d", "00000ba40c5795451705bb9c165b3af93c846894d3062a9cd7fcba090eb3bf78"],
      ["total_reviews", "1"],
      ["total_rating", "2"],
      ["last_rating", "1"],
      ["max_rate", "2"],
      ["min_rate", "5"],
      ["data_label", "rating"]
    ],
    "content": "",
    "sig": "456fdc0589a5ffe1b55d5474cef2826bf01f458d63cf409490def9c5af31052e0461d38aed4f386f5dcea999e9fe6001d27d592dbba54a0420687dce0652322f"
  }
]

A simple solution could be hashing the pubkey and using it as d tag instead of the pubkey, this way is going to be easy to find user reputation if you already have the pubkey and impossible to collect pubkeys from third parties.

Also we should avoid to publish this events right after an order is completed and do it after a period of time, every 60 minutes for example.

After implement this proposal the event would change to this:

[
  "EVENT",
  "RAND",
  {
    "id": "80909a120d17632f99995f92caff4801f25e9e523d7643bf8acb0166bd0932a6",
    "pubkey": "dbe0b1be7aafd3cfba92d7463edbd4e33b2969f61bd554d37ac56f032e13355a",
    "created_at": 1702637077,
    "kind": 38383,
    "tags": [
      ["d", "7c68497bc9200d516480e321d21dc7dfa7e945b6a51d6cb2f6c0027c456535a1"],
      ["total_reviews", "1"],
      ["total_rating", "2"],
      ["last_rating", "1"],
      ["max_rate", "2"],
      ["min_rate", "5"],
      ["data_label", "rating"]
    ],
    "content": "",
    "sig": "456fdc0589a5ffe1b55d5474cef2826bf01f458d63cf409490def9c5af31052e0461d38aed4f386f5dcea999e9fe6001d27d592dbba54a0420687dce0652322f"
  }
]
bilthon commented 6 months ago

A few comments here. First it's ok to have a delay, I'd add that this should be a random, not fixed delay. Say anything between 30 to 180 minutes.

The second is more of a question. I get that d parameter is used as an identifier in replaceable events, but wouldn't the public key info be better suited in a p tag?

Finally, on the topic of tags, this might just be me being nitpicky, but I feel we’re overusing them. In my opinion, this kind of information is perfectly suited for the content field. I believe tags are more appropriate for indexable data or metadata.

grunch commented 6 months ago

A few comments here. First it's ok to have a delay, I'd add that this should be a random, not fixed delay. Say anything between 30 to 180 minutes.

The second is more of a question. I get that d parameter is used as an identifier in replaceable events, but wouldn't the public key info be better suited in a p tag?

the point of this issue is avoid showing user's pubkey, we don't want to use a p tag

Finally, on the topic of tags, this might just be me being nitpicky, but I feel we’re overusing them. In my opinion, this kind of information is perfectly suited for the content field. I believe tags are more appropriate for indexable data or metadata.

I found tags a really clean way of publish data on nostr, I can't say the same thing on put everything as a content

bilthon commented 6 months ago

the point of this issue is avoid showing user's pubkey, we don't want to use a p tag

Well I guess that if we're going to be putting the hash of a public key then a d (identifier) tag indeed makes more sense.

I found tags a really clean way of publish data on nostr, I can't say the same thing on put everything as a content

Placing the content of your message in the content field is just as clean IMO and feels more appropriate. Think of a parcel, the contents go inside, tags with meta-data go on the outside as literal tags.

Here's an example:

[
  "EVENT",
  "RAND",
  {
    "id": "80909a120d17632f99995f92caff4801f25e9e523d7643bf8acb0166bd0932a6",
    "pubkey": "dbe0b1be7aafd3cfba92d7463edbd4e33b2969f61bd554d37ac56f032e13355a",
    "created_at": 1702637077,
    "kind": 38383,
    "sig": "456fdc0589a5ffe1b55d5474cef2826bf01f458d63cf409490def9c5af31052e0461d38aed4f386f5dcea999e9fe6001d27d592dbba54a0420687dce0652322f"
    "content": "{\"total_reviews\": 1, \"total_rating\": 2, \"last_rating\": 1,\"max_rate\": 2,\"min_rate\":5}",
    "tags": [
      ["d", "7c68497bc9200d516480e321d21dc7dfa7e945b6a51d6cb2f6c0027c456535a1"],
      ["data_label", "rating"]
    ],
  }
]

Another way of looking into this is that tags can be used by clients to quickly discard or identify events with useful information, with the userful information itself coming inside the content field.

grunch commented 1 week ago

closing this in favor of a new issue based on this discussion https://github.com/MostroP2P/mostro/discussions/362