refactor: remove Coinbase facilitator coupling and implement Faremeter #57
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r integration - Created IFacilitator interface to abstract facilitator implementations - Implemented FaremeterFacilitator class following the interface - Updated Webhooks.ts to use facilitator abstraction instead of direct Coinbase calls - Replaced Coinbase-specific credentials (apiKeyId/apiKeySecret) with generic facilitatorApiKey - Removed CoinbaseJWT.ts and coinbaseFacilitator.ts (Coinbase-specific code) - Maintained x402 protocol schema compatibility - EVM-only implementation (no Solana changes) Co-Authored-By: [email protected] <[email protected]>
Contributor
Original prompt from [email protected] |
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
I'm learning! Leave a 👍 or a 👎 to help me learn. Extra points if you leave a comment telling me why i'm wrong tagging @greptileai
6 files reviewed, 1 comment
Comment on lines
17
to
96
| const paymentRequirementsObj: IPaymentRequirements = { | ||
| scheme: paymentRequirements.scheme, | ||
| network: paymentRequirements.network, | ||
| maxAmountRequired: paymentRequirements.maxAmountRequired, | ||
| resource: paymentRequirements.resource, | ||
| description: paymentRequirements.description, | ||
| mimeType: paymentRequirements.mimeType, | ||
| outputSchema: paymentRequirements.outputSchema, | ||
| payTo: paymentRequirements.payTo, | ||
| maxTimeoutSeconds: paymentRequirements.maxTimeoutSeconds, | ||
| asset: paymentRequirements.asset, | ||
| extra: paymentRequirements.extra, | ||
| }; | ||
|
|
||
| const requestBody = { | ||
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | ||
| paymentPayload: { | ||
| ...paymentPayload, | ||
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | ||
| }, | ||
| paymentRequirements: paymentRequirementsObj, | ||
| }; | ||
|
|
||
| const requestDataStr = JSON.stringify(requestBody, null, 2); | ||
|
|
||
| console.log(`=== SENDING TO FAREMETER FACILITATOR (VERIFY) ===\n${requestDataStr}`); | ||
|
|
||
| const res = await fetch(`https://${FAREMETER_HOST}${FACILITATOR_VERIFY_PATH}`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Authorization': `Bearer ${this.apiKey}`, | ||
| }, | ||
| body: JSON.stringify(requestBody), | ||
| }); | ||
|
|
||
| const responseText = await res.text(); | ||
|
|
||
| console.log(`=== RAW RESPONSE FROM FAREMETER FACILITATOR (VERIFY) ===`); | ||
| console.log(`Status Code: ${res.status}`); | ||
| console.log(`Status Text: ${res.statusText}`); | ||
| console.log(`Raw Response Body:`, responseText); | ||
| console.log(`Response Headers:`, JSON.stringify(Object.fromEntries(res.headers.entries()), null, 2)); | ||
|
|
||
| const receivedLog = `=== RECEIVED FROM FAREMETER FACILITATOR (VERIFY) ===\nStatus: ${res.status} ${res.statusText}\nResponse: ${responseText}`; | ||
| console.log(receivedLog); | ||
|
|
||
| if (!res.ok) { | ||
| throw new Error(`/verify ${res.status}: ${responseText}`); | ||
| } | ||
|
|
||
| return JSON.parse(responseText) as { isValid: boolean; invalidReason?: string }; | ||
| } | ||
|
|
||
| async settlePayment( | ||
| paymentPayload: IPaymentPayload, | ||
| paymentRequirements: PaymentRequirements, | ||
| ): Promise<{ success: boolean; txHash?: string; error?: string }> { | ||
| const paymentRequirementsObj: IPaymentRequirements = { | ||
| scheme: paymentRequirements.scheme, | ||
| network: paymentRequirements.network, | ||
| maxAmountRequired: paymentRequirements.maxAmountRequired, | ||
| resource: paymentRequirements.resource, | ||
| description: paymentRequirements.description, | ||
| mimeType: paymentRequirements.mimeType, | ||
| outputSchema: paymentRequirements.outputSchema, | ||
| payTo: paymentRequirements.payTo, | ||
| maxTimeoutSeconds: paymentRequirements.maxTimeoutSeconds, | ||
| asset: paymentRequirements.asset, | ||
| extra: paymentRequirements.extra, | ||
| }; | ||
|
|
||
| const requestBody = { | ||
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | ||
| paymentPayload: { | ||
| ...paymentPayload, | ||
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | ||
| }, | ||
| paymentRequirements: paymentRequirementsObj, | ||
| }; |
There was a problem hiding this comment.
style: Significant code duplication between verifyPayment and settlePayment methods - identical logic for building paymentRequirementsObj and requestBody is repeated. Extract helper function to improve readability and maintainability.
Suggested change
| const paymentRequirementsObj: IPaymentRequirements = { | |
| scheme: paymentRequirements.scheme, | |
| network: paymentRequirements.network, | |
| maxAmountRequired: paymentRequirements.maxAmountRequired, | |
| resource: paymentRequirements.resource, | |
| description: paymentRequirements.description, | |
| mimeType: paymentRequirements.mimeType, | |
| outputSchema: paymentRequirements.outputSchema, | |
| payTo: paymentRequirements.payTo, | |
| maxTimeoutSeconds: paymentRequirements.maxTimeoutSeconds, | |
| asset: paymentRequirements.asset, | |
| extra: paymentRequirements.extra, | |
| }; | |
| const requestBody = { | |
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | |
| paymentPayload: { | |
| ...paymentPayload, | |
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | |
| }, | |
| paymentRequirements: paymentRequirementsObj, | |
| }; | |
| const requestDataStr = JSON.stringify(requestBody, null, 2); | |
| console.log(`=== SENDING TO FAREMETER FACILITATOR (VERIFY) ===\n${requestDataStr}`); | |
| const res = await fetch(`https://${FAREMETER_HOST}${FACILITATOR_VERIFY_PATH}`, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| 'Authorization': `Bearer ${this.apiKey}`, | |
| }, | |
| body: JSON.stringify(requestBody), | |
| }); | |
| const responseText = await res.text(); | |
| console.log(`=== RAW RESPONSE FROM FAREMETER FACILITATOR (VERIFY) ===`); | |
| console.log(`Status Code: ${res.status}`); | |
| console.log(`Status Text: ${res.statusText}`); | |
| console.log(`Raw Response Body:`, responseText); | |
| console.log(`Response Headers:`, JSON.stringify(Object.fromEntries(res.headers.entries()), null, 2)); | |
| const receivedLog = `=== RECEIVED FROM FAREMETER FACILITATOR (VERIFY) ===\nStatus: ${res.status} ${res.statusText}\nResponse: ${responseText}`; | |
| console.log(receivedLog); | |
| if (!res.ok) { | |
| throw new Error(`/verify ${res.status}: ${responseText}`); | |
| } | |
| return JSON.parse(responseText) as { isValid: boolean; invalidReason?: string }; | |
| } | |
| async settlePayment( | |
| paymentPayload: IPaymentPayload, | |
| paymentRequirements: PaymentRequirements, | |
| ): Promise<{ success: boolean; txHash?: string; error?: string }> { | |
| const paymentRequirementsObj: IPaymentRequirements = { | |
| scheme: paymentRequirements.scheme, | |
| network: paymentRequirements.network, | |
| maxAmountRequired: paymentRequirements.maxAmountRequired, | |
| resource: paymentRequirements.resource, | |
| description: paymentRequirements.description, | |
| mimeType: paymentRequirements.mimeType, | |
| outputSchema: paymentRequirements.outputSchema, | |
| payTo: paymentRequirements.payTo, | |
| maxTimeoutSeconds: paymentRequirements.maxTimeoutSeconds, | |
| asset: paymentRequirements.asset, | |
| extra: paymentRequirements.extra, | |
| }; | |
| const requestBody = { | |
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | |
| paymentPayload: { | |
| ...paymentPayload, | |
| x402Version: typeof paymentPayload.x402Version === 'string' ? parseInt(paymentPayload.x402Version, 10) : paymentPayload.x402Version ?? 1, | |
| }, | |
| paymentRequirements: paymentRequirementsObj, | |
| }; | |
| private buildRequestBody( | |
| paymentPayload: IPaymentPayload, | |
| paymentRequirements: PaymentRequirements, | |
| ) { | |
| const paymentRequirementsObj: IPaymentRequirements = { | |
| scheme: paymentRequirements.scheme, | |
| network: paymentRequirements.network, | |
| maxAmountRequired: paymentRequirements.maxAmountRequired, | |
| resource: paymentRequirements.resource, | |
| description: paymentRequirements.description, | |
| mimeType: paymentRequirements.mimeType, | |
| outputSchema: paymentRequirements.outputSchema, | |
| payTo: paymentRequirements.payTo, | |
| maxTimeoutSeconds: paymentRequirements.maxTimeoutSeconds, | |
| asset: paymentRequirements.asset, | |
| extra: paymentRequirements.extra, | |
| }; | |
| const x402Version = typeof paymentPayload.x402Version === 'string' | |
| ? parseInt(paymentPayload.x402Version, 10) | |
| : paymentPayload.x402Version ?? 1; | |
| return { | |
| x402Version, | |
| paymentPayload: { | |
| ...paymentPayload, | |
| x402Version, | |
| }, | |
| paymentRequirements: paymentRequirementsObj, | |
| }; | |
| } | |
| async verifyPayment( | |
| paymentPayload: IPaymentPayload, | |
| paymentRequirements: PaymentRequirements, | |
| ): Promise<{ isValid: boolean; invalidReason?: string }> { | |
| const requestBody = this.buildRequestBody(paymentPayload, paymentRequirements); |
Context Used: Rule from dashboard - When a function grows to 80+ lines, extract helper functions to improve readability and maintainabil... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/webhooks/executions/facilitator/faremeterFacilitator.ts
Line: 17:96
Comment:
**style:** Significant code duplication between `verifyPayment` and `settlePayment` methods - identical logic for building `paymentRequirementsObj` and `requestBody` is repeated. Extract helper function to improve readability and maintainability.
```suggestion
private buildRequestBody(
paymentPayload: IPaymentPayload,
paymentRequirements: PaymentRequirements,
) {
const paymentRequirementsObj: IPaymentRequirements = {
scheme: paymentRequirements.scheme,
network: paymentRequirements.network,
maxAmountRequired: paymentRequirements.maxAmountRequired,
resource: paymentRequirements.resource,
description: paymentRequirements.description,
mimeType: paymentRequirements.mimeType,
outputSchema: paymentRequirements.outputSchema,
payTo: paymentRequirements.payTo,
maxTimeoutSeconds: paymentRequirements.maxTimeoutSeconds,
asset: paymentRequirements.asset,
extra: paymentRequirements.extra,
};
const x402Version = typeof paymentPayload.x402Version === 'string'
? parseInt(paymentPayload.x402Version, 10)
: paymentPayload.x402Version ?? 1;
return {
x402Version,
paymentPayload: {
...paymentPayload,
x402Version,
},
paymentRequirements: paymentRequirementsObj,
};
}
async verifyPayment(
paymentPayload: IPaymentPayload,
paymentRequirements: PaymentRequirements,
): Promise<{ isValid: boolean; invalidReason?: string }> {
const requestBody = this.buildRequestBody(paymentPayload, paymentRequirements);
```
**Context Used:** Rule from `dashboard` - When a function grows to 80+ lines, extract helper functions to improve readability and maintainabil... ([source](https://app.greptile.com/review/custom-context?memory=16f48b22-d2d2-42d7-bc6e-3f38c5979688))
How can I resolve this? If you propose a fix, please make it concise.- Renamed webhooks directory to CrossmintWebhook to satisfy node-dirname-against-convention - Renamed CrossmintWebhooks.node.ts to CrossmintWebhook.node.ts to satisfy node-filename-against-convention - Replaced 'any' types with 'unknown' in paymentHelpers.ts, x402Types.ts, and paymentValidation.ts - Removed unused catch block variables in paymentValidation.ts - Updated package.json to reference new node path - All changes are mechanical lint fixes with no functional changes Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
I'm learning! Leave a 👍 or a 👎 to help me learn. Extra points if you leave a comment telling me why i'm wrong tagging @greptileai
Additional Comments (1)
-
nodes/CrossmintWebhook/executions/Webhooks.ts, line 136-166 (link)style: Nested try-catch blocks - the inner try-catch (lines 136-166) is nested within the outer try-catch (lines 90-183). Refactor using promise chaining for cleaner error handling.
const settleResponse = await facilitator.settlePayment( decodedXPaymentJson, verification.paymentRequirements!, ).catch((error) => { this.logger.error('Error in x402 webhook settlement, moving on...', error); return { success: true, txHash: 'TBD' }; }); if (!settleResponse.success) { resp.writeHead(402, { 'Content-Type': 'application/json' }); resp.end( JSON.stringify({ x402Version: 1, accepts: paymentRequirements, }), ); return { noWebhookResponse: true }; } return generateResponse( this, req, responseMode, responseData, settleResponse.txHash ?? 'UNKNOWN_TX', prepareOutput, decodedXPaymentJson.network, );Context Used: Rule from
dashboard- Avoid double-nesting try/catch blocks. Instead, use promise chains with.catch()methods to handle... (source)
11 files reviewed, 1 comment
Faremeter does not require API keys - trust comes from cryptographic signatures in the payment payload (EIP-712/EIP-3009), not server credentials. Changes: - Removed apiKey parameter from FaremeterFacilitator constructor - Removed Authorization header from verify/settle API calls - Removed facilitatorApiKey field from CrossmintApi credentials - Updated Webhooks.ts to instantiate FaremeterFacilitator without API key - Added Accept header to Faremeter API requests for proper content negotiation Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
I'm learning! Leave a 👍 or a 👎 to help me learn. Extra points if you leave a comment telling me why i'm wrong tagging @greptileai
3 files reviewed, 2 comments
Comment on lines
40
to
48
| console.log(`=== SENDING TO FAREMETER FACILITATOR (VERIFY) ===\n${requestDataStr}`); | ||
|
|
||
| const res = await fetch(`https://${FAREMETER_HOST}${FACILITATOR_VERIFY_PATH}`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Accept': 'application/json', | ||
| }, | ||
| body: JSON.stringify(requestBody), |
There was a problem hiding this comment.
logic: Missing authentication - no API key or authorization header sent to Faremeter API. The old Coinbase implementation required apiKeyId/apiKeySecret credentials and sent Authorization: Bearer ${token} headers, but this implementation sends no authentication at all. Either Faremeter requires no authentication (unlikely for a payment API) or this is a critical bug that will cause all API requests to fail with 401/403 errors.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/CrossmintWebhook/executions/facilitator/faremeterFacilitator.ts
Line: 40:48
Comment:
**logic:** Missing authentication - no API key or authorization header sent to Faremeter API. The old Coinbase implementation required `apiKeyId`/`apiKeySecret` credentials and sent `Authorization: Bearer ${token}` headers, but this implementation sends no authentication at all. Either Faremeter requires no authentication (unlikely for a payment API) or this is a critical bug that will cause all API requests to fail with 401/403 errors.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
38
to
+39
|
|
||
| // Coinbase credentials (apiKeyId/apiKeySecret) are required for x402 processing | ||
| const coinbaseKeyId = (credentials as any).apiKeyId as string | undefined; | ||
| const coinbaseKeySecret = (credentials as any).apiKeySecret as string | undefined; | ||
|
|
||
| if (!coinbaseKeyId || !coinbaseKeySecret) { | ||
| resp.writeHead(403); | ||
| resp.end('crossmintApi credential missing Coinbase apiKeyId or apiKeySecret'); | ||
| return { noWebhookResponse: true }; | ||
| } | ||
| const facilitator = new FaremeterFacilitator(); |
There was a problem hiding this comment.
logic: Facilitator instantiated without credentials - FaremeterFacilitator is created with no parameters, but the Faremeter API likely requires authentication (the old Coinbase implementation passed apiKeyId and apiKeySecret). The credentials are available in scope but not passed to the facilitator. Either add a facilitatorApiKey field to credentials and pass it here, or document why Faremeter requires no authentication.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/CrossmintWebhook/executions/Webhooks.ts
Line: 38:39
Comment:
**logic:** Facilitator instantiated without credentials - `FaremeterFacilitator` is created with no parameters, but the Faremeter API likely requires authentication (the old Coinbase implementation passed `apiKeyId` and `apiKeySecret`). The credentials are available in scope but not passed to the facilitator. Either add a `facilitatorApiKey` field to credentials and pass it here, or document why Faremeter requires no authentication.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
I'm learning! Leave a 👍 or a 👎 to help me learn. Extra points if you leave a comment telling me why i'm wrong tagging @greptileai
3 files reviewed, no comments
There was a problem hiding this comment.
I'm learning! Leave a 👍 or a 👎 to help me learn. Extra points if you leave a comment telling me why i'm wrong tagging @greptileai
1 file reviewed, 4 comments
| import type { IPaymentPayload, PaymentRequirements } from '../types/x402Types'; | ||
| import type { IFacilitator } from './IFacilitator'; | ||
|
|
||
| const FAREMETER_BASE_URL = 'http://localhost:3000/api/v1'; |
There was a problem hiding this comment.
logic: Hardcoded localhost:3000 will fail in production - should be configurable production URL
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/CrossmintWebhook/executions/facilitator/faremeterFacilitator.ts
Line: 4:4
Comment:
**logic:** Hardcoded `localhost:3000` will fail in production - should be configurable production URL
How can I resolve this? If you propose a fix, please make it concise.| ): Promise<{ isValid: boolean; invalidReason?: string }> { | ||
| // Format for your local Faremeter API | ||
| const requestBody = { | ||
| facilitatorUrl: 'https://facilitator.corbits.dev', |
There was a problem hiding this comment.
logic: Hardcoded facilitatorUrl should be configurable or derived from environment
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/CrossmintWebhook/executions/facilitator/faremeterFacilitator.ts
Line: 17:17
Comment:
**logic:** Hardcoded `facilitatorUrl` should be configurable or derived from environment
How can I resolve this? If you propose a fix, please make it concise.| // Format for your local Faremeter API settle endpoint | ||
| const requestBody = { | ||
| paytoAddress: paymentRequirements.payTo, | ||
| facilitatorUrl: 'https://facilitator.corbits.dev', |
There was a problem hiding this comment.
logic: Hardcoded facilitatorUrl should be configurable or derived from environment
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/CrossmintWebhook/executions/facilitator/faremeterFacilitator.ts
Line: 102:102
Comment:
**logic:** Hardcoded `facilitatorUrl` should be configurable or derived from environment
How can I resolve this? If you propose a fix, please make it concise.| network: paymentRequirements.network, | ||
| asset: paymentRequirements.asset, | ||
| resource: paymentRequirements.resource, | ||
| paymentAmount: parseFloat(paymentRequirements.maxAmountRequired) / 1000000, // Convert atomic units to USDC |
There was a problem hiding this comment.
logic: Hardcoded division by 1000000 assumes USDC with 6 decimals - breaks if asset is a different token with different decimals
Prompt To Fix With AI
This is a comment left during a code review.
Path: nodes/CrossmintWebhook/executions/facilitator/faremeterFacilitator.ts
Line: 107:107
Comment:
**logic:** Hardcoded division by `1000000` assumes USDC with 6 decimals - breaks if `asset` is a different token with different decimals
How can I resolve this? If you propose a fix, please make it concise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor: remove Coinbase facilitator coupling and implement Faremeter
Summary
This PR refactors the x402 webhook node to remove hard coupling to Coinbase's facilitator API and implements Faremeter as the new facilitator. The changes maintain the x402 protocol schema while providing a cleaner abstraction layer for future facilitator implementations.
Key changes:
IFacilitatorinterface to abstract facilitator implementationsFaremeterFacilitatorclass with Faremeter API integrationapiKeyId/apiKeySecret) with genericfacilitatorApiKeyCoinbaseJWT.ts,coinbaseFacilitator.ts)Architecture:
Review & Testing Checklist for Human
This implementation is based on Faremeter documentation but has NOT been tested against the actual Faremeter API. Please verify:
api.faremeter.com,/v1/x402/verify, and/v1/x402/settlematch actual Faremeter APIfacilitatorApiKeyis correct formatAdditional considerations:
{ isValid, invalidReason }for verify and{ success, transaction: { hash }, errorReason }for settleTest Plan Recommendation
Notes
IFacilitatorinterface makes it easy to add other facilitator implementations in the future