From: Matt Corallo <git@bluematt.me>
Date: Wed, 15 Aug 2018 19:43:29 +0000 (-0400)
Subject: Handle duplicate funding transaction gracefully.
X-Git-Tag: v0.0.12~350^2~1
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=609054eae0c7c8cf08645cc6399fd11e840a6d96;p=rust-lightning

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 8f7ec2df0..54cb838f8 100644
--- 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 14b21e23a..e142e8ad5 100644
--- 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)
 	}