Merge pull request #1202 from TheBlueMatt/2021-12-fix-retries-races

Fix payment retry races and inform users when a payment fails
This commit is contained in:
Matt Corallo 2021-12-15 04:58:46 +00:00 committed by GitHub
commit 54114c9d85
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 572 additions and 122 deletions

View file

@ -65,6 +65,7 @@
//! # fn retry_payment(
//! # &self, route: &Route, payment_id: PaymentId
//! # ) -> Result<(), PaymentSendFailure> { unimplemented!() }
//! # fn abandon_payment(&self, payment_id: PaymentId) { unimplemented!() }
//! # }
//! #
//! # struct FakeRouter {}
@ -187,6 +188,9 @@ pub trait Payer {
/// Retries a failed payment path for the [`PaymentId`] using the given [`Route`].
fn retry_payment(&self, route: &Route, payment_id: PaymentId) -> Result<(), PaymentSendFailure>;
/// Signals that no further retries for the given payment will occur.
fn abandon_payment(&self, payment_id: PaymentId);
}
/// A trait defining behavior for routing an [`Invoice`] payment.
@ -462,26 +466,30 @@ where
fn handle_event(&self, event: &Event) {
match event {
Event::PaymentPathFailed {
all_paths_failed, payment_id, payment_hash, rejected_by_dest, path,
short_channel_id, retry, ..
payment_id, payment_hash, rejected_by_dest, path, short_channel_id, retry, ..
} => {
if let Some(short_channel_id) = short_channel_id {
let path = path.iter().collect::<Vec<_>>();
self.scorer.lock().payment_path_failed(&path, *short_channel_id);
}
if *rejected_by_dest {
log_trace!(self.logger, "Payment {} rejected by destination; not retrying", log_bytes!(payment_hash.0));
} else if payment_id.is_none() {
if payment_id.is_none() {
log_trace!(self.logger, "Payment {} has no id; not retrying", log_bytes!(payment_hash.0));
} else if *rejected_by_dest {
log_trace!(self.logger, "Payment {} rejected by destination; not retrying", log_bytes!(payment_hash.0));
self.payer.abandon_payment(payment_id.unwrap());
} else if retry.is_none() {
log_trace!(self.logger, "Payment {} missing retry params; not retrying", log_bytes!(payment_hash.0));
self.payer.abandon_payment(payment_id.unwrap());
} else if self.retry_payment(payment_id.unwrap(), *payment_hash, retry.as_ref().unwrap()).is_ok() {
// We retried at least somewhat, don't provide the PaymentPathFailed event to the user.
return;
} else {
self.payer.abandon_payment(payment_id.unwrap());
}
if *all_paths_failed { self.payment_cache.lock().unwrap().remove(payment_hash); }
},
Event::PaymentFailed { payment_hash, .. } => {
self.remove_cached_payment(&payment_hash);
},
Event::PaymentPathSuccessful { path, .. } => {
let path = path.iter().collect::<Vec<_>>();
@ -511,12 +519,12 @@ mod tests {
use lightning::ln::PaymentPreimage;
use lightning::ln::features::{ChannelFeatures, NodeFeatures, InitFeatures};
use lightning::ln::functional_test_utils::*;
use lightning::ln::msgs::{ErrorAction, LightningError};
use lightning::ln::msgs::{ChannelMessageHandler, ErrorAction, LightningError};
use lightning::routing::network_graph::NodeId;
use lightning::routing::router::{Payee, Route, RouteHop};
use lightning::util::test_utils::TestLogger;
use lightning::util::errors::APIError;
use lightning::util::events::{Event, MessageSendEventsProvider};
use lightning::util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
use secp256k1::{SecretKey, PublicKey, Secp256k1};
use std::cell::RefCell;
use std::collections::VecDeque;
@ -1447,6 +1455,8 @@ mod tests {
self.check_value_msats(Amount::OnRetry(route.get_total_amount()));
self.check_attempts().map(|_| ())
}
fn abandon_payment(&self, _payment_id: PaymentId) { }
}
// *** Full Featured Functional Tests with a Real ChannelManager ***
@ -1569,4 +1579,177 @@ mod tests {
assert_eq!(htlc_msgs.len(), 2);
check_added_monitors!(nodes[0], 2);
}
#[test]
fn no_extra_retries_on_back_to_back_fail() {
// In a previous release, we had a race where we may exceed the payment retry count if we
// get two failures in a row with the second having `all_paths_failed` set.
// Generally, when we give up trying to retry a payment, we don't know for sure what the
// current state of the ChannelManager event queue is. Specifically, we cannot be sure that
// there are not multiple additional `PaymentPathFailed` or even `PaymentSent` events
// pending which we will see later. Thus, when we previously removed the retry tracking map
// entry after a `all_paths_failed` `PaymentPathFailed` event, we may have dropped the
// retry entry even though more events for the same payment were still pending. This led to
// us retrying a payment again even though we'd already given up on it.
//
// We now have a separate event - `PaymentFailed` which indicates no HTLCs remain and which
// is used to remove the payment retry counter entries instead. This tests for the specific
// excess-retry case while also testing `PaymentFailed` generation.
let chanmon_cfgs = create_chanmon_cfgs(3);
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let mut route = Route {
paths: vec![
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
short_channel_id: chan_1_scid,
channel_features: ChannelFeatures::known(),
fee_msat: 0,
cltv_expiry_delta: 100,
}, RouteHop {
pubkey: nodes[2].node.get_our_node_id(),
node_features: NodeFeatures::known(),
short_channel_id: chan_2_scid,
channel_features: ChannelFeatures::known(),
fee_msat: 100_000_000,
cltv_expiry_delta: 100,
}],
vec![RouteHop {
pubkey: nodes[1].node.get_our_node_id(),
node_features: NodeFeatures::known(),
short_channel_id: chan_1_scid,
channel_features: ChannelFeatures::known(),
fee_msat: 0,
cltv_expiry_delta: 100,
}, RouteHop {
pubkey: nodes[2].node.get_our_node_id(),
node_features: NodeFeatures::known(),
short_channel_id: chan_2_scid,
channel_features: ChannelFeatures::known(),
fee_msat: 100_000_000,
cltv_expiry_delta: 100,
}]
],
payee: Some(Payee::from_node_id(nodes[2].node.get_our_node_id())),
};
let router = ManualRouter(RefCell::new(VecDeque::new()));
router.expect_find_route(Ok(route.clone()));
// On retry, we'll only be asked for one path
route.paths.remove(1);
router.expect_find_route(Ok(route.clone()));
let expected_events: RefCell<VecDeque<&dyn Fn(&Event)>> = RefCell::new(VecDeque::new());
let event_handler = |event: &Event| {
let event_checker = expected_events.borrow_mut().pop_front().unwrap();
event_checker(event);
};
let scorer = RefCell::new(TestScorer::new());
let invoice_payer = InvoicePayer::new(nodes[0].node, router, &scorer, nodes[0].logger, event_handler, RetryAttempts(1));
assert!(invoice_payer.pay_invoice(&create_invoice_from_channelmanager(
&nodes[1].node, nodes[1].keys_manager, Currency::Bitcoin, Some(100_010_000), "Invoice".to_string()).unwrap())
.is_ok());
let htlc_updates = SendEvent::from_node(&nodes[0]);
check_added_monitors!(nodes[0], 1);
assert_eq!(htlc_updates.msgs.len(), 1);
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &htlc_updates.msgs[0]);
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &htlc_updates.commitment_msg);
check_added_monitors!(nodes[1], 1);
let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa);
check_added_monitors!(nodes[0], 1);
let second_htlc_updates = SendEvent::from_node(&nodes[0]);
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs);
check_added_monitors!(nodes[0], 1);
let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &second_htlc_updates.msgs[0]);
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &second_htlc_updates.commitment_msg);
check_added_monitors!(nodes[1], 1);
let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa);
check_added_monitors!(nodes[1], 1);
let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa);
check_added_monitors!(nodes[0], 1);
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]);
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_fail_update.commitment_signed);
check_added_monitors!(nodes[0], 1);
let (as_second_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa);
check_added_monitors!(nodes[1], 1);
let bs_second_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs);
check_added_monitors!(nodes[1], 1);
let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_second_fail_update.update_fail_htlcs[0]);
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_fail_update.commitment_signed);
check_added_monitors!(nodes[0], 1);
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
check_added_monitors!(nodes[0], 1);
let (as_third_raa, as_fourth_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_third_raa);
check_added_monitors!(nodes[1], 1);
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_fourth_cs);
check_added_monitors!(nodes[1], 1);
let bs_fourth_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_fourth_raa);
check_added_monitors!(nodes[0], 1);
// At this point A has sent two HTLCs which both failed due to lack of fee. It now has two
// pending `PaymentPathFailed` events, one with `all_paths_failed` unset, and the second
// with it set. The first event will use up the only retry we are allowed, with the second
// `PaymentPathFailed` being passed up to the user (us, in this case). Previously, we'd
// treated this as "HTLC complete" and dropped the retry counter, causing us to retry again
// if the final HTLC failed.
expected_events.borrow_mut().push_back(&|ev: &Event| {
if let Event::PaymentPathFailed { rejected_by_dest, all_paths_failed, .. } = ev {
assert!(!rejected_by_dest);
assert!(all_paths_failed);
} else { panic!("Unexpected event"); }
});
nodes[0].node.process_pending_events(&invoice_payer);
assert!(expected_events.borrow().is_empty());
let retry_htlc_updates = SendEvent::from_node(&nodes[0]);
check_added_monitors!(nodes[0], 1);
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &retry_htlc_updates.msgs[0]);
commitment_signed_dance!(nodes[1], nodes[0], &retry_htlc_updates.commitment_msg, false, true);
let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[0], nodes[1], &bs_fail_update.commitment_signed, false, true);
expected_events.borrow_mut().push_back(&|ev: &Event| {
if let Event::PaymentPathFailed { rejected_by_dest, all_paths_failed, .. } = ev {
assert!(!rejected_by_dest);
assert!(all_paths_failed);
} else { panic!("Unexpected event"); }
});
expected_events.borrow_mut().push_back(&|ev: &Event| {
if let Event::PaymentFailed { .. } = ev {
} else { panic!("Unexpected event"); }
});
nodes[0].node.process_pending_events(&invoice_payer);
assert!(expected_events.borrow().is_empty());
}
}

