Merge pull request #966 from TheBlueMatt/2021-06-workaround-broken-lnd
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Tue, 29 Jun 2021 16:28:38 +0000 (16:28 +0000)
committerGitHub <noreply@github.com>
Tue, 29 Jun 2021 16:28:38 +0000 (16:28 +0000)
Workaround lnd sending funding_locked before channel_reestablish

1  2 
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs

index 80240d0aa9d6fdffb13deba853274f5038affddb,c46330b525a3c183fbfbd158653c9b6efa9abd4c..9c81f797f1499ab3e5024768407fd8787a76da04
@@@ -434,6 -434,15 +434,15 @@@ pub(super) struct Channel<Signer: Sign
        next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
        #[cfg(any(test, feature = "fuzztarget"))]
        next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
+       /// lnd has a long-standing bug where, upon reconnection, if the channel is not yet confirmed
+       /// they will not send a channel_reestablish until the channel locks in. Then, they will send a
+       /// funding_locked *before* sending the channel_reestablish (which is clearly a violation of
+       /// the BOLT specs). We copy c-lightning's workaround here and simply store the funding_locked
+       /// message until we receive a channel_reestablish.
+       ///
+       /// See-also <https://github.com/lightningnetwork/lnd/issues/4006>
+       pub workaround_lnd_bug_4006: Option<msgs::FundingLocked>,
  }
  
  #[cfg(any(test, feature = "fuzztarget"))]
@@@ -633,6 -642,8 +642,8 @@@ impl<Signer: Sign> Channel<Signer> 
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
                        #[cfg(any(test, feature = "fuzztarget"))]
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
+                       workaround_lnd_bug_4006: None,
                })
        }
  
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
                        #[cfg(any(test, feature = "fuzztarget"))]
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
+                       workaround_lnd_bug_4006: None,
                };
  
                Ok(chan)
  
        pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError> {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent funding_locked when we needed a channel_reestablish".to_owned()));
+                       self.workaround_lnd_bug_4006 = Some(msg.clone());
+                       return Err(ChannelError::Ignore("Peer sent funding_locked when we needed a channel_reestablish. The peer is likely lnd, see https://github.com/lightningnetwork/lnd/issues/4006".to_owned()));
                }
  
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
@@@ -4606,7 -4620,7 +4620,7 @@@ impl<Signer: Sign> Writeable for Channe
  
                self.channel_update_status.write(writer)?;
  
 -              write_tlv_fields!(writer, {}, {(0, self.announcement_sigs)});
 +              write_tlv_fields!(writer, {(0, self.announcement_sigs, option)});
  
                Ok(())
        }
@@@ -4779,7 -4793,7 +4793,7 @@@ impl<'a, Signer: Sign, K: Deref> Readab
                let channel_update_status = Readable::read(reader)?;
  
                let mut announcement_sigs = None;
 -              read_tlv_fields!(reader, {}, {(0, announcement_sigs)});
 +              read_tlv_fields!(reader, {(0, announcement_sigs, option)});
  
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
                        #[cfg(any(test, feature = "fuzztarget"))]
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
+                       workaround_lnd_bug_4006: None,
                })
        }
  }
index e7b1ff48783d3a37d0dfbd0fa2f2dd9985b2c53a,dc472b52f34883de434072c8e7625894188fc246..edf6ba3d3399ac54974b6ead9e8aa2750ab1d6c1
@@@ -3365,7 -3365,7 +3365,7 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
        }
  
        fn internal_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), MsgHandleErrInternal> {
-               let (htlcs_failed_forward, chan_restoration_res) = {
+               let (htlcs_failed_forward, need_lnd_workaround, chan_restoration_res) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_state_lock;
  
                                                        msg,
                                                });
                                        }
-                                       (htlcs_failed_forward, handle_chan_restoration_locked!(self, channel_state_lock, channel_state, chan, revoke_and_ack, commitment_update, order, monitor_update_opt, Vec::new(), None, funding_locked))
+                                       let need_lnd_workaround = chan.get_mut().workaround_lnd_bug_4006.take();
+                                       (htlcs_failed_forward, need_lnd_workaround,
+                                               handle_chan_restoration_locked!(self, channel_state_lock, channel_state, chan, revoke_and_ack, commitment_update, order, monitor_update_opt, Vec::new(), None, funding_locked))
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
                        }
                };
                post_handle_chan_restoration!(self, chan_restoration_res);
                self.fail_holding_cell_htlcs(htlcs_failed_forward, msg.channel_id);
+               if let Some(funding_locked_msg) = need_lnd_workaround {
+                       self.internal_funding_locked(counterparty_node_id, &funding_locked_msg)?;
+               }
                Ok(())
        }
  
@@@ -4320,22 -4326,22 +4326,22 @@@ const MIN_SERIALIZATION_VERSION: u8 = 1
  
  impl_writeable_tlv_based_enum!(PendingHTLCRouting,
        (0, Forward) => {
 -              (0, onion_packet),
 -              (2, short_channel_id),
 -      }, {}, {},
 +              (0, onion_packet, required),
 +              (2, short_channel_id, required),
 +      },
        (1, Receive) => {
 -              (0, payment_data),
 -              (2, incoming_cltv_expiry),
 -      }, {}, {}
 +              (0, payment_data, required),
 +              (2, incoming_cltv_expiry, required),
 +      }
  ;);
  
  impl_writeable_tlv_based!(PendingHTLCInfo, {
 -      (0, routing),
 -      (2, incoming_shared_secret),
 -      (4, payment_hash),
 -      (6, amt_to_forward),
 -      (8, outgoing_cltv_value)
 -}, {}, {});
 +      (0, routing, required),
 +      (2, incoming_shared_secret, required),
 +      (4, payment_hash, required),
 +      (6, amt_to_forward, required),
 +      (8, outgoing_cltv_value, required)
 +});
  
  impl_writeable_tlv_based_enum!(HTLCFailureMsg, ;
        (0, Relay),
@@@ -4347,58 -4353,60 +4353,58 @@@ impl_writeable_tlv_based_enum!(PendingH
  );
  
  impl_writeable_tlv_based!(HTLCPreviousHopData, {
 -      (0, short_channel_id),
 -      (2, outpoint),
 -      (4, htlc_id),
 -      (6, incoming_packet_shared_secret)
 -}, {}, {});
 +      (0, short_channel_id, required),
 +      (2, outpoint, required),
 +      (4, htlc_id, required),
 +      (6, incoming_packet_shared_secret, required)
 +});
  
  impl_writeable_tlv_based!(ClaimableHTLC, {
 -      (0, prev_hop),
 -      (2, value),
 -      (4, payment_data),
 -      (6, cltv_expiry),
 -}, {}, {});
 +      (0, prev_hop, required),
 +      (2, value, required),
 +      (4, payment_data, required),
 +      (6, cltv_expiry, required),
 +});
  
  impl_writeable_tlv_based_enum!(HTLCSource,
        (0, OutboundRoute) => {
 -              (0, session_priv),
 -              (2, first_hop_htlc_msat),
 -      }, {}, {
 -              (4, path),
 -      };
 +              (0, session_priv, required),
 +              (2, first_hop_htlc_msat, required),
 +              (4, path, vec_type),
 +      }, ;
        (1, PreviousHopData)
  );
  
  impl_writeable_tlv_based_enum!(HTLCFailReason,
        (0, LightningError) => {
 -              (0, err),
 -      }, {}, {},
 +              (0, err, required),
 +      },
        (1, Reason) => {
 -              (0, failure_code),
 -      }, {}, {
 -              (2, data),
 +              (0, failure_code, required),
 +              (2, data, vec_type),
        },
  ;);
  
  impl_writeable_tlv_based_enum!(HTLCForwardInfo,
        (0, AddHTLC) => {
 -              (0, forward_info),
 -              (2, prev_short_channel_id),
 -              (4, prev_htlc_id),
 -              (6, prev_funding_outpoint),
 -      }, {}, {},
 +              (0, forward_info, required),
 +              (2, prev_short_channel_id, required),
 +              (4, prev_htlc_id, required),
 +              (6, prev_funding_outpoint, required),
 +      },
        (1, FailHTLC) => {
 -              (0, htlc_id),
 -              (2, err_packet),
 -      }, {}, {},
 +              (0, htlc_id, required),
 +              (2, err_packet, required),
 +      },
  ;);
  
  impl_writeable_tlv_based!(PendingInboundPayment, {
 -      (0, payment_secret),
 -      (2, expiry_time),
 -      (4, user_payment_id),
 -      (6, payment_preimage),
 -      (8, min_value_msat),
 -}, {}, {});
 +      (0, payment_secret, required),
 +      (2, expiry_time, required),
 +      (4, user_payment_id, required),
 +      (6, payment_preimage, required),
 +      (8, min_value_msat, required),
 +});
  
  impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable for ChannelManager<Signer, M, T, K, F, L>
        where M::Target: chain::Watch<Signer>,
                        session_priv.write(writer)?;
                }
  
 -              write_tlv_fields!(writer, {}, {});
 +              write_tlv_fields!(writer, {});
  
                Ok(())
        }
@@@ -4738,7 -4746,7 +4744,7 @@@ impl<'a, Signer: Sign, M: Deref, T: Der
                        }
                }
  
 -              read_tlv_fields!(reader, {}, {});
 +              read_tlv_fields!(reader, {});
  
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&args.keys_manager.get_secure_random_bytes());