Open vojtechsimetka opened 4 years ago
Trying the latest "stable" version of the library throws several incompatibility issues with other modules (the idea was to integrate our code, not fixing their integration). I think the wisest option is to wait a little bit more so we don't waste time fixing their integration issues. There's a pre-release version in the works. I suggest trying this task next sprint once that release is stable. I'll continue with the library extraction task
That makes sense. May I also suggest to do #12 ? Please coordinate on the library with @leandroyalet
The new class with the protocol primitives (where we added the new sendMessage API method)
To have in the radar when attacking this issue, when reusing a peer-id (key) it needs to be added in the bundle.
This is an idea that I have in mind: is to extends js-libp2p instead of forking it, and provide our own dht implementation (https://github.com/libp2p/js-libp2p/blob/29a96690ad1cfb27373e798d42cc11bacbd9be33/doc/CONFIGURATION.md#dht) in that case we are free to add any extension to the kademlia algorithm.
This steps should be in rif-communications-js:
Besides we are responsible for maintaining https://github.com/rootstock/js-libp2p-kad-dht and https://github.com/rootstock/k-bucket forks
IMHO this is the clearest solution because this would prevent constant synchronization in the libp2p core..
This makes sense for you? @raullaprida @vojtechsimetka
This is an idea that I have in mind: is to extends js-libp2p instead of forking it, and provide our own dht implementation (https://github.com/libp2p/js-libp2p/blob/29a96690ad1cfb27373e798d42cc11bacbd9be33/doc/CONFIGURATION.md#dht) in that case we are free to add any extension to the kademlia algorithm.
This steps should be in rif-communications-js:
- require original js-libp2p
- enhance libp2p module https://github.com/libp2p/js-libp2p/blob/master/src/content-routing.js#L18 with our sendMessage method (in other words this is a hack)
- require our own dht library https://github.com/rootstock/js-libp2p-kad-dht#stable
Besides we are responsible for maintaining https://github.com/rootstock/js-libp2p-kad-dht and https://github.com/rootstock/k-bucket forks
IMHO this is the clearest solution because this would prevent constant synchronization in the libp2p core..
This makes sense for you? @raullaprida @vojtechsimetka
The first link says: If this DHT implementation does not fulfill your needs and you want to create or use your own implementation, please get in touch with us through a github issue. We plan to work on improving the ability to bring your own DHT in a future release
The ability to bring your own DHT is not there. I created an inssue in their github proposing forwarding kademlia capabilities.. If that is well received then whe could propose a PR.
Regarding your proposal:
why is it in rif-communication-js and not in rif-communication-node ?
- I would need to know how this hack works and what it solves (I see we change one syncronization to libp2p of a few lines with maintaining a hack solution).
the process is described on the first answer https://stackoverflow.com/questions/24517744/extending-or-adding-functions-to-modules-of-node
why is it in rif-communication-js and not in rif-communication-node ?
Because rif-communications-js is the wrapper of libp2p library and hides all the customization provided by RIF
mmm, rif-communications-node uses libp2p.
Shouldn't rif-communications-js use rif-communications-node?
No the other way around. rif-communications-node
should use the rif-comunications-js
(which is a library). This makes it possible for anyone to connect to our RIF Communications network from within their code.
Ideally the node
and the chat app
should not have libp2p in the dependencies at all. Only @rsksmart/rif-communications
(which is the JS one)
Mayeb to make it clear we should not have the libp2p update here but in the library only. But please see https://github.com/rsksmart/rif-communications-js/issues/1
Description
libp2p is undergoing big rewrite from callbacks to promises. The current code would not work with the latest version
Tasks
ncu
or autopugrade withncu -u
)