Merge pull request #2127 from TheBlueMatt/2023-03-payment-metadata
[rust-lightning] / lightning / src / ln / channelmanager.rs
index 4bb830f0c6fb0feafa1316dfc73fc18b07adbcaf..3fa8fa5ba947080d52cb888f505f40b568adb2a1 100644 (file)
@@ -36,7 +36,7 @@ use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, Fee
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
 use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::events;
-use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
+use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination, PaymentFailureReason};
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
 // construct one themselves.
 use crate::ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret};
@@ -626,6 +626,61 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
 /// This is not exported to bindings users as Arcs don't make sense in bindings
 pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = ChannelManager<&'a M, &'b T, &'c KeysManager, &'c KeysManager, &'c KeysManager, &'d F, &'e DefaultRouter<&'f NetworkGraph<&'g L>, &'g L, &'h Mutex<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>>, &'g L>;
 
+/// A trivial trait which describes any [`ChannelManager`] used in testing.
+#[cfg(any(test, feature = "_test_utils"))]
+pub trait AChannelManager {
+       type Watch: chain::Watch<Self::Signer>;
+       type M: Deref<Target = Self::Watch>;
+       type Broadcaster: BroadcasterInterface;
+       type T: Deref<Target = Self::Broadcaster>;
+       type EntropySource: EntropySource;
+       type ES: Deref<Target = Self::EntropySource>;
+       type NodeSigner: NodeSigner;
+       type NS: Deref<Target = Self::NodeSigner>;
+       type Signer: WriteableEcdsaChannelSigner;
+       type SignerProvider: SignerProvider<Signer = Self::Signer>;
+       type SP: Deref<Target = Self::SignerProvider>;
+       type FeeEstimator: FeeEstimator;
+       type F: Deref<Target = Self::FeeEstimator>;
+       type Router: Router;
+       type R: Deref<Target = Self::Router>;
+       type Logger: Logger;
+       type L: Deref<Target = Self::Logger>;
+       fn get_cm(&self) -> &ChannelManager<Self::M, Self::T, Self::ES, Self::NS, Self::SP, Self::F, Self::R, Self::L>;
+}
+#[cfg(any(test, feature = "_test_utils"))]
+impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, L: Deref> AChannelManager
+for ChannelManager<M, T, ES, NS, SP, F, R, L>
+where
+       M::Target: chain::Watch<<SP::Target as SignerProvider>::Signer> + Sized,
+       T::Target: BroadcasterInterface + Sized,
+       ES::Target: EntropySource + Sized,
+       NS::Target: NodeSigner + Sized,
+       SP::Target: SignerProvider + Sized,
+       F::Target: FeeEstimator + Sized,
+       R::Target: Router + Sized,
+       L::Target: Logger + Sized,
+{
+       type Watch = M::Target;
+       type M = M;
+       type Broadcaster = T::Target;
+       type T = T;
+       type EntropySource = ES::Target;
+       type ES = ES;
+       type NodeSigner = NS::Target;
+       type NS = NS;
+       type Signer = <SP::Target as SignerProvider>::Signer;
+       type SignerProvider = SP::Target;
+       type SP = SP;
+       type FeeEstimator = F::Target;
+       type F = F;
+       type Router = R::Target;
+       type R = R;
+       type Logger = L::Target;
+       type L = L;
+       fn get_cm(&self) -> &ChannelManager<M, T, ES, NS, SP, F, R, L> { self }
+}
+
 /// Manager which keeps track of a number of channels and sends messages to the appropriate
 /// channel, also tracking HTLC preimages and forwarding onion packets appropriately.
 ///
@@ -1023,6 +1078,14 @@ pub(crate) const MPP_TIMEOUT_TICKS: u8 = 3;
 /// [`OutboundPayments::remove_stale_resolved_payments`].
 pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7;
 
+/// The number of ticks of [`ChannelManager::timer_tick_occurred`] where a peer is disconnected
+/// until we mark the channel disabled and gossip the update.
+pub(crate) const DISABLE_GOSSIP_TICKS: u8 = 10;
+
+/// The number of ticks of [`ChannelManager::timer_tick_occurred`] where a peer is connected until
+/// we mark the channel enabled and gossip the update.
+pub(crate) const ENABLE_GOSSIP_TICKS: u8 = 5;
+
 /// The maximum number of unfunded channels we can have per-peer before we start rejecting new
 /// (inbound) ones. The number of peers with unfunded channels is limited separately in
 /// [`MAX_UNFUNDED_CHANNEL_PEERS`].
