Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Major API changes #50

Closed
wants to merge 8 commits into from
Closed

Major API changes #50

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
128decd
Major API changes
vmx Nov 22, 2018
190de47
Remove `toJSON()`
vmx Nov 28, 2018
930398b
Rename properties
vmx Nov 28, 2018
888cd27
Update TOC
vmx Nov 28, 2018
3e79d97
Multicodecs should be in the Multicodecs table
vmx Nov 28, 2018
55585d4
Improve `cid()` documentation a bit
vmx Dec 6, 2018
f04424d
Replace "block" with "blob"
vmx Mar 19, 2019
bf2184d
Clarify details about the deserialized object
vmx Mar 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 60 additions & 40 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,81 +58,101 @@ Include this badge in your readme if you make a new module that implements inter

A valid (read: that follows this interface) IPLD format implementation the following API.

### IPLD format utils
IPLD Format APIs are restricted to a single IPLD Node, they never access any linked IPLD Nodes.

#### `util.serialize(dagNode, callback)`

> serializes a dagNode of an IPLD format into its binary format
### serialize(IpldNode)

`callback` must have the signature `function (err, binaryBlob)`, where `err` is an Error is the function fails and `binaryBlob` is a Buffer containing the serialized version.
> Serializes the internal representation of an IPLD Node into a binary blob.

#### `util.deserialize(binaryBlob, callback)`
`IpldNode` is a previously deserialized binary Block.
vmx marked this conversation as resolved.
Show resolved Hide resolved

> deserializes a binary blob into the instance
Returns a Promise containing a `Buffer` with the serialized version of the given IPLD Node.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really encourage to remove requirement to return Buffer in favor of Uint8Array. This requirement makes browser use case unnecessarily complicated, and in practice isn't really necessary. Other IPFS libraries can do the casting where necessary.

P.S. node Buffer is also a valid Uint8Array so node case will require no change.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love if there was some word about errors as well. Specifically how decoder can return error and how that error propagates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving from Node.js to Browser based primitives is certainly on my todo list, but out of scope for these API changes. I don't want to do everything at once.


`callback` must have the signature `function (err, dagNode)`, where `err` is an Error if the function fails and `dagNode` is the dagNode that got deserialized in the process.

#### `util.cid(binaryBlob[, options], callback)`
### deserialize(binaryBlob)

> get the CID of a binary blob
> Deserialize into internal representation.

Options include:
- version - the CID version to be used (defaults to 1)
- hashAlg - the hash algorithm to be used (default to the one set by the format)
The result is a JavaScript object, which may also be a Proxy object in case the data shouldn’t be deserialized as a whole. Its fields are the public API that can be resolved through. It’s up to the format to add convenient methods for manipulating the data. The returned object may also be a Proxy object in case the data shouldn’t be deserialized as a whole.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did wrote more about this above but I think I want add one more note here - Requirement for implementation to provide a proxy just to avoid full decoding makes a high bar, in fact I would argue that custom DataView class with getters is probably a far better alternative. However that still implies 5 allocations when accessing something like /deeply/A0/01/FC-nested/path.txt which may seem insignificant but in some cases (e.g. representing Virtual DOM changelists in dominion library) that makes a difference between fast and unusable performance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a requirement, "may also be a Proxy object", but I'm very negative on Proxies regardless. Aside from performance concerns, I think we've learned from many previous attempts in other languages that things like Proxies should be reserved for metaprogramming and other reflection use-cases rather than general magical data structures. Pitfalls abound.
Perhaps the way forward is to remove the reference to proxies completely and leave it up to each format and allow experimentation.

Alternatively go the container route which is much nicer--I'm not sure DataView is the right container, maybe a bit too low-level, something that feels more like a Map might be more accessible. In fact, a whole custom Map-like that you could .toObject() to fully instantiate as a non-lazy object representation might provide the best of both worlds?

let data = await format.deserialize(blob)
data.get('foo') // could be lazy
data.has('/deeply/A0/01/FC-nested/path.txt') // could be lazy
data.toObject() // skip laziness and recursively instantiate
// { foo: 'bar', deeply: { A0: { '01': { 'FC-nested': { 'path.txt': UInt8Array<00, 00, 00, 00> } } } } }

It would require specification, and even provision of a custom class, maybe just a simple extension of Map.

