From 611e4307545f9a5d8ed54b93cbae8bec1bd39651 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 17 Nov 2024 16:07:06 +1030 Subject: [PATCH] lightningd: injectpaymentonion should fail on re-attempts. This is clearer than transparently succeeding: the user might think they paid twice. Signed-off-by: Rusty Russell --- common/jsonrpc_errors.h | 1 + contrib/msggen/msggen/schema.json | 8 +++-- doc/schemas/lightning-injectpaymentonion.json | 8 +++-- lightningd/pay.c | 31 +++++++++++++++---- tests/test_pay.py | 13 ++++++++ 5 files changed, 51 insertions(+), 10 deletions(-) diff --git a/common/jsonrpc_errors.h b/common/jsonrpc_errors.h index a701a9559..000ae8b3a 100644 --- a/common/jsonrpc_errors.h +++ b/common/jsonrpc_errors.h @@ -52,6 +52,7 @@ enum jsonrpc_errcode { PAY_UNREACHABLE = 216, PAY_USER_ERROR = 217, PAY_INJECTPAYMENTONION_FAILED = 218, + PAY_INJECTPAYMENTONION_ALREADY_PAID = 219, /* `fundchannel` or `withdraw` errors */ FUND_MAX_EXCEEDED = 300, diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json index a4974fa32..7cab9563d 100644 --- a/contrib/msggen/msggen/schema.json +++ b/contrib/msggen/msggen/schema.json @@ -16441,7 +16441,7 @@ "title": "Send a payment with a custom onion packet", "description": [ "The **injectpaymentonion** RPC command causes the node to receive a payment attempt similar to the way it would receive one from a peer. The onion packet is unwrapped, then handled normally: either as a local payment, or forwarded to the next peer.", - "Compared to lightning-sendonion(7): the handling of blinded paths and self-payments is trivial, and the interface blocks until the payment succeeds or fails." + "Compared to lightning-sendonion(7): the handling of blinded paths and self-payments is trivial, and the interface blocks until the payment succeeds or fails. The call also fails if this payment_hash has already been successfully paid." ], "request": { "required": [ @@ -16542,7 +16542,11 @@ "", "- 218: injectpaymentonion failed", "", - "The *onionreply* is returned in the error details, which can be unwrapped to discover the error" + "The *onionreply* is returned in the error *data*, which can be unwrapped to discover the error", + "", + "- 219: injectpaymentonion already succeeded", + "", + "The *data* object contains the previous success, as per lightning-sendpay." ], "author": [ "Rusty Russell <> is mainly responsible." diff --git a/doc/schemas/lightning-injectpaymentonion.json b/doc/schemas/lightning-injectpaymentonion.json index 3965b0e4a..f5b23b316 100644 --- a/doc/schemas/lightning-injectpaymentonion.json +++ b/doc/schemas/lightning-injectpaymentonion.json @@ -6,7 +6,7 @@ "title": "Send a payment with a custom onion packet", "description": [ "The **injectpaymentonion** RPC command causes the node to receive a payment attempt similar to the way it would receive one from a peer. The onion packet is unwrapped, then handled normally: either as a local payment, or forwarded to the next peer.", - "Compared to lightning-sendonion(7): the handling of blinded paths and self-payments is trivial, and the interface blocks until the payment succeeds or fails." + "Compared to lightning-sendonion(7): the handling of blinded paths and self-payments is trivial, and the interface blocks until the payment succeeds or fails. The call also fails if this payment_hash has already been successfully paid." ], "request": { "required": [ @@ -107,7 +107,11 @@ "", "- 218: injectpaymentonion failed", "", - "The *onionreply* is returned in the error details, which can be unwrapped to discover the error" + "The *onionreply* is returned in the error *data*, which can be unwrapped to discover the error", + "", + "- 219: injectpaymentonion already succeeded", + "", + "The *data* object contains the previous success, as per lightning-sendpay." ], "author": [ "Rusty Russell <> is mainly responsible." diff --git a/lightningd/pay.c b/lightningd/pay.c index 3694f3bc6..a827d1e61 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -889,7 +889,8 @@ found: return channel; } -/* Check if payment already in progress. Returns NULL if all good */ +/* Check if payment already in progress. Returns NULL if all good. + * Sets *succeeded_payment if we had a previous successful payment for this hash. */ static struct command_result *check_progress(struct lightningd *ld, struct command *cmd, const struct sha256 *rhash, @@ -897,11 +898,14 @@ static struct command_result *check_progress(struct lightningd *ld, struct amount_msat total_msat, u64 partid, u64 group, - const struct node_id *destination) + const struct node_id *destination, + const struct wallet_payment **succeeded_payment) { bool have_complete = false; struct amount_msat msat_already_pending = AMOUNT_MSAT(0); + *succeeded_payment = NULL; + /* Now, do we already have one or more payments? */ for (struct db_stmt *stmt = payments_by_hash(cmd->ld->wallet, rhash); stmt; @@ -941,7 +945,8 @@ static struct command_result *check_progress(struct lightningd *ld, fmt_node_id(tmpctx, payment->destination)); } - return sendpay_success(cmd, payment, NULL); + *succeeded_payment = payment; + return NULL; case PAYMENT_PENDING: /* At most one payment group can be in-flight at any @@ -1083,14 +1088,18 @@ send_payment_core(struct lightningd *ld, struct htlc_out *hout; struct routing_failure *fail; struct command_result *ret; - struct wallet_payment *payment; + const struct wallet_payment *payment; /* Reconcile this with previous attempts */ ret = check_progress(ld, cmd, rhash, msat, total_msat, partid, group, - destination); + destination, &payment); if (ret) return ret; + /* Previous payment success is defined to be idempotent */ + if (payment) + return sendpay_success(cmd, payment, NULL); + ret = check_invoice_request_usage(cmd, local_invreq_id); if (ret) return ret; @@ -1834,6 +1843,7 @@ static struct command_result *json_injectpaymentonion(struct command *cmd, struct command_result *ret; const u8 *failmsg; struct htlc_out *hout; + const struct wallet_payment *prev_payment; if (!param_check(cmd, buffer, params, p_req("onion", param_bin_from_hex, &onion), @@ -1853,10 +1863,19 @@ static struct command_result *json_injectpaymentonion(struct command *cmd, * partid/groupid uniqueness: we don't know amount or total. */ ret = check_progress(cmd->ld, cmd, payment_hash, AMOUNT_MSAT(0), AMOUNT_MSAT(0), - *partid, *groupid, NULL); + *partid, *groupid, NULL, &prev_payment); if (ret) return ret; + if (prev_payment) { + struct json_stream *js = json_stream_fail(cmd, + PAY_INJECTPAYMENTONION_ALREADY_PAID, + "Already paid this invoice"); + json_add_payment_fields(js, prev_payment); + json_object_end(js); + return command_failed(cmd, js); + } + /* This checks we're not trying to pay our a locally-generated * invoice_request more than once. */ ret = check_invoice_request_usage(cmd, local_invreq_id); diff --git a/tests/test_pay.py b/tests/test_pay.py index 6e2872ab8..1cc1997c7 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -6137,6 +6137,19 @@ def test_injectpaymentonion_simple(node_factory, executor): assert lsp['payment_hash'] == inv1['payment_hash'] assert lsp['status'] == 'complete' + # We FAIL on reattempt + with pytest.raises(RpcError, match="Already paid this invoice") as err: + l1.rpc.injectpaymentonion(onion=onion['onion'], + payment_hash=inv1['payment_hash'], + amount_msat=1000, + cltv_expiry=blockheight + 18 + 6, + partid=1, + groupid=0) + # PAY_INJECTPAYMENTONION_ALREADY_PAID + assert err.value.error['code'] == 219 + assert 'onionreply' not in err.value.error['data'] + assert err.value.error['data'] == lsp + def test_injectpaymentonion_mpp(node_factory, executor): l1, l2 = node_factory.line_graph(2)