@@ -1362,15 +1425,15 @@ pub struct PhantomRouteHints {
 }
 
 macro_rules! handle_error {
-       ($self: ident, $internal: expr, $counterparty_node_id: expr) => {
+       ($self: ident, $internal: expr, $counterparty_node_id: expr) => { {
+               // In testing, ensure there are no deadlocks where the lock is already held upon
+               // entering the macro.
+               debug_assert_ne!($self.pending_events.held_by_thread(), LockHeldState::HeldByThread);
+               debug_assert_ne!($self.per_peer_state.held_by_thread(), LockHeldState::HeldByThread);
+
                match $internal {
                        Ok(msg) => Ok(msg),
                        Err(MsgHandleErrInternal { err, chan_id, shutdown_finish }) => {
-                               // In testing, ensure there are no deadlocks where the lock is already held upon
-                               // entering the macro.
-                               debug_assert_ne!($self.pending_events.held_by_thread(), LockHeldState::HeldByThread);
-                               debug_assert_ne!($self.per_peer_state.held_by_thread(), LockHeldState::HeldByThread);
-
                                let mut msg_events = Vec::with_capacity(2);
 
                                if let Some((shutdown_res, update_option)) = shutdown_finish {
@@ -1409,7 +1472,7 @@ macro_rules! handle_error {
                                Err(err)
                        },
                }
-       }
+       } }
 }
 
 macro_rules! update_maps_on_chan_removal {
@@ -1596,7 +1659,7 @@ macro_rules! handle_new_monitor_update {
        ($self: ident, $update_res: expr, $update_id: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, MANUALLY_REMOVING, $remove: expr) => { {
                // update_maps_on_chan_removal needs to be able to take id_to_peer, so make sure we can in
                // any case so that it won't deadlock.
-               debug_assert!($self.id_to_peer.try_lock().is_ok());
+               debug_assert_ne!($self.id_to_peer.held_by_thread(), LockHeldState::HeldByThread);
                match $update_res {
                        ChannelMonitorUpdateStatus::InProgress => {
                                log_debug!($self.logger, "ChannelMonitor update for {} in flight, holding messages until the update completes.",
@@ -1631,6 +1694,36 @@ macro_rules! handle_new_monitor_update {
        }
 }
 
+macro_rules! process_events_body {
+       ($self: expr, $event_to_handle: expr, $handle_event: expr) => {
+               // We'll acquire our total consistency lock until the returned future completes so that
+               // we can be sure no other persists happen while processing events.
+               let _read_guard = $self.total_consistency_lock.read().unwrap();
+
+               let mut result = NotifyOption::SkipPersist;
+
+               // TODO: This behavior should be documented. It's unintuitive that we query
+               // ChannelMonitors when clearing other events.
+               if $self.process_pending_monitor_events() {
+                       result = NotifyOption::DoPersist;
+               }
+
+               let pending_events = mem::replace(&mut *$self.pending_events.lock().unwrap(), vec![]);
+               if !pending_events.is_empty() {
+                       result = NotifyOption::DoPersist;
+               }
+
+               for event in pending_events {
+                       $event_to_handle = event;
+                       $handle_event;
+               }
+
+               if result == NotifyOption::DoPersist {
+                       $self.persistence_notifier.notify();
+               }
+       }
+}
+
 impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<M, T, ES, NS, SP, F, R, L>
 where
        M::Target: chain::Watch<<SP::Target as SignerProvider>::Signer>,
@@ -2382,7 +2475,14 @@ where
                                                // hopefully an attacker trying to path-trace payments cannot make this occur
                                                // on a small/per-node/per-channel scale.
                                                if !chan.is_live() { // channel_disabled
-                                                       break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, chan_update_opt));
+                                                       // If the channel_update we're going to return is disabled (i.e. the
+                                                       // peer has been disabled for some time), return `channel_disabled`,
+                                                       // otherwise return `temporary_channel_failure`.
+                                                       if chan_update_opt.as_ref().map(|u| u.contents.flags & 2 == 2).unwrap_or(false) {
+                                                               break Some(("Forwarding channel has been disconnected for some time.", 0x1000 | 20, chan_update_opt));
+                                                       } else {
+                                                               break Some(("Forwarding channel is not in a ready state.", 0x1000 | 7, chan_update_opt));
+                                                       }
                                                }
                                                if *outgoing_amt_msat < chan.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
                                                        break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
@@ -2507,11 +2607,18 @@ where
                log_trace!(self.logger, "Generating channel update for channel {}", log_bytes!(chan.channel_id()));
                let were_node_one = self.our_network_pubkey.serialize()[..] < chan.get_counterparty_node_id().serialize()[..];
 
+               let enabled = chan.is_usable() && match chan.channel_update_status() {
+                       ChannelUpdateStatus::Enabled => true,
+                       ChannelUpdateStatus::DisabledStaged(_) => true,
+                       ChannelUpdateStatus::Disabled => false,
+                       ChannelUpdateStatus::EnabledStaged(_) => false,
+               };
+
                let unsigned = msgs::UnsignedChannelUpdate {
                        chain_hash: self.genesis_hash,
                        short_channel_id,
                        timestamp: chan.get_update_time_counter(),
-                       flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
+                       flags: (!were_node_one) as u8 | ((!enabled as u8) << 1),
                        cltv_expiry_delta: chan.get_cltv_expiry_delta(),
                        htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
                        htlc_maximum_msat: chan.get_announced_htlc_max_msat(),
@@ -2726,7 +2833,7 @@ where
        /// [`Event::PaymentSent`]: events::Event::PaymentSent
        pub fn abandon_payment(&self, payment_id: PaymentId) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
-               self.pending_outbound_payments.abandon_payment(payment_id, &self.pending_events);
+               self.pending_outbound_payments.abandon_payment(payment_id, PaymentFailureReason::UserAbandoned, &self.pending_events);
        }
 
        /// Send a spontaneous payment, which is a payment that does not require the recipient to have
@@ -2801,29 +2908,34 @@ where
 
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
-               let (chan, msg) = {
-                       let (res, chan) = {
-                               match peer_state.channel_by_id.remove(temporary_channel_id) {
-                                       Some(mut chan) => {
-                                               let funding_txo = find_funding_output(&chan, &funding_transaction)?;
-
-                                               (chan.get_outbound_funding_created(funding_transaction, funding_txo, &self.logger)
-                                                       .map_err(|e| if let ChannelError::Close(msg) = e {
-                                                               MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.get_user_id(), chan.force_shutdown(true), None)
-                                                       } else { unreachable!(); })
-                                               , chan)
+               let (msg, chan) = match peer_state.channel_by_id.remove(temporary_channel_id) {
+                       Some(mut chan) => {
+                               let funding_txo = find_funding_output(&chan, &funding_transaction)?;
+
+                               let funding_res = chan.get_outbound_funding_created(funding_transaction, funding_txo, &self.logger)
+                                       .map_err(|e| if let ChannelError::Close(msg) = e {
+                                               MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.get_user_id(), chan.force_shutdown(true), None)
+                                       } else { unreachable!(); });
+                               match funding_res {
+                                       Ok(funding_msg) => (funding_msg, chan),
+                                       Err(_) => {
+                                               mem::drop(peer_state_lock);
+                                               mem::drop(per_peer_state);
+
+                                               let _ = handle_error!(self, funding_res, chan.get_counterparty_node_id());
+                                               return Err(APIError::ChannelUnavailable {
+                                                       err: "Signer refused to sign the initial commitment transaction".to_owned()
+                                               });
                                        },
-                                       None => { return Err(APIError::ChannelUnavailable { err: format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(*temporary_channel_id), counterparty_node_id) }) },
                                }
-                       };
-                       match handle_error!(self, res, chan.get_counterparty_node_id()) {
-                               Ok(funding_msg) => {
-                                       (chan, funding_msg)
-                               },
-                               Err(_) => { return Err(APIError::ChannelUnavailable {
-                                       err: "Signer refused to sign the initial commitment transaction".to_owned()
-                               }) },
-                       }
+                       },
+                       None => {
+                               return Err(APIError::ChannelUnavailable {
+                                       err: format!(
+                                               "Channel with id {} not found for the passed counterparty node_id {}",
+                                               log_bytes!(*temporary_channel_id), counterparty_node_id),
+                               })
+                       },
                };
 
                peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
@@ -3692,27 +3804,39 @@ where
                                                }
 
                                                match chan.channel_update_status() {
-                                                       ChannelUpdateStatus::Enabled if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::DisabledStaged),
-                                                       ChannelUpdateStatus::Disabled if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::EnabledStaged),
-                                                       ChannelUpdateStatus::DisabledStaged if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Enabled),
-                                                       ChannelUpdateStatus::EnabledStaged if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Disabled),
-                                                       ChannelUpdateStatus::DisabledStaged if !chan.is_live() => {
-                                                               if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
-                                                                       pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
-                                                                               msg: update
-                                                                       });
+                                                       ChannelUpdateStatus::Enabled if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::DisabledStaged(0)),
+                                                       ChannelUpdateStatus::Disabled if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::EnabledStaged(0)),
+                                                       ChannelUpdateStatus::DisabledStaged(_) if chan.is_live()
+                                                               => chan.set_channel_update_status(ChannelUpdateStatus::Enabled),
+                                                       ChannelUpdateStatus::EnabledStaged(_) if !chan.is_live()
+                                                               => chan.set_channel_update_status(ChannelUpdateStatus::Disabled),
+                                                       ChannelUpdateStatus::DisabledStaged(mut n) if !chan.is_live() => {
+                                                               n += 1;
+                                                               if n >= DISABLE_GOSSIP_TICKS {
+                                                                       chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
+                                                                       if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
+                                                                               pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
+                                                                                       msg: update
+                                                                               });
+                                                                       }
+                                                                       should_persist = NotifyOption::DoPersist;
+                                                               } else {
+                                                                       chan.set_channel_update_status(ChannelUpdateStatus::DisabledStaged(n));
                                                                }
