From 8dca0b47795db0eeee968d7a03ee0b1595484a52 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Viktor=20Tigerstr=C3=B6m?= Date: Wed, 26 Jan 2022 00:21:22 +0100 Subject: [PATCH] Add option to accept or reject inbound channels Add a new config flag `UserConfig::manually_accept_inbound_channels`, which when set to true allows the node operator to accept or reject new channel requests. When set to true, `Event::OpenChannelRequest` will be triggered once a request to open a new inbound channel is received. When accepting the request, `ChannelManager::accept_inbound_channel` should be called. Rejecting the request is done through `ChannelManager::force_close_channel`. --- lightning/src/ln/channel.rs | 52 +++++++++++++++++++++++++++--- lightning/src/ln/channelmanager.rs | 51 ++++++++++++++++++++++++++--- lightning/src/util/config.rs | 15 +++++++++ lightning/src/util/events.rs | 38 +++++++++++++++++++++- 4 files changed, 146 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e76656e75..0ba92af44 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -584,6 +584,19 @@ pub(super) struct Channel { #[cfg(not(test))] closing_fee_limits: Option<(u64, u64)>, + /// Flag that ensures that `accept_inbound_channel` must be called before `funding_created` + /// is executed successfully. The reason for this flag is that when the + /// `UserConfig::manually_accept_inbound_channels` config flag is set to true, inbound channels + /// are required to be manually accepted by the node operator before the `msgs::AcceptChannel` + /// message is created and sent out. During the manual accept process, `accept_inbound_channel` + /// is called by `ChannelManager::accept_inbound_channel`. + /// + /// The flag counteracts that a counterparty node could theoretically send a + /// `msgs::FundingCreated` message before the node operator has manually accepted an inbound + /// channel request made by the counterparty node. That would execute `funding_created` before + /// `accept_inbound_channel`, and `funding_created` should therefore not execute successfully. + inbound_awaiting_accept: bool, + /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, @@ -883,6 +896,8 @@ impl Channel { 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, @@ -1182,6 +1197,8 @@ impl Channel { 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, @@ -1973,6 +1990,9 @@ impl Channel { // channel. return Err(ChannelError::Close("Received funding_created after we got the channel!".to_owned())); } + if self.inbound_awaiting_accept { + 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 { @@ -4645,7 +4665,15 @@ impl Channel { } } - pub fn get_accept_channel(&self) -> msgs::AcceptChannel { + pub fn inbound_is_awaiting_accept(&self) -> bool { + self.inbound_awaiting_accept + } + + /// Marks an inbound channel as accepted and generates a [`msgs::AcceptChannel`] message which + /// should be sent back to the counterparty node. + /// + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + pub fn accept_inbound_channel(&mut self) -> msgs::AcceptChannel { if self.is_outbound() { panic!("Tried to send accept_channel for an outbound channel?"); } @@ -4655,7 +4683,21 @@ impl Channel { if self.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { panic!("Tried to send an accept_channel for a channel that has already advanced"); } + if !self.inbound_awaiting_accept { + panic!("The inbound channel has already been accepted"); + } + + self.inbound_awaiting_accept = false; + + self.generate_accept_channel_message() + } + /// This function is used to explicitly generate a [`msgs::AcceptChannel`] message for an + /// inbound channel. If the intention is to accept an inbound channel, use + /// [`Channel::accept_inbound_channel`] instead. + /// + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + fn generate_accept_channel_message(&self) -> msgs::AcceptChannel { 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(); @@ -6064,6 +6106,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel 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, @@ -6281,10 +6325,10 @@ 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::::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::::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(); + let mut accept_channel_msg = node_b_chan.accept_inbound_channel(); accept_channel_msg.dust_limit_satoshis = 546; node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap(); node_a_chan.holder_dust_limit_satoshis = 1560; @@ -6402,7 +6446,7 @@ mod tests { let mut node_b_chan = Channel::::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 - let accept_channel_msg = node_b_chan.get_accept_channel(); + let accept_channel_msg = node_b_chan.accept_inbound_channel(); node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap(); // Node A --> Node B: funding created diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9268afc66..0ae0f8c3e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4081,6 +4081,34 @@ impl ChannelMana } } + /// Called to accept a request to open a channel after [`Event::OpenChannelRequest`] has been + /// triggered. + /// + /// The `temporary_channel_id` parameter indicates which inbound channel should be accepted. + /// + /// [`Event::OpenChannelRequest`]: crate::util::events::Event::OpenChannelRequest + pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32]) -> Result<(), APIError> { + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); + + 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.clone()) { + 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: channel.get().get_counterparty_node_id(), + msg: channel.get_mut().accept_inbound_channel(), + }); + } + hash_map::Entry::Vacant(_) => { + return Err(APIError::ChannelUnavailable { err: "Can't accept a channel that doesn't exist".to_owned() }); + } + } + Ok(()) + } + fn internal_open_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> { if msg.chain_hash != self.genesis_hash { return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone())); @@ -4090,7 +4118,7 @@ impl ChannelMana return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone())); } - 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(); @@ -4098,10 +4126,23 @@ impl ChannelMana 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) => { - channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel { - node_id: counterparty_node_id.clone(), - msg: channel.get_accept_channel(), - }); + 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.accept_inbound_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(), + funding_satoshis: msg.funding_satoshis, + push_msat: msg.push_msat, + } + ); + } + entry.insert(channel); } } diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index c591cacb3..55d506e79 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -305,6 +305,20 @@ pub struct UserConfig { /// If this is set to false, we do not accept inbound requests to open a new channel. /// Default value: true. pub accept_inbound_channels: bool, + /// If this is set to true, the user needs to manually accept inbound requests to open a new + /// channel. + /// + /// When set to true, [`Event::OpenChannelRequest`] will be triggered once a request to open a + /// new inbound channel is received through a [`msgs::OpenChannel`] message. In that case, a + /// [`msgs::AcceptChannel`] message will not be sent back to the counterparty node unless the + /// user explicitly chooses to accept the request. + /// + /// Default value: false. + /// + /// [`Event::OpenChannelRequest`]: crate::util::events::Event::OpenChannelRequest + /// [`msgs::OpenChannel`]: crate::ln::msgs::OpenChannel + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + pub manually_accept_inbound_channels: bool, } impl Default for UserConfig { @@ -315,6 +329,7 @@ impl Default for UserConfig { channel_options: ChannelConfig::default(), accept_forwards_to_priv_channels: false, accept_inbound_channels: true, + manually_accept_inbound_channels: false, } } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index a4a733c8e..8c9f5af4c 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -29,7 +29,6 @@ use bitcoin::blockdata::script::Script; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::secp256k1::key::PublicKey; - use io; use prelude::*; use core::time::Duration; @@ -403,6 +402,34 @@ pub enum Event { /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain. path: Vec, }, + /// Indicates a request to open a new channel by a peer. + /// + /// To accept the request, call [`ChannelManager::accept_inbound_channel`]. To reject the + /// request, call [`ChannelManager::force_close_channel`]. + /// + /// The event is only triggered when a new open channel request is received and the + /// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. + /// + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`ChannelManager::force_close_channel`]: crate::ln::channelmanager::ChannelManager::force_close_channel + /// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels + OpenChannelRequest { + /// The temporary channel ID of the channel requested to be opened. + /// + /// When responding to the request, the `temporary_channel_id` should be passed + /// back to the ChannelManager with [`ChannelManager::accept_inbound_channel`] to accept, + /// or to [`ChannelManager::force_close_channel`] to reject. + /// + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`ChannelManager::force_close_channel`]: crate::ln::channelmanager::ChannelManager::force_close_channel + temporary_channel_id: [u8; 32], + /// The node_id of the counterparty requesting to open the channel. + counterparty_node_id: PublicKey, + /// The channel value of the requested channel. + funding_satoshis: u64, + /// Our starting balance in the channel if the request is accepted, in milli-satoshi. + push_msat: u64, + }, } impl Writeable for Event { @@ -515,6 +542,11 @@ impl Writeable for Event { (2, payment_hash, required), }) }, + &Event::OpenChannelRequest { .. } => { + 17u8.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 // data via `write_tlv_fields`. @@ -707,6 +739,10 @@ impl MaybeReadable for Event { }; f() }, + 17u8 => { + // Value 17 is used for `Event::OpenChannelRequest`. + Ok(None) + }, // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue. // Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt // reads. -- 2.39.5