Skip to content

Commit aad65c8

Browse files
committed
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.
1 parent 3da2e55 commit aad65c8

File tree

2 files changed

+8
-31
lines changed

2 files changed

+8
-31
lines changed

lightning/src/ln/functional_tests.rs

-23
Original file line numberDiff line numberDiff line change
@@ -1058,26 +1058,6 @@ fn fake_network_test() {
10581058
fail_payment(&nodes[1], &vec!(&nodes[3], &nodes[2], &nodes[1])[..], payment_hash_2);
10591059
claim_payment(&nodes[1], &vec!(&nodes[2], &nodes[3], &nodes[1])[..], payment_preimage_1);
10601060

1061-
// Add a duplicate new channel from 2 to 4
1062-
let chan_5 = create_announced_chan_between_nodes(&nodes, 1, 3, InitFeatures::known(), InitFeatures::known());
1063-
1064-
// Send some payments across both channels
1065-
let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0;
1066-
let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0;
1067-
let payment_preimage_5 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0;
1068-
1069-
1070-
route_over_limit(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000);
1071-
let events = nodes[0].node.get_and_clear_pending_msg_events();
1072-
assert_eq!(events.len(), 0);
1073-
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);
1074-
1075-
//TODO: Test that routes work again here as we've been notified that the channel is full
1076-
1077-
claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_3);
1078-
claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_4);
1079-
claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_5);
1080-
10811061
// Close down the channels...
10821062
close_channel(&nodes[0], &nodes[1], &chan_1.2, chan_1.3, true);
10831063
check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure);
@@ -1091,9 +1071,6 @@ fn fake_network_test() {
10911071
close_channel(&nodes[1], &nodes[3], &chan_4.2, chan_4.3, false);
10921072
check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure);
10931073
check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure);
1094-
close_channel(&nodes[1], &nodes[3], &chan_5.2, chan_5.3, false);
1095-
check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure);
1096-
check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure);
10971074
}
10981075

10991076
#[test]

lightning/src/routing/router.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ pub struct PaymentParameters {
237237
/// A value of 0 will allow payments up to and including a channel's total announced usable
238238
/// capacity, a value of one will only use up to half its capacity, two 1/4, etc.
239239
///
240-
/// Default value: 1
240+
/// Default value: 2
241241
pub max_channel_saturation_power_of_half: u8,
242242
}
243243

@@ -247,7 +247,7 @@ impl_writeable_tlv_based!(PaymentParameters, {
247247
(2, features, option),
248248
(3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)),
249249
(4, route_hints, vec_type),
250-
(5, max_channel_saturation_power_of_half, (default_value, 1)),
250+
(5, max_channel_saturation_power_of_half, (default_value, 2)),
251251
(6, expiry_time, option),
252252
});
253253

@@ -261,7 +261,7 @@ impl PaymentParameters {
261261
expiry_time: None,
262262
max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA,
263263
max_path_count: DEFAULT_MAX_PATH_COUNT,
264-
max_channel_saturation_power_of_half: 1,
264+
max_channel_saturation_power_of_half: 2,
265265
}
266266
}
267267

@@ -4745,7 +4745,7 @@ mod tests {
47454745
cltv_expiry_delta: 42,
47464746
htlc_minimum_msat: None,
47474747
htlc_maximum_msat: None,
4748-
}])]);
4748+
}])]).with_max_channel_saturation_power_of_half(0);
47494749

47504750
// Keep only two paths from us to nodes[2], both with a 99sat HTLC maximum, with one with
47514751
// no fee and one with a 1msat fee. Previously, trying to route 100 sats to nodes[2] here
@@ -5700,7 +5700,7 @@ mod tests {
57005700
flags: 0,
57015701
cltv_expiry_delta: (4 << 4) | 1,
57025702
htlc_minimum_msat: 0,
5703-
htlc_maximum_msat: OptionalField::Present(200_000_000),
5703+
htlc_maximum_msat: OptionalField::Present(250_000_000),
57045704
fee_base_msat: 0,
57055705
fee_proportional_millionths: 0,
57065706
excess_data: Vec::new()
@@ -5712,7 +5712,7 @@ mod tests {
57125712
flags: 0,
57135713
cltv_expiry_delta: (13 << 4) | 1,
57145714
htlc_minimum_msat: 0,
5715-
htlc_maximum_msat: OptionalField::Present(200_000_000),
5715+
htlc_maximum_msat: OptionalField::Present(250_000_000),
57165716
fee_base_msat: 0,
57175717
fee_proportional_millionths: 0,
57185718
excess_data: Vec::new()
@@ -5721,8 +5721,8 @@ mod tests {
57215721
let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
57225722
let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
57235723
let random_seed_bytes = keys_manager.get_secure_random_bytes();
5724-
// 150,000 sat is less than the available liquidity on each channel, set above.
5725-
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();
5724+
// 100,000 sat is less than the available liquidity on each channel, set above.
5725+
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();
57265726
assert_eq!(route.paths.len(), 2);
57275727
assert!((route.paths[0][1].short_channel_id == 4 && route.paths[1][1].short_channel_id == 13) ||
57285728
(route.paths[1][1].short_channel_id == 4 && route.paths[0][1].short_channel_id == 13));

0 commit comments

Comments
 (0)