For deserialization where you know the result is cheap and you want it all you could just instantiate it in the same call:

let copy = Object.extend((await format.deserialize(blob)).toObject(), { lastUpdate: new Date() })
Map {
  get (key/path)
  has (key/path)
  toObject()
  put() // throws? is mutability helpful here? maybe until it starts to hit awkward edges
}

Are there possible use-cases where getters might need to be async or is it safe to frontload that in decode()?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may have misunderstood what I meant by DataView - just a container tgat wraps underlying buffer and provides getters to access / decode fields on demand get key() {. So pretty much what you described as Map except named getters on prototype instead of get(key)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the way forward is to remove the reference to proxies completely and leave it up to each format and allow experimentation.

The whole point of IPLD is to provide common interface for traversing linked data no ? If each implementation is free to design one, the common interface is gone & I think that’s a step in a wrong direction - I much rather have common imperfect interface that can be used across arbitrary formats than have to learn each one

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a container tgat wraps underlying buffer and provides getters to access / decode fields on demand

But this then won't be lazy for nested objects, will it?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(2) usability for implementer (lower complexity)

For implementing eager case this will make no difference. For lazy case avoiding proxies and lazy getters would be significantly simpler than needing to have them

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

let a = await deserialize(blob).nested
let b = await a.b
let c = await b.c

When I wrote my initial response in this thread I was trying to imagine a use-case where the properties would need to be asynchronously accessed and I couldn't see an obvious one except for very large blobs that need to be stored on disk--but those wouldn't be passed in to a deserialize(blob) call in the first place. Maybe there's a case if there was some really heavy crypto applied to sections of a blob, but even then it's questionable. Async computational tasks are usually made pointless by the boundary crossing problems and is why async methods in Node's 'crypto' module have been mostly resisted, computationally intensive thread offloading can't be demonstrated except for very heavy work. The browser is a slightly different case because you're not maximising core use as with a server and responsiveness is important, maybe that's a worthy argument? I'm doubtful.

So again, it looks like premature optimisation in search of a use-case.

(2) usability for implementer (lower complexity)

For implementing eager case this will make no difference. For lazy case avoiding proxies and lazy getters would be significantly simpler than needing to have them

Agreed, and it'll help make quicker progress on this PR!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I care about JS implementation in browser context (on server go is likely a better choice anyway). In browser crypto APIs are async (for good reasons) ignoring that seems strange to me.

And honestly it’s not about optimizations, just about not making currently well supported use cases second class.

P.S. I am working on crypto heavy use case for browsers right now and this API channge will require me to switch to handrolled solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gozala ok this is good context, excuse my ignorance. In your use case, is it necessary that the crypto be handled at the format layer or is it reasonable to push it up to ipld itself? I was imagining ipld/ipld#64 being something in ipld and the blocks just providing the encrypted binary data. Perhaps an encrypted block coming from a format gets transposed into an unencrypted ipld block where the key is provided but this happens at the ipld layer, not in the format. Or are you seeing a different mechanism? Again, I'm fairly new to thinking about this stuff so additional insight is appreciated!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created #51 which hopefully accommodates all the points brought up here. It's a smaller change to the current API. Let's have a discussion over there.


`callback` must have the signature `function (err, cid)`, where `err` is an Error if the function fails and `cid` is a CID instance of the binary blob.
Returns a Promise containing the Javascript object. This object must be able to be serialized with a `serialize()` call.

### Local resolver methods

#### `resolver.resolve(binaryBlob, path, callback)`
### cid(binaryBlob, [options])

> resolves a path in block, returns the value and or a link and the partial missing path. This way the IPLD Resolver can fetch the link and continue to resolve.
> Return the CID of the binary blob.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more of a question.

From what I can tell implementation of the cid is pretty much the same across the board. Is there reason to require implementation of cid in first place ? Could this be made optional, so only implementation that need a custom behavior can provide it ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An IPLD Format has all the information it needs to calculate the CID. The multicodec code as well as the hash. So you can call cid() from layers that build on top of IPLD Formats without knowing those details.


`callback` must have the signature `function (err, result)`, where `err` is an Error if the function fails and `result` is an object with the following keys:
Possible `options` are:
- `version` (`number`, default: 1): the CID version to be used
- `hashAlg` (`Multicodec`, default: the one the format specifies): the hash algorithm to be used

- value: <> - The value resolved or an IPLD link if it was unable to resolve it through.
- remainderPath: <> - The remaining path that was not resolved under block scope.
This can be used to verify that some data actually has a certain CID.

If `path` is the root `/`, the result is a nested object that contains all paths that `tree()` returns. The values are the same as accessing them directly with the full path. Example:

`tree()` returns:
### toJSON(IpldNode)
mikeal marked this conversation as resolved.
Show resolved Hide resolved

```JSON
["author/name", "author/email"]
> Converts an IPLD Node into a JavaScript object that contains only basic types.

Returns a JavaScript object that can be used as input for `JSON.stringify()`. It is *not* a goal to have a JSON representation that is roundtripable back into an IPLD Node. It is meant as a representation that can be processed by third party consumers.

The [IPLD Data Model](https://github.com/ipld/specs/blob/master/IPLD-Data-Model-v1.md) defines two special types that need special attention when converting to JSON:
- Binary: the binary data is transformed into a Base64 encoded string.
- Links: the links are CID objects. Consumers of this JSON shouldn’t need to have their own CID parsing implementation, hence the CID is provided in its original base encoded format as well as the human readable one.

Example with [dag-cbor](https://github.com/ipld/js-ipld-dag-cbor):

```JavaScript
'use strict'

const CID = require('cids')
const ipldDagCbor = require('ipld-dag-cbor')

const input = {
binary: Buffer.from('1155fa3c', 'hex'),
link: new CID('zdpuAxdeot12gCeKJxANaDAL2juLQDB2QK4PFKnnxdAJLpAZf')
}

const serialized = await ipldDagCbor.serialize(input)
const ipldNode = await ipldDagCbor.deserialize(serialized)
const json = ipldDagCbor.toJSON(ipldNode)
console.log(JSON.stringify(json, null, 2))
```

`resolve(binaryblob, "/", callback)` would then have as a result:
The output is:

```JSON
{
"author": {
"name": "vmx",
"email": "[email protected]"
"binary": "EVX6PA==",
"link": {
"cid": "bafyreifvnutjz6sgkym5cw3fw5e2opfew2gy5dw4wui4tzpphylbmmjsci",
"multibase": "base32",
"version": 1,
"multicodec": "dag-cbor",
"multihash": {
"name": "sha2-256",
"bits": 256,
"digest": "b56d269cfa465619d15b65b749a73ca4b68d8e8edcb511c9e5ef3e1616313212"
}
}
}
```

Numbers within a path are interpreted as an array.

#### `resolver.tree(binaryBlob, callback)`

> returns all the paths available in this block.

`callback` must have the signature `function (err, result)`, where `err` is an Error if the function fails and `result` is a list of path such as `["/foo", "/bar", "/author/name", ...]`.

### Properties

#### `defaultHashAlg`
#### `defaultHashCode`

> Default hash algorithm of the format,

> Default hash algorithm of the format
Most formats have one specific hash algorithm, e.g. Bitcoin’s is `dbl-sha2-256`. CBOR can be used with any hash algorithm, though the default in the IPFS world is `sha256`. `defaultHashAlg` is used in the `cid()` call if no hash algorithm is given. The value of `defaultHashAlg` must be one code defined in the [Multihash Table](https://github.com/multiformats/multihash#table-for-multihash).

Most formats have one specific hash algorithm, e.g. Bitcoin’s is `dbl-sha2-256`. CBOR can be used with any hash algorithm, though the default in the IPFS world is `sha256`. `defaultHashAlg` is used in the `util.cid()` call if no hash algorithm was given. The value of `defaultHashAlg` must be one defined in the [Multihash Table](https://github.com/multiformats/multihash#table-for-multihash-v100-rc-semver).
#### `codec`

#### `multicodec`
> Identifier for the format implementation.

> Identifier for the format implementation
The `codec` property is used to register a format implementation in IPLD. It needs to be one of the codes specified in the [Multicodec Table](https://github.com/multiformats/multicodec#multicodec-table).

The `multicodec` property is used to register a format implementation in IPLD. It needs to be one specified in the [Multicodec Table](https://github.com/multiformats/multicodec#multicodec-table).

## Maintainers

Expand Down