Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Awesome IPLD Endeavour #398

Merged
merged 4 commits into from
Oct 28, 2016
Merged

Awesome IPLD Endeavour #398

merged 4 commits into from
Oct 28, 2016

Conversation

daviddias
Copy link
Contributor

No description provided.

@daviddias
Copy link
Contributor Author

I need a way to hint the block API which hash function to use and also, which version of CID, so that I can store blocks from different formats. This is also useful for Bitswap, since the Block API uses the BlockService and the BlockService is the gateway for Bitswap.

Currently, in js-ipfs, I pass the block and the CID, which packs the information I need.

@whyrusleeping I was checking the go-ipfs master branch to find how to hint the block API which hash algorithm, but noticed option isn't added.

» ipfs block put --help
USAGE
  ipfs block put <data> - Store input as an IPFS block.

SYNOPSIS
  ipfs block put [--] <data>

ARGUMENTS

  <data> - The data to be stored as an IPFS block.

DESCRIPTION

  'ipfs block put' is a plumbing command for storing raw ipfs blocks.
  It reads from stdin, and <key> is a base58 encoded multihash.

I believe you have this in the 'programmatic' interface and are defaulting to sha2-256 on the CLI, is this right? Will you add this feature to the Block API? so that I can have the same interface in both js-ipfs and js-ipfs-api. Thank you :)

@@ -2,7 +2,7 @@
"name": "ipfs-api",
"version": "9.0.0",
"description": "A client library for the IPFS HTTP API. Follows interface-ipfs-core spec",
"main": "lib/index.js",
"main": "src/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: remove before merge


module.exports = (send) => {
return {
get: promisify((args, opts, callback) => {
// TODO this needs to be adjusted with the new go-ipfs
// http-api
if (args && args.constructor && args.constructor.name === 'CID') {
Copy link
Contributor

Choose a reason for hiding this comment

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

better: if (args && cid.isCID(args))

@@ -32,6 +39,12 @@ module.exports = (send) => {
})
}),
stat: promisify((args, opts, callback) => {
// TODO this needs to be adjusted with the new go-ipfs
// http-api
if (args && args.constructor && args.constructor.name === 'CID') {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

// TODO this needs to be adjusted with the new go-ipfs
// http-api

if (typeof (cid) === 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for parens

@daviddias
Copy link
Contributor Author

Ready for review :) @dignifiedquire @victorbjelkholm

"socket.io": "^1.4.8",
"socket.io-client": "^1.4.8",
"socket.io": "^1.5.1",
"socket.io-client": "^1.5.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

where is socket.io actually used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the spawning of nodes when the requests come from the browser (IPFS Factory)

Copy link
Contributor

Choose a reason for hiding this comment

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

ah okay, thanks

@daviddias
Copy link
Contributor Author

@haadcode important notice, with the reduction of bundle size + increased performance with the introduction of webcrypto usage, we had to move any hashing operation to become async, this means that, although the interface of js-ipfs-api stays the same, the DAGNode interface of the methods .toJSON and .multihash changed to be a fully async one.

@haadcode
Copy link
Contributor

haadcode commented Oct 28, 2016

had to move any hashing operation to become async, this means that, although the interface of js-ipfs-api stays the same, the DAGNode interface of the methods .toJSON and .multihash changed to be a fully async one.

Got it. That's not ideal for the consumable API and we should, then, discuss and consider whether DAGNode is what should be returned.

What are the use cases from consumer's perspective to have it as a DAGNode instead of, say, a plain Object? What prevents us to NOT return an Object, ie. what are the arguments to return a DAGNode?

@daviddias
Copy link
Contributor Author

@haadcode now with CID's and support for multiple IPLD Formats, returning a plain object would mean that every IPLD format would need a 1:1 mapping to JSON.

Note that in the Files API you still get a JSON, this only applies to the object API

@haadcode
Copy link
Contributor

What does that mean? Can you elaborate a bit more?

@daviddias
Copy link
Contributor Author

Absolutely! So, in order to yield JSON objects from the object (and very soon, dag API), we would need for an IPFS daemon (go, js, etc) to know how to serialize that object into JSON, which might be trivial[1] with protobuf or cbor, however it doesn't hold true to other IPLD formats (git, ethereum blocks, bitcoin blocks, etc) as this would require:

  • Every IPLD format to have a .toJSON function and a .fromJSON that ensures a 1:1 mapping to and from JSON, otherwise, we wouldn't be able to guarantee that an ipfs object get $HASH | ipfs object put would yield the same $HASH.
  • Have a way to hint which is the correct data types

[1] - When I say trivial for the case of protobuf or cbor, it is actually not trivial at all, in fact, we are already 'hacking' the fact that JSON doesn't have any binary data types, while the dag-pb data field is binary, by converting the 'String' type that comes in the wire to a Buffer type, always, see: https://github.com/ipfs/interface-ipfs-core/blob/master/src/object.js#L85-L89

@daviddias daviddias merged commit f33b74f into master Oct 28, 2016
@daviddias daviddias deleted the awesome-ipld branch October 28, 2016 20:52
@haadcode
Copy link
Contributor

Are you saying that in the future go-ipfs HTTP API will return binary instead of JSON (current) for object.get?

@daviddias
Copy link
Contributor Author

@haadcode it returns JSON, but the JSON serialization casts the Buffer to a String. In the 'near future' we need a way to hint which data types are being transferred, otherwise, it will be impossible to infer at all times, as I mention above, we could only do this for dag-pb because we know the "Data" field is a Buffer, always.

@daviddias
Copy link
Contributor Author

daviddias commented Oct 29, 2016

Which can be added 'easily' by sending the serialized format, instead of a deserialized version, and deserializing at the client, enabling js-ipfs-api to return the right instance, always.

@haadcode
Copy link
Contributor

Ok. We had a call with @diasdavid to talk about this and it is indeed a complex issue across the stack.

We decided to move forwards and do two things:

  • Discuss with @whyrusleeping and other go-ipfs stakeholders if it makes sense to make the HTTP API return binary blobs instead of JSON (as we do now)
  • Discuss to see if it would make sense to add utility functions under ipfs.utils, eg. ipfs.utils.getJsonObject() similar to what we do for files

@daviddias
Copy link
Contributor Author

Discuss with @whyrusleeping and other go-ipfs stakeholders if it makes sense to make the HTTP API return binary blobs instead of JSON (as we do now)

on the new dag API, not the object API.

Another thing that we discussed is to make the dagPB.DAGNode instance more funciton than OOP, where mutating state returns a new instance, so that we avoid having to call .multihash(cb) and .toJSON(cb) every time. This has been a long hitch and now finally we've gathered a strong motivation for it.

@daviddias
Copy link
Contributor Author

For reference -> ipld/js-ipld-dag-pb#4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants