X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Frouting%2Frouter.rs;h=9da2d981e8442feaba72e94cac58cdcd1e5cfcc0;hb=431f807907ae390a255641719dd5cb3279bc9d5b;hp=0edeef0c3cb3d9629ddcfd4445dbf2f1fccf3af4;hpb=da7a851d4785891190b70ff03c21fa6340a7ec5a;p=rust-lightning diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 0edeef0c..9da2d981 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -49,13 +49,13 @@ pub struct RouteHop { } impl_writeable_tlv_based!(RouteHop, { - (0, pubkey), - (2, node_features), - (4, short_channel_id), - (6, channel_features), - (8, fee_msat), - (10, cltv_expiry_delta), -}, {}, {}); + (0, pubkey, required), + (2, node_features, required), + (4, short_channel_id, required), + (6, channel_features, required), + (8, fee_msat, required), + (10, cltv_expiry_delta, required), +}); /// A route directs a payment from the sender (us) to the recipient. If the recipient supports MPP, /// it can take multiple paths. Each path is composed of one or more hops through the network. @@ -83,7 +83,7 @@ impl Writeable for Route { hop.write(writer)?; } } - write_tlv_fields!(writer, {}, {}); + write_tlv_fields!(writer, {}); Ok(()) } } @@ -101,7 +101,7 @@ impl Readable for Route { } paths.push(hops); } - read_tlv_fields!(reader, {}, {}); + read_tlv_fields!(reader, {}); Ok(Route { paths }) } } @@ -168,7 +168,7 @@ struct DummyDirectionalChannelInfo { /// so that we can choose cheaper paths (as per Dijkstra's algorithm). /// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees. /// These fee values are useful to choose hops as we traverse the graph "payee-to-payer". -#[derive(Clone)] +#[derive(Clone, Debug)] struct PathBuildingHop<'a> { // The RouteHintHop fields which will eventually be used if this hop is used in a final Route. // Note that node_features is calculated separately after our initial graph walk. @@ -506,14 +506,19 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye // - when we want to stop looking for new paths. let mut already_collected_value_msat = 0; + log_trace!(logger, "Building path from {} (payee) to {} (us/payer) for value {} msat.", payee, our_node_id, final_value_msat); + macro_rules! add_entry { // Adds entry which goes from $src_node_id to $dest_node_id // over the channel with id $chan_id with fees described in // $directional_info. // $next_hops_fee_msat represents the fees paid for using all the channel *after* this one, // since that value has to be transferred over this channel. + // Returns whether this channel caused an update to `targets`. ( $chan_id: expr, $src_node_id: expr, $dest_node_id: expr, $directional_info: expr, $capacity_sats: expr, $chan_features: expr, $next_hops_fee_msat: expr, - $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => { + $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => { { + // We "return" whether we updated the path at the end, via this: + let mut did_add_update_path_to_src_node = false; // Channels to self should not be used. This is more of belt-and-suspenders, because in // practice these cases should be caught earlier: // - for regular channels at channel announcement (TODO) @@ -725,6 +730,7 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye { old_entry.value_contribution_msat = value_contribution_msat; } + did_add_update_path_to_src_node = true; } else if old_entry.was_processed && new_cost < old_cost { #[cfg(any(test, feature = "fuzztarget"))] { @@ -755,7 +761,8 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye } } } - }; + did_add_update_path_to_src_node + } } } let empty_node_features = NodeFeatures::empty(); @@ -858,22 +865,10 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye // it matters only if the fees are exactly the same. for hop in last_hops.iter() { let have_hop_src_in_graph = - if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&hop.src_node_id) { - // If this hop connects to a node with which we have a direct channel, ignore - // the network graph and add both the hop and our direct channel to - // the candidate set. - // - // Currently there are no channel-context features defined, so we are a - // bit lazy here. In the future, we should pull them out via our - // ChannelManager, but there's no reason to waste the space until we - // need them. - add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0); - true - } else { - // In any other case, only add the hop if the source is in the regular network - // graph: - network.get_nodes().get(&hop.src_node_id).is_some() - }; + // Only add the last hop to our candidate set if either we have a direct channel or + // they are in the regular network graph. + first_hop_targets.get(&hop.src_node_id).is_some() || + network.get_nodes().get(&hop.src_node_id).is_some(); if have_hop_src_in_graph { // BOLT 11 doesn't allow inclusion of features for the last hop hints, which // really sucks, cause we're gonna need that eventually. @@ -887,10 +882,23 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye htlc_maximum_msat: hop.htlc_maximum_msat, fees: hop.fees, }; - add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::, &empty_channel_features, 0, path_value_msat, 0); + if add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::, &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. + // + // Note that we *must* check if the last hop was added as `add_entry` + // always assumes that the third argument is a node to which we have a + // path. + if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&hop.src_node_id) { + add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0); + } + } } } + log_trace!(logger, "Starting main path collection loop with {} nodes pre-filled from first/last hops.", targets.len()); + // At this point, targets are filled with the data from first and // last hops communicated by the caller, and the payment receiver. let mut found_new_path = false; @@ -954,6 +962,9 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye ordered_hops.last_mut().unwrap().0.hop_use_fee_msat = 0; ordered_hops.last_mut().unwrap().0.cltv_expiry_delta = final_cltv; + log_trace!(logger, "Found a path back to us from the target with {} hops contributing up to {} msat: {:?}", + ordered_hops.len(), value_contribution_msat, ordered_hops); + let mut payment_path = PaymentPath {hops: ordered_hops}; // We could have possibly constructed a slightly inconsistent path: since we reduce @@ -989,8 +1000,9 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye // If we weren't capped by hitting a liquidity limit on a channel in the path, // we'll probably end up picking the same path again on the next iteration. // Decrease the available liquidity of a hop in the middle of the path. - let victim_liquidity = bookkeeped_channels_liquidity_available_msat.get_mut( - &payment_path.hops[(payment_path.hops.len() - 1) / 2].0.short_channel_id).unwrap(); + let victim_scid = payment_path.hops[(payment_path.hops.len() - 1) / 2].0.short_channel_id; + log_trace!(logger, "Disabling channel {} for future path building iterations to avoid duplicates.", victim_scid); + let victim_liquidity = bookkeeped_channels_liquidity_available_msat.get_mut(&victim_scid).unwrap(); *victim_liquidity = 0; } @@ -1032,6 +1044,8 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye // In the latter case, making another path finding attempt won't help, // because we deterministically terminated the search due to low liquidity. if already_collected_value_msat >= recommended_value_msat || !found_new_path { + log_trace!(logger, "Have now collected {} msat (seeking {} msat) in paths. Last path loop {} a new path.", + already_collected_value_msat, recommended_value_msat, if found_new_path { "found" } else { "did not find" }); break 'paths_collection; } else if found_new_path && already_collected_value_msat == final_value_msat && payment_paths.len() == 1 { // Further, if this was our first walk of the graph, and we weren't limited by an @@ -1040,8 +1054,10 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye // potentially allowing us to pay fees to meet the htlc_minimum on the new path while // still keeping a lower total fee than this path. if !hit_minimum_limit { + log_trace!(logger, "Collected exactly our payment amount on the first pass, without hitting an htlc_minimum_msat limit, exiting."); break 'paths_collection; } + log_trace!(logger, "Collected our payment amount on the first pass, but running again to collect extra paths with a potentially higher limit."); path_value_msat = recommended_value_msat; } } @@ -1152,13 +1168,13 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye } let route = Route { paths: selected_paths }; - log_trace!(logger, "Got route: {}", log_route!(route)); + log_info!(logger, "Got route to {}: {}", payee, log_route!(route)); Ok(route) } #[cfg(test)] mod tests { - use routing::router::{get_route, RouteHint, RouteHintHop, RoutingFees}; + use routing::router::{get_route, Route, RouteHint, RouteHintHop, RoutingFees}; use routing::network_graph::{NetworkGraph, NetGraphMsgHandler}; use chain::transaction::OutPoint; use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures}; @@ -1630,6 +1646,10 @@ mod tests { user_id: 0, outbound_capacity_msat: 100000, inbound_capacity_msat: 100000, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None, @@ -1950,6 +1970,10 @@ mod tests { user_id: 0, outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None, @@ -2000,6 +2024,10 @@ mod tests { user_id: 0, outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None, @@ -2067,6 +2095,10 @@ mod tests { user_id: 0, outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None, @@ -2206,6 +2238,10 @@ mod tests { user_id: 0, outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None, @@ -2306,11 +2342,7 @@ mod tests { assert_eq!(route.paths[0][4].channel_features.le_flags(), &Vec::::new()); // We can't learn any flags from invoices, sadly } - #[test] - fn unannounced_path_test() { - // We should be able to send a payment to a destination without any help of a routing graph - // if we have a channel with a common counterparty that appears in the first and last hop - // hints. + fn do_unannounced_path_test(last_hop_htlc_max: Option, last_hop_fee_prop: u32, outbound_capacity_msat: u64, route_val: u64) -> Result { let source_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 41).repeat(32)).unwrap()[..]).unwrap()); let middle_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 42).repeat(32)).unwrap()[..]).unwrap()); let target_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 43).repeat(32)).unwrap()[..]).unwrap()); @@ -2321,11 +2353,11 @@ mod tests { short_channel_id: 8, fees: RoutingFees { base_msat: 1000, - proportional_millionths: 0, + proportional_millionths: last_hop_fee_prop, }, cltv_expiry_delta: (8 << 8) | 1, htlc_minimum_msat: None, - htlc_maximum_msat: None, + htlc_maximum_msat: last_hop_htlc_max, }]); let our_chans = vec![channelmanager::ChannelDetails { channel_id: [0; 32], @@ -2335,31 +2367,63 @@ mod tests { counterparty_features: InitFeatures::from_le_bytes(vec![0b11]), channel_value_satoshis: 100000, user_id: 0, - outbound_capacity_msat: 100000, + outbound_capacity_msat: outbound_capacity_msat, inbound_capacity_msat: 100000, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None, }]; - let route = get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, None, Some(&our_chans.iter().collect::>()), &vec![&last_hops], 100, 42, Arc::new(test_utils::TestLogger::new())).unwrap(); + get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, None, Some(&our_chans.iter().collect::>()), &vec![&last_hops], route_val, 42, Arc::new(test_utils::TestLogger::new())) + } + #[test] + fn unannounced_path_test() { + // We should be able to send a payment to a destination without any help of a routing graph + // if we have a channel with a common counterparty that appears in the first and last hop + // hints. + let route = do_unannounced_path_test(None, 1, 2000000, 1000000).unwrap(); + + let middle_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 42).repeat(32)).unwrap()[..]).unwrap()); + let target_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 43).repeat(32)).unwrap()[..]).unwrap()); assert_eq!(route.paths[0].len(), 2); assert_eq!(route.paths[0][0].pubkey, middle_node_id); assert_eq!(route.paths[0][0].short_channel_id, 42); - assert_eq!(route.paths[0][0].fee_msat, 1000); + assert_eq!(route.paths[0][0].fee_msat, 1001); assert_eq!(route.paths[0][0].cltv_expiry_delta, (8 << 8) | 1); assert_eq!(route.paths[0][0].node_features.le_flags(), &[0b11]); assert_eq!(route.paths[0][0].channel_features.le_flags(), &[0; 0]); // We can't learn any flags from invoices, sadly assert_eq!(route.paths[0][1].pubkey, target_node_id); 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].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].channel_features.le_flags(), &[0; 0]); // We can't learn any flags from invoices, sadly } + #[test] + fn overflow_unannounced_path_test_liquidity_underflow() { + // Previously, when we had a last-hop hint connected directly to a first-hop channel, where + // the last-hop had a fee which overflowed a u64, we'd panic. + // This was due to us adding the first-hop from us unconditionally, causing us to think + // we'd built a path (as our node is in the "best candidate" set), when we had not. + // In this test, we previously hit a subtraction underflow due to having less available + // liquidity at the last hop than 0. + assert!(do_unannounced_path_test(Some(21_000_000_0000_0000_000), 0, 21_000_000_0000_0000_000, 21_000_000_0000_0000_000).is_err()); + } + + #[test] + fn overflow_unannounced_path_test_feerate_overflow() { + // This tests for the same case as above, except instead of hitting a subtraction + // underflow, we hit a case where the fee charged at a hop overflowed. + assert!(do_unannounced_path_test(Some(21_000_000_0000_0000_000), 50000, 21_000_000_0000_0000_000, 21_000_000_0000_0000_000).is_err()); + } + #[test] fn available_amount_while_routing_test() { // Tests whether we choose the correct available channel amount while routing. @@ -2471,6 +2535,10 @@ mod tests { user_id: 0, outbound_capacity_msat: 200_000_000, inbound_capacity_msat: 0, + to_self_reserve_satoshis: None, + to_remote_reserve_satoshis: 0, + confirmations_required: None, + spend_csv_on_our_commitment_funds: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, counterparty_forwarding_info: None,