Skip to content

Commit 4ba183d

Browse files
committed
Add reason to Event::PaymentFailed
This includes adding a reason to `PendingOutboundPayment::Abandoned` and using that reason when pushing an `Event::PaymentFailed`.
1 parent 432f0e6 commit 4ba183d

File tree

7 files changed

+101
-58
lines changed

7 files changed

+101
-58
lines changed

lightning/src/events/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,9 @@ pub enum Event {
475475
///
476476
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
477477
payment_hash: PaymentHash,
478+
/// The reason the payment failed. This is only `None` for events generated or serialized
479+
/// by versions prior to 0.0.115.
480+
reason: Option<PaymentFailureReason>,
478481
},
479482
/// Indicates that a path for an outbound payment was successful.
480483
///
@@ -940,10 +943,11 @@ impl Writeable for Event {
940943
(4, *path, vec_type)
941944
})
942945
},
943-
&Event::PaymentFailed { ref payment_id, ref payment_hash } => {
946+
&Event::PaymentFailed { ref payment_id, ref payment_hash, ref reason } => {
944947
15u8.write(writer)?;
945948
write_tlv_fields!(writer, {
946949
(0, payment_id, required),
950+
(1, reason, option),
947951
(2, payment_hash, required),
948952
})
949953
},
@@ -1245,13 +1249,16 @@ impl MaybeReadable for Event {
12451249
let f = || {
12461250
let mut payment_hash = PaymentHash([0; 32]);
12471251
let mut payment_id = PaymentId([0; 32]);
1252+
let mut reason = None;
12481253
read_tlv_fields!(reader, {
12491254
(0, payment_id, required),
1255+
(1, reason, upgradable_option),
12501256
(2, payment_hash, required),
12511257
});
12521258
Ok(Some(Event::PaymentFailed {
12531259
payment_id,
12541260
payment_hash,
1261+
reason,
12551262
}))
12561263
};
12571264
f()

lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, Fee
3636
use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
3737
use crate::chain::transaction::{OutPoint, TransactionData};
3838
use crate::events;
39-
use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
39+
use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination, PaymentFailureReason};
4040
// Since this struct is returned in `list_channels` methods, expose it here in case users want to
4141
// construct one themselves.
4242
use crate::ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret};
@@ -2711,7 +2711,7 @@ where
27112711
/// [`Event::PaymentSent`]: events::Event::PaymentSent
27122712
pub fn abandon_payment(&self, payment_id: PaymentId) {
27132713
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
2714-
self.pending_outbound_payments.abandon_payment(payment_id, &self.pending_events);
2714+
self.pending_outbound_payments.abandon_payment(payment_id, PaymentFailureReason::UserAbandoned, &self.pending_events);
27152715
}
27162716

27172717
/// Send a spontaneous payment, which is a payment that does not require the recipient to have

lightning/src/ln/functional_test_utils.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen, Watch, keysinterface::EntropySource};
1414
use crate::chain::channelmonitor::ChannelMonitor;
1515
use crate::chain::transaction::OutPoint;
16-
use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose};
16+
use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason};
1717
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
1818
use crate::ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA};
1919
use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate};
@@ -1937,9 +1937,14 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
19371937
};
19381938
if !conditions.expected_mpp_parts_remain {
19391939
match &payment_failed_events[1] {
1940-
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
1940+
Event::PaymentFailed { ref payment_hash, ref payment_id, ref reason } => {
19411941
assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash");
19421942
assert_eq!(*payment_id, expected_payment_id);
1943+
assert_eq!(reason.unwrap(), if expected_payment_failed_permanently {
1944+
PaymentFailureReason::RecipientRejected
1945+
} else {
1946+
PaymentFailureReason::RetriesExhausted
1947+
});
19431948
}
19441949
_ => panic!("Unexpected second event"),
19451950
}
@@ -2240,10 +2245,10 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
22402245
let expected_destinations: Vec<HTLCDestination> = repeat(HTLCDestination::FailedPayment { payment_hash: our_payment_hash }).take(expected_paths.len()).collect();
22412246
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(expected_paths[0].last().unwrap(), expected_destinations);
22422247

2243-
pass_failed_payment_back(origin_node, expected_paths, skip_last, our_payment_hash);
2248+
pass_failed_payment_back(origin_node, expected_paths, skip_last, our_payment_hash, PaymentFailureReason::RecipientRejected);
22442249
}
22452250

