fix: improve kk connectivity #11641
fix: improve kk connectivity #11641
Conversation
📝 WalkthroughWalkthroughThe changes implement a device discovery and reuse mechanism for KeepKey wallet connections. They introduce logic to detect existing WebUSB devices before pairing, refactor firmware validation with fallback handling, add PIN requirement checks, and alias device IDs through the keyring. Event handling is improved to prevent re-entrancy during active connections, and error logging is added to trace connection failures. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant Keyring
participant Device as KeepKey Device
Client->>Adapter: getDevices() - discover existing WebUSB devices
Adapter-->>Client: Return list of devices
Client->>Keyring: Check if device serial matches known wallet
alt Device found in Keyring
Keyring-->>Client: Return cached wallet
Client->>Client: Reuse existing wallet
else Device not in Keyring
Client->>Adapter: Initiate pairing with new device
Adapter->>Device: Request device connection
Device-->>Adapter: Device connected
Adapter-->>Client: Return new wallet
Client->>Client: Validate firmware
alt Firmware validation passed
Client->>Keyring: Alias and store deviceId
Client->>Client: Complete initialization
else Firmware outdated or PIN required
Client->>Client: Abort connection
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
Files:
**/*.{tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
Files:
**/*.{jsx,tsx}📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Files:
🧠 Learnings (19)📓 Common learnings📚 Learning: 2025-11-19T16:59:50.569ZApplied to files:
📚 Learning: 2025-10-01T07:42:40.195ZApplied to files:
📚 Learning: 2025-09-12T12:00:33.924ZApplied to files:
📚 Learning: 2025-09-12T12:00:33.924ZApplied to files:
📚 Learning: 2025-10-07T03:44:27.350ZApplied to files:
📚 Learning: 2025-10-21T17:11:18.087ZApplied to files:
📚 Learning: 2025-09-22T14:28:23.109ZApplied to files:
📚 Learning: 2025-11-24T21:20:04.979ZApplied to files:
📚 Learning: 2025-08-13T15:52:25.116ZApplied to files:
📚 Learning: 2025-09-12T13:16:27.004ZApplied to files:
📚 Learning: 2025-09-04T12:16:47.748ZApplied to files:
📚 Learning: 2025-09-12T13:44:17.019ZApplied to files:
📚 Learning: 2025-09-12T10:44:46.723ZApplied to files:
📚 Learning: 2025-11-24T21:20:04.979ZApplied to files:
📚 Learning: 2025-12-27T16:02:52.792ZApplied to files:
📚 Learning: 2025-10-22T22:11:22.918ZApplied to files:
📚 Learning: 2025-11-20T12:00:45.005ZApplied to files:
📚 Learning: 2025-09-10T15:35:46.223ZApplied to files:
⏰ 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)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| const existingDevice = existingDevices[0] | ||
| const existingWallet = state.keyring.get(existingDevice.serialNumber) | ||
| if (existingWallet) return existingWallet | ||
| // @ts-ignore |
There was a problem hiding this comment.
Unfortunately for this one we need to use HDD best practices as opposed to typed bits.
gomesalexandre
force-pushed
the
fix_kk
branch
from
ad989b7 to
8305fbf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/context/WalletProvider/KeepKey/components/Connect.tsx:
- Around line 76-89: The code calls secondAdapter?.pairDevice() twice creating
unreachable/duplicate logic; in the Connect component remove the inner "return
await secondAdapter?.pairDevice()" inside the existingDevices?.length > 0 branch
so that when an existingDevice has no matching wallet it falls through to the
single final "return await secondAdapter?.pairDevice()" path; keep the checks
for existingDevice and state.keyring.get(...) and ensure only one call to
secondAdapter?.pairDevice() remains.
📜 Review details
Configuration used: Organization 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.
📒 Files selected for processing (5)
src/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/WalletProvider.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always runyarn lint --fixandyarn type-checkafter making changes
Avoidletvariable assignments - preferconstwith inline IIFE switch statements or extract to functions for conditional logic
Files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern:userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables inuseMemoand callbacks inuseCallbackwhere possible
For static JSX icon elements (e.g.,<TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode usinguseColorModeValuehook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook:useTranslate()fromreact-polyglot
UseuseFeatureFlag('FlagName')hook to access feature flag values in components
Prefertypeoverinterfacefor type definitions
Use strict typing - avoidany
UseNominaltypes for domain identifiers (e.g.,WalletId,AccountId)
Import types from@shapeshiftoss/caipfor chain/account/asset IDs
UseuseAppSelectorfor Redux state
UseuseAppDispatchfor Redux actions
Memoize expensive computations withuseMemo
Memoize callbacks withuseCallback
**/*.{ts,tsx}: UseResult<T, E>pattern for error handling in swappers and APIs; ALWAYS useOk()andErr()from@sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from@shapeshiftoss/errorswith meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...
Files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{tsx,jsx}: ALWAYS wrap React components in error boundaries and provide user-friendly fallback components with error logging
ALWAYS useuseErrorToasthook for displaying errors with translated error messages and handle different error types appropriatelyUse PascalCase for React component names and match the component name to the file name
Files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Usehandleprefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names withis,has,can,shouldprefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names likedata,item,obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names likefn,func, orcallback
Files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{jsx,tsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary, and 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
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly in async operations
ALWAYS provide user-friendly error messages in error handling
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components using React.lazy for code splitting
ALWAYS use Suspense wrapper for lazy loaded components
USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places
Wrap components receiving props withmemofor performance optimization
Files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Ensure TypeScript types are explicit and proper; avoid use of
anytype
Files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
🧠 Learnings (17)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10596
File: src/components/Layout/Header/NavBar/WalletConnectedMenu.tsx:77-99
Timestamp: 2025-10-01T07:42:40.195Z
Learning: In WalletConnectedMenu.tsx's handleReconnectWallet handler, gomesalexandre prefers throwing an error for unsupported wallet types in the default case rather than gracefully handling with a fallback. His reasoning: "if we have a problem here, we have bigger problems" - only supported wallets (KeepKey, Ledger, MetaMask, Coinbase, Phantom) should reach the reconnect flow when disconnected/locked, so encountering an unsupported type indicates a larger architectural issue that should be surfaced explicitly rather than masked with graceful degradation.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/StructuredMessage.tsx:0-0
Timestamp: 2025-09-13T16:45:17.166Z
Learning: gomesalexandre appreciates safety-focused suggestions for UI rendering in WalletConnect components, specifically defensive programming approaches that prevent null/undefined values from displaying as literal "null"/"undefined" strings in the user interface.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerConnectionState.ts:71-95
Timestamp: 2025-09-22T14:28:23.109Z
Learning: gomesalexandre designed useLedgerConnectionState to distinguish between USB device connection state and wallet pairing state. When pairing fails but USB is connected, connectionState should remain 'idle' to allow retries, while 'failed' is reserved for when the USB device is physically disconnected.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-10-01T07:42:40.195Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10596
File: src/components/Layout/Header/NavBar/WalletConnectedMenu.tsx:77-99
Timestamp: 2025-10-01T07:42:40.195Z
Learning: In WalletConnectedMenu.tsx's handleReconnectWallet handler, gomesalexandre prefers throwing an error for unsupported wallet types in the default case rather than gracefully handling with a fallback. His reasoning: "if we have a problem here, we have bigger problems" - only supported wallets (KeepKey, Ledger, MetaMask, Coinbase, Phantom) should reach the reconnect flow when disconnected/locked, so encountering an unsupported type indicates a larger architectural issue that should be surfaced explicitly rather than masked with graceful degradation.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-09-22T14:28:23.109Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerConnectionState.ts:71-95
Timestamp: 2025-09-22T14:28:23.109Z
Learning: gomesalexandre designed useLedgerConnectionState to distinguish between USB device connection state and wallet pairing state. When pairing fails but USB is connected, connectionState should remain 'idle' to allow retries, while 'failed' is reserved for when the USB device is physically disconnected.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.ts
📚 Learning: 2025-11-20T12:00:45.005Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11078
File: src/setupVitest.ts:11-15
Timestamp: 2025-11-20T12:00:45.005Z
Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.ts
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.tssrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-12-27T16:02:52.792Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11536
File: src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx:252-265
Timestamp: 2025-12-27T16:02:52.792Z
Learning: When reviewing bug fixes, especially in shapeshift/web, prefer minimal changes that fix correctness over introducing broader refactors or quality-of-life improvements (e.g., extracting duplicated logic) unless such improvements are essential to the fix. Apply this guideline broadly to TSX files and related components, not just the specific location, to keep changes focused and maintainable.
Applied to files:
src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsxsrc/context/WalletProvider/KeepKey/components/Connect.tsxsrc/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.ts
📚 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/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/**/*.ts : Avoid side effects in swap logic; ensure swap methods are deterministic and stateless
Applied to files:
src/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.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/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-09-04T12:16:47.748Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10430
File: src/components/Layout/Header/NavBar/PopoverWallet.tsx:72-94
Timestamp: 2025-09-04T12:16:47.748Z
Learning: gomesalexandre declined to add error boundaries to the PopoverWallet component in src/components/Layout/Header/NavBar/PopoverWallet.tsx, stating he didn't touch this component and preferring not to expand the scope of the PR with error boundary additions.
Applied to files:
src/context/WalletProvider/KeepKey/components/Pin.tsxsrc/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Wallets are managed via `WalletProvider` context
Applied to files:
src/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-09-12T13:16:27.004Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Applied to files:
src/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-09-12T10:44:46.723Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/SendTransactionContent.tsx:0-0
Timestamp: 2025-09-12T10:44:46.723Z
Learning: gomesalexandre dismissed a clipboard error handling suggestion in PR #10461 for SendTransactionContent.tsx, demonstrating that the current navigator.clipboard.writeText implementation works as expected and preferring to keep it simple without additional try/catch error handling.
Applied to files:
src/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Account discovery runs per wallet on connection
Applied to files:
src/context/WalletProvider/WalletProvider.tsx
⏰ 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 (10)
src/context/WalletProvider/KeepKey/components/Pin.tsx (1)
86-86: LGTM!The
[KK:Pin]prefix improves error traceability in logs, aligning with the broader logging enhancements in this PR.src/context/WalletProvider/WalletProvider.tsx (1)
645-648: LGTM!Adding
console.error(e)beforedisconnect()ensures errors are captured for debugging before the wallet state is reset. This improves traceability without changing the error handling behavior.src/context/WalletProvider/NewWalletViews/routes/KeepKeyRoutes.tsx (2)
95-95: LGTM!Correctly added
state.keyringto the dependency array since it's used within the callback forstate.keyring.get(existingDevice.serialNumber).
70-78: Address WebUSBKeepKeyAdapter type and implementation issues.The device reuse logic is safe with optional chaining, but the code contains developer-added TODO comments (lines 80 and beyond) indicating known issues with
WebUSBKeepKeyAdapter. A TODO inconfig.ts(line 369) notes the adapter is not properly exported as a type from the hdwallet package, causing the@ts-ignoresuppressions onpairDevice()calls. This pattern is repeated across multiple files (Connect.tsx,WalletProvider.tsx), suggesting a systemic issue that needs resolution rather than workaround comments. ThegetDevices()call is protected by optional chaining and should be safe, but thepairDevice()call on line 81 lacks the same protection and may encounter runtime issues.src/context/WalletProvider/KeepKey/hooks/useKeepKeyEventHandler.ts (3)
38-42: Good fix for early listener attachment.Attaching listeners when
state.modalType === KeyManager.KeepKey(in addition tolocalWalletType) ensuresPINMATRIXREQUESTevents are caught even beforelocalWalletTypeis set during wallet initialization. This correctly addresses the PIN modal not showing issue.
219-221: Critical fix for the infinite loop issue.The
state.isConnectedguard preventshandleConnectfrom dispatchingSET_WALLETwhen already connected. Without this, theSET_WALLETdispatch would updatestate.walletInfo, triggering a re-render and re-attaching listeners, causinghandleConnectto fire again in an infinite loop.
283-286: Correct dependency refinement to primitives.Using
state.walletInfo?.deviceId,state.walletInfo?.icon, andstate.walletInfo?.nameinstead ofstate.walletInfo(object) prevents unnecessary effect re-runs due to object reference changes. Addingstate.isConnectedis required for the new guard inhandleConnect.src/context/WalletProvider/KeepKey/components/Connect.tsx (3)
113-116: Good consolidation of firmware validation.The check now properly guards against proceeding when firmware is outdated: only validate if
latestFirmwareis available, and set error if the device firmware doesn't meet the requirement.
131-134: Critical fix for premature modal closure.This guard ensures the flow returns early if the device requires PIN entry (
pinProtection && !pinCached). ThePINMATRIXREQUESTevent handler will open the PIN modal. Without this return, the code would proceed to dispatchSET_WALLET_MODAL: false, closing the modal before PIN entry could occur.
151-154: Correct use ofaliasDeviceIdfor local wallet storage.Using the aliased device ID ensures consistency with how KeepKey device IDs are resolved elsewhere (e.g., in
handleConnect). This prevents mismatches when comparing device IDs later.
Description
Root Cause
Infinite loop when switching wallets: The
useKeepKeyEventHandlerhook's dependency array containedstate.walletInfo(an object), causing the effect to re-run on every state update. Combined with thehandleConnectcallback dispatchingSET_WALLET, this created an infinite dispatch loop.PIN modal not showing: The event handler only attached listeners when
localWalletType === KeyManager.KeepKey, butlocalWalletTypeis set AFTER wallet initialization. This meant thePINMATRIXREQUESTevent was firing before listeners were attached.Modal closing before PIN entry: After
wallet.initialize(), the code immediately dispatchedSET_WALLET_MODAL: falsewithout checking if PIN was requested.Fix
state.walletInfotostate.walletInfo?.deviceId(primitive) + addedstate.isConnectedguard inhandleConnectstate.modalType === KeyManager.KeepKeyORlocalWalletType === KeyManager.KeepKey(early attachment)features.pinProtection && !features.pinCachedafterinitialize()- if PIN needed, return early instead of closing modalgetDevices()check beforepairDevice()to handle already-paired WebUSB devicesIssue (if applicable)
closes #11591
Risk
Low - can't break what's broken
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/c99c9e1c-63f7-429a-a9d3-b1e4d3e13a76
https://jam.dev/c/ac8acfc4-68eb-44cf-ba3b-89d4e1d31731
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.