Skip to content

feat: match utxo addresses in address book#10928

Merged
NeOMakinG merged 53 commits intodevelopfrom
utxo-match
Nov 11, 2025
Merged

feat: match utxo addresses in address book#10928
NeOMakinG merged 53 commits intodevelopfrom
utxo-match

Conversation

@NeOMakinG
Copy link
Collaborator

@NeOMakinG NeOMakinG commented Oct 28, 2025

Description

Implements UTXO address matching using cached addresses from unchained to identify when users enter their own addresses in the send flow.

When users enter a UTXO address in the send flow, we check if it belongs to one of their accounts by reading the addresses cached from unchained's account discovery response. If
it matches, we display the account label instead of showing the "Save" button.

This implementation leverages the React Query cache that was already populated during app load by unchained, eliminating manual address derivation entirely and supporting
addresses at any derivation index.

Issue (if applicable)

closes #10919

Risk

Low - address matching logic only, doesn't affect actual transactions

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

UTXO chains address matching in send flow (Bitcoin, Litecoin, Dogecoin, Bitcoin Cash)

Testing

  • Go to send flow for BTC/LTC/DOGE/BCH
  • You can also try Maya and Thorchain
  • Enter one of your own receive addresses (including those beyond index 20)
  • It should show your account label instead of the raw address
  • Try with addresses from different derivation paths (receive and change)

Engineering

Key Changes:

  • findUtxoAccountIdByAddress now uses queryClient.getQueryData() to read cached addresses from unchained's response
  • Added isUtxoAccountWithAddresses type guard in accounts.ts

How it works:

  • During app load, unchained returns all derived addresses in chainSpecific.addresses for each UTXO account
  • This data is cached permanently in React Query
  • When matching addresses, we simply read from this cache - no client-side derivation needed

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

image image image image image image image image

Summary by CodeRabbit

  • Bug Fixes
    • Address input display for UTXO accounts now includes account number along with the account label for better user clarity
    • Address book functionality improved to correctly identify and manage addresses on UTXO-based blockchain networks
    • Enhanced address validation to ensure account addresses are properly validated throughout the application

NeOMakinG and others added 30 commits October 3, 2025 18:01
- Gate AddressBook save feature with feature flag in SendAmountDetails
- Add aria-label to back button for accessibility
- Consolidate AddressBookEntry type to single source of truth
- Fix Date.now() timestamp inconsistency in addAddress reducer
- Add duplicate address validation to prevent saving same address twice
- Add defensive chainId validation in selectors

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@NeOMakinG NeOMakinG marked this pull request as ready for review October 29, 2025 13:49
@NeOMakinG NeOMakinG requested a review from a team as a code owner October 29, 2025 13:49
Base automatically changed from address-book to develop October 30, 2025 12:31
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Initial comments

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/lib/utils/accounts.ts (1)

57-60: Cache read is optimistic and may fail if data isn't pre-populated.

queryClient.getQueryData() synchronously reads from cache only. If the account data hasn't been fetched yet or the cache was evicted, this returns undefined and silently continues to the next account. While the PR description states the cache is "populated by unchained during app load," this creates fragility:

  • Race conditions on initial load before unchained finishes
  • Cache eviction under memory pressure
  • No feedback to the user when matching fails due to missing cache

Consider one of these approaches:

Option 1: Use queryClient.fetchQuery() to fetch if missing (requires making the function async and updating call sites)

