Luminarys / synapse

Synapse BitTorrent Daemon
https://synapse-bt.org
ISC License
853 stars 48 forks source link

Using tungstenite in synapse RPC #160

Closed markpash closed 4 years ago

markpash commented 4 years ago

I've been reading over the RPC code in synapse and I wonder if it would make sense to use a websocket crate like tungstenite to handle the the sockets and connections and the read/write implementations. I look forward to hearing opinions on this.

kpcyrd commented 4 years ago

sycli is using tungstenite for websockets, the rpc crate just contains the structs though. You might be interested in #159 but you might mean something else, I'm not sure what "the read/write implementations" is referring to :)

markpash commented 4 years ago

sycli is using tungstenite for websockets, the rpc crate just contains the structs though. You might be interested in #159 but you might mean something else, I'm not sure what "the read/write implementations" is referring to :)

I'm referring the the RPC component of the daemon, found here.

Luminarys commented 4 years ago

When I was initially writing the code I believe I looked at websocket libraries and couldn't find any that could handle async parsing the way I wanted, but giving tungstenite's docs another look now I do believe it could work.

Are you interested in taking a look at this @MarkPash?

markpash commented 4 years ago

When I was initially writing the code I believe I looked at websocket libraries and couldn't find any that could handle async parsing the way I wanted, but giving tungstenite's docs another look now I do believe it could work.

Are you interested in taking a look at this @MarkPash?

I would be interested in working on it yes, though it does look a little tricky and I may need some guidance in certain places. :)

Would it be better to pursue creating tests for synapse first before I look into altering the RPC code?

Luminarys commented 4 years ago

Yea happy to help with that. Feel free to ping me on Rizon (same handle). What sort of tests are you referring to here? I've run the whole autobahn suite against the websocket implementation, though I'm not sure how extensive that is when it comes to testing handshakes (which I suspect may have some issues).

Luminarys commented 4 years ago

I'm going to look at this more seriously now that I've already started the work of factoring out TLS and other websocket code.

Luminarys commented 4 years ago

So it turns out this was already actually done some time ago, closing this out.