Configure multiple LSPs as liquidity sources - #274
Conversation
|
👋 Thanks for assigning @benthecarman as a reviewer! |
| } | ||
|
|
||
| #[test] | ||
| fn test_multiple_liquidity_sources_from_file() { |
There was a problem hiding this comment.
could we also add a test for multiple but one of the lsp configs is invalid, make sure we don't just silently drop the config and properly throw an error
| [liquidity.lsps2_client] | ||
| # The public key of the LSPS2 LSP we source just-in-time liquidity from. | ||
| # LSPS Client Support | ||
| # Repeat this section to register several LSPs. Supported protocols (LSPS1/LSPS2) are |
There was a problem hiding this comment.
We only support LSPS2 for now
| # token = "" | ||
| # Accept 0-confirmation channels opened by this LSP. Required for JIT channels to be | ||
| # usable before the funding transaction confirms. | ||
| trust_peer_0conf = false |
There was a problem hiding this comment.
we should set this to true, otherwise the UX of JIT channels kinda sucks
| Registers a Liquidity Service Provider to source inbound liquidity from. Repeat the section | ||
| to register several LSPs. Each LSP's supported protocols are discovered on startup via | ||
| [bLIP-50 / LSPS0](https://github.com/lightning/blips/blob/master/blip-0050.md), so a single | ||
| entry covers both LSPS1 and LSPS2 depending on what the LSP advertises. |
| let lsps_clients = config.lsps_client_config.expect("liquidity sources configured"); | ||
| assert_eq!(lsps_clients.len(), 2); | ||
|
|
||
| assert_eq!( |
There was a problem hiding this comment.
instead of individually asserting each field would be cleaner (and more forwards compatible) to create each LSPSClientConfig and assert that it at the correct index
There was a problem hiding this comment.
Thanks, this has been updated
| let lsps2_client_config = self | ||
| .lsps2 | ||
| let lsps_client_config = self | ||
| .lsps |
There was a problem hiding this comment.
For here we would accept duplicate LSPs (same node id), seems we allow that in ldk-node and just silently drop them, should we throw an error here?
There was a problem hiding this comment.
Yes, I have updated it to throw an error here and also included a test for it
LDK Node's `add_liquidity_source()` registers an LSP generically and discovers its supported protocols (LSPS1/LSPS2) via bLIP-50 / LSPS0. Replace the single `[liquidity.lsps2_client]` section with a `[[liquidity.lsps_client]]` array so several LSPs can be registered. Each entry is passed to `add_liquidity_source()` on startup, after which LSPS2 JIT invoices select the cheapest fee offer across every LSPS2-capable LSP. Surface the per-LSP `trust_peer_0conf` flag, previously hardcoded to `false`. It is required rather than defaulted, since accepting 0-confirmation channels from an LSP is a trust decision each operator should make explicitly.
d62f7a6 to
0702f16
Compare
Exposes LDK Node's multi-LSP support so a server can register several LSPs instead of a single one.
Builds on lightningdevkit/ldk-node#792, which replaced the per-protocol builder setters with
add_liquidity_source()and added bLIP-50 / LSPS0 protocol discovery.LDK Node's
add_liquidity_source()registers an LSP generically and discovers its supported protocols (LSPS1/LSPS2) via bLIP-50 / LSPS0. Replace the single[liquidity.lsps2_client]section with a[[liquidity.lsps_client]]array so several LSPs can be registered. Each entry is passed toadd_liquidity_source()on startup, after which LSPS2 JIT invoices select the cheapest fee offer across every LSPS2-capable LSP.Surface the per-LSP
trust_peer_0confflag, previously hardcoded tofalse. It is required rather than defaulted, since accepting 0-confirmation channels from an LSP is a trust decision each operator should make explicitly.Breaking change
[liquidity.lsps2_client]is no longer accepted. Existing configurations must be migrated to[[liquidity.lsps_client]]with an explicittrust_peer_0confvalue: