Merge pull request #750 from TheBlueMatt/2020-11-dup-chan-id-crash

[rust-lightning] / lightning / src / ln / channelmanager.rs

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index f3f13d3fb91f9f223651b396cd1acf5010b81163..0f5e7f8ad63736dcf5f567f3cd5a9af920a57735 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -37,7 +37,7 @@ use bitcoin::secp256k1;
 use chain;
 use chain::Watch;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
-use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent};
+use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
 use chain::transaction::{OutPoint, TransactionData};
 use ln::channel::{Channel, ChannelError};
 use ln::features::{InitFeatures, NodeFeatures};
@@ -475,6 +475,7 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_P
 const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;
 
 /// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels
+#[derive(Clone)]
 pub struct ChannelDetails {
        /// The channel's ID (prior to funding transaction generation, this is a random 32 bytes,
        /// thereafter this is the txid of the funding transaction xor the funding transaction output).
@@ -2152,12 +2153,23 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                });
                        },
                        HTLCSource::PreviousHopData(hop_data) => {
+                               let prev_outpoint = hop_data.outpoint;
                                if let Err((counterparty_node_id, err)) = match self.claim_funds_from_hop(&mut channel_state_lock, hop_data, payment_preimage) {
                                        Ok(()) => Ok(()),
                                        Err(None) => {
-                                               // TODO: There is probably a channel monitor somewhere that needs to
-                                               // learn the preimage as the channel already hit the chain and that's
-                                               // why it's missing.
+                                               let preimage_update = ChannelMonitorUpdate {
+                                                       update_id: CLOSED_CHANNEL_UPDATE_ID,
+                                                       updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
+                                                               payment_preimage: payment_preimage.clone(),
+                                                       }],
+                                               };
+                                               // We update the ChannelMonitor on the backward link, after
+                                               // receiving an offchain preimage event from the forward link (the
+                                               // event being update_fulfill_htlc).
+                                               if let Err(e) = self.chain_monitor.update_channel(prev_outpoint, preimage_update) {
+                                                       log_error!(self.logger, "Critical error: failed to update channel monitor with preimage {:?}: {:?}",
+                                                                  payment_preimage, e);
+                                               }
                                                Ok(())
                                        },
                                        Err(Some(res)) => Err(res),
@@ -2351,7 +2363,12 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                        // channel, not the temporary_channel_id. This is compatible with ourselves, but the
                                        // spec is somewhat ambiguous here. Not a huge deal since we'll send error messages for
                                        // any messages referencing a previously-closed channel anyway.
-                                       return Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), funding_msg.channel_id, chan.force_shutdown(true), None));
+                                       // We do not do a force-close here as that would generate a monitor update for
+                                       // a monitor that we didn't manage to store (and that we don't care about - we
+                                       // don't respond with the funding_signed so the channel can never go on chain).
+                                       let (_funding_txo_option, _monitor_update, failed_htlcs) = chan.force_shutdown(true);
+                                       assert!(failed_htlcs.is_empty());
+                                       return Err(MsgHandleErrInternal::send_err_msg_no_close("ChannelMonitor storage failure".to_owned(), funding_msg.channel_id));
                                },
                                ChannelMonitorUpdateErr::TemporaryFailure => {
                                        // There's no problem signing a counterparty's funding transaction if our monitor