lnpeer: don't signal OPTION_ZEROCONF_OPT to untrusted peer
What the developer wrote
Authored by f321x
lnpeer: don't signal OPTION_ZEROCONF_OPT to untrusted peer
Only signal `OPTION_ZEROCONF_OPT` to peers if we either:
1. Have no trusted peer configured (assuming that we are LSP)
2. Have a trusted peer configured, and the peer we are connecting
to is this trusted peer.
Otherwise peers that are LSPs but are not the clients trusted LSP
might try to open a channel to the client but it would get rejected.
What changed, and why it matters
This commit changes how Electrum advertises a Lightning feature called 'zeroconf' (zero-confirmation channels). Previously, a user's wallet could tell any Lightning peer that it supports zeroconf channels, even untrusted ones. That could mislead a non-trusted Lightning Service Provider (LSP) into trying to open a special channel that the wallet would then reject, causing confusion, failed channel opens, and possibly minor privacy or denial-of-service issues. The fix makes the wallet only advertise zeroconf support to a specifically trusted LSP, or when the wallet itself acts as an LSP with no trusted peer configured.
Reviewers should confirm that `lnworker.network.lngossip` is the only non-wallet worker that could create a `Peer`, and that no other code path bypasses this feature-bit masking. Consider whether the feature bit should also be masked when `OPEN_ZEROCONF_CHANNELS` is disabled, since the current logic relies on the caller's `features` already lacking the bit. The added tests are a good start; running them should be part of CI.
Security signals we found
Feature-bit advertisement now restricted by trust relationship
Untrusted peers no longer receive OPTION_ZEROCONF_OPT
New unit tests cover trusted/untrusted/invalid/no-trusted configurations
Refactoring centralizes trusted node id parsing in a property
Evidence from the diff
The patch removes the OPTION_ZEROCONF_OPT feature bit from a Peer’s local features in two cases: (1) the worker is the gossip worker (lnworker == lnworker.network.lngossip), and (2) a trusted zeroconf node is configured but the remote pubkey is not that trusted node. It also refactors LNWallet.can_get_zeroconf_channel() to use a new trusted_zeroconf_node_id property that parses and caches the configured trusted node id. Tests are added to verify the feature bit is advertised only in the intended scenarios. The change is defensive and prevents untrusted peers from believing the local node accepts zeroconf channels.
Changed components
electrum/lnpeer.pyelectrum/lnworker.pytests/test_lnpeer.pyInspect captured patch +67 / −6
diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
index 5375762..bf8ec00 100644
--- a/electrum/lnpeer.py
+++ b/electrum/lnpeer.py
@@ -101,6 +101,11 @@ class Peer(Logger, EventListener):
self.pubkey = pubkey # remote pubkey
self.privkey = self.transport.privkey # local privkey
self.features = self.lnworker.features # type: LnFeatures
+ if lnworker == lnworker.network.lngossip or \
+ lnworker.config.ZEROCONF_TRUSTED_NODE and pubkey != lnworker.trusted_zeroconf_node_id:
+ # don't signal zeroconf support if we are client (a trusted node is configured),
+ # and Peer is not our trusted node
+ self.features &= ~LnFeatures.OPTION_ZEROCONF_OPT
self.their_features = LnFeatures(0) # type: LnFeatures
self.node_ids = [self.pubkey, privkey_to_pubkey(self.privkey)]
assert self.node_ids[0] != self.node_ids[1]
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
index 7fb38d2..f0435d1 100644
--- a/electrum/lnworker.py
+++ b/electrum/lnworker.py
@@ -3399,17 +3399,24 @@ class LNWallet(Logger):
return False
def can_get_zeroconf_channel(self) -> bool:
- if not self.config.OPEN_ZEROCONF_CHANNELS and self.config.ZEROCONF_TRUSTED_NODE:
- # check if zeroconf is accepted and client has trusted zeroconf node configured
+ if not self.config.OPEN_ZEROCONF_CHANNELS:
return False
- try:
- node_id = extract_nodeid(self.config.ZEROCONF_TRUSTED_NODE)[0]
- except ConnStringFormatError:
- # invalid connection string
+ node_id = self.trusted_zeroconf_node_id
+ if not node_id:
return False
# only return True if we are connected to the zeroconf provider
return self.lnpeermgr.get_peer_by_pubkey(node_id) is not None
+ @property
+ def trusted_zeroconf_node_id(self) -> Optional[bytes]:
+ if not self.config.ZEROCONF_TRUSTED_NODE:
+ return None
+ try:
+ return extract_nodeid(self.config.ZEROCONF_TRUSTED_NODE)[0]
+ except ConnStringFormatError:
+ self.logger.warning(f"invalid zeroconf node connection string configured")
+ return None
+
def _suggest_channels_for_rebalance(self, direction, amount_sat) -> Sequence[Tuple[Channel, int]]:
"""
Suggest a channel and amount to send/receive with that channel, so that we will be able to receive/send amount_sat
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 8669931..7af6087 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -634,6 +634,55 @@ class TestPeerUtils(TestPeer):
with self.assertRaises(InvalidGossipMsg):
ChannelDB.verify_channel_update(payload, start_node=alice_bob_peer.pubkey)
+ async def test_zeroconf_feature_bit(self):
+ workers = self.prepare_lnwallets(self.GRAPH_DEFINITIONS['single_chan'])
+
+ with self.subTest(msg="zeroconf is disabled in Alice LNWallet, so peers shouldn't signal it either"):
+ graph = self.prepare_chans_and_peers_in_graph(
+ self.GRAPH_DEFINITIONS['single_chan'],
+ workers=workers,
+ )
+ alice, _ = graph.peers.values()
+ self.assertFalse(alice.features.supports(LnFeatures.OPTION_ZEROCONF_OPT))
+
+ # enable zeroconf in alice LNWallet
+ workers['alice'].features |= LnFeatures.OPTION_ZEROCONF_OPT
+
+ with self.subTest(msg="no trusted zeroconf node, zeroconf should be signaled in new peers"):
+ graph = self.prepare_chans_and_peers_in_graph(
+ self.GRAPH_DEFINITIONS['single_chan'],
+ workers=workers,
+ )
+ alice, _ = graph.peers.values() # alice is LSP
+ self.assertTrue(alice.features.supports(LnFeatures.OPTION_ZEROCONF_OPT))
+
+ with self.subTest(msg="trusted node is configured, but it is not bob"):
+ workers['alice'].config.ZEROCONF_TRUSTED_NODE = f"{os.urandom(33).hex()}@1.1.1.1:9735"
+ graph = self.prepare_chans_and_peers_in_graph(
+ self.GRAPH_DEFINITIONS['single_chan'],
+ workers=workers,
+ )
+ alice, _ = graph.peers.values() # alice is client
+ self.assertFalse(alice.features.supports(LnFeatures.OPTION_ZEROCONF_OPT))
+
+ with self.subTest(msg="trusted node is configured, but it is invalid"):
+ workers['alice'].config.ZEROCONF_TRUSTED_NODE = f"{os.urandom(8).hex()}@1.1.1.1:9735"
+ graph = self.prepare_chans_and_peers_in_graph(
+ self.GRAPH_DEFINITIONS['single_chan'],
+ workers=workers,
+ )
+ alice, _ = graph.peers.values() # alice is client
+ self.assertFalse(alice.features.supports(LnFeatures.OPTION_ZEROCONF_OPT))
+
+ with self.subTest(msg="Alice uses Bob as her trusted LSP"):
+ workers['alice'].config.ZEROCONF_TRUSTED_NODE = workers['bob'].node_keypair.pubkey.hex()
+ graph = self.prepare_chans_and_peers_in_graph(
+ self.GRAPH_DEFINITIONS['single_chan'],
+ workers=workers,
+ )
+ alice, _ = graph.peers.values()
+ self.assertTrue(alice.features.supports(LnFeatures.OPTION_ZEROCONF_OPT))
+
class TestPeerDirect(TestPeer):
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.