Skip to content

Commit 491f582

Browse files
Ack monitor events immediately
Currently, the resolution of HTLCs (and decisions on when HTLCs can be forwarded) is the responsibility of Channel objects (a part of ChannelManager) until the channel is closed, and then the ChannelMonitor thereafter. This leads to some complexity around race conditions for HTLCs right around channel closure. Additionally, there is lots of complexity reconstructing the state of all HTLCs in the ChannelManager deserialization/loading logic. Instead, we want to do all resolution in ChannelMonitors (in response to ChannelMonitorUpdates) and pass them back to ChannelManager in the form of MonitorEvents (similar to how HTLCs are resolved after channels are closed). In order to have reliable resolution, we'll need to keep MonitorEvents around in the ChannelMonitor until the ChannelManager has finished processing them. This will simplify things - on restart instead of examining the set of HTLCs in monitors we can simply replay all the pending MonitorEvents. Here for the purposes of merging initial support for persistent monitor events, we ack each immediately after it is received/handled by the ChannelManager, which is equivalent to the behavior we had prior to monitor events becoming persistent. In upcoming work, we'll want to have much more special handling for HTLCUpdate monitor events in particular -- e.g. for outbound payment claim events, we should only ACK the monitor event when the PaymentSent event is processed, until that point we want it to keep being provided back to us on startup. All the other monitor events are trivial to ACK, since they don't need to be re-processed on startup.
1 parent cb4a804 commit 491f582

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ use crate::chain::chaininterface::{
4242
BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator,
4343
TransactionType,
4444
};
45+
use crate::chain::chainmonitor::MonitorEventSource;
4546
use crate::chain::channelmonitor::{
4647
ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, MonitorEvent,
4748
WithChannelMonitor, ANTI_REORG_DELAY, CLTV_CLAIM_BUFFER, HTLC_FAIL_BACK_BUFFER,
@@ -13522,7 +13523,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1352213523
for (funding_outpoint, channel_id, mut monitor_events, counterparty_node_id) in
1352313524
pending_monitor_events.drain(..)
1352413525
{
13525-
for (_event_id, monitor_event) in monitor_events.drain(..) {
13526+
for (event_id, monitor_event) in monitor_events.drain(..) {
13527+
let monitor_event_source = MonitorEventSource { event_id, channel_id };
1352613528
match monitor_event {
1352713529
MonitorEvent::HTLCEvent(htlc_update) => {
1352813530
let logger = WithContext::from(
@@ -13572,6 +13574,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1357213574
completion_update,
1357313575
);
1357413576
}
13577+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1357513578
},
1357613579
MonitorEvent::HolderForceClosed(_)
1357713580
| MonitorEvent::HolderForceClosedWithInfo { .. } => {
@@ -13605,6 +13608,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1360513608
failed_channels.push((Err(e), counterparty_node_id));
1360613609
}
1360713610
}
13611+
// Channel close monitor events do not need to be replayed on startup because we
13612+
// already check the monitors to see if the channel is closed.
13613+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1360813614
},
1360913615
MonitorEvent::CommitmentTxConfirmed(_) => {
1361013616
let per_peer_state = self.per_peer_state.read().unwrap();
@@ -13626,13 +13632,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1362613632
failed_channels.push((Err(e), counterparty_node_id));
1362713633
}
1362813634
}
13635+
// Channel close monitor events do not need to be replayed on startup because we
13636+
// already check the monitors to see if the channel is closed.
13637+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1362913638
},
1363013639
MonitorEvent::Completed { channel_id, monitor_update_id, .. } => {
1363113640
self.channel_monitor_updated(
1363213641
&channel_id,
1363313642
Some(monitor_update_id),
1363413643
&counterparty_node_id,
1363513644
);
13645+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1363613646
},
1363713647
}
1363813648
}

0 commit comments

Comments
 (0)