Merge pull request #491 from TheBlueMatt/2020-02-one-conf-lock
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Tue, 11 Feb 2020 05:36:01 +0000 (05:36 +0000)
committerGitHub <noreply@github.com>
Tue, 11 Feb 2020 05:36:01 +0000 (05:36 +0000)
Fix one-confirmation funding_locked generation

lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs

index 231ee832befe77a32c1166c8ab2ba87151623934..58ade789fbb16c16039b7c1cd200d224093aa180 100644 (file)
@@ -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)
        }
 
index 3c1c0b240ae6d666bae64b4c1ebc82cba1f4396e..daf66ffe218d3621eb8475a9f5b73f1abb5349b7 100644 (file)
@@ -1268,7 +1268,10 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
        }
 
        fn get_announcement_sigs(&self, chan: &Channel<ChanSigner>) -> Option<msgs::AnnouncementSignatures> {
-               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<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> 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<ChanSigner: ChannelKeys, M: Deref + Sync + Send> 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 {
index 4006c87a684a62d5d1a82a6e202a5a900bdd2f86..7c379d21356b21f42ba93f545e9861c905b55ca0 100644 (file)
@@ -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);