fix: bs58check v4 compatibility for Tron and UTXO derivation#763
fix: bs58check v4 compatibility for Tron and UTXO derivation#763gomesalexandre merged 3 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughUpgrade monorepo packages to 1.62.24 and standardize bs58/bs58check imports (switch to default imports and minor Buffer.from normalization); no public API signatures or control flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (27)
🚧 Files skipped from review as they are similar to previous changes (19)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-08-07T15:23:54.858ZApplied to files:
📚 Learning: 2025-08-07T15:27:03.179ZApplied to files:
📚 Learning: 2025-08-07T15:24:19.530ZApplied to files:
📚 Learning: 2025-08-07T15:47:29.207ZApplied to files:
📚 Learning: 2025-08-07T15:47:26.835ZApplied 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 (9)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.ts (1)
2-37: Consider wrappingbs58check.decodeinBuffer.fromhere for consistency with v4.In
Mnemonic.create, you now callbs58check.decode(xpub)directly:const xpubBuf = bs58check.decode(xpub);Given bs58check v4 may return a
Uint8Array, and elsewhere (e.g.,utxoUtils.ts) you explicitly wrap decode results inBuffer.from(...)to restore Buffer semantics, it could be safer and more consistent to do the same here, especially sincecheckType(SecP256K1.CompressedPoint, xpubBuf.slice(45))may rely on Buffer-typed inputs.A minimal change:
- const xpubBuf = bs58check.decode(xpub); + const xpubBuf = Buffer.from(bs58check.decode(xpub));This keeps the existing logic but guarantees
xpubBufis aBufferacross environments.
📜 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.
📒 Files selected for processing (7)
packages/hdwallet-core/package.json(1 hunks)packages/hdwallet-core/src/utxoUtils.ts(2 hunks)packages/hdwallet-ledger/package.json(1 hunks)packages/hdwallet-ledger/src/solana.ts(1 hunks)packages/hdwallet-native/package.json(1 hunks)packages/hdwallet-native/src/crypto/isolation/adapters/tron.ts(1 hunks)packages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 737
File: packages/hdwallet-trezor/src/ethereum.ts:122-138
Timestamp: 2025-11-20T11:04:44.808Z
Learning: In packages/hdwallet-trezor/src/ethereum.ts, the ethSignTypedData function correctly returns the signature from res.payload.signature without adding a "0x" prefix. This works correctly in practice and has been tested, despite appearing inconsistent with ethSignMessage which does add the prefix. The Trezor Connect ethereumSignTypedData response already provides the signature in the correct format for consumption.
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.
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.
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.
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.
📚 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-native/src/crypto/isolation/adapters/tron.tspackages/hdwallet-core/src/utxoUtils.tspackages/hdwallet-ledger/src/solana.tspackages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.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-native/src/crypto/isolation/adapters/tron.tspackages/hdwallet-ledger/src/solana.tspackages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.ts
📚 Learning: 2025-11-20T11:04:44.808Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 737
File: packages/hdwallet-trezor/src/ethereum.ts:122-138
Timestamp: 2025-11-20T11:04:44.808Z
Learning: In packages/hdwallet-trezor/src/ethereum.ts, the ethSignTypedData function correctly returns the signature from res.payload.signature without adding a "0x" prefix. This works correctly in practice and has been tested, despite appearing inconsistent with ethSignMessage which does add the prefix. The Trezor Connect ethereumSignTypedData response already provides the signature in the correct format for consumption.
Applied to files:
packages/hdwallet-native/src/crypto/isolation/adapters/tron.ts
📚 Learning: 2025-10-14T20:59:48.768Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 731
File: packages/hdwallet-gridplus/src/ethereum.ts:117-124
Timestamp: 2025-10-14T20:59:48.768Z
Learning: In packages/hdwallet-gridplus/src/ethereum.ts, the GridPlus SDK's sign method returns v as a number (integer), not a Buffer, so the Buffer.isBuffer(v) check is always false and vRaw = v is used directly.
Applied to files:
packages/hdwallet-native/src/crypto/isolation/adapters/tron.tspackages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.ts
📚 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.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-ledger/src/solana.tspackages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.ts
📚 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.jsonpackages/hdwallet-core/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-ledger/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-native/src/crypto/isolation/engines/dummy/bip39.ts
🧬 Code graph analysis (1)
packages/hdwallet-core/src/utxoUtils.ts (1)
packages/hdwallet-gridplus/src/constants.ts (1)
accountTypeToVersion(13-43)
⏰ 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 (6)
packages/hdwallet-native/package.json (1)
29-29: Add bs58check runtime dependency – looks correct; verify lockfile/build.Adding
"bs58check": "^4.0.0"here matches the new usages in hdwallet-native and keeps the dependency explicit rather than relying on transitive deps. Just make sureyarn.lockandyarn buildare run per your usual workflow before publishing. Based on learnings, this matches how you handle version bumps in this monorepo.packages/hdwallet-native/src/crypto/isolation/adapters/tron.ts (1)
3-3: Default bs58check import matches v4 usage; Tron adapter call site is fine.Switching to
import bs58check from "bs58check";while continuing to callbs58check.encode(addressBytes)is consistent with bs58check v4’s default export shape and should fix the Tron derivation issue without changing behavior.packages/hdwallet-ledger/src/solana.ts (1)
3-3: bs58 default import is fine; behavior of solanaGetAddress unchanged.Using
import bs58 from "bs58";and callingbs58.encode(...)should continue to work with a CJS bs58 module under typical TS/bundler interop settings, and doesn’t alter the Solana address derivation logic.packages/hdwallet-ledger/package.json (1)
35-35: bs58check upgrade in hdwallet-ledger is consistent with the rest of the PR.Bumping
"bs58check"to^4.0.0here matches the new import/usage patterns in the Ledger code and the rest of the monorepo, and keeps the dependency policy (exact hw-transport, caret for others) intact.packages/hdwallet-core/package.json (1)
20-20: Core bs58check dependency addition matches new utxoUtils usage.Adding
"bs58check": "^4.0.0"to hdwallet-core is the right move now thatutxoUtils.tsimports it directly, and it stays consistent with the versions used in other packages.packages/hdwallet-core/src/utxoUtils.ts (1)
1-85: Buffer-wrapping bs58check.decode correctly fixes bs58check v4 compatibility where Buffer-specific methods are used.The change in
convertXpubVersion:const payload = Buffer.from(bs58check.decode(xpub)); ... return bs58check.encode(Buffer.concat([desiredVersion, key]));ensures
versionis a realBuffersoversion.compare(desiredVersion)works with bs58check v4, which returnsUint8Array. This resolves the Ledger UTXO xpub-version conversion breakage.Other call sites in the codebase (cosmos.ts, thorchain.ts, osmosis.ts, mayachain.ts, and bip39.ts) use only
.slice(),.subarray(), and.lengthon the decoded result, which safely work on bothBufferandUint8Array, so no changes are needed there.
|
Should add to gridplus and keepkey as well and we can delete the @types to make sure there are no weird type conflicts. |
Not added on purpose to keep focus of the PR narrow @kaladinlight - unable to test KK but confirmed happy by ops and unable to test Gridplus atm either (@0xApotheosis gave it a try on release, there are other bugs at hand atm and it's not making it to release either way) |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/hdwallet-keepkey-webusb/package.json (1)
1-23: KeepKey source files require the same bs58check updates as other modules.The KeepKey packages use
bs58check.decode()extensively in source files:
packages/hdwallet-keepkey/src/mayachain.ts(line 55)packages/hdwallet-keepkey/src/thorchain.ts(line 48)packages/hdwallet-keepkey/src/osmosis.ts(line 49)packages/hdwallet-keepkey/src/cosmos.ts(line 49)These files import and call
bs58check.decode()identically to other modules that received updates in this PR. If the PR wrappedbs58check.decode()results withBuffer.from()in other packages (ledger, chromeusb, etc.) to handle Uint8Array returns, KeepKey modules require the same updates to maintain consistency and prevent type/runtime issues.
📜 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.
⛔ Files ignored due to path filters (1)
yarn.lockis 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(2 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(3 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 due to trivial changes (4)
- packages/hdwallet-walletconnect/package.json
- packages/hdwallet-portis/package.json
- packages/hdwallet-walletconnectV2/package.json
- lerna.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/hdwallet-core/package.json
- packages/hdwallet-native/package.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 737
File: packages/hdwallet-trezor/src/ethereum.ts:122-138
Timestamp: 2025-11-20T11:04:44.808Z
Learning: In packages/hdwallet-trezor/src/ethereum.ts, the ethSignTypedData function correctly returns the signature from res.payload.signature without adding a "0x" prefix. This works correctly in practice and has been tested, despite appearing inconsistent with ethSignMessage which does add the prefix. The Trezor Connect ethereumSignTypedData response already provides the signature in the correct format for consumption.
📚 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.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-trezor-connect/package.json
📚 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.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-trezor-connect/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.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-trezor-connect/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.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-trezor-connect/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.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-trezor-connect/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 (20)
packages/hdwallet-keepkey-electron/package.json (1)
3-3: Version bumps are consistent with coordinated monorepo update.The version bump from 1.62.23 to 1.62.24 and the matching dependency update for @shapeshiftoss/hdwallet-keepkey align with the PR's goal to propagate bs58check v4 compatibility fixes across packages.
Ensure that the parent dependency @shapeshiftoss/hdwallet-keepkey has been updated to properly handle bs58check v4 (default import pattern and Buffer API compatibility wrapping). If the keepkey adapter code hasn't been updated yet to match the bs58check v4 imports (as mentioned in the PR description for other packages), please verify that update is included in the referenced hdwallet-keepkey package.
Also applies to: 17-17
packages/hdwallet-keepkey-nodewebusb/package.json (1)
3-3: Version bumps are consistent and properly synchronized.The package version and all internal dependencies are correctly bumped to 1.62.24, maintaining consistency across the monorepo. Since KeepKey integration was intentionally excluded from bs58check import changes per the PR scope, only version synchronization here is appropriate.
Also applies to: 17-18
packages/hdwallet-keepkey-nodehid/package.json (1)
3-3: Verify KeepKey compatibility with updated hdwallet-core.While the version bump and dependency updates are consistent with the monorepo's versioning practice, the PR objectives note that KeepKey code changes were intentionally excluded to keep scope narrow. Since
@shapeshiftoss/hdwallet-corev1.62.24 now includes bs58check v4 compatibility changes (import pattern updates and Buffer wrapping), we need to confirm that KeepKey's code doesn't depend on these changed patterns.Please verify:
- Does KeepKey code use bs58check directly or indirectly through hdwallet-core utilities?
- Are there any runtime issues when KeepKey operates with the new hdwallet-core v1.62.24?
- Per the PR testing notes, has UTXO derivation for Ledger been tested? (If KeepKey shares similar code paths, those should also be validated.)
Also applies to: 17-17
packages/hdwallet-native-vault/package.json (1)
3-3: Version bumps are correctly aligned with the bs58check v4 fixes in hdwallet-native.The patch version bump (1.62.23 → 1.62.24) and the updated @shapeshiftoss/hdwallet-native dependency are properly aligned. The exact pinning of the internal dependency is consistent with monorepo practices for tight coordination between packages. The vault package will now inherit the bs58check v4 compatibility fixes from the updated native provider.
Also applies to: 18-18
packages/hdwallet-keepkey/package.json (1)
3-3: Version and dependency updates are consistent.The version bump to 1.62.24 and corresponding @shapeshiftoss/hdwallet-core dependency update align with the bs58check compatibility fixes in hdwallet-core. Per monorepo practice, these package version bumps precede publishing.
Also applies to: 24-24
packages/hdwallet-keepkey-chromeusb/package.json (1)
3-3: Dependency versions properly cascaded.Both @shapeshiftoss/hdwallet-core and @shapeshiftoss/hdwallet-keepkey are correctly updated to 1.62.24, ensuring this integration layer picks up the bs58check fixes from the core package.
Also applies to: 17-18
packages/hdwallet-keepkey-webusb/package.json (1)
3-3: Version and dependency updates consistent with variant package.The keepkey-webusb integration correctly tracks the 1.62.24 versions to maintain consistency with keepkey-chromeusb.
Also applies to: 17-18
integration/package.json (1)
3-3: Version bump and dependency alignment looks good.All
@shapeshiftoss/hdwallet-*dependencies are consistently updated to 1.62.24, matching the integration package version bump. This maintains consistency across the monorepo upgrade targeting bs58check v4 compatibility.Also applies to: 14-23
packages/hdwallet-trezor/package.json (1)
3-3: Version and core dependency updates are consistent.Trezor package correctly bumped to 1.62.24 with matching core dependency. Aligns with monorepo-wide upgrade pattern.
Also applies to: 20-20
packages/hdwallet-trezor-connect/package.json (1)
3-3: Dependency chain correctly updated.Both
@shapeshiftoss/hdwallet-coreand@shapeshiftoss/hdwallet-trezorproperly pinned to 1.62.24, maintaining transitive dependency consistency.Also applies to: 17-18
packages/hdwallet-keplr/package.json (1)
3-3: Version and core dependency properly aligned.Keplr package version and core dependency update to 1.62.24 are consistent. Other
@shapeshiftoss/*dependencies correctly maintain their independent versions.Also applies to: 20-20
packages/hdwallet-phantom/package.json (1)
3-3: Version and core dependency updates are correct.Phantom package version and core dependency bump to 1.62.24 maintain consistency with the coordinated release.
Also applies to: 18-18
packages/hdwallet-keepkey-tcp/package.json (1)
3-3: Dependency updates maintain correct chain.Both
@shapeshiftoss/hdwallet-coreand@shapeshiftoss/hdwallet-keepkeycorrectly updated to 1.62.24, ensuring KeepKey TCP transport stays in sync with core compatibility fixes.Also applies to: 17-18
packages/hdwallet-ledger-webhid/package.json (1)
3-3: WebHID package dependencies correctly aligned.Core and Ledger dependencies properly updated to 1.62.24. The exact pinning of
@ledgerhq/hw-transportto 6.31.8 is appropriate per the known type compatibility constraints. Based on learnings, this prevents type conflicts that occur with newer versions.Also applies to: 21-22
packages/hdwallet-metamask-multichain/package.json (1)
3-3: Package version alignment – approved.Version bumps to 1.62.24 and hdwallet-core dependency updates are consistent with the monorepo-wide coordination. Per learnings, version updates in PRs before publishing are expected workflow.
Also applies to: 20-20
packages/hdwallet-vultisig/package.json (1)
3-3: Package version alignment – approved.Version bumps to 1.62.24 and hdwallet-core dependency updates are consistent with the monorepo-wide coordination.
Also applies to: 20-20
packages/hdwallet-ledger/package.json (2)
3-3: Approve version bump; verify type definition compatibility.The bs58check ^4.0.0 dependency addition and version alignment are appropriate for the PR objectives. However, the @types/bs58check ^2.1.0 package (line 43) may have compatibility issues with bs58check v4 API changes. A prior reviewer recommended removing @types entirely to avoid type conflicts; this was intentionally not done per the PR scope decision.
Please confirm that @types/bs58check v2.1.0 accurately reflects the bs58check v4 API surface, or consider removing @types entirely if v4 provides its own type definitions or if types are causing issues during TypeScript compilation.
Also applies to: 30-30, 35-35
43-43: Consider removing or updating @types/bs58check for v4 compatibility.The @types/bs58check package has not been updated in over a year, and the type definitions use CommonJS export patterns that may not align with bs58check v4's ESM default import. The PR description mentions bs58check v4 uses "default import pattern," which represents a breaking change from the type definition.
Given that bs58 recently began providing its own type definitions, bs58check v4 may similarly provide inline types, making @types/bs58check obsolete. Consider either:
- Removing @types/bs58check entirely if v4 includes inline types
- Updating to a compatible version if available
- Adding a tsconfig exclusion if the type mismatch causes CI failures
This was flagged in earlier review comments but deferred; verify TypeScript compilation succeeds before release.
packages/hdwallet-coinbase/package.json (1)
3-3: Package version alignment – approved.Version bumps to 1.62.24 and hdwallet-core dependency updates are consistent with the monorepo-wide coordination.
Also applies to: 18-18
examples/sandbox/package.json (1)
3-3: Transitive dependency alignment – approved.Version bumps to 1.62.24 across all hdwallet packages and the sandbox example are consistent with the monorepo-wide coordination. This ensures the sandbox environment reflects the same dependency versions as the published packages.
Also applies to: 15-32
gomesalexandre
force-pushed
the
fix_tron_bs58check
branch
from
0afa259 to
16a2ffc
Compare
3 participants
Description
Fixes bs58check v4 compatibility issues that broke Tron address derivation and Ledger UTXO operations.
Testing
Issue
Tackles:
Screenshots
Summary by CodeRabbit
Chores
Dependency
✏️ Tip: You can customize this high-level summary in your review settings.