Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: Add support for specifying hash algorithms in files.add #1308

Merged

Conversation

vasco-santos
Copy link
Member

Since PR 1005 has been blocked for a while, I went through it.

As the PR origin is from an external organization, I created a new branch instead of rebasing it in its origin. Shortly, I got the @alanshaw (thanks :D ) implementation with js-ipfs master branch and made a few changes, as a consequence of the modifications that occurred meanwhile in components/objects, as well as in resources and a minor refactor.

Finally, I added tests for several hash algorithms, since there are more than 300. Should I add more to the tests?

@ghost ghost assigned vasco-santos Apr 12, 2018
@ghost ghost added the status/in-progress In progress label Apr 12, 2018
'keccak-256',
'keccak-384',
'keccak-512'
]
Copy link
Member

Choose a reason for hiding this comment

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

@vasco-santos good enough :)

const runOnAndOff = require('../utils/on-and-off')

// TODO: Test against all algorithms Object.keys(mh.names)
Copy link
Member

Choose a reason for hiding this comment

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

No need :)

@daviddias daviddias merged commit a2954cb into master Apr 12, 2018
@ghost ghost removed the status/in-progress In progress label Apr 12, 2018
@daviddias daviddias deleted the feat_add-support-for-specifying-hash-algorithm-in-files-add branch April 12, 2018 11:40
@JonKrone
Copy link
Contributor

JonKrone commented May 28, 2018

Hey @vasco-santos. I've been working with this this weekend and have run into a few pieces that I'd love your thoughts on:

go/js ipfs hash differences:
image
Shouldn't the hashes be the same?

DAG traversal
image

We can request the specific DAGNode if we know it's hash but it's not possible to traverse the DAGLinks of something added with a non-default algorithm. Seems like IPLD isn't aware that the links are of a different alg, they may be casting all hashes to dag-pb with something like new CID(0, 'dag-pb', <hash from specific algorithm>).

Do you know what's wrong here? Wanted to run this by you before creating an issue.

@vasco-santos
Copy link
Member Author

Hey @JonKrone

Thanks for reaching out. I tested across encoding + decoding and it seemed good. However, I tested your example, as well as using other hash algorithms, but the problem persists.

At a first sight, your suggestion seems to be the problem. Would you mind to create the issue and verify it?

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