Skip to content

Commit 46bf72b

Browse files
committed
Dont use PaymentPathFailed a probe fails without making it out
When we fail to forward a probe HTLC at all and immediately fail it (e.g. due to the first hop channel closing) we'd previously spuriously generate only a `PaymentPathFailed` event. This violates the expected API, as users expect a `ProbeFailed` event instead. This fixes the oversight by ensuring we generate the correct event.
1 parent 97e04b3 commit 46bf72b

File tree

2 files changed

+75
-11
lines changed

2 files changed

+75
-11
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3942,19 +3942,29 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
39423942
// channel here as we apparently can't relay through them anyway.
39433943
let scid = path.first().unwrap().short_channel_id;
39443944
retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
3945-
events::Event::PaymentPathFailed {
3946-
payment_id: Some(payment_id),
3947-
payment_hash: payment_hash.clone(),
3948-
payment_failed_permanently: false,
3949-
network_update: None,
3950-
all_paths_failed,
3951-
path: path.clone(),
3952-
short_channel_id: Some(scid),
3953-
retry,
3945+
3946+
if self.payment_is_probe(payment_hash, &payment_id) {
3947+
events::Event::ProbeFailed {
3948+
payment_id: payment_id,
3949+
payment_hash: payment_hash.clone(),
3950+
path: path.clone(),
3951+
short_channel_id: Some(scid),
3952+
}
3953+
} else {
3954+
events::Event::PaymentPathFailed {
3955+
payment_id: Some(payment_id),
3956+
payment_hash: payment_hash.clone(),
3957+
payment_failed_permanently: false,
3958+
network_update: None,
3959+
all_paths_failed,
3960+
path: path.clone(),
3961+
short_channel_id: Some(scid),
3962+
retry,
39543963
#[cfg(test)]
3955-
error_code: Some(*failure_code),
3964+
error_code: Some(*failure_code),
39563965
#[cfg(test)]
3957-
error_data: Some(data.clone()),
3966+
error_data: Some(data.clone()),
3967+
}
39583968
}
39593969
}
39603970
};

lightning/src/ln/payment_tests.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,3 +1200,57 @@ fn failed_probe_yields_event() {
12001200
_ => panic!(),
12011201
};
12021202
}
1203+
1204+
#[test]
1205+
fn onchain_failed_probe_yields_event() {
1206+
// Tests that an attempt to probe over a channel that is eventaully closed results in a failure
1207+
// event.
1208+
let chanmon_cfgs = create_chanmon_cfgs(3);
1209+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
1210+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
1211+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
1212+
1213+
let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
1214+
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
1215+
1216+
let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
1217+
1218+
// Send a dust HTLC, which will be treated as if it timed out once the channel hits the chain.
1219+
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 1_000, 42);
1220+
let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
1221+
1222+
// node[0] -- update_add_htlcs -> node[1]
1223+
check_added_monitors!(nodes[0], 1);
1224+
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
1225+
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
1226+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
1227+
check_added_monitors!(nodes[1], 0);
1228+
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
1229+
expect_pending_htlcs_forwardable!(nodes[1]);
1230+
1231+
check_added_monitors!(nodes[1], 1);
1232+
let _ = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
1233+
1234+
// Don't bother forwarding the HTLC onwards and just confirm the force-close transaction on
1235+
// Node A, which after 6 confirmations should result in a probe failure event.
1236+
let bs_txn = get_local_commitment_txn!(nodes[1], chan_id);
1237+
confirm_transaction(&nodes[0], &bs_txn[0]);
1238+
check_closed_broadcast!(&nodes[0], true);
1239+
check_added_monitors!(nodes[0], 1);
1240+
1241+
let mut events = nodes[0].node.get_and_clear_pending_events();
1242+
assert_eq!(events.len(), 2);
1243+
let mut found_probe_failed = false;
1244+
for event in events.drain(..) {
1245+
match event {
1246+
Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
1247+
assert_eq!(payment_id, ev_pid);
1248+
assert_eq!(payment_hash, ev_ph);
1249+
found_probe_failed = true;
1250+
},
1251+
Event::ChannelClosed { .. } => {},
1252+
_ => panic!(),
1253+
}
1254+
}
1255+
assert!(found_probe_failed);
1256+
}

0 commit comments

Comments
 (0)