Make funding_transaction_generated take a ChannelId by value

`ChannelId` is just a 32-byte array, so there's not a lot of value
in passing it by reference to `funding_transaction_generated`,
which we fix here.

This is also nice for bindings as languages like Java can better
analyze whether the `ChannelManager` ends up with a reference to
the `ChannelId`.

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -861,29 +861,29 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
 			$source.handle_accept_channel(&$dest.get_our_node_id(), &accept_channel);
 			let funding_output;
 			{
-				let events = $source.get_and_clear_pending_events();
+				let mut events = $source.get_and_clear_pending_events();
 				assert_eq!(events.len(), 1);
 				if let events::Event::FundingGenerationReady {
-					ref temporary_channel_id,
-					ref channel_value_satoshis,
-					ref output_script,
+					temporary_channel_id,
+					channel_value_satoshis,
+					output_script,
 					..
-				} = events[0]
+				} = events.pop().unwrap()
 				{
 					let tx = Transaction {
 						version: Version($chan_id),
 						lock_time: LockTime::ZERO,
 						input: Vec::new(),
 						output: vec![TxOut {
-							value: Amount::from_sat(*channel_value_satoshis),
-							script_pubkey: output_script.clone(),
+							value: Amount::from_sat(channel_value_satoshis),
+							script_pubkey: output_script,
 						}],
 					};
 					funding_output = OutPoint { txid: tx.txid(), index: 0 };
 					$source
 						.funding_transaction_generated(
-							&temporary_channel_id,
-							&$dest.get_our_node_id(),
+							temporary_channel_id,
+							$dest.get_our_node_id(),
 							tx.clone(),
 						)
 						.unwrap();

lightning-background-processor/src/lib.rs

@@ -1628,8 +1628,8 @@ mod tests {
 			$node_a
 				.node
 				.funding_transaction_generated(
-					&temporary_channel_id,
-					&$node_b.node.get_our_node_id(),
+					temporary_channel_id,
+					$node_b.node.get_our_node_id(),
 					tx.clone(),
 				)
 				.unwrap();
@@ -2062,7 +2062,7 @@ mod tests {
 			.expect("FundingGenerationReady not handled within deadline");
 		nodes[0]
 			.node
-			.funding_transaction_generated(&temporary_channel_id, &node_1_id, funding_tx.clone())
+			.funding_transaction_generated(temporary_channel_id, node_1_id, funding_tx.clone())
 			.unwrap();
 		let msg_0 = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, node_1_id);
 		nodes[1].node.handle_funding_created(&node_0_id, &msg_0);

lightning/src/ln/async_signer_tests.rs

@@ -49,7 +49,7 @@ fn test_async_commitment_signature_for_funding_created() {
 	// message...
 	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
 	nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &temporary_channel_id, SignerOp::SignCounterpartyCommitment);
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
+	nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
 	check_added_monitors(&nodes[0], 0);
 
 	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
@@ -96,7 +96,7 @@ fn test_async_commitment_signature_for_funding_signed() {
 
 	// nodes[0] --- funding_created --> nodes[1]
 	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
+	nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
 	check_added_monitors(&nodes[0], 0);
 
 	let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -236,7 +236,7 @@ fn test_async_commitment_signature_for_funding_signed_0conf() {
 
 	// nodes[0] --- funding_created --> nodes[1]
 	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
+	nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
 	check_added_monitors(&nodes[0], 0);
 
 	let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -1861,7 +1861,7 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
 
 	let (temporary_channel_id, funding_tx, funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
 
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
+	nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
 	check_added_monitors!(nodes[0], 0);
 
 	chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
@@ -2795,7 +2795,7 @@ fn do_test_outbound_reload_without_init_mon(use_0conf: bool) {
 
 	let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
 
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
+	nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
 	check_added_monitors!(nodes[0], 0);
 
 	let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -2886,7 +2886,7 @@ fn do_test_inbound_reload_without_init_mon(use_0conf: bool, lock_commitment: boo
 
 	let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
 
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
+	nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
 	check_added_monitors!(nodes[0], 0);
 
 	let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());

lightning/src/ln/channelmanager.rs

@@ -1452,7 +1452,7 @@ where
 ///                 channel_value_satoshis, output_script
 ///             );
 ///             match channel_manager.funding_transaction_generated(
-///                 &temporary_channel_id, &counterparty_node_id, funding_transaction
+///                 temporary_channel_id, counterparty_node_id, funding_transaction
 ///             ) {
 ///                 Ok(()) => println!("Funding channel {}", temporary_channel_id),
 ///                 Err(e) => println!("Error funding channel {}: {:?}", temporary_channel_id, e),
@@ -4444,17 +4444,17 @@ where
 	/// Handles the generation of a funding transaction, optionally (for tests) with a function
 	/// which checks the correctness of the funding transaction given the associated channel.
 	fn funding_transaction_generated_intern<FundingOutput: FnMut(&OutboundV1Channel<SP>) -> Result<OutPoint, &'static str>>(
-		&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction, is_batch_funding: bool,
+		&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding_transaction: Transaction, is_batch_funding: bool,
 		mut find_funding_output: FundingOutput, is_manual_broadcast: bool,
 	) -> Result<(), APIError> {
 		let per_peer_state = self.per_peer_state.read().unwrap();
-		let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+		let peer_state_mutex = per_peer_state.get(&counterparty_node_id)
 			.ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
 
 		let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 		let peer_state = &mut *peer_state_lock;
 		let funding_txo;
-		let (mut chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
+		let (mut chan, msg_opt) = match peer_state.channel_by_id.remove(&temporary_channel_id) {
 			Some(ChannelPhase::UnfundedOutboundV1(mut chan)) => {
 				macro_rules! close_chan { ($err: expr, $api_err: expr, $chan: expr) => { {
 					let counterparty;
@@ -4490,7 +4490,7 @@ where
 				}
 			},
 			Some(phase) => {
-				peer_state.channel_by_id.insert(*temporary_channel_id, phase);
+				peer_state.channel_by_id.insert(temporary_channel_id, phase);
 				return Err(APIError::APIMisuseError {
 					err: format!(
 						"Channel with id {} for the passed counterparty node_id {} is not an unfunded, outbound V1 channel",
@@ -4540,7 +4540,7 @@ where
 	}
 
 	#[cfg(test)]
-	pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
+	pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
 		let txid = funding_transaction.txid();
 		self.funding_transaction_generated_intern(temporary_channel_id, counterparty_node_id, funding_transaction, false, |_| {
 			Ok(OutPoint { txid, index: output_index })
@@ -4577,8 +4577,8 @@ where
 	///
 	/// [`Event::FundingGenerationReady`]: crate::events::Event::FundingGenerationReady
 	/// [`Event::ChannelClosed`]: crate::events::Event::ChannelClosed
-	pub fn funding_transaction_generated(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
-		self.batch_funding_transaction_generated(&[(temporary_channel_id, counterparty_node_id)], funding_transaction)
+	pub fn funding_transaction_generated(&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
+		self.batch_funding_transaction_generated(&[(&temporary_channel_id, &counterparty_node_id)], funding_transaction)
 	}
 
 
@@ -4609,10 +4609,10 @@ where
 	/// [`Event::FundingTxBroadcastSafe`]: crate::events::Event::FundingTxBroadcastSafe
 	/// [`Event::ChannelClosed`]: crate::events::Event::ChannelClosed
 	/// [`ChannelManager::funding_transaction_generated`]: crate::ln::channelmanager::ChannelManager::funding_transaction_generated
-	pub fn unsafe_manual_funding_transaction_generated(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding: OutPoint) -> Result<(), APIError> {
+	pub fn unsafe_manual_funding_transaction_generated(&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding: OutPoint) -> Result<(), APIError> {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
-		let temporary_channels = &[(temporary_channel_id, counterparty_node_id)];
+		let temporary_channels = &[(&temporary_channel_id, &counterparty_node_id)];
 		return self.batch_funding_transaction_generated_intern(temporary_channels, FundingType::Unchecked(funding));
 
 	}
@@ -4686,8 +4686,8 @@ where
 		let is_manual_broadcast = funding.is_manual_broadcast();
 		for &(temporary_channel_id, counterparty_node_id) in temporary_channels {
 			result = result.and_then(|_| self.funding_transaction_generated_intern(
-				temporary_channel_id,
-				counterparty_node_id,
+				*temporary_channel_id,
+				*counterparty_node_id,
 				funding.transaction_or_dummy(),
 				is_batch_funding,
 				|chan| {
@@ -13325,7 +13325,7 @@ mod tests {
 			assert_eq!(nodes[1].node.outpoint_to_peer.lock().unwrap().len(), 0);
 		}
 
-		nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
+		nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
 		{
 			// Assert that `nodes[0]`'s `outpoint_to_peer` map is populated with the channel as soon as
 			// as it has the funding transaction.
@@ -13537,7 +13537,7 @@ mod tests {
 				nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
 				let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100_000, 42);
 				funding_tx = Some(tx.clone());
-				nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx).unwrap();
+				nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx).unwrap();
 				let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
 
 				nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
@@ -14192,7 +14192,7 @@ pub mod bench {
 			tx = Transaction { version: Version::TWO, lock_time: LockTime::ZERO, input: Vec::new(), output: vec![TxOut {
 				value: Amount::from_sat(8_000_000), script_pubkey: output_script,
 			}]};
-			node_a.funding_transaction_generated(&temporary_channel_id, &node_b.get_our_node_id(), tx.clone()).unwrap();
+			node_a.funding_transaction_generated(temporary_channel_id, node_b.get_our_node_id(), tx.clone()).unwrap();
 		} else { panic!(); }
 
 		node_b.handle_funding_created(&node_a.get_our_node_id(), &get_event_msg!(node_a_holder, MessageSendEvent::SendFundingCreated, node_b.get_our_node_id()));

lightning/src/ln/functional_test_utils.rs

@@ -1218,7 +1218,7 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
 	let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, &node_b.node.get_our_node_id(), channel_value, 42);
 	assert_eq!(temporary_channel_id, expected_temporary_channel_id);
 
-	assert!(node_a.node.funding_transaction_generated(&temporary_channel_id, &node_b.node.get_our_node_id(), tx.clone()).is_ok());
+	assert!(node_a.node.funding_transaction_generated(temporary_channel_id, node_b.node.get_our_node_id(), tx.clone()).is_ok());
 	check_added_monitors!(node_a, 0);
 
 	let funding_created_msg = get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id());
@@ -1249,7 +1249,7 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
 	node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
 
 	// Ensure that funding_transaction_generated is idempotent.
-	assert!(node_a.node.funding_transaction_generated(&temporary_channel_id, &node_b.node.get_our_node_id(), tx.clone()).is_err());
+	assert!(node_a.node.funding_transaction_generated(temporary_channel_id, node_b.node.get_our_node_id(), tx.clone()).is_err());
 	assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
 	check_added_monitors!(node_a, 0);
 
@@ -1279,7 +1279,7 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, r
 	initiator.node.handle_accept_channel(&receiver.node.get_our_node_id(), &accept_channel);
 
 	let (temporary_channel_id, tx, _) = create_funding_transaction(&initiator, &receiver.node.get_our_node_id(), 100_000, 42);
-	initiator.node.funding_transaction_generated(&temporary_channel_id, &receiver.node.get_our_node_id(), tx.clone()).unwrap();
+	initiator.node.funding_transaction_generated(temporary_channel_id, receiver.node.get_our_node_id(), tx.clone()).unwrap();
 	let funding_created = get_event_msg!(initiator, MessageSendEvent::SendFundingCreated, receiver.node.get_our_node_id());
 
 	receiver.node.handle_funding_created(&initiator.node.get_our_node_id(), &funding_created);
@@ -1456,7 +1456,7 @@ pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &
 	nodes[a].node.handle_accept_channel(&nodes[b].node.get_our_node_id(), &accept_channel);
 
 	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[a], &nodes[b].node.get_our_node_id(), channel_value, 42);
-	nodes[a].node.funding_transaction_generated(&temporary_channel_id, &nodes[b].node.get_our_node_id(), tx.clone()).unwrap();
+	nodes[a].node.funding_transaction_generated(temporary_channel_id, nodes[b].node.get_our_node_id(), tx.clone()).unwrap();
 	nodes[b].node.handle_funding_created(&nodes[a].node.get_our_node_id(), &get_event_msg!(nodes[a], MessageSendEvent::SendFundingCreated, nodes[b].node.get_our_node_id()));
 	check_added_monitors!(nodes[b], 1);