diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3748c5688da..df9cecba756 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -618,6 +618,61 @@ pub type SimpleArcChannelManager = 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, &'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; + type M: Deref; + type Broadcaster: BroadcasterInterface; + type T: Deref; + type EntropySource: EntropySource; + type ES: Deref; + type NodeSigner: NodeSigner; + type NS: Deref; + type Signer: WriteableEcdsaChannelSigner; + type SignerProvider: SignerProvider; + type SP: Deref; + type FeeEstimator: FeeEstimator; + type F: Deref; + type Router: Router; + type R: Deref; + type Logger: Logger; + type L: Deref; + fn get_cm(&self) -> &ChannelManager; +} +#[cfg(any(test, feature = "_test_utils"))] +impl AChannelManager +for ChannelManager +where + M::Target: chain::Watch<::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 = ::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 { 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. /// @@ -1623,6 +1678,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 ChannelManager where M::Target: chain::Watch<::Signer>, @@ -5720,30 +5805,8 @@ where pub async fn process_pending_events_async 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 }); } } @@ -5825,26 +5888,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(&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)); } } @@ -8849,14 +8894,23 @@ pub mod bench { use test::Bencher; - struct NodeHolder<'a, P: Persist> { - node: &'a ChannelManager< - &'a ChainMonitor, - &'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, + &'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> { + node: &'a Manager<'a, P>, + } + impl<'a, P: Persist> 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)] @@ -8887,7 +8941,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); @@ -8897,7 +8951,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(); @@ -8993,15 +9047,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 } => { @@ -9012,12 +9066,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); } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b98db4bb957..f8ccce05847 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -15,7 +15,7 @@ use crate::chain::channelmonitor::ChannelMonitor; use crate::chain::transaction::OutPoint; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; -use crate::ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA}; +use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA}; use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate}; use crate::routing::router::{self, PaymentParameters, Route}; use crate::ln::features::InitFeatures; @@ -324,6 +324,8 @@ pub struct NodeCfg<'a> { pub override_init_features: Rc>>, } +type TestChannelManager<'a, 'b, 'c> = ChannelManager<&'b TestChainMonitor<'c>, &'c test_utils::TestBroadcaster, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'c test_utils::TestFeeEstimator, &'b test_utils::TestRouter<'c>, &'c test_utils::TestLogger>; + pub struct Node<'a, 'b: 'a, 'c: 'b> { pub chain_source: &'c test_utils::TestChainSource, pub tx_broadcaster: &'c test_utils::TestBroadcaster, @@ -331,7 +333,7 @@ pub struct Node<'a, 'b: 'a, 'c: 'b> { pub router: &'b test_utils::TestRouter<'c>, pub chain_monitor: &'b test_utils::TestChainMonitor<'c>, pub keys_manager: &'b test_utils::TestKeysInterface, - pub node: &'a ChannelManager<&'b TestChainMonitor<'c>, &'c test_utils::TestBroadcaster, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'c test_utils::TestFeeEstimator, &'b test_utils::TestRouter<'c>, &'c test_utils::TestLogger>, + pub node: &'a TestChannelManager<'a, 'b, 'c>, pub network_graph: &'a NetworkGraph<&'c test_utils::TestLogger>, pub gossip_sync: P2PGossipSync<&'b NetworkGraph<&'c test_utils::TestLogger>, &'c test_utils::TestChainSource, &'c test_utils::TestLogger>, pub node_seed: [u8; 32], @@ -367,6 +369,39 @@ impl NodePtr { unsafe impl Send for NodePtr {} unsafe impl Sync for NodePtr {} + +pub trait NodeHolder { + type CM: AChannelManager; + fn node(&self) -> &ChannelManager< + ::M, + ::T, + ::ES, + ::NS, + ::SP, + ::F, + ::R, + ::L>; + fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor>; +} +impl NodeHolder for &H { + type CM = H::CM; + fn node(&self) -> &ChannelManager< + ::M, + ::T, + ::ES, + ::NS, + ::SP, + ::F, + ::R, + ::L> { (*self).node() } + fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { (*self).chain_monitor() } +} +impl<'a, 'b: 'a, 'c: 'b> NodeHolder for Node<'a, 'b, 'c> { + type CM = TestChannelManager<'a, 'b, 'c>; + fn node(&self) -> &TestChannelManager<'a, 'b, 'c> { &self.node } + fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { Some(self.chain_monitor) } +} + impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { fn drop(&mut self) { if !panicking() { @@ -486,36 +521,27 @@ pub fn create_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(node_a: &'a Node<'b, } /// Gets an RAA and CS which were sent in response to a commitment update -/// -/// Should only be used directly when the `$node` is not actually a [`Node`]. -macro_rules! do_get_revoke_commit_msgs { - ($node: expr, $recipient: expr) => { { - let events = $node.node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 2); - (match events[0] { - MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { - assert_eq!(node_id, $recipient); - (*msg).clone() - }, - _ => panic!("Unexpected event"), - }, match events[1] { - MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => { - assert_eq!(node_id, $recipient); - assert!(updates.update_add_htlcs.is_empty()); - assert!(updates.update_fulfill_htlcs.is_empty()); - assert!(updates.update_fail_htlcs.is_empty()); - assert!(updates.update_fail_malformed_htlcs.is_empty()); - assert!(updates.update_fee.is_none()); - updates.commitment_signed.clone() - }, - _ => panic!("Unexpected event"), - }) - } } -} - -/// Gets an RAA and CS which were sent in response to a commitment update -pub fn get_revoke_commit_msgs(node: &Node, recipient: &PublicKey) -> (msgs::RevokeAndACK, msgs::CommitmentSigned) { - do_get_revoke_commit_msgs!(node, recipient) +pub fn get_revoke_commit_msgs>(node: &H, recipient: &PublicKey) -> (msgs::RevokeAndACK, msgs::CommitmentSigned) { + let events = node.node().get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + (match events[0] { + MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { + assert_eq!(node_id, recipient); + (*msg).clone() + }, + _ => panic!("Unexpected event"), + }, match events[1] { + MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => { + assert_eq!(node_id, recipient); + assert!(updates.update_add_htlcs.is_empty()); + assert!(updates.update_fulfill_htlcs.is_empty()); + assert!(updates.update_fail_htlcs.is_empty()); + assert!(updates.update_fail_malformed_htlcs.is_empty()); + assert!(updates.update_fee.is_none()); + updates.commitment_signed.clone() + }, + _ => panic!("Unexpected event"), + }) } #[macro_export] @@ -774,10 +800,12 @@ macro_rules! unwrap_send_err { } /// Check whether N channel monitor(s) have been added. -pub fn check_added_monitors(node: &Node, count: usize) { - let mut added_monitors = node.chain_monitor.added_monitors.lock().unwrap(); - assert_eq!(added_monitors.len(), count); - added_monitors.clear(); +pub fn check_added_monitors>(node: &H, count: usize) { + if let Some(chain_monitor) = node.chain_monitor() { + let mut added_monitors = chain_monitor.added_monitors.lock().unwrap(); + assert_eq!(added_monitors.len(), count); + added_monitors.clear(); + } } /// Check whether N channel monitor(s) have been added. @@ -1527,23 +1555,30 @@ macro_rules! commitment_signed_dance { bs_revoke_and_ack } }; - ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */) => { - { - let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards); - $node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack); - $crate::ln::functional_test_utils::check_added_monitors(&$node_a, 1); - extra_msg_option - } - }; ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, false /* no extra message */) => { - assert!(commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true).is_none()); + assert!($crate::ln::functional_test_utils::commitment_signed_dance_through_cp_raa(&$node_a, &$node_b, $fail_backwards).is_none()); }; ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => { $crate::ln::functional_test_utils::do_commitment_signed_dance(&$node_a, &$node_b, &$commitment_signed, $fail_backwards, false); } } - +/// Runs the commitment_signed dance after the initial commitment_signed is delivered through to +/// the initiator's `revoke_and_ack` response. i.e. [`do_main_commitment_signed_dance`] plus the +/// `revoke_and_ack` response to it. +/// +/// Returns any additional message `node_b` generated in addition to the `revoke_and_ack` response. +pub fn commitment_signed_dance_through_cp_raa(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, fail_backwards: bool) -> Option { + let (extra_msg_option, bs_revoke_and_ack) = do_main_commitment_signed_dance(node_a, node_b, fail_backwards); + node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &bs_revoke_and_ack); + check_added_monitors(node_a, 1); + extra_msg_option +} + +/// Does the main logic in the commitment_signed dance. After the first `commitment_signed` has +/// been delivered, this method picks up and delivers the response `revoke_and_ack` and +/// `commitment_signed`, returning the recipient's `revoke_and_ack` and any extra message it may +/// have included. pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, fail_backwards: bool) -> (Option, msgs::RevokeAndACK) { let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(node_a, node_b.node.get_our_node_id()); check_added_monitors!(node_b, 0); @@ -1572,6 +1607,11 @@ pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node< (extra_msg_option, bs_revoke_and_ack) } +/// Runs a full commitment_signed dance, delivering a commitment_signed, the responding +/// `revoke_and_ack` and `commitment_signed`, and then the final `revoke_and_ack` response. +/// +/// If `skip_last_step` is unset, also checks for the payment failure update for the previous hop +/// on failure or that no new messages are left over on success. pub fn do_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, commitment_signed: &msgs::CommitmentSigned, fail_backwards: bool, skip_last_step: bool) { check_added_monitors!(node_a, 0); assert!(node_a.node.get_and_clear_pending_msg_events().is_empty()); @@ -1713,6 +1753,44 @@ macro_rules! expect_payment_claimed { } } +pub fn expect_payment_sent>(node: &H, + expected_payment_preimage: PaymentPreimage, expected_fee_msat_opt: Option>, + expect_per_path_claims: bool, +) { + let events = node.node().get_and_clear_pending_events(); + let expected_payment_hash = PaymentHash( + bitcoin::hashes::sha256::Hash::hash(&expected_payment_preimage.0).into_inner()); + if expect_per_path_claims { + assert!(events.len() > 1); + } else { + assert_eq!(events.len(), 1); + } + let expected_payment_id = match events[0] { + Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => { + assert_eq!(expected_payment_preimage, *payment_preimage); + assert_eq!(expected_payment_hash, *payment_hash); + if let Some(expected_fee_msat) = expected_fee_msat_opt { + assert_eq!(*fee_paid_msat, expected_fee_msat); + } else { + assert!(fee_paid_msat.is_some()); + } + payment_id.unwrap() + }, + _ => panic!("Unexpected event"), + }; + if expect_per_path_claims { + for i in 1..events.len() { + match events[i] { + Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => { + assert_eq!(payment_id, expected_payment_id); + assert_eq!(payment_hash, Some(expected_payment_hash)); + }, + _ => panic!("Unexpected event"), + } + } + } +} + #[cfg(test)] #[macro_export] macro_rules! expect_payment_sent_without_paths { @@ -1732,40 +1810,10 @@ macro_rules! expect_payment_sent { ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr) => { $crate::expect_payment_sent!($node, $expected_payment_preimage, $expected_fee_msat_opt, true); }; - ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr, $expect_paths: expr) => { { - use bitcoin::hashes::Hash as _; - let events = $node.node.get_and_clear_pending_events(); - let expected_payment_hash = $crate::ln::PaymentHash( - bitcoin::hashes::sha256::Hash::hash(&$expected_payment_preimage.0).into_inner()); - if $expect_paths { - assert!(events.len() > 1); - } else { - assert_eq!(events.len(), 1); - } - let expected_payment_id = match events[0] { - $crate::events::Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => { - assert_eq!($expected_payment_preimage, *payment_preimage); - assert_eq!(expected_payment_hash, *payment_hash); - assert!(fee_paid_msat.is_some()); - if $expected_fee_msat_opt.is_some() { - assert_eq!(*fee_paid_msat, $expected_fee_msat_opt); - } - payment_id.unwrap() - }, - _ => panic!("Unexpected event"), - }; - if $expect_paths { - for i in 1..events.len() { - match events[i] { - $crate::events::Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => { - assert_eq!(payment_id, expected_payment_id); - assert_eq!(payment_hash, Some(expected_payment_hash)); - }, - _ => panic!("Unexpected event"), - } - } - } - } } + ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr, $expect_paths: expr) => { + $crate::ln::functional_test_utils::expect_payment_sent(&$node, $expected_payment_preimage, + $expected_fee_msat_opt.map(|o| Some(o)), $expect_paths); + } } #[cfg(test)] diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index fc376879264..fac15fc5aaa 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4897,15 +4897,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[1], &updates.commitment_signed, false); - - let events = nodes[0].node.get_and_clear_pending_events(); - match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => { - assert_eq!(*payment_preimage, our_payment_preimage); - assert_eq!(*payment_hash, duplicate_payment_hash); - } - _ => panic!("Unexpected event"), - } + expect_payment_sent(&nodes[0], our_payment_preimage, None, true); } #[test] @@ -9481,26 +9473,7 @@ fn test_inconsistent_mpp_params() { pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None); do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage); - let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 3); - match events[0] { - Event::PaymentSent { payment_hash, .. } => { // The payment was abandoned earlier, so the fee paid will be None - assert_eq!(payment_hash, our_payment_hash); - }, - _ => panic!("Unexpected event") - } - match events[1] { - Event::PaymentPathSuccessful { payment_hash, .. } => { - assert_eq!(payment_hash.unwrap(), our_payment_hash); - }, - _ => panic!("Unexpected event") - } - match events[2] { - Event::PaymentPathSuccessful { payment_hash, .. } => { - assert_eq!(payment_hash.unwrap(), our_payment_hash); - }, - _ => panic!("Unexpected event") - } + expect_payment_sent(&nodes[0], our_payment_preimage, Some(None), true); } #[test]