ln: fix race when doing concurrent ln payments
What the developer wrote
Authored by f321x
ln: fix race when doing concurrent ln payments
There is a race when initiating multiple lightning payments concurrently
(e.g. when doing a reverse swap with prepayment + swap payment).
suggest_splits might overallocate
split amounts for a channel as the splitting of both invoice amounts runs
concurrently and before acutal htlcs that reduce the channels balance
have been added to the channel yet. This results in a "not
enough balance" PaymentFailure once we try to send the htlcs and the
other payment attempt already reduced the available balance of the
channel.
This fix takes a lock from splitting the amount until the htlcs are
put on the channel, so suggest_splits always acts on the correct channel
balance.
What changed, and why it matters
This commit fixes a timing bug in Electrum's Lightning payments. If a user started multiple Lightning payments at the same time, the wallet could plan routes using outdated channel balances, promising more money than was actually available. When the payments were then sent, one would fail with a 'not enough balance' error. The fix adds a lock so that balance checks and route creation happen one payment at a time, preventing the stale-balance problem.
No immediate security response required; this is a reliability/DoS-avoidance fix. Users running concurrent Lightning payments should update to the patched version to avoid spurious payment failures. Reviewers may want to confirm the lock is held for the minimal necessary duration and does not introduce deadlocks with other locks in the Lightning path.
Security signals we found
Race condition in concurrent Lightning payment handling
Potential payment failure due to stale channel balance reads
Addition of asyncio.Lock to serialize balance-sensitive split/route creation
Fix explicitly described as preventing 'not enough balance' PaymentFailure
Evidence from the diff
The patch addresses a race condition in LNWallet’s payment loop. When concurrent payments run, suggest_splits / create_routes_for_payment may read channel balances before in-flight HTLCs from another payment are committed, leading to over-allocation of a channel’s sending capacity. The fix wraps route creation and HTLC dispatch in an asyncio.Lock (_channel_sending_capacity_lock) so that subsequent splitting attempts observe the updated channel state. A corresponding lock is added to MockLNWallet in tests.
Changed components
electrum/lnworker.pytests/test_lnpeer.pyInspect captured patch +26 / −18
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
index 6b9b073..d3cf02e 100644
--- a/electrum/lnworker.py
+++ b/electrum/lnworker.py
@@ -914,6 +914,7 @@ class LNWallet(LNWorker):
self._paysessions = dict() # type: Dict[bytes, PaySession]
self.sent_htlcs_info = dict() # type: Dict[SentHtlcKey, SentHtlcInfo]
self.received_mpp_htlcs = self.db.get_dict('received_mpp_htlcs') # type: Dict[str, ReceivedMPPStatus] # payment_key -> ReceivedMPPStatus
+ self._channel_sending_capacity_lock = asyncio.Lock()
# detect inflight payments
self.inflight_payments = set() # (not persisted) keys of invoices that are in PR_INFLIGHT state
@@ -1698,27 +1699,33 @@ class LNWallet(LNWorker):
try:
while True:
if (amount_to_send := paysession.get_outstanding_amount_to_send()) > 0:
- # 1. create a set of routes for remaining amount.
- # note: path-finding runs in a separate thread so that we don't block the asyncio loop
- # graph updates might occur during the computation
remaining_fee_budget_msat = (budget.fee_msat * amount_to_send) // amount_to_pay
- routes = self.create_routes_for_payment(
- paysession=paysession,
- amount_msat=amount_to_send,
- full_path=full_path,
- fwd_trampoline_onion=fwd_trampoline_onion,
- channels=channels,
- budget=budget._replace(fee_msat=remaining_fee_budget_msat),
- )
- # 2. send htlcs
- async for sent_htlc_info, cltv_delta, trampoline_onion in routes:
- await self.pay_to_route(
+ # splitting the amount of the payment between our channels requires the correct
+ # available channel balance. to prevent concurrent splitting attempts from
+ # using stale channel balances for the split calculation a lock needs to be
+ # taken until the htlcs are added to the channel so the next splitting attempt
+ # acts on a correct channel balance.
+ async with self._channel_sending_capacity_lock:
+ # 1. create a set of routes for remaining amount.
+ # note: path-finding runs in a separate thread so that we don't block the asyncio loop
+ # graph updates might occur during the computation
+ routes = self.create_routes_for_payment(
paysession=paysession,
- sent_htlc_info=sent_htlc_info,
- min_final_cltv_delta=cltv_delta,
- trampoline_onion=trampoline_onion,
- fw_payment_key=fw_payment_key,
+ amount_msat=amount_to_send,
+ full_path=full_path,
+ fwd_trampoline_onion=fwd_trampoline_onion,
+ channels=channels,
+ budget=budget._replace(fee_msat=remaining_fee_budget_msat),
)
+ # 2. send htlcs
+ async for sent_htlc_info, cltv_delta, trampoline_onion in routes:
+ await self.pay_to_route(
+ paysession=paysession,
+ sent_htlc_info=sent_htlc_info,
+ min_final_cltv_delta=cltv_delta,
+ trampoline_onion=trampoline_onion,
+ fw_payment_key=fw_payment_key,
+ )
# invoice_status is triggered in self.set_invoice_status when it actually changes.
# It is also triggered here to update progress for a lightning payment in the GUI
# (e.g. attempt counter)
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 6859888..be70b08 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -218,6 +218,7 @@ class MockLNWallet(Logger, EventListener, NetworkRetryManager[LNPeerAddr]):
self._payment_bundles_pkey_to_canon = {} # type: Dict[bytes, bytes]
self._payment_bundles_canon_to_pkeylist = {} # type: Dict[bytes, Sequence[bytes]]
self.config.INITIAL_TRAMPOLINE_FEE_LEVEL = 0
+ self._channel_sending_capacity_lock = asyncio.Lock()
self.logger.info(f"created LNWallet[{name}] with nodeID={local_keypair.pubkey.hex()}")
Why this scored 37/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.