Add a TODO for an important issue for making async mon updates safe
authorMatt Corallo <git@bluematt.me>
Tue, 16 Aug 2022 21:58:06 +0000 (21:58 +0000)
committerMatt Corallo <git@bluematt.me>
Thu, 29 Sep 2022 20:27:53 +0000 (20:27 +0000)
If we receive a monitor event from a forwarded-to channel which
contains a preimage for an HTLC, we have to propogate that preimage
back to the forwarded-from channel monitor. However, once we have
that update, we're running in a relatively unsafe state - we have
the preimage in memory, but if we were to crash the forwarded-to
channel monitor will not regenerate the update with the preimage
for us. If we haven't managed to write the monitor update to the
forwarded-from channel by that point, we've lost the preimage, and,
thus, money!

lightning/src/ln/channelmanager.rs

index fb73951a61495ccbe979f02945460e46023cce37..bbecc4b339fa09f5c336bfb45f3053402b3a7ba8 100644 (file)
@@ -4223,8 +4223,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        // event being update_fulfill_htlc).
                                        let update_res = self.chain_monitor.update_channel(prev_outpoint, preimage_update);
                                        if update_res != ChannelMonitorUpdateStatus::Completed {
+                                               // TODO: This needs to be handled somehow - if we receive a monitor update
+                                               // with a preimage we *must* somehow manage to propagate it to the upstream
+                                               // channel, or we must have an ability to receive the same event and try
+                                               // again on restart.
                                                log_error!(self.logger, "Critical error: failed to update channel monitor with preimage {:?}: {:?}",
-                                                                                        payment_preimage, update_res);
+                                                       payment_preimage, update_res);
                                        }
                                        // Note that we do *not* set `claimed_htlc` to false here. In fact, this
                                        // totally could be a duplicate claim, but we have no way of knowing