Merge pull request #986 from TheBlueMatt/2021-07-route-lasthop-value
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 26 Jul 2021 16:02:41 +0000 (16:02 +0000)
committerGitHub <noreply@github.com>
Mon, 26 Jul 2021 16:02:41 +0000 (16:02 +0000)
[router] Use the invoice value for last-hop hint channel capacity

1  2 
lightning/src/routing/router.rs

index 548febbd470d440dd39e4f2b6f049c3ce7c3a214,087e7ede215d302dea95560f0484b44f6f6ae0e2..dd5cadecc8322e88b56d369004cbde5a85e47e66
@@@ -459,10 -459,10 +459,10 @@@ pub fn get_route<L: Deref>(our_node_id
        if let Some(hops) = first_hops {
                for chan in hops {
                        let short_channel_id = chan.short_channel_id.expect("first_hops should be filled in with usable channels, not pending ones");
 -                      if chan.remote_network_id == *our_node_id {
 +                      if chan.counterparty.node_id == *our_node_id {
                                return Err(LightningError{err: "First hop cannot have our_node_id as a destination.".to_owned(), action: ErrorAction::IgnoreError});
                        }
 -                      first_hop_targets.insert(chan.remote_network_id, (short_channel_id, chan.counterparty_features.to_context(), chan.outbound_capacity_msat, chan.counterparty_features.to_context()));
 +                      first_hop_targets.insert(chan.counterparty.node_id, (short_channel_id, chan.counterparty.features.to_context(), chan.outbound_capacity_msat, chan.counterparty.features.to_context()));
                }
                if first_hop_targets.is_empty() {
                        return Err(LightningError{err: "Cannot route when there are no outbound routes away from us".to_owned(), action: ErrorAction::IgnoreError});
                                        htlc_maximum_msat: hop.htlc_maximum_msat,
                                        fees: hop.fees,
                                };
-                               if add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, &empty_channel_features, 0, path_value_msat, 0) {
+                               // We assume that the recipient only included route hints for routes which had
+                               // sufficient value to route `final_value_msat`. Note that in the case of "0-value"
+                               // invoices where the invoice does not specify value this may not be the case, but
+                               // better to include the hints than not.
+                               if add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, Some((final_value_msat + 999) / 1000), &empty_channel_features, 0, path_value_msat, 0) {
                                        // If this hop connects to a node with which we have a direct channel,
                                        // ignore the network graph and, if the last hop was added, add our
                                        // direct channel to the candidate set.
@@@ -1198,31 -1202,7 +1202,31 @@@ mod tests 
        use bitcoin::secp256k1::{Secp256k1, All};
  
        use prelude::*;
 -      use std::sync::Arc;
 +      use sync::{self, Arc};
 +
 +      fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
 +                      features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
 +              channelmanager::ChannelDetails {
 +                      channel_id: [0; 32],
 +                      counterparty: channelmanager::ChannelCounterparty {
 +                              features,
 +                              node_id,
 +                              unspendable_punishment_reserve: 0,
 +                              forwarding_info: None,
 +                      },
 +                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 +                      short_channel_id,
 +                      channel_value_satoshis: 0,
 +                      user_id: 0,
 +                      outbound_capacity_msat,
 +                      inbound_capacity_msat: 42,
 +                      unspendable_punishment_reserve: None,
 +                      confirmations_required: None,
 +                      force_close_spend_delay: None,
 +                      is_outbound: true, is_funding_locked: true,
 +                      is_usable: true, is_public: true,
 +              }
 +      }
  
        // Using the same keys for LN and BTC ids
        fn add_channel(net_graph_msg_handler: &NetGraphMsgHandler<Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>, secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey,
                }
        }
  
 -      fn build_graph() -> (Secp256k1<All>, NetGraphMsgHandler<std::sync::Arc<test_utils::TestChainSource>, std::sync::Arc<crate::util::test_utils::TestLogger>>, std::sync::Arc<test_utils::TestChainSource>, std::sync::Arc<test_utils::TestLogger>) {
 +      fn build_graph() -> (Secp256k1<All>, NetGraphMsgHandler<sync::Arc<test_utils::TestChainSource>, sync::Arc<crate::util::test_utils::TestLogger>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>) {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(test_utils::TestLogger::new());
                let chain_monitor = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
  
                // Simple route to 2 via 1
  
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(2),
 -                      remote_network_id: our_id,
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 100000,
 -                      user_id: 0,
 -                      outbound_capacity_msat: 100000,
 -                      inbound_capacity_msat: 100000,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(2), our_id, InitFeatures::from_le_bytes(vec![0b11]), 100000)];
  
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)) {
                        assert_eq!(err, "First hop cannot have our_node_id as a destination.");
                } else { panic!(); }
  
                // If we specify a channel to node7, that overrides our local channel view and that gets used
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(42),
 -                      remote_network_id: nodes[7].clone(),
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 0,
 -                      user_id: 0,
 -                      outbound_capacity_msat: 250_000_000,
 -                      inbound_capacity_msat: 0,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()),  &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
  
                // Disable nodes 1, 2, and 8 by requiring unknown feature bits
 -              let mut unknown_features = NodeFeatures::known();
 -              unknown_features.set_required_unknown_bits();
 +              let unknown_features = NodeFeatures::known().set_unknown_feature_required();
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[0], unknown_features.clone(), 1);
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[1], unknown_features.clone(), 1);
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[7], unknown_features.clone(), 1);
                } else { panic!(); }
  
                // If we specify a channel to node7, that overrides our local channel view and that gets used
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(42),
 -                      remote_network_id: nodes[7].clone(),
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 0,
 -                      user_id: 0,
 -                      outbound_capacity_msat: 250_000_000,
 -                      inbound_capacity_msat: 0,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                assert_eq!(route.paths[0][2].channel_features.le_flags(), &id_to_feature_flags(3));
  
                // If we specify a channel to node7, that overrides our local channel view and that gets used
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(42),
 -                      remote_network_id: nodes[7].clone(),
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 0,
 -                      user_id: 0,
 -                      outbound_capacity_msat: 250_000_000,
 -                      inbound_capacity_msat: 0,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
  
                // Simple test with outbound channel to 4 to test that last_hops and first_hops connect
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(42),
 -                      remote_network_id: nodes[3].clone(),
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 0,
 -                      user_id: 0,
 -                      outbound_capacity_msat: 250_000_000,
 -                      inbound_capacity_msat: 0,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(42), nodes[3].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let mut last_hops = last_hops(&nodes);
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, Some(&our_chans.iter().collect::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: last_hop_htlc_max,
                }]);
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(42),
 -                      remote_network_id: middle_node_id,
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 100000,
 -                      user_id: 0,
 -                      outbound_capacity_msat: outbound_capacity_msat,
 -                      inbound_capacity_msat: 100000,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)];
                get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, None, Some(&our_chans.iter().collect::<Vec<_>>()), &vec![&last_hops], route_val, 42, Arc::new(test_utils::TestLogger::new()))
        }
  
                });
  
                // Now, limit the first_hop by the outbound_capacity_msat of 200_000 sats.
 -              let our_chans = vec![channelmanager::ChannelDetails {
 -                      channel_id: [0; 32],
 -                      funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 -                      short_channel_id: Some(42),
 -                      remote_network_id: nodes[0].clone(),
 -                      counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
 -                      channel_value_satoshis: 0,
 -                      user_id: 0,
 -                      outbound_capacity_msat: 200_000_000,
 -                      inbound_capacity_msat: 0,
 -                      is_outbound: true, is_funding_locked: true,
 -                      is_usable: true, is_public: true,
 -                      counterparty_forwarding_info: None,
 -              }];
 +              let our_chans = vec![get_channel_details(Some(42), nodes[0].clone(), InitFeatures::from_le_bytes(vec![0b11]), 200_000_000)];
  
                {
                        // Attempt to route more than available results in a failure.