2246-
pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) {
2251+
pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash, expected_fail_reason: PaymentFailureReason) {
22472252
let mut expected_paths: Vec<_> = expected_paths_slice.iter().collect();
22482253
check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
22492254

@@ -2329,9 +2334,10 @@ pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
23292334
};
23302335
if i == expected_paths.len() - 1 {
23312336
match events[1] {
2332-
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
2337+
Event::PaymentFailed { ref payment_hash, ref payment_id, ref reason } => {
23332338
assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
23342339
assert_eq!(*payment_id, expected_payment_id);
2340+
assert_eq!(reason.unwrap(), expected_fail_reason);
23352341
}
23362342
_ => panic!("Unexpected second event"),
23372343
}

lightning/src/ln/functional_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::chain::channelmonitor;
1818
use crate::chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
1919
use crate::chain::transaction::OutPoint;
2020
use crate::chain::keysinterface::{ChannelSigner, EcdsaChannelSigner, EntropySource};
21-
use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination};
21+
use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason};
2222
use crate::ln::{PaymentPreimage, PaymentSecret, PaymentHash};
2323
use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT};
2424
use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
@@ -9608,7 +9608,7 @@ fn test_double_partial_claim() {
96089608
];
96099609
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], failed_destinations);
96109610

9611-
pass_failed_payment_back(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_hash);
9611+
pass_failed_payment_back(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_hash, PaymentFailureReason::RecipientRejected);
96129612

96139613
// nodes[1] now retries one of the two paths...
96149614
nodes[0].node.send_payment_with_route(&route, payment_hash,

lightning/src/ln/onion_route_tests.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
1414
use crate::chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS};
1515
use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient};
16-
use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure};
16+
use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason};
1717
use crate::ln::{PaymentHash, PaymentSecret};
1818
use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS;
1919
use crate::ln::channelmanager::{HTLCForwardInfo, FailureCode, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingAddHTLCInfo, PendingHTLCInfo, PendingHTLCRouting, PaymentId, RecipientOnionFields};
@@ -213,9 +213,14 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
213213
panic!("Unexpected event");
214214
}
215215
match events[1] {
216-
Event::PaymentFailed { payment_hash: ev_payment_hash, payment_id: ev_payment_id } => {
216+
Event::PaymentFailed { payment_hash: ev_payment_hash, payment_id: ev_payment_id, reason: ref ev_reason } => {
217217
assert_eq!(*payment_hash, ev_payment_hash);
218218
assert_eq!(payment_id, ev_payment_id);
219+
assert_eq!(if expected_retryable {
220+
PaymentFailureReason::RetriesExhausted
221+
} else {
222+
PaymentFailureReason::RecipientRejected
223+
}, ev_reason.unwrap());
219224
}
220225
_ => panic!("Unexpected second event"),
221226
}

lightning/src/ln/outbound_payment.rs

Lines changed: 59 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use bitcoin::hashes::sha256::Hash as Sha256;
1414
use bitcoin::secp256k1::{self, Secp256k1, SecretKey};
1515

1616
use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient};
17-
use crate::events;
17+
use crate::events::{self, PaymentFailureReason};
1818
use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
1919
use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId};
2020
use crate::ln::onion_utils::HTLCFailReason;
@@ -68,6 +68,8 @@ pub(crate) enum PendingOutboundPayment {
6868
Abandoned {
6969
session_privs: HashSet<[u8; 32]>,
7070
payment_hash: PaymentHash,
71+
/// Will be `None` if the payment was serialized before 0.0.115.
72+
reason: Option<PaymentFailureReason>,
7173
},
7274
}
7375

@@ -145,21 +147,22 @@ impl PendingOutboundPayment {
145147
*self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash, timer_ticks_without_htlcs: 0 };
146148
}
147149

