Trampoline payments - Githubissues

JssDWt commented 4 months ago

This PR adds trampoline payment functionality to the SDK.

If a client sends a payment, normally the greenlight node will find routes through the gossip graph, and attempt to pay over those routes. If a route fails, it tries again. For every attempt there are multiple rounds where the greenlight node has to communicate with the signer. Trampoline payments have the advantage there will be only one outgoing payment attempt, and the LSP will do the retrying, therefore less communication rounds are necessary, hopefully improving payment performance. The tradeoff is the user may pay more fees for the route, and the user loses some privacy in the current setup, because the LSP will learn the payment destination. The initial default trampoline policy will be 0,5%.

This PR leans on the corresponding greenlight PR for trampoline payments, which is still a work in process: https://github.com/Blockstream/greenlight/pull/475

The LSP will need to run this plugin: https://github.com/breez/trampoline

Some notes about this PR:

Trampoline payments should only be attempted when the LSP is not in the route hint. This is because the LSP side trampoline plugin doesn't cooperate nicely with lspd in it's current state.
The trampoline payments are automatically fetched with listpays. That's nice, because it doesn't require any changes to sync. The payments from listpays have the wrong amount_msat due to the way trampoline payments work. We solve that by putting the actual amount in a payment label for trampoline payments.

TODO:

needs update of the greenlight commit id before getting merged, since it's pointing at a PR

roeierez commented 4 months ago

Good points @dangeross . Regarding checking lsp liquidity since this check comes with some latency price for each payment and it is an edge case I would consider trying trampoline and if failing for liquidity fallback to regular payment. I think it should fail fast since the node plugin should check that before sending any htlcs

JssDWt commented 4 months ago

@dangeross @roeierez please check my last 7 commits. Instead of calling lsp_info (which is a network call) I added two changes:

the LSP pubkey is now cached for quick lookup.
the node_state now contains the peer's 'features' as well (currently only trampoline is a supported feature)

With these two pieces of information we can check:

does the route hint contain a hint to our LSP?
Does the LSP support trampoline?

Next to that I added a flag in the SendPaymentRequest and LnurlPayRequest to signal the user doesn't want to use trampoline.

JssDWt commented 4 months ago

I had a case where the trampoline payment was not attempted. I don't think this mechanism is robust enough. Maybe the peer feature check has to be removed?

JssDWt commented 4 months ago

The status of this PR today is:

Trampoline payments work.
There is a transport error from greenlight's trampoline plugin that makes it seem the payment failed.
The peer feature check doesn't seem robust enough. Trampoline payments were skipped at least once when testing. It could be we have to remove the following lines so the greenlight trampoline plugin is in charge of the feature check. If these lines are removed, the corresponding peerfeatures migration and the udl change can also be removed. https://github.com/breez/breez-sdk/pull/1034/files#diff-6f499d3ded5e493c1bd95b827a0a93aeb3d2bfa3d04a6f26be62d87aff026e43R370-R384
I wonder whether trampoline_pay has the same assurences as pay in the sense that it should always work and wait for the node to be available.