[add] Only-hash gives wrong result with wrap-with-directory

[djdv]

Version information:

go-ipfs version: 0.4.14-dev-bd32e73db
Repo version: 6
System version: amd64/windows
Golang version: go1.10rc1

Type:

Bug

Description:

Similar to #4065 except with wrap
add -n -w isn't consistent with the real outcome of add -w in some cases, here's the ones I noticed.

>ipfs add -n -r -w --pin=false --cid-version=1 "test data"
added zdj7Wmd9KWyFAMRcGcpEUdACURqNyYeQ941xecjWjEuMeVAxP test data/File 1
added zdj7WiWydjWvL84aCLtYsSXgrSgKQybkENQk1wXqgJBRReHqG test data/File 2
added zdj7WWuLweFipTVRwrq51wPksGQ6DSvHr32g7HQTMPxbv8m5r test data/File 3
added zdj7WYgq9R8Nb5ytBTgqsJNxY8diJw71icnoDdJLLVEwfdGeo test data
added QmcXqVU8WkYYLNHqNnkXvRcmuuPVMaJFJGjEv5GBD3t1LR

>ipfs add -r -w --pin=false --cid-version=1 "test data"
added zdj7Wmd9KWyFAMRcGcpEUdACURqNyYeQ941xecjWjEuMeVAxP test data/File 1
added zdj7WiWydjWvL84aCLtYsSXgrSgKQybkENQk1wXqgJBRReHqG test data/File 2
added zdj7WWuLweFipTVRwrq51wPksGQ6DSvHr32g7HQTMPxbv8m5r test data/File 3
added zdj7WYgq9R8Nb5ytBTgqsJNxY8diJw71icnoDdJLLVEwfdGeo test data
added zdj7Wjd5ruCNE29D5bm7Xi9noTLY7aw57iPAb8maser2VSuWM

>ipfs add -n -r -w --pin=false --hash=Blake2b-256 "test data"
added zDMZof1m5EigTKbJ4rSfhCbP91N4DFV857ZEZGyKQfG7zx2BFEYK test data/File 1
added zDMZof1ktLby57D5zHpR9ojy1au1qdXFRBWZsFLaMuoobbSfR7yY test data/File 2
added zDMZof1kpbehXSgoEr6A58RXnYCTSrZSHnSPj2uSgrhSYeTx8rLg test data/File 3
added zDMZof1m1wSMZakAeHE4zbgQgJ3r8NkLixv5gcuoqfhPsF87tq3f test data
added QmeN2nhSSDfPB39KmNmwCr6FVgfsoQo3CTv5G25qWMdPyZ

>ipfs add -r -w --pin=false --hash=Blake2b-256 "test data"
added zDMZof1m5EigTKbJ4rSfhCbP91N4DFV857ZEZGyKQfG7zx2BFEYK test data/File 1
added zDMZof1ktLby57D5zHpR9ojy1au1qdXFRBWZsFLaMuoobbSfR7yY test data/File 2
added zDMZof1kpbehXSgoEr6A58RXnYCTSrZSHnSPj2uSgrhSYeTx8rLg test data/File 3
added zDMZof1m1wSMZakAeHE4zbgQgJ3r8NkLixv5gcuoqfhPsF87tq3f test data
added zDMZof1kzedajkUuEZK57yyfDoDAJXBkhprX6bEEMD4AgwhQhvcm

[schomatis]

@Stebalien I'd like to work on this, but I'm not sure I'm understanding the issue correctly, the problem is the inconsistency of the hash of the directory wrapping the whole tree, right? Which should be the same for every pair of commands where the only difference is just the only-hash option?

[djdv]

@schomatis
Correct, the final hash given is inconsistent.
ipfs add -n -r -w --cid-version=1 "test data" -> QmcXqVU8WkYYLNHqNnkXvRcmuuPVMaJFJGjEv5GBD3t1LR
ipfs add -r -w --cid-version=1 "test data" -> zdj7Wjd5ruCNE29D5bm7Xi9noTLY7aw57iPAb8maser2VSuWM
They should both be the same but currently are not.

[schomatis]

@djdv Thanks for the clarification, I'll await for the green light form @Stebalien and start looking into this.

[schomatis]

It seems that the wrap-with-directory option just exposes the MFS root that it is always there, the main use of the Wrap variable seems to be (I may be misinterpreting the code) to hide or not the MFS root hash that is printed to the user.

The mentioned inconsistency is just exposed by the -w option but the core issue is why the MFS root node has a different hash in CIDv1 depending if the only-hash option is set or not (the rest of the nodes appear to have the same hash).

[schomatis]

The "empty" MFS root created for the only-hash option is based on the EmptyDirNode() function that creates a node with NodeWithData(). This function doesn't initialize the Prefix of the node and hence it will leave its Version in 0, which is the value for CIDv0.

The NodeWithData() and EmptyDirNode() could be extended to handle this particular case and create CIDv1 nodes, but most importantly (I think) there should be a revision of the implicit use of uninitialized Prefix variables as a way to indicate (a default) CIDv0.

[schomatis]

The problem with the --hash=Blake2b-256 option also stems from the fact the Prefix (and hence MhType) wasn't initialized.

[schomatis]

@djdv Would you mind testing #4755 and maybe help me write some tests for the PR?

[djdv]

@schomatis
Sure thing, I left a comment on the PR.
Thanks for taking this on!