ln: make event optional in EmitEventAndFreeOtherChannel
What the developer wrote
Authored by Carla Kirk-Cohen
ln: make event optional in EmitEventAndFreeOtherChannel
In the commits that follow, we want to be able to free the other
channel without emitting an event so that we can emit a single event
for trampoline payments with multiple incoming HTLCs. We still want
to go through the full claim flow for each incoming HTLC (and persist
the EmitEventAndFreeOtherChannel event to be picked up on restart), but
do not want multiple events for the same trampoline forward.
Changing from upgradable_required to upgradable_option is forwards
compatible - old versions of the software will always have written this
field, newer versions don't require it to be there but will be able to
read it as-is.
This change is not backwards compatible, because older versions of the
software will expect the field to be present but newer versions may not
write it. An alternative would be to add a new event type, but that
would need to have an even TLV (because the event must be understood
and processed on restart to claim the incoming HTLC), so that option
isn't backwards compatible either.
What changed, and why it matters
This commit changes how Lightning Dev Kit stores an internal event record when a payment is forwarded, specifically for an upcoming feature called trampoline payments. It makes one field optional so that future code can avoid emitting duplicate events when multiple incoming payments are part of the same trampoline forward. The change is forward-compatible but not backward-compatible: older versions of LDK will expect the field to always be present, so downgrading while trampoline forwards are in progress is not supported. There is no direct security vulnerability in this patch; it is a data-format change to enable a future feature.
No immediate security action required. Reviewers should verify that the optional event is correctly handled on restart and that downgrade paths reject or safely fail when the optional field is absent. Monitor follow-up commits that actually introduce trampoline payments with multiple incoming HTLCs to ensure the new behavior does not skip required event emission or channel unblocking.
Security signals we found
Serialization format change from required to optional TLV field
Explicit backward-compatibility break acknowledged by vendor
No input validation, cryptographic, or memory-safety changes
No privilege escalation, remote-triggerable behavior, or secret exposure in diff
Change gated by TLV upgrade/downgrade semantics
Evidence from the diff
The commit modifies the MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel variant in channelmanager.rs, changing the event field from events::Event to Option<events::Event>. The TLV serialization is changed from upgradable_required to upgradable_option. The construction site wraps the existing Event::PaymentForwarded in Some(...), and the consumption site only pushes the event into pending_events if it is Some. A pending changelog note is added stating that downgrade is not possible while in-flight trampoline forwards exist. The commit message explicitly states this is not backward compatible and explains the rationale.
Changed components
lightning/src/ln/channelmanager.rspending_changelog/4304.txtInspect captured patch +13 / −5
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index a5725a7..d9aa933 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1393,7 +1393,7 @@ pub(crate) enum MonitorUpdateCompletionAction {
/// edge completes, we will surface an [`Event::PaymentForwarded`] as well as unblock the
/// outbound edge.
EmitEventAndFreeOtherChannel {
- event: events::Event,
+ event: Option<events::Event>,
downstream_counterparty_and_funding_outpoint: Option<EventUnblockedChannel>,
},
/// Indicates we should immediately resume the operation of another channel, unless there is
@@ -1428,7 +1428,10 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
(5, downstream_channel_id, required),
},
(2, EmitEventAndFreeOtherChannel) => {
- (0, event, upgradable_required),
+ // LDK prior to 0.3 required this field. It will not be present for trampoline payments
+ // with multiple incoming HTLCS, so nodes cannot downgrade while trampoline payments
+ // are in the process of being resolved.
+ (0, event, upgradable_option),
// LDK prior to 0.0.116 did not have this field as the monitor update application order was
// required by clients. If we downgrade to something prior to 0.0.116 this may result in
// monitor updates which aren't properly blocked or resumed, however that's fine - we don't
@@ -9755,7 +9758,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
);
(
Some(MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel {
- event: events::Event::PaymentForwarded {
+ event: Some(events::Event::PaymentForwarded {
prev_htlcs: vec![events::HTLCLocator {
channel_id: prev_channel_id,
user_channel_id: prev_user_channel_id,
@@ -9770,7 +9773,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
skimmed_fee_msat,
claim_from_onchain_tx: from_onchain,
outbound_amount_forwarded_msat: forwarded_htlc_value_msat,
- },
+ }),
downstream_counterparty_and_funding_outpoint: chan_to_release,
}),
None,
@@ -10000,7 +10003,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
event,
downstream_counterparty_and_funding_outpoint,
} => {
- self.pending_events.lock().unwrap().push_back((event, None));
+ if let Some(event) = event {
+ self.pending_events.lock().unwrap().push_back((event, None));
+ }
if let Some(unblocked) = downstream_counterparty_and_funding_outpoint {
self.handle_monitor_update_release(
unblocked.counterparty_node_id,
diff --git a/pending_changelog/4304.txt b/pending_changelog/4304.txt
new file mode 100644
index 0000000..8c1580a
--- /dev/null
+++ b/pending_changelog/4304.txt
@@ -0,0 +1,3 @@
+## Backwards Compatibility
+
+* Downgrade is not possible while the node has in-flight trampoline forwards.
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.