Generate PaymentPathSuccessful event for each path
authorJeffrey Czyz <jkczyz@gmail.com>
Thu, 18 Nov 2021 22:24:14 +0000 (16:24 -0600)
committerJeffrey Czyz <jkczyz@gmail.com>
Tue, 23 Nov 2021 19:29:45 +0000 (13:29 -0600)
A single PaymentSent event is generated when a payment is fulfilled.
This is occurs when the preimage is revealed on the first claimed HTLC.
For subsequent HTLCs, the event is not generated.

In order to score channels involved with a successful payments, the
scorer must be notified of each successful path involved in the payment.
Add a PaymentPathSuccessful event for this purpose. Generate it whenever
a part is removed from a pending outbound payment. This avoids duplicate
events when reconnecting to a peer.

fuzz/src/chanmon_consistency.rs
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/monitor_tests.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/shutdown_tests.rs
lightning/src/util/events.rs

index acdf3cb3ca2dc3b2f902aa1a323e645b837d2368..3dd0cd5a45db5be0c1a1357af246a4a8fd01b42e 100644 (file)
@@ -829,6 +829,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                                        }
                                                },
                                                events::Event::PaymentSent { .. } => {},
+                                               events::Event::PaymentPathSuccessful { .. } => {},
                                                events::Event::PaymentPathFailed { .. } => {},
                                                events::Event::PaymentForwarded { .. } if $node == 1 => {},
                                                events::Event::PendingHTLCsForwardable { .. } => {
index dd717ca2f69fd485713e9c875eb4c98aa684e403..cb39b64a44834b83087bd415ee885aea4cf2f633 100644 (file)
@@ -551,6 +551,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
        nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke_and_ack);
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
        check_added_monitors!(nodes[0], 1);
+       expect_payment_path_successful!(nodes[0]);
 
        expect_pending_htlcs_forwardable!(nodes[1]);
 
@@ -1090,12 +1091,12 @@ fn test_monitor_update_fail_reestablish() {
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
-       let (our_payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
+       let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
 
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
 
-       assert!(nodes[2].node.claim_funds(our_payment_preimage));
+       assert!(nodes[2].node.claim_funds(payment_preimage));
        check_added_monitors!(nodes[2], 1);
        let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
        assert!(updates.update_add_htlcs.is_empty());
@@ -1159,13 +1160,7 @@ fn test_monitor_update_fail_reestablish() {
        assert_eq!(updates.update_fulfill_htlcs.len(), 1);
        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();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { payment_preimage, .. } => assert_eq!(payment_preimage, our_payment_preimage),
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent!(nodes[0], payment_preimage);
 }
 
 #[test]
@@ -1300,7 +1295,7 @@ fn claim_while_disconnected_monitor_update_fail() {
        let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
 
        // Forward a payment for B to claim
-       let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
@@ -1395,16 +1390,7 @@ fn claim_while_disconnected_monitor_update_fail() {
 
        nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_raa);
        check_added_monitors!(nodes[0], 1);
-
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(*payment_preimage, payment_preimage_1);
-                       assert_eq!(*payment_hash, payment_hash_1);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent!(nodes[0], payment_preimage_1);
 
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
@@ -1766,7 +1752,7 @@ fn monitor_update_claim_fail_no_response() {
        let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
 
        // Forward a payment for B to claim
-       let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        // Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
        let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
@@ -1802,16 +1788,7 @@ fn monitor_update_claim_fail_no_response() {
        let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], bs_updates.commitment_signed, false);
-
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(*payment_preimage, payment_preimage_1);
-                       assert_eq!(*payment_hash, payment_hash_1);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent!(nodes[0], payment_preimage_1);
 
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
@@ -2323,7 +2300,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
                        assert!(updates.update_fee.is_none());
                        assert_eq!(updates.update_fulfill_htlcs.len(), 1);
                        nodes[1].node.handle_update_fulfill_htlc(&nodes[0].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
-                       expect_payment_sent!(nodes[1], payment_preimage_0);
+                       expect_payment_sent_without_paths!(nodes[1], payment_preimage_0);
                        assert_eq!(updates.update_add_htlcs.len(), 1);
                        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
                        updates.commitment_signed
@@ -2342,7 +2319,18 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
 
        commitment_signed_dance!(nodes[1], nodes[0], (), false, true, false);
 
-       expect_pending_htlcs_forwardable!(nodes[1]);
+       let events = nodes[1].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 2);
+       match events[0] {
+               Event::PendingHTLCsForwardable { .. } => { },
+               _ => panic!("Unexpected event"),
+       };
+       match events[1] {
+               Event::PaymentPathSuccessful { .. } => { },
+               _ => panic!("Unexpected event"),
+       };
+
+       nodes[1].node.process_pending_htlc_forwards();
        expect_payment_received!(nodes[1], payment_hash_2, payment_secret_2, 100000);
 
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
@@ -2427,9 +2415,10 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
                bs_updates = Some(get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()));
                assert_eq!(bs_updates.as_ref().unwrap().update_fulfill_htlcs.len(), 1);
                nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.as_ref().unwrap().update_fulfill_htlcs[0]);
-               expect_payment_sent!(nodes[0], payment_preimage);
+               expect_payment_sent_without_paths!(nodes[0], payment_preimage);
                if htlc_status == HTLCStatusAtDupClaim::Cleared {
                        commitment_signed_dance!(nodes[0], nodes[1], &bs_updates.as_ref().unwrap().commitment_signed, false);
+                       expect_payment_path_successful!(nodes[0]);
                }
        } else {
                assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
@@ -2453,10 +2442,11 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
                bs_updates = Some(get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()));
                assert_eq!(bs_updates.as_ref().unwrap().update_fulfill_htlcs.len(), 1);
                nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.as_ref().unwrap().update_fulfill_htlcs[0]);
