Fix sending funding_locked with 1 conf.

We previously tracked funding transaction confirmation by marking
funding_tx_confirmations to 1 when we see it in a block and
incrementing each block thereafter if its non-0. To avoid
double-incrementing the first confirmation, we did the increment
(and funding_locked check) after doing the first-confirmation
checks. Thus, we'd never hit the funding_locked case during the
first confirmation.

To address this, we simply swap the order of the checks, though
bumping the funding_tx_confirmations increment up to the top.

Reported-by: Igor Cota <igor@codexapertus.com>

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 231ee832befe77a32c1166c8ab2ba87151623934..58ade789fbb16c16039b7c1cd200d224093aa180 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2987,49 +2987,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> {
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
                if header.bitcoin_hash() != self.last_block_connected {
-                       self.last_block_connected = header.bitcoin_hash();
-                       self.channel_monitor.last_block_hash = self.last_block_connected;
                        if self.funding_tx_confirmations > 0 {
                                self.funding_tx_confirmations += 1;
-                               if self.funding_tx_confirmations == self.minimum_depth as u64 {
-                                       let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
-                                               self.channel_state |= ChannelState::OurFundingLocked as u32;
-                                               true
-                                       } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
-                                               self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
-                                               self.channel_update_count += 1;
-                                               true
-                                       } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
-                                               // We got a reorg but not enough to trigger a force close, just update
-                                               // funding_tx_confirmed_in and return.
-                                               false
-                                       } else if self.channel_state < ChannelState::ChannelFunded as u32 {
-                                               panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state);
-                                       } else {
-                                               // We got a reorg but not enough to trigger a force close, just update
-                                               // funding_tx_confirmed_in and return.
-                                               false
-                                       };
-                                       self.funding_tx_confirmed_in = Some(header.bitcoin_hash());
-
-                                       //TODO: Note that this must be a duplicate of the previous commitment point they sent us,
-                                       //as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
-                                       //they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
-                                       //a protocol oversight, but I assume I'm just missing something.
-                                       if need_commitment_update {
-                                               if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
-                                                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
-                                                       let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
-                                                       return Ok(Some(msgs::FundingLocked {
-                                                               channel_id: self.channel_id,
-                                                               next_per_commitment_point: next_per_commitment_point,
-                                                       }));
-                                               } else {
-                                                       self.monitor_pending_funding_locked = true;
-                                                       return Ok(None);
-                                               }
-                                       }
-                               }
                        }
                }
                if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
@@ -3072,6 +3031,51 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                }
                        }
                }
+               if header.bitcoin_hash() != self.last_block_connected {
+                       self.last_block_connected = header.bitcoin_hash();
+                       self.channel_monitor.last_block_hash = self.last_block_connected;
+                       if self.funding_tx_confirmations > 0 {
+                               if self.funding_tx_confirmations == self.minimum_depth as u64 {
+                                       let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
+                                               self.channel_state |= ChannelState::OurFundingLocked as u32;
+                                               true
+                                       } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
+                                               self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
+                                               self.channel_update_count += 1;
+                                               true
+                                       } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
+                                               // We got a reorg but not enough to trigger a force close, just update
+                                               // funding_tx_confirmed_in and return.
+                                               false
+                                       } else if self.channel_state < ChannelState::ChannelFunded as u32 {
+                                               panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state);
+                                       } else {
+                                               // We got a reorg but not enough to trigger a force close, just update
+                                               // funding_tx_confirmed_in and return.
+                                               false
+                                       };
+                                       self.funding_tx_confirmed_in = Some(header.bitcoin_hash());
+
+                                       //TODO: Note that this must be a duplicate of the previous commitment point they sent us,
+                                       //as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
+                                       //they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
+                                       //a protocol oversight, but I assume I'm just missing something.
+                                       if need_commitment_update {
+                                               if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
+                                                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
+                                                       let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
+                                                       return Ok(Some(msgs::FundingLocked {
+                                                               channel_id: self.channel_id,
+                                                               next_per_commitment_point: next_per_commitment_point,
+                                                       }));
+                                               } else {
+                                                       self.monitor_pending_funding_locked = true;
+                                                       return Ok(None);
+                                               }
+                                       }
+                               }
+                       }
+               }
                Ok(None)
        }