Send initial closing_signed message asynchronously and handle errs
[rust-lightning] / lightning / src / ln / channel.rs
index 9a5fd0422228e1e46f4bdd3d27c6cb1c246575a8..373bfef13435359dbb7fe53e997a245e618d1533 100644 (file)
@@ -455,6 +455,11 @@ pub(super) struct Channel<Signer: Sign> {
 
        last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig)
 
+       /// If our counterparty sent us a closing_signed while we were waiting for a `ChannelMonitor`
+       /// update, we need to delay processing it until later. We do that here by simply storing the
+       /// closing_signed message and handling it in `maybe_propose_closing_signed`.
+       pending_counterparty_closing_signed: Option<msgs::ClosingSigned>,
+
        /// The hash of the block in which the funding transaction was included.
        funding_tx_confirmed_in: Option<BlockHash>,
        funding_tx_confirmation_height: u32,
@@ -574,6 +579,7 @@ pub const MIN_DUST_LIMIT_SATOSHIS: u64 = 330;
 /// channel_id in ChannelManager.
 pub(super) enum ChannelError {
        Ignore(String),
+       Warn(String),
        Close(String),
        CloseDelayBroadcast(String),
 }
@@ -582,6 +588,7 @@ impl fmt::Debug for ChannelError {
        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                match self {
                        &ChannelError::Ignore(ref e) => write!(f, "Ignore : {}", e),
+                       &ChannelError::Warn(ref e) => write!(f, "Warn : {}", e),
                        &ChannelError::Close(ref e) => write!(f, "Close : {}", e),
                        &ChannelError::CloseDelayBroadcast(ref e) => write!(f, "CloseDelayBroadcast : {}", e)
                }
@@ -693,6 +700,7 @@ impl<Signer: Sign> Channel<Signer> {
                        counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
 
                        last_sent_closing_fee: None,
+                       pending_counterparty_closing_signed: None,
 
                        funding_tx_confirmed_in: None,
                        funding_tx_confirmation_height: 0,
@@ -750,7 +758,12 @@ impl<Signer: Sign> Channel<Signer> {
                if feerate_per_kw < lower_limit {
                        return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
                }
-               let upper_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64  * 2;
+               // We only bound the fee updates on the upper side to prevent completely absurd feerates,
+               // always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
+               // We generally don't care too much if they set the feerate to something very high, but it
+               // could result in the channel being useless due to everything being dust.
+               let upper_limit = cmp::max(250 * 25,
+                       fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
                if feerate_per_kw as u64 > upper_limit {
                        return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
                }
@@ -947,6 +960,7 @@ impl<Signer: Sign> Channel<Signer> {
                        counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
 
                        last_sent_closing_fee: None,
+                       pending_counterparty_closing_signed: None,
 
                        funding_tx_confirmed_in: None,
                        funding_tx_confirmation_height: 0,
@@ -2366,9 +2380,8 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(())
        }
 
-       pub fn commitment_signed<F: Deref, L: Deref>(&mut self, msg: &msgs::CommitmentSigned, fee_estimator: &F, logger: &L) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>, Option<msgs::ClosingSigned>, ChannelMonitorUpdate), (Option<ChannelMonitorUpdate>, ChannelError)>
-       where F::Target: FeeEstimator,
-                               L::Target: Logger
+       pub fn commitment_signed<L: Deref>(&mut self, msg: &msgs::CommitmentSigned, logger: &L) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>, ChannelMonitorUpdate), (Option<ChannelMonitorUpdate>, ChannelError)>
+               where L::Target: Logger
        {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err((None, ChannelError::Close("Got commitment signed message when channel was not in an operational state".to_owned())));
@@ -2534,12 +2547,10 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                        log_debug!(logger, "Received valid commitment_signed from peer in channel {}, updated HTLC state but awaiting a monitor update resolution to reply.",
                                log_bytes!(self.channel_id));
-                       // TODO: Call maybe_propose_first_closing_signed on restoration (or call it here and
-                       // re-send the message on restoration)
                        return Err((Some(monitor_update), ChannelError::Ignore("Previous monitor update failure prevented generation of RAA".to_owned())));
                }
 
-               let (commitment_signed, closing_signed) = if need_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 {
+               let commitment_signed = if need_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 {
                        // If we're AwaitingRemoteRevoke we can't send a new commitment here, but that's ok -
                        // we'll send one right away when we get the revoke_and_ack when we
                        // free_holding_cell_htlcs().
@@ -2548,10 +2559,8 @@ impl<Signer: Sign> Channel<Signer> {
                        // strictly increasing by one, so decrement it here.
                        self.latest_monitor_update_id = monitor_update.update_id;
                        monitor_update.updates.append(&mut additional_update.updates);
-                       (Some(msg), None)
-               } else if !need_commitment {
-                       (None, self.maybe_propose_first_closing_signed(fee_estimator))
-               } else { (None, None) };
+                       Some(msg)
+               } else { None };
 
                log_debug!(logger, "Received valid commitment_signed from peer in channel {}, updating HTLC state and responding with{} a revoke_and_ack.",
                        log_bytes!(self.channel_id()), if commitment_signed.is_some() { " our own commitment_signed and" } else { "" });
@@ -2560,7 +2569,7 @@ impl<Signer: Sign> Channel<Signer> {
                        channel_id: self.channel_id,
                        per_commitment_secret,
                        next_per_commitment_point,
-               }, commitment_signed, closing_signed, monitor_update))
+               }, commitment_signed, monitor_update))
        }
 
        /// Public version of the below, checking relevant preconditions first.
@@ -2697,9 +2706,8 @@ impl<Signer: Sign> Channel<Signer> {
        /// waiting on this revoke_and_ack. The generation of this new commitment_signed may also fail,
        /// generating an appropriate error *after* the channel state has been updated based on the
        /// revoke_and_ack message.
-       pub fn revoke_and_ack<F: Deref, L: Deref>(&mut self, msg: &msgs::RevokeAndACK, fee_estimator: &F, logger: &L) -> Result<(Option<msgs::CommitmentUpdate>, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option<msgs::ClosingSigned>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>), ChannelError>
-               where F::Target: FeeEstimator,
-                                       L::Target: Logger,
+       pub fn revoke_and_ack<L: Deref>(&mut self, msg: &msgs::RevokeAndACK, logger: &L) -> Result<(Option<msgs::CommitmentUpdate>, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>), ChannelError>
+               where L::Target: Logger,
        {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Close("Got revoke/ACK message when channel was not in an operational state".to_owned()));
@@ -2880,7 +2888,7 @@ impl<Signer: Sign> Channel<Signer> {
                        self.monitor_pending_forwards.append(&mut to_forward_infos);
                        self.monitor_pending_failures.append(&mut revoked_htlcs);
                        log_debug!(logger, "Received a valid revoke_and_ack for channel {} but awaiting a monitor update resolution to reply.", log_bytes!(self.channel_id()));
-                       return Ok((None, Vec::new(), Vec::new(), None, monitor_update, Vec::new()))
+                       return Ok((None, Vec::new(), Vec::new(), monitor_update, Vec::new()))
                }
 
                match self.free_holding_cell_htlcs(logger)? {
@@ -2899,7 +2907,7 @@ impl<Signer: Sign> Channel<Signer> {
                                self.latest_monitor_update_id = monitor_update.update_id;
                                monitor_update.updates.append(&mut additional_update.updates);
 
-                               Ok((Some(commitment_update), to_forward_infos, revoked_htlcs, None, monitor_update, htlcs_to_fail))
+                               Ok((Some(commitment_update), to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail))
                        },
                        (None, htlcs_to_fail) => {
                                if require_commitment {
@@ -2919,14 +2927,13 @@ impl<Signer: Sign> Channel<Signer> {
                                                update_fail_malformed_htlcs,
                                                update_fee: None,
                                                commitment_signed
-                                       }), to_forward_infos, revoked_htlcs, None, monitor_update, htlcs_to_fail))
+                                       }), to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail))
                                } else {
                                        log_debug!(logger, "Received a valid revoke_and_ack for channel {} with no reply necessary.", log_bytes!(self.channel_id()));
-                                       Ok((None, to_forward_infos, revoked_htlcs, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, htlcs_to_fail))
+                                       Ok((None, to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail))
                                }
                        }
                }
