Handle monitor update failures during funding on the funder side
authorMatt Corallo <git@bluematt.me>
Fri, 26 Jul 2019 22:05:05 +0000 (18:05 -0400)
committerMatt Corallo <git@bluematt.me>
Mon, 29 Jul 2019 17:26:22 +0000 (13:26 -0400)
src/ln/chanmon_update_fail_tests.rs
src/ln/channel.rs
src/ln/channelmanager.rs
src/ln/functional_test_utils.rs

index 5ff531cb9dcce32575ed7391ee70262341830d0f..4803b0054e5d466bc6aaea5c4f43489681bac9f9 100644 (file)
@@ -6,7 +6,7 @@
 use ln::channelmanager::{RAACommitmentOrder, PaymentPreimage, PaymentHash};
 use ln::channelmonitor::ChannelMonitorUpdateErr;
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, LocalFeatures};
+use ln::msgs::{ChannelMessageHandler, LocalFeatures, RoutingMessageHandler};
 use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
 use util::errors::APIError;
 
@@ -1553,3 +1553,92 @@ fn monitor_update_claim_fail_no_response() {
 
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
+
+// Note that restore_between_fails with !fail_on_generate is useless
+// Also note that !fail_on_generate && !fail_on_signed is useless
+// Finally, note that !fail_on_signed is not possible with fail_on_generate && !restore_between_fails
+fn do_during_funding_monitor_fail(fail_on_generate: bool, restore_between_fails: bool, fail_on_signed: bool) {
+       // Test that if the monitor update generated by funding_transaction_generated fails we continue
+       // the channel setup happily after the update is restored.
+       let mut nodes = create_network(2, &[None, None]);
+
+       nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 43).unwrap();
+       nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), LocalFeatures::new(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id())).unwrap();
+       nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), LocalFeatures::new(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id())).unwrap();
+
+       let (temporary_channel_id, funding_tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 43);
+
+       if fail_on_generate {
+               *nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
+       }
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       check_added_monitors!(nodes[0], 1);
+
+       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id())).unwrap();
+       check_added_monitors!(nodes[1], 1);
+
+       if restore_between_fails {
+               assert!(fail_on_generate);
+               *nodes[0].chan_monitor.update_ret.lock().unwrap() = Ok(());
+               nodes[0].node.test_restore_channel_monitor();
+               check_added_monitors!(nodes[0], 1);
+               assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+       }
+
+       if fail_on_signed {
+               *nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
+       } else {
+               assert!(restore_between_fails || !fail_on_generate); // We can't switch to good now (there's no monitor update)
+               assert!(fail_on_generate); // Somebody has to fail
+       }
+       let funding_signed_res = nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()));
+       if fail_on_signed || !restore_between_fails {
+               if let msgs::HandleError { err, action: Some(msgs::ErrorAction::IgnoreError) } = funding_signed_res.unwrap_err() {
+                       if fail_on_generate && !restore_between_fails {
+                               assert_eq!(err, "Previous monitor update failure prevented funding_signed from allowing funding broadcast");
+                               check_added_monitors!(nodes[0], 0);
+                       } else {
+                               assert_eq!(err, "Failed to update ChannelMonitor");
+                               check_added_monitors!(nodes[0], 1);
+                       }
+               } else { panic!(); }
+
+               assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+               *nodes[0].chan_monitor.update_ret.lock().unwrap() = Ok(());
+               nodes[0].node.test_restore_channel_monitor();
+       } else {
+               funding_signed_res.unwrap();
+       }
+
+       check_added_monitors!(nodes[0], 1);
+
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
+               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
+                       assert_eq!(user_channel_id, 43);
+                       assert_eq!(*funding_txo, funding_output);
+               },
+               _ => panic!("Unexpected event"),
+       };
+
+       let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &funding_tx);
+       let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked);
+       for node in nodes.iter() {
+               assert!(node.router.handle_channel_announcement(&announcement).unwrap());
+               node.router.handle_channel_update(&as_update).unwrap();
+               node.router.handle_channel_update(&bs_update).unwrap();
+       }
+
+       send_payment(&nodes[0], &[&nodes[1]], 8000000);
+       close_channel(&nodes[0], &nodes[1], &channel_id, funding_tx, true);
+}
+
+#[test]
+fn during_funding_monitor_fail() {
+       do_during_funding_monitor_fail(false, false, true);
+       do_during_funding_monitor_fail(true, false, true);
+       do_during_funding_monitor_fail(true, true, true);
+       do_during_funding_monitor_fail(true, true, false);
+}
index e838305adbddf472a6e641b01727f8308ff21c7a..1a09b187fd824e368b4d4a898439f6ca678ff646 100644 (file)
@@ -1540,7 +1540,7 @@ impl Channel {
                if !self.channel_outbound {
                        return Err(ChannelError::Close("Received funding_signed for an inbound channel?"));
                }
-               if self.channel_state != ChannelState::FundingCreated as u32 {
+               if self.channel_state & !(ChannelState::MonitorUpdateFailed as u32) != ChannelState::FundingCreated as u32 {
                        return Err(ChannelError::Close("Received funding_signed in strange state!"));
                }
                if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
@@ -1561,10 +1561,14 @@ impl Channel {
                self.sign_commitment_transaction(&mut local_initial_commitment_tx, &msg.signature);
                self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx.clone(), local_keys, self.feerate_per_kw, Vec::new());
                self.last_local_commitment_txn = vec![local_initial_commitment_tx];
-               self.channel_state = ChannelState::FundingSent as u32;
+               self.channel_state = ChannelState::FundingSent as u32 | (self.channel_state & (ChannelState::MonitorUpdateFailed as u32));
                self.cur_local_commitment_transaction_number -= 1;
 
-               Ok(self.channel_monitor.clone())
+               if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
+                       Ok(self.channel_monitor.clone())
+               } else {
+                       Err(ChannelError::Ignore("Previous monitor update failure prevented funding_signed from allowing funding broadcast"))
+               }
        }
 
        pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError> {
@@ -2345,9 +2349,10 @@ impl Channel {
        /// 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) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) {
+       pub fn monitor_updating_restored(&mut self) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool) {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
+               let needs_broadcast_safe = self.channel_state & (ChannelState::FundingSent as u32) != 0 && self.channel_outbound;
 
                let mut forwards = Vec::new();
                mem::swap(&mut forwards, &mut self.monitor_pending_forwards);
@@ -2357,7 +2362,7 @@ impl Channel {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) != 0 {
                        self.monitor_pending_revoke_and_ack = false;
                        self.monitor_pending_commitment_signed = false;
-                       return (None, None, RAACommitmentOrder::RevokeAndACKFirst, forwards, failures);
+                       return (None, None, RAACommitmentOrder::RevokeAndACKFirst, forwards, failures, needs_broadcast_safe);
                }
 
                let raa = if self.monitor_pending_revoke_and_ack {
@@ -2370,11 +2375,12 @@ impl Channel {
                self.monitor_pending_revoke_and_ack = false;
                self.monitor_pending_commitment_signed = false;
                let order = self.resend_order.clone();
-               log_trace!(self, "Restored monitor updating resulting in {} commitment update and {} RAA, with {} first",
+               log_trace!(self, "Restored monitor updating resulting in {}{} commitment update and {} RAA, with {} first",
+                       if needs_broadcast_safe { "a funding broadcast safe, " } else { "" },
                        if commitment_update.is_some() { "a" } else { "no" },
                        if raa.is_some() { "an" } else { "no" },
                        match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
-               (raa, commitment_update, order, forwards, failures)
+               (raa, commitment_update, order, forwards, failures, needs_broadcast_safe)
        }
 
        pub fn update_fee(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::UpdateFee) -> Result<(), ChannelError> {
index 10c5ce0a11cb2a8985f5e1e5d4f39a15e7b2e706..3336aa31d1b524df61d6d23f25d3d717cef43e50 100644 (file)
@@ -1131,7 +1131,7 @@ impl ChannelManager {
        pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_txo: OutPoint) {
                let _ = self.total_consistency_lock.read().unwrap();
 
-               let (chan, msg, chan_monitor) = {
+               let (mut chan, msg, chan_monitor) = {
                        let (res, chan) = {
                                let mut channel_state = self.channel_state.lock().unwrap();
                                match channel_state.by_id.remove(temporary_channel_id) {
@@ -1162,8 +1162,30 @@ impl ChannelManager {
                };
                // Because we have exclusive ownership of the channel here we can release the channel_state
                // lock before add_update_monitor
-               if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
-                       unimplemented!();
+               if let Err(e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
+                       match e {
+                               ChannelMonitorUpdateErr::PermanentFailure => {
+                                       match handle_error!(self, Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", *temporary_channel_id, chan.force_shutdown(), None))) {
+                                               Err(e) => {
+                                                       log_error!(self, "Failed to store ChannelMonitor update for funding tx generation");
+                                                       let mut channel_state = self.channel_state.lock().unwrap();
+                                                       channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
+                                                               node_id: chan.get_their_node_id(),
+                                                               action: e.action,
+                                                       });
+                                                       return;
+                                               },
+                                               Ok(()) => unreachable!(),
+                                       }
+                               },
+                               ChannelMonitorUpdateErr::TemporaryFailure => {
+                                       // Its completely fine to continue with a FundingCreated until the monitor
+                                       // update is persisted, as long as we don't generate the FundingBroadcastSafe
+                                       // until the monitor has been safely persisted (as funding broadcast is not,
+                                       // in fact, safe).
+                                       chan.monitor_update_failed(false, false, Vec::new(), Vec::new());
+                               },
+                       }
                }
 
                let mut channel_state = self.channel_state.lock().unwrap();
@@ -1627,6 +1649,7 @@ impl ChannelManager {
                let mut close_results = Vec::new();
                let mut htlc_forwards = Vec::new();
                let mut htlc_failures = Vec::new();
+               let mut pending_events = Vec::new();
                let _ = self.total_consistency_lock.read().unwrap();
 
                {
@@ -1661,7 +1684,7 @@ impl ChannelManager {
                                                        ChannelMonitorUpdateErr::TemporaryFailure => true,
                                                }
                                        } else {
-                                               let (raa, commitment_update, order, pending_forwards, mut pending_failures) = channel.monitor_updating_restored();
+                                               let (raa, commitment_update, order, pending_forwards, mut pending_failures, needs_broadcast_safe) = channel.monitor_updating_restored();
                                                if !pending_forwards.is_empty() {
                                                        htlc_forwards.push((channel.get_short_channel_id().expect("We can't have pending forwards before funding confirmation"), pending_forwards));
                                                }
@@ -1693,12 +1716,20 @@ impl ChannelManager {
                                                                handle_cs!();
                                                        },
                                                }
+                                               if needs_broadcast_safe {
+                                                       pending_events.push(events::Event::FundingBroadcastSafe {
+                                                               funding_txo: channel.get_funding_txo().unwrap(),
+                                                               user_channel_id: channel.get_user_id(),
+                                                       });
+                                               }
                                                true
                                        }
                                } else { true }
                        });
                }
 
+               self.pending_events.lock().unwrap().append(&mut pending_events);
+
                for failure in htlc_failures.drain(..) {
                        self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), failure.0, &failure.1, failure.2);
                }
@@ -1806,8 +1837,8 @@ impl ChannelManager {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                        }
                                        let chan_monitor = try_chan_entry!(self, chan.get_mut().funding_signed(&msg), channel_state, chan);
-                                       if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
-                                               unimplemented!();
+                                       if let Err(e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
+                                               return_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::RevokeAndACKFirst, false, false);
                                        }
                                        (chan.get().get_funding_txo().unwrap(), chan.get().get_user_id())
                                },
index e1be9cc173fdc59f54ba5b61b30469bd8e2a9623..5ea6961bdbf75547c6409318e58b1fb9b5c33222 100644 (file)
@@ -157,36 +157,40 @@ macro_rules! get_feerate {
        }
 }
 
+pub fn create_funding_transaction(node: &Node, expected_chan_value: u64, expected_user_chan_id: u64) -> ([u8; 32], Transaction, OutPoint) {
+       let chan_id = *node.network_chan_count.borrow();
 
-pub fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64, a_flags: LocalFeatures, b_flags: LocalFeatures) -> Transaction {
-       node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
-       node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), a_flags, &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap();
-       node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), b_flags, &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap();
-
-       let chan_id = *node_a.network_chan_count.borrow();
-       let tx;
-       let funding_output;
-
-       let events_2 = node_a.node.get_and_clear_pending_events();
-       assert_eq!(events_2.len(), 1);
-       match events_2[0] {
+       let events = node.node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
                Event::FundingGenerationReady { ref temporary_channel_id, ref channel_value_satoshis, ref output_script, user_channel_id } => {
-                       assert_eq!(*channel_value_satoshis, channel_value);
-                       assert_eq!(user_channel_id, 42);
+                       assert_eq!(*channel_value_satoshis, expected_chan_value);
+                       assert_eq!(user_channel_id, expected_user_chan_id);
 
-                       tx = Transaction { version: chan_id as u32, lock_time: 0, input: Vec::new(), output: vec![TxOut {
+                       let tx = Transaction { version: chan_id as u32, lock_time: 0, input: Vec::new(), output: vec![TxOut {
                                value: *channel_value_satoshis, script_pubkey: output_script.clone(),
                        }]};
-                       funding_output = OutPoint::new(tx.txid(), 0);
-
-                       node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
-                       let mut added_monitors = node_a.chan_monitor.added_monitors.lock().unwrap();
-                       assert_eq!(added_monitors.len(), 1);
-                       assert_eq!(added_monitors[0].0, funding_output);
-                       added_monitors.clear();
+                       let funding_outpoint = OutPoint::new(tx.txid(), 0);
+                       (*temporary_channel_id, tx, funding_outpoint)
                },
                _ => panic!("Unexpected event"),
        }
+}
+
+pub fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64, a_flags: LocalFeatures, b_flags: LocalFeatures) -> Transaction {
+       node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
+       node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), a_flags, &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap();
+       node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), b_flags, &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap();
+
+       let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, channel_value, 42);
+
+       {
+               node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
+               let mut added_monitors = node_a.chan_monitor.added_monitors.lock().unwrap();
+               assert_eq!(added_monitors.len(), 1);
+               assert_eq!(added_monitors[0].0, funding_output);
+               added_monitors.clear();
+       }
 
        node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id())).unwrap();
        {