mpp_split: stop returning splits without amounts
What the developer wrote
Authored by f321x
mpp_split: stop returning splits without amounts
`mpp_split.suggest_splits()` would return payment split
configurations with no split amounts like:
`{(channel_id, node_id): []}`.
This is unintuitive and introduced two bugs:
1. `LNWallet.create_routes_for_payment()` evaluates
`is_multichan_mpp = len(sc.config.items()) > 1`.
So even if the actual payment amount gets split onto
a single channel this would falsely evaluate `True`
if there is a empty split.
2. `is_direct_path = all(node_id == paysession.invoice_pubkey for (chan_id, node_id) in sc.config.keys())`
similarly might incorrectly evaluates `False` if the split
contains an empty split for another `node_id`.
What changed, and why it matters
This commit fixes a bug in Electrum's Lightning payment splitting logic. Previously, the software could create payment plans that included empty 'splits' for channels that weren't actually being used. This caused two downstream checks to misclassify payments: a single-channel payment could be treated as a multi-channel payment, and a direct payment to the intended recipient could be treated as a routed payment. These misclassifications could lead to less efficient routing, higher fees, or routing failures rather than direct payment success.
Review the downstream effects in `LNWallet.create_routes_for_payment()` to confirm that removing empty splits fully resolves the misclassification issues. Consider whether any other consumers of `suggest_splits()` relied on the presence of empty entries. No immediate emergency action appears necessary, but users making Lightning payments should update to the fixed version.
Security signals we found
Logic bug in payment routing classification
Empty split entries cause incorrect multi-channel/direct-path detection
Could lead to suboptimal or failed Lightning payment routing
No explicit security disclosure in commit message
Evidence from the diff
The suggest_splits() function in electrum/mpp_split.py was returning SplitConfig objects containing entries like {(channel_id, node_id): []} where a channel was present in the config but had no allocated amount. This caused two bugs in LNWallet.create_routes_for_payment(): (1) is_multichan_mpp = len(sc.config.items()) > 1 evaluated to True even when the actual amount was on a single channel, and (2) is_direct_path = all(node_id == paysession.invoice_pubkey for ...) could evaluate to False due to an empty split for a different node_id. The patch ensures empty entries are not added to the config by using config.get(c, []) for reads and only calling config.setdefault(c, []).append(...) when a positive amount is actually being assigned.
Changed components
electrum/mpp_split.pyelectrum/lnworker.py (LNWallet.create_routes_for_payment, referenced but not patched)Inspect captured patch +10 / −11
diff --git a/electrum/mpp_split.py b/electrum/mpp_split.py
index c8d9d3e..dc0ed19 100644
--- a/electrum/mpp_split.py
+++ b/electrum/mpp_split.py
@@ -143,11 +143,10 @@ def suggest_splits(
random.shuffle(channel_keys)
# we check each channel and try to put the funds inside, break if we succeed
for c in channel_keys:
- if c not in config:
- config[c] = []
+ amounts = config.get(c, [])
channel_funds, channel_slots = channels_with_funds[c]
- if sum(config[c]) + amount <= channel_funds and len(config[c]) < channel_slots:
- config[c].append(amount)
+ if sum(amounts) + amount <= channel_funds and len(amounts) < channel_slots:
+ config.setdefault(c, []).append(amount)
break
# if we don't succeed to put the amount anywhere,
# we try to fill up channels and put the rest somewhere else
@@ -155,15 +154,17 @@ def suggest_splits(
distribute_amount = amount
for c in channel_keys:
channel_funds, channel_slots = channels_with_funds[c]
- slots_left = channel_slots - len(config[c])
+ amounts = config.get(c, [])
+ slots_left = channel_slots - len(amounts)
if slots_left == 0:
# no slot left in that channel
continue
- funds_left = channel_funds - sum(config[c])
+ funds_left = channel_funds - sum(amounts)
# it would be good to not fill the full channel if possible
add_amount = min(funds_left, distribute_amount)
- config[c].append(add_amount)
- distribute_amount -= add_amount
+ if add_amount:
+ config.setdefault(c, []).append(add_amount)
+ distribute_amount -= add_amount
if distribute_amount == 0:
break
if config.total_config_amount() != amount_msat:
diff --git a/tests/test_mpp_split.py b/tests/test_mpp_split.py
index 5da50a5..629bfa8 100644
--- a/tests/test_mpp_split.py
+++ b/tests/test_mpp_split.py
@@ -31,9 +31,7 @@ class TestMppSplit(ElectrumTestCase):
splits = mpp_split.suggest_splits(1_000_000_000, self.channels_with_funds, exclude_single_part_payments=True)
self.assertEqual({
(b"0", b"0"): [671_020_676],
- (b"1", b"1"): [328_979_324],
- (b"2", b"0"): [],
- (b"3", b"2"): []},
+ (b"1", b"1"): [328_979_324]},
splits[0].config
)
Why this scored 48/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.