-
        }
 
        /// Adds a pending update to this channel. See the doc for send_htlc for
@@ -2981,6 +2988,7 @@ impl<Signer: Sign> Channel<Signer> {
                // Upon reconnect we have to start the closing_signed dance over, but shutdown messages
                // will be retransmitted.
                self.last_sent_closing_fee = None;
+               self.pending_counterparty_closing_signed = None;
 
                let mut inbound_drop_count = 0;
                self.pending_inbound_htlcs.retain(|htlc| {
@@ -3111,8 +3119,27 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned()));
                }
                Channel::<Signer>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
+               let feerate_over_dust_buffer = msg.feerate_per_kw > self.get_dust_buffer_feerate();
+
                self.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
                self.update_time_counter += 1;
+               // If the feerate has increased over the previous dust buffer (note that
+               // `get_dust_buffer_feerate` considers the `pending_update_fee` status), check that we
+               // won't be pushed over our dust exposure limit by the feerate increase.
+               if feerate_over_dust_buffer {
+                       let inbound_stats = self.get_inbound_pending_htlc_stats();
+                       let outbound_stats = self.get_outbound_pending_htlc_stats();
+                       let holder_tx_dust_exposure = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat;
+                       let counterparty_tx_dust_exposure = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat;
+                       if holder_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() {
+                               return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)",
+                                       msg.feerate_per_kw, holder_tx_dust_exposure)));
+                       }
+                       if counterparty_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() {
+                               return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)",
+                                       msg.feerate_per_kw, counterparty_tx_dust_exposure)));
+                       }
+               }
                Ok(())
        }
 
@@ -3299,7 +3326,8 @@ impl<Signer: Sign> Channel<Signer> {
                                // now!
                                match self.free_holding_cell_htlcs(logger) {
                                        Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)),
-                                       Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast(_)) => panic!("Got non-channel-failing result from free_holding_cell_htlcs"),
+                                       Err(ChannelError::Warn(_)) | Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast(_)) =>
+                                               panic!("Got non-channel-failing result from free_holding_cell_htlcs"),
                                        Ok((Some((commitment_update, monitor_update)), htlcs_to_fail)) => {
                                                return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), htlcs_to_fail, shutdown_msg));
                                        },
@@ -3328,13 +3356,24 @@ impl<Signer: Sign> Channel<Signer> {
                }
        }
 
-       fn maybe_propose_first_closing_signed<F: Deref>(&mut self, fee_estimator: &F) -> Option<msgs::ClosingSigned>
-               where F::Target: FeeEstimator
+       pub fn maybe_propose_closing_signed<F: Deref, L: Deref>(&mut self, fee_estimator: &F, logger: &L)
+               -> Result<(Option<msgs::ClosingSigned>, Option<Transaction>), ChannelError>
+               where F::Target: FeeEstimator, L::Target: Logger
        {
-               if !self.is_outbound() || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() ||
-                               self.channel_state & (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32) != BOTH_SIDES_SHUTDOWN_MASK ||
+               if !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)
+                               != BOTH_SIDES_SHUTDOWN_MASK ||
                                self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() {
-                       return None;
+                       return Ok((None, None));
+               }
+
+               if !self.is_outbound() {
+                       if let Some(msg) = &self.pending_counterparty_closing_signed.take() {
+                               return self.closing_signed(fee_estimator, &msg);
+                       }
+                       return Ok((None, None));
                }
 
                let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
