-
Notifications
You must be signed in to change notification settings - Fork 10.2k
feat: BTCPay Server App #21197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: BTCPay Server App #21197
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details:
|
@TChukwuleta is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/08/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (05/08/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mrge found 27 issues across 22 files. Review them in mrge.io
packages/app-store/btcpayserver/components/BtcpayPaymentComponent.tsx
Outdated
Show resolved
Hide resolved
packages/app-store/btcpayserver/pages/setup/_getServerSideProps.tsx
Outdated
Show resolved
Hide resolved
packages/app-store/btcpayserver/pages/setup/_getServerSideProps.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice effort @TChukwuleta , can you first address comments from mrge
, making it draft until then
Hey @TusharBhatt1 thanks for pointing it out Resolved the comments. |
Hey all, great to see this integration taking shape, it’s a strong open-source match! 🚀 @TChukwuleta from our team is leading the dev side, but I’d love to explore a potential marketing collaboration to help us highlight this feature and reach both communities. We did something similar with InvoiceNinja a while back and it turned out great: link @PeerRich I’ve been following your work on X for years just sent you an email, but feel free to ping me there too: https://x.com/pavlenex |
yeah lets do it! |
Hey @anikdhabal I have addressed the reviews |
packages/app-store/btcpayserver/components/EventTypeAppSettingsInterface.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two blocking comments:
#21197 (comment)
#21197 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/app-store/btcpayserver/api/webhook.ts (1)
17-26
: Consider renaming function to reflect its actual purpose.The function name
verifyBTCPaySignature
is misleading as it only computes the signature and validates hex format, but doesn't actually verify/compare signatures. The actual verification happens later withtimingSafeEqual
.Consider renaming to better reflect the function's purpose:
-function verifyBTCPaySignature(rawBody: Buffer, expectedSignature: string, webhookSecret: string): string { +function computeBTCPaySignature(rawBody: Buffer, expectedSignature: string, webhookSecret: string): string {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/app-store/btcpayserver/api/webhook.ts
(1 hunks)packages/app-store/btcpayserver/components/EventTypeAppSettingsInterface.tsx
(1 hunks)packages/app-store/btcpayserver/lib/PaymentService.ts
(1 hunks)packages/app-store/btcpayserver/pages/setup/index.tsx
(1 hunks)packages/lib/server/repository/BookingPaymentRepository.interface.ts
(1 hunks)packages/lib/server/repository/PrismaBookingPaymentRepository.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/lib/server/repository/PrismaBookingPaymentRepository.ts
- packages/lib/server/repository/BookingPaymentRepository.interface.ts
- packages/app-store/btcpayserver/components/EventTypeAppSettingsInterface.tsx
- packages/app-store/btcpayserver/pages/setup/index.tsx
- packages/app-store/btcpayserver/lib/PaymentService.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts
: For Prisma queries, only select data you need; never useinclude
, always useselect
Ensure thecredential.key
field is never returned from tRPC endpoints or APIs
Files:
packages/app-store/btcpayserver/api/webhook.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()
in hot paths like loops
Files:
packages/app-store/btcpayserver/api/webhook.ts
🧠 Learnings (1)
packages/app-store/btcpayserver/api/webhook.ts (5)
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.917Z
Learning: Applies to **/*.ts : Ensure the credential.key
field is never returned from tRPC endpoints or APIs
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:50-60
Timestamp: 2025-07-18T17:54:04.613Z
Learning: For Microsoft Graph webhook handlers, when dealing with internal configuration errors (like missing MICROSOFT_WEBHOOK_TOKEN), it's better to return 200 OK with errors tracked in the response body rather than throwing 5xx errors. This prevents retry storms from Microsoft Graph and maintains webhook subscription health, while still providing visibility through error logs and structured error responses.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.917Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use include
, always use select
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Detect changes
🔇 Additional comments (8)
packages/app-store/btcpayserver/api/webhook.ts (8)
1-15
: LGTM! Well-structured imports and configuration.The imports are appropriate for a webhook handler requiring raw body parsing, cryptographic operations, and payment processing. The body parser configuration is correctly disabled for signature verification.
28-41
: LGTM! Comprehensive webhook schema validation.The Zod schema appropriately covers BTCPay webhook fields with proper optional field handling. Limiting supported events to "InvoiceSettled" and "InvoiceProcessing" is sensible for the payment flow.
43-48
: LGTM! Standard webhook request handling.Properly restricts to POST method and correctly parses raw body for signature verification while maintaining string representation for JSON parsing.
49-51
: LGTM! Thorough signature header validation.Properly handles case variations in BTCPay signature headers and validates the expected "sha256=" format with appropriate 401 status for authentication failures.
53-58
: LGTM! Proper webhook validation and filtering.Schema validation with Zod and event filtering to supported types follows webhook best practices. Returning 200 for unsupported events properly acknowledges receipt to prevent unnecessary retries.
78-91
: LGTM! Secure signature verification implemented.The signature verification properly:
- Extracts the signature from the header
- Computes the expected signature using HMAC SHA-256
- Performs length validation before
timingSafeEqual
to prevent crashes- Uses timing-safe comparison to prevent timing attacks
- Processes payment success only after verification
92-99
: LGTM! Improved error handling with proper status codes.The error handling now correctly:
- Maps HttpCode instances to their specific status codes
- Defaults to 500 for unexpected server errors
- Excludes stack traces in production for security
- Follows proper HTTP semantics instead of always returning 400
60-76
: Prisma select usage is compliantThe
findByExternalIdIncludeBookingUserCredentials
method inpackages/lib/server/repository/PrismaBookingPaymentRepository.ts
uses aselect
clause (notinclude
) onprismaClient.payment.findFirst
, so it already adheres to our guideline of selecting only the needed fields. No changes required here.
…te fn in handlePayment
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
E2E results are ready! |
* app initialization * btcpay-calcom payment * include logo and images * resolve comments * include USD and webhook cleaning * currency display * fix type error * payment service create error * type error fix * icon update * bot feedback update * Remove console * Remove currency suffix in price * fix coderRabbit comment * resolve extra comments * Use repositories and declarative installation ocode for app * use PrismaBookingPaymentRepository as well as fix UI view * Avoid fetching booking just for title which is already passed to create fn in handlePayment * fix type issues * return 200 if payment is already processed --------- Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com> Co-authored-by: Hariom Balhara <hariombalhara@gmail.com> Co-authored-by: Omar López <zomars@me.com>
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Summary by mrge
Added BTCPay Server as a new payment processor option in Cal.com, allowing users to accept Bitcoin payments for bookings.