f Fix features assignment and naming

Author: ViktorTigerstrom

lightning/src/routing/router.rs

@@ -602,6 +602,17 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 	}
 }
 
+/// 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`.
@@ -1101,8 +1112,7 @@ where L::Target: Logger {
 		} }
 	}
 
-	let mut onion_format_optional_features = NodeFeatures::empty();
-	onion_format_optional_features.set_variable_length_onion_optional();
+	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).
@@ -1134,7 +1144,7 @@ where L::Target: Logger {
 				let features = if let Some(node_info) = $node.announcement_info.as_ref() {
 					&node_info.features
 				} else {
-					&onion_format_optional_features
+					&default_node_features
 				};
 
 				if !features.requires_unknown_bits() {
@@ -1314,7 +1324,7 @@ where L::Target: Logger {
 			// 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(), onion_format_optional_features.clone()));
+				let mut ordered_hops = vec!((new_entry.clone(), default_node_features.clone()));
 
 				'path_walk: loop {
 					let mut features_set = false;
@@ -1332,7 +1342,7 @@ where L::Target: Logger {
 							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 = onion_format_optional_features.clone();
+								ordered_hops.last_mut().unwrap().1 = default_node_features.clone();
 							}
 						} else {
 							// We can fill in features for everything except hops which were
@@ -1359,7 +1369,7 @@ where L::Target: Logger {
 					// 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(), onion_format_optional_features.clone()));
+					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;
@@ -1686,7 +1696,7 @@ fn add_random_cltv_offset(route: &mut Route, payment_params: &PaymentParameters,
 #[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;
@@ -1847,10 +1857,6 @@ mod tests {
 		}
 	}
 
-	fn default_onion_format_flags() -> Vec<u8> {
-		vec![0, 2]
-	}
-
 	fn build_graph() -> (
 		Secp256k1<All>,
 		sync::Arc<NetworkGraph>,
@@ -2791,7 +2797,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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
 	}
 
@@ -2867,7 +2873,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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
 	}
 
@@ -2964,7 +2970,7 @@ mod tests {
 		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![0, 2]); // 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
 	}
 
@@ -3029,14 +3035,14 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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(), &default_onion_format_flags()); // 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
 	}
 
@@ -3127,7 +3133,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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
 	}
 
@@ -3157,7 +3163,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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;
@@ -3194,7 +3200,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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
@@ -3235,7 +3241,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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
 	}
 
@@ -3287,7 +3293,7 @@ mod tests {
 		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(), &default_onion_format_flags()); // 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
 	}