fix: gridplus derivation and signing #11333

NeOMakinG · 2025-12-09T12:50:09Z

Description

Fixes griplus signing by setting the wallet id at setup time + companion of the hdwallet gridplus derivation fix

Companion of shapeshift/hdwallet#764 , to be tested with it

Issue (if applicable)

Spotted in release

Risk

low

High Risk PRs Require 2 approvals

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

Testing

Clear your cache
Connect your gridplus
Try to derivate UTXOs
Try to sign a TX (swap on an evm chain for example)

Engineering

Operations

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

Screenshots (if applicable)

https://jam.dev/c/7fc407b0-5b31-41f3-bc7c-ae3caa3bf912

Summary by CodeRabbit

Chores
- Updated multiple hardware wallet integration dependencies.
Bug Fixes
- Improved wallet synchronization during GridPlus initialization.

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

coderabbitai · 2025-12-09T12:50:28Z

📝 Walkthrough

Walkthrough

Bumped multiple @shapeshiftoss/hdwallet- dependencies from 1.62.24 → 1.62.25 in root and package-specific package.json files. Added a conditional call to finalWallet.setExpectedActiveWalletId(activeWalletId, type) in GridPlus setup when both identifiers are present.

Changes

Cohort / File(s)	Summary
Root hdwallet dependencies `package.json`	18 `@shapeshiftoss/hdwallet-` packages (`coinbase`, `core`, `gridplus`, `keepkey`, `keepkey-webusb`, `keplr`, `ledger`, `ledger-webhid`, `ledger-webusb`, `metamask-multichain`, `native`, `native-vault`, `phantom`, `trezor`, `trezor-connect`, `vultisig`, `walletconnect`, `walletconnectv2`) updated from `1.62.24` → `1.62.25`
Chain adapters hdwallet dependencies `packages/chain-adapters/package.json`	`@shapeshiftoss/hdwallet-core`, `@shapeshiftoss/hdwallet-ledger` updated from `1.62.24` → `1.62.25`
Swapper hdwallet dependency `packages/swapper/package.json`	`@shapeshiftoss/hdwallet-core` updated from `1.62.24` → `1.62.25`
GridPlus setup logic `src/context/WalletProvider/GridPlus/components/GridPlusSetup.tsx`	Added conditional call `finalWallet.setExpectedActiveWalletId(activeWalletId, type)` when both `activeWalletId` and `type` are defined to synchronize expected active wallet before SafeCard setup

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Mostly homogeneous dependency version bumps (low review complexity).
Single localized logic change in GridPlusSetup.tsx — verify correct conditional and that finalWallet/method exist and handle absent values.

Possibly related PRs

feat: wire up hdwallet-gridplus safecard validation #11198 — Adds the same setExpectedActiveWalletId(activeWalletId, type) call in GridPlus setup (closely related).
feat: gridplus cleanup web fren #11170 — Refactors GridPlus session/pairing flow; touches the same GridPlus setup area.

Suggested reviewers

premiumjibles

Poem

🐰 I hopped through package.json with care,
Bumped hdwallets up a tiny stair.
GridPlus got a nudge to know who’s prime,
Syncing wallets, one bit at a time.
✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix: gridplus derivation and signing' directly summarizes the main change—fixing GridPlus signing and derivation issues by setting wallet ID at setup time.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch gridplus-deriv

📜 Recent 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 015b80b and c55047d.

⛔ Files ignored due to path filters (1)

yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (3)

