Skip to content

Commit d16f1aa

Browse files
committed
Fix Tests
1 parent 33ed8fc commit d16f1aa

10 files changed

+134
-65
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
853853
events::Event::PendingHTLCsForwardable { .. } => {
854854
nodes[$node].process_pending_htlc_forwards();
855855
},
856+
events::Event::PaymentForwardedFailed { .. } => {},
856857
_ => if out.may_fail.load(atomic::Ordering::Acquire) {
857858
return;
858859
} else {

lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
828828

829829
// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
830830
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
831-
expect_pending_htlcs_forwardable!(nodes[2]);
831+
expect_pending_htlcs_forwardable!(nodes[2], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
832832
check_added_monitors!(nodes[2], 1);
833833

834834
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
@@ -909,7 +909,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
909909
let (outpoint, latest_update, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_2.2).unwrap().clone();
910910
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update);
911911
check_added_monitors!(nodes[1], 0);
912-
expect_pending_htlcs_forwardable!(nodes[1]);
912+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
913913
check_added_monitors!(nodes[1], 1);
914914

915915
let mut events_3 = nodes[1].node.get_and_clear_pending_msg_events();
@@ -1689,7 +1689,7 @@ fn test_monitor_update_on_pending_forwards() {
16891689

16901690
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
16911691
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
1692-
expect_pending_htlcs_forwardable!(nodes[2]);
1692+
expect_pending_htlcs_forwardable!(nodes[2], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
16931693
check_added_monitors!(nodes[2], 1);
16941694

16951695
let cs_fail_update = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
@@ -1710,7 +1710,7 @@ fn test_monitor_update_on_pending_forwards() {
17101710
commitment_signed_dance!(nodes[1], nodes[2], payment_event.commitment_msg, false);
17111711

17121712
chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
1713-
expect_pending_htlcs_forwardable!(nodes[1]);
1713+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
17141714
check_added_monitors!(nodes[1], 1);
17151715
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
17161716
nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Failed to update ChannelMonitor".to_string(), 1);
@@ -2094,7 +2094,7 @@ fn test_fail_htlc_on_broadcast_after_claim() {
20942094
check_closed_broadcast!(nodes[1], true);
20952095
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
20962096
check_added_monitors!(nodes[1], 1);
2097-
expect_pending_htlcs_forwardable!(nodes[1]);
2097+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
20982098

20992099
nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]);
21002100
expect_payment_sent_without_paths!(nodes[0], payment_preimage);
@@ -2456,7 +2456,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
24562456
};
24572457
if second_fails {
24582458
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
2459-
expect_pending_htlcs_forwardable!(nodes[2]);
2459+
expect_pending_htlcs_forwardable!(nodes[2], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
24602460
check_added_monitors!(nodes[2], 1);
24612461
get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
24622462
} else {
@@ -2490,7 +2490,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
24902490

24912491
if second_fails {
24922492
reconnect_nodes(&nodes[1], &nodes[2], (false, false), (0, 0), (0, 0), (1, 0), (0, 0), (0, 0), (false, false));
2493-
expect_pending_htlcs_forwardable!(nodes[1]);
2493+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
24942494
} else {
24952495
reconnect_nodes(&nodes[1], &nodes[2], (false, false), (0, 0), (1, 0), (0, 0), (0, 0), (0, 0), (false, false));
24962496
}

lightning/src/ln/channelmanager.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4020,7 +4020,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
40204020
time_forwardable: time
40214021
});
40224022
}
4023-
40244023
let mut pending_events = self.pending_events.lock().unwrap();
40254024
pending_events.push(events::Event::PaymentForwardedFailed {
40264025
source_channel_id: outpoint.to_channel_id() , sink_node_id: self.get_our_node_id()
@@ -7245,7 +7244,7 @@ mod tests {
72457244
check_added_monitors!(nodes[1], 0);
72467245
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
72477246
expect_pending_htlcs_forwardable!(nodes[1]);
7248-
expect_pending_htlcs_forwardable!(nodes[1]);
7247+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
72497248
check_added_monitors!(nodes[1], 1);
72507249
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
72517250
assert!(updates.update_add_htlcs.is_empty());
@@ -7363,8 +7362,10 @@ mod tests {
73637362
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
73647363
check_added_monitors!(nodes[1], 0);
73657364
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
7365+
// We have to forward pending HTLCs twice - once tries to forward the payment forward (and
7366+
// fails), the second will process the resulting failure and fail the HTLC backward
73667367
expect_pending_htlcs_forwardable!(nodes[1]);
7367-
expect_pending_htlcs_forwardable!(nodes[1]);
7368+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
73687369
check_added_monitors!(nodes[1], 1);
73697370
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
73707371
assert!(updates.update_add_htlcs.is_empty());
@@ -7405,7 +7406,7 @@ mod tests {
74057406
check_added_monitors!(nodes[1], 0);
74067407
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
74077408
expect_pending_htlcs_forwardable!(nodes[1]);
7408-
expect_pending_htlcs_forwardable!(nodes[1]);
7409+
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
74097410
check_added_monitors!(nodes[1], 1);
74107411
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
74117412
assert!(updates.update_add_htlcs.is_empty());

lightning/src/ln/functional_test_utils.rs

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,7 @@ macro_rules! commitment_signed_dance {
11301130
{
11311131
commitment_signed_dance!($node_a, $node_b, $commitment_signed, $fail_backwards, true);
11321132
if $fail_backwards {
1133-
$crate::expect_pending_htlcs_forwardable!($node_a);
1133+
expect_pending_htlcs_forwardable!($node_a, PaymentForwardedFailedConditions::new().payment_forwarding_failed());
11341134
check_added_monitors!($node_a, 1);
11351135

11361136
let channel_state = $node_a.node.channel_state.lock().unwrap();
@@ -1191,24 +1191,81 @@ macro_rules! get_route_and_payment_hash {
11911191
}}
11921192
}
11931193

1194+
pub struct PaymentForwardedFailedConditions {
1195+
pub expected_pending_htlcs_forwardable: bool,
1196+
pub expected_payment_forwarding_failed: Option<u32>,
1197+
}
1198+
1199+
impl PaymentForwardedFailedConditions {
1200+
pub fn new() -> Self {
1201+
Self {
1202+
expected_pending_htlcs_forwardable: false,
1203+
expected_payment_forwarding_failed: None,
1204+
}
1205+
}
1206+
1207+
pub fn pending_htlcs_forwardable(mut self) -> Self {
1208+
self.expected_pending_htlcs_forwardable = true;
1209+
self
1210+
}
1211+
1212+
pub fn payment_forwarding_failed(mut self) -> Self {
1213+
self.expected_payment_forwarding_failed = Some(1);
1214+
self
1215+
}
1216+
1217+
pub fn payment_forwarding_failed_with_count(mut self, count: u32) -> Self {
1218+
self.expected_payment_forwarding_failed = Some(count);
1219+
self
1220+
}
1221+
}
1222+
11941223
#[macro_export]
1195-
/// Clears (and ignores) a PendingHTLCsForwardable event
1196-
macro_rules! expect_pending_htlcs_forwardable_ignore {
1197-
($node: expr) => {{
1224+
macro_rules! expect_pending_htlcs_forwardable_conditions {
1225+
($node: expr, $conditions: expr) => {{
11981226
let events = $node.node.get_and_clear_pending_events();
1199-
assert_eq!(events.len(), 1);
12001227
match events[0] {
12011228
$crate::util::events::Event::PendingHTLCsForwardable { .. } => { },
12021229
_ => panic!("Unexpected event"),
12031230
};
1231+
1232+
if let Some(count) = $conditions.expected_payment_forwarding_failed {
1233+
assert_eq!(events.len() as u32, count + 1u32);
1234+
match events[1] {
1235+
$crate::util::events::Event::PaymentForwardedFailed { .. } => { },
1236+
_ => panic!("Unexpected event"),
1237+
}
1238+
} else {
1239+
assert_eq!(events.len(), 1);
1240+
}
1241+
}}
1242+
}
1243+
1244+
#[macro_export]
1245+
/// Clears (and ignores) a PendingHTLCsForwardable event
1246+
macro_rules! expect_pending_htlcs_forwardable_ignore {
1247+
($node: expr) => {{
1248+
expect_pending_htlcs_forwardable_conditions!($node, $crate::ln::functional_test_utils::PaymentForwardedFailedConditions::new());
1249+
}};
1250+
1251+
($node: expr, $conditions: expr) => {{
1252+
expect_pending_htlcs_forwardable_conditions!($node, $conditions);
12041253
}}
12051254
}
12061255

12071256
#[macro_export]
12081257
/// Handles a PendingHTLCsForwardable event
12091258
macro_rules! expect_pending_htlcs_forwardable {
12101259
($node: expr) => {{
1211-
$crate::expect_pending_htlcs_forwardable_ignore!($node);
1260+
expect_pending_htlcs_forwardable_ignore!($node);
1261+
$node.node.process_pending_htlc_forwards();
1262+
1263+
// Ensure process_pending_htlc_forwards is idempotent.
1264+
$node.node.process_pending_htlc_forwards();
1265+
}};
1266+
1267+
($node: expr, $conditions: expr) => {{
1268+
expect_pending_htlcs_forwardable_ignore!($node, $conditions);
12121269
$node.node.process_pending_htlc_forwards();
12131270

12141271
// Ensure process_pending_htlc_forwards is idempotent.
@@ -1219,6 +1276,8 @@ macro_rules! expect_pending_htlcs_forwardable {
12191276
#[cfg(test)]
12201277
macro_rules! expect_pending_htlcs_forwardable_from_events {
12211278
($node: expr, $events: expr, $ignore: expr) => {{
1279+
// We need to clear pending events since there may possibly be `PaymentForwardingFailed` events here
1280+
$node.node.get_and_clear_pending_events();
12221281
assert_eq!($events.len(), 1);
12231282
match $events[0] {
12241283
Event::PendingHTLCsForwardable { .. } => { },
@@ -1692,7 +1751,8 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
16921751
assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
16931752
}
16941753
assert!(expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
1695-
expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap());
1754+
1755+
expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap(), PaymentForwardedFailedConditions::new().payment_forwarding_failed_with_count(expected_paths.len() as u32));
16961756
check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
16971757

16981758
let mut per_path_msgs: Vec<((msgs::UpdateFailHTLC, msgs::CommitmentSigned), PublicKey)> = Vec::with_capacity(expected_paths.len());
@@ -1727,7 +1787,7 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
17271787
node.node.handle_update_fail_htlc(&prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
17281788
commitment_signed_dance!(node, prev_node, next_msgs.as_ref().unwrap().1, update_next_node);
17291789
if !update_next_node {
1730-
expect_pending_htlcs_forwardable!(node);
1790+
expect_pending_htlcs_forwardable!(node, PaymentForwardedFailedConditions::new().payment_forwarding_failed());
17311791
}
17321792
}
17331793
let events = node.node.get_and_clear_pending_msg_events();

0 commit comments

Comments
 (0)