From bee42b1659c5989516e5d76501b85b9ff970c647 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 20 Aug 2022 01:03:27 +0000 Subject: [PATCH 1/2] Handle async initial ChannelMonitor persistence failing on restart If the initial ChannelMonitor persistence is done asynchronously but does not complete before the node restarts (with a ChannelManager persistence), we'll start back up with a channel present but no corresponding ChannelMonitor. Because the Channel is pending-monitor-update and has not yet broadcasted its initial funding transaction or sent channel_ready, this is not a violation of our API contract nor a safety violation. However, the previous code would refuse to deserialize the ChannelManager treating it as an API contract violation. The solution is to test for this case explicitly and drop the channel entirely as if the peer disconnected before we received the funding_signed for outbound channels or before sending the channel_ready for inbound channels. --- lightning/src/ln/chanmon_update_fail_tests.rs | 218 ++++++++++++++++++ lightning/src/ln/channel.rs | 36 +++ lightning/src/ln/channelmanager.rs | 10 + lightning/src/util/events.rs | 12 +- 4 files changed, 274 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 15d46b04688..ef7efdd221f 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -2745,3 +2745,221 @@ fn double_temp_error() { commitment_signed_dance!(nodes[0], nodes[1], commitment_signed_b2, false); expect_payment_sent!(nodes[0], payment_preimage_2); } + +fn do_test_outbound_reload_without_init_mon(use_0conf: bool) { + // Test that if the monitor update generated in funding_signed is stored async and we restart + // with the latest ChannelManager but the ChannelMonitor persistence never completed we happily + // drop the channel and move on. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + + let persister: test_utils::TestPersister; + let new_chain_monitor: test_utils::TestChainMonitor; + let nodes_0_deserialized: ChannelManager; + + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + chan_config.channel_handshake_limits.trust_own_funding_0conf = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(chan_config), Some(chan_config)]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 43, None).unwrap(); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), channelmanager::provided_init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id())); + + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + if use_0conf { + nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + } else { + nodes[1].node.accept_inbound_channel(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + } + }, + _ => panic!("Unexpected event"), + }; + + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), channelmanager::provided_init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id())); + + let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43); + + nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap(); + check_added_monitors!(nodes[0], 0); + + let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); + check_added_monitors!(nodes[1], 1); + + let bs_signed_locked = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(bs_signed_locked.len(), if use_0conf { 2 } else { 1 }); + match &bs_signed_locked[0] { + MessageSendEvent::SendFundingSigned { msg, .. } => { + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &msg); + check_added_monitors!(nodes[0], 1); + } + _ => panic!("Unexpected event"), + } + if use_0conf { + match &bs_signed_locked[1] { + MessageSendEvent::SendChannelReady { msg, .. } => { + nodes[0].node.handle_channel_ready(&nodes[1].node.get_our_node_id(), &msg); + } + _ => panic!("Unexpected event"), + } + } + + assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty()); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); + + // nodes[0] is now waiting on the first ChannelMonitor persistence to complete in order to + // broadcast the funding transaction. If nodes[0] restarts at this point with the + // ChannelMonitor lost, we should simply discard the channel. + + // The test framework checks that watched_txn/outputs match the monitor set, which they will + // not, so we have to clear them here. + nodes[0].chain_source.watched_txn.lock().unwrap().clear(); + nodes[0].chain_source.watched_outputs.lock().unwrap().clear(); + + let nodes_0_serialized = nodes[0].node.encode(); + 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(), nodes[0].logger, node_cfgs[0].fee_estimator, &persister, keys_manager); + nodes[0].chain_monitor = &new_chain_monitor; + + let mut nodes_0_read = &nodes_0_serialized[..]; + let config = UserConfig::default(); + nodes_0_deserialized = { + <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + default_config: config, + keys_manager, + fee_estimator: node_cfgs[0].fee_estimator, + chain_monitor: nodes[0].chain_monitor, + tx_broadcaster: nodes[0].tx_broadcaster.clone(), + logger: nodes[0].logger, + channel_monitors: HashMap::new(), + }).unwrap().1 + }; + nodes[0].node = &nodes_0_deserialized; + assert!(nodes_0_read.is_empty()); + + check_closed_event!(nodes[0], 1, ClosureReason::DisconnectedPeer); + assert!(nodes[0].node.list_channels().is_empty()); +} + +#[test] +fn test_outbound_reload_without_init_mon() { + do_test_outbound_reload_without_init_mon(true); + do_test_outbound_reload_without_init_mon(false); +} + +fn do_test_inbound_reload_without_init_mon(use_0conf: bool, lock_commitment: bool) { + // Test that if the monitor update generated by funding_transaction_generated is stored async + // and we restart with the latest ChannelManager but the ChannelMonitor persistence never + // completed we happily drop the channel and move on. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + + let persister: test_utils::TestPersister; + let new_chain_monitor: test_utils::TestChainMonitor; + let nodes_1_deserialized: ChannelManager; + + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + chan_config.channel_handshake_limits.trust_own_funding_0conf = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(chan_config), Some(chan_config)]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 43, None).unwrap(); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), channelmanager::provided_init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id())); + + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + if use_0conf { + nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + } else { + nodes[1].node.accept_inbound_channel(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + } + }, + _ => panic!("Unexpected event"), + }; + + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), channelmanager::provided_init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id())); + + let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43); + + nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap(); + check_added_monitors!(nodes[0], 0); + + let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); + check_added_monitors!(nodes[1], 1); + + // nodes[1] happily sends its funding_signed even though its awaiting the persistence of the + // initial ChannelMonitor, but it will decline to send its channel_ready even if the funding + // transaction is confirmed. + let funding_signed_msg = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed_msg); + check_added_monitors!(nodes[0], 1); + + let as_funding_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); + if lock_commitment { + confirm_transaction(&nodes[0], &as_funding_tx[0]); + confirm_transaction(&nodes[1], &as_funding_tx[0]); + } + if use_0conf || lock_commitment { + let as_ready = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReady, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_channel_ready(&nodes[0].node.get_our_node_id(), &as_ready); + } + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + + // nodes[1] is now waiting on the first ChannelMonitor persistence to complete in order to + // move the channel to ready (or is waiting on the funding transaction to confirm). If nodes[1] + // restarts at this point with the ChannelMonitor lost, we should simply discard the channel. + + // The test framework checks that watched_txn/outputs match the monitor set, which they will + // not, so we have to clear them here. + nodes[1].chain_source.watched_txn.lock().unwrap().clear(); + nodes[1].chain_source.watched_outputs.lock().unwrap().clear(); + + let nodes_1_serialized = nodes[1].node.encode(); + persister = test_utils::TestPersister::new(); + let keys_manager = &chanmon_cfgs[1].keys_manager; + new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[1].chain_source), nodes[1].tx_broadcaster.clone(), nodes[1].logger, node_cfgs[1].fee_estimator, &persister, keys_manager); + nodes[1].chain_monitor = &new_chain_monitor; + + let mut nodes_1_read = &nodes_1_serialized[..]; + let config = UserConfig::default(); + nodes_1_deserialized = { + <(BlockHash, ChannelManager)>::read(&mut nodes_1_read, ChannelManagerReadArgs { + default_config: config, + keys_manager, + fee_estimator: node_cfgs[1].fee_estimator, + chain_monitor: nodes[1].chain_monitor, + tx_broadcaster: nodes[1].tx_broadcaster.clone(), + logger: nodes[1].logger, + channel_monitors: HashMap::new(), + }).unwrap().1 + }; + nodes[1].node = &nodes_1_deserialized; + assert!(nodes_1_read.is_empty()); + + check_closed_event!(nodes[1], 1, ClosureReason::DisconnectedPeer); + assert!(nodes[1].node.list_channels().is_empty()); +} + +#[test] +fn test_inbound_reload_without_init_mon() { + do_test_inbound_reload_without_init_mon(true, true); + do_test_inbound_reload_without_init_mon(true, false); + do_test_inbound_reload_without_init_mon(false, true); + do_test_inbound_reload_without_init_mon(false, false); +} diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 48a701e3b04..55732d704fe 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4788,6 +4788,42 @@ impl Channel { self.channel_state >= ChannelState::FundingSent as u32 } + /// Returns true if the channel is awaiting the persistence of the initial ChannelMonitor. + /// If the channel is outbound, this implies we have not yet broadcasted the funding + /// transaction. If the channel is inbound, this implies simply that the channel has not + /// advanced state. + pub fn is_awaiting_initial_mon_persist(&self) -> bool { + if !self.is_awaiting_monitor_update() { return false; } + if self.channel_state & + !(ChannelState::TheirChannelReady as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) + == ChannelState::FundingSent as u32 { + // If we're not a 0conf channel, we'll be waiting on a monitor update with only + // FundingSent set, though our peer could have sent their channel_ready. + debug_assert!(self.minimum_depth.unwrap_or(1) > 0); + return true; + } + if self.cur_holder_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 && + self.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 { + // If we're a 0-conf channel, we'll move beyond FundingSent immediately even while + // waiting for the initial monitor persistence. Thus, we check if our commitment + // transaction numbers have both been iterated only exactly once (for the + // funding_signed), and we're awaiting monitor update. + // + // If we got here, we shouldn't have yet broadcasted the funding transaction (as the + // only way to get an awaiting-monitor-update state during initial funding is if the + // initial monitor persistence is still pending). + // + // Because deciding we're awaiting initial broadcast spuriously could result in + // funds-loss (as we don't have a monitor, but have the funding transaction confirmed), + // we hard-assert here, even in production builds. + if self.is_outbound() { assert!(self.funding_transaction.is_some()); } + assert!(self.monitor_pending_channel_ready); + assert_eq!(self.latest_monitor_update_id, 0); + return true; + } + false + } + /// Returns true if our channel_ready has been sent pub fn is_our_channel_ready(&self) -> bool { (self.channel_state & ChannelState::OurChannelReady as u32) != 0 || self.channel_state >= ChannelState::ChannelFunded as u32 diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f234ad9f91f..8f0f3811ba3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6921,6 +6921,16 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> } by_id.insert(channel.channel_id(), channel); } + } else if channel.is_awaiting_initial_mon_persist() { + // If we were persisted and shut down while the initial ChannelMonitor persistence + // was in-progress, we never broadcasted the funding transaction and can still + // safely discard the channel. + let _ = channel.force_shutdown(false); + channel_closures.push(events::Event::ChannelClosed { + channel_id: channel.channel_id(), + user_channel_id: channel.get_user_id(), + reason: ClosureReason::DisconnectedPeer, + }); } else { log_error!(args.logger, "Missing ChannelMonitor for channel {} needed by ChannelManager.", log_bytes!(channel.channel_id())); log_error!(args.logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 8ddd762e970..d2aa076bd21 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -111,11 +111,19 @@ pub enum ClosureReason { /// The peer disconnected prior to funding completing. In this case the spec mandates that we /// forget the channel entirely - we can attempt again if the peer reconnects. /// + /// This includes cases where we restarted prior to funding completion, including prior to the + /// initial [`ChannelMonitor`] persistence completing. + /// /// In LDK versions prior to 0.0.107 this could also occur if we were unable to connect to the /// peer because of mutual incompatibility between us and our channel counterparty. + /// + /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor DisconnectedPeer, - /// Closure generated from `ChannelManager::read` if the ChannelMonitor is newer than - /// the ChannelManager deserialized. + /// Closure generated from `ChannelManager::read` if the [`ChannelMonitor`] is newer than + /// the [`ChannelManager`] deserialized. + /// + /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor + /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager OutdatedChannelManager } From 958601f1afd754c65e5ba0bbc946a2798970695c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 5 Oct 2022 00:34:38 +0000 Subject: [PATCH 2/2] Rename `ChannelState::MonitorUpdateFailed` `MonitorUpdateInProgress` As we're moving towards monitor update async being a supported use-case, we shouldn't call an async monitor update "failed", but rather "in progress". This simply updates the internal channel.rs enum name to reflect the new thinking. --- lightning/src/ln/chanmon_update_fail_tests.rs | 15 ++-- lightning/src/ln/channel.rs | 72 +++++++++---------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index ef7efdd221f..11434ce3a89 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -2238,11 +2238,12 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) { let (route, payment_hash_1, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100000); let (payment_preimage_2, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(&nodes[1]); - // Do a really complicated dance to get an HTLC into the holding cell, with MonitorUpdateFailed - // set but AwaitingRemoteRevoke unset. When this test was written, any attempts to send an HTLC - // while MonitorUpdateFailed is set are immediately failed-backwards. Thus, the only way to get - // an AddHTLC into the holding cell is to add it while AwaitingRemoteRevoke is set but - // MonitorUpdateFailed is unset, and then swap the flags. + // Do a really complicated dance to get an HTLC into the holding cell, with + // MonitorUpdateInProgress set but AwaitingRemoteRevoke unset. When this test was written, any + // attempts to send an HTLC while MonitorUpdateInProgress is set are immediately + // failed-backwards. Thus, the only way to get an AddHTLC into the holding cell is to add it + // while AwaitingRemoteRevoke is set but MonitorUpdateInProgress is unset, and then swap the + // flags. // // We do this by: // a) routing a payment from node B to node A, @@ -2255,8 +2256,8 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) { // e) delivering A's commitment_signed from (b) and the resulting B revoke_and_ack message, // clearing AwaitingRemoteRevoke on node A. // - // Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c) - // will not be freed from the holding cell. + // Note that because, at the end, MonitorUpdateInProgress is still set, the HTLC generated in + // (c) will not be freed from the holding cell. let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000); nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap(); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 55732d704fe..cbd04ccf2a0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -273,9 +273,9 @@ enum ChannelState { /// dance. PeerDisconnected = 1 << 7, /// Flag which is set on ChannelFunded, FundingCreated, and FundingSent indicating the user has - /// told us they failed to update our ChannelMonitor somewhere and we should pause sending any - /// outbound messages until they've managed to do so. - MonitorUpdateFailed = 1 << 8, + /// told us a ChannelMonitor update is pending async persistence somewhere and we should pause + /// sending any outbound messages until they've managed to finish. + MonitorUpdateInProgress = 1 << 8, /// Flag which implies that we have sent a commitment_signed but are awaiting the responding /// revoke_and_ack message. During this time period, we can't generate new commitment_signed /// messages as then we will be unable to determine which HTLCs they included in their @@ -295,7 +295,7 @@ enum ChannelState { ShutdownComplete = 4096, } const BOTH_SIDES_SHUTDOWN_MASK: u32 = ChannelState::LocalShutdownSent as u32 | ChannelState::RemoteShutdownSent as u32; -const MULTI_STATE_FLAGS: u32 = BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32; +const MULTI_STATE_FLAGS: u32 = BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32; pub const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; @@ -1776,7 +1776,7 @@ impl Channel { where L::Target: Logger { // Assert that we'll add the HTLC claim to the holding cell in `get_update_fulfill_htlc` // (see equivalent if condition there). - assert!(self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) != 0); + assert!(self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32) != 0); let mon_update_id = self.latest_monitor_update_id; // Forget the ChannelMonitor update let fulfill_resp = self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, logger); self.latest_monitor_update_id = mon_update_id; @@ -1846,7 +1846,7 @@ impl Channel { }], }; - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 { + if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 { // Note that this condition is the same as the assertion in // `claim_htlc_while_disconnected_dropping_mon_update` and must match exactly - // `claim_htlc_while_disconnected_dropping_mon_update` would not work correctly if we @@ -1971,7 +1971,7 @@ impl Channel { } // Now update local state: - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 { + if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 { for pending_update in self.holding_cell_htlc_updates.iter() { match pending_update { &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => { @@ -2258,7 +2258,7 @@ impl Channel { if !self.is_outbound() { return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned())); } - if self.channel_state & !(ChannelState::MonitorUpdateFailed as u32) != ChannelState::FundingCreated as u32 { + if self.channel_state & !(ChannelState::MonitorUpdateInProgress as u32) != ChannelState::FundingCreated as u32 { return Err(ChannelError::Close("Received funding_signed in strange state!".to_owned())); } if self.commitment_secrets.get_min_seen_secret() != (1 << 48) || @@ -2316,7 +2316,7 @@ impl Channel { channel_monitor.provide_latest_counterparty_commitment_tx(counterparty_initial_bitcoin_tx.txid, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger); - assert_eq!(self.channel_state & (ChannelState::MonitorUpdateFailed as u32), 0); // We have no had any monitor(s) yet to fail update! + assert_eq!(self.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update! self.channel_state = ChannelState::FundingSent as u32; self.cur_holder_commitment_transaction_number -= 1; self.cur_counterparty_commitment_transaction_number -= 1; @@ -3078,7 +3078,7 @@ impl Channel { // send_commitment_no_status_check() next which will reset this to RAAFirst. self.resend_order = RAACommitmentOrder::CommitmentFirst; - if (self.channel_state & ChannelState::MonitorUpdateFailed as u32) != 0 { + if (self.channel_state & ChannelState::MonitorUpdateInProgress as u32) != 0 { // In case we initially failed monitor updating without requiring a response, we need // to make sure the RAA gets sent first. self.monitor_pending_revoke_and_ack = true; @@ -3125,7 +3125,7 @@ impl Channel { /// returns `(None, Vec::new())`. pub fn maybe_free_holding_cell_htlcs(&mut self, logger: &L) -> Result<(Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>), ChannelError> where L::Target: Logger { if self.channel_state >= ChannelState::ChannelFunded as u32 && - (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)) == 0 { + (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32)) == 0 { self.free_holding_cell_htlcs(logger) } else { Ok((None, Vec::new())) } } @@ -3133,7 +3133,7 @@ impl Channel { /// Used to fulfill holding_cell_htlcs when we get a remote ack (or implicitly get it by them /// fulfilling or failing the last pending HTLC) fn free_holding_cell_htlcs(&mut self, logger: &L) -> Result<(Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>), ChannelError> where L::Target: Logger { - assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, 0); + assert_eq!(self.channel_state & ChannelState::MonitorUpdateInProgress as u32, 0); if self.holding_cell_htlc_updates.len() != 0 || self.holding_cell_update_fee.is_some() { log_trace!(logger, "Freeing holding cell with {} HTLC updates{} in channel {}", self.holding_cell_htlc_updates.len(), if self.holding_cell_update_fee.is_some() { " and a fee update" } else { "" }, log_bytes!(self.channel_id())); @@ -3428,7 +3428,7 @@ impl Channel { } } - if (self.channel_state & ChannelState::MonitorUpdateFailed as u32) == ChannelState::MonitorUpdateFailed as u32 { + if (self.channel_state & ChannelState::MonitorUpdateInProgress as u32) == ChannelState::MonitorUpdateInProgress as u32 { // We can't actually generate a new commitment transaction (incl by freeing holding // cells) while we can't update the monitor, so we just return what we have. if require_commitment { @@ -3557,7 +3557,7 @@ impl Channel { return None; } - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 { + if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 { self.holding_cell_update_fee = Some(feerate_per_kw); return None; } @@ -3677,15 +3677,15 @@ impl Channel { self.monitor_pending_forwards.append(&mut pending_forwards); self.monitor_pending_failures.append(&mut pending_fails); self.monitor_pending_finalized_fulfills.append(&mut pending_finalized_claimed_htlcs); - self.channel_state |= ChannelState::MonitorUpdateFailed as u32; + self.channel_state |= ChannelState::MonitorUpdateInProgress as u32; } /// Indicates that the latest ChannelMonitor update has been committed by the client /// successfully and we should restore normal operation. Returns messages which should be sent /// to the remote side. pub fn monitor_updating_restored(&mut self, logger: &L, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32) -> MonitorRestoreUpdates where L::Target: Logger { - assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32); - self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32); + assert_eq!(self.channel_state & ChannelState::MonitorUpdateInProgress as u32, ChannelState::MonitorUpdateInProgress as u32); + self.channel_state &= !(ChannelState::MonitorUpdateInProgress as u32); // If we're past (or at) the FundingSent stage on an outbound channel, try to // (re-)broadcast the funding transaction as we may have declined to broadcast it when we @@ -3700,9 +3700,9 @@ impl Channel { funding_broadcastable = None; } - // We will never broadcast the funding transaction when we're in MonitorUpdateFailed (and - // we assume the user never directly broadcasts the funding transaction and waits for us to - // do it). Thus, we can only ever hit monitor_pending_channel_ready when we're + // We will never broadcast the funding transaction when we're in MonitorUpdateInProgress + // (and we assume the user never directly broadcasts the funding transaction and waits for + // us to do it). Thus, we can only ever hit monitor_pending_channel_ready when we're // * an inbound channel that failed to persist the monitor on funding_created and we got // the funding transaction confirmed before the monitor was persisted, or // * a 0-conf channel and intended to send the channel_ready before any broadcast at all. @@ -3941,7 +3941,7 @@ impl Channel { if self.channel_state & (ChannelState::FundingSent as u32) == ChannelState::FundingSent as u32 { // If we're waiting on a monitor update, we shouldn't re-send any channel_ready's. if self.channel_state & (ChannelState::OurChannelReady as u32) == 0 || - self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 { + self.channel_state & (ChannelState::MonitorUpdateInProgress as u32) != 0 { if msg.next_remote_commitment_number != 0 { return Err(ChannelError::Close("Peer claimed they saw a revoke_and_ack but we haven't sent channel_ready yet".to_owned())); } @@ -3975,7 +3975,7 @@ impl Channel { // Note that if we need to repeat our ChannelReady we'll do that in the next if block. None } else if msg.next_remote_commitment_number + 1 == (INITIAL_COMMITMENT_NUMBER - 1) - self.cur_holder_commitment_transaction_number { - if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 { + if self.channel_state & (ChannelState::MonitorUpdateInProgress as u32) != 0 { self.monitor_pending_revoke_and_ack = true; None } else { @@ -3992,7 +3992,7 @@ impl Channel { let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.cur_counterparty_commitment_transaction_number + if (self.channel_state & ChannelState::AwaitingRemoteRevoke as u32) != 0 { 1 } else { 0 }; let channel_ready = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number == 1 { - // We should never have to worry about MonitorUpdateFailed resending ChannelReady + // We should never have to worry about MonitorUpdateInProgress resending ChannelReady let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); Some(msgs::ChannelReady { channel_id: self.channel_id(), @@ -4008,7 +4008,7 @@ impl Channel { log_debug!(logger, "Reconnected channel {} with no loss", log_bytes!(self.channel_id())); } - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) == 0 { + if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) == 0 { // We're up-to-date and not waiting on a remote revoke (if we are our // channel_reestablish should result in them sending a revoke_and_ack), but we may // have received some updates while we were disconnected. Free the holding cell @@ -4055,7 +4055,7 @@ impl Channel { log_debug!(logger, "Reconnected channel {} with only lost remote commitment tx", log_bytes!(self.channel_id())); } - if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 { + if self.channel_state & (ChannelState::MonitorUpdateInProgress as u32) != 0 { self.monitor_pending_commitment_signed = true; Ok(ReestablishResponses { channel_ready, shutdown_msg, announcement_sigs, @@ -4137,7 +4137,7 @@ impl Channel { self.pending_inbound_htlcs.is_empty() && self.pending_outbound_htlcs.is_empty() && self.channel_state & (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32 | - ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) + ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32) == BOTH_SIDES_SHUTDOWN_MASK && self.pending_update_fee.is_none() } @@ -4333,7 +4333,7 @@ impl Channel { return Err(ChannelError::Close("Remote tried to send a closing_signed when we were supposed to propose the first one".to_owned())); } - if self.channel_state & ChannelState::MonitorUpdateFailed as u32 != 0 { + if self.channel_state & ChannelState::MonitorUpdateInProgress as u32 != 0 { self.pending_counterparty_closing_signed = Some(msg.clone()); return Ok((None, None)); } @@ -4780,7 +4780,7 @@ impl Channel { /// Returns true if this channel has been marked as awaiting a monitor update to move forward. /// Allowed in any state (including after shutdown) pub fn is_awaiting_monitor_update(&self) -> bool { - (self.channel_state & ChannelState::MonitorUpdateFailed as u32) != 0 + (self.channel_state & ChannelState::MonitorUpdateInProgress as u32) != 0 } /// Returns true if funding_created was sent/received. @@ -4795,7 +4795,7 @@ impl Channel { pub fn is_awaiting_initial_mon_persist(&self) -> bool { if !self.is_awaiting_monitor_update() { return false; } if self.channel_state & - !(ChannelState::TheirChannelReady as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) + !(ChannelState::TheirChannelReady as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32) == ChannelState::FundingSent as u32 { // If we're not a 0conf channel, we'll be waiting on a monitor update with only // FundingSent set, though our peer could have sent their channel_ready. @@ -4902,7 +4902,7 @@ impl Channel { }; if need_commitment_update { - if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 { + if self.channel_state & (ChannelState::MonitorUpdateInProgress as u32) == 0 { if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 { let next_per_commitment_point = self.holder_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &self.secp_ctx); @@ -5476,9 +5476,9 @@ impl Channel { /// * In cases where we're waiting on the remote peer to send us a revoke_and_ack, we /// wouldn't be able to determine what they actually ACK'ed if we have two sets of updates /// awaiting ACK. - /// * In cases where we're marked MonitorUpdateFailed, we cannot commit to a new state as we - /// may not yet have sent the previous commitment update messages and will need to regenerate - /// them. + /// * In cases where we're marked MonitorUpdateInProgress, we cannot commit to a new state as + /// we may not yet have sent the previous commitment update messages and will need to + /// regenerate them. /// /// You MUST call send_commitment prior to calling any other methods on this Channel! /// @@ -5579,7 +5579,7 @@ impl Channel { } // Now update local state: - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 { + if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 { self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC { amount_msat, payment_hash, @@ -5626,7 +5626,7 @@ impl Channel { if (self.channel_state & (ChannelState::PeerDisconnected as u32)) == (ChannelState::PeerDisconnected as u32) { panic!("Cannot create commitment tx while disconnected, as send_htlc will have returned an Err so a send_commitment precondition has been violated"); } - if (self.channel_state & (ChannelState::MonitorUpdateFailed as u32)) == (ChannelState::MonitorUpdateFailed as u32) { + if (self.channel_state & (ChannelState::MonitorUpdateInProgress as u32)) == (ChannelState::MonitorUpdateInProgress as u32) { panic!("Cannot create commitment tx while awaiting monitor update unfreeze, as send_htlc will have returned an Err so a send_commitment precondition has been violated"); } let mut have_updates = self.is_outbound() && self.pending_update_fee.is_some(); @@ -5818,7 +5818,7 @@ impl Channel { } } assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0); - if self.channel_state & (ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) != 0 { + if self.channel_state & (ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32) != 0 { return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()}); }