Skip to content

fix(nostr): do not drop NIP-57 receipts when relays tag is missing - #3227

Closed
HaydernCenterpoint wants to merge 3 commits into
stackernews:masterfrom
HaydernCenterpoint:fix/nip57-missing-relays-tag
Closed

fix(nostr): do not drop NIP-57 receipts when relays tag is missing#3227
HaydernCenterpoint wants to merge 3 commits into
stackernews:masterfrom
HaydernCenterpoint:fix/nip57-missing-relays-tag

Conversation

@HaydernCenterpoint

@HaydernCenterpoint HaydernCenterpoint commented Sep 9, 2026

Copy link
Copy Markdown

Description

Fixes #3218.

worker/nostr.js threw TypeError: Cannot read properties of undefined (reading 'slice') when a NIP-57 zap request had no relays tag. pay.js does not require the tag (NIP-57 Appendix D is advisory), so those notes reach the worker; the catch logs failed to publish NIP-57 receipt and the kind 9735 receipt is lost after a settled payment. Proxied (payInBolt11.nostrNote) and direct-receive (externalTransaction.nostrNote) paths both hit this.

Fix: relaysForNip57Receipt uses the request's relays when present, otherwise DEFAULT_CROSSPOSTING_RELAYS. Empty/malformed tags also fall back. Jest coverage in lib/nip57-relays.spec.js.

Did not change pay.js to reject missing relays — that would fail after some wallets already paid. Happy to add fail-fast validation in a follow-up if you prefer.

Screenshots

N/A (server-side worker)

Additional Context

Competing #3219 (same issue, closed 2026-09-07) did not include tests. This PR extracts a helper so the fallback is unit-tested without LND.

Please apply difficulty:* on #3218. Proposing difficulty:easy (100k sats) rather than good-first-issue: small diff, but it sits on the paid-receipt path.

Lint Check / Tests / ShellCheck are action_required for this first-time fork PR — please approve workflows.

Checklist

Are your changes backward compatible? Please answer below:

Yes. Tag present → same relays as before. Tag missing → receipts publish instead of throwing.

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

7/10. Reproduced the original .slice throw in node. Jest cases for present / missing / empty / malformed tags. No full LND/DB integration run.

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

N/A.

Did you introduce any new environment variables? If so, call them out explicitly here:

No.

Did you use AI for this? If so, how much did it assist you?

Yes. An AI assistant helped read the worker path, extract the helper, and write the Jest file. I verified the crash, the fallback, and that DEFAULT_CROSSPOSTING_RELAYS is the existing crosspost default.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change directly addresses the reported runtime exception with a minimal, well-scoped defensive fallback and low regression risk.

Pull request overview

This PR fixes a crash in the NIP-57 receipt publishing worker so zap receipts (kind 9735) are still published even when the originating zap request omits the optional relays tag, aligning behavior with NIP-57 Appendix D being advisory.

Changes:

  • Import DEFAULT_CROSSPOSTING_RELAYS into the NIP-57 worker.
  • Make relay extraction defensive: use the request’s relays tag when present, otherwise fall back to DEFAULT_CROSSPOSTING_RELAYS (and filter out falsy entries).
File summaries
File Description
worker/nostr.js Prevents a .slice(1) crash when the zap request has no relays tag by defaulting to DEFAULT_CROSSPOSTING_RELAYS, ensuring receipts still publish.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@HaydernCenterpoint

This comment was marked as low quality.

@HaydernCenterpoint

This comment was marked as low quality.

@HaydernCenterpoint

This comment was marked as low quality.

@Soxasora Soxasora closed this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NIP-57 receipt worker crashes (receipt silently lost) when the zap request has no relays tag

3 participants