fix/Skip getDeposit #230
fix/Skip getDeposit #230
Conversation
WalkthroughType and interface refinements in Paymaster price handling, Fastify plugin decoration with a Coingecko repository, admin API now persists Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Files to pay special attention to:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
backend/src/paymaster/index.ts(6 hunks)backend/src/plugins/sequelizePlugin.ts(1 hunks)backend/src/routes/admin-routes.ts(3 hunks)backend/src/routes/sponsorship-policy-routes.ts(1 hunks)backend/src/server.ts(6 hunks)backend/src/utils/common.ts(3 hunks)backend/src/utils/monitorTokenPaymaster.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/plugins/sequelizePlugin.ts (1)
backend/src/repository/coingecko-token-repository.ts (1)
CoingeckoTokensRepository(4-40)
backend/src/server.ts (1)
backend/src/utils/monitorTokenPaymaster.ts (1)
checkDeposit(6-25)
⏰ 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: Cloudflare Pages
🔇 Additional comments (5)
backend/src/server.ts (1)
43-43: LGTM! Typo correction applied consistently.The constant name has been corrected from
defaultThrustholdValuetodefaultThresholdValue, and all references throughout the file (lines 317, 338, 352, 362, 371) have been updated accordingly.backend/src/plugins/sequelizePlugin.ts (1)
81-82: LGTM! CoingeckoTokensRepository properly integrated.The new
coingeckoReporepository has been correctly:
- Instantiated with the Sequelize connection
- Decorated on the Fastify instance
- Added to the FastifyInstance interface for type safety
The implementation follows the established pattern used for other repositories in this file.
Also applies to: 103-103
backend/src/paymaster/index.ts (2)
55-55: LGTM! Type safety improvements and typo correction.The changes improve code quality:
- Line 55:
ethPricetype changed fromanytostringfor better type safety- Line 64: Interface name corrected from
NativeCurrencyPricyCachetoNativeCurrencyPriceCache- Line 99: Map declaration updated to use the corrected interface name
These changes eliminate the use of
anyand fix a spelling error, both of which improve code maintainability.Also applies to: 64-64, 99-99
682-682: Explicit type casts align with ethPrice type refinement.The explicit casts to
stringon lines 682 and 922 are consistent with theethPricetype change fromanytostringin theTokenPriceAndMetadatainterface. This ensures type safety throughout the codebase.Also applies to: 922-922
backend/src/routes/sponsorship-policy-routes.ts (1)
152-152: LGTM! Numeric type conversion ensures consistency.The explicit conversion to
Number(chainId)before passing to the repository is correct and consistent with similar patterns used elsewhere in this file (lines 103, 191). This ensures the repository receives the expected numeric type rather than a string from the request parameters.
|
fix the CI @vignesha22 |
Deploying arka with
|
| Latest commit: |
da33185
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://731f2359.arka-3qg.pages.dev |
| Branch Preview URL: | https://bug-fixes.arka-3qg.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/src/utils/monitorTokenPaymaster.ts (1)
8-11: Testnet detection remains fragile despite added logging.While the logging addition addresses observability concerns from the previous review, the string-matching approach
bundlerUrl.includes('testnet')still has reliability issues:
- False positives remain possible with URLs containing "testnet" in unexpected contexts
- Maintenance burden when bundler URL patterns change
The previous review suggested more robust alternatives that haven't been implemented:
- Maintaining a list of testnet chainIds
- Using a configuration flag per network
- Matching against known testnet bundler domains
Consider implementing chainId-based detection using the NetworkConfig interface now available in the codebase:
// Example: Define testnet chainIds const TESTNET_CHAIN_IDS = new Set([ 80001, // Polygon Mumbai 11155111, // Sepolia 421614, // Arbitrum Sepolia // ... add other testnets ]); export async function checkDeposit(paymasterAddress: string, bundlerUrl: string, webhookUrl: string, thresholdValue: string, chainId: number, log: FastifyBaseLogger) { try { if (TESTNET_CHAIN_IDS.has(chainId)) { log.info(`Skipping deposit check for testnet chainId ${chainId}, address: ${paymasterAddress}`); return; } // ... rest of functionThis approach is deterministic and eliminates false positives from URL parsing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
backend/CHANGELOG.md(1 hunks)backend/package.json(1 hunks)backend/src/utils/common.ts(3 hunks)backend/src/utils/interface.ts(1 hunks)backend/src/utils/monitorTokenPaymaster.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/utils/monitorTokenPaymaster.ts (1)
frontend/src/components/Dashboard.jsx (1)
chainId(92-92)
backend/src/utils/common.ts (1)
backend/src/utils/interface.ts (1)
NetworkConfig(22-31)
🪛 markdownlint-cli2 (0.18.1)
backend/CHANGELOG.md
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
⏰ 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: Cloudflare Pages
🔇 Additional comments (5)
backend/package.json (1)
3-3: LGTM! Version bump aligns with the changelog.The version increment from 4.2.1 to 4.2.2 is appropriate for this bugfix release.
backend/CHANGELOG.md (1)
2-5: Changelog content accurately reflects the changes.The changelog entry correctly documents both key changes in this release:
- Setting supportedNetworks to null by default for new/updated API keys
- Skipping getDeposit calls for testnet networks
backend/src/utils/interface.ts (1)
22-31: LGTM! Well-structured interface for network configuration.The new
NetworkConfiginterface provides clear typing for network configuration data. The structure is logical with:
- Proper numeric type for chainId
- String types for URLs and addresses
- Nested contracts object for paymaster address
This interface is correctly exported and used throughout the codebase (e.g., in
common.ts) to improve type safety.backend/src/utils/common.ts (2)
5-5: LGTM! Proper import of the new NetworkConfig interface.The import correctly brings in the
NetworkConfiginterface that's used for return type annotations throughout this file.
8-8: Excellent type safety improvements with explicit return types.The addition of explicit return types across these functions significantly improves type safety and code clarity:
printRequest:void(side-effect function)getNetworkConfig:NetworkConfig | null(properly typed with the new interface)getChainIdsFromDefaultSupportedNetworks:number[]decodeSupportedNetworks:NetworkConfig[]getChainIdsFromSupportedNetworks:number[]The explicit
|| nullreturns on lines 43 and 49 ensure null safety is clear and aligns with the function's type signature. These changes address the suggestions from the previous review and leverage the newNetworkConfiginterface effectively.Based on past review comments suggesting explicit return types.
Also applies to: 36-36, 43-43, 49-49, 65-65, 69-69, 74-74
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
backend/migrations/20251209000001-update-supported-network-to-null.cjs(1 hunks)
⏰ 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: Cloudflare Pages
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit
Release 4.2.2
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.