From e7716c86fb8cdbe91ca6d71aa9f6eadd70a3856e Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue, 13 Jul 2021 06:18:58 +0930
Subject: [PATCH] lightningd: insist on payment_secret if bit compulsory on
 invoices.

This grandfathers in old invoices for the moment.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 lightningd/htlc_set.c | 12 ++++++++++++
 tests/test_pay.py     | 17 ++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/lightningd/htlc_set.c b/lightningd/htlc_set.c
index 6afe60e76..618b46ba1 100644
--- a/lightningd/htlc_set.c
+++ b/lightningd/htlc_set.c
@@ -1,3 +1,4 @@
+#include <common/features.h>
 #include <common/timeout.h>
 #include <lightningd/htlc_end.h>
 #include <lightningd/htlc_set.h>
@@ -114,6 +115,17 @@ void htlc_set_add(struct lightningd *ld,
 		return;
 	}
 
+	/* If we insist on a payment secret, it must always have it */
+	if (feature_is_set(details->features, COMPULSORY_FEATURE(OPT_PAYMENT_SECRET))
+	    && !payment_secret) {
+		log_debug(ld->log, "Missing payment_secret, but required for %s",
+			  type_to_string(tmpctx, struct sha256,
+					 &hin->payment_hash));
+		local_fail_in_htlc(hin,
+				   take(failmsg_incorrect_or_unknown(NULL, ld, hin)));
+		return;
+	}
+
 	/* BOLT #4:
 	 *  - otherwise, if it supports `basic_mpp`:
 	 *    - MUST add it to the HTLC set corresponding to that `payment_hash`.
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 32a6c0d35..e39af35e3 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -562,6 +562,18 @@ def test_sendpay(node_factory):
         l1.rpc.sendpay([rs], rhash, payment_secret=inv['payment_secret'])
     assert invoice_unpaid(l2, 'testpayment2')
 
+    # Bad payment_secret
+    l1.rpc.sendpay([routestep], rhash, payment_secret="00" * 32)
+    with pytest.raises(RpcError):
+        l1.rpc.waitsendpay(rhash)
+    assert invoice_unpaid(l2, 'testpayment2')
+
+    # Missing payment_secret
+    l1.rpc.sendpay([routestep], rhash)
+    with pytest.raises(RpcError):
+        l1.rpc.waitsendpay(rhash)
+    assert invoice_unpaid(l2, 'testpayment2')
+
     # FIXME: test paying via another node, should fail to pay twice.
     p1 = l1.rpc.getpeer(l2.info['id'], 'info')
     p2 = l2.rpc.getpeer(l1.info['id'], 'info')
@@ -2506,9 +2518,8 @@ def test_pay_no_secret(node_factory, bitcoind):
 
     # Produced from old version (no secret!)
     inv_nosecret = 'lnbcrt1u1pwue4vapp5ve584t0cv27hwmy0cx9ca8uwyqyfw9y9dm3r8vus9fv36r2l9yjsdqaw3jhxazlwpshjhmwda0hxetrwfjhgxq8pmnt9qqcqp9570xsjyykvssa6ty8fjth6f2y8h09myngad9utesttwjwclv95fz3lgd402f9e5yzpnxmkypg55rkvpg522gcz4ymsjl2w3m4jhw4jsp55m7tl'
-    # This succeeds until we make secrets compulsory.
-    l1.rpc.pay(inv_nosecret)
-    l2.daemon.wait_for_log(r'HTLC set contains 1 HTLCs, for a total of 100000msat out of 100000msat \(no payment_secret\)')
+    with pytest.raises(RpcError, match=r"INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.*'erring_index': 1"):
+        l1.rpc.pay(inv_nosecret)
 
 
 @flaky