sendpay: don't allow a new part payment if any part has succeeded.

This wasn't important before, but now we have MPP it's good to enforce.

Reported-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/pay.c

@@ -789,6 +789,7 @@ send_payment_core(struct lightningd *ld,
 	struct htlc_out *hout;
 	struct routing_failure *fail;
 	struct amount_msat msat_already_pending = AMOUNT_MSAT(0);
+	bool have_complete = false;
 
 	/* Now, do we already have one or more payments? */
 	payments = wallet_payment_list(tmpctx, ld->wallet, rhash);
@@ -803,6 +804,7 @@ send_payment_core(struct lightningd *ld,
 
 		switch (payments[i]->status) {
 		case PAYMENT_COMPLETE:
+			have_complete = true;
 			if (payments[i]->partid != partid)
 				continue;
 
@@ -810,10 +812,12 @@ send_payment_core(struct lightningd *ld,
 			if (!amount_msat_eq(payments[i]->msatoshi, msat)) {
 				return command_fail(cmd, PAY_RHASH_ALREADY_USED,
 						    "Already succeeded "
-						    "with amount %s",
+						    "with amount %s (not %s)",
 						    type_to_string(tmpctx,
 								   struct amount_msat,
-								   &payments[i]->msatoshi));
+								   &payments[i]->msatoshi),
+						    type_to_string(tmpctx,
+								   struct amount_msat, &msat));
 			}
 			if (payments[i]->destination && destination
 			    && !node_id_eq(payments[i]->destination,
@@ -871,6 +875,12 @@ send_payment_core(struct lightningd *ld,
  		}
 	}
 
+	/* If any part has succeeded, you can't start a new one! */
+	if (have_complete) {
+		return command_fail(cmd, PAY_RHASH_ALREADY_USED,
+				    "Already succeeded other parts");
+	}
+
 	/* BOLT #4:
 	 *
 	 * - MUST NOT send another HTLC if the total `amount_msat` of the HTLC

tests/test_pay.py

@@ -2639,6 +2639,14 @@ def test_partial_payment(node_factory, bitcoind, executor):
     assert pay['number_of_parts'] == 2
     assert pay['amount_sent_msat'] == Millisatoshi(1002)
 
+    # It will immediately succeed if we pay again.
+    pay = l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=2)
+    assert pay['status'] == 'complete'
+
+    # If we try with an unknown partid, it will refuse.
+    with pytest.raises(RpcError, match=r'Already succeeded'):
+        l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=3)
+
 
 def test_partial_payment_timeout(node_factory, bitcoind):
     l1, l2 = node_factory.line_graph(2)