Skip to content

Commit 609a079

Browse files
committed
Add an immediately-freeing MonitorUpdateCompletionAction.
When `MonitorUpdateCompletionAction`s were added, we didn't consider the case of a duplicate claim during normal HTLC processing (as the handling only had an `if let` rather than a `match`, which made the branch easy to miss). This can lead to a channel freezing indefinitely if an HTLC is claimed (without a `commitment_signed`), the peer disconnects, and then the HTLC is claimed again, leading to a never-completing `MonitorUpdateCompletionAction`. The fix is simple - if we get back an `UpdateFulfillCommitFetch::DuplicateClaim` when claiming from the inbound edge, immediately unlock the outbound edge channel with a new `MonitorUpdateCompletionAction::FreeOtherChannelImmediately`. Here we add the new variant, which we start generating in the next commit.
1 parent c140e0d commit 609a079

File tree

1 file changed

+27
-0
lines changed

1 file changed

+27
-0
lines changed

lightning/src/ln/channelmanager.rs

+27
Original file line numberDiff line numberDiff line change
@@ -615,10 +615,28 @@ pub(crate) enum MonitorUpdateCompletionAction {
615615
event: events::Event,
616616
downstream_counterparty_and_funding_outpoint: Option<(PublicKey, OutPoint, RAAMonitorUpdateBlockingAction)>,
617617
},
618+
/// Indicates we should probably resume the operation of another channel.
619+
///
620+
/// This is usually generated when we've forwarded an HTLC and want to block the outbound edge
621+
/// from completing a monitor update which removes the payment preimage until the inbound edge
622+
/// completes a monitor update containing the payment preimage. However, we use this variant
623+
/// instead of [`Self::EmitEventAndFreeOtherChannel`] when we discover that the claim was in
624+
/// fact duplicative and we simply want to resume the outbound edge channel immediately.
625+
///
626+
/// This variant should thus never be written to disk, as it is processed inline rather than
627+
/// stored for later processing.
628+
FreeOtherChannelImmediately {
629+
downstream_counterparty_and_funding_outpoint: (PublicKey, OutPoint, RAAMonitorUpdateBlockingAction),
630+
},
618631
}
619632

620633
impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
621634
(0, PaymentClaimed) => { (0, payment_hash, required) },
635+
// Note that FreeOtherChannelImmediately should never be written - we were supposed to free
636+
// *immediately*. However, for simplicity we implement read/write here.
637+
(1, FreeOtherChannelImmediately) => {
638+
(0, downstream_counterparty_and_funding_outpoint, required),
639+
},
622640
(2, EmitEventAndFreeOtherChannel) => {
623641
(0, event, upgradable_required),
624642
// LDK prior to 0.0.116 did not have this field as the monitor update application order was
@@ -5585,6 +5603,12 @@ where
55855603
self.handle_monitor_update_release(node_id, funding_outpoint, Some(blocker));
55865604
}
55875605
},
5606+
MonitorUpdateCompletionAction::FreeOtherChannelImmediately {
5607+
downstream_counterparty_and_funding_outpoint
5608+
} => {
5609+
let (node_id, funding_outpoint, blocker) = downstream_counterparty_and_funding_outpoint;
5610+
self.handle_monitor_update_release(node_id, funding_outpoint, Some(blocker));
5611+
},
55885612
}
55895613
}
55905614
}
@@ -9987,6 +10011,9 @@ where
998710011
// anymore.
998810012
}
998910013
}
10014+
if let MonitorUpdateCompletionAction::FreeOtherChannelImmediately { .. } = action {
10015+
debug_assert!(false, "Non-event-generating channel freeing should not appear in our queue");
10016+
}
999010017
}
999110018
}
999210019
peer_state.lock().unwrap().monitor_update_blocked_actions = monitor_update_blocked_actions;

0 commit comments

Comments
 (0)