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

fix: missing id name constant #57

Merged
merged 1 commit into from
Aug 8, 2018
Merged

Conversation

vasco-santos
Copy link
Member

As go specifies the multihash ID constant, we should do the same here.

It is used for libp2p-id, and consequently for IPNS

@vasco-santos vasco-santos requested a review from daviddias August 8, 2018 16:23
@daviddias daviddias merged commit 42b9f0c into master Aug 8, 2018
@daviddias daviddias deleted the fix/missing-id-name-constant branch August 8, 2018 16:42
@achingbrain
Copy link
Member

Weirdly now this module exports a hash name that it can't use to hash a buffer after this change. Not sure if that was intentional:

const mh = require("multihashes")
const names = Object.keys(mh.names)

mh.encode(Buffer.from([0, 1, 2, 3]), names[0]) //  Error: Unrecognized hash function named: id

Just discovered this because the ipfs-unixfs-engine module arbitrarily use the first 40 hashes exported by this module in some of it's tests.

Also, since this is for go compatibility, id should be added to the codes and defaultLengths objects too since go does here and here.

@vasco-santos
Copy link
Member Author

Hello @achingbrain , sorry for the late answer

IPNS needs to verify if a multihash represents an ID. This way, it was added as a multihash name, the same way as go does. And yes, I agree that it should be added to the codes and defaultLengths. I can create a new PR adding them.

Regarding the ipfs-unixfs-engine, I believe that the name constants have the purpose of identifying types of multihashes, and consequently, not only hash algorithms. So, I think that we should obtain a set of hashing algorithms in a different fashion for those tests. What do you think?

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