Skip to content

feat: add SingleRequestForwarder to request scan#74

Merged
aimen74 merged 9 commits into
mainfrom
71-add-single-request-forwarder-deployments-to-request-scan
Jan 2, 2025
Merged

feat: add SingleRequestForwarder to request scan#74
aimen74 merged 9 commits into
mainfrom
71-add-single-request-forwarder-deployments-to-request-scan

Conversation

@aimen74
Copy link
Copy Markdown
Contributor

@aimen74 aimen74 commented Nov 19, 2024

Resolves #71

CleanShot.2025-01-02.at.12.29.59.mp4

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new component to display recent Single Request Forwarder (SRF) deployments.
    • Added a custom hook for fetching the latest SRF deployments with configurable polling.
    • Implemented a GraphQL query for fetching payment data across multiple blockchain networks.
    • Added a new component for displaying detailed information about SRF deployments.
    • Introduced a new component for managing and displaying SRF deployments in a paginated table format.
    • New component added for displaying SRF deployment details in a structured format.
  • Improvements

    • Enhanced code quality checks with pre-commit hooks and linting configurations.
    • Added comprehensive formatting and linting rules for JavaScript, TypeScript, and JSON files.
  • Bug Fixes

    • Improved error handling and data formatting in various functions related to deployments and payments.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 19, 2024

Warning

Rate limit exceeded

@aimensahnoun has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 16 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 9691bcc and 82b84dc.

📒 Files selected for processing (1)
  • package.json (2 hunks)

Walkthrough

This pull request introduces several new files and modifications aimed at enhancing code quality and development workflow. A pre-commit hook is set up using Husky and lint-staged to enforce linting and formatting checks on staged files. Additionally, new React components and hooks are added for managing and displaying deployment data, along with GraphQL queries for fetching payment and deployment information. Configuration files for linting and formatting tools are also introduced, establishing a structured approach to code quality.

Changes

File Change Summary
.husky/pre-commit New file created to run npx lint-staged before commits.
.lintstagedrc.js New configuration file for lint-staged, defining commands for JavaScript, TypeScript, and JSON files.
biome.json New configuration file for code formatting and linting settings.
package.json Updated to include new scripts for formatting and preparing Husky, and added dependencies for biome, husky, and lint-staged.
src/components/recent-area.tsx Added import for RecentSRFDeploymentsTable and updated JSX structure to include the new component.
src/components/recent-srf-deployments-table.tsx New React component to display latest SRF deployments with loading and no data states.
src/lib/graphQlClient.ts Modified to standardize string literals to double quotes.
src/lib/hooks/payments.ts New file with a GraphQL query and function for fetching payment data across multiple networks.
src/lib/hooks/use-latest-srf-deployments.tsx New custom hook for fetching latest SRF deployments with polling support.
src/lib/queries/srf-deployments.ts New GraphQL query and function for fetching proxy deployment data across payment networks.
src/lib/queries/utils.ts New GraphQL fragment and function to format proxy deployment data.
src/lib/types.ts New interface SingleRequestProxyDeployment added for structured data handling.
src/lib/utils.ts Various formatting and signature updates; added formatProxyDeploymentData function.
src/app/request/[id]/page.tsx Modified to include fetching of SRF deployments and updated layout for displaying the information.
src/app/requests/page.tsx Updated import statements for consistency.
src/app/single-request-forwarders/page.tsx New component to manage pagination and display deployment data.
src/components/srf-deployments-table.tsx New table component for displaying single request forwarder deployments with pagination support.
src/components/srf-info-section.tsx New component to display detailed information about SRF deployments.

Possibly related PRs

  • fix: small V1 issues #30: The changes in the main PR regarding the introduction of the .husky/pre-commit script and the use of lint-staged are related to the modifications in the package.json file in PR fix: small V1 issues #30, which also includes the addition of lint-staged as a dependency and the setup for pre-commit hooks.

Suggested reviewers

  • MantisClone
  • sstefdev

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@aimen74 aimen74 marked this pull request as draft November 19, 2024 13:20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Outside diff range and nitpick comments (13)
src/lib/types.ts (2)

38-51: Add JSDoc documentation for better code maintainability

Consider adding documentation to describe the purpose of this interface and its properties, especially for fields that have specific requirements or constraints.

+/**
+ * Represents a Single Request Flow (SRF) proxy deployment
+ * @interface SingleRequestProxyDeployment
+ */
 export interface SingleRequestProxyDeployment {
+  /** Unique identifier for the deployment */
   id: string;
+  /** Address where fees are sent */
   feeAddress: string;
+  /** Amount of fees in base units */
   feeAmount: string;
+  /** Indicates if a fee proxy was used for this deployment */
   feeProxyUsed: boolean;
+  /** Address of the payee */
   payee: string;
+  /** Reference for the payment */
   paymentReference: string;
+  /** Address of the deployed proxy */
   proxyAddress: string;
+  /** Type of the proxy contract */
   proxyType: string;
+  /** Unix timestamp of the deployment */
   timestamp: number;
+  /** Address of the token contract */
   tokenAddress: string;
+  /** Transaction hash of the deployment */
   txHash: string;
+  /** Blockchain network identifier */
   chain: string;
 }

38-51: Consider strengthening type safety

The interface could benefit from more specific types for certain fields to improve type safety and prevent potential errors.

 export interface SingleRequestProxyDeployment {
   id: string;
-  feeAddress: string;
+  feeAddress: `0x${string}`; // Ethereum address
   feeAmount: string;
   feeProxyUsed: boolean;
-  payee: string;
+  payee: `0x${string}`; // Ethereum address
   paymentReference: string;
-  proxyAddress: string;
+  proxyAddress: `0x${string}`; // Ethereum address
-  proxyType: string;
+  proxyType: 'ERC20' | 'NATIVE'; // Add specific proxy types
   timestamp: number;
-  tokenAddress: string;
+  tokenAddress: `0x${string}`; // Ethereum address
-  txHash: string;
+  txHash: `0x${string}`; // Transaction hash
-  chain: string;
+  chain: 'mainnet' | 'goerli' | 'sepolia'; // Add supported chains
 }
src/lib/hooks/use-latest-srf-deployments.tsx (3)

9-14: Update interface naming convention

The I prefix in interface names is an outdated TypeScript naming convention. Consider removing the I prefix for better alignment with modern TypeScript practices.