-                                                               should_persist = NotifyOption::DoPersist;
-                                                               chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
                                                        },
-                                                       ChannelUpdateStatus::EnabledStaged if chan.is_live() => {
-                                                               if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
-                                                                       pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
-                                                                               msg: update
-                                                                       });
+                                                       ChannelUpdateStatus::EnabledStaged(mut n) if chan.is_live() => {
+                                                               n += 1;
+                                                               if n >= ENABLE_GOSSIP_TICKS {
+                                                                       chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
+                                                                       if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
+                                                                               pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
+                                                                                       msg: update
+                                                                               });
+                                                                       }
+                                                                       should_persist = NotifyOption::DoPersist;
+                                                               } else {
+                                                                       chan.set_channel_update_status(ChannelUpdateStatus::EnabledStaged(n));
                                                                }
-                                                               should_persist = NotifyOption::DoPersist;
-                                                               chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
                                                        },
                                                        _ => {},
                                                }
@@ -5764,30 +5888,8 @@ where
        pub async fn process_pending_events_async<Future: core::future::Future, H: Fn(Event) -> Future>(
                &self, handler: H
        ) {
-               // We'll acquire our total consistency lock until the returned future completes so that
-               // we can be sure no other persists happen while processing events.
-               let _read_guard = self.total_consistency_lock.read().unwrap();
-
-               let mut result = NotifyOption::SkipPersist;
-
-               // TODO: This behavior should be documented. It's unintuitive that we query
-               // ChannelMonitors when clearing other events.
-               if self.process_pending_monitor_events() {
-                       result = NotifyOption::DoPersist;
-               }
-
-               let pending_events = mem::replace(&mut *self.pending_events.lock().unwrap(), vec![]);
-               if !pending_events.is_empty() {
-                       result = NotifyOption::DoPersist;
-               }
-
-               for event in pending_events {
-                       handler(event).await;
-               }
-
-               if result == NotifyOption::DoPersist {
-                       self.persistence_notifier.notify();
-               }
+               let mut ev;
+               process_events_body!(self, ev, { handler(ev).await });
        }
 }
 