@@ -3344,28 +3383,27 @@ impl<Signer: Sign> Channel<Signer> {
                assert!(self.shutdown_scriptpubkey.is_some());
                let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
                let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000;
+               log_trace!(logger, "Proposing initial closing signed for our counterparty with a feerate of {} sat/kWeight (= {} sats)",
+                       proposed_feerate, proposed_total_fee_satoshis);
 
                let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                let sig = self.holder_signer
                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
-                       .ok();
-               assert!(closing_tx.get_weight() as u64 <= tx_weight);
-               if sig.is_none() { return None; }
+                       .map_err(|()| ChannelError::Close("Failed to get signature for closing transaction.".to_owned()))?;
 
-               self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, sig.clone().unwrap()));
-               Some(msgs::ClosingSigned {
+               self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, sig.clone()));
+               Ok((Some(msgs::ClosingSigned {
                        channel_id: self.channel_id,
                        fee_satoshis: total_fee_satoshis,
-                       signature: sig.unwrap(),
-               })
+                       signature: sig,
+                       fee_range: None,
+               }), None))
        }
 
-       pub fn shutdown<F: Deref, K: Deref>(
-               &mut self, fee_estimator: &F, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown
-       ) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>), ChannelError>
-       where
-               F::Target: FeeEstimator,
-               K::Target: KeysInterface<Signer = Signer>
+       pub fn shutdown<K: Deref>(
+               &mut self, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown
+       ) -> Result<(Option<msgs::Shutdown>, Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>), ChannelError>
+       where K::Target: KeysInterface<Signer = Signer>
        {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
                        return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish".to_owned()));
@@ -3453,7 +3491,7 @@ impl<Signer: Sign> Channel<Signer> {
                self.channel_state |= ChannelState::LocalShutdownSent as u32;
                self.update_time_counter += 1;
 
-               Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, dropped_outbound_htlcs))
+               Ok((shutdown, monitor_update, dropped_outbound_htlcs))
        }
 
        fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, sig: &Signature) {
@@ -3494,6 +3532,11 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Remote tried to send us a closing tx with > 21 million BTC fee".to_owned()));
                }
 
+               if self.channel_state & ChannelState::MonitorUpdateFailed as u32 != 0 {
+                       self.pending_counterparty_closing_signed = Some(msg.clone());
+                       return Ok((None, None));
+               }
+
                let funding_redeemscript = self.get_funding_redeemscript();
                let (mut closing_tx, used_total_fee) = self.build_closing_transaction(msg.fee_satoshis, false);
                if used_total_fee != msg.fee_satoshis {
@@ -3540,6 +3583,7 @@ impl<Signer: Sign> Channel<Signer> {
                                        channel_id: self.channel_id,
                                        fee_satoshis: used_total_fee,
                                        signature: sig,
+                                       fee_range: None,
                                }), None))
                        }
                }
@@ -3581,6 +3625,7 @@ impl<Signer: Sign> Channel<Signer> {
                        channel_id: self.channel_id,
                        fee_satoshis: msg.fee_satoshis,
                        signature: sig,
+                       fee_range: None,
                }), Some(closing_tx)))
        }
 
@@ -3684,7 +3729,11 @@ impl<Signer: Sign> Channel<Signer> {
                // whichever is higher. This ensures that we aren't suddenly exposed to significantly
                // more dust balance if the feerate increases when we have several HTLCs pending
                // which are near the dust limit.
-               cmp::max(2530, self.feerate_per_kw * 1250 / 1000)
+               let mut feerate_per_kw = self.feerate_per_kw;
+               if let Some((feerate, _)) = self.pending_update_fee {
+                       feerate_per_kw = cmp::max(feerate_per_kw, feerate);
+               }
+               cmp::max(2530, feerate_per_kw * 1250 / 1000)
        }
 
        pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 {
@@ -3782,6 +3831,16 @@ impl<Signer: Sign> Channel<Signer> {
                self.channel_state >= ChannelState::FundingSent as u32
        }
 
+       /// Returns true if our peer has either initiated or agreed to shut down the channel.
+       pub fn received_shutdown(&self) -> bool {
+               (self.channel_state & ChannelState::RemoteShutdownSent as u32) != 0
+       }
+
+       /// Returns true if we either initiated or agreed to shut down the channel.
+       pub fn sent_shutdown(&self) -> bool {
+               (self.channel_state & ChannelState::LocalShutdownSent as u32) != 0
+       }
+
        /// Returns true if this channel is fully shut down. True here implies that no further actions
        /// may/will be taken on this channel, and thus this object should be freed. Any future changes
        /// will be handled appropriately by the chain monitor.
@@ -4462,6 +4521,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
        /// Only fails in case of bad keys
        fn send_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
+               log_trace!(logger, "Updating HTLC state for a newly-sent commitment_signed...");
                // We can upgrade the status of some HTLCs that are waiting on a commitment, even if we
                // fail to generate this, we still are at least at a position where upgrading their status
                // is acceptable.
@@ -4470,6 +4530,7 @@ impl<Signer: Sign> Channel<Signer> {
                                Some(InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info.clone()))
                        } else { None };
                        if let Some(state) = new_state {
+                               log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce {} to AwaitingAnnouncedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
                                htlc.state = state;
                        }
                }
