Skip to content

Commit c7c6f51

Browse files
jkczyzjkczyz
committed
Make payment_hash optional in Event::PaymentFailed
When abandoning a BOLT12 payment before a Bolt12Invoice is received, an Event::InvoiceRequestFailed is generated and the abandonment reason is lost. Make payment_hash optional in Event::PaymentFailed so that Event::InvoiceRequestFailed can be removed in favor of it.
1 parent 1fdabcc commit c7c6f51

10 files changed

+43
-29
lines changed

‎lightning/src/events/mod.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -879,10 +879,12 @@ pub enum Event {
879879
///
880880
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
881881
payment_id: PaymentId,
882-
/// The hash that was given to [`ChannelManager::send_payment`].
882+
/// The hash that was given to [`ChannelManager::send_payment`]. `None` if the payment failed
883+
/// before receiving an invoice when paying a BOLT12 [`Offer`].
883884
///
884885
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
885-
payment_hash: PaymentHash,
886+
/// [`Offer`]: crate::offers::offer::Offer
887+
payment_hash: Option<PaymentHash>,
886888
/// The reason the payment failed. This is only `None` for events generated or serialized
887889
/// by versions prior to 0.0.115.
888890
reason: Option<PaymentFailureReason>,
@@ -1555,10 +1557,15 @@ impl Writeable for Event {
15551557
},
15561558
&Event::PaymentFailed { ref payment_id, ref payment_hash, ref reason } => {
15571559
15u8.write(writer)?;
1560+
let (payment_hash, invoice_received) = match payment_hash {
1561+
Some(payment_hash) => (payment_hash, true),
1562+
None => (&PaymentHash([0; 32]), false),
1563+
};
15581564
write_tlv_fields!(writer, {
15591565
(0, payment_id, required),
15601566
(1, reason, option),
15611567
(2, payment_hash, required),
1568+
(3, invoice_received, required),
15621569
})
15631570
},
15641571
&Event::OpenChannelRequest { .. } => {
@@ -1932,11 +1939,17 @@ impl MaybeReadable for Event {
19321939
let mut payment_hash = PaymentHash([0; 32]);
19331940
let mut payment_id = PaymentId([0; 32]);
19341941
let mut reason = None;
1942+
let mut invoice_received: Option<bool> = None;
19351943
read_tlv_fields!(reader, {
19361944
(0, payment_id, required),
19371945
(1, reason, upgradable_option),
19381946
(2, payment_hash, required),
1947+
(3, invoice_received, option),
19391948
});
1949+
let payment_hash = match invoice_received {
1950+
Some(invoice_received) => invoice_received.then(|| payment_hash),
1951+
None => (payment_hash != PaymentHash([0; 32])).then(|| payment_hash),
1952+
};
19401953
Ok(Some(Event::PaymentFailed {
19411954
payment_id,
19421955
payment_hash,

‎lightning/src/ln/blinded_payment_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ fn blinded_path_retries() {
10681068
assert_eq!(evs.len(), 1);
10691069
match evs[0] {
10701070
Event::PaymentFailed { payment_hash: ev_payment_hash, reason, .. } => {
1071-
assert_eq!(ev_payment_hash, payment_hash);
1071+
assert_eq!(ev_payment_hash, Some(payment_hash));
10721072
// We have 1 retry attempt remaining, but we're out of blinded paths to try.
10731073
assert_eq!(reason, Some(PaymentFailureReason::RouteNotFound));
10741074
},

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1776,7 +1776,7 @@ fn test_monitor_update_on_pending_forwards() {
17761776
} else { panic!("Unexpected event!"); }
17771777
match events[2] {
17781778
Event::PaymentFailed { payment_hash, .. } => {
1779-
assert_eq!(payment_hash, payment_hash_1);
1779+
assert_eq!(payment_hash, Some(payment_hash_1));
17801780
},
17811781
_ => panic!("Unexpected event"),
17821782
}

‎lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1681,7 +1681,8 @@ where
16811681
/// channel_manager.process_pending_events(&|event| {
16821682
/// match event {
16831683
/// Event::PaymentSent { payment_hash, .. } => println!("Paid {}", payment_hash),
1684-
/// Event::PaymentFailed { payment_hash, .. } => println!("Failed paying {}", payment_hash),
1684+
/// Event::PaymentFailed { payment_hash: Some(payment_hash), .. } =>
1685+
/// println!("Failed paying {}", payment_hash),
16851686
/// // ...
16861687
/// # _ => {},
16871688
/// }

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,7 +2520,7 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
25202520
if !conditions.expected_mpp_parts_remain {
25212521
match &payment_failed_events[1] {
25222522
Event::PaymentFailed { ref payment_hash, ref payment_id, ref reason } => {
2523-
assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash");
2523+
assert_eq!(*payment_hash, Some(expected_payment_hash), "unexpected second payment_hash");
25242524
assert_eq!(*payment_id, expected_payment_id);
25252525
assert_eq!(reason.unwrap(), if expected_payment_failed_permanently {
25262526
PaymentFailureReason::RecipientRejected
@@ -3162,7 +3162,7 @@ pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
31623162
if i == expected_paths.len() - 1 {
31633163
match events[1] {
31643164
Event::PaymentFailed { ref payment_hash, ref payment_id, ref reason } => {
3165-
assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
3165+
assert_eq!(*payment_hash, Some(our_payment_hash), "unexpected second payment_hash");
31663166
assert_eq!(*payment_id, expected_payment_id);
31673167
assert_eq!(reason.unwrap(), expected_fail_reason);
31683168
}

‎lightning/src/ln/functional_tests.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3380,7 +3380,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
33803380
)));
33813381
assert!(events.iter().any(|ev| matches!(
33823382
ev,
3383-
Event::PaymentFailed { ref payment_hash, .. } if *payment_hash == fourth_payment_hash
3383+
Event::PaymentFailed { ref payment_hash, .. } if *payment_hash == Some(fourth_payment_hash)
33843384
)));
33853385

33863386
nodes[1].node.process_pending_htlc_forwards();
@@ -3442,7 +3442,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
34423442
}
34433443
match events[1] {
34443444
Event::PaymentFailed { ref payment_hash, .. } => {
3445-
assert_eq!(*payment_hash, first_payment_hash);
3445+
assert_eq!(*payment_hash, Some(first_payment_hash));
34463446
},
34473447
_ => panic!("Unexpected event"),
34483448
}
@@ -3454,7 +3454,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
34543454
}
34553455
match events[3] {
34563456
Event::PaymentFailed { ref payment_hash, .. } => {
3457-
assert_eq!(*payment_hash, second_payment_hash);
3457+
assert_eq!(*payment_hash, Some(second_payment_hash));
34583458
},
34593459
_ => panic!("Unexpected event"),
34603460
}
@@ -3466,7 +3466,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
34663466
}
34673467
match events[5] {
34683468
Event::PaymentFailed { ref payment_hash, .. } => {
3469-
assert_eq!(*payment_hash, third_payment_hash);
3469+
assert_eq!(*payment_hash, Some(third_payment_hash));
34703470
},
34713471
_ => panic!("Unexpected event"),
34723472
}
@@ -3572,7 +3572,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
35723572
}
35733573
match events[1] {
35743574
Event::PaymentFailed { payment_hash, .. } => {
3575-
assert_eq!(payment_hash, failed_payment_hash);
3575+
assert_eq!(payment_hash, Some(failed_payment_hash));
35763576
},
35773577
_ => panic!("Unexpected event"),
35783578
}
@@ -3851,7 +3851,7 @@ fn test_simple_peer_disconnect() {
38513851
}
38523852
match events[3] {
38533853
Event::PaymentFailed { payment_hash, .. } => {
3854-
assert_eq!(payment_hash, payment_hash_5);
3854+
assert_eq!(payment_hash, Some(payment_hash_5));
38553855
},
38563856
_ => panic!("Unexpected event"),
38573857
}
@@ -6007,7 +6007,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
60076007
}
60086008
match &events[1] {
60096009
&Event::PaymentFailed { ref payment_hash, .. } => {
6010-
assert_eq!(our_payment_hash.clone(), *payment_hash);
6010+
assert_eq!(Some(our_payment_hash), *payment_hash);
60116011
},
60126012
_ => panic!("Unexpected event"),
60136013
}
@@ -6095,7 +6095,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
60956095
}
60966096
match &events[1] {
60976097
&Event::PaymentFailed { ref payment_hash, .. } => {
6098-
assert_eq!(payment_hash_2.clone(), *payment_hash);
6098+
assert_eq!(Some(payment_hash_2), *payment_hash);
60996099
},
61006100
_ => panic!("Unexpected event"),
61016101
}
@@ -7048,7 +7048,7 @@ fn test_channel_failed_after_message_with_badonion_node_perm_bits_set() {
70487048
}
70497049
match events_5[1] {
70507050
Event::PaymentFailed { payment_hash, .. } => {
7051-
assert_eq!(payment_hash, our_payment_hash);
7051+
assert_eq!(payment_hash, Some(our_payment_hash));
70527052
},
70537053
_ => panic!("Unexpected event"),
70547054
}

‎lightning/src/ln/offers_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2168,7 +2168,7 @@ fn fails_paying_invoice_with_unknown_required_features() {
21682168
match get_event!(david, Event::PaymentFailed) {
21692169
Event::PaymentFailed {
21702170
payment_id: event_payment_id,
2171-
payment_hash: event_payment_hash,
2171+
payment_hash: Some(event_payment_hash),
21722172
reason: Some(event_reason),
21732173
} => {
21742174
assert_eq!(event_payment_id, payment_id);

‎lightning/src/ln/onion_route_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(
215215
}
216216
match events[1] {
217217
Event::PaymentFailed { payment_hash: ev_payment_hash, payment_id: ev_payment_id, reason: ref ev_reason } => {
218-
assert_eq!(*payment_hash, ev_payment_hash);
218+
assert_eq!(Some(*payment_hash), ev_payment_hash);
219219
assert_eq!(payment_id, ev_payment_id);
220220
assert_eq!(if expected_retryable {
221221
PaymentFailureReason::RetriesExhausted

‎lightning/src/ln/outbound_payment.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,7 @@ impl OutboundPayments {
961961
if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = pmt {
962962
pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
963963
payment_id: *pmt_id,
964-
payment_hash: *payment_hash,
964+
payment_hash: Some(*payment_hash),
965965
reason: *reason,
966966
}, None));
967967
retain = false;
@@ -1127,7 +1127,7 @@ impl OutboundPayments {
11271127
if $payment.get().remaining_parts() == 0 {
11281128
pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
11291129
payment_id,
1130-
payment_hash,
1130+
payment_hash: Some(payment_hash),
11311131
reason: *reason,
11321132
}, None));
11331133
$payment.remove();
@@ -1795,7 +1795,7 @@ impl OutboundPayments {
17951795
if !payment_is_probe {
17961796
full_failure_ev = Some(events::Event::PaymentFailed {
17971797
payment_id: *payment_id,
1798-
payment_hash: *payment_hash,
1798+
payment_hash: Some(*payment_hash),
17991799
reason: *reason,
18001800
});
18011801
}
@@ -1864,7 +1864,7 @@ impl OutboundPayments {
18641864
if payment.get().remaining_parts() == 0 {
18651865
pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
18661866
payment_id,
1867-
payment_hash: *payment_hash,
1867+
payment_hash: Some(*payment_hash),
18681868
reason: *reason,
18691869
}, None));
18701870
payment.remove();
@@ -2337,7 +2337,7 @@ mod tests {
23372337
);
23382338
assert!(!outbound_payments.has_pending_payments());
23392339

2340-
let payment_hash = invoice.payment_hash();
2340+
let payment_hash = Some(invoice.payment_hash());
23412341
let reason = Some(PaymentFailureReason::PaymentExpired);
23422342

23432343
assert!(!pending_events.lock().unwrap().is_empty());
@@ -2398,7 +2398,7 @@ mod tests {
23982398
);
23992399
assert!(!outbound_payments.has_pending_payments());
24002400

2401-
let payment_hash = invoice.payment_hash();
2401+
let payment_hash = Some(invoice.payment_hash());
24022402
let reason = Some(PaymentFailureReason::RouteNotFound);
24032403

24042404
assert!(!pending_events.lock().unwrap().is_empty());

‎lightning/src/ln/payment_tests.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2267,7 +2267,7 @@ fn do_automatic_retries(test: AutoRetry) {
22672267
} else {
22682268
match events[1] {
22692269
Event::PaymentFailed { payment_hash: ev_payment_hash, .. } => {
2270-
assert_eq!(payment_hash, ev_payment_hash);
2270+
assert_eq!(Some(payment_hash), ev_payment_hash);
22712271
},
22722272
_ => panic!("Unexpected event"),
22732273
}
@@ -2350,7 +2350,7 @@ fn do_automatic_retries(test: AutoRetry) {
23502350
assert_eq!(events.len(), 1);
23512351
match events[0] {
23522352
Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
2353-
assert_eq!(payment_hash, *ev_payment_hash);
2353+
assert_eq!(Some(payment_hash), *ev_payment_hash);
23542354
assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
23552355
assert_eq!(PaymentFailureReason::RetriesExhausted, ev_reason.unwrap());
23562356
},
@@ -2387,7 +2387,7 @@ fn do_automatic_retries(test: AutoRetry) {
23872387
assert_eq!(events.len(), 1);
23882388
match events[0] {
23892389
Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
2390-
assert_eq!(payment_hash, *ev_payment_hash);
2390+
assert_eq!(Some(payment_hash), *ev_payment_hash);
23912391
assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
23922392
assert_eq!(PaymentFailureReason::RetriesExhausted, ev_reason.unwrap());
23932393
},
@@ -2408,7 +2408,7 @@ fn do_automatic_retries(test: AutoRetry) {
24082408
assert_eq!(events.len(), 1);
24092409
match events[0] {
24102410
Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
2411-
assert_eq!(payment_hash, *ev_payment_hash);
2411+
assert_eq!(Some(payment_hash), *ev_payment_hash);
24122412
assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
24132413
assert_eq!(PaymentFailureReason::RouteNotFound, ev_reason.unwrap());
24142414
},
@@ -3098,7 +3098,7 @@ fn no_extra_retries_on_back_to_back_fail() {
30983098
}
30993099
match events[1] {
31003100
Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
3101-
assert_eq!(payment_hash, *ev_payment_hash);
3101+
assert_eq!(Some(payment_hash), *ev_payment_hash);
31023102
assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
31033103
assert_eq!(PaymentFailureReason::RetriesExhausted, ev_reason.unwrap());
31043104
},

0 commit comments

Comments
 (0)