Skip to content

Commit 5fa0a6b

Browse files
committed
Add a separate PaymentSendFailure for idempotency violation
When a user attempts to send a payment but it fails due to idempotency key violation, they need to know that this was the reason as they need to handle the error programmatically differently from other errors. Here we simply add a new `PaymentSendFailure` enum variant for `DuplicatePayment` to allow for that.
1 parent 24918f2 commit 5fa0a6b

File tree

3 files changed

+16
-7
lines changed

3 files changed

+16
-7
lines changed

lightning-invoice/src/payment.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ where
543543
Err(e) => match e {
544544
PaymentSendFailure::ParameterError(_) => Err(e),
545545
PaymentSendFailure::PathParameterError(_) => Err(e),
546+
PaymentSendFailure::DuplicatePayment => Err(e),
546547
PaymentSendFailure::AllFailedResendSafe(_) => {
547548
let mut payment_cache = self.payment_cache.lock().unwrap();
548549
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
@@ -658,6 +659,10 @@ where
658659
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
659660
self.retry_payment(payment_id, payment_hash, params)
660661
},
662+
Err(PaymentSendFailure::DuplicatePayment) => {
663+
log_info!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
664+
Err(())
665+
}
661666
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
662667
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
663668
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it

lightning/src/ln/channelmanager.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,12 @@ pub enum PaymentSendFailure {
12071207
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
12081208
/// for this payment.
12091209
AllFailedResendSafe(Vec<APIError>),
1210+
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
1211+
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
1212+
/// [`ChannelManager::abandon_payment`]).
1213+
///
1214+
/// [`Event::PaymentSent`]: events::Event::PaymentSent
1215+
DuplicatePayment,
12101216
/// Some paths which were attempted failed to send, though possibly not all. At least some
12111217
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
12121218
/// in over-/re-payment.
@@ -2611,9 +2617,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
26112617

26122618
let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
26132619
match pending_outbounds.entry(payment_id) {
2614-
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
2615-
err: "Payment already in progress"
2616-
})),
2620+
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
26172621
hash_map::Entry::Vacant(entry) => {
26182622
let payment = entry.insert(PendingOutboundPayment::Retryable {
26192623
session_privs: HashSet::new(),

lightning/src/ln/payment_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
12751275
// payment_id, it should be rejected.
12761276
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
12771277
match send_result {
1278-
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
1278+
Err(PaymentSendFailure::DuplicatePayment) => {},
12791279
_ => panic!("Unexpected send result: {:?}", send_result),
12801280
}
12811281

12821282
// Further, if we try to send a spontaneous payment with the same payment_id it should
12831283
// also be rejected.
12841284
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
12851285
match send_result {
1286-
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
1286+
Err(PaymentSendFailure::DuplicatePayment) => {},
12871287
_ => panic!("Unexpected send result: {:?}", send_result),
12881288
}
12891289
}
@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
13471347
// payment_id, it should be rejected.
13481348
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
13491349
match send_result {
1350-
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
1350+
Err(PaymentSendFailure::DuplicatePayment) => {},
13511351
_ => panic!("Unexpected send result: {:?}", send_result),
13521352
}
13531353

13541354
// Further, if we try to send a spontaneous payment with the same payment_id it should
13551355
// also be rejected.
13561356
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
13571357
match send_result {
1358-
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
1358+
Err(PaymentSendFailure::DuplicatePayment) => {},
13591359
_ => panic!("Unexpected send result: {:?}", send_result),
13601360
}
13611361
}

0 commit comments

Comments
 (0)