@@ -4477,12 +4538,14 @@ impl<Signer: Sign> Channel<Signer> {
                        if let Some(fail_reason) = if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut fail_reason) = &mut htlc.state {
                                Some(fail_reason.take())
                        } else { None } {
+                               log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
                                htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(fail_reason);
                        }
                }
                if let Some((feerate, update_state)) = self.pending_update_fee {
                        if update_state == FeeUpdateState::AwaitingRemoteRevokeToAnnounce {
                                debug_assert!(!self.is_outbound());
+                               log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce fee update {} to Committed", feerate);
                                self.feerate_per_kw = feerate;
                                self.pending_update_fee = None;
                        }
@@ -4925,15 +4988,11 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                self.update_time_counter.write(writer)?;
                self.feerate_per_kw.write(writer)?;
 
-               match self.last_sent_closing_fee {
-                       Some((feerate, fee, sig)) => {
-                               1u8.write(writer)?;
-                               feerate.write(writer)?;
-                               fee.write(writer)?;
-                               sig.write(writer)?;
-                       },
-                       None => 0u8.write(writer)?,
-               }
+               // Versions prior to 0.0.100 expected to read the fields of `last_sent_closing_fee` here,
+               // however we are supposed to restart shutdown fee negotiation on reconnect (and wipe
+               // `last_send_closing_fee` in `remove_uncommitted_htlcs_and_mark_paused`) so we should never
+               // consider the stale state on reload.
+               0u8.write(writer)?;
 
                self.funding_tx_confirmed_in.write(writer)?;
                self.funding_tx_confirmation_height.write(writer)?;
@@ -5141,11 +5200,19 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                let update_time_counter = Readable::read(reader)?;
                let feerate_per_kw = Readable::read(reader)?;
 
-               let last_sent_closing_fee = match <u8 as Readable>::read(reader)? {
-                       0 => None,
-                       1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)),
+               // Versions prior to 0.0.100 expected to read the fields of `last_sent_closing_fee` here,
+               // however we are supposed to restart shutdown fee negotiation on reconnect (and wipe
+               // `last_send_closing_fee` in `remove_uncommitted_htlcs_and_mark_paused`) so we should never
+               // consider the stale state on reload.
+               match <u8 as Readable>::read(reader)? {
+                       0 => {},
+                       1 => {
+                               let _: u32 = Readable::read(reader)?;
+                               let _: u64 = Readable::read(reader)?;
+                               let _: Signature = Readable::read(reader)?;
+                       },
                        _ => return Err(DecodeError::InvalidValue),
-               };
+               }
 
                let funding_tx_confirmed_in = Readable::read(reader)?;
                let funding_tx_confirmation_height = Readable::read(reader)?;
@@ -5273,7 +5340,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        #[cfg(debug_assertions)]
                        counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
 
-                       last_sent_closing_fee,
+                       last_sent_closing_fee: None,
+                       pending_counterparty_closing_signed: None,
 
                        funding_tx_confirmed_in,
                        funding_tx_confirmation_height,