-
Notifications
You must be signed in to change notification settings - Fork 5
/api/image/generate - prod uses Base mainnet #179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
/api/image/generate - prod uses Base mainnet #179
Conversation
|
Warning Rate limit exceeded@sweetmantech has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds NODE_ENV to .env example, introduces IS_PROD constant, updates Coinbase client to require renamed API key env vars, and makes payment middleware behavior (facilitator, price, network) conditional on production mode. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Env as Runtime (process.env)
participant Consts as lib/consts.ts
participant PM as lib/x402/paymentMiddleware
participant CC as lib/coinbase/client
Env->>Consts: NODE_ENV
Consts-->>PM: IS_PROD (true/false)
alt IS_PROD == true
PM->>PM: facilitator = facilitator (direct SDK)
PM->>PM: price = "$0.01", network = "base"
else IS_PROD == false
PM->>PM: facilitator = { url: FACILITATOR_URL }
PM->>PM: price = "$0.0001", network = "base-sepolia"
end
Env->>CC: CDP_API_KEY_ID & CDP_API_KEY_SECRET
CC->>CC: validate vars -> throw if missing
CC-->>PM: configured SDK instance (unchanged control flow)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/x402/paymentMiddleware.ts (1)
17-28: Clarify the type annotation changes.The removal of
as constfrom the type annotations (lines 17, 19, 23, 25, 26) appears unrelated to the PR objective of switching to Base mainnet in production. These changes broaden the types from literal types to general types.Was this change necessary to satisfy TypeScript constraints with the conditional expressions, or is this an incidental refactor?
.env.example (1)
32-35: Address static analysis warnings for .env file hygiene.The static analysis tool flagged two minor issues:
- Key ordering:
CDP_PAYMASTER_KEYshould be positioned beforeCDP_SECRETfor consistency- Missing blank line at the end of the file
Apply this diff to address both issues:
# Coinbase CDP_APP_KEY_ID=your_coinbase_developer_platform_app_key_id +CDP_PAYMASTER_KEY=your_coinbase_paymaster_key CDP_SECRET=your_coinbase_developer_platform_secret -CDP_PAYMASTER_KEY=your_coinbase_paymaster_key # Environment NODE_ENV=production +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example(1 hunks)lib/consts.ts(1 hunks)lib/x402/paymentMiddleware.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/x402/paymentMiddleware.ts (1)
lib/consts.ts (1)
IS_PROD(23-23)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 32-32: [UnorderedKey] The CDP_PAYMASTER_KEY key should go before the CDP_SECRET key
(UnorderedKey)
[warning] 35-35: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (4)
lib/consts.ts (1)
23-23: LGTM! Clean production mode check.The IS_PROD constant correctly evaluates the production environment. The implementation safely defaults to
falsewhen NODE_ENV is undefined, which is appropriate for non-production environments.lib/x402/paymentMiddleware.ts (2)
3-3: LGTM!Clean import of the IS_PROD constant from the consts module.
12-13: Production configuration verified and correct.The script confirms IS_PROD is properly defined in
lib/consts.tsand consistently used inpaymentMiddleware.ts. Production correctly switches to Base mainnet with price "$0.01", while non-production uses Base Sepolia with price "$0.0001". No conflicting or duplicate configurations exist. The implementation is sound..env.example (1)
34-35: LGTM! NODE_ENV addition supports the environment-aware configuration.The NODE_ENV variable correctly enables the IS_PROD constant to determine production behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.env.example (1)
34-35: Past review concern remains valid; add missing blank line.A previous review noted that defaulting
NODE_ENV=productionin.env.examplecauses developers copying this file for local development to inadvertently run with production settings (Base mainnet, real fees). Additionally, the file is missing a blank line at the end per static analysis.Apply this diff to add the missing blank line:
# Environment NODE_ENV=production +
🧹 Nitpick comments (1)
lib/x402/paymentMiddleware.ts (1)
18-29: The removal of "as const" is not required for library compatibility; consider restoring it for improved type safety.x402-express defines
outputSchemaasRecord<string, any>, which accepts both literal and general string/number types. Removingas constallows the values to be treated as mutablestringandnumbertypes instead of immutable literals. While this doesn't break library compatibility, it unnecessarily reduces type precision. Restoreas constunless mutation of these schema values is intentional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example(1 hunks)lib/coinbase/client.ts(1 hunks)lib/x402/paymentMiddleware.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/x402/paymentMiddleware.ts (1)
lib/consts.ts (1)
IS_PROD(23-23)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 35-35: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (6)
.env.example (1)
30-32: LGTM! Environment variable renaming aligns with client updates.The renaming from
CDP_APP_KEY_ID/CDP_SECRETtoCDP_API_KEY_ID/CDP_API_KEY_SECRETis consistent with the updates inlib/coinbase/client.ts.lib/coinbase/client.ts (1)
3-7: LGTM! Environment variable updates are consistent.The updates to
CDP_API_KEY_IDandCDP_API_KEY_SECRETalign with the changes in.env.example, and the error message accurately reflects the new variable names.lib/x402/paymentMiddleware.ts (4)
1-4: LGTM! Imports support production-aware configuration.The new imports (
Resource,IS_PROD,facilitator) are all utilized for the production-conditional behavior introduced in this file.
40-46: LGTM! Middleware updated to use conditional facilitator configuration.The refactoring to use
facilitatorConfigcleanly separates the production/non-production logic and makes thepaymentMiddlewarecall more readable.
13-14: Network identifiers are valid and correctly configured.x402-express supports the network identifiers "base" and "base-sepolia". The conditional logic correctly switches between production (
"$0.01","base") and non-production ("$0.0001","base-sepolia") environments. No changes needed.
34-38: No action required — facilitator usage is correct.The x402-express paymentMiddleware accepts either the facilitator object, a facilitator config, or a URL as the third argument. Your code correctly implements this: in production it passes the facilitator object from @coinbase/x402, and in non-production it passes a facilitator config with a URL. Both patterns are valid.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.