hashAlg option of put method is not yet supported

[atfornes]

HashAlg option of the put method is not yet supported, as stated here: https://github.com/ipld/js-ipld-resolver/blob/master/src/index.js#L192

However, in the documentation appears as required, as well as in other documentations: https://github.com/ipfs/interface-ipfs-core/tree/master/API/dag#dagput

if this option is provided to the method, a message informing that it is not yet supported and the hash algorithm used (sha2-256) would improve the development experience.

[daviddias]

You are right, this is a missing feature. The actually lack of implementation is not in the check of arguments but actually passing the hash function to the CID of the resolver:

https://github.com/ipld/js-ipld-resolver/blob/master/src/index.js#L200-L205

Would you like to take a stab at it? I'll be very welcome to a PR :)

[daviddias]

See also ipld/interface-ipld-format#2

[richardschneider]

@diasdavid @vmx I would like to get this working. I think the following needs to done

• add resolver.defaultHashAlg to all implementations (Implement defaultHashAlg property interface-ipld-format#37)
• breaking change util.cid(dagNode, hashAlg, callback)
  if hashAlg is not specified (null or undefined) then use defaultHashAlg
  each implementation should return an error if the hashAlg is not supported for the format

Both dag-cbor and dag-pb should allow any hashing algorithm. Others like ethereum will error if the default is not used.

BTW, a defaultHashAlg is not really required to get this working, but it seems like a good idea.

[vmx]

@richardschneider The breaking change for util.cid() should even be that it takes the binary blob instead of the DAG node. This change would then also introduce the hashAlg option. See ipld/interface-ipld-format#32 for more information.

[richardschneider]

@vmx Good catch! I see that you already changed the util.cid spec in https://github.com/ipld/interface-ipld-format/pull/24/files. But it looks like no code implements this!

• should hashAlg be optional? I think it should always be passed?
• I would like to hold off changing dagNode to binaryBlob. One change at a time!

[richardschneider]

@vmx On second thought, I like your idea of an options argument. So the new signature is

util.cid(dagNode, callback)
util.cid(dagNode, options, callback)

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

Should ipld-dag-pb always use version 0, if the hash-alg is sha2-256?

[vmx]

should hashAlg be optional? I think it should always be passed?

I think hashAlg should be optional, this is what defaultHashAlg is for.

I would like to hold off changing dagNode to binaryBlob. One change at a time!

I can see why, but then we'd have to breaking API changes in a row, although we already know that we'll break it again. In an ideal world we'd do the defaultHashAlg change first, then doing the full cid() change (perhaps even split doing the "binary blob"-change first with options.hashAlg as a no-op, and then doing the "hashAlg" change .

To gain insight and get priorities right: is this a change you need urgently for something else, or just something that it makes sense to fix?

[richardschneider]

I'm doing a C#/.Net implementation of the Core API. I started to test my DAG API implementation against js-ipfs and found richardschneider/net-ipfs-engine#23

So I'm willing to contribute to js-ipld to improve things. But would like to take baby steps. One of my mantras is to fix one thing at a time.

[vmx]

Improving one thing at a time totally makes sense. So the things one at a time could be:

1. Add defaultHashAlg
2. Breaking change: Change cid() from DAG Node to binary blob, making options a no-op.
3. Adding options.hashAlg

[richardschneider]

I would like to swap priorities

2 is adding options to cid() and support hashAlg and version
3 is changing DAG node to blob

This way only one breaking change is added DAG node to blob.

[vmx]

Good point, let's swap it. I was too concerned about two subsequent breaking changes, but that's really not a problem if there aren't any releases in between.

[richardschneider]

Sounds like a plan!

I'll start by adding defaultHashAlg to all ipld resolvers.

[richardschneider]

hashAlg is now being passed to all resolvers.

[daviddias]

👏 👏 👏 👏 👏