Skip to content

Commit d5286bd

Browse files
ViktorTigerstromViktorTigerstrom
committed
Don't clone the Vec in remove_first_msg_event_to_node 
1 parent 5263c67 commit d5286bd

File tree

6 files changed

+49
-42
lines changed

6 files changed

+49
-42
lines changed

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,8 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
926926
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_2.2 }]);
927927
check_added_monitors!(nodes[1], 1);
928928

929-
let mut events_3 = nodes[1].node.get_and_clear_pending_msg_events();
929+
let mut events_3_vec = nodes[1].node.get_and_clear_pending_msg_events();
930+
let events_3 = &mut events_3_vec;
930931
if test_ignore_second_cs {
931932
assert_eq!(events_3.len(), 3);
932933
} else {
@@ -935,7 +936,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
935936

936937
// Note that the ordering of the events for different nodes is non-prescriptive, though the
937938
// ordering of the two events that both go to nodes[2] have to stay in the same order.
938-
let (nodes_0_event, events_3) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events_3);
939+
let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), events_3);
939940
let messages_a = match nodes_0_event {
940941
MessageSendEvent::UpdateHTLCs { node_id, mut updates } => {
941942
assert_eq!(node_id, nodes[0].node.get_our_node_id());
@@ -949,12 +950,12 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
949950
_ => panic!("Unexpected event type!"),
950951
};
951952

952-
let (nodes_2_event, events_3) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events_3);
953+
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events_3);
953954
let send_event_b = SendEvent::from_event(nodes_2_event);
954955
assert_eq!(send_event_b.node_id, nodes[2].node.get_our_node_id());
955956

956957
let raa = if test_ignore_second_cs {
957-
let (nodes_2_event, _events_3) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events_3);
958+
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events_3);
958959
match nodes_2_event {
959960
MessageSendEvent::SendRevokeAndACK { node_id, msg } => {
960961
assert_eq!(node_id, nodes[2].node.get_our_node_id());

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -572,12 +572,12 @@ macro_rules! get_htlc_update_msgs {
572572
}
573573

574574
/// Fetches the first `msg_event` to the passed `node_id` in the passed `msg_events` vec.
575-
/// Returns the `msg_event`, along with an updated `msg_events` vec with the message removed.
575+
/// Returns the `msg_event`.
576576
///
577577
/// Note that even though `BroadcastChannelAnnouncement` and `BroadcastChannelUpdate`
578578
/// `msg_events` are stored under specific peers, this function does not fetch such `msg_events` as
579579
/// such messages are intended to all peers.
580-
pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &Vec<MessageSendEvent>) -> (MessageSendEvent, Vec<MessageSendEvent>) {
580+
pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &mut Vec<MessageSendEvent>) -> MessageSendEvent {
581581
let ev_index = msg_events.iter().position(|e| { match e {
582582
MessageSendEvent::SendAcceptChannel { node_id, .. } => {
583583
node_id == msg_node_id
@@ -641,9 +641,7 @@ pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &Vec<
641641
},
642642
}});
643643
if ev_index.is_some() {
644-
let mut updated_msg_events = msg_events.to_vec();
645-
let ev = updated_msg_events.remove(ev_index.unwrap());
646-
(ev, updated_msg_events)
644+
msg_events.remove(ev_index.unwrap())
647645
} else {
648646
panic!("Couldn't find any MessageSendEvent to the node!")
649647
}
@@ -1482,9 +1480,10 @@ pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<
14821480
check_added_monitors!(node_b, 1);
14831481
node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &as_commitment_signed);
14841482
let (bs_revoke_and_ack, extra_msg_option) = {
1485-
let events = node_b.node.get_and_clear_pending_msg_events();
1483+
let mut events_vec = node_b.node.get_and_clear_pending_msg_events();
1484+
let events = &mut events_vec;
14861485
assert!(events.len() <= 2);
1487-
let (node_a_event, events) = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &events);
1486+
let node_a_event = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), events);
14881487
(match node_a_event {
14891488
MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
14901489
assert_eq!(*node_id, node_a.node.get_our_node_id());
@@ -1936,11 +1935,11 @@ pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path
19361935
}
19371936

19381937
pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) {
1939-
let mut events = origin_node.node.get_and_clear_pending_msg_events();
1938+
let mut events_vec = origin_node.node.get_and_clear_pending_msg_events();
1939+
let events = &mut events_vec;
19401940
assert_eq!(events.len(), expected_route.len());
19411941
for (path_idx, expected_path) in expected_route.iter().enumerate() {
1942-
let (ev, updated_events) = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &events);
1943-
events = updated_events;
1942+
let ev = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), events);
19441943
// Once we've gotten through all the HTLCs, the last one should result in a
19451944
// PaymentClaimable (but each previous one should not!), .
19461945
let expect_payment = path_idx == expected_route.len() - 1;
@@ -1991,17 +1990,17 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
19911990
}
19921991
}
19931992
let mut per_path_msgs: Vec<((msgs::UpdateFulfillHTLC, msgs::CommitmentSigned), PublicKey)> = Vec::with_capacity(expected_paths.len());
1994-
let mut events = expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events();
1993+
let mut events_vec = expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events();
1994+
let events = &mut events_vec;
19951995
assert_eq!(events.len(), expected_paths.len());
19961996

