Skip to content

Commit a50d521

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 6269b59 commit a50d521

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,
@@ -13687,7 +13688,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1368713688
for (funding_outpoint, channel_id, mut monitor_events, counterparty_node_id) in
1368813689
pending_monitor_events.drain(..)
1368913690
{
13690-
for (_event_id, monitor_event) in monitor_events.drain(..) {
13691+
for (event_id, monitor_event) in monitor_events.drain(..) {
13692+
let monitor_event_source = MonitorEventSource { event_id, channel_id };
1369113693
match monitor_event {
1369213694
MonitorEvent::HTLCEvent(htlc_update) => {
1369313695
let logger = WithContext::from(
@@ -13737,6 +13739,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1373713739
completion_update,
1373813740
);
1373913741
}
13742+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1374013743
},
1374113744
MonitorEvent::HolderForceClosed(_)
1374213745
| MonitorEvent::HolderForceClosedWithInfo { .. } => {
@@ -13770,6 +13773,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1377013773
failed_channels.push((Err(e), counterparty_node_id));
1377113774
}
1377213775
}
13776+
// Channel close monitor events do not need to be replayed on startup because we
13777+
// already check the monitors to see if the channel is closed.
13778+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1377313779
},
1377413780
MonitorEvent::CommitmentTxConfirmed(_) => {
1377513781
let per_peer_state = self.per_peer_state.read().unwrap();
@@ -13791,13 +13797,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1379113797
failed_channels.push((Err(e), counterparty_node_id));
1379213798
}
1379313799
}
13800+
// Channel close monitor events do not need to be replayed on startup because we
13801+
// already check the monitors to see if the channel is closed.
13802+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1379413803
},
1379513804
MonitorEvent::Completed { channel_id, monitor_update_id, .. } => {
1379613805
self.channel_monitor_updated(
1379713806
&channel_id,
1379813807
Some(monitor_update_id),
1379913808
&counterparty_node_id,
1380013809
);
13810+
self.chain_monitor.ack_monitor_event(monitor_event_source);
1380113811
},
1380213812
}
1380313813
}

0 commit comments

Comments
 (0)