Merge pull request #1859 from TheBlueMatt/2022-11-rm-redundant-holding-cell-wipe
[rust-lightning] / lightning / src / ln / channelmanager.rs
index 04adc124d65a0fa6c0379d85c88f7e488d2c209d..b22dacdd686d1ceca2511c8b44046c14fa7e787f 100644 (file)
@@ -1521,134 +1521,6 @@ macro_rules! emit_channel_ready_event {
        }
 }
 
-macro_rules! handle_chan_restoration_locked {
-       ($self: ident, $channel_lock: expr, $channel_state: expr, $channel_entry: expr,
-        $raa: expr, $commitment_update: expr, $order: expr, $chanmon_update: expr,
-        $pending_forwards: expr, $funding_broadcastable: expr, $channel_ready: expr, $announcement_sigs: expr) => { {
-               let mut htlc_forwards = None;
-
-               let chanmon_update: Option<ChannelMonitorUpdate> = $chanmon_update; // Force type-checking to resolve
-               let chanmon_update_is_none = chanmon_update.is_none();
-               let counterparty_node_id = $channel_entry.get().get_counterparty_node_id();
-               let res = loop {
-                       let forwards: Vec<(PendingHTLCInfo, u64)> = $pending_forwards; // Force type-checking to resolve
-                       if !forwards.is_empty() {
-                               htlc_forwards = Some(($channel_entry.get().get_short_channel_id().unwrap_or($channel_entry.get().outbound_scid_alias()),
-                                       $channel_entry.get().get_funding_txo().unwrap(), forwards));
-                       }
-
-                       if chanmon_update.is_some() {
-                               // On reconnect, we, by definition, only resend a channel_ready if there have been
-                               // no commitment updates, so the only channel monitor update which could also be
-                               // associated with a channel_ready would be the funding_created/funding_signed
-                               // monitor update. That monitor update failing implies that we won't send
-                               // channel_ready until it's been updated, so we can't have a channel_ready and a
-                               // monitor update here (so we don't bother to handle it correctly below).
-                               assert!($channel_ready.is_none());
-                               // A channel monitor update makes no sense without either a channel_ready or a
-                               // commitment update to process after it. Since we can't have a channel_ready, we
-                               // only bother to handle the monitor-update + commitment_update case below.
-                               assert!($commitment_update.is_some());
-                       }
-
-                       if let Some(msg) = $channel_ready {
-                               // Similar to the above, this implies that we're letting the channel_ready fly
-                               // before it should be allowed to.
-                               assert!(chanmon_update.is_none());
-                               send_channel_ready!($self, $channel_state.pending_msg_events, $channel_entry.get(), msg);
-                       }
-                       if let Some(msg) = $announcement_sigs {
-                               $channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
-                                       node_id: counterparty_node_id,
-                                       msg,
-                               });
-                       }
-
-                       emit_channel_ready_event!($self, $channel_entry.get_mut());
-
-                       let funding_broadcastable: Option<Transaction> = $funding_broadcastable; // Force type-checking to resolve
-                       if let Some(monitor_update) = chanmon_update {
-                               // We only ever broadcast a funding transaction in response to a funding_signed
-                               // message and the resulting monitor update. Thus, on channel_reestablish
-                               // message handling we can't have a funding transaction to broadcast. When
-                               // processing a monitor update finishing resulting in a funding broadcast, we
-                               // cannot have a second monitor update, thus this case would indicate a bug.
-                               assert!(funding_broadcastable.is_none());
-                               // Given we were just reconnected or finished updating a channel monitor, the
-                               // only case where we can get a new ChannelMonitorUpdate would be if we also
-                               // have some commitment updates to send as well.
-                               assert!($commitment_update.is_some());
-                               match $self.chain_monitor.update_channel($channel_entry.get().get_funding_txo().unwrap(), monitor_update) {
-                                       ChannelMonitorUpdateStatus::Completed => {},
-                                       e => {
-                                               // channel_reestablish doesn't guarantee the order it returns is sensical
-                                               // for the messages it returns, but if we're setting what messages to
-                                               // re-transmit on monitor update success, we need to make sure it is sane.
-                                               let mut order = $order;
-                                               if $raa.is_none() {
-                                                       order = RAACommitmentOrder::CommitmentFirst;
-                                               }
-                                               break handle_monitor_update_res!($self, e, $channel_entry, order, $raa.is_some(), true);
-                                       }
-                               }
-                       }
-
-                       macro_rules! handle_cs { () => {
-                               if let Some(update) = $commitment_update {
-                                       $channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
-                                               node_id: counterparty_node_id,
-                                               updates: update,
-                                       });
-                               }
-                       } }
-                       macro_rules! handle_raa { () => {
-                               if let Some(revoke_and_ack) = $raa {
-                                       $channel_state.pending_msg_events.push(events::MessageSendEvent::SendRevokeAndACK {
-                                               node_id: counterparty_node_id,
-                                               msg: revoke_and_ack,
-                                       });
-                               }
-                       } }
-                       match $order {
-                               RAACommitmentOrder::CommitmentFirst => {
-                                       handle_cs!();
-                                       handle_raa!();
-                               },
-                               RAACommitmentOrder::RevokeAndACKFirst => {
-                                       handle_raa!();
-                                       handle_cs!();
-                               },
-                       }
-                       if let Some(tx) = funding_broadcastable {
-                               log_info!($self.logger, "Broadcasting funding transaction with txid {}", tx.txid());
-                               $self.tx_broadcaster.broadcast_transaction(&tx);
-                       }
-                       break Ok(());
-               };
-
-               if chanmon_update_is_none {
-                       // If there was no ChannelMonitorUpdate, we should never generate an Err in the res loop
-                       // above. Doing so would imply calling handle_err!() from channel_monitor_updated() which
-                       // should *never* end up calling back to `chain_monitor.update_channel()`.
-                       assert!(res.is_ok());
-               }
-
-               (htlc_forwards, res, counterparty_node_id)
-       } }
-}
-
-macro_rules! post_handle_chan_restoration {
-       ($self: ident, $locked_res: expr) => { {
-               let (htlc_forwards, res, counterparty_node_id) = $locked_res;
-
-               let _ = handle_error!($self, res, counterparty_node_id);
-
-               if let Some(forwards) = htlc_forwards {
-                       $self.forward_htlcs(&mut [forwards][..]);
-               }
-       } }
-}
-
 impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F, L>
        where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
