-
Notifications
You must be signed in to change notification settings - Fork 15
feat: Rewrite from SIO+ws-relay to protobuf+p2p-circuit #43
Conversation
(If the code does not work this might be because it says draft and thats because I didn't test)
PR is ready for review |
@libp2p/javascript-team Anybody mind reviewing this (and server pr)? Can I just merge that? |
@mkg20001 I need to review this PR and make sure it follows the model we have to achieve with libp2p/js-libp2p#130. I plan to prioritize that and do it this week. |
message IdentifyResponse { | ||
required string id = 1; | ||
required string pubKey = 2; | ||
required bytes signature = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not have all the same fields as the identify protocol?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the same identify protocol. Additionally in the libp2p-identify protocol the public keys never get exchanged.
Status Update:This PR is currently blocked on libp2p/js-libp2p#159 which will bring things like DI setup to libp2p. Lot's of work happening there. Also, we will only be confident to move into a p2p-circuit solution once ipfs/interop#6 is completely done. Things that are part of this PR that are not part of the migration to p2p-circuit should happen in a separate PR, i.e Fixing memory leaks. @mkg20001 can you do that? |
@diasdavid The memory leaks are mainly socket.io's fault. |
Check out https://www.npmjs.com/package/pull-ws which is the one we use for https://github.com/libp2p/js-libp2p-websockets |
I think this is no longer needed |
This PR is a full rewrite of libp2p-websocket-star.
Major changes:
Server pr: libp2p/js-libp2p-websocket-star-rendezvous#11