View file

@ -152,6 +152,10 @@ where
) -> Result<(), PaymentSendFailure> {
self.retry_payment(route, payment_id)
}
fn abandon_payment(&self, payment_id: PaymentId) {
self.abandon_payment(payment_id)
}
}
#[cfg(test)]

View file

@ -454,6 +454,17 @@ pub(crate) enum PendingOutboundPayment {
session_privs: HashSet<[u8; 32]>,
payment_hash: Option<PaymentHash>,
},
/// When a payer gives up trying to retry a payment, they inform us, letting us generate a
/// `PaymentFailed` event when all HTLCs have irrevocably failed. This avoids a number of race
/// conditions in MPP-aware payment retriers (1), where the possibility of multiple
/// `PaymentPathFailed` events with `all_paths_failed` can be pending at once, confusing a
/// downstream event handler as to when a payment has actually failed.
///
/// (1) https://github.com/lightningdevkit/rust-lightning/issues/1164
Abandoned {
session_privs: HashSet<[u8; 32]>,
payment_hash: PaymentHash,
},
}
impl PendingOutboundPayment {
@ -469,6 +480,12 @@ impl PendingOutboundPayment {
_ => false,
}
}
fn abandoned(&self) -> bool {
match self {
PendingOutboundPayment::Abandoned { .. } => true,
_ => false,
}
}
fn get_pending_fee_msat(&self) -> Option<u64> {
match self {
PendingOutboundPayment::Retryable { pending_fee_msat, .. } => pending_fee_msat.clone(),
@ -481,6 +498,7 @@ impl PendingOutboundPayment {
PendingOutboundPayment::Legacy { .. } => None,
PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash),
PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash,
PendingOutboundPayment::Abandoned { payment_hash, .. } => Some(*payment_hash),
}
}
@ -489,19 +507,38 @@ impl PendingOutboundPayment {
core::mem::swap(&mut session_privs, match self {
PendingOutboundPayment::Legacy { session_privs } |
PendingOutboundPayment::Retryable { session_privs, .. } |
PendingOutboundPayment::Fulfilled { session_privs, .. }
=> session_privs
PendingOutboundPayment::Fulfilled { session_privs, .. } |
PendingOutboundPayment::Abandoned { session_privs, .. }
=> session_privs,
});
let payment_hash = self.payment_hash();
*self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash };
}
fn mark_abandoned(&mut self) -> Result<(), ()> {
let mut session_privs = HashSet::new();
let our_payment_hash;
core::mem::swap(&mut session_privs, match self {
PendingOutboundPayment::Legacy { .. } |
PendingOutboundPayment::Fulfilled { .. } =>
return Err(()),
PendingOutboundPayment::Retryable { session_privs, payment_hash, .. } |
PendingOutboundPayment::Abandoned { session_privs, payment_hash, .. } => {
our_payment_hash = *payment_hash;
session_privs
},
});
*self = PendingOutboundPayment::Abandoned { session_privs, payment_hash: our_payment_hash };
Ok(())
}
/// panics if path is None and !self.is_fulfilled
fn remove(&mut self, session_priv: &[u8; 32], path: Option<&Vec<RouteHop>>) -> bool {
let remove_res = match self {
PendingOutboundPayment::Legacy { session_privs } |
PendingOutboundPayment::Retryable { session_privs, .. } |
PendingOutboundPayment::Fulfilled { session_privs, .. } => {
PendingOutboundPayment::Fulfilled { session_privs, .. } |
PendingOutboundPayment::Abandoned { session_privs, .. } => {
session_privs.remove(session_priv)
}
};
@ -524,7 +561,8 @@ impl PendingOutboundPayment {
PendingOutboundPayment::Retryable { session_privs, .. } => {
session_privs.insert(session_priv)
}
PendingOutboundPayment::Fulfilled { .. } => false
PendingOutboundPayment::Fulfilled { .. } => false,
PendingOutboundPayment::Abandoned { .. } => false,
};
if insert_res {
if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
@ -542,7 +580,8 @@ impl PendingOutboundPayment {
match self {
PendingOutboundPayment::Legacy { session_privs } |
PendingOutboundPayment::Retryable { session_privs, .. } |
PendingOutboundPayment::Fulfilled { session_privs, .. } => {
PendingOutboundPayment::Fulfilled { session_privs, .. } |
PendingOutboundPayment::Abandoned { session_privs, .. } => {
session_privs.len()
}
}
@ -799,6 +838,10 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRA
#[allow(dead_code)]
const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;
/// The number of blocks before we consider an outbound payment for expiry if it doesn't have any
/// pending HTLCs in flight.
pub(crate) const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
/// Information needed for constructing an invoice route hint for this channel.
#[derive(Clone, Debug, PartialEq)]
pub struct CounterpartyForwardingInfo {
@ -2328,10 +2371,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
///
/// Errors returned are a superset of those returned from [`send_payment`], so see
/// [`send_payment`] documentation for more details on errors. This method will also error if the
/// retry amount puts the payment more than 10% over the payment's total amount, or if the payment
/// for the given `payment_id` cannot be found (likely due to timeout or success).
/// retry amount puts the payment more than 10% over the payment's total amount, if the payment
/// for the given `payment_id` cannot be found (likely due to timeout or success), or if
/// further retries have been disabled with [`abandon_payment`].
///
/// [`send_payment`]: [`ChannelManager::send_payment`]
/// [`abandon_payment`]: [`ChannelManager::abandon_payment`]
pub fn retry_payment(&self, route: &Route, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
const RETRY_OVERFLOW_PERCENTAGE: u64 = 10;
for path in route.paths.iter() {
@ -2363,8 +2408,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}))
},
PendingOutboundPayment::Fulfilled { .. } => {
return Err(PaymentSendFailure::ParameterError(APIError::RouteError {
err: "Payment already completed"
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
err: "Payment already completed".to_owned()
}));
},
PendingOutboundPayment::Abandoned { .. } => {
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
err: "Payment already abandoned (with some HTLCs still pending)".to_owned()
}));
},
}
@ -2377,6 +2427,37 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
return self.send_payment_internal(route, payment_hash, &payment_secret, None, Some(payment_id), Some(total_msat)).map(|_| ())
}
/// Signals that no further retries for the given payment will occur.
///
/// After this method returns, any future calls to [`retry_payment`] for the given `payment_id`
/// will fail with [`PaymentSendFailure::ParameterError`]. If no such event has been generated,
/// an [`Event::PaymentFailed`] event will be generated as soon as there are no remaining
/// pending HTLCs for this payment.
///
/// Note that calling this method does *not* prevent a payment from succeeding. You must still
/// wait until you receive either a [`Event::PaymentFailed`] or [`Event::PaymentSent`] event to
/// determine the ultimate status of a payment.
///
/// [`retry_payment`]: Self::retry_payment
/// [`Event::PaymentFailed`]: events::Event::PaymentFailed
/// [`Event::PaymentSent`]: events::Event::PaymentSent
pub fn abandon_payment(&self, payment_id: PaymentId) {
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
if let Ok(()) = payment.get_mut().mark_abandoned() {
if payment.get().remaining_parts() == 0 {
self.pending_events.lock().unwrap().push(events::Event::PaymentFailed {
payment_id,
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
});
payment.remove();
}
}
}
}
/// Send a spontaneous payment, which is a payment that does not require the recipient to have
/// generated an invoice. Optionally, you may specify the preimage. If you do choose to specify
/// the preimage, it must be a cryptographically secure random value that no intermediate node
@ -3204,22 +3285,28 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
})
} else { None };
self.pending_events.lock().unwrap().push(
events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash,
rejected_by_dest: false,
network_update: None,
all_paths_failed: payment.get().remaining_parts() == 0,
path: path.clone(),
short_channel_id: None,
retry,
#[cfg(test)]
error_code: None,
#[cfg(test)]
error_data: None,
}
);
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push(events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash,
rejected_by_dest: false,
network_update: None,
all_paths_failed: payment.get().remaining_parts() == 0,
path: path.clone(),
short_channel_id: None,
retry,
#[cfg(test)]
error_code: None,
#[cfg(test)]
error_data: None,
});
if payment.get().abandoned() && payment.get().remaining_parts() == 0 {
pending_events.push(events::Event::PaymentFailed {
payment_id,
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
});
payment.remove();
}
}
} else {
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@ -3250,6 +3337,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
session_priv_bytes.copy_from_slice(&session_priv[..]);
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
let mut all_paths_failed = false;
let mut full_failure_ev = None;
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@ -3261,6 +3349,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}
if payment.get().remaining_parts() == 0 {
all_paths_failed = true;
if payment.get().abandoned() {
full_failure_ev = Some(events::Event::PaymentFailed {
payment_id,
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
});
payment.remove();
}
}
} else {
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@ -3276,7 +3371,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
})
} else { None };
log_trace!(self.logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
match &onion_error {
let path_failure = match &onion_error {
&HTLCFailReason::LightningError { ref err } => {
#[cfg(test)]
let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
@ -3285,22 +3381,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
// TODO: If we decided to blame ourselves (or one of our channels) in
// process_onion_failure we should close that channel as it implies our
// next-hop is needlessly blaming us!
self.pending_events.lock().unwrap().push(
events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash: payment_hash.clone(),
rejected_by_dest: !payment_retryable,
network_update,
all_paths_failed,
path: path.clone(),
short_channel_id,
retry,
events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash: payment_hash.clone(),
rejected_by_dest: !payment_retryable,
network_update,
all_paths_failed,
path: path.clone(),
short_channel_id,
retry,
#[cfg(test)]
error_code: onion_error_code,
error_code: onion_error_code,
#[cfg(test)]
error_data: onion_error_data
}
);
error_data: onion_error_data
}
},
&HTLCFailReason::Reason {
#[cfg(test)]
@ -3315,24 +3409,25 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
// ChannelDetails.
// TODO: For non-temporary failures, we really should be closing the
// channel here as we apparently can't relay through them anyway.
self.pending_events.lock().unwrap().push(
events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash: payment_hash.clone(),
rejected_by_dest: path.len() == 1,
network_update: None,
all_paths_failed,
path: path.clone(),
short_channel_id: Some(path.first().unwrap().short_channel_id),
retry,
events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash: payment_hash.clone(),
rejected_by_dest: path.len() == 1,
network_update: None,
all_paths_failed,
path: path.clone(),
short_channel_id: Some(path.first().unwrap().short_channel_id),
retry,
#[cfg(test)]
error_code: Some(*failure_code),
error_code: Some(*failure_code),
#[cfg(test)]
error_data: Some(data.clone()),
}
);
error_data: Some(data.clone()),
}
}
}
};
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push(path_failure);
if let Some(ev) = full_failure_ev { pending_events.push(ev); }
},
HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, .. }) => {
let err_packet = match onion_error {
@ -4861,14 +4956,19 @@ where
inbound_payment.expiry_time > header.time as u64
});
let mut pending_events = self.pending_events.lock().unwrap();
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
outbounds.retain(|_, payment| {
const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
outbounds.retain(|payment_id, payment| {
if payment.remaining_parts() != 0 { return true }
if let PendingOutboundPayment::Retryable { starting_block_height, .. } = payment {
return *starting_block_height + PAYMENT_EXPIRY_BLOCKS > height
}
true
if let PendingOutboundPayment::Retryable { starting_block_height, payment_hash, .. } = payment {
if *starting_block_height + PAYMENT_EXPIRY_BLOCKS <= height {
log_info!(self.logger, "Timing out payment with id {} and hash {}", log_bytes!(payment_id.0), log_bytes!(payment_hash.0));
pending_events.push(events::Event::PaymentFailed {
payment_id: *payment_id, payment_hash: *payment_hash,
});
false
} else { true }
} else { true }
});
}
@ -5620,6 +5720,10 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
(8, pending_amt_msat, required),
(10, starting_block_height, required),
},
(3, Abandoned) => {
(0, session_privs, required),
(2, payment_hash, required),
},
);
impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable for ChannelManager<Signer, M, T, K, F, L>
@ -5713,7 +5817,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
// For backwards compat, write the session privs and their total length.
let mut num_pending_outbounds_compat: u64 = 0;
for (_, outbound) in pending_outbound_payments.iter() {
if !outbound.is_fulfilled() {
if !outbound.is_fulfilled() && !outbound.abandoned() {
num_pending_outbounds_compat += outbound.remaining_parts() as u64;
}
}
@ -5727,6 +5831,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
}
}
PendingOutboundPayment::Fulfilled { .. } => {},
PendingOutboundPayment::Abandoned { .. } => {},
}
}

