pytest: update tests for fulfilled HTLC no-force-close behavior

Update test_htlc_no_force_close and test_htlc_in_timeout to work
with the new behavior where nodes don't force-close when a fulfilled
HTLC hits the deadline but removal is already in progress.

test_htlc_no_force_close: l3 no longer force-closes for the fulfilled
HTLC — it logs a warning instead.  l2 still force-closes for the
offered HTLC timeout.  Add dev-no-reconnect to prevent l2/l3 from
reconnecting and resolving normally (which would skip the on-chain
scenario the test exercises).

test_htlc_in_timeout: l2 no longer force-closes for the fulfilled
HTLC.  Instead, l1 force-closes when the offered HTLC hits its
deadline (cltv_expiry + 1), and l2 claims on-chain using the preimage
from l1's unilateral close (THEIR_UNILATERAL/THEIR_HTLC instead of
OUR_UNILATERAL/THEIR_HTLC).  Add dev-no-reconnect to l2 to prevent
automatic reconnection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: vincenzopalazzo

tests/test_closing.py

@@ -3887,7 +3887,16 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind):
 @pytest.mark.parametrize("anchors", [False, True])
 def test_htlc_no_force_close(node_factory, bitcoind, anchors):
     """l2<->l3 force closes while an HTLC is in flight from l1, but l2 can't timeout because the feerate has spiked.  It should do so anyway."""
-    opts = [{}, {}, {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC']}]
+    # l3 disconnects before sending update_fulfill_htlc to l2, and
+    # dev-no-reconnect prevents automatic reconnection.  This leaves
+    # l3's incoming HTLC in SENT_REMOVE_HTLC: l3 has the preimage but
+    # can't deliver it.  l3 no longer force-closes for this (the
+    # preimage is safe in the DB and onchaind will claim on-chain),
+    # so l2 force-closes first for the offered HTLC timeout.
+    opts = [{'dev-no-reconnect': None},
+            {'dev-no-reconnect': None},
+            {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'],
+             'dev-no-reconnect': None}]
     if anchors is False:
         for opt in opts:
             opt['dev-force-features'] = "-23"
@@ -3911,17 +3920,20 @@ def test_htlc_no_force_close(node_factory, bitcoind, anchors):
 
     htlc_txs = []
 
-    # l3 drops to chain, holding htlc (but we stop it xmitting txs)
+    # l3 won't force-close (removal in progress), so censor its
+    # onchaind txs for when l2 goes on-chain and l3 sees the
+    # commitment.
     def censoring_sendrawtx(r):
         htlc_txs.append(r['params'][0])
         return {'id': r['id'], 'result': {}}
 
     l3.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx)
 
-    # l3 gets upset, drops to chain when there are < 4 blocks remaining.
-    # But tx doesn't get mined...
+    # l3 should NOT force-close: it has the preimage and the removal is
+    # already in progress.  It just logs a warning.
     bitcoind.generate_block(8)
-    l3.daemon.wait_for_log("Peer permanent failure in CHANNELD_NORMAL: Fulfilled HTLC 0 SENT_REMOVE_.* cltv 119 hit deadline")
+    l3.daemon.wait_for_log(r'but removal already in progress')
+    assert not l3.daemon.is_in_log(r'Peer permanent failure in CHANNELD_NORMAL: Fulfilled HTLC 0 SENT_REMOVE')
 
     # l2 closes drops the commitment tx at block 115 (one block after timeout)
     bitcoind.generate_block(4)

tests/test_misc.py

@@ -521,13 +521,18 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor):
 def test_htlc_in_timeout(node_factory, bitcoind, executor):
     """Test that we drop onchain if the peer doesn't accept fulfilled HTLC"""
 
-    # HTLC 1->2, 1 fails after 2 has sent committed the fulfill
+    # HTLC 1->2, 1 fails after 2 has sent committed the fulfill.
+    # l2 has the preimage but can't deliver it (l1 disconnected and
+    # won't reconnect).  l2 no longer force-closes for this (removal
+    # is in progress, preimage safe in DB).  Instead l1 force-closes
+    # when the offered HTLC hits its deadline, and l2 claims on-chain
+    # using the preimage via onchaind.
     disconnects = ['-WIRE_REVOKE_AND_ACK*2']
     # Feerates identical so we don't get gratuitous commit to update them
     l1 = node_factory.get_node(disconnect=disconnects,
                                options={'dev-no-reconnect': None},
                                feerates=(7500, 7500, 7500, 7500))
-    l2 = node_factory.get_node()
+    l2 = node_factory.get_node(options={'dev-no-reconnect': None})
     # Give it some sats for anchor spend!
     l2.fundwallet(25000, mine_block=False)
 
@@ -556,15 +561,23 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor):
     assert not l2.daemon.is_in_log('hit deadline')
     bitcoind.generate_block(1)
 
-    l2.daemon.wait_for_log('Fulfilled HTLC 0 SENT_REMOVE_COMMIT cltv .* hit deadline')
-    l2.daemon.wait_for_log('sendrawtx exit 0')
-    l2.bitcoin.generate_block(1)
-    l2.daemon.wait_for_log(' to ONCHAIN')
+    # l2 has the preimage but removal is in progress — it should NOT
+    # force-close.  It logs a warning instead.
+    l2.daemon.wait_for_log(r'but removal already in progress')
+    assert not l2.daemon.is_in_log(r'Peer permanent failure in CHANNELD_NORMAL: Fulfilled HTLC')
+
+    # l1's offered HTLC hits deadline (cltv_expiry + 1), which is a
+    # few blocks after l2's fulfilled deadline.  l1 force-closes.
+    bitcoind.generate_block(4 + shadowlen)
+    l1.daemon.wait_for_log('Offered HTLC 0 SENT_ADD_ACK_REVOCATION cltv .* hit deadline')
+    l1.daemon.wait_for_log('sendrawtx exit 0')
+    l1.bitcoin.generate_block(1)
     l1.daemon.wait_for_log(' to ONCHAIN')
+    l2.daemon.wait_for_log(' to ONCHAIN')
 
-    # L2 will collect HTLC (iff no shadow route)
+    # L2 will collect HTLC using the preimage (from l1's unilateral)
     _, txid, blocks = l2.wait_for_onchaind_tx('OUR_HTLC_SUCCESS_TX',
-                                              'OUR_UNILATERAL/THEIR_HTLC')
+                                              'THEIR_UNILATERAL/THEIR_HTLC')
     assert blocks == 0
 
     # If we try to reuse the same output as we used for the anchor spend, then