Merge pull request #1405 from TheBlueMatt/2022-04-log-scoring
authorvalentinewallace <valentinewallace@users.noreply.github.com>
Wed, 27 Apr 2022 16:34:08 +0000 (12:34 -0400)
committerGitHub <noreply@github.com>
Wed, 27 Apr 2022 16:34:08 +0000 (12:34 -0400)
Log as channel liquidities are/not updated in ProbabilisticScorer

1  2 
lightning/src/routing/router.rs

index 212ac2f10cae7d62695f032c6fd8ab0a4e523ffa,3946f0ba7da02f61ddf6b685ca2d4fc6171a5249..913c1a20a4653ebd0f9c58454973e2cfeeb35525
@@@ -602,17 -602,6 +602,17 @@@ fn compute_fees(amount_msat: u64, chann
        }
  }
  
 +/// The default `features` we assume for a node in a route, when no `features` are known about that
 +/// specific node.
 +///
 +/// Default features are:
 +/// * variable_length_onion_optional
 +fn default_node_features() -> NodeFeatures {
 +      let mut features = NodeFeatures::empty();
 +      features.set_variable_length_onion_optional();
 +      features
 +}
 +
  /// Finds a route from us (payer) to the given target node (payee).
  ///
  /// If the payee provided features in their invoice, they should be provided via `params.payee`.
@@@ -1112,8 -1101,7 +1112,8 @@@ where L::Target: Logger 
                } }
        }
  
 -      let empty_node_features = NodeFeatures::empty();
 +      let default_node_features = default_node_features();
 +
        // Find ways (channels with destination) to reach a given node and store them
        // in the corresponding data structures (routing graph etc).
        // $fee_to_target_msat represents how much it costs to reach to this node from the payee,
                                let features = if let Some(node_info) = $node.announcement_info.as_ref() {
                                        &node_info.features
                                } else {
 -                                      &empty_node_features
 +                                      &default_node_features
                                };
  
                                if !features.requires_unknown_bits() {
                        // traversing the graph and arrange the path out of what we found.
                        if node_id == our_node_id {
                                let mut new_entry = dist.remove(&our_node_id).unwrap();
 -                              let mut ordered_hops = vec!((new_entry.clone(), NodeFeatures::empty()));
 +                              let mut ordered_hops = vec!((new_entry.clone(), default_node_features.clone()));
  
                                'path_walk: loop {
                                        let mut features_set = false;
                                                        if let Some(node_info) = node.announcement_info.as_ref() {
                                                                ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
                                                        } else {
 -                                                              ordered_hops.last_mut().unwrap().1 = NodeFeatures::empty();
 +                                                              ordered_hops.last_mut().unwrap().1 = default_node_features.clone();
                                                        }
                                                } else {
                                                        // We can fill in features for everything except hops which were
                                        // so that fees paid for a HTLC forwarding on the current channel are
                                        // associated with the previous channel (where they will be subtracted).
                                        ordered_hops.last_mut().unwrap().0.fee_msat = new_entry.hop_use_fee_msat;
 -                                      ordered_hops.push((new_entry.clone(), NodeFeatures::empty()));
 +                                      ordered_hops.push((new_entry.clone(), default_node_features.clone()));
                                }
                                ordered_hops.last_mut().unwrap().0.fee_msat = value_contribution_msat;
                                ordered_hops.last_mut().unwrap().0.hop_use_fee_msat = 0;
@@@ -1696,7 -1684,7 +1696,7 @@@ fn add_random_cltv_offset(route: &mut R
  #[cfg(test)]
  mod tests {
        use routing::network_graph::{NetworkGraph, NetGraphMsgHandler, NodeId};
 -      use routing::router::{get_route, add_random_cltv_offset, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA};
 +      use routing::router::{get_route, add_random_cltv_offset, default_node_features, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA};
        use routing::scoring::Score;
        use chain::transaction::OutPoint;
        use chain::keysinterface::KeysInterface;
                                node_id,
                                unspendable_punishment_reserve: 0,
                                forwarding_info: None,
 +                              outbound_htlc_minimum_msat: None,
 +                              outbound_htlc_maximum_msat: None,
                        },
                        funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
                        channel_type: None,
                        force_close_spend_delay: None,
                        is_outbound: true, is_funding_locked: true,
                        is_usable: true, is_public: true,
 +                      inbound_htlc_minimum_msat: None,
 +                      inbound_htlc_maximum_msat: None,
                }
        }
  
                assert_eq!(route.paths[0][4].short_channel_id, 8);
                assert_eq!(route.paths[0][4].fee_msat, 100);
                assert_eq!(route.paths[0][4].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][4].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][4].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][4].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
  
                assert_eq!(route.paths[0][4].short_channel_id, 8);
                assert_eq!(route.paths[0][4].fee_msat, 100);
                assert_eq!(route.paths[0][4].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][4].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][4].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][4].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
  
                assert_eq!(route.paths[0][3].short_channel_id, last_hops[0].0[1].short_channel_id);
                assert_eq!(route.paths[0][3].fee_msat, 100);
                assert_eq!(route.paths[0][3].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][3].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][3].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
  
                assert_eq!(route.paths[0][2].short_channel_id, last_hops[0].0[0].short_channel_id);
                assert_eq!(route.paths[0][2].fee_msat, 0);
                assert_eq!(route.paths[0][2].cltv_expiry_delta, 129);
 -              assert_eq!(route.paths[0][2].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][2].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][2].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
  
                assert_eq!(route.paths[0][3].pubkey, nodes[6]);
                assert_eq!(route.paths[0][3].short_channel_id, last_hops[0].0[1].short_channel_id);
                assert_eq!(route.paths[0][3].fee_msat, 100);
                assert_eq!(route.paths[0][3].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][3].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][3].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
  
                assert_eq!(route.paths[0][4].short_channel_id, 8);
                assert_eq!(route.paths[0][4].fee_msat, 100);
                assert_eq!(route.paths[0][4].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][4].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][4].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][4].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
  
                assert_eq!(route.paths[0][1].short_channel_id, 8);
                assert_eq!(route.paths[0][1].fee_msat, 100);
                assert_eq!(route.paths[0][1].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][1].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][1].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][1].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
  
                last_hops[0].0[0].fees.base_msat = 1000;
                assert_eq!(route.paths[0][3].short_channel_id, 10);
                assert_eq!(route.paths[0][3].fee_msat, 100);
                assert_eq!(route.paths[0][3].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][3].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][3].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
  
                // ...but still use 8 for larger payments as 6 has a variable feerate
                assert_eq!(route.paths[0][4].short_channel_id, 8);
                assert_eq!(route.paths[0][4].fee_msat, 2000);
                assert_eq!(route.paths[0][4].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][4].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][4].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][4].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
  
                assert_eq!(route.paths[0][1].short_channel_id, 8);
                assert_eq!(route.paths[0][1].fee_msat, 1000000);
                assert_eq!(route.paths[0][1].cltv_expiry_delta, 42);
 -              assert_eq!(route.paths[0][1].node_features.le_flags(), &[0; 0]); // We dont pass flags in from invoices yet
 +              assert_eq!(route.paths[0][1].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][1].channel_features.le_flags(), &[0; 0]); // We can't learn any flags from invoices, sadly
        }
  
                                let payment_params = PaymentParameters::from_node_id(dst);
                                let amt = seed as u64 % 200_000_000;
                                let params = ProbabilisticScoringParameters::default();