@@ -5869,26 +5971,8 @@ where
        /// An [`EventHandler`] may safely call back to the provider in order to handle an event.
        /// However, it must not call [`Writeable::write`] as doing so would result in a deadlock.
        fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler {
-               PersistenceNotifierGuard::optionally_notify(&self.total_consistency_lock, &self.persistence_notifier, || {
-                       let mut result = NotifyOption::SkipPersist;
-
-                       // TODO: This behavior should be documented. It's unintuitive that we query
-                       // ChannelMonitors when clearing other events.
-                       if self.process_pending_monitor_events() {
-                               result = NotifyOption::DoPersist;
-                       }
-
-                       let pending_events = mem::replace(&mut *self.pending_events.lock().unwrap(), vec![]);
-                       if !pending_events.is_empty() {
-                               result = NotifyOption::DoPersist;
-                       }
-
-                       for event in pending_events {
-                               handler.handle_event(event);
-                       }
-
-                       result
-               });
+               let mut ev;
+               process_events_body!(self, ev, handler.handle_event(ev));
        }
 }
 
@@ -8920,14 +9004,23 @@ pub mod bench {
 
        use test::Bencher;
 
-       struct NodeHolder<'a, P: Persist<InMemorySigner>> {
-               node: &'a ChannelManager<
-                       &'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
-                               &'a test_utils::TestBroadcaster, &'a test_utils::TestFeeEstimator,
-                               &'a test_utils::TestLogger, &'a P>,
-                       &'a test_utils::TestBroadcaster, &'a KeysManager, &'a KeysManager, &'a KeysManager,
-                       &'a test_utils::TestFeeEstimator, &'a test_utils::TestRouter<'a>,
-                       &'a test_utils::TestLogger>,
+       type Manager<'a, P> = ChannelManager<
+               &'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
+                       &'a test_utils::TestBroadcaster, &'a test_utils::TestFeeEstimator,
+                       &'a test_utils::TestLogger, &'a P>,
+               &'a test_utils::TestBroadcaster, &'a KeysManager, &'a KeysManager, &'a KeysManager,
+               &'a test_utils::TestFeeEstimator, &'a test_utils::TestRouter<'a>,
+               &'a test_utils::TestLogger>;
+
+       struct ANodeHolder<'a, P: Persist<InMemorySigner>> {
+               node: &'a Manager<'a, P>,
+       }
+       impl<'a, P: Persist<InMemorySigner>> NodeHolder for ANodeHolder<'a, P> {
+               type CM = Manager<'a, P>;
+               #[inline]
+               fn node(&self) -> &Manager<'a, P> { self.node }
+               #[inline]
+               fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { None }
        }
 
        #[cfg(test)]
