Skip to content

Commit 0273ac5

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #1084 from valentinewallace/2021-09-rename-paymentfailed
Rename Event PaymentFailed -> PaymentPathFailed
2 parents ed3a6ee + e5310dd commit 0273ac5

9 files changed

+56
-39
lines changed

‎fuzz/src/chanmon_consistency.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
827827
}
828828
},
829829
events::Event::PaymentSent { .. } => {},
830-
events::Event::PaymentFailed { .. } => {},
830+
events::Event::PaymentPathFailed { .. } => {},
831831
events::Event::PaymentForwarded { .. } if $node == 1 => {},
832832
events::Event::PendingHTLCsForwardable { .. } => {
833833
nodes[$node].process_pending_htlc_forwards();

‎lightning/src/ln/chanmon_update_fail_tests.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ fn do_test_simple_monitor_permanent_update_fail(persister_fail: bool) {
7878
};
7979

8080
// TODO: Once we hit the chain with the failure transaction we should check that we get a
81-
// PaymentFailed event
81+
// PaymentPathFailed event
8282

8383
assert_eq!(nodes[0].node.list_channels().len(), 0);
8484
check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
@@ -267,7 +267,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail
267267
check_closed_broadcast!(nodes[0], true);
268268

269269
// TODO: Once we hit the chain with the failure transaction we should check that we get a
270-
// PaymentFailed event
270+
// PaymentPathFailed event
271271

272272
assert_eq!(nodes[0].node.list_channels().len(), 0);
273273
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
@@ -1819,7 +1819,7 @@ fn test_monitor_update_on_pending_forwards() {
18191819

18201820
let events = nodes[0].node.get_and_clear_pending_events();
18211821
assert_eq!(events.len(), 2);
1822-
if let Event::PaymentFailed { payment_hash, rejected_by_dest, .. } = events[0] {
1822+
if let Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] {
18231823
assert_eq!(payment_hash, payment_hash_1);
18241824
assert!(rejected_by_dest);
18251825
} else { panic!("Unexpected event!"); }

‎lightning/src/ln/channelmanager.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
489489
/// The session_priv bytes of outbound payments which are pending resolution.
490490
/// The authoritative state of these HTLCs resides either within Channels or ChannelMonitors
491491
/// (if the channel has been force-closed), however we track them here to prevent duplicative
492-
/// PaymentSent/PaymentFailed events. Specifically, in the case of a duplicative
492+
/// PaymentSent/PaymentPathFailed events. Specifically, in the case of a duplicative
493493
/// update_fulfill_htlc message after a reconnect, we may "claim" a payment twice.
494494
/// Additionally, because ChannelMonitors are often not re-serialized after connecting block(s)
495495
/// which may generate a claim event, we may receive similar duplicate claim/fail MonitorEvents
@@ -2875,18 +2875,19 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28752875
self.fail_htlc_backwards_internal(channel_state,
28762876
htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data});
28772877
},
2878-
HTLCSource::OutboundRoute { session_priv, mpp_id, .. } => {
2878+
HTLCSource::OutboundRoute { session_priv, mpp_id, path, .. } => {
28792879
let mut session_priv_bytes = [0; 32];
28802880
session_priv_bytes.copy_from_slice(&session_priv[..]);
28812881
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
28822882
if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) {
28832883
if sessions.get_mut().remove(&session_priv_bytes) {
28842884
self.pending_events.lock().unwrap().push(
2885-
events::Event::PaymentFailed {
2885+
events::Event::PaymentPathFailed {
28862886
payment_hash,
28872887
rejected_by_dest: false,
28882888
network_update: None,
28892889
all_paths_failed: sessions.get().len() == 0,
2890+
path: path.clone(),
28902891
#[cfg(test)]
28912892
error_code: None,
28922893
#[cfg(test)]
@@ -2951,11 +2952,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
29512952
// process_onion_failure we should close that channel as it implies our
29522953
// next-hop is needlessly blaming us!
29532954
self.pending_events.lock().unwrap().push(
2954-
events::Event::PaymentFailed {
2955+
events::Event::PaymentPathFailed {
29552956
payment_hash: payment_hash.clone(),
29562957
rejected_by_dest: !payment_retryable,
29572958
network_update,
29582959
all_paths_failed,
2960+
path: path.clone(),
29592961
#[cfg(test)]
29602962
error_code: onion_error_code,
29612963
#[cfg(test)]
@@ -2977,11 +2979,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
29772979
// TODO: For non-temporary failures, we really should be closing the
29782980
// channel here as we apparently can't relay through them anyway.
29792981
self.pending_events.lock().unwrap().push(
2980-
events::Event::PaymentFailed {
2982+
events::Event::PaymentPathFailed {
29812983
payment_hash: payment_hash.clone(),
29822984
rejected_by_dest: path.len() == 1,
29832985
network_update: None,
29842986
all_paths_failed,
2987+
path: path.clone(),
29852988
#[cfg(test)]
29862989
error_code: Some(*failure_code),
29872990
#[cfg(test)]

‎lightning/src/ln/functional_test_utils.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,7 @@ macro_rules! expect_payment_failed_with_update {
10731073
let events = $node.node.get_and_clear_pending_events();
10741074
assert_eq!(events.len(), 1);
10751075
match events[0] {
1076-
Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => {
1076+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => {
10771077
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
10781078
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
10791079
assert!(error_code.is_some(), "expected error_code.is_some() = true");
@@ -1102,7 +1102,7 @@ macro_rules! expect_payment_failed {
11021102
let events = $node.node.get_and_clear_pending_events();
11031103
assert_eq!(events.len(), 1);
11041104
match events[0] {
1105-
Event::PaymentFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => {
1105+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => {
11061106
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
11071107
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
11081108
assert!(error_code.is_some(), "expected error_code.is_some() = true");
@@ -1399,10 +1399,13 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
13991399
let events = origin_node.node.get_and_clear_pending_events();
14001400
assert_eq!(events.len(), 1);
14011401
match events[0] {
1402-
Event::PaymentFailed { payment_hash, rejected_by_dest, all_paths_failed, .. } => {
1402+
Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, .. } => {
14031403
assert_eq!(payment_hash, our_payment_hash);
14041404
assert!(rejected_by_dest);
14051405
assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
1406+
for (idx, hop) in expected_route.iter().enumerate() {
1407+
assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
1408+
}
14061409
},
14071410
_ => panic!("Unexpected event"),
14081411
}

‎lightning/src/ln/functional_tests.rs

+15-15
Original file line numberDiff line numberDiff line change
@@ -3020,7 +3020,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
30203020
_ => panic!("Unexepected event"),
30213021
}
30223022
match events[1] {
3023-
Event::PaymentFailed { ref payment_hash, .. } => {
3023+
Event::PaymentPathFailed { ref payment_hash, .. } => {
30243024
assert_eq!(*payment_hash, fourth_payment_hash);
30253025
},
30263026
_ => panic!("Unexpected event"),
@@ -3076,7 +3076,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
30763076
let events = nodes[0].node.get_and_clear_pending_events();
30773077
assert_eq!(events.len(), 3);
30783078
match events[0] {
3079-
Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
3079+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
30803080
assert!(failed_htlcs.insert(payment_hash.0));
30813081
// If we delivered B's RAA we got an unknown preimage error, not something
30823082
// that we should update our routing table for.
@@ -3087,14 +3087,14 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
30873087
_ => panic!("Unexpected event"),
30883088
}
30893089
match events[1] {
3090-
Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
3090+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
30913091
assert!(failed_htlcs.insert(payment_hash.0));
30923092
assert!(network_update.is_some());
30933093
},
30943094
_ => panic!("Unexpected event"),
30953095
}
30963096
match events[2] {
3097-
Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
3097+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
30983098
assert!(failed_htlcs.insert(payment_hash.0));
30993099
assert!(network_update.is_some());
31003100
},
@@ -3190,7 +3190,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
31903190
assert_eq!(events.len(), 2);
31913191
// Check that Alice fails backward the pending HTLC from the second payment.
31923192
match events[0] {
3193-
Event::PaymentFailed { payment_hash, .. } => {
3193+
Event::PaymentPathFailed { payment_hash, .. } => {
31943194
assert_eq!(payment_hash, failed_payment_hash);
31953195
},
31963196
_ => panic!("Unexpected event"),
@@ -3392,7 +3392,7 @@ fn test_simple_peer_disconnect() {
33923392
_ => panic!("Unexpected event"),
33933393
}
33943394
match events[1] {
3395-
Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
3395+
Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } => {
33963396
assert_eq!(payment_hash, payment_hash_5);
33973397
assert!(rejected_by_dest);
33983398
},
@@ -4192,7 +4192,7 @@ fn test_dup_htlc_onchain_fails_on_reload() {
41924192
//
41934193
// If, due to an on-chain event, an HTLC is failed/claimed, and then we serialize the
41944194
// ChannelManager, we generally expect there not to be a duplicate HTLC fail/claim (eg via a
4195-
// PaymentFailed event appearing). However, because we may not serialize the relevant
4195+
// PaymentPathFailed event appearing). However, because we may not serialize the relevant
41964196
// ChannelMonitor at the same time, this isn't strictly guaranteed. In order to provide this
41974197
// consistency, the ChannelManager explicitly tracks pending-onchain-resolution outbound HTLCs
41984198
// and de-duplicates ChannelMonitor events.
@@ -5518,7 +5518,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
55185518
let mut as_failds = HashSet::new();
55195519
let mut as_updates = 0;
55205520
for event in as_events.iter() {
5521-
if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
5521+
if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
55225522
assert!(as_failds.insert(*payment_hash));
55235523
if *payment_hash != payment_hash_2 {
55245524
assert_eq!(*rejected_by_dest, deliver_last_raa);
@@ -5543,7 +5543,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
55435543
let mut bs_failds = HashSet::new();
55445544
let mut bs_updates = 0;
55455545
for event in bs_events.iter() {
5546-
if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
5546+
if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
55475547
assert!(bs_failds.insert(*payment_hash));
55485548
if *payment_hash != payment_hash_1 && *payment_hash != payment_hash_5 {
55495549
assert_eq!(*rejected_by_dest, deliver_last_raa);
@@ -6068,7 +6068,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
60686068
let events = nodes[0].node.get_and_clear_pending_events();
60696069
assert_eq!(events.len(), 1);
60706070
match &events[0] {
6071-
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
6071+
&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed, path: _ } => {
60726072
assert_eq!(our_payment_hash.clone(), *payment_hash);
60736073
assert_eq!(*rejected_by_dest, false);
60746074
assert_eq!(*all_paths_failed, true);
@@ -6155,7 +6155,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
61556155
let events = nodes[0].node.get_and_clear_pending_events();
61566156
assert_eq!(events.len(), 1);
61576157
match &events[0] {
6158-
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
6158+
&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed, path: _ } => {
61596159
assert_eq!(payment_hash_2.clone(), *payment_hash);
61606160
assert_eq!(*rejected_by_dest, false);
61616161
assert_eq!(*all_paths_failed, true);
@@ -7107,12 +7107,12 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
71077107
assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
71087108
connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
71097109
let events = nodes[0].node.get_and_clear_pending_events();
7110-
// Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx
7110+
// Only 2 PaymentPathFailed events should show up, over-dust HTLC has to be failed by timeout tx
71117111
assert_eq!(events.len(), 2);
71127112
let mut first_failed = false;
71137113
for event in events {
71147114
match event {
7115-
Event::PaymentFailed { payment_hash, .. } => {
7115+
Event::PaymentPathFailed { payment_hash, .. } => {
71167116
if payment_hash == payment_hash_1 {
71177117
assert!(!first_failed);
71187118
first_failed = true;
@@ -7202,14 +7202,14 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
72027202
assert_eq!(events.len(), 2);
72037203
let first;
72047204
match events[0] {
7205-
Event::PaymentFailed { payment_hash, .. } => {
7205+
Event::PaymentPathFailed { payment_hash, .. } => {
72067206
if payment_hash == dust_hash { first = true; }
72077207
else { first = false; }
72087208
},
72097209
_ => panic!("Unexpected event"),
72107210
}
72117211
match events[1] {
7212-
Event::PaymentFailed { payment_hash, .. } => {
7212+
Event::PaymentPathFailed { payment_hash, .. } => {
72137213
if first { assert_eq!(payment_hash, non_dust_hash); }
72147214
else { assert_eq!(payment_hash, dust_hash); }
72157215
},

‎lightning/src/ln/onion_route_tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
163163

164164
let events = nodes[0].node.get_and_clear_pending_events();
165165
assert_eq!(events.len(), 1);
166-
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed } = &events[0] {
166+
if let &Event::PaymentPathFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed, path: _ } = &events[0] {
167167
assert_eq!(*rejected_by_dest, !expected_retryable);
168168
assert_eq!(*all_paths_failed, true);
169169
assert_eq!(*error_code, expected_error_code);

‎lightning/src/routing/network_graph.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
113113
impl<C: Deref, L: Deref> EventHandler for NetGraphMsgHandler<C, L>
114114
where C::Target: chain::Access, L::Target: Logger {
115115
fn handle_event(&self, event: &Event) {
116-
if let Event::PaymentFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
116+
if let Event::PaymentPathFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
117117
if let Some(network_update) = network_update {
118118
self.handle_network_update(network_update);
119119
}
@@ -127,7 +127,7 @@ where C::Target: chain::Access, L::Target: Logger {
127127
/// Provides interface to help with initial routing sync by
128128
/// serving historical announcements.
129129
///
130-
/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentFailed`] to the
130+
/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the
131131
/// [`NetworkGraph`].
132132
pub struct NetGraphMsgHandler<C: Deref, L: Deref>
133133
where C::Target: chain::Access, L::Target: Logger
@@ -1725,10 +1725,11 @@ mod tests {
17251725

17261726
assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
17271727

1728-
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
1728+
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
17291729
payment_hash: PaymentHash([0; 32]),
17301730
rejected_by_dest: false,
17311731
all_paths_failed: true,
1732+
path: vec![],
17321733
network_update: Some(NetworkUpdate::ChannelUpdateMessage {
17331734
msg: valid_channel_update,
17341735
}),
@@ -1748,10 +1749,11 @@ mod tests {
17481749
}
17491750
};
17501751

1751-
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
1752+
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
17521753
payment_hash: PaymentHash([0; 32]),
17531754
rejected_by_dest: false,
17541755
all_paths_failed: true,
1756+
path: vec![],
17551757
network_update: Some(NetworkUpdate::ChannelClosed {
17561758
short_channel_id,
17571759
is_permanent: false,
@@ -1770,10 +1772,11 @@ mod tests {
17701772

17711773
// Permanent closing deletes a channel
17721774
{
1773-
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
1775+
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
17741776
payment_hash: PaymentHash([0; 32]),
17751777
rejected_by_dest: false,
17761778
all_paths_failed: true,
1779+
path: vec![],
17771780
network_update: Some(NetworkUpdate::ChannelClosed {
17781781
short_channel_id,
17791782
is_permanent: true,

‎lightning/src/routing/router.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use core::cmp;
2828
use core::ops::Deref;
2929

3030
/// A hop in a route
31-
#[derive(Clone, Hash, PartialEq, Eq)]
31+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
3232
pub struct RouteHop {
3333
/// The node_id of the node at this hop.
3434
pub pubkey: PublicKey,

0 commit comments

Comments
 (0)