tests: simplify MockLNWallet, add fixme for cyclic inconsistency
What the developer wrote
Authored by SomberNight
tests: simplify MockLNWallet, add fixme for cyclic inconsistency
What changed, and why it matters
This commit is a test-code cleanup and a small defensive guard in non-production code. It removes an unused mock helper class, simplifies a test wallet constructor, and adds a null check before cancelling an onion-message task group. There is no user-facing security fix here.
No security action required. Treat as routine test/maintenance commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff touches two files: electrum/onion_message.py and tests/test_lnpeer.py. In onion_message.py, stop() now checks if self.taskgroup: before calling cancel_remaining(), preventing an AttributeError if stop() is called before the taskgroup is initialized. In tests/test_lnpeer.py, the MockADB class is removed, the MockLNWallet constructor drops the optional ln_xprv parameter, and several explicit None assignments (lnwatcher, swap_manager, onion_message_manager, listen_server) are removed because the parent class now sets them. A commented-out assertion with a FIXME note about cyclic wallet/lnworker state is added. None of these changes patch a vulnerability in production code.
Changed components
electrum/onion_message.pytests/test_lnpeer.pyInspect captured patch +5 / −17
diff --git a/electrum/onion_message.py b/electrum/onion_message.py
index a6225f3..1fdb3bb 100644
--- a/electrum/onion_message.py
+++ b/electrum/onion_message.py
@@ -456,7 +456,8 @@ class OnionMessageManager(Logger):
self.logger.info("taskgroup stopped.")
async def stop(self) -> None:
- await self.taskgroup.cancel_remaining()
+ if self.taskgroup:
+ await self.taskgroup.cancel_remaining()
async def process_forward_queue(self) -> None:
while True:
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index d2d03b7..1e1c641 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -111,15 +111,6 @@ class MockBlockchain:
return False
-class MockADB:
- def __init__(self):
- self._blockchain = MockBlockchain()
- def add_transaction(self, tx):
- pass
- def get_local_height(self):
- return self._blockchain.height()
-
-
class MockLNGossip:
def get_sync_progress_estimate(self):
return None, None, None
@@ -159,7 +150,7 @@ class MockLNWallet(LNWallet):
TIMEOUT_SHUTDOWN_FAIL_PENDING_HTLCS = 0
MPP_SPLIT_PART_FRACTION = 1 # this disables the forced splitting
- def __init__(self, *, name, has_anchors, ln_xprv: str = None):
+ def __init__(self, *, name, has_anchors):
self.name = name
self._user_dir = tempfile.mkdtemp(prefix="electrum-lnpeer-test-")
@@ -173,6 +164,7 @@ class MockLNWallet(LNWallet):
"9dk", path=None, passphrase=name, config=self.config)['wallet'] # type: Abstract_Wallet
wallet.is_up_to_date = lambda: True
wallet.adb.network = wallet.network = network
+ #assert wallet.lnworker is None # FIXME xxxxx wallet already has another lnworker by now >.<
features = LnFeatures(0)
features |= LnFeatures.OPTION_DATA_LOSS_PROTECT_OPT
@@ -184,8 +176,7 @@ class MockLNWallet(LNWallet):
features |= LnFeatures.OPTION_SCID_ALIAS_OPT
features |= LnFeatures.OPTION_STATIC_REMOTEKEY_OPT
- if ln_xprv is None:
- ln_xprv = _bip32_from_name(name).to_xprv()
+ ln_xprv = _bip32_from_name(name).to_xprv()
LNWallet.__init__(self, wallet=wallet, xprv=ln_xprv, features=features)
self.lnpeermgr = MockLNPeerManager(
@@ -195,10 +186,6 @@ class MockLNWallet(LNWallet):
lnwallet=self,
network=network,
)
- self.lnwatcher = None
- self.swap_manager = None
- self.onion_message_manager = None
- self.listen_server = None
self.logger.info(f"created LNWallet[{name}] with nodeID={self.node_keypair.pubkey.hex()}")
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.