View file

@ -336,10 +336,11 @@ pub fn create_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(node_a: &'a Node<'b,
}
#[macro_export]
/// Gets an RAA and CS which were sent in response to a commitment update
macro_rules! get_revoke_commit_msgs {
($node: expr, $node_id: expr) => {
{
use util::events::MessageSendEvent;
use $crate::util::events::MessageSendEvent;
let events = $node.node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
(match events[0] {
@ -400,8 +401,8 @@ macro_rules! get_event {
}
}
#[cfg(test)]
#[macro_export]
/// Gets an UpdateHTLCs MessageSendEvent
macro_rules! get_htlc_update_msgs {
($node: expr, $node_id: expr) => {
{
@ -933,6 +934,9 @@ impl SendEvent {
}
}
#[macro_export]
/// Performs the "commitment signed dance" - the series of message exchanges which occur after a
/// commitment update.
macro_rules! commitment_signed_dance {
($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */) => {
{
@ -999,7 +1003,7 @@ macro_rules! commitment_signed_dance {
{
commitment_signed_dance!($node_a, $node_b, $commitment_signed, $fail_backwards, true);
if $fail_backwards {
expect_pending_htlcs_forwardable!($node_a);
$crate::expect_pending_htlcs_forwardable!($node_a);
check_added_monitors!($node_a, 1);
let channel_state = $node_a.node.channel_state.lock().unwrap();
@ -1057,6 +1061,8 @@ macro_rules! get_route_and_payment_hash {
}}
}
#[macro_export]
/// Clears (and ignores) a PendingHTLCsForwardable event
macro_rules! expect_pending_htlcs_forwardable_ignore {
($node: expr) => {{
let events = $node.node.get_and_clear_pending_events();
@ -1068,9 +1074,11 @@ macro_rules! expect_pending_htlcs_forwardable_ignore {
}}
}
#[macro_export]
/// Handles a PendingHTLCsForwardable event
macro_rules! expect_pending_htlcs_forwardable {
($node: expr) => {{
expect_pending_htlcs_forwardable_ignore!($node);
$crate::expect_pending_htlcs_forwardable_ignore!($node);
$node.node.process_pending_htlc_forwards();
// Ensure process_pending_htlc_forwards is idempotent.
@ -1199,58 +1207,114 @@ macro_rules! expect_payment_forwarded {
}
}
pub struct PaymentFailedConditions<'a> {
pub(crate) expected_htlc_error_data: Option<(u16, &'a [u8])>,
pub(crate) expected_blamed_scid: Option<u64>,
pub(crate) expected_blamed_chan_closed: Option<bool>,
pub(crate) expected_mpp_parts_remain: bool,
}
impl<'a> PaymentFailedConditions<'a> {
pub fn new() -> Self {
Self {
expected_htlc_error_data: None,
expected_blamed_scid: None,
expected_blamed_chan_closed: None,
expected_mpp_parts_remain: false,
}
}
pub fn mpp_parts_remain(mut self) -> Self {
self.expected_mpp_parts_remain = true;
self
}
pub fn blamed_scid(mut self, scid: u64) -> Self {
self.expected_blamed_scid = Some(scid);
self
}
pub fn blamed_chan_closed(mut self, closed: bool) -> Self {
self.expected_blamed_chan_closed = Some(closed);
self
}
pub fn expected_htlc_error_data(mut self, code: u16, data: &'a [u8]) -> Self {
self.expected_htlc_error_data = Some((code, data));
self
}
}
#[cfg(test)]
macro_rules! expect_payment_failed_with_update {
($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, ref path, ref retry, .. } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
assert!(retry.is_some(), "expected retry.is_some()");
assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
assert_eq!(retry.as_ref().unwrap().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
assert!(error_code.is_some(), "expected error_code.is_some() = true");
assert!(error_data.is_some(), "expected error_data.is_some() = true");
match network_update {
&Some(NetworkUpdate::ChannelUpdateMessage { ref msg }) if !$chan_closed => {
assert_eq!(msg.contents.short_channel_id, $scid);
assert_eq!(msg.contents.flags & 2, 0);
},
&Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if $chan_closed => {
assert_eq!(short_channel_id, $scid);
assert!(is_permanent);
},
Some(_) => panic!("Unexpected update type"),
None => panic!("Expected update"),
}
},
_ => panic!("Unexpected event"),
}
expect_payment_failed_conditions!($node, $expected_payment_hash, $rejected_by_dest,
$crate::ln::functional_test_utils::PaymentFailedConditions::new().blamed_scid($scid).blamed_chan_closed($chan_closed));
}
}
#[cfg(test)]
macro_rules! expect_payment_failed {
($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
#[allow(unused_mut)]
let mut conditions = $crate::ln::functional_test_utils::PaymentFailedConditions::new();
$(
conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data);
)*
expect_payment_failed_conditions!($node, $expected_payment_hash, $rejected_by_dest, conditions);
};
}
#[cfg(test)]
macro_rules! expect_payment_failed_conditions {
($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $conditions: expr) => {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, ref path, ref retry, .. } => {
let expected_payment_id = match events[0] {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data, ref path, ref retry, ref payment_id, ref network_update, .. } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
assert!(retry.is_some(), "expected retry.is_some()");
assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
assert_eq!(retry.as_ref().unwrap().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
assert!(error_code.is_some(), "expected error_code.is_some() = true");
assert!(error_data.is_some(), "expected error_data.is_some() = true");
$(
assert_eq!(error_code.unwrap(), $expected_error_code, "unexpected error code");
assert_eq!(&error_data.as_ref().unwrap()[..], $expected_error_data, "unexpected error data");
)*
if let Some((code, data)) = $conditions.expected_htlc_error_data {
assert_eq!(error_code.unwrap(), code, "unexpected error code");
assert_eq!(&error_data.as_ref().unwrap()[..], data, "unexpected error data");
}
if let Some(chan_closed) = $conditions.expected_blamed_chan_closed {
match network_update {
&Some($crate::routing::network_graph::NetworkUpdate::ChannelUpdateMessage { ref msg }) if !chan_closed => {
if let Some(scid) = $conditions.expected_blamed_scid {
assert_eq!(msg.contents.short_channel_id, scid);
}
assert_eq!(msg.contents.flags & 2, 0);
},
&Some($crate::routing::network_graph::NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if chan_closed => {
if let Some(scid) = $conditions.expected_blamed_scid {
assert_eq!(short_channel_id, scid);
}
assert!(is_permanent);
},
Some(_) => panic!("Unexpected update type"),
None => panic!("Expected update"),
}
}
payment_id.unwrap()
},
_ => panic!("Unexpected event"),
};
if !$conditions.expected_mpp_parts_remain {
$node.node.abandon_payment(expected_payment_id);
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected second payment_hash");
assert_eq!(*payment_id, expected_payment_id);
}
_ => panic!("Unexpected second event"),
}
}
}
}
@ -1555,16 +1619,29 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
commitment_signed_dance!(origin_node, prev_node, next_msgs.as_ref().unwrap().1, false);
let events = origin_node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, .. } => {
let expected_payment_id = match events[0] {
Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, ref payment_id, .. } => {
assert_eq!(payment_hash, our_payment_hash);
assert!(rejected_by_dest);
assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
for (idx, hop) in expected_route.iter().enumerate() {
assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
}
payment_id.unwrap()
},
_ => panic!("Unexpected event"),
};
if i == expected_paths.len() - 1 {
origin_node.node.abandon_payment(expected_payment_id);
let events = origin_node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
assert_eq!(*payment_id, expected_payment_id);
}
_ => panic!("Unexpected second event"),
}
}
}
}

View file

@ -19,11 +19,11 @@ use chain::transaction::OutPoint;
use chain::keysinterface::BaseSign;
use ln::{PaymentPreimage, PaymentSecret, PaymentHash};
use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT};
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, PAYMENT_EXPIRY_BLOCKS };
use ln::channel::{Channel, ChannelError};
use ln::{chan_utils, onion_utils};
use ln::chan_utils::{HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, HTLCOutputInCommitment};
use routing::network_graph::{NetworkUpdate, RoutingFees};
use routing::network_graph::RoutingFees;
use routing::router::{Payee, Route, RouteHop, RouteHint, RouteHintHop, RouteParameters, find_route, get_route};
use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
use ln::msgs;
@ -3149,9 +3149,10 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
mine_transaction(&nodes[1], &revoked_local_txn[0]);
check_added_monitors!(nodes[1], 1);
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
assert!(ANTI_REORG_DELAY > PAYMENT_EXPIRY_BLOCKS); // We assume payments will also expire
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), if deliver_bs_raa { 2 } else { 3 });
assert_eq!(events.len(), if deliver_bs_raa { 2 } else { 4 });
match events[0] {
Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => { },
_ => panic!("Unexepected event"),
@ -3164,6 +3165,12 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
}
if !deliver_bs_raa {
match events[2] {
Event::PaymentFailed { ref payment_hash, .. } => {
assert_eq!(*payment_hash, fourth_payment_hash);
},
_ => panic!("Unexpected event"),
}
match events[3] {
Event::PendingHTLCsForwardable { .. } => { },
_ => panic!("Unexpected event"),
};
@ -4181,7 +4188,14 @@ fn do_test_holding_cell_htlc_add_timeouts(forwarded_htlc: bool) {
}
expect_payment_failed_with_update!(nodes[0], second_payment_hash, false, chan_2.0.contents.short_channel_id, false);
} else {
expect_payment_failed!(nodes[1], second_payment_hash, true);
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
if let Event::PaymentPathFailed { ref payment_hash, .. } = events[0] {
assert_eq!(*payment_hash, second_payment_hash);
} else { panic!("Unexpected event"); }
if let Event::PaymentFailed { ref payment_hash, .. } = events[1] {
assert_eq!(*payment_hash, second_payment_hash);
} else { panic!("Unexpected event"); }
}
}
@ -5837,7 +5851,14 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
check_added_monitors!(nodes[0], 1);
check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed);
} else {
expect_payment_failed!(nodes[0], our_payment_hash, true);
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
if let Event::PaymentPathFailed { ref payment_hash, .. } = events[0] {
assert_eq!(*payment_hash, our_payment_hash);
} else { panic!("Unexpected event"); }
if let Event::PaymentFailed { ref payment_hash, .. } = events[1] {
assert_eq!(*payment_hash, our_payment_hash);
} else { panic!("Unexpected event"); }
}
}

