Repository navigation
Feature 16 cryptographic payment receipts - #38
vishal0316 wants to merge 6 commits into
Conversation
📝 WalkthroughWalkthroughAdds a cryptographic receipt system: deterministic receipt creation and ECDSA signing, in-memory TTL-backed storage with cleanup and GET lookup, integrates receipts into the summarize flow and response headers, removes per-request timeout middleware/config, updates env vars and dependencies, and adds receipt unit tests. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Gateway as Gateway/API
participant Verifier as Verifier
participant AI as OpenRouter
participant Store as ReceiptStore
participant Signer as ServerSigner
Client->>Gateway: POST /api/ai/summarize + payment proof
Gateway->>Verifier: Verify payment
Verifier-->>Gateway: valid
Gateway->>AI: Request AI result
AI-->>Gateway: AI response
Note over Gateway,Signer: Build receipt (ID, hashes, payment, service)
Gateway->>Signer: signReceipt(receipt)
Signer-->>Gateway: SignedReceipt (signature + pubkey)
Gateway->>Store: Store(signedReceipt, ttl)
Store-->>Gateway: ack
Gateway-->>Client: 200 + X-402-Receipt header + receipt body
alt Receipt lookup
Client->>Gateway: GET /api/receipts/:id
Gateway->>Store: Get(id)
Store-->>Gateway: SignedReceipt / not found
Gateway-->>Client: 200 / 404
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (4)
-
gateway/main.go, line 408-416 (link)logic: goroutine leak: spawns unbounded goroutines that can't be stopped
Each call to
storeReceiptspawns a goroutine that sleeps for the TTL duration. Under high traffic, this creates thousands of goroutines that can't be canceled, consuming memory and goroutine scheduler resources.Consider implementing a single background cleanup goroutine with a timer heap (like
time.Timer) or checking expiry on access rather than spawning a goroutine per receipt. -
gateway/main.go, line 234-235 (link)logic: silently ignoring errors in production-critical path
The request body marshaling error is silently ignored. While unlikely to fail, if it does,
reqBodywill be nil and produce incorrect hashes in the receipt. -
gateway/main.go, line 253 (link)logic: silently ignoring marshaling error
If
json.Marshal(receipt)fails, empty data will be base64-encoded and sent to the client, providing invalid receipt data. -
gateway/receipt.go, line 102-114 (link)style: misleading comments about JSON field ordering
The comments claim "fields are always serialized in declaration order", but Go's
json.Marshalactually serializes struct fields in alphabetical order by JSON tag name, not declaration order.While this still ensures deterministic output (which is what matters for signatures), the explanation is technically incorrect. The struct happens to work because the fields will serialize alphabetically as:
id,payment,service,timestamp,version.Update the comments to accurately reflect that alphabetical ordering by JSON tag ensures determinism.
7 files reviewed, 4 comments
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
gateway/receipt_test.go (3)
152-161: Handle thejson.Unmarshalerror.The unmarshaling error is silently ignored. While unlikely to fail in a test context, checking the error is good practice and prevents silent test failures.
Proposed fix
// Verify all fields are present var decoded map[string]interface{} - json.Unmarshal(json1, &decoded) + if err := json.Unmarshal(json1, &decoded); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + }
164-204: Consider test isolation for receipt store tests.This test stores a receipt in the global
receiptStorewithout cleanup. While not immediately problematic, it could cause test pollution if tests are run in parallel or if subsequent tests depend on store state. Consider adding cleanup after the test.Proposed fix
func TestStoreAndRetrieveReceipt(t *testing.T) { + // Cleanup after test + defer func() { + receiptMutex.Lock() + delete(receiptStore, signedReceipt.Receipt.ID) + receiptMutex.Unlock() + }() + signedReceipt := &SignedReceipt{Alternatively, consider extracting an interface for the receipt store to allow test doubles.
263-269: Handle potential errors from hex decoding.The signature hex decoding error is silently ignored with
_. If the signature format is malformed, this will cause a confusing panic later. Add error checking for robustness.Proposed fix
// Remove "0x" prefix from signature sigHex := signedReceipt.Signature[2:] - sigBytes, _ := hex.DecodeString(sigHex) + sigBytes, err := hex.DecodeString(sigHex) + if err != nil { + t.Fatalf("Failed to decode signature hex: %v", err) + }gateway/main.go (3)
233-243: Handle JSON marshaling error for request body.The error from
json.Marshal(req)is silently ignored. While unlikely to fail for a simple struct, this could cause incorrect request hashes if marshaling fails.Proposed fix
// 4. Generate Receipt - reqBody, _ := json.Marshal(req) + reqBody, err := json.Marshal(req) + if err != nil { + log.Printf("Failed to marshal request body for receipt: %v", err) + c.JSON(200, gin.H{"result": summary}) + return + } respBody := []byte(summary)
252-254: Handle JSON marshaling error for receipt header.The error from
json.Marshal(receipt)is ignored. If marshaling fails, the X-402-Receipt header will contain an invalid base64 string.Proposed fix
// Add receipt to headers (base64 encoded) - receiptJSON, _ := json.Marshal(receipt) - c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) + receiptJSON, err := json.Marshal(receipt) + if err != nil { + log.Printf("Failed to marshal receipt for header: %v", err) + } else { + c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) + }
441-460: Consider adding rate limiting to the receipt lookup endpoint.The PR objectives mention rate limiting for receipt lookups as a desired feature. Without it, this endpoint could be abused for enumeration or DoS attacks.
Would you like me to help design rate limiting specifically for the receipt lookup endpoint, or should this be tracked as a follow-up issue?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gateway/go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
.env.examplegateway/go.modgateway/main.gogateway/main_test.gogateway/receipt.gogateway/receipt_test.go
💤 Files with no reviewable changes (1)
- gateway/main_test.go
🧰 Additional context used
🧬 Code graph analysis (3)
gateway/receipt_test.go (1)
gateway/receipt.go (4)
Receipt(24-30)PaymentDetails(33-40)ServiceDetails(43-47)SignedReceipt(50-54)
gateway/main.go (1)
gateway/receipt.go (3)
SignedReceipt(50-54)GenerateReceipt(57-78)Receipt(24-30)
gateway/receipt.go (1)
gateway/main.go (1)
PaymentContext(26-32)
⏰ 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: Greptile Review
🔇 Additional comments (10)
.env.example (2)
10-10: LGTM!The placeholder clearly indicates the expected format (hex without 0x prefix), which aligns with how
crypto.HexToECDSAexpects the input ingateway/main.go.
23-31: LGTM!The Receipt System section is well-documented with a sensible default TTL and placeholders for future storage backends. The commented-out options provide good guidance for future implementations.
gateway/receipt_test.go (1)
14-37: LGTM!Good test for receipt ID format and uniqueness. Testing 100 IDs provides reasonable confidence in the randomness.
gateway/main.go (1)
49-56: LGTM!Thread-safe singleton pattern for the private key using
sync.Onceis appropriate. The global receipt store with mutex provides safe concurrent access.gateway/receipt.go (5)
15-21: LGTM - Interface designed for extensibility.The
ReceiptStoreinterface is well-designed for future storage backends (Redis, PostgreSQL). It's currently unused as the in-memory implementation uses package-level functions inmain.go, which is acceptable for the initial implementation.
23-54: LGTM!The data structures are well-defined with appropriate JSON tags. The struct field ordering is stable, ensuring deterministic JSON serialization for consistent signatures.
102-138: LGTM - Well-implemented signing flow.The signing implementation correctly:
- Uses Go's deterministic struct JSON marshaling (fields in declaration order).
- Applies Ethereum-compatible Keccak256 hashing.
- Returns the signature and public key in the expected hex format with
0xprefix.The comment about deterministic marshaling (lines 103-105) is helpful for future maintainers.
3-13: Remove unused importcrypto/ecdsa.The
crypto/ecdsapackage is imported but only used for the type assertion on line 130 (*ecdsa.PublicKey). The type is returned byprivateKey.Public(), which is already an ECDSA key fromcrypto.HexToECDSA. Consider whether this import is necessary or if it can be removed.Actually, the import is needed for the type assertion on line 130. Disregard this comment.
80-91: Timestamp fallback may produce colliding IDs.The fallback ID using
%012xformat withUnixNano()will truncate the timestamp since nanosecond timestamps exceed 12 hex characters (e.g.,time.Now().UnixNano()≈ 17+ hex digits). This could lead to ID collisions if the fallback is triggered multiple times within the same truncated time window.Proposed fix - use more hex digits or a different approach
if _, err := rand.Read(bytes); err != nil { // Fallback to timestamp-based ID if random fails - // This maintains uniqueness even if entropy is exhausted timestamp := time.Now().UnixNano() - return fmt.Sprintf("rcpt_%012x", timestamp) + // Use full timestamp hex representation for uniqueness + return fmt.Sprintf("rcpt_%x", timestamp)[:17] // Maintain 17 char total length }Alternatively, consider using a UUID-based fallback or increasing the hex length.
Likely an incorrect or invalid review comment.
gateway/go.mod (1)
6-6: Version v1.14.13 is appropriate—it includes the CVE-2025-24883 security patch.The go-ethereum package is suitable for Ethereum-compatible ECDSA signing. Version v1.14.13 patched CVE-2025-24883 (DoS via malicious p2p message), which affected versions 1.14.0–1.14.12. This version is secure and includes the necessary curve-point validity checks.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @gateway/main.go:
- Around line 269-271: Handle the error returned by json.Marshal(receipt) before
encoding and setting the header: check the error from json.Marshal(receipt), log
or return an appropriate error response (or skip adding the header) if
marshaling fails, and only call c.Header("X-402-Receipt",
base64.StdEncoding.EncodeToString(receiptJSON)) when err == nil so you never
base64-encode a nil/invalid receiptJSON.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gateway/main.go
🧰 Additional context used
🧬 Code graph analysis (1)
gateway/main.go (1)
gateway/receipt.go (3)
SignedReceipt(50-54)GenerateReceipt(57-78)Receipt(24-30)
🔇 Additional comments (6)
gateway/main.go (6)
49-62: LGTM!The receipt entry structure and global state management look correct. The RWMutex provides appropriate concurrency control, and the sync.Once pattern ensures thread-safe singleton initialization for the private key.
64-73: Good improvement over the previous approach.The single cleanup goroutine with a periodic ticker is much more scalable than spawning a goroutine per receipt. The 5-minute interval is a reasonable balance between memory cleanup and CPU overhead.
156-173: LGTM!CORS configuration correctly exposes the
X-402-Receiptheader, and the new receipt endpoint is properly registered.
396-416: LGTM with note on error caching behavior.The sync.Once singleton pattern is correctly implemented. Note that if the private key is missing or invalid, the error is cached permanently—subsequent calls will return the same error until the service restarts. This is appropriate for configuration that should be set at startup.
443-459: LGTM!The read path correctly uses
RLockfor better concurrency. Checking expiration at read time provides accurate results even before the next cleanup cycle. The expired entry will be cleaned up by the periodic goroutine, so not deleting here is fine and avoids upgrading to a write lock on the read path.
474-493: LGTM!The receipt retrieval endpoint is correctly implemented with appropriate 404 handling and informative error messages.
| // Add receipt to headers (base64 encoded) | ||
| receiptJSON, _ := json.Marshal(receipt) | ||
| c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) |
There was a problem hiding this comment.
Handle json.Marshal error to avoid malformed header.
If json.Marshal(receipt) fails, receiptJSON will be nil, and the base64-encoded header will be empty or malformed. While unlikely for a well-formed struct, handling this error maintains robustness.
🔎 Proposed fix
- // Add receipt to headers (base64 encoded)
- receiptJSON, _ := json.Marshal(receipt)
- c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON))
+ // Add receipt to headers (base64 encoded)
+ receiptJSON, err := json.Marshal(receipt)
+ if err != nil {
+ log.Printf("Failed to marshal receipt for header: %v", err)
+ } else {
+ c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON))
+ }📝 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.
| // Add receipt to headers (base64 encoded) | |
| receiptJSON, _ := json.Marshal(receipt) | |
| c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) | |
| // Add receipt to headers (base64 encoded) | |
| receiptJSON, err := json.Marshal(receipt) | |
| if err != nil { | |
| log.Printf("Failed to marshal receipt for header: %v", err) | |
| } else { | |
| c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) | |
| } |
🤖 Prompt for AI Agents
In @gateway/main.go around lines 269-271, Handle the error returned by
json.Marshal(receipt) before encoding and setting the header: check the error
from json.Marshal(receipt), log or return an appropriate error response (or skip
adding the header) if marshaling fails, and only call c.Header("X-402-Receipt",
base64.StdEncoding.EncodeToString(receiptJSON)) when err == nil so you never
base64-encode a nil/invalid receiptJSON.
|
@vishal0316 update to latest main branch and the conflicts |
…into feature-16-cryptographic-payment-receipts
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @gateway/main.go:
- Around line 309-315: The call to GenerateReceipt is using the context package
identifier instead of the local paymentCtx variable, causing a compile error;
replace the first argument in the GenerateReceipt(...) invocation with the
paymentCtx variable (so call GenerateReceipt(paymentCtx,
verifyResp.RecoveredAddress, c.Request.URL.Path, reqBody, respBody)) to pass the
correct request context.
♻️ Duplicate comments (1)
gateway/main.go (1)
324-326: Handlejson.Marshalerror to avoid malformed header.The error from
json.Marshal(receipt)is discarded. If marshaling fails,receiptJSONwill be nil, and the base64-encoded header will be empty or malformed.🔎 Proposed fix
// Add receipt to headers (base64 encoded) - receiptJSON, _ := json.Marshal(receipt) - c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) + receiptJSON, err := json.Marshal(receipt) + if err != nil { + log.Printf("Failed to marshal receipt for header: %v", err) + } else { + c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) + }
🧹 Nitpick comments (1)
gateway/main.go (1)
463-481: Consider stripping0xprefix from private key hex string.
crypto.HexToECDSAexpects a hex string without the0xprefix. Users commonly include the prefix in.envfiles (e.g., from MetaMask exports). Consider stripping it for robustness.🔎 Proposed fix
func getServerPrivateKey() (*ecdsa.PrivateKey, error) { serverPrivateKeyOnce.Do(func() { privateKeyHex := os.Getenv("SERVER_WALLET_PRIVATE_KEY") if privateKeyHex == "" { serverPrivateKeyErr = fmt.Errorf("SERVER_WALLET_PRIVATE_KEY not set in environment") return } + // Strip 0x prefix if present + privateKeyHex = strings.TrimPrefix(privateKeyHex, "0x") + privateKey, err := crypto.HexToECDSA(privateKeyHex)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.examplegateway/main.go
🚧 Files skipped from review as they are similar to previous changes (1)
- .env.example
🧰 Additional context used
🧬 Code graph analysis (1)
gateway/main.go (1)
gateway/receipt.go (3)
SignedReceipt(50-54)GenerateReceipt(57-78)Receipt(24-30)
🪛 GitHub Actions: Go Lint
gateway/main.go
[error] 310-310: vet: ./main.go:310:3: use of package context not in selector
🪛 GitHub Actions: Go Tests
gateway/main.go
[error] 310-310: go test -v ./... failed with compile error: use of package context not in selector.
🔇 Additional comments (5)
gateway/main.go (5)
54-67: LGTM!Good design with
receiptEntrywrapping receipts with expiration time, and proper thread-safe patterns usingsync.RWMutexfor storage andsync.Oncefor singleton key loading.
69-78: LGTM!Single cleanup goroutine with periodic sweep is the correct approach, addressing the earlier scalability concern. Properly stops the ticker on cleanup.
483-524: LGTM!Receipt storage and retrieval functions are correctly implemented with proper mutex usage. The double-check for expiration in
getReceiptis a good defensive measure between cleanup cycles.
539-558: LGTM!The receipt lookup endpoint correctly returns 404 for missing/expired receipts and exposes the receipt data publicly as intended by the design (using non-enumerable random IDs for security).
170-170: LGTM!Adding
X-402-Receiptto exposed headers is required for clients to access the receipt in cross-origin requests.
| receipt, err := GenerateReceipt( | ||
| context, | ||
| verifyResp.RecoveredAddress, | ||
| c.Request.URL.Path, | ||
| reqBody, | ||
| respBody, | ||
| ) |
There was a problem hiding this comment.
Critical: Using package name context instead of variable paymentCtx.
This causes a compile error as indicated by the pipeline failure. The first argument to GenerateReceipt should be the paymentCtx variable defined at line 228, not the context package.
🔎 Proposed fix
receipt, err := GenerateReceipt(
- context,
+ paymentCtx,
verifyResp.RecoveredAddress,
c.Request.URL.Path,
reqBody,
respBody,
)🧰 Tools
🪛 GitHub Actions: Go Lint
[error] 310-310: vet: ./main.go:310:3: use of package context not in selector
🪛 GitHub Actions: Go Tests
[error] 310-310: go test -v ./... failed with compile error: use of package context not in selector.
🤖 Prompt for AI Agents
In @gateway/main.go around lines 309-315, The call to GenerateReceipt is using
the context package identifier instead of the local paymentCtx variable, causing
a compile error; replace the first argument in the GenerateReceipt(...)
invocation with the paymentCtx variable (so call GenerateReceipt(paymentCtx,
verifyResp.RecoveredAddress, c.Request.URL.Path, reqBody, respBody)) to pass the
correct request context.
…ithub.com/vishal0316/MicroAI-Paygate into feature-16-cryptographic-payment-receipts
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @gateway/main.go:
- Around line 125-127: The if block using os.Getenv("CHAIN_ID") in main.go has
inconsistent leading spaces instead of tabs; replace the leading spaces before
that line and its inner fmt.Println line with tabs to match Go formatting (or
simply run gofmt/gofmt -w) so the indentation is consistent with the rest of the
file and passes go fmt checks.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gateway/main.go (2)
214-224: Add timeout to verification HTTP call.The HTTP POST to the verifier service lacks a timeout, which could cause the request to hang indefinitely if the verifier is unresponsive. This can lead to goroutine leaks and degraded gateway performance under load.
🔎 Proposed fix
verifyBody, _ := json.Marshal(verifyReq) verifierURL := os.Getenv("VERIFIER_URL") if verifierURL == "" { verifierURL = "http://127.0.0.1:3002" } - resp, err := http.Post(verifierURL+"/verify", "application/json", bytes.NewBuffer(verifyBody)) + + client := &http.Client{ + Timeout: 10 * time.Second, + } + resp, err := client.Post(verifierURL+"/verify", "application/json", bytes.NewBuffer(verifyBody)) if err != nil { c.JSON(500, gin.H{"error": "Verification service unavailable"}) return } defer resp.Body.Close()
335-388: Add timeout to OpenRouter HTTP client.The HTTP client used for calling the OpenRouter API lacks a timeout configuration. This could cause requests to hang indefinitely if the API is slow or unresponsive, leading to goroutine leaks and resource exhaustion under load.
🔎 Proposed fix
req, _ := http.NewRequest("POST", "https://openrouter.ai/api/v1/chat/completions", bytes.NewBuffer(reqBody)) req.Header.Set("Authorization", "Bearer "+apiKey) req.Header.Set("Content-Type", "application/json") - client := &http.Client{} + client := &http.Client{ + Timeout: 30 * time.Second, // Adjust based on expected AI response time + } resp, err := client.Do(req)
♻️ Duplicate comments (1)
gateway/main.go (1)
269-271: Handle json.Marshal error for receipt header.This issue was flagged in a previous review but remains unaddressed. If
json.Marshal(receipt)fails, the header will contain an invalid base64-encoded value, which could cause client-side parsing errors.🔎 Proposed fix
// Add receipt to headers (base64 encoded) - receiptJSON, _ := json.Marshal(receipt) - c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) + receiptJSON, err := json.Marshal(receipt) + if err != nil { + log.Printf("Failed to marshal receipt for header: %v", err) + } else { + c.Header("X-402-Receipt", base64.StdEncoding.EncodeToString(receiptJSON)) + }
🧹 Nitpick comments (4)
gateway/main.go (4)
201-207: Consider renaming variable to avoid confusion.The variable name
contextmay cause confusion as it conceptually shadows thecontextpackage name (even though the package isn't imported in this file). Consider renaming topaymentCtxorpaymentContextfor clarity.🔎 Proposed refactor
- context := PaymentContext{ + paymentCtx := PaymentContext{ Recipient: getRecipientAddress(), Token: "USDC", Amount: getPaymentAmount(), Nonce: nonce, ChainID: getChainID(), } verifyReq := VerifyRequest{ - Context: context, + Context: paymentCtx, Signature: signature, }And update line 255:
receipt, err := GenerateReceipt( - context, + paymentCtx, verifyResp.RecoveredAddress, c.Request.URL.Path, reqBody, respBody, )
214-214: Handle json.Marshal error.Ignoring the error from
json.Marshalcould result in sending an empty or malformed request body to the verifier. While unlikely with a well-formed struct, handling this error maintains robustness.🔎 Proposed fix
- verifyBody, _ := json.Marshal(verifyReq) + verifyBody, err := json.Marshal(verifyReq) + if err != nil { + c.JSON(500, gin.H{"error": "Failed to encode verification request"}) + return + } + verifierURL := os.Getenv("VERIFIER_URL")
251-252: Handle json.Marshal error.Ignoring the error from
json.Marshal(req)could result in an incorrect request hash in the receipt. Consider handling this error to maintain data integrity.🔎 Proposed fix
- reqBody, _ := json.Marshal(req) + reqBody, err := json.Marshal(req) + if err != nil { + log.Printf("Failed to marshal request for receipt: %v", err) + c.JSON(200, gin.H{"result": summary}) + return + } respBody := []byte(summary)
554-568: Minor formatting: extra blank line.Line 558 contains an unnecessary blank line in the middle of the
getRateLimitKeyfunction logic. While not functionally incorrect, removing it would improve consistency.🔎 Proposed cleanup
func getRateLimitKey(c *gin.Context) string { signature := c.GetHeader("X-402-Signature") nonce := c.GetHeader("X-402-Nonce") - + // Only use nonce-based key if BOTH signature and nonce are present
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.env.example.github/workflows/pr-branch-check.yml.gitignoreREADME.mdgateway/README.mdgateway/config.gogateway/config_test.gogateway/main.gogateway/main_test.gogateway/middleware.gogateway/ratelimit.gogateway/ratelimit_test.gogateway/timeout_extra_test.gogateway/timeout_test.gopackage.jsonrun_e2e.shtests/README.md
💤 Files with no reviewable changes (5)
- gateway/timeout_extra_test.go
- gateway/timeout_test.go
- gateway/middleware.go
- .gitignore
- gateway/config.go
✅ Files skipped from review due to trivial changes (3)
- gateway/ratelimit.go
- gateway/ratelimit_test.go
- gateway/main_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
gateway/main.go (2)
gateway/receipt.go (3)
SignedReceipt(50-54)GenerateReceipt(57-78)Receipt(24-30)src/services/payment.ts (1)
PaymentContext(8-14)
⏰ 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: rust-tests
🔇 Additional comments (23)
.github/workflows/pr-branch-check.yml (1)
50-50: LGTM: Correct GitHub Actions syntax.The change from
${{ failure() }}tofailure()is correct. In GitHub Actionsifconditions, context expressions likefailure()don't require the${{ }}wrapper, and the simpler syntax is preferred.tests/README.md (1)
42-43: LGTM: Simplified test commands.The updated documentation using direct
go test -vandcargo testcommands is clearer and aligns with standard Go and Rust testing practices. This change is consistent with the removal of aggregated test scripts from package.json.Also applies to: 50-51
gateway/README.md (1)
37-41: LGTM: Documentation reflects new receipt system.The updated environment variable documentation correctly captures the new receipt system requirements (CHAIN_ID, RECIPIENT_ADDRESS) and aligns with the test command simplification. The changes are clear and well-documented.
Also applies to: 48-48
.env.example (3)
23-30: LGTM: Receipt system configuration is well-documented.The receipt system environment variables are clearly documented with sensible defaults:
RECEIPT_TTL=86400(24 hours) aligns with the PR objectives- Future storage backend options are appropriately commented as placeholders
This provides a clear path for future Redis/Postgres integration while starting with an in-memory implementation.
17-17: USDC_TOKEN_ADDRESS is already used and documented. The environment variable is referenced insrc/config.tsand documented inREADME.mdas "USDC contract address (default: Base USDC)". No action required.
10-10: No action needed. The private key loading code ingateway/main.gocorrectly handles the documented format. The code retrievesSERVER_WALLET_PRIVATE_KEYfrom the environment and passes it directly tocrypto.HexToECDSA(), which expects an unprefixed hex string. This is consistent with the.env.examplespecification and the go-ethereum library's interface.package.json (1)
12-12: No CI/CD breakage risk from removed test scripts.Verified that the removed
test:unit,test:go,test:rust, andtest:allscripts are not referenced in any GitHub Actions workflows or automation scripts. The workflows run tests directly using language-specific tools (go test,cargo test) rather than through npm scripts, so the removal is safe.README.md (2)
193-195: LGTM!The explicit per-component test commands improve clarity and make it easier for contributors to test individual services.
201-210: LGTM!The environment variable documentation is clear and accurate. The addition of
VERIFIER_URLas an optional configuration aligns well with the gateway's flexibility.run_e2e.sh (1)
30-30: LGTM!The explicit
go run main.gomakes the entry point clearer and is a good practice for maintainability.gateway/config_test.go (1)
3-21: LGTM!The simplified test file correctly retains the essential configuration validation tests while removing the deprecated timeout-related tests. This aligns with the broader refactoring away from per-request timeout middleware.
gateway/main.go (12)
3-24: LGTM!The new imports are appropriate for the cryptographic receipt functionality and thread-safe storage implementation.
49-73: Excellent improvement!The refactored receipt storage using a single periodic cleanup goroutine (every 5 minutes) instead of per-receipt goroutines is a much more scalable solution. The thread-safe design with
sync.RWMutexandsync.Oncefor the server key is well implemented.
156-162: LGTM!The CORS configuration correctly includes the new
X-402-Receiptheader in the exposed headers list, allowing clients to access the receipt data.
171-173: LGTM!The route registrations are clean and properly organized. The previous duplicate route issue has been resolved.
254-260: LGTM!The receipt generation logic correctly uses the payment context variable and properly handles errors. The integration with
GenerateReceiptfromgateway/receipt.gois clean and well-structured.
398-416: LGTM!The singleton pattern using
sync.Onceensures thread-safe, one-time initialization of the server's private key. The error handling is appropriate, and the implementation prevents race conditions.
418-428: LGTM!The
storeReceiptfunction correctly stores receipts with expiration times and uses proper mutex locking for thread safety.
430-441: LGTM!The periodic cleanup function efficiently removes expired receipts and uses proper locking to prevent race conditions.
443-459: LGTM!The
getReceiptfunction correctly checks both existence and expiration, using read locks for optimal concurrency. The double-check on expiration at retrieval time prevents race conditions with the cleanup goroutine.
461-472: LGTM!The TTL configuration function properly reads from the environment with a sensible 24-hour default and handles parsing errors gracefully.
474-493: LGTM!The HTTP handler provides clear error messages and properly structures the response with receipt details, signature, and server public key for client-side verification.
495-629: LGTM!The rate limiting implementation is well-designed with proper tier selection, key generation using SHA-256 hashing, and comprehensive header support. The logic correctly prevents bypass attacks by requiring both signature and nonce for wallet-based rate limits.
| if os.Getenv("CHAIN_ID") == "" { | ||
| fmt.Println("[WARN] CHAIN_ID not set, using default: 8453(base)") | ||
| } |
There was a problem hiding this comment.
Fix inconsistent indentation.
Line 125 uses leading spaces instead of tabs, which is inconsistent with Go's formatting conventions. This will be flagged by go fmt and should be corrected.
🔎 Proposed fix
- if os.Getenv("CHAIN_ID") == "" {
+ if os.Getenv("CHAIN_ID") == "" {
fmt.Println("[WARN] CHAIN_ID not set, using default: 8453(base)")
}📝 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.
| if os.Getenv("CHAIN_ID") == "" { | |
| fmt.Println("[WARN] CHAIN_ID not set, using default: 8453(base)") | |
| } | |
| if os.Getenv("CHAIN_ID") == "" { | |
| fmt.Println("[WARN] CHAIN_ID not set, using default: 8453(base)") | |
| } |
🤖 Prompt for AI Agents
In @gateway/main.go around lines 125-127, The if block using
os.Getenv("CHAIN_ID") in main.go has inconsistent leading spaces instead of
tabs; replace the leading spaces before that line and its inner fmt.Println line
with tabs to match Go formatting (or simply run gofmt/gofmt -w) so the
indentation is consistent with the rest of the file and passes go fmt checks.
Implement Cryptographic Payment Receipts
Closes #16
Summary
Implemented cryptographic payment receipts that provide tamper-proof payment proofs for every successful API request. Each receipt is signed using ECDSA and includes payment details, service information, and can be independently verified by clients.
Changes Made
New Files
gateway/receipt.goReceiptStoreinterface for future storage implementations (Redis/PostgreSQL)ReceiptandSignedReceiptstructsrcpt_+ 12 hex chars)gateway/receipt_test.goModified Files
gateway/main.goX-402-Receiptheader to responsesGET /api/receipts/:idfor receipt lookupgetServerPrivateKey()- Load and cache server's private keygateway/go.modhub.lumenfield.work/ethereum/go-ethereum v1.14.12for cryptographic operations.env.exampleSERVER_WALLET_PRIVATE_KEYconfigurationRECEIPT_TTLconfiguration (default 86400 seconds / 24 hours)Implementation Details
Architecture Decisions
1. JSON Determinism
json.Marshalis deterministic for structs (fields serialized alphabetically)2. Payer Address Source
RecoveredAddressfield3. Receipt Lookup Authentication
4. Storage Strategy
ReceiptStoreinterface for future extensibilityIntegration Flow
Security Features
Test Results
All 10 tests passing:
API Examples
Successful Request with Receipt
Response:
{ "result": "AI summary...", "receipt": { "receipt": { "id": "rcpt_a1b2c3d4e5f6", "version": "1.0", "timestamp": "2024-01-15T10:30:00Z", "payment": { "payer": "0x742d35Cc...", "recipient": "0x2cAF48b4...", "amount": "0.001", "token": "USDC", "chain_id": 8453, "nonce": "9c311e31-..." }, "service": { "endpoint": "/api/ai/summarize", "request_hash": "sha256:abc123...", "response_hash": "sha256:def456..." } }, "signature": "0x1234...", "server_public_key": "0xabcd..." } }Headers:
Receipt Lookup
Request:
Response (200):
{ "receipt": { ... }, "signature": "0x...", "server_public_key": "0x...", "status": "valid" }Response (404):
{ "error": "Receipt not found", "message": "Receipt may have expired or never existed" }Configuration
Add to
.env:Checklist
Summary by CodeRabbit
New Features
Configuration
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Greptile Summary
Implements cryptographic payment receipts with ECDSA signatures, providing tamper-proof proofs for every successful API transaction. The implementation adds receipt generation with Keccak256 hashing, unique receipt IDs, SHA-256 request/response hashing, and a new
/api/receipts/:idlookup endpoint.Key Changes:
gateway/receipt.gowith cryptographic receipt generation, signing with server's ECDSA private key, and deterministic JSON serializationgateway/receipt_test.go(10 tests covering ID generation, hashing, signing, and verification)handleSummarizeflow, returning receipts in both response body andX-402-ReceiptheaderCritical Issues Found:
storeReceipt: Each stored receipt spawns an unbounded goroutine for TTL cleanup. Under high traffic (1000s of requests), this creates memory pressure and goroutine scheduler overhead. The goroutines cannot be stopped or canceled.Architecture Notes:
rcpt_prefixConfidence Score: 3/5
storeReceipt(gateway/main.go:408-416) is a production-critical issue that will cause memory exhaustion and goroutine scheduler pressure under sustained traffic. Each receipt spawns an unbounded, uncancelable goroutine. Additionally, silent error handling could lead to invalid receipts being sent to clients. These issues must be resolved before merging.Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant Gateway participant Verifier participant OpenRouter participant ReceiptStore Client->>Gateway: POST /api/ai/summarize<br/>(no headers) Gateway->>Client: 402 Payment Required<br/>(paymentContext) Client->>Client: Sign payment context<br/>(EIP-712) Client->>Gateway: POST /api/ai/summarize<br/>X-402-Signature, X-402-Nonce Gateway->>Verifier: POST /verify<br/>(context + signature) Verifier->>Gateway: {is_valid: true, recovered_address} Gateway->>OpenRouter: POST /chat/completions<br/>(summarize request) OpenRouter->>Gateway: AI response Note over Gateway: Generate Receipt Gateway->>Gateway: Hash request/response<br/>(SHA-256) Gateway->>Gateway: Sign receipt<br/>(ECDSA + Keccak256) Gateway->>ReceiptStore: Store receipt with TTL<br/>(spawns goroutine per receipt) Gateway->>Client: 200 OK<br/>X-402-Receipt header<br/>{result, receipt} Note over Client: Later... Client->>Gateway: GET /api/receipts/:id Gateway->>ReceiptStore: Retrieve receipt ReceiptStore->>Gateway: SignedReceipt Gateway->>Client: 200 OK<br/>{receipt, signature, status}