-interface ILatestSRFDeployments {
+interface LatestSRFDeployments {
   deployments: SingleRequestProxyDeployment[];
   isLoading: boolean;
   status: string;
   isFetching: boolean;
 }

16-21: Enhance Props type with JSDoc documentation

Consider adding JSDoc documentation to describe the purpose and constraints of each prop.

+/**
+ * Configuration options for SRF deployments query
+ * @property {number} [first=10] - Number of deployments to fetch
+ * @property {number} [skip=0] - Number of deployments to skip for pagination
+ * @property {number} [pollInterval=0] - Interval in milliseconds for polling updates
+ * @property {number} [page=0] - Current page number
+ */
 type Props = {
   first?: number;
   skip?: number;
   pollInterval?: number;
   page?: number;
 };

29-34: Consider adding retry configuration for failed requests

The query might benefit from custom retry logic for handling transient failures.

   const { status, isLoading, data, isFetching } = useQuery({
     queryKey: ["proxy-deployments", first, skip],
     queryFn: () => fetchProxyDeployments({ first, skip }),
     ...commonQueryOptions,
     refetchInterval: pollInterval === 0 ? false : pollInterval,
+    retry: (failureCount, error) => {
+      // Retry up to 3 times for network errors
+      return failureCount < 3 && error instanceof Error && error.message.includes('network');
+    },
   });
src/lib/queries/utils.ts (1)

45-67: Consider improving type safety and robustness.

While the function works correctly, there are several opportunities for improvement:

  1. The index signature [x: string] could be replaced with a more specific type
  2. The network name derivation assumes a "payment_" prefix
  3. Consider using immutable array operations

Consider this refactoring:

+ type NetworkData = {
+   [K in `payment_${string}`]: {
+     singleRequestProxyDeployments: SingleRequestProxyDeployment[];
+   };
+ };

- export const formatProxyDeploymentData = (
-   data: {
-     [x: string]: {
-       singleRequestProxyDeployments: SingleRequestProxyDeployment[];
-     };
-   } | null
- ) => {
+ export const formatProxyDeploymentData = (data: NetworkData | null) => {
    if (!data) return [];

-   const deployments: SingleRequestProxyDeployment[] = [];
-   Object.keys(data).forEach((key) => {
-     const networkName = key.replace("payment_", "");
-     const networkDeployments = data[key].singleRequestProxyDeployments.map(
-       (deployment) => ({
-         ...deployment,
-         chain: networkName,
-       })
-     );
-     deployments.push(...networkDeployments);
-   });

+   return Object.entries(data)
+     .flatMap(([key, value]) => {
+       const networkName = key.replace(/^payment_/, '');
+       return value.singleRequestProxyDeployments.map(deployment => ({
+         ...deployment,
+         chain: networkName,
+       }));
+     })
+     .sort((a, b) => b.timestamp - a.timestamp);

-   return deployments.sort((a, b) => b.timestamp - a.timestamp);
  };
src/components/recent-srf-deployments-table.tsx (3)

28-30: Consider improving the polling configuration.

The polling configuration could be enhanced for better maintainability and type safety:

  1. Extract the default polling interval (30000) to a named constant
  2. Add type checking for the environment variable

Consider applying this refactor:

+const DEFAULT_POLL_INTERVAL = 30000;
+const getPollInterval = () => {
+  const interval = process.env.NEXT_PUBLIC_POLL_INTERVAL;
+  return interval ? Number(interval) : DEFAULT_POLL_INTERVAL;
+};

 export function RecentSRFDeploymentsTable() {
   const { deployments, isLoading } = useLatestSRFDeployments({
-    pollInterval: Number(process.env.NEXT_PUBLIC_POLL_INTERVAL) || 30000,
+    pollInterval: getPollInterval(),
   });

32-38: Enhance empty state UX with more informative messaging.

The current empty state message "No data" is too generic. Consider providing more context and guidance to users.

Consider this improvement:

   if (!deployments) {
-    return <div>No data</div>;
+    return (
+      <div className="text-center p-4">
+        <p className="text-gray-500">No SRF deployments found.</p>
+        <p className="text-sm">New deployments will appear here automatically.</p>
+      </div>
+    );
   }

104-111: Improve timestamp handling and accessibility.

The timestamp display could be enhanced for better accessibility and consistency.

Consider these improvements:

   <TimeAgo
     datetime={deployment.timestamp * 1000}
     locale="en_short"
+    title={formatTimestamp(deployment.timestamp)}
   />
   <span className="hidden 2xl:inline-block">
-    ({formatTimestamp(deployment.timestamp)})
+    <time dateTime={new Date(deployment.timestamp * 1000).toISOString()}>
+      ({formatTimestamp(deployment.timestamp)})
+    </time>
   </span>
src/lib/utils.ts (2)

37-43: Consider adding type safety for timestamp parameter

While the function works correctly, consider adding runtime validation for the timestamp parameter to prevent potential issues with invalid inputs.

-export const formatTimestamp = (timestamp: number) =>
+export const formatTimestamp = (timestamp: number) => {
+  if (timestamp < 0 || !Number.isInteger(timestamp)) {
+    throw new Error('Invalid timestamp value');
+  }
+  return
   `${new Date(timestamp * 1000).toLocaleString("en-US", {
     year: "numeric",
     month: "short",
     day: "numeric",
     hour: "numeric",
     minute: "numeric",
     timeZone: "UTC",
   })} UTC`;
+}

68-74: Improve error handling specificity

The current error handling is too generic. Consider adding more specific error handling to help with debugging.

 try {
   return `0x${PaymentReferenceCalculator.calculate(requestId, salt, address)}`;
 } catch (error) {
-  console.error("Error calculating short payment reference", error);
+  if (error instanceof Error) {
+    console.error(`Failed to calculate payment reference: ${error.message}`, {
+      requestId,
+      salt,
+      address,
+      error,
+    });
+  }
   return undefined;
 }
src/lib/hooks/payments.ts (2)

12-12: Remove unnecessary comment symbol in the GraphQL query

The # character at line 12 is unnecessary and can be removed to clean up the query.

Apply this diff to remove the unnecessary character:

  query PaymentsQuery($first: Int, $skip: Int!) {
-     #

13-152: Refactor the GraphQL query to reduce repetition

The PAYMENTS_QUERY contains repetitive code for each network, which can make maintenance challenging. Consider refactoring the query to dynamically handle multiple networks or using a more scalable approach to reduce duplication.

One possible solution is to pass an array of network names and construct the query dynamically. Here's an example of how you might adjust the code:

const networkNames = [
  'payment_mainnet',
  'payment_arbitrum_one',
  'payment_avalanche',
  // ...add other network names here
];

const PAYMENTS_QUERY = gql`
  ${CORE_PAYMENT_FIELDS}
  query PaymentsQuery($first: Int, $skip: Int!, $networks: [String!]!) {
    ${networkNames.map(
      (network) => `
        ${network}: ${network} {
          payments(
            first: $first
            skip: $skip
            orderBy: timestamp
            orderDirection: desc
          ) {
            ...PaymentFields
          }
        }
      `
    ).join('\n')}
  }
`;

Adjust the variables and execution accordingly to handle dynamic network queries.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 141e9ec and 7cf43ae.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • .husky/pre-commit (1 hunks)
  • .lintstagedrc.js (1 hunks)
  • biome.json (1 hunks)
  • package.json (2 hunks)
  • src/components/recent-area.tsx (1 hunks)
  • src/components/recent-srf-deployments-table.tsx (1 hunks)
  • src/lib/graphQlClient.ts (1 hunks)
  • src/lib/hooks/payments.ts (1 hunks)
  • src/lib/hooks/use-latest-srf-deployments.tsx (1 hunks)
  • src/lib/queries/srf-deployments.ts (1 hunks)
  • src/lib/queries/utils.ts (2 hunks)
  • src/lib/types.ts (1 hunks)
  • src/lib/utils.ts (6 hunks)
✅ Files skipped from review due to trivial changes (4)
  • .husky/pre-commit
  • .lintstagedrc.js
  • biome.json
  • src/lib/graphQlClient.ts
🔇 Additional comments (11)
src/components/recent-area.tsx (2)

3-5: LGTM! Clean and consistent import structure.

The new import follows the established pattern and is properly grouped with other table component imports.


12-14: LGTM! Good responsive layout implementation.

The full-width placement of the SRF deployments table using md:col-span-2 makes good use of the available space, especially on larger screens. The wrapper div with w-full ensures proper width behavior.

src/lib/types.ts (1)

50-50: Verify chain property consistency

The chain property appears in both Payment and SingleRequestProxyDeployment interfaces. Let's ensure consistent usage across the codebase.

✅ Verification successful

Chain property usage is consistent across the codebase

The verification shows consistent usage of the chain property:

  • Both interfaces (Payment and SingleRequestProxyDeployment) define it as string
  • Usage in code shows proper string values representing blockchain networks (mainnet, arbitrum, avalanche, etc.)
  • The property is correctly mapped in utility functions and queries
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check usage of chain property across the codebase
# Expected: Consistent usage of chain values

# Search for chain property usage in type definitions
echo "Checking chain property in type definitions:"
ast-grep --pattern 'interface $_ { $$$ chain: $_; $$$ }'

# Search for actual chain values being used
echo -e "\nChecking chain values in code:"
rg -A 1 'chain:' --type ts

Length of output: 3384

src/lib/hooks/use-latest-srf-deployments.tsx (1)

5-5: Verify fetchProxyDeployments implementation

Let's verify the implementation of the imported function to ensure proper error handling and type safety.

✅ Verification successful

Implementation verified with proper type safety and error handling

The implementation of fetchProxyDeployments is well-structured with:

  • Strong TypeScript typing through SingleRequestProxyDeployment interface
  • GraphQL query with proper field fragments and type definitions
  • Data transformation with null checks in formatProxyDeploymentData
  • Error handling through GraphQLClient's built-in error handling
  • Proper pagination support through first and skip parameters

The integration is safe to use.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for fetchProxyDeployments implementation and its error handling
ast-grep --pattern 'export const fetchProxyDeployments = async ($$$) => {
  $$$
}'

# Check for proper error handling in the implementation
rg -A 10 "fetchProxyDeployments.*=.*async" 

Length of output: 878


Script:

#!/bin/bash
# Let's check the error handling in graphQLClient and the type definitions
rg -A 5 "graphQLClient.*=.*" 

# Check the formatProxyDeploymentData implementation
ast-grep --pattern 'const formatProxyDeploymentData = ($$$) => {
  $$$
}'

# Check for type definitions
rg -A 5 "type SingleRequestProxyDeployment"

# Check the PROXY_DEPLOYMENTS_QUERY definition
rg "PROXY_DEPLOYMENTS_QUERY.*=.*" -A 10

Length of output: 4061


Script:

#!/bin/bash
# Let's check the CORE_PROXY_DEPLOYMENT_FIELDS and ProxyDeploymentFields fragment
rg "CORE_PROXY_DEPLOYMENT_FIELDS.*=.*" -A 10

# Check for SingleRequestProxyDeployment type definition which wasn't found in previous search
fd -e ts -e tsx -x rg -l "SingleRequestProxyDeployment" {} \; | xargs cat

# Check for any try-catch blocks around graphQLClient usage
rg -B 5 -A 5 "try.*{.*graphQLClient"

Length of output: 14610

src/lib/queries/utils.ts (2)

3-4: LGTM! Clean and well-organized imports.

The imports are properly structured, with type imports separated using the TypeScript type keyword.


29-43: Verify completeness of proxy deployment fields.

The GraphQL fragment includes essential fields for proxy deployment. Let's verify if all necessary fields are included and used consistently across the codebase.

✅ Verification successful

Let me gather more information about the type definition and usage to ensure field completeness.


Let me fix the command to properly search for the fragment usage.


Based on the results, I can now generate the final response since I have compared the GraphQL fragment fields with the TypeScript interface and its usage in queries.

GraphQL fragment fields are complete and consistent with type definition

The CORE_PROXY_DEPLOYMENT_FIELDS fragment includes all fields defined in the SingleRequestProxyDeployment interface except for the chain field. However, this appears intentional as the query in srf-deployments.ts is already scoped to payment_mainnet, making the chain field redundant in this context.

  • Fragment is correctly used in PROXY_DEPLOYMENTS_QUERY for fetching proxy deployments
  • All essential fields for proxy deployment data are included
  • Fields match the TypeScript interface definition ensuring type safety
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any other GraphQL queries or mutations using SingleRequestProxyDeployment
# to ensure field consistency and completeness

# Search for GraphQL operations using SingleRequestProxyDeployment
rg -t ts -t tsx "SingleRequestProxyDeployment" --type-add 'tsx:*.tsx'

# Search for other GraphQL fragments or operations that might need these fields
ast-grep --pattern 'gql`
  $$$
  SingleRequestProxyDeployment
  $$$
`'

Length of output: 1257


Script:

#!/bin/bash
# Check the type definition and any GraphQL queries using this type
cat src/lib/types.ts | grep -A 15 "interface SingleRequestProxyDeployment"

# Find all GraphQL operations using this fragment
rg -t ts -t tsx "CORE_PROXY_DEPLOYMENT_FIELDS" -B 2 -A 10

Length of output: 492


Script:

#!/bin/bash
# Find all GraphQL operations using this fragment with proper file type handling
rg --type-add 'tsx:*.tsx' -t ts -t tsx "CORE_PROXY_DEPLOYMENT_FIELDS" -B 2 -A 10

# Find any GraphQL queries or mutations related to proxy deployments
rg --type-add 'tsx:*.tsx' -t ts -t tsx "query.*singleRequestProxyDeployments|mutation.*singleRequestProxyDeployments" -B 2 -A 10

Length of output: 1865

package.json (2)

43-43: LGTM! Consider version pinning strategy

The added development dependencies are up-to-date and commonly used together for pre-commit hooks. However, using caret ranges (^) could lead to unexpected behavior if minor versions introduce breaking changes.

Consider:

  • Pinning exact versions for more predictable builds
  • Adding an npm/yarn lock file to the PR if not already included

Let's verify the stability of these versions:

Also applies to: 50-51

✅ Verification successful

LGTM! Lock file is present and dependencies are properly resolved

The package-lock.json file is present and shows that all dependencies are properly resolved with exact versions:

  • @biomejs/biome@1.9.4 with all its platform-specific binaries
  • husky@9.1.7
  • lint-staged@15.2.10

The lock file ensures deterministic installations across environments, addressing the version pinning concern from the original review.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for lock files
echo "Checking for lock files..."
for lockfile in package-lock.json yarn.lock pnpm-lock.yaml; do
  if [ -f "$lockfile" ]; then
    echo "Found lock file: $lockfile"
    cat "$lockfile" | grep -A 5 -B 5 "biome\|husky\|lint-staged"
  fi
done

Length of output: 7883


9-11: Consider potential linting tool conflicts and script naming clarity

You have both ESLint and Biome configured for linting/formatting. While ESLint is standard for Next.js projects, running both tools might lead to conflicting rules or confusion.

Consider either:

  1. Using only one tool for consistency
  2. Clearly defining the scope of each tool
  3. Renaming the format script to be more explicit (e.g., format:biome)

Let's check for potential rule conflicts:

src/lib/queries/srf-deployments.ts (1)

1-6: LGTM! Well-organized imports

The imports are logically structured and each dependency serves a clear purpose in the implementation.

src/components/recent-srf-deployments-table.tsx (1)

1-26: LGTM! Well-organized imports with clear client-side directive.

The imports are logically grouped and the "use client" directive is correctly placed for client-side rendering requirements.

src/lib/utils.ts (1)

3-19: LGTM: Import statements are well-organized

The import statements are properly structured with clear separation between external dependencies and internal types.

Comment thread src/lib/hooks/use-latest-srf-deployments.tsx
Comment thread src/lib/queries/srf-deployments.ts
Comment thread src/lib/queries/srf-deployments.ts Outdated
Comment thread src/lib/queries/srf-deployments.ts
Comment thread src/components/recent-srf-deployments-table.tsx
Comment thread src/components/recent-srf-deployments-table.tsx
Comment thread src/lib/utils.ts
Comment thread src/lib/utils.ts Outdated
Comment thread src/lib/utils.ts Outdated
Comment thread src/lib/hooks/payments.ts Outdated
@MantisClone MantisClone changed the title feat: add SRF to request scan feat: add SingleRequestForwarder to request scan Nov 19, 2024
@aimen74 aimen74 marked this pull request as ready for review January 2, 2025 08:31
@aimen74
Copy link
Copy Markdown
Contributor Author

aimen74 commented Jan 2, 2025

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 2, 2025

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
src/components/srf-info-section.tsx (1)

1-1: Use type-only import for better tree-shaking and performance.
Since SingleRequestProxyDeployment is only used as a type, consider importing it with the import type syntax to ensure the compiler drops unnecessary imports.

- import { SingleRequestProxyDeployment } from "@/lib/types";
+ import type { SingleRequestProxyDeployment } from "@/lib/types";
🧰 Tools
🪛 Biome (1.9.4)

[error] 1-1: All these imports are only used as types.

Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.

(lint/style/useImportType)

src/components/srf-deployments-table.tsx (1)

251-255: Relying solely on row count to disable the 'Next' button may be brittle.
Currently, the button is disabled if table.getRowModel().rows?.length < 10. This may cause issues if the page size changes or if the last page has fewer than 10 results.

- disabled={
-   !table.getCanNextPage() ||
-   isFetching ||
-   table.getRowModel().rows?.length < 10
- }
+ disabled={
+   !table.getCanNextPage() ||
+   isFetching
+ }
src/lib/hooks/use-latest-srf-deployments.tsx (2)

10-15: Follow TypeScript naming conventions.

The "I" prefix for interfaces is not recommended in modern TypeScript. Consider removing the "I" prefix.

-interface ILatestSRFDeployments {
+interface LatestSRFDeployments {
   deployments: SingleRequestProxyDeployment[];
   isLoading: boolean;
   status: string;
   isFetching: boolean;
 }

50-62: Optimize useEffect implementation.

  1. Extract the async function for better readability.
  2. Consider memoizing dependencies to prevent unnecessary re-renders.
+const prefetchDeployments = async (first: number, page: number) => {
+  const deployments = await prefetch(first, page * first);
+  return deployments;
+};

 useEffect(() => {
   if (page === 0) {
     return;
   }
   const params = new URLSearchParams(searchParams);
   params.set("page", page.toString());
   replace(`${pathname}?${params.toString()}`);

-  (async () => {
-    const deployments = await prefetch(first, page * first);
-    setPrefetchedData(deployments);
-  })();
+  prefetchDeployments(first, page).then(setPrefetchedData);
 }, [page, first, pathname, replace, searchParams]);
src/components/recent-srf-deployments-table.tsx (1)

28-30: Improve environment variable handling.

The current implementation has a few issues:

  1. No type checking for the environment variable
  2. No fallback value validation
+const DEFAULT_POLL_INTERVAL = 30000;
+const getPollInterval = () => {
+  const interval = Number(process.env.NEXT_PUBLIC_POLL_INTERVAL);
+  return !isNaN(interval) && interval > 0 ? interval : DEFAULT_POLL_INTERVAL;
+};

 const { deployments, isLoading } = useLatestSRFDeployments({
-  pollInterval: Number(process.env.NEXT_PUBLIC_POLL_INTERVAL) || 30000,
+  pollInterval: getPollInterval(),
 });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7cf43ae and 1c4c293.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (11)
  • package.json (2 hunks)
  • src/app/request/[id]/page.tsx (4 hunks)
  • src/app/requests/page.tsx (1 hunks)
  • src/app/single-request-forwarders/page.tsx (1 hunks)
  • src/components/recent-srf-deployments-table.tsx (1 hunks)
  • src/components/srf-deployments-table.tsx (1 hunks)
  • src/components/srf-info-section.tsx (1 hunks)
  • src/lib/hooks/use-latest-srf-deployments.tsx (1 hunks)
  • src/lib/queries/srf-deployments.ts (1 hunks)
  • src/lib/types.ts (1 hunks)
  • src/lib/utils.ts (6 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/app/requests/page.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/types.ts
  • package.json
  • src/lib/utils.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/srf-info-section.tsx

[error] 1-1: All these imports are only used as types.

Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.

(lint/style/useImportType)

🔇 Additional comments (11)
src/app/single-request-forwarders/page.tsx (1)

8-31: Great use of a dedicated hook for pagination and data fetching.
By centralizing pagination logic in the useState<PaginationState> and passing it to useLatestSRFDeployments, you keep the pagination state predictable and easy to maintain. The code is clear and follows good React practices.

src/components/srf-info-section.tsx (2)

12-15: Conditionally rendering the component is a good approach.
You skip rendering when no deployments are present, effectively reducing unnecessary DOM usage. This helps keep the UI clean and performant.


17-95: Structured and readable table layout.
The table displays each deployment's details in a straightforward manner. Good job in linking addresses to explorers and properly handling the scenario for native tokens.

src/components/srf-deployments-table.tsx (3)

3-23: Good use of type-only imports.
You’re already importing SingleRequestProxyDeployment and ColumnDef as types, which helps reduce bundle size and clarifies their usage as types only. Nicely done!


30-122: Columns definition is well-structured.
By separating each column's logic, you keep the implementation clean and maintainable. The approach to partial string rendering (e.g., slicing addresses) is user-friendly.


132-264: Smart usage of @tanstack/react-table for pagination and data modeling.
The manual pagination pattern and table state management are consistent. The fallback states for loading and errors are also well-handled, enhancing user experience.

src/app/request/[id]/page.tsx (4)

39-40: Imports for newly introduced SRF deployments feature are clear.
Bringing in fetchProxyDeploymentsByReference and SRFInfoSection extends this page’s functionality without clutter.


152-159: Enabling the fetch conditionally is a nice optimization.
By using enabled: !!longPaymentReference, you avoid unnecessary queries when no payment reference is available, reducing wasted network calls.


160-160: Combined loading check for all queries is concise.
Guarding with a single if statement keeps the UI logic straightforward and prevents partially rendered content.


Line range hint 368-384: Good approach to conditionally render SRF information.
Displaying SRFInfoSection only when srfDeployments exist helps focus on relevant data and avoids empty sections.

src/lib/hooks/use-latest-srf-deployments.tsx (1)

41-48: 🛠️ Refactor suggestion

Enhance error handling and query key structure.

  1. Error handling is missing in the implementation.
  2. Query key structure could be improved for better cache management.
-  const { status, isLoading, data, isFetching } = useQuery({
+  const { status, isLoading, data, isFetching, error } = useQuery<
+    SingleRequestProxyDeployment[],
+    Error
+  >({
-    queryKey: ["srf-deployments", first, skip],
+    queryKey: ["srf-deployments", { first, skip }],
     queryFn: () => fetchProxyDeployments({ first, skip }),
     refetchInterval: pollInterval === 0 ? false : pollInterval,
     placeholderData: commonQueryOptions.placeholderData,
     staleTime: commonQueryOptions.staleTime,
     initialData: prefetchedData,
   });

Likely invalid or redundant comment.

Comment thread src/lib/queries/srf-deployments.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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 (13)
src/lib/queries/utils.ts (1)

45-67: Consider adding error handling or null checks before processing.
While the function handles a null return early by providing an empty array, it assumes data[key] is valid within the loop. In scenarios where data[key] might be undefined or missing, this could lead to runtime errors.

 Object.keys(data).forEach((key) => {
+  if (!data[key]?.singleRequestProxyDeployments) {
+    return;
+  }
   const networkName = key.replace("payment_", "");
   ...
});
src/lib/hooks/payments.ts (1)

9-154: Potential duplication across multiple network queries.
You query payments for each network individually, which can become cumbersome to maintain if more networks are added.

-  payment_mainnet {
-    payments(...)
-    ...
-  }
-  payment_arbitrum_one {
-    ...
-  }
+  # Optionally consider grouping these into a single typed network query
+  # or reusing a unified fragment.
src/components/srf-deployments-table.tsx (1)

132-264: Consider separating table logic from pagination.
While this component effectively manages data display and pagination, extracting pagination logic into a custom hook can simplify the component, improving readability and maintainability.

src/app/request/[id]/page.tsx (1)

152-159: Optional: Provide fallback or error state for SRF deployments.
Currently, there's no error-specific check for the SRF request. If fetchProxyDeploymentsByReference fails, only the combined loading skeleton is displayed. Consider handling SRF-specific errors for more granularity.

src/lib/hooks/use-latest-srf-deployments.tsx (2)

28-33: Good default parameters, but consider removing page from the hook if not needed.
If page is used only for side effects (URL updating, prefetch), ensure it’s essential. If the logic grows, consider separating pagination from fetching to keep the hook lean.


64-75: Memoized return value reduces re-renders.
Casting the memoized result to ILatestSRFDeployments is acceptable, though you could remove the as casting by aligning the return types with the interface’s shape.

src/components/srf-info-section.tsx (1)

1-1: Use import type to optimize bundling for imports used solely as types.
Static analysis suggests these imports are only used as types. Replacing them with import type can reduce bundle size if compiled accordingly.

-import { SingleRequestProxyDeployment } from "@/lib/types";
+import type { SingleRequestProxyDeployment } from "@/lib/types";
🧰 Tools
🪛 Biome (1.9.4)

[error] 1-1: All these imports are only used as types.

Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.

(lint/style/useImportType)

src/components/recent-srf-deployments-table.tsx (1)

40-120: Limiting results to 10.
There is a .slice(0, 10) usage that might limit visibility for the user. To expand functionality, consider adding a “Load more” or pagination for a more scalable approach.

src/components/recent-area.tsx (1)

12-14: LGTM with accessibility enhancement suggestion

The grid layout and component integration look good. Consider adding an ARIA label or heading to improve accessibility for this new section.

-      <div className="md:col-span-2 w-full">
+      <div className="md:col-span-2 w-full" aria-label="Recent SRF Deployments">
+        <h2 className="sr-only">Recent SRF Deployments</h2>
         <RecentSRFDeploymentsTable />
src/app/requests/page.tsx (1)

4-7: LGTM with performance optimization suggestion

The type imports and client-side directive are correctly implemented. Consider memoizing the pagination state setter to prevent unnecessary re-renders.

-  const [pagination, setPagination] = useState<PaginationState>({
+  const [pagination, setPagination] = useState<PaginationState>(() => ({
     pageIndex: 0,
     pageSize: 10,
-  });
+  }));
src/lib/types.ts (1)

39-52: Add JSDoc documentation for the interface and its fields.

The interface looks well-structured, but adding documentation would improve maintainability and help other developers understand the purpose of each field.

Consider adding documentation like this:

+/**
+ * Represents a single request proxy deployment configuration
+ */
 export interface SingleRequestProxyDeployment {
+  /** Unique identifier for the deployment */
   id: string;
+  /** Address where fees are sent */
   feeAddress: string;
   // ... (add docs for other fields)
 }
package.json (2)

9-11: Consider adding combined and check-only scripts.

The scripts look good, but consider adding:

  1. A combined script to run both lint and format
  2. A check-only format script that doesn't write changes

Add these scripts:

 "scripts": {
   "lint": "next lint",
   "format": "biome check --write .",
+  "format:check": "biome check .",
+  "check": "npm run lint && npm run format:check",
   "prepare": "husky install"
 }

44-44: Consider version locking for consistent builds.

The new dev dependencies use caret (^) versioning which allows minor version updates. For development tools that affect the build process, consider using exact versions to ensure consistent behavior across all environments.

-    "@biomejs/biome": "^1.9.4",
+    "@biomejs/biome": "1.9.4",
-    "husky": "^9.1.7",
+    "husky": "9.1.7",
-    "lint-staged": "^15.2.10"
+    "lint-staged": "15.2.10"

Also applies to: 51-52

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 51c90e6 and 1c4c293.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (18)
  • .husky/pre-commit (1 hunks)
  • .lintstagedrc.js (1 hunks)
  • biome.json (1 hunks)
  • package.json (2 hunks)
  • src/app/request/[id]/page.tsx (4 hunks)
  • src/app/requests/page.tsx (1 hunks)
  • src/app/single-request-forwarders/page.tsx (1 hunks)
  • src/components/recent-area.tsx (1 hunks)
  • src/components/recent-srf-deployments-table.tsx (1 hunks)
  • src/components/srf-deployments-table.tsx (1 hunks)
  • src/components/srf-info-section.tsx (1 hunks)
  • src/lib/graphQlClient.ts (1 hunks)
  • src/lib/hooks/payments.ts (1 hunks)
  • src/lib/hooks/use-latest-srf-deployments.tsx (1 hunks)
  • src/lib/queries/srf-deployments.ts (1 hunks)
  • src/lib/queries/utils.ts (2 hunks)
  • src/lib/types.ts (1 hunks)
  • src/lib/utils.ts (6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/srf-info-section.tsx

[error] 1-1: All these imports are only used as types.

Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.

(lint/style/useImportType)

🔇 Additional comments (39)
src/lib/queries/utils.ts (2)

3-4: Imports look good.
There are no concerns with these import statements.


29-43: GraphQL fragment naming is consistent and comprehensive.
The chosen fragment name ProxyDeploymentFields clearly reflects its purpose, and the selected fields cover the essential attributes of a SingleRequestProxyDeployment. No issues found.

src/lib/hooks/payments.ts (1)

156-164: Add error handling in fetchPayments function
This concern was raised in a previous review. Wrapping the GraphQL request in a try-catch block prevents unhandled rejections and provides clearer error management.

src/lib/queries/srf-deployments.ts (4)

7-121: Consider optimizing the GraphQL query structure
This recommendation was mentioned before, suggesting reusable fragments or a more compact approach.


9-9: Fix inconsistent parameter requirements
Previously noted: $first is optional while $skip is mandatory. They typically work together for pagination and should be equally required.


123-134: Add error handling and input validation
This function lacks checks for invalid or out-of-range parameters and does not handle network request failures.


197-206: Enhance type safety and error handling
A try-catch pattern and stricter return types can improve robustness of fetchProxyDeploymentsByReference.

src/components/srf-deployments-table.tsx (1)

30-122: Validate numeric conversions for feeAmount.
In lines 90-91, converting feeAmount to a BigInt may throw if the value is not a valid numeric string. Consider safe parsing or extra checks.

const feeAmount = row.getValue("feeAmount") as string;
+if (!/^\d+$/.test(feeAmount)) {
+  // handle invalid numeric string scenario or fallback
+}
return formatUnits(BigInt(feeAmount), 18);
src/app/request/[id]/page.tsx (3)

39-40: Imports are appropriate.
Adding fetchProxyDeploymentsByReference and SRFInfoSection aligns with the new SRF deployment feature.


160-160: Loading condition is seamlessly integrated.
Including isLoadingSRF in the overall loading check ensures consistent user experience.


Line range hint 368-384: Conditional SRF section is clear and unobtrusive.
The optional rendering of SRFInfoSection is a sensible approach, showing SRF data only when available.

src/app/single-request-forwarders/page.tsx (4)

1-2: Use of "use client" is correct.
This ensures the component is properly compiled as a client component in Next.js’ App Router.


8-13: Initialization of pagination state is clear.
The usage of useState<PaginationState>() with pageIndex and pageSize is a good approach for controlling pagination.


14-19: Query parameters align well with pagination state.
It’s good to see that skip is computed as pageIndex * pageSize, ensuring correct offset for pagination. Also, verifying that the calls to useLatestSRFDeployments match the typed parameters is recommended.


20-31: Table integration is straightforward.
Passing pagination and setPagination to <SRFDeploymentsTable> is a sound approach for controlling pagination externally. This approach fosters reusability of the table component.

src/lib/hooks/use-latest-srf-deployments.tsx (4)

1-2: Client directive is appropriate for a hook using Next.js’ App Router.
No issues found.


24-26: Smooth prefetch method.
Exporting an async function prefetch which directly calls fetchProxyDeployments is straightforward. This can be a valuable approach for SSR usage or anticipating queries.


34-40: Usage of router for side-effect is beneficial.
Updating URL params on page change is well-handled. Continue verifying that the new query param logic does not break direct links or push states if the user navigates back.


50-62: Replacing URL query parameters.
Ensure that the updated URL does not cause undesired re-renders or infinite loops. So far, your checks are sound (returning early if page === 0).

src/components/srf-info-section.tsx (2)

8-15: Graceful handling of empty or null deployments.
Returning null early avoids rendering empty UI elements. This is good for cleanliness.


17-95: Comprehensive display of forwarder details.
Link usage, chain identification, token handling, and timestamps are implemented correctly. Including clickable addresses to block explorers and payees is a great user experience.

src/components/recent-srf-deployments-table.tsx (3)

3-30: Clear separation of UI and data fetching through useLatestSRFDeployments.
It’s good that you retrieve the data within this component using the hook. The environment-based poll interval is well structured.


32-38: Skeleton usage.
Returning a skeleton placeholder while loading is a good UX choice, ensuring users see immediate feedback.


36-38: No data fallback.
Displaying “No data” for empty or undefined deployments simplifies the user experience and clarifies the absence of data.

src/lib/utils.ts (9)

3-19: All good on the import statements.
No major concerns with the typed imports and references; the usage of type imports promotes clarity.


37-43: Function looks fine.
The date formatting logic is appropriate for displaying a UTC timestamp with clear date/time info.


48-55: Missing explicit handling for undefined currency details.
You have not incorporated the previous suggestion to add a warning or explicit handling for unusable currency data. The code still quietly falls back to default decimals & symbol.


69-80: Console error usage is acceptable.
The short/long payment references are calculated correctly. Logging the caught error is reasonable, though you may consider a more descriptive or user-facing message.


93-138: Consider transitioning to a typed approach for the chain keys.
Refactoring to a more type-safe approach (e.g., using a typed map or deriving the chain keys dynamically) helps avoid the risk of missing or misspelling a chain key and improves maintainability.


178-180: Great use of BigInt for summing amounts.
Storing and handling priced data with BigInt is a robust approach for financial calculations.


187-192: Address rendering logic is concise.
Joining and filtering truthy parts ensures a clean, human-readable address.


196-196: Utility functions are straightforward.
Both safeTruncateEthAddress and capitalize are neatly implemented and easy to read.

Also applies to: 198-199


201-229: Remove debug logging in production code.
Console logs can inadvertently expose sensitive data and clutter logs. Please remove or transform them into trace-level logs if needed for debugging.

.husky/pre-commit (1)

1-4: Pre-commit hook is well-configured.
Running lint-staged ensures code quality checks are performed automatically before committing.

.lintstagedrc.js (1)

1-4: Configuration looks good.
Applying biome format --write and biome lint to staged files validates code style and correctness on each commit.

src/lib/graphQlClient.ts (1)

3-15: No issues with updated string literal formatting.
Switching to double quotes and preserving environment variable usage is consistent with the linting rules.

Also applies to: 17-17

biome.json (2)

23-31: Review disabled linting rules

The following rules are disabled:

  • noExplicitAny: Could lead to type-safety issues
  • noNonNullAssertion: May hide potential null/undefined errors
  • noForEach: Performance consideration for large datasets

Consider if these rules align with the project's quality standards. If they must be disabled, document the reasoning in the configuration file.


3-7: Verify VCS configuration impact

The VCS configuration is disabled and ignores .gitignore. This could lead to Biome processing files that should be ignored (e.g., build artifacts, node_modules).

Consider enabling VCS integration:

-		"enabled": false,
+		"enabled": true,
-		"useIgnoreFile": false
+		"useIgnoreFile": true
src/lib/types.ts (1)

39-52: Verify type consistency with existing interfaces.

The interface structure looks good, but let's verify the type consistency:

  1. Common fields like id, txHash, chain match types in the Payment interface
  2. Consider making some fields optional if they might not always be available
  3. Consider using more specific types for proxyType instead of string

Let's check the usage of these fields across the codebase:

✅ Verification successful

Let me gather more information about the type consistency.


Type consistency verified, but proxyType can be improved

Based on the verification results:

  1. Common fields like id, txHash, chain, tokenAddress, feeAddress, feeAmount, and timestamp match the types in the Payment interface.
  2. All fields appear to be required in practice as there are no optional field accesses (?.) specifically for SingleRequestProxyDeployment fields.
  3. The proxyType field is defined as string, but no concrete values were found in the codebase, suggesting this might be an area for improvement.

Recommendations:

  • Consider defining a union type for proxyType when the possible values are known
  • Keep all fields required as the current usage suggests they are always present
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of SingleRequestProxyDeployment fields
# to verify type consistency and optional fields

# Check proxyType values
rg -A 1 "proxyType.*=.*" 

# Check if any fields are conditionally accessed
rg -A 1 "deployment\?\.|\?\." 

Length of output: 13834


Script:

#!/bin/bash
# Search for Payment interface definition and other related types
ast-grep --pattern 'interface Payment {
  $$$
}'

# Search for proxyType values being assigned
rg "proxyType: ['\"].*['\"]"

# Search for SingleRequestProxyDeployment usage
rg "SingleRequestProxyDeployment" -A 2

Length of output: 5135

@aimen74 aimen74 marked this pull request as draft January 2, 2025 09:02
@aimen74 aimen74 marked this pull request as ready for review January 2, 2025 11:44
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (8)
src/lib/hooks/payments.ts (3)

12-12: Remove empty comment.

The empty comment # serves no purpose and should be removed.


9-154: Consider reducing query duplication using variables.

The current implementation repeats the same query structure for each network, leading to maintenance challenges. Consider refactoring to use GraphQL aliases and a more dynamic approach.

Here's a suggested approach:

const NETWORK_NAMES = [
  'mainnet', 'arbitrum_one', 'avalanche', 'base', 'bsc', 
  'celo', 'fantom', 'fuse', 'matic', 'moonbeam', 
  'optimism', 'sepolia', 'xdai', 'zksyncera'
] as const;

export const PAYMENTS_QUERY = gql`
  ${CORE_PAYMENT_FIELDS}
  query PaymentsQuery($first: Int, $skip: Int!) {
    ${NETWORK_NAMES.map(network => `
      payment_${network}: payment_${network} {
        payments(
          first: $first
          skip: $skip
          orderBy: timestamp
          orderDirection: desc
        ) {
          ...PaymentFields
        }
      }
    `).join('')}
  }
`;

156-169: Consider enhancing error handling and adding retries.

While the basic error handling is in place, consider these improvements:

  1. Add specific error types for better error handling
  2. Include more context in error logs
  3. Implement retry mechanism for transient failures

Here's a suggested implementation:

type FetchPaymentsError = {
  message: string;
  code: string;
  networks?: string[];
};

export const fetchPayments = async (variables: {
  first: number;
  skip: number;
}): Promise<Payment[]> => {
  const maxRetries = 3;
  let attempt = 0;

  while (attempt < maxRetries) {
    try {
      const data: { [x: string]: { payments: Payment[] } } =
        await graphQLClient.request(PAYMENTS_QUERY, variables);
      return formatPaymentData(data);
    } catch (err) {
      attempt++;
      const error = err as FetchPaymentsError;
      console.error(
        `Error fetching payments (attempt ${attempt}/${maxRetries}):`,
        {
          message: error.message,
          code: error.code,
          networks: error.networks,
          variables
        }
      );
      
      if (attempt === maxRetries) {
        return [];
      }
      // Exponential backoff
      await new Promise(resolve => 
        setTimeout(resolve, Math.pow(2, attempt) * 1000)
      );
    }
  }
  return [];
};
src/lib/hooks/use-latest-srf-deployments.tsx (4)

18-23: Remove unused page parameter from Props interface

The page parameter in the Props interface is defined but not used in the query function. Consider removing it if it's not essential for the component's functionality.

 type Props = {
   first?: number;
   skip?: number;
   pollInterval?: number;
-  page?: number;
 };

38-40: Simplify prefetchedData state type

The type union with null is unnecessary as undefined is already included by the optional type.

 const [prefetchedData, setPrefetchedData] = useState<
-  SingleRequestProxyDeployment[] | null
+  SingleRequestProxyDeployment[]
 >();

51-63: Optimize useEffect dependencies

The current implementation might cause unnecessary re-renders due to searchParams being included in the dependency array. Consider using searchParams.get('page') instead.

 useEffect(() => {
   if (page === 0) {
     return;
   }
   const params = new URLSearchParams(searchParams);
   params.set("page", page.toString());
   replace(`${pathname}?${params.toString()}`);

   (async () => {
     const deployments = await prefetch(first, page * first);
     setPrefetchedData(deployments);
   })();
-}, [page, first, pathname, replace, searchParams]);
+}, [page, first, pathname, replace]);

65-77: Remove type assertion for better type safety

The type assertion can be avoided by properly typing the useMemo hook.

-  const value = useMemo(
+  const value = useMemo<LatestSRFDeployments>(
     () => ({
       deployments: data,
       isLoading,
       status,
       isFetching,
       error,
     }),
     [data, isLoading, status, isFetching, error]
   );

-  return value as LatestSRFDeployments;
+  return value;
src/lib/queries/srf-deployments.ts (1)

23-137: Optimize GraphQL query structure using fragments

The query is repetitive and could be simplified using a network-specific fragment.

+const NETWORK_DEPLOYMENTS_FRAGMENT = gql`
+  fragment NetworkDeployments on Network {
+    singleRequestProxyDeployments(
+      first: $first
+      skip: $skip
+      orderBy: timestamp
+      orderDirection: desc
+    ) {
+      ...ProxyDeploymentFields
+    }
+  }
+`;

 export const PROXY_DEPLOYMENTS_QUERY = gql`
   ${CORE_PROXY_DEPLOYMENT_FIELDS}
+  ${NETWORK_DEPLOYMENTS_FRAGMENT}
   query ProxyDeploymentsQuery($first: Int!, $skip: Int!) {
     payment_mainnet {
-      singleRequestProxyDeployments(
-        first: $first
-        skip: $skip
-        orderBy: timestamp
-        orderDirection: desc
-      ) {
-        ...ProxyDeploymentFields
-      }
+      ...NetworkDeployments
     }
     payment_arbitrum_one {
-      singleRequestProxyDeployments(
-        first: $first
-        skip: $skip
-        orderBy: timestamp
-        orderDirection: desc
-      ) {
-        ...ProxyDeploymentFields
-      }
+      ...NetworkDeployments
     }
     # ... repeat for other networks
   }
`;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c4c293 and b46f7b7.

📒 Files selected for processing (5)
  • src/components/recent-srf-deployments-table.tsx (1 hunks)
  • src/lib/hooks/payments.ts (1 hunks)
  • src/lib/hooks/use-latest-srf-deployments.tsx (1 hunks)
  • src/lib/queries/srf-deployments.ts (1 hunks)
  • src/lib/utils.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/components/recent-srf-deployments-table.tsx
  • src/lib/utils.ts
🔇 Additional comments (3)
src/lib/hooks/payments.ts (2)

1-8: LGTM! Clean and well-organized imports.

All necessary dependencies are properly imported, and types are correctly referenced.


10-10: Verify CORE_PAYMENT_FIELDS fragment definition.

The query depends on the CORE_PAYMENT_FIELDS fragment. Let's verify its definition and fields.

✅ Verification successful

Let me gather more information to see the complete fragment definition and its usage.


Let me verify one more thing to ensure we have the complete picture of how this fragment is being used.


Let me try a different approach to see the usage context.


Let me try one final approach to see the complete context.


CORE_PAYMENT_FIELDS fragment is properly defined and used

The fragment is correctly defined in src/lib/queries/utils.ts as:

fragment PaymentFields on Payment {
  currency
  amount
  amountInCrypto
  block
  contractAddress
  feeAddress
  feeAmount
  feeAmountInCrypto
  from
  gasPrice
}

And it's properly:

  1. Imported from ../queries/utils
  2. Used in the PAYMENTS_QUERY to share common payment fields across multiple chain-specific queries
  3. Referenced correctly in the fragment spread ...PaymentFields
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the CORE_PAYMENT_FIELDS fragment definition
rg -l "CORE_PAYMENT_FIELDS.*=.*gql" 
rg "fragment PaymentFields on" -A 10

Length of output: 560


Script:

#!/bin/bash
# Get the complete CORE_PAYMENT_FIELDS definition and its usage
rg "CORE_PAYMENT_FIELDS" -A 5 src/lib/queries/utils.ts

Length of output: 185


Script:

#!/bin/bash
# Check how CORE_PAYMENT_FIELDS is being used in the payments hook
rg -B 2 -A 5 "\${CORE_PAYMENT_FIELDS}" src/lib/hooks/payments.ts

Length of output: 172


Script:

#!/bin/bash
# Escape the special characters in the pattern
rg -B 2 -A 5 "\\\${CORE_PAYMENT_FIELDS}" src/lib/hooks/payments.ts

Length of output: 175


Script:

#!/bin/bash
# Just show the content of the payments.ts file
cat src/lib/hooks/payments.ts

Length of output: 3353

src/lib/queries/srf-deployments.ts (1)

139-155: 🛠️ Refactor suggestion

Improve type safety in fetchProxyDeployments

The function uses a generic object type for the GraphQL response. Consider using a proper interface for better type safety.

+interface NetworkResponse {
+  singleRequestProxyDeployments: SingleRequestProxyDeployment[];
+}

+interface ProxyDeploymentsResponse {
+  payment_mainnet?: NetworkResponse;
+  payment_arbitrum_one?: NetworkResponse;
+  payment_avalanche?: NetworkResponse;
+  // ... add other networks
+}

 export const fetchProxyDeployments = async (variables: {
   first: number;
   skip: number;
 }): Promise<SingleRequestProxyDeployment[]> => {
   try {
-    const data: {
-      [x: string]: {
-        singleRequestProxyDeployments: SingleRequestProxyDeployment[];
-      };
-    } = await graphQLClient.request(PROXY_DEPLOYMENTS_QUERY, variables);
+    const data = await graphQLClient.request<ProxyDeploymentsResponse>(
+      PROXY_DEPLOYMENTS_QUERY,
+      variables
+    );

     return formatProxyDeploymentData(data);
   } catch (error) {
     console.error("Error fetching proxy deployments:", error);
     throw error;
   }
 };

Likely invalid or redundant comment.

Comment thread src/lib/hooks/use-latest-srf-deployments.tsx
Comment thread src/lib/queries/srf-deployments.ts Outdated
Comment thread src/lib/queries/srf-deployments.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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 (3)
src/lib/queries/srf-deployments.ts (3)

123-139: Enhance type safety for network keys

Consider using a union type for network keys instead of the generic string index type.

+type NetworkKey =
+  | "payment_mainnet"
+  | "payment_arbitrum_one"
+  | "payment_avalanche"
+  | "payment_base"
+  | "payment_bsc"
+  | "payment_celo"
+  | "payment_matic"
+  | "payment_optimism"
+  | "payment_sepolia"
+  | "payment_xdai"
+  | "payment_zksyncera";

 const data: {
-  [x: string]: {
+  [K in NetworkKey]?: {
     singleRequestProxyDeployments: SingleRequestProxyDeployment[];
   };
 }

202-216: Apply consistent type safety improvements

For consistency with the previous function, consider adding type annotation for the GraphQL response.

 export const fetchProxyDeploymentsByReference = async (variables: {
   reference: string;
 }): Promise<SingleRequestProxyDeployment[]> => {
   try {
-    const data = await graphQLClient.request(
+    const data = await graphQLClient.request<{
+      [K in NetworkKey]?: {
+        singleRequestProxyDeployments: SingleRequestProxyDeployment[];
+      };
+    }>(
       PROXY_DEPLOYMENTS_BY_REFERENCE_QUERY,
       variables
     );

1-216: Consider centralizing network list management

The list of supported networks is duplicated across queries. Consider extracting network names into a shared constant to make maintenance easier.

export const SUPPORTED_NETWORKS = [
  'payment_mainnet',
  'payment_arbitrum_one',
  // ... other networks
] as const;

export type NetworkKey = typeof SUPPORTED_NETWORKS[number];
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b46f7b7 and 9691bcc.

📒 Files selected for processing (1)
  • src/lib/queries/srf-deployments.ts (1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/lib/queries/srf-deployments.ts (1)
Learnt from: aimensahnoun
PR: RequestNetwork/request-scan#74
File: src/lib/queries/srf-deployments.ts:218-232
Timestamp: 2025-01-02T11:51:08.073Z
Learning: The user aimensahnoun prefers returning an empty array instead of throwing errors in functions like fetchProxyDeploymentsByReference to avoid crashing the entire app.
🔇 Additional comments (3)
src/lib/queries/srf-deployments.ts (3)

1-6: LGTM! Clean and well-organized imports

All imports are necessary and properly organized.


7-121: Consider optimizing the GraphQL query structure

The query structure is repetitive across networks and could benefit from using a reusable fragment.


141-200: Query structure follows the same pattern as PROXY_DEPLOYMENTS_QUERY

@aimen74 aimen74 merged commit 6cb2e5b into main Jan 2, 2025
@aimen74 aimen74 deleted the 71-add-single-request-forwarder-deployments-to-request-scan branch January 2, 2025 20:58
@MantisClone MantisClone linked an issue Jan 7, 2025 that may be closed by this pull request
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Single Request Forwarder deployments to Request Scan

2 participants