pay: Remove the presplitter modifier

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.
This commit is contained in:
Christian Decker
2023-07-14 12:53:13 +02:00
committed by Rusty Russell
parent c3a983375c
commit b768804734

View File

@@ -933,26 +933,23 @@ struct payment_modifier *paymod_mods[] = {
&exemptfee_pay_mod, &exemptfee_pay_mod,
&directpay_pay_mod, &directpay_pay_mod,
&shadowroute_pay_mod, &shadowroute_pay_mod,
/* NOTE: The order in which these three paymods are executed is /* NOTE: The order in which these two paymods are executed is
* significant! * significant! `routehints` *must* execute first before
* routehints *must* execute first before payee_incoming_limit * payee_incoming_limit.
* which *must* execute bfore presplit.
* *
* FIXME: Giving an ordered list of paymods to the paymod * FIXME: Giving an ordered list of paymods to the paymod
* system is the wrong interface, given that the order in * system is the wrong interface, given that the order in
* which paymods execute is significant. * which paymods execute is significant. (This is typical of
* (This is typical of Entity-Component-System pattern.) * Entity-Component-System pattern.) What should be done is
* What should be done is that libplugin-pay should have a * that libplugin-pay should have a canonical list of paymods
* canonical list of paymods in the order they execute * in the order they execute correctly, and whether they are
* correctly, and whether they are default-enabled/default-disabled, * default-enabled/default-disabled, and then clients like
* and then clients like `pay` and `keysend` will disable/enable * `pay` and `keysend` will disable/enable paymods that do not
* paymods that do not help them, instead of the current interface * help them, instead of the current interface where clients
* where clients provide an *ordered* list of paymods they want to * provide an *ordered* list of paymods they want to use.
* use.
*/ */
&routehints_pay_mod, &routehints_pay_mod,
&payee_incoming_limit_pay_mod, &payee_incoming_limit_pay_mod,
&presplit_pay_mod,
&waitblockheight_pay_mod, &waitblockheight_pay_mod,
&retry_pay_mod, &retry_pay_mod,
&adaptive_splitter_pay_mod, &adaptive_splitter_pay_mod,
@@ -1211,7 +1208,6 @@ static struct command_result *json_pay(struct command *cmd,
} }
shadow_route = payment_mod_shadowroute_get_data(p); 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_adaptive_splitter_get_data(p)->disable = disablempp;
payment_mod_route_exclusions_get_data(p)->exclusions = exclusions; payment_mod_route_exclusions_get_data(p)->exclusions = exclusions;