← Watch feed
Make get_latest_mon_update_id a helper on TestChainMonitor
Public commit record
50/100 · Thin
What the developer wrote
Authored by Wilmer Paulino
Make get_latest_mon_update_id a helper on TestChainMonitor
✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This commit is a simple code cleanup in the project's test utilities. It moves a small helper function that reads the latest monitor update ID from one test file into a shared test helper struct, so other tests can call it more cleanly. There is no change to production code, no security fix, and no behavior change.
Recommended action
No security action needed. This is a benign test-only refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff refactors get_latest_mon_update_id from a free function in chanmon_update_fail_tests.rs into a method on TestChainMonitor in test_utils.rs. All call sites are updated from get_latest_mon_update_id(&node, channel_id) to node.chain_monitor.get_latest_mon_update_id(channel_id). The implementation is identical: it locks latest_monitor_update_id, looks up the channel, and clones the (u64, u64) tuple. This only affects test code under #[cfg(test)]-style modules.
Changed components
lightning/src/ln/chanmon_update_fail_tests.rslightning/src/util/test_utils.rsInspect captured patch +37 / −39
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 3fa2073..8e61692 100644
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -49,13 +49,6 @@ use crate::prelude::*;
use crate::sync::{Arc, Mutex};
use bitcoin::hashes::Hash;
-fn get_latest_mon_update_id<'a, 'b, 'c>(
- node: &Node<'a, 'b, 'c>, channel_id: ChannelId,
-) -> (u64, u64) {
- let monitor_id_state = node.chain_monitor.latest_monitor_update_id.lock().unwrap();
- monitor_id_state.get(&channel_id).unwrap().clone()
-}
-
#[test]
fn test_monitor_and_persister_update_fail() {
// Test that if both updating the `ChannelMonitor` and persisting the updated
@@ -212,7 +205,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
}
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[0], 0);
@@ -404,7 +397,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
// Now fix monitor updating...
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[0], 0);
@@ -757,7 +750,7 @@ fn test_monitor_update_fail_cs() {
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[1], 0);
let responses = nodes[1].node.get_and_clear_pending_msg_events();
@@ -792,7 +785,7 @@ fn test_monitor_update_fail_cs() {
}
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[0], 0);
@@ -868,7 +861,7 @@ fn test_monitor_update_fail_no_rebroadcast() {
check_added_monitors(&nodes[1], 1);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
check_added_monitors(&nodes[1], 0);
@@ -938,7 +931,7 @@ fn test_monitor_update_raa_while_paused() {
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
check_added_monitors(&nodes[0], 1);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[0], 0);
@@ -1080,7 +1073,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
// Restore monitor updating, ensuring we immediately get a fail-back update and a
// update_add update.
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_2.2);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_2.2);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_2.2, latest_update);
check_added_monitors(&nodes[1], 0);
expect_and_process_pending_htlcs_and_htlc_handling_failed(
@@ -1354,7 +1347,7 @@ fn test_monitor_update_fail_reestablish() {
assert_eq!(bs_channel_upd.contents.channel_flags & 2, 0);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_1.2);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_1.2);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_1.2, latest_update);
check_added_monitors(&nodes[1], 0);
@@ -1439,7 +1432,7 @@ fn raa_no_response_awaiting_raa_state() {
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
check_added_monitors(&nodes[1], 1);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
// nodes[1] should be AwaitingRAA here!
check_added_monitors(&nodes[1], 0);
@@ -1568,7 +1561,7 @@ fn claim_while_disconnected_monitor_update_fail() {
// Now un-fail the monitor, which will result in B sending its original commitment update,
// receiving the commitment update from A, and the resulting commitment dances.
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[1], 0);
@@ -1697,7 +1690,7 @@ fn monitor_failed_no_reestablish_response() {
get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, node_b_id);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[1], 0);
let bs_responses = get_revoke_commit_msgs(&nodes[1], &node_a_id);
@@ -1795,7 +1788,7 @@ fn first_message_on_recv_ordering() {
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[1], 0);
@@ -1894,7 +1887,7 @@ fn test_monitor_update_fail_claim() {
// Now restore monitor updating on the 0<->1 channel and claim the funds on B.
let channel_id = chan_1.2;
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
check_added_monitors(&nodes[1], 0);
@@ -2023,7 +2016,7 @@ fn test_monitor_update_on_pending_forwards() {
check_added_monitors(&nodes[1], 1);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_1.2);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_1.2);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_1.2, latest_update);
check_added_monitors(&nodes[1], 0);
@@ -2094,7 +2087,7 @@ fn monitor_update_claim_fail_no_response() {
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
check_added_monitors(&nodes[1], 0);
@@ -2166,7 +2159,7 @@ fn do_during_funding_monitor_fail(
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[0], 0);
expect_channel_pending_event(&nodes[0], &node_b_id);
@@ -2221,7 +2214,7 @@ fn do_during_funding_monitor_fail(
}
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
check_added_monitors(&nodes[1], 0);
@@ -2339,7 +2332,7 @@ fn test_path_paused_mpp() {
// And check that, after we successfully update the monitor for chan_2 we can pass the second
// HTLC along to nodes[3] and claim the whole payment back to nodes[0].
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], chan_2_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(chan_2_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_2_id, latest_update);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -2787,7 +2780,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
// If we finish updating the monitor, we should free the holding cell right away (this did
// not occur prior to #756). This should result in a new monitor update.
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (mon_id, _) = get_latest_mon_update_id(&nodes[0], chan_id);
+ let (mon_id, _) = nodes[0].chain_monitor.get_latest_mon_update_id(chan_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_id, mon_id);
expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);
check_added_monitors(&nodes[0], 1);
@@ -3039,7 +3032,7 @@ fn test_temporary_error_during_shutdown() {
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
+ let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
nodes[1].node.handle_closing_signed(
node_a_id,
@@ -3049,7 +3042,7 @@ fn test_temporary_error_during_shutdown() {
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
nodes[0].node.handle_closing_signed(
@@ -3095,7 +3088,7 @@ fn double_temp_error() {
// `claim_funds` results in a ChannelMonitorUpdate.
nodes[1].node.claim_funds(payment_preimage_1);
check_added_monitors(&nodes[1], 1);
- let (latest_update_1, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update_1, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
@@ -3104,7 +3097,7 @@ fn double_temp_error() {
check_added_monitors(&nodes[1], 1);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
- let (latest_update_2, _) = get_latest_mon_update_id(&nodes[1], channel_id);
+ let (latest_update_2, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update_1);
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
check_added_monitors(&nodes[1], 0);
@@ -3511,7 +3504,7 @@ fn do_test_blocked_chan_preimage_release(completion_mode: BlockedUpdateComplMode
reconnect_nodes(a_b_reconnect);
reconnect_nodes(ReconnectArgs::new(&nodes[2], &nodes[1]));
} else if completion_mode == BlockedUpdateComplMode::Async {
- let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_id_2);
+ let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_2);
nodes[1]
.chain_monitor
.chain_monitor
@@ -3689,7 +3682,7 @@ fn do_test_inverted_mon_completion_order(
// (Finally) complete the A <-> B ChannelMonitorUpdate, ensuring the preimage is durably on
// disk in the proper ChannelMonitor, unblocking the B <-> C ChannelMonitor updating
// process.
- let (_, ab_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
+ let (_, ab_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
nodes[1]
.chain_monitor
.chain_monitor
@@ -3722,7 +3715,7 @@ fn do_test_inverted_mon_completion_order(
// ChannelMonitorUpdate hasn't yet completed.
reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
- let (_, ab_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
+ let (_, ab_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
nodes[1]
.chain_monitor
.chain_monitor
@@ -3935,7 +3928,7 @@ fn do_test_durable_preimages_on_closed_channel(
// Once the blocked `ChannelMonitorUpdate` *finally* completes, the pending
// `PaymentForwarded` event will finally be released.
- let (_, ab_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
+ let (_, ab_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_id_ab, ab_update_id);
// If the A<->B channel was closed before we reload, we'll replay the claim against it on
@@ -4047,7 +4040,7 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) {
mine_transaction_without_consistency_checks(&nodes[1], &as_closing_tx[0]);
}
- let (_, bc_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_bc);
+ let (_, bc_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
let mut events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), if close_during_reload { 2 } else { 1 });
expect_payment_forwarded(
@@ -4072,7 +4065,7 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) {
// Once we run event processing the monitor should free, check that it was indeed the B<->C
// channel which was updated.
check_added_monitors(&nodes[1], if close_during_reload { 2 } else { 1 });
- let (_, post_ev_bc_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_bc);
+ let (_, post_ev_bc_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
assert!(bc_update_id != post_ev_bc_update_id);
// Finally, check that there's nothing left to do on B<->C reconnect and the channel operates
@@ -4162,7 +4155,7 @@ fn do_test_glacial_peer_cant_hang(hold_chan_a: bool) {
// ...but once we complete the A<->B channel preimage persistence, the B<->C channel
// unlocks and we send both peers commitment updates.
- let (ab_update_id, _) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
+ let (ab_update_id, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
assert!(nodes[1]
.chain_monitor
.chain_monitor
@@ -5122,7 +5115,7 @@ fn test_mpp_claim_to_holding_cell() {
check_added_monitors(&nodes[3], 2);
// Complete the B <-> D monitor update, freeing the first fulfill.
- let (latest_id, _) = get_latest_mon_update_id(&nodes[3], chan_3_id);
+ let (latest_id, _) = nodes[3].chain_monitor.get_latest_mon_update_id(chan_3_id);
nodes[3].chain_monitor.chain_monitor.channel_monitor_updated(chan_3_id, latest_id).unwrap();
let mut b_claim = get_htlc_update_msgs(&nodes[3], &node_b_id);
@@ -5133,7 +5126,7 @@ fn test_mpp_claim_to_holding_cell() {
// Finally, complete the C <-> D monitor update. Previously, this unlock failed to be processed
// due to the existence of the blocked RAA update above.
- let (latest_id, _) = get_latest_mon_update_id(&nodes[3], chan_4_id);
+ let (latest_id, _) = nodes[3].chain_monitor.get_latest_mon_update_id(chan_4_id);
nodes[3].chain_monitor.chain_monitor.channel_monitor_updated(chan_4_id, latest_id).unwrap();
// Once we process monitor events (in this case by checking for the `PaymentClaimed` event, the
// RAA monitor update blocked above will be released.
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 34f5d5f..d7f23d3 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -581,6 +581,11 @@ impl<'a> TestChainMonitor<'a> {
self.added_monitors.lock().unwrap().push((channel_id, monitor));
self.chain_monitor.load_existing_monitor(channel_id, new_monitor)
}
+
+ pub fn get_latest_mon_update_id(&self, channel_id: ChannelId) -> (u64, u64) {
+ let monitor_id_state = self.latest_monitor_update_id.lock().unwrap();
+ monitor_id_state.get(&channel_id).unwrap().clone()
+ }
}
impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
fn watch_channel(
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.