@@ -8958,7 +9051,7 @@ pub mod bench {
                        network,
                        best_block: BestBlock::from_network(network),
                });
-               let node_a_holder = NodeHolder { node: &node_a };
+               let node_a_holder = ANodeHolder { node: &node_a };
 
                let logger_b = test_utils::TestLogger::with_id("node a".to_owned());
                let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b);
@@ -8968,7 +9061,7 @@ pub mod bench {
                        network,
                        best_block: BestBlock::from_network(network),
                });
-               let node_b_holder = NodeHolder { node: &node_b };
+               let node_b_holder = ANodeHolder { node: &node_b };
 
                node_a.peer_connected(&node_b.get_our_node_id(), &Init { features: node_b.init_features(), remote_network_address: None }, true).unwrap();
                node_b.peer_connected(&node_a.get_our_node_id(), &Init { features: node_a.init_features(), remote_network_address: None }, false).unwrap();
@@ -9064,15 +9157,15 @@ pub mod bench {
                                let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());
                                $node_b.handle_update_add_htlc(&$node_a.get_our_node_id(), &payment_event.msgs[0]);
                                $node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &payment_event.commitment_msg);
-                               let (raa, cs) = do_get_revoke_commit_msgs!(NodeHolder { node: &$node_b }, &$node_a.get_our_node_id());
+                               let (raa, cs) = get_revoke_commit_msgs(&ANodeHolder { node: &$node_b }, &$node_a.get_our_node_id());
                                $node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &raa);
                                $node_a.handle_commitment_signed(&$node_b.get_our_node_id(), &cs);
-                               $node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_a }, MessageSendEvent::SendRevokeAndACK, $node_b.get_our_node_id()));
+                               $node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &get_event_msg!(ANodeHolder { node: &$node_a }, MessageSendEvent::SendRevokeAndACK, $node_b.get_our_node_id()));
 
-                               expect_pending_htlcs_forwardable!(NodeHolder { node: &$node_b });
-                               expect_payment_claimable!(NodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
+                               expect_pending_htlcs_forwardable!(ANodeHolder { node: &$node_b });
+                               expect_payment_claimable!(ANodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
                                $node_b.claim_funds(payment_preimage);
-                               expect_payment_claimed!(NodeHolder { node: &$node_b }, payment_hash, 10_000);
+                               expect_payment_claimed!(ANodeHolder { node: &$node_b }, payment_hash, 10_000);
 
                                match $node_b.get_and_clear_pending_msg_events().pop().unwrap() {
                                        MessageSendEvent::UpdateHTLCs { node_id, updates } => {
@@ -9083,12 +9176,12 @@ pub mod bench {
                                        _ => panic!("Failed to generate claim event"),
                                }
 
-                               let (raa, cs) = do_get_revoke_commit_msgs!(NodeHolder { node: &$node_a }, &$node_b.get_our_node_id());
+                               let (raa, cs) = get_revoke_commit_msgs(&ANodeHolder { node: &$node_a }, &$node_b.get_our_node_id());
                                $node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &raa);
                                $node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &cs);
-                               $node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_b }, MessageSendEvent::SendRevokeAndACK, $node_a.get_our_node_id()));
+                               $node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &get_event_msg!(ANodeHolder { node: &$node_b }, MessageSendEvent::SendRevokeAndACK, $node_a.get_our_node_id()));
 
-                               expect_payment_sent!(NodeHolder { node: &$node_a }, payment_preimage);
+                               expect_payment_sent!(ANodeHolder { node: &$node_a }, payment_preimage);
                        }
                }