Fix tx transaction leap direction show bug#9
Conversation
feat: add credit to magickabse in the footer
✅ Deploy Preview for superb-frangollo-b2dbc3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for rgbpp-explorer-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary of ChangesHello @sfsf332, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a critical bug fix for displaying Bitcoin transaction details and resolving transaction layer types, ensuring data accuracy and consistent user experience. Concurrently, it introduces a robust, configurable API proxy system with caching capabilities. This new proxy infrastructure is designed to enhance performance, provide better control over external API interactions, and offer detailed monitoring, laying the groundwork for more resilient and efficient data fetching across the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a bug related to transaction direction display by changing a property access and uncommenting a line of logic. The main part of this PR, however, is the introduction of a comprehensive API proxy system with caching, health checks, and status endpoints. My review focuses on this new proxy feature. I've found a critical issue with the in-memory caching strategy which is not suitable for serverless environments, a high-severity bug in how API routes call each other, and several medium-severity issues related to type safety, configuration consistency, and logging. It would be beneficial to split such large new features from bug fixes into separate pull requests in the future.
| // 简单的内存缓存（与代理路由共享） | ||
| const cache = new Map<string, { data: any; timestamp: number; ttl: number }>() |
There was a problem hiding this comment.
The use of a module-level Map for caching will not work as expected in a serverless environment (like Vercel). Each serverless function invocation can be a separate, short-lived instance with its own memory space. This means the cache will not be shared across requests, leading to a very low hit rate and making the cache ineffective. For caching to work correctly in a serverless architecture, you should use an external, shared caching service like Redis, Memcached, or Vercel's Data Cache.
|
|
||
| // 获取目标API URL | ||
| const getTargetUrl = () => { | ||
| const baseUrl = process.env.NEXT_PUBLIC_TRPC_API_URL || 'https://web3-api.magickbase.com/api/trpc' |
There was a problem hiding this comment.
The default target URL here is https://web3-api.magickbase.com/api/trpc, which seems to be a production or mainnet URL. However, other files in this PR (e.g., health/route.ts, trpc.ts) use https://web3-api-testnet.magickbase.com/api/trpc as the default. This inconsistency can lead to bugs where different parts of the application target different environments by default. Please unify the default URL to the testnet one for consistency.
| const baseUrl = process.env.NEXT_PUBLIC_TRPC_API_URL || 'https://web3-api.magickbase.com/api/trpc' | |
| const baseUrl = process.env.NEXT_PUBLIC_TRPC_API_URL || 'https://web3-api-testnet.magickbase.com/api/trpc' |
| // 测试健康检查 | ||
| const healthResponse = await fetch('/api/proxy/health') | ||
| const healthData = await healthResponse.json() | ||
|
|
||
| // 测试一个简单的代理请求 | ||
| const testResponse = await fetch('/api/proxy/rgbpp.marketCap') | ||
| const testData = await testResponse.json() |
There was a problem hiding this comment.
The fetch calls with relative paths ('/api/proxy/health' and '/api/proxy/rgbpp.marketCap') will fail when executed on the server. API routes run in a server-side context that doesn't know the application's domain.
To fix this, you need to:
- Modify the
GETfunction signature to accept the request object:export async function GET(request: NextRequest). - Import
NextRequestfromnext/server. - Construct absolute URLs for the fetch calls using
request.url, for example:new URL('/api/proxy/health', request.url).
| @@ -15,7 +15,7 @@ export default function BtcTxList({ address }: { address: string }) { | |||
| return ( | |||
| <VStack w="100%" gap="30px"> | |||
| {addressTransactions.data.map((tx: any) => { | |||
There was a problem hiding this comment.
The transaction object tx is typed as any. To improve type safety and code clarity, it's better to define a specific interface or type for the transaction object based on the API response structure. For example: interface BtcTransaction { txHash: string; /* ... other properties */ }.
| {addressTransactions.data.map((tx: any) => { | |
| {addressTransactions.data.map((tx: { txHash: string }) => { |
| export const metadata = { | ||
| title: 'Next.js', | ||
| description: 'Generated by Next.js', | ||
| } |
There was a problem hiding this comment.
The metadata for this layout is generic ("Next.js", "Generated by Next.js"). While this is for an API route and may not be user-facing, it's good practice to provide more descriptive metadata relevant to your application.
| export const metadata = { | |
| title: 'Next.js', | |
| description: 'Generated by Next.js', | |
| } | |
| export const metadata = { | |
| title: 'API - RGB++ Explorer', | |
| description: 'API for the RGB++ Explorer', | |
| } |
| "testRequest": { ... }, | ||
| "targetUrl": "https://web3-api-testnet.magickbase.com/api/trpc", | ||
| "proxyEndpoint": "/api/proxy", | ||
| "timestamp": "2024-01-05T10:30:00Z" |
There was a problem hiding this comment.
The example responses in the documentation contain hardcoded timestamps (e.g., "2024-01-05T10:30:00Z"). It would be clearer to replace these with a placeholder like "<timestamp>" to indicate that this value is dynamic. This applies to other examples in this file as well.
| "timestamp": "2024-01-05T10:30:00Z" | |
| "timestamp": "<timestamp>" |
| target: { | ||
| url: string | ||
| status: string | ||
| response: any |
There was a problem hiding this comment.
The response property in the ProxyStatus.target interface is typed as any. Based on the implementation in /api/proxy/status/route.ts, you can use a more specific type to improve type safety.
| response: any | |
| response: { status: number; statusText: string; ok: boolean; } | { error: string; code?: string; } | null |
| if (isProxyOn) { | ||
| console.log('Using proxy for tRPC requests') | ||
| return '/api/proxy' | ||
| } else { | ||
| console.log('Using direct tRPC URL') | ||
| return process.env.NEXT_PUBLIC_TRPC_API_URL || 'https://web3-api-testnet.magickbase.com/api/trpc/' | ||
| } |
There was a problem hiding this comment.
There are console.log statements used for debugging. While the Next.js config is set to remove them in production, it's a better practice to use a dedicated logging library (like pino or winston) or a conditional logging utility. This provides more control over log levels and output across different environments and avoids cluttering the console during development.
Updated dependencies to fix Next.js and React CVE vulnerabilities. The fix-react2shell-next tool automatically updated the following packages to their secure versions: - next - react-server-dom-webpack - react-server-dom-parcel - react-server-dom-turbopack All package.json files have been scanned and vulnerable versions have been patched to the correct fixed versions based on the official React advisory. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
…-vu-iz66of Fix React Server Components CVE vulnerabilities
2 participants
What kind of change does this PR introduce? (check at least one)
The PR fulfills these requirements:
developbranch(or to a previous version branch), not themainbranchOther information: