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.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9340dbbee0381b5243e510532cf1762cf0d96056..aa5f27606f04188be7c92147392ec733521bdf21 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -615,10 +615,34 @@ pub(crate) enum MonitorUpdateCompletionAction {
                event: events::Event,
                downstream_counterparty_and_funding_outpoint: Option<(PublicKey, OutPoint, RAAMonitorUpdateBlockingAction)>,
        },
+       /// Indicates we should immediately resume the operation of another channel, unless there is
+       /// some other reason why the channel is blocked. In practice this simply means immediately
+       /// removing the [`RAAMonitorUpdateBlockingAction`] provided from the blocking set.
+       ///
+       /// This is usually generated when we've forwarded an HTLC and want to block the outbound edge
+       /// from completing a monitor update which removes the payment preimage until the inbound edge
+       /// completes a monitor update containing the payment preimage. However, we use this variant
+       /// instead of [`Self::EmitEventAndFreeOtherChannel`] when we discover that the claim was in
+       /// fact duplicative and we simply want to resume the outbound edge channel immediately.
+       ///
+       /// This variant should thus never be written to disk, as it is processed inline rather than
+       /// stored for later processing.
+       FreeOtherChannelImmediately {
+               downstream_counterparty_node_id: PublicKey,
+               downstream_funding_outpoint: OutPoint,
+               blocking_action: RAAMonitorUpdateBlockingAction,
+       },
 }
 
 impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
        (0, PaymentClaimed) => { (0, payment_hash, required) },
+       // Note that FreeOtherChannelImmediately should never be written - we were supposed to free
+       // *immediately*. However, for simplicity we implement read/write here.
+       (1, FreeOtherChannelImmediately) => {
+               (0, downstream_counterparty_node_id, required),
+               (2, downstream_funding_outpoint, required),
+               (4, blocking_action, required),
+       },
        (2, EmitEventAndFreeOtherChannel) => {
                (0, event, upgradable_required),
                // LDK prior to 0.0.116 did not have this field as the monitor update application order was
@@ -5585,6 +5609,15 @@ where
                                                self.handle_monitor_update_release(node_id, funding_outpoint, Some(blocker));
                                        }
                                },
+                               MonitorUpdateCompletionAction::FreeOtherChannelImmediately {
+                                       downstream_counterparty_node_id, downstream_funding_outpoint, blocking_action,
+                               } => {
+                                       self.handle_monitor_update_release(
+                                               downstream_counterparty_node_id,
+                                               downstream_funding_outpoint,
+                                               Some(blocking_action),
+                                       );
+                               },
                        }
                }
        }
@@ -9989,6 +10022,9 @@ where
                                                                // anymore.
                                                        }
                                                }
+                                               if let MonitorUpdateCompletionAction::FreeOtherChannelImmediately { .. } = action {
+                                                       debug_assert!(false, "Non-event-generating channel freeing should not appear in our queue");
+                                               }
                                        }
                                }
                                peer_state.lock().unwrap().monitor_update_blocked_actions = monitor_update_blocked_actions;