View file

@ -16,7 +16,6 @@ use ln::channelmanager::BREAKDOWN_TIMEOUT;
use ln::features::InitFeatures;
use ln::msgs::ChannelMessageHandler;
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
use routing::network_graph::NetworkUpdate;
use bitcoin::blockdata::script::Builder;
use bitcoin::blockdata::opcodes;

View file

@ -67,7 +67,7 @@ fn retry_single_path_payment() {
check_added_monitors!(nodes[1], 1);
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false);
expect_payment_failed!(nodes[0], payment_hash, false);
expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
// Rebalance the channel so the retry succeeds.
send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
@ -170,7 +170,7 @@ fn mpp_retry() {
check_added_monitors!(nodes[2], 1);
nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[0], nodes[2], htlc_updates.commitment_signed, false);
expect_payment_failed!(nodes[0], payment_hash, false);
expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
// Rebalance the channel so the second half of the payment can succeed.
send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
@ -437,7 +437,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
confirm_transaction(&nodes[0], &as_htlc_timeout_txn[0]);
}
nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
expect_payment_failed!(nodes[0], payment_hash, false);
expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
// Finally, retry the payment (which was reloaded from the ChannelMonitor when nodes[0] was
// reloaded) via a route over the new channel, which work without issue and eventually be

View file

@ -15,7 +15,6 @@ use chain::{Confirm, Watch};
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
use ln::features::InitFeatures;
use ln::msgs::ChannelMessageHandler;
use routing::network_graph::NetworkUpdate;
use util::enforcing_trait_impls::EnforcingSigner;
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
use util::test_utils;

View file

@ -13,7 +13,6 @@ use chain::keysinterface::KeysInterface;
use chain::transaction::OutPoint;
use ln::channelmanager::PaymentSendFailure;
use routing::router::{Payee, get_route};
use routing::network_graph::NetworkUpdate;
use ln::features::{InitFeatures, InvoiceFeatures};
use ln::msgs;
use ln::msgs::{ChannelMessageHandler, ErrorAction};

View file

@ -225,14 +225,20 @@ pub enum Event {
/// [`Route::get_total_fees`]: crate::routing::router::Route::get_total_fees
fee_paid_msat: Option<u64>,
},
/// Indicates an outbound payment we made failed. Probably some intermediary node dropped
/// Indicates an outbound HTLC we sent failed. Probably some intermediary node dropped
/// something. You may wish to retry with a different route.
///
/// Note that this does *not* indicate that all paths for an MPP payment have failed, see
/// [`Event::PaymentFailed`] and [`all_paths_failed`].
///
/// [`all_paths_failed`]: Self::all_paths_failed
PaymentPathFailed {
/// The id returned by [`ChannelManager::send_payment`] and used with
/// [`ChannelManager::retry_payment`].
/// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
payment_id: Option<PaymentId>,
/// The hash that was given to [`ChannelManager::send_payment`].
///
@ -254,6 +260,20 @@ pub enum Event {
/// For both single-path and multi-path payments, this is set if all paths of the payment have
/// failed. This will be set to false if (1) this is an MPP payment and (2) other parts of the
/// larger MPP payment were still in flight when this event was generated.
///
/// Note that if you are retrying individual MPP parts, using this value to determine if a
/// payment has fully failed is race-y. Because multiple failures can happen prior to events
/// being processed, you may retry in response to a first failure, with a second failure
/// (with `all_paths_failed` set) still pending. Then, when the second failure is processed
/// you will see `all_paths_failed` set even though the retry of the first failure still
/// has an associated in-flight HTLC. See (1) for an example of such a failure.
///
/// If you wish to retry individual MPP parts and learn when a payment has failed, you must
/// call [`ChannelManager::abandon_payment`] and wait for a [`Event::PaymentFailed`] event.
///
/// (1) <https://github.com/lightningdevkit/rust-lightning/issues/1164>
///
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
all_paths_failed: bool,
/// The payment path that failed.
path: Vec<RouteHop>,
@ -274,6 +294,27 @@ pub enum Event {
#[cfg(test)]
error_data: Option<Vec<u8>>,
},
/// Indicates an outbound payment failed. Individual [`Event::PaymentPathFailed`] events
/// provide failure information for each MPP part in the payment.
///
/// This event is provided once there are no further pending HTLCs for the payment and the
/// payment is no longer retryable, either due to a several-block timeout or because
/// [`ChannelManager::abandon_payment`] was previously called for the corresponding payment.
///
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
PaymentFailed {
/// The id returned by [`ChannelManager::send_payment`] and used with
/// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
payment_id: PaymentId,
/// The hash that was given to [`ChannelManager::send_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
payment_hash: PaymentHash,
},
/// Used to indicate that [`ChannelManager::process_pending_htlc_forwards`] should be called at
/// a time in the future.
///
@ -462,6 +503,13 @@ impl Writeable for Event {
(4, path, vec_type)
})
},
&Event::PaymentFailed { ref payment_id, ref payment_hash } => {
15u8.write(writer)?;
write_tlv_fields!(writer, {
(0, payment_id, required),
(2, payment_hash, required),
})
},
// Note that, going forward, all new events must only write data inside of
// `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
// data via `write_tlv_fields`.
@ -639,6 +687,21 @@ impl MaybeReadable for Event {
};
f()
},
15u8 => {
let f = || {
let mut payment_hash = PaymentHash([0; 32]);
let mut payment_id = PaymentId([0; 32]);
read_tlv_fields!(reader, {
(0, payment_id, required),
(2, payment_hash, required),
});
Ok(Some(Event::PaymentFailed {
payment_id,
payment_hash,
}))
};
f()
},
// Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
// Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
// reads.