feat: use @shapeshiftoss/bitcoinjs-lib instead of @bitgo/utxo-lib for Ledger Zcash PSBTs #768
Conversation
📝 WalkthroughWalkthroughRemoved Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (27)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-08-07T15:27:03.179ZApplied to files:
📚 Learning: 2025-08-07T15:23:54.858ZApplied to files:
📚 Learning: 2025-08-07T15:24:19.530ZApplied to files:
📚 Learning: 2025-12-09T13:48:55.139ZApplied to files:
📚 Learning: 2025-08-07T15:47:29.207ZApplied to files:
📚 Learning: 2025-08-07T15:47:26.835ZApplied to files:
📚 Learning: 2025-12-12T11:20:00.907ZApplied 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 (21)
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-ledger/src/bitcoin.ts (1)
270-275: LGTM!Explicit Zcash v5 transaction configuration with correct NU6 consensus branch ID (0x4dec4df0). The PSBT methods properly configure the transaction format.
Consider removing the unused
ZCASH_VERSION_GROUP_ID[4]entry (line 18) if v4 transactions are no longer supported, to avoid dead code:const ZCASH_VERSION_GROUP_ID: Record<number, number> = { - 4: 0x892f2085, };
📜 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 (29)
examples/sandbox/package.json(2 hunks)integration/jest.config.js(0 hunks)integration/package.json(0 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(0 hunks)packages/hdwallet-ledger/src/bitcoin.ts(4 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 with no reviewable changes (3)
- integration/package.json
- packages/hdwallet-ledger/package.json
- integration/jest.config.js
🧰 Additional context used
🧠 Learnings (9)
📓 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.
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: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-walletconnect/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger-webhid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-keepkey/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.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger-webhid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-keepkey/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-walletconnect/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger-webhid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-keepkey/package.json
📚 Learning: 2025-12-09T13:48:55.139Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 764
File: packages/hdwallet-gridplus/package.json:0-0
Timestamp: 2025-12-09T13:48:55.139Z
Learning: In the shapeshiftoss/hdwallet monorepo, the dist/ directory is not version controlled (not tracked in git). Build artifacts are generated during the build/publish workflow, not committed to the repository.
Applied to files:
packages/hdwallet-vultisig/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger-webhid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-keepkey/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-walletconnect/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger-webhid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-ledger/src/bitcoin.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-vultisig/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-ledger-webhid/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-ledger/src/bitcoin.ts
📚 Learning: 2025-08-07T15:24:34.076Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: examples/sandbox/package.json:15-31
Timestamp: 2025-08-07T15:24:34.076Z
Learning: Lerna v6+ supports workspace protocol syntax (workspace:*) but does not automatically convert exact versions to workspace protocol during version bumps. It only preserves existing workspace protocol syntax. Teams using Lerna for automated version bumps would need manual conversion to use workspace protocol, which negates automation benefits.
Applied to files:
lerna.json
📚 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-ledger/src/bitcoin.ts
⏰ 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 (23)
lerna.json (1)
3-3: Verify the rationale for the version downgrade.The monorepo version is being downgraded from 1.62.28 to 1.62.26. While the change is consistent across all dependent files, the downgrade direction is unusual for a feature PR (feat: typically implies version increment, not decrement). This pattern is consistent with a rollback or correction of a previous erroneous version bump.
Please confirm: Is this downgrade intentional (e.g., rolling back to a known-good version before the feature), or should the version be incrementing instead?
packages/hdwallet-core/package.json (1)
1-31: Version downgrade is consistent across the monorepo.The version change to 1.62.26 aligns with the lerna.json downgrade. Dependencies remain stable and no direct removals are visible here.
packages/hdwallet-portis/package.json (1)
1-30: Consistent version alignment with monorepo and core dependency.Version and core dependency are properly synchronized to 1.62.26. No concerning changes.
packages/hdwallet-gridplus/package.json (1)
1-39: Consistent version alignment with monorepo and core dependency.Version and core dependency are properly synchronized to 1.62.26.
packages/hdwallet-vultisig/package.json (1)
1-30: Consistent version alignment with monorepo and core dependency.Version and core dependency are properly synchronized to 1.62.26.
packages/hdwallet-keepkey/package.json (1)
1-50: Consistent version alignment with monorepo and core dependency.Version and core dependency are properly synchronized to 1.62.26.
packages/hdwallet-metamask-multichain/package.json (1)
1-31: Consistent version alignment with monorepo and core dependency.Version and core dependency are properly synchronized to 1.62.26.
packages/hdwallet-ledger-webusb/package.json (2)
1-31: Consistent version alignment and proper dependency cascade.Version and internal dependency versions are synchronized to 1.62.26. Ledger transport dependencies correctly remain pinned to exact versions per project standards. Based on learnings, exact pins for
ledgerhq/hw-transportare intentional due to CI type-checking issues.
1-31: Main code changes for Zcash PSBT refactoring are not provided in this review batch.The PR objective is to replace
@bitgo/utxo-libwith@shapeshiftoss/bitcoinjs-libfor Ledger Zcash PSBT handling. However, the actual implementation changes (mentioned in the AI summary as occurring inpackages/hdwallet-ledger/src/bitcoin.ts) are not included in the provided files. Similarly, the removal of@bitgo/utxo-libfrompackages/hdwallet-ledger/package.jsonand integration files is not visible here.Please ensure that the following are reviewed as part of this PR:
- Code changes in
packages/hdwallet-ledger/src/bitcoin.tsshowing the Zcash PSBT refactoring with@shapeshiftoss/bitcoinjs-lib- Removal of
@bitgo/utxo-libfrompackages/hdwallet-ledger/package.json- Integration changes mentioned in the AI summary (integration/jest.config.js, integration/package.json)
- Verification that the PR's testing checklist has been executed (Zcash sends, two consecutive Zcash sends, DOGE sanity test)
examples/sandbox/package.json (1)
3-3: Verify the rationale for version downgrade rather than upgrade.All package versions and internal @shapeshiftoss/hdwallet-* dependencies are downgraded from 1.62.28 to 1.62.26. This is unusual—typically version changes increment, not decrement. Confirm this is intentional (e.g., bug fix, rollback) rather than an accidental regression.
Also applies to: 15-32
packages/hdwallet-native-vault/package.json (1)
3-3: LGTM!Version and internal dependency updates are consistent. Based on learnings, version bumps referencing unpublished versions during PR review phase is expected behavior in this monorepo's release workflow.
Also applies to: 18-18
packages/hdwallet-keepkey-nodehid/package.json (1)
3-3: LGTM!Version and dependency updates are consistent with the monorepo-wide version alignment.
Also applies to: 17-17
packages/hdwallet-keepkey-chromeusb/package.json (1)
3-3: LGTM!Version and dependency updates are consistent with the monorepo-wide version alignment.
Also applies to: 17-18
packages/hdwallet-ledger-webhid/package.json (1)
3-3: LGTM!Version and dependency updates are consistent with the monorepo-wide version alignment.
Also applies to: 21-22
packages/hdwallet-keepkey-webusb/package.json (1)
3-3: LGTM!Version and dependency updates are consistent with the monorepo-wide version alignment.
Also applies to: 17-18
packages/hdwallet-ledger/src/bitcoin.ts (4)
220-224: LGTM!Clean refactor replacing BitGo PSBT with
@shapeshiftoss/bitcoinjs-lib. TheforkCoinparameter correctly enables Zcash-specific serialization when needed.
257-257: LGTM!Unified output addition using address field simplifies the code by letting the library handle address-to-script conversion internally.
277-277: LGTM!Unified approach for extracting unsigned transaction hex from the PSBT works correctly with the new implementation.
371-380: The blockHeight fallback value is correct. The NU6 activation height for Zcash mainnet is 2,726,400, confirming the constant used in the code. The logic correctly usesMath.maxto find the highest confirmed input blockHeight with this fallback, properly handling edge cases (undefined, zero, or negative values).packages/hdwallet-walletconnectV2/package.json (1)
3-3: Version downgrade aligns with monorepo convention, but PR objective changes not visible in this file.The version downgrade from 1.62.28 to 1.62.26 is consistent across package and core dependency. However, the PR objectives describe replacing @bitgo/utxo-lib with @shapeshiftoss/bitcoinjs-lib for Ledger Zcash PSBT handling, which is not evident in this file. This suggests the feature implementation is in other files (likely packages/hdwallet-ledger).
Also applies to: 18-18
packages/hdwallet-trezor/package.json (1)
3-3: Version downgrade consistent, but this Trezor package is unrelated to the Ledger Zcash PSBT feature.The version downgrade from 1.62.28 to 1.62.26 is consistent with other files reviewed. However, this package is Trezor-specific and not directly related to the Ledger Zcash PSBT handling feature described in the PR objectives.
Also applies to: 20-20
packages/hdwallet-walletconnect/package.json (1)
3-3: Version downgrade is consistent across packages, but no feature changes visible in this file.The version downgrade to 1.62.26 follows the same pattern as other packages. This WalletConnect integration file does not contain the Zcash PSBT handling changes mentioned in the PR objectives, which are likely in the hdwallet-ledger package.
Also applies to: 18-18
packages/hdwallet-trezor-connect/package.json (1)
3-3: Version downgrade is consistent, but the actual Zcash PSBT feature changes are not visible in these files.All 4 package.json files show a uniform version downgrade from 1.62.28 to 1.62.26. However, the PR objectives mention replacing @bitgo/utxo-lib with @shapeshiftoss/bitcoinjs-lib for Ledger Zcash PSBT handling. These changes are not visible in any of the provided files, suggesting the feature implementation is in the hdwallet-ledger package (not included in this review).
Verification needed:
Please confirm that the hdwallet-ledger package.json contains the actual feature changes:
- Removal of @bitgo/utxo-lib dependency
- Addition of @shapeshiftoss/bitcoinjs-lib dependency
- Any corresponding version updates for the ledger package itself
Additionally, as per your monorepo's standard workflow (from learnings), ensure that
yarn buildhas been executed to regenerate dist artifacts after these dependency changes.Also applies to: 17-17, 18-18
gomesalexandre
force-pushed
the
feat_bitgo_bitcoinjs_lib
branch
from
ce612ea to
2b18cd2
Compare
gomesalexandre
force-pushed
the
feat_bitgo_bitcoinjs_lib
branch
4 times, most recently
from
ae9cdc1 to
74a311c
Compare
… Ledger Zcash PSBTs
gomesalexandre
force-pushed
the
feat_bitgo_bitcoinjs_lib
branch
from
74a311c to
352aa01
Compare
There was a problem hiding this comment.
https://jam.dev/c/d507240e-e0e3-422d-b8b7-bc3568736f22
Both broadcasted as expected!
Edit: fml, tested with native wallet first: https://jam.dev/c/e0b84366-e8d0-46dd-82fe-cfe60187286f
I failed at launching 2 TX in a row, but it might be because my ledger wallet doesn't have a lot of assets, happy to give it a stamp and leave to OPS to further testing with more funds as nothing seems to be breaking that from code reading!
Resolved conflict in integration/package.json by keeping origin/master versions (1.62.29) and removing @bitgo/utxo-lib dependency as intended.
gomesalexandre
force-pushed
the
feat_bitgo_bitcoinjs_lib
branch
from
6e494e1 to
2284412
Compare
|
@NeOMakinG tyvm ser! Moving to draft since this one is not urgent at all (things work perf as-is) to give it another go of testing with Ledger + Native and triple check this behaviour! |
Nice catch! That is definitely a bug! It working when I tested was sheer luck because of the blockHeight back then, see repro: https://jam.dev/c/a4be2615-6c5b-4ba7-8d6b-466a688a03ef |
gomesalexandre
force-pushed
the
feat_bitgo_bitcoinjs_lib
branch
from
1849618 to
bdd545f
Compare
|
@NeOMakinG should be happy after bdd545f though that commit shouldn't really matter, the actual fix is in web properly passing blockHeight https://github.com/shapeshift/web/pull/11374/changes#diff-c57ed3cf3c7d6cac82d78256887a33acb17e5619c68ac7d25b5db0c363a85849R357) See Jam after fix https://jam.dev/c/03f55e0d-3818-4f53-8f64-34ff29de7b45 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/hdwallet-ledger/src/bitcoin.ts (1)
371-377: Consider extracting the fallback height to a named constant.The block height logic correctly handles edge cases (empty/undefined heights). The magic number
3150000(above NU6.1 activation) could be extracted to a named constant for self-documentation:+const ZCASH_NU6_1_FALLBACK_HEIGHT = 3150000; // Above NU6.1 activation (3146400) + // ... blockHeight: msg.coin === "Zcash" - ? Math.max(...blockHeights.filter((h): h is number => h !== undefined && h > 0), 3150000) + ? Math.max(...blockHeights.filter((h): h is number => h !== undefined && h > 0), ZCASH_NU6_1_FALLBACK_HEIGHT) : undefined,
📜 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 (3)
integration/package.json(1 hunks)packages/hdwallet-ledger/package.json(0 hunks)packages/hdwallet-ledger/src/bitcoin.ts(4 hunks)
💤 Files with no reviewable changes (1)
- packages/hdwallet-ledger/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- integration/package.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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/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-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-ledger/src/bitcoin.ts
📚 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/src/bitcoin.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/src/bitcoin.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/src/bitcoin.ts
📚 Learning: 2025-12-12T11:19:53.263Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 769
File: packages/hdwallet-walletconnectV2/src/walletconnectV2.ts:150-152
Timestamp: 2025-12-12T11:19:53.263Z
Learning: In the shapeshift/hdwallet monorepo, remove reliance on ethSupportsNetwork() across wallet implementations. This legacy method is no longer used to determine chain support. Instead, rely on the wallet class flags like _supportsMonad, _supportsPlasma, _supportsHyperEvm. Review all wallet implementations for ethSupportsNetwork() usage and migrate checks to the corresponding _supports* flags, updating tests and any affected logic accordingly.
Applied to files:
packages/hdwallet-ledger/src/bitcoin.ts
🧬 Code graph analysis (1)
packages/hdwallet-ledger/src/bitcoin.ts (1)
packages/hdwallet-ledger/src/utils.ts (1)
networksUtil(121-592)
⏰ 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 (4)
packages/hdwallet-ledger/src/bitcoin.ts (4)
220-224: Clean unified PSBT initialization.The refactor to use a single PSBT constructor with
forkCoinparameter simplifies the code path and removes the need for separate BitGo PSBT handling for Zcash.
257-258: Consistent output value handling using BigInt.The unified output construction with
BigInt(output.amount)aligns with the OP_RETURN output handling and modern bitcoinjs-lib conventions.
277-277: Centralized transaction extraction.Using
psbt.data.getTransaction()provides a unified way to extract the unsigned transaction across all coin types.
270-275: No changes needed. The hardcoded version 5 and consensus branch ID0x4dec4df0are correct and current for NU6.1 mainnet, which activated on November 24, 2025.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@NeOMakinG definitive fix for mempool things is in web, see retest in https://jam.dev/c/24cd9fee-2db5-4ef6-bbcb-cbc6706e951a Going to land this guy! |
Description
Does what it says on the box.
Testing
Screenshots
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.