Merge pull request #1678 from TheBlueMatt/2022-08-funding-locked-mon-persist-fail
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 19 Oct 2022 16:55:01 +0000 (16:55 +0000)
committerGitHub <noreply@github.com>
Wed, 19 Oct 2022 16:55:01 +0000 (16:55 +0000)
Handle async initial ChannelMonitor persistence failing on restart

lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/util/events.rs

index 15d46b04688930d03525b6347d6b714a28713b28..11434ce3a89f8a7d3bdef67e5547fb19489599c2 100644 (file)
@@ -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();
@@ -2745,3 +2746,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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+
+       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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+
+       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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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);
+}
index 48a701e3b04fb712772a3a9a293536e839ee6de4..cbd04ccf2a072d880b87a3f8a0958405ce8d1c4e 100644 (file)
@@ -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<Signer: Sign> Channel<Signer> {
        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<Signer: Sign> Channel<Signer> {
                        }],
                };
 
-               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<Signer: Sign> Channel<Signer> {
                }
 
                // 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<Signer: Sign> Channel<Signer> {
                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<Signer: Sign> Channel<Signer> {
 
                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<Signer: Sign> Channel<Signer> {
                // 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<Signer: Sign> Channel<Signer> {
        /// returns `(None, Vec::new())`.
        pub fn maybe_free_holding_cell_htlcs<L: Deref>(&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<Signer: Sign> Channel<Signer> {
        /// 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<L: Deref>(&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<Signer: Sign> Channel<Signer> {
                        }
                }
 
-               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<Signer: Sign> Channel<Signer> {
                        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<Signer: Sign> Channel<Signer> {
                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<L: Deref>(&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<Signer: Sign> Channel<Signer> {
                        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<Signer: Sign> Channel<Signer> {
                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<Signer: Sign> Channel<Signer> {
                        // 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<Signer: Sign> Channel<Signer> {
                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<Signer: Sign> Channel<Signer> {
                                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<Signer: Sign> Channel<Signer> {
                                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<Signer: Sign> Channel<Signer> {
                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<Signer: Sign> Channel<Signer> {
                        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<Signer: Sign> Channel<Signer> {
        /// 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.
@@ -4788,6 +4788,42 @@ impl<Signer: Sign> Channel<Signer> {
                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::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.
+                       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
@@ -4866,7 +4902,7 @@ impl<Signer: Sign> Channel<Signer> {
                };
 
                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);
@@ -5440,9 +5476,9 @@ impl<Signer: Sign> Channel<Signer> {
        /// * 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!
        ///
@@ -5543,7 +5579,7 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                // 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,
@@ -5590,7 +5626,7 @@ impl<Signer: Sign> Channel<Signer> {
                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();
@@ -5782,7 +5818,7 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                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()});
                }
 
index 95771449ba47b1cf5e2001bf9e17b90b88776690..fa98e8e9b3fe3a859837c89bbcc468059594b7e5 100644 (file)
@@ -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,");
index 20f1c5b786cf66d4c90fbe0a28c6dca3d351f8dc..83e861889bc356bff2cd2305cfd92077e13b6ded 100644 (file)
@@ -112,11 +112,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
 }