tests: lnpeer: test_dont_settle_partial_mpp_trigger_with_invalid_cltv_htlc
What changed, and why it matters
This commit only adds a new automated test to Electrum's Lightning code. It does not change any production code, so it cannot by itself introduce or fix a security vulnerability in running software. The test checks a corner case in multi-part Lightning payments to make sure a bug (settling part of an incomplete payment) does not occur.
No action required for security; review the test for correctness and ensure the behavior it asserts is already implemented in the production Lightning peer code.
Security signals we found
Test-only change: no production code modified
Test covers Lightning MPP partial-settlement behavior
Test guards against preimage release when MPP is incomplete
Evidence from the diff
The diff adds a single unit test, test_dont_settle_partial_mpp_trigger_with_invalid_cltv_htlc, in tests/test_lnpeer.py. The test simulates two HTLCs intended as a multi-path payment (MPP) to an invoice. One HTLC has a valid CLTV delta (400), the other has an invalid, too-small CLTV delta (1, below the invoice’s min_final_cltv_delta=144). The test asserts that when the invalid HTLC is failed, the remaining valid HTLC is also failed rather than settled, and that the payment preimage is never released. No library or runtime code is modified.
Changed components
tests/test_lnpeer.pyInspect captured patch +78 / −0
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 7cf778f..26a166f 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -1336,6 +1336,84 @@ class TestPeerDirect(TestPeer):
with self.assertRaises(SuccessfulTest):
await f()
+ async def test_dont_settle_partial_mpp_trigger_with_invalid_cltv_htlc(self):
+ """Alice gets two htlcs as part of a mpp, one has a cltv too close to expiry and will get failed.
+ Test that the other htlc won't get settled if the mpp isn't complete anymore after failing the other htlc.
+ """
+ alice_channel, bob_channel = create_test_channels()
+ p1, p2, w1, w2, _q1, _q2 = self.prepare_peers(alice_channel, bob_channel)
+ async def pay():
+ await util.wait_for2(p1.initialized, 1)
+ await util.wait_for2(p2.initialized, 1)
+ w2.features |= LnFeatures.BASIC_MPP_OPT
+ lnaddr1, _pay_req = self.prepare_invoice(w2, amount_msat=10_000, min_final_cltv_delta=144)
+ self.assertTrue(lnaddr1.get_features().supports(LnFeatures.BASIC_MPP_OPT))
+ route = (await w1.create_routes_from_invoice(amount_msat=10_000, decoded_invoice=lnaddr1))[0][0].route
+
+ # now p1 sends two htlcs, one is valid (1 msat), one is invalid (9_999 msat)
+ p1.pay(
+ route=route,
+ chan=alice_channel,
+ amount_msat=1,
+ total_msat=lnaddr1.get_amount_msat(),
+ payment_hash=lnaddr1.paymenthash,
+ # this htlc is valid and will get accepted, but it shouldn't get settled
+ min_final_cltv_delta=400,
+ payment_secret=lnaddr1.payment_secret,
+ )
+ await asyncio.sleep(0.1)
+ assert w1.get_preimage(lnaddr1.paymenthash) is None
+ p1.pay(
+ route=route,
+ chan=alice_channel,
+ amount_msat=9_999,
+ total_msat=lnaddr1.get_amount_msat(),
+ payment_hash=lnaddr1.paymenthash,
+ # this htlc will get failed directly as the cltv is too close to expiry (< 144)
+ min_final_cltv_delta=1,
+ payment_secret=lnaddr1.payment_secret,
+ )
+
+ while nhtlc_success + nhtlc_failed < 2:
+ await htlc_resolved.wait()
+ # both htlcs of the mpp set should get failed and w2 shouldn't release the preimage
+ self.assertEqual(0, nhtlc_success, f"{nhtlc_success=} | {nhtlc_failed=}")
+ self.assertEqual(2, nhtlc_failed, f"{nhtlc_success=} | {nhtlc_failed=}")
+ assert w1.get_preimage(lnaddr1.paymenthash) is None, "w1 shouldn't get the preimage"
+ raise SuccessfulTest()
+
+ async def f():
+ async with OldTaskGroup() as group:
+ await group.spawn(p1._message_loop())
+ await group.spawn(p1.htlc_switch())
+ await group.spawn(p2._message_loop())
+ await group.spawn(p2.htlc_switch())
+ await asyncio.sleep(0.01)
+ await group.spawn(pay())
+
+ htlc_resolved = asyncio.Event()
+ nhtlc_success = 0
+ nhtlc_failed = 0
+ async def on_htlc_fulfilled(*args):
+ htlc_resolved.set()
+ htlc_resolved.clear()
+ nonlocal nhtlc_success
+ nhtlc_success += 1
+ async def on_htlc_failed(*args):
+ htlc_resolved.set()
+ htlc_resolved.clear()
+ nonlocal nhtlc_failed
+ nhtlc_failed += 1
+ util.register_callback(on_htlc_fulfilled, ["htlc_fulfilled"])
+ util.register_callback(on_htlc_failed, ["htlc_failed"])
+
+ try:
+ with self.assertRaises(SuccessfulTest):
+ await f()
+ finally:
+ util.unregister_callback(on_htlc_fulfilled)
+ util.unregister_callback(on_htlc_failed)
+
async def test_legacy_shutdown_low(self):
await self._test_shutdown(alice_fee=100, bob_fee=150)
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.