lnrouter: LiquidityHints: add note about thread-safety
What the developer wrote
Authored by SomberNight
lnrouter: LiquidityHints: add note about thread-safety
and maybe make reading the LiquidAmounts thread-safe(?) without actually locking
What changed, and why it matters
This commit adds a code comment warning that a performance-sensitive part of Electrum's Lightning payment routing reads shared data without a lock. It also makes a tiny code change to store a reference to an object before reading it, which slightly reduces the chance of a value changing between two consecutive reads. There is no actual fix for a race condition, and no evidence this is exploitable as a security vulnerability.
No immediate action required. Treat as minor hardening/documentation. If concerned, monitor for follow-up commits that add proper locking around pathfinding or make LiquidityHint reads atomic.
Security signals we found
Race condition acknowledged in new comment: concurrent update could happen while reading hints without lock
No actual synchronization introduced
Change reduces but does not eliminate TOCTOU window in property getters
Evidence from the diff
The patch modifies LiquidityHint getters to cache the LiquidAmount reference before calling get_valid_amount(), avoiding a second attribute lookup. The main change is a new comment in LiquidityHintMgr.get_penalty() explaining that self.lock is intentionally not acquired during pathfinding (~100k calls) and acknowledging that concurrent updates could still occur. This is a documentation/hardening micro-change, not a security patch.
Changed components
electrum/lnrouter.pyLiquidityHintLiquidityHintMgrInspect captured patch +13 / −5
diff --git a/electrum/lnrouter.py b/electrum/lnrouter.py
index 57fca3e..f0222eb 100644
--- a/electrum/lnrouter.py
+++ b/electrum/lnrouter.py
@@ -192,7 +192,8 @@ class LiquidityHint:
@property
def can_send_forward(self) -> Optional[int]:
- return self._can_send_forward.get_valid_amount() if self._can_send_forward else None
+ la = self._can_send_forward
+ return la.get_valid_amount() if la else None
@can_send_forward.setter
def can_send_forward(self, amount_msat: int) -> None:
@@ -208,7 +209,8 @@ class LiquidityHint:
@property
def can_send_backward(self) -> Optional[int]:
- return self._can_send_backward.get_valid_amount() if self._can_send_backward else None
+ la = self._can_send_backward
+ return la.get_valid_amount() if la else None
@can_send_backward.setter
def can_send_backward(self, amount_msat: int) -> None:
@@ -221,7 +223,8 @@ class LiquidityHint:
@property
def cannot_send_forward(self) -> Optional[int]:
- return self._cannot_send_forward.get_valid_amount() if self._cannot_send_forward else None
+ la = self._cannot_send_forward
+ return la.get_valid_amount() if la else None
@cannot_send_forward.setter
def cannot_send_forward(self, amount_msat: int) -> None:
@@ -239,7 +242,8 @@ class LiquidityHint:
@property
def cannot_send_backward(self) -> Optional[int]:
- return self._cannot_send_backward.get_valid_amount() if self._cannot_send_backward else None
+ la = self._cannot_send_backward
+ return la.get_valid_amount() if la else None
@cannot_send_backward.setter
def cannot_send_backward(self, amount_msat: int) -> None:
@@ -375,7 +379,11 @@ class LiquidityHintMgr:
was chosen such that the penalty will be able to compete with the regular
base and relative fees.
"""
- # we only evaluate hints here, so use dict get (to not create many hints with self.get_hint)
+ # note: self.lock is not taken for performance reasons, as we are called ~100k times per path-finding,
+ # and just acquiring+releasing locks that many times is expensive(?).
+ # (tho find_path_for_payment() could take LiquidityHintMgr.lock for the whole duration of the pathfinding)
+ # We only read the hints, so this should mostly be fine. Except a concurrent update could still happen...
+ # note: we only evaluate hints here, so use dict get (to not create many hints with self.get_hint)
hint = self._liquidity_hints.get(channel_id)
if not hint:
can_send, cannot_send, num_inflight_htlcs = None, None, 0
Why this scored 11/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.