Merge pull request #1699 from TheBlueMatt/2022-08-announcement-rework
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 9 Sep 2022 00:34:10 +0000 (00:34 +0000)
committerGitHub <noreply@github.com>
Fri, 9 Sep 2022 00:34:10 +0000 (00:34 +0000)
1  2 
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/util/events.rs
lightning/src/util/test_utils.rs

index 794ac0db72277b40f90964df70a8b1d2a10e7525,edfad1865c0d0f50343dcec555468dfa76e2a982..0080753336b94bb9ffcdbb72a82add6a3df39570
@@@ -1126,8 -1126,8 +1126,8 @@@ fn test_monitor_update_fail_reestablish
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
  
-       let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
-       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let as_reestablish = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
+       let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
  
        nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
  
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
  
-       assert!(as_reestablish == get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id()));
-       assert!(bs_reestablish == get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id()));
+       assert_eq!(get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap(), as_reestablish);
+       assert_eq!(get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap(), bs_reestablish);
  
        nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
        assert_eq!(
@@@ -1319,8 -1319,8 +1319,8 @@@ fn claim_while_disconnected_monitor_upd
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
  
-       let as_reconnect = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
-       let bs_reconnect = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let as_reconnect = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
+       let bs_reconnect = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
  
        nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reconnect);
        let _as_channel_update = get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
@@@ -1451,8 -1451,8 +1451,8 @@@ fn monitor_failed_no_reestablish_respon
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
  
-       let as_reconnect = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
-       let bs_reconnect = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let as_reconnect = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
+       let bs_reconnect = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
  
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reconnect);
        let _bs_channel_update = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());
@@@ -1744,9 -1744,9 +1744,9 @@@ fn test_monitor_update_on_pending_forwa
  
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
 -      if let Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] {
 +      if let Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } = events[0] {
                assert_eq!(payment_hash, payment_hash_1);
 -              assert!(rejected_by_dest);
 +              assert!(payment_failed_permanently);
        } else { panic!("Unexpected event!"); }
        match events[1] {
                Event::PendingHTLCsForwardable { .. } => { },
@@@ -2032,9 -2032,9 +2032,9 @@@ fn test_pending_update_fee_ack_on_recon
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
  
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
-       let as_connect_msg = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+       let as_connect_msg = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
-       let bs_connect_msg = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let bs_connect_msg = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
  
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_connect_msg);
        let bs_resend_msgs = nodes[1].node.get_and_clear_pending_msg_events();
@@@ -2160,9 -2160,9 +2160,9 @@@ fn do_update_fee_resend_test(deliver_up
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
  
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
-       let as_connect_msg = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+       let as_connect_msg = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
-       let bs_connect_msg = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let bs_connect_msg = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
  
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_connect_msg);
        get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());
index b75eefb1349c49b3cb47ccedf54305ce30279c21,e27d794b254dc85565474bb0e4c310d7869f98da..afbd040b61c30d8a1543050f20c1739ef1aab29c
@@@ -54,7 -54,6 +54,6 @@@ use chain::keysinterface::{Sign, KeysIn
  use util::config::{UserConfig, ChannelConfig};
  use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
  use util::{byte_utils, events};
- use util::crypto::sign;
  use util::wakers::{Future, Notifier};
  use util::scid_utils::fake_scid;
  use util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
@@@ -764,10 -763,6 +763,6 @@@ pub struct ChannelManager<Signer: Sign
        /// keeping additional state.
        probing_cookie_secret: [u8; 32],
  
-       /// Used to track the last value sent in a node_announcement "timestamp" field. We ensure this
-       /// value increases strictly since we don't assume access to a time source.
-       last_node_announcement_serial: AtomicUsize,
        /// The highest block timestamp we've seen, which is usually a good guess at the current time.
        /// Assuming most miners are generating blocks with reasonable timestamps, this shouldn't be
        /// very far in the past, and can only ever be up to two hours in the future.
@@@ -1617,7 -1612,6 +1612,6 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
  
                        probing_cookie_secret: keys_manager.get_secure_random_bytes(),
  
-                       last_node_announcement_serial: AtomicUsize::new(0),
                        highest_seen_timestamp: AtomicUsize::new(0),
  
                        per_peer_state: RwLock::new(HashMap::new()),
                })
        }
  
