Skip to content
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

[TASK-7977] : feat migrate wallet context to separate hook #625

Merged
merged 12 commits into from
Jan 15, 2025

Conversation

kushagrasarathe
Copy link
Collaborator

@kushagrasarathe kushagrasarathe commented Jan 14, 2025

  • migrated wallet context to separate hook
  • managing wallet states using redux
  • also migrated zero dev context to a separate hook and redux for managing states
  • optmized zerodev method calling to prevent un-necessary rpc calls if address already exists

Summary by CodeRabbit

Based on the comprehensive summary of changes, here are the release notes:

Release Notes

  • Refactoring

    • Restructured wallet and ZeroDev hooks from context-based to a dedicated hooks directory
    • Consolidated wallet and ZeroDev state management using Redux
    • Simplified context provider hierarchy
  • New Features

    • Added Redux slices for wallet and ZeroDev state management
    • Introduced new custom hooks for wallet and ZeroDev functionality
    • Enhanced wallet connection and management capabilities
  • Improvements

    • Improved modularity of wallet and authentication hooks
    • Streamlined import statements across multiple components
    • Updated state management approach for better performance and maintainability
  • Technical Updates

    • Migrated wallet context to a more flexible hook-based implementation
    • Added new selector hooks for accessing wallet and ZeroDev state
    • Implemented comprehensive state types for wallet and ZeroDev features

Copy link

vercel bot commented Jan 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
peanut-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 15, 2025 4:43pm

Copy link

Copy link
Contributor

coderabbitai bot commented Jan 14, 2025

Walkthrough

This pull request involves a comprehensive refactoring of wallet and authentication-related functionality across the application. The primary change is the migration of wallet and ZeroDev context management from a context-based structure to a hooks-based approach. This involves creating new hooks (useWallet and useZeroDev), introducing Redux slices for state management, and updating import paths across multiple components to use the new hooks.

Changes

File Path Change Summary
src/hooks/useWallet.ts New custom hook for wallet management with Redux integration
src/hooks/useZeroDev.ts New custom hook for ZeroDev kernel client management
src/redux/slices/wallet-slice.ts New Redux slice for wallet state management
src/redux/slices/zerodev-slice.ts New Redux slice for ZeroDev state management
src/context/walletContext/* Removed context-based wallet management files
Multiple components Updated import paths for useWallet and useZeroDev

Sequence Diagram

sequenceDiagram
    participant App
    participant useWallet
    participant Redux
    participant WalletProvider
    
    App->>useWallet: Initialize
    useWallet->>Redux: Fetch initial state
    useWallet->>WalletProvider: Get wallet connections
    WalletProvider-->>useWallet: Return wallet data
    useWallet->>Redux: Update wallet state
    App->>useWallet: Select wallet
    useWallet->>Redux: Update selected wallet
Loading

Possibly related PRs

Suggested reviewers

  • Hugo0

Poem

🐰 Wallet hooks hop and dance,
Contexts fade, Redux takes its chance
Refactoring code with glee and might
Hooks bring clarity, future burning bright!
CodeRabbit's magic, clean and neat 🌟

Finishing Touches

  • 📝 Generate Docstrings (Beta)

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.

@kushagrasarathe kushagrasarathe requested a review from Hugo0 January 14, 2025 15:11
Copy link
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

🔭 Outside diff range comments (3)
src/components/Create/useCreateLink.tsx (3)

Line range hint 22-30: Add validation for tokenValue to prevent NaN errors.

In the checkUserHasEnoughBalance function, converting tokenValue to a Number without validation may result in NaN if tokenValue is not a valid numeric string. This can cause unexpected behavior in comparisons.

Consider validating tokenValue before using it:

const amount = Number(tokenValue)
if (isNaN(amount) || amount < 0.000001) {
  throw new Error('The minimum amount to send is 0.000001')
}

Also, remove address from the dependency array since it's not used inside the function.


Line range hint 171-187: Enhance error handling when waiting for transaction receipt.

The current retry logic waits for the transaction receipt with a fixed number of attempts and interval:

for (let attempt = 0; attempt < 3; attempt++) {
  try {
    await waitForTransactionReceipt(config, {
      confirmations: 4,
      hash: hash,
      chainId: Number(selectedChainID),
    })
    break
  } catch (error) {
    if (attempt < 2) {
      await new Promise((resolve) => setTimeout(resolve, 500))
    } else {
      console.error('Failed to wait for transaction receipt after 3 attempts', error)
    }
  }
}

Consider using an exponential backoff strategy or increasing the number of retries and intervals to account for network congestion or delays. Alternatively, you can use a recursive function or a library to handle retries more elegantly.


Line range hint 134-154: Handle fetch errors and provide user feedback in submitClaimLinkInit.

The submitClaimLinkInit function uses fetch to make network requests but doesn't catch exceptions that may occur due to network errors or server issues. To improve reliability and user experience, consider adding error handling for the fetch call and providing user feedback.

try {
  const response = await fetch('/api/peanut/submit-claim-link/init', { /* ... */ })
  if (!response.ok) {
    throw new Error(`HTTP error! status: ${response.status}`)
  }
  const data = await response.json()
  return data
} catch (error) {
  console.error('Failed to submit claim link (init):', error)
  // Provide user feedback here, e.g., show a toast notification
  throw error
}
🧹 Nitpick comments (21)
src/hooks/useZeroDev.ts (2)

123-123: Simplify condition using optional chaining.

Instead of checking both kernelClient and kernelClient.account?.address, you can simplify the condition using optional chaining:

if (kernelClient?.account?.address) {
  dispatch(zerodevActions.setIsKernelClientReady(true))
  dispatch(zerodevActions.setIsRegistering(false))
  dispatch(zerodevActions.setIsLoggingIn(false))
  return
}

This makes the code cleaner and easier to read.

🧰 Tools
🪛 Biome (1.9.4)

[error] 123-123: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


150-152: Consider providing user feedback on kernel client initialization failure.

Currently, when an error occurs during kernel client initialization, it's logged to the console. It might improve user experience to inform the user of this failure, possibly through a toast notification or error state, so they are aware that something went wrong.

src/components/Create/useCreateLink.tsx (3)

Line range hint 31-53: Simplify token amount parsing by removing redundant Number conversion.

In the generateLinkDetails function, the token amount is parsed as:

tokenAmount: parseFloat(Number(tokenValue).toFixed(6)),

The Number conversion is redundant since parseFloat can handle the string conversion. You can simplify the code:

tokenAmount: parseFloat(parseFloat(tokenValue).toFixed(6)),

Or simply:

tokenAmount: Number(parseFloat(tokenValue).toFixed(6)),

Line range hint 80-100: Refactor nested try-catch blocks for better readability.

In the estimateGasFee function, nested try-catch blocks can make the code harder to read and maintain. Consider refactoring the code to use a single try-catch block with fallback logic:

try {
  // Primary attempt
  const feeOptions = await peanut.setFeeOptions({ /* primary options */ })
  // ...
  return { feeOptions, transactionCostUSD }
} catch (primaryError) {
  console.warn('Primary fee estimation failed, attempting fallback:', primaryError)
  // Fallback attempt
  const feeOptions = await peanut.setFeeOptions({ /* fallback options */ })
  // ...
  return { feeOptions, transactionCostUSD }
}

This approach keeps the code flatter and improves readability.


Line range hint 22-30: Remove unused dependency address from useCallback dependencies array.

The address variable is included in the dependencies of checkUserHasEnoughBalance but is not used within the function. Including unnecessary dependencies can cause unwanted re-renders.

src/redux/types/zerodev.types.ts (1)

1-7: Add JSDoc comments to document the interface properties.

While the interface is well-structured, adding JSDoc comments would improve maintainability by documenting:

  • The purpose of each boolean flag
  • When the address might be undefined
  • Any invariants between these states

Example documentation:

+/**
+ * Represents the state of ZeroDev authentication and operations
+ */
 export interface ZeroDevState {
+    /** Indicates if the kernel client is initialized and ready */
     isKernelClientReady: boolean
+    /** Tracks ongoing user registration process */
     isRegistering: boolean
+    /** Tracks ongoing login process */
     isLoggingIn: boolean
+    /** Indicates if a user operation is being processed */
     isSendingUserOp: boolean
+    /** The user's ZeroDev address, available after successful authentication */
     address?: string
 }
src/redux/types/wallet.types.ts (1)

1-9: Consider separating UI and wallet state concerns.

The current interface combines UI-specific state (signInModalVisible) with wallet-specific state (selectedAddress, wallets). Consider splitting this into separate interfaces for better separation of concerns:

/** UI-specific wallet state */
interface WalletUIState {
    signInModalVisible: boolean
    walletColor: string
}

/** Wallet connection state */
interface WalletState {
    selectedAddress?: string
    wallets: IWallet[]
    isConnected: boolean
}

/** Combined state type */
interface CombinedWalletState extends WalletUIState, WalletState {}

Also consider:

  1. Using a union type for walletColor if there's a fixed set of colors
  2. Adding JSDoc comments to document the purpose of each property
src/redux/hooks.ts (1)

9-10: LGTM! Consider adding type annotations for better type safety.

The new selector hooks follow the established pattern and maintain consistency. For better type safety, consider explicitly typing the return values:

-export const useWalletStore = () => useAppSelector((state) => state.wallet)
-export const useZerodevStore = () => useAppSelector((state) => state.zeroDev)
+export const useWalletStore = () => useAppSelector((state): RootState['wallet'] => state.wallet)
+export const useZerodevStore = () => useAppSelector((state): RootState['zeroDev'] => state.zeroDev)
src/app/(setup)/layout.tsx (1)

13-31: Consider adding a loading state to prevent content flash.

During the authentication check and redirection, there might be a brief flash of the setup content. Consider adding a loading state:

 const SetupLayout = ({ children }: { children?: React.ReactNode }) => {
     const { user } = useAuth()
     const router = useRouter()
     const dispatch = useAppDispatch()
     const isPWA = usePWAStatus()
+    const [isLoading, setIsLoading] = useState(true)

     useEffect(() => {
         // if user is logged in, redirect to home
         if (user) {
             router.push('/home')
             return
         }

         // filter steps and set them in redux state
         const filteredSteps = setupSteps.filter((step) => {
             return step.screenId !== 'pwa-install' || !isPWA
         })
         dispatch(setupActions.setSteps(filteredSteps))
+        setIsLoading(false)
     }, [dispatch, isPWA, user])

-    return <>{children}</>
+    if (isLoading) {
+        return null // or a loading spinner
+    }
+    return <>{children}</>
 }
src/redux/slices/zerodev-slice.ts (1)

13-36: Add type annotations to action payloads.