Option 2: Add explicit cache verification and user feedback when cache is missing:

 for (const accountId of relevantAccountIds) {
   try {
     const account = queryClient.getQueryData<Account<KnownChainIds>>(
       accountManagement.getAccount(accountId).queryKey,
     )

-    if (!account || !isUtxoAccountWithAddresses(account)) {
+    if (!account) {
+      // Cache miss - data not yet loaded
+      console.warn(`Account data for ${accountId} not in cache - may need to wait for initial load`)
       continue
     }

+    if (!isUtxoAccountWithAddresses(account)) {
+      continue
+    }

Option 3: Document the cache dependency clearly and ensure unchained queries have appropriate staleTime and cacheTime to prevent eviction.

Based on learnings

src/state/slices/addressBookSlice/selectors.ts (1)

126-131: Misleading JSDoc: No progressive derivation or batching occurs.

The JSDoc comment claims "progressive address derivation" and "intelligently searches in batches (20, 50, 100)", but the implementation simply reads from React Query cache via findUtxoAccountIdByAddress. There's no incremental derivation or batching—it relies entirely on pre-populated cache from unchained at app load.

Update the comment to reflect the actual behavior:

-    // For UTXO chains, use progressive address derivation to find matches
-    // This intelligently searches in batches (20, 50, 100) to handle edge cases
-    // where users have many transactions and the address is beyond the standard gap limit
+    // For UTXO chains, search cached account addresses from unchained
+    // Relies on React Query cache populated during app load
+    // Returns null if account data is not yet cached
     if (chainNamespace === CHAIN_NAMESPACE.Utxo) {
       return findUtxoAccountIdByAddress(accountAddress, nonEvmAccountIds, chainId)
     }

Based on learnings

🧹 Nitpick comments (2)
src/lib/utils/accounts.ts (2)

37-41: Type guard is sound but optional chaining on account?.chainId is unnecessary.

The parameter type Account<KnownChainIds> guarantees account is defined, so account?.chainId could be account.chainId. However, this defensive check is harmless.

Apply this diff if you prefer to remove the unnecessary optional chaining:

-  return Boolean(isUtxoChainId(account?.chainId) && 'addresses' in account.chainSpecific)
+  return Boolean(isUtxoChainId(account.chainId) && 'addresses' in account.chainSpecific)

66-66: Optional chaining is redundant after type guard.

Line 62 confirms isUtxoAccountWithAddresses(account), which verifies 'addresses' in account.chainSpecific. The subsequent account.chainSpecific.addresses?.map() is defensively safe but the optional chaining is technically unnecessary after the type guard.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 22d2f95 and 2bd84fa.

📒 Files selected for processing (3)
  • src/components/Modals/Send/AddressInput/AddressInput.tsx (1 hunks)
  • src/lib/utils/accounts.ts (2 hunks)
  • src/state/slices/addressBookSlice/selectors.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern

**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...

Files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
  • src/state/slices/addressBookSlice/selectors.ts
  • src/lib/utils/accounts.ts
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern

**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components

Files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
**/*

📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)

**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case

Files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
  • src/state/slices/addressBookSlice/selectors.ts
  • src/lib/utils/accounts.ts
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

**/*.{tsx,jsx}: ALWAYS use useMemo for expensive computations, object/array creations, and filtered data
ALWAYS use useMemo for derived values and computed properties
ALWAYS use useMemo for conditional values and simple transformations
ALWAYS use useCallback for event handlers and functions passed as props
ALWAYS use useCallback for any function that could be passed as a prop or dependency
ALWAYS include all dependencies in useEffect, useMemo, useCallback dependency arrays
NEVER use // eslint-disable-next-line react-hooks/exhaustive-deps unless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped with memo

Files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

USE Redux only for global state shared across multiple places

Files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
  • src/state/slices/addressBookSlice/selectors.ts
  • src/lib/utils/accounts.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.

Applied to files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
📚 Learning: 2025-08-29T07:07:49.332Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.

Applied to files:

  • src/components/Modals/Send/AddressInput/AddressInput.tsx
📚 Learning: 2025-09-04T10:18:34.140Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10427
File: src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx:40-40
Timestamp: 2025-09-04T10:18:34.140Z
Learning: In the shapeshift/web codebase, src/state/slices/selectors.ts uses wildcard exports (`export * from './[sliceName]/selectors'`) to re-export all selectors from individual slice selector files, making them available through the barrel import. This means selectors like selectTxByFilter from txHistorySlice/selectors are properly accessible via '@/state/slices/selectors' even though they don't appear in explicit named exports.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
  • src/lib/utils/accounts.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter is expected to have an assetId property. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property (signature: (state, { assetId })), not a raw AssetId string. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-04T09:33:49.191Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10181
File: packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.ts:0-0
Timestamp: 2025-08-04T09:33:49.191Z
Learning: In Solana web3.js VersionedTransaction.message.addressTableLookups, the addressTableLookups property is typed as always defined (not optional), so optional chaining operators are unnecessary when accessing this property.

Applied to files:

  • src/lib/utils/accounts.ts
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.

Applied to files:

  • src/lib/utils/accounts.ts
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.

Applied to files:

  • src/lib/utils/accounts.ts
📚 Learning: 2025-08-05T22:41:35.473Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.

Applied to files:

  • src/lib/utils/accounts.ts
📚 Learning: 2025-08-21T22:16:55.203Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 10326
File: src/hooks/useActionCenterSubscribers/useFetchBasePortfolio.ts:33-35
Timestamp: 2025-08-21T22:16:55.203Z
Learning: In the ShapeShift web codebase Base portfolio management hack, the accountsById[accountId] check in upsertBasePortfolio serves as a security safeguard to ensure only user-associated accounts get balance updates, not just an existence check - this boundary should be maintained even during degraded Base state.

Applied to files:

  • src/lib/utils/accounts.ts
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.

Applied to files:

  • src/lib/utils/accounts.ts
🧬 Code graph analysis (3)
src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
src/state/slices/portfolioSlice/utils/index.ts (1)
  • accountIdToLabel (80-119)
src/state/slices/addressBookSlice/selectors.ts (2)
packages/caip/src/constants.ts (1)
  • CHAIN_NAMESPACE (77-82)
src/lib/utils/accounts.ts (1)
  • findUtxoAccountIdByAddress (43-79)
src/lib/utils/accounts.ts (3)
packages/types/src/base.ts (1)
  • UtxoChainId (54-58)
src/lib/utils/utxo.ts (1)
  • isUtxoChainId (27-28)
src/react-queries/queries/accountManagement.ts (1)
  • accountManagement (7-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install and Cache
🔇 Additional comments (3)
src/components/Modals/Send/AddressInput/AddressInput.tsx (1)

137-139: LGTM! Clear labeling improvement for UTXO accounts.

The account number prefix provides helpful context for users managing multiple UTXO accounts (e.g., "Account 0 - Segwit Native"). The formatting is consistent with the translation pattern used elsewhere.

src/state/slices/addressBookSlice/selectors.ts (1)

6-6: Verify placement of UTXO address matching logic.

As noted in past comments, this utility touches low-level account address resolution which might conceptually belong in chain-adapters. However, since it reads from React Query cache (a UI-layer concern), the current placement in lib/utils/accounts.ts is reasonable. No action needed unless you plan to decouple cache access.

src/lib/utils/accounts.ts (1)

48-48: The review comment misidentifies the concern—case-insensitivity is handled correctly, not a problem.

The code properly lowercases both input and cached addresses for comparison, which is a valid normalization approach per the CashAddr specification that states "Lower case is preferred for cashaddr, but uppercase is accepted." Both sides are lowercased (line 48 and line 67 of accounts.ts), so case-sensitivity is not an issue.

However, the review conflates two separate concerns:

  • Case-sensitivity (correctly handled by lowercasing) ✓
  • Prefix normalization (with/without bitcoincash:)—the actual distinct concern

The review also mentions bchtest: as a potential prefix, but this prefix does not appear anywhere in the actual codebase; only bitcoincash: is used. The code's case-handling approach is sound per specification. If there is a genuine prefix-handling concern, it should be framed separately from case-insensitivity verification.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

LGTM! https://jam.dev/c/77fcf35e-1f12-4254-87e6-c0c453f4f72d

Will let you resolve comments at your convenience, also this guy if you want to tackle here or in a quick follow-up: Bitcoin Cash addies first four last four is rugged because of bech32 bitcoincash: prefix
image

FYI, that's also happening at confirm time

image

We should probably strip the bitcoincash: prefix at display time (i.e whenever we render first four last four, do conventional str.replace('bitcoincash:", ""))

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/lib/utils/account/utxo.ts (1)

10-12: Optional chaining is unnecessary given the type signature.

The parameter is typed as Account<KnownChainIds>, which should always have a chainId property. The optional chaining account?.chainId suggests the function might receive null/undefined, but the type doesn't reflect this.

Apply this diff to align with the type:

-export const isUtxoAccount = (account: Account<KnownChainIds>): account is Account<UtxoChainId> => {
-  return isUtxoChainId(account?.chainId)
-}
+export const isUtxoAccount = (account: Account<KnownChainIds>): account is Account<UtxoChainId> => {
+  return isUtxoChainId(account.chainId)
+}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between dae8b45 and 46df267.

📒 Files selected for processing (2)
  • src/lib/utils/account/utxo.ts (1 hunks)
  • src/state/slices/addressBookSlice/selectors.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern

**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...

Files:

  • src/lib/utils/account/utxo.ts
  • src/state/slices/addressBookSlice/selectors.ts
**/*

📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)

**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case

Files:

  • src/lib/utils/account/utxo.ts
  • src/state/slices/addressBookSlice/selectors.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

USE Redux only for global state shared across multiple places

Files:

  • src/lib/utils/account/utxo.ts
  • src/state/slices/addressBookSlice/selectors.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-09-04T10:18:34.140Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10427
File: src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx:40-40
Timestamp: 2025-09-04T10:18:34.140Z
Learning: In the shapeshift/web codebase, src/state/slices/selectors.ts uses wildcard exports (`export * from './[sliceName]/selectors'`) to re-export all selectors from individual slice selector files, making them available through the barrel import. This means selectors like selectTxByFilter from txHistorySlice/selectors are properly accessible via '@/state/slices/selectors' even though they don't appear in explicit named exports.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-21T22:16:55.203Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 10326
File: src/hooks/useActionCenterSubscribers/useFetchBasePortfolio.ts:33-35
Timestamp: 2025-08-21T22:16:55.203Z
Learning: In the ShapeShift web codebase Base portfolio management hack, the accountsById[accountId] check in upsertBasePortfolio serves as a security safeguard to ensure only user-associated accounts get balance updates, not just an existence check - this boundary should be maintained even during degraded Base state.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-13T15:52:25.116Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10272
File: src/context/WalletProvider/MobileWallet/mobileMessageHandlers.ts:61-0
Timestamp: 2025-08-13T15:52:25.116Z
Learning: In the ShapeShift web codebase, specifically in src/context/WalletProvider/MobileWallet/mobileMessageHandlers.ts, message variants in the Message union type do not include inline comments documenting their expected return types. The codebase follows a pattern of keeping these type definitions clean without such documentation comments.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter is expected to have an assetId property. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property (signature: (state, { assetId })), not a raw AssetId string. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.

Applied to files:

  • src/state/slices/addressBookSlice/selectors.ts
🧬 Code graph analysis (2)
src/lib/utils/account/utxo.ts (3)
packages/types/src/base.ts (1)
  • UtxoChainId (54-58)
src/lib/utils/utxo.ts (1)
  • isUtxoChainId (27-28)
src/react-queries/queries/accountManagement.ts (1)
  • accountManagement (7-17)
src/state/slices/addressBookSlice/selectors.ts (2)
packages/caip/src/constants.ts (1)
  • CHAIN_NAMESPACE (77-82)
src/lib/utils/account/utxo.ts (1)
  • findUtxoAccountIdByAddress (14-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install and Cache
🔇 Additional comments (2)
src/state/slices/addressBookSlice/selectors.ts (1)

126-132: LGTM! UTXO branch correctly integrated.

The new UTXO handling branch correctly:

  • Checks for CHAIN_NAMESPACE.Utxo before delegating to findUtxoAccountIdByAddress
  • Passes accountAddress without normalization (UTXO addresses are case-sensitive due to checksums, unlike EVM)
  • Uses nonEvmAccountIds for the search scope
  • Returns AccountId | null consistent with other branches

The logic flow now properly separates EVM (cross-chain matching), UTXO (cached address matching), and other non-EVM chains (exact match).

src/lib/utils/account/utxo.ts (1)

14-52: No issues found. All implementation details are correct.

Verification confirms:

  • The function properly returns null when React Query cache is unpopulated or no match is found
  • The parent selector selectInternalAccountIdByAddress declares the correct return type of AccountId | null and propagates the null value appropriately
  • Error handling with try-catch is in place and continues iteration on failures
  • Chain ID filtering and type guards are implemented correctly

The code safely handles the edge case where cache might not be fully populated, gracefully returning null to the caller rather than throwing errors or accessing undefined data.

@NeOMakinG NeOMakinG merged commit ee2fe60 into develop Nov 11, 2025
4 checks passed
@NeOMakinG NeOMakinG deleted the utxo-match branch November 11, 2025 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Match owned wallet utxos in address book

2 participants