From 09eb7110e05c06dc8e974816dae0cf4acdcced67 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 21 Jul 2020 13:52:31 +0930 Subject: [PATCH] sendpay: insist that partid be an exact duplicate if in progress. The test had part 1 and 2 backward, but still worked. When I copied that to *after* the test had succeeded, it complained. It should always complain, to catch bugs. Signed-off-by: Rusty Russell --- lightningd/pay.c | 23 ++++++++++++++++++++++- tests/test_pay.py | 9 ++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index 716a54842..178d1b187 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -837,8 +837,29 @@ send_payment_core(struct lightningd *ld, "Already have %s payment in progress", payments[i]->partid ? "parallel" : "non-parallel"); } - if (payments[i]->partid == partid) + if (payments[i]->partid == partid) { + /* You can't change details while it's pending */ + if (!amount_msat_eq(payments[i]->msatoshi, msat)) { + return command_fail(cmd, PAY_RHASH_ALREADY_USED, + "Already pending " + "with amount %s (not %s)", + type_to_string(tmpctx, + struct amount_msat, + &payments[i]->msatoshi), + type_to_string(tmpctx, + struct amount_msat, &msat)); + } + if (payments[i]->destination && destination + && !node_id_eq(payments[i]->destination, + destination)) { + return command_fail(cmd, PAY_RHASH_ALREADY_USED, + "Already pending to %s", + type_to_string(tmpctx, + struct node_id, + payments[i]->destination)); + } return json_sendpay_in_progress(cmd, payments[i]); + } /* You shouldn't change your mind about amount being * sent, since we'll use it in onion! */ else if (!amount_msat_eq(payments[i]->total_msat, diff --git a/tests/test_pay.py b/tests/test_pay.py index 782c84361..48c38dab6 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2609,9 +2609,12 @@ def test_partial_payment(node_factory, bitcoind, executor): l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=3) - # But repeat is a NOOP. - l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1) - l1.rpc.sendpay(route=r134, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=2) + # But repeat is a NOOP, as long as they're exactly the same! + with pytest.raises(RpcError, match=r'Already pending with amount 501msat \(not 499msat\)'): + l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1) + + l1.rpc.sendpay(route=r134, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1) + l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=2) # Make sure they've done the suppress-commitment thing before we unsuppress l2.daemon.wait_for_log(r'dev_disconnect')