fix: dht validate if receiving stream #890
Conversation
ghost
assigned
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
5 times, most recently
from
79e1c4a to
bc9f037
Compare
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
from
bc9f037 to
390699a
Compare
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
8 times, most recently
from
d6c24c5 to
173950c
Compare
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
from
173950c to
eb54a85
Compare
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
from
eb54a85 to
f7f0c86
Compare
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
from
f7f0c86 to
f703c17
Compare
src/dht/findpeer.js
Outdated
| addrs = res.responses[0].addrs | ||
| } | ||
|
|
||
| // inconsistencies js / go - go does not add `/ipfs/{id}` to the address |
There was a problem hiding this comment.
Do we need to fix this here?
There was a problem hiding this comment.
So, go-ipfs response contains only address. As a consequence, it will fail the following test:
ipfs/interface-ipfs-core/js/src/dht/findpeer.js#L48
I can change the test in interface-ipfs-core and remove this if you feel that it is less hacky.
There was a problem hiding this comment.
What I'm concerned about is that we're making assumptions about the format of the address. What does go-ipfs return when it's a circuit address? Would that cause a false positive?
Ideally both implementations should return the same format! Do you know the reason why go-ipfs returns the addr without the peer ID? Perhaps for a smaller payload? It would be pretty easy for js-ipfs to decapsulate the peer id from the addrs wouldn't it?
src/dht/findpeer.js
Outdated
| } else { | ||
| id = res.responses[0].id | ||
| addrs = res.responses[0].addrs | ||
| } |
There was a problem hiding this comment.
Apologies if I wasn't clear, interface-ipfs-core should return lowered property names but the HTTP API should be compatible with go-ipfs.
Here I expect us to be converting "Uppered" property names to lowered form. In js-ipfs the HTTP API should take the lowered property names from core and "Upper" them to be compatible. I know this is a PITA but this is consistent with all our APIs.
There was a problem hiding this comment.
Will change it! Thanks for clarifying.
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
3 times, most recently
from
268be9a to
bc35a9c
Compare
|
|
src/dht/findpeer.js
Outdated
| return peerInfo | ||
| }) | ||
|
|
||
| callback(null, responses) |
There was a problem hiding this comment.
Needs switch to returning a single response :D
vasco-santos
force-pushed
the
fix/dht-validate-if-receiving-stream
branch
from
bc35a9c to
c9dabf2
Compare
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
With
go-ipfswe receive a stream of data. However, we currently do not receive a stream usingjs-ipfs. Types of notifications ingo-ipfsare defined here.Needs: