Skip to content

Commit 8683df9

Browse files
TheBlueMattTheBlueMatt
committed
Remove more .node.get_our_node_id() cruft in async_signer_tests
Having tons of `.node.get_our_nod_id()` spewn across all our tests doesn't help readability, so here we replace some further cases of it with local variables in `async_signer_tests.rs`.
1 parent 808d1dc commit 8683df9

File tree

1 file changed

+46
-74
lines changed

1 file changed

+46
-74
lines changed

‎lightning/src/ln/async_signer_tests.rs

Lines changed: 46 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,9 @@ fn do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(
294294
// Send a payment.
295295
let src = &nodes[0];
296296
let dst = &nodes[1];
297+
let src_node_id = src.node.get_our_node_id();
298+
let dst_node_id = dst.node.get_our_node_id();
299+
297300
let (route, our_payment_hash, _our_payment_preimage, our_payment_secret) =
298301
get_route_and_payment_hash!(src, dst, 8000000);
299302
let recipient_fields = RecipientOnionFields::secret_only(our_payment_secret);
@@ -309,52 +312,37 @@ fn do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(
309312
assert_eq!(events.len(), 1);
310313
SendEvent::from_event(events.remove(0))
311314
};
312-
assert_eq!(payment_event.node_id, dst.node.get_our_node_id());
315+
assert_eq!(payment_event.node_id, dst_node_id);
313316
assert_eq!(payment_event.msgs.len(), 1);
314317

315-
dst.node.handle_update_add_htlc(src.node.get_our_node_id(), &payment_event.msgs[0]);
318+
dst.node.handle_update_add_htlc(src_node_id, &payment_event.msgs[0]);
316319

317320
// Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a
318321
// `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`.
319-
dst.disable_channel_signer_op(
320-
&src.node.get_our_node_id(),
321-
&chan_id,
322-
SignerOp::GetPerCommitmentPoint,
323-
);
324-
dst.disable_channel_signer_op(
325-
&src.node.get_our_node_id(),
326-
&chan_id,
327-
SignerOp::ReleaseCommitmentSecret,
328-
);
329-
dst.disable_channel_signer_op(
330-
&src.node.get_our_node_id(),
331-
&chan_id,
332-
SignerOp::SignCounterpartyCommitment,
333-
);
334-
dst.node.handle_commitment_signed_batch_test(
335-
src.node.get_our_node_id(),
336-
&payment_event.commitment_msg,
337-
);
322+
dst.disable_channel_signer_op(&src_node_id, &chan_id, SignerOp::GetPerCommitmentPoint);
323+
dst.disable_channel_signer_op(&src_node_id, &chan_id, SignerOp::ReleaseCommitmentSecret);
324+
dst.disable_channel_signer_op(&src_node_id, &chan_id, SignerOp::SignCounterpartyCommitment);
325+
dst.node.handle_commitment_signed_batch_test(src_node_id, &payment_event.commitment_msg);
338326
check_added_monitors(dst, 1);
339327

340328
let mut enabled_signer_ops = new_hash_set();
341329
log_trace!(dst.logger, "enable_signer_op_order={:?}", enable_signer_op_order);
342330
for op in enable_signer_op_order {
343331
enabled_signer_ops.insert(op);
344-
dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, op);
345-
dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id)));
332+
dst.enable_channel_signer_op(&src_node_id, &chan_id, op);
333+
dst.node.signer_unblocked(Some((src_node_id, chan_id)));
346334

