From 944538233ad6d29441c1e95c259eb3990d4187b6 Mon Sep 17 00:00:00 2001 From: QPotato Date: Mon, 4 Jul 2022 13:08:03 -0300 Subject: [PATCH 01/10] Add flag to disable broadcasting when it's dangerous due to information loss --- lightning/src/chain/channelmonitor.rs | 43 ++++++++++++++++++++++++--- lightning/src/ln/channelmanager.rs | 6 ++-- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 52cfe9972f7..4aaee5eeb2f 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -783,6 +783,10 @@ pub(crate) struct ChannelMonitorImpl { counterparty_node_id: Option, secp_ctx: Secp256k1, //TODO: dedup this a bit... + + // Used to track that the channel was updated with ChannelForceClosed {should_broadcast: false} + // implying that it's unsafe to broadcast the latest holder commitment transaction. + allow_automated_broadcast: bool, } /// Transaction outputs to watch for on-chain spends. @@ -1130,6 +1134,8 @@ impl ChannelMonitor { counterparty_node_id: Some(counterparty_node_id), secp_ctx, + + allow_automated_broadcast: true, }) } @@ -1180,7 +1186,18 @@ impl ChannelMonitor { payment_hash, payment_preimage, broadcaster, fee_estimator, logger) } - pub(crate) fn broadcast_latest_holder_commitment_txn( + pub(crate) fn maybe_broadcast_latest_holder_commitment_txn( + &self, + broadcaster: &B, + logger: &L, + ) where + B::Target: BroadcasterInterface, + L::Target: Logger, + { + self.inner.lock().unwrap().maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger) + } + + pub(crate) fn force_broadcast_latest_holder_commitment_txn_unsafe( &self, broadcaster: &B, logger: &L, @@ -1188,7 +1205,7 @@ impl ChannelMonitor { B::Target: BroadcasterInterface, L::Target: Logger, { - self.inner.lock().unwrap().broadcast_latest_holder_commitment_txn(broadcaster, logger) + self.inner.lock().unwrap().force_broadcast_latest_holder_commitment_txn_unsafe(broadcaster, logger) } /// Updates a ChannelMonitor on the basis of some new information provided by the Channel @@ -2148,7 +2165,22 @@ impl ChannelMonitorImpl { } } - pub(crate) fn broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) + pub(crate) fn maybe_broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) + where B::Target: BroadcasterInterface, + L::Target: Logger, + { + if self.allow_automated_broadcast{ + for tx in self.get_latest_holder_commitment_txn(logger).iter() { + log_info!(logger, "Broadcasting local {}", log_tx!(tx)); + broadcaster.broadcast_transaction(tx); + } + self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0)); + } else { + log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take"); + } + } + + pub(crate) fn force_broadcast_latest_holder_commitment_txn_unsafe(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger, { @@ -2215,8 +2247,9 @@ impl ChannelMonitorImpl { log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast); self.lockdown_from_offchain = true; if *should_broadcast { - self.broadcast_latest_holder_commitment_txn(broadcaster, logger); + self.maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger); } else if !self.holder_tx_signed { + self.allow_automated_broadcast = false; log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take"); } else { // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager @@ -3692,6 +3725,8 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> counterparty_node_id, secp_ctx, + + allow_automated_broadcast: true, }))) } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e8830ab5ffe..d049791d221 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5409,7 +5409,7 @@ impl ChannelMana /// Channel object. fn handle_init_event_channel_failures(&self, mut failed_channels: Vec) { for mut failure in failed_channels.drain(..) { - // Either a commitment transactions has been confirmed on-chain or + // Either a commitment transaction has been confirmed on-chain or // Channel::block_disconnected detected that the funding transaction has been // reorganized out of the main chain. // We cannot broadcast our latest local state via monitor update (as @@ -6951,7 +6951,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> log_bytes!(channel.channel_id()), monitor.get_latest_update_id(), channel.get_latest_monitor_update_id()); let (_, mut new_failed_htlcs) = channel.force_shutdown(true); failed_htlcs.append(&mut new_failed_htlcs); - monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); + monitor.maybe_broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); channel_closures.push(events::Event::ChannelClosed { channel_id: channel.channel_id(), user_channel_id: channel.get_user_id(), @@ -6980,7 +6980,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> for (ref funding_txo, ref mut monitor) in args.channel_monitors.iter_mut() { if !funding_txo_set.contains(funding_txo) { log_info!(args.logger, "Broadcasting latest holder commitment transaction for closed channel {}", log_bytes!(funding_txo.to_channel_id())); - monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); + monitor.maybe_broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); } } From f7c2023832978c1f8ae49b604461b39d4ca6f28a Mon Sep 17 00:00:00 2001 From: QPotato Date: Sun, 10 Jul 2022 21:00:31 -0300 Subject: [PATCH 02/10] - Add the new flag on de/serialization - Add a functional test - Fix some typos I found while reading the code --- lightning/src/chain/channelmonitor.rs | 27 ++++++---- lightning/src/ln/functional_tests.rs | 73 ++++++++++++++++++++++++++- lightning/src/ln/reorg_tests.rs | 2 +- 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 4aaee5eeb2f..d791cff04a8 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1013,6 +1013,7 @@ impl Writeable for ChannelMonitorImpl { self.lockdown_from_offchain.write(writer)?; self.holder_tx_signed.write(writer)?; + self.allow_automated_broadcast.write(writer)?; write_tlv_fields!(writer, { (1, self.funding_spend_confirmed, option), @@ -2165,11 +2166,12 @@ impl ChannelMonitorImpl { } } + // Broadcasts the latest commitment transaction only if it's safe to do so. pub(crate) fn maybe_broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger, { - if self.allow_automated_broadcast{ + if self.allow_automated_broadcast { for tx in self.get_latest_holder_commitment_txn(logger).iter() { log_info!(logger, "Broadcasting local {}", log_tx!(tx)); broadcaster.broadcast_transaction(tx); @@ -2180,7 +2182,9 @@ impl ChannelMonitorImpl { } } - pub(crate) fn force_broadcast_latest_holder_commitment_txn_unsafe(&mut self, broadcaster: &B, logger: &L) + // Broadcasts the latest commitment transaction, even if we can't ensure it's safe to do so + // due to missing information. + pub fn force_broadcast_latest_holder_commitment_txn_unsafe(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger, { @@ -2248,14 +2252,16 @@ impl ChannelMonitorImpl { self.lockdown_from_offchain = true; if *should_broadcast { self.maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger); - } else if !self.holder_tx_signed { - self.allow_automated_broadcast = false; - log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take"); } else { - // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager - // will still give us a ChannelForceClosed event with !should_broadcast, but we - // shouldn't print the scary warning above. - log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction."); + self.allow_automated_broadcast = false; + if !self.holder_tx_signed { + log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take"); + } else { + // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager + // will still give us a ChannelForceClosed event with !should_broadcast, but we + // shouldn't print the scary warning above. + log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction."); + } } }, ChannelMonitorUpdateStep::ShutdownScript { scriptpubkey } => { @@ -3638,6 +3644,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> let lockdown_from_offchain = Readable::read(reader)?; let holder_tx_signed = Readable::read(reader)?; + let allow_automated_broadcast: bool = Readable::read(reader)?; if let Some(prev_commitment_tx) = prev_holder_signed_commitment_tx.as_mut() { let prev_holder_value = onchain_tx_handler.get_prev_holder_commitment_to_self_value(); @@ -3726,7 +3733,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> secp_ctx, - allow_automated_broadcast: true, + allow_automated_broadcast, }))) } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index c41d5402ff3..9124ab006f7 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2471,7 +2471,7 @@ fn revoked_output_claim() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - // node[0] is gonna to revoke an old state thus node[1] should be able to claim the revoked output + // node[0] is going to revoke an old state thus node[1] should be able to claim the revoked output let revoked_local_txn = get_local_commitment_txn!(nodes[0], chan_1.2); assert_eq!(revoked_local_txn.len(), 1); // Only output is the full channel value back to nodes[0]: @@ -4752,6 +4752,75 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { } } +#[test] +fn test_deserialize_monitor_force_closed_without_broadcasting_txn() { + // Test deserializing a manager with a monitor that was force closed with {should_broadcast: false}. + // We expect not to broadcast the txn during deseriliazization, but to still be able to force-broadcast it. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let logger; + let fee_estimator; + let persister: test_utils::TestPersister; + let new_chain_monitor: test_utils::TestChainMonitor; + let deserialized_chanmgr: ChannelManager; + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let (_, _, channel_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + + node_chanmgrs[0].force_close_without_broadcasting_txn(&channel_id, &nodes[1].node.get_our_node_id()).unwrap(); + + // Serialize the monitor and node. + let mut chanmon_writer = test_utils::TestVecWriter(Vec::new()); + get_monitor!(nodes[0], channel_id).write(&mut chanmon_writer).unwrap(); + let serialized_chanmon = chanmon_writer.0; + let serialized_node = nodes[0].node.encode(); + + logger = test_utils::TestLogger::new(); + fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }; + persister = test_utils::TestPersister::new(); + let keys_manager = &chanmon_cfgs[0].keys_manager; + new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager); + nodes[0].chain_monitor = &new_chain_monitor; + + // Deserialize the result. + let mut chanmon_read = &serialized_chanmon[..]; + let (_, mut deserialized_chanmon) = <(BlockHash, ChannelMonitor)>::read(&mut chanmon_read, keys_manager).unwrap(); + assert!(chanmon_read.is_empty()); + let mut node_read = &serialized_node[..]; + let tx_broadcaster = nodes[0].tx_broadcaster.clone(); + let channel_monitors = HashMap::from([(deserialized_chanmon.get_funding_txo().0, &mut deserialized_chanmon)]); + let (_, deserialized_chanmgr_temp) = + <(BlockHash, ChannelManager)>::read(&mut node_read, ChannelManagerReadArgs { + default_config: UserConfig::default(), + keys_manager, + fee_estimator: &fee_estimator, + chain_monitor: &new_chain_monitor, + tx_broadcaster, + logger: &logger, + channel_monitors, + }).unwrap(); + deserialized_chanmgr = deserialized_chanmgr_temp; + nodes[0].node = &deserialized_chanmgr; + + { // Assert that the latest commitment txn hasn't been broadcasted. + let txn = tx_broadcaster.txn_broadcasted.lock().unwrap(); + assert_eq!(txn.len(), 0); + } + + { // Assert that we can still force-broadcast the latest commitment txn. + deserialized_chanmon.force_broadcast_latest_holder_commitment_txn_unsafe(&tx_broadcaster, &&logger); + let txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + assert_eq!(txn[0].input[0].previous_output.txid, funding_tx.txid()); + } + + assert!(nodes[0].chain_monitor.watch_channel(deserialized_chanmon.get_funding_txo().0, deserialized_chanmon).is_ok()); + check_added_monitors!(nodes[0], 1); + nodes[0].node.get_and_clear_pending_msg_events(); + nodes[0].node.get_and_clear_pending_events(); +} + macro_rules! check_spendable_outputs { ($node: expr, $keysinterface: expr) => { { @@ -7578,7 +7647,7 @@ fn test_force_close_without_broadcast() { #[test] fn test_check_htlc_underpaying() { // Send payment through A -> B but A is maliciously - // sending a probe payment (i.e less than expected value0 + // sending a probe payment (i.e less than expected value) // to B, B should refuse payment. let chanmon_cfgs = create_chanmon_cfgs(2); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index e4b916c9345..9fd395f9d2a 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -368,7 +368,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_ } } // With expect_channel_force_closed set the TestChainMonitor will enforce that the next update - // is a ChannelForcClosed on the right channel with should_broadcast set. + // is a ChannelForceClosed on the right channel with should_broadcast set. *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); From dff01e1c406356632229d7709bd49519264535e5 Mon Sep 17 00:00:00 2001 From: QPotato Date: Sun, 10 Jul 2022 21:15:18 -0300 Subject: [PATCH 03/10] remove pub(crate) --- lightning/src/chain/channelmonitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index d791cff04a8..c37844085a5 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2167,7 +2167,7 @@ impl ChannelMonitorImpl { } // Broadcasts the latest commitment transaction only if it's safe to do so. - pub(crate) fn maybe_broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) + pub fn maybe_broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger, { From 7342d8c4ade4f595085fa3e6945be20560a3b402 Mon Sep 17 00:00:00 2001 From: QPotato Date: Tue, 19 Jul 2022 23:36:11 -0300 Subject: [PATCH 04/10] Partially address review - Use `check_closed_event` instead of clearing pending events - Write new property and read it from TLVs --- lightning/src/chain/channelmonitor.rs | 4 +--- lightning/src/ln/functional_tests.rs | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c37844085a5..b25d1f7fc4b 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1013,7 +1013,6 @@ impl Writeable for ChannelMonitorImpl { self.lockdown_from_offchain.write(writer)?; self.holder_tx_signed.write(writer)?; - self.allow_automated_broadcast.write(writer)?; write_tlv_fields!(writer, { (1, self.funding_spend_confirmed, option), @@ -3644,7 +3643,6 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> let lockdown_from_offchain = Readable::read(reader)?; let holder_tx_signed = Readable::read(reader)?; - let allow_automated_broadcast: bool = Readable::read(reader)?; if let Some(prev_commitment_tx) = prev_holder_signed_commitment_tx.as_mut() { let prev_holder_value = onchain_tx_handler.get_prev_holder_commitment_to_self_value(); @@ -3733,7 +3731,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> secp_ctx, - allow_automated_broadcast, + allow_automated_broadcast: allow_automated_broadcast.unwrap(), }))) } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 9124ab006f7..03444ae8ec0 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4817,8 +4817,7 @@ fn test_deserialize_monitor_force_closed_without_broadcasting_txn() { assert!(nodes[0].chain_monitor.watch_channel(deserialized_chanmon.get_funding_txo().0, deserialized_chanmon).is_ok()); check_added_monitors!(nodes[0], 1); - nodes[0].node.get_and_clear_pending_msg_events(); - nodes[0].node.get_and_clear_pending_events(); + check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed); } macro_rules! check_spendable_outputs { From 0da0f9383e13b172ccb60645df7af9bceb176493 Mon Sep 17 00:00:00 2001 From: QPotato Date: Wed, 20 Jul 2022 00:01:51 -0300 Subject: [PATCH 05/10] Use pub on `force_broadcast_latest_holder_commitment_txn_unsafe` so that it's exposed to library users and not an unreferenced symbol. --- lightning/src/chain/channelmonitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index b25d1f7fc4b..5d3b2a6c275 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1197,7 +1197,7 @@ impl ChannelMonitor { self.inner.lock().unwrap().maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger) } - pub(crate) fn force_broadcast_latest_holder_commitment_txn_unsafe( + pub fn force_broadcast_latest_holder_commitment_txn_unsafe( &self, broadcaster: &B, logger: &L, From f0f8c832e4ab170678eee2aa047d198ca89be6a1 Mon Sep 17 00:00:00 2001 From: QPotato Date: Wed, 20 Jul 2022 00:27:03 -0300 Subject: [PATCH 06/10] Remove a reference to toxic holder commitment transaction --- lightning/src/chain/channelmonitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 5d3b2a6c275..a0b86777de3 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2177,7 +2177,7 @@ impl ChannelMonitorImpl { } self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0)); } else { - log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take"); + log_info!(logger, "Not broadcasting local commitment txn. Automated broadcasting is disabled."); } } From 140152ba3326eaaad7911220327d1cf3bda56da4 Mon Sep 17 00:00:00 2001 From: QPotato Date: Wed, 20 Jul 2022 00:50:23 -0300 Subject: [PATCH 07/10] Remove second reference to toxic holder commitment txn but keeping the info log in the case the txn is already signed. --- lightning/src/chain/channelmonitor.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a0b86777de3..5a4f53fc04d 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2249,19 +2249,11 @@ impl ChannelMonitorImpl { ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => { log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast); self.lockdown_from_offchain = true; - if *should_broadcast { - self.maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger); - } else { - self.allow_automated_broadcast = false; - if !self.holder_tx_signed { - log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take"); - } else { - // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager - // will still give us a ChannelForceClosed event with !should_broadcast, but we - // shouldn't print the scary warning above. - log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction."); - } + self.allow_automated_broadcast = *should_broadcast; + if !*should_broadcast && self.holder_tx_signed { + log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction."); } + self.maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger); }, ChannelMonitorUpdateStep::ShutdownScript { scriptpubkey } => { log_trace!(logger, "Updating ChannelMonitor with shutdown script"); From 2971adc6020eaec2a802ec542b88940b515a72d4 Mon Sep 17 00:00:00 2001 From: QPotato Date: Thu, 21 Jul 2022 20:56:03 -0300 Subject: [PATCH 08/10] Re-add changes lost on rebase --- lightning/src/chain/channelmonitor.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 5a4f53fc04d..b550d94afce 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1021,6 +1021,7 @@ impl Writeable for ChannelMonitorImpl { (7, self.funding_spend_seen, required), (9, self.counterparty_node_id, option), (11, self.confirmed_commitment_tx_counterparty_output, option), + (13, self.allow_automated_broadcast, required), }); Ok(()) @@ -3658,6 +3659,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> let mut funding_spend_seen = Some(false); let mut counterparty_node_id = None; let mut confirmed_commitment_tx_counterparty_output = None; + let mut allow_automated_broadcast = None; read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, vec_type), @@ -3665,6 +3667,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> (7, funding_spend_seen, option), (9, counterparty_node_id, option), (11, confirmed_commitment_tx_counterparty_output, option), + (13, allow_automated_broadcast, option), }); let mut secp_ctx = Secp256k1::new(); From 214c222b79ea9ac78fab0d7126085ec3cdc4ecb5 Mon Sep 17 00:00:00 2001 From: QPotato Date: Thu, 21 Jul 2022 21:14:54 -0300 Subject: [PATCH 09/10] Add documentation on public method --- lightning/src/chain/channelmonitor.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index b550d94afce..756ccbe638e 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1187,6 +1187,7 @@ impl ChannelMonitor { payment_hash, payment_preimage, broadcaster, fee_estimator, logger) } + // Broadcasts the latest commitment transaction only if it's safe to do so. pub(crate) fn maybe_broadcast_latest_holder_commitment_txn( &self, broadcaster: &B, @@ -1198,6 +1199,8 @@ impl ChannelMonitor { self.inner.lock().unwrap().maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger) } + // Broadcasts the latest commitment transaction, even if we can't ensure it's safe to do so + // due to missing information. pub fn force_broadcast_latest_holder_commitment_txn_unsafe( &self, broadcaster: &B, From 616e69b8838814bccefb397500771308b3b6f89f Mon Sep 17 00:00:00 2001 From: QPotato Date: Mon, 5 Sep 2022 15:41:13 -0300 Subject: [PATCH 10/10] Use triple slash on public method docs --- lightning/src/chain/channelmonitor.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 756ccbe638e..89f9d2aa359 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1187,7 +1187,7 @@ impl ChannelMonitor { payment_hash, payment_preimage, broadcaster, fee_estimator, logger) } - // Broadcasts the latest commitment transaction only if it's safe to do so. + /// Broadcasts the latest commitment transaction only if it's safe to do so. pub(crate) fn maybe_broadcast_latest_holder_commitment_txn( &self, broadcaster: &B, @@ -1199,8 +1199,8 @@ impl ChannelMonitor { self.inner.lock().unwrap().maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger) } - // Broadcasts the latest commitment transaction, even if we can't ensure it's safe to do so - // due to missing information. + /// Broadcasts the latest commitment transaction, even if we can't ensure it's safe to do so + /// due to missing information. pub fn force_broadcast_latest_holder_commitment_txn_unsafe( &self, broadcaster: &B, @@ -2169,7 +2169,7 @@ impl ChannelMonitorImpl { } } - // Broadcasts the latest commitment transaction only if it's safe to do so. + /// Broadcasts the latest commitment transaction only if it's safe to do so. pub fn maybe_broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger, @@ -2185,8 +2185,8 @@ impl ChannelMonitorImpl { } } - // Broadcasts the latest commitment transaction, even if we can't ensure it's safe to do so - // due to missing information. + /// Broadcasts the latest commitment transaction, even if we can't ensure it's safe to do so + /// due to missing information. pub fn force_broadcast_latest_holder_commitment_txn_unsafe(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger,