Merge pull request #1084 from valentinewallace/2021-09-rename-paymentfailed

Rename Event PaymentFailed -> PaymentPathFailed
This commit is contained in:
Matt Corallo 2021-09-21 22:14:42 +00:00 committed by GitHub
commit 0273ac52db
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 56 additions and 39 deletions

View file

@ -827,7 +827,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
}
},
events::Event::PaymentSent { .. } => {},
events::Event::PaymentFailed { .. } => {},
events::Event::PaymentPathFailed { .. } => {},
events::Event::PaymentForwarded { .. } if $node == 1 => {},
events::Event::PendingHTLCsForwardable { .. } => {
nodes[$node].process_pending_htlc_forwards();

View file

@ -78,7 +78,7 @@ fn do_test_simple_monitor_permanent_update_fail(persister_fail: bool) {
};
// TODO: Once we hit the chain with the failure transaction we should check that we get a
// PaymentFailed event
// PaymentPathFailed event
assert_eq!(nodes[0].node.list_channels().len(), 0);
check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
@ -267,7 +267,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail
check_closed_broadcast!(nodes[0], true);
// TODO: Once we hit the chain with the failure transaction we should check that we get a
// PaymentFailed event
// PaymentPathFailed event
assert_eq!(nodes[0].node.list_channels().len(), 0);
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
@ -1819,7 +1819,7 @@ fn test_monitor_update_on_pending_forwards() {
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
if let Event::PaymentFailed { payment_hash, rejected_by_dest, .. } = events[0] {
if let Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] {
assert_eq!(payment_hash, payment_hash_1);
assert!(rejected_by_dest);
} else { panic!("Unexpected event!"); }

View file

@ -489,7 +489,7 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
/// The session_priv bytes of outbound payments which are pending resolution.
/// The authoritative state of these HTLCs resides either within Channels or ChannelMonitors
/// (if the channel has been force-closed), however we track them here to prevent duplicative
/// PaymentSent/PaymentFailed events. Specifically, in the case of a duplicative
/// PaymentSent/PaymentPathFailed events. Specifically, in the case of a duplicative
/// update_fulfill_htlc message after a reconnect, we may "claim" a payment twice.
/// Additionally, because ChannelMonitors are often not re-serialized after connecting block(s)
/// which may generate a claim event, we may receive similar duplicate claim/fail MonitorEvents
@ -2875,18 +2875,19 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
self.fail_htlc_backwards_internal(channel_state,
htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data});
},
HTLCSource::OutboundRoute { session_priv, mpp_id, .. } => {
HTLCSource::OutboundRoute { session_priv, mpp_id, path, .. } => {
let mut session_priv_bytes = [0; 32];
session_priv_bytes.copy_from_slice(&session_priv[..]);
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) {
if sessions.get_mut().remove(&session_priv_bytes) {
self.pending_events.lock().unwrap().push(
events::Event::PaymentFailed {
events::Event::PaymentPathFailed {
payment_hash,
rejected_by_dest: false,
network_update: None,
all_paths_failed: sessions.get().len() == 0,
path: path.clone(),
#[cfg(test)]
error_code: None,
#[cfg(test)]
@ -2951,11 +2952,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
// 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::PaymentFailed {
events::Event::PaymentPathFailed {
payment_hash: payment_hash.clone(),
rejected_by_dest: !payment_retryable,
network_update,
all_paths_failed,
path: path.clone(),
#[cfg(test)]
error_code: onion_error_code,
#[cfg(test)]
@ -2977,11 +2979,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
// 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::PaymentFailed {
events::Event::PaymentPathFailed {
payment_hash: payment_hash.clone(),
rejected_by_dest: path.len() == 1,
network_update: None,
all_paths_failed,
path: path.clone(),
#[cfg(test)]
error_code: Some(*failure_code),
#[cfg(test)]

View file

@ -1073,7 +1073,7 @@ macro_rules! expect_payment_failed_with_update {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
assert!(error_code.is_some(), "expected error_code.is_some() = true");
@ -1102,7 +1102,7 @@ macro_rules! expect_payment_failed {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
assert!(error_code.is_some(), "expected error_code.is_some() = true");
@ -1399,10 +1399,13 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
let events = origin_node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { payment_hash, rejected_by_dest, all_paths_failed, .. } => {
Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, .. } => {
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);
}
},
_ => panic!("Unexpected event"),
}

View file

@ -3020,7 +3020,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
_ => panic!("Unexepected event"),
}
match events[1] {
Event::PaymentFailed { ref payment_hash, .. } => {
Event::PaymentPathFailed { ref payment_hash, .. } => {
assert_eq!(*payment_hash, fourth_payment_hash);
},
_ => panic!("Unexpected event"),
@ -3076,7 +3076,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 3);
match events[0] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
assert!(failed_htlcs.insert(payment_hash.0));
// If we delivered B's RAA we got an unknown preimage error, not something
// that we should update our routing table for.
@ -3087,14 +3087,14 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
assert!(failed_htlcs.insert(payment_hash.0));
assert!(network_update.is_some());
},
_ => panic!("Unexpected event"),
}
match events[2] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
assert!(failed_htlcs.insert(payment_hash.0));
assert!(network_update.is_some());
},
@ -3190,7 +3190,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
assert_eq!(events.len(), 2);
// Check that Alice fails backward the pending HTLC from the second payment.
match events[0] {
Event::PaymentFailed { payment_hash, .. } => {
Event::PaymentPathFailed { payment_hash, .. } => {
assert_eq!(payment_hash, failed_payment_hash);
},
_ => panic!("Unexpected event"),
@ -3392,7 +3392,7 @@ fn test_simple_peer_disconnect() {
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } => {
assert_eq!(payment_hash, payment_hash_5);
assert!(rejected_by_dest);
},
@ -4192,7 +4192,7 @@ fn test_dup_htlc_onchain_fails_on_reload() {
//
// If, due to an on-chain event, an HTLC is failed/claimed, and then we serialize the
// ChannelManager, we generally expect there not to be a duplicate HTLC fail/claim (eg via a
// PaymentFailed event appearing). However, because we may not serialize the relevant
// PaymentPathFailed event appearing). However, because we may not serialize the relevant
// ChannelMonitor at the same time, this isn't strictly guaranteed. In order to provide this
// consistency, the ChannelManager explicitly tracks pending-onchain-resolution outbound HTLCs
// and de-duplicates ChannelMonitor events.
@ -5518,7 +5518,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
let mut as_failds = HashSet::new();
let mut as_updates = 0;
for event in as_events.iter() {
if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
assert!(as_failds.insert(*payment_hash));
if *payment_hash != payment_hash_2 {
assert_eq!(*rejected_by_dest, deliver_last_raa);
@ -5543,7 +5543,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
let mut bs_failds = HashSet::new();
let mut bs_updates = 0;
for event in bs_events.iter() {
if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
assert!(bs_failds.insert(*payment_hash));
if *payment_hash != payment_hash_1 && *payment_hash != payment_hash_5 {
assert_eq!(*rejected_by_dest, deliver_last_raa);
@ -6068,7 +6068,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match &events[0] {
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed, path: _ } => {
assert_eq!(our_payment_hash.clone(), *payment_hash);
assert_eq!(*rejected_by_dest, false);
assert_eq!(*all_paths_failed, true);
@ -6155,7 +6155,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match &events[0] {
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed, path: _ } => {
assert_eq!(payment_hash_2.clone(), *payment_hash);
assert_eq!(*rejected_by_dest, false);
assert_eq!(*all_paths_failed, true);
@ -7107,12 +7107,12 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
let events = nodes[0].node.get_and_clear_pending_events();
// Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx
// Only 2 PaymentPathFailed events should show up, over-dust HTLC has to be failed by timeout tx
assert_eq!(events.len(), 2);
let mut first_failed = false;
for event in events {
match event {
Event::PaymentFailed { payment_hash, .. } => {
Event::PaymentPathFailed { payment_hash, .. } => {
if payment_hash == payment_hash_1 {
assert!(!first_failed);
first_failed = true;
@ -7202,14 +7202,14 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
assert_eq!(events.len(), 2);
let first;
match events[0] {
Event::PaymentFailed { payment_hash, .. } => {
Event::PaymentPathFailed { payment_hash, .. } => {
if payment_hash == dust_hash { first = true; }
else { first = false; }
},
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PaymentFailed { payment_hash, .. } => {
Event::PaymentPathFailed { payment_hash, .. } => {
if first { assert_eq!(payment_hash, non_dust_hash); }
else { assert_eq!(payment_hash, dust_hash); }
},

View file

@ -163,7 +163,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed } = &events[0] {
if let &Event::PaymentPathFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed, path: _ } = &events[0] {
assert_eq!(*rejected_by_dest, !expected_retryable);
assert_eq!(*all_paths_failed, true);
assert_eq!(*error_code, expected_error_code);

View file

@ -113,7 +113,7 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
impl<C: Deref, L: Deref> EventHandler for NetGraphMsgHandler<C, L>
where C::Target: chain::Access, L::Target: Logger {
fn handle_event(&self, event: &Event) {
if let Event::PaymentFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
if let Event::PaymentPathFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
if let Some(network_update) = network_update {
self.handle_network_update(network_update);
}
@ -127,7 +127,7 @@ where C::Target: chain::Access, L::Target: Logger {
/// Provides interface to help with initial routing sync by
/// serving historical announcements.
///
/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentFailed`] to the
/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the
/// [`NetworkGraph`].
pub struct NetGraphMsgHandler<C: Deref, L: Deref>
where C::Target: chain::Access, L::Target: Logger
@ -1725,10 +1725,11 @@ mod tests {
assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
payment_hash: PaymentHash([0; 32]),
rejected_by_dest: false,
all_paths_failed: true,
path: vec![],
network_update: Some(NetworkUpdate::ChannelUpdateMessage {
msg: valid_channel_update,
}),
@ -1748,10 +1749,11 @@ mod tests {
}
};
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
payment_hash: PaymentHash([0; 32]),
rejected_by_dest: false,
all_paths_failed: true,
path: vec![],
network_update: Some(NetworkUpdate::ChannelClosed {
short_channel_id,
is_permanent: false,
@ -1770,10 +1772,11 @@ mod tests {
// Permanent closing deletes a channel
{
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
payment_hash: PaymentHash([0; 32]),
rejected_by_dest: false,
all_paths_failed: true,
path: vec![],
network_update: Some(NetworkUpdate::ChannelClosed {
short_channel_id,
is_permanent: true,

View file

@ -28,7 +28,7 @@ use core::cmp;
use core::ops::Deref;
/// A hop in a route
#[derive(Clone, Hash, PartialEq, Eq)]
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct RouteHop {
/// The node_id of the node at this hop.
pub pubkey: PublicKey,

View file

@ -20,6 +20,7 @@ use ln::msgs::DecodeError;
use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
use routing::network_graph::NetworkUpdate;
use util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper};
use routing::router::RouteHop;
use bitcoin::blockdata::script::Script;
@ -170,8 +171,8 @@ pub enum Event {
/// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target
/// and we got back the payment preimage for it).
///
/// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentFailed` event.
/// In this situation, you SHOULD treat this payment as having succeeded.
/// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentPathFailed`
/// event. In this situation, you SHOULD treat this payment as having succeeded.
PaymentSent {
/// The preimage to the hash given to ChannelManager::send_payment.
/// Note that this serves as a payment receipt, if you wish to have such a thing, you must
@ -180,7 +181,7 @@ pub enum Event {
},
/// Indicates an outbound payment we made failed. Probably some intermediary node dropped
/// something. You may wish to retry with a different route.
PaymentFailed {
PaymentPathFailed {
/// The hash which was given to ChannelManager::send_payment.
payment_hash: PaymentHash,
/// Indicates the payment was rejected for some reason by the recipient. This implies that
@ -200,6 +201,8 @@ pub enum Event {
/// 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.
all_paths_failed: bool,
/// The payment path that failed.
path: Vec<RouteHop>,
#[cfg(test)]
error_code: Option<u16>,
#[cfg(test)]
@ -291,7 +294,8 @@ impl Writeable for Event {
(0, payment_preimage, required),
});
},
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed,
&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update,
ref all_paths_failed, ref path,
#[cfg(test)]
ref error_code,
#[cfg(test)]
@ -307,6 +311,7 @@ impl Writeable for Event {
(1, network_update, option),
(2, rejected_by_dest, required),
(3, all_paths_failed, required),
(5, path, vec_type),
});
},
&Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@ -403,17 +408,20 @@ impl MaybeReadable for Event {
let mut rejected_by_dest = false;
let mut network_update = None;
let mut all_paths_failed = Some(true);
let mut path: Option<Vec<RouteHop>> = Some(vec![]);
read_tlv_fields!(reader, {
(0, payment_hash, required),
(1, network_update, ignorable),
(2, rejected_by_dest, required),
(3, all_paths_failed, option),
(5, path, vec_type),
});
Ok(Some(Event::PaymentFailed {
Ok(Some(Event::PaymentPathFailed {
payment_hash,
rejected_by_dest,
network_update,
all_paths_failed: all_paths_failed.unwrap(),
path: path.unwrap(),
#[cfg(test)]
error_code,
#[cfg(test)]