Skip to content

Commit 63d17a1

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 d450e0f commit 63d17a1

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)]
@@ -4191,6 +4198,20 @@ where
41914198
}
41924199
let _ = handle_error!(self, res, counterparty_node_id);
41934200
},
4201+
BackgroundEvent::MonitorUpdatesComplete { counterparty_node_id, channel_id } => {
4202+
let per_peer_state = self.per_peer_state.read().unwrap();
4203+
if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
4204+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
4205+
let peer_state = &mut *peer_state_lock;
4206+
if let Some(chan) = peer_state.channel_by_id.get_mut(&channel_id) {
4207+
handle_monitor_update_completion!(self, peer_state_lock, peer_state, per_peer_state, chan);
4208+
} else {
4209+
let update_actions = peer_state.monitor_update_blocked_actions
4210+
.remove(&channel_id).unwrap_or(Vec::new());
4211+
self.handle_monitor_update_completion_actions(update_actions);
4212+
}
4213+
}
4214+
},
41944215
}
41954216
}
41964217
NotifyOption::DoPersist
@@ -8518,6 +8539,16 @@ where
85188539
update: update.clone(),
85198540
});
85208541
}
8542+
if $chan_in_flight_upds.is_empty() {
8543+
// We had some updates to apply, but it turns out they had completed before we
8544+
// were serialized, we just weren't notified of that. Thus, we may have to run
8545+
// the completion actions for any monitor updates, but otherwise are done.
8546+
pending_background_events.push(
8547+
BackgroundEvent::MonitorUpdatesComplete {
8548+
counterparty_node_id: $counterparty_node_id,
8549+
channel_id: $funding_txo.to_channel_id(),
8550+
});
8551+
}
85218552
if $peer_state.in_flight_monitor_updates.insert($funding_txo, $chan_in_flight_upds).is_some() {
85228553
log_error!(args.logger, "Duplicate in-flight monitor update set for the same channel!");
85238554
return Err(DecodeError::InvalidValue);

0 commit comments

Comments
 (0)