From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue, 11 Feb 2020 05:36:01 +0000 (+0000) Subject: Merge pull request #491 from TheBlueMatt/2020-02-one-conf-lock X-Git-Tag: v0.0.12~139 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=1619d081630edddb5ee3cadc50ab579994c1b4f8;hp=b7f4f764afbe7105d9fa5f70e2c30c71ad90caa2;p=rust-lightning Merge pull request #491 from TheBlueMatt/2020-02-one-conf-lock Fix one-confirmation funding_locked generation --- diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 231ee832b..58ade789f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2987,49 +2987,8 @@ impl Channel { pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result, 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 Channel { } } } + 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) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3c1c0b240..daf66ffe2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1268,7 +1268,10 @@ impl ChannelManager where M::T } fn get_announcement_sigs(&self, chan: &Channel) -> Option { - if !chan.should_announce() { return None } + if !chan.should_announce() { + log_trace!(self, "Can't send announcement_signatures for private channel {}", log_bytes!(chan.channel_id())); + return None + } let (announcement, our_bitcoin_sig) = match chan.get_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone()) { Ok(res) => res, @@ -1984,6 +1987,7 @@ impl ChannelManager where M::T } try_chan_entry!(self, chan.get_mut().funding_locked(&msg), channel_state, chan); if let Some(announcement_sigs) = self.get_announcement_sigs(chan.get()) { + log_trace!(self, "Sending announcement_signatures for {} in response to funding_locked", log_bytes!(chan.get().channel_id())); // If we see locking block before receiving remote funding_locked, we broadcast our // announcement_sigs at remote funding_locked reception. If we receive remote // funding_locked before seeing locking block, we broadcast our announcement_sigs at locking @@ -2578,10 +2582,13 @@ impl ChainListener for ChannelM msg: funding_locked, }); if let Some(announcement_sigs) = self.get_announcement_sigs(channel) { + log_trace!(self, "Sending funding_locked and announcement_signatures for {}", log_bytes!(channel.channel_id())); pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures { node_id: channel.get_their_node_id(), msg: announcement_sigs, }); + } else { + log_trace!(self, "Sending funding_locked WITHOUT announcement_signatures for {}", log_bytes!(channel.channel_id())); } short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id()); } else if let Err(e) = chan_res { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4006c87a6..7c379d213 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -378,6 +378,41 @@ fn test_multi_flight_update_fee() { check_added_monitors!(nodes[1], 1); } +#[test] +fn test_1_conf_open() { + // Previously, if the minium_depth config was set to 1, we'd never send a funding_locked. This + // tests that we properly send one in that case. + let mut alice_config = UserConfig::default(); + alice_config.own_channel_config.minimum_depth = 1; + alice_config.channel_options.announced_channel = true; + alice_config.peer_channel_config_limits.force_announced_channel_preference = false; + let mut bob_config = UserConfig::default(); + bob_config.own_channel_config.minimum_depth = 1; + bob_config.channel_options.announced_channel = true; + bob_config.peer_channel_config_limits.force_announced_channel_preference = false; + let node_cfgs = create_node_cfgs(2); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(alice_config), Some(bob_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::supported(), InitFeatures::supported()); + assert!(nodes[0].chain_monitor.does_match_tx(&tx)); + assert!(nodes[1].chain_monitor.does_match_tx(&tx)); + + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + nodes[1].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version; 1]); + nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id())); + + nodes[0].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version; 1]); + let (funding_locked, _) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]); + let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked); + + for node in nodes { + assert!(node.router.handle_channel_announcement(&announcement).unwrap()); + node.router.handle_channel_update(&as_update).unwrap(); + node.router.handle_channel_update(&bs_update).unwrap(); + } +} + #[test] fn test_update_fee_vanilla() { let node_cfgs = create_node_cfgs(2);