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

Specifying hash algorithms in files.add doesn't work #1373

Closed
JonKrone opened this issue May 30, 2018 · 12 comments
Closed

Specifying hash algorithms in files.add doesn't work #1373

JonKrone opened this issue May 30, 2018 · 12 comments
Assignees
Labels
exp/wizard Extensive knowledge (implications, ramifications) required P1 High: Likely tackled by core team if no one steps up

Comments

@JonKrone
Copy link
Contributor

Subsystem:

files.add and/or IPLD

Type:

Bug

Severity:

Medium

Description:

jsipfs files add --hash=<alg> ... generates and stores an incorrect DAG.
The option to specify a different hash alg was added with #1308.

Go/JS ipfs hash differences:
image
The hashes are both keccak-512-encoded yet we have different hashes - our object content must be different:

image

object get <key> returns a serialized blob that has a hash different from the one requested. It seems that on unixfs import, IPLD is generating a blob where the hash is coerced from the keccak-512 format to something else. I understood dag-pb to just be a style of serialization and not a hashing function itself so I'm confused what the codec of a CID holding a keccak-512 multihash should be.

@vmx do you have some insight as to what's happening above?

I think the coercion is happening in IPLD/unixfs-engine as unixfs-engine uses IPLD to generate the DAG tree that unixfs later writes to the fs.

Could it be caused by something like new CID's assumption that non-CID Buffers are dag-pb (here). It may also be that IPLD can't yet handle these custom hash functions.

@JonKrone
Copy link
Contributor Author

JonKrone commented May 30, 2018

I don't know if this is strictly related but it strikes me as unexpected behavior of --hash as well:

sha2-256 is the default hashing algorithm yet specifying it produces different output than the standard output. For both Go and JS

image

@alanshaw
Copy link
Member

alanshaw commented Jun 1, 2018

I think @achingbrain is all over this with #1371 - @JonKrone can you verify if this PR fixes your issues?

@alanshaw
Copy link
Member

alanshaw commented Jun 1, 2018

I just realised that PR doesn't actually change any dependencies, we might need to wait for the dependent PRs to be merged.

@JonKrone
Copy link
Contributor Author

JonKrone commented Jun 1, 2018

@alanshaw cool, thanks for bringing that to my attention! That might resolve the bit about go/js having different hashes.

I think we will still be generating an incorrect DAG. object.get shows that a blob created via --hash has an incorrect encoding/format of hash and links.hash, which is different than just hashing different content.

@daviddias daviddias added exp/wizard Extensive knowledge (implications, ramifications) required P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked labels Jun 4, 2018
@vmx
Copy link
Member

vmx commented Jun 6, 2018

@JonKrone
Copy link
Contributor Author

JonKrone commented Jun 6, 2018

Thanks for the confirmation @vmx. Does this mean that we aren't currently able to support #1308? Something like: jsipfs files add --hash=sha3-512

@vmx
Copy link
Member

vmx commented Jun 7, 2018

@JonKrone I'd be surprised if it would work.

@JonKrone
Copy link
Contributor Author

JonKrone commented Jun 9, 2018

@alanshaw As this feature is broken and has a more involved fix with no timeline, I think it should be reverted or we should otherwise indicate that it's not ready for use.

@alanshaw
Copy link
Member

alanshaw commented Aug 2, 2018

@JonKrone I believe this is now resolved - #1371 was merged and IPLD now supports specifying the hash algorithm ipld/js-ipld#133

Are you able to double check?

@alanshaw alanshaw closed this as completed Aug 2, 2018
@ghost ghost removed the status/ready Ready to be worked label Aug 2, 2018
@JonKrone
Copy link
Contributor Author

JonKrone commented Aug 3, 2018

Hey @alanshaw!

I was excited when I saw this but after fresh installs of the most recent go and js versions, I still see mismatched hashes for the add --hash=keccak-512 command and see that the object stored by js-ipfs still lists a CIDv0 hash

@achingbrain
Copy link
Member

@JonKrone which version are you using? I see CIDv1 with --hash=keccak-512:

$ jsipfs add foo.txt --hash=keccak-512
added zBurK9v3LNabF2nosKPjnPWznRAQRjAEuRLx2Mbsa2qVtrYRA48dYybAXUGsXUhB1KQipPnYie1mQmnnxYik1DHThzReM foo.txt
$ jsipfs --version
0.31.1

@JonKrone
Copy link
Contributor Author

JonKrone commented Aug 7, 2018

@achingbrain Yep! And that part works! What seems to fail is:

jsipfs object get of a CIDv1 returns a blob with a v0 hash property.

I think the part of the add stream where this happens is when unix-fs passes off to IPLD, as unix-fs is storing the object under the correct CID but the blob it stores is incorrect.

I'm using 0.31.2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/wizard Extensive knowledge (implications, ramifications) required P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

No branches or pull requests

5 participants