@@ -4494,10 +4366,73 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                self.our_network_pubkey.clone()
        }
 
+       /// Handles a channel reentering a functional state, either due to reconnect or a monitor
+       /// update completion.
+       fn handle_channel_resumption(&self, pending_msg_events: &mut Vec<MessageSendEvent>,
+               channel: &mut Channel<<K::Target as KeysInterface>::Signer>, raa: Option<msgs::RevokeAndACK>,
+               commitment_update: Option<msgs::CommitmentUpdate>, order: RAACommitmentOrder,
+               pending_forwards: Vec<(PendingHTLCInfo, u64)>, funding_broadcastable: Option<Transaction>,
+               channel_ready: Option<msgs::ChannelReady>, announcement_sigs: Option<msgs::AnnouncementSignatures>)
+       -> Option<(u64, OutPoint, Vec<(PendingHTLCInfo, u64)>)> {
+               let mut htlc_forwards = None;
+
+               let counterparty_node_id = channel.get_counterparty_node_id();
+               if !pending_forwards.is_empty() {
+                       htlc_forwards = Some((channel.get_short_channel_id().unwrap_or(channel.outbound_scid_alias()),
+                               channel.get_funding_txo().unwrap(), pending_forwards));
+               }
+
+               if let Some(msg) = channel_ready {
+                       send_channel_ready!(self, pending_msg_events, channel, msg);
+               }
+               if let Some(msg) = announcement_sigs {
+                       pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+                               node_id: counterparty_node_id,
+                               msg,
+                       });
+               }
+
+               emit_channel_ready_event!(self, channel);
+
+               macro_rules! handle_cs { () => {
+                       if let Some(update) = commitment_update {
+                               pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
+                                       node_id: counterparty_node_id,
+                                       updates: update,
+                               });
+                       }
+               } }
+               macro_rules! handle_raa { () => {
+                       if let Some(revoke_and_ack) = raa {
+                               pending_msg_events.push(events::MessageSendEvent::SendRevokeAndACK {
+                                       node_id: counterparty_node_id,
+                                       msg: revoke_and_ack,
+                               });
+                       }
+               } }
+               match order {
+                       RAACommitmentOrder::CommitmentFirst => {
+                               handle_cs!();
+                               handle_raa!();
+                       },
+                       RAACommitmentOrder::RevokeAndACKFirst => {
+                               handle_raa!();
+                               handle_cs!();
+                       },
+               }
+
+               if let Some(tx) = funding_broadcastable {
+                       log_info!(self.logger, "Broadcasting funding transaction with txid {}", tx.txid());
+                       self.tx_broadcaster.broadcast_transaction(&tx);
+               }
+
+               htlc_forwards
+       }
+
        fn channel_monitor_updated(&self, funding_txo: &OutPoint, highest_applied_update_id: u64) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
