Skip to content

Conversation

@jeremy0x
Copy link
Contributor

@jeremy0x jeremy0x commented Jan 6, 2026

Description

Implements cross-chain balance display in the wallet panel, allowing users to see all their token balances across all supported networks without manually switching between chains. This addresses the need for users to check "what chain and token" when money arrives in their Noblocks wallet.

Key Changes:

  • Added parallel balance fetching across all supported networks in BalanceContext
  • Updated desktop and mobile wallet balance displays to group balances by network
  • Updated navbar and sidebar total balance to show cross-chain total across all networks
  • Fixed CNGN rate calculation to use network-specific rates instead of selected network rate
  • Selected network shows all balances first, other networks show only non-zero balances
  • Follows the same UI grouping pattern as transaction history dates for consistency
  • Silently handles RPC failures to avoid blocking the UI

Additional Improvements:

  • Enhanced CNGN balance handling with network fallback system for rate fetching reliability
  • Added robust error handling for CNGN rate failures
  • Fixed loading states with proper skeleton loaders for all balance displays

Display Logic:

  • Selected network: All token balances (including zeros)
  • Other networks: Only non-zero balances
  • Sorting: Selected network first, then alphabetical by network name

Technical Implementation:

  • Uses Promise.allSettled() for parallel network requests
  • Graceful error handling for failed RPC calls
  • Network grouping with dashed separators matching transaction history pattern
  • Cross-platform support (desktop and mobile)

Additional Technical Enhancements:

  • CNGN rate fallback system for improved reliability
  • Enhanced loading states with skeleton loaders
  • Network-specific rate caching for better performance

Screenshots

Loading State Final View
mobile skeleton mobile view
desktop skeleton desktop view

References

Closes #276

Additional Context:

Addresses cross-chain balance display requirements while implementing additional reliability improvements for CNGN rate handling discovered during testing.

Testing

Manual Testing Steps:

  1. Connect wallet with balances on multiple networks
  2. Verify that navbar and sidebar show total balance across all networks
  3. Switch between different networks and verify balances display correctly
  4. Check that selected network shows all balances while others show only non-zero
  5. Verify network grouping and sorting order
  6. Test mobile view responsiveness and scrolling
  7. Verify loading states show skeleton loaders during balance fetching

Additional Testing (CNGN Improvements):

  • Test CNGN balance display shows raw token amounts with USD equivalents
  • Verify network switching maintains correct CNGN rates
  • Test rate failure scenarios show appropriate error handling

Environment:

  • Tested across supported networks: Ethereum, Polygon, BNB Smart Chain, Arbitrum, Base, Celo, Lisk

  • Verified on both desktop and mobile breakpoints

  • Tested CNGN rate fallback scenarios (network failures, API timeouts, cache recovery)

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation and tests for new/changed functionality in this PR
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not main

By submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.

Summary by CodeRabbit

  • New Features

    • Cross-chain balance view grouped by network with per-network headers and mobile-optimized loading cards
    • Mobile wallet view now accepts and displays cross-chain balances
  • Performance

    • CNGN exchange rate caching with TTL and parallel fallbacks for more reliable, faster rate fetches
    • Request cancellation support to avoid unnecessary work
  • Improvements

    • CNGN token balance handling refined for accurate display and validation
    • Non-selected networks hide zero balances for a cleaner UI
    • package manager updated to latest spec

✏️ Tip: You can customize this high-level summary in your review settings.

- Add parallel balance fetching across all supported networks
- Update wallet displays to group balances by network with sorting
- Implement comprehensive CNGN rate fallback system with caching
- Add loading states and error handling for rate failures
- Fix balance display to show raw CNGN amounts with USD equivalents
- Optimize rate fetching with 5min TTL cache and parallel fallbacks
- Ensure CNGN balances excluded from totals when rates unavailable

Closes paycrest#276
- Extract repeated CNGN conversion logic into reusable applyCNGNBalanceConversion function
- Remove code duplication across 4 different balance handling sections
- Maintain same functionality while improving maintainability
- Single source of truth for CNGN balance conversion rules
- Fixed type mismatch in Promise.allSettled result filtering
- Replaced problematic type predicate with type assertion
- Resolves build compilation error while maintaining type safety
- Upgraded pnpm from version 10.14.0 to 10.27.0 for improved package management and performance.
- Removed dollar value display for CNGN tokens on mobile wallet view
- Mobile now shows only raw CNGN amounts for cleaner UI
- Desktop continues to show both raw amounts and dollar equivalents
- Maintains consistent raw balance display across platforms
Copilot AI review requested due to automatic review settings January 6, 2026 09:46
@jeremy0x jeremy0x requested a review from chibie as a code owner January 6, 2026 09:46
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 6, 2026

