Make channel req response only require channel_id

Make the only information the respond_to_open_channel_request function
require about the channel be the the temporary channel ID. This removes
the need to pass excessive amounts of data with the OpenChannelRequest
event.

Author: ViktorTigerstrom

lightning/src/ln/channel.rs

@@ -536,6 +536,17 @@ pub(super) struct Channel<Signer: Sign> {
 	#[cfg(not(test))]
 	closing_fee_limits: Option<(u64, u64)>,
 
+	/// Flag that ensures that the `get_accept_channel` function must be called before the
+	/// `funding_created` function is executed successfully. The reason for need of this flag is
+	/// that when the util::config::UserConfig.manually_accept_inbound_channels is set to true,
+	/// inbound channels are required to be manually accepted by the node operator before the
+	/// `SendAcceptChannel` message is created and sent out. The `get_accept_channel` function is
+	/// called during that process.
+	/// A counterparty node could theoretically send a `FundingCreated` message before the node
+	/// operator has accepted the inbound channel. That would would execute the `funding_created`
+	/// function before the `get_accept_channel` function, and should therefore be rejected.
+	inbound_awaiting_accept: bool,
+
 	/// The hash of the block in which the funding transaction was included.
 	funding_tx_confirmed_in: Option<BlockHash>,
 	funding_tx_confirmation_height: u32,
@@ -833,6 +844,8 @@ impl<Signer: Sign> Channel<Signer> {
 			closing_fee_limits: None,
 			target_closing_feerate_sats_per_kw: None,
 
+			inbound_awaiting_accept: false,
+
 			funding_tx_confirmed_in: None,
 			funding_tx_confirmation_height: 0,
 			short_channel_id: None,
@@ -1130,6 +1143,8 @@ impl<Signer: Sign> Channel<Signer> {
 			closing_fee_limits: None,
 			target_closing_feerate_sats_per_kw: None,
 
+			inbound_awaiting_accept: true,
+
 			funding_tx_confirmed_in: None,
 			funding_tx_confirmation_height: 0,
 			short_channel_id: None,
@@ -1918,6 +1933,10 @@ impl<Signer: Sign> Channel<Signer> {
 			// channel.
 			return Err(ChannelError::Close("Received funding_created after we got the channel!".to_owned()));
 		}
+		if self.inbound_awaiting_accept {
+			// The Channel must manually accepted before the FundingCreated is received.
+			return Err(ChannelError::Close("FundingCreated message received before the channel was accepted".to_owned()));
+		}
 		if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
 				self.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
 				self.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
@@ -4504,7 +4523,11 @@ impl<Signer: Sign> Channel<Signer> {
 		}
 	}
 
-	pub fn get_accept_channel(&self) -> msgs::AcceptChannel {
+	pub fn inbound_is_awaiting_accept(&self) -> bool {
+		self.inbound_awaiting_accept
+	}
+
+	pub fn get_accept_channel(&mut self) -> msgs::AcceptChannel {
 		if self.is_outbound() {
 			panic!("Tried to send accept_channel for an outbound channel?");
 		}
@@ -4515,6 +4538,8 @@ impl<Signer: Sign> Channel<Signer> {
 			panic!("Tried to send an accept_channel for a channel that has already advanced");
 		}
 
+		self.inbound_awaiting_accept = false;
+
 		let first_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
 		let keys = self.get_holder_pubkeys();
 
@@ -5840,6 +5865,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			closing_fee_limits: None,
 			target_closing_feerate_sats_per_kw,
 
+			inbound_awaiting_accept: false,
+
 			funding_tx_confirmed_in,
 			funding_tx_confirmation_height,
 			short_channel_id,
@@ -6050,7 +6077,7 @@ mod tests {
 		// Make sure A's dust limit is as we expect.
 		let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
+		let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
 
 		// Node B --> Node A: accept channel, explicitly setting B's dust limit.
 		let mut accept_channel_msg = node_b_chan.get_accept_channel();

lightning/src/ln/channelmanager.rs

@@ -688,30 +688,6 @@ pub(super) struct ChannelHolder<Signer: Sign> {
 	pub(super) pending_msg_events: Vec<MessageSendEvent>,
 }
 
-/// Holds the information required for a user to respond to an open channel request, when the
-/// manually_accept_inbound_channels config flag is set to true.
-#[derive(Clone, Debug)]
-pub struct OpenChannelRequestInfo {
-	/// The public key of the peer node which is requesting to open a channel.
-	pub counterparty_node_id: PublicKey,
-	/// The init features sent in the init message.
-	pub their_features: InitFeatures,
-	/// The Open channel message sent by the peer when requesting to open the channel.
-	pub msg: msgs::OpenChannel
-}
-
-impl Writeable for OpenChannelRequestInfo {
-	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
-		11u8.write(writer)?;
-		write_tlv_fields!(writer, {
-			(0, self.counterparty_node_id, required),
-			(2, self.their_features, required),
-			(4, self.msg, required),
-		});
-		Ok(())
-	}
-}
-
 /// Events which we process internally but cannot be procsesed immediately at the generation site
 /// for some reason. They are handled in timer_tick_occurred, so may be processed with
 /// quite some time lag.
@@ -4128,24 +4104,27 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// Called to respond to a request to open a channel after the
 	/// [`events::Event::OpenChannelRequest`] has been triggered.
 	///
-	/// If 'accept_channel' is set to true, the requested channel will be opened. If
-	/// 'accept_channel' is false, the open channel request will be rejected.
+	/// If `accept_channel` is set to true, the requested channel will be opened. If
+	/// `accept_channel` is false, the open channel request will be rejected.
 	///
-	/// The 'open_channel_request_info' parameter in the [`events::Event::OpenChannelRequest`]
-	/// event, should be directly passed as input to this function.
+	/// The `temporary_channel_id` parameter indicates which channel the reponse is for.
 	///
 	/// [`events::Event::OpenChannelRequest`]: crate::util::events::Event::OpenChannelRequest
-	pub fn respond_to_open_channel_request(&self, accept_channel: bool, open_channel_request_info: OpenChannelRequestInfo) {
+	pub fn respond_to_open_channel_request(&self, accept_channel: bool, temporary_channel_id: [u8; 32]) -> Result<(), APIError> {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
-		match accept_channel {
-			true => {
-				let _ = handle_error!(self, self.internal_create_requested_channel(&open_channel_request_info.counterparty_node_id, open_channel_request_info.their_features, &open_channel_request_info.msg), open_channel_request_info.counterparty_node_id);
-			}
-			false => {
-				let err_res: Result<(), MsgHandleErrInternal> = Err(MsgHandleErrInternal::send_err_msg_no_close("The peer rejected the request to open the channel".to_owned(), open_channel_request_info.msg.temporary_channel_id.clone()));
-				let _ = handle_error!(self, err_res, open_channel_request_info.counterparty_node_id);
+
+		if let Some(counterparty_node_id) = self.internal_get_counterparty_node_id(temporary_channel_id) {
+			match accept_channel {
+				true => {
+					return self.accept_requested_channel(temporary_channel_id);
+				}
+				false => {
+					let err_res: Result<(), MsgHandleErrInternal> = Err(MsgHandleErrInternal::send_err_msg_no_close("The peer rejected the request to open the channel".to_owned(), temporary_channel_id));
+					let _ = handle_error!(self, err_res, counterparty_node_id);
+					return Ok(());
+				}
 			}
-		}
+		} else { return Err(APIError::ChannelUnavailable { err: "No channel with the inputted temporary_channel_id found".to_owned() }); }
 	}
 
 	fn internal_open_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
@@ -4157,42 +4136,69 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone()));
 		}
 
-		if self.default_configuration.manually_accept_inbound_channels {
-			let mut pending_events = self.pending_events.lock().unwrap();
-			pending_events.push(
-				events::Event::OpenChannelRequest {
-					open_channel_request_info: OpenChannelRequestInfo {
-						counterparty_node_id: counterparty_node_id.clone(),
-						their_features: their_features.clone(),
-						msg: msg.clone()
-					},
-				}
-			);
-			return Ok(())
-		}
-
-		return self.internal_create_requested_channel(counterparty_node_id, their_features, msg);
-	}
-
-	fn internal_create_requested_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal>{
-		let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(),
+		let mut channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(),
 				&their_features, msg, 0, &self.default_configuration, self.best_block.read().unwrap().height(), &self.logger)
 			.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
 		let mut channel_state_lock = self.channel_state.lock().unwrap();
 		let channel_state = &mut *channel_state_lock;
 		match channel_state.by_id.entry(channel.channel_id()) {
 			hash_map::Entry::Occupied(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision!".to_owned(), msg.temporary_channel_id.clone())),
 			hash_map::Entry::Vacant(entry) => {
+				if !self.default_configuration.manually_accept_inbound_channels {
+					channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
+						node_id: counterparty_node_id.clone(),
+						msg: channel.get_accept_channel(),
+					});
+				} else {
+					let mut pending_events = self.pending_events.lock().unwrap();
+					pending_events.push(
+						events::Event::OpenChannelRequest {
+							temporary_channel_id: msg.temporary_channel_id.clone(),
+							counterparty_node_id: counterparty_node_id.clone(),
+							chain_hash: msg.chain_hash.clone(),
+							funding_satoshis: msg.funding_satoshis,
+							push_msat: msg.push_msat,
+						}
+					);
+				}
+
+				entry.insert(channel);
+			}
+		}
+		Ok(())
+	}
+
+	fn accept_requested_channel(&self, temporary_channel_id: [u8; 32]) -> Result<(), APIError> {
+		let mut channel_state_lock = self.channel_state.lock().unwrap();
+		let channel_state = &mut *channel_state_lock;
+		match channel_state.by_id.entry(temporary_channel_id) {
+			hash_map::Entry::Occupied(mut channel) => {
+				if !channel.get().inbound_is_awaiting_accept() {
+					return Err(APIError::APIMisuseError { err: "The channel isn't currently awaiting to be accepted.".to_owned() });
+				}
 				channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
-					node_id: counterparty_node_id.clone(),
-					msg: channel.get_accept_channel(),
+					node_id: channel.get().get_counterparty_node_id(),
+					msg: channel.get_mut().get_accept_channel(),
 				});
-				entry.insert(channel);
+			}
+			hash_map::Entry::Vacant(_) => {
+				return Err(APIError::ChannelUnavailable { err: "Can't accept a channel that doesn't exist".to_owned() });
 			}
 		}
 		Ok(())
 	}
 
