Skip to content

Conversation

@gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 1, 2025

Description

This PR:

  • adds SUI support to hdwallet-Ledger
  • ensures we use hardened paths (as per SLIP-10 standards - note native is fixed through matching web PR)

Issue

Screenshots

Ledger E2E test:

https://jam.dev/s/d03029f6-c63c-434c-b729-27da9fb5a642

Ledger live addy:

Screenshot 2025-12-01 at 17 45 02

This diff - Ledger addy (same as Ledger Live):

Screenshot 2025-12-01 at 19 19 33

Trust addy:

IMG_6868

This diff - native addy (same as trust):

Screenshot 2025-12-01 at 19 18 20

Summary by CodeRabbit

  • New Features
    • Ledger devices now support Sui: obtain Sui addresses and sign Sui transactions via WebHID and WebUSB, with Sui exposed across the Ledger integration.
  • Chores
    • Project packages bumped to 1.62.20 and multiple internal dependencies pinned to exact 1.62.20 versions across examples and integrations.

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

gomesalexandre and others added 22 commits December 1, 2025 13:28
Implements Tron support for Ledger hardware wallets following the established
pattern for chain integrations.

Changes:
- Add @ledgerhq/hw-app-trx dependency (v6.31.9)
- Create packages/hdwallet-ledger/src/tron.ts with:
  - tronGetAddress() - Derives Tron address from BIP44 path
  - tronSignTx() - Signs transactions using rawDataHex
  - tronGetAccountPaths() - Returns standard Tron BIP44 paths
  - tronNextAccountPath() - Account path iteration
- Update LedgerHDWalletInfo to implement TronWalletInfo interface
- Update LedgerHDWallet to implement TronWallet interface
- Add Tron app name mapping (SLIP44: 195 -> "Tron")
- Add "Tron" to LedgerTransportCoinType and type mappings
- Add Tron case to describePath()

Technical Details:
- BIP44 Path: m/44'/195'/0'/0/0 (SLIP44 code 195)
- Uses hw-app-trx library for Ledger device communication
- Signature format: rawDataHex + signature (65 bytes)
- Address format: Base58-encoded, starts with 'T'

This enables users to manage Tron assets with Ledger hardware wallets,
including native TRX and TRC20 token transactions.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Auto-fix import order and array formatting to match project style.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Add Tron coin case to translateCoinAndMethod in both WebUSB and WebHID
transport implementations. This allows the Ledger device to properly route
Tron method calls to the hw-app-trx library.

Also added missing Cosmos, Solana, and Thorchain support to WebHID transport
for feature parity with WebUSB.

Changes:
- Import Trx from @ledgerhq/hw-app-trx
- Add "Tron" case to translateCoinAndMethod switch statements
- WebHID: Add Cosmos, Solana, Thorchain cases for completeness

This fixes the "TypeError: coin" error when trying to connect Tron accounts
via Ledger hardware wallets.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Remove ^ prefix from all 1.62.17 version references to use exact versions
instead of ranges. This ensures consistent dependency resolution.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Implements Ledger hardware wallet integration for Sui blockchain:
- Add @mysten/ledgerjs-hw-app-sui dependency
- Create sui.ts module for Ledger-specific Sui operations
- Implement suiGetAddress() and suiSignTx() for Ledger transport
- Add Sui support to LedgerHDWallet and LedgerHDWalletInfo classes
- Update transport type system and implementations (WebUSB, WebHID)
- Add Sui (784) to appNameBySlip44 mapping

