From a0adc51da1628a463b44f9e14ad537cbf2ffa9ba Mon Sep 17 00:00:00 2001 From: =?utf8?q?Viktor=20Tigerstr=C3=B6m?= <11711198+ViktorTigerstrom@users.noreply.github.com> Date: Thu, 12 Jan 2023 22:50:43 +0100 Subject: [PATCH] Don't clone the Vec in `remove_first_msg_event_to_node` --- lightning/src/ln/chanmon_update_fail_tests.rs | 6 ++--- lightning/src/ln/functional_test_utils.rs | 18 +++++-------- lightning/src/ln/functional_tests.rs | 26 +++++++++---------- lightning/src/ln/payment_tests.rs | 8 +++--- lightning/src/ln/reload_tests.rs | 4 +-- lightning/src/ln/reorg_tests.rs | 2 +- 6 files changed, 30 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 385f5b535..e1ea256a6 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -935,7 +935,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) { // Note that the ordering of the events for different nodes is non-prescriptive, though the // ordering of the two events that both go to nodes[2] have to stay in the same order. - let (nodes_0_event, events_3) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events_3); + let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events_3); let messages_a = match nodes_0_event { MessageSendEvent::UpdateHTLCs { node_id, mut updates } => { assert_eq!(node_id, nodes[0].node.get_our_node_id()); @@ -949,12 +949,12 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) { _ => panic!("Unexpected event type!"), }; - let (nodes_2_event, events_3) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events_3); + let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events_3); let send_event_b = SendEvent::from_event(nodes_2_event); assert_eq!(send_event_b.node_id, nodes[2].node.get_our_node_id()); let raa = if test_ignore_second_cs { - let (nodes_2_event, _events_3) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events_3); + let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events_3); match nodes_2_event { MessageSendEvent::SendRevokeAndACK { node_id, msg } => { assert_eq!(node_id, nodes[2].node.get_our_node_id()); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index c2ce158f8..760fc7f50 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -572,12 +572,12 @@ macro_rules! get_htlc_update_msgs { } /// Fetches the first `msg_event` to the passed `node_id` in the passed `msg_events` vec. -/// Returns the `msg_event`, along with an updated `msg_events` vec with the message removed. +/// Returns the `msg_event`. /// /// Note that even though `BroadcastChannelAnnouncement` and `BroadcastChannelUpdate` /// `msg_events` are stored under specific peers, this function does not fetch such `msg_events` as /// such messages are intended to all peers. -pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &Vec) -> (MessageSendEvent, Vec) { +pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &mut Vec) -> MessageSendEvent { let ev_index = msg_events.iter().position(|e| { match e { MessageSendEvent::SendAcceptChannel { node_id, .. } => { node_id == msg_node_id @@ -641,9 +641,7 @@ pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &Vec< }, }}); if ev_index.is_some() { - let mut updated_msg_events = msg_events.to_vec(); - let ev = updated_msg_events.remove(ev_index.unwrap()); - (ev, updated_msg_events) + msg_events.remove(ev_index.unwrap()) } else { panic!("Couldn't find any MessageSendEvent to the node!") } @@ -1482,9 +1480,9 @@ pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node< check_added_monitors!(node_b, 1); node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &as_commitment_signed); let (bs_revoke_and_ack, extra_msg_option) = { - let events = node_b.node.get_and_clear_pending_msg_events(); + let mut events = node_b.node.get_and_clear_pending_msg_events(); assert!(events.len() <= 2); - let (node_a_event, events) = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &events); + let node_a_event = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &mut events); (match node_a_event { MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { assert_eq!(*node_id, node_a.node.get_our_node_id()); @@ -1939,8 +1937,7 @@ pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou let mut events = origin_node.node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), expected_route.len()); for (path_idx, expected_path) in expected_route.iter().enumerate() { - let (ev, updated_events) = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &events); - events = updated_events; + let ev = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &mut events); // Once we've gotten through all the HTLCs, the last one should result in a // PaymentClaimable (but each previous one should not!), . let expect_payment = path_idx == expected_route.len() - 1; @@ -1999,9 +1996,8 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, } else { for expected_path in expected_paths.iter() { // For MPP payments, we always want the message to the first node in the path. - let (ev, updated_events) = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &events); + let ev = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &mut events); per_path_msgs.push(msgs_from_ev!(&ev)); - events = updated_events; } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index d20458407..eedde1e93 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2747,7 +2747,7 @@ fn test_htlc_on_chain_success() { }, _ => panic!() } - let events = nodes[1].node.get_and_clear_pending_msg_events(); + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); { let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); assert_eq!(added_monitors.len(), 2); @@ -2757,8 +2757,8 @@ fn test_htlc_on_chain_success() { } assert_eq!(events.len(), 3); - let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events); - let (nodes_0_event, events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events); + let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); + let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events); match nodes_2_event { MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {}, @@ -3196,14 +3196,15 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use _ => panic!("Unexpected event"), } } + nodes[1].node.process_pending_htlc_forwards(); check_added_monitors!(nodes[1], 1); let mut events = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), if deliver_bs_raa { 4 } else { 3 }); - let events = if deliver_bs_raa { - let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events); + if deliver_bs_raa { + let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); match nodes_2_event { MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, .. } } => { assert_eq!(nodes[2].node.get_our_node_id(), *node_id); @@ -3214,10 +3215,9 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use }, _ => panic!("Unexpected event"), } - events - } else { events }; + } - let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events); + let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); match nodes_2_event { MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => { assert_eq!(channel_id, chan_2.2); @@ -3226,7 +3226,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use _ => panic!("Unexpected event"), } - let (nodes_0_event, events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events); + let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events); match nodes_0_event { MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed, .. } } => { assert!(update_add_htlcs.is_empty()); @@ -4642,10 +4642,10 @@ fn test_onchain_to_onchain_claim() { _ => panic!("Unexpected event"), } check_added_monitors!(nodes[1], 1); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + let mut msg_events = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 3); - let (nodes_2_event, msg_events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &msg_events); - let (nodes_0_event, msg_events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &msg_events); + let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut msg_events); + let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut msg_events); match nodes_2_event { MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {}, @@ -9284,7 +9284,7 @@ fn test_double_partial_claim() { let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 2); - let (node_1_msgs, _events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events); + let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, false, None); // At this point nodes[3] has received one half of the payment, and the user goes to handle diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index c6b170364..1c06c0b32 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -153,11 +153,11 @@ fn mpp_retry() { assert_eq!(events.len(), 2); // Pass half of the payment along the success path. - let (success_path_msgs, mut events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events); + let success_path_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 2_000_000, payment_hash, Some(payment_secret), success_path_msgs, false, None); // Add the HTLC along the first hop. - let (fail_path_msgs_1, _events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events); + let fail_path_msgs_1 = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); let (update_add, commitment_signed) = match fail_path_msgs_1 { MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => { assert_eq!(update_add_htlcs.len(), 1); @@ -237,7 +237,7 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) { assert_eq!(events.len(), 2); // Pass half of the payment along the first path. - let (node_1_msgs, mut events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events); + let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 200_000, payment_hash, Some(payment_secret), node_1_msgs, false, None); if send_partial_mpp { @@ -265,7 +265,7 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) { expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain().expected_htlc_error_data(23, &[][..])); } else { // Pass half of the payment along the second path. - let (node_2_msgs, _events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events); + let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash, Some(payment_secret), node_2_msgs, true, None); // Even after MPP_TIMEOUT_TICKS we should not timeout the MPP if we have all the parts diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index 2ca3f21fe..dcdd43dbb 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -701,8 +701,8 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) { // Send the payment through to nodes[3] *without* clearing the PaymentClaimable event let mut send_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(send_events.len(), 2); - let (node_1_msgs, mut send_events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &send_events); - let (node_2_msgs, _send_events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &send_events); + let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut send_events); + let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut send_events); do_pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, true, false, None); do_pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_2_msgs, true, false, None); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 55c1341c3..bc1b996f5 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -14,7 +14,7 @@ use crate::chain::transaction::OutPoint; use crate::chain::Confirm; use crate::ln::channelmanager::ChannelManager; use crate::ln::msgs::ChannelMessageHandler; -use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination}; +use crate::util::events::{Event, MessageSendEventsProvider, ClosureReason, HTLCDestination}; use crate::util::test_utils; use crate::util::ser::Writeable; -- 2.39.5