Merge pull request #651 from naumenkogs/2020-06-routing-data-improvements
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 27 Jul 2020 17:18:13 +0000 (10:18 -0700)
committerGitHub <noreply@github.com>
Mon, 27 Jul 2020 17:18:13 +0000 (10:18 -0700)
Routing improvements

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

index fc3b4fd81746f7dc878a0bf138735ee805026c01,7a4c76e282b06cdc201cb82b24db970fa55efbf5..c416c676d78c514723c19e93a77446233ad5a810
@@@ -785,6 -785,13 +785,6 @@@ impl<ChanSigner: ChannelKeys> Channel<C
                Ok(chan)
        }
  
 -      // Utilities to derive keys:
 -
 -      fn build_local_commitment_secret(&self, idx: u64) -> SecretKey {
 -              let res = self.local_keys.commitment_secret(idx);
 -              SecretKey::from_slice(&res).unwrap()
 -      }
 -
        // Utilities to build transactions:
  
        fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
        /// The result is a transaction which we can revoke ownership of (ie a "local" transaction)
        /// TODO Some magic rust shit to compile-time check this?
        fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 -              let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 +              let per_commitment_point = self.local_keys.get_per_commitment_point(commitment_number, &self.secp_ctx);
                let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
                let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
                let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
                        }
                }
  
 -              let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1));
 -              let per_commitment_secret = self.local_keys.commitment_secret(self.cur_local_commitment_transaction_number + 1);
 +              let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number - 1, &self.secp_ctx);
 +              let per_commitment_secret = self.local_keys.release_commitment_secret(self.cur_local_commitment_transaction_number + 1);
  
                // Update state now that we've passed all the can-fail calls...
                let mut need_our_commitment = false;
                let funding_locked = if self.monitor_pending_funding_locked {
                        assert!(!self.channel_outbound, "Funding transaction broadcast without FundingBroadcastSafe!");
                        self.monitor_pending_funding_locked = false;
 -                      let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 -                      let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
 +                      let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
                                channel_id: self.channel_id(),
                                next_per_commitment_point: next_per_commitment_point,
        }
  
        fn get_last_revoke_and_ack(&self) -> msgs::RevokeAndACK {
 -              let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number));
 -              let per_commitment_secret = self.local_keys.commitment_secret(self.cur_local_commitment_transaction_number + 2);
 +              let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
 +              let per_commitment_secret = self.local_keys.release_commitment_secret(self.cur_local_commitment_transaction_number + 2);
                msgs::RevokeAndACK {
                        channel_id: self.channel_id,
                        per_commitment_secret,
                if msg.next_remote_commitment_number > 0 {
                        match msg.data_loss_protect {
                                OptionalField::Present(ref data_loss) => {
 -                                      if self.local_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1) != data_loss.your_last_per_commitment_secret {
 +                                      let expected_point = self.local_keys.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.secp_ctx);
 +                                      let given_secret = SecretKey::from_slice(&data_loss.your_last_per_commitment_secret)
 +                                              .map_err(|_| ChannelError::Close("Peer sent a garbage channel_reestablish with unparseable secret key".to_owned()))?;
 +                                      if expected_point != PublicKey::from_secret_key(&self.secp_ctx, &given_secret) {
                                                return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided".to_owned()));
                                        }
                                        if msg.next_remote_commitment_number > INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number {
                        }
  
                        // We have OurFundingLocked set!
 -                      let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 -                      let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
 +                      let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                        return Ok((Some(msgs::FundingLocked {
                                channel_id: self.channel_id(),
                                next_per_commitment_point: next_per_commitment_point,
  
                let resend_funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number == 1 {
                        // We should never have to worry about MonitorUpdateFailed resending FundingLocked
 -                      let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 -                      let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
 +                      let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
                                channel_id: self.channel_id(),
                                next_per_commitment_point: next_per_commitment_point,
                self.our_htlc_minimum_msat
        }
  
+       /// Allowed in any state (including after shutdown)
+       pub fn get_announced_htlc_max_msat(&self) -> u64 {
+               return cmp::min(
+                       // Upper bound by capacity. We make it a bit less than full capacity to prevent attempts
+                       // to use full capacity. This is an effort to reduce routing failures, because in many cases
+                       // channel might have been used to route very small values (either by honest users or as DoS).
+                       self.channel_value_satoshis * 9 / 10,
+                       Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis)
+               );
+       }
        /// Allowed in any state (including after shutdown)
        pub fn get_their_htlc_minimum_msat(&self) -> u64 {
                self.our_htlc_minimum_msat
                                        //a protocol oversight, but I assume I'm just missing something.
                                        if need_commitment_update {
                                                if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
 -                                                      let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 -                                                      let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
 +                                                      let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                                                        return Ok((Some(msgs::FundingLocked {
                                                                channel_id: self.channel_id,
                                                                next_per_commitment_point: next_per_commitment_point,
                        panic!("Tried to send an open_channel for a channel that has already advanced");
                }
  
 -              let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 +              let first_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                let local_keys = self.local_keys.pubkeys();
  
                msgs::OpenChannel {
                        payment_point: local_keys.payment_point,
                        delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
                        htlc_basepoint: local_keys.htlc_basepoint,
 -                      first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 +                      first_per_commitment_point,
                        channel_flags: if self.config.announced_channel {1} else {0},
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
                }
                        panic!("Tried to send an accept_channel for a channel that has already advanced");
                }
  
 -              let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 +              let first_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                let local_keys = self.local_keys.pubkeys();
  
                msgs::AcceptChannel {
                        payment_point: local_keys.payment_point,
                        delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
                        htlc_basepoint: local_keys.htlc_basepoint,
 -                      first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 +                      first_per_commitment_point,
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
                }
        }
index ce9ec1d5853adc9851fc4475de83b7f312570877,9526189e7791947c073743fa876f52c8a23901d9..35371c56ffc836c866655fe792b75e448bd666f5
@@@ -15,7 -15,7 +15,7 @@@ use ln::{chan_utils, onion_utils}
  use routing::router::{Route, RouteHop, get_route};
  use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
  use ln::msgs;
- use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction};
+ use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction, OptionalField};
  use util::enforcing_trait_impls::EnforcingChannelKeys;
  use util::{byte_utils, test_utils};
  use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
@@@ -1612,7 -1612,7 +1612,7 @@@ fn test_fee_spike_violation_fails_htlc(
                let chan_keys = local_chan.get_local_keys();
                let pubkeys = chan_keys.pubkeys();
                (pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
 -               chan_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
 +               chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
        };
        let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_secret1) = {
                let chan_lock = nodes[1].node.channel_state.lock().unwrap();
                let chan_keys = remote_chan.get_local_keys();
                let pubkeys = chan_keys.pubkeys();
                (pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
 -               chan_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
 +               chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
        };
  
        // Assemble the set of keys we can use for signatures for our commitment_signed message.
@@@ -6058,6 -6058,7 +6058,7 @@@ impl msgs::ChannelUpdate 
                                flags: 0,
                                cltv_expiry_delta: 0,
                                htlc_minimum_msat: 0,
+                               htlc_maximum_msat: OptionalField::Absent,
                                fee_base_msat: 0,
                                fee_proportional_millionths: 0,
                                excess_data: vec![],
@@@ -8132,8 -8133,8 +8133,8 @@@ fn test_counterparty_raa_skip_no_crash(
        let local_keys = &guard.by_id.get_mut(&channel_id).unwrap().local_keys;
        const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
        let next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
 -              &SecretKey::from_slice(&local_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
 -      let per_commitment_secret = local_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER);
 +              &SecretKey::from_slice(&local_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
 +      let per_commitment_secret = local_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER);
  
        nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
                &msgs::RevokeAndACK { channel_id, per_commitment_secret, next_per_commitment_point });