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

Discrepancy between util.serialize and DAGNode.create #28

Closed
mitar opened this issue Feb 27, 2017 · 7 comments
Closed

Discrepancy between util.serialize and DAGNode.create #28

mitar opened this issue Feb 27, 2017 · 7 comments
Labels
kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked

Comments

@mitar
Copy link
Contributor

mitar commented Feb 27, 2017

I am seeing strange discrepancy between util.serialize and DAGNode.create. Maybe I am using it wrong.

I have the following object:

obj = { data: '{"Luck":0.4,"Proof":"<TEE signature>"}',
  links: 
   [ { multihash: 'QmUxD5gZfKzm8UN4WaguAMAZjw2TzZ2ZUmcqm2qXPtais7',
       name: 'payload',
       size: 819 } ] }

If I do:

DAGNode.create(obj.data, obj.links, 'sha2-256', ...)

Then the JSON output of DAGNode is:

 { data: '{"Luck":0.4,"Proof":"<TEE signature>"}',
  links: 
   [ { name: 'payload',
       size: 819,
       multihash: 'QmUxD5gZfKzm8UN4WaguAMAZjw2TzZ2ZUmcqm2qXPtais7' } ],
  multihash: 'QmYe6tCaRi5M2b64uTU4ikugBQvAq2jBrwBT1rqppvNFpi',
  size: 909 }

But if I do util.serialize and then util.deserialize, and do JSON output, then I get:

{ data: <Buffer 7b 22 4c 75 63 6b 22 3a 30 2e 34 2c 22 50 72 6f 6f 66 22 3a 22 3c 54 45 45 20 73 69 67 6e 61 74 75 72 65 3e 22 7d>,
  links: 
   [ { name: 'payload',
       size: 819,
       multihash: '9tmuqeLc7iDENbPatFnZDFRjVqf5f2itXQo4RLUu3LkjwPmGXnB4fqP2hd248Wa' } ],
  multihash: 'QmbVLF93bYdDoxctYAihFY1wFVQVFouG5wdZ39acA9vfjp',
  size: 921 }

Observe how multihash of links is different, and size value of the whole JSON.

@daviddias
Copy link
Member

#11 fixed this one, essentially, data is always expected to be a Buffer, we just didn't have a strict conversion. Please test master and let me know.

@mitar
Copy link
Contributor Author

mitar commented Mar 16, 2017

No, this is now fixed. See the following reproduction:

const dagPB = require('ipld-dag-pb');
const obj = { data: '{"Luck":0.4,"Proof":"<TEE signature>"}',
  links: 
   [ { multihash: 'QmUxD5gZfKzm8UN4WaguAMAZjw2TzZ2ZUmcqm2qXPtais7',
       name: 'payload',
       size: 819 } ] };
dagPB.DAGNode.create(obj.data, obj.links, 'sha2-256', (error, dag) => {console.log(dag.toJSON())});
dagPB.util.serialize(obj, (error, serialized) => {dagPB.util.deserialize(serialized, (error, dag) => { console.log(dag.toJSON())})});

See the difference in multihash and size fields.

Tested with ipld-dag-pb@0.10.0.

@daviddias daviddias added the kind/bug A bug in existing code (including security flaws) label Mar 17, 2017
@daviddias
Copy link
Member

See the different in the multihash of the link

// your code, just indented
const dagPB = require('../src')

const obj = {
  data: '{"Luck":0.4,"Proof":"<TEE signature>"}',
  links: [{
    multihash: 'QmUxD5gZfKzm8UN4WaguAMAZjw2TzZ2ZUmcqm2qXPtais7',
    name: 'payload',
    size: 819
  }]
}

dagPB.DAGNode.create(obj.data, obj.links, 'sha2-256', (err, dagNode) => {
  if (err) {
    throw err
  }
  console.log('create', dagNode.toJSON())
})

dagPB.util.serialize(obj, (err, serialized) => {
  if (err) {
    throw err
  }
  dagPB.util.deserialize(serialized, (err, dagNode) => {
    if (err) {
      throw err
    }
    console.log(dagNode.toJSON())
  })
})
» node create.js
create { data: <Buffer 7b 22 4c 75 63 6b 22 3a 30 2e 34 2c 22 50 72 6f 6f 66 22 3a 22 3c 54 45 45 20 73 69 67 6e 61 74 75 72 65 3e 22 7d>,
  links:
   [ { name: 'payload',
       size: 819,
       multihash: 'QmUxD5gZfKzm8UN4WaguAMAZjw2TzZ2ZUmcqm2qXPtais7' } ],
  multihash: 'QmYe6tCaRi5M2b64uTU4ikugBQvAq2jBrwBT1rqppvNFpi',
  size: 909 }
{ data: <Buffer 7b 22 4c 75 63 6b 22 3a 30 2e 34 2c 22 50 72 6f 6f 66 22 3a 22 3c 54 45 45 20 73 69 67 6e 61 74 75 7265 3e 22 7d>,
  links:
   [ { name: 'payload',
       size: 819,
       multihash: '9tmuqeLc7iDENbPatFnZDFRjVqf5f2itXQo4RLUu3LkjwPmGXnB4fqP2hd248Wa' } ],
  multihash: 'QmbVLF93bYdDoxctYAihFY1wFVQVFouG5wdZ39acA9vfjp',
  size: 921 }

The issue is that you are passing a JSON object to serialize, where it expects a DAGNode instance. If you pass that dagNode you get from create, you get the right hashes.

It seems that we could indeed support both, currently, the util.serialize expects for the multihash of the links to be a Buffer and not a Base58 encoded string, should be easy to add a check and decode it if necessary. Wanna PR that in?

@mitar
Copy link
Contributor Author

mitar commented Mar 17, 2017

I think I got confused because in js-ipld-dag-cbor documentation you are passing to serialize simply an object, not dag node instance. So when I switched to js-ipld-dag-pb I expected it to behave the same.

I would assume both libraries are using the same API, just a different serialization.

Sadly, I do not think I will have time for PR at the moment.

@daviddias
Copy link
Member

I would assume both libraries are using the same API, just a different serialization.

It is the same API, just dag-cbor takes json (because of the one to one mapping) and dag-pb takes DAGNode classes (pb aware objects).

Nevertheless, it is indeed possible to achieve what you tried. I'll leave this issue open as a feature request, once my availability increases, I'll look into it again :)

@atvanguard
Copy link
Contributor

@diasdavid Rolled out a PR. PTAL.

@daviddias daviddias added the status/ready Ready to be worked label Oct 13, 2017
@daviddias
Copy link
Member

Thanks @atvanguard ! Let's continue tracking this on that PR #43 👍

daviddias pushed a commit that referenced this issue Dec 2, 2017
…em [Fi… (#43)

* If link hashes to serializer are base58 encoded, then decode them [Fixes #28]

* Add test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

3 participants