-       #[allow(dead_code)]
-       // Messages of up to 64KB should never end up more than half full with addresses, as that would
-       // be absurd. We ensure this by checking that at least 100 (our stated public contract on when
-       // broadcast_node_announcement panics) of the maximum-length addresses would fit in a 64KB
-       // message...
-       const HALF_MESSAGE_IS_ADDRS: u32 = ::core::u16::MAX as u32 / (NetAddress::MAX_LEN as u32 + 1) / 2;
-       #[deny(const_err)]
-       #[allow(dead_code)]
-       // ...by failing to compile if the number of addresses that would be half of a message is
-       // smaller than 100:
-       const STATIC_ASSERT: u32 = Self::HALF_MESSAGE_IS_ADDRS - 100;
-       /// Regenerates channel_announcements and generates a signed node_announcement from the given
-       /// arguments, providing them in corresponding events via
-       /// [`get_and_clear_pending_msg_events`], if at least one public channel has been confirmed
-       /// on-chain. This effectively re-broadcasts all channel announcements and sends our node
-       /// announcement to ensure that the lightning P2P network is aware of the channels we have and
-       /// our network addresses.
-       ///
-       /// `rgb` is a node "color" and `alias` is a printable human-readable string to describe this
-       /// node to humans. They carry no in-protocol meaning.
-       ///
-       /// `addresses` represent the set (possibly empty) of socket addresses on which this node
-       /// accepts incoming connections. These will be included in the node_announcement, publicly
-       /// tying these addresses together and to this node. If you wish to preserve user privacy,
-       /// addresses should likely contain only Tor Onion addresses.
-       ///
-       /// Panics if `addresses` is absurdly large (more than 100).
-       ///
-       /// [`get_and_clear_pending_msg_events`]: MessageSendEventsProvider::get_and_clear_pending_msg_events
-       pub fn broadcast_node_announcement(&self, rgb: [u8; 3], alias: [u8; 32], mut addresses: Vec<NetAddress>) {
-               let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
-               if addresses.len() > 100 {
-                       panic!("More than half the message size was taken up by public addresses!");
-               }
-               // While all existing nodes handle unsorted addresses just fine, the spec requires that
-               // addresses be sorted for future compatibility.
-               addresses.sort_by_key(|addr| addr.get_id());
-               let announcement = msgs::UnsignedNodeAnnouncement {
-                       features: NodeFeatures::known(),
-                       timestamp: self.last_node_announcement_serial.fetch_add(1, Ordering::AcqRel) as u32,
-                       node_id: self.get_our_node_id(),
-                       rgb, alias, addresses,
-                       excess_address_data: Vec::new(),
-                       excess_data: Vec::new(),
-               };
-               let msghash = hash_to_message!(&Sha256dHash::hash(&announcement.encode()[..])[..]);
-               let node_announce_sig = sign(&self.secp_ctx, &msghash, &self.our_network_key);
-               let mut channel_state_lock = self.channel_state.lock().unwrap();
-               let channel_state = &mut *channel_state_lock;
-               let mut announced_chans = false;
-               for (_, chan) in channel_state.by_id.iter() {
-                       if let Some(msg) = chan.get_signed_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone(), self.best_block.read().unwrap().height()) {
-                               channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement {
-                                       msg,
-                                       update_msg: match self.get_channel_update_for_broadcast(chan) {
-                                               Ok(msg) => msg,
-                                               Err(_) => continue,
-                                       },
-                               });
-                               announced_chans = true;
-                       } else {
-                               // If the channel is not public or has not yet reached channel_ready, check the
-                               // next channel. If we don't yet have any public channels, we'll skip the broadcast
-                               // below as peers may not accept it without channels on chain first.
-                       }
-               }
-               if announced_chans {
-                       channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastNodeAnnouncement {
-                               msg: msgs::NodeAnnouncement {
-                                       signature: node_announce_sig,
-                                       contents: announcement
-                               },
-                       });
-               }
-       }
        /// Atomically updates the [`ChannelConfig`] for the given channels.
        ///
        /// Once the updates are applied, each eligible channel (advertised with a known short channel
                counterparty_node_id: &PublicKey
        ) {
                for (htlc_src, payment_hash) in htlcs_to_fail.drain(..) {
 -                      match htlc_src {
 -                              HTLCSource::PreviousHopData(HTLCPreviousHopData { .. }) => {
 -                                      let (failure_code, onion_failure_data) =
 -                                              match self.channel_state.lock().unwrap().by_id.entry(channel_id) {
 -                                                      hash_map::Entry::Occupied(chan_entry) => {
 -                                                              self.get_htlc_inbound_temp_fail_err_and_data(0x1000|7, &chan_entry.get())
 -                                                      },
 -                                                      hash_map::Entry::Vacant(_) => (0x4000|10, Vec::new())
 -                                              };
 -                                      let channel_state = self.channel_state.lock().unwrap();
 +                      let mut channel_state = self.channel_state.lock().unwrap();
 +                      let (failure_code, onion_failure_data) =
 +                              match channel_state.by_id.entry(channel_id) {
 +                                      hash_map::Entry::Occupied(chan_entry) => {
 +                                              self.get_htlc_inbound_temp_fail_err_and_data(0x1000|7, &chan_entry.get())
 +                                      },
 +                                      hash_map::Entry::Vacant(_) => (0x4000|10, Vec::new())
 +                              };
  
 -                                      let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id };
 -                                      self.fail_htlc_backwards_internal(channel_state, htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data }, receiver)
 -                              },
 -                              HTLCSource::OutboundRoute { session_priv, payment_id, path, payment_params, .. } => {
 -                                      let mut session_priv_bytes = [0; 32];
 -                                      session_priv_bytes.copy_from_slice(&session_priv[..]);
 -                                      let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 -                                      if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
 -                                              if payment.get_mut().remove(&session_priv_bytes, Some(&path)) && !payment.get().is_fulfilled() {
 -                                                      let retry = if let Some(payment_params_data) = payment_params {
 -                                                              let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
 -                                                              Some(RouteParameters {
 -                                                                      payment_params: payment_params_data,
 -                                                                      final_value_msat: path_last_hop.fee_msat,
 -                                                                      final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
 -                                                              })
 -                                                      } else { None };
 -                                                      let mut pending_events = self.pending_events.lock().unwrap();
 -                                                      pending_events.push(events::Event::PaymentPathFailed {
 -                                                              payment_id: Some(payment_id),
 -                                                              payment_hash,
 -                                                              rejected_by_dest: false,
 -                                                              network_update: None,
 -                                                              all_paths_failed: payment.get().remaining_parts() == 0,
 -                                                              path: path.clone(),
 -                                                              short_channel_id: None,
 -                                                              retry,
 -                                                              #[cfg(test)]
 -                                                              error_code: None,
 -                                                              #[cfg(test)]
 -                                                              error_data: None,
 -                                                      });
 -                                                      if payment.get().abandoned() && payment.get().remaining_parts() == 0 {
 -                                                              pending_events.push(events::Event::PaymentFailed {
 -                                                                      payment_id,
 -                                                                      payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
 -                                                              });
 -                                                              payment.remove();
 -                                                      }
 -                                              }
 -                                      } else {
 -                                              log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
 -                                      }
 -                              },
 -                      };
 +                      let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id };
 +                      self.fail_htlc_backwards_internal(channel_state, htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data }, receiver);
                }
        }
  
                                                        events::Event::PaymentPathFailed {
                                                                payment_id: Some(payment_id),
                                                                payment_hash: payment_hash.clone(),
 -                                                              rejected_by_dest: !payment_retryable,
 +                                                              payment_failed_permanently: !payment_retryable,
                                                                network_update,
                                                                all_paths_failed,
                                                                path: path.clone(),
                                                // channel here as we apparently can't relay through them anyway.
                                                let scid = path.first().unwrap().short_channel_id;
                                                retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
 -                                              events::Event::PaymentPathFailed {
 -                                                      payment_id: Some(payment_id),
 -                                                      payment_hash: payment_hash.clone(),
 -                                                      rejected_by_dest: path.len() == 1,
 -                                                      network_update: None,
 -                                                      all_paths_failed,
 -                                                      path: path.clone(),
 -                                                      short_channel_id: Some(scid),
 -                                                      retry,
 +
 +                                              if self.payment_is_probe(payment_hash, &payment_id) {
 +                                                      events::Event::ProbeFailed {
 +                                                              payment_id: payment_id,
 +                                                              payment_hash: payment_hash.clone(),
 +                                                              path: path.clone(),
 +                                                              short_channel_id: Some(scid),
 +                                                      }
 +                                              } else {
 +                                                      events::Event::PaymentPathFailed {
 +                                                              payment_id: Some(payment_id),
 +                                                              payment_hash: payment_hash.clone(),
 +                                                              payment_failed_permanently: false,
 +                                                              network_update: None,
 +                                                              all_paths_failed,
 +                                                              path: path.clone(),
 +                                                              short_channel_id: Some(scid),
 +                                                              retry,
  #[cfg(test)]
 -                                                      error_code: Some(*failure_code),
 +                                                              error_code: Some(*failure_code),
  #[cfg(test)]
 -                                                      error_data: Some(data.clone()),
 +                                                              error_data: Some(data.clone()),
 +                                                      }
                                                }
                                        }
                                };
