Merge pull request #954 from TheBlueMatt/2021-06-no-spurious-forward-fails
[rust-lightning] / lightning / src / ln / channel.rs
index 06308b01cee0a3c1141ea426b7fcbd07343a2a99..1d3e264a3fcbf263eec75e3b85e8d0b65c81e024 100644 (file)
@@ -288,7 +288,7 @@ impl HTLCCandidate {
 }
 
 /// Information needed for constructing an invoice route hint for this channel.
-#[derive(Clone)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct CounterpartyForwardingInfo {
        /// Base routing fee in millisatoshis.
        pub fee_base_msat: u32,
@@ -434,6 +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 @@ 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,
                })
        }
 
@@ -876,6 +887,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,
                };
 
                Ok(chan)
@@ -1703,7 +1716,8 @@ impl<Signer: Sign> Channel<Signer> {
 
        pub fn funding_locked<L: Deref>(&mut self, msg: &msgs::FundingLocked, logger: &L) -> Result<(), ChannelError> where L::Target: Logger {
                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);
@@ -3499,7 +3513,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// is_usable() and considers things like the channel being temporarily disabled.
        /// Allowed in any state (including after shutdown)
        pub fn is_live(&self) -> bool {
-               self.is_usable() && (self.channel_state & (ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) == 0)
+               self.is_usable() && (self.channel_state & (ChannelState::PeerDisconnected as u32) == 0)
        }
 
        /// Returns true if this channel has been marked as awaiting a monitor update to move forward.
@@ -4016,10 +4030,18 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Adds a pending outbound HTLC to this channel, note that you probably want
        /// send_htlc_and_commit instead cause you'll want both messages at once.
-       /// This returns an option instead of a pure UpdateAddHTLC as we may be in a state where we are
-       /// waiting on the remote peer to send us a revoke_and_ack during which time we cannot add new
-       /// HTLCs on the wire or we wouldn't be able to determine what they actually ACK'ed.
-       /// You MUST call send_commitment prior to any other calls on this Channel
+       ///
+       /// This returns an optional UpdateAddHTLC as we may be in a state where we cannot add HTLCs on
+       /// the wire:
+       /// * In cases where we're waiting on the remote peer to send us a revoke_and_ack, we
+       ///   wouldn't be able to determine what they actually ACK'ed if we have two sets of updates
+       ///   awaiting ACK.
+       /// * In cases where we're marked MonitorUpdateFailed, we cannot commit to a new state as we
+       ///   may not yet have sent the previous commitment update messages and will need to regenerate
+       ///   them.
+       ///
+       /// You MUST call send_commitment prior to calling any other methods on this Channel!
+       ///
        /// If an Err is returned, it's a ChannelError::Ignore!
        pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelFunded as u32) {
@@ -4038,14 +4060,14 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Ignore(format!("Cannot send less than their minimum HTLC value ({})", self.counterparty_htlc_minimum_msat)));
                }
 
-               if (self.channel_state & (ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 {
+               if (self.channel_state & (ChannelState::PeerDisconnected as u32)) != 0 {
                        // Note that this should never really happen, if we're !is_live() on receipt of an
                        // incoming HTLC for relay will result in us rejecting the HTLC and we won't allow
                        // the user to send directly into a !is_live() channel. However, if we
                        // disconnected during the time the previous hop was doing the commitment dance we may
                        // end up getting here after the forwarding delay. In any case, returning an
                        // IgnoreError will get ChannelManager to do the right thing and fail backwards now.
-                       return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected/frozen for channel monitor update".to_owned()));
+                       return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned()));
                }
 
                let (outbound_htlc_count, htlc_outbound_value_msat) = self.get_outbound_pending_htlc_stats();
@@ -4090,7 +4112,7 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                // Now update local state:
-               if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
+               if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
                                amount_msat,
                                payment_hash,
@@ -4647,7 +4669,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
                self.channel_update_status.write(writer)?;
 
-               write_tlv_fields!(writer, {}, {(0, self.announcement_sigs)});
+               write_tlv_fields!(writer, {(0, self.announcement_sigs, option)});
 
                Ok(())
        }
@@ -4820,7 +4842,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                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());
@@ -4904,6 +4926,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for 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,
                })
        }
 }