-
Notifications
You must be signed in to change notification settings - Fork 805
feat(pay): add HTTP provider for browser/Node.js environments #7132
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
base: v2.0
Are you sure you want to change the base?
Conversation
Update yttrium-wcpay Android dependency from 0.9.119 to 0.10.3.
Add HttpProvider that makes direct HTTP calls to the Pay API, enabling the Pay SDK to work in any JavaScript environment with fetch support (browsers, Node.js 18+, etc.). Changes: - Add HttpProvider implementation with full API support - Add payment ID extraction from various link formats - Add action caching and build action resolution - Add automatic polling for payment status - Update provider detection to use HTTP when native unavailable - Add comprehensive test suite with 30 tests Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
14babd8 to
2c7572c
Compare
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.
Pull request overview
This PR adds an HTTP provider that enables the WalletConnect Pay SDK to work in browser and Node.js environments by making direct HTTP calls to the Pay API, removing the dependency on native/WASM modules for web environments.
Changes:
- Added
HttpProviderclass that usesfetchAPI for all payment operations - Updated provider auto-detection to prioritize native → HTTP → WASM
- Extended
PayProviderTypeto include "http" option
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/pay/src/providers/http.ts | New HTTP provider implementation with payment ID extraction, action caching, and automatic polling |
| packages/pay/src/providers/index.ts | Updated provider detection to include HTTP provider as second priority |
| packages/pay/src/types/provider.ts | Added "http" to PayProviderType union |
| packages/pay/test/http-provider.spec.ts | Comprehensive test suite with 30 tests covering all HTTP provider functionality |
| packages/react-native-compat/android/build.gradle | Updated yttrium-wcpay dependency version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export class HttpProvider implements PayProvider { | ||
| private readonly baseUrl: string; | ||
| private readonly headers: Record<string, string>; | ||
| private cachedOptions: CachedPaymentOption[] = []; |
Copilot
AI
Jan 16, 2026
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.
The cachedOptions array is shared across multiple payment flows and could lead to incorrect behavior if the same HttpProvider instance is used for multiple different payments simultaneously. Consider keying the cache by paymentId (e.g., Map<string, CachedPaymentOption[]>) to isolate cache entries per payment.
| ); | ||
|
|
||
| // Cache the raw options for use by getRequiredPaymentActions | ||
| this.cachedOptions = response.options.map((o) => ({ |
Copilot
AI
Jan 16, 2026
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.
The cache is completely replaced on each getPaymentOptions call, which means if a user calls getPaymentOptions for payment A, then for payment B, the cached actions for payment A are lost. This breaks the caching mechanism if multiple payments are handled with the same provider instance. The cache should be keyed by paymentId to avoid this issue.
| this.cachedOptions = response.options.map((o) => ({ | |
| this.cachedOptions[paymentId] = response.options.map((o) => ({ |
| while (!result.isFinal) { | ||
| const delay = result.pollInMs ?? 1000; | ||
| await new Promise((resolve) => setTimeout(resolve, delay)); |
Copilot
AI
Jan 16, 2026
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.
The infinite polling loop has no timeout or maximum retry limit, which could cause the function to hang indefinitely if the server never returns isFinal: true. Consider adding a maximum retry count or total timeout to prevent infinite loops.
| const cached = this.cachedOptions.find((o) => o.optionId === params.optionId); | ||
|
|
||
| let rawActions: RawAction[]; | ||
| if (cached && cached.actions.length > 0) { |
Copilot
AI
Jan 16, 2026
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.
The condition cached.actions.length > 0 excludes valid cached options that have zero actions. If an option legitimately has no actions, it should still be served from cache rather than making an unnecessary API call. Consider removing the length check or explicitly checking for null/undefined.
| if (cached && cached.actions.length > 0) { | |
| if (cached && Array.isArray(cached.actions)) { |
| throw new PayError( | ||
| "UNKNOWN", |
Copilot
AI
Jan 16, 2026
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.
All HTTP errors are categorized as 'UNKNOWN' error code. Consider mapping HTTP error responses to more specific error codes (e.g., 404 → PAYMENT_OPTIONS, 422 → CONFIRM_PAYMENT) to provide more meaningful error information to SDK consumers.
| throw new PayError( | |
| "UNKNOWN", | |
| // Map HTTP status codes to more specific PayError codes where possible | |
| let errorCode: string; | |
| switch (response.status) { | |
| case 404: | |
| errorCode = "PAYMENT_OPTIONS"; | |
| break; | |
| case 422: | |
| errorCode = "CONFIRM_PAYMENT"; | |
| break; | |
| default: | |
| errorCode = "UNKNOWN"; | |
| break; | |
| } | |
| throw new PayError( | |
| errorCode, |
| expect(mockFetch).toHaveBeenCalledWith( | ||
| expect.stringContaining("/v1/gateway/payment/pay_headers/options"), | ||
| expect.objectContaining({ | ||
| method: "POST", | ||
| headers: expect.objectContaining({ | ||
| "Content-Type": "application/json", | ||
| "Api-Key": "test-api-key", | ||
| "Sdk-Name": "test-sdk", | ||
| "Sdk-Version": "1.0.0", | ||
| "Sdk-Platform": "test", | ||
| }), | ||
| }), | ||
| ); |
Copilot
AI
Jan 16, 2026
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.
The test validates that headers are sent but doesn't verify that the bundleId from the config is included in any header or request. If bundleId should be sent (which is part of PayProviderConfig), this should be tested, or if it's intentionally not used by HttpProvider, this should be documented.
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.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| isFinal: statusResponse.isFinal, | ||
| pollInMs: statusResponse.pollInMs, | ||
| }; | ||
| } |
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.
Infinite polling loop with no timeout in confirmPayment
High Severity
The confirmPayment method contains a while (!result.isFinal) polling loop with no maximum iteration limit, timeout, or cancellation mechanism. If the API server never returns isFinal: true (due to a bug, network issues, or stuck state), this loop will poll indefinitely, causing the application to hang and potentially exhausting resources.
| this.cachedOptions = response.options.map((o) => ({ | ||
| optionId: o.id, | ||
| actions: o.actions, | ||
| })); |
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.
Action cache not scoped by paymentId causes stale data
Medium Severity
The cachedOptions stores actions keyed only by optionId, but the cache is completely replaced on each getPaymentOptions call. When getRequiredPaymentActions looks up cached data, it only matches by optionId without verifying the paymentId. If the same HttpProvider instance is used for multiple payments with overlapping option IDs, wrong cached actions could be returned.
Additional Locations (1)
| assetSymbol: "USDC", | ||
| assetName: "USD Coin", | ||
| decimals: 6, | ||
| iconUrl: "https://example.com/usdc.png", |
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.
External Domain URL Detected
Low Severity · Bugbot Rules
This change introduces a URL pointing to an external domain: https://example.com/usdc.png. Please verify that these external dependencies are intentional and review for potential security, privacy, or compliance implications. Approved company domains are: reown.com, walletconnect.com, walletconnect.org
| } | ||
|
|
||
| return null; | ||
| }; |
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.
URL hash fragments not stripped from payment ID
Medium Severity
The extractPaymentId function doesn't strip URL hash fragments (#). For URLs like https://pay.walletconnect.com/pay_123#section or ?pid=pay_123#hash, the extracted payment ID would include the hash portion (e.g., pay_123#section), causing API calls to fail with invalid payment IDs. The code only splits on ? but not on #.
| if (fetchedAction.type === "walletRpc") { | ||
| resolved.push(this.transformAction(fetchedAction.data as RawWalletRpcAction)); | ||
| } | ||
| } |
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.
Nested build actions silently dropped during resolution
Medium Severity
When resolveActions processes a build action by calling the fetch endpoint, it only handles walletRpc actions from the response. If the fetch endpoint returns additional build actions (which the RawFetchResponse type explicitly allows), those are silently ignored. This could result in incomplete payment actions being returned, causing payments to fail because required transactions are missing.
Summary
HttpProviderthat makes direct HTTP calls to the Pay API, enabling the SDK to work in any JavaScript environment withfetchsupport (browsers, Node.js 18+, etc.)Changes
New
HttpProvider(src/providers/http.ts)getPaymentOptionsfor efficientgetRequiredPaymentActionscalls/fetchendpointconfirmPaymentUpdated provider detection (
src/providers/index.ts)fetchis available and native module isn'tUpdated types (
src/types/provider.ts)"http"toPayProviderTypeTest plan
test/http-provider.spec.ts)Usage
🚧 Work in Progress - Ready for initial review
🤖 Generated with Claude Code
Note
Enables Pay SDK to run in any fetch-capable JS runtime by adding a new HTTP provider and wiring it into provider detection.
HttpProvider(providers/http.ts): direct API calls, robust payment ID extraction (URLs,wc:URIs, query params), action caching, build-action resolution via/fetch, and auto-polling inconfirmPaymentproviders/index.ts): auto-detect order nownative → http → wasm; exports updatedtypes/provider.ts): adds"http"toPayProviderTypetest/http-provider.spec.ts): comprehensive suite covering options, action resolution, polling, errorsyttrium-wcpayto0.10.4inbuild.gradleWritten by Cursor Bugbot for commit 2c7572c. This will update automatically on new commits. Configure here.