chore(dht): add API to allow options in findProvs()

[0x-r4bbit]

As discussed in: ipfs/js-ipfs#1322 (comment)

License: MIT
Signed-off-by: Pascal Precht pascal.precht@gmail.com

[0x-r4bbit]

I'd like to add a test for this, but I'm not sure how to test this timeout.

Appreciate any pointers, in case anybody wants this as well :)

[alanshaw]

I'd set the timeout really low and give it a hash that no-one will have, the callback should be called with an error before mocha times out the test.

[0x-r4bbit]

Okay, I'm looking into this now and - sorry if this is a dumb question - but I'm wondering how I should know what hash would be a hash that no one will have? I mean I can't really know this upfront right?

Or is it enough to know, that no one of the connected peers will have it in the testing scenario, which in this case would be nodeB, I guess? Considering this already existing testing setup:

common.setup((err, factory) => {
        expect(err).to.not.exist()

        spawnNodesWithId(2, factory, (err, nodes) => {
          expect(err).to.not.exist()

          nodeA = nodes[0]
          nodeB = nodes[1]

          connect(nodeB, nodeA.peerId.addresses[0], done)
        })
})

What I've tried now, was to create an empty dagNode object within nodeB to get hold of its CID, but I don't actually announce the content in the network with provide():

it('should take options to override timout config', function (done) {
  const options = {
    timeout: 1
  }

  waterfall([
    (cb) => nodeB.object.new(cb),
    (dagNode, cb) => {

      // Notice I'm not announcing the hash in the network using `provide()`
      // but simply return CID

      const cidV0 = new CID(dagNode.toJSON().multihash)
      cb(cidV0)
    },
    (cidV0, cb) => nodeA.dht.findprovs(cidV0, options, (err) => {

      // I'd expect this to be truthy as nodeA shouldn't find a provider 
      // for the given hash

      expect(err).to.exist()
      cb(err)
    }),
  ], done) // this fails due to non-error being passed to done
})

So this test fails because done() doesn't get called with an error object, which means that findProvs doesn't throw and error here.

Can nodes find providers, even though other nodes didn't announce their content to the network?

[alanshaw]

I think that by adding content via object.new that you become a provider for that CID automatically.

We have a helper for the webui tests that creates a CID from random bytes without adding content to IPFS. Perhaps something similar here might work?

https://github.com/ipfs-shipyard/ipfs-webui/blob/revamp/test/helpers/cid.js

[0x-r4bbit]

Yes, that could work. I'll give it a spin

[alanshaw]

Nitpick - could this please be changed to ipfs.dht.findprovs(hash, [options], [callback])? The other way makes it look like an array which I think is a bit confusing!

[0x-r4bbit]

Of course! I honestly wasn't sure what the right syntax expression would be here. Will update accordingly

[0x-r4bbit]

@alanshaw maybe you can take another look here... either something's off with waterfall or I don't get the API 🙈

So this test above fails (again) with:

Error: done() invoked with non-Error: {"codec":"dag-pb","version":0,"hash":{"type":"Buffer","data":[18,32,10,127,75,184,57,207,189,183,14,51,90,127,23,252,203,8,162,154,95,81
,255,167,31,137,30,83,10,25,73,32,51,7]}}

Even though, the last callback is invoked with null. The same happens btw. when invoking cb() without any arguments. Now, if I change waterfalls main callback from

done

to

(err, result) => {
  done()
}

Everything works fine. But I doubt it's supposed to be like that, considering how all other test specs look like..

[alanshaw]

The issue is that fakeCid returns a CID in the error position of the callback

[alanshaw]

Unused?

[0x-r4bbit]

Yeap, that's a left over

[alanshaw]

cb(null, new CID(0, 'dag-pb', mh))

[0x-r4bbit]

Ah! Thanks!!

And also... D'oh! 🤦‍♂️

[alanshaw]

The issue is that fakeCid returns a CID in the error position of the callback

[0x-r4bbit]

@alanshaw thanks again for taking a look.

This is now updated and passes tests on js-ipfs.