19971997
if events.len() == 1 {
19981998
per_path_msgs.push(msgs_from_ev!(&events[0]));
19991999
} else {
20002000
for expected_path in expected_paths.iter() {
20012001
// For MPP payments, we always want the message to the first node in the path.
2002-
let (ev, updated_events) = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &events);
2002+
let ev = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), events);
20032003
per_path_msgs.push(msgs_from_ev!(&ev));
2004-
events = updated_events;
20052004
}
20062005
}
20072006

‎lightning/src/ln/functional_tests.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2747,7 +2747,8 @@ fn test_htlc_on_chain_success() {
27472747
},
27482748
_ => panic!()
27492749
}
2750-
let events = nodes[1].node.get_and_clear_pending_msg_events();
2750+
let mut events_vec = nodes[1].node.get_and_clear_pending_msg_events();
2751+
let events = &mut events_vec;
27512752
{
27522753
let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
27532754
assert_eq!(added_monitors.len(), 2);
@@ -2757,8 +2758,8 @@ fn test_htlc_on_chain_success() {
27572758
}
27582759
assert_eq!(events.len(), 3);
27592760

2760-
let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
2761-
let (nodes_0_event, events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events);
2761+
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events);
2762+
let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), events);
27622763

27632764
match nodes_2_event {
27642765
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
@@ -3196,14 +3197,16 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
31963197
_ => panic!("Unexpected event"),
31973198
}
31983199
}
3200+
31993201
nodes[1].node.process_pending_htlc_forwards();
32003202
check_added_monitors!(nodes[1], 1);
32013203

3202-
let mut events = nodes[1].node.get_and_clear_pending_msg_events();
3204+
let mut events_vec = nodes[1].node.get_and_clear_pending_msg_events();
3205+
let events = &mut events_vec;
32033206
assert_eq!(events.len(), if deliver_bs_raa { 4 } else { 3 });
32043207

3205-
let events = if deliver_bs_raa {
3206-
let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
3208+
if deliver_bs_raa {
3209+
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events);
32073210
match nodes_2_event {
32083211
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, .. } } => {
32093212
assert_eq!(nodes[2].node.get_our_node_id(), *node_id);
@@ -3214,10 +3217,9 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
32143217
},
32153218
_ => panic!("Unexpected event"),
32163219
}
3217-
events
3218-
} else { events };
3220+
}
32193221

3220-
let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
3222+
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events);
32213223
match nodes_2_event {
32223224
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => {
32233225
assert_eq!(channel_id, chan_2.2);
@@ -3226,7 +3228,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
32263228
_ => panic!("Unexpected event"),
32273229
}
32283230

3229-
let (nodes_0_event, events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events);
3231+
let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), events);
32303232
match nodes_0_event {
32313233
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, .. } } => {
32323234
assert!(update_add_htlcs.is_empty());
@@ -4642,10 +4644,11 @@ fn test_onchain_to_onchain_claim() {
46424644
_ => panic!("Unexpected event"),
46434645
}
46444646
check_added_monitors!(nodes[1], 1);
4645-
let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
4647+
let mut msg_events_vec = nodes[1].node.get_and_clear_pending_msg_events();
4648+
let msg_events = &mut msg_events_vec;
46464649
assert_eq!(msg_events.len(), 3);
4647-
let (nodes_2_event, msg_events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &msg_events);
4648-
let (nodes_0_event, msg_events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &msg_events);
4650+
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), msg_events);
4651+
let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), msg_events);
46494652