package.json (1 hunks)
packages/chain-adapters/package.json (1 hunks)
packages/swapper/package.json (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

package.json

🧰 Additional context used

🧠 Learnings (17)

📓 Common learnings

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: 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: 10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.

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: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:46-59
Timestamp: 2025-09-10T15:34:54.593Z
Learning: After extensive testing by gomesalexandre in PR #10458, dApps do not send EIP-712 domain.chainId as hex or bigint values in practice. The simple String(domain.chainId) conversion is sufficient for real-world usage in WalletConnect dApps structured signing.

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10767
File: package.json:324-324
Timestamp: 2025-11-19T22:20:25.661Z
Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:27:02.203Z
Learning: In shapeshift/web, while gomesalexandre generally prefers pinned dependencies for security, semver with a caret range (^7.7.2) is acceptable as an exception since it's already a transient dependency of many packages and is being made explicit for hdwallet support.

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:16:12.898Z
Learning: In shapeshift/web, the semver package must be included in dependencies (not devDependencies) because hdwallet packages have transient dependencies that require semver but don't ship it themselves. This ensures semver is available at runtime for hdwallet functionality.

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.

Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10375
File: src/state/migrations/index.ts:214-215
Timestamp: 2025-08-27T13:49:48.668Z
Learning: The clearAssetsMigrations system in the ShapeShift web app works independently of the root persistConfig version. Migration numbers (like 171) don't need to match the root persistConfig version field, and the system has been functioning correctly for months/years without manual version bumps.

Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10375
File: src/state/migrations/index.ts:214-215
Timestamp: 2025-08-27T13:49:48.668Z
Learning: The ShapeShift web app uses an automated versioning system for Redux Persist migrations. The version is calculated as `Math.max(...Object.keys(clearAssetsMigrations).map(Number))`, which automatically uses the highest migration number as the version. This eliminates the need to manually update persistConfig versions when adding new migrations - the system automatically bumps the version when new migration numbers are added to the migration objects.

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-11-19T22:20:25.661Z

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10767
File: package.json:324-324
Timestamp: 2025-11-19T22:20:25.661Z
Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).

Applied to files:

packages/chain-adapters/package.json
packages/swapper/package.json

📚 Learning: 2025-08-08T20:16:12.898Z

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:16:12.898Z
Learning: In shapeshift/web, the semver package must be included in dependencies (not devDependencies) because hdwallet packages have transient dependencies that require semver but don't ship it themselves. This ensures semver is available at runtime for hdwallet functionality.

Applied to files:

packages/chain-adapters/package.json
packages/swapper/package.json

📚 Learning: 2025-12-04T22:57:50.850Z

Learnt from: kaladinlight
Repo: shapeshift/web PR: 11290
File: packages/chain-adapters/src/utxo/zcash/ZcashChainAdapter.ts:48-51
Timestamp: 2025-12-04T22:57:50.850Z
Learning: In packages/chain-adapters/src/**/*ChainAdapter.ts files, the getName() method uses the pattern `const enumIndex = Object.values(ChainAdapterDisplayName).indexOf(ChainAdapterDisplayName.XXX); return Object.keys(ChainAdapterDisplayName)[enumIndex]` to reverse-lookup the enum key from its value. This is the established pattern used consistently across almost all chain adapters (Bitcoin, Ethereum, Litecoin, Dogecoin, Polygon, Arbitrum, Cosmos, etc.) and should be preserved for consistency when adding new chain adapters.

Applied to files:

packages/chain-adapters/package.json

📚 Learning: 2025-08-08T20:27:02.203Z

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:27:02.203Z
Learning: In shapeshift/web, while gomesalexandre generally prefers pinned dependencies for security, semver with a caret range (^7.7.2) is acceptable as an exception since it's already a transient dependency of many packages and is being made explicit for hdwallet support.

Applied to files:

packages/chain-adapters/package.json
packages/swapper/package.json

📚 Learning: 2025-08-29T18:09:45.982Z

Learnt from: kaladinlight
Repo: shapeshift/web PR: 10376
File: vite.config.mts:136-137
Timestamp: 2025-08-29T18:09:45.982Z
Learning: In the ShapeShift web repository vite.config.mts, the commonjsOptions.exclude configuration using bare package name strings like ['shapeshiftoss/caip', 'shapeshiftoss/types'] works correctly for excluding specific packages from CommonJS transformation, despite theoretical concerns about module ID matching patterns.

Applied to files:

packages/chain-adapters/package.json
packages/swapper/package.json

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

packages/chain-adapters/package.json
packages/swapper/package.json

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

packages/chain-adapters/package.json

📚 Learning: 2025-08-07T11:20:05.201Z

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: package.json:0-0
Timestamp: 2025-08-07T11:20:05.201Z
Learning: gomesalexandre prefers pinned dependencies (exact versions without caret ranges) as a general security practice to prevent vulnerabilities from being introduced through automatic version bumps. He referenced the LedgerHQ/connect-kit vulnerability (GitHub issue #29) as an example of security risks from dependency updates.

Applied to files:

packages/chain-adapters/package.json