While the slice implementation is correct, consider adding explicit type annotations to improve type safety:

 reducers: {
-    setIsKernelClientReady(state, action: PayloadAction<boolean>) {
+    setIsKernelClientReady: (state: ZeroDevState, action: PayloadAction<boolean>) => {
         state.isKernelClientReady = action.payload
     },
-    setIsRegistering(state, action: PayloadAction<boolean>) {
+    setIsRegistering: (state: ZeroDevState, action: PayloadAction<boolean>) => {
         state.isRegistering = action.payload
     },
     // Apply similar changes to other reducers
 }
src/redux/slices/wallet-slice.ts (2)

18-23: Consider moving user preferences update to middleware or thunk.

The reducer contains a side effect (updating user preferences). Consider moving this to Redux middleware or a thunk to maintain reducer purity.

// Example thunk implementation
export const updateSelectedAddress = (address: string) => (dispatch) => {
    updateUserPreferences({
        lastSelectedWallet: { address },
    });
    dispatch(walletActions.setSelectedAddress(address));
};

36-46: Optimize wallet balance update logic.

Consider using array map for better readability and performance:

-updateWalletBalance: (state, action) => {
-    const { address, balance, balances } = action.payload
-    const walletIndex = state.wallets.findIndex((w) => w.address === address)
-    if (walletIndex !== -1) {
-        state.wallets[walletIndex] = {
-            ...state.wallets[walletIndex],
-            balance,
-            balances,
-        }
-    }
-},
+updateWalletBalance: (state, action) => {
+    const { address, balance, balances } = action.payload
+    state.wallets = state.wallets.map(wallet => 
+        wallet.address === address 
+            ? { ...wallet, balance, balances }
+            : wallet
+    )
+},
src/components/Setup/Views/SetupPasskey.tsx (1)

Line range hint 15-46: Enhance error messages for better user experience.

Consider providing more descriptive error messages to help users understand and resolve issues:

-.catch((e) => {
-    console.error('Error adding account', e)
-    setError('Error adding account')
+.catch((e: Error) => {
+    console.error('Error adding account:', e)
+    setError(`Failed to add account: ${e.message || 'Unknown error'}`)
 })
src/components/Home/HomeHeader.tsx (1)

1-5: Consider organizing imports for better maintainability.

Consider grouping related imports together:

  1. External dependencies
  2. Internal components
  3. Internal hooks
  4. Internal utilities
-import { useToast } from '@/components/0_Bruddle/Toast'
-import { useAuth } from '@/context/authContext'
-import useAvatar from '@/hooks/useAvatar'
-import { useWallet } from '@/hooks/useWallet'
-import { useZeroDev } from '@/hooks/useZeroDev'
+// External dependencies
+
+// Internal components
+import { useToast } from '@/components/0_Bruddle/Toast'
+
+// Internal hooks
+import { useAuth } from '@/context/authContext'
+import useAvatar from '@/hooks/useAvatar'
+import { useWallet } from '@/hooks/useWallet'
+import { useZeroDev } from '@/hooks/useZeroDev'
src/components/Profile/Components/ProfileWalletBalance.tsx (1)

Line range hint 28-36: Address TODO comment about chain image.

The TODO comment indicates missing functionality for dynamic chain image loading.

Would you like me to help implement a solution for dynamically loading chain images? I can:

  1. Create a utility function to map chain IDs to their respective images
  2. Add chain icons to the assets directory
  3. Open a GitHub issue to track this enhancement
src/app/(mobile-ui)/layout.tsx (1)

Line range hint 89-91: Fix typo in link text.

There's a typo in the "Don't have a peanut wallet?" link text.

-                        Don't have a penanut wallet? Get one now.
+                        Don't have a peanut wallet? Get one now.
src/components/Create/Create.tsx (1)

Line range hint 73-84: Consider implementing error handling and retry logic for API calls.

The fetchAndSetCrossChainDetails function could benefit from improved error handling and retry logic for better resilience.

 const fetchAndSetCrossChainDetails = async () => {
+    const MAX_RETRIES = 3;
+    let retries = 0;
+    
+    while (retries < MAX_RETRIES) {
         try {
             const response = await fetch('https://apiplus.squidrouter.com/v2/chains', {
                 headers: {
                     'x-integrator-id': '11CBA45B-5EE9-4331-B146-48CCD7ED4C7C',
                 },
             })
             if (!response.ok) {
                 throw new Error('Squid: Network response was not ok')
             }
             const data = await response.json()
             setCrossChainDetails(data.chains)
+            return;
         } catch (error) {
+            retries++;
+            if (retries === MAX_RETRIES) {
+                console.error('Failed to fetch cross-chain details:', error);
+                setCrossChainDetails([]);
+            }
+            // Exponential backoff
+            await new Promise(resolve => setTimeout(resolve, Math.pow(2, retries) * 1000));
         }
+    }
 }
src/components/Claim/Claim.tsx (1)

Line range hint 89-124: Consider implementing rate limiting for token price fetching.

The checkLink function fetches token prices without rate limiting, which could lead to API rate limits being exceeded when processing multiple links.

+const RATE_LIMIT_DELAY = 1000; // 1 second delay between requests
+
+const fetchTokenPriceWithRateLimit = async (tokenAddress: string, chainId: string) => {
+    await new Promise(resolve => setTimeout(resolve, RATE_LIMIT_DELAY));
+    return utils.fetchTokenPrice(tokenAddress.toLowerCase(), chainId);
+};
+
 const checkLink = async (link: string) => {
     try {
         const linkDetails: interfaces.ILinkDetails = await peanut.getLinkDetails({
             link,
         })
         // ... other code ...
-        const tokenPrice = await utils.fetchTokenPrice(linkDetails.tokenAddress.toLowerCase(), linkDetails.chainId)
+        const tokenPrice = await fetchTokenPriceWithRateLimit(linkDetails.tokenAddress, linkDetails.chainId)
         tokenPrice && setTokenPrice(tokenPrice?.price)
         // ... rest of the code ...
     } catch (error) {
         setLinkState(_consts.claimLinkStateType.NOT_FOUND)
     }
 }
🧰 Tools
🪛 Biome (1.9.4)

[error] 18-18: Unexpected empty object pattern.

(lint/correctness/noEmptyPattern)

src/components/Dashboard/index.tsx (1)

2-2: LGTM with a minor suggestion for the CSV download feature.

The migration to the new useWallet hook and the addition of the CSV download feature are well implemented.

Consider adding a timestamp to the CSV filename to help users distinguish between multiple downloads:

-        filename="links.csv"
+        filename={`links_${new Date().toISOString().split('T')[0]}.csv`}

Also applies to: 4-4, 5-5, 7-7, 12-12, 16-16

src/components/Create/Link/Input.view.tsx (1)

16-16: LGTM with a suggestion for error handling.

The migration to the new hooks is correctly implemented and used consistently throughout the component.

Consider adding more specific error messages for different failure scenarios in the handleOnNext function. For example:

         } catch (error) {
             const errorString = ErrorHandler(error)
+            let errorMessage = errorString
+            if (error instanceof Error) {
+                if (error.message.includes('insufficient balance')) {
+                    errorMessage = 'You do not have enough balance to complete this transaction'
+                } else if (error.message.includes('user rejected')) {
+                    errorMessage = 'Transaction was rejected. Please try again'
+                }
+            }
             setErrorState({
                 showError: true,
-                errorMessage: errorString
+                errorMessage: errorMessage
             })

Also applies to: 17-17, 18-18

src/components/Request/Pay/Views/Initial.view.tsx (1)

Line range hint 365-385: Points estimation feature needs implementation.

The TODO comment indicates missing points estimation functionality. This could affect user experience as points are not being calculated correctly.

Would you like me to help implement the points estimation feature or create a GitHub issue to track this task?

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4c94823 and 2eab8cb.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (47)
  • src/app/(mobile-ui)/history/page.tsx (1 hunks)
  • src/app/(mobile-ui)/home/page.tsx (2 hunks)
  • src/app/(mobile-ui)/layout.tsx (1 hunks)
  • src/app/(mobile-ui)/wallet/page.tsx (1 hunks)
  • src/app/(setup)/layout.tsx (1 hunks)
  • src/components/Cashout/Components/Initial.view.tsx (1 hunks)
  • src/components/Claim/Claim.tsx (1 hunks)
  • src/components/Claim/Generic/SenderClaim.view.tsx (1 hunks)
  • src/components/Claim/Link/Initial.view.tsx (1 hunks)
  • src/components/Claim/Link/Onchain/Confirm.view.tsx (1 hunks)
  • src/components/Claim/Link/Onchain/Success.view.tsx (1 hunks)
  • src/components/Claim/useClaimLink.tsx (1 hunks)
  • src/components/Create/Create.tsx (1 hunks)
  • src/components/Create/Link/Confirm.view.tsx (1 hunks)
  • src/components/Create/Link/Input.view.tsx (1 hunks)
  • src/components/Create/useCreateLink.tsx (1 hunks)
  • src/components/Dashboard/index.tsx (1 hunks)
  • src/components/Global/ChainSelector/index.tsx (1 hunks)
  • src/components/Global/Header/index.tsx (1 hunks)
  • src/components/Global/TokenSelector/TokenSelector.tsx (1 hunks)
  • src/components/Global/WalletHeader/index.tsx (1 hunks)
  • src/components/Home/HomeHeader.tsx (1 hunks)
  • src/components/Home/HomeWaitlist.tsx (1 hunks)
  • src/components/Home/WalletToggleButton.tsx (1 hunks)
  • src/components/Profile/Components/ProfileWalletBalance.tsx (1 hunks)
  • src/components/Profile/Components/SkeletonPage.tsx (1 hunks)
  • src/components/Profile/index.tsx (1 hunks)
  • src/components/Refund/index.tsx (1 hunks)
  • src/components/Request/Create/Create.tsx (1 hunks)
  • src/components/Request/Create/Views/Initial.view.tsx (1 hunks)
  • src/components/Request/Pay/Views/Initial.view.tsx (1 hunks)
  • src/components/Setup/Views/SetupPasskey.tsx (1 hunks)
  • src/components/Setup/Views/Welcome.tsx (1 hunks)
  • src/context/contextProvider.tsx (1 hunks)
  • src/context/walletContext/index.ts (0 hunks)
  • src/context/walletContext/walletContext.tsx (0 hunks)
  • src/context/walletContext/zeroDevContext.context.tsx (0 hunks)
  • src/hooks/useWallet.ts (1 hunks)
  • src/hooks/useWalletType.tsx (1 hunks)
  • src/hooks/useZeroDev.ts (1 hunks)
  • src/redux/constants/index.ts (1 hunks)
  • src/redux/hooks.ts (1 hunks)
  • src/redux/slices/wallet-slice.ts (1 hunks)
  • src/redux/slices/zerodev-slice.ts (1 hunks)
  • src/redux/store.ts (1 hunks)
  • src/redux/types/wallet.types.ts (1 hunks)
  • src/redux/types/zerodev.types.ts (1 hunks)
💤 Files with no reviewable changes (3)
  • src/context/walletContext/index.ts
  • src/context/walletContext/walletContext.tsx
  • src/context/walletContext/zeroDevContext.context.tsx
✅ Files skipped from review due to trivial changes (11)
  • src/components/Global/TokenSelector/TokenSelector.tsx
  • src/components/Create/Link/Confirm.view.tsx
  • src/components/Claim/Link/Onchain/Confirm.view.tsx
  • src/components/Claim/Link/Initial.view.tsx
  • src/components/Request/Create/Create.tsx
  • src/components/Global/ChainSelector/index.tsx
  • src/hooks/useWalletType.tsx
  • src/app/(mobile-ui)/history/page.tsx
  • src/components/Cashout/Components/Initial.view.tsx
  • src/components/Claim/Generic/SenderClaim.view.tsx
  • src/components/Claim/useClaimLink.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Request/Create/Views/Initial.view.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.
🪛 Biome (1.9.4)
src/hooks/useZeroDev.ts

[error] 123-123: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (27)
src/hooks/useWallet.ts (1)

120-122: Simplify logic by utilizing optional chaining.

When accessing the address property of the wallet, you can use optional chaining to handle cases where wallet might be undefined.

return wallet ? { ...wallet, connected: isWalletConnected(wallet) } : undefined

However, since you're already checking if wallet exists, this is acceptable. Ensure that the wallets array and selectedAddress are always in sync to prevent any potential issues.

src/redux/constants/index.ts (1)

2-3: LGTM! Constants follow Redux naming conventions.

The new slice constants follow the established naming pattern and Redux conventions.

src/redux/store.ts (1)

3-4: Verify store type definitions and consider performance optimization.

The store configuration looks good. Consider these enhancements:

  1. Verify that the Redux state type is properly exported:

  2. Consider enabling Redux Toolkit's serializable state checks in development:

 const store = configureStore({
     reducer: {
         setup: setupReducer,
         wallet: walletReducer,
         zeroDev: zeroDevReducer,
     },
+    middleware: (getDefaultMiddleware) =>
+        getDefaultMiddleware({
+            serializableCheck: {
+                // Ignore these action types
+                ignoredActions: ['your-non-serializable-action-type'],
+            },
+        }),
 })

Also applies to: 9-10

✅ Verification successful

Type definitions are properly configured

The Redux store's type system is correctly implemented:

  • RootState is properly defined and exported
  • TypedUseSelectorHook is correctly configured
  • Custom hooks provide type-safe access to the store
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if RootState type is exported and used consistently
rg -l "RootState" src/

Length of output: 66


Script:

#!/bin/bash
# Check RootState definition and usage
echo "=== RootState Definition ==="
rg "RootState" src/redux/types/index.ts -A 2

echo -e "\n=== RootState Usage in Hooks ==="
rg "RootState" src/redux/hooks.ts -A 2

Length of output: 560

src/context/contextProvider.tsx (1)

12-14: Verify complete migration of context dependencies.

The removal of ZeroDevProvider and WalletProvider aligns with the migration to Redux. However, we should verify that all components have been updated to use the new hooks instead of the removed contexts.

✅ Verification successful

Context migration successfully verified

The removal of ZeroDevProvider and WalletProvider is safe as there are no remaining dependencies on these contexts or their hooks in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to the removed contexts
echo "Checking for remaining WalletContext usage..."
rg -l "WalletContext|useWalletContext" --type ts --type tsx

echo "Checking for remaining ZeroDevContext usage..."
rg -l "ZeroDevContext|useZeroDevContext" --type ts --type tsx

echo "Checking for direct imports from removed context files..."
rg -l "from ['\"]\@?/context/(wallet|zeroDev)Context" --type ts --type tsx

Length of output: 709


Script:

#!/bin/bash
# Search for context and hook usages with corrected syntax
echo "Checking WalletContext usage..."
rg "WalletContext" 

echo "Checking useWalletContext hook usage..."
rg "useWalletContext"

echo "Checking ZeroDevContext usage..."
rg "ZeroDevContext"

echo "Checking useZeroDevContext hook usage..."
rg "useZeroDevContext"

echo "Checking context file imports..."
rg "from ['\"].*context/(wallet|zeroDev)"

Length of output: 532

src/components/Home/WalletToggleButton.tsx (1)

1-1: LGTM! Import path updated correctly.

The import path has been updated to use the new hook location while preserving the component's functionality.

src/app/(setup)/layout.tsx (1)

19-23: LGTM! Authentication check properly implemented.

The early return pattern is well-implemented, ensuring authenticated users are redirected to home before processing setup steps.

src/components/Setup/Views/Welcome.tsx (1)

4-4: LGTM! Import path updated correctly.

The import path change aligns with the PR's objective of migrating from context to hooks.

src/redux/slices/zerodev-slice.ts (1)

5-11: LGTM! Well-structured initial state.

The state structure is clear and comprehensive, with appropriate boolean flags for tracking different states of the ZeroDev operations.

src/components/Setup/Views/SetupPasskey.tsx (1)

4-4: LGTM! Import changes are correct.

The import changes align with the PR's objective of migrating to hooks.

Also applies to: 7-7

src/components/Profile/Components/ProfileWalletBalance.tsx (1)

3-3: LGTM! Import path updated correctly.

The migration from context to hook is consistent with the PR objectives.

src/app/(mobile-ui)/layout.tsx (1)

10-11: LGTM! Import paths updated correctly.

The migration from context to hooks is consistent with the PR objectives.

src/components/Home/HomeWaitlist.tsx (2)

4-10: LGTM! Import organization improved.

The imports have been reorganized and the migration from context to hooks is consistent with the PR objectives.


Line range hint 71-98: Verify the relocation of register/login components.

The register/login components have been removed with a comment indicating they should be on a different page.

Let's verify that these components exist in the setup page:

✅ Verification successful

Components successfully relocated to setup flow

The register/login components have been properly moved to src/components/Setup/Views/Welcome.tsx with enhanced functionality:

  • Sign up (formerly Register) button initiates the setup flow
  • Login button maintains loading state and error handling
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify register/login components in setup page
# Expected: Find Button components with "Register" and "Login" text in setup page

# Search for Register/Login buttons in setup page
rg -A 5 'Register|Login' 'src/**/setup/**'

# Search for handleLogin usage in setup page
ast-grep --pattern 'handleLogin()' 'src/**/setup/**'

Length of output: 218


Script:

#!/bin/bash
# First, find setup-related files
echo "=== Setup-related files ==="
fd -t f setup

# Then search for Register and Login buttons in all TypeScript/JavaScript files
echo -e "\n=== Register/Login components ==="
rg -l "Register|Login" "src/**/*.{tsx,jsx,ts,js}"

# Search for handleLogin usage
echo -e "\n=== handleLogin usage ==="
ast-grep --pattern 'handleLogin()'

Length of output: 1380


Script:

#!/bin/bash
# Search for Register/Login in setup components with correct path
echo "=== Register/Login in Setup components ==="
rg -A 5 "Register|Login" src/components/Setup/

# Look at Welcome.tsx content as it uses handleLogin
echo -e "\n=== Welcome.tsx content ==="
cat src/components/Setup/Views/Welcome.tsx

Length of output: 3568

src/components/Claim/Link/Onchain/Success.view.tsx (2)

1-1: LGTM! Import statements are properly organized.

The migration from wallet context to the new useWallet hook is correctly implemented.

Also applies to: 3-3, 5-6


Line range hint 33-38: Verify cross-chain transaction handling.

The component correctly handles cross-chain transactions by displaying source chain, Axelar routing, and destination chain details. However, let's verify that the transaction details are properly displayed for both regular and cross-chain transactions.

✅ Verification successful

Cross-chain transaction handling is properly implemented

The component correctly handles both regular and cross-chain transactions:

  • Regular transactions display a single transaction hash with explorer link
  • Cross-chain transactions show the complete flow:
    • Source chain transaction
    • Axelar routing transaction
    • Destination chain transaction (with loading state)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of transaction details display logic
# Test: Search for components that handle transaction details display
rg -A 5 "transactionHash.*type.*=.*'claimxchain'" .

Length of output: 564


Script:

#!/bin/bash
# Get the complete Success.view.tsx implementation
echo "=== Success.view.tsx content ==="
cat ./src/components/Claim/Link/Onchain/Success.view.tsx

echo -e "\n=== fetchDestinationChain implementation ==="
# Search for fetchDestinationChain function
ast-grep --pattern 'const fetchDestinationChain = $_'

echo -e "\n=== Other transaction type handling ==="
# Search for transaction type handling
rg "type.*===.*'" ./src/components/Claim/Link/Onchain/

Length of output: 6676

src/components/Create/Create.tsx (1)

7-7: LGTM! Wallet hook migration is properly implemented.

The migration from wallet context to the new useWallet hook is correctly implemented.

src/components/Global/WalletHeader/index.tsx (2)

7-7: LGTM! Wallet hook migration is properly implemented.

The migration from wallet context to the new useWallet hook is correctly implemented.


Line range hint 29-38: Verify wallet selection handling.

The component handles wallet selection correctly, but let's verify that the wallet selection is properly synchronized between the new hook and wagmi's useAccount.

✅ Verification successful

Wallet selection is properly synchronized

The wallet selection handling in the WalletHeader component is correctly implemented. The component properly combines wagmi's connection state with the local wallet selection state, maintaining a clear separation of concerns that prevents synchronization issues.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify wallet selection handling
# Test: Search for components that handle wallet selection
rg -A 5 "setSelectedWallet.*wallet" .

Length of output: 3237


Script:

#!/bin/bash
# Check useWallet hook implementation and its interaction with useAccount
ast-grep --pattern 'export function useWallet() {
  $$$
}'

# Search for useAccount usage
rg -A 5 "useAccount\(" .

Length of output: 3763


Script:

#!/bin/bash
# Check wallet store implementation
rg -A 10 "createSlice.*wallet" .

# Check for any connection handlers
rg -A 5 "connect.*wallet" .

Length of output: 5826

src/components/Claim/Claim.tsx (1)

3-16: LGTM! Import statements are properly organized.

The migration from wallet context to the new useWallet hook is correctly implemented, and imports are well-organized.

src/components/Profile/index.tsx (1)

5-5: LGTM! Import path updated correctly.

The import statement has been properly updated to use the new hook-based implementation.

src/app/(mobile-ui)/home/page.tsx (1)

9-10: LGTM! Import paths updated correctly.

Both useWallet and useZeroDev imports have been properly updated to use the new hook-based implementations.

src/components/Refund/index.tsx (1)

8-8: LGTM! Import path updated correctly.

The useWallet import has been properly updated to use the new hook-based implementation, maintaining all existing refund functionality.

src/components/Request/Create/Views/Initial.view.tsx (1)

1-16: LGTM! Imports reorganized and updated correctly.

The imports have been properly restructured:

  1. UI components consolidated from Bruddle
  2. Wallet hook updated to new implementation
  3. Interfaces and utilities properly organized

This improves code organization while maintaining functionality.

src/components/Profile/Components/SkeletonPage.tsx (1)

5-5: LGTM!

The migration to the new useWallet hook is correctly implemented and used consistently throughout the component.

src/components/Request/Pay/Views/Initial.view.tsx (1)

12-13: LGTM! Import changes align with the PR objective.

The migration of hooks from context to a dedicated hooks directory improves code organization and maintainability.

src/app/(mobile-ui)/wallet/page.tsx (2)

3-3: LGTM! Import organization enhances code readability.

The changes improve code organization by:

  1. Moving the useWallet hook to the hooks directory
  2. Grouping related imports together

Also applies to: 8-8, 11-12


8-8: Verify Redux implementation for wallet state management.

While the hook migration is complete, let's verify the Redux implementation mentioned in the PR objectives.

✅ Verification successful

Redux implementation for wallet state management is complete

The wallet state management has been properly implemented using Redux, with:

  • Redux slice with proper state and reducers
  • Type-safe Redux hooks integration in useWallet
  • Proper store configuration and actions setup
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Redux store configuration and wallet-related reducers
echo "Searching for Redux store setup..."
rg -l "configureStore|createSlice" src/

echo "Searching for wallet-related Redux state management..."
rg -l "walletSlice|useWallet.*createSlice" src/

Length of output: 440


Script:

#!/bin/bash
# Check store configuration and wallet hook implementation
echo "=== Store Configuration ==="
rg "wallet.*reducer" src/redux/store.ts -A 2

echo "=== Wallet Slice Content ==="
rg "createSlice|initialState|reducers" src/redux/slices/wallet-slice.ts -A 5

echo "=== useWallet Hook Implementation ==="
rg "useSelector|useDispatch" src/hooks/useWallet.ts

Length of output: 1069


Script:

#!/bin/bash
# Find useWallet hook file and check its implementation
echo "=== Finding useWallet hook file ==="
fd useWallet src/hooks/

echo "=== Checking hook implementation ==="
rg "useSelector|useDispatch" --type ts --type tsx src/hooks/useWallet*

Length of output: 371


Script:

#!/bin/bash
echo "=== Checking useWallet.ts ==="
rg "useSelector|useDispatch|useAppSelector|useAppDispatch" src/hooks/useWallet.ts -A 2

echo "=== Checking useWalletType.tsx ==="
rg "useSelector|useDispatch|useAppSelector|useAppDispatch" src/hooks/useWalletType.tsx -A 2

Length of output: 660

src/hooks/useWallet.ts Outdated Show resolved Hide resolved
src/hooks/useWallet.ts Outdated Show resolved Hide resolved
src/components/Global/Header/index.tsx Show resolved Hide resolved
Copy link
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/hooks/useWallet.ts (5)

41-61: Add error handling for wallet icon generation.

While the implementation is solid, consider adding error handling for the avatar generation to prevent potential runtime errors.

 const getWalletIcon = useCallback(
     (address: string) => {
         if (connector?.icon) return connector.icon
-
-        return createAvatar(identicon, { seed: address.toLowerCase(), size: 128 }).toDataUri()
+        try {
+            return createAvatar(identicon, { seed: address.toLowerCase(), size: 128 }).toDataUri()
+        } catch (error) {
+            console.error('Error generating wallet icon:', error)
+            return undefined
+        }
     },
     [connector]
 )

118-138: Add validation for wallet address format.

Consider adding address format validation before using it for wallet selection and color generation.

 const selectedWallet = useMemo(() => {
     if (!selectedAddress || !wallets.length) return undefined
+    try {
+        // Validate address format
+        const normalizedAddress = getAddress(selectedAddress)
+        const wallet = wallets.find((w) => areEvmAddressesEqual(w.address, normalizedAddress))
-        const wallet = wallets.find((w) => w.address === selectedAddress)
         return wallet ? { ...wallet, connected: isWalletConnected(wallet) } : undefined
+    } catch (error) {
+        console.error('Invalid wallet address format:', error)
+        return undefined
+    }
 }, [selectedAddress, wallets, isWalletConnected])

139-159: Consider adding rate limiting for external wallet additions.

To prevent potential abuse, consider implementing rate limiting when adding multiple external wallets.

+const MAX_CONCURRENT_ADDITIONS = 3
+const ADDITION_DELAY_MS = 1000

 useEffect(() => {
     if (!user || !wagmiAddresses || !wallets.length) return

     const newAddresses = wagmiAddresses.filter(
         (addr) => !wallets.some((wallet) => areEvmAddressesEqual(addr, wallet.address))
     )
     if (newAddresses.length) {
-        newAddresses.forEach((address) => {
+        // Process only a limited number of additions at once
+        newAddresses.slice(0, MAX_CONCURRENT_ADDITIONS).forEach(async (address, index) => {
+            // Add delay between additions
+            await new Promise(resolve => setTimeout(resolve, index * ADDITION_DELAY_MS))
             addAccount({
                 accountIdentifier: address,
                 accountType: interfaces.WalletProviderType.BYOW,
                 userId: user.user.userId,
             }).catch((error) => {
                 const errorMsg = error.message.includes('Account already exists')
                     ? 'Could not add external wallet, already associated with another account'
                     : 'Unexpected error adding external wallet'
                 toast.error(errorMsg)
             })
         })
     }
 }, [wagmiAddresses, wallets, user, addAccount, toast])

161-190: Add debouncing to balance refetching.

To prevent excessive RPC calls, consider implementing debouncing for the balance refetch operation.

+import debounce from 'lodash/debounce'

 const refetchBalances = useCallback(
-    async (address: string) => {
+    debounce(async (address: string) => {
         const wallet = wallets.find((w) => w.address === address)
         if (!wallet) return
         // ... rest of the implementation
-    },
+    }, 1000),  // 1 second delay
     [wallets, dispatch]
 )

192-196: Add validation to external wallet selection.

Consider adding validation to ensure the selected wallet address is valid and active.

 const selectExternalWallet = useCallback(() => {
-    if (wagmiAddresses?.length) {
-        dispatch(walletActions.setSelectedAddress(wagmiAddresses[0]))
+    if (!wagmiAddresses?.length) return
+    
+    try {
+        const address = getAddress(wagmiAddresses[0])
+        const isActive = wallets.some(w => 
+            areEvmAddressesEqual(w.address, address) && w.connected
+        )
+        
+        if (isActive) {
+            dispatch(walletActions.setSelectedAddress(address))
+        }
+    } catch (error) {
+        console.error('Invalid external wallet address:', error)
     }
 }, [wagmiAddresses, dispatch])
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2eab8cb and e208f46.

📒 Files selected for processing (3)
  • src/components/0_Bruddle/Toast.tsx (2 hunks)
  • src/components/Global/Header/index.tsx (2 hunks)
  • src/hooks/useWallet.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/components/0_Bruddle/Toast.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Global/Header/index.tsx
🔇 Additional comments (4)
src/hooks/useWallet.ts (4)

1-31: LGTM! Well-structured imports and utility functions.

The imports are comprehensive and the utility functions are well-defined with clear single responsibilities.


32-40: LGTM! Clean hook initialization with proper state management.

The hook follows React hooks best practices and correctly integrates with Redux for state management.


93-94: Ensure consistent use of token decimals when parsing balances.

The balance parsing uses inconsistent decimal values between lines 93 and 107.


198-218: LGTM! Well-structured hook interface.

The hook returns a comprehensive and well-organized interface for wallet management.

Comment on lines 63 to 116
useQuery({
queryKey: ['wallets', user?.accounts, wagmiAddresses],
queryFn: async () => {
if (!user && !wagmiAddresses) return []

const processedWallets = user
? await Promise.all(
user.accounts.map(async (account) => {
const dbWallet: interfaces.IDBWallet = {
walletProviderType: account.account_type as unknown as interfaces.WalletProviderType,
protocolType: interfaces.WalletProtocolType.EVM,
address: account.account_identifier,
walletIcon: getWalletIcon(account.account_identifier),
}

let balance = BigInt(0)
let balances: interfaces.IUserBalance[] | undefined

if (isPeanut(dbWallet)) {
balance = await peanutPublicClient.readContract({
address: PEANUT_WALLET_TOKEN,
abi: erc20Abi,
functionName: 'balanceOf',
args: [getAddress(dbWallet.address)],
})
} else {
const { balances: fetchedBalances, totalBalance } = await fetchWalletBalances(
dbWallet.address
)
balances = fetchedBalances
balance = parseUnits(totalBalance.toString(), 6)
}

return { ...dbWallet, balance, balances, connected: isWalletConnected(dbWallet) }
})
)
: wagmiAddresses
? await Promise.all(
wagmiAddresses.map(async (address) => {
const { balances, totalBalance } = await fetchWalletBalances(address)
return {
...createDefaultDBWallet(address),
connected: isWalletConnected(createDefaultDBWallet(address)),
balances,
balance: parseUnits(totalBalance.toString(), PEANUT_WALLET_TOKEN_DECIMALS),
}
})
)
: []

dispatch(walletActions.setWallets(processedWallets))
return processedWallets
},
})
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling in wallet data fetching.

The query function should handle potential errors during wallet data fetching and balance retrieval.

 useQuery({
     queryKey: ['wallets', user?.accounts, wagmiAddresses],
     queryFn: async () => {
         if (!user && !wagmiAddresses) return []
+        try {
             const processedWallets = user
                 ? await Promise.all(
                       user.accounts.map(async (account) => {
+                        try {
                             const dbWallet: interfaces.IDBWallet = {
                                 walletProviderType: account.account_type as unknown as interfaces.WalletProviderType,
                                 protocolType: interfaces.WalletProtocolType.EVM,
                                 address: account.account_identifier,
                                 walletIcon: getWalletIcon(account.account_identifier),
                             }
                             // ... rest of the wallet processing
                             return { ...dbWallet, balance, balances, connected: isWalletConnected(dbWallet) }
+                        } catch (error) {
+                            console.error(`Error processing wallet ${account.account_identifier}:`, error)
+                            return undefined
+                        }
                       })
                   ).then(wallets => wallets.filter(Boolean))
                 : // ... rest of the logic
+        } catch (error) {
+            console.error('Error fetching wallet data:', error)
+            return []
+        }
     },
+    retry: 3,
+    retryDelay: 1000,
 })

Committable suggestion skipped: line range outside the PR's diff.

src/hooks/useWallet.ts Show resolved Hide resolved
Copy link
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 (2)
src/hooks/useWallet.ts (2)

140-140: Simplify conditional check using optional chaining

The condition connector && connector.icon can be simplified using optional chaining for better readability:

- connector && connector.icon
+ connector?.icon
🧰 Tools
🪛 Biome (1.9.4)

[error] 140-140: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🪛 GitHub Actions: Code Formatting

[warning] Code style issues found. Run 'pnpm format' to fix formatting issues.


146-151: Improve error handling by checking specific error codes

When adding accounts, the error handling relies on checking if the error message includes 'Account already exists'. This approach can be fragile if the error messages change. If the addAccount function provides specific error codes or exception types, consider using them for more robust and maintainable error handling.

🧰 Tools
🪛 GitHub Actions: Code Formatting

[warning] Code style issues found. Run 'pnpm format' to fix formatting issues.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e208f46 and d6c804b.

📒 Files selected for processing (2)
  • src/components/Global/WalletHeader/index.tsx (1 hunks)
  • src/hooks/useWallet.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Global/WalletHeader/index.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/hooks/useWallet.ts

[error] 140-140: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🪛 GitHub Actions: Code Formatting
src/hooks/useWallet.ts

[warning] Code style issues found. Run 'pnpm format' to fix formatting issues.

🔇 Additional comments (1)
src/hooks/useWallet.ts (1)

81-82: Ensure consistent use of token decimals when parsing balances

In line 81, the balance is parsed using a hardcoded decimal value of 6:

balance = parseUnits(totalBalance.toString(), 6)

While in line 95, the balance is parsed using PEANUT_WALLET_TOKEN_DECIMALS:

balance: parseUnits(totalBalance.toString(), PEANUT_WALLET_TOKEN_DECIMALS)

To maintain consistency and prevent potential issues with token balances, consider using the same decimal constant. If PEANUT_WALLET_TOKEN_DECIMALS is the appropriate variable, update line 81 accordingly.

Apply this diff to fix the inconsistency:

- balance = parseUnits(totalBalance.toString(), 6)
+ balance = parseUnits(totalBalance.toString(), PEANUT_WALLET_TOKEN_DECIMALS)
🧰 Tools
🪛 GitHub Actions: Code Formatting

[warning] Code style issues found. Run 'pnpm format' to fix formatting issues.

src/hooks/useWallet.ts Outdated Show resolved Hide resolved
Copy link
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

♻️ Duplicate comments (2)
src/hooks/useWallet.ts (2)

81-82: ⚠️ Potential issue

Ensure consistent use of token decimals when parsing balances.

The balance parsing uses different decimal values:

  • Line 81: Hardcoded 6
  • Line 95: PEANUT_WALLET_TOKEN_DECIMALS

This inconsistency could lead to incorrect balance calculations.

Apply this diff to maintain consistency:

- balance = parseUnits(totalBalance.toString(), 6)
+ balance = parseUnits(totalBalance.toString(), PEANUT_WALLET_TOKEN_DECIMALS)

Also applies to: 95-96


127-154: ⚠️ Potential issue

Fix potential infinite loop in external wallet management.

As noted in the previous review, this useEffect can enter an infinite loop when trying to connect an existing account.

Add a flag to prevent the infinite loop:

+ const [isProcessingNewAddresses, setIsProcessingNewAddresses] = useState(false)

  useEffect(() => {
    if (!user || !wagmiAddresses || !wallets.length) return
+   if (isProcessingNewAddresses) return

    const newAddresses = wagmiAddresses.filter(
      (addr) => !wallets.some((wallet) => areEvmAddressesEqual(addr, wallet.address))
    )
    if (newAddresses.length) {
+     setIsProcessingNewAddresses(true)
      newAddresses.forEach((address) => {
        addAccount({
          accountIdentifier: address,
          accountType: interfaces.WalletProviderType.BYOW,
          userId: user.user.userId,
          connector:
            connector && connector.icon
              ? {
                  iconUrl: connector.icon,
                  name: connector.name,
                }
              : undefined,
        }).catch((error) => {
          const errorMsg = error.message.includes('Account already exists')
            ? 'Could not add external wallet, already associated with another account'
            : 'Unexpected error adding external wallet'
          toast.error(errorMsg)
+       }).finally(() => {
+         setIsProcessingNewAddresses(false)
        })
      })
    }
-   }, [wagmiAddresses, wallets, user, addAccount, toast])
+   }, [wagmiAddresses, wallets, user, addAccount, toast, connector, isProcessingNewAddresses])
🧰 Tools
🪛 Biome (1.9.4)

[error] 140-140: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🧹 Nitpick comments (4)
src/hooks/useWallet.ts (4)

38-49: Consider memoizing wagmiAddresses array transformation.

The isWalletConnected callback's dependency array includes wagmiAddresses, which could trigger unnecessary re-renders if the addresses change but maintain the same equality check results.

Consider memoizing the transformed addresses:

+ const memoizedWagmiAddresses = useMemo(
+   () => wagmiAddresses?.map(addr => addr.toLowerCase()),
+   [wagmiAddresses]
+ );

  const isWalletConnected = useCallback(
    (wallet: interfaces.IDBWallet): boolean => {
      if (isPeanut(wallet) && kernelClientAddress) {
        return isKernelClientReady && areEvmAddressesEqual(kernelClientAddress, wallet.address)
      }
-     if (wagmiAddresses) {
-       return isWagmiConnected && wagmiAddresses.some((addr) => areEvmAddressesEqual(addr, wallet.address))
+     if (memoizedWagmiAddresses) {
+       return isWagmiConnected && memoizedWagmiAddresses.includes(wallet.address.toLowerCase())
      }
      return false
    },
-   [isKernelClientReady, kernelClientAddress, isWagmiConnected, wagmiAddresses]
+   [isKernelClientReady, kernelClientAddress, isWagmiConnected, memoizedWagmiAddresses]
  )

112-119: Add error handling for initial wallet selection.

The initial wallet selection could fail silently if both conditions (find(isPeanut) and wallets[0]) return undefined.

Consider adding error handling:

  useEffect(() => {
    if (!selectedAddress && wallets.length) {
      const initialWallet = wallets.find(isPeanut) || wallets[0]
      if (initialWallet) {
        dispatch(walletActions.setSelectedAddress(initialWallet.address))
+     } else {
+       console.warn('No valid wallet found for initial selection')
      }
    }
  }, [wallets, selectedAddress, dispatch])

140-145: Use optional chaining for connector properties.

The current implementation could be simplified using optional chaining.

- connector && connector.icon
+ connector?.icon
    ? {
        iconUrl: connector.icon,
        name: connector.name,
      }
    : undefined,
🧰 Tools
🪛 Biome (1.9.4)

[error] 140-140: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


156-185: Enhance error handling in balance refetching.

The current error handling only logs to console. Consider providing user feedback for failed balance updates.

  } catch (error) {
    console.error('Error refetching balance:', error)
+   toast.error('Failed to update wallet balance. Please try again.')
  }
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between d6c804b and 37af875.

📒 Files selected for processing (1)
  • src/hooks/useWallet.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/hooks/useWallet.ts

[error] 140-140: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (2)
src/hooks/useWallet.ts (2)

1-29: LGTM! Well-organized imports and utility functions.

The imports are properly organized, and utility functions are well-defined with clear responsibilities.


193-213: LGTM! Comprehensive hook return value.

The hook returns a well-organized object with all necessary wallet management functions and state.

- also fixed wallet connector images not rendering properly
- redux serialization errors
Copy link
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

♻️ Duplicate comments (2)
src/hooks/useWallet.ts (2)

103-103: ⚠️ Potential issue

Use consistent token decimals.

The balance is parsed using a hardcoded decimal value of 6, while line 117 uses PEANUT_WALLET_TOKEN_DECIMALS. This inconsistency could lead to incorrect balance calculations.

-balance = parseUnits(totalBalance.toString(), 6)
+balance = parseUnits(totalBalance.toString(), PEANUT_WALLET_TOKEN_DECIMALS)

59-128: 🛠️ Refactor suggestion

Enhance error handling in wallet processing.

The Promise chains lack proper error handling, which could lead to silent failures when processing wallets.

Consider wrapping the wallet processing logic in try-catch blocks and handling errors appropriately:

 const processedWallets = user
     ? await Promise.all(
         user.accounts
+            .filter((account) => {
+              try {
                 return Object.values(interfaces.WalletProviderType).includes(
                     account.account_type as unknown as interfaces.WalletProviderType
                 )
+              } catch (error) {
+                console.error('Error filtering account:', error)
+                return false
+              }
+            })
             .sort((a, b) => {
+              try {
                 if (interfaces.AccountType.PEANUT_WALLET === a.account_type) {
                     return -1
                 } else if (interfaces.AccountType.PEANUT_WALLET === b.account_type) {
                     return 1
                 }
                 const dateA = new Date(a.created_at)
                 const dateB = new Date(b.created_at)
                 return dateA.getTime() - dateB.getTime()
+              } catch (error) {
+                console.error('Error sorting accounts:', error)
+                return 0
+              }
             })
             .map(async (account) => {
+              try {
                 // ... existing wallet processing logic
+              } catch (error) {
+                console.error(`Error processing wallet ${account.account_identifier}:`, error)
+                return undefined
+              }
             })
     )
+    .then(wallets => wallets.filter(Boolean))
🧹 Nitpick comments (1)
src/hooks/useWallet.ts (1)

152-197: Consider optimizing address processing.

The address processing effect could be optimized to reduce unnecessary iterations:

-const existingAddresses = new Set(wallets.map((wallet) => wallet.address.toLowerCase()))
+const existingAddresses = new Set(wallets.map((wallet) => wallet.address.toLowerCase()))
+const newAddresses = wagmiAddresses.reduce((acc, addr) => {
+    const lowerAddr = addr.toLowerCase()
+    if (!existingAddresses.has(lowerAddr) && !processedAddressesRef.current.has(lowerAddr)) {
+        processedAddressesRef.current.add(lowerAddr)
+        acc.push(addr)
+    }
+    return acc
+}, [] as string[])
-const newAddresses = wagmiAddresses.filter((addr) => {
-    const lowerAddr = addr.toLowerCase()
-    if (!existingAddresses.has(lowerAddr) && !processedAddressesRef.current.has(lowerAddr)) {
-        processedAddressesRef.current.add(lowerAddr)
-        return true
-    }
-    return false
-})
🧰 Tools
🪛 Biome (1.9.4)

[error] 170-170: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 37af875 and 7c07ce6.

📒 Files selected for processing (4)
  • src/components/Home/WalletCard.tsx (1 hunks)
  • src/components/Setup/Views/Welcome.tsx (1 hunks)
  • src/hooks/useWallet.ts (1 hunks)
  • src/redux/store.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/components/Home/WalletCard.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Setup/Views/Welcome.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/hooks/useWallet.ts

[error] 170-170: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (6)
src/redux/store.ts (1)

13-16: Consider alternatives to disabling serialization checks.

Disabling Redux serialization checks can mask potential issues with non-serializable values in the state. Consider:

  1. Using a custom serialization check that ignores specific paths where non-serializable values are expected
  2. Transforming non-serializable values before storing them
src/hooks/useWallet.ts (5)

17-28: LGTM! Well-organized utility functions.

The utility functions are pure, focused, and follow single responsibility principle.


41-52: LGTM! Comprehensive wallet connection check.

The callback correctly handles both Peanut and external wallets with appropriate dependency tracking.


131-135: LGTM! Efficient wallet memoization.

The useMemo implementation is correct with appropriate dependencies.


199-228: LGTM! Robust balance refetching implementation.

The callback handles both wallet types appropriately with proper error handling.


230-256: LGTM! Comprehensive wallet management interface.

The hook provides a complete and well-structured API for wallet management.

Copy link
Contributor

@jjramirezn jjramirezn left a comment

Choose a reason for hiding this comment

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

Looks good, smoke test pass

@jjramirezn jjramirezn merged commit da7efcb into peanut-wallet Jan 15, 2025
4 checks passed
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.

2 participants