@@@ -5754,7 -5700,6 +5665,6 @@@ wher
                                }
                        }
                }
-               max_time!(self.last_node_announcement_serial);
                max_time!(self.highest_seen_timestamp);
                let mut payment_secrets = self.pending_inbound_payments.lock().unwrap();
                payment_secrets.retain(|_, inbound_payment| {
@@@ -6104,8 -6049,8 +6014,8 @@@ impl<Signer: Sign, M: Deref , T: Deref 
                                        &events::MessageSendEvent::SendClosingSigned { ref node_id, .. } => node_id != counterparty_node_id,
                                        &events::MessageSendEvent::SendShutdown { ref node_id, .. } => node_id != counterparty_node_id,
                                        &events::MessageSendEvent::SendChannelReestablish { ref node_id, .. } => node_id != counterparty_node_id,
+                                       &events::MessageSendEvent::SendChannelAnnouncement { ref node_id, .. } => node_id != counterparty_node_id,
                                        &events::MessageSendEvent::BroadcastChannelAnnouncement { .. } => true,
-                                       &events::MessageSendEvent::BroadcastNodeAnnouncement { .. } => true,
                                        &events::MessageSendEvent::BroadcastChannelUpdate { .. } => true,
                                        &events::MessageSendEvent::SendChannelUpdate { ref node_id, .. } => node_id != counterparty_node_id,
                                        &events::MessageSendEvent::HandleError { ref node_id, .. } => node_id != counterparty_node_id,
                let channel_state = &mut *channel_state_lock;
                let pending_msg_events = &mut channel_state.pending_msg_events;
                channel_state.by_id.retain(|_, chan| {
-                       if chan.get_counterparty_node_id() == *counterparty_node_id {
+                       let retain = if chan.get_counterparty_node_id() == *counterparty_node_id {
                                if !chan.have_received_message() {
                                        // If we created this (outbound) channel while we were disconnected from the
                                        // peer we probably failed to send the open_channel message, which is now
                                        });
                                        true
                                }
-                       } else { true }
+                       } else { true };
+                       if retain && chan.get_counterparty_node_id() != *counterparty_node_id {
+                               if let Some(msg) = chan.get_signed_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone(), self.best_block.read().unwrap().height()) {
+                                       if let Ok(update_msg) = self.get_channel_update_for_broadcast(chan) {
+                                               pending_msg_events.push(events::MessageSendEvent::SendChannelAnnouncement {
+                                                       node_id: *counterparty_node_id,
+                                                       msg, update_msg,
+                                               });
+                                       }
+                               }
+                       }
+                       retain
                });
                //TODO: Also re-broadcast announcement_signatures
        }
                        let _ = self.force_close_channel_with_peer(&msg.channel_id, counterparty_node_id, Some(&msg.data), true);
                }
        }
+       fn provided_node_features(&self) -> NodeFeatures {
+               NodeFeatures::known()
+       }
  }
  
  const SERIALIZATION_VERSION: u8 = 1;
@@@ -6621,7 -6581,10 +6546,10 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
                        }
                }
  
-               (self.last_node_announcement_serial.load(Ordering::Acquire) as u32).write(writer)?;
+               // Prior to 0.0.111 we tracked node_announcement serials here, however that now happens in
+               // `PeerManager`, and thus we simply write the `highest_seen_timestamp` twice, which is
+               // likely to be identical.
+               (self.highest_seen_timestamp.load(Ordering::Acquire) as u32).write(writer)?;
                (self.highest_seen_timestamp.load(Ordering::Acquire) as u32).write(writer)?;
  
                (pending_inbound_payments.len() as u64).write(writer)?;
@@@ -6940,7 -6903,7 +6868,7 @@@ impl<'a, Signer: Sign, M: Deref, T: Der
                        }
                }
  
-               let last_node_announcement_serial: u32 = Readable::read(reader)?;
+               let _last_node_announcement_serial: u32 = Readable::read(reader)?; // Only used < 0.0.111
                let highest_seen_timestamp: u32 = Readable::read(reader)?;
  
                let pending_inbound_payment_count: u64 = Readable::read(reader)?;
                        our_network_pubkey,
                        secp_ctx,
  
-                       last_node_announcement_serial: AtomicUsize::new(last_node_announcement_serial as usize),
                        highest_seen_timestamp: AtomicUsize::new(highest_seen_timestamp as usize),
  
                        per_peer_state: RwLock::new(per_peer_state),
