Renepay patch htlc_max=0 cases #7159
Conversation
Lagrang3
changed the title
Add spendable/receivable and is_local fields to the callback function used in gossmods_from_listpeerchannels. This allows to do more fine grained use of the listpeerchannels call. The first use case is renepay, for which we need to ignore the htlc_max of the local channels only.
In the Min. Cost Flow solver we put a constraint on arcs capacities, such that the total flow that can be allocated through a channel does not exceed htlc_max. This solves two previous problem of the htlc_max constraint at the MCF level.
Lagrang3
force-pushed
the
renepay-htlcmax0-patch
branch
from
10769a9
to
d94d15d
Compare
There was a problem hiding this comment.
Concept ACK d94d15d
I am still digesting the match around raypay and the logic, so I can not of for an ACK, but code LGMT :)
Added a small comment inside the test that it is more an idea
| l1.rpc.call("renepay", {"invstring": inv["bolt11"]}) | ||
| l1.wait_for_htlcs() | ||
| invoice = only_one(l6.rpc.listinvoices("inv")["invoices"]) | ||
| assert invoice["amount_received_msat"] >= Millisatoshi("600000sat") |
There was a problem hiding this comment.
This is not relevant to this PR, but I think that maybe would be good for this kind of testing to know what is the path that the payment is taking.
Maybe we can add this as a pay result?
There was a problem hiding this comment.
Neither pay does this, we get this kind of information from logs alone.
There was a problem hiding this comment.
Neither pay does this, we get this kind of information from logs alone.
My idea was also to implement it inside vanilla pay
There was a problem hiding this comment.
It would be nice if renepay gave the successful paths in the response, though that's pretty verbose!
There was a problem hiding this comment.
What if the response from waitsendpay (as well as the payment success/failure notification) had a field with the route?
In renepay there is an internal database of these routes pushed to sendpay so that the plugin knows when a specific (groupid, partid) fail or succeeds it can recover which route was that and act accordingly, for example by updating the uncertainty/knowledge. If the route was already provided through the result of waitsendpay it would be easier and less fragile on renepay's side.
| l1.rpc.call("renepay", {"invstring": inv["bolt11"]}) | ||
| l1.wait_for_htlcs() | ||
| invoice = only_one(l6.rpc.listinvoices("inv")["invoices"]) | ||
| assert invoice["amount_received_msat"] >= Millisatoshi("600000sat") |
There was a problem hiding this comment.
It would be nice if renepay gave the successful paths in the response, though that's pretty verbose!
is_local, ie. "is this side of the channel ours?" is not needed since we can determine that predicate by evaluating `scidd->dir == node_id_idx(self, peer)`
Lagrang3
force-pushed
the
renepay-htlcmax0-patch
branch
from
590f6fa
to
e5c705f
Compare
This solves issue #7150.
Renepay does not handle properly the case in which
htlcmax=0in the gossmap channels.There are two cases that are to be considered separately: local channels and remote channels.
Local channels
When a local channel has
htlcmax=0the sender should ignore this value because thehtlcmin/htlcmaxconstraints only apply to forwarded HTLCs, see setchannel documentation. We handle this case by setting a gossmap modifier (gossmap_localmods) in which htlc constraints are lifted from local channels, i.e.htlcmin=0andhtlcmax=spendable.For reviewers: renepay creates the localmods by parsing a call to listpeerchannels, but recently it has been changed to use the
gossmods_from_listpeerchannelsAPI. The current implementation of that API would automatically forward to the user's callback function a unified valuemax = min(htlmax, spendable).I've changed that, so now instead of the unified
maxvalue the callback function receives separatehtlcmaxandspendableamounts, so than I can use this functionality to ignore thehtlcmaxlimit on local channels.Remote channels
Not just
htlc_max=0is a problem but also small values of it. I think the best option here is to limit the arcs' capacitiesin the Min. Cost Flow solver---as suggested by @renepickhardt in #6905 (comment) ---such that the maximum allowed flow in a channel does not exceed
htlc_max.To-do list: