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 <rusty@rustcorp.com.au>

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,

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 <<rusty@rustcorp.com.au>> is mainly responsible."

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 <<rusty@rustcorp.com.au>> is mainly responsible."

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);

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)