Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ILP over WebSockets #326

Closed
5 tasks
emschwartz opened this issue Sep 24, 2019 · 9 comments
Closed
5 tasks

ILP over WebSockets #326

emschwartz opened this issue Sep 24, 2019 · 9 comments
Labels

Comments

@emschwartz
Copy link
Member

@kincaidoneil requested supporting ILP-over-Websockets, along the lines of what @adrianhopebailie proposed in https://github.com/adrianhopebailie/ilp-transport. It shouldn't be too hard to support this. We would need to:

  • Decide on the number format used for the correlation ID (is u32 good enough?)
  • Decide whether the endpoint would be /websocket, /ws/, /ilp/websocket, /ilp/ws, or something else
  • Figure out how to support authentication for browser-based clients (possibly using the Sec-Websocket-Protocol header as suggested here, which is apparently supported by the browser's WebSocket API) in addition to normal HTTP auth headers for other clients
  • Implement a warp filter for the server side (very easy)
  • Implement the client for it (slightly more complex because we need the API to also create new client connections when accounts are added, like Connect to BTP server when account is added with a btp_uri field #285 for BTP)
@adrianhopebailie
Copy link
Collaborator

Figure out how to support authentication for browser-based clients (possibly using the Sec-Websocket-Protocol header as suggested here, which is apparently supported by the browser's WebSocket API) in addition to normal HTTP auth headers for other clients

I'd recommend just using cookies.

i.e. POST to a /login endpoint using the bearer token in the headers. That endpoints should auth the client and set a cookie. Then open a WS to /ilp. The browser will pass the cookie in the handshake (although you may need to verify this in some cases because there's some lock-down of cross-origin cookies in Safari and Firefox these days)

There's an old issue requesting that Auth headers can be added via the browser WS API, maybe add some support for it too?
https://discourse.wicg.io/t/headers-support-for-websocket-upgrade-request/1575/3

@emschwartz
Copy link
Member Author

What's the advantage of using cookies over the Sec-Websocket-Protocol header? As far as I can tell, that's part of the browser websocket standard and it seems slightly more straightforward to have the server look for the auth details in one of two headers that may be present in the initial connection, rather than having a second endpoint just to support browser-based clients.

@kincaidoneil
Copy link
Member

What's the advantage of using cookies over the Sec-Websocket-Protocol header?

The cookie flow Adrian described is much more common, but the Sec-Websocket-Protocol header approach seems simpler to implement (on both sides), since we wouldn't need to support sessions.

@adrianhopebailie
Copy link
Collaborator

What's the advantage of using cookies over the Sec-Websocket-Protocol header?

Using that header is a hack. That header is intended for sub-protocol negotiation (which we should probably use since we're designing a sub-protocol) not for arbitrary data.

@sublimator
Copy link
Member

Apologies if I'm not really understanding the reasons/implications for/of this. Sharing some experience from using BTP/STREAM.

If you go down the path of making a replacement for BTP, it might be worth considering being able to use one socket for multiple Plugins (or whatever other interface for bidirectional RPC)

Currently the createConnection(...) export from ilp-protocol-stream which will generally be passed a Plugin with an underlying BTP WebSocket actually disconnects the Plugin upon connection close.

https://github.com/interledgerjs/ilp-protocol-stream/blob/9b49b1cad11d4b7a71fb31a8da61c729fbba7d9a/src/index.ts#L69-L74

Essentially this means you can't reuse a socket for even consecutive STREAM connections, let alone parallel.

It's not a huge deal, but the Coil WM extension currently spawns new WebSockets every time it changes streaming context (i.e. focus tab to another WM page). You could debate how useful it would be in practice to be able to recycle sockets, but it would be nice to have the option. For example, it would be nice to be able to gracefully shutdown (maybe simply pause it for a configurable period before completely closing) a connection, while starting up another on context change.

It looks like the batch-id from ilp-transport would get part of the way there, but the definition seems to imply that only one batch can be active at a time.

@kincaidoneil
Copy link
Member

If you go down the path of making a replacement for BTP, it might be worth considering being able to use one socket for multiple Plugins (or whatever other interface for bidirectional RPC)

Yep! This will enable you to use a single socket for ILP packets and multiple settlement engines (successor to plugins) simultaneously, for each peer.

The rationale is this simplifies the framing, since we can just use ILP packets instead of BTP packets (ILP packets are used for framing settlement messages in ILP-over-HTTP, so this makes everything more consistent), and the auth is no longer weird & complex where the connection is opened, then dropped if the token isn't provided via a subprotocol.

Currently the createConnection(...) export from ilp-protocol-stream which will generally be passed a Plugin with an underlying BTP WebSocket actually disconnects the Plugin upon connection close.

We also had issues with this. We had to wrap the plugin in another plugin, overriding disconnect so Stream didn't disconnect it. But that should probably be fixed

@sublimator
Copy link
Member

@kincaidoneil
Thanks the info! How will this effort fit in with STREAM ? Will this be a transport layer that STREAM can use ?

@adrianhopebailie
Copy link
Collaborator

Based on the work done so far I've taken a stab at a new protocol definition:
https://github.com/adrianhopebailie/ilp-protocol-pxp

It's super-simple and designed to work via either WebSockets or QUIC.

I need to do some experiments to ensure all assumptions hold but I don't think there is anything in there likely to be an issue.

Next step is some basic client/server implementations and then migrations for BTP.

Please log issues and PRs if you have suggestions or comments.

@emschwartz
Copy link
Member Author

I'm going to close this for now. If we want to pursue this in the future, I think it should be proposed in the RFCs first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants