Change ShutdownResult type to better capture the possibilites

The return value from Channel::force_shutdown previously always
returned a `ChannelMonitorUpdate`, but expected it to only be
applied in the case that it *also* returned a Some for the funding
transaction output.

This is confusing, instead we move the `ChannelMontiorUpdate`
inside the Option, making it hold a tuple instead.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 633652222d11352f64be2f1f13a75d543e07eca4..16d1a0f828c66e6156d02469ca2021eed11af226 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4179,7 +4179,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
        /// Also returns the list of payment_hashes for channels which we can safely fail backwards
        /// immediately (others we will have to allow to time out).
-       pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
+       pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>) {
                assert!(self.channel_state != ChannelState::ShutdownComplete as u32);
 
                // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
@@ -4193,7 +4193,7 @@ impl<Signer: Sign> Channel<Signer> {
                                _ => {}
                        }
                }
-               let funding_txo = if let Some(funding_txo) = self.get_funding_txo() {
+               let monitor_update = if let Some(funding_txo) = self.get_funding_txo() {
                        // If we haven't yet exchanged funding signatures (ie channel_state < FundingSent),
                        // returning a channel monitor update here would imply a channel monitor update before
                        // we even registered the channel monitor to begin with, which is invalid.
@@ -4202,17 +4202,17 @@ impl<Signer: Sign> Channel<Signer> {
                        // monitor update to the user, even if we return one).
                        // See test_duplicate_chan_id and test_pre_lockin_no_chan_closed_update for more.
                        if self.channel_state & (ChannelState::FundingSent as u32 | ChannelState::ChannelFunded as u32 | ChannelState::ShutdownComplete as u32) != 0 {
-                               Some(funding_txo.clone())
+                               self.latest_monitor_update_id += 1;
+                               Some((funding_txo, ChannelMonitorUpdate {
+                                       update_id: self.latest_monitor_update_id,
+                                       updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
+                               }))
                        } else { None }
                } else { None };
 
                self.channel_state = ChannelState::ShutdownComplete as u32;
                self.update_time_counter += 1;
-               self.latest_monitor_update_id += 1;
-               (funding_txo, ChannelMonitorUpdate {
-                       update_id: self.latest_monitor_update_id,
-                       updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
-               }, dropped_outbound_htlcs)
+               (monitor_update, dropped_outbound_htlcs)
        }
 }
 

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 893cb4b32fd8dc4f7814ccfb64557e9f28f1576a..4ac2833de57a3b76e8057682e72439f25f89633f 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -206,7 +206,7 @@ pub struct PaymentPreimage(pub [u8;32]);
 #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)]
 pub struct PaymentSecret(pub [u8;32]);
 
-type ShutdownResult = (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>);
+type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>);
 
 /// Error type returned across the channel_state mutex boundary. When an Err is generated for a
 /// Channel, we generally end up with a ChannelError::Close for which we have to close the channel
@@ -942,12 +942,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        #[inline]
        fn finish_force_close_channel(&self, shutdown_res: ShutdownResult) {
-               let (funding_txo_option, monitor_update, mut failed_htlcs) = shutdown_res;
+               let (monitor_update_option, mut failed_htlcs) = shutdown_res;
                log_trace!(self.logger, "Finishing force-closure of channel {} HTLCs to fail", failed_htlcs.len());
                for htlc_source in failed_htlcs.drain(..) {
                        self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
                }
-               if let Some(funding_txo) = funding_txo_option {
+               if let Some((funding_txo, monitor_update)) = monitor_update_option {
                        // There isn't anything we can do if we get an update failure - we're already
                        // force-closing. The monitor update on the required in-memory copy should broadcast
                        // the latest local state, which is the best we can do anyway. Thus, it is safe to
@@ -2418,7 +2418,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        // We do not do a force-close here as that would generate a monitor update for
                                        // a monitor that we didn't manage to store (and that we don't care about - we
                                        // don't respond with the funding_signed so the channel can never go on chain).
-                                       let (_funding_txo_option, _monitor_update, failed_htlcs) = chan.force_shutdown(true);
+                                       let (_monitor_update, failed_htlcs) = chan.force_shutdown(true);
                                        assert!(failed_htlcs.is_empty());
                                        return Err(MsgHandleErrInternal::send_err_msg_no_close("ChannelMonitor storage failure".to_owned(), funding_msg.channel_id));
                                },
@@ -4071,7 +4071,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                                channel.get_cur_counterparty_commitment_transaction_number() > monitor.get_cur_counterparty_commitment_number() ||
                                                channel.get_latest_monitor_update_id() < monitor.get_latest_update_id() {
                                        // But if the channel is behind of the monitor, close the channel:
-                                       let (_, _, mut new_failed_htlcs) = channel.force_shutdown(true);
+                                       let (_, mut new_failed_htlcs) = channel.force_shutdown(true);
                                        failed_htlcs.append(&mut new_failed_htlcs);
                                        monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
                                } else {