Skip to content

Commit aa582f0

Browse files
valentinewallacevalentinewallace
committed
Rename PaymentFailed -> PaymentPathFailed
Since we don't want to imply to users that a payment has completely failed when it really has just partially failed
1 parent 2cf42aa commit aa582f0

8 files changed

+37
-37
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

+4-4
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
@@ -2882,7 +2882,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
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,
@@ -2951,7 +2951,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
29512951
// process_onion_failure we should close that channel as it implies our
29522952
// next-hop is needlessly blaming us!
29532953
self.pending_events.lock().unwrap().push(
2954-
events::Event::PaymentFailed {
2954+
events::Event::PaymentPathFailed {
29552955
payment_hash: payment_hash.clone(),
29562956
rejected_by_dest: !payment_retryable,
29572957
network_update,
@@ -2977,7 +2977,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
29772977
// TODO: For non-temporary failures, we really should be closing the
29782978
// channel here as we apparently can't relay through them anyway.
29792979
self.pending_events.lock().unwrap().push(
2980-
events::Event::PaymentFailed {
2980+
events::Event::PaymentPathFailed {
29812981
payment_hash: payment_hash.clone(),
29822982
rejected_by_dest: path.len() == 1,
29832983
network_update: None,

‎lightning/src/ln/functional_test_utils.rs

+3-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,7 +1399,7 @@ 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, .. } => {
14031403
assert_eq!(payment_hash, our_payment_hash);
14041404
assert!(rejected_by_dest);
14051405
assert_eq!(all_paths_failed, i == expected_paths.len() - 1);

‎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 } => {
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 } => {
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 } = &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

+5-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,7 +1725,7 @@ 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,
@@ -1748,7 +1748,7 @@ mod tests {
17481748
}
17491749
};
17501750

1751-
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
1751+
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
17521752
payment_hash: PaymentHash([0; 32]),
17531753
rejected_by_dest: false,
17541754
all_paths_failed: true,
@@ -1770,7 +1770,7 @@ mod tests {
17701770

17711771
// Permanent closing deletes a channel
17721772
{
1773-
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
1773+
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
17741774
payment_hash: PaymentHash([0; 32]),
17751775
rejected_by_dest: false,
17761776
all_paths_failed: true,

‎lightning/src/util/events.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ pub enum Event {
170170
/// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target
171171
/// and we got back the payment preimage for it).
172172
///
173-
/// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentFailed` event.
174-
/// In this situation, you SHOULD treat this payment as having succeeded.
173+
/// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentPathFailed`
174+
/// event. In this situation, you SHOULD treat this payment as having succeeded.
175175
PaymentSent {
176176
/// The preimage to the hash given to ChannelManager::send_payment.
177177
/// Note that this serves as a payment receipt, if you wish to have such a thing, you must
@@ -180,7 +180,7 @@ pub enum Event {
180180
},
181181
/// Indicates an outbound payment we made failed. Probably some intermediary node dropped
182182
/// something. You may wish to retry with a different route.
183-
PaymentFailed {
183+
PaymentPathFailed {
184184
/// The hash which was given to ChannelManager::send_payment.
185185
payment_hash: PaymentHash,
186186
/// Indicates the payment was rejected for some reason by the recipient. This implies that
@@ -291,7 +291,7 @@ impl Writeable for Event {
291291
(0, payment_preimage, required),
292292
});
293293
},
294-
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed,
294+
&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed,
295295
#[cfg(test)]
296296
ref error_code,
297297
#[cfg(test)]
@@ -404,7 +404,7 @@ impl MaybeReadable for Event {
404404
(2, rejected_by_dest, required),
405405
(3, all_paths_failed, option),
406406
});
407-
Ok(Some(Event::PaymentFailed {
407+
Ok(Some(Event::PaymentPathFailed {
408408
payment_hash,
409409
rejected_by_dest,
410410
network_update,

0 commit comments

Comments
 (0)