Handle duplicate funding transaction gracefully.

This can otherwise cause strange behavior, including a panic on
force_close_all_channels found by full_stack_target fuzzing.

diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs
index 8f7ec2df0a76013918e2ec7b154deceed7057cab..54cb838f857c654337a26e5511df66425d68954c 100644 (file)
--- a/fuzz/fuzz_targets/full_stack_target.rs
+++ b/fuzz/fuzz_targets/full_stack_target.rs
@@ -300,13 +300,21 @@ pub fn do_test(data: &[u8]) {
                                }
                        },
                        10 => {
-                               for funding_generation in  pending_funding_generation.drain(..) {
+                               for funding_generation in pending_funding_generation.drain(..) {
                                        let mut tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: vec![TxOut {
                                                        value: funding_generation.1, script_pubkey: funding_generation.2,
                                                }] };
                                        let funding_output = OutPoint::new(Sha256dHash::from_data(&serialize(&tx).unwrap()[..]), 0);
-                                       channelmanager.funding_transaction_generated(&funding_generation.0, funding_output.clone());
-                                       pending_funding_signatures.insert(funding_output, tx);
+                                       let mut found_duplicate_txo = false;
+                                       for chan in channelmanager.list_channels() {
+                                               if chan.channel_id == funding_output.to_channel_id() {
+                                                       found_duplicate_txo = true;
+                                               }
+                                       }
+                                       if !found_duplicate_txo {
+                                               channelmanager.funding_transaction_generated(&funding_generation.0, funding_output.clone());
+                                               pending_funding_signatures.insert(funding_output, tx);
+                                       }
                                }
                        },
                        11 => {

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 14b21e23a79b4928454c77a5c0786b2ac975adcf..e142e8ad5091bb52a9290e1694fbf0e6df6f000f 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -770,6 +770,8 @@ impl ChannelManager {
 
        /// Call this upon creation of a funding transaction for the given channel.
        /// Panics if a funding transaction has already been provided for this channel.
+       /// May panic if the funding_txo is duplicative with some other channel (note that this should
+       /// be trivially prevented by using unique funding transaction keys per-channel).
        pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_txo: OutPoint) {
 
                macro_rules! add_pending_event {
@@ -812,7 +814,14 @@ impl ChannelManager {
                });
 
                let mut channel_state = self.channel_state.lock().unwrap();
-               channel_state.by_id.insert(chan.channel_id(), chan);
+               match channel_state.by_id.entry(chan.channel_id()) {
+                       hash_map::Entry::Occupied(_) => {
+                               panic!("Generated duplicate funding txid?");
+                       },
+                       hash_map::Entry::Vacant(e) => {
+                               e.insert(chan);
+                       }
+               }
        }
 
        fn get_announcement_sigs(&self, chan: &Channel) -> Result<Option<msgs::AnnouncementSignatures>, HandleError> {
@@ -1350,7 +1359,17 @@ impl ChannelMessageHandler for ChannelManager {
                        unimplemented!();
                }
                let mut channel_state = self.channel_state.lock().unwrap();
-               channel_state.by_id.insert(funding_msg.channel_id, chan);
+               match channel_state.by_id.entry(funding_msg.channel_id) {
+                       hash_map::Entry::Occupied(_) => {
+                               return Err(HandleError {
+                                       err: "Duplicate channel_id!",
+                                       action: Some(msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id: funding_msg.channel_id, data: "Already had channel with the new channel_id".to_owned() } })
+                               });
+                       },
+                       hash_map::Entry::Vacant(e) => {
+                               e.insert(chan);
+                       }
+               }
                Ok(funding_msg)
        }