Skip to content

Commit

Permalink
Fix Tests
Browse files Browse the repository at this point in the history
  • Loading branch information
jurvis committed Apr 7, 2022
1 parent 33ed8fc commit d16f1aa
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 65 deletions.
1 change: 1 addition & 0 deletions fuzz/src/chanmon_consistency.rs
Expand Up @@ -853,6 +853,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
events::Event::PendingHTLCsForwardable { .. } => {
nodes[$node].process_pending_htlc_forwards();
},
events::Event::PaymentForwardedFailed { .. } => {},
_ => if out.may_fail.load(atomic::Ordering::Acquire) {
return;
} else {
Expand Down
14 changes: 7 additions & 7 deletions lightning/src/ln/chanmon_update_fail_tests.rs
Expand Up @@ -828,7 +828,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {

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

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

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

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

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

chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
check_added_monitors!(nodes[1], 1);
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Failed to update ChannelMonitor".to_string(), 1);
Expand Down Expand Up @@ -2094,7 +2094,7 @@ fn test_fail_htlc_on_broadcast_after_claim() {
check_closed_broadcast!(nodes[1], true);
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
check_added_monitors!(nodes[1], 1);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());

nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]);
expect_payment_sent_without_paths!(nodes[0], payment_preimage);
Expand Down Expand Up @@ -2456,7 +2456,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
};
if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
expect_pending_htlcs_forwardable!(nodes[2]);
expect_pending_htlcs_forwardable!(nodes[2], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
check_added_monitors!(nodes[2], 1);
get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
} else {
Expand Down Expand Up @@ -2490,7 +2490,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f

if second_fails {
reconnect_nodes(&nodes[1], &nodes[2], (false, false), (0, 0), (0, 0), (1, 0), (0, 0), (0, 0), (false, false));
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
} else {
reconnect_nodes(&nodes[1], &nodes[2], (false, false), (0, 0), (1, 0), (0, 0), (0, 0), (0, 0), (false, false));
}
Expand Down
9 changes: 5 additions & 4 deletions lightning/src/ln/channelmanager.rs
Expand Up @@ -4020,7 +4020,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
time_forwardable: time
});
}

let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push(events::Event::PaymentForwardedFailed {
source_channel_id: outpoint.to_channel_id() , sink_node_id: self.get_our_node_id()
Expand Down Expand Up @@ -7245,7 +7244,7 @@ mod tests {
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
Expand Down Expand Up @@ -7363,8 +7362,10 @@ mod tests {
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
// We have to forward pending HTLCs twice - once tries to forward the payment forward (and
// fails), the second will process the resulting failure and fail the HTLC backward
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
Expand Down Expand Up @@ -7405,7 +7406,7 @@ mod tests {
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_pending_htlcs_forwardable!(nodes[1], PaymentForwardedFailedConditions::new().payment_forwarding_failed());
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
Expand Down
76 changes: 68 additions & 8 deletions lightning/src/ln/functional_test_utils.rs
Expand Up @@ -1130,7 +1130,7 @@ macro_rules! commitment_signed_dance {
{
commitment_signed_dance!($node_a, $node_b, $commitment_signed, $fail_backwards, true);
if $fail_backwards {
$crate::expect_pending_htlcs_forwardable!($node_a);
expect_pending_htlcs_forwardable!($node_a, PaymentForwardedFailedConditions::new().payment_forwarding_failed());
check_added_monitors!($node_a, 1);

let channel_state = $node_a.node.channel_state.lock().unwrap();
Expand Down Expand Up @@ -1191,24 +1191,81 @@ macro_rules! get_route_and_payment_hash {
}}
}

pub struct PaymentForwardedFailedConditions {
pub expected_pending_htlcs_forwardable: bool,
pub expected_payment_forwarding_failed: Option<u32>,
}

impl PaymentForwardedFailedConditions {
pub fn new() -> Self {
Self {
expected_pending_htlcs_forwardable: false,
expected_payment_forwarding_failed: None,
}
}

pub fn pending_htlcs_forwardable(mut self) -> Self {
self.expected_pending_htlcs_forwardable = true;
self
}

pub fn payment_forwarding_failed(mut self) -> Self {
self.expected_payment_forwarding_failed = Some(1);
self
}

pub fn payment_forwarding_failed_with_count(mut self, count: u32) -> Self {
self.expected_payment_forwarding_failed = Some(count);
self
}
}

#[macro_export]
/// Clears (and ignores) a PendingHTLCsForwardable event
macro_rules! expect_pending_htlcs_forwardable_ignore {
($node: expr) => {{
macro_rules! expect_pending_htlcs_forwardable_conditions {
($node: expr, $conditions: expr) => {{
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
$crate::util::events::Event::PendingHTLCsForwardable { .. } => { },
_ => panic!("Unexpected event"),
};

if let Some(count) = $conditions.expected_payment_forwarding_failed {
assert_eq!(events.len() as u32, count + 1u32);
match events[1] {
$crate::util::events::Event::PaymentForwardedFailed { .. } => { },
_ => panic!("Unexpected event"),
}
} else {
assert_eq!(events.len(), 1);
}
}}
}

#[macro_export]
/// Clears (and ignores) a PendingHTLCsForwardable event
macro_rules! expect_pending_htlcs_forwardable_ignore {
($node: expr) => {{
expect_pending_htlcs_forwardable_conditions!($node, $crate::ln::functional_test_utils::PaymentForwardedFailedConditions::new());
}};

($node: expr, $conditions: expr) => {{
expect_pending_htlcs_forwardable_conditions!($node, $conditions);
}}
}

#[macro_export]
/// Handles a PendingHTLCsForwardable event
macro_rules! expect_pending_htlcs_forwardable {
($node: expr) => {{
$crate::expect_pending_htlcs_forwardable_ignore!($node);
expect_pending_htlcs_forwardable_ignore!($node);
$node.node.process_pending_htlc_forwards();

// Ensure process_pending_htlc_forwards is idempotent.
$node.node.process_pending_htlc_forwards();
}};

($node: expr, $conditions: expr) => {{
expect_pending_htlcs_forwardable_ignore!($node, $conditions);
$node.node.process_pending_htlc_forwards();

// Ensure process_pending_htlc_forwards is idempotent.
Expand All @@ -1219,6 +1276,8 @@ macro_rules! expect_pending_htlcs_forwardable {
#[cfg(test)]
macro_rules! expect_pending_htlcs_forwardable_from_events {
($node: expr, $events: expr, $ignore: expr) => {{
// We need to clear pending events since there may possibly be `PaymentForwardingFailed` events here
$node.node.get_and_clear_pending_events();
assert_eq!($events.len(), 1);
match $events[0] {
Event::PendingHTLCsForwardable { .. } => { },
Expand Down Expand Up @@ -1692,7 +1751,8 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
}
assert!(expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap());

expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap(), PaymentForwardedFailedConditions::new().payment_forwarding_failed_with_count(expected_paths.len() as u32));
check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());

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

0 comments on commit d16f1aa

Please sign in to comment.