X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=26d9dfed0d162d5d7fb9853e68ae476d6083348b;hb=649af07205748358c6996a848e7e399e6be31d88;hp=e20bfc87cbcae087ad0168d5d18a0fbdcd5bc0bf;hpb=a265fc206260fb6909352d38c2eeef16a7f72432;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e20bfc87..26d9dfed 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], @@ -835,6 +837,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, @@ -942,14 +945,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 +954,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 +1137,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), @@ -1811,7 +1815,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 +1838,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 +1850,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 +1868,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 +1922,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(()) } @@ -2142,7 +2149,7 @@ impl Channel { log_info!(logger, "Received funding_locked from peer for channel {}", log_bytes!(self.channel_id())); - Ok(self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height()).ok()) + Ok(self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height(), logger)) } /// Returns transaction if there is pending funding transaction that is yet to broadcast @@ -3416,7 +3423,7 @@ impl Channel { }) } else { None }; - let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block_height).ok(); + let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block_height, logger); let mut accepted_htlcs = Vec::new(); mem::swap(&mut accepted_htlcs, &mut self.monitor_pending_forwards); @@ -3609,7 +3616,7 @@ impl Channel { }) } else { None }; - let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height()).ok(); + let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height(), logger); if self.channel_state & (ChannelState::FundingSent as u32) == ChannelState::FundingSent as u32 { // If we're waiting on a monitor update, we shouldn't re-send any funding_locked's. @@ -4419,8 +4426,8 @@ impl Channel { txdata: &TransactionData, genesis_block_hash: BlockHash, node_pk: PublicKey, logger: &L) -> Result<(Option, Option), ClosureReason> where L::Target: Logger { let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS); - for &(index_in_block, tx) in txdata.iter() { - if let Some(funding_txo) = self.get_funding_txo() { + if let Some(funding_txo) = self.get_funding_txo() { + for &(index_in_block, tx) in txdata.iter() { // If we haven't yet sent a funding_locked, but are in FundingSent (ignoring // whether they've sent a funding_locked or not), check if we should send one. if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 { @@ -4464,7 +4471,7 @@ impl Channel { // may have already happened for this block). if let Some(funding_locked) = self.check_get_funding_locked(height) { log_info!(logger, "Sending a funding_locked to our peer for channel {}", log_bytes!(self.channel_id)); - let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok(); + let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger); return Ok((Some(funding_locked), announcement_sigs)); } } @@ -4518,7 +4525,7 @@ impl Channel { if let Some(funding_locked) = self.check_get_funding_locked(height) { let announcement_sigs = if let Some((genesis_block_hash, node_pk)) = genesis_node_pk { - self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok() + self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger) } else { None }; log_info!(logger, "Sending a funding_locked to our peer for channel {}", log_bytes!(self.channel_id)); return Ok((Some(funding_locked), timed_out_htlcs, announcement_sigs)); @@ -4554,7 +4561,7 @@ impl Channel { } let announcement_sigs = if let Some((genesis_block_hash, node_pk)) = genesis_node_pk { - self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok() + self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger) } else { None }; Ok((None, timed_out_htlcs, announcement_sigs)) } @@ -4725,8 +4732,8 @@ impl Channel { /// Gets an UnsignedChannelAnnouncement for this channel. The channel must be publicly /// announceable and available for use (have exchanged FundingLocked messages in both - /// directions). Should be used for both loose and in response to an AnnouncementSignatures - /// message from the remote peer. + /// directions). Should be used for both broadcasted announcements and in response to an + /// AnnouncementSignatures message from the remote peer. /// /// Will only fail if we're not in a state where channel_announcement may be sent (including /// closing). @@ -4759,29 +4766,43 @@ impl Channel { Ok(msg) } - fn get_announcement_sigs(&mut self, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32) -> Result { + fn get_announcement_sigs(&mut self, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32, logger: &L) + -> Option where L::Target: Logger { if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height { - return Err(ChannelError::Ignore("Funding not yet fully confirmed".to_owned())); + return None; } if !self.is_usable() { - return Err(ChannelError::Ignore("Channel not yet available for use".to_owned())); + return None; } if self.channel_state & ChannelState::PeerDisconnected as u32 != 0 { - return Err(ChannelError::Ignore("Peer currently disconnected".to_owned())); + log_trace!(logger, "Cannot create an announcement_signatures as our peer is disconnected"); + return None; } if self.announcement_sigs_state != AnnouncementSigsState::NotSent { - return Err(ChannelError::Ignore("Announcement signatures already sent".to_owned())); + return None; } - let announcement = self.get_channel_announcement(node_pk, genesis_block_hash)?; - let (our_node_sig, our_bitcoin_sig) = self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx) - .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?; + log_trace!(logger, "Creating an announcement_signatures message for channel {}", log_bytes!(self.channel_id())); + let announcement = match self.get_channel_announcement(node_pk, genesis_block_hash) { + Ok(a) => a, + Err(_) => { + log_trace!(logger, "Cannot create an announcement_signatures as channel is not public."); + return None; + } + }; + let (our_node_sig, our_bitcoin_sig) = match self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx) { + Err(_) => { + log_error!(logger, "Signer rejected channel_announcement signing. Channel will not be announced!"); + return None; + }, + Ok(v) => v + }; self.announcement_sigs_state = AnnouncementSigsState::MessageSent; - Ok(msgs::AnnouncementSignatures { + Some(msgs::AnnouncementSignatures { channel_id: self.channel_id(), short_channel_id: self.get_short_channel_id().unwrap(), node_signature: our_node_sig, @@ -5988,6 +6009,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(), @@ -6140,6 +6166,13 @@ 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, } @@ -6249,7 +6282,7 @@ mod tests { // Node B --> Node A: accept channel, explicitly setting B's dust limit. let mut accept_channel_msg = node_b_chan.get_accept_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. @@ -6366,7 +6399,7 @@ mod tests { // 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(); + 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();