lightningdevkit / lightning-liquidity

Other
27 stars 17 forks source link

initial implementation of transport layer #2

Closed johncantrell97 closed 1 year ago

johncantrell97 commented 1 year ago

As title says, just a first pass on implementing the LSPS transport layer. Looking to get feedback on this direction.

I thought we would want to expose the raw jsonrpc::Request/Response/Notification interface to library users because each LSP will likely end up supporting different subset of the full spec and probably custom extensions. This is why I added the LSPMessageHandler trait that lets someone implement handlers for it. I assume when we implement LSPS request-based protocols we'd provide a handler that handles the logic for that particular protocol.

I'm not so sure on this LSPManager object. My thought was it handled the layer of going from raw string <==> rpc request/response/notification for the user. It would also expose some api to a higher level and perform whatever bookkeeping is necessary (managing pending request's / ids for them?).

I could use some ideas for how to proceed from here. I probably should think about how a user of the library will want to use it and work backwards from there.

johncantrell97 commented 1 year ago

Thanks for the initiative!

After a first pass, I'm not entirely convinced we should expose the Json objects, but rather add helper methods on LSPSMessage to customize them. I think having an interface trait to interact is a good idea though.

Hm okay. I agree it's not really necessary for standard jit_channel and channel_request protocols. I was thinking further down the road the client/lsps might end up with custom protocols or protocol extensions that use jsonrpc and they'd need access to the raw jsonrpc requests to be able to implement it. I guess it would almost be like the LDK CustomMessageHandler where this library provides handlers for jsonrpc requests that are standard and part of the LSPS and if we receive jsonrpc requests that are non-standard and they provided a CustomJsonRPCRequestHandler or something like that then we could push them to that to handle.

I'm not so sure on this LSPManager object. My thought was it handled the layer of going from raw string <==> rpc request/response/notification for the user. It would also expose some api to a higher level and perform whatever bookkeeping is necessary (managing pending request's / ids for them?).

Yes, I'm also not sure yet, we probably still want to keep it flexible. Also, even though it's very early for that, I want to note that we probably want to have some tests checking that our CMH implementation works nicely with lightning-custom-message to allow users to combine multiple message handlers.

Yeah, it is hard to develop this completely bottom-up so I started another branch to start imagining how I would want to use it as a user of the library (what apis and events I would want to react to and call into) and I think that will help inform how the transport and end-user should connect and what the library needs to handle vs the end-user.

johncantrell97 commented 1 year ago

Ok, just pushed an updated version of this that allows the user to register multiple LSPMessageHandlers (one per prefix) to the LSPManager / CustomMessageHandler.

This means each protocol the user wants to support can be registered independently on the custom message handler and messages will be delegated to the appropriate handler based on the prefix.

I extended the trait by adding a get_and_clear_pending_msg method so that these protocol message handlers can generate their own custom messages that will get sent to the counterparty.

I also added a get_protocol_number method so the LSPS0 handler could implement the listprotocols method by iterating the registered handlers and collecting all of the protocol numbers it supports.

johncantrell97 commented 1 year ago

I pushed another commit that is an attempt at your idea of hiding jsonrpc types from the protocol level message handlers. I am finding it extremely awkward to go this route. the commit is not complete but hopefully illustrates some of the pain.

curious to hear if you think this path is really worth going down and if so, any ideas on how to avoid all the pain?