From c37b844f664df3b59644a0d06f13200f3d16d5ac Mon Sep 17 00:00:00 2001 From: SomberNight Date: Thu, 15 Jan 2026 15:55:19 +0000 Subject: [PATCH] lnutil: change ReceivedMPPStatus.htlcs to frozenset, i.e. immutable As ThomasV says: > ReceivedMPPStatus is a Namedtuple, which is immutable, but it contains > a mutable field. Since ReceivedMPPStatus is not a StoredObject, > no patch will be created when the htlcs list is modified, and we may > end up not saving the change to disk if partial writes are enabled. patch taken from https://github.com/spesmilo/electrum/pull/10395#pullrequestreview-3634244541 closes https://github.com/spesmilo/electrum/pull/10395 Co-authored-by: f321x --- electrum/lnpeer.py | 23 ++++++++++++++--------- electrum/lnutil.py | 4 ++-- electrum/lnworker.py | 13 +++++++++---- electrum/util.py | 2 +- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 17b836d7f..ae525abca 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 ff33f3914..9512721c5 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 a255c0999..453de6da2 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 8e5a391a1..2ef8e2dfd 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()