Skip to content

Commit 59aa700

Browse files
Update completion for persistent mon events w/ fwd event
If persistent_monitor_events is enabled, for forwarded HTLC claims there is no need to block the outbound edge's RAA monitor update until the preimage makes it into the inbound edge -- the outbound edge's preimage monitor event is persistent and will keep being returned to us until the inbound edge gets the preimage, instead. Therefore, clean up the claim flow when persistent monitor events is enabled to omit any usage of/reference to RAA monitor update blocking actions.
1 parent effb00f commit 59aa700

1 file changed

Lines changed: 68 additions & 13 deletions

File tree

lightning/src/ln/channelmanager.rs

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,6 +1512,17 @@ pub(crate) enum MonitorUpdateCompletionAction {
15121512
/// action is handled.
15131513
monitor_event_source: Option<MonitorEventSource>,
15141514
},
1515+
/// Indicates an [`events::Event`] should be surfaced to the user once the associated
1516+
/// [`ChannelMonitorUpdate`] has been durably persisted.
1517+
///
1518+
/// This is used when `persistent_monitor_events` is enabled for forwarded payment claims.
1519+
/// Unlike [`Self::EmitEventOptionAndFreeOtherChannel`], this variant does not carry any
1520+
/// channel-unblocking information because persistent monitor events never add
1521+
/// [`RAAMonitorUpdateBlockingAction::ForwardedPaymentInboundClaim`] blockers.
1522+
EmitEventAndAckMonitorEvent {
1523+
event: events::Event,
1524+
monitor_event_source: Option<MonitorEventSource>,
1525+
},
15151526
/// Indicates that one or more [`MonitorEvent`]s should be acknowledged via
15161527
/// [`chain::Watch::ack_monitor_event`] once the associated [`ChannelMonitorUpdate`] has been
15171528
/// durably persisted.
@@ -1542,6 +1553,10 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
15421553
(3, AckMonitorEvents) => {
15431554
(0, monitor_events_to_ack, required_vec),
15441555
},
1556+
(4, EmitEventAndAckMonitorEvent) => {
1557+
(0, event, upgradable_required),
1558+
(1, monitor_event_source, option),
1559+
},
15451560
);
15461561

15471562
/// Result of attempting to resume a channel after a monitor update completes while locks are held.
@@ -9768,20 +9783,51 @@ impl<
97689783
)
97699784
} else {
97709785
let event = make_payment_forwarded_event(htlc_claim_value_msat);
9771-
if let Some(ref payment_forwarded) = event {
9772-
debug_assert!(matches!(
9773-
payment_forwarded,
9774-
&events::Event::PaymentForwarded { .. }
9775-
));
9786+
if self.persistent_monitor_events {
9787+
match (event, monitor_event_source) {
9788+
// Typically monitor_event_source is None here. The usual flow is: receive
9789+
// update_fulfill_htlc from outbound edge peer, call claim_funds with
9790+
// monitor_event_source = None, generate event here. Then, preimage gets into
9791+
// outbound edge monitor, get a preimage monitor event, call claim_funds again and
9792+
// hit the duplicate claim flow above. However, in the case that the ChannelManager
9793+
// is outdated and we got here after restart from an un-acked preimage monitor event,
9794+
// we can end up here with monitor event source = Some.
9795+
(Some(ev), mes) => (
9796+
Some(MonitorUpdateCompletionAction::EmitEventAndAckMonitorEvent {
9797+
event: ev,
9798+
monitor_event_source: mes,
9799+
}),
9800+
None,
9801+
),
9802+
(None, Some(mes)) => (
9803+
Some(MonitorUpdateCompletionAction::AckMonitorEvents {
9804+
monitor_events_to_ack: vec![mes],
9805+
}),
9806+
None,
9807+
),
9808+
(None, None) => {
9809+
debug_assert!(false, "Monitor event source should always be set if persistent_monitor_events is set");
9810+
(None, None)
9811+
},
9812+
}
9813+
} else {
9814+
if let Some(ref payment_forwarded) = event {
9815+
debug_assert!(matches!(
9816+
payment_forwarded,
9817+
&events::Event::PaymentForwarded { .. }
9818+
));
9819+
}
9820+
(
9821+
Some(
9822+
MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel {
9823+
event,
9824+
downstream_counterparty_and_funding_outpoint: chan_to_release,
9825+
monitor_event_source,
9826+
},
9827+
),
9828+
None,
9829+
)
97769830
}
9777-
(
9778-
Some(MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel {
9779-
event,
9780-
downstream_counterparty_and_funding_outpoint: chan_to_release,
9781-
monitor_event_source,
9782-
}),
9783-
None,
9784-
)
97859831
}
97869832
},
97879833
);
@@ -10571,6 +10617,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1057110617
Some(blocking_action),
1057210618
);
1057310619
},
10620+
MonitorUpdateCompletionAction::EmitEventAndAckMonitorEvent {
10621+
event,
10622+
monitor_event_source,
10623+
} => {
10624+
if let Some(source) = monitor_event_source {
10625+
self.chain_monitor.ack_monitor_event(source);
10626+
}
10627+
self.pending_events.lock().unwrap().push_back((event, None));
10628+
},
1057410629
MonitorUpdateCompletionAction::AckMonitorEvents { monitor_events_to_ack } => {
1057510630
for source in monitor_events_to_ack {
1057610631
self.chain_monitor.ack_monitor_event(source);

0 commit comments

Comments
 (0)