Router disables central node within route even if there is only one path #3685
Comments
The original motivation for the block was to ensure MPPs vary their paths a bit by disabling the 'central' hop in any found paths. However, we are disabling this block, as it: 1) causes MPP routing failures if there is only one path from payer -> payee 2) forces MPPs to be routed across less optimal paths Issue: lightningdevkit#3685 The one possible privacy concern is that any hops which forward multiple shards of the same MPP can link those shards together, as each shard uses the same payment hash (since LDK implements Simplified Multipath Payments, not Atomic Multipath Payments). But this concern seems moot, as sending the payment over a single shard leaks almost the same information anyway. As far as I (and SOTA AI models) can tell, disabling this block doesn't violate any correctness constraints or security concerns in surrounding code. License: PolyForm Noncommercial License 1.0.0
The original motivation for the block was to ensure MPPs vary their paths a bit by disabling the 'central' hop in any found paths. However, we are disabling this block, as it: 1) causes MPP routing failures if there is only one path from payer -> payee 2) forces MPPs to be routed across less optimal paths Issue: lightningdevkit#3685 The one possible privacy concern is that any hops which forward multiple shards of the same MPP can link those shards together, as each shard uses the same payment hash (since LDK implements Simplified Multipath Payments, not Atomic Multipath Payments). But this concern seems moot, as sending the payment over a single shard leaks almost the same information anyway. As far as I (and SOTA AI models) can tell, disabling this block doesn't violate any correctness constraints or security concerns in surrounding code. License: PolyForm Noncommercial License 1.0.0
|
Odd, I'm unable to reproduce this on 0.0.125 with this test that results in ~identical logs up until the part where your logs disable a channel: #[test]
fn xxx() {
let mut cfg = test_default_channel_config();
cfg.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
let mut high_fee_cfg = cfg.clone();
high_fee_cfg.channel_config.forwarding_fee_base_msat = 1_050_000;
let chanmon_cfgs = create_chanmon_cfgs(3);
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(cfg.clone()), Some(high_fee_cfg.clone()), Some(cfg.clone())]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 200_000, 0);
create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 300_000, 0);
create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 600_000, 0);
let amt_msat = 350_000_000;
let invoice = crate::ln::invoice_utils::create_invoice_from_channelmanager(
&nodes[2].node, &nodes[2].keys_manager, &nodes[2].logger, Currency::BitcoinTestnet,
Some(amt_msat), "".to_string(), 100000000, None
).unwrap();
let (payment_hash, recipient_onion_fields, mut route_params) =
crate::ln::bolt11_payment::payment_parameters_from_invoice(&invoice).unwrap();
route_params.max_total_routing_fee_msat = Some(u64::MAX);
nodes[0].node.send_payment(
payment_hash, recipient_onion_fields, PaymentId([42; 32]), route_params, Retry::Attempts(0)
).unwrap();
assert!(nodes[0].node.list_recent_payments().len() == 1);
check_added_monitors(&nodes[0], 2); // one monitor update per MPP part
nodes[0].node.get_and_clear_pending_msg_events();
}To confirm, this is on 0.0.125? |
|
This is on 0.1.1, but my understanding is this behavior was added years ago. @valentinewallace I don't think it should affect the reproducibility, but we tested with intercept hints. I'll run your code and see if I can get it to reproduce. |
|
Ah okay, if it's 0.1.1 let me send you an updated test since the interfaces changed a bit, gimme a sec. |
|
@MaxFangX check out this commit based on 0.1.1: valentinewallace@e66c746 |
|
I'm assuming the 0.1.1 version didn't reproduce either? |
|
That's correct :/ and the logs still look the same until they diverge. |
|
Edit: Setting I imported Lexe's This is my first time working with LDK's test utils, I don't quite understand what's going on here. Code here: https://github.com/lexe-app/rust-lightning/tree/max/repro-central-route-issue (you can cherry pick the last commit) |
|
I've uploaded the full logs from our integration test which reliably reproduces this error here. The stdout output includes the results of calls to https://gist.github.com/MaxFangX/3bfeb8328d7b55d624330965c7d56944 I've also updated the repro branch to include the exact commits in Lexe's LDK fork, including an extra commit which reverts our workaround for this issue. Appreciate the help @valentinewallace, we already have a workaround for this so there's no rush. |
Consider this specific configuration:
If Charlie includes just one route hint, the maximum payment amount for which LDK routing succeeds is around ~295.5k sats., i.e. it's not possible to send a MPP payment which uses more than one of Alice's channels.
Here's the relevant logs from testing this:
I tracked the issue down to this block in
router.rs:If I disable this block, the payment succeeds. In this case,
payment_path.hops.len()is2, so we disablepayment_path.hops[1], which happens to be the Bob -> Charlie hop.If there is only one path from a sender to a recipient, it seems wrong to disable any hop within it, since it prevents other MPP shards from finding any route at all. I understand the motivation for varying our paths for MPPs, but it shouldn't cost at the cost of routing reliability.
The text was updated successfully, but these errors were encountered: