X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=fb9213142ae740e4b487786be3e50c7a9268807b;hb=710954f88bc39d58713e982caaad05f4c61d92ed;hp=72c60e8ed616f0518aca326ec4230d549eeb5a8f;hpb=ed1163a5bfa3ce02daaee505df33fef119556b25;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 72c60e8e..fb921314 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -39,7 +39,7 @@ use util::events::ClosureReason; use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use util::logger::Logger; use util::errors::APIError; -use util::config::{UserConfig,ChannelConfig}; +use util::config::{UserConfig, ChannelConfig, ChannelHandshakeLimits}; use util::scid_utils::scid_from_parts; use io; @@ -484,6 +484,8 @@ pub(super) struct Channel { #[cfg(not(any(test, feature = "_test_utils")))] config: ChannelConfig, + inbound_handshake_limits_override: Option, + user_id: u64, channel_id: [u8; 32], @@ -582,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, @@ -835,6 +850,7 @@ impl Channel { Ok(Channel { user_id, config: config.channel_options.clone(), + inbound_handshake_limits_override: Some(config.peer_channel_config_limits.clone()), channel_id: keys_provider.get_secure_random_bytes(), channel_state: ChannelState::OurInitSent as u32, @@ -880,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, @@ -942,14 +960,6 @@ impl Channel { fn check_remote_fee(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError> where F::Target: FeeEstimator { - let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing - // occasional issues with feerate disagreements between an initiator that wants a feerate - // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250 - // sat/kw before the comparison here. - if feerate_per_kw + 250 < lower_limit { - return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit))); - } // We only bound the fee updates on the upper side to prevent completely absurd feerates, // always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee. // We generally don't care too much if they set the feerate to something very high, but it @@ -959,6 +969,14 @@ impl Channel { if feerate_per_kw as u64 > upper_limit { return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit))); } + let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); + // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing + // occasional issues with feerate disagreements between an initiator that wants a feerate + // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250 + // sat/kw before the comparison here. + if feerate_per_kw + 250 < lower_limit { + return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit))); + } Ok(()) } @@ -1134,6 +1152,7 @@ impl Channel { let chan = Channel { user_id, config: local_config, + inbound_handshake_limits_override: None, channel_id: msg.temporary_channel_id, channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32), @@ -1178,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, @@ -1811,7 +1832,9 @@ impl Channel { // Message handlers: - pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: &InitFeatures) -> Result<(), ChannelError> { + pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, default_limits: &ChannelHandshakeLimits, their_features: &InitFeatures) -> Result<(), ChannelError> { + let peer_limits = if let Some(ref limits) = self.inbound_handshake_limits_override { limits } else { default_limits }; + // Check sanity of message fields: if !self.is_outbound() { return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned())); @@ -1832,7 +1855,7 @@ impl Channel { if msg.htlc_minimum_msat >= full_channel_value_msat { return Err(ChannelError::Close(format!("Minimum htlc value ({}) is full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat))); } - let max_delay_acceptable = u16::min(config.peer_channel_config_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT); + let max_delay_acceptable = u16::min(peer_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT); if msg.to_self_delay > max_delay_acceptable { return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_delay_acceptable, msg.to_self_delay))); } @@ -1844,17 +1867,17 @@ impl Channel { } // Now check against optional parameters as set by config... - if msg.htlc_minimum_msat > config.peer_channel_config_limits.max_htlc_minimum_msat { - return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat, config.peer_channel_config_limits.max_htlc_minimum_msat))); + if msg.htlc_minimum_msat > peer_limits.max_htlc_minimum_msat { + return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat, peer_limits.max_htlc_minimum_msat))); } - if msg.max_htlc_value_in_flight_msat < config.peer_channel_config_limits.min_max_htlc_value_in_flight_msat { - return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.max_htlc_value_in_flight_msat, config.peer_channel_config_limits.min_max_htlc_value_in_flight_msat))); + if msg.max_htlc_value_in_flight_msat < peer_limits.min_max_htlc_value_in_flight_msat { + return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.max_htlc_value_in_flight_msat, peer_limits.min_max_htlc_value_in_flight_msat))); } - if msg.channel_reserve_satoshis > config.peer_channel_config_limits.max_channel_reserve_satoshis { - return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, config.peer_channel_config_limits.max_channel_reserve_satoshis))); + if msg.channel_reserve_satoshis > peer_limits.max_channel_reserve_satoshis { + return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, peer_limits.max_channel_reserve_satoshis))); } - if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs { - return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs))); + if msg.max_accepted_htlcs < peer_limits.min_max_accepted_htlcs { + return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, peer_limits.min_max_accepted_htlcs))); } if msg.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); @@ -1862,8 +1885,8 @@ impl Channel { if msg.dust_limit_satoshis > MAX_CHAN_DUST_LIMIT_SATOSHIS { return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS))); } - if msg.minimum_depth > config.peer_channel_config_limits.max_minimum_depth { - return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth))); + if msg.minimum_depth > peer_limits.max_minimum_depth { + return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", peer_limits.max_minimum_depth, msg.minimum_depth))); } if msg.minimum_depth == 0 { // Note that if this changes we should update the serialization minimum version to @@ -1916,6 +1939,7 @@ impl Channel { self.counterparty_shutdown_scriptpubkey = counterparty_shutdown_scriptpubkey; self.channel_state = ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32; + self.inbound_handshake_limits_override = None; // We're done enforcing limits on our peer's handshake now. Ok(()) } @@ -1966,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 { @@ -2245,8 +2272,15 @@ impl Channel { /// This is the amount that would go to us if we close the channel, ignoring any on-chain fees. /// See also [`Channel::get_inbound_outbound_available_balance_msat`] pub fn get_balance_msat(&self) -> u64 { - self.value_to_self_msat - - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat + // Include our local balance, plus any inbound HTLCs we know the preimage for, minus any + // HTLCs sent or which will be sent after commitment signed's are exchanged. + let mut balance_msat = self.value_to_self_msat; + for ref htlc in self.pending_inbound_htlcs.iter() { + if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) = htlc.state { + balance_msat += htlc.amount_msat; + } + } + balance_msat - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat } pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option) { @@ -4631,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?"); } @@ -4641,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(); @@ -4667,6 +4723,15 @@ impl Channel { } } + /// Enables the possibility for tests to extract a [`msgs::AcceptChannel`] message for an + /// inbound channel without accepting it. + /// + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + #[cfg(test)] + pub fn get_accept_channel_message(&self) -> msgs::AcceptChannel { + self.generate_accept_channel_message() + } + /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created) fn get_outbound_funding_created_signature(&mut self, logger: &L) -> Result where L::Target: Logger { let counterparty_keys = self.build_remote_transaction_keys()?; @@ -4731,9 +4796,6 @@ impl Channel { /// Will only fail if we're not in a state where channel_announcement may be sent (including /// closing). /// - /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see - /// https://github.com/lightningnetwork/lightning-rfc/issues/468 - /// /// This will only return ChannelError::Ignore upon failure. fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result { if !self.config.announced_channel { @@ -6002,6 +6064,11 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel user_id, config: config.unwrap(), + + // Note that we don't care about serializing handshake limits as we only ever serialize + // channel data after the handshake has completed. + inbound_handshake_limits_override: None, + channel_id, channel_state, announcement_sigs_state: announcement_sigs_state.unwrap(), @@ -6048,6 +6115,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, @@ -6119,7 +6188,7 @@ mod tests { use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, htlc_success_tx_weight, htlc_timeout_tx_weight}; use chain::BestBlock; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; - use chain::keysinterface::{InMemorySigner, KeyMaterial, KeysInterface, BaseSign}; + use chain::keysinterface::{InMemorySigner, Recipient, KeyMaterial, KeysInterface, BaseSign}; use chain::transaction::OutPoint; use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; @@ -6154,13 +6223,20 @@ mod tests { "MAX_FUNDING_SATOSHIS is greater than all satoshis in existence"); } + #[test] + fn test_no_fee_check_overflow() { + // Previously, calling `check_remote_fee` with a fee of 0xffffffff would overflow in + // arithmetic, causing a panic with debug assertions enabled. + assert!(Channel::::check_remote_fee(&&TestFeeEstimator { fee_est: 42 }, u32::max_value()).is_err()); + } + struct Keys { signer: InMemorySigner, } impl KeysInterface for Keys { type Signer = InMemorySigner; - fn get_node_secret(&self) -> SecretKey { panic!(); } + fn get_node_secret(&self, _recipient: Recipient) -> Result { panic!(); } fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); } fn get_destination_script(&self) -> Script { let secp_ctx = Secp256k1::signing_only(); @@ -6180,7 +6256,7 @@ mod tests { } fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] } fn read_chan_signer(&self, _data: &[u8]) -> Result { panic!(); } - fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result { panic!(); } + fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient) -> Result { panic!(); } } fn public_from_secret_hex(secp_ctx: &Secp256k1, hex: &str) -> PublicKey { @@ -6258,12 +6334,12 @@ 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, &InitFeatures::known()).unwrap(); + node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap(); node_a_chan.holder_dust_limit_satoshis = 1560; // Put some inbound and outbound HTLCs in A's channel. @@ -6379,8 +6455,8 @@ 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(); - node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap(); + 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 let output_script = node_a_chan.get_funding_redeemscript();