From a6161210e251ada6309f7bdeee560ed13ec15443 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 2 Aug 2018 06:23:53 -0400 Subject: [PATCH] Fix panic on reorg through the funding_locked-generating block We had a TODO to handle "lost confirmation" in block_connected, which we recently did in block_disconnected (calling force_shutdown in case we get too many blocks disconnected) but didn't handle the case where we had a simple reorg through the block that resulted in us generating a funding_locked. --- src/ln/channel.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/ln/channel.rs b/src/ln/channel.rs index 43f913eb2..68a23dc67 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -1933,27 +1933,38 @@ impl Channel { self.last_block_connected = header.bitcoin_hash(); self.funding_tx_confirmations += 1; if self.funding_tx_confirmations == CONF_TARGET as u64 { - if non_shutdown_state == ChannelState::FundingSent as u32 { + 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 & BOTH_SIDES_SHUTDOWN_MASK); self.channel_update_count += 1; - //TODO: Something about a state where we "lost confirmation" + true + } else if self.channel_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?"); - } + 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. - 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).unwrap(); - return Ok(Some(msgs::FundingLocked { - channel_id: self.channel_id, - next_per_commitment_point: next_per_commitment_point, - })); + if need_commitment_update { + 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).unwrap(); + return Ok(Some(msgs::FundingLocked { + channel_id: self.channel_id, + next_per_commitment_point: next_per_commitment_point, + })); + } } } } -- 2.39.5