Conversation
…version warning to SKILL.md - Add decision table mapping scenarios to libraries and runtime environments - Add explicit warnings about Bitcoin Connect being browser-only - Add cross-library unit conversion guidance (millisats vs sats) - Helps agents pick the right library immediately and avoid subtle amount bugs
- New error-handling.md with comprehensive error type reference table - Document all Nip47 error classes and when they occur - List common NIP-47 wallet error codes (INSUFFICIENT_BALANCE, QUOTA_EXCEEDED, etc.) - Include try/catch examples for payments and invoice creation - Add retry pattern for transient network/timeout errors - Link from nwc-client.md referenced files
- New common-operations.md covering previously undocumented methods: getInfo, getBalance, makeInvoice, lookupInvoice, listTransactions, getBudget, signMessage, multiPayInvoice - All examples show proper millisat-to-sat conversion for display - Includes filtering transactions by type and time range - Shows how to access the lightning address from the client - Link from nwc-client.md referenced files
- Add note to nwc-client.md clarifying these are advanced classes - Prevents agents from hallucinating usage patterns for undocumented APIs - Directs agents to NWCClient for typical application development
- Document client.close() for proper shutdown - Show unsubscribe + close pattern for notification listeners - Show SIGINT handler for graceful process exit - Prevents leaked WebSocket connections in server apps
…at.md - Add getFormattedFiatValue section for locale-formatted display strings - Fix incorrect 'fiat.getFiatCurrencies()' call to 'getFiatCurrencies()' - getFormattedFiatValue is the most user-friendly fiat function and was missing
- Document the three subpath imports: /lnurl, /fiat, /bolt11 - Explicitly warn against importing from the package root - Prevents import resolution errors agents commonly produce
- Explain default proxy behavior in browsers (CORS avoidance) - Show how to disable proxy for Node.js / server-side usage - Show how to provide a custom proxy URL - Prevents CORS-related failures in browser apps
- New cross-library-recipe.md with full end-to-end example - Pattern: receive USD payment → convert fiat → create invoice → wait for payment → forward 90% to a lightning address - Exercises: fiat conversion, makeInvoice, subscribeNotifications, LightningAddress.requestInvoice, payInvoice, error handling, cleanup - Highlights unit conversion at every boundary between libraries - Link from SKILL.md
…ect integration) - Add browser initialization: user input, CDN/ESM, and Bitcoin Connect bridge - Show how to extract NWCClient from a Bitcoin Connect provider for advanced ops - Add browser cleanup: beforeunload, React useEffect pattern - Add long-lived Node.js process note for notification subscriptions - Separates Node.js vs browser patterns throughout
- Add SSR crash warning with framework-specific solutions: Next.js App Router, Next.js Pages Router, Nuxt 3, SvelteKit - Add general rule: never top-level import in server-rendered files - Add init() placement guidance for React, Vue, Next.js, plain HTML - Warn against calling init() multiple times or in render loops
- Note all packages are ESM-only, require type:module in package.json - Provide minimal tsconfig.json with nodenext module resolution - Show install commands for each library individually and combined - Include WebLN types setup for TypeScript + Bitcoin Connect - Prevents new projects from failing on first import
… invoice docs - Add decodeInvoice() standalone function example for quick parsing - Add invoice expiration section: hasExpired(), remaining time, guard pattern - Add validateInvoiceBeforePayment() helper pattern - Expand preimage verification with context - Clarify LNURL-Verify: when it works, when verify is null - Show all Invoice properties (paymentHash, satoshi, createdDate, expiryDate)
- Add makeInvoice with metadata (comment, recipient_data) - Add payInvoice with metadata (comment, payer_data with name/email) - Add amountless invoice payment pattern with amount override - Add reading transaction metadata from listTransactions/lookupInvoice - Documents Nip47TransactionMetadata fields that were previously hidden
- Add requestInvoice with comment (LUD-12) example - Add requestInvoice with payerdata (LUD-18) example - Add metadata introspection section: min/max amounts, fixed amounts - Add amount validation helper pattern - Add comment support check (commentAllowed) - Add payer data requirements check (mandatory vs optional fields) - Add Nostr integration metadata (pubkey, relays, allowsNostr) - Prevents min/max amount errors and unsupported field errors
- New zaps.md covering the full NIP-57 zap workflow - zapInvoice(): generate a zap invoice without paying (for NWC/QR) - zap(): generate and pay in one step (WebLN/browser) - generateZapEvent(): manual zap event creation for advanced use - Profile zaps vs note zaps (with/without event ID) - Custom Nostr provider for server-side zaps (nostr-tools) - Browser flow with NIP-07 (window.nostr) - Cross-library pattern: Lightning Tools zap + NWC Client payment - Link from lightning-tools.md
Client (consuming L402 resources): - fetchWithL402 with Bitcoin Connect (browser) - fetchWithL402 with NostrWebLNProvider/NWC (Node.js) - Token caching with MemoryStorage and NoStorage - LSAT backwards compatibility (headerKey option) - Manual L402 flow with parseL402 for custom payment logic Server (protecting resources with L402): - Full Express.js L402 middleware implementation - Macaroon creation and HMAC verification - Preimage-to-payment-hash verification - Configurable pricing, TTL, and resource identifiers - Multiple protected routes with different prices - Explanation of the verification flow Testing: - End-to-end test using faucet testing wallets - Link from lightning-tools.md
📝 WalkthroughWalkthroughThis PR expands documentation and examples across the Alby SDK ecosystem, introducing new protocol guidance (L402 HTTP 402 paywalls, Nostr Zaps), cross-library integration examples, and enhanced operation guides for NWC Client and Lightning Tools. One code sample updates the createTestWallet function signature with a new TestWallet type alias and additional parameters. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@references/bitcoin-connect/bitcoin-connect.md`:
- Line 126: The phrase "at the top level" in the React usage note should use the
compound adjective form "top‑level"; update the sentence under "**React (Vite /
CRA):** in `main.tsx` or `App.tsx`, outside any component, at the top level" to
read "at the top‑level" so it becomes "**React (Vite / CRA):** in `main.tsx` or
`App.tsx`, outside any component, at the top‑level", preserving the surrounding
context and backtick filenames.
In `@references/lightning-tools/l402.md`:
- Around line 167-204: The code currently falls back to a hardcoded
MACAROON_SECRET ("change-me-in-production"); add a fail-fast check that detects
this default when running in production (e.g., process.env.NODE_ENV ===
"production") and throw a clear error or exit early so the service won't start
with an insecure secret; update the initialization of MACAROON_SECRET and any
startup path that uses it (references: MACAROON_SECRET, createMacaroon,
verifyMacaroon) to ensure the app refuses to run unless a non-default secret is
provided.
🧹 Nitpick comments (3)
references/nwc-client/error-handling.md (1)
105-126: Consider true exponential backoff for RATE_LIMITED scenarios.The retry pattern documentation mentions "exponential backoff" at line 101, but the implementation uses linear backoff (
delayMs * (i + 1)). For rate limiting scenarios, true exponential backoff (e.g.,delayMs * Math.pow(2, i)) is more effective at preventing thundering herd issues.♻️ Optional enhancement for true exponential backoff
async function withRetry<T>(fn: () => Promise<T>, retries = 3, delayMs = 1000): Promise<T> { for (let i = 0; i < retries; i++) { try { return await fn(); } catch (error) { const isTransient = error instanceof Nip47TimeoutError || error instanceof Nip47NetworkError; if (!isTransient || i === retries - 1) { throw error; } - await new Promise((r) => setTimeout(r, delayMs * (i + 1))); + await new Promise((r) => setTimeout(r, delayMs * Math.pow(2, i))); } } throw new Error("Unreachable"); }This changes the delay progression from 1s, 2s, 3s (linear) to 1s, 2s, 4s (exponential).
references/automated-testing.md (1)
48-48: Optional: unreachable fallback in error throw.The
?? new Error(...)fallback at line 48 is defensive but unreachable in practice, sincelastErrorwill always be set after the first failed attempt. The code is correct, but you could simplify tothrow lastError!with a non-null assertion if preferred.references/cross-library-recipe.md (1)
71-85: Consider cleanup on payment failure.The current code only calls
unsub()andclient.close()after the try-catch block (line 84-85), which means cleanup happens even ifpayInvoicefails. However, if an unexpected error is thrown (notNip47WalletError), it will be re-thrown at line 80 and the cleanup won't execute.🔧 Consider moving cleanup to a finally block
try { const payResponse = await client.payInvoice({ invoice: recipientInvoice.paymentRequest, }); console.log("Forwarded payment! Preimage:", payResponse.preimage); } catch (error) { if (error instanceof Nip47WalletError) { console.error(`Payment failed [${error.code}]: ${error.message}`); } else { throw error; } + } finally { + unsub(); + client.close(); } - - unsub(); - client.close(); });This ensures cleanup happens regardless of success or failure.
|
|
||
| **Where to call `init()`:** | ||
|
|
||
| - **React (Vite / CRA):** in `main.tsx` or `App.tsx`, outside any component, at the top level |
There was a problem hiding this comment.
Use “top‑level” as a compound adjective.
Change “at the top level” → “at the top‑level” for standard hyphenation in technical docs.
✏️ Proposed fix
-- **React (Vite / CRA):** in `main.tsx` or `App.tsx`, outside any component, at the top level
+- **React (Vite / CRA):** in `main.tsx` or `App.tsx`, outside any component, at the top‑level📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **React (Vite / CRA):** in `main.tsx` or `App.tsx`, outside any component, at the top level | |
| - **React (Vite / CRA):** in `main.tsx` or `App.tsx`, outside any component, at the top‑level |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~126-~126: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...App.tsx`, outside any component, at the top level - React (Next.js): inside the dynam...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
In `@references/bitcoin-connect/bitcoin-connect.md` at line 126, The phrase "at
the top level" in the React usage note should use the compound adjective form
"top‑level"; update the sentence under "**React (Vite / CRA):** in `main.tsx` or
`App.tsx`, outside any component, at the top level" to read "at the top‑level"
so it becomes "**React (Vite / CRA):** in `main.tsx` or `App.tsx`, outside any
component, at the top‑level", preserving the surrounding context and backtick
filenames.
| // --- Macaroon helpers --- | ||
|
|
||
| const MACAROON_SECRET = process.env.MACAROON_SECRET || "change-me-in-production"; | ||
|
|
||
| interface MacaroonData { | ||
| paymentHash: string; | ||
| identifier: string; | ||
| expiresAt: number; | ||
| } | ||
|
|
||
| function createMacaroon(data: MacaroonData): string { | ||
| const payload = JSON.stringify(data); | ||
| const signature = crypto | ||
| .createHmac("sha256", MACAROON_SECRET) | ||
| .update(payload) | ||
| .digest("hex"); | ||
| return Buffer.from(JSON.stringify({ payload, signature })).toString("base64"); | ||
| } | ||
|
|
||
| function verifyMacaroon(macaroon: string): MacaroonData | null { | ||
| try { | ||
| const { payload, signature } = JSON.parse( | ||
| Buffer.from(macaroon, "base64").toString() | ||
| ); | ||
| const expectedSig = crypto | ||
| .createHmac("sha256", MACAROON_SECRET) | ||
| .update(payload) | ||
| .digest("hex"); | ||
| if (signature !== expectedSig) return null; | ||
|
|
||
| const data: MacaroonData = JSON.parse(payload); | ||
| if (data.expiresAt < Date.now()) return null; | ||
|
|
||
| return data; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Security: Ensure MACAROON_SECRET is set in production.
Line 169 uses a default secret "change-me-in-production" which is clearly marked but could pose a security risk if forgotten. Consider failing fast when the default is detected in production environments.
🔒 Optional: Enhanced secret validation
const MACAROON_SECRET = process.env.MACAROON_SECRET || "change-me-in-production";
+
+if (MACAROON_SECRET === "change-me-in-production" && process.env.NODE_ENV === "production") {
+ throw new Error("MACAROON_SECRET must be set in production");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // --- Macaroon helpers --- | |
| const MACAROON_SECRET = process.env.MACAROON_SECRET || "change-me-in-production"; | |
| interface MacaroonData { | |
| paymentHash: string; | |
| identifier: string; | |
| expiresAt: number; | |
| } | |
| function createMacaroon(data: MacaroonData): string { | |
| const payload = JSON.stringify(data); | |
| const signature = crypto | |
| .createHmac("sha256", MACAROON_SECRET) | |
| .update(payload) | |
| .digest("hex"); | |
| return Buffer.from(JSON.stringify({ payload, signature })).toString("base64"); | |
| } | |
| function verifyMacaroon(macaroon: string): MacaroonData | null { | |
| try { | |
| const { payload, signature } = JSON.parse( | |
| Buffer.from(macaroon, "base64").toString() | |
| ); | |
| const expectedSig = crypto | |
| .createHmac("sha256", MACAROON_SECRET) | |
| .update(payload) | |
| .digest("hex"); | |
| if (signature !== expectedSig) return null; | |
| const data: MacaroonData = JSON.parse(payload); | |
| if (data.expiresAt < Date.now()) return null; | |
| return data; | |
| } catch { | |
| return null; | |
| } | |
| } | |
| // --- Macaroon helpers --- | |
| const MACAROON_SECRET = process.env.MACAROON_SECRET || "change-me-in-production"; | |
| if (MACAROON_SECRET === "change-me-in-production" && process.env.NODE_ENV === "production") { | |
| throw new Error("MACAROON_SECRET must be set in production"); | |
| } | |
| interface MacaroonData { | |
| paymentHash: string; | |
| identifier: string; | |
| expiresAt: number; | |
| } | |
| function createMacaroon(data: MacaroonData): string { | |
| const payload = JSON.stringify(data); | |
| const signature = crypto | |
| .createHmac("sha256", MACAROON_SECRET) | |
| .update(payload) | |
| .digest("hex"); | |
| return Buffer.from(JSON.stringify({ payload, signature })).toString("base64"); | |
| } | |
| function verifyMacaroon(macaroon: string): MacaroonData | null { | |
| try { | |
| const { payload, signature } = JSON.parse( | |
| Buffer.from(macaroon, "base64").toString() | |
| ); | |
| const expectedSig = crypto | |
| .createHmac("sha256", MACAROON_SECRET) | |
| .update(payload) | |
| .digest("hex"); | |
| if (signature !== expectedSig) return null; | |
| const data: MacaroonData = JSON.parse(payload); | |
| if (data.expiresAt < Date.now()) return null; | |
| return data; | |
| } catch { | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@references/lightning-tools/l402.md` around lines 167 - 204, The code
currently falls back to a hardcoded MACAROON_SECRET ("change-me-in-production");
add a fail-fast check that detects this default when running in production
(e.g., process.env.NODE_ENV === "production") and throw a clear error or exit
early so the service won't start with an insecure secret; update the
initialization of MACAROON_SECRET and any startup path that uses it (references:
MACAROON_SECRET, createMacaroon, verifyMacaroon) to ensure the app refuses to
run unless a non-default secret is provided.
| </script> | ||
| ``` | ||
|
|
||
| ## ⚠️ SSR / SSG Warning (Next.js, Nuxt, SvelteKit, Remix, Astro) |
There was a problem hiding this comment.
Nice additional docs. I would link to a separate md file to reduce the context for non-SSR apps (progressive disclosure)
| IMPORTANT: read the [typings](./index.d.ts) to better understand how this works. | ||
|
|
||
| ## Decode an invoice | ||
| ## Decode an Invoice (Quick) |
There was a problem hiding this comment.
Do we need both really? I would just expose the new Invoice method. In this case I think it's an unnecessary optimization which is possibly worse (more context)
|
|
||
| ## Invoice Expiration & Safety Checklist | ||
|
|
||
| Lightning invoices expire — typically after 1 hour, but the expiry varies by wallet. Always check expiry before presenting an invoice to a user or attempting to pay it. |
There was a problem hiding this comment.
typically after 1 hour should be removed I think.
| - Decode the invoice and ensure the `paymentHash` matches what you expect to pay. | ||
| - Check expiry and warn if the remaining time is low (e.g., under 60 seconds). | ||
| - If the invoice is amountless, ensure you set the intended amount explicitly when paying. | ||
| - When mixing libraries, remember units: `NWCClient` uses **msats**, Lightning Tools / WebLN / Bitcoin Connect use **sats**. |
There was a problem hiding this comment.
It's weird this is under the invoice expiry section, wonder why the AI did it this way. And I'm not sure this is the best place to do it. I don't think lightning-tools should reference NWCClient. We could put interaction between multiple libraries in the parent SKILL.md or a file that is referenced from there.
| ### Check if an invoice has expired | ||
|
|
||
| ```ts | ||
| if (invoice.hasExpired()) { |
There was a problem hiding this comment.
I think this is kind of duplicated with the section below. I think we should try to keep the documentation minimal unless the AI really can't figure it out itself
| - NWC millisats → sats: `Math.floor(millisats / 1000)` | ||
| - sats → NWC millisats: `sats * 1000` | ||
|
|
||
| ## Node.js Project Setup |
There was a problem hiding this comment.
If this is specific to Node.js maybe it can be referenced as a separate file
| } | ||
| ``` | ||
|
|
||
| 3. Install dependencies based on what you need: |
There was a problem hiding this comment.
The individual tools already have installation instructions. I don't think we should add them here, and we shouldn't specify which package manager to use
| If they do not have a wallet yet [here are some options](./references/production-wallets.md) No newline at end of file | ||
| If they do not have a wallet yet [here are some options](./references/production-wallets.md) | ||
|
|
||
| ## Quickstart Decision Guide |
There was a problem hiding this comment.
I think we are duplicating a lot of info here and putting it all in the root of the skill is not so great. We already link to individual reference files and describe what each reference file is for
| - **L402 server** → `NWCClient.makeInvoice` + macaroon verification (see `lightning-tools/l402.md`). | ||
| - **Testing** → Always prefer [testing wallets](./references/testing-wallets.md) and wire them into automated tests (Jest/Vitest/Playwright) per [automated testing](./references/automated-testing.md). | ||
|
|
||
| ## NWC Secret Handling (Security) |
There was a problem hiding this comment.
This is mostly duplicated (already in nwc-client.md) but could also be added to bitcoin connect (the browser specific stuff)
| const store = new NoStorage(); | ||
| ``` | ||
|
|
||
| ### LSAT Backwards Compatibility |
There was a problem hiding this comment.
Do we still need LSAT backward compatibility?
| nostrWalletConnectUrl: process.env.NWC_URL, | ||
| }); | ||
|
|
||
| // --- Macaroon helpers --- |
There was a problem hiding this comment.
all these helpers would be great to move to js-lightning-tools!
| - `@getalby/lightning-tools/fiat` — Fiat currency conversion functions | ||
| - `@getalby/lightning-tools/bolt11` — `Invoice` class and `decodeInvoice` | ||
|
|
||
| Do NOT import from the package root (e.g. `import { LightningAddress } from "@getalby/lightning-tools"`). Always use the subpath imports shown in the examples. |
There was a problem hiding this comment.
Here we are probably compensating for a dumb model. I think we should remove it, a better model can look at the examples and follow them properly.
| - [Request BOLT-11 invoices from a lightning address](./lnurl.md) | ||
| - [Convert between bitcoin and fiat currency amounts](./fiat.md) | ||
| - [Decode and work with invoices](./invoice.md) | ||
| - [Nostr Zaps: send lightning payments tied to Nostr events and profiles](./zaps.md) |
There was a problem hiding this comment.
Do we want to add zaps knowledge here? it's a slippery slope
| @@ -0,0 +1,104 @@ | |||
| # Cross-Library Recipe: NWC Client + Lightning Tools | |||
There was a problem hiding this comment.
I don't think we need this. AI is smart enough to figure out how to combine usage of the 2 packages.
If we think it is an issue maybe it's better to focus on the root problem that we have 2 separate npm packages
|
Recommendation: I could list the good additions and create a new PR with those minimally added. |
2 participants
potentially some inspiration. needs review.
Summary by CodeRabbit
Documentation
New Features