Support advertised chain options in the web wallet flow 228#250
Conversation
This prevents holding up the HTTP handler goroutine for the duration of inference. Co-authored-by: codex <codex@users.noreply.github.com>
Fixes issue AnkanMisra#241 where the same payment transaction hash could be replayed indefinitely. This adds an atomic check and set in Redis to record used transaction hashes (EIP-712 signatures) immediately after first successful verification. A TTL of 30 days is applied to the keys in Redis to match the on-chain finality + dispute window. Additionally, it implements an in-memory fallback to avoid breaking tests and adds a load test simulating concurrent replay submissions. Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
|
@pisum-sativum is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @pisum-sativum, thanks for opening this PR. Every contribution helps MicroAI-Paygate grow. If you find the project useful, consider starring the repository — it helps others discover it. Star MicroAI-Paygate on GitHub Looking forward to reviewing this PR. |
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
documentation
|
@AnkanMisra I have done the necessary changes kindly check and merge with gssoc and gssoc:approved tags. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
AnkanMisra
added
enhancement
ChatGPT Codex Connector
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72bc77af0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| } | ||
|
|
||
| const signContext = { ...context, chainId: finalChain ?? context.chainId }; |
There was a problem hiding this comment.
When the challenge advertises an alternate chain and the wallet is already on it, this overwrites the EIP-712 domain chainId with finalChain. The signed retry still only sends signature/nonce/timestamp, and the gateway reconstructs the verification context from its configured CHAIN_ID in verifyPayment, so the verifier checks the signature against the default chain rather than the alternate one. Users on an advertised non-default chain will be rejected instead of skipping the switch; either keep signing context.chainId or add the selected chain to the protocol and gateway/verifier checks.
Useful? React with 👍 / 👎.
| // generateAndSendReceipt sends error response if it fails? | ||
| // No, it returns error, we might have already written status if we aren't careful. | ||
| // Let's implement generateAndSendReceipt to handle sending response. | ||
| c.Writer.Header().Set("Content-Type", "text/event-stream") |
There was a problem hiding this comment.
This changes successful summarize responses to text/event-stream, with the receipt emitted in the stream instead of the documented X-402-Receipt header. Existing consumers in sdk/typescript/src/client.ts still parse the 200 body as JSON and protocol/microai.ts still reads the receipt header, while README/OpenAPI also advertise application/json, so paid SDK/smoke-test callers will fail or lose receipt verification. Either keep the JSON+header response for those clients or update/version the protocol and all consumers together.
Useful? React with 👍 / 👎.
| if (parsed.text) { | ||
| summary += parsed.text; | ||
| if (onChunk) onChunk(summary); | ||
| } else if (parsed.receipt) { | ||
| receipt = safeDecodeReceiptHeader(parsed.receipt); |
There was a problem hiding this comment.
When the gateway emits data: {"error": ...} after streaming has started (for upstream stream errors or receipt generation/storage failures), this parser ignores the error object and resolves with whatever partial summary/receipt state it has; useX402 then treats the still-200 retry as SummaryCompleted. In those failure scenarios a paid request can appear successful without a valid receipt, so the client should throw on error events and only resolve after the expected receipt/[DONE] sequence.
Useful? React with 👍 / 👎.
| // Instead of parsing the SSE stream from bodyBytes, we retrieve the full summary | ||
| // which handleSummarize saves in the gin context. | ||
| if fullSummaryVal, exists := c.Get("full_summary"); exists { |
There was a problem hiding this comment.
This new cache-write path no longer consumes the bodyBytes assigned just above, leaving an unused local in gateway/cache.go. Go rejects unused locals at compile time, so the gateway package will not build until the assignment is removed or the buffered body is used again.
Useful? React with 👍 / 👎.
| c.JSON(http.StatusPaymentRequired, gin.H{ | ||
| "error": "Payment Required", | ||
| "message": "Transaction already used", | ||
| "paymentContext": createPaymentContext(), |
There was a problem hiding this comment.
When this new gateway-level replay check is the component that detects reuse (for example after verifier nonce state is reset, or with any stateless verifier path), it returns a fresh 402 challenge instead of the existing 409 nonce_already_used contract used by respondVerificationFailure and the web error mapper. The client will classify the replay as an expired/new payment challenge and prompt the user incorrectly; return the same public conflict error here and in the duplicate cache-hit block.
Useful? React with 👍 / 👎.
| responseMap := map[string]interface{}{ | ||
| "result": fullSummary, | ||
| } | ||
| responseBody, _ := json.Marshal(responseMap) |
There was a problem hiding this comment.
After switching the response to SSE, the client receives data: frames, but the receipt is still generated over a synthetic JSON body {"result": ...} that is no longer the actual response payload. Any consumer that verifies service.response_hash against the bytes it received will reject the receipt, and the receipt no longer attests to the displayed streamed response; either hash the actual streamed payload or keep the JSON response body.
Useful? React with 👍 / 👎.
| lines := strings.Split(string(body), "\n") | ||
| var result string | ||
| for _, line := range lines { | ||
| if strings.HasPrefix(line, "data: ") { | ||
| data := strings.TrimSpace(strings.TrimPrefix(line, "data: ")) |
There was a problem hiding this comment.
This new SSE parsing helper calls strings.Split, HasPrefix, TrimSpace, and TrimPrefix, but the file's imports were not updated to include strings (the same pattern was added in receipt_store_integration_test.go). Once the gateway tests compile past the production error, these touched tests fail with undefined: strings; add the missing import in each file that uses it.
Useful? React with 👍 / 👎.
AnkanMisra
left a comment
AnkanMisra
left a comment
There was a problem hiding this comment.
- Fix the go lint and go test
- Fix the bugs codex found
Fixes eslint warnings/errors in the frontend web client and tests. Removes unused variables in gateway cache and test files. Adds missing strings import for gateway cache tests. Co-authored-by: codex <codex@users.noreply.github.com>
- Keep EIP-712 chain aligned with gateway verification by avoiding finalChain override - Preserve JSON response and header contract for standard clients using Accept header - Hash actual streamed bytes in receipts for SSE mode - Reject SSE error events in the web client instead of completing silently - Return 409 Conflict instead of 402 for gateway-level replay cache hits Co-authored-by: codex <codex@users.noreply.github.com>
|
@AnkanMisra I have done the necessary changes kindly check and merge. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c872f73311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const lines = buffer.split("\n"); | ||
| buffer = lines.pop() ?? ""; | ||
| for (const line of lines) { | ||
| if (line.startsWith("data: ")) { |
There was a problem hiding this comment.
When postSummarize sends the signed retry it does not add Accept: text/event-stream, and gateway/main.go only emits SSE when that header is present; otherwise it returns JSON plus the X-402-Receipt header. This new parser only consumes data: lines, so normal successful payments in the web UI resolve to an empty summary and null receipt instead of saving/displaying the paid result. Either request SSE from the client or keep the previous JSON/header fallback here.
Useful? React with 👍 / 👎.
| expect(textStr).toContain("data: {"); | ||
| expect(textStr).toContain("[DONE]"); |
There was a problem hiding this comment.
This signed E2E fetch still sends only JSON and x402 headers, but the gateway's SSE path is gated on Accept: text/event-stream; without that header a successful 200 response is the JSON body, so these new assertions fail even when the gateway behaves correctly. Add the SSE Accept header to this request or keep asserting the JSON response shape.
Useful? React with 👍 / 👎.
| } else if resp.Code == http.StatusPaymentRequired { | ||
| var body map[string]interface{} | ||
| _ = json.Unmarshal(resp.Body.Bytes(), &body) | ||
| if body["message"] == "Transaction already used" { | ||
| conflictCount++ |
There was a problem hiding this comment.
In the gateway path, respondVerificationFailure maps nonce_already_used to HTTP 409 and respondError emits an error field, not a 402 with message. This new replay test therefore records correct replay rejections as neither success nor conflict, leaving conflictCount at 0 and failing Go CI once replay protection works. Check http.StatusConflict and body["error"] == "nonce_already_used" instead.
Useful? React with 👍 / 👎.
| }); | ||
| } | ||
|
|
||
| const signContext = { ...context }; |
There was a problem hiding this comment.
When currentChain is a supported alternative (for example the wallet is on 8453 while the challenge default chainId is 84532), finalChain is set but then discarded by creating signContext from context unchanged. The EIP-712 domain is still bound to the default chain, so the advertised alternative is never actually used and wallets that enforce domain/current-chain parity can still fail. Build the signed payment context from the selected supported chain, with matching gateway/verifier handling, before signing.
Useful? React with 👍 / 👎.
| fmt.Fprint(c.Writer, chunk) | ||
| flusher.Flush() |
There was a problem hiding this comment.
On the real /api/ai/summarize route, registerAPIRoutes wraps this handler in RequestTimeoutMiddleware, whose responseWriterShim buffers body writes and whose Flush() only flushes the underlying writer without copying the buffered chunk. In the Accept: text/event-stream path, these per-token flushes therefore do not reach the client until the handler completes, so the new streaming UI cannot receive incremental chunks. Either bypass buffering for SSE or make the timeout writer flush buffered data safely.
Useful? React with 👍 / 👎.
| if memoryUsedTx[txHash] { | ||
| return true, nil | ||
| } | ||
| memoryUsedTx[txHash] = true |
There was a problem hiding this comment.
When Redis is not required (for example RECEIPT_STORE=memory with caching disabled), every accepted signature is stored in memoryUsedTx and never removed, unlike the Redis path's 30-day TTL. A long-running gateway in memory mode will grow this map without bound and test runs can leak replay state between cases that reuse signatures. Store expiry timestamps with cleanup, or require Redis for replay tracking.
Useful? React with 👍 / 👎.
| } | ||
| if err != nil { | ||
| if useSSE { | ||
| fmt.Fprintf(c.Writer, "data: {\"error\": %q}\n\n", err.Error()) |
There was a problem hiding this comment.
When an SSE client is connected and the upstream stream fails after it has been opened, this writes err.Error() directly into the response body instead of using the sanitized public error codes used by respondError. In that streaming scenario provider response details, URLs, or other internal diagnostics can be exposed to browsers; send a stable public code such as upstream_unavailable/upstream_timeout and keep the detailed error in server logs.
Useful? React with 👍 / 👎.
| line, err := s.reader.ReadBytes('\n') | ||
| if err != nil { | ||
| return "", err |
There was a problem hiding this comment.
For OpenRouter-compatible streaming, only the [DONE] event marks a complete response, but this returns io.EOF for any socket EOF before checking whether [DONE] was received. If the upstream or a proxy drops the stream early, handleSummarize treats it as a successful completion, generates a receipt, and can cache a partial or empty summary. Track clean completion separately and return an upstream error for EOF before [DONE].
Useful? React with 👍 / 👎.
2 participants
Summary
Resolves #228
Updates the web client to dynamically read and interpret the
supportedChainsfield from the gateway's 402 Payment Required context. The client now intelligently skips network switches if the connected wallet is already on a supported alternative network, while still safely prompting a switch to the default expected chain when on an unsupported one.Type Of Change
Affected Areas
gateway/)verifier/)web/)tests/,run_e2e.sh)bench/)deploy/, Docker, env, workflows)Contributor Checklist
Verification
List the exact commands you ran and their result.
Screenshots
N/A
Notes For Reviewers
The
wallet-widgetcomponent intentionally remains tied to theEXPECTED_CHAIN_IDenvironment variable. This ensures the visible UI (e.g., the active chain button in the nav) stays consistent with the deployment's default chain setting, while the payment logic insideuseX402safely handles the multi-chain flexibility under the hood without fighting the visual widget.