← Watch feed
create_channel_storage: return dict instead of StoredDict
Public commit record
65/100 · Adequate
What the developer wrote
Authored by ThomasV
create_channel_storage: return dict instead of StoredDict
We should avoid dangling StoredDict; this does not make sense
if the DB is not in memory.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides an explanatory body
What changed, and why it matters
This commit changes how Electrum stores newly-created Lightning channel data during the brief setup phase. Previously, a 'StoredDict' object was created before the channel was fully committed to the wallet database, which could leave a 'dangling' reference and cause problems when the database is not fully loaded in memory. The fix makes the setup use a plain Python dictionary first, then converts it to a proper StoredDict only when the channel is officially added to the wallet. The commit message itself frames this as a robustness improvement, not a security fix, and no exploit is demonstrated.
Recommended action
Review as a defensive hardening/robustness change. No immediate security patch urgency is indicated by the commit itself. If this commit is part of a release, include it normally. If there is an external security advisory claiming this fixes a vulnerability, verify it against the actual diff and vendor disclosure before assigning a CVE or treating it as a security fix.
Security signals we found
01
Structural change to object lifecycle and storage ownership
02
Commit message explicitly states non-security motivation: avoid dangling StoredDict when DB is not in memory
03
No explicit security claim, CVE, advisory, or exploit in commit or supplied references
04
Change touches Lightning channel state initialization and locking, which are security-sensitive areas
Evidence from the diff
The patch refactors channel creation in Electrum’s Lightning implementation. create_channel_storage now returns a plain dict instead of a StoredDict. lnpeer.py constructs a temporary Channel (temp_chan) backed by that dict, performs commitment signing and zeroconf handling, then calls lnworker.add_new_channel(temp_chan). add_new_channel now copies the dict into the wallet’s channels StoredDict, deletes the temporary Channel, and reconstructs a new Channel backed by the StoredDict. lnchannel.py and lnhtlc.py are updated to use a threading.RLock when storage is a plain dict, avoiding reliance on StoredDict.lock. This prevents a ‘dangling StoredDict’ scenario when the DB is not in memory. The change is structural/robustness; no specific vulnerability or exploit is described in the commit or supplied references.
Changed components
electrum/lnchannel.pyelectrum/lnhtlc.pyelectrum/lnpeer.pyelectrum/lnworker.pytests/lnhelpers.pyInspect captured patch +49 / −32
diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py
index ce9085c..10789af 100644
--- a/electrum/lnchannel.py
+++ b/electrum/lnchannel.py
@@ -26,6 +26,7 @@ from typing import (
Iterable, Sequence, TYPE_CHECKING, Iterator, Union, Mapping)
from abc import ABC, abstractmethod
import itertools
+import threading
from aiorpcx import NetAddress
@@ -790,7 +791,7 @@ class Channel(AbstractChannel):
Logger.__init__(self) # should be after short_channel_id is set
self.lnworker = lnworker
self.storage = state
- self.db_lock = self.storage.lock
+ self.db_lock = threading.RLock() if type(self.storage) is dict else self.storage.lock
self.config = {}
self.config[LOCAL] = state["local_config"]
self.config[REMOTE] = state["remote_config"]
@@ -799,7 +800,7 @@ class Channel(AbstractChannel):
self.node_id = bfh(state["node_id"])
self.onion_keys = state['onion_keys'] # type: Dict[int, bytes]
self.data_loss_protect_remote_pcp = state['data_loss_protect_remote_pcp']
- self.hm = HTLCManager(log=state['log'], initiator = LOCAL if self.constraints.is_initiator else REMOTE, initial_feerate=initial_feerate)
+ self.hm = HTLCManager(log=state['log'], initiator = LOCAL if self.constraints.is_initiator else REMOTE, initial_feerate=initial_feerate, lock=self.db_lock)
self.unfulfilled_htlcs = state["unfulfilled_htlcs"] # type: Dict[int, Optional[str]]
# ^ htlc_id -> onion_packet_hex
self._state = ChannelState[state['state']]
diff --git a/electrum/lnhtlc.py b/electrum/lnhtlc.py
index 9901efe..1d36442 100644
--- a/electrum/lnhtlc.py
+++ b/electrum/lnhtlc.py
@@ -1,5 +1,6 @@
from copy import deepcopy
from typing import Sequence, Tuple, Dict, TYPE_CHECKING, Set
+import threading
from .lnutil import SENT, RECEIVED, LOCAL, REMOTE, HTLCOwner, UpdateAddHtlc, Direction, FeeUpdate
from .util import bfh, with_lock
@@ -21,7 +22,7 @@ LOG_TEMPLATE = {
class HTLCManager:
- def __init__(self, log: 'StoredDict', *, initiator=None, initial_feerate=None):
+ def __init__(self, log: 'StoredDict', *, initiator=None, initial_feerate=None, lock=None):
if len(log) == 0:
# note: "htlc_id" keys in dict are str! but due to json_db magic they can *almost* be treated as int...
@@ -41,7 +42,7 @@ class HTLCManager:
# lnchannel sometimes calls us with Channel.db_lock (== log.lock) already taken,
# and we ourselves often take log.lock (via StoredDict.__getitem__).
# Hence, to avoid deadlocks, we reuse this same lock.
- self.lock = log.lock
+ self.lock = lock if lock else threading.RLock()
self._init_maybe_active_htlc_ids()
diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
index 67f89f7..a85d3a2 100644
--- a/electrum/lnpeer.py
+++ b/electrum/lnpeer.py
@@ -52,7 +52,6 @@ from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, ChannelConf
from .lntransport import LNTransport, LNTransportBase, LightningPeerConnectionClosed, HandshakeFailed
from .lnmsg import encode_msg, decode_msg, UnknownOptionalMsgType, FailedToParseMsg
from .interface import GracefulDisconnect
-from .json_db import StoredDict
from .invoices import PR_PAID
from .fee_policy import FEE_LN_ETA_TARGET, FEERATE_PER_KW_MIN_RELAY_LIGHTNING
from .channel_db import FLAG_DIRECTION
@@ -1180,18 +1179,19 @@ class Peer(Logger, EventListener):
)
storage = self.create_channel_storage(
channel_id, outpoint, local_config, remote_config, constraints, our_channel_type)
- chan = Channel(
+ # temporary channel object, not stored (storage is a dict)
+ temp_chan = Channel(
storage,
lnworker=self.lnworker,
initial_feerate=feerate
)
- chan.storage['funding_inputs'] = [txin.prevout.to_json() for txin in funding_tx.inputs()]
- chan.storage['has_onchain_backup'] = has_onchain_backup
- chan.storage['init_height'] = self.lnworker.network.get_local_height()
- chan.storage['init_timestamp'] = int(time.time())
+ temp_chan.storage['funding_inputs'] = [txin.prevout.to_json() for txin in funding_tx.inputs()]
+ temp_chan.storage['has_onchain_backup'] = has_onchain_backup
+ temp_chan.storage['init_height'] = self.lnworker.network.get_local_height()
+ temp_chan.storage['init_timestamp'] = int(time.time())
if isinstance(self.transport, LNTransport):
- chan.add_or_update_peer_addr(self.transport.peer_addr)
- sig_64, _ = chan.sign_next_commitment()
+ temp_chan.add_or_update_peer_addr(self.transport.peer_addr)
+ sig_64, _ = temp_chan.sign_next_commitment()
self.temp_id_to_id[temp_channel_id] = channel_id
self.send_message("funding_created",
@@ -1206,15 +1206,15 @@ class Peer(Logger, EventListener):
self.logger.info('received funding_signed')
remote_sig = payload['signature']
try:
- chan.receive_new_commitment(remote_sig, [])
+ temp_chan.receive_new_commitment(remote_sig, [])
except LNProtocolWarning as e:
self.send_warning(channel_id, message=str(e), close_connection=True)
- chan.open_with_first_pcp(remote_per_commitment_point, remote_sig)
- chan.set_state(ChannelState.OPENING)
+ temp_chan.open_with_first_pcp(remote_per_commitment_point, remote_sig)
+ temp_chan.set_state(ChannelState.OPENING)
+ chan = self.lnworker.add_new_channel(temp_chan)
if zeroconf:
chan.set_state(ChannelState.FUNDED)
self.send_channel_ready(chan)
- self.lnworker.add_new_channel(chan)
return chan, funding_tx
def create_channel_storage(self, channel_id, outpoint, local_config, remote_config, constraints, channel_type):
@@ -1235,7 +1235,7 @@ class Peer(Logger, EventListener):
"revocation_store": {},
"channel_type": channel_type,
}
- return StoredDict(chan_dict, self.lnworker.db)
+ return chan_dict
@non_blocking_msg_handler
async def on_open_channel(self, payload):
@@ -1407,37 +1407,38 @@ class Peer(Logger, EventListener):
outpoint = Outpoint(funding_txid, funding_idx)
chan_dict = self.create_channel_storage(
channel_id, outpoint, local_config, remote_config, constraints, channel_type)
- chan = Channel(
+ # temporary channel object, not stored (storage is a dict)
+ temp_chan = Channel(
chan_dict,
lnworker=self.lnworker,
initial_feerate=feerate,
- jit_opening_fee = channel_opening_fee,
+ jit_opening_fee=channel_opening_fee,
)
- chan.storage['init_height'] = self.lnworker.network.get_local_height()
- chan.storage['init_timestamp'] = int(time.time())
+ temp_chan.storage['init_height'] = self.lnworker.network.get_local_height()
+ temp_chan.storage['init_timestamp'] = int(time.time())
if isinstance(self.transport, LNTransport):
- chan.add_or_update_peer_addr(self.transport.peer_addr)
+ temp_chan.add_or_update_peer_addr(self.transport.peer_addr)
remote_sig = funding_created['signature']
try:
- chan.receive_new_commitment(remote_sig, [])
+ temp_chan.receive_new_commitment(remote_sig, [])
except LNProtocolWarning as e:
self.send_warning(channel_id, message=str(e), close_connection=True)
- sig_64, _ = chan.sign_next_commitment()
+ sig_64, _ = temp_chan.sign_next_commitment()
self.send_message('funding_signed',
channel_id=channel_id,
signature=sig_64,
)
self.temp_id_to_id[temp_chan_id] = channel_id
- self.funding_signed_sent.add(chan.channel_id)
- chan.open_with_first_pcp(payload['first_per_commitment_point'], remote_sig)
- chan.set_state(ChannelState.OPENING)
+ self.funding_signed_sent.add(temp_chan.channel_id)
+ temp_chan.open_with_first_pcp(payload['first_per_commitment_point'], remote_sig)
+ temp_chan.set_state(ChannelState.OPENING)
+ chan = self.lnworker.add_new_channel(temp_chan)
if is_zeroconf:
# FIXME shouldn't we wait until funding_tx is at least in the mempool?!
# We haven't even validated funding_tx really contains the multisig funding output!
# This is unsafe. MUST be reworked before mainnet usage.
chan.set_state(ChannelState.FUNDED)
self.send_channel_ready(chan)
- self.lnworker.add_new_channel(chan)
def _cleanup_temp_channelids(self) -> None:
self.temp_id_to_id = {
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
index 777c2e2..d9d0429 100644
--- a/electrum/lnworker.py
+++ b/electrum/lnworker.py
@@ -1662,9 +1662,22 @@ class LNWallet(Logger):
self.lnwatcher.add_channel(chan)
def add_new_channel(self, chan: Channel):
- self.add_channel(chan)
+ # delete the old channel object, becauses it uses a dict
+ assert type(chan.storage) is dict
+ channel_id = chan.channel_id.hex()
channels_db = self.db.get_dict('channels')
- channels_db[chan.channel_id.hex()] = chan.storage
+ channels_db[channel_id] = chan.storage
+ jit_opening_fee = chan.jit_opening_fee
+ peer_state = chan.peer_state
+ del chan
+ storage = channels_db[channel_id] # StoredDict
+ chan = Channel(
+ storage,
+ lnworker=self,
+ jit_opening_fee=jit_opening_fee,
+ )
+ chan.peer_state = peer_state
+ self.add_channel(chan)
self.wallet.set_reserved_addresses_for_chan(chan, reserved=True)
try:
self.save_channel(chan)
@@ -1672,6 +1685,8 @@ class LNWallet(Logger):
chan.set_state(ChannelState.REDEEMED)
self.remove_channel(chan.channel_id)
raise
+ # return new channel object
+ return chan
def make_local_config_for_new_channel(
self,
diff --git a/tests/lnhelpers.py b/tests/lnhelpers.py
index d5207a3..a36f933 100644
--- a/tests/lnhelpers.py
+++ b/tests/lnhelpers.py
@@ -22,7 +22,6 @@ from electrum.lnrouter import LNPathFinder
from electrum.channel_db import ChannelDB
from electrum.lnworker import LNWallet, PaySession
from electrum.simple_config import SimpleConfig
-from electrum.stored_dict import StoredDict
from electrum.fee_policy import FeeTimeEstimates, FEE_ETA_TARGETS
from electrum.wallet import Standard_Wallet
@@ -392,7 +391,7 @@ def _create_channel_state(
'revocation_store': {},
'channel_type': channel_type,
}
- return StoredDict(state, None)
+ return state
def create_test_channels(
Risk score
Our methodology →Why this scored 34/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.