Skip to content

Commit d97a168

Browse files
committed
Run monitor update completion actions for pre-startup completion
If a `ChannelMonitorUpdate` completes being persisted, but the `ChannelManager` isn't informed thereof (or isn't persisted) before shutdown, on startup we may still have it listed as in-flight. When we compare the available `ChannelMonitor` with the in-flight set, we'll notice it completed and remove it, however this may leave some post-update actions dangling which need to complete. Here we handle this with a new `BackgroundEvent` indicating we need to handle any post-update action(s) for a given channel.
1 parent 9409377 commit d97a168

File tree

2 files changed

+40
-3
lines changed

2 files changed

+40
-3
lines changed

lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,6 +2346,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
23462346
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
23472347
check_added_monitors!(nodes[0], 0);
23482348

2349+
let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
23492350
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
23502351
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
23512352
nodes[0].node.claim_funds(payment_preimage_0);
@@ -2365,8 +2366,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
23652366
// disconnect the peers. Note that the fuzzer originally found this issue because
23662367
// deserializing a ChannelManager in this state causes an assertion failure.
23672368
if reload_a {
2368-
let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
23692369
reload_node!(nodes[0], &nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
2370+
persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
2371+
persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
23702372
} else {
23712373
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
23722374
}
@@ -2407,8 +2409,12 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
24072409
} else { panic!(); }
24082410

24092411
// There should be no monitor updates as we are still pending awaiting a failed one.
2410-
check_added_monitors!(nodes[0], 0);
2411-
check_added_monitors!(nodes[1], 0);
2412+
if reload_a {
2413+
check_added_monitors(&nodes[0], 2);
2414+
} else {
2415+
check_added_monitors(&nodes[0], 0);
2416+
}
2417+
check_added_monitors(&nodes[1], 0);
24122418
}
24132419

24142420
// If we finish updating the monitor, we should free the holding cell right away (this did

lightning/src/ln/channelmanager.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,13 @@ enum BackgroundEvent {
530530
funding_txo: OutPoint,
531531
update: ChannelMonitorUpdate
532532
},
533+
/// Some [`ChannelMonitorUpdate`] (s) completed before we were serialized but we still have
534+
/// them marked pending, thus we need to run any [`MonitorUpdateCompletionAction`] (s) pending
535+
/// on a channel.
536+
MonitorUpdatesComplete {
537+
counterparty_node_id: PublicKey,
538+
channel_id: [u8; 32],
539+
},
533540
}
534541

535542
#[derive(Debug)]
@@ -4143,6 +4150,20 @@ where
41434150
}
41444151
let _ = handle_error!(self, res, counterparty_node_id);
41454152
},
4153+
BackgroundEvent::MonitorUpdatesComplete { counterparty_node_id, channel_id } => {
4154+
let per_peer_state = self.per_peer_state.read().unwrap();
4155+
if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
4156+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
4157+
let peer_state = &mut *peer_state_lock;
4158+
if let Some(chan) = peer_state.channel_by_id.get_mut(&channel_id) {
4159+
handle_monitor_update_completion!(self, peer_state_lock, peer_state, per_peer_state, chan);
4160+
} else {
4161+
let update_actions = peer_state.monitor_update_blocked_actions
4162+
.remove(&channel_id).unwrap_or(Vec::new());
4163+
self.handle_monitor_update_completion_actions(update_actions);
4164+
}
4165+
}
4166+
},
41464167
}
41474168
}
41484169
NotifyOption::DoPersist
@@ -8454,6 +8475,16 @@ where
84548475
update: update.clone(),
84558476
});
84568477
}
8478+
if $chan_in_flight_upds.is_empty() {
8479+
// We had some updates to apply, but it turns out they had completed before we
8480+
// were serialized, we just weren't notified of that. Thus, we may have to run
8481+
// the completion actions for any monitor updates, but otherwise are done.
8482+
pending_background_events.push(
8483+
BackgroundEvent::MonitorUpdatesComplete {
8484+
counterparty_node_id: $counterparty_node_id,
8485+
channel_id: $funding_txo.to_channel_id(),
8486+
});
8487+
}
84578488
if $peer_state.in_flight_monitor_updates.insert($funding_txo, $chan_in_flight_upds).is_some() {
84588489
log_error!(args.logger, "Duplicate in-flight monitor update set for the same channel!");
84598490
return Err(DecodeError::InvalidValue);

0 commit comments

Comments
 (0)