From: Matt Corallo Date: Tue, 12 Jul 2022 21:13:30 +0000 (+0000) Subject: Reduce default max_channel_saturation_power_of_half to 2 (max 1/4) X-Git-Tag: v0.0.110~11^2 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=refs%2Fheads%2F2022-07-no-rand-path-selection;p=rust-lightning Reduce default max_channel_saturation_power_of_half to 2 (max 1/4) Saturating a channel beyond 1/4 of its capacity seems like a more reasonable threshold for avoiding a path than 1/2, especially given we should still be willing to send a payment with a lower saturation limit if it comes to that. This requires an (obvious) change to some router tests, but also requires a change to the `fake_network_test`, opting to simply remove some over-limit test code there - `fake_network_test` was our first ever functional test, and while it worked great to ensure LDK worked at all on day one, we now have a rather large breadth of functional tests, and a broad "does it work at all" test is no longer all that useful. --- diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4894ed02b..1da45bf2c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1059,26 +1059,6 @@ fn fake_network_test() { fail_payment(&nodes[1], &vec!(&nodes[3], &nodes[2], &nodes[1])[..], payment_hash_2); claim_payment(&nodes[1], &vec!(&nodes[2], &nodes[3], &nodes[1])[..], payment_preimage_1); - // Add a duplicate new channel from 2 to 4 - let chan_5 = create_announced_chan_between_nodes(&nodes, 1, 3, InitFeatures::known(), InitFeatures::known()); - - // Send some payments across both channels - let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0; - let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0; - let payment_preimage_5 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0; - - - route_over_limit(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000); - let events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 0); - nodes[0].logger.assert_log_regex("lightning::ln::channelmanager".to_string(), regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap(), 1); - - //TODO: Test that routes work again here as we've been notified that the channel is full - - claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_3); - claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_4); - claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_5); - // Close down the channels... close_channel(&nodes[0], &nodes[1], &chan_1.2, chan_1.3, true); check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure); @@ -1092,9 +1072,6 @@ fn fake_network_test() { close_channel(&nodes[1], &nodes[3], &chan_4.2, chan_4.3, false); check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure); check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure); - close_channel(&nodes[1], &nodes[3], &chan_5.2, chan_5.3, false); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure); - check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure); } #[test] diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 1114f9901..1b14e9c73 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -238,7 +238,7 @@ pub struct PaymentParameters { /// A value of 0 will allow payments up to and including a channel's total announced usable /// capacity, a value of one will only use up to half its capacity, two 1/4, etc. /// - /// Default value: 1 + /// Default value: 2 pub max_channel_saturation_power_of_half: u8, /// A list of SCIDs which this payment was previously attempted over and which caused the @@ -253,7 +253,7 @@ impl_writeable_tlv_based!(PaymentParameters, { (2, features, option), (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)), (4, route_hints, vec_type), - (5, max_channel_saturation_power_of_half, (default_value, 1)), + (5, max_channel_saturation_power_of_half, (default_value, 2)), (6, expiry_time, option), (7, previously_failed_channels, vec_type), }); @@ -268,7 +268,7 @@ impl PaymentParameters { expiry_time: None, max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, max_path_count: DEFAULT_MAX_PATH_COUNT, - max_channel_saturation_power_of_half: 1, + max_channel_saturation_power_of_half: 2, previously_failed_channels: Vec::new(), } } @@ -4757,7 +4757,7 @@ mod tests { cltv_expiry_delta: 42, htlc_minimum_msat: None, htlc_maximum_msat: None, - }])]); + }])]).with_max_channel_saturation_power_of_half(0); // Keep only two paths from us to nodes[2], both with a 99sat HTLC maximum, with one with // no fee and one with a 1msat fee. Previously, trying to route 100 sats to nodes[2] here @@ -5741,7 +5741,7 @@ mod tests { flags: 0, cltv_expiry_delta: (4 << 4) | 1, htlc_minimum_msat: 0, - htlc_maximum_msat: OptionalField::Present(200_000_000), + htlc_maximum_msat: OptionalField::Present(250_000_000), fee_base_msat: 0, fee_proportional_millionths: 0, excess_data: Vec::new() @@ -5753,7 +5753,7 @@ mod tests { flags: 0, cltv_expiry_delta: (13 << 4) | 1, htlc_minimum_msat: 0, - htlc_maximum_msat: OptionalField::Present(200_000_000), + htlc_maximum_msat: OptionalField::Present(250_000_000), fee_base_msat: 0, fee_proportional_millionths: 0, excess_data: Vec::new() @@ -5762,8 +5762,8 @@ mod tests { let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known()); let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - // 150,000 sat is less than the available liquidity on each channel, set above. - let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 150_000_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); + // 100,000 sats is less than the available liquidity on each channel, set above. + let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100_000_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); assert_eq!(route.paths.len(), 2); assert!((route.paths[0][1].short_channel_id == 4 && route.paths[1][1].short_channel_id == 13) || (route.paths[1][1].short_channel_id == 4 && route.paths[0][1].short_channel_id == 13));