347335
if enabled_signer_ops.contains(&SignerOp::GetPerCommitmentPoint)
348336
&& enabled_signer_ops.contains(&SignerOp::ReleaseCommitmentSecret)
349337
{
350338
// We are just able to send revoke_and_ack
351339
if op == SignerOp::GetPerCommitmentPoint || op == SignerOp::ReleaseCommitmentSecret {
352-
get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id());
340+
get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src_node_id);
353341
}
354342
// We either just sent or previously sent revoke_and_ack
355343
// and now we are able to send commitment_signed
356344
if op == SignerOp::SignCounterpartyCommitment {
357-
get_htlc_update_msgs(dst, &src.node.get_our_node_id());
345+
get_htlc_update_msgs(dst, &src_node_id);
358346
}
359347
} else {
360348
// We can't send either message until RAA is unblocked
@@ -533,6 +521,9 @@ fn do_test_async_raa_peer_disconnect(
533521
// Send a payment.
534522
let src = &nodes[0];
535523
let dst = &nodes[1];
524+
let src_node_id = src.node.get_our_node_id();
525+
let dst_node_id = dst.node.get_our_node_id();
526+
536527
let (route, our_payment_hash, _our_payment_preimage, our_payment_secret) =
537528
get_route_and_payment_hash!(src, dst, 8000000);
538529
let recipient_fields = RecipientOnionFields::secret_only(our_payment_secret);
@@ -548,10 +539,10 @@ fn do_test_async_raa_peer_disconnect(
548539
assert_eq!(events.len(), 1);
549540
SendEvent::from_event(events.remove(0))
550541
};
551-
assert_eq!(payment_event.node_id, dst.node.get_our_node_id());
542+
assert_eq!(payment_event.node_id, dst_node_id);
552543
assert_eq!(payment_event.msgs.len(), 1);
553544

554-
dst.node.handle_update_add_htlc(src.node.get_our_node_id(), &payment_event.msgs[0]);
545+
dst.node.handle_update_add_htlc(src_node_id, &payment_event.msgs[0]);
555546

556547
if test_case == UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored {
557548
// Fail to persist the monitor update when handling the commitment_signed.
@@ -560,47 +551,44 @@ fn do_test_async_raa_peer_disconnect(
560551

561552
// Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a
562553
// `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`.
563-
dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op);
564-
dst.node.handle_commitment_signed_batch_test(
565-
src.node.get_our_node_id(),
566-
&payment_event.commitment_msg,
567-
);
554+
dst.disable_channel_signer_op(&src_node_id, &chan_id, block_raa_signer_op);
555+
dst.node.handle_commitment_signed_batch_test(src_node_id, &payment_event.commitment_msg);
568556
check_added_monitors(dst, 1);
569557

570558
let events = dst.node.get_and_clear_pending_msg_events();
571559
assert!(events.is_empty(), "expected no message, got {}", events.len());
572560

573561
// Now disconnect and reconnect the peers.
574-
src.node.peer_disconnected(dst.node.get_our_node_id());
575-
dst.node.peer_disconnected(src.node.get_our_node_id());
562+
src.node.peer_disconnected(dst_node_id);
563+
dst.node.peer_disconnected(src_node_id);
576564

577565
// do reestablish stuff
578566
let init_msg = &msgs::Init {
579567
features: dst.node.init_features(),
580568
networks: None,
581569
remote_network_address: None,
582570
};
583-
src.node.peer_connected(dst.node.get_our_node_id(), init_msg, true).unwrap();
571+
src.node.peer_connected(dst_node_id, init_msg, true).unwrap();
584572
let reestablish_1 = get_chan_reestablish_msgs!(src, dst);
585573
assert_eq!(reestablish_1.len(), 1);
586574
let init_msg = &msgs::Init {
587575
features: src.node.init_features(),
588576
networks: None,
589577
remote_network_address: None,
590578
};
591-
dst.node.peer_connected(src.node.get_our_node_id(), init_msg, false).unwrap();
579+
dst.node.peer_connected(src_node_id, init_msg, false).unwrap();
592580
let reestablish_2 = get_chan_reestablish_msgs!(dst, src);
593581
assert_eq!(reestablish_2.len(), 1);
594582

595583
if test_case == UnblockSignerAcrossDisconnectCase::BeforeReestablish {
596584
// Reenable the signer before the reestablish.
597-
dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op);
585+
dst.enable_channel_signer_op(&src_node_id, &chan_id, block_raa_signer_op);
598586
}
599587

600-
dst.node.handle_channel_reestablish(src.node.get_our_node_id(), &reestablish_1[0]);
588+
dst.node.handle_channel_reestablish(src_node_id, &reestablish_1[0]);
601589

602590
if test_case == UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored {
603-
dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op);
591+
dst.enable_channel_signer_op(&src_node_id, &chan_id, block_raa_signer_op);
604592
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
605593
let latest_update;
606594
{
@@ -624,8 +612,8 @@ fn do_test_async_raa_peer_disconnect(
624612
}
625613

626614
// Mark dst's signer as available and retry: we now expect to see dst's RAA + CS.
627-
dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op);
628-
dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id)));
615+
dst.enable_channel_signer_op(&src_node_id, &chan_id, block_raa_signer_op);
616+
dst.node.signer_unblocked(Some((src_node_id, chan_id)));
629617

