feat(wallet): display real coins and wire wallet view management to API#115
Open
ice-brontes wants to merge 33 commits into
Open
feat(wallet): display real coins and wire wallet view management to API#115ice-brontes wants to merge 33 commits into
ice-brontes wants to merge 33 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements a comprehensive wallet management system across the frontend monorepo. It adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthFlow as Auth Flow
participant App as App/SplashScreen
participant WalletClient as Wallet Client
participant WalletAPI as Wallet API
participant Store as Wallet Store
participant UI as UI Layer
User->>AuthFlow: Authenticate
AuthFlow->>App: onAuthSuccess(username)
App->>WalletClient: initializeWalletClient(identityClient, username)
App->>WalletClient: loadWalletViewData()
WalletClient->>WalletAPI: listWalletViews(username)
WalletAPI-->>WalletClient: wallet summaries
loop For each wallet summary
WalletClient->>WalletAPI: getWalletView(username, walletId)
WalletAPI-->>WalletClient: wallet detail
WalletClient->>WalletClient: convertWalletView(detail)
end
WalletClient->>Store: batchUpdate(walletViews, activeId)
Store-->>WalletClient: success
WalletClient-->>App: success
App->>UI: Navigate to Main with wallet data loaded
UI-->>User: Display wallet with coin list
sequenceDiagram
participant User
participant UI as Create Wallet Screen
participant Action as createWalletView Action
participant Store as Wallet Store
participant API as Wallet API
User->>UI: Submit wallet name
UI->>Action: createWalletView(name)
Action->>Store: Optimistic insert (loading, temp id)
Store-->>Action: Updated state
Action->>API: client.createWalletView(username, {name, items: [], symbolGroups: []})
API-->>Action: Created wallet (server id)
Action->>API: getWalletView(username, serverId)
API-->>Action: Wallet detail
Action->>Action: convertWalletView(detail)
Action->>Store: Replace temp entry with server data
Store-->>Action: Updated state
Action-->>UI: Success
UI-->>User: Navigate to main wallet list
Note over Action,API: On API failure:<br/>Store reverts optimistic insert<br/>Error thrown to showWalletError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Connects the wallet UI to real server-side wallet view data (balances + coin groups) and wires wallet view lifecycle operations (load/create/rename/delete) to the identity-client API with optimistic updates and improved loading UX.
Changes:
- Reorganizes
@ion/walletintoactions/,stores/,hooks/,converters/and introduces richer domain types for coins and wallet views. - Implements wallet view conversion/sorting + UI coin list rendering with skeleton (shimmer) loading states.
- Adds shared UI formatters and improves network/identity-client handling for empty bodies / delete responses.
Reviewed changes
Copilot reviewed 72 out of 75 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| frontend/packages/wallet/src/wallet-view-store.ts | Extends initial store objects to include server-driven wallet/coin fields. |
| frontend/packages/wallet/src/wallet-view-store.test.ts | Updates store test expectations for the expanded WalletView shape. |
| frontend/packages/wallet/src/use-wallet-views.ts | Updates imports to new module locations. |
| frontend/packages/wallet/src/use-active-wallet-view.ts | Updates imports to new module locations. |
| frontend/packages/wallet/src/types.ts | Adds coin + grouping domain types and extends WalletView with server/loading fields. |
| frontend/packages/wallet/src/switch-wallet-view.ts | Updates store import path after reorg. |
| frontend/packages/wallet/src/switch-wallet-view.test.ts | Removes old switch-wallet-view tests (replaced under actions/). |
| frontend/packages/wallet/src/stores/wallet-notification-config.ts | Adds global wallet error notifier registration. |
| frontend/packages/wallet/src/stores/wallet-client-config.ts | Adds singleton identity-client config + reset behavior. |
| frontend/packages/wallet/src/stores/wallet-client-config.test.ts | Tests for wallet client initialization/reset behavior. |
| frontend/packages/wallet/src/rename-wallet-view.ts | Removes local-only rename action (replaced with API-backed action). |
| frontend/packages/wallet/src/rename-wallet-view.test.ts | Removes tests for old local-only rename action. |
| frontend/packages/wallet/src/index.ts | Updates public exports to new structure + new APIs. |
| frontend/packages/wallet/src/delete-wallet-view.ts | Removes local-only delete action (replaced with API-backed action). |
| frontend/packages/wallet/src/delete-wallet-view.test.ts | Removes tests for old local-only delete action. |
| frontend/packages/wallet/src/create-wallet-view.ts | Removes local-only create action (replaced with API-backed action). |
| frontend/packages/wallet/src/create-wallet-view.test.ts | Removes tests for old local-only create action. |
| frontend/packages/wallet/src/converters/search-aggregation-item.ts | Adds aggregation matching + wallet-key builder for de-duping balances. |
| frontend/packages/wallet/src/converters/search-aggregation-item.test.ts | Adds tests for aggregation item search/matching. |
| frontend/packages/wallet/src/converters/from-blockchain-units.ts | Adds BigInt-based blockchain-unit conversion helper. |
| frontend/packages/wallet/src/converters/from-blockchain-units.test.ts | Adds tests for blockchain-unit conversion helper. |
| frontend/packages/wallet/src/converters/extract-contract-address.ts | Adds helper to extract contract address from wallet assets. |
| frontend/packages/wallet/src/converters/convert-wallet-view.ts | Converts API wallet view detail into grouped coin balances + totals. |
| frontend/packages/wallet/src/converters/convert-wallet-view.test.ts | Tests grouping, sorting, totals, and de-duplication logic. |
| frontend/packages/wallet/src/converters/compare-coin-groups.ts | Adds deterministic sorting for groups/coins (ION-first, balance-based, etc.). |
| frontend/packages/wallet/src/converters/compare-coin-groups.test.ts | Tests sorting behavior for groups and coins. |
| frontend/packages/wallet/src/actions/switch-wallet-view.test.ts | Adds tests for the rehomed switch-wallet-view action. |
| frontend/packages/wallet/src/actions/rename-wallet-view.ts | Adds optimistic rename wired to updateWalletView API with revert-on-failure. |
| frontend/packages/wallet/src/actions/rename-wallet-view.test.ts | Tests optimistic rename + revert-on-failure behavior. |
| frontend/packages/wallet/src/actions/load-wallet-view-data.ts | Loads all wallet views in parallel and populates store with converted balances. |
| frontend/packages/wallet/src/actions/load-wallet-view-data.test.ts | Tests load behavior, parallel fetch, and error handling. |
| frontend/packages/wallet/src/actions/delete-wallet-view.ts | Adds optimistic delete wired to API with snapshot-based revert. |
| frontend/packages/wallet/src/actions/delete-wallet-view.test.ts | Tests delete behavior + revert-on-failure + guardrails. |
| frontend/packages/wallet/src/actions/create-wallet-view.ts | Adds optimistic create + fetch detail + temp-id replacement. |
| frontend/packages/wallet/src/actions/create-wallet-view.test.ts | Tests optimistic create, ID replacement, and revert-on-failure. |
| frontend/packages/wallet/package.json | Adds @ion/identity-client dependency for wallet API integration. |
| frontend/packages/wallet-ui/src/translations/fr.ts | Adds translated wallet error strings. |
| frontend/packages/wallet-ui/src/translations/en.ts | Adds wallet error strings (EN). |
| frontend/packages/wallet-ui/src/translations/de.ts | Adds translated wallet error strings. |
| frontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsx | Uses exported WalletView type for list props. |
| frontend/packages/wallet-ui/src/screens/use-wallet-error-notifier.ts | Registers a wallet error notifier (currently console.warn). |
| frontend/packages/wallet-ui/src/screens/WalletScreen.tsx | Installs error notifier + passes balance visibility into coins section. |
| frontend/packages/wallet-ui/src/components/ManageCoinsButton.tsx | Adjusts icon color to match accent styling. |
| frontend/packages/wallet-ui/src/components/CoinsSectionSkeleton.tsx | Adds shimmer skeleton for coins section loading state. |
| frontend/packages/wallet-ui/src/components/CoinsSectionCoinsPage.tsx | Adds conditional rendering for loading/list/empty coin states. |
| frontend/packages/wallet-ui/src/components/CoinsSection.tsx | Wires coins tab to real active wallet view + skeleton/list rendering. |
| frontend/packages/wallet-ui/src/components/CoinSearchBar.tsx | Enables autoFocus on search input. |
| frontend/packages/wallet-ui/src/components/CoinListItemSkeleton.tsx | Adds skeleton item component. |
| frontend/packages/wallet-ui/src/components/CoinListItem.tsx | Adds real coin group list item rendering with new formatters. |
| frontend/packages/wallet-ui/src/components/CoinList.tsx | Adds searchable grouped coin list. |
| frontend/packages/wallet-ui/src/components/CoinIcon.tsx | Adds icon rendering with SVG fetch + caching + fallback icon. |
| frontend/packages/wallet-ui/src/components/BalanceDisplay.tsx | Uses active wallet view balance from store/API. |
| frontend/packages/wallet-ui/package.json | Adds react-native-svg peer dependency for SVG rendering. |
| frontend/packages/ui/src/index.ts | Exports new public amount formatters. |
| frontend/packages/ui/src/icons/icon-types.ts | Adds coin-fallback icon type. |
| frontend/packages/ui/src/icons/icon-registry.ts | Registers coin-fallback icon component. |
| frontend/packages/ui/src/icons/generated/CoinFallbackIcon.tsx | Adds generated fallback coin icon component. |
| frontend/packages/ui/src/icons/assets/icon_coin-fallback.svg | Adds SVG asset for fallback coin icon. |
| frontend/packages/ui/src/formatters/format-usd-amount.ts | Adds USD formatter with tiny-value handling. |
| frontend/packages/ui/src/formatters/format-usd-amount.test.ts | Tests USD formatter behavior. |
| frontend/packages/ui/src/formatters/format-subscript-notation.ts | Adds subscript formatting for very small numbers. |
| frontend/packages/ui/src/formatters/format-subscript-notation.test.ts | Tests subscript formatting logic. |
| frontend/packages/ui/src/formatters/format-crypto-amount.ts | Adds crypto amount formatter (abbrev/decimals/subscript). |
| frontend/packages/ui/src/formatters/format-crypto-amount.test.ts | Tests crypto amount formatting behavior. |
| frontend/packages/ui/src/components/SearchBar.tsx | Adds autoFocus prop + focused border state. |
| frontend/packages/network/src/axios-transport.ts | Treats empty/nullable response bodies as valid undefined. |
| frontend/packages/identity-client/src/index.ts | Exports WalletViewAggregationWallet type. |
| frontend/packages/identity-client/src/data-sources/wallet-views-data-source.ts | Silences parse-error delete warning and treats parse errors as success. |
| frontend/apps/web/src/components/splash-screen.tsx | Initializes wallet client + loads wallet data on authenticated app entry. |
| frontend/apps/web/src/app.tsx | Loads wallet views on auth success via shared handler. |
| frontend/apps/web/package.json | Bumps Next.js to ^15.5.15 and dependency ordering changes. |
| frontend/apps/mobile/src/components/splash-screen.tsx | Initializes wallet client + loads wallet data on authenticated app entry. |
| frontend/apps/mobile/package.json | Adds @ion/wallet dependency. |
| frontend/apps/mobile/App.tsx | Loads wallet views on auth success via shared handler. |
Files not reviewed (1)
- frontend/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (5)
frontend/packages/wallet/src/converters/search-aggregation-item.ts:1
buildAggregationWalletKeyuses the contract string as-is, butisMatchingWalletcompares contract addresses case-insensitively. If the API returns the same contract with different casing across entries, theconsumedde-duplication inconvertWalletViewwill fail and balances may be double-counted. Normalize the contract (and possiblycoinId) in the key (e.g., lowercasing contract) to make de-duplication consistent with matching logic.
frontend/packages/wallet/src/converters/from-blockchain-units.ts:1- This conversion returns a JavaScript
numberviaparseFloat, which will lose precision for many realistic on-chain balances (large integers and/or highdecimals, e.g., 18). Because the PR uses this output to compute USD totals and ordering, precision loss can lead to visibly incorrect balances and sorting. Consider returning a decimal-safe representation (e.g., string) and formatting from that, or using a decimal/bignumber library for arithmetic while keeping UI formatting separate.
frontend/packages/wallet/src/converters/convert-wallet-view.ts:1 coin.decimals > 0will filter out coins withdecimals === 0, which are valid for some assets (andfromBlockchainUnitsexplicitly supports0decimals). This will cause those coins to disappear from the wallet view entirely. Allowdecimals >= 0(and keep the other validation checks) so 0-decimal assets are not dropped.
frontend/packages/wallet/src/stores/wallet-notification-config.ts:1- The PR description mentions a “console.warn fallback” for wallet error notifications, but
notifyWalletErrorcurrently no-ops if no notifier was registered. To match the stated behavior (and avoid silent failures), implement a fallback behavior whenerrorNotifierisnull(e.g.,console.warn("[wallet]", message)), or update the PR description if the fallback is intentionally handled elsewhere.
frontend/packages/wallet-ui/src/screens/use-wallet-error-notifier.ts:1 - This hook registers a module-level notifier but never clears it on unmount. If the screen is mounted multiple times (or in multi-root scenarios), the notifier can become stale or be overwritten unexpectedly. Add a cleanup function in the
useEffectthat callsclearWalletErrorNotifier()to ensure lifecycle correctness and reduce cross-screen coupling.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+32
| const svgCache = new Map<string, string>(); | ||
| const failedUrls = new Set<string>(); | ||
|
|
||
| function isSvgUrl(url: string): boolean { | ||
| return url.toLowerCase().endsWith(".svg"); | ||
| } | ||
|
|
||
| function useSvgXml(uri: string | null) { | ||
| const isSvg = uri !== null && isSvgUrl(uri); | ||
| const cached = isSvg ? svgCache.get(uri) : undefined; | ||
| const [xml, setXml] = useState<string | null>(cached ?? null); | ||
|
|
||
| useEffect(() => { | ||
| if (!isSvg || svgCache.has(uri) || failedUrls.has(uri)) return; | ||
| let cancelled = false; | ||
| fetch(uri) | ||
| .then((res) => res.text()) | ||
| .then((text) => { | ||
| svgCache.set(uri, text); | ||
| if (!cancelled) setXml(text); | ||
| }) | ||
| .catch(() => { | ||
| failedUrls.add(uri); |
There was a problem hiding this comment.
svgCache and failedUrls are unbounded module-level caches; over time (especially with many icons / changing URLs) they can grow without limit and increase memory usage. Add an eviction policy (size cap + LRU), or periodically clear entries, and consider scoping the cache to a higher-level image cache if one exists in the app.
Suggested change
| const svgCache = new Map<string, string>(); | |
| const failedUrls = new Set<string>(); | |
| function isSvgUrl(url: string): boolean { | |
| return url.toLowerCase().endsWith(".svg"); | |
| } | |
| function useSvgXml(uri: string | null) { | |
| const isSvg = uri !== null && isSvgUrl(uri); | |
| const cached = isSvg ? svgCache.get(uri) : undefined; | |
| const [xml, setXml] = useState<string | null>(cached ?? null); | |
| useEffect(() => { | |
| if (!isSvg || svgCache.has(uri) || failedUrls.has(uri)) return; | |
| let cancelled = false; | |
| fetch(uri) | |
| .then((res) => res.text()) | |
| .then((text) => { | |
| svgCache.set(uri, text); | |
| if (!cancelled) setXml(text); | |
| }) | |
| .catch(() => { | |
| failedUrls.add(uri); | |
| const MAX_SVG_CACHE_ENTRIES = 100; | |
| const MAX_FAILED_URL_CACHE_ENTRIES = 100; | |
| const svgCache = new Map<string, string>(); | |
| const failedUrls = new Map<string, true>(); | |
| function getLruCacheValue<K, V>(cache: Map<K, V>, key: K): V | undefined { | |
| const value = cache.get(key); | |
| if (value === undefined) return undefined; | |
| cache.delete(key); | |
| cache.set(key, value); | |
| return value; | |
| } | |
| function hasLruCacheKey<K, V>(cache: Map<K, V>, key: K): boolean { | |
| const value = cache.get(key); | |
| if (value === undefined) return false; | |
| cache.delete(key); | |
| cache.set(key, value); | |
| return true; | |
| } | |
| function setLruCacheValue<K, V>(cache: Map<K, V>, key: K, value: V, maxEntries: number) { | |
| if (cache.has(key)) { | |
| cache.delete(key); | |
| } | |
| cache.set(key, value); | |
| if (cache.size > maxEntries) { | |
| const oldestKey = cache.keys().next().value as K | undefined; | |
| if (oldestKey !== undefined) { | |
| cache.delete(oldestKey); | |
| } | |
| } | |
| } | |
| function isSvgUrl(url: string): boolean { | |
| return url.toLowerCase().endsWith(".svg"); | |
| } | |
| function useSvgXml(uri: string | null) { | |
| const isSvg = uri !== null && isSvgUrl(uri); | |
| const cached = isSvg ? getLruCacheValue(svgCache, uri) : undefined; | |
| const [xml, setXml] = useState<string | null>(cached ?? null); | |
| useEffect(() => { | |
| if (!isSvg || hasLruCacheKey(svgCache, uri) || hasLruCacheKey(failedUrls, uri)) return; | |
| let cancelled = false; | |
| fetch(uri) | |
| .then((res) => res.text()) | |
| .then((text) => { | |
| setLruCacheValue(svgCache, uri, text, MAX_SVG_CACHE_ENTRIES); | |
| if (!cancelled) setXml(text); | |
| }) | |
| .catch(() => { | |
| setLruCacheValue(failedUrls, uri, true, MAX_FAILED_URL_CACHE_ENTRIES); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsx (1)
19-24:⚠️ Potential issue | 🟡 MinorConsider logging the suppressed error.
The catch block silently swallows the error. As per coding guidelines, every catch block should re-throw, return a fallback, or log the error. While the comment explains the intent, adding a debug log would aid troubleshooting.
🛡️ Suggested fix
try { switchWalletView(walletId); } catch { - // wallet view not found — ignore + // wallet view not found — ignore (user already sees no change) + console.debug("switchWalletView: wallet not found", walletId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsx` around lines 19 - 24, The try/catch around switchWalletView(walletId) in WalletViewSwitcherScreen.tsx currently swallows errors; change the catch to capture the error (e.g., catch (err)) and emit a debug/log message including the walletId and error details (use the app's logger if available or console.debug) before continuing to preserve the original “ignore” behavior.
🧹 Nitpick comments (12)
frontend/packages/ui/src/components/SearchBar.tsx (1)
63-65: Replace hardcoded border width with a named/tokenized value.Line 64 uses a raw
1forborderWidth. Please source it from scale/theme or a named constant to avoid magic numbers.Suggested change
- const focusedBorder = useMemo(() => ( - isFocused ? { borderWidth: 1, borderColor: theme.colors.primaryAccent } : null - ), [isFocused, theme.colors.primaryAccent]); + const focusedBorderWidth = scale(1); + const focusedBorder = useMemo(() => ( + isFocused ? { borderWidth: focusedBorderWidth, borderColor: theme.colors.primaryAccent } : null + ), [isFocused, focusedBorderWidth, theme.colors.primaryAccent]);As per coding guidelines: "Do not use magic numbers, metaprogramming, decorators (except framework-required ones), or 'eval()'."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/components/SearchBar.tsx` around lines 63 - 65, The focusedBorder useMemo currently uses a magic number for borderWidth; replace the literal `1` with a tokenized value from your theme or a named constant (e.g. use theme.borderWidths.small or a project constant like BORDER_WIDTH_SMALL) so the style becomes { borderWidth: <theme token>, borderColor: theme.colors.primaryAccent } when isFocused is true; update the reference in the focusedBorder expression and ensure the theme token/constant is imported/accessible where useMemo is defined (focusedBorder, useMemo, isFocused, theme.colors.primaryAccent).frontend/packages/ui/src/formatters/format-subscript-notation.test.ts (1)
19-47: Consider adding edge-case scenarios for zero and out-of-range values.Current cases are good for tiny numbers, but adding explicit expectations for
0and non-subscript ranges would lock in safe fallback behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/formatters/format-subscript-notation.test.ts` around lines 19 - 47, Add unit tests to cover edge cases for formatSubscriptNotation: include explicit expectations for input 0 (and -0) to ensure it returns a sensible fallback like "0" or with currency prefix, and add tests for values that are outside the tiny-subscript range (e.g., typical decimals like 1.23 and small-but-not-subscript values like 0.01) to assert they use normal formatting rather than subscript notation; update test cases in format-subscript-notation.test.ts referencing the formatSubscriptNotation function and include checks for currency behavior with zero and non-subscript values.frontend/packages/wallet/src/stores/wallet-view-store.ts (1)
6-7: Deduplicate the default wallet view initializerLine 6 and Line 58 duplicate the same
WalletViewliteral. Extracting a shared factory/constant will prevent drift when theWalletViewshape changes again.♻️ Suggested refactor
+function createDefaultWalletView(): WalletView { + return { + id: "1", + name: "ion.wallet", + balance: "$0.00", + isMain: true, + coinGroups: [], + isLoading: false, + serverId: null, + originalItems: [], + originalSymbolGroups: [], + }; +} + let walletViews: readonly WalletView[] = [ - { id: "1", name: "ion.wallet", balance: "$0.00", isMain: true, coinGroups: [], isLoading: false, serverId: null, originalItems: [], originalSymbolGroups: [] }, + createDefaultWalletView(), ]; @@ export function resetWalletViewStore(): void { walletViews = [ - { id: "1", name: "ion.wallet", balance: "$0.00", isMain: true, coinGroups: [], isLoading: false, serverId: null, originalItems: [], originalSymbolGroups: [] }, + createDefaultWalletView(), ];Also applies to: 57-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/stores/wallet-view-store.ts` around lines 6 - 7, Duplicate inline WalletView literal (id: "1", name: "ion.wallet", balance: "$0.00", isMain: true, coinGroups: [], isLoading: false, serverId: null, originalItems: [], originalSymbolGroups: []) appears twice; extract a single reusable constant or factory (e.g., DEFAULT_WALLET_VIEW or createDefaultWalletView()) and replace both literals with references to it, making sure to return a fresh copy (deep clone) if the store mutates the object to avoid shared state; update usages in wallet-view-store.ts where the literal is currently initialized and where it appears again.frontend/packages/wallet/src/converters/compare-coin-groups.test.ts (1)
101-119: Add a coin-level test for low-balance ICE demotion
compareCoinsreuses the ICE threshold rule, but the suite only asserts that behavior for groups. A dedicated coin-level case would better protect against regressions in shared comparator logic.✅ Suggested test case
describe('compareCoins', () => { + it('pushes ICE coin with balance < $0.01 behind non-ICE coin', () => { + const ice = makeCoinWithBalance({symbolGroup: 'ice'}, {balanceUSD: 0.001}); + const other = makeCoinWithBalance({symbolGroup: 'random'}, {balanceUSD: 0}); + expect(compareCoins(ice, other)).toBeGreaterThan(0); + }); + it('sorts native before non-native within same group', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/converters/compare-coin-groups.test.ts` around lines 101 - 119, The test suite lacks a coin-level assertion that low-balance ICE coins get demoted by compareCoins; add a unit test in compare-coin-groups.test.ts that uses makeCoinWithBalance to create an ICE coin with a very low balance (e.g., balanceUSD: 0) and a non-ICE coin with a higher balance, then assert compareCoins(lowBalanceIceCoin, higherBalanceOtherCoin) indicates the ICE coin is sorted after (i.e., returns > 0), referencing compareCoins and makeCoinWithBalance to locate the comparator and factory.frontend/apps/web/src/components/splash-screen.tsx (1)
41-41: Use structured logger instead ofconsole.errorfor wallet preload failures.On Line 41, this bypasses the diagnostics pipeline used elsewhere in the file, which makes production triage harder.
Suggested refinement
- loadWalletViewData().catch(console.error); + loadWalletViewData().catch((error: unknown) => { + Logger.error("Wallet data load failed during splash", { + tag: "wallet", + error: error instanceof Error ? error : new Error(String(error)), + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/apps/web/src/components/splash-screen.tsx` at line 41, Replace the inline console.error call used on the loadWalletViewData promise with the component's structured logger used elsewhere in this file (e.g., logger.error or the file's existing logging utility) so failures flow through the diagnostics pipeline; call the structured logger with a clear contextual message like "failed to preload wallet view" and include the caught error object/stack and any relevant identifiers so the log contains structured fields for easier production triage.frontend/packages/ui/src/formatters/format-usd-amount.test.ts (1)
4-4: Use scenario-based suite naming instead of function-name naming.
describe("formatUsdAmount")is function-centric. Rename the suite to a scenario/context style (e.g.,"when formatting USD balances").As per coding guidelines, tests should “name tests by scenario, not by function name.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/formatters/format-usd-amount.test.ts` at line 4, Rename the test suite from the function-centric title to a scenario/context style: update the describe call currently using "formatUsdAmount" to something like "when formatting USD balances" (or another scenario-focused phrase) in the test file so the suite names tests by scenario rather than by the function name (locate the describe("formatUsdAmount", ...) and change its string accordingly).frontend/packages/wallet/src/stores/wallet-client-config.test.ts (1)
8-8: Rename suite to a scenario-oriented description.
describe("walletClientConfig")is implementation-centric; prefer scenario wording like"when wallet client config is initialized/reset".As per coding guidelines, tests should be named by scenario, not by function name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/stores/wallet-client-config.test.ts` at line 8, Rename the test suite description from the implementation-centric "walletClientConfig" to a scenario-oriented phrase (e.g., "when wallet client config is initialized/reset") so the describe block expresses the test scenario; update the string argument passed to describe(...) where the suite is declared to the new scenario wording.frontend/apps/mobile/src/components/splash-screen.tsx (1)
31-33: Use structured logger for wallet-load failure in splash path.Line 32 uses
console.error; this file already usesLogger.errorwith structured context. PreferLogger.errorhere for consistent diagnostics and filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/apps/mobile/src/components/splash-screen.tsx` around lines 31 - 33, Replace the unstructured console.error used on the loadWalletViewData promise rejection with the shared structured logger: change the catch handler on loadWalletViewData() to call Logger.error and pass the error as structured context (e.g., Logger.error("Failed to load wallet view data", { err, source: "splash-screen", action: "loadWalletViewData" })), keeping initializeWalletClient(identityClient, users[0]!) unchanged; ensure the error object is passed under a key like err to match existing logging conventions.frontend/packages/wallet/src/converters/from-blockchain-units.test.ts (1)
4-4: Rename top-level suite to scenario language.
describe('fromBlockchainUnits')is implementation-centric; prefer scenario wording (e.g.,"when converting blockchain units to display amounts").As per coding guidelines, tests should be named by scenario, not by function name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/converters/from-blockchain-units.test.ts` at line 4, Rename the top-level test suite from the implementation-centric title "fromBlockchainUnits" to a scenario description like "when converting blockchain units to display amounts"; locate the describe block that currently references fromBlockchainUnits and replace its string argument with a scenario-focused phrase so the suite reads as behavior-driven test naming rather than a function name.frontend/packages/ui/src/formatters/format-usd-amount.ts (1)
6-6: Align formatter API with string-based monetary type guideline.
formatUsdAmountcurrently acceptsnumber; repo guidance requires monetary amounts to usestringat boundaries. Please switch the input type tostring(with parsing/validation inside) for consistency.As per coding guidelines, monetary amounts should use
stringtype.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/formatters/format-usd-amount.ts` at line 6, The formatUsdAmount function currently takes a number but must accept a string per monetary-boundary guidelines; change the signature of formatUsdAmount to accept value: string, parse and validate the string inside (e.g., trim, ensure it represents a valid numeric amount using parseFloat/Number or a decimal library), handle invalid input gracefully (throw a clear Error or return a fallback), and then format the parsed numeric value as USD; update any internal references to use the parsed number while keeping the function name formatUsdAmount unchanged.frontend/packages/wallet/src/actions/rename-wallet-view.test.ts (1)
6-8: Avoid timer-based promise flushing in this suite.Using
setTimeout(..., 0)can become flaky when fake timers leak from other tests. Prefer microtask flushing (await Promise.resolve()) and/or force real timers in teardown.♻️ Suggested change
-function flushPromises(): Promise<void> { - return new Promise((resolve) => setTimeout(resolve, 0)); -} +async function flushPromises(): Promise<void> { + await Promise.resolve(); +}Based on learnings: "In Vitest tests, do not assume
vi.clearAllMocks()orvi.restoreAllMocks()will reset fake timers... Prefer callingvi.useRealTimers()inafterEach."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/actions/rename-wallet-view.test.ts` around lines 6 - 8, The test helper flushPromises() uses setTimeout(..., 0) which is flaky with fake timers; replace it to flush microtasks by returning Promise.resolve() (or using await Promise.resolve() in callers) and add a test teardown to ensure real timers are restored by calling vi.useRealTimers() in afterEach; update the helper function name flushPromises and any tests that call it to use the microtask-based implementation and ensure afterEach invokes vi.useRealTimers() to avoid leaked fake timers.frontend/packages/wallet/src/converters/search-aggregation-item.test.ts (1)
52-53: Use scenario-focused suite names instead of function names.The current
describe(...)labels are implementation-centric; please rename them to behavior scenarios so tests read as product rules.As per coding guidelines, "Write tests that verify behavior and scenarios, not implementation details; name tests by scenario, not by function name."
Also applies to: 93-94, 135-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/converters/search-aggregation-item.test.ts` around lines 52 - 53, The test suites are named after the implementation (describe('isMatchingWallet') etc. at the three locations), but should be scenario-focused; update the describe labels (including the suites currently named 'isMatchingWallet' and the other two described at the referenced locations) to describe user-facing scenarios or behavior (e.g., "when wallet matches by id or contract address (case-insensitive)" or "when wallet does not match due to different contract") and adjust any it(...) titles if needed so the suite and specs read as product behavior rules rather than function names; keep the existing test logic and references to the helper/assertion functions intact (e.g., any uses of isMatchingWallet in tests remain unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/network/src/axios-transport.ts`:
- Around line 100-101: The function parseResponseData<T> illegally masks
undefined by returning "undefined as T" which breaks the TransportResponse<T>
contract and creates unsafe usage in callers like user-data-source.ts and
wallet-views-data-source.ts; fix by making the response/body type explicit
(e.g., change parseResponseData to return T | undefined and update
TransportResponse<T> to allow body?: T or adjust specific endpoint types to
TransportResponse<void> for 204/empty responses), then update the callers
(identity-client/user-data-source.ts and wallet-views-data-source.ts) to handle
undefined (null-checks or treat as void) or change those API method signatures
to return TransportResponse<void> when no content is expected. Ensure
references: parseResponseData, TransportResponse<T>, and the mentioned caller
files are updated accordingly so no "as T" cast remains.
In `@frontend/packages/ui/src/formatters/format-subscript-notation.ts`:
- Around line 3-10: This file currently exports two functions (toSubscript and
formatSubscriptNotation); enforce the one-export-per-file rule by either making
toSubscript internal (remove its export keyword and keep it as a private helper
inside this file) or by moving toSubscript into its own file that exports a
single function (e.g., export function toSubscript(...) in a new file named
toSubscript.ts). Update any call sites/imports that referenced toSubscript to
the new path if you move it, or leave imports unchanged if you make it
non-exported and only used here; ensure this file now only publicly exports
formatSubscriptNotation.
- Around line 10-25: formatSubscriptNotation can produce negative zeroCount for
zero or non-small inputs; add an early guard after computing exponentMatch (in
formatSubscriptNotation) to return a simple fallback (e.g.,
`${sign}${symbol}${value}` or an empty string) when value === 0 or when the
computed exponent would make zeroCount < 0 (i.e., exponent <= 0 or
Number(exponentMatch[2]) >= 0 depending on your intended range). Ensure this
check runs before using toSubscript(zeroCount) so you never call toSubscript
with a negative count.
In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx`:
- Around line 17-38: useSvgXml can show stale SVGs and hang on failures because
state (xml) isn’t reset when uri changes and failures don’t force a fallback;
update the hook so that when uri or isSvg changes you immediately setXml(cached
?? null) to synchronize state with the new uri, and in the fetch .catch block
call setXml(null) in addition to adding uri to failedUrls so the component can
render the fallback; keep the existing svgCache/failedUrls logic and cancel
flag/cleanup in useEffect and reference the useSvgXml function, svgCache,
failedUrls, xml, and setXml so you modify the right places.
In `@frontend/packages/wallet-ui/src/components/CoinList.tsx`:
- Around line 49-52: The filter uses the raw searchQuery so leading/trailing
spaces prevent matches; trim the query before using it in the useMemo: compute a
trimmedQuery (e.g., const trimmedQuery = searchQuery.trim()) and use
trimmedQuery in the emptiness check and when calling matchesSearch(group,
trimmedQuery) inside the useMemo for filteredGroups, keeping coinGroups and
searchQuery in the dependency array.
In `@frontend/packages/wallet-ui/src/screens/use-wallet-error-notifier.ts`:
- Around line 5-9: The hook registers a global notifier via
setWalletErrorNotifier inside useEffect but never unregisters it, causing a
stale global callback; modify the useEffect to return a cleanup function that
restores the previous notifier (or sets the notifier to null/no-op) when the
component unmounts, ensuring setWalletErrorNotifier is called again in the
cleanup to remove the handler and avoid stale callbacks.
In `@frontend/packages/wallet/src/actions/create-wallet-view.test.ts`:
- Around line 3-5: Add a beforeEach block to clear global test state by calling
resetWalletClient() and resetting the wallet view store (e.g.,
walletViewStore.reset() or the store's API to restore its initial state) so each
test starts with a fresh walletClient and walletViewStore; place this beforeEach
near the top of create-wallet-view.test.ts alongside the existing imports
(walletViewStore, initializeWalletClient, resetWalletClient) to prevent state
leakage across tests.
In `@frontend/packages/wallet/src/actions/create-wallet-view.ts`:
- Around line 41-44: revertOptimisticCreate currently unconditionally restores
previousActiveId which can overwrite a newer active selection; change it to read
the current active id from walletViewStore.getActiveId() and only pass
previousActiveId into batchUpdate if the current active equals the tempId being
removed, otherwise keep the current active; apply the same guard to the other
rollback that sets previousActiveId at the 75-77 block so you only restore the
previous active when the removed temp view was still the active one.
- Around line 46-53: createWalletView inserts an optimistic wallet via
addOptimisticWallet then calls getWalletClient(), but getWalletClient() can
throw synchronously leaving the optimistic item stuck; wrap the
getWalletClient() call (and the subsequent client.createWalletView promise
chain) in a try/catch so that if getWalletClient() throws you revert the
optimistic state (remove the optimistic wallet and restore previousActiveId
using walletViewStore and any ID returned by addOptimisticWallet), then rethrow
or return early; reference createWalletView, getWalletClient,
addOptimisticWallet, and walletViewStore when making the changes.
In `@frontend/packages/wallet/src/actions/delete-wallet-view.test.ts`:
- Around line 56-63: The test scenario name doesn't match the asserted behavior:
currently it asserts the main-wallet guard when deleting wallet "1" but is named
"throws when trying to delete the last wallet"; update the test to make name and
assertions consistent—either rename the it(...) text to something like "throws
when trying to delete the main wallet" to match the deleteWalletView("1")
assertion, or change the setup/assertion to actually test deleting the last
wallet (e.g., attempt to delete the only remaining wallet after calling
createWalletView("Second"), deleteWalletView(secondId), then call
deleteWalletView(<remainingId>) and expect the "last wallet" behavior). Update
the description string accordingly and keep the calls to createWalletView,
getSecondWalletId, deleteWalletView, and walletViewStore.getWalletViews intact.
In `@frontend/packages/wallet/src/actions/delete-wallet-view.ts`:
- Around line 29-31: The current revertDelete function replaces the entire view
state from DeleteSnapshot (previousViews/previousActiveId) which can clobber
concurrent updates; change it to a merge-based rollback in revertDelete: read
the current views and activeId via the same selectors/getters used elsewhere,
compute a new views list by re-adding only view(s) present in
snapshot.previousViews that are missing from currentViews (do not overwrite
existing views), and only update the activeId if the current activeId equals the
id that was deleted in this snapshot (or is null) to avoid overriding newer
active changes; then call batchUpdate with the merged views and the computed
activeId. Ensure you reference DeleteSnapshot.previousViews and
DeleteSnapshot.previousActiveId and use the same view id(s) logic used in the
delete flow so rollback is conflict-safe.
In `@frontend/packages/wallet/src/actions/rename-wallet-view.ts`:
- Around line 21-24: The rollback currently unconditionally resets the view name
to previousName on any failed request; change the catch handler to first read
the current view via walletViewStore.getWalletViews(), find the view with
walletId, and only call setWalletViews to revert (mapping w.id === walletId ? {
...w, name: previousName } : w) if that view exists and its name strictly equals
the optimistic new name (newName) used by the failed request; otherwise skip the
revert and still call notifyWalletError. Ensure you reference
walletViewStore.getWalletViews(), walletId, previousName, newName,
setWalletViews and notifyWalletError in the fix.
In `@frontend/packages/wallet/src/actions/switch-wallet-view.test.ts`:
- Around line 23-30: The test reads walletViewStore immediately after calling
createWalletView("Second"), causing a race with the async creation; update the
test to wait for the created view to be finalized before using switchWalletView:
either await the promise returned by createWalletView("Second") if it returns
one, or poll/wait on walletViewStore.getWalletViews() until the created view's
id is not the optimistic/temp id (or its isOptimistic flag is false), then use
that finalized id (secondId) with switchWalletView and assert
getActiveWalletViewId() accordingly.
In `@frontend/packages/wallet/src/converters/compare-coin-groups.ts`:
- Around line 62-75: The file currently exports two functions (compareGroups and
compareCoins); split them so each file exports exactly one function: move
compareGroups into its own file named to match the export and keep compareCoins
in its own file (or make one of them internal/private by not exporting it).
Update imports/usages to reference the new module paths and ensure both files
import the shared compare utility and hasAnyPrioritized helper as needed; ensure
each file exports only the single function (compareGroups in one, compareCoins
in the other) to satisfy the single-export-per-file rule.
- Around line 49-51: Extract the magic number 0.01 into a named constant (e.g.,
LOW_BALANCE_ICE_THRESHOLD) and use that constant inside isLowBalanceIce instead
of the hardcoded value; locate isLowBalanceIce and replace the inline literal
with the constant, defining the constant near the top of the module (export it
if needed for tests) so the ICE threshold is a single named source of truth.
In `@frontend/packages/wallet/src/converters/convert-wallet-view.ts`:
- Around line 59-63: The code multiplies amount by parseFloat(coin.priceUSD)
which yields NaN if coin.priceUSD is invalid; update convert-wallet-view to
parse and validate the price once (e.g. const price = parseFloat(coin.priceUSD
?? '') and if (!Number.isFinite(price)) set price = 0) and then use that safe
price for balanceUSD and the other occurrences (lines referenced around
balanceUSD, the block at 108-117, and at 131) so downstream balances never
become NaN; reference coin.priceUSD, balanceUSD, and the converted value
locations when applying the change.
In `@frontend/packages/wallet/src/converters/search-aggregation-item.ts`:
- Around line 82-87: The aggregation key generation in buildAggregationWalletKey
preserves contract casing which breaks case-insensitive matching; update
buildAggregationWalletKey (used with WalletViewAggregationWallet and
extractContractAddress on wallet.asset) to normalize the contract address (e.g.,
call toLowerCase() on the extracted contract when present) before composing the
key `${wallet.walletId}|${wallet.network}|...` so identical addresses with
different casing produce the same key; leave fallback to wallet.coinId
unchanged.
In `@frontend/packages/wallet/src/types.ts`:
- Line 21: Update the monetary type annotations from number to string to avoid
precision loss: change the properties balanceUSD, totalBalanceUSD, and
usdBalance in the relevant type/interface definitions (e.g., the type that
currently declares balanceUSD) from number to string, and update any related
type exports so API boundaries use string for monetary amounts instead of
number.
---
Outside diff comments:
In
`@frontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsx`:
- Around line 19-24: The try/catch around switchWalletView(walletId) in
WalletViewSwitcherScreen.tsx currently swallows errors; change the catch to
capture the error (e.g., catch (err)) and emit a debug/log message including the
walletId and error details (use the app's logger if available or console.debug)
before continuing to preserve the original “ignore” behavior.
---
Nitpick comments:
In `@frontend/apps/mobile/src/components/splash-screen.tsx`:
- Around line 31-33: Replace the unstructured console.error used on the
loadWalletViewData promise rejection with the shared structured logger: change
the catch handler on loadWalletViewData() to call Logger.error and pass the
error as structured context (e.g., Logger.error("Failed to load wallet view
data", { err, source: "splash-screen", action: "loadWalletViewData" })), keeping
initializeWalletClient(identityClient, users[0]!) unchanged; ensure the error
object is passed under a key like err to match existing logging conventions.
In `@frontend/apps/web/src/components/splash-screen.tsx`:
- Line 41: Replace the inline console.error call used on the loadWalletViewData
promise with the component's structured logger used elsewhere in this file
(e.g., logger.error or the file's existing logging utility) so failures flow
through the diagnostics pipeline; call the structured logger with a clear
contextual message like "failed to preload wallet view" and include the caught
error object/stack and any relevant identifiers so the log contains structured
fields for easier production triage.
In `@frontend/packages/ui/src/components/SearchBar.tsx`:
- Around line 63-65: The focusedBorder useMemo currently uses a magic number for
borderWidth; replace the literal `1` with a tokenized value from your theme or a
named constant (e.g. use theme.borderWidths.small or a project constant like
BORDER_WIDTH_SMALL) so the style becomes { borderWidth: <theme token>,
borderColor: theme.colors.primaryAccent } when isFocused is true; update the
reference in the focusedBorder expression and ensure the theme token/constant is
imported/accessible where useMemo is defined (focusedBorder, useMemo, isFocused,
theme.colors.primaryAccent).
In `@frontend/packages/ui/src/formatters/format-subscript-notation.test.ts`:
- Around line 19-47: Add unit tests to cover edge cases for
formatSubscriptNotation: include explicit expectations for input 0 (and -0) to
ensure it returns a sensible fallback like "0" or with currency prefix, and add
tests for values that are outside the tiny-subscript range (e.g., typical
decimals like 1.23 and small-but-not-subscript values like 0.01) to assert they
use normal formatting rather than subscript notation; update test cases in
format-subscript-notation.test.ts referencing the formatSubscriptNotation
function and include checks for currency behavior with zero and non-subscript
values.
In `@frontend/packages/ui/src/formatters/format-usd-amount.test.ts`:
- Line 4: Rename the test suite from the function-centric title to a
scenario/context style: update the describe call currently using
"formatUsdAmount" to something like "when formatting USD balances" (or another
scenario-focused phrase) in the test file so the suite names tests by scenario
rather than by the function name (locate the describe("formatUsdAmount", ...)
and change its string accordingly).
In `@frontend/packages/ui/src/formatters/format-usd-amount.ts`:
- Line 6: The formatUsdAmount function currently takes a number but must accept
a string per monetary-boundary guidelines; change the signature of
formatUsdAmount to accept value: string, parse and validate the string inside
(e.g., trim, ensure it represents a valid numeric amount using parseFloat/Number
or a decimal library), handle invalid input gracefully (throw a clear Error or
return a fallback), and then format the parsed numeric value as USD; update any
internal references to use the parsed number while keeping the function name
formatUsdAmount unchanged.
In `@frontend/packages/wallet/src/actions/rename-wallet-view.test.ts`:
- Around line 6-8: The test helper flushPromises() uses setTimeout(..., 0) which
is flaky with fake timers; replace it to flush microtasks by returning
Promise.resolve() (or using await Promise.resolve() in callers) and add a test
teardown to ensure real timers are restored by calling vi.useRealTimers() in
afterEach; update the helper function name flushPromises and any tests that call
it to use the microtask-based implementation and ensure afterEach invokes
vi.useRealTimers() to avoid leaked fake timers.
In `@frontend/packages/wallet/src/converters/compare-coin-groups.test.ts`:
- Around line 101-119: The test suite lacks a coin-level assertion that
low-balance ICE coins get demoted by compareCoins; add a unit test in
compare-coin-groups.test.ts that uses makeCoinWithBalance to create an ICE coin
with a very low balance (e.g., balanceUSD: 0) and a non-ICE coin with a higher
balance, then assert compareCoins(lowBalanceIceCoin, higherBalanceOtherCoin)
indicates the ICE coin is sorted after (i.e., returns > 0), referencing
compareCoins and makeCoinWithBalance to locate the comparator and factory.
In `@frontend/packages/wallet/src/converters/from-blockchain-units.test.ts`:
- Line 4: Rename the top-level test suite from the implementation-centric title
"fromBlockchainUnits" to a scenario description like "when converting blockchain
units to display amounts"; locate the describe block that currently references
fromBlockchainUnits and replace its string argument with a scenario-focused
phrase so the suite reads as behavior-driven test naming rather than a function
name.
In `@frontend/packages/wallet/src/converters/search-aggregation-item.test.ts`:
- Around line 52-53: The test suites are named after the implementation
(describe('isMatchingWallet') etc. at the three locations), but should be
scenario-focused; update the describe labels (including the suites currently
named 'isMatchingWallet' and the other two described at the referenced
locations) to describe user-facing scenarios or behavior (e.g., "when wallet
matches by id or contract address (case-insensitive)" or "when wallet does not
match due to different contract") and adjust any it(...) titles if needed so the
suite and specs read as product behavior rules rather than function names; keep
the existing test logic and references to the helper/assertion functions intact
(e.g., any uses of isMatchingWallet in tests remain unchanged).
In `@frontend/packages/wallet/src/stores/wallet-client-config.test.ts`:
- Line 8: Rename the test suite description from the implementation-centric
"walletClientConfig" to a scenario-oriented phrase (e.g., "when wallet client
config is initialized/reset") so the describe block expresses the test scenario;
update the string argument passed to describe(...) where the suite is declared
to the new scenario wording.
In `@frontend/packages/wallet/src/stores/wallet-view-store.ts`:
- Around line 6-7: Duplicate inline WalletView literal (id: "1", name:
"ion.wallet", balance: "$0.00", isMain: true, coinGroups: [], isLoading: false,
serverId: null, originalItems: [], originalSymbolGroups: []) appears twice;
extract a single reusable constant or factory (e.g., DEFAULT_WALLET_VIEW or
createDefaultWalletView()) and replace both literals with references to it,
making sure to return a fresh copy (deep clone) if the store mutates the object
to avoid shared state; update usages in wallet-view-store.ts where the literal
is currently initialized and where it appears again.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 301967a0-ef2d-4053-bb2d-0f08cda28232
⛔ Files ignored due to path filters (3)
frontend/packages/ui/src/icons/assets/icon_coin-fallback.svgis excluded by!**/*.svgfrontend/packages/ui/src/icons/generated/CoinFallbackIcon.tsxis excluded by!**/generated/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (72)
frontend/apps/mobile/App.tsxfrontend/apps/mobile/package.jsonfrontend/apps/mobile/src/components/splash-screen.tsxfrontend/apps/web/package.jsonfrontend/apps/web/src/app.tsxfrontend/apps/web/src/components/splash-screen.tsxfrontend/packages/identity-client/src/data-sources/wallet-views-data-source.tsfrontend/packages/identity-client/src/index.tsfrontend/packages/network/src/axios-transport.tsfrontend/packages/ui/src/components/SearchBar.tsxfrontend/packages/ui/src/formatters/format-crypto-amount.test.tsfrontend/packages/ui/src/formatters/format-crypto-amount.tsfrontend/packages/ui/src/formatters/format-subscript-notation.test.tsfrontend/packages/ui/src/formatters/format-subscript-notation.tsfrontend/packages/ui/src/formatters/format-usd-amount.test.tsfrontend/packages/ui/src/formatters/format-usd-amount.tsfrontend/packages/ui/src/icons/icon-registry.tsfrontend/packages/ui/src/icons/icon-types.tsfrontend/packages/ui/src/index.tsfrontend/packages/wallet-ui/package.jsonfrontend/packages/wallet-ui/src/components/BalanceDisplay.tsxfrontend/packages/wallet-ui/src/components/CoinIcon.tsxfrontend/packages/wallet-ui/src/components/CoinList.tsxfrontend/packages/wallet-ui/src/components/CoinListItem.tsxfrontend/packages/wallet-ui/src/components/CoinListItemSkeleton.tsxfrontend/packages/wallet-ui/src/components/CoinSearchBar.tsxfrontend/packages/wallet-ui/src/components/CoinsSection.tsxfrontend/packages/wallet-ui/src/components/CoinsSectionCoinsPage.tsxfrontend/packages/wallet-ui/src/components/CoinsSectionSkeleton.tsxfrontend/packages/wallet-ui/src/components/ManageCoinsButton.tsxfrontend/packages/wallet-ui/src/screens/WalletScreen.tsxfrontend/packages/wallet-ui/src/screens/use-wallet-error-notifier.tsfrontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsxfrontend/packages/wallet-ui/src/translations/de.tsfrontend/packages/wallet-ui/src/translations/en.tsfrontend/packages/wallet-ui/src/translations/fr.tsfrontend/packages/wallet/package.jsonfrontend/packages/wallet/src/actions/create-wallet-view.test.tsfrontend/packages/wallet/src/actions/create-wallet-view.tsfrontend/packages/wallet/src/actions/delete-wallet-view.test.tsfrontend/packages/wallet/src/actions/delete-wallet-view.tsfrontend/packages/wallet/src/actions/load-wallet-view-data.test.tsfrontend/packages/wallet/src/actions/load-wallet-view-data.tsfrontend/packages/wallet/src/actions/rename-wallet-view.test.tsfrontend/packages/wallet/src/actions/rename-wallet-view.tsfrontend/packages/wallet/src/actions/switch-wallet-view.test.tsfrontend/packages/wallet/src/actions/switch-wallet-view.tsfrontend/packages/wallet/src/converters/compare-coin-groups.test.tsfrontend/packages/wallet/src/converters/compare-coin-groups.tsfrontend/packages/wallet/src/converters/convert-wallet-view.test.tsfrontend/packages/wallet/src/converters/convert-wallet-view.tsfrontend/packages/wallet/src/converters/extract-contract-address.tsfrontend/packages/wallet/src/converters/from-blockchain-units.test.tsfrontend/packages/wallet/src/converters/from-blockchain-units.tsfrontend/packages/wallet/src/converters/search-aggregation-item.test.tsfrontend/packages/wallet/src/converters/search-aggregation-item.tsfrontend/packages/wallet/src/create-wallet-view.test.tsfrontend/packages/wallet/src/create-wallet-view.tsfrontend/packages/wallet/src/delete-wallet-view.test.tsfrontend/packages/wallet/src/delete-wallet-view.tsfrontend/packages/wallet/src/hooks/use-active-wallet-view.tsfrontend/packages/wallet/src/hooks/use-wallet-views.tsfrontend/packages/wallet/src/index.tsfrontend/packages/wallet/src/rename-wallet-view.test.tsfrontend/packages/wallet/src/rename-wallet-view.tsfrontend/packages/wallet/src/stores/wallet-client-config.test.tsfrontend/packages/wallet/src/stores/wallet-client-config.tsfrontend/packages/wallet/src/stores/wallet-notification-config.tsfrontend/packages/wallet/src/stores/wallet-view-store.test.tsfrontend/packages/wallet/src/stores/wallet-view-store.tsfrontend/packages/wallet/src/switch-wallet-view.test.tsfrontend/packages/wallet/src/types.ts
💤 Files with no reviewable changes (7)
- frontend/packages/wallet/src/delete-wallet-view.test.ts
- frontend/packages/wallet/src/create-wallet-view.test.ts
- frontend/packages/wallet/src/delete-wallet-view.ts
- frontend/packages/wallet/src/rename-wallet-view.test.ts
- frontend/packages/wallet/src/switch-wallet-view.test.ts
- frontend/packages/wallet/src/create-wallet-view.ts
- frontend/packages/wallet/src/rename-wallet-view.ts
| coin: CoinDisplayInfo; | ||
| amount: number; | ||
| rawAmount: string; | ||
| balanceUSD: number; |
There was a problem hiding this comment.
Use string types for monetary fields to avoid precision loss.
balanceUSD, totalBalanceUSD, and usdBalance are monetary values but are typed as number. This can introduce rounding drift in financial displays and API payload transforms.
Proposed type fix
export interface CoinWithBalance {
coin: CoinDisplayInfo;
amount: number;
rawAmount: string;
- balanceUSD: number;
+ balanceUSD: string;
walletId: string | null;
walletAssetContractAddress: string | null;
}
@@
export interface CoinsGroup {
@@
- totalBalanceUSD: number;
+ totalBalanceUSD: string;
}
@@
export interface WalletViewData {
@@
- usdBalance: number;
+ usdBalance: string;
isMainWalletView: boolean;
}As per coding guidelines, “Never use 'any' type on API boundaries; use 'string' type for monetary amounts.”
Also applies to: 33-33, 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/wallet/src/types.ts` at line 21, Update the monetary type
annotations from number to string to avoid precision loss: change the properties
balanceUSD, totalBalanceUSD, and usdBalance in the relevant type/interface
definitions (e.g., the type that currently declares balanceUSD) from number to
string, and update any related type exports so API boundaries use string for
monetary amounts instead of number.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/identity-client/src/data-sources/two-fa-data-source.ts (1)
81-85:⚠️ Potential issue | 🟠 MajorAdd fallback for optional response body (Line 85).
HttpResponse<T>explicitly typesbodyasT | undefined. The non-null assertionbody!suppresses the TypeScript warning but does not prevent runtimeundefined, which would violate the function's return contractPromise<RequestTwoFAResponse>. Other data sources in the same module validate before returning (e.g.,validateActionChallengeResponse); this function lacks that guard.Use a concrete fallback:
return body ?? {};since{}is a validRequestTwoFAResponse(matchesRecord<string, never>).Proposed fix
const { body } = await httpClient.put<RequestTwoFAResponse>( buildVerificationUrl(options.userId, options.twoFAOption), { body: input, headers: buildHeaders(options.username, options.userAction) }, ); - return body!; + return body ?? {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/identity-client/src/data-sources/two-fa-data-source.ts` around lines 81 - 85, The code currently returns body! from the HTTP PUT in the two-fa flow which can be undefined at runtime; update the return to provide a concrete fallback instead of a non-null assertion: after the httpClient.put call in the function that calls buildVerificationUrl and returns RequestTwoFAResponse, replace the direct non-null assertion (body!) with a guard that returns body ?? {} (an empty object matching RequestTwoFAResponse) or reuse the module's validateActionChallengeResponse pattern to verify and return a safe default when body is undefined.
♻️ Duplicate comments (1)
frontend/packages/ui/src/formatters/format-subscript-notation.ts (1)
10-20:⚠️ Potential issue | 🟡 MinorGuard non-small values before building subscript notation.
Line 19 filters only non-negative exponents, so values like
0.1(exponent-1) still produce malformed output (0.0₀1). Add an early guard for unsupported ranges (e.g.,absValue === 0 || absValue >= 0.1) before computingzeroCount.Proposed fix
export function formatSubscriptNotation(value: number, symbol = ""): string { const absValue = Math.abs(value); + if (absValue === 0 || absValue >= 0.1) { + return ""; + } + const exponentMatch = absValue.toExponential(12).match(/^(\d(?:\.\d+)?)e([+-]\d+)$/); if (!exponentMatch) { return ""; } @@ if (Number(exponentMatch[2]) >= 0) return ""; const zeroCount = exponent - 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/formatters/format-subscript-notation.ts` around lines 10 - 20, In formatSubscriptNotation, guard against unsupported magnitudes before building subscript output: check absValue and return "" early when absValue === 0 or absValue >= 0.1 (i.e., not a "small" negative-exponent number) so you don't proceed after computing exponentMatch/zeroCount; keep the existing exponentMatch and negative-exponent checks but add this early range guard (referencing absValue, exponentMatch, zeroCount) to avoid malformed outputs like for 0.1 or 0.
🧹 Nitpick comments (3)
frontend/packages/wallet-ui/src/components/CoinIcon.tsx (1)
10-11: Consider boundingsvgCache/failedUrlsgrowth.Both collections are unbounded and can grow indefinitely with high-cardinality URLs (e.g., cache-busted asset URLs). A small LRU/size cap would prevent gradual memory growth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx` around lines 10 - 11, svgCache and failedUrls are unbounded and can grow indefinitely; limit their size by adding eviction logic (LRU or simple size cap). Replace or wrap the current Map and Set (svgCache, failedUrls) with a bounded structure: introduce constants like MAX_SVG_CACHE_SIZE and MAX_FAILED_URLS and, whenever inserting into svgCache or failedUrls, check size and evict the oldest entry (for svgCache use Map insertion-order to delete the first key; for failedUrls either switch to a Map with timestamps or remove the first value from the Set via values().next()). Ensure insert/remove operations used by the existing code paths (those referencing svgCache and failedUrls) use the new bounded behavior so memory cannot grow unbounded.frontend/packages/ui/src/formatters/format-subscript-notation.ts (1)
12-23: Replace embedded numeric literals with named constants.Line 12 (
12), Line 20 (1), and Line 23 (2) are configuration values; extracting them improves readability and future tuning.Proposed refactor
const SUBSCRIPT_DIGITS = ["₀", "₁", "₂", "₃", "₄", "₅", "₆", "₇", "₈", "₉"]; +const EXPONENTIAL_PRECISION = 12; +const ZERO_COUNT_OFFSET = 1; +const TRAILING_SIGNIFICANT_DIGITS = 2; @@ - const exponentMatch = absValue.toExponential(12).match(/^(\d(?:\.\d+)?)e([+-]\d+)$/); + const exponentMatch = absValue + .toExponential(EXPONENTIAL_PRECISION) + .match(/^(\d(?:\.\d+)?)e([+-]\d+)$/); @@ - const zeroCount = exponent - 1; + const zeroCount = exponent - ZERO_COUNT_OFFSET; @@ - const trailing = digits.substring(0, 2).replace(/0+$/, "") || "0"; + const trailing = + digits.substring(0, TRAILING_SIGNIFICANT_DIGITS).replace(/0+$/, "") || "0";As per coding guidelines, “Do not use magic numbers, metaprogramming, decorators (except framework-required ones), or
eval().”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/formatters/format-subscript-notation.ts` around lines 12 - 23, Replace the magic numbers used in the exponential formatting: extract the numeric literals passed to absValue.toExponential(12), the subtraction by 1 when computing zeroCount (exponent - 1), and the substring length 2 used when building trailing into named constants (e.g., MAX_EXP_FRACTION_DIGITS, ZERO_OFFSET, TRAILING_DIGITS). Update the code around the exponentMatch creation (absValue.toExponential(...)), the zeroCount computation (zeroCount = exponent - ...), and the trailing substring operation (digits.substring(0, ...)) to use these constants; keep the existing logic for mantissa, digits, exponent parsing, and the negative-exponent early return unchanged.frontend/packages/wallet/src/converters/search-aggregation-item.ts (1)
8-88: Split this module to satisfy the single-export file rule.This file exports three functions (
isMatchingWallet,searchAggregationItem,buildAggregationWalletKey). The repository guideline requires exactly one export per file, so these should be split into separate files (or only one kept exported and others made internal where possible).As per coding guidelines: "Each file must export exactly one class, one hook, one function, or one component; the file name must match the export".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/converters/search-aggregation-item.ts` around lines 8 - 88, This file currently exports three functions; split them so each file exports exactly one function (file name matching the export) or make helpers internal: create isMatchingWallet.ts exporting the isMatchingWallet function (or keep it non-exported if only used by searchAggregationItem), create searchAggregationItem.ts exporting searchAggregationItem (importing isMatchingWallet from its file or using a local non-exported helper), and create buildAggregationWalletKey.ts exporting buildAggregationWalletKey; update imports where these functions are used, ensure each new file has a single export (prefer default or named per project convention) and keep any helper-only functions non-exported to satisfy the single-export-per-file rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/config/src/remote-config/fetch-config-from-network.ts`:
- Line 32: The code uses a non-null assertion when calling
identity.parser(response.body!) which can pass undefined into the parser if a
200 response has no body; update the fetch-config-from-network logic to
explicitly check when response.status === 200 that response.body is not
null/undefined and throw a ConfigError with a clear message (e.g., "empty body
for 200 response") if it is missing, then call identity.parser(response.body)
without the non-null assertion; reference identity.parser, response.body, parsed
and ConfigError when making the change.
In `@frontend/packages/file-storage/src/resolve-file.ts`:
- Line 6: The function resolveFile currently returns response.body! without
runtime checks; change it to explicitly guard for undefined by checking
response.body after the HTTP call (e.g., if (!response.body) throw new Error or
return a rejected Promise with a descriptive message) so the function always
fulfills its Promise<FileResolution> contract; update the return to return
response.body when present and include a clear error message mentioning
resolveFile and the request context.
In `@frontend/packages/identity-client/src/auth/execute-signed-request.ts`:
- Around line 53-58: The current execute-signed-request implementation uses a
dead DELETE branch and a non-null assertion on HttpResponse<T>['body'], which
can mask undefined at runtime; remove the unused DELETE branch and explicitly
handle request bodies: call input.httpClient[method] with headers and include
body only for methods that accept it (POST/PUT/PATCH) by checking method and
passing input.body when present, then avoid the "!" by returning resp.body if
defined or throwing a descriptive error (e.g., from executeSignedRequest or the
calling context) when resp.body is undefined so callers like createWallet,
generateSignature, and makeTransfer never receive a falsely-typed undefined.
In `@frontend/packages/identity-client/src/data-sources/coins-data-source.ts`:
- Line 37: Replace all non-null assertions on response.body in
frontend/packages/identity-client/src/data-sources/coins-data-source.ts with
explicit guards: check if the parsed body is undefined and throw a descriptive
error (e.g., "Missing response body for <operation>") from the methods that
currently use body! so the callers receive a proper exception rather than
undefined. Update the try-catch at the PARSE_ERROR handling location to also
catch and handle these missing-body errors (or let them propagate if
appropriate) so they are not swallowed; reference the methods in this file that
currently use body! and the PARSE_ERROR catch block when making the changes.
Ensure return types remain the concrete types promised by each method by only
returning when body is defined.
In
`@frontend/packages/identity-client/src/data-sources/credentials-data-source.ts`:
- Line 57: The three unsafe assertions using body! in credentials-data-source.ts
should be replaced with explicit runtime guards: in each function that currently
does "return body!" (identify by the local variable name body and the file
credentials-data-source.ts), check that response.body is not undefined (and
optionally response.ok) before returning; if the body is undefined throw a clear
error (e.g., throw new Error("Missing response body from <functionName>")) or
return a validated default, and update the function signatures/ callers if
needed to handle the error. Ensure you replace the three occurrences (the ones
at the current return sites) with these checks so undefined cannot leak into
non-optional return types.
In `@frontend/packages/identity-client/src/data-sources/keys-data-source.ts`:
- Line 23: The function listKeys currently returns body! (compile-time
assertion) without a runtime check; add a runtime guard in listKeys to verify
the HTTP response body is present and matches ListKeysResponse before
returning—if body is missing or invalid, either throw a descriptive error (e.g.,
"Empty response from keys API in listKeys") or return a safe default that
conforms to ListKeysResponse; locate the return in listKeys and replace the
body! usage with the guard and appropriate error/default handling.
In `@frontend/packages/identity-client/src/data-sources/networks-data-source.ts`:
- Line 16: The return wraps undefined into [undefined], breaking the
Promise<EstimateFee[]> contract; update the logic around the `body` variable in
networks-data-source.ts so that if `body` is undefined or null you return an
empty array (or otherwise a valid EstimateFee[]), e.g., check `if (!body) return
[];` before the Array.isArray branch; ensure the function that returns
Promise<EstimateFee[]> (the method using `body`) never returns [undefined] by
explicitly handling undefined/null cases.
In `@frontend/packages/identity-client/src/data-sources/user-data-source.ts`:
- Line 14: The returned non-null assertion response.body! is unsafe; locate the
function in user-data-source.ts that returns Promise<User> and replace the
assertion with an explicit guard: check if response.body is undefined/null and
throw a clear Error (or reject) indicating an empty API response (include
context like the function name, e.g., getUser/fetchUser in the message),
otherwise return response.body; this ensures you fail fast instead of leaking
undefined to callers.
In
`@frontend/packages/identity-client/src/data-sources/user-profile-data-source.ts`:
- Line 40: The code returns response bodies using non-null assertion (return
body!) for SocialProfile and UpdateSocialProfileResult which can leak undefined;
update the methods in user-profile-data-source.ts that return SocialProfile and
UpdateSocialProfileResult to explicitly check if response.body is null/undefined
and throw a domain-level error (e.g., SocialProfileNotFoundError or a specific
Validation/ApiError) before returning the body; locate the two return body!
occurrences and replace them with a guard that throws when missing and only then
returns the typed body.
In
`@frontend/packages/identity-client/src/data-sources/wallet-views-data-source.ts`:
- Line 40: Replace unsafe non-null assertions on response.body in
wallet-views-data-source.ts by explicitly validating HttpResponse<T>. For each
method that currently does `return response.body!`, check `if (response.body ===
undefined)` and handle it (throw a clear Error or return a defined fallback)
before returning `response.body`; update the method signatures/return types if
you choose to return null/undefined. Specifically, remove `response.body!` usage
and add guards around the HttpResponse<T>.body access so callers of these
data-source methods get a reliable runtime error or explicit empty value instead
of a silent undefined.
In `@frontend/packages/identity-client/src/data-sources/wallets-data-source.ts`:
- Line 53: Several methods in wallets-data-source.ts are returning response
bodies using non-null assertion (return body!) without guarding for undefined;
replace those assertions with a runtime check that validates body is defined and
either throws a clear, typed error (e.g., new Error('Empty response body from
<methodName>')) or returns a safe default value matching the promised return
type. Locate each occurrence of "return body!" (currently at the occurrences
flagged — lines 53, 59, 65, 82, 89, 96, 108) and for each method replace the
unconditional return with: if (body === undefined) { throw new Error('Empty
response body in <methodName>'); } return body; (or return a typed empty/default
object if that better fits the method contract) so callers never receive
undefined where a concrete type is expected.
In `@frontend/packages/media-upload/src/request-delegation.ts`:
- Line 19: The code currently returns body! which uses a non-null assertion and
can violate the Promise<DelegationResponse> contract if the HTTP body is
undefined; update the function in request-delegation.ts to explicitly guard the
parsed `body` variable (the DelegationResponse payload) — e.g., check `if
(!body)` and throw a descriptive Error (or return a validated default
DelegationResponse) instead of using `body!`, and then return the validated
`body`; ensure the thrown error message includes context like "empty delegation
response" so callers can diagnose the failure.
In `@frontend/packages/network/src/long-poll-client.ts`:
- Line 126: The code currently returns response.body! (non-null asserted) which
can be undefined; change the poll request boundary to validate response.body is
present before returning so handlePollSuccess always receives a concrete
SyncResponse. Locate the function that returns response.body (the long-poll
request in long-poll-client.ts) and replace the non-null assertion with an
explicit check: if response.body is undefined, throw a descriptive Error (or
return an explicit failure value) and ensure the function's return type reflects
a guaranteed SyncResponse; update callers only if you choose to return a failure
value so handlePollSuccess always gets a real SyncResponse.
In `@frontend/packages/ton-storage/src/ton-storage-client.ts`:
- Line 43: The code calls mapBagDetails(response.body!) (and a similar call at
the other site) but HttpResponse<T> allows body to be undefined; replace the
non-null assertion with an explicit check on response.body before passing it to
mapBagDetails (and the other mapper), and handle the absent-body case
deterministically (e.g., throw a descriptive error or return a safe/fallback
value). Locate the usages of response.body in ton-storage-client.ts where
mapBagDetails and the other mapping function are invoked and add the
guard/handling logic so you never call the mapper with an undefined argument.
In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx`:
- Around line 29-33: The fetch handler in CoinIcon.tsx currently calls
res.text() for all responses and caches the result into svgCache even on HTTP
errors; change the logic in the fetch(uri) promise chain to first check the
Response.ok (or status) on res and only call res.text() and cache via
svgCache.set(uri, text) if res.ok is true; if not ok, avoid caching, optionally
throw or handle the error so the existing fallback rendering path (using
cancelled and setXml) runs correctly—look for the fetch(uri) -> .then((res) =>
res.text()) block and update it to guard with res.ok before calling res.text()
and caching.
- Around line 60-64: The component keeps hasError true after an image onError so
future valid uri values still show the fallback; reset hasError when uri changes
by adding an effect that calls setHasError(false) whenever uri updates. Locate
the CoinIcon component state variables (hasError, setHasError), the uri
prop/variable, and the existing handleError/useSvgXml logic, and implement a
useEffect watching uri to clear hasError (so failedUrls logic still applies but
per-uri errors don’t persist).
In `@frontend/packages/wallet/src/converters/coin-sort-utils.ts`:
- Around line 9-15: This module currently exports three symbols
(LOW_BALANCE_ICE_THRESHOLD, CompareInput, compareBySortRules); split them so the
file exports exactly one top-level symbol: pick one of these to remain in
coin-sort-utils.ts (rename file if needed to match that single export), move the
other exports into dedicated files (e.g., low-balance-ice-threshold.ts exporting
LOW_BALANCE_ICE_THRESHOLD and compare-input.ts exporting the CompareInput type)
or make them non-exported/internal if they are only used locally, and update all
import sites to point to the new files; ensure file names match their single
exported symbol and that compareBySortRules remains the only export in its file
if you choose to keep it there.
In `@frontend/packages/wallet/src/converters/search-aggregation-item.ts`:
- Around line 17-25: The current logic in search-aggregation-item.ts lets the
coinId fallback run even when a contract address check already failed, causing
wallets with wallet.coinId === null to match incorrect coins; update the
conditional around contract/coin.contractAddress so that if both addresses exist
and contract.toLowerCase() !== coin.contractAddress.toLowerCase() you return
false immediately, otherwise keep the existing true branch and the final
fallback return wallet.coinId === null || wallet.coinId === coin.id; reference
the variables/locations contract, coin.contractAddress, wallet.coinId, and
coin.id to make the change.
---
Outside diff comments:
In `@frontend/packages/identity-client/src/data-sources/two-fa-data-source.ts`:
- Around line 81-85: The code currently returns body! from the HTTP PUT in the
two-fa flow which can be undefined at runtime; update the return to provide a
concrete fallback instead of a non-null assertion: after the httpClient.put call
in the function that calls buildVerificationUrl and returns
RequestTwoFAResponse, replace the direct non-null assertion (body!) with a guard
that returns body ?? {} (an empty object matching RequestTwoFAResponse) or reuse
the module's validateActionChallengeResponse pattern to verify and return a safe
default when body is undefined.
---
Duplicate comments:
In `@frontend/packages/ui/src/formatters/format-subscript-notation.ts`:
- Around line 10-20: In formatSubscriptNotation, guard against unsupported
magnitudes before building subscript output: check absValue and return "" early
when absValue === 0 or absValue >= 0.1 (i.e., not a "small" negative-exponent
number) so you don't proceed after computing exponentMatch/zeroCount; keep the
existing exponentMatch and negative-exponent checks but add this early range
guard (referencing absValue, exponentMatch, zeroCount) to avoid malformed
outputs like for 0.1 or 0.
---
Nitpick comments:
In `@frontend/packages/ui/src/formatters/format-subscript-notation.ts`:
- Around line 12-23: Replace the magic numbers used in the exponential
formatting: extract the numeric literals passed to absValue.toExponential(12),
the subtraction by 1 when computing zeroCount (exponent - 1), and the substring
length 2 used when building trailing into named constants (e.g.,
MAX_EXP_FRACTION_DIGITS, ZERO_OFFSET, TRAILING_DIGITS). Update the code around
the exponentMatch creation (absValue.toExponential(...)), the zeroCount
computation (zeroCount = exponent - ...), and the trailing substring operation
(digits.substring(0, ...)) to use these constants; keep the existing logic for
mantissa, digits, exponent parsing, and the negative-exponent early return
unchanged.
In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx`:
- Around line 10-11: svgCache and failedUrls are unbounded and can grow
indefinitely; limit their size by adding eviction logic (LRU or simple size
cap). Replace or wrap the current Map and Set (svgCache, failedUrls) with a
bounded structure: introduce constants like MAX_SVG_CACHE_SIZE and
MAX_FAILED_URLS and, whenever inserting into svgCache or failedUrls, check size
and evict the oldest entry (for svgCache use Map insertion-order to delete the
first key; for failedUrls either switch to a Map with timestamps or remove the
first value from the Set via values().next()). Ensure insert/remove operations
used by the existing code paths (those referencing svgCache and failedUrls) use
the new bounded behavior so memory cannot grow unbounded.
In `@frontend/packages/wallet/src/converters/search-aggregation-item.ts`:
- Around line 8-88: This file currently exports three functions; split them so
each file exports exactly one function (file name matching the export) or make
helpers internal: create isMatchingWallet.ts exporting the isMatchingWallet
function (or keep it non-exported if only used by searchAggregationItem), create
searchAggregationItem.ts exporting searchAggregationItem (importing
isMatchingWallet from its file or using a local non-exported helper), and create
buildAggregationWalletKey.ts exporting buildAggregationWalletKey; update imports
where these functions are used, ensure each new file has a single export (prefer
default or named per project convention) and keep any helper-only functions
non-exported to satisfy the single-export-per-file rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2e3fbd0-4553-49ac-bdd5-c6d07891a155
📒 Files selected for processing (44)
frontend/packages/config/src/remote-config/fetch-config-from-network.tsfrontend/packages/file-storage/src/resolve-file.tsfrontend/packages/identity-client/src/auth/execute-signed-request.tsfrontend/packages/identity-client/src/data-sources/coins-data-source.tsfrontend/packages/identity-client/src/data-sources/credentials-data-source.tsfrontend/packages/identity-client/src/data-sources/keys-data-source.tsfrontend/packages/identity-client/src/data-sources/login-data-source.tsfrontend/packages/identity-client/src/data-sources/networks-data-source.tsfrontend/packages/identity-client/src/data-sources/recovery-data-source.tsfrontend/packages/identity-client/src/data-sources/registration-data-source.tsfrontend/packages/identity-client/src/data-sources/session-data-source.tsfrontend/packages/identity-client/src/data-sources/two-fa-data-source.tsfrontend/packages/identity-client/src/data-sources/user-action-data-source.tsfrontend/packages/identity-client/src/data-sources/user-data-source.tsfrontend/packages/identity-client/src/data-sources/user-profile-data-source.tsfrontend/packages/identity-client/src/data-sources/wallet-views-data-source.tsfrontend/packages/identity-client/src/data-sources/wallets-data-source.tsfrontend/packages/media-upload/src/request-delegation.tsfrontend/packages/network/src/axios-transport.tsfrontend/packages/network/src/http-types.tsfrontend/packages/network/src/long-poll-client.tsfrontend/packages/network/src/transport-types.tsfrontend/packages/ton-storage/src/ton-storage-client.tsfrontend/packages/ui/src/formatters/format-subscript-notation.test.tsfrontend/packages/ui/src/formatters/format-subscript-notation.tsfrontend/packages/ui/src/index.tsfrontend/packages/wallet-ui/src/components/CoinIcon.tsxfrontend/packages/wallet-ui/src/components/CoinList.tsxfrontend/packages/wallet-ui/src/screens/use-wallet-error-notifier.tsfrontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsxfrontend/packages/wallet/src/actions/create-wallet-view.test.tsfrontend/packages/wallet/src/actions/create-wallet-view.tsfrontend/packages/wallet/src/actions/delete-wallet-view.test.tsfrontend/packages/wallet/src/actions/delete-wallet-view.tsfrontend/packages/wallet/src/actions/rename-wallet-view.tsfrontend/packages/wallet/src/converters/coin-sort-utils.tsfrontend/packages/wallet/src/converters/compare-coin-groups.test.tsfrontend/packages/wallet/src/converters/compare-coin-groups.tsfrontend/packages/wallet/src/converters/compare-coins.tsfrontend/packages/wallet/src/converters/convert-wallet-view.tsfrontend/packages/wallet/src/converters/search-aggregation-item.test.tsfrontend/packages/wallet/src/converters/search-aggregation-item.tsfrontend/packages/wallet/src/index.tsfrontend/packages/wallet/src/stores/wallet-view-store.ts
✅ Files skipped from review due to trivial changes (3)
- frontend/packages/ui/src/formatters/format-subscript-notation.test.ts
- frontend/packages/wallet/src/stores/wallet-view-store.ts
- frontend/packages/wallet/src/converters/search-aggregation-item.test.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- frontend/packages/network/src/axios-transport.ts
- frontend/packages/wallet-ui/src/screens/use-wallet-error-notifier.ts
- frontend/packages/wallet/src/converters/compare-coin-groups.ts
- frontend/packages/ui/src/index.ts
- frontend/packages/wallet/src/actions/create-wallet-view.test.ts
- frontend/packages/wallet/src/actions/delete-wallet-view.test.ts
- frontend/packages/wallet/src/actions/delete-wallet-view.ts
- frontend/packages/wallet/src/actions/rename-wallet-view.ts
- frontend/packages/wallet-ui/src/components/CoinList.tsx
- frontend/packages/wallet/src/converters/compare-coin-groups.test.ts
- frontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsx
- frontend/packages/wallet/src/converters/convert-wallet-view.ts
- frontend/packages/wallet/src/actions/create-wallet-view.ts
Comment on lines
+9
to
+15
| export const LOW_BALANCE_ICE_THRESHOLD = 0.01; | ||
|
|
||
| export interface CompareInput { | ||
| symbolGroup: string; | ||
| totalBalanceUSD: number; | ||
| isPrioritized: boolean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split this module to keep a single export per file.
This file exports multiple symbols (LOW_BALANCE_ICE_THRESHOLD, CompareInput, compareBySortRules). Please keep one export in this file and move shared type/constant to dedicated files (or keep them non-exported if local only).
As per coding guidelines, "Each file must export exactly one class, one hook, one function, or one component; the file name must match the export."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/wallet/src/converters/coin-sort-utils.ts` around lines 9 -
15, This module currently exports three symbols (LOW_BALANCE_ICE_THRESHOLD,
CompareInput, compareBySortRules); split them so the file exports exactly one
top-level symbol: pick one of these to remain in coin-sort-utils.ts (rename file
if needed to match that single export), move the other exports into dedicated
files (e.g., low-balance-ice-threshold.ts exporting LOW_BALANCE_ICE_THRESHOLD
and compare-input.ts exporting the CompareInput type) or make them
non-exported/internal if they are only used locally, and update all import sites
to point to the new files; ensure file names match their single exported symbol
and that compareBySortRules remains the only export in its file if you choose to
keep it there.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/packages/wallet-ui/src/components/CoinIcon.tsx (1)
20-21:⚠️ Potential issue | 🟡 MinorPrevent stale SVG flash when
urichanges.
xmlcan briefly hold the previous URI’s content until the effect runs, so a newly selected coin may render the old icon for one frame.💡 Proposed fix
function useSvgXml(uri: string | null) { const isSvg = uri !== null && isSvgUrl(uri); const cached = isSvg ? svgCache.get(uri) : undefined; - const [xml, setXml] = useState<string | null>(cached ?? null); + const [state, setState] = useState<{ uri: string | null; xml: string | null }>({ + uri, + xml: cached ?? null, + }); useEffect(() => { - if (!isSvg || !uri) { setXml(null); return; } + if (!isSvg || !uri) { setState({ uri, xml: null }); return; } const currentCached = svgCache.get(uri); - if (currentCached) { setXml(currentCached); return; } - if (failedUrls.has(uri)) { setXml(null); return; } - setXml(null); + if (currentCached) { setState({ uri, xml: currentCached }); return; } + if (failedUrls.has(uri)) { setState({ uri, xml: null }); return; } + setState({ uri, xml: null }); let cancelled = false; fetch(uri) @@ .then((text) => { svgCache.set(uri, text); - if (!cancelled) setXml(text); + if (!cancelled) setState({ uri, xml: text }); }) .catch(() => { failedUrls.add(uri); - if (!cancelled) setXml(null); + if (!cancelled) setState({ uri, xml: null }); }); return () => { cancelled = true; }; }, [uri, isSvg]); - return isSvg ? xml : null; + return isSvg && state.uri === uri ? state.xml : null; }Also applies to: 45-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx` around lines 20 - 21, The component sometimes shows a stale SVG because xml (state controlled by useState) retains previous content while the effect fetching new SVG for uri runs; in the useEffect that depends on uri (and when cached changes) call setXml(null) immediately when uri changes (or setXml(cached ?? null) at the start of the effect) before starting the fetch so the old icon is cleared, and ensure the existing cached logic updates xml via setXml(cached ?? null) when cached applies; update references to xml/setXml, cached, uri, and the useEffect that performs the fetch accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx`:
- Around line 11-12: Replace the module-level Set failedUrls with a
timestamp-based map (e.g., failedUrlTimestamps: Map<string, number>) and update
all places that read/write failedUrls (calls that used failedUrls.has, .add,
.delete) so failures store Date.now() and checks expire entries older than 60
seconds: when checking if a URL should be skipped, remove it from the map and
allow retry if its timestamp is >60s old (or absent); when an image load fails,
set failedUrlTimestamps.set(url, Date.now()); when an image loads successfully,
remove any entry with failedUrlTimestamps.delete(url).
---
Duplicate comments:
In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx`:
- Around line 20-21: The component sometimes shows a stale SVG because xml
(state controlled by useState) retains previous content while the effect
fetching new SVG for uri runs; in the useEffect that depends on uri (and when
cached changes) call setXml(null) immediately when uri changes (or setXml(cached
?? null) at the start of the effect) before starting the fetch so the old icon
is cleared, and ensure the existing cached logic updates xml via setXml(cached
?? null) when cached applies; update references to xml/setXml, cached, uri, and
the useEffect that performs the fetch accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2167b7da-fa91-439c-a4c5-3aad31c44d5e
📒 Files selected for processing (18)
frontend/packages/config/src/remote-config/fetch-config-from-network.tsfrontend/packages/file-storage/src/resolve-file.tsfrontend/packages/identity-client/src/auth/execute-signed-request.tsfrontend/packages/identity-client/src/data-sources/coins-data-source.tsfrontend/packages/identity-client/src/data-sources/credentials-data-source.tsfrontend/packages/identity-client/src/data-sources/keys-data-source.tsfrontend/packages/identity-client/src/data-sources/networks-data-source.tsfrontend/packages/identity-client/src/data-sources/two-fa-data-source.tsfrontend/packages/identity-client/src/data-sources/user-data-source.tsfrontend/packages/identity-client/src/data-sources/user-profile-data-source.tsfrontend/packages/identity-client/src/data-sources/wallet-views-data-source.tsfrontend/packages/identity-client/src/data-sources/wallets-data-source.tsfrontend/packages/media-upload/src/request-delegation.tsfrontend/packages/network/src/long-poll-client.tsfrontend/packages/ton-storage/src/ton-storage-client.tsfrontend/packages/ui/src/formatters/format-subscript-notation.tsfrontend/packages/wallet-ui/src/components/CoinIcon.tsxfrontend/packages/wallet/src/converters/search-aggregation-item.ts
✅ Files skipped from review due to trivial changes (4)
- frontend/packages/identity-client/src/data-sources/networks-data-source.ts
- frontend/packages/identity-client/src/data-sources/credentials-data-source.ts
- frontend/packages/identity-client/src/data-sources/user-profile-data-source.ts
- frontend/packages/wallet/src/converters/search-aggregation-item.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/packages/identity-client/src/data-sources/two-fa-data-source.ts
- frontend/packages/config/src/remote-config/fetch-config-from-network.ts
- frontend/packages/identity-client/src/data-sources/keys-data-source.ts
- frontend/packages/file-storage/src/resolve-file.ts
- frontend/packages/identity-client/src/data-sources/coins-data-source.ts
- frontend/packages/identity-client/src/data-sources/wallet-views-data-source.ts
- frontend/packages/identity-client/src/auth/execute-signed-request.ts
- frontend/packages/identity-client/src/data-sources/user-data-source.ts
- frontend/packages/media-upload/src/request-delegation.ts
- frontend/packages/ui/src/formatters/format-subscript-notation.ts
Comment on lines
+11
to
+12
| const failedUrls = new Set<string>(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CoinIcon.tsx" -type fRepository: ice-blockchain/core-v2
Length of output: 123
🏁 Script executed:
cat -n ./frontend/packages/wallet-ui/src/components/CoinIcon.tsxRepository: ice-blockchain/core-v2
Length of output: 3609
Replace permanent failed URL blacklist with time-windowed retry mechanism.
The failedUrls Set causes transient fetch failures (offline/timeout) to permanently prevent retry for the app session, leaving icons stuck on the fallback. Move to a timestamp-based approach where failures automatically expire after 60 seconds, allowing retries when network recovers.
💡 Proposed fix
-const failedUrls = new Set<string>();
+const failedUrlTimestampByUri = new Map<string, number>();
+const FAILED_URL_RETRY_WINDOW_MS = 60_000;
+
+function hasRecentFailure(uri: string): boolean {
+ const failedAt = failedUrlTimestampByUri.get(uri);
+ return failedAt !== undefined && Date.now() - failedAt < FAILED_URL_RETRY_WINDOW_MS;
+}
@@
- if (failedUrls.has(uri)) { setXml(null); return; }
+ if (hasRecentFailure(uri)) { setXml(null); return; }
+ failedUrlTimestampByUri.delete(uri);
@@
.catch(() => {
- failedUrls.add(uri);
+ failedUrlTimestampByUri.set(uri, Date.now());
if (!cancelled) setXml(null);
});
@@
- if (!uri || hasError || failedUrls.has(uri)) {
+ if (!uri || hasError || hasRecentFailure(uri)) {Also applies to: 26, 39, 69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/wallet-ui/src/components/CoinIcon.tsx` around lines 11 -
12, Replace the module-level Set failedUrls with a timestamp-based map (e.g.,
failedUrlTimestamps: Map<string, number>) and update all places that read/write
failedUrls (calls that used failedUrls.has, .add, .delete) so failures store
Date.now() and checks expire entries older than 60 seconds: when checking if a
URL should be skipped, remove it from the map and allow retry if its timestamp
is >60s old (or absent); when an image load fails, set
failedUrlTimestamps.set(url, Date.now()); when an image loads successfully,
remove any entry with failedUrlTimestamps.delete(url).
ice-hades
reviewed
Apr 13, 2026
| .then(({ created, detail }) => finalizeCreatedView(optimistic.id, created, detail)) | ||
| .catch((error) => { | ||
| revertOptimisticCreate(optimistic.id, previousActiveId); | ||
| notifyWalletError("Failed to create wallet"); |
Contributor
There was a problem hiding this comment.
Hardcoded English string shown to users. The translation key walletUi:createWalletError already exists in en/fr/de — use translate("walletUi:createWalletError") instead.
Same issue in:
delete-wallet-view.ts—"Failed to delete wallet"→walletUi:deleteWalletErrorload-wallet-view-data.ts—"Failed to load wallet data"→walletUi:loadWalletErrorrename-wallet-view.ts—"Failed to rename wallet"→walletUi:renameWalletError
…fix review findings
…max-lines-per-function lint
ice-brontes
force-pushed
the
feature/wallet-coins-display
branch
from
2122aad to
a1d4e6e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
frontend/packages/ui/src/components/SearchBar.tsx (1)
12-12: Align boolean prop naming with repo convention.
autoFocusworks functionally, but this repo’s convention requires boolean names to start withis/has/should/can. Consider renaming toisAutoFocusand mapping it toRNTextInput’sautoFocus.♻️ Proposed rename
export interface SearchBarProps { value: string; onChangeText: (text: string) => void; placeholder?: string; style?: StyleProp<ViewStyle>; - autoFocus?: boolean; + isAutoFocus?: boolean; testID?: string; } export function SearchBar(props: SearchBarProps) { - const { value, onChangeText, placeholder = "Search", style, autoFocus, testID } = props; + const { value, onChangeText, placeholder = "Search", style, isAutoFocus, testID } = props; // ... <RNTextInput // ... - autoFocus={autoFocus} + autoFocus={isAutoFocus} onFocus={handleFocus} onBlur={handleBlur} />As per coding guidelines: "Boolean variables and functions must be prefixed with 'is', 'has', 'should', or 'can'."
Also applies to: 57-57, 78-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/components/SearchBar.tsx` at line 12, Rename the boolean prop autoFocus in the SearchBar component to isAutoFocus to match repo convention: update the SearchBarProps interface (or prop type) to use isAutoFocus, adjust the component’s prop destructuring and default value handling, and when rendering the underlying RN TextInput (RNTextInput / TextInput), pass its autoFocus prop the value of isAutoFocus (e.g., autoFocus={isAutoFocus}). Also update any internal references, usages, and exports of autoFocus (including the occurrences flagged around the component and helper functions) to the new name to avoid breaking callers.frontend/packages/ui/src/formatters/format-crypto-amount.ts (1)
23-25: Replace raw numeric literals with named constants.Thresholds/precisions like
10,2,3,6, and0.000001are repeated and should be centralized as named constants for maintainability.As per coding guidelines, "Do not use magic numbers, metaprogramming, decorators (except framework-required ones), or 'eval()'."
Also applies to: 26-29, 34-35, 40-44, 48-49, 56-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/ui/src/formatters/format-crypto-amount.ts` around lines 23 - 25, The function formatCryptoAmount (and its helper formatAbbreviated) uses repeated magic numbers (10, 2, 3, 6, 0.000001, etc.) and literal thresholds (e.g., TRILLION usage and other numeric comparisons); replace these raw literals with named constants (e.g., BASE_TEN = 10, PRECISION_TWO = 2, PRECISION_THREE = 3, PRECISION_SIX = 6, MIN_NON_ZERO = 0.000001, and any abbreviation thresholds) declared at the top of the file (or in a shared constants module) and update all comparisons/formatting calls (including calls to formatAbbreviated and branches around TRILLION) to use those constants for readability and maintainability.frontend/packages/wallet/src/actions/load-wallet-view-data.ts (1)
13-13: Use full callback variable names instead of abbreviations.Please replace short names like
vandswith explicit names (view,summary) for consistency with repo naming rules.As per coding guidelines, "Use full words in naming; no abbreviations like 'txSvc' or 'getBal' — use 'transaction.service.ts' and 'getUserBalance()' instead".
Also applies to: 46-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/actions/load-wallet-view-data.ts` at line 13, Replace abbreviated callback parameter names in the setWalletViews mapping with full, descriptive names: change the arrow callback used in setWalletViews(views.map((v) => ({ ...v, isLoading }))); to use names like view (and similarly replace any occurrences of s to summary at the other location mentioned). Update the map callback for setWalletViews and the mapped callback at the other noted spot (around line 46) so they use full identifiers (e.g., view, summary) instead of single-letter names to comply with repository naming guidelines.frontend/packages/wallet/src/errors.ts (1)
17-20: TypeActionError.codeasWalletErrorCode.Using
stringhere lets callers construct impossible wallet error codes and loses exhaustiveness anywhere this value is switched on. Keep the field and constructor parameter constrained to the enum.Suggested change
export class ActionError extends Error { - code: string; + code: WalletErrorCode; userMessage: string; - constructor(code: string, userMessage: string) { + constructor(code: WalletErrorCode, userMessage: string) { super(userMessage); this.code = code; this.userMessage = userMessage; this.name = "ActionError"; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet/src/errors.ts` around lines 17 - 20, Change the ActionError.code property and its constructor parameter from string to the WalletErrorCode enum so callers must supply a valid enum value; update the class field declaration for code and the constructor signature in ActionError to use WalletErrorCode (and adjust any import of WalletErrorCode if missing), and then fix any call sites constructing ActionError to pass a WalletErrorCode value instead of a raw string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/apps/mobile/App.tsx`:
- Around line 113-117: The wallet state isn't cleared when the authenticated
user changes; update the auth success/logout flow to call resetWalletClient()
before reinitializing or when logging out so resetWalletViewStore() is cleared;
specifically, in the handleAuthSuccess path (where initializeWalletClient and
loadWalletViewData are used) call resetWalletClient() prior to
initializeWalletClient(identityClient, username), and ensure the logout handler
also invokes resetWalletClient() so prior account A views are cleared before B's
fetch runs.
In `@frontend/packages/ui/src/formatters/format-crypto-amount.ts`:
- Around line 53-66: In formatCryptoAmount, add a finite-number guard before the
existing buckets: check Number.isFinite(value) and if false (NaN, +Infinity,
-Infinity) return the same zero string used for non-positive inputs (e.g.,
"0.00") so you don't fall through to
formatSubscriptNotation/formatSmallNumber/formatMediumNumber/formatLargeNumber
and produce outputs like "NaNM" or "InfinityT".
In `@frontend/packages/wallet/src/actions/load-wallet-view-data.ts`:
- Around line 40-43: When summaries.length === 0 you currently only call
setAllViewsLoading(false) and return, leaving previously cached views intact;
update this branch to explicitly clear the stored wallet views before returning
(e.g., call the mutation/dispatch that resets the views state such as
setViews([]) or clearWalletViews()), then call setAllViewsLoading(false) and
return. Make the change in load-wallet-view-data surrounding the summaries check
so cached summaries are removed whenever summaries is empty.
In `@frontend/packages/wallet/src/errors.ts`:
- Around line 1-23: Current file exports two symbols; split them so each file
has a single export and its filename matches that export. Move the
WalletErrorCode enum into a new WalletErrorCode.ts file exporting the enum
(named export), move the ActionError class into a new ActionError.ts file
exporting the class, update any imports to reference the new files, and ensure
the original errors.ts is removed or re-exported from an index if you need a
single import surface.
---
Nitpick comments:
In `@frontend/packages/ui/src/components/SearchBar.tsx`:
- Line 12: Rename the boolean prop autoFocus in the SearchBar component to
isAutoFocus to match repo convention: update the SearchBarProps interface (or
prop type) to use isAutoFocus, adjust the component’s prop destructuring and
default value handling, and when rendering the underlying RN TextInput
(RNTextInput / TextInput), pass its autoFocus prop the value of isAutoFocus
(e.g., autoFocus={isAutoFocus}). Also update any internal references, usages,
and exports of autoFocus (including the occurrences flagged around the component
and helper functions) to the new name to avoid breaking callers.
In `@frontend/packages/ui/src/formatters/format-crypto-amount.ts`:
- Around line 23-25: The function formatCryptoAmount (and its helper
formatAbbreviated) uses repeated magic numbers (10, 2, 3, 6, 0.000001, etc.) and
literal thresholds (e.g., TRILLION usage and other numeric comparisons); replace
these raw literals with named constants (e.g., BASE_TEN = 10, PRECISION_TWO = 2,
PRECISION_THREE = 3, PRECISION_SIX = 6, MIN_NON_ZERO = 0.000001, and any
abbreviation thresholds) declared at the top of the file (or in a shared
constants module) and update all comparisons/formatting calls (including calls
to formatAbbreviated and branches around TRILLION) to use those constants for
readability and maintainability.
In `@frontend/packages/wallet/src/actions/load-wallet-view-data.ts`:
- Line 13: Replace abbreviated callback parameter names in the setWalletViews
mapping with full, descriptive names: change the arrow callback used in
setWalletViews(views.map((v) => ({ ...v, isLoading }))); to use names like view
(and similarly replace any occurrences of s to summary at the other location
mentioned). Update the map callback for setWalletViews and the mapped callback
at the other noted spot (around line 46) so they use full identifiers (e.g.,
view, summary) instead of single-letter names to comply with repository naming
guidelines.
In `@frontend/packages/wallet/src/errors.ts`:
- Around line 17-20: Change the ActionError.code property and its constructor
parameter from string to the WalletErrorCode enum so callers must supply a valid
enum value; update the class field declaration for code and the constructor
signature in ActionError to use WalletErrorCode (and adjust any import of
WalletErrorCode if missing), and then fix any call sites constructing
ActionError to pass a WalletErrorCode value instead of a raw string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2900a273-9a4f-4a74-a398-84ed52d2ee98
⛔ Files ignored due to path filters (3)
frontend/packages/ui/src/icons/assets/icon_coin-fallback.svgis excluded by!**/*.svgfrontend/packages/ui/src/icons/generated/CoinFallbackIcon.tsxis excluded by!**/generated/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (102)
frontend/apps/mobile/App.tsxfrontend/apps/mobile/package.jsonfrontend/apps/mobile/src/components/splash-screen.tsxfrontend/apps/web/package.jsonfrontend/apps/web/src/app.tsxfrontend/apps/web/src/components/splash-screen.tsxfrontend/packages/config/src/remote-config/fetch-config-from-network.tsfrontend/packages/file-storage/src/resolve-file.tsfrontend/packages/identity-client/src/auth/execute-signed-request.tsfrontend/packages/identity-client/src/data-sources/coins-data-source.tsfrontend/packages/identity-client/src/data-sources/credentials-data-source.tsfrontend/packages/identity-client/src/data-sources/keys-data-source.tsfrontend/packages/identity-client/src/data-sources/login-data-source.tsfrontend/packages/identity-client/src/data-sources/networks-data-source.tsfrontend/packages/identity-client/src/data-sources/recovery-data-source.tsfrontend/packages/identity-client/src/data-sources/registration-data-source.tsfrontend/packages/identity-client/src/data-sources/session-data-source.tsfrontend/packages/identity-client/src/data-sources/two-fa-data-source.tsfrontend/packages/identity-client/src/data-sources/user-action-data-source.tsfrontend/packages/identity-client/src/data-sources/user-data-source.tsfrontend/packages/identity-client/src/data-sources/user-profile-data-source.tsfrontend/packages/identity-client/src/data-sources/wallet-views-data-source.tsfrontend/packages/identity-client/src/data-sources/wallets-data-source.tsfrontend/packages/identity-client/src/index.tsfrontend/packages/media-upload/src/request-delegation.tsfrontend/packages/network/src/axios-transport.tsfrontend/packages/network/src/http-types.tsfrontend/packages/network/src/long-poll-client.tsfrontend/packages/network/src/transport-types.tsfrontend/packages/ton-storage/src/ton-storage-client.tsfrontend/packages/ui/src/components/SearchBar.tsxfrontend/packages/ui/src/formatters/format-crypto-amount.test.tsfrontend/packages/ui/src/formatters/format-crypto-amount.tsfrontend/packages/ui/src/formatters/format-subscript-notation.test.tsfrontend/packages/ui/src/formatters/format-subscript-notation.tsfrontend/packages/ui/src/formatters/format-usd-amount.test.tsfrontend/packages/ui/src/formatters/format-usd-amount.tsfrontend/packages/ui/src/icons/icon-registry.tsfrontend/packages/ui/src/icons/icon-types.tsfrontend/packages/ui/src/index.tsfrontend/packages/wallet-ui/package.jsonfrontend/packages/wallet-ui/src/components/BalanceDisplay.tsxfrontend/packages/wallet-ui/src/components/CoinIcon.tsxfrontend/packages/wallet-ui/src/components/CoinList.tsxfrontend/packages/wallet-ui/src/components/CoinListItem.tsxfrontend/packages/wallet-ui/src/components/CoinListItemSkeleton.tsxfrontend/packages/wallet-ui/src/components/CoinSearchBar.tsxfrontend/packages/wallet-ui/src/components/CoinsSection.tsxfrontend/packages/wallet-ui/src/components/CoinsSectionCoinsPage.tsxfrontend/packages/wallet-ui/src/components/CoinsSectionSkeleton.tsxfrontend/packages/wallet-ui/src/components/ManageCoinsButton.tsxfrontend/packages/wallet-ui/src/index.tsfrontend/packages/wallet-ui/src/screens/WalletScreen.tsxfrontend/packages/wallet-ui/src/screens/wallet-view-management/CreateWalletViewScreen.tsxfrontend/packages/wallet-ui/src/screens/wallet-view-management/DeleteWalletViewConfirmScreen.tsxfrontend/packages/wallet-ui/src/screens/wallet-view-management/EditWalletViewScreen.tsxfrontend/packages/wallet-ui/src/screens/wallet-view-management/WalletViewSwitcherScreen.tsxfrontend/packages/wallet-ui/src/show-wallet-error.tsfrontend/packages/wallet-ui/src/translations/de.tsfrontend/packages/wallet-ui/src/translations/en.tsfrontend/packages/wallet-ui/src/translations/fr.tsfrontend/packages/wallet/package.jsonfrontend/packages/wallet/src/actions/create-wallet-view.test.tsfrontend/packages/wallet/src/actions/create-wallet-view.tsfrontend/packages/wallet/src/actions/delete-wallet-view.test.tsfrontend/packages/wallet/src/actions/delete-wallet-view.tsfrontend/packages/wallet/src/actions/load-wallet-view-data.test.tsfrontend/packages/wallet/src/actions/load-wallet-view-data.tsfrontend/packages/wallet/src/actions/rename-wallet-view.test.tsfrontend/packages/wallet/src/actions/rename-wallet-view.tsfrontend/packages/wallet/src/actions/switch-wallet-view.test.tsfrontend/packages/wallet/src/actions/switch-wallet-view.tsfrontend/packages/wallet/src/converters/coin-sort-utils.tsfrontend/packages/wallet/src/converters/compare-coin-groups.test.tsfrontend/packages/wallet/src/converters/compare-coin-groups.tsfrontend/packages/wallet/src/converters/compare-coins.tsfrontend/packages/wallet/src/converters/convert-wallet-view.test.tsfrontend/packages/wallet/src/converters/convert-wallet-view.tsfrontend/packages/wallet/src/converters/extract-contract-address.tsfrontend/packages/wallet/src/converters/from-blockchain-units.test.tsfrontend/packages/wallet/src/converters/from-blockchain-units.tsfrontend/packages/wallet/src/converters/search-aggregation-item.test.tsfrontend/packages/wallet/src/converters/search-aggregation-item.tsfrontend/packages/wallet/src/create-wallet-view.test.tsfrontend/packages/wallet/src/create-wallet-view.tsfrontend/packages/wallet/src/delete-wallet-view.test.tsfrontend/packages/wallet/src/delete-wallet-view.tsfrontend/packages/wallet/src/error-messages.tsfrontend/packages/wallet/src/errors.tsfrontend/packages/wallet/src/hooks/use-active-wallet-view.tsfrontend/packages/wallet/src/hooks/use-wallet-views.tsfrontend/packages/wallet/src/index.tsfrontend/packages/wallet/src/rename-wallet-view.test.tsfrontend/packages/wallet/src/rename-wallet-view.tsfrontend/packages/wallet/src/stores/wallet-client-config.test.tsfrontend/packages/wallet/src/stores/wallet-client-config.tsfrontend/packages/wallet/src/stores/wallet-view-store.test.tsfrontend/packages/wallet/src/stores/wallet-view-store.tsfrontend/packages/wallet/src/switch-wallet-view.test.tsfrontend/packages/wallet/src/test-setup.tsfrontend/packages/wallet/src/types.tsfrontend/packages/wallet/vitest.config.ts
💤 Files with no reviewable changes (7)
- frontend/packages/wallet/src/delete-wallet-view.test.ts
- frontend/packages/wallet/src/rename-wallet-view.test.ts
- frontend/packages/wallet/src/delete-wallet-view.ts
- frontend/packages/wallet/src/switch-wallet-view.test.ts
- frontend/packages/wallet/src/rename-wallet-view.ts
- frontend/packages/wallet/src/create-wallet-view.test.ts
- frontend/packages/wallet/src/create-wallet-view.ts
✅ Files skipped from review due to trivial changes (38)
- frontend/packages/wallet-ui/src/components/ManageCoinsButton.tsx
- frontend/packages/identity-client/src/data-sources/keys-data-source.ts
- frontend/packages/wallet-ui/src/components/CoinSearchBar.tsx
- frontend/packages/wallet/src/stores/wallet-view-store.test.ts
- frontend/packages/wallet-ui/src/index.ts
- frontend/packages/network/src/transport-types.ts
- frontend/packages/identity-client/src/data-sources/recovery-data-source.ts
- frontend/packages/wallet/package.json
- frontend/packages/network/src/axios-transport.ts
- frontend/packages/identity-client/src/data-sources/wallet-views-data-source.ts
- frontend/packages/identity-client/src/data-sources/networks-data-source.ts
- frontend/packages/wallet/vitest.config.ts
- frontend/packages/wallet/src/hooks/use-active-wallet-view.ts
- frontend/packages/wallet-ui/package.json
- frontend/packages/wallet-ui/src/components/BalanceDisplay.tsx
- frontend/packages/identity-client/src/index.ts
- frontend/packages/wallet/src/actions/switch-wallet-view.ts
- frontend/packages/wallet/src/error-messages.ts
- frontend/packages/identity-client/src/data-sources/wallets-data-source.ts
- frontend/packages/wallet/src/stores/wallet-view-store.ts
- frontend/packages/ui/src/formatters/format-usd-amount.ts
- frontend/packages/ui/src/formatters/format-subscript-notation.test.ts
- frontend/packages/ui/src/index.ts
- frontend/packages/wallet/src/actions/switch-wallet-view.test.ts
- frontend/packages/wallet/src/stores/wallet-client-config.ts
- frontend/packages/wallet/src/converters/from-blockchain-units.test.ts
- frontend/packages/wallet-ui/src/components/CoinListItemSkeleton.tsx
- frontend/packages/wallet/src/converters/compare-coins.ts
- frontend/packages/wallet/src/converters/from-blockchain-units.ts
- frontend/packages/wallet-ui/src/translations/fr.ts
- frontend/packages/wallet/src/actions/rename-wallet-view.test.ts
- frontend/packages/wallet/src/hooks/use-wallet-views.ts
- frontend/packages/wallet/src/actions/delete-wallet-view.test.ts
- frontend/packages/wallet/src/actions/load-wallet-view-data.test.ts
- frontend/packages/wallet/src/converters/convert-wallet-view.test.ts
- frontend/packages/wallet/src/stores/wallet-client-config.test.ts
- frontend/packages/wallet/src/actions/create-wallet-view.ts
- frontend/packages/wallet/src/converters/convert-wallet-view.ts
🚧 Files skipped from review as they are similar to previous changes (40)
- frontend/apps/mobile/package.json
- frontend/packages/ui/src/icons/icon-registry.ts
- frontend/packages/config/src/remote-config/fetch-config-from-network.ts
- frontend/packages/file-storage/src/resolve-file.ts
- frontend/packages/identity-client/src/data-sources/session-data-source.ts
- frontend/packages/ui/src/icons/icon-types.ts
- frontend/packages/wallet/src/converters/extract-contract-address.ts
- frontend/packages/identity-client/src/data-sources/user-data-source.ts
- frontend/packages/media-upload/src/request-delegation.ts
- frontend/packages/wallet-ui/src/screens/WalletScreen.tsx
- frontend/packages/ton-storage/src/ton-storage-client.ts
- frontend/packages/identity-client/src/data-sources/login-data-source.ts
- frontend/packages/identity-client/src/auth/execute-signed-request.ts
- frontend/apps/web/src/components/splash-screen.tsx
- frontend/apps/web/src/app.tsx
- frontend/packages/identity-client/src/data-sources/coins-data-source.ts
- frontend/packages/network/src/http-types.ts
- frontend/packages/identity-client/src/data-sources/user-action-data-source.ts
- frontend/apps/mobile/src/components/splash-screen.tsx
- frontend/packages/identity-client/src/data-sources/credentials-data-source.ts
- frontend/packages/wallet-ui/src/components/CoinsSectionSkeleton.tsx
- frontend/packages/wallet/src/converters/compare-coin-groups.ts
- frontend/packages/wallet-ui/src/translations/de.ts
- frontend/packages/wallet-ui/src/components/CoinsSectionCoinsPage.tsx
- frontend/packages/ui/src/formatters/format-subscript-notation.ts
- frontend/packages/wallet/src/actions/delete-wallet-view.ts
- frontend/packages/wallet-ui/src/components/CoinListItem.tsx
- frontend/packages/wallet-ui/src/components/CoinsSection.tsx
- frontend/packages/wallet-ui/src/translations/en.ts
- frontend/packages/wallet/src/types.ts
- frontend/packages/ui/src/formatters/format-usd-amount.test.ts
- frontend/packages/wallet/src/actions/create-wallet-view.test.ts
- frontend/apps/web/package.json
- frontend/packages/wallet-ui/src/components/CoinIcon.tsx
- frontend/packages/wallet-ui/src/components/CoinList.tsx
- frontend/packages/wallet/src/actions/rename-wallet-view.ts
- frontend/packages/identity-client/src/data-sources/user-profile-data-source.ts
- frontend/packages/wallet/src/converters/coin-sort-utils.ts
- frontend/packages/wallet/src/converters/search-aggregation-item.ts
- frontend/packages/wallet/src/index.ts
Comment on lines
+1
to
+23
| export enum WalletErrorCode { | ||
| NAME_EMPTY = "NAME_EMPTY", | ||
| WALLET_NOT_FOUND = "WALLET_NOT_FOUND", | ||
| MAX_WALLETS_REACHED = "MAX_WALLETS_REACHED", | ||
| CANNOT_DELETE_MAIN = "CANNOT_DELETE_MAIN", | ||
| CANNOT_DELETE_LAST = "CANNOT_DELETE_LAST", | ||
| CREATE_FAILED = "CREATE_FAILED", | ||
| RENAME_FAILED = "RENAME_FAILED", | ||
| DELETE_FAILED = "DELETE_FAILED", | ||
| LOAD_FAILED = "LOAD_FAILED", | ||
| } | ||
|
|
||
| export class ActionError extends Error { | ||
| code: string; | ||
| userMessage: string; | ||
|
|
||
| constructor(code: string, userMessage: string) { | ||
| super(userMessage); | ||
| this.code = code; | ||
| this.userMessage = userMessage; | ||
| this.name = "ActionError"; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split this module to match the repo’s single-export rule.
errors.ts now exports both WalletErrorCode and ActionError, and the filename matches neither symbol. Please split these into dedicated files, or keep one public export here and re-export the rest elsewhere.
As per coding guidelines, "Each file must export exactly one class, one hook, one function, or one component; the file name must match the export."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/wallet/src/errors.ts` around lines 1 - 23, Current file
exports two symbols; split them so each file has a single export and its
filename matches that export. Move the WalletErrorCode enum into a new
WalletErrorCode.ts file exporting the enum (named export), move the ActionError
class into a new ActionError.ts file exporting the class, update any imports to
reference the new files, and ensure the original errors.ts is removed or
re-exported from an index if you need a single import surface.
… shared ActionError
…minated-union results
ice-hades
approved these changes
Apr 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Connect the wallet UI to real server data so users see their token balances after login. Implement wallet view parser, coin list UI with shimmer loading states, number formatters, and wire wallet view CRUD operations (create, rename, delete) to the identity-client API with optimistic updates.
Replaces #114 (closed due to branch recreation).
Done
@ion/walletpackage intostores/,actions/,hooks/,converters/subdirectoriesCoinDisplayInfo,CoinWithBalance,CoinsGroup,WalletViewDatafromBlockchainUnits,extractContractAddress,searchAggregationItem,compareCoinGroups,convertWalletViewformatCryptoAmountandformatUsdAmountformatters to@ion/uiCoinListItem,CoinList,CoinListItemSkeleton,CoinsSectionSkeletoncomponentsCoinsSectionto render real coins or skeleton loading stateBalanceDisplayto show real USD balance from APIloadWalletViewDatainitializeWalletClient/resetWalletClientmodule-level singleton for identity clientcreateWalletViewto API with optimistic update + revert on failurerenameWalletViewto API with optimistic update + revert on failuredeleteWalletViewto API with optimistic update + revert on failuregetWalletViewcall)console.warnfallback@ion/networkHow to test
Summary by CodeRabbit
Release Notes
New Features
Improvements