-                               let scorer = ProbabilisticScorer::new(params, &graph);
-                               if get_route(src, &payment_params, &graph.read_only(), None, amt, 42, &test_utils::TestLogger::new(), &scorer, &random_seed_bytes).is_ok() {
+                               let logger = test_utils::TestLogger::new();
+                               let scorer = ProbabilisticScorer::new(params, &graph, &logger);
+                               if get_route(src, &payment_params, &graph.read_only(), None, amt, 42, &logger, &scorer, &random_seed_bytes).is_ok() {
                                        continue 'load_endpoints;
                                }
                        }
                                let payment_params = PaymentParameters::from_node_id(dst).with_features(InvoiceFeatures::known());
                                let amt = seed as u64 % 200_000_000;
                                let params = ProbabilisticScoringParameters::default();
-                               let scorer = ProbabilisticScorer::new(params, &graph);
-                               if get_route(src, &payment_params, &graph.read_only(), None, amt, 42, &test_utils::TestLogger::new(), &scorer, &random_seed_bytes).is_ok() {
+                               let logger = test_utils::TestLogger::new();
+                               let scorer = ProbabilisticScorer::new(params, &graph, &logger);
+                               if get_route(src, &payment_params, &graph.read_only(), None, amt, 42, &logger, &scorer, &random_seed_bytes).is_ok() {
                                        continue 'load_endpoints;
                                }
                        }
@@@ -5433,6 -5419,7 +5435,7 @@@ mod benches 
        use ln::features::{InitFeatures, InvoiceFeatures};
        use routing::scoring::{FixedPenaltyScorer, ProbabilisticScorer, ProbabilisticScoringParameters, Scorer};
        use util::logger::{Logger, Record};
+       use util::test_utils::TestLogger;
  
        use test::Bencher;
  
                                node_id,
                                unspendable_punishment_reserve: 0,
                                forwarding_info: None,
 +                              outbound_htlc_minimum_msat: None,
 +                              outbound_htlc_maximum_msat: None,
                        },
                        funding_txo: Some(OutPoint {
                                txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0
                        is_funding_locked: true,
                        is_usable: true,
                        is_public: true,
 +                      inbound_htlc_minimum_msat: None,
 +                      inbound_htlc_maximum_msat: None,
                }
        }
  
  
        #[bench]
        fn generate_routes_with_probabilistic_scorer(bench: &mut Bencher) {
+               let logger = TestLogger::new();
                let network_graph = read_network_graph();
                let params = ProbabilisticScoringParameters::default();
-               let scorer = ProbabilisticScorer::new(params, &network_graph);
+               let scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
                generate_routes(bench, &network_graph, scorer, InvoiceFeatures::empty());
        }
  
        #[bench]
        fn generate_mpp_routes_with_probabilistic_scorer(bench: &mut Bencher) {
+               let logger = TestLogger::new();
                let network_graph = read_network_graph();
                let params = ProbabilisticScoringParameters::default();
-               let scorer = ProbabilisticScorer::new(params, &network_graph);
+               let scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
                generate_routes(bench, &network_graph, scorer, InvoiceFeatures::known());
        }