-               expect_payment_sent!(nodes[0], payment_preimage);
+               expect_payment_sent_without_paths!(nodes[0], payment_preimage);
        }
        if htlc_status != HTLCStatusAtDupClaim::Cleared {
                commitment_signed_dance!(nodes[0], nodes[1], &bs_updates.as_ref().unwrap().commitment_signed, false);
+               expect_payment_path_successful!(nodes[0]);
        }
 }
 
@@ -2620,7 +2610,7 @@ fn double_temp_error() {
        assert_eq!(node_id, nodes[0].node.get_our_node_id());
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &update_fulfill_1);
        check_added_monitors!(nodes[0], 0);
-       expect_payment_sent!(nodes[0], payment_preimage_1);
+       expect_payment_sent_without_paths!(nodes[0], payment_preimage_1);
        nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed_b1);
        check_added_monitors!(nodes[0], 1);
        nodes[0].node.process_pending_htlc_forwards();
@@ -2658,6 +2648,7 @@ fn double_temp_error() {
        };
        nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &raa_b2);
        check_added_monitors!(nodes[0], 1);
+       expect_payment_path_successful!(nodes[0]);
 
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &update_fulfill_2);
        check_added_monitors!(nodes[0], 0);
index 1e60dcdccd401538cf4b2210b030024818e2ef17..9ad97150205a847ddaf1395ce88c4b2ba51a5228 100644 (file)
@@ -449,6 +449,7 @@ pub(crate) enum PendingOutboundPayment {
        /// and add a pending payment that was already fulfilled.
        Fulfilled {
                session_privs: HashSet<[u8; 32]>,
+               payment_hash: Option<PaymentHash>,
        },
 }
 
@@ -472,15 +473,24 @@ impl PendingOutboundPayment {
                }
        }
 
+       fn payment_hash(&self) -> Option<PaymentHash> {
+               match self {
+                       PendingOutboundPayment::Legacy { .. } => None,
+                       PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash),
+                       PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash,
+               }
+       }
+
        fn mark_fulfilled(&mut self) {
                let mut session_privs = HashSet::new();
                core::mem::swap(&mut session_privs, match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        PendingOutboundPayment::Retryable { session_privs, .. } |
-                       PendingOutboundPayment::Fulfilled { session_privs }
+                       PendingOutboundPayment::Fulfilled { session_privs, .. }
                                => session_privs
                });
-               *self = PendingOutboundPayment::Fulfilled { session_privs };
+               let payment_hash = self.payment_hash();
+               *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash };
        }
 
        /// panics if path is None and !self.is_fulfilled
