plugins/pay.c: Fix the routehints/presplit conflict.

Changelog-Fixed: pay: Fixed a bug where routehints would be ignored if the payment exceeded 10,000 satoshi. This is particularly bad if the payee is only reachable via routehints in an invoice.

plugins/pay.c

@@ -1881,9 +1881,25 @@ struct payment_modifier *paymod_mods[] = {
 	&local_channel_hints_pay_mod,
 	&exemptfee_pay_mod,
 	&directpay_pay_mod,
-	&presplit_pay_mod,
 	&shadowroute_pay_mod,
+	/* NOTE: The ordeer in which these two paymods are executed is
+	 * significant!
+	 * routehints *must* execute first before presplit.
+	 *
+	 * 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.
+	 */
 	&routehints_pay_mod,
+	&presplit_pay_mod,
 	&waitblockheight_pay_mod,
 	&retry_pay_mod,
 	&adaptive_splitter_pay_mod,

tests/test_pay.py

@@ -3223,10 +3223,9 @@ def test_bolt11_null_after_pay(node_factory, bitcoind):
     assert('amount_msat' in pays[0] and pays[0]['amount_msat'] == amt)
 
 
-@pytest.mark.xfail(strict=True)
 def test_mpp_presplit_routehint_conflict(node_factory, bitcoind):
     '''
-    We have a bug where pre-splitting the payment prevents *any*
+    We had a bug where pre-splitting the payment prevents *any*
     routehints from being taken.
     We tickle that bug here by building l1->l2->l3, but with
     l2->l3 as an unpublished channel.