Follows same pattern as Tron Ledger implementation (PR #754).

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Bump all packages to 1.62.19-sui.0
- Fix Uint8Array to string conversions in sui.ts for Ledger API

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

Co-Authored-By: Claude <noreply@anthropic.com>
Ledger Sui app expects paths without 'm/' prefix (e.g., "44'/784'/0'/0'/0'"
not "m/44'/784'/0'/0'/0'"). This was causing 'transfer cancelled' errors.

Follows same pattern as Solana Ledger implementation.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Adds detailed console logging to track:
- Address derivation calls and responses
- Transaction signing calls and responses
- Public key retrieval
- Error handling at each step

Helps diagnose 'transfer cancelled' USB errors with Sui Ledger app.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Ledger Sui app requires all derivation path segments to be hardened
per SLIP-0010 specification for Ed25519. Move hardening logic from
web chain adapter to hdwallet Ledger implementation to match Native
wallet behavior.

Both Native and Ledger now receive non-hardened paths and handle
hardening internally using addressNListToHardenedBIP32().
Remove console.log statements added during debugging.
Keep console.error for production error tracking.
Reset all package.json files to match feat_tron_ledger base branch,
keeping only the @mysten/ledgerjs-hw-app-sui dependency addition.
Reset lerna.json version from 1.62.19-sui.2 back to 1.62.18
@gomesalexandre gomesalexandre requested a review from a team as a code owner December 1, 2025 16:26
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

📝 Walkthrough

Walkthrough

Adds Sui support across Ledger integrations: new Sui module (address/publicKey/signing and account-path helpers), transport adapter support (WebHID/WebUSB), transport typings and SLIP-44 mapping, LedgerHDWallet/Info surfaces to call Sui methods, and package version/dependency bumps to include the Sui ledger app package.

Changes

Cohort / File(s) Summary
Transport adapters (WebHID / WebUSB)
packages/hdwallet-ledger-webhid/src/transport.ts, packages/hdwallet-ledger-webusb/src/transport.ts
Import Sui adapter and extend translateCoinAndMethod to instantiate Sui and return bound Sui methods for coin "Sui".
Ledger core integration
packages/hdwallet-ledger/src/ledger.ts
Import sui module; add Sui to public types/flags (_supportsSuiInfo, _supportsSui), implement core.SuiWalletInfo/core.SuiWallet surfaces: suiGetAccountPaths, suiNextAccountPath, suiGetAddress, suiSignTx, and describePath handling for "Sui".
New Sui module
packages/hdwallet-ledger/src/sui.ts
New module implementing BIP32 path conversion, suiGetAddress, internal suiGetPublicKey, suiSignTx, suiGetAccountPaths, and suiNextAccountPath; uses Ledger Sui app calls and hex conversions with error handling.
Transport typing & slip mapping
packages/hdwallet-ledger/src/transport.ts, packages/hdwallet-ledger/src/utils.ts
Add Sui to LedgerTransportCoinType and method map typing; add SLIP-44 mapping 784 -> "Sui".
Package metadata / deps / pins
packages/hdwallet-ledger/package.json, multiple package.json files (repo-wide), lerna.json, examples/sandbox/package.json, integration/package.json
Bumped versions to 1.62.20, add dependency @mysten/ledgerjs-hw-app-sui and pin internal package versions (caret → exact).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant App as Consumer
participant Ledger as LedgerHDWallet
participant SuiMod as sui.ts
participant Device as Ledger Device (Sui app)
App->>Ledger: request suiGetAddress / suiSignTx
Ledger->>Ledger: validate active app == "Sui"
Ledger->>SuiMod: call suiGetAddress / suiSignTx(transport, params)
SuiMod->>Device: APDU getPublicKey / signTransaction
Device-->>SuiMod: publicKey / signature (Uint8Array)
SuiMod-->>Ledger: hex-encoded address or {signature, publicKey}
Ledger-->>App: return address or signed tx

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review focus:
    • packages/hdwallet-ledger/src/sui.ts — BIP32 path conversion, byte/hex conversions, error handling.
    • packages/hdwallet-ledger/src/ledger.ts — app validation and delegation to sui module.
    • transport adapters and typings — method binding and type map consistency.
    • package.json and lerna.json — dependency addition and version pinning correctness.

Possibly related PRs

Suggested reviewers

  • NeOMakinG

Poem

🐰 I hopped down the code-paths, nibbling bytes with delight,
Sui found a warm ledger burrow — snug, hex-lined, and bright.
Keys hop into place, signatures tucked in a row,
Bindings and transports hum low as the new features grow. 🥕

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 pull request title 'feat: add SUI support to hdwallet-ledger' accurately summarizes the main change: adding SUI coin support to the Ledger HDWallet package. The title is concise, specific, and clearly conveys the primary objective.
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 feat_sui_ledger

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • SLIP-10: Entity not found: Issue - Could not find referenced Issue.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- Move suiGetPublicKey function before suiSignTx to fix use-before-define
- Fix prettier formatting for multiline ternary operators
- Auto-fix import sorting issues in transport files
Base automatically changed from feat_tron_ledger to master December 1, 2025 23:07
Copy link

@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: 0

🧹 Nitpick comments (1)
packages/hdwallet-ledger/src/sui.ts (1)

78-97: Consider validating the account index is hardened.

The function validates that indices 0, 1, 3, and 4 match the expected hardened values, but doesn't verify that index 2 (the account index) is also hardened. For Sui's Ed25519 paths, all segments should be hardened per BIP44 standard (m/44'/784'/account'/0'/0').

Consider adding a validation check:

 export function suiNextAccountPath(msg: core.SuiAccountPath): core.SuiAccountPath | undefined {
   const addressNList = msg.addressNList;
   if (
     addressNList[0] === 0x80000000 + 44 &&
     addressNList[1] === 0x80000000 + core.slip44ByCoin("Sui") &&
+    (addressNList[2] & 0x80000000) === 0x80000000 &&
     addressNList[3] === 0x80000000 + 0 &&
     addressNList[4] === 0x80000000 + 0
   ) {

This ensures only properly hardened Sui paths are incremented.

📜 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 0f3179d and c3cabb5.

📒 Files selected for processing (7)
  • packages/hdwallet-ledger-webhid/src/transport.ts (2 hunks)
  • packages/hdwallet-ledger-webusb/src/transport.ts (2 hunks)
  • packages/hdwallet-ledger/package.json (1 hunks)
  • packages/hdwallet-ledger/src/ledger.ts (8 hunks)
  • packages/hdwallet-ledger/src/sui.ts (1 hunks)
  • packages/hdwallet-ledger/src/transport.ts (3 hunks)
  • packages/hdwallet-ledger/src/utils.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-07T15:27:03.179Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger/package.json:36-36
Timestamp: 2025-08-07T15:27:03.179Z
Learning: In the shapeshiftoss/hdwallet monorepo, the ledgerhq/hw-transport dependency in packages/hdwallet-ledger/package.json is pinned to an exact version (without caret) due to type mismatches that occur with newer versions. Other Ledger dependencies can safely use caret ranges.

Applied to files:

  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/src/transport.ts
  • packages/hdwallet-ledger/src/transport.ts
  • packages/hdwallet-ledger-webusb/src/transport.ts
📚 Learning: 2025-08-07T15:24:19.530Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger-webusb/package.json:3-3
Timestamp: 2025-08-07T15:24:19.530Z
Learning: In the shapeshiftoss/hdwallet monorepo, the team runs `yarn build` before bumping versions and also before publishing packages. This ensures that dist/ artifacts (including UMD bundles) are properly regenerated after dependency updates.

Applied to files:

  • packages/hdwallet-ledger/package.json
📚 Learning: 2025-08-07T15:23:54.858Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-coinbase/package.json:18-18
Timestamp: 2025-08-07T15:23:54.858Z
Learning: In the shapeshiftoss/hdwallet monorepo, package version bumps are done in PRs before publishing. The packages are published after the PR is merged, so dependency versions may reference unpublished versions during the PR review phase. This is expected behavior in their release workflow.

Applied to files:

  • packages/hdwallet-ledger/package.json
📚 Learning: 2025-08-07T15:47:29.207Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger/src/transport.ts:10-10
Timestamp: 2025-08-07T15:47:29.207Z
Learning: In the shapeshiftoss/hdwallet monorepo, ts-ignore is used instead of ts-expect-error for Ledger transport imports because the code works locally without TypeScript errors but has issues in CI environment. Using ts-expect-error would fail locally since there are no actual errors to suppress.

Applied to files:

  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/src/transport.ts
  • packages/hdwallet-ledger/src/transport.ts
  • packages/hdwallet-ledger-webusb/src/transport.ts
  • packages/hdwallet-ledger/src/sui.ts
  • packages/hdwallet-ledger/src/ledger.ts
📚 Learning: 2025-08-07T15:47:26.835Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger-webusb/src/transport.ts:12-12
Timestamp: 2025-08-07T15:47:26.835Z
Learning: In the shapeshiftoss/hdwallet monorepo, ts-ignore is used instead of ts-expect-error for Ledger transport imports because the CI environment has different type checking behavior than local development. The code works locally without errors, but CI reports type issues, so ts-ignore is necessary to suppress the inconsistent type checking across environments.

Applied to files:

  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/src/transport.ts
  • packages/hdwallet-ledger/src/transport.ts
  • packages/hdwallet-ledger-webusb/src/transport.ts
  • packages/hdwallet-ledger/src/ledger.ts
🧬 Code graph analysis (3)
packages/hdwallet-ledger-webhid/src/transport.ts (1)
packages/hdwallet-ledger/src/transport.ts (2)
  • LedgerTransportMethodName (53-55)
  • LedgerTransportMethod (73-77)
packages/hdwallet-ledger-webusb/src/transport.ts (1)
packages/hdwallet-ledger/src/transport.ts (2)
  • LedgerTransportMethodName (53-55)
  • LedgerTransportMethod (73-77)
packages/hdwallet-ledger/src/sui.ts (4)
packages/hdwallet-core/src/wallet.ts (1)
  • BIP32Path (22-22)
packages/hdwallet-ledger/src/ledger.ts (4)
  • suiGetAddress (652-655)
  • suiSignTx (657-660)
  • suiGetAccountPaths (247-249)
  • suiNextAccountPath (251-253)
packages/hdwallet-core/src/sui.ts (5)
  • SuiGetAddress (4-7)
  • SuiSignTx (9-13)
  • SuiSignedTx (15-18)
  • SuiGetAccountPaths (20-22)
  • SuiAccountPath (24-26)
packages/hdwallet-ledger/src/utils.ts (1)
  • handleError (8-58)
⏰ 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: Build and Release
🔇 Additional comments (20)
packages/hdwallet-ledger/src/utils.ts (1)

604-604: LGTM!

The Sui slip44 mapping (784) is correctly added to enable coin-to-app-name resolution for Sui Ledger operations.

packages/hdwallet-ledger-webusb/src/transport.ts (2)

8-8: LGTM!

Sui import added to support Sui Ledger operations via WebUSB transport.


109-113: LGTM!

The Sui case in the transport translator follows the established pattern for other coins (Btc, Eth, Solana, Tron), correctly instantiating the Sui app and binding methods.

packages/hdwallet-ledger/src/transport.ts (3)

7-7: LGTM!

Sui type import added to enable type-safe Sui transport methods.


21-21: LGTM!

Sui correctly added to the LedgerTransportCoinType union, enabling Sui as a valid transport coin type.


48-49: LGTM!

The Sui branch in the conditional type map correctly extracts Sui methods, maintaining type safety across the transport layer.

packages/hdwallet-ledger-webhid/src/transport.ts (2)

8-8: LGTM!

Sui import added to support Sui Ledger operations via WebHID transport.


89-93: LGTM!

The Sui case in the WebHID transport translator matches the WebUSB implementation and follows the established pattern for other coins.

packages/hdwallet-ledger/src/sui.ts (5)

6-10: LGTM!

The path conversion correctly hardens all segments per Ed25519 requirements and strips the m/ prefix as expected by Ledger's Sui app.


12-29: LGTM!

The address retrieval correctly calls the Sui app, handles errors, and formats the address as a 0x-prefixed hex string.


31-48: LGTM!

The internal public key retrieval function correctly extracts and formats the public key as a hex string.


50-72: LGTM!

The transaction signing correctly passes the intent message bytes to the Sui app, retrieves both the signature and public key, and returns the properly formatted signed transaction.


74-76: LGTM!

The account path retrieval correctly delegates to the core implementation, consistent with other coin integrations.

packages/hdwallet-ledger/src/ledger.ts (6)

9-9: LGTM!

Sui module import added to enable Sui-specific wallet operations.


152-152: LGTM!

LedgerHDWalletInfo correctly implements the SuiWalletInfo interface and exposes the support flag, consistent with other coin integrations.

Also applies to: 161-161


247-253: LGTM!

The Sui account path methods correctly delegate to the sui module, following the pattern established by other coins like Solana.


308-309: LGTM!

The describePath method correctly handles Sui paths by delegating to the core implementation, consistent with other coins.


389-389: LGTM!

LedgerHDWallet correctly implements the SuiWallet interface and exposes the support flag, enabling Sui operations on Ledger devices.

Also applies to: 408-408


652-660: LGTM!

The Sui address and transaction signing methods correctly validate the Sui app is active before delegating to the sui module, following the established pattern for secure Ledger operations.

packages/hdwallet-ledger/package.json (1)

28-28: No action needed — package version is valid and current.

The @mysten/ledgerjs-hw-app-sui@0.7.0 package exists, is the latest stable version (published a week ago), and has no known vulnerabilities. The package is maintained by the official Mysten Labs team and uses legitimate dependencies from the @ledgerhq organization.

NeOMakinG
NeOMakinG previously approved these changes Dec 2, 2025
Copy link
Contributor

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

Ok finally, after updating my ledger:

  • Derivation is fine and seems good compared with trust
  • Sending and swapping did work

One thing I noticed: my ledger and web isn't prompting the "please open app" message so it's quite hard to use as a end user

Not blocking thought

Copy link

@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

📜 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 912d93c and dbdf91c.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (27)
  • examples/sandbox/package.json (2 hunks)
  • integration/package.json (2 hunks)
  • lerna.json (1 hunks)
  • packages/hdwallet-coinbase/package.json (2 hunks)
  • packages/hdwallet-core/package.json (1 hunks)
  • packages/hdwallet-gridplus/package.json (2 hunks)
  • packages/hdwallet-keepkey-chromeusb/package.json (2 hunks)
  • packages/hdwallet-keepkey-electron/package.json (2 hunks)
  • packages/hdwallet-keepkey-nodehid/package.json (2 hunks)
  • packages/hdwallet-keepkey-nodewebusb/package.json (2 hunks)
  • packages/hdwallet-keepkey-tcp/package.json (2 hunks)
  • packages/hdwallet-keepkey-webusb/package.json (2 hunks)
  • packages/hdwallet-keepkey/package.json (2 hunks)
  • packages/hdwallet-keplr/package.json (2 hunks)
  • packages/hdwallet-ledger-webhid/package.json (2 hunks)
  • packages/hdwallet-ledger-webusb/package.json (2 hunks)
  • packages/hdwallet-ledger/package.json (2 hunks)
  • packages/hdwallet-metamask-multichain/package.json (2 hunks)
  • packages/hdwallet-native-vault/package.json (2 hunks)
  • packages/hdwallet-native/package.json (2 hunks)
  • packages/hdwallet-phantom/package.json (2 hunks)
  • packages/hdwallet-portis/package.json (2 hunks)
  • packages/hdwallet-trezor-connect/package.json (2 hunks)
  • packages/hdwallet-trezor/package.json (2 hunks)
  • packages/hdwallet-vultisig/package.json (2 hunks)
  • packages/hdwallet-walletconnect/package.json (2 hunks)
  • packages/hdwallet-walletconnectV2/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
  • integration/package.json
  • packages/hdwallet-walletconnect/package.json
  • packages/hdwallet-core/package.json
  • packages/hdwallet-keplr/package.json
  • packages/hdwallet-phantom/package.json
  • examples/sandbox/package.json
  • packages/hdwallet-keepkey-electron/package.json
  • packages/hdwallet-keepkey-chromeusb/package.json
  • packages/hdwallet-trezor-connect/package.json
  • packages/hdwallet-keepkey-webusb/package.json
  • packages/hdwallet-trezor/package.json
  • packages/hdwallet-keepkey/package.json
  • packages/hdwallet-walletconnectV2/package.json
  • packages/hdwallet-coinbase/package.json
  • lerna.json
  • packages/hdwallet-metamask-multichain/package.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger/package.json:36-36
Timestamp: 2025-08-07T15:27:03.179Z
Learning: In the shapeshiftoss/hdwallet monorepo, the ledgerhq/hw-transport dependency in packages/hdwallet-ledger/package.json is pinned to an exact version (without caret) due to type mismatches that occur with newer versions. Other Ledger dependencies can safely use caret ranges.
📚 Learning: 2025-08-07T15:27:03.179Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger/package.json:36-36
Timestamp: 2025-08-07T15:27:03.179Z
Learning: In the shapeshiftoss/hdwallet monorepo, the ledgerhq/hw-transport dependency in packages/hdwallet-ledger/package.json is pinned to an exact version (without caret) due to type mismatches that occur with newer versions. Other Ledger dependencies can safely use caret ranges.

Applied to files:

  • packages/hdwallet-vultisig/package.json
  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/package.json
  • packages/hdwallet-native-vault/package.json
  • packages/hdwallet-keepkey-nodehid/package.json
  • packages/hdwallet-keepkey-tcp/package.json
  • packages/hdwallet-ledger-webusb/package.json
  • packages/hdwallet-keepkey-nodewebusb/package.json
  • packages/hdwallet-gridplus/package.json
  • packages/hdwallet-portis/package.json
  • packages/hdwallet-native/package.json
📚 Learning: 2025-08-07T15:23:54.858Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-coinbase/package.json:18-18
Timestamp: 2025-08-07T15:23:54.858Z
Learning: In the shapeshiftoss/hdwallet monorepo, package version bumps are done in PRs before publishing. The packages are published after the PR is merged, so dependency versions may reference unpublished versions during the PR review phase. This is expected behavior in their release workflow.

Applied to files:

  • packages/hdwallet-vultisig/package.json
  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/package.json
  • packages/hdwallet-native-vault/package.json
  • packages/hdwallet-keepkey-nodehid/package.json
  • packages/hdwallet-keepkey-tcp/package.json
  • packages/hdwallet-ledger-webusb/package.json
  • packages/hdwallet-keepkey-nodewebusb/package.json
  • packages/hdwallet-gridplus/package.json
  • packages/hdwallet-portis/package.json
  • packages/hdwallet-native/package.json
📚 Learning: 2025-08-07T15:24:19.530Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger-webusb/package.json:3-3
Timestamp: 2025-08-07T15:24:19.530Z
Learning: In the shapeshiftoss/hdwallet monorepo, the team runs `yarn build` before bumping versions and also before publishing packages. This ensures that dist/ artifacts (including UMD bundles) are properly regenerated after dependency updates.

Applied to files:

  • packages/hdwallet-vultisig/package.json
  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/package.json
  • packages/hdwallet-native-vault/package.json
  • packages/hdwallet-keepkey-nodehid/package.json
  • packages/hdwallet-keepkey-tcp/package.json
  • packages/hdwallet-ledger-webusb/package.json
  • packages/hdwallet-keepkey-nodewebusb/package.json
  • packages/hdwallet-gridplus/package.json
  • packages/hdwallet-portis/package.json
  • packages/hdwallet-native/package.json
📚 Learning: 2025-08-07T15:47:29.207Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger/src/transport.ts:10-10
Timestamp: 2025-08-07T15:47:29.207Z
Learning: In the shapeshiftoss/hdwallet monorepo, ts-ignore is used instead of ts-expect-error for Ledger transport imports because the code works locally without TypeScript errors but has issues in CI environment. Using ts-expect-error would fail locally since there are no actual errors to suppress.

Applied to files:

  • packages/hdwallet-vultisig/package.json
  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/package.json
  • packages/hdwallet-native-vault/package.json
  • packages/hdwallet-keepkey-nodehid/package.json
  • packages/hdwallet-keepkey-tcp/package.json
  • packages/hdwallet-ledger-webusb/package.json
  • packages/hdwallet-keepkey-nodewebusb/package.json
  • packages/hdwallet-gridplus/package.json
  • packages/hdwallet-portis/package.json
  • packages/hdwallet-native/package.json
📚 Learning: 2025-08-07T15:47:26.835Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger-webusb/src/transport.ts:12-12
Timestamp: 2025-08-07T15:47:26.835Z
Learning: In the shapeshiftoss/hdwallet monorepo, ts-ignore is used instead of ts-expect-error for Ledger transport imports because the CI environment has different type checking behavior than local development. The code works locally without errors, but CI reports type issues, so ts-ignore is necessary to suppress the inconsistent type checking across environments.

Applied to files:

  • packages/hdwallet-vultisig/package.json
  • packages/hdwallet-ledger/package.json
  • packages/hdwallet-ledger-webhid/package.json
  • packages/hdwallet-native-vault/package.json
  • packages/hdwallet-keepkey-nodehid/package.json
  • packages/hdwallet-keepkey-tcp/package.json
  • packages/hdwallet-ledger-webusb/package.json
  • packages/hdwallet-keepkey-nodewebusb/package.json
  • packages/hdwallet-gridplus/package.json
  • packages/hdwallet-portis/package.json
  • packages/hdwallet-native/package.json
⏰ 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: Build and Release
🔇 Additional comments (14)
packages/hdwallet-portis/package.json (2)

3-3: Version bump aligns with monorepo coordination.

The version bump is appropriate as part of the coordinated monorepo release cycle.


19-19: Verify intent of exact dependency pin on hdwallet-core.

The caret has been removed from @shapeshiftoss/hdwallet-core, pinning it to an exact version (1.62.20). Based on learnings, exact pins in this monorepo are typically applied to hw-transport due to type mismatches, while other dependencies retain caret ranges. Clarify whether this exact pin is intentional as part of a broader monorepo versioning policy, or if the caret should be retained for flexibility.

packages/hdwallet-keepkey-nodehid/package.json (2)

3-3: Version bump aligns with feature addition.

The patch version bump is appropriate for this PR's scope.


17-17: Verify the intent and consistency of exact dependency pinning.

The dependency @shapeshiftoss/hdwallet-keepkey has been switched from a caret range (^1.62.19) to an exact pin (1.62.20). Based on learnings from the monorepo, exact pinning is typically used strategically to address type mismatches (e.g., @ledgerhq/hw-transport), not universally across all dependencies.

Please clarify:

  1. Is this exact pin intentional as part of a broader versioning strategy change in this PR?
  2. Does the keepkey package have reported type issues that warrant exact pinning?
  3. Are other keepkey-related packages (e.g., keepkey-webusb, if present) updated consistently?

Additionally, confirm that yarn build has been run after version bumps to regenerate dist artifacts, as per the monorepo's release workflow.

packages/hdwallet-native/package.json (1)

3-3: Version bump and dependency pinning are consistent with PR strategy.

The patch version bump (1.62.19 → 1.62.20) and removal of the caret from @shapeshiftoss/hdwallet-core align with the stacked PR approach for adding Sui support. Based on learnings, exact version pinning of internal dependencies in PRs before publishing is expected behavior in this monorepo's release workflow.

Also applies to: 20-20

packages/hdwallet-native-vault/package.json (1)

3-3: Dependency chain consistency maintained.

Version bump and caret removal mirror the pattern in hdwallet-native/package.json, ensuring the transitive dependency chain (native-vault → native → core) uses pinned exact versions. This is consistent with the stacked PR's release workflow.

Also applies to: 18-18

packages/hdwallet-keepkey-tcp/package.json (1)

3-3: Version and dependency pins align with PR objectives.

The patch version bump (1.62.19 → 1.62.20) and exact pinning of core dependencies (@shapeshiftoss/hdwallet-core, @shapeshiftoss/hdwallet-keepkey) to 1.62.20 are consistent with the broader monorepo synchronization and follow the established pattern of tightly coupling core package versions.

packages/hdwallet-gridplus/package.json (1)

3-3: The exact-pin versioning for @shapeshiftoss/hdwallet-core at version 1.62.20 is the intended design pattern across the entire monorepo. All 18 packages that depend on hdwallet-core consistently use exact pins (no caret ranges), along with internal dependencies like hdwallet-keepkey, hdwallet-ledger, and hdwallet-trezor. This coordinated exact-pinning strategy is consistent with the learnings about how this monorepo intentionally uses exact pins to manage type compatibility issues.

Likely an incorrect or invalid review comment.

packages/hdwallet-ledger/package.json (2)

3-3: Version bump is consistent across the package ecosystem.

The version 1.62.19 → 1.62.20 bump aligns with updates in related packages (webhid, webusb, vultisig, and keepkey-nodewebusb).


30-30: Internal hdwallet dependencies are exact-pinned for coherence.

The "@shapeshiftoss/hdwallet-core" is pinned to exact version "1.62.20" (no caret), consistent with the stacked-PR approach and internal dependency management strategy observed across related packages. Based on learnings, this intentional pattern ensures version coherence during development and PR review cycles.

packages/hdwallet-ledger-webhid/package.json (1)

3-3: Version and dependency pins are consistent with PR strategy.

Version bump to 1.62.20 and exact-pinned hdwallet dependencies (core and ledger at 1.62.20) follow the coherent versioning strategy applied across the package ecosystem in this PR.

Also applies to: 21-22

packages/hdwallet-vultisig/package.json (1)

3-3: Version and dependencies align with overall PR versioning strategy.

Version 1.62.20 and exact-pinned "@shapeshiftoss/hdwallet-core" maintain consistency across the package ecosystem, even though this package does not directly implement Sui features.

Also applies to: 20-20

packages/hdwallet-ledger-webusb/package.json (1)

3-3: Version and exact-pinned dependencies maintain ecosystem coherence.

Version bump to 1.62.20 and exact-pinned hdwallet dependencies (core and ledger at 1.62.20) are consistent with the PR's versioning strategy across all transport adapters.

Also applies to: 23-24

packages/hdwallet-keepkey-nodewebusb/package.json (1)

3-3: Version and dependencies follow consistent PR strategy.

Version 1.62.20 and exact-pinned hdwallet-core and hdwallet-keepkey at 1.62.20 maintain coherence across the package ecosystem, even for non-Ledger wallet integrations.

Also applies to: 17-18

@gomesalexandre gomesalexandre merged commit 5f3f26f into master Dec 2, 2025
3 checks passed
@gomesalexandre gomesalexandre deleted the feat_sui_ledger branch December 2, 2025 16:11
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.

3 participants