dag-pb-spec: steb CR #115
dag-pb-spec: steb CR #115
Conversation
Stebalien
commented
Apr 25, 2019
Stebalien
commented
- Fix link/hash encoding (not CBOR).
- Describe a bunch of options for pathing.
- Expand on canonical encoding.
1. Fix link/hash encoding (not CBOR). 2. Describe a bunch of options for pathing. 3. Expand on canonical encoding.
ghost
assigned 
| This is non-standard and means that the 'Tsize' attribute of a link, and | ||
| neither the 'Data' field nor the 'Links' field of the PBNode are available | ||
| through the ipld query language. | ||
| All links in an object must either be blank or unique within the object. |
There was a problem hiding this comment.
A link itself can be blank? Or PBNode.Links can be blank? If the former, does that mean that "optional" in all the PBLink fields are truly optional? In the JS implementation they're not, a CID is required and links created from within JS will have a default name of empty-string if none is supplied.
There was a problem hiding this comment.
Sorry, that should have said "link names".
| @@ -10,10 +10,10 @@ The DAG-PB IPLD format is a legacy format implemented with a single protobuf. | |||
| // An IPFS MerkleDAG Link | |||
| message PBLink { | |||
|
|
|||
| // multihash of the target object | |||
| // binary CID (with no multibase prefix) of the target object | |||
There was a problem hiding this comment.
Would "binary version 0 CID (no multibase prefix) of the target object" be accurate? It might be nice to be explicit about why this is odd.
There was a problem hiding this comment.
Any CID can go here. The key part is that we're not including the multibase (which we sometimes (e.g., CBOR) do include in binary).
| TODO: See https:/ipld/specs/issues/55 | ||
| TODO: Also see https:/ipfs/unixfs-v2/issues/24 (specifically: the "Revise History" option). | ||
|
|
||
| Neither go-ipfs nor js-ipfs agree so we have some room here because we're going to break something. |
There was a problem hiding this comment.
How about for the purpose of the spec, the only required path mechanism that need be implemented is the /Links/0/Hash style. In js-ipld-dag-pb the names are just sugar and I'm guessing that the go implementation does the /named style as sugar as well and it just doesn't expose a resolution that can get into the paths array? Maybe it should and that's the consistency between the two. /Links/named/Hash is the JS sugar and /named is the Go sugar and that's just how it is.
Either way, that's a blocking issue that's going to hold this up so it needs to get resolved.
There was a problem hiding this comment.
I'd rather not leave sugar like that. We can get rid of it but resolve(node, '/some/path') should behave the same way everywhere.
| As far as I can tell, js-ipfs supports pathing both by name and by index with | ||
| paths like: `/Links/$name/Hash/...` or `/Links/$idx/Hash`. | ||
|
|
||
| ### Alternative: Correct IPLD |
There was a problem hiding this comment.
Yeah, this one. Would it be difficult to implement this in the Go version? (I can't find the code) Having this unresolved and listing "Alternative"s isn't a very helpful spec. Listing what the two versions do is helpful, it'd be nice to have some concrete middle for the spec.
There was a problem hiding this comment.
These sections are just here for discussion. The final spec will have just one.
Switching to "correct IPLD" will take a bit of work in go-ipfs but we'll have to break something no matter what.
Gitignore the _book directory created by `gitbook serve`
|
Closing this one if favour of #201. |