630618
if test_case == UnblockSignerAcrossDisconnectCase::AtEnd {
631619
let (_, revoke_and_ack, commitment_signed, resend_order) =
@@ -681,6 +669,9 @@ fn do_test_async_commitment_signature_peer_disconnect(
681669
// Send a payment.
682670
let src = &nodes[0];
683671
let dst = &nodes[1];
672+
let src_node_id = src.node.get_our_node_id();
673+
let dst_node_id = dst.node.get_our_node_id();
674+
684675
let (route, our_payment_hash, _our_payment_preimage, our_payment_secret) =
685676
get_route_and_payment_hash!(src, dst, 8000000);
686677
let recipient_fields = RecipientOnionFields::secret_only(our_payment_secret);
@@ -696,10 +687,10 @@ fn do_test_async_commitment_signature_peer_disconnect(
696687
assert_eq!(events.len(), 1);
697688
SendEvent::from_event(events.remove(0))
698689
};
699-
assert_eq!(payment_event.node_id, dst.node.get_our_node_id());
690+
assert_eq!(payment_event.node_id, dst_node_id);
700691
assert_eq!(payment_event.msgs.len(), 1);
701692

702-
dst.node.handle_update_add_htlc(src.node.get_our_node_id(), &payment_event.msgs[0]);
693+
dst.node.handle_update_add_htlc(src_node_id, &payment_event.msgs[0]);
703694

704695
if test_case == UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored {
705696
// Fail to persist the monitor update when handling the commitment_signed.
@@ -708,60 +699,45 @@ fn do_test_async_commitment_signature_peer_disconnect(
708699

709700
// Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a
710701
// `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`.
711-
dst.disable_channel_signer_op(
712-
&src.node.get_our_node_id(),
713-
&chan_id,
714-
SignerOp::SignCounterpartyCommitment,
715-
);
716-
dst.node.handle_commitment_signed_batch_test(
717-
src.node.get_our_node_id(),
718-
&payment_event.commitment_msg,
719-
);
702+
dst.disable_channel_signer_op(&src_node_id, &chan_id, SignerOp::SignCounterpartyCommitment);
703+
dst.node.handle_commitment_signed_batch_test(src_node_id, &payment_event.commitment_msg);
720704
check_added_monitors(dst, 1);
721705

722706
if test_case != UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored {
723-
get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id());
707+
get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src_node_id);
724708
}
725709

726710
// Now disconnect and reconnect the peers.
727-
src.node.peer_disconnected(dst.node.get_our_node_id());
728-
dst.node.peer_disconnected(src.node.get_our_node_id());
711+
src.node.peer_disconnected(dst_node_id);
712+
dst.node.peer_disconnected(src_node_id);
729713

730714
// do reestablish stuff
731715
let init_msg = &msgs::Init {
732716
features: dst.node.init_features(),
733717
networks: None,
734718
remote_network_address: None,
735719
};
736-
src.node.peer_connected(dst.node.get_our_node_id(), init_msg, true).unwrap();
720+
src.node.peer_connected(dst_node_id, init_msg, true).unwrap();
737721
let reestablish_1 = get_chan_reestablish_msgs!(src, dst);
738722
assert_eq!(reestablish_1.len(), 1);
739723
let init_msg = &msgs::Init {
740724
features: src.node.init_features(),
741725
networks: None,
742726
remote_network_address: None,
743727
};
744-
dst.node.peer_connected(src.node.get_our_node_id(), init_msg, false).unwrap();
728+
dst.node.peer_connected(src_node_id, init_msg, false).unwrap();
745729
let reestablish_2 = get_chan_reestablish_msgs!(dst, src);
746730
assert_eq!(reestablish_2.len(), 1);
747731

748732
if test_case == UnblockSignerAcrossDisconnectCase::BeforeReestablish {
749733
// Reenable the signer before the reestablish.
750-
dst.enable_channel_signer_op(
751-
&src.node.get_our_node_id(),
752-
&chan_id,
753-
SignerOp::SignCounterpartyCommitment,
754-
);
734+
dst.enable_channel_signer_op(&src_node_id, &chan_id, SignerOp::SignCounterpartyCommitment);
755735
}
756736

757-
dst.node.handle_channel_reestablish(src.node.get_our_node_id(), &reestablish_1[0]);
737+
dst.node.handle_channel_reestablish(src_node_id, &reestablish_1[0]);
758738

759739
if test_case == UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored {
760-
dst.enable_channel_signer_op(
761-
&src.node.get_our_node_id(),
762-
&chan_id,
763-
SignerOp::SignCounterpartyCommitment,
764-
);
740+
dst.enable_channel_signer_op(&src_node_id, &chan_id, SignerOp::SignCounterpartyCommitment);
765741
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
766742
let latest_update;
767743
{
@@ -782,12 +758,8 @@ fn do_test_async_commitment_signature_peer_disconnect(
782758
}
783759

784760
// Mark dst's signer as available and retry: we now expect to see dst's `commitment_signed`.
785-
dst.enable_channel_signer_op(
786-
&src.node.get_our_node_id(),
787-
&chan_id,
788-
SignerOp::SignCounterpartyCommitment,
789-
);
790-
dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id)));
761+
dst.enable_channel_signer_op(&src_node_id, &chan_id, SignerOp::SignCounterpartyCommitment);
762+
dst.node.signer_unblocked(Some((src_node_id, chan_id)));
791763

792764
if test_case == UnblockSignerAcrossDisconnectCase::AtEnd {
793765
let (_, _, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src);

0 commit comments

Comments
 (0)