📝 Walkthrough

Walkthrough

This PR adds cross‑chain balance fetching, conversion and aggregation to the BalanceContext, surfaces per‑network grouped balances to wallet UI components (mobile + desktop), introduces CNGN rate caching/fallbacks and responsive skeletons, and updates packageManager to [email protected].

Changes

Cohort / File(s) Summary
Skeleton Components
app/components/BalanceSkeleton.tsx
Added MobileBalanceCardSkeleton and CrossChainBalanceSkeleton (responsive isMobile branching) for mobile/desktop loading states.
Wallet UI & Mobile View
app/components/WalletDetails.tsx, app/components/wallet-mobile-modal/WalletView.tsx, app/components/MobileDropdown.tsx
Replaced single-network balance rendering with per-network grouped rendering; added crossChainBalances prop to WalletView, memoized sortedCrossChainBalances in MobileDropdown, inserted dividers, filtering (skip empty networks, show zeros for selected network) and swapped skeleton usage.
Balance Context & Cross‑Chain Fetching
app/context/BalanceContext.tsx, app/context/index.ts
Introduced CrossChainBalanceEntry type, added crossChainBalances state and crossChainTotal, implemented parallel multi-network fetch (with rawBalances capture), applied per-network CNGN conversion (applyCNGNBalanceConversion), and re-exported the new type.
CNGN Rate Hook
app/hooks/useCNGNRate.ts
Added client caching (TTL), timeouts, AbortController handling, primary + parallel fallback fetch strategy, and autoFetch option in UseCNGNRateOptions.
Transaction Form
app/pages/TransactionForm.tsx
Special-cased CNGN to use activeBalance.rawBalances[token] when present, otherwise fallback to corrected balances.
API + Types
app/api/aggregator.ts, app/types.ts
fetchRate now accepts an AbortSignal; RatePayload gains optional signal?: AbortSignal.
Package Manifest
package.json
Updated packageManager from [email protected] to [email protected].

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant WalletUI as Wallet Panel\n(WalletDetails / WalletView)
    participant BalanceCtx as BalanceContext
    participant Networks as Supported Networks
    participant RateSvc as useCNGNRate
    participant Cache as LocalStorage

    User->>WalletUI: Open wallet panel
    WalletUI->>BalanceCtx: useBalance() / refreshBalance()
    activate BalanceCtx

    BalanceCtx->>Cache: Check cached CNGN rates
    alt Cache fresh
        Cache-->>BalanceCtx: Return cached rate
    end

    par Fetch balances across networks
        loop for each network
            BalanceCtx->>Networks: fetchBalance(network)
            Networks-->>BalanceCtx: raw balances
        end
    end

    BalanceCtx->>RateSvc: getCNGNRateForNetwork(network)
    activate RateSvc
    RateSvc->>Cache: Check cached rate
    alt cached fresh
        Cache-->>RateSvc: cached rate
    else
        RateSvc->>Networks: Primary rate fetch (5s timeout)
        alt primary succeeds
            Networks-->>RateSvc: rate
        else
            par fallback fetches (3s)
                RateSvc->>Networks: fetch from backup1
                RateSvc->>Networks: fetch from backup2
            end
            Networks-->>RateSvc: first successful rate
        end
        RateSvc->>Cache: Cache rate & timestamp
    end
    deactivate RateSvc

    BalanceCtx->>BalanceCtx: applyCNGNBalanceConversion() per-network
    BalanceCtx->>BalanceCtx: compute crossChainTotal and sort (selected first)
    BalanceCtx-->>WalletUI: crossChainBalances[], crossChainTotal
    deactivate BalanceCtx

    WalletUI->>WalletUI: Group by network, filter/format balances
    WalletUI-->>User: Render grouped cross-chain balances
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • chibie
  • 5ran6

Poem

