feat: x402 middleware #6114
feat: x402 middleware #6114
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ 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 |
…t, registerX402Config()
|
Claude finished @odilitime's task —— View job PR Review: x402 MiddlewareTodo List:
Review Summary❌ CRITICAL: No tests for payment verification logic → Fix: Add comprehensive tests for all payment verification methods ( ❌ CRITICAL: Extensive use of
💡 SUGGESTION: Hardcoded payment addresses → Consider: Make addresses configurable per deployment 💡 SUGGESTION: Missing rate limiting → Consider: Add rate limiting for payment verification attempts 💡 SUGGESTION: No payment audit trail → Consider: Log successful payments for audit purposes 💡 SUGGESTION: Missing timeout handling → Consider: Add configurable timeouts for blockchain RPC calls Security AnalysisPayment Logic:
Architecture:
Critical Missing TestsThe payment verification logic handles real money transactions but has no test coverage:
This is unacceptable for financial code. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| if (authValue < expectedUnits) { | ||
| console.error('Amount too low:', authValue, 'vs', expectedUnits); | ||
| return false; |
There was a problem hiding this comment.
Bug: Incorrect EIP-712 Payment Exaggerates Price
EIP-712 payment verification incorrectly treats priceInCents as a dollar amount, causing the required payment to be calculated 100 times higher than intended, which rejects legitimate payments. Additionally, a missing null check on authorization.to could lead to runtime errors during recipient validation.
Additional Locations (1)
|
Claude finished @odilitime's task —— View job PR Review: x402 MiddlewareTodo List:
Review Summary❌ CRITICAL: No tests for payment verification logic → Fix: Add comprehensive tests for all payment verification methods ( ❌ CRITICAL: Extensive use of
💡 SUGGESTION: Hardcoded payment addresses → Consider: Make addresses configurable per deployment 💡 SUGGESTION: Missing rate limiting → Consider: Add rate limiting for payment verification attempts 💡 SUGGESTION: No payment audit trail → Consider: Log successful payments for audit purposes 💡 SUGGESTION: Missing timeout handling → Consider: Add configurable timeouts for blockchain RPC calls Security AnalysisPayment Logic:
Architecture:
Critical Missing TestsThe payment verification logic handles real money transactions but has no test coverage:
This is unacceptable for financial code. |
|
Claude finished @odilitime's task —— View job PR Review: x402 MiddlewareTodo List:
Review Summary❌ CRITICAL: No tests for payment verification logic → Fix: Add comprehensive tests for all payment verification methods ( ❌ CRITICAL: Extensive use of
💡 SUGGESTION: Hardcoded payment addresses → Consider: Make addresses configurable per deployment 💡 SUGGESTION: Missing rate limiting → Consider: Add rate limiting for payment verification attempts 💡 SUGGESTION: No payment audit trail → Consider: Log successful payments for audit purposes 💡 SUGGESTION: Missing timeout handling → Consider: Add configurable timeouts for blockchain RPC calls Security AnalysisPayment Logic:
Architecture:
Critical Missing TestsThe payment verification logic handles real money transactions but has no test coverage:
This is unacceptable for financial code. |
| if (tx.value < expectedUnits) { | ||
| logError('Transaction amount too low'); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Bug: ERC-20 Transfers Misverified by ETH Value Check
The verifyEvmTransaction function incorrectly verifies ERC-20 token transfers by checking tx.value, which represents native ETH value, not the ERC-20 token amount. For USDC transfers (ERC-20 tokens), tx.value is typically 0, while the actual token amount is encoded in the transaction's input data. This will cause all legitimate USDC payment verifications to fail because the code compares 0 (tx.value) against the expected USDC units (e.g., 100,000 for $0.10), which will always be less than expected.
To fix this, the code needs to decode the ERC-20 transfer function call from tx.input to extract the actual token amount being transferred, rather than checking tx.value.
|
Claude finished @odilitime's task —— View job PR Review: x402 MiddlewareTodo List:
Review Summary❌ CRITICAL: No tests for payment verification logic → Fix: Add comprehensive tests for all payment verification methods ( ❌ CRITICAL: Extensive use of
💡 SUGGESTION: Hardcoded payment addresses → Consider: Make addresses configurable per deployment 💡 SUGGESTION: Missing rate limiting → Consider: Add rate limiting for payment verification attempts 💡 SUGGESTION: No payment audit trail → Consider: Log successful payments for audit purposes 💡 SUGGESTION: Missing timeout handling → Consider: Add configurable timeouts for blockchain RPC calls Security AnalysisPayment Logic:
Architecture:
Critical Missing TestsThe payment verification logic handles real money transactions but has no test coverage for:
This is unacceptable for financial code. |
| 'SOLANA': 'solana', | ||
| 'POLYGON': 'polygon' | ||
| }; | ||
| return networkMap[network]; |
There was a problem hiding this comment.
Bug: Type Mismatch: Unknown Networks Break Mapping Interfaces
Type mismatch between Network type and Record<Network, ...> usage. The Network type is defined as a generic string in @elizaos/core, but toX402Network() uses Record<Network, X402ScanNetwork> which only contains mappings for 'BASE', 'SOLANA', and 'POLYGON'. When a custom network (like 'ARBITRUM') is passed to this function, networkMap[network] will return undefined, causing a runtime error. The same issue exists in PAYMENT_ADDRESSES (line 57) and NETWORK_ASSETS (line 131). These Records should either have an index signature to handle unknown networks, or the function should validate the network is one of the built-in types before accessing the Record, or provide a default/fallback value.
|
Claude finished @odilitime's task —— View job PR Review: x402 MiddlewareTodo List:
Review Summary❌ CRITICAL: EIP-712 payment calculation exaggerates price by 100x → Fix: Line 677 treats ❌ CRITICAL: ERC-20 verification checks wrong value → Fix: Line 587 checks ❌ CRITICAL: Type mismatch breaks network mapping → Fix: ❌ CRITICAL: Missing tests for payment verification logic → Fix: Financial code handling real money has zero test coverage for core verification methods (
💡 SUGGESTION: Remove development security bypasses → Consider: Restrict 💡 SUGGESTION: Add rate limiting → Consider: Implement rate limiting for payment verification attempts to prevent abuse 💡 SUGGESTION: Add audit logging → Consider: Log successful payments for compliance and debugging Security AnalysisPayment Logic:
Architecture:
Critical Math BugsEIP-712 Payment Bug ( const expectedUnits = expectedCents * 10000; // ❌ WRONG: 100x too high
// Should be: expectedCents * 100 (since $1 = 100 cents = 1,000,000 USDC units)ERC-20 Transfer Bug ( if (tx.value < expectedUnits) { // ❌ WRONG: tx.value is always 0 for ERC-20
// Should check decoded transfer amount from tx.input instead
}These bugs would cause:
Test Coverage Assessment✅ Strong test coverage overall (5 comprehensive test files) |
ChristopherTrimboli
left a comment
ChristopherTrimboli
left a comment
There was a problem hiding this comment.
plz address Math bugs as claude noted
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive x402 payment middleware to ElizaOS, enabling micropayment protection for plugin routes across multiple blockchains (Base, Polygon, Solana). The implementation includes cryptographic signature verification (EIP-712), payment validation, and x402scan protocol compliance.
Key changes:
- Adds payment protection middleware with multi-chain support (Base, Polygon, Solana)
- Implements EIP-712 signature verification for gasless ERC-3009 transfers
- Provides payment config registry for custom tokens/networks
- Includes comprehensive test coverage (51 tests) and documentation
Reviewed Changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
packages/server/src/middleware/x402/payment-wrapper.ts |
Core payment verification logic with EIP-712 signature recovery |
packages/server/src/middleware/x402/x402-types.ts |
Type definitions and validation for x402scan protocol compliance |
packages/server/src/middleware/x402/payment-config.ts |
Payment configuration registry and network mapping |
packages/server/src/middleware/x402/startup-validator.ts |
Startup validation for payment configs and routes |
packages/server/src/middleware/x402/types.ts |
Strict TypeScript types for x402 middleware |
packages/core/src/types/payment.ts |
Core payment types for plugin developers |
packages/server/src/api/index.ts |
Integration of payment middleware into route handler |
packages/server/package.json |
Added dependencies: viem, @solana/web3.js |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Null check before toLowerCase() | ||
| if (!authorization.to) { | ||
| console.error('Authorization missing "to" field'); | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Redundant null check at lines 662-666. The authorization.to field is already validated at line 651 in the same function scope, making this duplicate check unnecessary.
| // Null check before toLowerCase() | |
| if (!authorization.to) { | |
| console.error('Authorization missing "to" field'); | |
| return false; | |
| } |
| if (network === 'SOLANA') { | ||
| const solanaToken = Object.values(SOLANA_TOKENS).find(t => t.symbol === asset); | ||
| if (solanaToken) return solanaToken.decimals; | ||
| } | ||
| if (network === 'BASE') { | ||
| const baseToken = Object.values(BASE_TOKENS).find(t => t.symbol === asset); | ||
| if (baseToken) return baseToken.decimals; | ||
| } | ||
| if (network === 'POLYGON') { |
There was a problem hiding this comment.
Variable 'network' is of type undefined, but it is compared to an expression of type string.
| if (network === 'SOLANA') { | |
| const solanaToken = Object.values(SOLANA_TOKENS).find(t => t.symbol === asset); | |
| if (solanaToken) return solanaToken.decimals; | |
| } | |
| if (network === 'BASE') { | |
| const baseToken = Object.values(BASE_TOKENS).find(t => t.symbol === asset); | |
| if (baseToken) return baseToken.decimals; | |
| } | |
| if (network === 'POLYGON') { | |
| if (network && network === 'SOLANA') { | |
| const solanaToken = Object.values(SOLANA_TOKENS).find(t => t.symbol === asset); | |
| if (solanaToken) return solanaToken.decimals; | |
| } | |
| if (network && network === 'BASE') { | |
| const baseToken = Object.values(BASE_TOKENS).find(t => t.symbol === asset); | |
| if (baseToken) return baseToken.decimals; | |
| } | |
| if (network && network === 'POLYGON') { |
| if (network === 'SOLANA') { | ||
| const solanaToken = Object.values(SOLANA_TOKENS).find(t => t.symbol === asset); | ||
| if (solanaToken) return solanaToken.decimals; | ||
| } | ||
| if (network === 'BASE') { | ||
| const baseToken = Object.values(BASE_TOKENS).find(t => t.symbol === asset); | ||
| if (baseToken) return baseToken.decimals; | ||
| } | ||
| if (network === 'POLYGON') { |
There was a problem hiding this comment.
Variable 'network' is of type undefined, but it is compared to an expression of type string.
| if (network === 'SOLANA') { | |
| const solanaToken = Object.values(SOLANA_TOKENS).find(t => t.symbol === asset); | |
| if (solanaToken) return solanaToken.decimals; | |
| } | |
| if (network === 'BASE') { | |
| const baseToken = Object.values(BASE_TOKENS).find(t => t.symbol === asset); | |
| if (baseToken) return baseToken.decimals; | |
| } | |
| if (network === 'POLYGON') { | |
| if (network !== undefined && network === 'SOLANA') { | |
| const solanaToken = Object.values(SOLANA_TOKENS).find(t => t.symbol === asset); | |
| if (solanaToken) return solanaToken.decimals; | |
| } | |
| if (network !== undefined && network === 'BASE') { | |
| const baseToken = Object.values(BASE_TOKENS).find(t => t.symbol === asset); | |
| if (baseToken) return baseToken.decimals; | |
| } | |
| if (network !== undefined && network === 'POLYGON') { |
| if (typeof paymentData !== 'object' || paymentData === null) { | ||
| logError('Invalid payment data: must be an object'); |
There was a problem hiding this comment.
Variable 'paymentData' is of type date, object or regular expression, but it is compared to an expression of type null.
| if (typeof paymentData !== 'object' || paymentData === null) { | |
| logError('Invalid payment data: must be an object'); | |
| if ( | |
| typeof paymentData !== 'object' || | |
| paymentData === null || | |
| Object.getPrototypeOf(paymentData) !== Object.prototype | |
| ) { | |
| logError('Invalid payment data: must be a plain object'); |
| if (paymentAwareHandler) { | ||
| paymentAwareHandler(req, res, runtime); | ||
| } |
There was a problem hiding this comment.
This guard always evaluates to true.
| if (paymentAwareHandler) { | |
| paymentAwareHandler(req, res, runtime); | |
| } | |
| paymentAwareHandler(req, res, runtime); |
| if (paymentAwareHandler) { | ||
| paymentAwareHandler(req, res, runtime); | ||
| } |
There was a problem hiding this comment.
This guard always evaluates to true.
| if (paymentAwareHandler) { | |
| paymentAwareHandler(req, res, runtime); | |
| } | |
| paymentAwareHandler(req, res, runtime); |
| * Tests all payment verification methods and security features | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach, mock } from 'bun:test'; |
There was a problem hiding this comment.
Unused imports beforeEach, mock.
| import { describe, it, expect, beforeEach, mock } from 'bun:test'; | |
| import { describe, it, expect } from 'bun:test'; |
| describe('Input Sanitization', () => { | ||
| it('should reject payment ID with invalid characters', () => { | ||
| // Test sanitizePaymentId indirectly via verification | ||
| const runtime = createMockRuntime(); |
There was a problem hiding this comment.
Unused variable runtime.
| const runtime = createMockRuntime(); |
| * Tests actual verification logic with mocked blockchain/facilitator responses | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach, mock } from 'bun:test'; |
There was a problem hiding this comment.
Unused imports beforeEach, mock.
| import { describe, it, expect, beforeEach, mock } from 'bun:test'; | |
| import { describe, it, expect } from 'bun:test'; |
pulled enhanced response & health check from Otaku
Note
Introduces x402 multi-chain micropayment protection, integrates it into plugin routes, and adds core types, tests, and docs.
packages/core/src/types/payment.tswithPaymentEnabledRoute,X402Config, validators, and OpenAPI helpers; export viatypes/index.ts.middleware/x402(payment config/registry, EIP‑712/Solana verification, 402 response schema, startup validator, types, README).middleware/index.tsand packagesrc/index.tsfor plugin use.src/api/index.ts) viacreatePaymentAwareHandlerfor routes declaringx402.viemand@solana/web3.js; includecorsin server deps.getX402Health).Written by Cursor Bugbot for commit 644b9b4. This will update automatically on new commits. Configure here.