index d8c1b75fad206cd1a144c06131bb9197c860fb9e,417206126cd1fb487beb34df1c2370e37cc99f56..554e60ba7379117c6b1842f01416fbca11bc7718
@@@ -304,18 -304,9 +304,18 @@@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 
        fn drop(&mut self) {
                if !panicking() {
                        // Check that we processed all pending events
 -                      assert!(self.node.get_and_clear_pending_msg_events().is_empty());
 -                      assert!(self.node.get_and_clear_pending_events().is_empty());
 -                      assert!(self.chain_monitor.added_monitors.lock().unwrap().is_empty());
 +                      let msg_events = self.node.get_and_clear_pending_msg_events();
 +                      if !msg_events.is_empty() {
 +                              panic!("Had excess message events on node {}: {:?}", self.logger.id, msg_events);
 +                      }
 +                      let events = self.node.get_and_clear_pending_events();
 +                      if !events.is_empty() {
 +                              panic!("Had excess events on node {}: {:?}", self.logger.id, events);
 +                      }
 +                      let added_monitors = self.chain_monitor.added_monitors.lock().unwrap().split_off(0);
 +                      if !added_monitors.is_empty() {
 +                              panic!("Had {} excess added monitors on node {}", added_monitors.len(), self.logger.id);
 +                      }
  
                        // Check that if we serialize the Router, we can deserialize it again.
                        {
@@@ -901,60 -892,10 +901,10 @@@ pub fn create_unannounced_chan_between_
  }
  
  pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, ann: &msgs::ChannelAnnouncement, upd_1: &msgs::ChannelUpdate, upd_2: &msgs::ChannelUpdate) {
-       nodes[a].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new());
-       let a_events = nodes[a].node.get_and_clear_pending_msg_events();
-       assert!(a_events.len() >= 2);
-       // ann should be re-generated by broadcast_node_announcement - check that we have it.
-       let mut found_ann_1 = false;
-       for event in a_events.iter() {
-               match event {
-                       MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
-                               if msg == ann { found_ann_1 = true; }
-                       },
-                       MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
-                       _ => panic!("Unexpected event {:?}", event),
-               }
-       }
-       assert!(found_ann_1);
-       let a_node_announcement = match a_events.last().unwrap() {
-               MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {
-                       (*msg).clone()
-               },
-               _ => panic!("Unexpected event"),
-       };
-       nodes[b].node.broadcast_node_announcement([1, 1, 1], [1; 32], Vec::new());
-       let b_events = nodes[b].node.get_and_clear_pending_msg_events();
-       assert!(b_events.len() >= 2);
-       // ann should be re-generated by broadcast_node_announcement - check that we have it.
-       let mut found_ann_2 = false;
-       for event in b_events.iter() {
-               match event {
-                       MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
-                               if msg == ann { found_ann_2 = true; }
-                       },
-                       MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
-                       _ => panic!("Unexpected event"),
-               }
-       }
-       assert!(found_ann_2);
-       let b_node_announcement = match b_events.last().unwrap() {
-               MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {
-                       (*msg).clone()
-               },
-               _ => panic!("Unexpected event"),
-       };
        for node in nodes {
                assert!(node.gossip_sync.handle_channel_announcement(ann).unwrap());
                node.gossip_sync.handle_channel_update(upd_1).unwrap();
                node.gossip_sync.handle_channel_update(upd_2).unwrap();
-               node.gossip_sync.handle_node_announcement(&a_node_announcement).unwrap();
-               node.gossip_sync.handle_node_announcement(&b_node_announcement).unwrap();
  
                // Note that channel_updates are also delivered to ChannelManagers to ensure we have
                // forwarding info for local channels even if its not accepted in the network graph.
@@@ -1589,9 -1530,9 +1539,9 @@@ impl<'a> PaymentFailedConditions<'a> 
  
  #[cfg(test)]
  macro_rules! expect_payment_failed_with_update {
 -      ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => {
 +      ($node: expr, $expected_payment_hash: expr, $payment_failed_permanently: expr, $scid: expr, $chan_closed: expr) => {
                $crate::ln::functional_test_utils::expect_payment_failed_conditions(
 -                      &$node, $expected_payment_hash, $rejected_by_dest,
 +                      &$node, $expected_payment_hash, $payment_failed_permanently,
                        $crate::ln::functional_test_utils::PaymentFailedConditions::new()
                                .blamed_scid($scid).blamed_chan_closed($chan_closed));
        }
  
  #[cfg(test)]
  macro_rules! expect_payment_failed {
 -      ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
 +      ($node: expr, $expected_payment_hash: expr, $payment_failed_permanently: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
                #[allow(unused_mut)]
                let mut conditions = $crate::ln::functional_test_utils::PaymentFailedConditions::new();
                $(
                        conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data);
                )*
 -              $crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $rejected_by_dest, conditions);
 +              $crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $payment_failed_permanently, conditions);
        };
  }
  
  pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
        node: &'a Node<'b, 'c, 'd>, payment_failed_event: Event, expected_payment_hash: PaymentHash,
 -      expected_rejected_by_dest: bool, conditions: PaymentFailedConditions<'e>
 +      expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e>
  ) {
        let expected_payment_id = match payment_failed_event {
 -              Event::PaymentPathFailed { payment_hash, rejected_by_dest, path, retry, payment_id, network_update, short_channel_id,
 +              Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, network_update, short_channel_id,
                        #[cfg(test)]
                        error_code,
                        #[cfg(test)]
                        error_data, .. } => {
                        assert_eq!(payment_hash, expected_payment_hash, "unexpected payment_hash");
 -                      assert_eq!(rejected_by_dest, expected_rejected_by_dest, "unexpected rejected_by_dest value");
 +                      assert_eq!(payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
                        assert!(retry.is_some(), "expected retry.is_some()");
                        assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
                        assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
  }
  
  pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
 -      node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_rejected_by_dest: bool,
 +      node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_payment_failed_permanently: bool,
        conditions: PaymentFailedConditions<'e>
  ) {
        let mut events = node.node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
 -      expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_rejected_by_dest, conditions);
 +      expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_payment_failed_permanently, conditions);
  }
  
  pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