+	fn internal_get_counterparty_node_id(&self, channel_id: [u8; 32]) -> Option<PublicKey> {
+		match self.channel_state.lock().unwrap().by_id.entry(channel_id) {
+			hash_map::Entry::Occupied(channel) => {
+				return Some(channel.get().get_counterparty_node_id());
+			}
+			hash_map::Entry::Vacant(_) => {
+				return None;
+			}
+		}
+	}
+
 	fn internal_accept_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
 		let (value, output_script, user_id) = {
 			let mut channel_lock = self.channel_state.lock().unwrap();

lightning/src/util/events.rs

@@ -15,7 +15,7 @@
 //! few other things.
 
 use chain::keysinterface::SpendableOutputDescriptor;
-use ln::channelmanager::{PaymentId, OpenChannelRequestInfo};
+use ln::channelmanager::{PaymentId};
 use ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
 use ln::msgs;
 use ln::msgs::DecodeError;
@@ -29,7 +29,7 @@ use bitcoin::blockdata::script::Script;
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::secp256k1::key::PublicKey;
-
+use bitcoin::hash_types::{BlockHash};
 use io;
 use prelude::*;
 use core::time::Duration;
@@ -412,16 +412,21 @@ pub enum Event {
 	/// [`ChannelManager::respond_to_open_channel_request`]: crate::ln::channelmanager::ChannelManager::respond_to_open_channel_request
 	/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
 	OpenChannelRequest {
-		/// The OpenChannelRequestInfo holds the information required to handle an open channel request.
-		/// The struct includes the [`msgs::OpenChannel`] message, which holds the information that may be relevant to
-		/// the user in order to decide whether the request should be accepted or rejected.
+		/// The temporary channel ID of the channel requested to be opened.
 		///
-		/// When accepting or rejecting to the request, the entire stuct should be passed back to the ChannelManager
+		/// When accepting or rejecting to the request, the `temporary_channel_id` should be passed back to the ChannelManager
 		/// with [`ChannelManager::respond_to_open_channel_request`].
 		///
-		/// [`msgs::OpenChannel`]: crate::ln::msgs::OpenChannel
 		/// [`ChannelManager::respond_to_open_channel_request`]: crate::ln::channelmanager::ChannelManager::respond_to_open_channel_request
-		open_channel_request_info: OpenChannelRequestInfo,
+		temporary_channel_id: [u8; 32],
+		/// The node_id of the counterparty requesting to open the channel.
+		counterparty_node_id: PublicKey,
+		/// The genesis hash of the blockchain where the channel is requested to be opened.
+		chain_hash: BlockHash,
+		/// The channel value of the requested channel.
+		funding_satoshis: u64,
+		/// The amount to push to the counterparty if the channel request is accepted, in milli-satoshi.
+		push_msat: u64,
 	},
 }
 
@@ -535,11 +540,10 @@ impl Writeable for Event {
 					(2, payment_hash, required),
 				})
 			},
-			&Event::OpenChannelRequest { ref open_channel_request_info } => {
-				11u8.write(writer)?;
-				write_tlv_fields!(writer, {
-					(0, open_channel_request_info, required),
-				})
+			&Event::OpenChannelRequest { .. } => {
+				16u8.write(writer)?;
+				// We never write the OpenChannelRequest events as, upon disconnection, peers
+				// drop any channels which have not yet exchanged funding_signed.
 			},
 			// Note that, going forward, all new events must only write data inside of
 			// `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write