@@ -488,7 +498,7 @@ impl PendingOutboundPayment {
                let remove_res = match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        PendingOutboundPayment::Retryable { session_privs, .. } |
-                       PendingOutboundPayment::Fulfilled { session_privs } => {
+                       PendingOutboundPayment::Fulfilled { session_privs, .. } => {
                                session_privs.remove(session_priv)
                        }
                };
@@ -529,7 +539,7 @@ impl PendingOutboundPayment {
                match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        PendingOutboundPayment::Retryable { session_privs, .. } |
-                       PendingOutboundPayment::Fulfilled { session_privs } => {
+                       PendingOutboundPayment::Fulfilled { session_privs, .. } => {
                                session_privs.len()
                        }
                }
@@ -3491,14 +3501,23 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 
        fn finalize_claims(&self, mut sources: Vec<HTLCSource>) {
+               let mut pending_events = self.pending_events.lock().unwrap();
                for source in sources.drain(..) {
-                       if let HTLCSource::OutboundRoute { session_priv, payment_id, .. } = source {
+                       if let HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } = source {
                                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) {
                                        assert!(payment.get().is_fulfilled());
-                                       payment.get_mut().remove(&session_priv_bytes, None);
+                                       if payment.get_mut().remove(&session_priv_bytes, None) {
+                                               pending_events.push(
+                                                       events::Event::PaymentPathSuccessful {
+                                                               payment_id,
+                                                               payment_hash: payment.get().payment_hash(),
+                                                               path,
+                                                       }
+                                               );
+                                       }
                                        if payment.get().remaining_parts() == 0 {
                                                payment.remove();
                                        }
@@ -3515,9 +3534,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                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) {
-                                       let found_payment = !payment.get().is_fulfilled();
-                                       let fee_paid_msat = payment.get().get_pending_fee_msat();
-                                       payment.get_mut().mark_fulfilled();
+                                       let mut pending_events = self.pending_events.lock().unwrap();
+                                       if !payment.get().is_fulfilled() {
+                                               let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
+                                               let fee_paid_msat = payment.get().get_pending_fee_msat();
+                                               pending_events.push(
+                                                       events::Event::PaymentSent {
+                                                               payment_id: Some(payment_id),
+                                                               payment_preimage,
+                                                               payment_hash,
+                                                               fee_paid_msat,
+                                                       }
+                                               );
+                                               payment.get_mut().mark_fulfilled();
+                                       }
+
                                        if from_onchain {
                                                // We currently immediately remove HTLCs which were fulfilled on-chain.
                                                // This could potentially lead to removing a pending payment too early,
@@ -3525,22 +3556,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // restart.
                                                // TODO: We should have a second monitor event that informs us of payments
                                                // irrevocably fulfilled.
-                                               payment.get_mut().remove(&session_priv_bytes, Some(&path));
+                                               if payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
+                                                       let payment_hash = Some(PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()));
+                                                       pending_events.push(
+                                                               events::Event::PaymentPathSuccessful {
+                                                                       payment_id,
+                                                                       payment_hash,
+                                                                       path,
+                                                               }
+                                                       );
+                                               }
+
                                                if payment.get().remaining_parts() == 0 {
                                                        payment.remove();
                                                }
                                        }
-                                       if found_payment {
-                                               let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentSent {
-                                                               payment_id: Some(payment_id),
-                                                               payment_preimage,
-                                                               payment_hash: payment_hash,
-                                                               fee_paid_msat,
-                                                       }
-                                               );
-                                       }
                                } else {
                                        log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
                                }
@@ -4635,6 +4665,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        pub fn has_pending_payments(&self) -> bool {
                !self.pending_outbound_payments.lock().unwrap().is_empty()
        }
+
+       #[cfg(test)]
+       pub fn clear_pending_payments(&self) {
+               self.pending_outbound_payments.lock().unwrap().clear()
+       }
 }
 
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> MessageSendEventsProvider for ChannelManager<Signer, M, T, K, F, L>
@@ -5559,6 +5594,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
        },
        (1, Fulfilled) => {
                (0, session_privs, required),
+               (1, payment_hash, option),
        },
        (2, Retryable) => {
                (0, session_privs, required),
@@ -6325,9 +6361,10 @@ mod tests {
                nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
                check_added_monitors!(nodes[0], 1);
 
-               // Note that successful MPP payments will generate 1 event upon the first path's success. No
-               // further events will be generated for subsequence path successes.
+               // Note that successful MPP payments will generate a single PaymentSent event upon the first
+               // path's success and a PaymentPathSuccessful event for each path's success.
                let events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 3);
                match events[0] {
                        Event::PaymentSent { payment_id: ref id, payment_preimage: ref preimage, payment_hash: ref hash, .. } => {
                                assert_eq!(Some(payment_id), *id);
@@ -6336,6 +6373,22 @@ mod tests {
                        },
                        _ => panic!("Unexpected event"),
                }
+               match events[1] {
+                       Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
+                               assert_eq!(payment_id, *actual_payment_id);
+                               assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
+                               assert_eq!(route.paths[0], *path);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+               match events[2] {
+                       Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
+                               assert_eq!(payment_id, *actual_payment_id);
+                               assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
+                               assert_eq!(route.paths[0], *path);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
        }
 
        #[test]
index e9e36e607ec45950dc4ebadab6a8c940c756f114..deff0f704c0d00422375dddb6db565bc180eb794 100644 (file)
@@ -1079,24 +1079,65 @@ macro_rules! expect_payment_received {
        }
 }
 
+#[cfg(test)]
+macro_rules! expect_payment_sent_without_paths {
+       ($node: expr, $expected_payment_preimage: expr) => {
+               expect_payment_sent!($node, $expected_payment_preimage, None::<u64>, false);
+       };
+       ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr) => {
+               expect_payment_sent!($node, $expected_payment_preimage, $expected_fee_msat_opt, false);
+       }
+}
+
 macro_rules! expect_payment_sent {
        ($node: expr, $expected_payment_preimage: expr) => {
-               expect_payment_sent!($node, $expected_payment_preimage, None::<u64>);
+               expect_payment_sent!($node, $expected_payment_preimage, None::<u64>, true);
        };
        ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr) => {
+               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) => {
                let events = $node.node.get_and_clear_pending_events();
                let expected_payment_hash = PaymentHash(Sha256::hash(&$expected_payment_preimage.0).into_inner());
-               assert_eq!(events.len(), 1);
-               match events[0] {
-                       Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
+               if $expect_paths {
+                       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);
                                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] {
+                                       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_rules! expect_payment_path_successful {
+       ($node: expr) => {
+               let events = $node.node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 1);
+               match events[0] {
+                       Event::PaymentPathSuccessful { .. } => {},
+                       _ => panic!("Unexpected event"),
                }
        }
 }
index 8e9893870dadf8c128b2025b6b2142443df7505c..370e94e636a9d4f817163b0e11f2a80ca9b650ed 100644 (file)
@@ -1904,7 +1904,7 @@ fn channel_reserve_in_flight_removes() {
        nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_removes.commitment_signed);
        check_added_monitors!(nodes[0], 1);
        let as_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
-       expect_payment_sent!(nodes[0], payment_preimage_1);
+       expect_payment_sent_without_paths!(nodes[0], payment_preimage_1);
 
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send_1.msgs[0]);
        nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send_1.commitment_msg);
@@ -1933,7 +1933,7 @@ fn channel_reserve_in_flight_removes() {
        nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_cs.commitment_signed);
        check_added_monitors!(nodes[0], 1);
        let as_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
-       expect_payment_sent!(nodes[0], payment_preimage_2);
+       expect_payment_sent_without_paths!(nodes[0], payment_preimage_2);
 
        nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa);
        check_added_monitors!(nodes[1], 1);
@@ -1946,6 +1946,7 @@ fn channel_reserve_in_flight_removes() {
        // resolve the second HTLC from A's point of view.
        nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_raa);
        check_added_monitors!(nodes[0], 1);
+       expect_payment_path_successful!(nodes[0]);
        let as_cs = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
 
        // Now that B doesn't have the second RAA anymore, but A still does, send a payment from B back
@@ -1975,6 +1976,7 @@ fn channel_reserve_in_flight_removes() {
 
        nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_raa);
        check_added_monitors!(nodes[0], 1);
+       expect_payment_path_successful!(nodes[0]);
        let as_cs = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
 
        nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_cs.commitment_signed);
@@ -2654,7 +2656,7 @@ fn test_htlc_on_chain_success() {
        check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 3);
+       assert_eq!(events.len(), 5);
        let mut first_claimed = false;
        for event in events {
                match event {
@@ -2667,6 +2669,7 @@ fn test_htlc_on_chain_success() {
                                        assert_eq!(payment_hash, payment_hash_2);
                                }
                        },
+                       Event::PaymentPathSuccessful { .. } => {},
                        Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => {},
                        _ => panic!("Unexpected event"),
                }
