From e53c5bdb44670cb4d134df80f97440c9c0157c76 Mon Sep 17 00:00:00 2001 From: atalw Date: Wed, 30 Mar 2022 14:21:45 +0530 Subject: [PATCH] Add `source_channel_id` in `PaymentForwarded` event --- lightning/src/ln/chanmon_update_fail_tests.rs | 10 +++---- lightning/src/ln/channelmanager.rs | 3 ++ lightning/src/ln/functional_test_utils.rs | 21 ++++++++++---- lightning/src/ln/functional_tests.rs | 28 ++++++++++++++----- lightning/src/ln/payment_tests.rs | 2 +- lightning/src/ln/reorg_tests.rs | 2 +- lightning/src/ln/shutdown_tests.rs | 4 +-- lightning/src/util/events.rs | 10 +++++-- 8 files changed, 57 insertions(+), 23 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 94af00c7..58fe30ba 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -1102,7 +1102,7 @@ fn test_monitor_update_fail_reestablish() { assert!(updates.update_fee.is_none()); assert_eq!(updates.update_fulfill_htlcs.len(), 1); nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); - expect_payment_forwarded!(nodes[1], Some(1000), false); + expect_payment_forwarded!(nodes[1], nodes[0], Some(1000), false); check_added_monitors!(nodes[1], 1); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false); @@ -2087,7 +2087,7 @@ fn test_fail_htlc_on_broadcast_after_claim() { nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]); let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); check_added_monitors!(nodes[1], 1); - expect_payment_forwarded!(nodes[1], Some(1000), false); + expect_payment_forwarded!(nodes[1], nodes[0], Some(1000), false); mine_transaction(&nodes[1], &bs_txn[0]); check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed); @@ -2423,7 +2423,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2; + let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2; let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100_000); @@ -2450,7 +2450,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f } let fulfill_msg = msgs::UpdateFulfillHTLC { - channel_id: chan_2, + channel_id: chan_id_2, htlc_id: 0, payment_preimage, }; @@ -2468,7 +2468,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]); } nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &fulfill_msg); - expect_payment_forwarded!(nodes[1], Some(1000), false); + expect_payment_forwarded!(nodes[1], nodes[0], Some(1000), false); check_added_monitors!(nodes[1], 1); let mut bs_updates = None; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cfa2da06..a7badc4f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4291,7 +4291,10 @@ impl ChannelMana } else { None }; let mut pending_events = self.pending_events.lock().unwrap(); + + let source_channel_id = Some(prev_outpoint.to_channel_id()); pending_events.push(events::Event::PaymentForwarded { + source_channel_id, fee_earned_msat, claim_from_onchain_tx: from_onchain, }); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index aa6fdc97..1d033f32 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1328,12 +1328,16 @@ macro_rules! expect_payment_path_successful { } macro_rules! expect_payment_forwarded { - ($node: expr, $expected_fee: expr, $upstream_force_closed: expr) => { + ($node: expr, $source_node: expr, $expected_fee: expr, $upstream_force_closed: expr) => { let events = $node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentForwarded { fee_earned_msat, claim_from_onchain_tx } => { + Event::PaymentForwarded { fee_earned_msat, source_channel_id, claim_from_onchain_tx } => { assert_eq!(fee_earned_msat, $expected_fee); + if fee_earned_msat.is_some() { + // Is the event channel_id in one of the channels between the two nodes? + assert!($node.node.list_channels().iter().any(|x| x.counterparty.node_id == $source_node.node.get_our_node_id() && x.channel_id == source_channel_id.unwrap())); + } assert_eq!(claim_from_onchain_tx, $upstream_force_closed); }, _ => panic!("Unexpected event"), @@ -1572,11 +1576,11 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, } } macro_rules! mid_update_fulfill_dance { - ($node: expr, $prev_node: expr, $new_msgs: expr) => { + ($node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => { { $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0); let fee = $node.node.channel_state.lock().unwrap().by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap().config.forwarding_fee_base_msat; - expect_payment_forwarded!($node, Some(fee as u64), false); + expect_payment_forwarded!($node, $next_node, Some(fee as u64), false); expected_total_fee_msat += fee as u64; check_added_monitors!($node, 1); let new_next_msgs = if $new_msgs { @@ -1600,7 +1604,14 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, assert_eq!(expected_next_node, node.node.get_our_node_id()); let update_next_msgs = !skip_last || idx != expected_route.len() - 1; if next_msgs.is_some() { - mid_update_fulfill_dance!(node, prev_node, update_next_msgs); + // Since we are traversing in reverse, next_node is actually the previous node + let next_node: &Node; + if idx == expected_route.len() - 1 { + next_node = origin_node; + } else { + next_node = expected_route[expected_route.len() - 1 - idx - 1]; + } + mid_update_fulfill_dance!(node, prev_node, next_node, update_next_msgs); } else { assert!(!update_next_msgs); assert!(node.node.get_and_clear_pending_msg_events().is_empty()); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 525b4f33..00f1ea46 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2684,10 +2684,23 @@ fn test_htlc_on_chain_success() { Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => {} _ => panic!("Unexpected event"), } - if let Event::PaymentForwarded { fee_earned_msat: Some(1000), claim_from_onchain_tx: true } = forwarded_events[1] { - } else { panic!(); } - if let Event::PaymentForwarded { fee_earned_msat: Some(1000), claim_from_onchain_tx: true } = forwarded_events[2] { - } else { panic!(); } + let chan_id = Some(chan_1.2); + match forwarded_events[1] { + Event::PaymentForwarded { fee_earned_msat, source_channel_id, claim_from_onchain_tx } => { + assert_eq!(fee_earned_msat, Some(1000)); + assert_eq!(source_channel_id, chan_id); + assert_eq!(claim_from_onchain_tx, true); + }, + _ => panic!() + } + match forwarded_events[2] { + Event::PaymentForwarded { fee_earned_msat, source_channel_id, claim_from_onchain_tx } => { + assert_eq!(fee_earned_msat, Some(1000)); + assert_eq!(source_channel_id, chan_id); + assert_eq!(claim_from_onchain_tx, true); + }, + _ => panic!() + } let events = nodes[1].node.get_and_clear_pending_msg_events(); { let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); @@ -5104,8 +5117,9 @@ fn test_onchain_to_onchain_claim() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentForwarded { fee_earned_msat, claim_from_onchain_tx } => { + Event::PaymentForwarded { fee_earned_msat, source_channel_id, claim_from_onchain_tx } => { assert_eq!(fee_earned_msat, Some(1000)); + assert_eq!(source_channel_id, Some(chan_1.2)); assert_eq!(claim_from_onchain_tx, true); }, _ => panic!("Unexpected event"), @@ -5271,7 +5285,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { // Note that the fee paid is effectively double as the HTLC value (including the nodes[1] fee // and nodes[2] fee) is rounded down and then claimed in full. mine_transaction(&nodes[1], &htlc_success_txn[0]); - expect_payment_forwarded!(nodes[1], Some(196*2), true); + expect_payment_forwarded!(nodes[1], nodes[0], Some(196*2), true); let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); assert!(updates.update_add_htlcs.is_empty()); assert!(updates.update_fail_htlcs.is_empty()); @@ -8849,7 +8863,7 @@ 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], if go_onchain_before_fulfill || force_closing_node == 1 { None } else { Some(1000) }, false); + expect_payment_forwarded!(nodes[1], nodes[0], if go_onchain_before_fulfill || force_closing_node == 1 { None } else { Some(1000) }, 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 346fb98b..46d5d22b 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -495,7 +495,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { let bs_htlc_claim_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(bs_htlc_claim_txn.len(), 1); check_spends!(bs_htlc_claim_txn[0], as_commitment_tx); - expect_payment_forwarded!(nodes[1], None, false); + expect_payment_forwarded!(nodes[1], nodes[0], None, false); if !confirm_before_reload { mine_transaction(&nodes[0], &as_commitment_tx); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 8eb39cfe..7b36ae0f 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -138,7 +138,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) { // ChannelManager only polls chain::Watch::release_pending_monitor_events when we // probe it for events, so we probe non-message events here (which should just be the // PaymentForwarded event). - expect_payment_forwarded!(nodes[1], Some(1000), true); + expect_payment_forwarded!(nodes[1], nodes[0], Some(1000), true); } else { // Confirm the timeout tx and check that we fail the HTLC backwards let block = Block { diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 557aff84..42c0e3b2 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -110,7 +110,7 @@ fn updates_shutdown_wait() { assert!(updates.update_fee.is_none()); assert_eq!(updates.update_fulfill_htlcs.len(), 1); nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); - expect_payment_forwarded!(nodes[1], Some(1000), false); + expect_payment_forwarded!(nodes[1], nodes[0], Some(1000), false); check_added_monitors!(nodes[1], 1); let updates_2 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false); @@ -279,7 +279,7 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) { assert!(updates.update_fee.is_none()); assert_eq!(updates.update_fulfill_htlcs.len(), 1); nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); - expect_payment_forwarded!(nodes[1], Some(1000), false); + expect_payment_forwarded!(nodes[1], nodes[0], Some(1000), false); check_added_monitors!(nodes[1], 1); let updates_2 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false); diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index ea50398b..d7eb16ee 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -343,6 +343,9 @@ pub enum Event { /// This event is generated when a payment has been successfully forwarded through us and a /// forwarding fee earned. PaymentForwarded { + /// The channel between the source node and us. Optional because versions prior to 0.0.107 + /// do not serialize this field. + source_channel_id: Option<[u8; 32]>, /// The fee, in milli-satoshis, which was earned as a result of the payment. /// /// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC @@ -520,10 +523,11 @@ impl Writeable for Event { (0, VecWriteWrapper(outputs), required), }); }, - &Event::PaymentForwarded { fee_earned_msat, claim_from_onchain_tx } => { + &Event::PaymentForwarded { fee_earned_msat, source_channel_id, claim_from_onchain_tx } => { 7u8.write(writer)?; write_tlv_fields!(writer, { (0, fee_earned_msat, option), + (1, source_channel_id, option), (2, claim_from_onchain_tx, required), }); }, @@ -684,12 +688,14 @@ impl MaybeReadable for Event { 7u8 => { let f = || { let mut fee_earned_msat = None; + let mut source_channel_id = None; let mut claim_from_onchain_tx = false; read_tlv_fields!(reader, { (0, fee_earned_msat, option), + (1, source_channel_id, option), (2, claim_from_onchain_tx, required), }); - Ok(Some(Event::PaymentForwarded { fee_earned_msat, claim_from_onchain_tx })) + Ok(Some(Event::PaymentForwarded { fee_earned_msat, source_channel_id, claim_from_onchain_tx })) }; f() }, -- 2.30.2