148-
fn mark_abandoned(&mut self) -> Result<(), ()> {
149-
let mut session_privs = HashSet::new();
150-
let our_payment_hash;
151-
core::mem::swap(&mut session_privs, match self {
150+
fn mark_abandoned(&mut self, reason: PaymentFailureReason) -> Result<(), ()> {
151+
match self {
152152
PendingOutboundPayment::Legacy { .. } |
153-
PendingOutboundPayment::Fulfilled { .. } =>
154-
return Err(()),
155-
PendingOutboundPayment::Retryable { session_privs, payment_hash, .. } |
156-
PendingOutboundPayment::Abandoned { session_privs, payment_hash, .. } => {
157-
our_payment_hash = *payment_hash;
158-
session_privs
153+
PendingOutboundPayment::Fulfilled { .. } => Err(()),
154+
PendingOutboundPayment::Retryable { session_privs, payment_hash, .. } => {
155+
let mut our_session_privs = HashSet::new();
156+
core::mem::swap(&mut our_session_privs, session_privs);
157+
*self = PendingOutboundPayment::Abandoned {
158+
session_privs: our_session_privs,
159+
payment_hash: *payment_hash,
160+
reason: Some(reason)
161+
};
162+
Ok(())
159163
},
160-
});
161-
*self = PendingOutboundPayment::Abandoned { session_privs, payment_hash: our_payment_hash };
162-
Ok(())
164+
PendingOutboundPayment::Abandoned { .. } => Ok(()),
165+
}
163166
}
164167

165168
/// panics if path is None and !self.is_fulfilled
@@ -588,10 +591,12 @@ impl OutboundPayments {
588591
outbounds.retain(|pmt_id, pmt| {
589592
let mut retain = true;
590593
if !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 {
591-
if pmt.mark_abandoned().is_ok() {
594+
let _ = pmt.mark_abandoned(PaymentFailureReason::RetriesExhausted);
595+
if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = pmt {
592596
pending_events.lock().unwrap().push(events::Event::PaymentFailed {
593597
payment_id: *pmt_id,
594-
payment_hash: pmt.payment_hash().expect("PendingOutboundPayments::Retryable always has a payment hash set"),
598+
payment_hash: *payment_hash,
599+
reason: *reason,
595600
});
596601
retain = false;
597602
}
@@ -671,7 +676,7 @@ impl OutboundPayments {
671676
#[cfg(feature = "std")] {
672677
if has_expired(&route_params) {
673678
log_error!(logger, "Payment params expired on retry, abandoning payment {}", log_bytes!(payment_id.0));
674-
self.abandon_payment(payment_id, pending_events);
679+
self.abandon_payment(payment_id, PaymentFailureReason::PaymentExpired, pending_events);
675680
return
676681
}
677682
}
@@ -684,14 +689,14 @@ impl OutboundPayments {
684689
Ok(route) => route,
685690
Err(e) => {
686691
log_error!(logger, "Failed to find a route on retry, abandoning payment {}: {:#?}", log_bytes!(payment_id.0), e);
687-
self.abandon_payment(payment_id, pending_events);
692+
self.abandon_payment(payment_id, PaymentFailureReason::RouteNotFound, pending_events);
688693
return
689694
}
690695
};
691696
for path in route.paths.iter() {
692697
if path.len() == 0 {
693698
log_error!(logger, "length-0 path in route");
694-
self.abandon_payment(payment_id, pending_events);
699+
self.abandon_payment(payment_id, PaymentFailureReason::UnexpectedError, pending_events);
695700
return
696701
}
697702
}
@@ -703,13 +708,17 @@ impl OutboundPayments {
703708
}
704709

705710
macro_rules! abandon_with_entry {
706-
($payment: expr) => {
707-
if $payment.get_mut().mark_abandoned().is_ok() && $payment.get().remaining_parts() == 0 {
708-
pending_events.lock().unwrap().push(events::Event::PaymentFailed {
709-
payment_id,
710-
payment_hash,
711-
});
712-
$payment.remove();
711+
($payment: expr, $reason: expr) => {
712+
let _ = $payment.get_mut().mark_abandoned($reason);
713+
if let PendingOutboundPayment::Abandoned { reason, .. } = $payment.get() {
714+
if $payment.get().remaining_parts() == 0 {
715+
pending_events.lock().unwrap().push(events::Event::PaymentFailed {
716+
payment_id,
717+
payment_hash,
718+
reason: *reason,
719+
});
720+
$payment.remove();
721+
}
713722
}
714723
}
715724
}
@@ -724,7 +733,7 @@ impl OutboundPayments {
724733
let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
725734
if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
726735
log_error!(logger, "retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat);
727-
abandon_with_entry!(payment);
736+
abandon_with_entry!(payment, PaymentFailureReason::UnexpectedError);
728737
return
729738
}
730739
(*total_msat, RecipientOnionFields {
@@ -746,7 +755,7 @@ impl OutboundPayments {
746755
};
747756
if !payment.get().is_retryable_now() {
748757
log_error!(logger, "Retries exhausted for payment id {}", log_bytes!(payment_id.0));
749-
abandon_with_entry!(payment);
758+
abandon_with_entry!(payment, PaymentFailureReason::RetriesExhausted);
750759
return
751760
}
752761
payment.get_mut().increment_attempts();
@@ -803,12 +812,13 @@ impl OutboundPayments {
803812
// initial HTLC-Add messages yet.
804813
},
805814
PaymentSendFailure::PathParameterError(results) => {
815+
log_error!(logger, "Failed to send to route due to parameter error in a single path. Your router is buggy");
806816
Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, results.into_iter(), pending_events);
807-
self.abandon_payment(payment_id, pending_events);
817+
self.abandon_payment(payment_id, PaymentFailureReason::UnexpectedError, pending_events);
808818
},
809819
PaymentSendFailure::ParameterError(e) => {
810820
log_error!(logger, "Failed to send to route due to parameter error: {:?}. Your router is buggy", e);
811-
self.abandon_payment(payment_id, pending_events);
821+
self.abandon_payment(payment_id, PaymentFailureReason::UnexpectedError, pending_events);
812822
},
813823
PaymentSendFailure::DuplicatePayment => debug_assert!(false), // unreachable
814824
}
@@ -1216,15 +1226,21 @@ impl OutboundPayments {
12161226
}
12171227

12181228
if payment_is_probe || !is_retryable_now || !payment_retryable {
1219-
let _ = payment.get_mut().mark_abandoned(); // we'll only Err if it's a legacy payment
1229+
let reason = if !payment_retryable {
1230+
PaymentFailureReason::RecipientRejected
1231+
} else {
1232+
PaymentFailureReason::RetriesExhausted
1233+
};
1234+
let _ = payment.get_mut().mark_abandoned(reason); // we'll only Err if it's a legacy payment
12201235
is_retryable_now = false;
12211236
}
12221237
if payment.get().remaining_parts() == 0 {
1223-
if payment.get().abandoned() {
1238+
if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. }= payment.get() {
12241239
if !payment_is_probe {
12251240
full_failure_ev = Some(events::Event::PaymentFailed {
12261241
payment_id: *payment_id,
1227-
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
1242+
payment_hash: *payment_hash,
1243+
reason: *reason,
12281244
});
12291245
}
12301246
payment.remove();
@@ -1282,15 +1298,17 @@ impl OutboundPayments {
12821298
}
12831299

12841300
pub(super) fn abandon_payment(
1285-
&self, payment_id: PaymentId, pending_events: &Mutex<Vec<events::Event>>
1301+
&self, payment_id: PaymentId, reason: PaymentFailureReason, pending_events: &Mutex<Vec<events::Event>>
12861302
) {
12871303
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
12881304
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
1289-
if let Ok(()) = payment.get_mut().mark_abandoned() {
1305+
let _ = payment.get_mut().mark_abandoned(reason);
1306+
if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = payment.get() {
12901307
if payment.get().remaining_parts() == 0 {
12911308
pending_events.lock().unwrap().push(events::Event::PaymentFailed {
12921309
payment_id,
1293-
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
1310+
payment_hash: *payment_hash,
1311+
reason: *reason,
12941312
});
12951313
payment.remove();
12961314
}
@@ -1352,6 +1370,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
13521370
},
13531371
(3, Abandoned) => {
13541372
(0, session_privs, required),
1373+
(1, reason, option),
13551374
(2, payment_hash, required),
13561375
},
13571376
);
@@ -1361,7 +1380,7 @@ mod tests {
13611380
use bitcoin::network::constants::Network;
13621381
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
13631382

1364-
use crate::events::{Event, PathFailure};
1383+
use crate::events::{Event, PathFailure, PaymentFailureReason};
13651384
use crate::ln::PaymentHash;
13661385
use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
13671386
use crate::ln::features::{ChannelFeatures, NodeFeatures};
@@ -1410,7 +1429,9 @@ mod tests {
14101429
&pending_events, &|_, _, _, _, _, _, _, _| Ok(()));
14111430
let events = pending_events.lock().unwrap();
14121431
assert_eq!(events.len(), 1);
1413-
if let Event::PaymentFailed { .. } = events[0] { } else { panic!("Unexpected event"); }
1432+
if let Event::PaymentFailed { ref reason, .. } = events[0] {
1433+
assert_eq!(reason.unwrap(), PaymentFailureReason::PaymentExpired);
1434+
} else { panic!("Unexpected event"); }
14141435
} else {
14151436
let err = outbound_payments.send_payment(
14161437
PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]),

0 commit comments

Comments
 (0)