-
Notifications
You must be signed in to change notification settings - Fork 199
feat: wire up hdwallet-gridplus safecard validation #11198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
a636023
feat: wire up GridPlus SafeCard wallet UID validation
gomesalexandre 0cd71ce
fix: properly destructure wallet from pairConnectedDevice result
gomesalexandre 9c9b3e7
fix: capture wallet UID for all GridPlus SafeCard flows
gomesalexandre 334cd26
feat: add UI enhancements for GridPlus SafeCards
gomesalexandre ba339c9
feat(gridplus): add SafeCard validation for improved UX
gomesalexandre 0f6dd54
fix: update web to use new hdwallet adapter API
gomesalexandre 91b4a59
refactor: rename walletUid to activeWalletId for consistency
gomesalexandre 2225d12
chore: update hdwallet packages to .13
gomesalexandre 5c107c0
fix: remove duplicate connectDevice() call in pairConnectedDevice
gomesalexandre c454ac6
chore: update hdwallet packages to .14
gomesalexandre fcc7785
fix: restore physicalDeviceId storage for Add New and Connect buttons
gomesalexandre a3302da
feat: display specific error message for SafeCard mismatch
gomesalexandre 969a2e2
refactor: update SafeCard tag display to use consistent color
gomesalexandre 864fcce
feat: add context-aware error messages for SafeCard validation
gomesalexandre bdfea04
feat: wire up expectedType parameter through reconnection flow
gomesalexandre 9238b52
chore: cleanup - remove comments and console.errors
gomesalexandre a385cfb
[skip ci] refactor: remove password argument from connectDevice call
gomesalexandre c3400c1
[skip ci] chore: move hardcoded text to translations and cleanup
gomesalexandre 40aea87
[skip ci] fix: store physicalDeviceId before wallet check
gomesalexandre 2c7216a
[skip ci] chore: simplify undefined checks to boolean
gomesalexandre f027dfc
[skip ci] chore: remove unnecessary method existence check
gomesalexandre 2eea5a4
Merge remote-tracking branch 'origin/develop' into feat_gridplus_vali…
gomesalexandre 11675f3
[skip ci] chore: merge develop
gomesalexandre f3af080
[skip ci] chore: remove unnecessary braces from single-line return
gomesalexandre 566e685
Merge branch 'develop' into feat_gridplus_validation
gomesalexandre 7c92ff4
feat: bump
gomesalexandre 07bd2c8
chore: trigger CI
gomesalexandre ba5dca5
fix: lint
gomesalexandre File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
|
gomesalexandre marked this conversation as resolved.
Outdated
|
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # GridPlus SafeCard Validation | ||
|
|
||
| ## Problem | ||
|
|
||
| Previously, ShapeShift operated on a "trust me bro" basis when handling GridPlus SafeCards. The app would: | ||
| - Store SafeCards using app-generated UUIDs only | ||
| - Never validate which physical SafeCard was actually inserted | ||
| - Allow users to connect the wrong SafeCard without detection | ||
| - Proceed with signing transactions without verifying the correct card was present | ||
|
|
||
| This could lead to confusing UX where users might accidentally use the wrong wallet or end up in weird app states. | ||
|
|
||
| ## Solution | ||
|
|
||
| Implement hardware-based wallet UID validation at three critical points: | ||
|
|
||
| ### 1. Initial Pairing | ||
| When a user first pairs a SafeCard, we now: | ||
| - Fetch the hardware wallet UID from the device (32-byte identifier from firmware) | ||
| - Store this UID alongside the user's SafeCard in Redux state | ||
| - Use this UID as the source of truth for that SafeCard | ||
|
|
||
| ### 2. Reconnection | ||
| When the app loads and finds a GridPlus wallet from a previous session, we now: | ||
| - Validate that the inserted SafeCard's UID matches the expected stored UID | ||
| - Reject the connection if the wrong SafeCard is inserted | ||
| - Provide clear error messages to guide users to insert the correct card | ||
|
|
||
| ### 3. Just-In-Time (JIT) Signing Validation | ||
| Before every signing operation (transaction, message, typed data), we now: | ||
| - Re-validate that the correct SafeCard is still inserted | ||
| - Prevent signing if the SafeCard was swapped after connection | ||
| - Support all chains: Ethereum, Bitcoin, Solana, Cosmos, Thorchain, Mayachain | ||
|
|
||
| ## Implementation | ||
|
|
||
| ### hdwallet Package Changes (shapeshift/hdwallet) | ||
| Added to `GridPlusHDWallet` class: | ||
| - `setExpectedWalletUid(walletUid: string)`: Configure expected UID for validation | ||
| - `validateActiveWallet(expectedUid?: string)`: Fetch current wallet UID and validate | ||
| - Private `validateBeforeSigning()`: Called before all signing methods | ||
|
|
||
| ### Web Changes (shapeshift/web) | ||
| - **WalletProvider reconnection**: Validates SafeCard on app load | ||
| - **finalizeWalletSetup**: Sets expected UID after connection for JIT validation | ||
| - **GridPlus utils**: Wire up validation through connection flow | ||
| - **Redux state**: Store `walletUid` and `isExternal` flag per SafeCard | ||
|
|
||
| ## User Experience | ||
|
|
||
| Users will now see clear feedback when: | ||
| - They try to reconnect with the wrong SafeCard inserted | ||
| - They swap SafeCards after connecting but before signing | ||
| - Legacy SafeCards without stored UIDs (requires cache clear to use) | ||
|
|
||
| This ensures the app state always matches the physical hardware, preventing confusion and improving overall UX. | ||
|
|
||
| ## Migration | ||
|
|
||
| No data migration provided. Users with existing SafeCards will need to clear their cache and re-pair their devices. This is a one-time operation that establishes the UID mapping for future sessions. |
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
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
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
38 changes: 38 additions & 0 deletions
38
src/context/WalletProvider/GridPlus/components/GridPlusMenu.tsx
|
gomesalexandre marked this conversation as resolved.
|
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { RepeatIcon } from '@chakra-ui/icons' | ||
| import { MenuItem } from '@chakra-ui/react' | ||
| import { useCallback } from 'react' | ||
| import { useTranslate } from 'react-polyglot' | ||
|
|
||
| import { ManageAccountsMenuItem } from '@/components/Layout/Header/NavBar/ManageAccountsMenuItem' | ||
| import { WalletActions } from '@/context/WalletProvider/actions' | ||
| import { useWallet } from '@/hooks/useWallet/useWallet' | ||
|
|
||
| const switchIcon = <RepeatIcon /> | ||
|
|
||
| type GridPlusMenuProps = { | ||
| onClose?: () => void | ||
| } | ||
|
|
||
| export const GridPlusMenu = ({ onClose }: GridPlusMenuProps) => { | ||
| const translate = useTranslate() | ||
| const { dispatch } = useWallet() | ||
|
|
||
| const handleSwitchSafeCard = useCallback(() => { | ||
| dispatch({ type: WalletActions.RESET_STATE }) | ||
| dispatch({ | ||
| type: WalletActions.SET_INITIAL_ROUTE, | ||
| payload: '/gridplus/connect', | ||
| }) | ||
| dispatch({ type: WalletActions.SET_WALLET_MODAL, payload: true }) | ||
| onClose?.() | ||
| }, [dispatch, onClose]) | ||
|
|
||
| return ( | ||
| <> | ||
| <MenuItem icon={switchIcon} onClick={handleSwitchSafeCard}> | ||
| {translate('walletProvider.gridplus.menu.switchSafeCard')} | ||
| </MenuItem> | ||
| <ManageAccountsMenuItem onClose={onClose} /> | ||
| </> | ||
| ) | ||
| } |
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.