From 7016c2f20203d804f843ab6fa04720af62f85621 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 2 May 2022 02:51:50 +0000 Subject: [PATCH] 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. --- lightning/src/ln/channel.rs | 8 ++++++-- lightning/src/ln/reorg_tests.rs | 18 +++--------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1cb7a689a..1eeb3d428 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4720,10 +4720,14 @@ impl Channel { } // 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 7b36ae0fc..2f97864b1 100644 --- 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); -- 2.39.5