xpay: add xpay-slow-mode to force waiting for all parts before returning.

This was requested by Michael of Boltz; it's mainly useful if you plan to
try failed payments on a *different* node.  In that case, there's a
theoretical possibility that slow parts of this payment could combine with
that from a different node and overpay.

We don't allow this from the same node, already.

Changelog-Added: xpay: `xpay-slow-mode` makes xpay wait for all parts of a payment to complete before returning success or failure.
Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

contrib/msggen/msggen/schema.json

@@ -21207,6 +21207,12 @@
                 "source": "default",
                 "plugin": "/root/lightning/plugins/cln-xpay",
                 "dynamic": true
+              },
+              "xpay-slow-mode": {
+                "value_bool": false,
+                "source": "default",
+                "plugin": "/root/lightning/plugins/cln-xpay",
+                "dynamic": true
               }
             }
           }

doc/lightningd-config.5.md

@@ -546,6 +546,10 @@ command, so they invoices can also be paid onchain.
 
   Setting this makes `xpay` intercept simply `pay` commands (default `false`).
 
+* **xpay-slow-mode**=*BOOL* [plugin `xpay`, *dynamic*]
+
+  Setting this makes `xpay` wait until all parts have failed/succeeded before returning.  Usually this is unnecessary, as xpay will return on the first success (we have the preimage, if they don't take all the parts that's their problem) or failure (the destination could succeed another part, but it would mean it was only partially paid).  The default is `false`.
+
 ### Networking options
 
 Note that for simple setups, the implicit *autolisten* option does the

doc/schemas/lightning-listconfigs.json

@@ -2989,6 +2989,12 @@
             "source": "default",
             "plugin": "/root/lightning/plugins/cln-xpay",
             "dynamic": true
+          },
+          "xpay-slow-mode": {
+            "value_bool": false,
+            "source": "default",
+            "plugin": "/root/lightning/plugins/cln-xpay",
+            "dynamic": true
           }
         }
       }

plugins/xpay/xpay.c

@@ -36,6 +36,8 @@ struct xpay {
 	u32 blockheight;
 	/* Do we take over "pay" commands? */
 	bool take_over_pay;
+	/* Are we to wait for all parts to complete before returning? */
+	bool slow_mode;
 };
 
 static struct xpay *xpay_of(struct plugin *plugin)
@@ -353,13 +355,26 @@ static struct amount_msat total_sent(const struct payment *payment)
 	return total;
 }
 