@@ -3301,13 +3304,13 @@ fn test_dup_events_on_peer_disconnect() {
        check_added_monitors!(nodes[1], 1);
        let claim_msgs = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &claim_msgs.update_fulfill_htlcs[0]);
-       expect_payment_sent!(nodes[0], payment_preimage);
+       expect_payment_sent_without_paths!(nodes[0], payment_preimage);
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
 
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (1, 0), (0, 0), (0, 0), (0, 0), (false, false));
-       assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+       expect_payment_path_successful!(nodes[0]);
 }
 
 #[test]
@@ -3347,7 +3350,7 @@ fn test_simple_peer_disconnect() {
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (1, 0), (1, 0), (false, false));
        {
                let events = nodes[0].node.get_and_clear_pending_events();
-               assert_eq!(events.len(), 2);
+               assert_eq!(events.len(), 3);
                match events[0] {
                        Event::PaymentSent { payment_preimage, payment_hash, .. } => {
                                assert_eq!(payment_preimage, payment_preimage_3);
@@ -3362,6 +3365,10 @@ fn test_simple_peer_disconnect() {
                        },
                        _ => panic!("Unexpected event"),
                }
+               match events[2] {
+                       Event::PaymentPathSuccessful { .. } => {},
+                       _ => panic!("Unexpected event"),
+               }
        }
 
        claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), payment_preimage_4);
@@ -3551,15 +3558,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
        if messages_delivered < 2 {
                reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (1, 0), (0, 0), (0, 0), (0, 0), (false, false));
                if messages_delivered < 1 {
-                       let events_4 = nodes[0].node.get_and_clear_pending_events();
-                       assert_eq!(events_4.len(), 1);
-                       match events_4[0] {
-                               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                                       assert_eq!(payment_preimage_1, *payment_preimage);
-                                       assert_eq!(payment_hash_1, *payment_hash);
-                               },
-                               _ => panic!("Unexpected event"),
-                       }
+                       expect_payment_sent!(nodes[0], payment_preimage_1);
                } else {
                        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                }
@@ -3577,10 +3576,18 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
                reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
        }
 
+       if messages_delivered == 1 || messages_delivered == 2 {
+               expect_payment_path_successful!(nodes[0]);
+       }
+
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
+       if messages_delivered > 2 {
+               expect_payment_path_successful!(nodes[0]);
+       }
+
        // Channel should still work fine...
        let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
        let payment_preimage_2 = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0;
@@ -3892,6 +3899,7 @@ fn test_drop_messages_peer_disconnect_dual_htlc() {
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
        check_added_monitors!(nodes[0], 1);
 
+       expect_payment_path_successful!(nodes[0]);
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
@@ -5559,24 +5567,16 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
 
-       let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], if use_dust { 50000 } else { 3000000 });
+       let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1]], if use_dust { 50000 } else { 3000000 });
 
        // Claim the payment, but don't deliver A's commitment_signed, resulting in the HTLC only being
        // present in B's local commitment transaction, but none of A's commitment transactions.
-       assert!(nodes[1].node.claim_funds(our_payment_preimage));
+       assert!(nodes[1].node.claim_funds(payment_preimage));
        check_added_monitors!(nodes[1], 1);
 
        let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]);
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { payment_preimage, payment_hash, .. } => {
-                       assert_eq!(payment_preimage, our_payment_preimage);
-                       assert_eq!(payment_hash, our_payment_hash);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent_without_paths!(nodes[0], payment_preimage);
 
        nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_updates.commitment_signed);
        check_added_monitors!(nodes[0], 1);
@@ -5998,15 +5998,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
        let update_msgs = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &update_msgs.update_fulfill_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], update_msgs.commitment_signed, false, true);
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(*payment_preimage, payment_preimage_1);
-                       assert_eq!(*payment_hash, payment_hash_1);
-               }
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent!(nodes[0], payment_preimage_1);
 }
 
 // Test that if we fail to forward an HTLC that is being freed from the holding cell that the
