search

ssbc / ssb-schema-definitions

Standardised schema definitions for ssb message types using is-my-json-valid
3 stars 1 forks source link

Incompatible with Manyverse link mentions #4

Open christianbundy opened 4 years ago

christianbundy commented 4 years ago

Example post: %Mcs+W3JW6xLSAB/isERCWsCJhziY9MZjrGDfDIH6RD0=.sha256:

{
  "previous": "%4TBbz2XTWEwiZTngvre36z9u0eNNh5S3LSZdDhwL7X0=.sha256",
  "sequence": 450,
  "author": "@9hl0g6iMzu7U4vXUdSJtFJ1v+E5WJkYVX90C1kt/7Ks=.ed25519",
  "timestamp": 1574042820636,
  "hash": "sha256",
  "content": {
    "text": "In case that's wrong, I think the relevant code is here: https://github.com/ssbc/ssb-db/blob/6c0cdd580b6166b106283e3f1745585e189252ce/create.js#L46\n\nStrongly agree that this is confusing, I think there's even an issue where I asked the same question because I couldn't figure out the magic permutation. Would love more docs and less stuff to learn. 🥴",
    "type": "post",
    "root": "%RouG6IVwpEQhfYLpyFkjyJrj9uhR6GlQDH+4Ngt/6Bg=.sha256",
    "mentions": [
      {
        "link": "https://github.com/ssbc/ssb-db/blob/6c0cdd580b6166b106283e3f1745585e189252ce/create.js#L46",
        "name": "https://github.com/ssbc/ssb-db/blob/6c0cdd580b6166b106283e3f1745585e189252ce/create.js#L46"
      }
    ]
  },
  "signature": "S2Ktu/9TGAQkcLUTRFow+qRgWfAKLo5a9frwyeT3KiipeTze1vhdY51kb07qzG3QcNB3O/vaxiby30L0vBuTAA==.sig.ed25519"
}

It looks like there's a disagreement between this schema and how Manyverse is publishing links. My guess is that the schema definition is "correct", but I'm not 100% sure. Also, what should we do when we notice that messages from a popular client are failing a schema? Maybe there's a way to fix (or hide) the properties that are acting unexpectedly? Hiding would work fine here, but we wouldn't want to hide recps or similar. :grimacing:

cc: @mixmix @staltz

See also: #2

staltz commented 4 years ago

Might be remark-ssb-mentions's fault (to which I can gladly put a fix), but help me understand, what is the actual disagreement with this schema and Manyverse? Can you put a diff or an expected-versus-actual?

christianbundy commented 4 years ago

Totally, I've just pushed f09bc49482b11d362d1550b9b2928fed18ee020e with a failing test:

not ok 8 manyverse link mention
  ---
    operator: ok
    expected: true
    actual:   false
    at: Test.t (/home/christianbundy/src/ssbc/ssb-schema-definitions/tests/mentions.js:43:9)
    stack: |-
      Error: manyverse link mention
          at Test.assert [as _assert] (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:225:54)
          at Test.bound [as _assert] (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:77:32)
          at Test.assert (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:343:10)
          at Test.bound [as true] (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:77:32)
          at Test.t (/home/christianbundy/src/ssbc/ssb-schema-definitions/tests/mentions.js:43:9)
          at Test.bound [as _cb] (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:77:32)
          at Test.run (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:93:10)
          at Test.bound [as run] (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/test.js:77:32)
          at Immediate.next [as _onImmediate] (/home/christianbundy/src/ssbc/ssb-schema-definitions/node_modules/tape/lib/results.js:81:19)
          at runCallback (timers.js:705:18)
  ...
[
  {
    "field": "data.mentions",
    "message": "referenced schema does not match",
    "value": [
      {
        "link": "https://github.com/ssbc/ssb-db/blob/6c0cdd580b6166b106283e3f1745585e189252ce/create.js#L46",
        "name": "https://github.com/ssbc/ssb-db/blob/6c0cdd580b6166b106283e3f1745585e189252ce/create.js#L46"
      }
    ],
    "schemaPath": [
      "properties",
      "mentions"
    ]
  }
]

The schema expects each item of the mentions array to be one of these values, and since mentions[].link isn't a message/feed/blob it's failing the schema.

christianbundy commented 4 years ago

Putting it another way, this schema only expects message/feed/blob links in mentions[].link, so it fails when you pass a URL.

mixmix commented 4 years ago

yeah @staltz it's seems like that mention is kinda not adding anything

Aside from redundancy, I think we're bumping up against an assumption here that mentions are expected to be cypherlinks - because we're looking for contexts which might need notifications?

Mentions was originally used to do nicer @mentioning like : 

{
  text: 'hey @mix nice to see you '
  mentions: [
    { name: '@mix', link: '@ye+4......sha256' }
  ]
}

So there could be an argument that you would use this to make nicer looking links ... but you're using markdown ... so ....

staltz commented 4 years ago

I'm treating it as a bug in remark-ssb-mentions, because looking at the code it seemed I missed an obvious array.filter(isLinkSSBMention).

staltz commented 4 years ago

Released remark-ssb-mentions 2.0.0 and began using that in Manyverse, a new version of the app to be released soon.

Can this issue be closed?

christianbundy commented 4 years ago

I think so. I'm still curious about:

Also, what should we do when we notice that messages from a popular client are failing a schema? Maybe there's a way to fix (or hide) the properties that are acting unexpectedly? Hiding would work fine here, but we wouldn't want to hide recps or similar. :grimacing:

But maybe I should open another thread?

staltz commented 4 years ago

In general, because SSB is decentralized and permissionless, all message types should never be trusted to be in the correct shape and should always be checked.

christianbundy commented 4 years ago

Totally. My question is moreso: what options do we have when we find a message with an invalid schema? When publishing a message, we can fail loudly if we see a message that doesn't pass the schema, but when building thread graphs it's less clear.

Concrete example: for Oasis I'm recursively building a thread graph by starting with a node and finding its ancestors: