Skip to content

Commit f744ae2

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 ccf1751 commit f744ae2

1 file changed

Lines changed: 11 additions & 1 deletion

File tree

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,
@@ -13692,7 +13693,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1369213693
for (funding_outpoint, channel_id, mut monitor_events, counterparty_node_id) in
1369313694
pending_monitor_events.drain(..)
1369413695
{
13695-
for (_event_id, monitor_event) in monitor_events.drain(..) {
13696+
for (event_id, monitor_event) in monitor_events.drain(..) {
13697+
let monitor_event_source = MonitorEventSource { event_id, channel_id };
1369613698
match monitor_event {
1369713699
MonitorEvent::HTLCEvent(htlc_update) => {
1369813700
let logger = WithContext::from(
@@ -13742,6 +13744,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1374213744
completion_update,
1374313745
);
1374413746
}
13747+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1374513748
},
1374613749
MonitorEvent::HolderForceClosed(_)
1374713750
| MonitorEvent::HolderForceClosedWithInfo { .. } => {
@@ -13775,6 +13778,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1377513778
failed_channels.push((Err(e), counterparty_node_id));
1377613779
}
1377713780
}
13781+
// Channel close monitor events do not need to be replayed on startup because we
13782+
// already check the monitors to see if the channel is closed.
13783+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1377813784
},
1377913785
MonitorEvent::CommitmentTxConfirmed(_) => {
1378013786
let per_peer_state = self.per_peer_state.read().unwrap();
@@ -13796,13 +13802,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1379613802
failed_channels.push((Err(e), counterparty_node_id));
1379713803
}
1379813804
}
13805+
// Channel close monitor events do not need to be replayed on startup because we
13806+
// already check the monitors to see if the channel is closed.
13807+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1379913808
},
1380013809
MonitorEvent::Completed { channel_id, monitor_update_id, .. } => {
1380113810
self.channel_monitor_updated(
1380213811
&channel_id,
1380313812
Some(monitor_update_id),
1380413813
&counterparty_node_id,
1380513814
);
13815+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1380613816
},
1380713817
}
1380813818
}

0 commit comments

Comments
 (0)