feat(v2): implement x402 v2 protocol with multi-chain support #25
Conversation
Implement Phase 4 of v2 protocol - EVM signer with EIP-3009 authorization: - v2/internal/eip3009: EIP-712 typed data and signature creation utilities - v2/signers/evm: Signer implementation for Ethereum-compatible chains - NewSigner/NewSignerFromKey constructors - CanSign/Sign methods for payment payload generation - Support for max amount limits via WithMaxAmount option - CAIP-2 network identifier support Tests use Foundry/Anvil default test key for consistency and clarity.
Implement the v2 SVM (Solana) signer for the x402 protocol v2. New files: - v2/internal/solana/solana.go - Utility functions for Solana - BuildTransferCheckedInstruction: SPL Token TransferChecked builder - BuildSetComputeUnitLimitInstruction: Compute budget instruction - BuildSetComputeUnitPriceInstruction: Priority fee instruction - DeriveAssociatedTokenAddress: ATA derivation - GetRPCURL: CAIP-2 network to RPC URL mapping - v2/signers/svm/signer.go - SVM Signer implementation - NewSigner: Create from base58 private key - NewSignerFromKey: Create from existing key - NewSignerFromKeygenFile: Create from Solana keygen JSON file - Full v2.Signer interface implementation - CAIP-2 network validation (rejects EVM networks) - Fee payer extraction from requirements.Extra Key differences from v1: - Uses CAIP-2 network identifiers (e.g., solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp) - Returns v2.SVMPayload type directly - Uses v2.PaymentRequirements with Amount field - Sets X402Version to 2 This completes Phase 5 of the v2 implementation plan.
…port Implements Phase 6 of the v2 protocol implementation plan. New files: - v2/http/internal/helpers/helpers.go - HTTP helper functions - ParsePaymentHeader, SendPaymentRequired, AddPaymentResponseHeader - ParsePaymentRequirements, ParseSettlement, BuildPaymentHeader - BuildResourceURL for constructing resource URLs from requests - v2/http/middleware.go - HTTP payment middleware - Config struct with facilitator options, authorization, and hooks - NewX402Middleware factory with automatic requirement enrichment - settlementInterceptor for deferred settlement on success - Support for verify-only mode and fallback facilitators - GetPaymentFromContext for handler access to payment info - v2/http/transport.go - HTTP transport with 402 handling - X402Transport implementing http.RoundTripper - Automatic 402 detection and payment retry - Payment callbacks for monitoring (attempt, success, failure) - v2/http/client.go - HTTP client wrapper - NewClient with functional options pattern - WithSigner, WithSelector, WithHTTPClient options - WithPaymentCallback, WithPaymentCallbacks options - GetSettlement helper for extracting settlement from response All tests pass with race detection enabled.
Implements Phase 7 of the v2 protocol plan. - Add v2/http/gin/middleware.go with Gin-compatible middleware - Support CAIP-2 network identifiers (e.g., eip155:84532) - Add PaymentContextKey for storing verified payment in Gin context - Add GetPaymentFromContext helper function - Support verify-only mode - Support primary and fallback facilitators - Add comprehensive test coverage The middleware follows the same pattern as v1 Gin middleware but uses v2 types with x402Version: 2 and restructured PaymentRequired/PaymentPayload.
Implements Phase 8 of the v2 protocol plan - MCP (Model Context Protocol) integration for payment-gated AI tools. Server-side components: - v2/mcp/types.go: MCP-specific payment requirements type (v2 format) - v2/mcp/errors.go: MCP-specific error types and payment error wrapper - v2/mcp/server/config.go: Server configuration with tool payment mapping - v2/mcp/server/server.go: X402Server wrapping MCP server with payment support - v2/mcp/server/handler.go: HTTP handler for payment verification/settlement - v2/mcp/server/facilitator.go: HTTP facilitator client for MCP server - v2/mcp/server/requirements.go: Requirement validation for MCP tools Client-side components: - v2/mcp/client/config.go: Client configuration with signers and callbacks - v2/mcp/client/transport.go: Transport wrapper for automatic 402 handling Key v2 protocol features: - CAIP-2 network identifiers (e.g., eip155:84532) - Resource info object in payment requirements - Extensions passthrough support - Payment in _meta['x402/payment'] for tool calls - Settlement response in _meta['x402/payment-response'] in results Tests cover: - Free and paid tool handling - Payment verification and settlement flow - VerifyOnly mode - Tool execution error handling (no settlement) - Payment requirements extraction (v2 format) - Configuration options
Add Phase 9 examples demonstrating v2 protocol usage: - examples/v2/http/main.go: HTTP server/client with payment middleware - examples/v2/gin/main.go: Gin framework integration example - examples/v2/mcp/main.go: MCP server with paid tools and client All examples demonstrate: - CAIP-2 network identifiers (e.g., eip155:84532) - v2 PaymentRequirements configuration - EVM and SVM signer creation - Automatic payment handling - Payment event callbacks This completes Phase 9 of the v2 implementation plan.
Add unit tests for the internal eip3009 package covering: - GenerateNonce: randomness, uniqueness, non-zero values - CreateAuthorization: field population, time bounds, nonce uniqueness - SignAuthorization: signature format, determinism, chain separation These tests ensure the cryptographic foundations of EVM payment signing work correctly across different chains, tokens, and amounts.
📝 WalkthroughWalkthroughThis PR adds a complete x402 v2 implementation: protocol types, chain configs, signers (EVM/Solana), selector, encoding/validation, HTTP and MCP transports/middleware, facilitator client, Gin/PocketBase integrations, MCP server/client glue, examples, and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Server as HTTP Server
participant Facilitator as Facilitator
participant Signer as Signer
Client->>Server: GET /data
Server-->>Client: 402 Payment Required (PaymentRequired)
Client->>Signer: SelectAndSign(requirements)
Signer-->>Client: PaymentPayload
Client->>Server: GET /data + X-PAYMENT header
Server->>Facilitator: Verify(PaymentPayload, requirements)
Facilitator-->>Server: VerifyResponse
alt Verify Success
Server->>Facilitator: Settle(PaymentPayload, requirements)
Facilitator-->>Server: SettleResponse
Server-->>Client: 200 OK + X-PAYMENT-RESPONSE (settlement)
else Verify Failed
Server-->>Client: 402 Payment Required
end
sequenceDiagram
participant MCPClient as MCP Client
participant MCPServer as MCP Server
participant Facilitator as Facilitator
participant Signer as Signer
MCPClient->>MCPServer: JSON-RPC call (no _meta.x402)
MCPServer-->>MCPClient: JSON-RPC error 402 (PaymentRequirements)
MCPClient->>Signer: SelectAndSign(requirements)
Signer-->>MCPClient: PaymentPayload
MCPClient->>MCPServer: JSON-RPC call (_meta.x402.payment=PaymentPayload)
MCPServer->>Facilitator: Verify(PaymentPayload, requirements)
Facilitator-->>MCPServer: VerifyResponse
alt Verify Success
MCPServer->>Facilitator: Settle(PaymentPayload, requirements)
Facilitator-->>MCPServer: SettleResponse
MCPServer-->>MCPClient: JSON-RPC result (_meta.x402.payment-response=settlement)
else Verify Failed
MCPServer-->>MCPClient: JSON-RPC error 402
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
- Fix errcheck: add explicit error handling for json.Encode and w.Write calls - Fix staticcheck SA5011: use t.Fatal instead of t.Error for nil checks - Fix staticcheck SA4006: properly use nonce variable in eip3009_test.go - Remove unused mockSigner2 type and math/big import from client_test.go
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @v2/http/facilitator.go:
- Around line 298-336: EnrichRequirements currently uses context.Background(),
preventing callers from controlling cancellation/timeouts; change the method
signature of FacilitatorClient.EnrichRequirements to accept a context.Context as
the first parameter (e.g., EnrichRequirements(ctx context.Context, requirements
[]v2.PaymentRequirements) ([]v2.PaymentRequirements, error)), replace the
internal call to c.Supported(context.Background()) with c.Supported(ctx), and
propagate this new ctx to all callers and unit tests (update any call sites and
mocks) so cancellation and deadlines flow through; keep the rest of the merging
logic unchanged.
In @v2/http/gin/middleware.go:
- Around line 60-88: The http.Client instances inside NewX402Middleware are
created without timeouts, so set each Client's Timeout to the request timeout
from the provided config (use config.Timeouts.RequestTimeout or equivalent) when
constructing the v2http.FacilitatorClient for both facilitator and
fallbackFacilitator; update the Client field assignment in the
v2http.FacilitatorClient initializers (in NewX402Middleware) to use
&http.Client{Timeout: config.Timeouts.RequestTimeout} so requests cannot hang
indefinitely.
In @v2/http/middleware.go:
- Around line 287-293: The Hijack implementation on settlementInterceptor
currently delegates to i.w without ensuring the response is settled, which can
allow a client-visible success before settlement; update
settlementInterceptor.Hijack to first ensure settlement (e.g., call the
interceptor's settle/commit method or a new settleIfNeeded helper that checks
the committed state) and return any settlement error, then only if settlement
succeeds proceed to assert i.w as http.Hijacker and call Hijack; if the writer
does not implement http.Hijacker, return an appropriate error after attempting
settlement.
In @v2/mcp/client/config.go:
- Around line 40-45: The WithSigner option currently appends a v2.Signer without
checking for nil, which can introduce nil-interface entries in Config.Signers;
update the WithSigner function to check if signer == nil and simply return
without modifying c.Signers when nil (i.e., only append when signer is non-nil)
so callers who accidentally pass nil won’t create a nil entry that breaks
signing later.
In @v2/mcp/client/transport.go:
- Around line 58-74: When handling a 402 response, don't marshal a nil
resp.Error.Data into json.RawMessage (which leads to an "unexpected end of JSON
input" unmarshal error); instead check if resp.Error.Data == nil and immediately
return ErrNoPaymentRequirements (or call t.extractPaymentRequirements with a
nil/empty indicator that yields ErrNoPaymentRequirements). Update the 402
handling blocks that build `data` (the one around the resp.Error.Data check and
the similar block at lines 114-120) to return the clearer
ErrNoPaymentRequirements when resp.Error.Data is nil, referencing the existing
function t.extractPaymentRequirements and the ErrNoPaymentRequirements sentinel.
In @v2/mcp/server/config.go:
- Around line 97-107: GetPaymentConfig currently takes toolName and returns
&config where config is the map value copy, so callers get a pointer to a local
copy (misleading). Fix by choosing one approach: (A) change the signature to
GetPaymentConfig(toolName string) (ToolPaymentConfig, bool) and return the value
and existence flag (preferred for read-only); or (B) change c.PaymentTools to
map[string]*ToolPaymentConfig and return the stored pointer from
GetPaymentConfig(toolName string) *ToolPaymentConfig (update map initialization
and all callers to expect pointer semantics). Update all call sites to match the
new return type/semantics.
In @v2/mcp/server/handler.go:
- Around line 252-277: After unmarshalling into v2.PaymentPayload in
extractPayment, validate the payment.X402Version field against the supported
version(s) before returning (or before any verify/settle logic uses it); if the
version is missing or unsupported, return nil (or an error) so downstream
verify/settle routines do not operate on an incompatible payload. Update
extractPayment (and any callers that immediately verify/settle) to reference
v2.PaymentPayload and payment.X402Version and perform this check against your
canonical supported constant (e.g., X402SupportedVersion) before proceeding.
- Around line 72-79: The initializeFacilitators function currently panics on
missing config; change it to return an error instead by updating its signature
to (Facilitator, Facilitator, error), replace the panic("x402: ...") with a
formatted error (e.g., fmt.Errorf("x402: at least one facilitator URL must be
provided")), and return nil, nil, err; update any callers (notably the
X402Handler constructor) to accept the error and change the constructor
signature to return (*X402Handler, error), propagating the error up so library
users can handle configuration failures instead of crashing.
- Around line 389-411: Change the verify-only branch (where settleResp == nil)
to reflect verification success instead of marking settlement-skipped as
failure: when verifyResp indicates verification succeeded, set
meta["x402/payment-response"] to a v2.SettleResponse with Success: true,
Network: payment.Accepted.Network, Payer from verifyResp (and omit or leave
Transaction empty); only set Success: false and include an errorReason when
verifyResp indicates an actual verification failure. Update the code around
jsonrpcResp.Result handling (the block using settleResp, verifyResp,
v2.SettleResponse, and payment.Accepted.Network) to implement this logic.
In @v2/signers/svm/signer.go:
- Around line 199-214: The RPC call to fetch a recent blockhash can hang because
it uses context.Background(); wrap the call in a context with timeout (e.g.,
context.WithTimeout) and pass that ctx to client.GetLatestBlockhash, then defer
cancel() to ensure cancellation; keep using solutil.GetRPCURL and rpc.New for
client creation but replace the plain Background context with the new timed
context around the GetLatestBlockhash call (reference the GetLatestBlockhash
method and the surrounding client variable and ctx usage).
- Around line 283-314: Insert an idempotent ATA-creation instruction for destATA
before the TransferChecked instruction in buildPartiallySignedTransfer: call the
appropriate helper (e.g., a CreateAssociatedTokenAccountIdempotent or
BuildCreateAssociatedTokenAccountInstruction) that sets feePayer as the payer
and recipient as the owner for mint, add that instruction to the instructions
slice immediately before the BuildTransferCheckedInstruction so the transaction
will create the recipient ATA (if needed) in the same transaction and let the
facilitator sponsor the rent-exempt balance.
🟡 Minor comments (10)
v2/http/facilitator_test.go-206-209 (1)
206-209: Unchecked error return from json.Encode.Static analysis flagged that the error return from
json.NewEncoder(w).Encode(response)is not checked. While unlikely to fail in tests, it's good practice to check.🔧 Suggested fix
response := v2.VerifyResponse{IsValid: true} w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(response) + if err := json.NewEncoder(w).Encode(response); err != nil { + t.Errorf("Failed to encode response: %v", err) + } }))This pattern appears in multiple mock handlers throughout the test file. Consider applying consistent error handling to all instances.
v2/http/transport.go-36-41 (1)
36-41: Potential race condition when settingt.Base.Assigning
t.Base = http.DefaultTransportmutates the transport struct. IfRoundTripis called concurrently on the sameX402Transportinstance, this could cause a data race. Consider initializingBasein a constructor or using a local variable.🔒️ Suggested fix
func (t *X402Transport) RoundTrip(req *http.Request) (*http.Response, error) { // Ensure we have a base transport - if t.Base == nil { - t.Base = http.DefaultTransport - } + base := t.Base + if base == nil { + base = http.DefaultTransport + } // Clone the request to avoid modifying the original reqCopy := req.Clone(req.Context()) // Make the first attempt - resp, err := t.Base.RoundTrip(reqCopy) + resp, err := base.RoundTrip(reqCopy)And update line 119 similarly:
- respRetry, err := t.Base.RoundTrip(reqRetry) + respRetry, err := base.RoundTrip(reqRetry)v2/mcp/client/config.go-37-38 (1)
37-38: Fix misleading docstring: Option configuresConfig, notTransport.
Line 37 says “configuring the Transport” butOptionisfunc(*Config).v2/mcp/client/transport.go-3-13 (1)
3-13: Import grouping doesn’t match repo guidelines (stdlib / external / internal).
Please separategithub.com/mark3labs/mcp-go/...(external) fromgithub.com/mark3labs/x402-go/...(internal) with a blank line.v2/internal/solana/solana.go-43-56 (1)
43-56: Preferencoding/binaryfor little-endian packing (safer + clearer).Proposed refactor
import ( + "encoding/binary" "fmt" "github.com/gagliardetto/solana-go" "github.com/gagliardetto/solana-go/programs/token" "github.com/gagliardetto/solana-go/rpc" v2 "github.com/mark3labs/x402-go/v2" ) @@ func BuildSetComputeUnitLimitInstruction(units uint32) solana.Instruction { data := make([]byte, 5) data[0] = 2 // SetComputeUnitLimit discriminator - data[1] = byte(units) - data[2] = byte(units >> 8) - data[3] = byte(units >> 16) - data[4] = byte(units >> 24) + binary.LittleEndian.PutUint32(data[1:], units) @@ func BuildSetComputeUnitPriceInstruction(microlamports uint64) solana.Instruction { data := make([]byte, 9) data[0] = 3 // SetComputeUnitPrice discriminator - data[1] = byte(microlamports) - data[2] = byte(microlamports >> 8) - data[3] = byte(microlamports >> 16) - data[4] = byte(microlamports >> 24) - data[5] = byte(microlamports >> 32) - data[6] = byte(microlamports >> 40) - data[7] = byte(microlamports >> 48) - data[8] = byte(microlamports >> 56) + binary.LittleEndian.PutUint64(data[1:], microlamports)Also applies to: 61-78
v2/http/facilitator.go-361-374 (1)
361-374:extractPayershould handle the concretev2.EVMPayloadtype (not only decoded maps).Proposed fix
func extractPayer(payload v2.PaymentPayload) string { - // Try to extract from EVM payload - if evmPayload, ok := payload.Payload.(map[string]interface{}); ok { - if auth, ok := evmPayload["authorization"].(map[string]interface{}); ok { - if from, ok := auth["from"].(string); ok { - return from - } - } - } + switch p := payload.Payload.(type) { + case v2.EVMPayload: + return p.Authorization.From + case map[string]interface{}: + if auth, ok := p["authorization"].(map[string]interface{}); ok { + if from, ok := auth["from"].(string); ok { + return from + } + } + } // For SVM payloads, the payer would need to be extracted from the transaction // which requires decoding. The facilitator should provide this in the response. return "" }v2/mcp/server/facilitator.go-105-123 (1)
105-123: Nil-checkpaymentbefore dereferencing in Verify/Settle.Proposed fix
func (f *HTTPFacilitator) Verify(ctx context.Context, payment *v2.PaymentPayload, requirement v2.PaymentRequirements) (*v2.VerifyResponse, error) { + if payment == nil { + return nil, fmt.Errorf("facilitator verify failed: %w", v2.ErrMalformedHeader) + } resp, err := f.client.Verify(ctx, *payment, requirement)v2/http/internal/helpers/helpers.go-42-45 (1)
42-45: Unchecked error return from JSON encoding.The return value of
json.NewEncoder(w).Encode(response)is not checked. While encoding to aResponseWriterrarely fails, ignoring the error could mask issues in edge cases.🔧 Proposed fix
w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusPaymentRequired) - json.NewEncoder(w).Encode(response) + if err := json.NewEncoder(w).Encode(response); err != nil { + // Log the error; response headers are already sent so we can't change status + return + } }Alternatively, if logging is preferred:
if err := json.NewEncoder(w).Encode(response); err != nil { // Consider logging: the client may have disconnected _ = err // suppress lint if logging not available }v2/http/client_test.go-286-319 (1)
286-319: Remove unusedmockSigner2type.This type is defined but never used. Since both test files are in the same
httppackage,mockSignerfromtransport_test.gois accessible here. Remove this duplicate to fix the linter warning.🔧 Proposed fix
-// mockSigner2 is a duplicate for this file since we can't import from transport_test.go -type mockSigner2 struct { - network string - scheme string - tokens []v2.TokenConfig - maxAmount *big.Int - priority int - signFunc func(*v2.PaymentRequirements) (*v2.PaymentPayload, error) -} - -func (m *mockSigner2) Network() string { return m.network } -func (m *mockSigner2) Scheme() string { return m.scheme } -func (m *mockSigner2) GetPriority() int { return m.priority } -func (m *mockSigner2) GetTokens() []v2.TokenConfig { return m.tokens } -func (m *mockSigner2) GetMaxAmount() *big.Int { return m.maxAmount } -func (m *mockSigner2) CanSign(req *v2.PaymentRequirements) bool { - return req.Network == m.network && req.Scheme == m.scheme -} -func (m *mockSigner2) Sign(req *v2.PaymentRequirements) (*v2.PaymentPayload, error) { - if m.signFunc != nil { - return m.signFunc(req) - } - return &v2.PaymentPayload{ - X402Version: 2, - Accepted: v2.PaymentRequirements{ - Scheme: req.Scheme, - Network: req.Network, - Amount: req.Amount, - }, - Payload: map[string]interface{}{ - "signature": "0xmocksig", - }, - }, nil -}Also remove the unused
"math/big"import if no longer needed after this removal.v2/http/client_test.go-181-214 (1)
181-214: Fix unchecked error returns in test handler (pipeline failure).The pipeline is failing due to unchecked
json.NewEncoder(w).Encode()at line 199. While in test code, fixing this ensures the linter passes and prevents silent test failures.🔧 Proposed fix
w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusPaymentRequired) - json.NewEncoder(w).Encode(paymentReq) + if err := json.NewEncoder(w).Encode(paymentReq); err != nil { + t.Errorf("Failed to encode payment request: %v", err) + } return } settlement := v2.SettleResponse{ Success: true, Transaction: "0x1234567890abcdef", Network: "eip155:84532", Payer: "0xPayerAddress", } encoded, _ := encoding.EncodeSettlement(settlement) w.Header().Set("X-PAYMENT-RESPONSE", encoded) w.WriteHeader(http.StatusOK) - w.Write([]byte("Protected content")) + _, _ = w.Write([]byte("Protected content")) }))
🧹 Nitpick comments (30)
examples/v2/mcp/main.go (1)
198-206: Consider validating type assertions for handler arguments.The type assertion
message, _ := args["message"].(string)silently ignores if the value is not a string. While this is example code, it could lead to empty strings being echoed without any indication of a problem.💡 Suggested improvement
func echoHandler(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { args := req.GetArguments() - message, _ := args["message"].(string) + message, ok := args["message"].(string) + if !ok || message == "" { + return &mcp.CallToolResult{ + Content: []mcp.Content{ + mcp.NewTextContent("Error: message argument is required"), + }, + IsError: true, + }, nil + } return &mcp.CallToolResult{ Content: []mcp.Content{ mcp.NewTextContent(fmt.Sprintf("Echo (x402 v2): %s", message)), }, }, nil }v2/mcp/client/transport_test.go (1)
122-126: Unused transport variable.The
Transportinstance on line 123 is created but never used. The test manually implements the injection logic instead of calling the transport's method. Consider either removing the unused variable or testing the actual transport method if available.💡 Suggested fix
func TestInjectPaymentMeta(t *testing.T) { - _ = &Transport{ - config: DefaultConfig("http://example.com"), - } - payment := &v2.PaymentPayload{v2/internal/eip3009/eip3009_test.go (1)
416-430: Consider removing or enhancing the struct field test.This test only verifies compile-time correctness (field types), which the compiler already guarantees. If kept, consider adding runtime assertions for expected zero-value behavior or remove this test as it provides no runtime validation value.
v2/config.go (1)
45-58: Consider validating RequestTimeout.The
Validate()method checksVerifyTimeoutandSettleTimeoutbut doesn't validateRequestTimeout. Consider adding:
RequestTimeout > 0check for consistency- Optionally,
RequestTimeout >= SettleTimeoutto ensure the overall request timeout accommodates settlement operations.♻️ Proposed enhancement
func (tc TimeoutConfig) Validate() error { if tc.VerifyTimeout <= 0 { return fmt.Errorf("verify timeout must be positive, got %v", tc.VerifyTimeout) } if tc.SettleTimeout <= 0 { return fmt.Errorf("settle timeout must be positive, got %v", tc.SettleTimeout) } + if tc.RequestTimeout <= 0 { + return fmt.Errorf("request timeout must be positive, got %v", tc.RequestTimeout) + } if tc.SettleTimeout < tc.VerifyTimeout { return fmt.Errorf("settle timeout (%v) should be >= verify timeout (%v)", tc.SettleTimeout, tc.VerifyTimeout) } + if tc.RequestTimeout < tc.SettleTimeout { + return fmt.Errorf("request timeout (%v) should be >= settle timeout (%v)", + tc.RequestTimeout, tc.SettleTimeout) + } return nil }v2/signers/evm/signer_test.go (1)
160-188: Good error path test for amount limits.Validates that
ErrAmountExceededis returned when the amount exceeds the configured max. Consider usingerrors.Is()for error comparison for more robust error checking.♻️ Proposed improvement
+import "errors" + func TestSignAmountExceeded(t *testing.T) { // ... existing setup ... _, err = signer.Sign(requirements) - if err != v2.ErrAmountExceeded { + if !errors.Is(err, v2.ErrAmountExceeded) { t.Errorf("Expected ErrAmountExceeded, got %v", err) } }v2/selector_test.go (1)
204-241: Good max amount limit test.Tests both the rejection path (amount exceeds limit) and success path (amount within limit). Consider adding an explicit assertion on the error code for the exceeded case for consistency with other tests.
♻️ Proposed enhancement for consistency
_, err := selector.SelectAndSign([]Signer{signer}, requirements) if err == nil { t.Error("SelectAndSign() should fail when amount exceeds limit") } + var paymentErr *PaymentError + if errors.As(err, &paymentErr) { + if paymentErr.Code != ErrCodeNoValidSigner { + t.Errorf("error code = %s, want %s", paymentErr.Code, ErrCodeNoValidSigner) + } + }v2/http/gin/middleware.go (1)
90-98: Enrichment error handling could be improved.When enrichment fails, the middleware logs a warning and continues with original requirements. This is reasonable, but consider whether this should be configurable behavior—some deployments may want to fail fast if the facilitator is unreachable at startup.
v2/chains_test.go (1)
362-374: Consider usingstrings.Containsfrom stdlib.The custom
containsStringandcontainsSubstringhelpers duplicate functionality provided bystrings.Contains. Using the stdlib function would be simpler and more idiomatic.♻️ Suggested simplification
+import ( + "errors" + "strings" + "testing" +) -// Helper function -func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr)) -} - -func containsSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} +// Helper function +func containsString(s, substr string) bool { + return strings.Contains(s, substr) +}v2/http/middleware_test.go (1)
143-157: Check error return value ofw.Write.Static analysis indicates the error return value of
w.Writeis not checked. While this is in test code and unlikely to fail, addressing it would satisfy the linter.♻️ Suggested fix
w.WriteHeader(http.StatusOK) - w.Write([]byte("OK")) + _, _ = w.Write([]byte("OK")) }))v2/signers/svm/signer_test.go (2)
96-111: Consider usingerrors.Isfor sentinel error checking.The test uses
errTargetbut doesn't actually verify the error wrapping witherrors.Is. CurrentlyerrTargetis set but not used in error comparison.♻️ Suggested improvement
t.Run(tt.name, func(t *testing.T) { signer, err := NewSigner(tt.network, tt.key, tt.tokens, tt.opts...) if tt.wantErr { if err == nil { t.Fatal("expected error, got nil") } + if tt.errTarget != nil && !errors.Is(err, tt.errTarget) { + t.Errorf("expected error wrapping %v, got %v", tt.errTarget, err) + } return }Don't forget to add
"errors"to imports.
708-720: Consider usingstrings.Containsfrom stdlib.Similar to the chains_test.go file, these helper functions duplicate stdlib functionality.
♻️ Suggested simplification
+import ( + "strings" + // ... other imports +) // Helper function to check if error message contains expected string func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || indexOfSubstring(s, substr) >= 0) + return strings.Contains(s, substr) } - -func indexOfSubstring(s, substr string) int { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return i - } - } - return -1 -}v2/validation/validation.go (1)
94-107:url.Parserarely returns errors - consider additional validation.
url.Parseis very permissive and will parse almost any string without error. Consider additional validation such as checking for a valid scheme (http/https/mcp) if stricter URL validation is needed.♻️ Suggested enhancement
func ValidateResourceInfo(resource v2.ResourceInfo) error { if resource.URL == "" { return fmt.Errorf("resource URL cannot be empty") } // Validate URL format - if _, err := url.Parse(resource.URL); err != nil { + parsed, err := url.Parse(resource.URL) + if err != nil { return fmt.Errorf("invalid resource URL: %w", err) } + + // Optionally validate scheme + if parsed.Scheme == "" { + return fmt.Errorf("resource URL must have a scheme") + } return nil }v2/http/transport.go (1)
138-160: Consider emitting failure callback when settlement indicates failure.Currently,
OnPaymentSuccessis only called whensettlement.Successis true, but there's no corresponding failure callback when settlement exists but indicates failure. This could leave payment failures unobserved.♻️ Suggested enhancement
// Parse settlement response settlement := helpers.ParseSettlement(respRetry.Header.Get("X-PAYMENT-RESPONSE")) // Trigger success callback if settlement indicates success if settlement != nil && settlement.Success && t.OnPaymentSuccess != nil { event := v2.PaymentEvent{ Type: v2.PaymentEventSuccess, Timestamp: time.Now(), Method: "HTTP", URL: req.URL.String(), Transaction: settlement.Transaction, Payer: settlement.Payer, Duration: duration, } if selectedRequirement != nil { event.Network = selectedRequirement.Network event.Scheme = selectedRequirement.Scheme event.Amount = selectedRequirement.Amount event.Asset = selectedRequirement.Asset event.Recipient = selectedRequirement.PayTo } t.OnPaymentSuccess(event) + } else if settlement != nil && !settlement.Success && t.OnPaymentFailure != nil { + event := v2.PaymentEvent{ + Type: v2.PaymentEventFailure, + Timestamp: time.Now(), + Method: "HTTP", + URL: req.URL.String(), + Error: fmt.Errorf("settlement failed: %s", settlement.ErrorReason), + Duration: duration, + } + t.OnPaymentFailure(event) }Note: You'll need to add
"fmt"to the imports.v2/validation/validation_test.go (1)
83-89: Test naming nit: “EVM mainnet/testnet” labels don’t match the chain IDs used.
eip155:8453/eip155:84532are Base mainnet / Base Sepolia, not Ethereum mainnet/testnet.v2/mcp/client/transport.go (2)
141-183: Usepayment.Acceptedfor the attempt callback (avoid ambiguous “re-find” logic).
Matching only onNetwork+Schemecan pick the wrong requirement when multiple accepts share those fields.payment.Acceptedis already the selected requirement.Proposed simplification
// Use selector to choose signer and create payment payment, err := t.config.Selector.SelectAndSign(t.config.Signers, requirements) @@ - // Find the requirement that was actually selected by matching the payment's network and scheme - // This ensures the payment attempt event reflects the actual requirement that was chosen - var selectedReq *v2.PaymentRequirements - for i := range requirements { - if requirements[i].Network == payment.Accepted.Network && requirements[i].Scheme == payment.Accepted.Scheme { - selectedReq = &requirements[i] - break - } - } - - // Trigger payment attempt callback with the actually selected requirement - if t.config.OnPaymentAttempt != nil && selectedReq != nil { + // Trigger payment attempt callback with the selected requirement + if t.config.OnPaymentAttempt != nil { t.config.OnPaymentAttempt(v2.PaymentEvent{ Type: v2.PaymentEventAttempt, Timestamp: startTime, Method: "MCP", - Amount: selectedReq.Amount, - Asset: selectedReq.Asset, - Network: selectedReq.Network, - Recipient: selectedReq.PayTo, - Scheme: selectedReq.Scheme, + Amount: payment.Accepted.Amount, + Asset: payment.Accepted.Asset, + Network: payment.Accepted.Network, + Recipient: payment.Accepted.PayTo, + Scheme: payment.Accepted.Scheme, }) }
244-258: Consider firingOnPaymentFailurefor non-402 JSON-RPC errors on retry too.
Right now only retry transport errors and “still 402” trigger the failure hook; other error codes return without emitting failure.v2/encoding/encoding.go (1)
17-129: Optional: reduce duplication with shared helpers (or generics) to keep behavior consistent.
Not required, but it’ll make future additions less error-prone.v2/mcp/server/server.go (1)
83-93: Consider adding graceful shutdown support.
http.ListenAndServeblocks indefinitely with no shutdown mechanism. For production use, consider accepting a context or returning*http.Serverto enable graceful shutdown.♻️ Suggested improvement
-// Start starts the MCP server on the given address. -func (s *X402Server) Start(addr string) error { +// Start starts the MCP server on the given address and returns the server for shutdown control. +func (s *X402Server) Start(addr string) (*http.Server, error) { handler := s.Handler() if s.config.Verbose { fmt.Printf("Starting x402 v2 MCP server on %s\n", addr) fmt.Printf("Facilitator URL: %s\n", s.config.FacilitatorURL) fmt.Printf("Verify-only mode: %v\n", s.config.VerifyOnly) fmt.Printf("Protected tools: %d\n", len(s.config.PaymentTools)) } - return http.ListenAndServe(addr, handler) + server := &http.Server{Addr: addr, Handler: handler} + return server, server.ListenAndServe() }examples/v2/gin/main.go (1)
245-314: Consider extracting duplicated signer creation logic.The signer creation blocks for SVM (lines 246-275) and EVM (lines 276-314) share a similar pattern. For an example, this is acceptable, but extracting a helper could improve maintainability.
examples/v2/http/main.go (1)
136-167: Consider handling JSON encoding errors.The
json.NewEncoder(w).Encode(response)calls on lines 154 and 166 discard errors. While acceptable for examples, this could silently fail.♻️ Suggested improvement
w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(response) + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, "failed to encode response", http.StatusInternalServerError) + return + }v2/selector.go (1)
129-139: Consider retry with next candidate on signing failure.Currently, if signing fails for the highest-priority candidate, the error is returned immediately. Depending on use case, it might be beneficial to try the next candidate.
v2/http/gin/middleware_test.go (1)
19-452: Consider consolidating tests using table-driven pattern.Based on learnings, test files should use table-driven tests. The current tests are well-structured individually but could be consolidated for similar test cases (e.g., NoPayment, VerifyOnly, and InvalidHeader tests share setup patterns).
v2/http/client.go (1)
53-94: Consider usinggetOrCreateTransporthelper consistently.
WithSignerandWithSelectorduplicate the transport creation logic that's already extracted intogetOrCreateTransport(lines 139-151). Using the helper would reduce duplication.♻️ Suggested refactor for WithSigner
func WithSigner(signer v2.Signer) ClientOption { return func(c *Client) error { - // Get or create the X402Transport - transport, ok := c.Transport.(*X402Transport) - if !ok { - // Wrap the existing transport - transport = &X402Transport{ - Base: c.Transport, - Signers: []v2.Signer{}, - Selector: v2.NewDefaultPaymentSelector(), - } - c.Transport = transport - } - - // Add the signer + transport := getOrCreateTransport(c) transport.Signers = append(transport.Signers, signer) return nil } }v2/types.go (3)
196-213: Consider adding JSON tags for consistency.Unlike all other types in this file,
TokenConfiglacks JSON struct tags. If this type is intended for internal configuration only (not serialized to/from JSON), this is fine. However, for consistency and future-proofing, consider adding JSON tags.♻️ Optional: Add JSON tags
// TokenConfig defines a token supported by a signer. type TokenConfig struct { // Address is the token contract address (EVM) or mint address (Solana). - Address string + Address string `json:"address"` // Symbol is the token symbol (e.g., "USDC"). - Symbol string + Symbol string `json:"symbol"` // Decimals is the number of decimal places for the token. - Decimals int + Decimals int `json:"decimals"` // Priority is the token's priority level within the signer. // Lower numbers indicate higher priority (1 > 2 > 3). - Priority int + Priority int `json:"priority,omitempty"` // Name is an optional human-readable token name. - Name string + Name string `json:"name,omitempty"` }
215-234: Consider validating non-negative decimals.The function works correctly for typical use cases. However, if
decimalsis negative,big.Int.Exp(10, negative, nil)returns 0, which would silently convert any amount to 0 with "Exact" accuracy. While callers should provide valid decimal values, a defensive check would prevent subtle bugs.♻️ Add decimals validation
func AmountToBigInt(amount string, decimals int) (*big.Int, error) { + if decimals < 0 { + return nil, ErrInvalidAmount + } + // Parse decimal string and convert to atomic units value := new(big.Float) if _, ok := value.SetString(amount); !ok { return nil, ErrInvalidAmount }
236-249: Same decimals validation concern applies here.Similar to
AmountToBigInt, negativedecimalswould create a zero divisor.big.Float.Quowith zero divisor produces infinity, andText()would output"+Inf"instead of a valid amount string.♻️ Add decimals validation
func BigIntToAmount(value *big.Int, decimals int) string { if value == nil { return "0" } + if decimals < 0 { + return "0" + } // Convert to float and divide by 10^decimals f := new(big.Float).SetInt(value)Alternatively, return an error (which would require changing the function signature) or panic for invalid input if this is considered a programming error.
v2/http/internal/helpers/helpers.go (1)
92-99: Consider proxy-aware scheme detection.
r.TLS != nilcheck won't detect HTTPS when the server is behind a reverse proxy (nginx, load balancer). Consider checkingX-Forwarded-Protoheader for proxy deployments.♻️ Proxy-aware implementation
func BuildResourceURL(r *http.Request) string { scheme := "http" - if r.TLS != nil { + if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" { + scheme = proto + } else if r.TLS != nil { scheme = "https" } return scheme + "://" + r.Host + r.RequestURI }v2/mcp/server/requirements.go (1)
10-25: Potential redundant scheme validation.The check at line 20 (
req.Scheme != "exact") appears redundant sincevalidation.ValidatePaymentRequirements(per v2/validation/validation.go lines 134-140) already validates the scheme and only allows "exact" or empty (which it rejects). The MCP-specific error message is more descriptive though, so keeping it provides better context.If the intent is to provide MCP-specific error messages, consider adding a comment explaining why:
// MCP-specific validation: only 'exact' scheme is supported // Note: This check is intentionally redundant with shared validation // to provide a more descriptive MCP-specific error message. if req.Scheme != "exact" {v2/http/client_test.go (1)
14-27: Test logic is correct despite static analysis warning.The staticcheck warning about possible nil pointer dereference is a false positive. Line 20 checks
client == niland returns early viat.Error, so line 24 is only reached whenclientis non-nil.However, consider using
t.Fatalinstead oft.Erroron line 21 to make the control flow clearer:♻️ Clearer control flow
if client == nil { - t.Error("Expected non-nil client") + t.Fatal("Expected non-nil client") }v2/http/transport_test.go (1)
50-73: Test logic is correct; consider suppressing linter warning.The unchecked
w.Writereturn value at line 53 is flagged by the linter. In test code this is low-risk, but for consistency:♻️ Suppress linter warning
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - w.Write([]byte("OK")) + _, _ = w.Write([]byte("OK")) }))
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
AGENTS.mdexamples/v2/gin/main.goexamples/v2/http/main.goexamples/v2/mcp/main.gov2/chains.gov2/chains_test.gov2/config.gov2/encoding/encoding.gov2/encoding/encoding_test.gov2/errors.gov2/events.gov2/facilitator/interface.gov2/http/client.gov2/http/client_test.gov2/http/facilitator.gov2/http/facilitator_test.gov2/http/gin/middleware.gov2/http/gin/middleware_test.gov2/http/internal/helpers/helpers.gov2/http/internal/helpers/helpers_test.gov2/http/middleware.gov2/http/middleware_test.gov2/http/transport.gov2/http/transport_test.gov2/internal/eip3009/eip3009.gov2/internal/eip3009/eip3009_test.gov2/internal/solana/solana.gov2/mcp/client/config.gov2/mcp/client/transport.gov2/mcp/client/transport_test.gov2/mcp/errors.gov2/mcp/server/config.gov2/mcp/server/facilitator.gov2/mcp/server/handler.gov2/mcp/server/handler_test.gov2/mcp/server/requirements.gov2/mcp/server/server.gov2/mcp/types.gov2/selector.gov2/selector_test.gov2/signer.gov2/signers/evm/signer.gov2/signers/evm/signer_test.gov2/signers/svm/signer.gov2/signers/svm/signer_test.gov2/types.gov2/types_test.gov2/validation/validation.gov2/validation/validation_test.go
💤 Files with no reviewable changes (1)
- AGENTS.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Group imports as: stdlib, external dependencies, then internal packages with blank lines between groups
Use camelCase for variables and functions, PascalCase for exported items, avoid abbreviations
Always check errors; wrap with context usingfmt.Errorf("context: %w", err)for error handling
For exported functions, start comments with function name; use//for inline comments,/* */for block comments
Prefer channels over mutexes for concurrency; always handle goroutine lifecycles properly
Files:
v2/signer.gov2/encoding/encoding_test.gov2/http/transport.gov2/mcp/client/transport_test.gov2/http/internal/helpers/helpers_test.gov2/config.gov2/selector_test.gov2/validation/validation.gov2/http/facilitator_test.gov2/facilitator/interface.gov2/mcp/server/server.gov2/http/gin/middleware.gov2/mcp/server/requirements.gov2/signers/svm/signer_test.gov2/encoding/encoding.gov2/mcp/client/config.gov2/validation/validation_test.gov2/http/internal/helpers/helpers.gov2/mcp/client/transport.gov2/internal/solana/solana.gov2/http/client.gov2/selector.gov2/internal/eip3009/eip3009_test.gov2/http/middleware_test.gov2/chains_test.gov2/errors.gov2/signers/evm/signer_test.gov2/types_test.gov2/internal/eip3009/eip3009.gov2/signers/evm/signer.gov2/http/client_test.gov2/http/transport_test.gov2/events.gov2/mcp/server/handler_test.gov2/http/gin/middleware_test.gov2/mcp/errors.gov2/mcp/server/config.gov2/http/facilitator.goexamples/v2/http/main.goexamples/v2/mcp/main.goexamples/v2/gin/main.gov2/http/middleware.gov2/mcp/types.gov2/mcp/server/facilitator.gov2/types.gov2/chains.gov2/mcp/server/handler.gov2/signers/svm/signer.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Test files must end with
_test.go; use table-driven tests; mock external dependencies
Files:
v2/encoding/encoding_test.gov2/mcp/client/transport_test.gov2/http/internal/helpers/helpers_test.gov2/selector_test.gov2/http/facilitator_test.gov2/signers/svm/signer_test.gov2/validation/validation_test.gov2/internal/eip3009/eip3009_test.gov2/http/middleware_test.gov2/chains_test.gov2/signers/evm/signer_test.gov2/types_test.gov2/http/client_test.gov2/http/transport_test.gov2/mcp/server/handler_test.gov2/http/gin/middleware_test.go
🧠 Learnings (1)
📚 Learning: 2025-12-04T10:19:11.236Z
Learnt from: CR
Repo: mark3labs/x402-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:19:11.236Z
Learning: Applies to **/*_test.go : Test files must end with `_test.go`; use table-driven tests; mock external dependencies
Applied to files:
v2/mcp/client/transport_test.gov2/http/internal/helpers/helpers_test.gov2/validation/validation_test.gov2/types_test.gov2/http/client_test.gov2/mcp/server/handler_test.gov2/http/gin/middleware_test.go
🧬 Code graph analysis (35)
v2/signer.go (1)
v2/types.go (3)
PaymentRequirements(31-52)PaymentPayload(82-99)TokenConfig(197-213)
v2/encoding/encoding_test.go (2)
v2/types.go (8)
PaymentPayload(82-99)X402Version(15-15)ResourceInfo(18-27)PaymentRequirements(31-52)SettleResponse(152-167)PaymentRequired(64-79)Extension(55-61)VerifyResponse(140-149)v2/encoding/encoding.go (8)
EncodePayment(17-23)DecodePayment(28-41)EncodeSettlement(47-53)DecodeSettlement(58-71)EncodeRequirements(76-82)DecodeRequirements(87-100)EncodeVerifyResponse(105-111)DecodeVerifyResponse(116-129)
v2/http/transport.go (1)
v2/http/internal/helpers/helpers.go (3)
ParsePaymentRequirements(58-70)BuildPaymentHeader(88-90)ParseSettlement(74-85)
v2/mcp/client/transport_test.go (5)
v2/mcp/client/transport.go (1)
Transport(16-19)v2/mcp/client/config.go (6)
DefaultConfig(99-106)WithPaymentAttemptCallback(64-68)WithPaymentSuccessCallback(71-75)WithPaymentFailureCallback(78-82)WithPaymentCallback(55-61)WithSelector(85-89)v2/mcp/server/config.go (1)
DefaultConfig(67-75)v2/mcp/errors.go (1)
ErrNoPaymentRequirements(27-27)v2/mcp/types.go (1)
PaymentRequirements(15-30)
v2/http/internal/helpers/helpers_test.go (2)
v2/types.go (4)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)SettleResponse(152-167)v2/http/internal/helpers/helpers.go (6)
ParsePaymentHeader(14-31)SendPaymentRequired(34-45)AddPaymentResponseHeader(48-55)ParsePaymentRequirements(58-70)ParseSettlement(74-85)BuildResourceURL(93-99)
v2/selector_test.go (4)
v2/types.go (6)
TokenConfig(197-213)PaymentRequirements(31-52)PaymentPayload(82-99)X402Version(15-15)EVMPayload(102-108)EVMAuthorization(111-129)v2/selector.go (1)
FindMatchingRequirement(148-160)v2/signer.go (1)
Signer(8-32)v2/errors.go (5)
ErrorCode(63-63)ErrCodeNoValidSigner(67-67)ErrCodeInvalidRequirements(73-73)PaymentError(86-98)ErrCodeSigningFailed(76-76)
v2/validation/validation.go (2)
v2/chains.go (3)
ValidateNetwork(197-231)NetworkTypeEVM(16-16)NetworkTypeSVM(18-18)v2/types.go (5)
ResourceInfo(18-27)PaymentRequirements(31-52)PaymentPayload(82-99)X402Version(15-15)PaymentRequired(64-79)
v2/http/facilitator_test.go (4)
v2/types.go (7)
VerifyResponse(140-149)PaymentPayload(82-99)X402Version(15-15)PaymentRequirements(31-52)SettleResponse(152-167)SupportedResponse(185-194)SupportedKind(170-182)v2/http/facilitator.go (2)
FacilitatorClient(41-81)AuthorizationProvider(26-26)v2/config.go (2)
DefaultTimeouts(21-25)TimeoutConfig(9-18)v2/errors.go (3)
ErrVerificationFailed(47-47)ErrSettlementFailed(50-50)ErrFacilitatorUnavailable(44-44)
v2/facilitator/interface.go (1)
v2/types.go (6)
PaymentPayload(82-99)PaymentRequirements(31-52)VerifyResponse(140-149)SettleResponse(152-167)SupportedResponse(185-194)X402Version(15-15)
v2/http/gin/middleware.go (2)
v2/http/internal/helpers/helpers.go (2)
BuildResourceURL(93-99)ParsePaymentHeader(14-31)v2/types.go (3)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)
v2/mcp/server/requirements.go (3)
v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/validation/validation.go (1)
ValidatePaymentRequirements(111-167)v2/types.go (1)
ResourceInfo(18-27)
v2/signers/svm/signer_test.go (4)
v2/signers/svm/signer.go (5)
Option(32-32)WithPriority(107-112)WithMaxAmount(99-104)NewSigner(35-42)NewSignerFromKeygenFile(78-96)v2/chains.go (3)
NetworkSolanaMainnet(36-36)NetworkSolanaDevnet(37-37)NetworkBaseSepolia(30-30)v2/types.go (1)
X402Version(15-15)v2/internal/solana/solana.go (1)
ComputeBudgetProgramID(15-15)
v2/encoding/encoding.go (1)
v2/types.go (4)
PaymentPayload(82-99)SettleResponse(152-167)PaymentRequired(64-79)VerifyResponse(140-149)
v2/mcp/client/config.go (1)
v2/mcp/server/config.go (2)
Config(21-64)DefaultConfig(67-75)
v2/validation/validation_test.go (1)
v2/validation/validation.go (7)
ValidateAmount(28-45)ValidateNetwork(49-61)ValidateAddress(66-92)ValidateResourceInfo(96-107)ValidatePaymentRequirements(111-167)ValidatePaymentPayload(171-201)ValidatePaymentRequired(204-224)
v2/mcp/client/transport.go (4)
v2/mcp/client/config.go (3)
Config(11-35)Option(38-38)DefaultConfig(99-106)v2/mcp/errors.go (2)
WrapX402Error(74-82)ErrNoPaymentRequirements(27-27)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/types.go (2)
ResourceInfo(18-27)X402Version(15-15)
v2/internal/solana/solana.go (1)
v2/chains.go (2)
NetworkSolanaMainnet(36-36)NetworkSolanaDevnet(37-37)
v2/http/client.go (3)
v2/http/transport.go (1)
X402Transport(13-31)v2/types.go (1)
SettleResponse(152-167)v2/http/internal/helpers/helpers.go (1)
ParseSettlement(74-85)
v2/selector.go (3)
v2/signer.go (1)
Signer(8-32)v2/types.go (2)
PaymentRequirements(31-52)PaymentPayload(82-99)v2/errors.go (8)
NewPaymentError(114-121)ErrCodeNoValidSigner(67-67)ErrNoValidSigner(8-8)ErrCodeInvalidRequirements(73-73)ErrInvalidRequirements(14-14)ErrCodeSigningFailed(76-76)ErrCodeUnsupportedScheme(82-82)ErrUnsupportedScheme(59-59)
v2/http/middleware_test.go (2)
v2/types.go (4)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)SettleResponse(152-167)v2/http/middleware.go (3)
Config(16-59)NewX402Middleware(71-216)GetPaymentFromContext(305-315)
v2/signers/evm/signer_test.go (3)
v2/signers/evm/signer.go (3)
NewSigner(28-57)WithMaxAmount(91-96)GetChainID(201-222)v2/types.go (1)
X402Version(15-15)v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)
v2/types_test.go (1)
v2/types.go (15)
X402Version(15-15)ResourceInfo(18-27)PaymentRequirements(31-52)PaymentRequired(64-79)PaymentPayload(82-99)EVMPayload(102-108)EVMAuthorization(111-129)Extension(55-61)VerifyResponse(140-149)SettleResponse(152-167)SupportedResponse(185-194)SupportedKind(170-182)AmountToBigInt(217-234)BigIntToAmount(238-249)TokenConfig(197-213)
v2/http/transport_test.go (2)
v2/types.go (4)
X402Version(15-15)PaymentRequired(64-79)ResourceInfo(18-27)SettleResponse(152-167)v2/http/transport.go (1)
X402Transport(13-31)
v2/http/gin/middleware_test.go (2)
v2/http/gin/middleware.go (4)
Config(18-18)NewX402Middleware(60-208)GetPaymentFromContext(225-235)PaymentContextKey(21-21)v2/types.go (3)
ResourceInfo(18-27)PaymentRequired(64-79)X402Version(15-15)
v2/mcp/server/config.go (4)
v2/types.go (1)
ResourceInfo(18-27)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/mcp/server/handler.go (4)
AuthorizationProvider(50-50)OnBeforeFunc(53-53)OnAfterVerifyFunc(56-56)OnAfterSettleFunc(59-59)v2/mcp/client/config.go (2)
DefaultConfig(99-106)Config(11-35)
v2/http/facilitator.go (5)
v2/types.go (7)
PaymentPayload(82-99)PaymentRequirements(31-52)VerifyResponse(140-149)SettleResponse(152-167)X402Version(15-15)SupportedResponse(185-194)SupportedKind(170-182)v2/config.go (1)
TimeoutConfig(9-18)v2/facilitator/interface.go (2)
Interface(16-28)VerifyRequest(31-40)retry/retry.go (1)
WithRetry(32-81)v2/errors.go (3)
ErrFacilitatorUnavailable(44-44)ErrVerificationFailed(47-47)ErrSettlementFailed(50-50)
examples/v2/http/main.go (3)
v2/chains.go (1)
GetChainConfig(186-192)v2/types.go (1)
ResourceInfo(18-27)v2/signers/svm/signer.go (1)
NewSignerFromKeygenFile(78-96)
examples/v2/mcp/main.go (3)
v2/chains.go (1)
GetChainConfig(186-192)v2/types.go (1)
ResourceInfo(18-27)v2/signers/svm/signer.go (1)
NewSignerFromKeygenFile(78-96)
v2/http/middleware.go (1)
v2/http/internal/helpers/helpers.go (4)
BuildResourceURL(93-99)SendPaymentRequired(34-45)ParsePaymentHeader(14-31)AddPaymentResponseHeader(48-55)
v2/mcp/types.go (1)
v2/types.go (3)
X402Version(15-15)ResourceInfo(18-27)Extension(55-61)
v2/mcp/server/facilitator.go (4)
v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/types.go (1)
SettleResponse(152-167)v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)v2/mcp/server/handler.go (4)
AuthorizationProvider(50-50)OnBeforeFunc(53-53)OnAfterVerifyFunc(56-56)OnAfterSettleFunc(59-59)
v2/types.go (2)
v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)v2/errors.go (1)
ErrInvalidAmount(23-23)
v2/chains.go (3)
v2/errors.go (1)
ErrInvalidNetwork(29-29)v2/validation/validation.go (1)
ValidateNetwork(49-61)v2/types.go (1)
TokenConfig(197-213)
v2/mcp/server/handler.go (5)
v2/mcp/server/config.go (3)
Config(21-64)DefaultConfig(67-75)ToolPaymentConfig(12-18)v2/mcp/server/facilitator.go (1)
Facilitator(15-21)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/types.go (2)
SettleResponse(152-167)X402Version(15-15)v2/mcp/server/requirements.go (1)
SetToolResource(29-33)
v2/signers/svm/signer.go (2)
v2/internal/solana/solana.go (7)
GetRPCURL(90-99)DeriveAssociatedTokenAddress(81-87)BuildSetComputeUnitLimitInstruction(43-56)DefaultComputeUnits(18-18)BuildSetComputeUnitPriceInstruction(61-78)DefaultComputeUnitPrice(21-21)BuildTransferCheckedInstruction(24-38)v2/types.go (1)
X402Version(15-15)
🪛 GitHub Actions: golangci-lint
v2/http/client_test.go
[error] 199-199: golangci-lint: Error return value of (*encoding/json.Encoder).Encode is not checked (errcheck).
🪛 GitHub Check: lint
v2/http/facilitator_test.go
[failure] 208-208:
Error return value of (*encoding/json.Encoder).Encode is not checked (errcheck)
v2/http/internal/helpers/helpers.go
[failure] 44-44:
Error return value of (*encoding/json.Encoder).Encode is not checked (errcheck)
v2/internal/eip3009/eip3009_test.go
[failure] 24-24:
SA4006: this value of nonce is never used (staticcheck)
v2/http/middleware_test.go
[failure] 156-156:
Error return value of w.Write is not checked (errcheck)
v2/http/client_test.go
[failure] 287-287:
type mockSigner2 is unused (unused)
[failure] 24-24:
SA5011: possible nil pointer dereference (staticcheck)
[failure] 20-20:
SA5011(related information): this check suggests that the pointer can be nil (staticcheck)
[failure] 212-212:
Error return value of w.Write is not checked (errcheck)
[failure] 199-199:
Error return value of (*encoding/json.Encoder).Encode is not checked (errcheck)
v2/http/transport_test.go
[failure] 53-53:
Error return value of w.Write is not checked (errcheck)
🪛 Gitleaks (8.30.0)
v2/internal/eip3009/eip3009_test.go
[high] 17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
v2/signers/evm/signer_test.go
[high] 12-12: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (1)
- GitHub Check: test
| // EnrichRequirements fetches supported payment types from the facilitator and | ||
| // enriches the provided payment requirements with network-specific data like feePayer. | ||
| // This is particularly useful for SVM chains where the feePayer must be specified. | ||
| func (c *FacilitatorClient) EnrichRequirements(requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) { | ||
| // Fetch supported payment types | ||
| supported, err := c.Supported(context.Background()) | ||
| if err != nil { | ||
| return requirements, fmt.Errorf("failed to fetch supported payment types: %w", err) | ||
| } | ||
|
|
||
| // Create a lookup map for supported kinds by network and scheme | ||
| supportedMap := make(map[string]v2.SupportedKind) | ||
| for _, kind := range supported.Kinds { | ||
| key := kind.Network + "-" + kind.Scheme | ||
| supportedMap[key] = kind | ||
| } | ||
|
|
||
| // Enrich each requirement with extra data from the facilitator | ||
| enriched := make([]v2.PaymentRequirements, len(requirements)) | ||
| for i, req := range requirements { | ||
| enriched[i] = req | ||
| key := req.Network + "-" + req.Scheme | ||
| if kind, ok := supportedMap[key]; ok && kind.Extra != nil { | ||
| // Initialize Extra map if it doesn't exist | ||
| if enriched[i].Extra == nil { | ||
| enriched[i].Extra = make(map[string]interface{}) | ||
| } | ||
| // Merge facilitator's extra data into requirement | ||
| for k, v := range kind.Extra { | ||
| // Only set if not already present (user-specified values take precedence) | ||
| if _, exists := enriched[i].Extra[k]; !exists { | ||
| enriched[i].Extra[k] = v | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return enriched, nil | ||
| } |
There was a problem hiding this comment.
Don’t hardcode context.Background() in EnrichRequirements (kills cancellation/timeout control).
Proposed fix (API change, but new surface)
-func (c *FacilitatorClient) EnrichRequirements(requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) {
+func (c *FacilitatorClient) EnrichRequirements(ctx context.Context, requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) {
// Fetch supported payment types
- supported, err := c.Supported(context.Background())
+ supported, err := c.Supported(ctx)
if err != nil {
return requirements, fmt.Errorf("failed to fetch supported payment types: %w", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // EnrichRequirements fetches supported payment types from the facilitator and | |
| // enriches the provided payment requirements with network-specific data like feePayer. | |
| // This is particularly useful for SVM chains where the feePayer must be specified. | |
| func (c *FacilitatorClient) EnrichRequirements(requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) { | |
| // Fetch supported payment types | |
| supported, err := c.Supported(context.Background()) | |
| if err != nil { | |
| return requirements, fmt.Errorf("failed to fetch supported payment types: %w", err) | |
| } | |
| // Create a lookup map for supported kinds by network and scheme | |
| supportedMap := make(map[string]v2.SupportedKind) | |
| for _, kind := range supported.Kinds { | |
| key := kind.Network + "-" + kind.Scheme | |
| supportedMap[key] = kind | |
| } | |
| // Enrich each requirement with extra data from the facilitator | |
| enriched := make([]v2.PaymentRequirements, len(requirements)) | |
| for i, req := range requirements { | |
| enriched[i] = req | |
| key := req.Network + "-" + req.Scheme | |
| if kind, ok := supportedMap[key]; ok && kind.Extra != nil { | |
| // Initialize Extra map if it doesn't exist | |
| if enriched[i].Extra == nil { | |
| enriched[i].Extra = make(map[string]interface{}) | |
| } | |
| // Merge facilitator's extra data into requirement | |
| for k, v := range kind.Extra { | |
| // Only set if not already present (user-specified values take precedence) | |
| if _, exists := enriched[i].Extra[k]; !exists { | |
| enriched[i].Extra[k] = v | |
| } | |
| } | |
| } | |
| } | |
| return enriched, nil | |
| } | |
| // EnrichRequirements fetches supported payment types from the facilitator and | |
| // enriches the provided payment requirements with network-specific data like feePayer. | |
| // This is particularly useful for SVM chains where the feePayer must be specified. | |
| func (c *FacilitatorClient) EnrichRequirements(ctx context.Context, requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) { | |
| // Fetch supported payment types | |
| supported, err := c.Supported(ctx) | |
| if err != nil { | |
| return requirements, fmt.Errorf("failed to fetch supported payment types: %w", err) | |
| } | |
| // Create a lookup map for supported kinds by network and scheme | |
| supportedMap := make(map[string]v2.SupportedKind) | |
| for _, kind := range supported.Kinds { | |
| key := kind.Network + "-" + kind.Scheme | |
| supportedMap[key] = kind | |
| } | |
| // Enrich each requirement with extra data from the facilitator | |
| enriched := make([]v2.PaymentRequirements, len(requirements)) | |
| for i, req := range requirements { | |
| enriched[i] = req | |
| key := req.Network + "-" + req.Scheme | |
| if kind, ok := supportedMap[key]; ok && kind.Extra != nil { | |
| // Initialize Extra map if it doesn't exist | |
| if enriched[i].Extra == nil { | |
| enriched[i].Extra = make(map[string]interface{}) | |
| } | |
| // Merge facilitator's extra data into requirement | |
| for k, v := range kind.Extra { | |
| // Only set if not already present (user-specified values take precedence) | |
| if _, exists := enriched[i].Extra[k]; !exists { | |
| enriched[i].Extra[k] = v | |
| } | |
| } | |
| } | |
| } | |
| return enriched, nil | |
| } |
🤖 Prompt for AI Agents
In @v2/http/facilitator.go around lines 298 - 336, EnrichRequirements currently
uses context.Background(), preventing callers from controlling
cancellation/timeouts; change the method signature of
FacilitatorClient.EnrichRequirements to accept a context.Context as the first
parameter (e.g., EnrichRequirements(ctx context.Context, requirements
[]v2.PaymentRequirements) ([]v2.PaymentRequirements, error)), replace the
internal call to c.Supported(context.Background()) with c.Supported(ctx), and
propagate this new ctx to all callers and unit tests (update any call sites and
mocks) so cancellation and deadlines flow through; keep the rest of the merging
logic unchanged.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @v2/config.go:
- Around line 45-58: The Validate method on TimeoutConfig currently checks
VerifyTimeout and SettleTimeout but omits RequestTimeout; update
TimeoutConfig.Validate to also ensure RequestTimeout is positive and that it
makes sense relative to other fields (e.g., RequestTimeout > 0 and optionally <=
SettleTimeout or >= VerifyTimeout depending on intended semantics). Modify the
function (TimeoutConfig.Validate) to return an error like "request timeout must
be positive, got %v" when RequestTimeout <= 0 and add any necessary comparative
checks (refer to existing SettleTimeout vs VerifyTimeout check) so all three
timeouts are validated consistently.
In @v2/http/client_test.go:
- Around line 156-169: The test TestClient_WithHTTPClient mistakenly sets
http.Client.Timeout to the integer 30 (nanoseconds) instead of a time.Duration;
update the customClient.Timeout to a proper duration (e.g., 30*time.Second or
time.Second*30) and ensure the time package is imported so the test asserts the
intended timeout when constructing via NewClient(WithHTTPClient(...)); leave the
rest of the test logic unchanged and reference the customClient used by the
NewClient call.
In @v2/internal/solana/solana.go:
- Around line 89-99: The fmt.Errorf call in GetRPCURL incorrectly places the %w
before contextual text which breaks error wrapping; update the return in
GetRPCURL to format the message with the network context first and the sentinel
error wrapped with %w (i.e., make the format "invalid network %s: %w" and pass
network then v2.ErrInvalidNetwork) so errors.Is/As can properly unwrap
v2.ErrInvalidNetwork.
In @v2/signers/svm/signer.go:
- Around line 220-223: The call site uses amount.Uint64() which silently
truncates big.Int values; fix by explicitly checking for overflow before
conversion: compare amount against a big.Int set to math.MaxUint64 (e.g., max :=
new(big.Int).SetUint64(math.MaxUint64)) and if amount.Cmp(max) > 0 return an
error (or handle as appropriate) instead of converting, otherwise safely cast
with amount.Uint64(); update the code around the call that passes mintAddress,
recipient, amount.Uint64(), decimals to perform this check and fail early to
avoid incorrect transfer amounts.
🧹 Nitpick comments (45)
v2/internal/solana/solana.go (2)
40-56: Consider usingencoding/binaryfor clearer byte encoding.The manual bit-shifting is correct but could be more readable using the standard library's
binary.LittleEndian.PutUint32.♻️ Suggested refactor
+import "encoding/binary" + func BuildSetComputeUnitLimitInstruction(units uint32) solana.Instruction { data := make([]byte, 5) data[0] = 2 // SetComputeUnitLimit discriminator - data[1] = byte(units) - data[2] = byte(units >> 8) - data[3] = byte(units >> 16) - data[4] = byte(units >> 24) + binary.LittleEndian.PutUint32(data[1:], units) return solana.NewInstruction( ComputeBudgetProgramID, solana.AccountMetaSlice{}, data, ) }
58-78: Same refactor opportunity withencoding/binary.Consistent with the previous suggestion, this can be simplified using
binary.LittleEndian.PutUint64.♻️ Suggested refactor
func BuildSetComputeUnitPriceInstruction(microlamports uint64) solana.Instruction { data := make([]byte, 9) data[0] = 3 // SetComputeUnitPrice discriminator - data[1] = byte(microlamports) - data[2] = byte(microlamports >> 8) - data[3] = byte(microlamports >> 16) - data[4] = byte(microlamports >> 24) - data[5] = byte(microlamports >> 32) - data[6] = byte(microlamports >> 40) - data[7] = byte(microlamports >> 48) - data[8] = byte(microlamports >> 56) + binary.LittleEndian.PutUint64(data[1:], microlamports) return solana.NewInstruction( ComputeBudgetProgramID, solana.AccountMetaSlice{}, data, ) }v2/http/transport_test.go (1)
297-328: Consider verifying OnPaymentFailure callback is invoked.This test validates that an error is returned when no signers are configured, but doesn't verify that
OnPaymentFailurecallback is actually invoked. Consider adding callback verification for completeness.💡 Suggested enhancement
func TestTransport_FailureCallback(t *testing.T) { + var failureCalled bool + var failureEvent v2.PaymentEvent + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { paymentReq := v2.PaymentRequired{ X402Version: 2, Accepts: []v2.PaymentRequirements{ { Scheme: "exact", Network: "eip155:84532", Amount: "10000", }, }, } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusPaymentRequired) _ = json.NewEncoder(w).Encode(paymentReq) })) defer server.Close() // No signer configured transport := &X402Transport{ Base: http.DefaultTransport, Signers: []v2.Signer{}, Selector: v2.NewDefaultPaymentSelector(), + OnPaymentFailure: func(event v2.PaymentEvent) { + failureCalled = true + failureEvent = event + }, } req, _ := http.NewRequest("GET", server.URL+"/api/data", nil) _, err := transport.RoundTrip(req) // This should fail because no signers are configured if err == nil { t.Error("Expected error for no signers") } + + if !failureCalled { + t.Error("OnPaymentFailure callback was not called") + } + if failureEvent.Type != v2.PaymentEventFailure { + t.Errorf("Expected failure event type, got %s", failureEvent.Type) + } }v2/types_test.go (1)
61-94: Consider extending PaymentRequirements test assertions.The test only verifies
Scheme,Network, andAmountafter round-trip. Consider adding assertions forAsset,PayTo,MaxTimeoutSeconds, andExtrato ensure complete round-trip integrity.💡 Suggested enhancement
if decoded.Amount != req.Amount { t.Errorf("Amount = %s; want %s", decoded.Amount, req.Amount) } + if decoded.Asset != req.Asset { + t.Errorf("Asset = %s; want %s", decoded.Asset, req.Asset) + } + if decoded.PayTo != req.PayTo { + t.Errorf("PayTo = %s; want %s", decoded.PayTo, req.PayTo) + } + if decoded.MaxTimeoutSeconds != req.MaxTimeoutSeconds { + t.Errorf("MaxTimeoutSeconds = %d; want %d", decoded.MaxTimeoutSeconds, req.MaxTimeoutSeconds) + } }v2/http/internal/helpers/helpers_test.go (1)
333-340: Consider using io.NopCloser from stdlib instead of custom helper.Go's standard library provides
io.NopCloserwhich serves the same purpose.💡 Use stdlib io.NopCloser
+import "io" + // Replace nopCloser usage with io.NopCloser resp := &http.Response{ StatusCode: 402, - Body: &nopCloser{strings.NewReader(string(body))}, + Body: io.NopCloser(strings.NewReader(string(body))), } - -// nopCloser is a helper to create a ReadCloser from a Reader -type nopCloser struct { - *strings.Reader -} - -func (n *nopCloser) Close() error { - return nil -}v2/http/client_test.go (1)
261-283: Header name casing may affect test reliability.The test uses
"X-Payment-Response"(line 271) while the production code uses"X-PAYMENT-RESPONSE". Go'shttp.Headeris case-insensitive for lookups, so this works, but using consistent casing improves clarity.💡 Use consistent header casing
resp := &http.Response{ Header: http.Header{ - "X-Payment-Response": []string{encoded}, + "X-PAYMENT-RESPONSE": []string{encoded}, }, }v2/signers/svm/signer_test.go (2)
15-18: Address static analysis false positive with clearer documentation.Gitleaks flagged this as a potential API key. While the comment indicates it's a test key, consider adding a more explicit marker that tools can recognize, or document that this is intentional.
💡 Add nosec comment to suppress false positive
-// Test private key (DO NOT use in production) -// This is a randomly generated Solana key for testing purposes only -const testPrivateKeyBase58 = "4Z7cXSyeFR8wNGMVXUE1TwtKn5D5Vu7FzEv69dokLv8KrQk7h2ByqYCKQBWUrbXdqeqSHXv2YvPRzYMNL8hFmjXu" +// testPrivateKeyBase58 is a randomly generated Solana key for testing purposes only. +// DO NOT use in production. This key has no real funds and is publicly known. +// nolint:gosec // Test key - intentionally committed for testing +const testPrivateKeyBase58 = "4Z7cXSyeFR8wNGMVXUE1TwtKn5D5Vu7FzEv69dokLv8KrQk7h2ByqYCKQBWUrbXdqeqSHXv2YvPRzYMNL8hFmjXu" //nolint:gosec
708-720: Use strings.Contains instead of custom helper.The custom
containsStringandindexOfSubstringfunctions replicate stdlib functionality.♻️ Use stdlib strings.Contains
+import "strings" + // In test assertions, replace: -if !containsString(err.Error(), tt.errContains) { +if !strings.Contains(err.Error(), tt.errContains) { -// Remove these helper functions: -func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || indexOfSubstring(s, substr) >= 0) -} - -func indexOfSubstring(s, substr string) int { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return i - } - } - return -1 -}v2/internal/eip3009/eip3009.go (1)
99-99: Consider clearer slice construction.The nested
appendpattern works correctly but can be made more readable by pre-allocating:♻️ Suggested improvement
- rawData := append([]byte{0x19, 0x01}, append(domainSeparator, messageHash...)...) + rawData := make([]byte, 0, 2+len(domainSeparator)+len(messageHash)) + rawData = append(rawData, 0x19, 0x01) + rawData = append(rawData, domainSeparator...) + rawData = append(rawData, messageHash...)v2/mcp/client/config.go (1)
98-106: Consider notinghttp.DefaultClienthas no timeout.Using
http.DefaultClientas the default is convenient but worth noting in documentation that it has no timeout configured. Users making requests to untrusted servers may want to set a custom client with timeouts.examples/v2/mcp/main.go (1)
184-191: Graceful shutdown could be improved.The shutdown goroutine calls
os.Exit(0)directly without allowing in-flight requests to complete. For an example, this is acceptable, but production code should use proper graceful shutdown withhttp.Server.Shutdown(ctx).v2/errors.go (1)
123-127:WithDetailsmutates the receiver in-place.Unlike the value-receiver fluent methods in
config.go,WithDetailsmutates the receiver. This is acceptable for error building but could cause issues if an error is shared and modified. Document this behavior or consider defensive copying.v2/mcp/errors.go (1)
48-54: Note: TwoPaymentErrortypes in different packages.This
PaymentError(inmcppackage) has a different structure thanv2.PaymentError. Both are valid but the naming overlap may cause confusion. Consider renaming toMCPPaymentErrorfor clarity, or document the distinction.v2/chains.go (1)
268-278:NewUSDCTokenConfigignoreschain.Decimals.The function hardcodes
Decimals: 6instead of usingchain.Decimals. While USDC is always 6 decimals, using the chain's value would be more consistent.♻️ Suggested improvement
func NewUSDCTokenConfig(chain ChainConfig, priority int) TokenConfig { return TokenConfig{ Address: chain.USDCAddress, Symbol: "USDC", - Decimals: 6, + Decimals: int(chain.Decimals), Priority: priority, Name: "USD Coin", } }v2/types.go (2)
196-213: Missing JSON tags onTokenConfig- inconsistent with other types.All other exported structs in this file include JSON tags for serialization, but
TokenConfiglacks them. This inconsistency may cause issues if this type needs to be marshaled/unmarshaled in the future, or if it's used in contexts that expect JSON field names (e.g., configuration files, APIs).♻️ Suggested fix to add JSON tags
// TokenConfig defines a token supported by a signer. type TokenConfig struct { // Address is the token contract address (EVM) or mint address (Solana). - Address string + Address string `json:"address"` // Symbol is the token symbol (e.g., "USDC"). - Symbol string + Symbol string `json:"symbol"` // Decimals is the number of decimal places for the token. - Decimals int + Decimals int `json:"decimals"` // Priority is the token's priority level within the signer. // Lower numbers indicate higher priority (1 > 2 > 3). - Priority int + Priority int `json:"priority"` // Name is an optional human-readable token name. - Name string + Name string `json:"name,omitempty"` }
215-234: Consider edge cases for decimal conversion.The
big.Floatapproach works for most cases, and thebig.Exactaccuracy check is good for catching precision loss. However, a couple of edge cases to consider:
- Negative
decimalsvalues would cause unexpected behavior withExp()- Very small fractional amounts near the precision limit could theoretically have rounding issues before the accuracy check catches them
These are unlikely in practice since token decimals are typically 6-18, but defensive validation could help.
♻️ Optional: Add defensive check for decimals
func AmountToBigInt(amount string, decimals int) (*big.Int, error) { + if decimals < 0 { + return nil, ErrInvalidAmount + } + // Parse decimal string and convert to atomic units value := new(big.Float) if _, ok := value.SetString(amount); !ok { return nil, ErrInvalidAmount }v2/chains_test.go (1)
362-374: Replace custom string helpers withstrings.Contains.The
containsStringandcontainsSubstringhelper functions reimplementstrings.Containsfrom the standard library. Using the stdlib function is clearer and more maintainable.♻️ Suggested simplification
+import ( + "errors" + "strings" + "testing" +) -// Helper function -func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr)) -} - -func containsSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -}Then update the usage at line 150:
- if err == nil || !containsString(err.Error(), tt.errContains) { + if err == nil || !strings.Contains(err.Error(), tt.errContains) {v2/validation/validation.go (2)
94-107: URL validation may be too permissive.
url.Parseaccepts almost any string including empty paths and relative URLs. Consider validating that the URL has a scheme (http/https) for resource URLs.♻️ Suggested stricter URL validation
func ValidateResourceInfo(resource v2.ResourceInfo) error { if resource.URL == "" { return fmt.Errorf("resource URL cannot be empty") } // Validate URL format - if _, err := url.Parse(resource.URL); err != nil { + parsedURL, err := url.Parse(resource.URL) + if err != nil { return fmt.Errorf("invalid resource URL: %w", err) } + if parsedURL.Scheme == "" || parsedURL.Host == "" { + return fmt.Errorf("invalid resource URL: must be an absolute URL with scheme and host") + } return nil }
169-201: Consider validating the fullAcceptedrequirements inValidatePaymentPayload.Currently,
ValidatePaymentPayloadonly validatesSchemeandNetworkofpayload.Accepted, but doesn't callValidatePaymentRequirementson the fullAcceptedstruct. This could allow invalidAmount,Asset, orPayTovalues to pass through.♻️ Suggested fix to validate full Accepted requirements
func ValidatePaymentPayload(payload v2.PaymentPayload) error { if payload.X402Version != v2.X402Version { return fmt.Errorf("unsupported x402 version: %d (expected %d)", payload.X402Version, v2.X402Version) } - // Validate accepted requirements - if payload.Accepted.Scheme == "" { - return fmt.Errorf("accepted scheme cannot be empty") - } - - if payload.Accepted.Network == "" { - return fmt.Errorf("accepted network cannot be empty") - } - - if _, err := v2.ValidateNetwork(payload.Accepted.Network); err != nil { - return fmt.Errorf("invalid accepted network: %w", err) + // Validate accepted requirements using the full validation + if err := ValidatePaymentRequirements(payload.Accepted); err != nil { + return fmt.Errorf("invalid accepted: %w", err) } if payload.Payload == nil { return fmt.Errorf("payload cannot be nil") }v2/http/internal/helpers/helpers.go (1)
33-45: Consider handling JSON encoding error.The error from
json.NewEncoder(w).Encode(response)is silently discarded. While unlikely to fail for well-formed structs, logging or handling the error would improve observability.♻️ Suggested error logging
func SendPaymentRequired(w http.ResponseWriter, resource v2.ResourceInfo, requirements []v2.PaymentRequirements, errMsg string) { response := v2.PaymentRequired{ X402Version: v2.X402Version, Error: errMsg, Resource: resource, Accepts: requirements, } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusPaymentRequired) - _ = json.NewEncoder(w).Encode(response) + if err := json.NewEncoder(w).Encode(response); err != nil { + // Log encoding failure - response headers already sent + // Consider using a logger if available + } }v2/signers/evm/signer_test.go (1)
37-111: Test uses placeholder PayTo address "0x receiver".The PayTo field
"0x receiver"is not a valid EVM address. While this works forCanSigntests (which may only check network/scheme/asset), it could cause issues ifCanSignis updated to validate addresses. Consider using a valid placeholder address for consistency.♻️ Suggested fix
{ name: "valid requirements", requirements: &v2.PaymentRequirements{ Scheme: "exact", Network: "eip155:84532", Asset: "0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913", Amount: "1000000", - PayTo: "0x receiver", + PayTo: "0x1234567890123456789012345678901234567890", MaxTimeoutSeconds: 300, }, expected: true, },Apply similar changes to other test cases.
v2/mcp/server/requirements.go (2)
10-25: Scheme validation is redundant with shared validation.
validation.ValidatePaymentRequirementsalready validates thatSchemeis "exact" (see v2/validation/validation.go lines 137-144). The MCP-specific check at line 20 will never trigger because the shared validation would already return an error for non-"exact" schemes.However, if MCP semantics might diverge from the shared validation in the future, keeping this explicit check provides clarity and future-proofing. The current code is correct but redundant.
♻️ If redundancy is intentional for clarity, add a comment
// Use shared validation for most fields if err := validation.ValidatePaymentRequirements(req); err != nil { return err } - // MCP-specific validation: only 'exact' scheme is supported + // MCP-specific validation: only 'exact' scheme is supported. + // Note: This is also enforced by ValidatePaymentRequirements, but kept + // explicit here for MCP-specific error messaging and future-proofing. if req.Scheme != "exact" { return fmt.Errorf("invalid requirement: unsupported scheme %s (only 'exact' is supported in MCP)", req.Scheme) }
27-33: Consider validating or sanitizing toolName.
SetToolResourcedirectly interpolatestoolNameinto the URL without validation. Whilemcp://URLs may have different semantics than HTTP URLs, consider validating thattoolNamedoesn't contain characters that could cause parsing issues (e.g.,/,?,#).♻️ Suggested validation
+import ( + "fmt" + "strings" + + v2 "github.com/mark3labs/x402-go/v2" + "github.com/mark3labs/x402-go/v2/validation" +) + // SetToolResource sets the resource URL based on the tool name. // Returns a ResourceInfo with the standard MCP tool URL format. func SetToolResource(toolName string) v2.ResourceInfo { + // Sanitize toolName to prevent URL parsing issues + toolName = strings.ReplaceAll(toolName, "/", "_") + toolName = strings.ReplaceAll(toolName, "?", "_") + toolName = strings.ReplaceAll(toolName, "#", "_") return v2.ResourceInfo{ URL: fmt.Sprintf("mcp://tools/%s", toolName), } }v2/mcp/client/transport_test.go (2)
100-105: Consider validating specific error types whenwantErrTypeis set.The test returns early on error without checking
wantErrType. The "no accepts" case setswantErrType: mcp.ErrNoPaymentRequirementsbut this is never verified.Proposed fix
if tt.wantErr { if err == nil { t.Error("Expected error, got nil") } + if tt.wantErrType != nil && !errors.Is(err, tt.wantErrType) { + t.Errorf("Expected error type %v, got %v", tt.wantErrType, err) + } return }Note: You'd need to add
"errors"to the imports.
122-126: Unused transport variable.The transport is created but assigned to
_and never used. The test manually reimplements the injection logic rather than testing the actual transport method.Suggested improvement
Either remove the unused transport creation or refactor to test the actual transport injection method if one exists:
- _ = &Transport{ - config: DefaultConfig("http://example.com"), - }examples/v2/gin/main.go (1)
189-388: Consider extracting shared client code.The
runClient()function is nearly identical to the one inexamples/v2/http/main.go. For example applications this duplication is acceptable as it keeps each example self-contained, but if you add more examples, consider extracting common client logic to a shared package.v2/http/gin/middleware_test.go (2)
85-131: Test doesn't fully exercise VerifyOnly mode behavior.This test verifies 402 is returned without a payment header, but that's the same behavior regardless of
VerifyOnly. The distinctive behavior ofVerifyOnly=trueis that it skips settlement when a valid payment IS provided. Consider adding a test that provides a valid payment and verifies settlement is NOT called while the request still succeeds.
344-375: Consider table-driven test for GetPaymentFromContext.Per coding guidelines, test files should use table-driven tests. This test has three distinct scenarios that could be consolidated.
♻️ Suggested table-driven approach
func TestGetPaymentFromContext(t *testing.T) { expectedPayment := &v2.VerifyResponse{ IsValid: true, Payer: "0x742d35Cc6634C0532925a3b844Bc9e7595f0bEb0", } tests := []struct { name string setup func(*gin.Context) expected *v2.VerifyResponse }{ { name: "empty context", setup: func(c *gin.Context) {}, expected: nil, }, { name: "valid payment in context", setup: func(c *gin.Context) { c.Set(PaymentContextKey, expectedPayment) }, expected: expectedPayment, }, { name: "wrong type in context", setup: func(c *gin.Context) { c.Set(PaymentContextKey, "not a payment") }, expected: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c, _ := gin.CreateTestContext(httptest.NewRecorder()) tt.setup(c) result := GetPaymentFromContext(c) if result != tt.expected { t.Errorf("expected %v, got %v", tt.expected, result) } }) } }v2/http/facilitator_test.go (1)
351-395: Retry test has potential flakiness.The test relies on
http.Hijackerwhich may behave differently across environments. The fallback to 503 is good, but the assertionattempts < 2at line 392-394 could theoretically pass if the server implementation changes. Consider asserting exact expected attempts count for the success case (should be exactly 3 based on the mock logic).♻️ Suggested improvement
- if atomic.LoadInt32(&attempts) < 2 { - t.Errorf("Expected at least 2 attempts, got %d", attempts) + finalAttempts := atomic.LoadInt32(&attempts) + if finalAttempts != 3 { + t.Errorf("Expected exactly 3 attempts (2 failures + 1 success), got %d", finalAttempts) }v2/http/client.go (2)
55-73: Refactor to usegetOrCreateTransporthelper.The transport creation logic here duplicates the logic in
getOrCreateTransport(lines 138-151). Using the helper would reduce duplication and ensure consistent behavior.♻️ Proposed refactor
func WithSigner(signer v2.Signer) ClientOption { return func(c *Client) error { - // Get or create the X402Transport - transport, ok := c.Transport.(*X402Transport) - if !ok { - // Wrap the existing transport - transport = &X402Transport{ - Base: c.Transport, - Signers: []v2.Signer{}, - Selector: v2.NewDefaultPaymentSelector(), - } - c.Transport = transport - } - + transport := getOrCreateTransport(c) // Add the signer transport.Signers = append(transport.Signers, signer) return nil } }
76-94: Same DRY issue - usegetOrCreateTransporthelper.Apply the same refactor here for consistency.
♻️ Proposed refactor
func WithSelector(selector v2.PaymentSelector) ClientOption { return func(c *Client) error { - // Get or create the X402Transport - transport, ok := c.Transport.(*X402Transport) - if !ok { - // Wrap the existing transport - transport = &X402Transport{ - Base: c.Transport, - Signers: []v2.Signer{}, - Selector: selector, - } - c.Transport = transport - } else { - transport.Selector = selector - } - + transport := getOrCreateTransport(c) + transport.Selector = selector return nil } }v2/http/gin/middleware.go (2)
62-72: Consider making http.Client configurable or setting timeouts.The
http.Client{}is created without timeouts, which could cause requests to hang indefinitely if the facilitator is unresponsive. Consider usingv2.DefaultTimeoutsto configure the client's timeout or allowing thehttp.Clientto be passed via config.♻️ Suggested improvement
facilitator := &v2http.FacilitatorClient{ BaseURL: config.FacilitatorURL, - Client: &http.Client{}, + Client: &http.Client{Timeout: v2.DefaultTimeouts.VerifyTimeout + v2.DefaultTimeouts.SettleTimeout}, Timeouts: v2.DefaultTimeouts,Or better, allow the client to be configured via
Config.
90-98: EnrichRequirements blocks middleware initialization.This call happens during middleware creation and will block server startup if the facilitator is slow or unavailable. While the fallback behavior is good, consider whether this should be lazy-loaded on first request or performed asynchronously.
v2/mcp/server/server.go (1)
83-93: Consider using structured logging instead of fmt.Printf.Other parts of this codebase use
slog.Default()for logging (e.g., inmiddleware.go). Usingfmt.Printfhere is inconsistent. Consider using structured logging for consistency and better observability.♻️ Suggested improvement
+import "log/slog" + func (s *X402Server) Start(addr string) error { handler := s.Handler() if s.config.Verbose { - fmt.Printf("Starting x402 v2 MCP server on %s\n", addr) - fmt.Printf("Facilitator URL: %s\n", s.config.FacilitatorURL) - fmt.Printf("Verify-only mode: %v\n", s.config.VerifyOnly) - fmt.Printf("Protected tools: %d\n", len(s.config.PaymentTools)) + slog.Info("starting x402 v2 MCP server", + "addr", addr, + "facilitatorURL", s.config.FacilitatorURL, + "verifyOnly", s.config.VerifyOnly, + "protectedTools", len(s.config.PaymentTools), + ) } return http.ListenAndServe(addr, handler) }v2/selector.go (1)
148-160: Consider case-insensitive matching for network identifiers.The matching at line 151 is case-sensitive (
req.Network == payment.Accepted.Network). While CAIP-2 identifiers should be normalized, defensive case-insensitive matching (likestrings.EqualFold) would be more robust against inconsistent casing from different sources.♻️ Suggested improvement
func FindMatchingRequirement(payment *PaymentPayload, requirements []PaymentRequirements) (*PaymentRequirements, error) { for i := range requirements { req := &requirements[i] - if req.Network == payment.Accepted.Network && req.Scheme == payment.Accepted.Scheme { + if strings.EqualFold(req.Network, payment.Accepted.Network) && strings.EqualFold(req.Scheme, payment.Accepted.Scheme) { return req, nil } }v2/signers/svm/signer.go (1)
184-191: Decimals defaults to zero if token not found in configured tokens.If the loop at lines 186-191 doesn't find a matching token (which shouldn't happen since
CanSignpassed),decimalsremains at its zero value. WhileCanSignshould prevent this path, defensive coding would ensure this edge case is handled explicitly.Suggested improvement
// Get decimals for this token var decimals uint8 + var tokenFound bool for _, token := range s.tokens { if strings.EqualFold(token.Address, requirements.Asset) { decimals = uint8(token.Decimals) + tokenFound = true break } } + if !tokenFound { + return nil, fmt.Errorf("token %s not found in signer configuration", requirements.Asset) + }v2/mcp/server/handler.go (3)
72-79: Consider returning an error instead of panicking.Panicking at startup when the facilitator URL is missing can crash the application unexpectedly. Returning an error from
NewX402Handlerwould allow callers to handle the configuration error gracefully.Suggested approach
-func NewX402Handler(mcpHandler http.Handler, config *Config) *X402Handler { +func NewX402Handler(mcpHandler http.Handler, config *Config) (*X402Handler, error) { if config == nil { config = DefaultConfig() } - facilitator, fallbackFacilitator := initializeFacilitators(config) + facilitator, fallbackFacilitator, err := initializeFacilitators(config) + if err != nil { + return nil, err + } -func initializeFacilitators(config *Config) (Facilitator, Facilitator) { +func initializeFacilitators(config *Config) (Facilitator, Facilitator, error) { var facilitator, fallbackFacilitator Facilitator primaryURL := config.FacilitatorURL if primaryURL == "" { - panic("x402: at least one facilitator URL must be provided") + return nil, nil, fmt.Errorf("x402: at least one facilitator URL must be provided") }
159-172: Double JSON parsing can be consolidated.The
_metafield is parsed twice - once via struct unmarshaling and again viamap[string]interface{}. This could be simplified by parsing directly into a map.
253-277: Marshal/unmarshal roundtrip for payment extraction.The
extractPaymentmethod marshals and unmarshals the payment data from the already-parsed map. Consider usingmapstructureor direct field mapping to avoid the roundtrip, though this approach is functionally correct.v2/mcp/client/transport.go (1)
244-258: Payment failure callback fires for any non-402 error response.The callback at lines 246-256 only fires when
resp.Error.Code == 402. Other error codes (e.g., -32603 for internal errors, -32602 for invalid params) after payment injection won't trigger the failure callback. This may be intentional but could miss some failure scenarios.Consider broadening failure callback scope
// Check if payment succeeded if resp.Error != nil { - if resp.Error.Code == 402 && t.config.OnPaymentFailure != nil { + if t.config.OnPaymentFailure != nil { t.config.OnPaymentFailure(v2.PaymentEvent{ Type: v2.PaymentEventFailure, Timestamp: time.Now(), Method: "MCP", - Error: fmt.Errorf("payment rejected: %s", resp.Error.Message), + Error: fmt.Errorf("request failed after payment (code %d): %s", resp.Error.Code, resp.Error.Message), Network: payment.Accepted.Network, Scheme: payment.Accepted.Scheme, Duration: duration, }) } return resp, nil }v2/signers/evm/signer.go (2)
166-168: Usev2.X402Versionconstant instead of hardcoded value.The constant
v2.X402Version(value 2) is defined inv2/types.goand should be used for consistency.Suggested fix
payload := &v2.PaymentPayload{ - X402Version: 2, + X402Version: v2.X402Version, Accepted: *requirements,
201-222: Consider extracting chain ID from CAIP-2 identifier programmatically.The
GetChainIDfunction uses a switch statement with hardcoded network strings. Since EVM CAIP-2 identifiers follow the patterneip155:<chainId>, the chain ID could be parsed directly from the string, reducing maintenance burden when adding new networks.Suggested approach
func GetChainID(network string) (int64, error) { if !strings.HasPrefix(network, "eip155:") { return 0, v2.ErrInvalidNetwork } chainIDStr := strings.TrimPrefix(network, "eip155:") chainID, err := strconv.ParseInt(chainIDStr, 10, 64) if err != nil { return 0, fmt.Errorf("%w: invalid chain ID format", v2.ErrInvalidNetwork) } return chainID, nil }This would automatically support any EVM network without code changes.
v2/http/facilitator.go (2)
262-270:Supportedendpoint reusesVerifyTimeout- consider dedicated timeout.The
/supportedendpoint usesVerifyTimeoutwhich may not be appropriate since it's a simple GET request that should complete quickly. A dedicatedSupportedTimeoutor a shorter default would be more fitting.
298-306:EnrichRequirementsusescontext.Background()- consider accepting context parameter.Using
context.Background()means this call cannot be cancelled or have its deadline controlled by the caller. Since this is called during middleware initialization (not per-request), it may be acceptable, but accepting a context parameter would provide more flexibility.Suggested improvement
-func (c *FacilitatorClient) EnrichRequirements(requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) { +func (c *FacilitatorClient) EnrichRequirements(ctx context.Context, requirements []v2.PaymentRequirements) ([]v2.PaymentRequirements, error) { // Fetch supported payment types - supported, err := c.Supported(context.Background()) + supported, err := c.Supported(ctx) if err != nil { return requirements, fmt.Errorf("failed to fetch supported payment types: %w", err) }v2/mcp/server/facilitator.go (1)
15-21: Interface uses pointer*v2.PaymentPayloadwhile underlying client uses value.The
Facilitatorinterface accepts*v2.PaymentPayload, but the underlyingv2http.FacilitatorClient.Verify/Settlemethods acceptv2.PaymentPayloadby value (see lines 107, 117 where it's dereferenced). This inconsistency works but adds cognitive overhead. Consider aligning the interface signature with the underlying implementation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
AGENTS.mdexamples/v2/gin/main.goexamples/v2/http/main.goexamples/v2/mcp/main.gov2/chains.gov2/chains_test.gov2/config.gov2/encoding/encoding.gov2/encoding/encoding_test.gov2/errors.gov2/events.gov2/facilitator/interface.gov2/http/client.gov2/http/client_test.gov2/http/facilitator.gov2/http/facilitator_test.gov2/http/gin/middleware.gov2/http/gin/middleware_test.gov2/http/internal/helpers/helpers.gov2/http/internal/helpers/helpers_test.gov2/http/middleware.gov2/http/middleware_test.gov2/http/transport.gov2/http/transport_test.gov2/internal/eip3009/eip3009.gov2/internal/eip3009/eip3009_test.gov2/internal/solana/solana.gov2/mcp/client/config.gov2/mcp/client/transport.gov2/mcp/client/transport_test.gov2/mcp/errors.gov2/mcp/server/config.gov2/mcp/server/facilitator.gov2/mcp/server/handler.gov2/mcp/server/handler_test.gov2/mcp/server/requirements.gov2/mcp/server/server.gov2/mcp/types.gov2/selector.gov2/selector_test.gov2/signer.gov2/signers/evm/signer.gov2/signers/evm/signer_test.gov2/signers/svm/signer.gov2/signers/svm/signer_test.gov2/types.gov2/types_test.gov2/validation/validation.gov2/validation/validation_test.go
💤 Files with no reviewable changes (1)
- AGENTS.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Group imports as: stdlib, external dependencies, then internal packages with blank lines between groups
Use camelCase for variables and functions, PascalCase for exported items, avoid abbreviations
Always check errors; wrap with context usingfmt.Errorf("context: %w", err)for error handling
For exported functions, start comments with function name; use//for inline comments,/* */for block comments
Prefer channels over mutexes for concurrency; always handle goroutine lifecycles properly
Files:
v2/http/middleware.gov2/events.gov2/signers/svm/signer_test.gov2/mcp/server/requirements.goexamples/v2/http/main.gov2/http/internal/helpers/helpers_test.goexamples/v2/gin/main.gov2/mcp/types.gov2/http/facilitator_test.gov2/mcp/client/transport_test.gov2/http/gin/middleware.gov2/internal/eip3009/eip3009_test.gov2/selector.gov2/mcp/client/config.gov2/facilitator/interface.gov2/http/gin/middleware_test.gov2/encoding/encoding.gov2/config.gov2/http/middleware_test.goexamples/v2/mcp/main.gov2/validation/validation.gov2/http/transport.gov2/mcp/errors.gov2/http/client_test.gov2/encoding/encoding_test.gov2/http/client.gov2/http/transport_test.gov2/http/internal/helpers/helpers.gov2/errors.gov2/internal/eip3009/eip3009.gov2/mcp/server/handler.gov2/chains_test.gov2/mcp/server/server.gov2/signer.gov2/selector_test.gov2/types_test.gov2/validation/validation_test.gov2/mcp/server/handler_test.gov2/signers/svm/signer.gov2/chains.gov2/signers/evm/signer_test.gov2/internal/solana/solana.gov2/mcp/server/config.gov2/types.gov2/mcp/client/transport.gov2/mcp/server/facilitator.gov2/signers/evm/signer.gov2/http/facilitator.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Test files must end with
_test.go; use table-driven tests; mock external dependencies
Files:
v2/signers/svm/signer_test.gov2/http/internal/helpers/helpers_test.gov2/http/facilitator_test.gov2/mcp/client/transport_test.gov2/internal/eip3009/eip3009_test.gov2/http/gin/middleware_test.gov2/http/middleware_test.gov2/http/client_test.gov2/encoding/encoding_test.gov2/http/transport_test.gov2/chains_test.gov2/selector_test.gov2/types_test.gov2/validation/validation_test.gov2/mcp/server/handler_test.gov2/signers/evm/signer_test.go
🧠 Learnings (1)
📚 Learning: 2025-12-04T10:19:11.236Z
Learnt from: CR
Repo: mark3labs/x402-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:19:11.236Z
Learning: Applies to **/*_test.go : Test files must end with `_test.go`; use table-driven tests; mock external dependencies
Applied to files:
v2/http/internal/helpers/helpers_test.gov2/mcp/client/transport_test.gov2/internal/eip3009/eip3009_test.gov2/http/transport_test.gov2/chains_test.gov2/types_test.gov2/mcp/server/handler_test.go
🧬 Code graph analysis (31)
v2/http/middleware.go (3)
v2/types.go (1)
ResourceInfo(18-27)v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)v2/http/internal/helpers/helpers.go (4)
BuildResourceURL(93-99)SendPaymentRequired(34-45)ParsePaymentHeader(14-31)AddPaymentResponseHeader(48-55)
v2/signers/svm/signer_test.go (4)
v2/signers/svm/signer.go (5)
Option(32-32)WithPriority(107-112)WithMaxAmount(99-104)NewSigner(35-42)NewSignerFromKeygenFile(78-96)v2/chains.go (2)
NetworkSolanaMainnet(36-36)NetworkSolanaDevnet(37-37)v2/types.go (1)
X402Version(15-15)v2/internal/solana/solana.go (1)
ComputeBudgetProgramID(15-15)
v2/mcp/server/requirements.go (3)
v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/validation/validation.go (1)
ValidatePaymentRequirements(111-167)v2/types.go (1)
ResourceInfo(18-27)
v2/mcp/types.go (1)
v2/types.go (3)
X402Version(15-15)ResourceInfo(18-27)Extension(55-61)
v2/http/gin/middleware.go (3)
v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)v2/http/internal/helpers/helpers.go (2)
BuildResourceURL(93-99)ParsePaymentHeader(14-31)v2/types.go (3)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)
v2/internal/eip3009/eip3009_test.go (1)
v2/internal/eip3009/eip3009.go (4)
GenerateNonce(46-52)CreateAuthorization(26-44)SignAuthorization(54-110)Authorization(17-24)
v2/selector.go (3)
v2/signer.go (1)
Signer(8-32)v2/types.go (2)
PaymentRequirements(31-52)PaymentPayload(82-99)v2/errors.go (8)
NewPaymentError(114-121)ErrCodeNoValidSigner(67-67)ErrNoValidSigner(8-8)ErrCodeInvalidRequirements(73-73)ErrInvalidRequirements(14-14)ErrCodeSigningFailed(76-76)ErrCodeUnsupportedScheme(82-82)ErrUnsupportedScheme(59-59)
v2/facilitator/interface.go (1)
v2/types.go (6)
PaymentPayload(82-99)PaymentRequirements(31-52)VerifyResponse(140-149)SettleResponse(152-167)SupportedResponse(185-194)X402Version(15-15)
v2/http/gin/middleware_test.go (2)
v2/http/gin/middleware.go (4)
Config(18-18)NewX402Middleware(60-208)GetPaymentFromContext(225-235)PaymentContextKey(21-21)v2/types.go (3)
ResourceInfo(18-27)PaymentRequired(64-79)X402Version(15-15)
v2/http/middleware_test.go (2)
v2/types.go (4)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)SettleResponse(152-167)v2/http/middleware.go (3)
Config(16-59)NewX402Middleware(71-216)GetPaymentFromContext(305-315)
examples/v2/mcp/main.go (3)
v2/chains.go (1)
GetChainConfig(186-192)v2/types.go (1)
ResourceInfo(18-27)v2/signers/svm/signer.go (1)
NewSignerFromKeygenFile(78-96)
v2/validation/validation.go (2)
v2/chains.go (3)
ValidateNetwork(197-231)NetworkTypeEVM(16-16)NetworkTypeSVM(18-18)v2/types.go (5)
ResourceInfo(18-27)PaymentRequirements(31-52)PaymentPayload(82-99)X402Version(15-15)PaymentRequired(64-79)
v2/http/transport.go (1)
v2/http/internal/helpers/helpers.go (3)
ParsePaymentRequirements(58-70)BuildPaymentHeader(88-90)ParseSettlement(74-85)
v2/http/client_test.go (3)
v2/http/client.go (8)
NewClient(21-40)Client(13-15)WithSigner(55-73)WithSelector(76-94)WithPaymentCallback(97-116)WithPaymentCallbacks(120-136)WithHTTPClient(43-51)GetSettlement(155-161)v2/http/transport.go (1)
X402Transport(13-31)v2/types.go (1)
SettleResponse(152-167)
v2/encoding/encoding_test.go (2)
v2/types.go (8)
PaymentPayload(82-99)X402Version(15-15)ResourceInfo(18-27)PaymentRequirements(31-52)SettleResponse(152-167)PaymentRequired(64-79)Extension(55-61)VerifyResponse(140-149)v2/encoding/encoding.go (8)
EncodePayment(17-23)DecodePayment(28-41)EncodeSettlement(47-53)DecodeSettlement(58-71)EncodeRequirements(76-82)DecodeRequirements(87-100)EncodeVerifyResponse(105-111)DecodeVerifyResponse(116-129)
v2/http/client.go (3)
v2/http/transport.go (1)
X402Transport(13-31)v2/types.go (1)
SettleResponse(152-167)v2/http/internal/helpers/helpers.go (1)
ParseSettlement(74-85)
v2/http/transport_test.go (2)
v2/types.go (4)
X402Version(15-15)PaymentRequired(64-79)ResourceInfo(18-27)SettleResponse(152-167)v2/http/transport.go (1)
X402Transport(13-31)
v2/http/internal/helpers/helpers.go (1)
v2/types.go (4)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)SettleResponse(152-167)
v2/chains_test.go (4)
v2/chains.go (16)
ChainConfig(41-56)BaseMainnet(62-68)PolygonMainnet(72-78)AvalancheMainnet(82-88)EthereumMainnet(92-98)BaseSepolia(105-111)PolygonAmoy(115-121)AvalancheFuji(125-131)Sepolia(135-141)SolanaMainnet(148-154)SolanaDevnet(158-164)ValidateNetwork(197-231)GetChainID(235-251)GetSolanaGenesisHash(255-266)GetChainConfig(186-192)NewUSDCTokenConfig(270-278)v2/validation/validation_test.go (1)
TestValidateNetwork(76-128)v2/validation/validation.go (1)
ValidateNetwork(49-61)v2/errors.go (1)
ErrInvalidNetwork(29-29)
v2/signer.go (1)
v2/types.go (3)
PaymentRequirements(31-52)PaymentPayload(82-99)TokenConfig(197-213)
v2/selector_test.go (4)
v2/types.go (6)
TokenConfig(197-213)PaymentRequirements(31-52)PaymentPayload(82-99)X402Version(15-15)EVMPayload(102-108)EVMAuthorization(111-129)v2/selector.go (1)
FindMatchingRequirement(148-160)v2/signer.go (1)
Signer(8-32)v2/errors.go (5)
ErrorCode(63-63)ErrCodeNoValidSigner(67-67)ErrCodeInvalidRequirements(73-73)PaymentError(86-98)ErrCodeSigningFailed(76-76)
v2/types_test.go (1)
v2/types.go (15)
X402Version(15-15)ResourceInfo(18-27)PaymentRequirements(31-52)PaymentRequired(64-79)PaymentPayload(82-99)EVMPayload(102-108)EVMAuthorization(111-129)Extension(55-61)VerifyResponse(140-149)SettleResponse(152-167)SupportedResponse(185-194)SupportedKind(170-182)AmountToBigInt(217-234)BigIntToAmount(238-249)TokenConfig(197-213)
v2/validation/validation_test.go (2)
v2/validation/validation.go (7)
ValidateAmount(28-45)ValidateNetwork(49-61)ValidateAddress(66-92)ValidateResourceInfo(96-107)ValidatePaymentRequirements(111-167)ValidatePaymentPayload(171-201)ValidatePaymentRequired(204-224)v2/types.go (5)
ResourceInfo(18-27)PaymentRequirements(31-52)PaymentPayload(82-99)X402Version(15-15)PaymentRequired(64-79)
v2/signers/svm/signer.go (2)
v2/internal/solana/solana.go (7)
GetRPCURL(90-99)DeriveAssociatedTokenAddress(81-87)BuildSetComputeUnitLimitInstruction(43-56)DefaultComputeUnits(18-18)BuildSetComputeUnitPriceInstruction(61-78)DefaultComputeUnitPrice(21-21)BuildTransferCheckedInstruction(24-38)v2/types.go (1)
X402Version(15-15)
v2/chains.go (3)
v2/errors.go (1)
ErrInvalidNetwork(29-29)v2/validation/validation.go (1)
ValidateNetwork(49-61)v2/types.go (1)
TokenConfig(197-213)
v2/signers/evm/signer_test.go (3)
v2/signers/evm/signer.go (3)
NewSigner(28-57)WithMaxAmount(91-96)GetChainID(201-222)v2/types.go (1)
X402Version(15-15)v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)
v2/internal/solana/solana.go (1)
v2/chains.go (2)
NetworkSolanaMainnet(36-36)NetworkSolanaDevnet(37-37)
v2/mcp/server/config.go (3)
v2/types.go (1)
ResourceInfo(18-27)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/mcp/server/handler.go (4)
AuthorizationProvider(50-50)OnBeforeFunc(53-53)OnAfterVerifyFunc(56-56)OnAfterSettleFunc(59-59)
v2/types.go (2)
v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)v2/errors.go (1)
ErrInvalidAmount(23-23)
v2/mcp/client/transport.go (4)
v2/mcp/client/config.go (3)
Config(11-35)Option(38-38)DefaultConfig(99-106)v2/mcp/errors.go (1)
WrapX402Error(74-82)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/types.go (2)
ResourceInfo(18-27)X402Version(15-15)
v2/signers/evm/signer.go (2)
v2/internal/eip3009/eip3009.go (3)
CreateAuthorization(26-44)SignAuthorization(54-110)Authorization(17-24)v2/types.go (1)
X402Version(15-15)
🪛 Gitleaks (8.30.0)
v2/signers/svm/signer_test.go
[high] 18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Return ErrNoPaymentRequirements when resp.Error.Data is nil or empty instead of attempting to unmarshal and causing 'unexpected end of JSON input' errors. This provides clearer error messages for clients when payment requirements are missing from 402 responses.
- Fix case-insensitive base58 address comparison (Solana base58 is case-sensitive) - Add uint64 overflow check before converting big.Int amounts - Add nil requirements validation in CanSign - Add negative/zero amount validation - Add token decimals range validation (0-255) - Add explicit token not found error when asset doesn't match - Update test to expect case-sensitive behavior This fixes a critical security issue where incorrect token addresses could match due to case-insensitive comparison, and prevents silent truncation of large amounts during uint64 conversion.
Fix error wrapping in GetRPCURL to place sentinel error at end with %w verb, allowing errors.Is/As to properly unwrap v2.ErrInvalidNetwork. Previous format broke error chain unwrapping.
Add idempotent associated token account creation instruction before SPL token transfers to ensure the destination ATA exists. The facilitator sponsors the rent-exempt balance as fee payer. Without this instruction, transfers fail when the recipient's ATA does not exist, which is a functional requirement of the exact_svm specification. Fixes critical issue from code review: v2/signers/svm/signer.go lines 283-314
Fixes issue where Hijack() could bypass payment settlement for WebSocket upgrades and other connection takeover scenarios. Now settles payment first before allowing hijack, preventing potential payment bypass.
The GetPaymentConfig function was returning a pointer to a local copy of the map value, which is misleading for callers who might expect to modify the stored configuration. Changed the signature to return (ToolPaymentConfig, bool) for clearer read-only semantics that match Go idioms (similar to map lookups).
Replace panic() with error returns in NewX402Handler and initializeFacilitators. Panicking in library code crashes consuming applications; returning errors allows callers to handle configuration failures gracefully. Changes: - NewX402Handler now returns (*X402Handler, error) instead of *X402Handler - initializeFacilitators now returns (Facilitator, Facilitator, error) - Handler() methods in X402Server now return (http.Handler, error) - Updated Start() methods to handle handler creation errors - All tests pass with race detection Fixes issue from scratch/comments.md (v2/mcp/server/handler.go:72-79)
Reject payment payloads with unsupported x402Version to prevent processing incompatible payments. This ensures verify/settle logic only operates on version 2 payloads as expected by the protocol.
- Change EnrichRequirements signature to accept context.Context as first parameter - Allows callers to control timeouts and cancellation for facilitator requests - Update all call sites to use context.WithTimeout(context.Background(), RequestTimeout) - Add context import to http/pocketbase/middleware.go - Update both v1 and v2 implementations for consistency - All tests pass with race detection
- Set HTTP client timeout to DefaultTimeouts.RequestTimeout in all middleware - Prevents indefinite hangs when facilitator is unresponsive - Affects v1 and v2 standard middleware, Gin middleware, and PocketBase middleware - All 6 facilitator client instantiations now have explicit timeout - Tests pass with race detection Resolves issue from scratch/comments.md: HTTP clients without timeout configuration
The GetLatestBlockhash call was using context.Background() which could hang indefinitely if the RPC node is unresponsive. Now uses a timeout context with DefaultTimeouts.VerifyTimeout to prevent hangs.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
http/middleware.go (1)
294-300: Hijack() may bypass settlement.If the handler calls
Hijack()beforeWrite()orWriteHeader(), thecommittedflag remains false and settlement never occurs. This could allow bypassing payment for WebSocket upgrades or similar connection hijacking scenarios.Consider triggering settlement before allowing the hijack, similar to how Write triggers WriteHeader:
🔧 Proposed fix
func (i *settlementInterceptor) Hijack() (net.Conn, *bufio.ReadWriter, error) { + // Ensure settlement occurs before connection hijack + if !i.committed { + i.WriteHeader(http.StatusOK) + } + if i.hijacked { + return nil, nil, errors.New("settlement failed, cannot hijack") + } if hijacker, ok := i.w.(http.Hijacker); ok { return hijacker.Hijack() } return nil, nil, errors.New("hijacking not supported") }
🤖 Fix all issues with AI agents
In `@ralph.nu`:
- Around line 1-19: The file ralph.nu is a local development automation script
(defines prompt and a for loop using variables like prompt and the loop "for i
in 1..10") and should not be committed; remove ralph.nu from the PR or add its
path to .gitignore and delete it from the commit (or move it to a non-tracked
location), then amend the commit so the script is no longer in the repo; if you
prefer to keep it but not track changes, add its filename to .gitignore and run
git rm --cached on ralph.nu before committing.
In `@v2/signers/svm/signer.go`:
- Around line 336-342: The ATA creation call currently uses
associatedtokenaccount.NewCreateInstruction(feePayer, recipient, mint).Build()
but that is not idempotent; replace it with
associatedtokenaccount.NewCreateIdempotentInstruction(feePayer, recipient,
mint).Build() so the transaction succeeds if the ATA already exists, and update
the inline comment to reflect true idempotency; ensure any references to
NewCreateInstruction in the same file (e.g., the instruction assembly around
signer.go's ATA creation) are updated accordingly.
🧹 Nitpick comments (15)
v2/internal/solana/solana.go (1)
40-56: Consider usingencoding/binaryfor little-endian encoding.The manual byte manipulation is correct, but using
binary.LittleEndian.PutUint32could improve readability and reduce the chance of encoding errors in future modifications.♻️ Optional refactor
+import "encoding/binary" + func BuildSetComputeUnitLimitInstruction(units uint32) solana.Instruction { data := make([]byte, 5) data[0] = 2 // SetComputeUnitLimit discriminator - data[1] = byte(units) - data[2] = byte(units >> 8) - data[3] = byte(units >> 16) - data[4] = byte(units >> 24) + binary.LittleEndian.PutUint32(data[1:], units) return solana.NewInstruction( ComputeBudgetProgramID, solana.AccountMetaSlice{}, data, ) }http/pocketbase/middleware.go (1)
202-252: Consider using shared helpers to reduce code duplication.The
parsePaymentHeaderFromRequestandfindMatchingRequirementPocketBasefunctions duplicate logic that exists in thehelperspackage. The Gin middleware (line 205 inhttp/gin/middleware.go) useshelpers.ParsePaymentHeaderFromRequest. Consider refactoring to use the shared helpers for consistency and maintainability.♻️ Suggested refactor
+import ( + "github.com/mark3labs/x402-go/http/internal/helpers" +) -// parsePaymentHeaderFromRequest parses the X-PAYMENT header from an http.Request. -// It decodes the base64-encoded JSON, unmarshals it, and validates the protocol version. -func parsePaymentHeaderFromRequest(r *http.Request) (x402.PaymentPayload, error) { - // ... implementation ... -} +// parsePaymentHeaderFromRequest parses the X-PAYMENT header from an http.Request. +func parsePaymentHeaderFromRequest(r *http.Request) (x402.PaymentPayload, error) { + return helpers.ParsePaymentHeaderFromRequest(r) +} -// findMatchingRequirementPocketBase finds a payment requirement matching the payment's scheme and network. -func findMatchingRequirementPocketBase(payment x402.PaymentPayload, requirements []x402.PaymentRequirement) (x402.PaymentRequirement, error) { - // ... implementation ... -} +// findMatchingRequirementPocketBase finds a payment requirement that matches the provided payment. +func findMatchingRequirementPocketBase(payment x402.PaymentPayload, requirements []x402.PaymentRequirement) (x402.PaymentRequirement, error) { + return helpers.FindMatchingRequirement(payment, requirements) +}mcp/server/handler.go (1)
406-413: Clarify the semantics ofSuccess=falsein verify-only mode.The comment states that
Success=falseindicates settlement was skipped, but this could be confusing to API consumers who might interpret it as a failure. Consider using a separate field (e.g.,Skipped: true) or documenting this behavior more explicitly in the response structure.v2/signers/svm/signer_test.go (3)
95-112: Consider usingerrors.Is()for sentinel error assertions.For test cases with
errTarget, useerrors.Is()to properly handle wrapped errors, which aligns with the project's error wrapping practices usingfmt.Errorf("context: %w", err).Proposed fix
t.Run(tt.name, func(t *testing.T) { signer, err := NewSigner(tt.network, tt.key, tt.tokens, tt.opts...) if tt.wantErr { if err == nil { t.Fatal("expected error, got nil") } + if tt.errTarget != nil && !errors.Is(err, tt.errTarget) { + t.Errorf("expected error %v, got %v", tt.errTarget, err) + } return }
728-740: Usestrings.Containsfrom the standard library.The helper functions
containsStringandindexOfSubstringduplicate functionality available instrings.Contains.Proposed fix
+import "strings" + // Helper function to check if error message contains expected string -func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || indexOfSubstring(s, substr) >= 0) -} - -func indexOfSubstring(s, substr string) int { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return i - } - } - return -1 -} +func containsString(s, substr string) bool { + return strings.Contains(s, substr) +}
358-366: Inconsistent error assertion logic.The test uses both
err != tt.wantErr(direct comparison) andcontainsStringas a fallback. For sentinel errors, useerrors.Is()for proper comparison of wrapped errors.Proposed fix
if tt.wantErr != nil { if err == nil { t.Fatalf("expected error %v, got nil", tt.wantErr) } - if err != tt.wantErr && !containsString(err.Error(), tt.wantErr.Error()) { + if !errors.Is(err, tt.wantErr) { t.Fatalf("expected error %v, got %v", tt.wantErr, err) } return }v2/mcp/client/config.go (1)
101-109: Consider documenting the sharedhttp.DefaultClientusage.Using
http.DefaultClientis convenient but callers modifying its transport will affect all users. Consider adding a comment noting this, or creating a new client instance with default settings.Alternative: create a new client instance
func DefaultConfig(serverURL string) *Config { return &Config{ ServerURL: serverURL, - HTTPClient: http.DefaultClient, + HTTPClient: &http.Client{}, Selector: v2.NewDefaultPaymentSelector(), Signers: make([]v2.Signer, 0), } }v2/mcp/client/transport.go (3)
34-37: Redundant selector initialization.
DefaultConfigat line 106 in config.go already initializesSelectortov2.NewDefaultPaymentSelector(). This check is unnecessary unless opts can set it to nil.Consider removing redundant check
- // Use default selector if none provided - if config.Selector == nil { - config.Selector = v2.NewDefaultPaymentSelector() - }
220-226: Shallow copy may cause unintended side effects.Line 222-223 creates a shallow copy of
req, butparamsis a map that was potentially extracted fromreq.Params. Modifications toparams(line 218-219) could affect the original request if the caller retains a reference.Consider deep copying the params map
// Create modified request - modifiedReq := req - modifiedReq.Params = params + modifiedReq := transport.JSONRPCRequest{ + JSONRPC: req.JSONRPC, + Method: req.Method, + ID: req.ID, + Params: params, + } return modifiedReq, nil
248-262: Non-402 errors after retry should also trigger failure callback.Currently, only 402 errors trigger
OnPaymentFailureafter retry. Other error codes (e.g., 500, 400) indicate the payment attempt failed but don't invoke the callback, which could leave observability gaps.Trigger failure callback for all error responses
// Check if payment succeeded if resp.Error != nil { - if resp.Error.Code == 402 && t.config.OnPaymentFailure != nil { + if t.config.OnPaymentFailure != nil { t.config.OnPaymentFailure(v2.PaymentEvent{ Type: v2.PaymentEventFailure, Timestamp: time.Now(), Method: "MCP", - Error: fmt.Errorf("payment rejected: %s", resp.Error.Message), + Error: fmt.Errorf("request failed (code %d): %s", resp.Error.Code, resp.Error.Message), Network: payment.Accepted.Network, Scheme: payment.Accepted.Scheme, Duration: duration, }) } return resp, nil }v2/mcp/server/handler_test.go (1)
52-95: Consider adding assertion for response body content.The test verifies status code but doesn't validate the actual response body matches
mcpResponse. This could miss serialization issues.🔧 Suggested enhancement
if resp.StatusCode != http.StatusOK { t.Errorf("Expected status 200, got %d", resp.StatusCode) } + + var jsonrpcResp map[string]interface{} + if err := json.NewDecoder(resp.Body).Decode(&jsonrpcResp); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + if _, hasResult := jsonrpcResp["result"]; !hasResult { + t.Error("Expected result in response") + }v2/http/facilitator_test.go (1)
93-96: Consider setting Timeouts field for consistency.This test creates a
FacilitatorClientwithout theTimeoutsfield, which may rely on zero-value behavior. For consistency with other tests, consider settingTimeouts: v2.DefaultTimeouts.v2/mcp/server/server.go (1)
90-96: Consider using structured logger instead offmt.Printffor verbose output.The verbose startup messages use
fmt.Printfdirectly, while the rest of the codebase usesslog.Logger. For consistency and to enable structured logging in production, consider usings.config.Logger(if available) instead.♻️ Suggested refactor
if s.config.Verbose { - fmt.Printf("Starting x402 v2 MCP server on %s\n", addr) - fmt.Printf("Facilitator URL: %s\n", s.config.FacilitatorURL) - fmt.Printf("Verify-only mode: %v\n", s.config.VerifyOnly) - fmt.Printf("Protected tools: %d\n", len(s.config.PaymentTools)) + s.config.Logger.Info("Starting x402 v2 MCP server", + "address", addr, + "facilitatorURL", s.config.FacilitatorURL, + "verifyOnly", s.config.VerifyOnly, + "protectedTools", len(s.config.PaymentTools)) }v2/mcp/server/handler.go (1)
122-128: Consider limiting body read size to prevent memory exhaustion.The
io.ReadAll(r.Body)at line 123 reads the entire request body without a size limit. A malicious client could send an extremely large payload to exhaust server memory.♻️ Suggested fix using io.LimitReader
+const maxBodySize = 10 * 1024 * 1024 // 10MB limit + // Read request body -bodyBytes, err := io.ReadAll(r.Body) +bodyBytes, err := io.ReadAll(io.LimitReader(r.Body, maxBodySize)) if err != nil { h.writeError(w, nil, -32700, "Parse error", nil) return }v2/http/facilitator.go (1)
262-296: Consider adding retry capability toSupportedfor consistency.The
Supportedmethod doesn't use the retry mechanism thatVerifyandSettleuse. While this may be intentional (since it's typically called at startup), transient network issues during server initialization could cause unnecessary failures.Is the absence of retry on
Supportedintentional? If it's called during middleware initialization (like in the Gin middleware), a transient failure would prevent the server from starting.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
http/facilitator.gohttp/gin/middleware.gohttp/middleware.gohttp/pocketbase/middleware.gomcp/server/handler.gomcp/server/server.goralph.nuv2/config.gov2/http/client_test.gov2/http/facilitator.gov2/http/facilitator_test.gov2/http/gin/middleware.gov2/http/middleware.gov2/internal/solana/solana.gov2/mcp/client/config.gov2/mcp/client/transport.gov2/mcp/server/config.gov2/mcp/server/handler.gov2/mcp/server/handler_test.gov2/mcp/server/server.gov2/signers/svm/signer.gov2/signers/svm/signer_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- v2/mcp/server/config.go
- v2/http/middleware.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Group imports as: stdlib, external dependencies, then internal packages with blank lines between groups
Use camelCase for variables and functions, PascalCase for exported items, avoid abbreviations
Always check errors; wrap with context usingfmt.Errorf("context: %w", err)for error handling
For exported functions, start comments with function name; use//for inline comments,/* */for block comments
Prefer channels over mutexes for concurrency; always handle goroutine lifecycles properly
Files:
http/pocketbase/middleware.gov2/http/gin/middleware.gohttp/gin/middleware.gomcp/server/handler.gohttp/facilitator.gov2/mcp/server/server.gov2/signers/svm/signer.gov2/internal/solana/solana.gov2/mcp/server/handler.gov2/signers/svm/signer_test.gov2/http/facilitator_test.gov2/mcp/client/transport.gov2/mcp/server/handler_test.gov2/mcp/client/config.gomcp/server/server.gov2/http/client_test.gohttp/middleware.gov2/http/facilitator.gov2/config.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Test files must end with
_test.go; use table-driven tests; mock external dependencies
Files:
v2/signers/svm/signer_test.gov2/http/facilitator_test.gov2/mcp/server/handler_test.gov2/http/client_test.go
🧠 Learnings (2)
📚 Learning: 2025-12-04T10:19:11.236Z
Learnt from: CR
Repo: mark3labs/x402-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:19:11.236Z
Learning: Applies to **/*.go : Always check errors; wrap with context using `fmt.Errorf("context: %w", err)` for error handling
Applied to files:
v2/internal/solana/solana.go
📚 Learning: 2025-12-04T10:19:11.236Z
Learnt from: CR
Repo: mark3labs/x402-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:19:11.236Z
Learning: Applies to **/*_test.go : Test files must end with `_test.go`; use table-driven tests; mock external dependencies
Applied to files:
v2/mcp/server/handler_test.go
🧬 Code graph analysis (14)
v2/http/gin/middleware.go (3)
http/middleware.go (3)
Config(16-56)NewX402Middleware(68-223)PaymentContextKey(62-62)v2/http/middleware.go (3)
Config(16-59)NewX402Middleware(71-218)PaymentContextKey(65-65)v2/types.go (3)
X402Version(15-15)ResourceInfo(18-27)PaymentRequired(64-79)
http/gin/middleware.go (6)
http/client.go (1)
Client(12-14)config.go (1)
DefaultTimeouts(21-25)v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)http/facilitator.go (1)
AuthorizationProvider(26-26)v2/http/facilitator.go (1)
AuthorizationProvider(26-26)mcp/types.go (1)
PaymentRequirements(9-13)
mcp/server/handler.go (2)
v2/mcp/server/handler.go (2)
NewX402Handler(24-40)X402Handler(16-21)mcp/server/facilitator.go (1)
Facilitator(18-24)
http/facilitator.go (1)
types.go (1)
PaymentRequirement(60-93)
v2/mcp/server/server.go (6)
v2/mcp/server/config.go (3)
Config(21-64)DefaultConfig(67-75)ToolPaymentConfig(12-18)mcp/server/server.go (2)
NewX402Server(19-36)X402Server(13-16)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/mcp/server/requirements.go (2)
ValidateRequirement(13-25)SetToolResource(29-33)v2/mcp/server/handler.go (1)
NewX402Handler(24-40)mcp/server/handler.go (1)
NewX402Handler(26-42)
v2/signers/svm/signer.go (2)
v2/internal/solana/solana.go (7)
GetRPCURL(90-99)DeriveAssociatedTokenAddress(81-87)BuildSetComputeUnitLimitInstruction(43-56)DefaultComputeUnits(18-18)BuildSetComputeUnitPriceInstruction(61-78)DefaultComputeUnitPrice(21-21)BuildTransferCheckedInstruction(24-38)v2/types.go (1)
X402Version(15-15)
v2/internal/solana/solana.go (1)
v2/chains.go (2)
NetworkSolanaMainnet(36-36)NetworkSolanaDevnet(37-37)
v2/http/facilitator_test.go (6)
v2/types.go (7)
VerifyResponse(140-149)PaymentPayload(82-99)X402Version(15-15)PaymentRequirements(31-52)SettleResponse(152-167)SupportedResponse(185-194)SupportedKind(170-182)http/facilitator.go (1)
AuthorizationProvider(26-26)v2/http/facilitator.go (1)
AuthorizationProvider(26-26)v2/config.go (2)
DefaultTimeouts(21-25)TimeoutConfig(9-18)v2/internal/eip3009/eip3009.go (1)
Authorization(17-24)v2/errors.go (3)
ErrVerificationFailed(47-47)ErrSettlementFailed(50-50)ErrFacilitatorUnavailable(44-44)
v2/mcp/client/transport.go (5)
v2/mcp/client/config.go (3)
Config(11-35)Option(38-38)DefaultConfig(102-109)v2/mcp/server/config.go (2)
Config(21-64)DefaultConfig(67-75)v2/mcp/errors.go (2)
ErrNoPaymentRequirements(27-27)WrapX402Error(74-82)v2/mcp/types.go (1)
PaymentRequirements(15-30)v2/types.go (2)
ResourceInfo(18-27)X402Version(15-15)
v2/mcp/client/config.go (1)
v2/mcp/server/config.go (2)
Config(21-64)DefaultConfig(67-75)
mcp/server/server.go (1)
v2/mcp/server/server.go (1)
X402Server(13-16)
v2/http/client_test.go (3)
v2/http/client.go (8)
NewClient(21-40)Client(13-15)WithSigner(55-73)WithSelector(76-94)WithPaymentCallback(97-116)WithPaymentCallbacks(120-136)WithHTTPClient(43-51)GetSettlement(155-161)v2/http/transport.go (1)
X402Transport(13-31)v2/types.go (3)
PaymentRequired(64-79)X402Version(15-15)SettleResponse(152-167)
http/middleware.go (3)
http/client.go (1)
Client(12-14)config.go (1)
DefaultTimeouts(21-25)mcp/types.go (1)
PaymentRequirements(9-13)
v2/http/facilitator.go (7)
v2/types.go (6)
PaymentPayload(82-99)PaymentRequirements(31-52)VerifyResponse(140-149)SettleResponse(152-167)X402Version(15-15)SupportedResponse(185-194)v2/config.go (1)
TimeoutConfig(9-18)v2/facilitator/interface.go (1)
Interface(16-28)http/middleware.go (1)
Config(16-56)v2/http/middleware.go (1)
Config(16-59)retry/retry.go (1)
WithRetry(32-81)v2/errors.go (3)
ErrFacilitatorUnavailable(44-44)ErrVerificationFailed(47-47)ErrSettlementFailed(50-50)
🪛 Gitleaks (8.30.0)
v2/signers/svm/signer_test.go
[high] 18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (67)
v2/internal/solana/solana.go (6)
1-12: LGTM!Package declaration and imports are well-organized. Import grouping follows the coding guidelines with stdlib, external dependencies, and internal packages properly separated by blank lines.
14-21: LGTM!Constants are properly typed and documented. Using
MustPublicKeyFromBase58for the well-known Compute Budget program ID is appropriate since the value is static and validated at compile/init time.
23-38: LGTM!Clean implementation using the SPL Token library's builder pattern. The function signature is clear and the builder chain is readable.
58-78: LGTM!Correct implementation of the SetComputeUnitPrice instruction. The same optional refactor using
binary.LittleEndian.PutUint64could apply here for consistency with the suggestion above.
80-87: LGTM!Proper error wrapping with context following the coding guidelines. Discarding the bump seed is appropriate here since it's only needed when creating the ATA, not when deriving its address.
89-99: LGTM!Error wrapping format is now correct with context preceding the wrapped error. The network constants properly reference
v2.NetworkSolanaMainnetandv2.NetworkSolanaDevnetfromv2/chains.go, ensuring consistency with the CAIP-2 identifiers defined there.http/pocketbase/middleware.go (2)
56-87: LGTM!The timeout-bound enrichment and facilitator client configuration look correct. The context timeout is appropriately scoped to the initialization phase, and the fallback to original requirements on enrichment failure provides good resilience.
90-199: LGTM!The middleware handler correctly uses request-scoped context (
e.Request.Context()) for verification and settlement operations, implements proper fallback to the secondary facilitator, and stores payment info in the request store for downstream handler access.http/facilitator.go (4)
323-361: LGTM!The
EnrichRequirementsmethod correctly propagates the context to the underlyingSupportedcall, enabling timeout-aware enrichment. The logic properly prioritizes user-specified Extra values over facilitator-provided values.
92-191: LGTM!The
Verifymethod correctly handles context propagation, applies timeout only when the parent context lacks a deadline, and properly wraps errors for unwrapping. The retry logic with exponential backoff is well-structured.
194-227: LGTM!The
Supportedmethod correctly uses the provided context for timeout control. UsingVerifyTimeoutis reasonable for this fast lookup endpoint.
230-321: LGTM!The
Settlemethod correctly usesSettleTimeout(60 seconds) which is appropriate for blockchain settlement operations. The retry logic and callback handling mirror theVerifymethod for consistency.http/gin/middleware.go (2)
50-82: LGTM!The facilitator client initialization and timeout-bound enrichment follow the same correct pattern as the PocketBase middleware. The context is properly scoped to the initialization phase.
85-200: LGTM!The middleware handler correctly uses request-scoped context, leverages shared helpers from the
internal/helperspackage, and provides dual storage in both Gin and stdlib contexts for maximum compatibility.v2/http/client_test.go (5)
99-155: LGTM!The callback tests effectively verify that the payment callbacks are properly wired through the transport by using closure flags to confirm invocation.
157-170: LGTM!The timeout is now correctly specified as
30 * time.Secondinstead of the previous incorrect value of30(nanoseconds). Good fix.
181-249: LGTM!Excellent end-to-end test that covers the complete automatic payment flow: initial 402 response, payment signing via mockSigner, retry with payment header, and settlement extraction. The test properly validates the X402Transport's handling of 402 responses.
251-284: LGTM!The settlement helper tests correctly verify both the nil case for missing headers and the parsing of valid encoded settlements. The mixed-case header name (
X-Payment-Response) works correctly since Go'shttp.Header.Getis case-insensitive.
29-49: No action required. ThemockSignertype is properly defined inv2/http/transport_test.goand is correctly accessible fromv2/http/client_test.gosince all*_test.gofiles in a package are compiled together during testing. Moving it to a separate helper file is unnecessary.mcp/server/handler.go (2)
26-42: LGTM! Proper error propagation for handler initialization.The change from panicking to returning errors is a good improvement for production robustness. The error handling flow is clean and allows callers to handle initialization failures gracefully.
65-115: LGTM! Clean error propagation in facilitator initialization.The refactored
initializeFacilitatorsproperly validates configuration and returns contextual errors. The error message at line 88 clearly indicates the missing requirement.v2/signers/svm/signer_test.go (1)
16-18: Test key is appropriately documented - Gitleaks alert is a false positive.The static analysis flagged this as a potential API key leak, but this is a randomly generated test key clearly documented as not for production use. The comment is sufficient to clarify intent.
v2/mcp/client/config.go (1)
40-48: Nil signer guard properly implemented.The fix correctly prevents nil signers from being appended, avoiding potential nil-interface panics during signing operations.
v2/signers/svm/signer.go (4)
132-156: LGTM! Case-sensitive base58 address matching implemented correctly.The direct string comparison
token.Address == requirements.Assetproperly handles Solana's case-sensitive base58 encoding, and the nil check for requirements prevents panics.
165-185: LGTM! Comprehensive amount validation.The code properly validates:
- Invalid amount format (line 167-169)
- Non-positive amounts (lines 172-174)
- Max amount limits (lines 177-179)
- uint64 overflow (lines 182-185)
This prevents silent truncation and invalid payment amounts.
232-238: LGTM! Timeout added for RPC blockhash fetch.The
context.WithTimeoutprevents indefinite hangs when fetching the latest blockhash from Solana RPC.
199-214: LGTM! Token decimals validation with explicit not-found error.The code validates decimal range (0-255) and returns
ErrInvalidTokenif the token is not found, providing clear error semantics.v2/mcp/client/transport.go (2)
58-74: LGTM! Nil data check properly handles missing 402 payload.The guard at line 62-64 returns
ErrNoPaymentRequirementswhenresp.Error.Datais nil, preventing the "unexpected end of JSON input" error from the previous review.
115-134: LGTM! Robust payment requirements extraction.The function properly validates:
- Empty data (line 117-119)
- x402 version compatibility (line 125-127)
- Presence of accepts array (line 129-131)
mcp/server/server.go (3)
68-76: LGTM on Handler() error propagation.The updated signature correctly returns errors from
NewX402Handler, enabling callers to handle configuration or initialization failures gracefully.
79-90: LGTM on Start() error handling.Error is properly checked before proceeding with verbose output and server startup. The error wrapping with
fmt.Errorf("failed to create handler: %w", err)follows the coding guidelines for contextual error wrapping.
44-66: LGTM on AddPayableTool implementation.The function properly validates requirements, sets the resource URL for each requirement, and stores payment configuration before registering the tool with the underlying MCP server.
http/middleware.go (2)
72-73: LGTM on timeout configuration for HTTP clients.Both primary and fallback facilitator clients are correctly configured with
x402.DefaultTimeouts.RequestTimeoutfor overall request timeouts and theTimeoutsfield for operation-specific timeouts.Also applies to: 87-88
99-101: LGTM on context-aware enrichment.The timeout-bounded context for
EnrichRequirementsensures the enrichment operation doesn't block indefinitely during middleware initialization.v2/mcp/server/handler_test.go (3)
14-50: LGTM on mock implementations.The
mockFacilitatorproperly tracks method invocations and allows configurable responses/errors. ThemockMCPHandlerprovides a simple way to simulate MCP server responses with configurable status codes.
634-686: Good use of table-driven tests.The
TestValidateRequirementfunction follows Go testing best practices with clear test case names and well-structured table entries. This aligns with the coding guidelines for test files.
521-593: Verify settlement is not called on tool execution errors.Good test coverage for ensuring settlement doesn't occur when the tool returns an error. This is critical for preventing charges when the requested operation fails.
v2/config.go (3)
45-61: LGTM on Validate() implementation.The validation now includes
RequestTimeoutcheck (addressing the past review comment). All three timeouts are validated for positive values, and the relative constraint betweenSettleTimeoutandVerifyTimeoutis enforced.
20-25: LGTM on default timeout values.The defaults are reasonable: 5s for verification (quick check), 60s for settlement (blockchain finality), and 120s for overall request timeout (allows for retries and network delays).
27-43: LGTM on fluent configuration methods.The
With*methods follow the builder pattern idiomatically by returning a copy of the struct with the updated field, enabling chained configuration.v2/http/facilitator_test.go (5)
16-77: LGTM on basic Verify test.Comprehensive test covering path, method, content-type validation, and response parsing. Properly verifies both
IsValidandPayerfields.
191-227: Good test for AuthorizationProvider precedence.This test verifies that
AuthorizationProvidertakes precedence over staticAuthorizationwhen both are set, and uses atomic operations to safely count invocations.
351-395: Clever retry simulation using connection hijacking.The test effectively simulates transient failures by hijacking and closing connections on early attempts. The fallback to 503 status ensures the test works even if hijacking isn't supported.
733-786: Good test for enrichment merge behavior.This test verifies that user-specified values in
Extraare preserved while facilitator-provided values are merged in. This is important for allowing user overrides of facilitator defaults.
811-836: LGTM on timeout test.Verifies that
VerifyTimeoutis honored and that timeout errors are properly wrapped asErrFacilitatorUnavailable.v2/http/gin/middleware.go (6)
6-15: LGTM! Imports properly grouped.Imports follow the coding guidelines with stdlib first, external dependencies second, and internal packages last with blank lines between groups.
60-88: HTTP client timeout issue has been addressed.The
http.Clientinstances now includeTimeout: v2.DefaultTimeouts.RequestTimeout, resolving the previous concern about requests potentially hanging indefinitely.
90-100: LGTM! Enrichment context handling is correct.The context timeout and cancellation pattern is properly implemented. The synchronous
EnrichRequirementscall completes beforedefer cancel()executes, and the fallback to original requirements on error provides resilience.
145-157: Pointer dereference afterFindMatchingRequirementis safe.The
requirementreturned byFindMatchingRequirement(line 136) is checked for error before use at line 145. The dereference*requirementis safe since error handling at lines 137-141 returns early if no match is found.
200-209: LGTM! Dual context storage provides good compatibility.Storing the payment response in both Gin context and stdlib context enables consumers to use either pattern for accessing verified payment information.
225-237: LGTM! Safe context extraction pattern.
GetPaymentFromContextproperly handles both the case where the key doesn't exist and the case where the type assertion fails, returningnilin both scenarios.v2/mcp/server/server.go (3)
3-10: LGTM! Imports properly grouped.Standard library, external MCP dependencies, and internal v2 package are correctly separated with blank lines.
18-36: LGTM! Defensive constructor with proper nil handling.The constructor properly handles nil config with a default and initializes the
PaymentToolsmap to prevent nil map assignment panics in subsequentAddPayableToolcalls.
44-72: LGTM! Thorough validation with contextual errors.
AddPayableToolproperly validates requirements before registration, with error messages that include both the requirement index and tool name for easier debugging. Error wrapping uses%was per coding guidelines.v2/mcp/server/handler.go (6)
3-13: LGTM! Imports properly grouped.Standard library imports are grouped together, followed by the internal v2 package import, following the coding guidelines.
23-40: Past review concern addressed: constructor now returns error instead of panicking.The
NewX402Handlersignature correctly returns(*X402Handler, error), allowing callers to handle configuration failures gracefully instead of crashing.
75-108: LGTM! Facilitator initialization returns errors instead of panicking.The function correctly returns a descriptive error when the primary facilitator URL is missing, allowing callers to handle the failure appropriately.
279-285: Past review concern addressed: X402Version is now validated.The
extractPaymentfunction now validatespayment.X402Versionagainstv2.X402Versionbefore returning, preventing operations on incompatible payloads.
408-421: Past review concern addressed: verify-only mode now setsSuccess: true.When verification succeeds in verify-only mode, the response now correctly sets
Success: truewith an emptyTransactionfield, properly indicating that verification passed but settlement was not attempted.
448-466: LGTM! JSON-RPC error handling follows specification.The
writeErrorfunction correctly returns HTTP 200 with the error in the JSON-RPC response body, as per JSON-RPC 2.0 specification.v2/http/facilitator.go (7)
4-17: LGTM! Imports properly grouped.Standard library, retry package, and v2 packages are correctly separated following the coding guidelines.
40-81: LGTM! Well-documented struct with comprehensive configuration options.The
FacilitatorClientstruct provides clear documentation for each field, including thread-safety considerations forAuthorizationProvider. The lifecycle hooks enable observability and custom logic integration.
109-127: LGTM! Retry configuration correctly distinguishes retries from attempts.The
MaxAttempts = maxRetries + 1calculation correctly interpretsMaxRetriesas the number of retry attempts after the initial request, with sensible defaults for delay and exponential backoff.
150-189: LGTM! Robust Verify implementation with proper retry and timeout handling.The implementation correctly:
- Respects caller-provided context deadlines while applying defaults when none exists
- Uses retry with appropriate
isFacilitatorUnavailableErrorpredicate- Wraps errors with context as per coding guidelines
- Extracts payer from payload as fallback when not in response
298-336: Past review concern addressed:EnrichRequirementsnow accepts context.The method signature correctly accepts
ctx context.Contextas the first parameter and propagates it to theSupportedcall, enabling proper cancellation and timeout control.
376-380: LGTM! Proper use oferrors.Isfor wrapped error detection.Using
errors.Iscorrectly handles the case whereErrFacilitatorUnavailableis wrapped with additional context viafmt.Errorf("%w", ...).
361-374: SVM payer extraction not implemented—verify if needed for SVM payment handling.The
extractPayerfunction handles only EVM payloads by extracting the payer fromauthorization.from. For SVM (Solana) payloads, it returns an empty string. While the facilitator does handle Solana payments, the payer information is embedded within the base64-encoded transaction in the payload, which would require decoding to extract. Determine whether payer extraction from SVM transactions is necessary in the current payment flow, or if the empty string return is acceptable for non-critical use (e.g., logging).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| let prompt = $" | ||
| STUDY scratch/comments.md | ||
| STUDY scratch/NOTES.md | ||
|
|
||
| Pick the one most important issue to fix and then fix it. | ||
|
|
||
| If there are no issues left to fix, simply kill this PID: ($nu.pid) | ||
|
|
||
| Important: | ||
| - make sure all tests pass when done | ||
| - git commit after you are done | ||
| - save any notes or note any problems in scratch/NOTES.md | ||
| " | ||
|
|
||
| for i in 1..10 { | ||
| print $"Iteration #($i)..." | ||
| opencode run $prompt -m anthropic/claude-sonnet-4-5 | ||
| print "Done!" | ||
| } |
There was a problem hiding this comment.
This file appears to be a development automation script and should not be committed.
This Nu shell script appears to be a local development tool for iteratively running AI-assisted fixes. It references scratch/comments.md and scratch/NOTES.md which are likely temporary working files. Consider adding this to .gitignore or removing it from the PR, as it doesn't appear to be part of the x402 v2 protocol implementation.
🤖 Prompt for AI Agents
In `@ralph.nu` around lines 1 - 19, The file ralph.nu is a local development
automation script (defines prompt and a for loop using variables like prompt and
the loop "for i in 1..10") and should not be committed; remove ralph.nu from the
PR or add its path to .gitignore and delete it from the commit (or move it to a
non-tracked location), then amend the commit so the script is no longer in the
repo; if you prefer to keep it but not track changes, add its filename to
.gitignore and run git rm --cached on ralph.nu before committing.
| // Instruction 2: Create associated token account (idempotent - won't fail if it exists) | ||
| // The feePayer sponsors the rent-exempt balance for the destination ATA | ||
| associatedtokenaccount.NewCreateInstruction( | ||
| feePayer, // payer (facilitator sponsors the rent) | ||
| recipient, // wallet address (owner of the ATA) | ||
| mint, // SPL token mint address | ||
| ).Build(), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
solana-go CreateIdempotent associated token account instruction
💡 Result:
Short answer: use the Associated Token Account program's CreateIdempotent instruction variant (the ATA instruction variant whose instruction ID is CreateIdempotent). In solana-go there are packages that expose that variant (the program has a CreateIdempotent type / NewCreateIdempotentInstruction helper). Example (pseudo-Go; adapt to the solana-go package you use):
-
derive ATA:
ata, _, _ := common.FindAssociatedTokenAddress(ownerPubKey, mintPubKey) -
build idempotent ATA instruction (example names vary by fork; adapt to your import):
inst := associatedtokenaccount.NewCreateIdempotentInstruction(
payerPubKey, // funding account (payer)
ata, // associated token account (PDA)
ownerPubKey, // owner of ATA
mintPubKey, // token mint
systemProgramID, // system program id (usually solana.SystemProgramID)
tokenProgramID, // token program id (usually spl-token program id)
) -
add inst to transaction and send.
References:
- associated-token-account Go package (shows Instruction_CreateIdempotent / CreateIdempotent type). [1]
- gaggliardetto/solana-go issue discussing adding idempotent create instruction. [2]
- SPL Token docs showing the createAssociatedTokenAccountIdempotent instruction behavior (JS reference). [3]
If you want, tell me which solana-go repo/version you are using (gagliardetto, xetrics, BBleae, etc.) and I’ll give an exact compile-ready snippet for that package.
🏁 Script executed:
# First, let's examine the actual code at the specified location
find . -name "signer.go" -path "*/v2/signers/svm/*" | head -5Repository: mark3labs/x402-go
Length of output: 87
🏁 Script executed:
# Let's examine the actual code at the specified lines
sed -n '330,350p' ./v2/signers/svm/signer.goRepository: mark3labs/x402-go
Length of output: 1028
Replace NewCreateInstruction with NewCreateIdempotentInstruction for ATA creation.
The code currently uses associatedtokenaccount.NewCreateInstruction(), which is not idempotent and will fail if the ATA already exists. Use NewCreateIdempotentInstruction() instead, which is the recommended approach per Solana best practices and ensures the transaction succeeds whether the ATA exists or not. The inline comment claiming idempotency is misleading—the actual code must match the intended behavior.
🤖 Prompt for AI Agents
In `@v2/signers/svm/signer.go` around lines 336 - 342, The ATA creation call
currently uses associatedtokenaccount.NewCreateInstruction(feePayer, recipient,
mint).Build() but that is not idempotent; replace it with
associatedtokenaccount.NewCreateIdempotentInstruction(feePayer, recipient,
mint).Build() so the transaction succeeds if the ATA already exists, and update
the inline comment to reflect true idempotency; ensure any references to
NewCreateInstruction in the same file (e.g., the instruction assembly around
signer.go's ATA creation) are updated accordingly.
Summary
Features
Summary by CodeRabbit
New Features
Bug Fixes / API
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.