@@@ -2026,9 -1967,9 +1976,9 @@@ pub fn pass_failed_payment_back<'a, 'b
                        let events = origin_node.node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 1);
                        let expected_payment_id = match events[0] {
 -                              Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, ref payment_id, .. } => {
 +                              Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
                                        assert_eq!(payment_hash, our_payment_hash);
 -                                      assert!(rejected_by_dest);
 +                                      assert!(payment_failed_permanently);
                                        assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
                                        for (idx, hop) in expected_route.iter().enumerate() {
                                                assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
@@@ -2333,15 -2274,27 +2283,27 @@@ macro_rules! get_channel_value_stat 
  macro_rules! get_chan_reestablish_msgs {
        ($src_node: expr, $dst_node: expr) => {
                {
+                       let mut announcements = $crate::prelude::HashSet::new();
                        let mut res = Vec::with_capacity(1);
                        for msg in $src_node.node.get_and_clear_pending_msg_events() {
                                if let MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } = msg {
                                        assert_eq!(*node_id, $dst_node.node.get_our_node_id());
                                        res.push(msg.clone());
+                               } else if let MessageSendEvent::SendChannelAnnouncement { ref node_id, ref msg, .. } = msg {
+                                       assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                                       announcements.insert(msg.contents.short_channel_id);
                                } else {
                                        panic!("Unexpected event")
                                }
                        }
+                       for chan in $src_node.node.list_channels() {
+                               if chan.is_public && chan.counterparty.node_id != $dst_node.node.get_our_node_id() {
+                                       if let Some(scid) = chan.short_channel_id {
+                                               assert!(announcements.remove(&scid));
+                                       }
+                               }
+                       }
+                       assert!(announcements.is_empty());
                        res
                }
        }
index 2efeb8852c088cc6f505c44ec5376d660de24268,420e36afeacac3df65d973251fce9b231abb3cc3..21b7d9a7d6b4eb774bd2090faa99ad0b3de57fa5
@@@ -2560,7 -2560,7 +2560,7 @@@ fn claim_htlc_outputs_shared_tx() 
                // ANTI_REORG_DELAY confirmations.
                mine_transaction(&nodes[1], &node_txn[0]);
                connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 -              expect_payment_failed!(nodes[1], payment_hash_2, true);
 +              expect_payment_failed!(nodes[1], payment_hash_2, false);
        }
        get_announce_close_broadcast_events(&nodes, 0, 1);
        assert_eq!(nodes[0].node.list_channels().len(), 0);
@@@ -2642,7 -2642,7 +2642,7 @@@ fn claim_htlc_outputs_single_tx() 
                mine_transaction(&nodes[1], &node_txn[3]);
                mine_transaction(&nodes[1], &node_txn[4]);
                connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 -              expect_payment_failed!(nodes[1], payment_hash_2, true);
 +              expect_payment_failed!(nodes[1], payment_hash_2, false);
        }
        get_announce_close_broadcast_events(&nodes, 0, 1);
        assert_eq!(nodes[0].node.list_channels().len(), 0);
@@@ -3266,7 -3266,7 +3266,7 @@@ fn do_test_commitment_revoked_fail_back
                        let events = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 3);
                        match events[0] {
 -                              Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
 +                              Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        // If we delivered B's RAA we got an unknown preimage error, not something
                                        // that we should update our routing table for.
                                _ => panic!("Unexpected event"),
                        }
                        match events[1] {
 -                              Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
 +                              Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        assert!(network_update.is_some());
                                },
                                _ => panic!("Unexpected event"),
                        }
                        match events[2] {
 -                              Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
 +                              Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        assert!(network_update.is_some());
                                },
@@@ -3614,9 -3614,9 +3614,9 @@@ fn test_simple_peer_disconnect() 
                        _ => panic!("Unexpected event"),
                }
                match events[1] {
 -                      Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } => {
 +                      Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } => {
                                assert_eq!(payment_hash, payment_hash_5);
 -                              assert!(rejected_by_dest);
 +                              assert!(payment_failed_permanently);
                        },
                        _ => panic!("Unexpected event"),
                }
@@@ -3895,9 -3895,9 +3895,9 @@@ fn test_funding_peer_disconnect() 
        assert!(events_2.is_empty());
  
        nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
-       let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+       let as_reestablish = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
-       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
  
        // nodes[0] hasn't yet received a channel_ready, so it only sends that on reconnect.
        nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
        check_added_monitors!(nodes[0], 1);
  
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
-       // The channel announcement should be re-generated exactly by broadcast_node_announcement.
-       nodes[0].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new());
-       let msgs = nodes[0].node.get_and_clear_pending_msg_events();
-       let mut found_announcement = false;
-       for event in msgs.iter() {
-               match event {
-                       MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
-                               if *msg == chan_announcement { found_announcement = true; }
-                       },
-                       MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
-                       _ => panic!("Unexpected event"),
-               }
-       }
-       assert!(found_announcement);
  }
  
  #[test]
@@@ -4737,19 -4722,23 +4722,23 @@@ fn test_manager_serialize_deserialize_i
        claim_payment(&nodes[2], &[&nodes[0], &nodes[1]], our_payment_preimage);
  
        nodes[3].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
