From 46453bf07862ae91de3681846a165fc71b38f42e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 27 Aug 2023 20:37:36 +0000 Subject: [PATCH] Correct `expect_payment_forwarded` upstream channel checking `expect_payment_forwarded` takes a bool to indicate that the inbound channel on which we received a forwarded payment has been closed, but then ignores it in favor of looking at the fee in the event. While this is generally correct, in cases where we process an event after a channel was closed, which was generated before a channel closed this is incorrect. Instead, we examine the bool we already passed and use that. --- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 3 ++- lightning/src/ln/payment_tests.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index cd03ae097..90edd4abb 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1997,7 +1997,7 @@ macro_rules! expect_payment_forwarded { outbound_amount_forwarded_msat: _ } => { assert_eq!(fee_earned_msat, $expected_fee); - if fee_earned_msat.is_some() { + if !$upstream_force_closed { // Is the event prev_channel_id in one of the channels between the two nodes? assert!($node.node.list_channels().iter().any(|x| x.counterparty.node_id == $prev_node.node.get_our_node_id() && x.channel_id == prev_channel_id.unwrap())); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index e55adab85..c0e334326 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -8777,7 +8777,8 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain assert_eq!(carol_updates.update_fulfill_htlcs.len(), 1); nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &carol_updates.update_fulfill_htlcs[0]); - expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], if go_onchain_before_fulfill || force_closing_node == 1 { None } else { Some(1000) }, false, false); + let went_onchain = go_onchain_before_fulfill || force_closing_node == 1; + expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], if went_onchain { None } else { Some(1000) }, went_onchain, false); // If Alice broadcasted but Bob doesn't know yet, here he prepares to tell her about the preimage. if !go_onchain_before_fulfill && broadcast_alice { let events = nodes[1].node.get_and_clear_pending_msg_events(); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 440af3c9a..d88730c22 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -611,7 +611,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &htlc_fulfill_updates.update_fulfill_htlcs[0]); check_added_monitors!(nodes[1], 1); commitment_signed_dance!(nodes[1], nodes[2], htlc_fulfill_updates.commitment_signed, false); - expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], None, false, false); + expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], None, true, false); if confirm_before_reload { let best_block = nodes[0].blocks.lock().unwrap().last().unwrap().clone(); -- 2.39.5