Skip to content

Commit 29f5f85

Browse files
valentinewallacevalentinewallace
authored
Merge pull request #4377 from wpaulino/hold-in-flight-updates-until-background-event-processing
2 parents e9fd567 + f128b85 commit 29f5f85

4 files changed

Lines changed: 193 additions & 66 deletions

File tree

‎lightning/src/ln/chanmon_update_fail_tests.rs‎

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,6 @@ use crate::prelude::*;
4949
use crate::sync::{Arc, Mutex};
5050
use bitcoin::hashes::Hash;
5151

52-
fn get_latest_mon_update_id<'a, 'b, 'c>(
53-
node: &Node<'a, 'b, 'c>, channel_id: ChannelId,
54-
) -> (u64, u64) {
55-
let monitor_id_state = node.chain_monitor.latest_monitor_update_id.lock().unwrap();
56-
monitor_id_state.get(&channel_id).unwrap().clone()
57-
}
58-
5952
#[test]
6053
fn test_monitor_and_persister_update_fail() {
6154
// Test that if both updating the `ChannelMonitor` and persisting the updated
@@ -213,7 +206,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
213206
}
214207

215208
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
216-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
209+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
217210
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
218211
check_added_monitors(&nodes[0], 0);
219212

@@ -405,7 +398,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
405398

406399
// Now fix monitor updating...
407400
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
408-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
401+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
409402
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
410403
check_added_monitors(&nodes[0], 0);
411404

@@ -758,7 +751,7 @@ fn test_monitor_update_fail_cs() {
758751
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
759752

760753
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
761-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
754+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
762755
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
763756
check_added_monitors(&nodes[1], 0);
764757
let responses = nodes[1].node.get_and_clear_pending_msg_events();
@@ -793,7 +786,7 @@ fn test_monitor_update_fail_cs() {
793786
}
794787

795788
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
796-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
789+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
797790
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
798791
check_added_monitors(&nodes[0], 0);
799792

@@ -869,7 +862,7 @@ fn test_monitor_update_fail_no_rebroadcast() {
869862
check_added_monitors(&nodes[1], 1);
870863

871864
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
872-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
865+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
873866
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
874867
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
875868
check_added_monitors(&nodes[1], 0);
@@ -939,7 +932,7 @@ fn test_monitor_update_raa_while_paused() {
939932
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
940933
check_added_monitors(&nodes[0], 1);
941934

942-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
935+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
943936
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
944937
check_added_monitors(&nodes[0], 0);
945938

@@ -1081,7 +1074,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
10811074
// Restore monitor updating, ensuring we immediately get a fail-back update and a
10821075
// update_add update.
10831076
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
1084-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_2.2);
1077+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_2.2);
10851078
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_2.2, latest_update);
10861079
check_added_monitors(&nodes[1], 0);
10871080
expect_and_process_pending_htlcs_and_htlc_handling_failed(
@@ -1355,7 +1348,7 @@ fn test_monitor_update_fail_reestablish() {
13551348
assert_eq!(bs_channel_upd.contents.channel_flags & 2, 0);
13561349

13571350
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
1358-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_1.2);
1351+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_1.2);
13591352
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_1.2, latest_update);
13601353
check_added_monitors(&nodes[1], 0);
13611354

@@ -1440,7 +1433,7 @@ fn raa_no_response_awaiting_raa_state() {
14401433
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
14411434
check_added_monitors(&nodes[1], 1);
14421435

1443-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
1436+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
14441437
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
14451438
// nodes[1] should be AwaitingRAA here!
14461439
check_added_monitors(&nodes[1], 0);
@@ -1569,7 +1562,7 @@ fn claim_while_disconnected_monitor_update_fail() {
15691562
// Now un-fail the monitor, which will result in B sending its original commitment update,
15701563
// receiving the commitment update from A, and the resulting commitment dances.
15711564
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
1572-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
1565+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
15731566
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
15741567
check_added_monitors(&nodes[1], 0);
15751568

@@ -1698,7 +1691,7 @@ fn monitor_failed_no_reestablish_response() {
16981691
get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, node_b_id);
16991692

17001693
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
1701-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
1694+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
17021695
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
17031696
check_added_monitors(&nodes[1], 0);
17041697
let bs_responses = get_revoke_commit_msgs(&nodes[1], &node_a_id);
@@ -1796,7 +1789,7 @@ fn first_message_on_recv_ordering() {
17961789
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
17971790

17981791
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
1799-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
1792+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
18001793
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
18011794
check_added_monitors(&nodes[1], 0);
18021795

@@ -1895,7 +1888,7 @@ fn test_monitor_update_fail_claim() {
18951888

18961889
// Now restore monitor updating on the 0<->1 channel and claim the funds on B.
18971890
let channel_id = chan_1.2;
1898-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
1891+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
18991892
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
19001893
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
19011894
check_added_monitors(&nodes[1], 0);
@@ -2024,7 +2017,7 @@ fn test_monitor_update_on_pending_forwards() {
20242017
check_added_monitors(&nodes[1], 1);
20252018

20262019
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
2027-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_1.2);
2020+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_1.2);
20282021
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_1.2, latest_update);
20292022
check_added_monitors(&nodes[1], 0);
20302023

@@ -2095,7 +2088,7 @@ fn monitor_update_claim_fail_no_response() {
20952088
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
20962089

20972090
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
2098-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
2091+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
20992092
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
21002093
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
21012094
check_added_monitors(&nodes[1], 0);
@@ -2167,7 +2160,7 @@ fn do_during_funding_monitor_fail(
21672160
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
21682161
assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
21692162
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
2170-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
2163+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
21712164
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
21722165
check_added_monitors(&nodes[0], 0);
21732166
expect_channel_pending_event(&nodes[0], &node_b_id);
@@ -2222,7 +2215,7 @@ fn do_during_funding_monitor_fail(
22222215
}
22232216

22242217
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
2225-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
2218+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
22262219
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
22272220
check_added_monitors(&nodes[1], 0);
22282221

@@ -2340,7 +2333,7 @@ fn test_path_paused_mpp() {
23402333

23412334
// And check that, after we successfully update the monitor for chan_2 we can pass the second
23422335
// HTLC along to nodes[3] and claim the whole payment back to nodes[0].
2343-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], chan_2_id);
2336+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(chan_2_id);
23442337
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_2_id, latest_update);
23452338

23462339
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -2788,7 +2781,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
27882781
// If we finish updating the monitor, we should free the holding cell right away (this did
27892782
// not occur prior to #756). This should result in a new monitor update.
27902783
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
2791-
let (mon_id, _) = get_latest_mon_update_id(&nodes[0], chan_id);
2784+
let (mon_id, _) = nodes[0].chain_monitor.get_latest_mon_update_id(chan_id);
27922785
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_id, mon_id);
27932786
expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);
27942787
check_added_monitors(&nodes[0], 1);
@@ -3040,7 +3033,7 @@ fn test_temporary_error_during_shutdown() {
30403033
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
30413034
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
30423035

3043-
let (latest_update, _) = get_latest_mon_update_id(&nodes[0], channel_id);
3036+
let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
30443037
nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
30453038
nodes[1].node.handle_closing_signed(
30463039
node_a_id,
@@ -3050,7 +3043,7 @@ fn test_temporary_error_during_shutdown() {
30503043
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
30513044

30523045
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
3053-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], channel_id);
3046+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
30543047
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
30553048

30563049
nodes[0].node.handle_closing_signed(
@@ -3096,7 +3089,7 @@ fn double_temp_error() {
30963089
// `claim_funds` results in a ChannelMonitorUpdate.
30973090
nodes[1].node.claim_funds(payment_preimage_1);
30983091
check_added_monitors(&nodes[1], 1);
3099-
let (latest_update_1, _) = get_latest_mon_update_id(&nodes[1], channel_id);
3092+
let (latest_update_1, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
31003093

31013094
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
31023095
// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
@@ -3105,7 +3098,7 @@ fn double_temp_error() {
31053098
check_added_monitors(&nodes[1], 1);
31063099
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
31073100

3108-
let (latest_update_2, _) = get_latest_mon_update_id(&nodes[1], channel_id);
3101+
let (latest_update_2, _) = nodes[1].chain_monitor.get_latest_mon_update_id(channel_id);
31093102
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update_1);
31103103
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
31113104
check_added_monitors(&nodes[1], 0);
@@ -3512,7 +3505,7 @@ fn do_test_blocked_chan_preimage_release(completion_mode: BlockedUpdateComplMode
35123505
reconnect_nodes(a_b_reconnect);
35133506
reconnect_nodes(ReconnectArgs::new(&nodes[2], &nodes[1]));
35143507
} else if completion_mode == BlockedUpdateComplMode::Async {
3515-
let (latest_update, _) = get_latest_mon_update_id(&nodes[1], chan_id_2);
3508+
let (latest_update, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_2);
35163509
nodes[1]
35173510
.chain_monitor
35183511
.chain_monitor
@@ -3690,7 +3683,7 @@ fn do_test_inverted_mon_completion_order(
36903683
// (Finally) complete the A <-> B ChannelMonitorUpdate, ensuring the preimage is durably on
36913684
// disk in the proper ChannelMonitor, unblocking the B <-> C ChannelMonitor updating
36923685
// process.
3693-
let (_, ab_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
3686+
let (_, ab_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
36943687
nodes[1]
36953688
.chain_monitor
36963689
.chain_monitor
@@ -3723,7 +3716,7 @@ fn do_test_inverted_mon_completion_order(
37233716
// ChannelMonitorUpdate hasn't yet completed.
37243717
reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
37253718

3726-
let (_, ab_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
3719+
let (_, ab_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
37273720
nodes[1]
37283721
.chain_monitor
37293722
.chain_monitor
@@ -3936,7 +3929,7 @@ fn do_test_durable_preimages_on_closed_channel(
39363929

39373930
// Once the blocked `ChannelMonitorUpdate` *finally* completes, the pending
39383931
// `PaymentForwarded` event will finally be released.
3939-
let (_, ab_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
3932+
let (_, ab_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
39403933
nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(chan_id_ab, ab_update_id);
39413934

39423935
// If the A<->B channel was closed before we reload, we'll replay the claim against it on
@@ -4048,7 +4041,7 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) {
40484041
mine_transaction_without_consistency_checks(&nodes[1], &as_closing_tx[0]);
40494042
}
40504043

4051-
let (_, bc_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_bc);
4044+
let (_, bc_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
40524045
let mut events = nodes[1].node.get_and_clear_pending_events();
40534046
assert_eq!(events.len(), if close_during_reload { 2 } else { 1 });
40544047
expect_payment_forwarded(
@@ -4073,7 +4066,7 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) {
40734066
// Once we run event processing the monitor should free, check that it was indeed the B<->C
40744067
// channel which was updated.
40754068
check_added_monitors(&nodes[1], if close_during_reload { 2 } else { 1 });
4076-
let (_, post_ev_bc_update_id) = get_latest_mon_update_id(&nodes[1], chan_id_bc);
4069+
let (_, post_ev_bc_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
40774070
assert!(bc_update_id != post_ev_bc_update_id);
40784071

40794072
// Finally, check that there's nothing left to do on B<->C reconnect and the channel operates
@@ -4163,7 +4156,7 @@ fn do_test_glacial_peer_cant_hang(hold_chan_a: bool) {
41634156

41644157
// ...but once we complete the A<->B channel preimage persistence, the B<->C channel
41654158
// unlocks and we send both peers commitment updates.
4166-
let (ab_update_id, _) = get_latest_mon_update_id(&nodes[1], chan_id_ab);
4159+
let (ab_update_id, _) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_ab);
41674160
assert!(nodes[1]
41684161
.chain_monitor
41694162
.chain_monitor
@@ -5123,7 +5116,7 @@ fn test_mpp_claim_to_holding_cell() {
51235116
check_added_monitors(&nodes[3], 2);
51245117

51255118
// Complete the B <-> D monitor update, freeing the first fulfill.
5126-
let (latest_id, _) = get_latest_mon_update_id(&nodes[3], chan_3_id);
5119+
let (latest_id, _) = nodes[3].chain_monitor.get_latest_mon_update_id(chan_3_id);
51275120
nodes[3].chain_monitor.chain_monitor.channel_monitor_updated(chan_3_id, latest_id).unwrap();
51285121
let mut b_claim = get_htlc_update_msgs(&nodes[3], &node_b_id);
51295122

@@ -5134,7 +5127,7 @@ fn test_mpp_claim_to_holding_cell() {
51345127

51355128
// Finally, complete the C <-> D monitor update. Previously, this unlock failed to be processed
51365129
// due to the existence of the blocked RAA update above.
5137-
let (latest_id, _) = get_latest_mon_update_id(&nodes[3], chan_4_id);
5130+
let (latest_id, _) = nodes[3].chain_monitor.get_latest_mon_update_id(chan_4_id);
51385131
nodes[3].chain_monitor.chain_monitor.channel_monitor_updated(chan_4_id, latest_id).unwrap();
51395132
// Once we process monitor events (in this case by checking for the `PaymentClaimed` event, the
51405133
// RAA monitor update blocked above will be released.

0 commit comments

Comments
 (0)