Merge pull request #1292 from TheBlueMatt/2022-02-override-handshake-limits
authorvalentinewallace <valentinewallace@users.noreply.github.com>
Sat, 12 Feb 2022 00:45:44 +0000 (19:45 -0500)
committerGitHub <noreply@github.com>
Sat, 12 Feb 2022 00:45:44 +0000 (19:45 -0500)
Store override counterparty handshake limits until we enforce them

1  2 
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs

index 7e6c00f1ad7692fa4834279383c608390d6dadec,26d9dfed0d162d5d7fb9853e68ae476d6083348b..e76656e75ce38e9ee9b65c513ebc8c916e7b4ac7
@@@ -39,7 -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 +484,8 @@@ pub(super) struct Channel<Signer: Sign
        #[cfg(not(any(test, feature = "_test_utils")))]
        config: ChannelConfig,
  
+       inbound_handshake_limits_override: Option<ChannelHandshakeLimits>,
        user_id: u64,
  
        channel_id: [u8; 32],
@@@ -835,6 -837,7 +837,7 @@@ impl<Signer: Sign> Channel<Signer> 
                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,
                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),
  
        // 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()));
                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)));
                }
                }
  
                // 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)));
                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
                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(())
        }
        /// 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<u64>) {
        /// 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<msgs::UnsignedChannelAnnouncement, ChannelError> {
                if !self.config.announced_channel {
@@@ -6006,6 -6009,11 +6013,11 @@@ impl<'a, Signer: Sign, K: Deref> Readab
                        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(),
@@@ -6274,7 -6282,7 +6286,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.
  
                // 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();
index 451d6d5f8efab76b6273f91927eee7c091c004e1,47ccb491cf11d799bb72416758d061e3296d40bb..9268afc66f7e97cf1a3f2829093f88d1f5b9be40
@@@ -4117,7 -4117,7 +4117,7 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
                                        }
-                                       try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, &their_features), channel_state, chan);
+                                       try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration.peer_channel_config_limits, &their_features), channel_state, chan);
                                        (chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id())
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))
@@@ -6901,7 -6901,7 +6901,7 @@@ mod tests 
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
 -              let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 +              let scorer = test_utils::TestScorer::with_penalty(0);
  
                // To start (1), send a regular payment but don't claim it.
                let expected_route = [&nodes[1]];
                };
                let network_graph = nodes[0].network_graph;
                let first_hops = nodes[0].node.list_usable_channels();
 -              let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 +              let scorer = test_utils::TestScorer::with_penalty(0);
                let route = find_route(
                        &payer_pubkey, &route_params, network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
                        nodes[0].logger, &scorer
                };
                let network_graph = nodes[0].network_graph;
                let first_hops = nodes[0].node.list_usable_channels();
 -              let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 +              let scorer = test_utils::TestScorer::with_penalty(0);
                let route = find_route(
                        &payer_pubkey, &route_params, network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
                        nodes[0].logger, &scorer
@@@ -7143,6 -7143,7 +7143,6 @@@ pub mod bench 
        use ln::msgs::{ChannelMessageHandler, Init};
        use routing::network_graph::NetworkGraph;
        use routing::router::{PaymentParameters, get_route};
 -      use routing::scoring::Scorer;
        use util::test_utils;
        use util::config::UserConfig;
        use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};
                                let usable_channels = $node_a.list_usable_channels();
                                let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id())
                                        .with_features(InvoiceFeatures::known());
 -                              let scorer = Scorer::with_fixed_penalty(0);
 +                              let scorer = test_utils::TestScorer::with_penalty(0);
                                let route = get_route(&$node_a.get_our_node_id(), &payment_params, &dummy_graph,
                                        Some(&usable_channels.iter().map(|r| r).collect::<Vec<_>>()), 10_000, TEST_FINAL_CLTV, &logger_a, &scorer).unwrap();