ssbc / ssb-tangle

8 stars 2 forks source link

Map() should output a graph even if it only contains the root #5

Closed Powersource closed 4 years ago

Powersource commented 4 years ago

At least in my (tired at midnight) opinion.

I haven't pushed my code anywhere yet (for wiki) but the usecase is basically this:

Someone wants to make an addition to the tangle (e.g. an edit of something) so they need to get the current heads. So they call getHeads(Map(msgs, getThread), but since they're the first person publishing an edit to the tangle, Map() returns {} and makes the call find no heads even if the root should be counted as the head, right?

(~might be getHeads(Map(... but from trying to read the getHeads function I think it wants the map reversed?~ oops yeah it's not supposed to be reversed, changed it above)

Powersource commented 4 years ago

Copying the graph from maps.js

// map objects which represent forward and backward linking maps of the graph
// e.g.
//
//     A   (root)
//    / \
//   B   C
//    \ /
//     D
//
// map = {
//   A: { B: 1, C: 1 },
//   B: { D: 1 },
//   C: { D: 1 }
// }

I guess this issue could maybe be phrased "Nodes without children should also be put in the map", which in this graph's case would mean that D would also be added, e.g. like D: {}. How much of this module's code would this change break?

Although they're slightly different cases maybe, since from this graph you can infer that D exists, but in the only-root scenario you can't tell that case apart from a no-node-exists scenario.

Powersource commented 4 years ago

Handled it pretty easily for now https://github.com/ssbc/ssb-wiki/blob/master/index.js#L34

mixmix commented 4 years ago

thanks for writing this up. Do you want to make a PR which adds a test for this, so we can see it fail, and we can add the corrected functionality.

mixmix commented 4 years ago

I think I'm gonna focus on getting resolution of heads working, and if this ends up being the solution, then that's good. however I suspect this won't be the solution, as the map object is all about recording the edges, not the nodes / vertexes

mixmix commented 4 years ago

I hope this is resolved to your satisfaction with getHeads

I strongly believe you should not include dangling nodes in a wiki, as it will lead to non-deterministic behaviour (I think). Happy to discuss more if you'd like