+/* Should we finish command now? */
+static bool should_finish_command(const struct payment *payment)
+{
+	const struct xpay *xpay = xpay_of(payment->plugin);
+
+	if (!xpay->slow_mode)
+		return true;
+
+	/* In slow mode, only finish when no remaining attempts
+	 * (caller has already moved it to past_attempts). */
+	return list_empty(&payment->current_attempts);
+}
+
 static void payment_succeeded(struct payment *payment,
 			      const struct preimage *preimage)
 {
 	struct json_stream *js;
 
 	/* Only succeed once */
-	if (payment->cmd) {
+	if (payment->cmd && should_finish_command(payment)) {
 		js = jsonrpc_stream_success(payment->cmd);
 		json_add_preimage(js, "payment_preimage", preimage);
 		json_add_amount_msat(js, "amount_msat", payment->amount);
@@ -388,6 +403,18 @@ static void payment_failed(struct command *aux_cmd,
 			   ...)
 	PRINTF_FMT(4,5);
 
+/* Returns NULL if no past attempts succeeded, otherwise the preimage */
+static const struct preimage *
+any_attempts_succeeded(const struct payment *payment)
+{
+	struct attempt *attempt;
+	list_for_each(&payment->past_attempts, attempt, list) {
+		if (attempt->preimage)
+			return attempt->preimage;
+	}
+	return NULL;
+}
+
 static void payment_failed(struct command *aux_cmd,
 			   struct payment *payment,
 			   enum jsonrpc_errcode code,
@@ -402,9 +429,18 @@ static void payment_failed(struct command *aux_cmd,
 	va_end(args);
 
 	/* Only fail once */
-	if (payment->cmd) {
-		was_pending(command_fail(payment->cmd, code, "%s", msg));
-		payment->cmd = NULL;
+	if (payment->cmd && should_finish_command(payment)) {
+		const struct preimage *preimage;
+
+		/* Corner case: in slow_mode, an earlier one could have
+		 * theoretically succeeded. */
+		preimage = any_attempts_succeeded(payment);
+		if (preimage)
+			payment_succeeded(payment, preimage);
+		else {
+			was_pending(command_fail(payment->cmd, code, "%s", msg));
+			payment->cmd = NULL;
+		}
 	}
 
 	/* If no commands outstanding, we can now clean up */
@@ -2057,6 +2093,7 @@ int main(int argc, char *argv[])
 	setup_locale();
 	xpay = tal(NULL, struct xpay);
 	xpay->take_over_pay = false;
+	xpay->slow_mode = false;
 	plugin_main(argv, init, take(xpay),
 		    PLUGIN_RESTARTABLE, true, NULL,
 		    commands, ARRAY_SIZE(commands),
@@ -2066,5 +2103,8 @@ int main(int argc, char *argv[])
 		    plugin_option_dynamic("xpay-handle-pay", "bool",
 					  "Make xpay take over pay commands it can handle.",
 					  bool_option, bool_jsonfmt, &xpay->take_over_pay),
+		    plugin_option_dynamic("xpay-slow-mode", "bool",
+					  "Wait until all parts have completed before returning success or failure",
+					  bool_option, bool_jsonfmt, &xpay->slow_mode),
 		    NULL);
 }

tests/test_xpay.py

@@ -212,7 +212,8 @@ def test_xpay_selfpay(node_factory):
 
 @pytest.mark.slow_test
 @unittest.skipIf(TEST_NETWORK != 'regtest', '29-way split for node 17 is too dusty on elements')
-def test_xpay_fake_channeld(node_factory, bitcoind, chainparams):
+@pytest.mark.parametrize("slow_mode", [False, True])
+def test_xpay_fake_channeld(node_factory, bitcoind, chainparams, slow_mode):
     outfile = tempfile.NamedTemporaryFile(prefix='gossip-store-')
     nodeids = subprocess.check_output(['devtools/gossmap-compress',
                                        'decompress',
@@ -239,6 +240,8 @@ def test_xpay_fake_channeld(node_factory, bitcoind, chainparams):
     shaseed = subprocess.check_output(["tools/hsmtool", "dumpcommitments", l1.info['id'], "1", "0", hsmfile]).decode('utf-8').strip().partition(": ")[2]
     l1.rpc.dev_peer_shachain(l2.info['id'], shaseed)
 
+    # Toggle whether we wait for all the parts to finish.
+    l1.rpc.setconfig('xpay-slow-mode', slow_mode)
     failed_parts = []
     for n in range(0, 100):
         if n in (62, 76, 80, 97):
@@ -677,3 +680,33 @@ def test_xpay_bolt12_no_mpp(node_factory, chainparams):
     assert ret['successful_parts'] == 2
     assert ret['amount_msat'] == AMOUNT
     assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 1
+
+
+def test_xpay_slow_mode(node_factory, bitcoind):
+    # l1 -> l2 -> l3 -> l5
+    #         \-> l4 -/^
+    l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[{'xpay-slow-mode': True},
+                                                         {}, {}, {}, {}])
+    node_factory.join_nodes([l2, l3, l5])
+    node_factory.join_nodes([l2, l4, l5])
+
+    # Make sure l1 can see all paths.
+    node_factory.join_nodes([l1, l2])
+    bitcoind.generate_block(5)
+    wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 10)
+
+    # First try an MPP which fails
+    inv = l5.rpc.invoice(500000000, 'test_xpay_slow_mode_fail', 'test_xpay_slow_mode_fail', preimage='01' * 32)['bolt11']
+    l5.rpc.delinvoice('test_xpay_slow_mode_fail', status='unpaid')
+
+    with pytest.raises(RpcError, match=r"Destination said it doesn't know invoice: incorrect_or_unknown_payment_details"):
+        l1.rpc.xpay(inv)
+
+    # Now a successful one
+    inv = l5.rpc.invoice(500000000, 'test_xpay_slow_mode', 'test_xpay_slow_mode', preimage='00' * 32)['bolt11']
+
+    assert l1.rpc.xpay(inv) == {'payment_preimage': '00' * 32,
+                                'amount_msat': 500000000,
+                                'amount_sent_msat': 500010002,
+                                'failed_parts': 0,
+                                'successful_parts': 2}