lnutil: change ReceivedMPPStatus.htlcs to frozenset, i.e. immutable
What changed, and why it matters
This commit fixes a data-integrity bug in Electrum's Lightning payment handling. A particular data structure was supposed to be immutable, but it secretly contained a mutable list. When the list changed, Electrum's storage system did not notice, so the change might not be saved to disk—especially when partial writes are enabled. That could leave the wallet's record of incoming Lightning payments inconsistent after a restart or crash, potentially causing funds to appear stuck or payments to be mishandled. The fix makes the list truly immutable and updates the code to create new copies whenever it changes, so the storage layer reliably saves the state.
Apply the patch. It is a defensive correctness fix for Lightning state persistence. Users running Lightning nodes, especially with partial writes enabled, should upgrade. No immediate emergency response is indicated, but verify backups and wallet state after updating.
Security signals we found
Mutable field inside an immutable NamedTuple bypasses storage change detection
Potential state inconsistency for received multi-path Lightning payments after crash or restart
Partial-write mode increases risk of unsaved mutations
Type mismatch fix: payment_hash.hex() used instead of raw bytes for set membership check
Patch converts all in-place set mutations to immutable frozenset replacements
Evidence from the diff
ReceivedMPPStatus is a NamedTuple and therefore immutable at the top level, but its htlcs field was a mutable set. Because it is not a StoredObject, mutating htlcs in place did not trigger a patch/write to the wallet database, risking state loss with partial writes enabled. The patch changes the field type to frozenset and replaces all in-place mutations (add, remove, update, clear) with _replace(…) calls that assign a new frozenset back into self.lnworker.received_mpp_htlcs, ensuring the storage layer detects the change. A related bug is fixed where payment_hash (bytes) was compared against dont_settle_htlcs, which apparently stores hex strings, by adding .hex(). JSON encoding is also updated to serialize frozenset.
Changed components
electrum/lnutil.pyelectrum/lnpeer.pyelectrum/lnworker.pyelectrum/util.pyInspect captured patch +26 / −16
diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
index 17b836d..ae525ab 100644
--- a/electrum/lnpeer.py
+++ b/electrum/lnpeer.py
@@ -2200,7 +2200,7 @@ class Peer(Logger, EventListener):
# get payment hash of any htlc in the set (they are all the same)
payment_hash = htlc_set.get_payment_hash()
assert payment_hash is not None, htlc_set
- assert payment_hash not in self.lnworker.dont_settle_htlcs
+ assert payment_hash.hex() not in self.lnworker.dont_settle_htlcs
self.lnworker.dont_expire_htlcs.pop(payment_hash.hex(), None) # htlcs wont get expired anymore
for mpp_htlc in list(htlc_set.htlcs):
htlc_id = mpp_htlc.htlc.htlc_id
@@ -2214,10 +2214,12 @@ class Peer(Logger, EventListener):
if chan.hm.was_htlc_preimage_released(htlc_id=htlc_id, htlc_proposer=REMOTE):
# this check is intended to gracefully handle stale htlcs in the set, e.g. after a crash
self.logger.debug(f"{mpp_htlc=} was already settled before, dropping it.")
- htlc_set.htlcs.remove(mpp_htlc)
+ htlc_set = htlc_set._replace(htlcs=htlc_set.htlcs - {mpp_htlc})
+ self.lnworker.received_mpp_htlcs[payment_key] = htlc_set
continue
self._fulfill_htlc(chan, htlc_id, preimage)
- htlc_set.htlcs.remove(mpp_htlc)
+ htlc_set = htlc_set._replace(htlcs=htlc_set.htlcs - {mpp_htlc})
+ self.lnworker.received_mpp_htlcs[payment_key] = htlc_set
# reset just-in-time opening fee of channel
chan.jit_opening_fee = None
@@ -2255,7 +2257,8 @@ class Peer(Logger, EventListener):
if chan.hm.was_htlc_failed(htlc_id=htlc_id, htlc_proposer=REMOTE):
# this check is intended to gracefully handle stale htlcs in the set, e.g. after a crash
self.logger.debug(f"{mpp_htlc=} was already failed before, dropping it.")
- htlc_set.htlcs.remove(mpp_htlc)
+ htlc_set = htlc_set._replace(htlcs=htlc_set.htlcs - {mpp_htlc})
+ self.lnworker.received_mpp_htlcs[payment_key] = htlc_set
continue
onion_packet = self._parse_onion_packet(mpp_htlc.unprocessed_onion)
processed_onion_packet = self._process_incoming_onion_packet(
@@ -2286,7 +2289,8 @@ class Peer(Logger, EventListener):
htlc_id=htlc_id,
error_bytes=error_bytes,
)
- htlc_set.htlcs.remove(mpp_htlc)
+ htlc_set = htlc_set._replace(htlcs=htlc_set.htlcs - {mpp_htlc})
+ self.lnworker.received_mpp_htlcs[payment_key] = htlc_set
def fail_htlc(self, *, chan: Channel, htlc_id: int, error_bytes: bytes):
self.logger.info(f"fail_htlc. chan {chan.short_channel_id}. htlc_id {htlc_id}.")
@@ -3148,11 +3152,12 @@ class Peer(Logger, EventListener):
if not parent:
parent = ReceivedMPPStatus(
resolution=RecvMPPResolution.WAITING,
- htlcs=set(),
+ htlcs=frozenset(),
)
- self.lnworker.received_mpp_htlcs[mpp_set.parent_set_key] = parent
- parent.htlcs.update(mpp_set.htlcs)
- mpp_set.htlcs.clear()
+ self.lnworker.received_mpp_htlcs[mpp_set.parent_set_key] = parent._replace(
+ htlcs=parent.htlcs | mpp_set.htlcs
+ )
+ self.lnworker.received_mpp_htlcs[payment_key] = mpp_set._replace(htlcs=frozenset())
return None, None, None # this set will get deleted as there are no htlcs in it anymore
assert not mpp_set.parent_set_key
diff --git a/electrum/lnutil.py b/electrum/lnutil.py
index ff33f39..9512721 100644
--- a/electrum/lnutil.py
+++ b/electrum/lnutil.py
@@ -1980,7 +1980,7 @@ class ReceivedMPPHtlc(NamedTuple):
class ReceivedMPPStatus(NamedTuple):
resolution: RecvMPPResolution
- htlcs: set[ReceivedMPPHtlc]
+ htlcs: frozenset[ReceivedMPPHtlc]
# parent_set_key is needed as trampoline allows MPP to be nested, the parent_set_key is the
# payment key of the final mpp set (derived from inner trampoline onion payment secret)
# to which the separate trampoline sets htlcs get added once they are complete.
@@ -2005,7 +2005,7 @@ class ReceivedMPPStatus(NamedTuple):
@stored_in('received_mpp_htlcs', tuple)
def from_tuple(resolution, htlc_list, parent_set_key=None) -> 'ReceivedMPPStatus':
assert isinstance(resolution, int)
- htlc_set = set(ReceivedMPPHtlc.from_tuple(*htlc_data) for htlc_data in htlc_list)
+ htlc_set = frozenset(ReceivedMPPHtlc.from_tuple(*htlc_data) for htlc_data in htlc_list)
return ReceivedMPPStatus(
resolution=RecvMPPResolution(resolution),
htlcs=htlc_set,
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
index a255c09..453de6d 100644
--- a/electrum/lnworker.py
+++ b/electrum/lnworker.py
@@ -2807,7 +2807,7 @@ class LNWallet(Logger):
self.logger.debug(f"creating new mpp set for {payment_key=}")
mpp_status = ReceivedMPPStatus(
resolution=RecvMPPResolution.WAITING,
- htlcs=set(),
+ htlcs=frozenset(),
)
if mpp_status.resolution > RecvMPPResolution.WAITING:
@@ -2827,8 +2827,9 @@ class LNWallet(Logger):
)
assert new_htlc not in mpp_status.htlcs, "each htlc should make it here only once?"
assert isinstance(unprocessed_onion_packet, str)
- mpp_status.htlcs.add(new_htlc) # side-effecting htlc_set
- self.received_mpp_htlcs[payment_key] = mpp_status
+ new_htlcs = set(mpp_status.htlcs)
+ new_htlcs.add(new_htlc)
+ self.received_mpp_htlcs[payment_key] = mpp_status._replace(htlcs=frozenset(new_htlcs))
def set_mpp_resolution(self, payment_key: str, new_resolution: RecvMPPResolution) -> ReceivedMPPStatus:
mpp_status = self.received_mpp_htlcs[payment_key]
@@ -2910,10 +2911,14 @@ class LNWallet(Logger):
assert chan._state == ChannelState.REDEEMED
for payment_key_hex, mpp_status in list(self.received_mpp_htlcs.items()):
htlcs_to_remove = [htlc for htlc in mpp_status.htlcs if htlc.channel_id == chan.channel_id]
+ new_htlcs = set(mpp_status.htlcs)
for stale_mpp_htlc in htlcs_to_remove:
assert mpp_status.resolution != RecvMPPResolution.WAITING
self.logger.info(f'maybe_cleanup_mpp: removing htlc of MPP {payment_key_hex}')
- mpp_status.htlcs.remove(stale_mpp_htlc) # side-effecting htlc_set
+ new_htlcs.remove(stale_mpp_htlc)
+ if htlcs_to_remove:
+ mpp_status = mpp_status._replace(htlcs=frozenset(new_htlcs))
+ self.received_mpp_htlcs[payment_key_hex] = mpp_status # save changes to db
if len(mpp_status.htlcs) == 0:
self.logger.info(f'maybe_cleanup_mpp: removing mpp {payment_key_hex}')
del self.received_mpp_htlcs[payment_key_hex]
diff --git a/electrum/util.py b/electrum/util.py
index 8e5a391..2ef8e2d 100644
--- a/electrum/util.py
+++ b/electrum/util.py
@@ -331,7 +331,7 @@ class MyEncoder(json.JSONEncoder):
if isinstance(obj, datetime):
# note: if there is a timezone specified, this will include the offset
return obj.isoformat(' ', timespec="minutes")
- if isinstance(obj, set):
+ if isinstance(obj, (set, frozenset)):
return list(obj)
if isinstance(obj, bytes): # for nametuples in lnchannel
return obj.hex()
Why this scored 55/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.