46504653
match nodes_2_event {
46514654
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
@@ -9282,9 +9285,10 @@ fn test_double_partial_claim() {
92829285
nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap();
92839286
check_added_monitors!(nodes[0], 2);
92849287

9285-
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
9288+
let mut events_vec = nodes[0].node.get_and_clear_pending_msg_events();
9289+
let events = &mut events_vec;
92869290
assert_eq!(events.len(), 2);
9287-
let (node_1_msgs, _events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events);
9291+
let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), events);
92889292
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, false, None);
92899293

92909294
// At this point nodes[3] has received one half of the payment, and the user goes to handle

‎lightning/src/ln/payment_tests.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,16 @@ fn mpp_retry() {
149149
let payment_id = PaymentId(payment_hash.0);
150150
nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), payment_id).unwrap();
151151
check_added_monitors!(nodes[0], 2); // one monitor per path
152-
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
152+
let mut events_vec = nodes[0].node.get_and_clear_pending_msg_events();
153+
let events = &mut events_vec;
153154
assert_eq!(events.len(), 2);
154155

155156
// Pass half of the payment along the success path.
156-
let (success_path_msgs, mut events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events);
157+
let success_path_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), events);
157158
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 2_000_000, payment_hash, Some(payment_secret), success_path_msgs, false, None);
158159

159160
// Add the HTLC along the first hop.
160-
let (fail_path_msgs_1, _events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
161+
let fail_path_msgs_1 = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events);
161162
let (update_add, commitment_signed) = match fail_path_msgs_1 {
162163
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 } } => {
163164
assert_eq!(update_add_htlcs.len(), 1);
@@ -233,11 +234,12 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
233234
// Initiate the MPP payment.
234235
nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap();
235236
check_added_monitors!(nodes[0], 2); // one monitor per path
236-
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
237+
let mut events_vec = nodes[0].node.get_and_clear_pending_msg_events();
238+
let events = &mut events_vec;
237239
assert_eq!(events.len(), 2);
238240

239241
// Pass half of the payment along the first path.
240-
let (node_1_msgs, mut events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events);
242+
let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), events);
241243
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 200_000, payment_hash, Some(payment_secret), node_1_msgs, false, None);
242244

243245
if send_partial_mpp {
@@ -265,7 +267,7 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
265267
expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain().expected_htlc_error_data(23, &[][..]));
266268
} else {
267269
// Pass half of the payment along the second path.
268-
let (node_2_msgs, _events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
270+
let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), events);
269271
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash, Some(payment_secret), node_2_msgs, true, None);
270272

271273
// Even after MPP_TIMEOUT_TICKS we should not timeout the MPP if we have all the parts

‎lightning/src/ln/reload_tests.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -699,10 +699,11 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
699699
check_added_monitors!(nodes[0], 2);
700700

701701
// Send the payment through to nodes[3] *without* clearing the PaymentClaimable event
702-
let mut send_events = nodes[0].node.get_and_clear_pending_msg_events();
702+
let mut send_events_vec = nodes[0].node.get_and_clear_pending_msg_events();
703+
let send_events = &mut send_events_vec;
703704
assert_eq!(send_events.len(), 2);
704-
let (node_1_msgs, mut send_events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &send_events);
705-
let (node_2_msgs, _send_events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &send_events);
705+
let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), send_events);
706+
let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), send_events);
706707
do_pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, true, false, None);
707708
do_pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_2_msgs, true, false, None);
708709

‎lightning/src/ln/reorg_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::chain::transaction::OutPoint;
1414
use crate::chain::Confirm;
1515
use crate::ln::channelmanager::ChannelManager;
1616
use crate::ln::msgs::ChannelMessageHandler;
17-
use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
17+
use crate::util::events::{Event, MessageSendEventsProvider, ClosureReason, HTLCDestination};
1818
use crate::util::test_utils;
1919
use crate::util::ser::Writeable;
2020

0 commit comments

Comments
 (0)