index 0e4733ea526d2c33e7206c85be5dc335613ad771..cfde643bcb93c3ed80b49d102f082fed2a4d15d5 100644 (file)
@@ -249,7 +249,7 @@ fn do_test_claim_value_force_close(prev_commitment_tx: bool) {
        if prev_commitment_tx {
                // To build a previous commitment transaction, deliver one round of commitment messages.
                nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &b_htlc_msgs.update_fulfill_htlcs[0]);
-               expect_payment_sent!(nodes[0], payment_preimage);
+               expect_payment_sent_without_paths!(nodes[0], payment_preimage);
                nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &b_htlc_msgs.commitment_signed);
                check_added_monitors!(nodes[0], 1);
                let (as_raa, as_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
@@ -400,7 +400,9 @@ fn do_test_claim_value_force_close(prev_commitment_tx: bool) {
        // After broadcasting the HTLC claim transaction, node A will still consider the HTLC
        // possibly-claimable up to ANTI_REORG_DELAY, at which point it will drop it.
        mine_transaction(&nodes[0], &b_broadcast_txn[0]);
-       if !prev_commitment_tx {
+       if prev_commitment_tx {
+               expect_payment_path_successful!(nodes[0]);
+       } else {
                expect_payment_sent!(nodes[0], payment_preimage);
        }
        assert_eq!(sorted_vec(vec![Balance::MaybeClaimableHTLCAwaitingTimeout {
index f3681946d194f614583c64488bb809103fccc9ec..e80cf98d7b9b358e7713f21447a9f7516d5ed3f0 100644 (file)
@@ -477,6 +477,11 @@ fn test_onion_failure() {
        bogus_route.paths[0][route_len-1].fee_msat = amt_to_forward;
        run_onion_failure_test("amount_below_minimum", 0, &nodes, &bogus_route, &payment_hash, &payment_secret, |_| {}, ||{}, true, Some(UPDATE|11), Some(NetworkUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy(short_channel_id)}), Some(short_channel_id));
 
+       // Clear pending payments so that the following positive test has the correct payment hash.
+       for node in nodes.iter() {
+               node.node.clear_pending_payments();
+       }
+
        // Test a positive test-case with one extra msat, meeting the minimum.
        bogus_route.paths[0][route_len-1].fee_msat = amt_to_forward + 1;
        let preimage = send_along_route(&nodes[0], bogus_route, &[&nodes[1], &nodes[2]], amt_to_forward+1).0;
index f685c375f38913c6fbedc9ce40a064d7e7373598..7c9ff3199d15bfe53e2e72967866c85b7b1702ae 100644 (file)
@@ -664,7 +664,7 @@ fn test_fulfill_restart_failure() {
        check_added_monitors!(nodes[1], 1);
        let htlc_fulfill_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &htlc_fulfill_updates.update_fulfill_htlcs[0]);
-       expect_payment_sent!(nodes[0], payment_preimage);
+       expect_payment_sent_without_paths!(nodes[0], payment_preimage);
 
        // Now reload nodes[1]...
        persister = test_utils::TestPersister::new();
@@ -707,4 +707,5 @@ fn test_fulfill_restart_failure() {
        commitment_signed_dance!(nodes[0], nodes[1], htlc_fail_updates.commitment_signed, false);
        // nodes[0] shouldn't generate any events here, while it just got a payment failure completion
        // it had already considered the payment fulfilled, and now they just got free money.
+       assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
 }
index fcec4703e44f76306b0889d138c71ebf0b28e1a7..db76a2c760ccb917d6f98cbbd57735c0114156a8 100644 (file)
@@ -83,7 +83,7 @@ fn updates_shutdown_wait() {
        let logger = test_utils::TestLogger::new();
        let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
-       let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
+       let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
 
        nodes[0].node.close_channel(&chan_1.2).unwrap();
        let node_0_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
@@ -103,7 +103,7 @@ fn updates_shutdown_wait() {
        unwrap_send_err!(nodes[0].node.send_payment(&route_1, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable {..}, {});
        unwrap_send_err!(nodes[1].node.send_payment(&route_2, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable {..}, {});
 
-       assert!(nodes[2].node.claim_funds(our_payment_preimage));
+       assert!(nodes[2].node.claim_funds(payment_preimage));
        check_added_monitors!(nodes[2], 1);
        let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
        assert!(updates.update_add_htlcs.is_empty());
@@ -124,16 +124,7 @@ fn updates_shutdown_wait() {
        assert_eq!(updates_2.update_fulfill_htlcs.len(), 1);
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates_2.update_fulfill_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], updates_2.commitment_signed, false, true);
-
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(our_payment_preimage, *payment_preimage);
-                       assert_eq!(our_payment_hash, *payment_hash);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent!(nodes[0], payment_preimage);
 
        let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
        nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed);
@@ -241,7 +232,7 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
-       let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
+       let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
 
        nodes[1].node.close_channel(&chan_1.2).unwrap();
        let node_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id());
@@ -281,7 +272,7 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
        assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
 
-       assert!(nodes[2].node.claim_funds(our_payment_preimage));
+       assert!(nodes[2].node.claim_funds(payment_preimage));
        check_added_monitors!(nodes[2], 1);
        let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
        assert!(updates.update_add_htlcs.is_empty());
@@ -302,16 +293,7 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
        assert_eq!(updates_2.update_fulfill_htlcs.len(), 1);
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates_2.update_fulfill_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], updates_2.commitment_signed, false, true);
-
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(our_payment_preimage, *payment_preimage);
-                       assert_eq!(our_payment_hash, *payment_hash);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent!(nodes[0], payment_preimage);
 
        let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
        if recv_count > 0 {
index 6bbd6b9946ccc49e18ae7b85a715b23d2478f4ad..b42556841eac8ee6cdb0b30522a000b62c9e038c 100644 (file)
@@ -216,7 +216,7 @@ pub enum Event {
                /// Note that this serves as a payment receipt, if you wish to have such a thing, you must
                /// store it somehow!
                payment_preimage: PaymentPreimage,
-               /// The hash which was given to [`ChannelManager::send_payment`].
+               /// The hash that was given to [`ChannelManager::send_payment`].
                ///
                /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
                payment_hash: PaymentHash,
@@ -240,7 +240,9 @@ pub enum Event {
                /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
                /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
                payment_id: Option<PaymentId>,
-               /// The hash which was given to ChannelManager::send_payment.
+               /// The hash that was given to [`ChannelManager::send_payment`].
+               ///
+               /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
                payment_hash: PaymentHash,
                /// 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
@@ -338,7 +340,27 @@ pub enum Event {
                channel_id: [u8; 32],
                /// The full transaction received from the user
                transaction: Transaction
-       }
+       },
+       /// Indicates that a path for an outbound payment was successful.
+       ///
+       /// Always generated after [`Event::PaymentSent`] and thus useful for scoring channels. See
+       /// [`Event::PaymentSent`] for obtaining the payment preimage.
+       PaymentPathSuccessful {
+               /// The id returned by [`ChannelManager::send_payment`] and used with
+               /// [`ChannelManager::retry_payment`].
+               ///
+               /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
+               /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
+               payment_id: PaymentId,
+               /// The hash that was given to [`ChannelManager::send_payment`].
+               ///
+               /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
+               payment_hash: Option<PaymentHash>,
+               /// The payment path that was successful.
+               ///
+               /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
+               path: Vec<RouteHop>,
+       },
 }
 
 impl Writeable for Event {
@@ -438,6 +460,14 @@ impl Writeable for Event {
                                        (2, transaction, required)
                                })
                        },
+                       &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+                               13u8.write(writer)?;
+                               write_tlv_fields!(writer, {
+                                       (0, payment_id, required),
+                                       (2, payment_hash, option),
+                                       (4, path, vec_type)
+                               })
+                       },
                        // Note that, going forward, all new events must only write data inside of
                        // `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
                        // data via `write_tlv_fields`.
@@ -600,6 +630,24 @@ impl MaybeReadable for Event {
                                };
                                f()
                        },
+                       13u8 => {
+                               let f = || {
+                                       let mut payment_id = PaymentId([0; 32]);
+                                       let mut payment_hash = None;
+                                       let mut path: Option<Vec<RouteHop>> = Some(vec![]);
+                                       read_tlv_fields!(reader, {
+                                               (0, payment_id, required),
+                                               (2, payment_hash, option),
+                                               (4, path, vec_type),
+                                       });
+                                       Ok(Some(Event::PaymentPathSuccessful {
+                                               payment_id,
+                                               payment_hash,
+                                               path: path.unwrap(),
+                                       }))
+                               };
+                               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.