-       let reestablish = get_event_msg!(nodes[3], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let reestablish = get_chan_reestablish_msgs!(nodes[3], nodes[0]).pop().unwrap();
        nodes[0].node.peer_connected(&nodes[3].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
        nodes[0].node.handle_channel_reestablish(&nodes[3].node.get_our_node_id(), &reestablish);
-       let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(msg_events.len(), 1);
-       if let MessageSendEvent::HandleError { ref action, .. } = msg_events[0] {
-               match action {
-                       &ErrorAction::SendErrorMessage { ref msg } => {
-                               assert_eq!(msg.channel_id, channel_id);
-                       },
-                       _ => panic!("Unexpected event!"),
+       let mut found_err = false;
+       for msg_event in nodes[0].node.get_and_clear_pending_msg_events() {
+               if let MessageSendEvent::HandleError { ref action, .. } = msg_event {
+                       match action {
+                               &ErrorAction::SendErrorMessage { ref msg } => {
+                                       assert_eq!(msg.channel_id, channel_id);
+                                       assert!(!found_err);
+                                       found_err = true;
+                               },
+                               _ => panic!("Unexpected event!"),
+                       }
                }
        }
+       assert!(found_err);
  }
  
  macro_rules! check_spendable_outputs {
@@@ -4960,7 -4949,7 +4949,7 @@@ fn test_static_spendable_outputs_timeou
        mine_transaction(&nodes[1], &node_txn[1]);
        check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 -      expect_payment_failed!(nodes[1], our_payment_hash, true);
 +      expect_payment_failed!(nodes[1], our_payment_hash, false);
  
        let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
        assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote, timeout_tx.output
@@@ -5715,12 -5704,12 +5704,12 @@@ fn do_test_fail_backwards_unrevoked_rem
        let mut as_failds = HashSet::new();
        let mut as_updates = 0;
        for event in as_events.iter() {
 -              if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
 +              if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref network_update, .. } = event {
                        assert!(as_failds.insert(*payment_hash));
                        if *payment_hash != payment_hash_2 {
 -                              assert_eq!(*rejected_by_dest, deliver_last_raa);
 +                              assert_eq!(*payment_failed_permanently, deliver_last_raa);
                        } else {
 -                              assert!(!rejected_by_dest);
 +                              assert!(!payment_failed_permanently);
                        }
                        if network_update.is_some() {
                                as_updates += 1;
        let mut bs_failds = HashSet::new();
        let mut bs_updates = 0;
        for event in bs_events.iter() {
 -              if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
 +              if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref network_update, .. } = event {
                        assert!(bs_failds.insert(*payment_hash));
                        if *payment_hash != payment_hash_1 && *payment_hash != payment_hash_5 {
 -                              assert_eq!(*rejected_by_dest, deliver_last_raa);
 +                              assert_eq!(*payment_failed_permanently, deliver_last_raa);
                        } else {
 -                              assert!(!rejected_by_dest);
 +                              assert!(!payment_failed_permanently);
                        }
                        if network_update.is_some() {
                                bs_updates += 1;
@@@ -5818,7 -5807,7 +5807,7 @@@ fn test_dynamic_spendable_outputs_local
  
        mine_transaction(&nodes[0], &htlc_timeout);
        connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
 -      expect_payment_failed!(nodes[0], our_payment_hash, true);
 +      expect_payment_failed!(nodes[0], our_payment_hash, false);
  
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
        let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
@@@ -5900,7 -5889,7 +5889,7 @@@ fn test_key_derivation_params() 
  
        mine_transaction(&nodes[0], &htlc_timeout);
        connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
 -      expect_payment_failed!(nodes[0], our_payment_hash, true);
 +      expect_payment_failed!(nodes[0], our_payment_hash, false);
  
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
        let new_keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
@@@ -6264,13 -6253,15 +6253,13 @@@ fn test_fail_holding_cell_htlc_upon_fre
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match &events[0] {
 -              &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => {
 +              &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
                        assert_eq!(our_payment_id, *payment_id.as_ref().unwrap());
                        assert_eq!(our_payment_hash.clone(), *payment_hash);
 -                      assert_eq!(*rejected_by_dest, false);
 +                      assert_eq!(*payment_failed_permanently, false);
                        assert_eq!(*all_paths_failed, true);
                        assert_eq!(*network_update, None);
 -                      assert_eq!(*short_channel_id, None);
 -                      assert_eq!(*error_code, None);
 -                      assert_eq!(*error_data, None);
 +                      assert_eq!(*short_channel_id, Some(route.paths[0][0].short_channel_id));
                },
                _ => panic!("Unexpected event"),
        }
@@@ -6348,13 -6339,15 +6337,13 @@@ fn test_free_and_fail_holding_cell_htlc
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match &events[0] {
 -              &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => {
 +              &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
                        assert_eq!(payment_id_2, *payment_id.as_ref().unwrap());
                        assert_eq!(payment_hash_2.clone(), *payment_hash);
 -                      assert_eq!(*rejected_by_dest, false);
 +                      assert_eq!(*payment_failed_permanently, false);
                        assert_eq!(*all_paths_failed, true);
                        assert_eq!(*network_update, None);
 -                      assert_eq!(*short_channel_id, None);
 -                      assert_eq!(*error_code, None);
 -                      assert_eq!(*error_data, None);
 +                      assert_eq!(*short_channel_id, Some(route_2.paths[0][0].short_channel_id));
                },
                _ => panic!("Unexpected event"),
        }
@@@ -7300,7 -7293,7 +7289,7 @@@ fn do_test_sweep_outbound_htlc_failure_
                mine_transaction(&nodes[0], &as_commitment_tx[0]);
                check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed);
                connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 -              expect_payment_failed!(nodes[0], dust_hash, true);
 +              expect_payment_failed!(nodes[0], dust_hash, false);
  
                connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS - ANTI_REORG_DELAY);
                check_closed_broadcast!(nodes[0], true);
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
                mine_transaction(&nodes[0], &timeout_tx[0]);
                connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 -              expect_payment_failed!(nodes[0], non_dust_hash, true);
 +              expect_payment_failed!(nodes[0], non_dust_hash, false);
        } else {
                // We fail dust-HTLC 1 by broadcast of remote commitment tx. If revoked, fail also non-dust HTLC
                mine_transaction(&nodes[0], &bs_commitment_tx[0]);
                check_spends!(timeout_tx[0], bs_commitment_tx[0]);
                // For both a revoked or non-revoked commitment transaction, after ANTI_REORG_DELAY the
                // dust HTLC should have been failed.
 -              expect_payment_failed!(nodes[0], dust_hash, true);
 +              expect_payment_failed!(nodes[0], dust_hash, false);
  
                if !revoked {
                        assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
                mine_transaction(&nodes[0], &timeout_tx[0]);
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
                connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 -              expect_payment_failed!(nodes[0], non_dust_hash, true);
 +              expect_payment_failed!(nodes[0], non_dust_hash, false);
        }
  }
  
@@@ -9038,7 -9031,7 +9027,7 @@@ fn test_htlc_no_detection() 
        let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[0], &Block { header: header_201, txdata: vec![htlc_timeout.clone()] });
        connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 -      expect_payment_failed!(nodes[0], our_payment_hash, true);
 +      expect_payment_failed!(nodes[0], our_payment_hash, false);
  }
  
  fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain_before_fulfill: bool) {
index 3666f3e79ab3be1eed785b9e318f8958b4079571,18f97d8f75a3d6b4576ac5336f8d698bbd0e22b0..dcad041f49cd78c2214260ff9a542b41c32b2e27
@@@ -457,7 -457,7 +457,7 @@@ fn do_retry_with_no_persist(confirm_bef
        // Now nodes[1] should send a channel reestablish, which nodes[0] will respond to with an
        // error, as the channel has hit the chain.
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
-       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
        nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
        let as_err = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(as_err.len(), 1);
@@@ -676,7 -676,7 +676,7 @@@ fn do_test_completed_payment_not_retrya
        // Now nodes[1] should send a channel reestablish, which nodes[0] will respond to with an
        // error, as the channel has hit the chain.
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
-       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
        nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
        let as_err = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(as_err.len(), 1);
@@@ -894,7 -894,7 +894,7 @@@ fn do_test_dup_htlc_onchain_fails_on_re
                nodes[0].chain_monitor.chain_monitor.channel_monitor_updated(funding_txo, update).unwrap();
        }
        if payment_timeout {
 -              expect_payment_failed!(nodes[0], payment_hash, true);
 +              expect_payment_failed!(nodes[0], payment_hash, false);
        } else {
                expect_payment_sent!(nodes[0], payment_preimage);
        }
        if persist_manager_post_event {
                assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
        } else if payment_timeout {
 -              expect_payment_failed!(nodes[0], payment_hash, true);
 +              expect_payment_failed!(nodes[0], payment_hash, false);
        } else {
                expect_payment_sent!(nodes[0], payment_preimage);
        }
@@@ -1200,57 -1200,3 +1200,57 @@@ fn failed_probe_yields_event() 
                _ => panic!(),
        };
  }
 +
 +#[test]
 +fn onchain_failed_probe_yields_event() {
 +      // Tests that an attempt to probe over a channel that is eventaully closed results in a failure
 +      // event.
 +      let chanmon_cfgs = create_chanmon_cfgs(3);
 +      let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
 +      let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
 +      let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 +
 +      let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
 +      create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 +
 +      let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
 +
 +      // Send a dust HTLC, which will be treated as if it timed out once the channel hits the chain.
 +      let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 1_000, 42);
 +      let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
 +
 +      // node[0] -- update_add_htlcs -> node[1]
 +      check_added_monitors!(nodes[0], 1);
 +      let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
 +      let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
 +      nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
 +      check_added_monitors!(nodes[1], 0);
 +      commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
 +      expect_pending_htlcs_forwardable!(nodes[1]);
 +
 +      check_added_monitors!(nodes[1], 1);
 +      let _ = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
 +
 +      // Don't bother forwarding the HTLC onwards and just confirm the force-close transaction on
 +      // Node A, which after 6 confirmations should result in a probe failure event.
 +      let bs_txn = get_local_commitment_txn!(nodes[1], chan_id);
 +      confirm_transaction(&nodes[0], &bs_txn[0]);
 +      check_closed_broadcast!(&nodes[0], true);
 +      check_added_monitors!(nodes[0], 1);
 +
 +      let mut events = nodes[0].node.get_and_clear_pending_events();
 +      assert_eq!(events.len(), 2);
 +      let mut found_probe_failed = false;
 +      for event in events.drain(..) {
 +              match event {
 +                      Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
 +                              assert_eq!(payment_id, ev_pid);
 +                              assert_eq!(payment_hash, ev_ph);
 +                              found_probe_failed = true;
 +                      },
 +                      Event::ChannelClosed { .. } => {},
 +                      _ => panic!(),
 +              }
 +      }
 +      assert!(found_probe_failed);
 +}
