Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -283,12 +283,13 @@ fn check_payment_err(send_err: PaymentSendFailure) {
PaymentSendFailure::PathParameterError(per_path_results) => {
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
for api_err in per_path_results { check_api_err(api_err); }
},
PaymentSendFailure::PartialFailure { results, .. } => {
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
},
PaymentSendFailure::DuplicatePayment => panic!(),
}
}

Expand Down
9 changes: 7 additions & 2 deletions lightning-invoice/src/payment.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,7 +543,8 @@ where
Err(e) => match e {
PaymentSendFailure::ParameterError(_) => Err(e),
PaymentSendFailure::PathParameterError(_) => Err(e),
PaymentSendFailure::AllFailedRetrySafe(_) => {
PaymentSendFailure::DuplicatePayment => Err(e),
PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
payment_info.attempts.count += 1;
Expand DownExpand Up@@ -655,9 +656,13 @@ where
log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
Err(())
},
Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
self.retry_payment(payment_id, payment_hash, params)
},
Err(PaymentSendFailure::DuplicatePayment) => {
log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
Err(())
}
Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
// If a `PartialFailure` error contains a result that is an `Ok()`, it means that
// part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
Expand Down
40 changes: 27 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1188,24 +1188,40 @@ impl ChannelDetails {
#[derive(Clone, Debug)]
pub enum PaymentSendFailure {
/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
/// send the payment at all. No channel state has been changed or messages sent to peers, and
/// once you've changed the parameter at error, you can freely retry the payment in full.
/// send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
ParameterError(APIError),
/// A parameter in a single path which was passed to send_payment was invalid, preventing us
/// from attempting to send the payment at all. No channel state has been changed or messages
/// sent to peers, and once you've changed the parameter at error, you can freely retry the
/// payment in full.
/// from attempting to send the payment at all.
///
/// You can freely resend the payment in full (with the parameter error fixed).
///
/// The results here are ordered the same as the paths in the route object which was passed to
/// send_payment.
///
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
PathParameterError(Vec<Result<(), APIError>>),
/// All paths which were attempted failed to send, with no channel state change taking place.
/// You can freely retry the payment in full (though you probably want to do so over different
/// You can freely resend the payment in full (though you probably want to do so over different
Comment thread
TheBlueMatt marked this conversation as resolved.
/// paths than the ones selected).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// paths than the ones selected).
/// paths than the ones previously selected).

@TheBlueMattTheBlueMattNov 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather replace selected with attempted, but not sure its worth it - I don't think anyone is gonna be confused here.

///
/// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
/// [`ChannelManager::retry_payment`] will *not* work for this payment.
AllFailedRetrySafe(Vec<APIError>),
/// Because the payment failed outright, no payment tracking is done, you do not need to call
/// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
/// for this payment.
AllFailedResendSafe(Vec<APIError>),
/// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
/// yet completed (i.e. generated an [`Event::PaymentSent`]) or has been abandoned (via

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - "not yet completed or been abandoned" has the opposite meaning from "not yet completed or has been abandoned" :).

/// [`ChannelManager::abandon_payment`]).
///
/// [`Event::PaymentSent`]: events::Event::PaymentSent
DuplicatePayment,
/// Some paths which were attempted failed to send, though possibly not all. At least some
/// paths have irrevocably committed to the HTLC and retrying the payment in full would result
/// in over-/re-payment.
Expand DownExpand Up@@ -2610,9 +2626,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
match pending_outbounds.entry(payment_id) {
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already in progress"
})),
hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
hash_map::Entry::Vacant(entry) => {
let payment = entry.insert(PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
Expand DownExpand Up@@ -2726,7 +2740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
// `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
debug_assert!(removed, "We should always have a pending payment to remove here");
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -587,7 +587,7 @@ macro_rules! get_local_commitment_txn {
macro_rules! unwrap_send_err {
($res: expr, $all_failed: expr, $type: pat, $check: expr) => {
match &$res {
&Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => {
&Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => {
assert_eq!(fails.len(), 1);
match fails[0] {
$type => { $check },
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1333,7 +1333,7 @@ fn test_basic_channel_reserve() {
let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
let err = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
match err {
PaymentSendFailure::AllFailedRetrySafe(ref fails) => {
PaymentSendFailure::AllFailedResendSafe(ref fails) => {
match &fails[0] {
&APIError::ChannelUnavailable{ref err} =>
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)),
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/payment_tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -1275,15 +1275,15 @@ fn claimed_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

// Further, if we try to send a spontaneous payment with the same payment_id it should
// also be rejected.
let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}
}
Expand DownExpand Up@@ -1347,15 +1347,15 @@ fn abandoned_send_payment_idempotent() {
// payment_id, it should be rejected.
let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
match send_result {
Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
Err(PaymentSendFailure::DuplicatePayment) => {},
_ => panic!("Unexpected send result: {:?}", send_result),
}

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