🐇 I hopped through chains both near and far,
Fetched tiny balances, each shining star.
Sorted first the chosen land, then A to Z,
Now one warm wallet shows them all to me!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: implement cross-chain balance display' is clear, specific, and directly matches the main objective of the PR—implementing cross-chain balance functionality.
Description check ✅ Passed The PR description comprehensively covers all template sections: description with background and impacts, references (closes #276), testing with detailed manual steps and environment details, and a completed checklist addressing all requirements.
Linked Issues check ✅ Passed The code changes fully implement all acceptance criteria from issue #276: cross-chain balance fetching via parallel requests, sorting with selected network first then alphabetically, per-network UI grouping with separators, all-balances display for selected network and non-zero-only for others, and graceful RPC failure handling.
Out of Scope Changes check ✅ Passed All code changes align with PR objectives: cross-chain balance display, parallel fetching, network grouping, CNGN rate improvements, and robustness enhancements. The package.json pnpm version update is a minor maintenance change that does not detract from the primary feature scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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: 2

🤖 Fix all issues with AI Agents
In @app/context/BalanceContext.tsx:
- Around line 224-227: The cross-chain fetch (fetchCrossChainBalances called
with smartWalletAccount.address) is not awaited so setIsLoading(false) runs
before cross-chain results arrive; either await the fetchCrossChainBalances
calls where invoked (so setIsLoading(false) happens after completion) or
introduce a separate isCrossChainLoading state and update it around
fetchCrossChainBalances (start true before call, set false when that promise
resolves) and keep setIsLoading for primary data only; update any UI uses of
isLoading to reflect this new split-loading behavior.

In @app/hooks/useCNGNRate.ts:
- Around line 78-79: The timeout AbortController created in useCNGNRate and
getCNGNRateForNetwork is never passed to fetchRate so controller.abort() doesn't
cancel the HTTP request; update fetchRate signature (e.g., add optional signal?:
AbortSignal to the RatePayload param or an extended param) and forward that
signal into axios.get (pass { params, signal }), then pass controller.signal
from every fetchRate call in useCNGNRate and getCNGNRateForNetwork (both primary
and fallback attempts) so aborting the controller cancels the request.
🧹 Nitpick comments (8)
app/hooks/useCNGNRate.ts (3)

5-5: Unused import.

The networks import from ../mocks is never used in this file.

🔎 Proposed fix
-import { networks } from "../mocks";

144-162: Misleading comment: Promise.allSettled does not race.

The comment says "Race condition: First successful response wins," but Promise.allSettled waits for all promises to settle before iterating through results. This means both fallback networks must complete (or timeout) before a result is used.

If true racing behavior is desired (first success wins immediately), consider Promise.any:

🔎 Proposed fix using Promise.any for true racing
-    // Race condition: First successful response wins
-    try {
-      const results = await Promise.allSettled(fallbackPromises);
-      for (const result of results) {
-        if (result.status === 'fulfilled' && result.value) {
-          const { network: successfulNetwork, rate } = result.value;
+    // Race: First successful response wins
+    try {
+      const result = await Promise.any(fallbackPromises.map(p => 
+        p.then(r => r ? r : Promise.reject(new Error('No rate')))
+      ));
+      if (result) {
+        const { network: successfulNetwork, rate } = result;
           setRate(rate);
           setLastSuccessfulRate(rate);
           // Cache with timestamp
           if (typeof window !== 'undefined') {
             localStorage.setItem('cngn_last_rate', rate.toString());
             localStorage.setItem('cngn_cache_time', Date.now().toString());
           }
           setError(null);
           setIsLoading(false);
           console.log(`Successfully fetched CNGN rate from fallback ${successfulNetwork}: ${rate}`);
           return;
-        }
       }
     } catch (err) {
-      console.warn('Parallel fallback failed:', err);
+      console.warn('All fallback networks failed:', err);
     }

211-324: Consider extracting shared fetch logic to reduce duplication.

getCNGNRateForNetwork duplicates most of the fetch/cache/fallback logic from useCNGNRate. Extracting a shared helper (e.g., fetchCNGNRateWithFallback) would reduce maintenance burden and ensure consistency.

app/components/MobileDropdown.tsx (1)

146-165: Dead code: trackLogoutWithFetch is defined but never called.

This function is not used anywhere in the component. Consider removing it or wiring it into handleLogout if tracking is intended.

🔎 If tracking is intended, wire it into handleLogout
 const handleLogout = async () => {
   setIsLoggingOut(true);
   try {
+    // Fire-and-forget tracking
+    if (activeWallet?.address) {
+      trackLogoutWithFetch({
+        walletAddress: activeWallet.address,
+        logoutMethod: isInjectedWallet ? 'injected' : 'embedded'
+      });
+    }
     // Disconnect external wallet if connected
     await logout();

Otherwise, remove the unused function.

app/components/wallet-mobile-modal/WalletView.tsx (1)

166-172: Hardcoded image path may fail for missing token logos.

The path /logos/${token.toLowerCase()}-logo.svg assumes all tokens have corresponding logo files. Consider adding error handling or a fallback image.

🔎 Proposed fix with fallback
 <Image
-  src={`/logos/${token.toLowerCase()}-logo.svg`}
+  src={`/logos/${token.toLowerCase()}-logo.svg`}
   alt={token}
   width={14}
   height={14}
   className="size-3.5"
+  onError={(e) => {
+    (e.target as HTMLImageElement).src = '/logos/default-token-logo.svg';
+  }}
 />

Alternatively, use the existing getTokenImageUrl prop that's already passed to this component but unused in this section.

app/components/WalletDetails.tsx (2)

35-35: Unused import: getCNGNRateForNetwork

This function is imported but never used in this component. Only useCNGNRate is actually used.

🔎 Proposed fix
-import { useCNGNRate, getCNGNRateForNetwork } from "../hooks/useCNGNRate";
+import { useCNGNRate } from "../hooks/useCNGNRate";

383-437: LGTM with minor formatting suggestion.

The balance rendering logic correctly handles CNGN display with raw balances and USD equivalents. The visual indicator (red text) when USD equivalent is unavailable but balance exists is a good UX pattern.

Consider extracting the token balance row into a separate component for better readability and reusability, but this is optional given the current scope.

app/context/BalanceContext.tsx (1)

162-167: Consider using a type predicate for cleaner type narrowing.

The current type assertion is safe but could be more elegant with a type predicate helper.

🔎 Optional improvement
// Helper type predicate
const isFulfilled = <T>(result: PromiseSettledResult<T>): result is PromiseFulfilledResult<T> =>
  result.status === "fulfilled";

// Usage
const successfulResults = results.filter(isFulfilled).map((result) => result.value);
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a605397 and 9bc4d16.

📒 Files selected for processing (9)
  • app/components/BalanceSkeleton.tsx
  • app/components/MobileDropdown.tsx
  • app/components/WalletDetails.tsx
  • app/components/wallet-mobile-modal/WalletView.tsx
  • app/context/BalanceContext.tsx
  • app/context/index.ts
  • app/hooks/useCNGNRate.ts
  • app/pages/TransactionForm.tsx
  • package.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.

Applied to files:

  • app/components/MobileDropdown.tsx
  • app/context/BalanceContext.tsx
  • app/components/WalletDetails.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.

Applied to files:

  • app/pages/TransactionForm.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Applied to files:

  • app/context/BalanceContext.tsx
  • app/components/wallet-mobile-modal/WalletView.tsx
  • app/components/WalletDetails.tsx
🧬 Code graph analysis (1)
app/hooks/useCNGNRate.ts (2)
app/utils.ts (2)
  • getPreferredRateToken (1110-1137)
  • normalizeNetworkForRateFetch (593-595)
app/api/aggregator.ts (1)
  • fetchRate (42-129)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Agent
🔇 Additional comments (11)
package.json (1)

106-106: LGTM - Package manager version bump.

The pnpm version update from 10.14.0 to 10.27.0 aligns with the PR's tooling updates.

app/context/index.ts (1)

3-7: LGTM - Clean type re-export.

The addition of type CrossChainBalanceEntry to the barrel export follows TypeScript best practices and maintains consistency with the existing export pattern.

app/components/MobileDropdown.tsx (1)

105-116: LGTM - Clean sorting implementation.

The sorting logic correctly prioritizes the selected network first, then sorts remaining networks alphabetically. The useMemo dependencies are appropriate.

app/components/BalanceSkeleton.tsx (1)

24-72: LGTM - Well-structured skeleton components.

The new MobileBalanceCardSkeleton and CrossChainBalanceSkeleton components follow the existing pattern and provide appropriate loading states for cross-chain balance displays. The isMobile prop offers good flexibility for responsive layouts.

app/components/wallet-mobile-modal/WalletView.tsx (3)

17-19: LGTM - Simple Divider component.

Clean inline component for visual separation.


125-139: LGTM - Correct filtering logic for cross-chain balances.

The implementation correctly shows all balances (including zeros) for the selected network while filtering to only non-zero balances for other networks. Skipping networks with no displayable balances prevents empty sections.


153-158: LGTM - CNGN raw balance display.

The logic correctly displays raw token amounts for CNGN while using converted balances for other tokens, consistent with the TransactionForm approach.

Also applies to: 181-185

app/pages/TransactionForm.tsx (1)

121-124: LGTM - CNGN raw balance handling is correct.

The logic properly uses rawBalances for CNGN tokens to avoid USD-equivalent values, with appropriate fallback chain: rawBalances[token]balances[token]0. The case-insensitive check handles both "CNGN" and "cNGN" variants correctly. The activeBalance type (WalletBalances | null) includes the optional rawBalances property, and optional chaining ensures type safety. This approach is consistent with the same pattern in WalletDetails.tsx and WalletView.tsx.

app/components/WalletDetails.tsx (1)

88-100: LGTM!

The sorting logic correctly prioritizes the selected network and sorts others alphabetically. The memoization with appropriate dependencies is well implemented.

app/context/BalanceContext.tsx (2)

37-75: LGTM!

The helper function correctly handles CNGN balance conversion with proper edge case handling for null/zero rates. The mutation pattern is acceptable here since it's clearly documented and used intentionally within controlled contexts.


333-336: LGTM!

The cross-chain total calculation is straightforward and correct. The balances are already CNGN-corrected at this point, so summing totals produces the accurate USD value.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements cross-chain balance display functionality, enabling users to view their token balances across all supported networks without manually switching chains. The implementation includes a robust CNGN rate fallback system with caching, parallel network balance fetching, and consistent UI grouping patterns for both desktop and mobile views.

Key Changes:

  • Parallel cross-chain balance fetching with graceful RPC failure handling
  • Enhanced CNGN rate system with network fallbacks, caching (5-minute TTL), and expired cache recovery
  • Cross-chain balance grouping UI with network-specific displays (selected network shows all balances, others show non-zero only)

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
package.json Updates pnpm version to 10.27.0
app/hooks/useCNGNRate.ts Implements CNGN rate fallback system with caching, timeouts, and multi-network retry logic
app/context/BalanceContext.tsx Adds cross-chain balance fetching with parallel network requests and CNGN conversion logic
app/context/index.ts Exports CrossChainBalanceEntry type for cross-chain balance data structure
app/pages/TransactionForm.tsx Updates balance display to show raw CNGN amounts instead of USD equivalents
app/components/WalletDetails.tsx Implements desktop UI for cross-chain balance display with network grouping and dividers
app/components/wallet-mobile-modal/WalletView.tsx Implements mobile UI for cross-chain balance display with network grouping
app/components/MobileDropdown.tsx Integrates cross-chain balances into mobile dropdown component
app/components/BalanceSkeleton.tsx Adds skeleton loaders for cross-chain balance loading states

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add AbortController signal support to fetchRate for proper request cancellation
- Extract constants (CNGN_CACHE_TTL, timeouts, RELIABLE_NETWORKS) for maintainability
- Implement try-finally wrappers to guarantee timeout cleanup and prevent memory leaks
- Remove unused imports (networks from mocks, getCNGNRateForNetwork from WalletDetails)
- Remove unused lastSuccessfulRate state variable
- Remove all console.log statements for production readiness
- Refactor applyCNGNBalanceConversion to pure function (returns new object)
- Add concurrency batching (3 networks at a time) to prevent RPC endpoint overload
- Fix loading state by awaiting fetchCrossChainBalances for complete data
- Remove blocking refetchCNGNRate calls (redundant, each network fetches own rate)
- Fix CNGN balance fallback logic to check rawBalances existence explicitly
- Update misleading "race condition" comment to describe actual behavior

This addresses 17 legitimate issues and 2 best-practice violations from CodeRabbit and Copilot review feedback.
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/WalletDetails.tsx (1)

74-81: Remove unused rate and rateError from useCNGNRate destructuring.

The values are destructured but never used in the component. Since BalanceContext now provides per-network CNGN conversion via getCNGNRateForNetwork, these can be removed along with the hook call if no longer needed.

🧹 Nitpick comments (2)
app/api/aggregator.ts (1)

106-129: Consider distinguishing abort errors from actual failures.

When a request is aborted via AbortController.abort(), axios throws a CanceledError. Currently, this is logged to console.error and tracked as an "External API Error", which may pollute error logs and analytics with expected cancellation events.

Suggested improvement
   } catch (error) {
     const responseTime = Date.now() - startTime;

+    // Don't log aborted requests as errors - they're expected cancellations
+    if (axios.isCancel(error)) {
+      throw error;
+    }
+
     // Track API error
     trackServerEvent("External API Error", {
app/hooks/useCNGNRate.ts (1)

140-156: Unused destructured variable successfulNetwork.

The variable is destructured but never used. Consider using an underscore prefix to indicate it's intentionally ignored, or remove it.

-        if (result.status === "fulfilled" && result.value) {
-          const { network: successfulNetwork, rate } = result.value;
+        if (result.status === "fulfilled" && result.value) {
+          const { rate } = result.value;
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9bc4d16 and a319be6.

📒 Files selected for processing (6)
  • app/api/aggregator.ts
  • app/components/WalletDetails.tsx
  • app/context/BalanceContext.tsx
  • app/hooks/useCNGNRate.ts
  • app/pages/TransactionForm.tsx
  • app/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/pages/TransactionForm.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.

Applied to files:

  • app/components/WalletDetails.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Applied to files:

  • app/context/BalanceContext.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.

Applied to files:

  • app/context/BalanceContext.tsx
🧬 Code graph analysis (1)
app/context/BalanceContext.tsx (4)
app/types.ts (1)
  • Network (263-271)
app/hooks/useCNGNRate.ts (1)
  • useCNGNRate (35-197)
app/mocks.ts (1)
  • networks (44-88)
app/utils.ts (3)
  • getRpcUrl (159-178)
  • fetchWalletBalance (460-506)
  • calculateCorrectedTotalBalance (515-541)
🔇 Additional comments (12)
app/types.ts (1)

114-121: LGTM! AbortSignal support enables proper request cancellation.

The addition of signal?: AbortSignal to RatePayload allows propagating cancellation tokens through rate-fetching paths, which is correctly utilized by the updated fetchRate function and the useCNGNRate hook's timeout logic.

app/api/aggregator.ts (1)

42-49: LGTM! Signal correctly propagated to axios request.

The signal parameter is properly destructured and passed to axios.get, enabling request cancellation when the AbortController triggers.

Also applies to: 75-75

app/hooks/useCNGNRate.ts (2)

6-10: LGTM! Constants properly extracted for maintainability.

The configuration values for cache TTL, timeouts, and reliable networks are now centralized, addressing previous feedback about magic numbers and hardcoded values.


71-102: LGTM! AbortController signal correctly integrated with proper cleanup.

The signal is now passed to fetchRate, and the try/finally block ensures clearTimeout is called regardless of success or failure. This addresses the previous review feedback about non-functional abort controllers.

app/components/WalletDetails.tsx (3)

41-43: LGTM! Divider component matches TransactionList styling.

The dashed border separator is consistent with the UI grouping pattern used elsewhere in the application.


93-105: LGTM! Sorting logic correctly implements requirements.

Selected network is placed first, and remaining networks are sorted alphabetically by chain name.


383-446: LGTM! Cross-chain balance display with correct CNGN handling.

The implementation correctly:

  • Uses rawBalances for CNGN token amount display
  • Uses the converted balance value for USD equivalent
  • Applies red styling when rate fetch failed (usdEquivalent === 0 with non-zero balance)
  • Filters zero balances for non-selected networks while showing all balances for the selected network
app/context/BalanceContext.tsx (5)

25-25: LGTM! Concurrency limit prevents RPC overload.

The batch size of 3 concurrent requests is a reasonable limit to avoid overwhelming RPC endpoints while still achieving parallelism.


45-81: LGTM! CNGN conversion refactored to pure function.

The function now returns a new object instead of mutating the input, addressing previous feedback about functional programming principles. The logic correctly handles both rate-available and rate-unavailable scenarios.


127-188: LGTM! Cross-chain balance fetching with proper batching and error handling.

The implementation correctly:

  • Processes networks in batches of 3 to limit concurrency
  • Uses Promise.allSettled to handle individual network failures gracefully
  • Fetches per-network CNGN rates (which leverage caching from getCNGNRateForNetwork)
  • Preserves raw balances before conversion for UI display
  • Silently skips failed networks per requirements

244-246: Good fix: Cross-chain fetch is now awaited.

This addresses the previous review feedback about setIsLoading(false) executing before cross-chain data arrived. The loading state now accurately reflects the complete fetch operation.

Also applies to: 318-319


357-360: LGTM! Cross-chain total correctly sums CNGN-corrected network totals.

The calculation iterates over crossChainBalances entries where each entry.balances.total is already CNGN-corrected, ensuring accurate USD totals.

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.

[Feature Request] Cross-Chain Balance Display in Wallet Panel

2 participants