Conversation
09703c3 to
bdb6563
Compare
bdb6563 to
7575ec9
Compare
js/src/dht/findpeer.js
Outdated
| expect(err).to.not.exist() | ||
| // TODO upgrade the answer, format is weird | ||
| expect(peer[0].Responses[0].ID).to.be.equal(nodeB.peerId.id) | ||
| expect(res.Responses[0].Addrs).to.deep.include(nodeB.peerId.addresses[0]) |
There was a problem hiding this comment.
is deep necessary - the values are just strings right?
There was a problem hiding this comment.
Can we please fix the object that is returned here to not have go-style uppercased first letter properties? - for consistency with the rest of the API.
There was a problem hiding this comment.
According to http api docs, the addrs is an array of addresses. that's why I am using the deep.include.
Changed the uppercase letter. However, the inconsistencies will have to be solved in ipfs-api in ipfs/js-ipfs-api#890. go-ipfs sends the uppercase style, and js-ipfs (aiming to have its core implementation compliant with this interface) must send its response in lowercase. As a result, ipfs-api using js-ipfs will receive the lowercase ones. I added the logic to deal with this inconsistency in the ipfs-api PR, but I am not sure if it is the better approach in this case.
js/src/dht/findprovs.js
Outdated
| (cidV0, cb) => nodeA.dht.findprovs(cidV0, cb), | ||
| (provs, cb) => { | ||
| expect(provs.map((p) => p.id.toB58String())) | ||
| expect(provs.Responses.map((p) => p.ID)) |
There was a problem hiding this comment.
Can we please fix the object that is returned here to not have go-style uppercased first letter properties? - for consistency with the rest of the API.
There was a problem hiding this comment.
Documentation or implementation for dht.findprovs needs to be updated - provs is not an array of PeerInfo instances.
https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/DHT.md#dhtfindprovs
7a510f9 to
685ba00
Compare
886ddd1 to
f258ad7
Compare
f258ad7 to
8eead51
Compare
alanshaw
left a comment
There was a problem hiding this comment.
Looks great, just one small change
js/src/dht/findprovs.js
Outdated
| (dagNode, cb) => { | ||
| const cidV0 = new CID(dagNode.toJSON().multihash) | ||
| const cidV0 = new CID(dagNode.toJSON().hash) | ||
| nodeB.dht.provide(cidV0, (err) => cb(err, cidV0)) |
There was a problem hiding this comment.
object.new now returns a CID
8eead51 to
6db447f
Compare
In the context of the ipfs/js-ipfs#856 and ipfs/js-ipfs-api/pull/890