chore(deps): bump invoices from 2.0.6 to 6.0.5 - #916
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
I found one blocking sequencing/runtime issue.
invoices@6.0.5 declares engines.node >=22, but this PR's current head still advertises/supports Node 20 in the checked tree (package.json keeps engines.node >=20.0.0 and the Dockerfile still builds/runs on node:20-alpine). The PR body says this depends on #915, but as-is this head can still be merged or tested without the Node 22 runtime changes, which would leave production on an unsupported Node version for the new direct dependency.
Please either include the Node 22 runtime/CI changes here, or rebase/retarget this PR on top of #915 after that PR updates every runtime/CI path to Node 22. Once the effective merge result has Node 22 everywhere, the dependency bump itself looks fine.
Verification performed on the current head:
- GitHub checks for
6f3f67243d0861748b5887d000c1c0a76fc23213: CodeQL, Analyze, Lint, andci_to_mainare green. - Local Node:
v22.22.2/ npm10.9.7. npm ci --ignore-scripts,npx tsc --noEmit, andnpm run lintpassed.- Confirmed
invoices@6.0.5still exportsparsePaymentRequest,createUnsignedRequest, andcreateSignedRequest; its package metadata requires Node>=22. - Full local tests were blocked by missing system canvas build deps on this runner, so I relied on the GitHub CI result for full install/test coverage.
|
closing in favor of #917 |
Summary
invoicesfrom2.0.6to6.0.5.Note:
invoices@6.0.0+requires Node.js >= 22. This PR depends on #915 (bump Node engine to 22) and will fail CI until that one is merged intomain.Why
invoices@6.0.0+includes a correctness/security fix inbolt11/request_details.js: when a BOLT11 payment request had multipleidtagged fields, the parser kept overwritingsum.idwith the last one found instead of the first. We rely oninvoice.id(the payment request hash) as the source of truth for payment tracking:bot/validations.tsvalidatesinvoice.idexists before accepting an invoiceln/pay_request.tsstores it asorder.payout_hashln/resubscribe_invoices.tsandln/subscribe_invoice.tslook up orders byhash: invoice.idA malformed or adversarial invoice with a duplicated
idtag could previously cause the bot to track the wrong payment hash. This fix makes that path correct.Changes
package.json:invoices2.0.6→6.0.5package-lock.jsonupdated accordinglyBreaking changes reviewed
Went through the full
invoicesCHANGELOG from2.0.6to6.0.5. All breaking changes are Node.js engine bumps (16 → 18 → 20 → 22), already covered by #915. No other breaking change applies to this codebase — we only useparsePaymentRequestin production code (createUnsignedRequest/createSignedRequestare only used in tests), and none of the affected fields (CLTV delta default, P2TR fallback addresses, signet support) are used here.Testing
npm install— installs cleanlynpx tsc --noEmit— no type errorsnpm run lint— no lint errorsnpm test— 233 passing, 4 pre-existing failures unrelated to this change (missinggetCommunityByIdentifier, from unrelated in-progress work)/selland/buyflows end-to-end