Skip to content

Commit

Permalink
pay: Remove the presplitter modifier
Browse files Browse the repository at this point in the history
The presplitter modifier would split a payment before trying the first
attempt based on some common sizes. Its goal was to have smaller parts
in flight over different paths, in order to make it more difficult for
a forwarding node to learn payment amount. However it was causing some
issues for direct payments, and estimates on spendable amounts which
considers only the first HTLC being added, but presplitter would
always cause multiple HTLCs to be kicked off, causing the estimate to
be off.

Removing the presplitter fixes this, making draining channels easier,
and worse success rates, due to more HTLCs in flight directly
impacting the changes of getting stuck.

Changelog-Removed: pay: `pay` no longer splits based on common size, as it was causing issues in various scenarios.
  • Loading branch information
cdecker authored and rustyrussell committed Jul 20, 2023
1 parent c3a9833 commit b768804
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions plugins/pay.c
Original file line number Diff line number Diff line change
Expand Up @@ -933,26 +933,23 @@ struct payment_modifier *paymod_mods[] = {
&exemptfee_pay_mod,
&directpay_pay_mod,
&shadowroute_pay_mod,
/* NOTE: The order in which these three paymods are executed is
* significant!
* routehints *must* execute first before payee_incoming_limit
* which *must* execute bfore presplit.
/* NOTE: The order in which these two paymods are executed is
* significant! `routehints` *must* execute first before
* payee_incoming_limit.
*
* FIXME: Giving an ordered list of paymods to the paymod
* system is the wrong interface, given that the order in
* which paymods execute is significant.
* (This is typical of Entity-Component-System pattern.)
* What should be done is that libplugin-pay should have a
* canonical list of paymods in the order they execute
* correctly, and whether they are default-enabled/default-disabled,
* and then clients like `pay` and `keysend` will disable/enable
* paymods that do not help them, instead of the current interface
* where clients provide an *ordered* list of paymods they want to
* use.
* which paymods execute is significant. (This is typical of
* Entity-Component-System pattern.) What should be done is
* that libplugin-pay should have a canonical list of paymods
* in the order they execute correctly, and whether they are
* default-enabled/default-disabled, and then clients like
* `pay` and `keysend` will disable/enable paymods that do not
* help them, instead of the current interface where clients
* provide an *ordered* list of paymods they want to use.
*/
&routehints_pay_mod,
&payee_incoming_limit_pay_mod,
&presplit_pay_mod,
&waitblockheight_pay_mod,
&retry_pay_mod,
&adaptive_splitter_pay_mod,
Expand Down Expand Up @@ -1211,7 +1208,6 @@ static struct command_result *json_pay(struct command *cmd,
}

shadow_route = payment_mod_shadowroute_get_data(p);
payment_mod_presplit_get_data(p)->disable = disablempp;
payment_mod_adaptive_splitter_get_data(p)->disable = disablempp;
payment_mod_route_exclusions_get_data(p)->exclusions = exclusions;

Expand Down

0 comments on commit b768804

Please sign in to comment.