What the developer wrote
Authored by f321x
plugin: nwc: improved budget accounting
Increase the payment budget before attempting the payment
and decrease again if the payment fails. This prevents a race
where multiple concurrent payments could pass the budget check
before the budget is incremented through any of the other payments.
A lock around the budget is not suitable either as then one long
stuck payment (hold invoice) would render the budget inaccessible for
all other payment attempts.
What changed, and why it matters
This commit fixes a race condition in Electrum's NWC (Nostr Wallet Connect) plugin that could let a user spend more than their daily budget. Previously, the plugin checked the budget before each payment but only recorded the spend after a payment succeeded. If several payments were started at the same time, they could all pass the budget check and collectively exceed the limit. The fix records the spend before attempting the payment and refunds it only if the payment fails. The commit message explicitly describes this as preventing a race condition.
Treat this as a security fix for the NWC plugin and include it in the next release. Users relying on NWC daily spending limits should upgrade. Review whether other asynchronous spend paths (e.g., other plugins, API endpoints) use similar check-then-record budget patterns and apply the same debit-first/refund-on-fail pattern if needed.
Security signals we found
Race condition in budget enforcement (check-then-act)
TOCTOU between budget check and budget recording
Concurrent payments could exceed configured daily spending limit
Fix moves budget debit before payment attempt and refunds on failure
Commit message explicitly describes the race and its security/financial consequence
Evidence from the diff
In electrum/plugins/nwc/nwcserver.py, the pay_invoice flow previously called budget_allows_spend(), then awaited pay_invoice(), and only called add_to_budget() on success. This created a TOCTOU-style race: concurrent payment requests could each observe an unchanged budget and all be approved, then all succeed and push total spend over the configured daily limit. The patch moves add_to_budget() before the async payment attempt, captures the returned budget_item, and adds a finally block that calls remove_from_budget() only when success is False. add_to_budget now returns the appended [amount_msat, timestamp] list so it can be removed by identity. A lock was explicitly rejected by the authors because a stuck hold invoice would block all other payments. The commit message frames the change as fixing a race that could allow budget over-spending.
Changed components
electrum/plugins/nwc/nwcserver.pyNWCServer.pay_invoiceNWCServer.add_to_budgetNWCServer.remove_from_budgetInspect captured patch +22 / −7
diff --git a/electrum/plugins/nwc/nwcserver.py b/electrum/plugins/nwc/nwcserver.py
index 48e0245..0894bb8 100644
--- a/electrum/plugins/nwc/nwcserver.py
+++ b/electrum/plugins/nwc/nwcserver.py
@@ -813,8 +813,10 @@ class NWCServer(Logger, EventListener):
if not self.budget_allows_spend(request_pub, msat_requested=amount_msat or invoice.get_amount_msat()):
return self.get_error_response("QUOTA_EXCEEDED", "Payment exceeds daily limit")
+ budget_item = self.add_to_budget(request_pub, amount_msat=amount_msat or invoice.get_amount_msat())
self.wallet.save_invoice(invoice)
+ success = None
try:
success, log = await self.wallet.lnworker.pay_invoice(
invoice=invoice,
@@ -823,22 +825,25 @@ class NWCServer(Logger, EventListener):
except Exception as e:
self.logger.exception(f"failed to pay nwc invoice")
return self.get_error_response("PAYMENT_FAILED", str(e))
+ finally:
+ if success is False:
+ # If the user shuts down or the application crashes before the payment ends, it will not
+ # get deducted from the budget, even if the htlcs later get failed on wallet restart.
+ self.remove_from_budget(request_pub, budget_item)
preimage: bytes = self.wallet.lnworker.get_preimage(bytes.fromhex(invoice.rhash))
response = {}
if not success or not preimage:
return self.get_error_response("PAYMENT_FAILED", str(log))
- else:
- self.add_to_budget(request_pub, amount_msat=amount_msat or invoice.get_amount_msat())
- response['result'] = {
- 'preimage': preimage.hex(),
- }
+ response['result'] = {
+ 'preimage': preimage.hex(),
+ }
if success:
self.logger.info(f"paid invoice request from NWC for {invoice.get_amount_sat()} sat")
else:
self.logger.info(f"failed to pay invoice request from NWC: {log}")
return response
- def add_to_budget(self, client_pub: str, *, amount_msat: int) -> None:
+ def add_to_budget(self, client_pub: str, *, amount_msat: int) -> list[int]:
"""
If client_pub has a budget, check if the amount is within the budget and add it to the budget.
Return True if the payment is allowed (within the budget)
@@ -846,7 +851,17 @@ class NWCServer(Logger, EventListener):
if 'budget_spends' not in self.connections[client_pub]:
self.connections[client_pub]['budget_spends'] = []
# tuples don't work because jsondb converts them to lists on reload
- self.connections[client_pub]['budget_spends'].append([amount_msat, int(time.time())])
+ budget_item = [amount_msat, int(time.time())]
+ self.connections[client_pub]['budget_spends'].append(budget_item)
+ return budget_item
+
+ def remove_from_budget(self, client_pub: str, budget_item: list[int]) -> None:
+ assert len(budget_item) == 2, budget_item
+ budget_spends = self.connections[client_pub].get('budget_spends', [])
+ try:
+ budget_spends.remove(budget_item)
+ except ValueError:
+ pass
def get_used_budget_msat(self, client_pub: str) -> int:
"""
Why this scored 55/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.