From b768804734f5963219acd147d233f5e6305533d8 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Fri, 14 Jul 2023 12:53:13 +0200 Subject: [PATCH] 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. --- plugins/pay.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/plugins/pay.c b/plugins/pay.c index a667f1bd0..ac3fe0de9 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -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, @@ -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;