plugin: nwc: give NWCServer its own taskgroup
What the developer wrote
Authored by f321x
plugin: nwc: give NWCServer its own taskgroup
It seemed weird to pass the taskgroup of the plugin into
the NWCServer, this looks a bit cleaner. It also allows
to get multiple tasks of the NWCServer cancelled by cancelling
the NWCServers taskgroup.
What changed, and why it matters
This commit rewrites how background tasks are managed inside Electrum's Nostr Wallet Connect (NWC) plugin. Previously, the plugin and the NWC server shared one task group, which could accidentally cancel an in-flight Lightning payment when the server restarted its event handler. The change gives the NWC server its own temporary task group and shields active payment tasks from cancellation. It is best read as a reliability/robustness improvement that likely closes a small window where a payment could be interrupted, not as a fix for a clear-cut remote exploit.
Treat as a normal code-quality/reliability patch. Reviewers should verify that asyncio.shield does not leave pay_invoice tasks dangling after server stop, and that local taskgroup cancellation does not race with publish_notification_event. No urgent security deployment is indicated by the diff alone.
Security signals we found
Concurrency/lifecycle fix: prevents cancellation of in-flight pay_invoice during taskgroup restart
Adds guard clauses before taskgroup.spawn in publish_notification_event
Removes cross-component shared taskgroup between plugin and NWCServer
No explicit security disclosure, CVE, or researcher attribution in commit or references
Evidence from the diff
The patch refactors NWCServer task lifecycle: NWCServer no longer receives the plugin’s OldTaskGroup; instead it creates a local async-with OldTaskGroup() inside run(). publish_info_event and handle_requests are spawned into that local group. restart_event_handler now cancels the current local taskgroup rather than a single stored event_handler_task. pay_invoice is wrapped in asyncio.create_task and awaited via asyncio.shield so a taskgroup restart does not cancel an ongoing payment. publish_notification_event gains guards for missing taskgroup/manager. The change removes run_sync_function_on_asyncio_thread import and the event_handler_task field. This is a defensive concurrency fix; no direct memory-safety bug or cryptographic flaw is visible in the diff.
Changed components
electrum/plugins/nwc/nwcserver.pyNWCServer.run()NWCServer.restart_event_handler()NWCServer.pay_invoice() / pay_invoice handlerNWCServer.publish_notification_event()Inspect captured patch +16 / −12
diff --git a/electrum/plugins/nwc/nwcserver.py b/electrum/plugins/nwc/nwcserver.py
index 6e124d9..7ea1bb0 100644
--- a/electrum/plugins/nwc/nwcserver.py
+++ b/electrum/plugins/nwc/nwcserver.py
@@ -39,7 +39,7 @@ from electrum.plugin import BasePlugin, hook
from electrum.logging import Logger
from electrum.util import log_exceptions, ca_path, OldTaskGroup, get_asyncio_loop, InvoiceError, \
LightningHistoryItem, event_listener, EventListener, make_aiohttp_proxy_connector, \
- get_running_loop, run_sync_function_on_asyncio_thread
+ get_running_loop
from electrum.invoices import Invoice, Request, PR_UNKNOWN, PR_PAID, BaseInvoice, PR_INFLIGHT, PR_FAILED, PR_EXPIRED, PR_UNPAID
from electrum import constants
from electrum.lnutil import RECEIVED, PaymentFeeBudget
@@ -74,7 +74,7 @@ class NWCServerPlugin(BasePlugin):
storage = self.get_storage(wallet)
self.connections = storage.setdefault('connections', {})
self.delete_expired_connections()
- self.nwc_server = NWCServer(self.config, wallet, self.taskgroup, self.connections)
+ self.nwc_server = NWCServer(self.config, wallet, self.connections)
asyncio.run_coroutine_threadsafe(self.taskgroup.spawn(self.nwc_server.run()), get_asyncio_loop())
self.initialized = True
@@ -189,7 +189,6 @@ class NWCServer(Logger, EventListener):
self,
config: 'SimpleConfig',
wallet: 'Abstract_Wallet',
- taskgroup: 'OldTaskGroup',
connection_storage: dict,
):
Logger.__init__(self)
@@ -198,11 +197,9 @@ class NWCServer(Logger, EventListener):
self.connections = connection_storage # type: dict[str, dict] # client hex pubkey -> connection data
self.relays = config.NOSTR_RELAYS.split(",") or [] # type: List[str]
self.do_stop = False
- self.taskgroup = taskgroup # type: 'OldTaskGroup'
+ self.taskgroup = None # type: Optional[OldTaskGroup]
self.ssl_context = ssl.create_default_context(purpose=ssl.Purpose.SERVER_AUTH, cafile=ca_path)
self.manager = None # type: Optional[aionostr.Manager]
- # the task is stored so it can be cancelled when the connections change
- self.event_handler_task = None # type: Optional[asyncio.Task]
self.register_callbacks()
def get_relay_manager(self) -> aionostr.Manager:
@@ -239,9 +236,10 @@ class NWCServer(Logger, EventListener):
continue
try:
- await self.publish_info_event()
- self.event_handler_task = await self.taskgroup.spawn(self.handle_requests())
- await self.event_handler_task
+ async with OldTaskGroup() as tg:
+ self.taskgroup = tg
+ await tg.spawn(self.publish_info_event())
+ await tg.spawn(self.handle_requests())
except asyncio.CancelledError:
if self.do_stop:
return
@@ -252,6 +250,8 @@ class NWCServer(Logger, EventListener):
await self.manager.close()
self.manager = None
await asyncio.sleep(60)
+ finally:
+ self.taskgroup = None
async def refresh_manager(self) -> bool:
"""Checks if manager is still connected to relays, if not recreates it and reconnects"""
@@ -278,8 +278,8 @@ class NWCServer(Logger, EventListener):
def restart_event_handler(self) -> None:
"""To be called when the connections change so we restart with a new filter"""
- if self.event_handler_task:
- run_sync_function_on_asyncio_thread(self.event_handler_task.cancel, block=True)
+ if tg := self.taskgroup:
+ asyncio.run_coroutine_threadsafe(tg.cancel_remaining(), get_asyncio_loop())
@event_listener
def on_event_proxy_set(self, *args):
@@ -444,7 +444,9 @@ class NWCServer(Logger, EventListener):
"""
invoice: str = params.get('invoice', "")
amount_msat: Optional[int] = params.get('amount')
- response = await self.pay_invoice(invoice, amount_msat, request_event.pubkey)
+ pay_task = asyncio.create_task(self.pay_invoice(invoice, amount_msat, request_event.pubkey))
+ # prevent the payment attempt from getting canceled during taskgroup restart
+ response = await asyncio.shield(pay_task)
response['result_type'] = 'pay_invoice'
await self.send_encrypted_response(request_event.pubkey, json.dumps(response), request_event.id)
@@ -954,6 +956,8 @@ class NWCServer(Logger, EventListener):
"""
https://github.com/nostr-protocol/nips/blob/75f246ed987c23c99d77bfa6aeeb1afb669e23f7/47.md#notification-events
"""
+ if not self.taskgroup or not self.manager:
+ return
self.logger.debug(f"Publishing notification event: {content}")
for client_pubkey, connection in list(self.connections.items()):
coro = self.taskgroup.spawn(aionostr._add_event(
Why this scored 35/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.