index be1979c77c93cfb6739ae01ef2da78ba25e75b75,ca09672e5693199970671a59c0d734c2aec7bb3b..8ddd762e97036bac77fee08c2be7819ec707388e
@@@ -376,7 -376,7 +376,7 @@@ pub enum Event 
                /// Indicates the payment was rejected for some reason by the recipient. This implies that
                /// the payment has failed, not just the route in question. If this is not set, you may
                /// retry the payment via a different route.
 -              rejected_by_dest: bool,
 +              payment_failed_permanently: bool,
                /// Any failure information conveyed via the Onion return packet by a node along the failed
                /// payment route.
                ///
@@@ -643,7 -643,7 +643,7 @@@ impl Writeable for Event 
                                });
                        },
                        &Event::PaymentPathFailed {
 -                              ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update,
 +                              ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update,
                                ref all_paths_failed, ref path, ref short_channel_id, ref retry,
                                #[cfg(test)]
                                ref error_code,
                                write_tlv_fields!(writer, {
                                        (0, payment_hash, required),
                                        (1, network_update, option),
 -                                      (2, rejected_by_dest, required),
 +                                      (2, payment_failed_permanently, required),
                                        (3, all_paths_failed, required),
                                        (5, path, vec_type),
                                        (7, short_channel_id, option),
@@@ -827,7 -827,7 +827,7 @@@ impl MaybeReadable for Event 
                                        #[cfg(test)]
                                        let error_data = Readable::read(reader)?;
                                        let mut payment_hash = PaymentHash([0; 32]);
 -                                      let mut rejected_by_dest = false;
 +                                      let mut payment_failed_permanently = false;
                                        let mut network_update = None;
                                        let mut all_paths_failed = Some(true);
                                        let mut path: Option<Vec<RouteHop>> = Some(vec![]);
                                        read_tlv_fields!(reader, {
                                                (0, payment_hash, required),
                                                (1, network_update, ignorable),
 -                                              (2, rejected_by_dest, required),
 +                                              (2, payment_failed_permanently, required),
                                                (3, all_paths_failed, option),
                                                (5, path, vec_type),
                                                (7, short_channel_id, option),
                                        Ok(Some(Event::PaymentPathFailed {
                                                payment_id,
                                                payment_hash,
 -                                              rejected_by_dest,
 +                                              payment_failed_permanently,
                                                network_update,
                                                all_paths_failed: all_paths_failed.unwrap(),
                                                path: path.unwrap(),
                                                (4, path, vec_type),
                                                (6, short_channel_id, option),
                                        });
 -                                      Ok(Some(Event::ProbeFailed{
 +                                      Ok(Some(Event::ProbeFailed {
                                                payment_id,
                                                payment_hash,
                                                path: path.unwrap(),
                                };
                                f()
                        },
 +                      25u8 => {
 +                              let f = || {
 +                                      let mut prev_channel_id = [0; 32];
 +                                      let mut failed_next_destination_opt = None;
 +                                      read_tlv_fields!(reader, {
 +                                              (0, prev_channel_id, required),
 +                                              (2, failed_next_destination_opt, ignorable),
 +                                      });
 +                                      if let Some(failed_next_destination) = failed_next_destination_opt {
 +                                              Ok(Some(Event::HTLCHandlingFailed {
 +                                                      prev_channel_id,
 +                                                      failed_next_destination,
 +                                              }))
 +                                      } else {
 +                                              // If we fail to read a `failed_next_destination` assume it's because
 +                                              // `MaybeReadable::read` returned `Ok(None)`, though it's also possible we
 +                                              // were simply missing the field.
 +                                              Ok(None)
 +                                      }
 +                              };
 +                              f()
 +                      },
                        // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
                        // Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
                        // reads.
@@@ -1137,25 -1115,32 +1137,32 @@@ pub enum MessageSendEvent 
                /// The message which should be sent.
                msg: msgs::ChannelReestablish,
        },
+       /// Used to send a channel_announcement and channel_update to a specific peer, likely on
+       /// initial connection to ensure our peers know about our channels.
+       SendChannelAnnouncement {
+               /// The node_id of the node which should receive this message
+               node_id: PublicKey,
+               /// The channel_announcement which should be sent.
+               msg: msgs::ChannelAnnouncement,
+               /// The followup channel_update which should be sent.
+               update_msg: msgs::ChannelUpdate,
+       },
        /// Used to indicate that a channel_announcement and channel_update should be broadcast to all
        /// peers (except the peer with node_id either msg.contents.node_id_1 or msg.contents.node_id_2).
        ///
-       /// Note that after doing so, you very likely (unless you did so very recently) want to call
-       /// ChannelManager::broadcast_node_announcement to trigger a BroadcastNodeAnnouncement event.
-       /// This ensures that any nodes which see our channel_announcement also have a relevant
+       /// Note that after doing so, you very likely (unless you did so very recently) want to
+       /// broadcast a node_announcement (e.g. via [`PeerManager::broadcast_node_announcement`]). This
+       /// ensures that any nodes which see our channel_announcement also have a relevant
        /// node_announcement, including relevant feature flags which may be important for routing
        /// through or to us.
+       ///
+       /// [`PeerManager::broadcast_node_announcement`]: crate::ln::peer_handler::PeerManager::broadcast_node_announcement
        BroadcastChannelAnnouncement {
                /// The channel_announcement which should be sent.
                msg: msgs::ChannelAnnouncement,
                /// The followup channel_update which should be sent.
                update_msg: msgs::ChannelUpdate,
        },
-       /// Used to indicate that a node_announcement should be broadcast to all peers.
-       BroadcastNodeAnnouncement {
-               /// The node_announcement which should be sent.
-               msg: msgs::NodeAnnouncement,
-       },
        /// Used to indicate that a channel_update should be broadcast to all peers.
        BroadcastChannelUpdate {
                /// The channel_update which should be sent.
index 7a32b13e60da6fbaef3c734d813d912fada9f440,b274a0d82539d0d4519f4321f68c867a96f47fe1..42ac228c144d8317a0bca0d3804f576e1539b9b7
@@@ -17,7 -17,7 +17,7 @@@ use chain::channelmonitor
  use chain::channelmonitor::MonitorEvent;
  use chain::transaction::OutPoint;
  use chain::keysinterface;
- use ln::features::{ChannelFeatures, InitFeatures};
+ use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
  use ln::{msgs, wire};
  use ln::script::ShutdownScript;
  use routing::scoring::FixedPenaltyScorer;
@@@ -357,6 -357,9 +357,9 @@@ impl msgs::ChannelMessageHandler for Te
        fn handle_error(&self, _their_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
                self.received_msg(wire::Message::Error(msg.clone()));
        }
+       fn provided_node_features(&self) -> NodeFeatures {
+               NodeFeatures::empty()
+       }
  }
  
  impl events::MessageSendEventsProvider for TestChannelMessageHandler {
@@@ -517,7 -520,10 +520,7 @@@ impl events::MessageSendEventsProvider 
  
  pub struct TestLogger {
        level: Level,
 -      #[cfg(feature = "std")]
 -      id: String,
 -      #[cfg(not(feature = "std"))]
 -      _id: String,
 +      pub(crate) id: String,
        pub lines: Mutex<HashMap<(String, String), usize>>,
  }
  
@@@ -528,7 -534,10 +531,7 @@@ impl TestLogger 
        pub fn with_id(id: String) -> TestLogger {
                TestLogger {
                        level: Level::Trace,
 -                      #[cfg(feature = "std")]
                        id,
 -                      #[cfg(not(feature = "std"))]
 -                      _id: id,
                        lines: Mutex::new(HashMap::new())
                }
        }
                assert_eq!(l, count)
        }
  
 -    /// Search for the number of occurrences of logged lines which
 -    /// 1. belong to the specified module and
 -    /// 2. match the given regex pattern.
 -    /// Assert that the number of occurrences equals the given `count`
 +      /// Search for the number of occurrences of logged lines which
 +      /// 1. belong to the specified module and
 +      /// 2. match the given regex pattern.
 +      /// Assert that the number of occurrences equals the given `count`
        pub fn assert_log_regex(&self, module: String, pattern: regex::Regex, count: usize) {
                let log_entries = self.lines.lock().unwrap();
                let l: usize = log_entries.iter().filter(|&(&(ref m, ref l), _c)| {