Force-close channels on reorg only if the funding is unconfirmed

Currently, if a channel's funding is locked in and then later
reorg'd back to half of the channel's minimum-depth we will
immediately force-close the channel. However, this can happen at
the fork-point while processing a reorg, and generally reorgs do
not reduce the block height at all, making this a rather useless
endeavor.

Ideally we'd never auto-force-close channels at all due to a reorg,
instead simply marking it as inactive until the funding
transaction is re-confirmed (or allowing the user to attempt to
force-close or force-closing once we're confident we have
completed reorg processing if we're at risk of losing funds
already received in the channel).

Sadly, we currently do not support changing a channel's SCID and
updating our SCID maps, so we cannot yet remove the automated
force-close logic. Still, there is no reason to do it until a
funding transaction has been removed from the chain.

This implements that change - only force-closeing once a channel's
funding transaction has been reorg'd out (still potentially at a
reorg's fork point). This continues to imply a 1-confirmation
channel will always be force-closed after a reorg of the funding
transaction, and will imply a similar behavior with 0-conf
channels.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 1cb7a689a21a1b710413e93afbba8e1881d48e90..1eeb3d428096748c7e440c89671a63a19062aa04 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4720,10 +4720,14 @@ impl<Signer: Sign> Channel<Signer> {
                        }
 
                        // If we've sent funding_locked (or have both sent and received funding_locked), and
-                       // the funding transaction's confirmation count has dipped below minimum_depth / 2,
+                       // the funding transaction has become unconfirmed,
                        // close the channel and hope we can get the latest state on chain (because presumably
                        // the funding transaction is at least still in the mempool of most nodes).
-                       if funding_tx_confirmations < self.minimum_depth.unwrap() as i64 / 2 {
+                       //
+                       // Note that ideally we wouldn't force-close if we see *any* reorg on a 1-conf channel,
+                       // but not doing so may lead to the `ChannelManager::short_to_id` map being
+                       // inconsistent, so we currently have to.
+                       if funding_tx_confirmations == 0 && self.funding_tx_confirmed_in.is_some() {
                                let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.",
                                        self.minimum_depth.unwrap(), funding_tx_confirmations);
                                return Err(ClosureReason::ProcessingError { err: err_reason });

diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index 7b36ae0fc4a9d2af97badcbb0c503a1262fdf64d..2f97864b10ec7c5a0fa7cf961547ff517406d11d 100644 (file)
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -212,11 +212,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_
                } else {
                        disconnect_all_blocks(&nodes[0]);
                }
-               if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed {
-                       handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Channel closed because of an exception: Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs.");
-               } else {
-                       handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Channel closed because of an exception: Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.");
-               }
+               handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Channel closed because of an exception: Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.");
                check_added_monitors!(nodes[1], 1);
                {
                        let channel_state = nodes[0].node.channel_state.lock().unwrap();
@@ -280,11 +276,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_
                } else {
                        disconnect_all_blocks(&nodes[0]);
                }
-               if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed {
-                       handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Channel closed because of an exception: Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs.");
-               } else {
-                       handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Channel closed because of an exception: Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.");
-               }
+               handle_announce_close_broadcast_events(&nodes, 0, 1, true, "Channel closed because of an exception: Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.");
                check_added_monitors!(nodes[1], 1);
                {
                        let channel_state = nodes[0].node.channel_state.lock().unwrap();
@@ -297,11 +289,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_
        *nodes[0].chain_monitor.expect_channel_force_closed.lock().unwrap() = Some((chan.2, true));
        nodes[0].node.test_process_background_events(); // Required to free the pending background monitor update
        check_added_monitors!(nodes[0], 1);
-       let expected_err = if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed {
-               "Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs."
-       } else {
-               "Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs."
-       };
+       let expected_err = "Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.";
        check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: "Channel closed because of an exception: ".to_owned() + expected_err });
        check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: expected_err.to_owned() });
        assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);