lightningdevkit / lightning-liquidity

Other
27 stars 17 forks source link

LSPS0 message handling #4

Closed johncantrell97 closed 1 year ago

johncantrell97 commented 1 year ago

Implements LSPS0 message handling and lays groundwork for other LSPS protocol handling.

johncantrell97 commented 1 year ago

Ok, addressed all of your comments and updated CI to run against 1.48.0 successfully.

johncantrell97 commented 1 year ago

I made the minor nit doc changes and linked where made sense. I added a little bit more information to LSPManager around expected usage. The problem is that in this version there's really no real use without any of the protocols added. There's no events or even public api to use.

The only outstanding "issue" imo is naming. The only exposed objects are the LSPConfig and LSPManager. "LSP" typically refers to the server, the one doing the providing. I think you said the spec uses "lsp" and "client".

The challenge is currently there is a singular config/'manager' for both use cases. I don't think the LSP prefixed words are the worst but am open to suggestions if you have some.

I could see maybe moving to using a word like Liquidity instead of LSP but I think an "LSP" encompasses more than just liquidity.

tnull commented 1 year ago

I think we need to incorporate this fix also to unbreak CI: https://github.com/lightningdevkit/rust-lightning/pull/2353