-               let chan_restoration_res;
+               let htlc_forwards;
                let (mut pending_failures, finalized_claims, counterparty_node_id) = {
                        let mut channel_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_lock;
@@ -4524,14 +4459,16 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                        })
                                } else { None }
                        } else { None };
-                       chan_restoration_res = handle_chan_restoration_locked!(self, channel_lock, channel_state, channel, updates.raa, updates.commitment_update, updates.order, None, updates.accepted_htlcs, updates.funding_broadcastable, updates.channel_ready, updates.announcement_sigs);
+                       htlc_forwards = self.handle_channel_resumption(&mut channel_state.pending_msg_events, channel.get_mut(), updates.raa, updates.commitment_update, updates.order, updates.accepted_htlcs, updates.funding_broadcastable, updates.channel_ready, updates.announcement_sigs);
                        if let Some(upd) = channel_update {
                                channel_state.pending_msg_events.push(upd);
                        }
 
                        (updates.failed_htlcs, updates.finalized_claimed_htlcs, counterparty_node_id)
                };
-               post_handle_chan_restoration!(self, chan_restoration_res);
+               if let Some(forwards) = htlc_forwards {
+                       self.forward_htlcs(&mut [forwards][..]);
+               }
                self.finalize_claims(finalized_claims);
                for failure in pending_failures.drain(..) {
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id: funding_txo.to_channel_id() };
@@ -5282,8 +5219,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        }
 
        fn internal_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), MsgHandleErrInternal> {
-               let chan_restoration_res;
-               let (htlcs_failed_forward, need_lnd_workaround) = {
+               let htlc_forwards;
+               let need_lnd_workaround = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_state_lock;
 
@@ -5317,19 +5254,21 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                }
                                        }
                                        let need_lnd_workaround = chan.get_mut().workaround_lnd_bug_4006.take();
-                                       chan_restoration_res = handle_chan_restoration_locked!(
-                                               self, channel_state_lock, channel_state, chan, responses.raa, responses.commitment_update, responses.order,
-                                               responses.mon_update, Vec::new(), None, responses.channel_ready, responses.announcement_sigs);
+                                       htlc_forwards = self.handle_channel_resumption(
+                                               &mut channel_state.pending_msg_events, chan.get_mut(), responses.raa, responses.commitment_update, responses.order,
+                                               Vec::new(), None, responses.channel_ready, responses.announcement_sigs);
                                        if let Some(upd) = channel_update {
                                                channel_state.pending_msg_events.push(upd);
                                        }
-                                       (responses.holding_cell_failed_htlcs, need_lnd_workaround)
+                                       need_lnd_workaround
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
                        }
                };
-               post_handle_chan_restoration!(self, chan_restoration_res);
-               self.fail_holding_cell_htlcs(htlcs_failed_forward, msg.channel_id, counterparty_node_id);
+
+               if let Some(forwards) = htlc_forwards {
+                       self.forward_htlcs(&mut [forwards][..]);
+               }
 
                if let Some(channel_ready_msg) = need_lnd_workaround {
                        self.internal_channel_ready(counterparty_node_id, &channel_ready_msg)?;