feat: hdwallet-ledger zcash support #765
Conversation
📝 WalkthroughWalkthroughAdds Zcash (ZIP-244 v4/v5) support to the Ledger integration, including network/config entries, signing/PSBT changes with a trusted-input monkey-patch, docs for Ledger implementation, and multiple package version/dependency bumps (including Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 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-08-07T15:47:29.207ZApplied to files:
📚 Learning: 2025-08-07T15:47:26.835ZApplied to files:
📚 Learning: 2025-12-09T13:48:55.139ZApplied to files:
📚 Learning: 2025-08-07T15:24:34.076ZApplied 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 (15)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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: 5
♻️ Duplicate comments (1)
packages/hdwallet-ledger-webusb/package.json (1)
17-22: WebUSB Ledger version pins consistent; same hw-app-btc alignment question as WebHIDThese Ledger dependency bumps look reasonable and match the WebHID package, including the pin of
@ledgerhq/hw-transportto6.31.13. As noted in the WebHID review,@ledgerhq/hw-app-btcis10.13.0here whilepackages/hdwallet-ledger/package.jsonpins10.12.0; consider aligning or documenting the difference to avoid multiple hw-app-btc copies.
📜 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 (8)
ZCASH_LEDGER_IMPLEMENTATION.md(1 hunks)lerna.json(1 hunks)packages/hdwallet-ledger-webhid/package.json(1 hunks)packages/hdwallet-ledger-webusb/package.json(1 hunks)packages/hdwallet-ledger/package.json(1 hunks)packages/hdwallet-ledger/src/bitcoin.ts(6 hunks)packages/hdwallet-ledger/src/currencies.ts(1 hunks)packages/hdwallet-ledger/src/utils.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 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:
ZCASH_LEDGER_IMPLEMENTATION.mdpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-ledger/src/bitcoin.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-webusb/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-ledger/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-webusb/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/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-webusb/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-ledger/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-ledger-webusb/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-ledger/src/bitcoin.ts
📚 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-ledger-webusb/package.jsonpackages/hdwallet-ledger-webhid/package.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/utils.tspackages/hdwallet-ledger/src/bitcoin.ts
🧬 Code graph analysis (1)
packages/hdwallet-ledger/src/bitcoin.ts (2)
packages/hdwallet-core/src/bitcoin.ts (1)
BTCSignTxInputLedger(135-139)packages/hdwallet-ledger/src/utils.ts (1)
networksUtil(121-592)
🪛 markdownlint-cli2 (0.18.1)
ZCASH_LEDGER_IMPLEMENTATION.md
43-43: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (5)
lerna.json (1)
11-16: Formatting improvement—no semantic changes.The
ignoreChangesarray has been reformatted for readability with no impact on configuration logic.packages/hdwallet-ledger/src/utils.ts (1)
466-488: Zcash network wiring and appName mapping look consistentThe SLIP‑133 entry (unit/name/apiName,
pubKeyHash/scriptHash0x1cb8/0x1cbd,additionals: ["zcash","sapling"],areTransactionTimestamped: true) andisSegwitSupported: truetogether give you the rightsplitTransaction(hex, true, true, ["zcash","sapling"])behavior, while still only exposing legacy P2PKH at higher layers. Adding133: "Zcash"toappNameBySlip44keepscoinToLedgerAppNamein sync with the new coin.Also applies to: 602-602
packages/hdwallet-ledger/src/currencies.ts (1)
22-25: Zcash currency entry completes btcGetPublicKeys mappingAdding
ZcashwithxpubVersion: 0x0488b21ematches the standard transparent ZEC HD key versions and keepscurrencies[coin]usable forbtcGetPublicKeyswhencoin === "Zcash". No issues here.packages/hdwallet-ledger-webhid/package.json (1)
17-20: Verify hw-app-btc version alignment across Ledger packagesThe version mismatch is real:
@ledgerhq/hw-app-btcdiffers across the workspace—packages/hdwallet-ledgeruses10.12.0, whilepackages/hdwallet-ledger-webhidandpackages/hdwallet-ledger-webusbboth use10.13.0. While the reviewed file is already at the newer version, ensure this split is intentional or alignpackages/hdwallet-ledgerto10.13.0as well to avoid duplicate package copies in the workspace.packages/hdwallet-ledger/src/bitcoin.ts (1)
238-245: Zcash input blockHeight wiring and trusted-input metadata look sound; sequences intentionally defaultedThe Zcash implementation in
btcSignTxis coherent:
- Per‑input
blockHeightis collected intoblockHeights[](line 238) and passed as the 5th tuple element exclusively whenmsg.coin === "Zcash"(lines 357–365), matching Ledger's ZEC API signature[tx, index, redeemScript?, sequence?, blockHeight?].- Global
blockHeightis set from the first input's height (lines 396–399), mirroring Ledger Live's pattern.expiryHeightdefaults toBuffer.alloc(4)(4‑byte zero buffer, line 400) for no expiry.- Sequences are deliberately left as an empty array (line 351), allowing all coins—including Zcash—to use Ledger's
DEFAULT_SEQUENCE (0xffffffff). The code explicitly warns that setting sequence to 0 would enable RBF/locktime, which causes issues.- For Zcash only,
_customZcashTxIdand_customZcashAmountare attached to each input's transaction object (lines 375–376), exactly what the monkey patch expects, gated behindmsg.coin === "Zcash".Non‑Zcash flows remain unchanged.
- Align @ledgerhq/hw-app-btc to v10.13.0 across all packages - Fix bare URL markdown lint issue in ZCASH_LEDGER_IMPLEMENTATION.md - Update expiry height documentation to reflect actual implementation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
gomesalexandre
changed the title
Add Zcash with SpendAddress (P2PKH) support to the supported script types map. This ensures path validation and capability checking work correctly for Zcash. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/hdwallet-ledger/src/bitcoin.ts (1)
325-366: Validate required Zcash input metadata (txid, amount, blockHeight) at transaction creation timeFor Zcash correctness, three per-input fields are critical:
txidandamountfor the trusted-input monkey patch (_customZcashTxId,_customZcashAmount)blockHeightfor Ledger's consensusBranchId calculationCurrently, all three are optional. If any are missing, the code silently falls back to the original (broken-for-ZEC)
getTrustedInputBIP143, potentially yielding invalid ZEC transactions instead of failing loudly.For the Zcash code path, throw an error if any input is missing
txid,amount, orblockHeight. This surfaces upstream integration bugs at creation time rather than at broadcast or validation time.Also applies to: 368-380
🧹 Nitpick comments (4)
packages/hdwallet-ledger/src/bitcoin.ts (3)
2-3: Consider insulating against BitGo internal path changesImporting
ZcashPsbt/ZcashTransactionfrom@bitgo/utxo-lib/dist/src/bitgoties you to an internal file layout that could change in a minor update. If BitGo exposes these types via the top-level entry point (now or in the future), it would be safer to import from there or to wrap this deep import in a tiny local adapter so only one place needs updating if BitGo reorganizes.
19-25: Clarify v4/v5 version-group handling for future maintainersYou use
ZcashTransaction.VERSION4_BRANCH_NU6_1when callingsetDefaultsForVersion, then explicitly overwriteversionGroupIdwithZCASH_VERSION_GROUP_ID[5](v5) andconsensusBranchIdwithZCASH_CONSENSUS_BRANCH_ID. That combination is subtle and easy to “fix” incorrectly later.I’d suggest adding a short comment explaining why
VERSION4_BRANCH_NU6_1plus the v5versionGroupIdis intentional and what exact header (version + groupId + branchId) the network currently expects. That will prevent accidental regressions if someone tries to “clean this up.”Also applies to: 288-294
41-82: Validate Zcash-specific txid/amount before constructing the trusted-input bufferThe monkey patch cleanly gates on
additionals.includes("zcash")and the presence of_customZcashTxId/_customZcashAmount, which is good. One remaining edge case is malformed data: if either field is present but not well-formed (e.g., txid not 64 hex chars, amount not an integer string),Buffer.from(txid, "hex")/BigInt(amount)will throw in a slightly opaque way.Consider a minimal guard here (length check on
txid,Number.isInteger(BigInt(...))try/catch, or at least a more specific error) so ZEC integration issues fail with a clear message rather than a generic buffer/BigInt exception.ZCASH_LEDGER_IMPLEMENTATION.md (1)
1-101: Documentation now accurately reflects the implementation and constraintsThis guide lines up well with the code: SLIP-44 path, transparent-only support, NU6.1 branch ID, the strict
@ledgerhq/hw-app-btc@10.13.0pin, the Zcash-specificsplitTransactionflags, the trusted-input monkey patch, and the hardcodedexpiryHeight = 0are all correctly captured. It’s a solid reference for anyone touching the ZEC path later.Only suggestion: when the network or Ledger libs evolve again (new consensusBranchId, v5 support, etc.), this doc should be updated in lockstep with the code so the “Known Limitations” and branch IDs stay authoritative.
📜 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 (3)
ZCASH_LEDGER_IMPLEMENTATION.md(1 hunks)packages/hdwallet-ledger/package.json(1 hunks)packages/hdwallet-ledger/src/bitcoin.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 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.tsZCASH_LEDGER_IMPLEMENTATION.mdpackages/hdwallet-ledger/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-ledger/src/bitcoin.tsZCASH_LEDGER_IMPLEMENTATION.mdpackages/hdwallet-ledger/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-ledger/src/bitcoin.tsZCASH_LEDGER_IMPLEMENTATION.mdpackages/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/src/bitcoin.tsZCASH_LEDGER_IMPLEMENTATION.mdpackages/hdwallet-ledger/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/src/bitcoin.tspackages/hdwallet-ledger/package.json
📚 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-ledger/src/bitcoin.ts
📚 Learning: 2025-10-15T23:22:26.842Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 731
File: packages/hdwallet-gridplus/src/thormaya.ts:99-105
Timestamp: 2025-10-15T23:22:26.842Z
Learning: In packages/hdwallet-gridplus/src/thormaya.ts, the GridPlus SDK (gridplus-sdk) automatically pads the r and s signature components to 32 bytes, so explicit padding in the code may be redundant but is not required. The thorchainSignTx implementation works without explicit padding because the SDK handles it.
Applied to files:
packages/hdwallet-ledger/src/bitcoin.ts
🧬 Code graph analysis (1)
packages/hdwallet-ledger/src/bitcoin.ts (1)
packages/hdwallet-core/src/bitcoin.ts (1)
BTCSignTxInputLedger(135-139)
🔇 Additional comments (4)
packages/hdwallet-ledger/package.json (1)
18-27: Ledger dependency pins and BitGo PSBT lib align with Zcash integrationThe new
@bitgo/utxo-libaddition and the strict pins on Ledger deps (including@ledgerhq/hw-app-btc@10.13.0and@ledgerhq/hw-transport@6.31.13) look consistent with the Zcash flow and with the existing convention of pinninghw-transportexactly. No issues from my side here.Based on learnings, hw-transport is expected to remain an exact pin while other Ledger packages may also be pinned when stability is critical.
packages/hdwallet-ledger/src/bitcoin.ts (3)
84-93: Zcash support matrix (coin + script type) looks consistent
"Zcash"is now included insupportedCoins, andbtcSupportsScriptTypeadvertises onlySpendAddressfor ZEC, which matches the “transparent P2PKH only” constraint from the docs. This keeps capability checks in hdwallet-core aligned with what the adapter actually supports.Also applies to: 101-110
227-274: BitGo PSBT usage for Zcash and output handling look correctUsing BitGo’s Zcash PSBT (
createPsbtForNetworkwithZcashTransaction.VERSION4_BRANCH_NU6_1) and switching to{ script, value }outputs viazcashAddress.toOutputScriptis the right approach for ZEC. The OP_RETURN handling reuses the same PSBT instance and usesBigIntvalues consistently, which lines up with modern PSBT expectations. No additional changes suggested here.Also applies to: 286-307
382-402: Zcash-specific CreateTransactionArg fields align with the documented behaviorThe
additionals: ["zcash", "sapling"], per-transactionblockHeightderived from the first input, andexpiryHeight: Buffer.alloc(4)(no expiry) match what’s described inZCASH_LEDGER_IMPLEMENTATION.md. Given the comments about Ledger Live using a current-block-height approximation, this shape looks reasonable and keeps non-ZEC coins unchanged.
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
https://jam.dev/c/05875136-2aef-43ab-ac1c-737b260dff53
Signing/derivations/send seems to be working as expected! 🐐
gomesalexandre
force-pushed
the
feat_ledger_zcash
branch
from
3f55fda to
feb9c64
Compare
gomesalexandre
force-pushed
the
feat_ledger_zcash
branch
from
feb9c64 to
f82a981
Compare
There was a problem hiding this comment.
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.
📒 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 due to trivial changes (2)
- packages/hdwallet-trezor-connect/package.json
- packages/hdwallet-trezor/package.json
🚧 Files skipped from review as they are similar to previous changes (5)
- lerna.json
- packages/hdwallet-keepkey/package.json
- packages/hdwallet-ledger/package.json
- packages/hdwallet-ledger-webusb/package.json
- packages/hdwallet-ledger-webhid/package.json
🧰 Additional context used
🧠 Learnings (7)
📓 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: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:
integration/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-walletconnect/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-webusb/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:
integration/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-walletconnect/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-webusb/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:
integration/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-walletconnect/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-webusb/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:
integration/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-walletconnect/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-webusb/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:
integration/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-coinbase/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-webusb/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:
integration/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-walletconnect/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keepkey-webusb/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-nodehid/package.json (1)
3-3: Inconsistency between AI summary and actual code.The AI summary states that "@shapeshiftoss/hdwallet-core" was updated, but the actual file shows "@shapeshiftoss/hdwallet-keepkey" was updated at line 17.
The version bumps themselves look correct: both the package version and its dependency are consistently bumped from 1.62.25 to 1.62.26. This aligns with standard monorepo version coordination practices.
Also applies to: 17-17
packages/hdwallet-keepkey-electron/package.json (1)
3-3: Version and dependency bump are consistent.The package version and the
@shapeshiftoss/hdwallet-keepkeydependency are both correctly updated from 1.62.25 to 1.62.26. This appears to be part of a coordinated monorepo-wide version bump, which aligns with the expected release workflow.Also applies to: 17-17
integration/package.json (1)
3-3: Version bump is consistent across all packages.All @shapeshiftoss/hdwallet-* dependencies are updated to 1.62.26, matching the integration package version. The exact version pinning (no caret ranges) is appropriate for internal monorepo dependencies.
Per learnings from this team's workflow, version bumps across interdependent packages in this monorepo occur in PRs before publishing, and references to unpublished versions during PR review is expected behavior. This follows the established pattern for coordinating feature additions (like Zcash support) across the hdwallet ecosystem.
Also applies to: 14-23
examples/sandbox/package.json (1)
3-3: Version bump aligns with PR objectives.The sandbox example is correctly updated to v1.62.26 with all @shapeshiftoss/hdwallet-* dependencies pinned to the same exact version. This ensures consistency across the example as the main hdwallet packages are updated for ZCash support. The exact version pins align with the monorepo's release workflow, where unpublished versions are referenced during the PR review phase before publication.
Also applies to: 15-32
packages/hdwallet-portis/package.json (1)
3-3: Version and dependency bumps look good.The patch-level version bump (1.62.25 → 1.62.26) and the corresponding @shapeshiftoss/hdwallet-core dependency update are consistent with the monorepo's release workflow for this feature addition.
Also applies to: 19-19
packages/hdwallet-phantom/package.json (1)
3-3: Version bumps are consistent with monorepo release cycle.The patch-level version bump (1.62.25 → 1.62.26) and corresponding update to the internal
@shapeshiftoss/hdwallet-coredependency align with the monorepo's release pattern. Since Phantom (Solana support) is unaffected by the ZCash/Ledger changes, no external dependency modifications are required here.Also applies to: 18-18
packages/hdwallet-walletconnectV2/package.json (1)
3-3: Version and dependency bump aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core dependency. Based on learnings, version bumps in PRs before publishing are expected behavior in this monorepo's release workflow.
Also applies to: 18-18
packages/hdwallet-vultisig/package.json (1)
3-3: Version and dependency bump aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core dependency.
Also applies to: 20-20
packages/hdwallet-walletconnect/package.json (1)
3-3: Version and dependency bump aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core dependency.
Also applies to: 18-18
packages/hdwallet-core/package.json (1)
3-3: Core package version bump consistent with downstream updates.Patch version update (1.62.25 → 1.62.26) for the foundation package; all dependents correctly updated their hdwallet-core references to match.
packages/hdwallet-keplr/package.json (1)
3-3: Version and dependency bump aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core dependency.
Also applies to: 20-20
packages/hdwallet-keepkey-chromeusb/package.json (1)
3-3: Version and dependency bumps aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core and keepkey dependencies.
Also applies to: 17-17, 18-18
packages/hdwallet-keepkey-webusb/package.json (1)
3-3: Version and dependency bumps aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core and keepkey dependencies.
Also applies to: 17-17, 18-18
packages/hdwallet-keepkey-nodewebusb/package.json (1)
3-3: Version and dependency bumps aligned with monorepo release.Straightforward patch version update (1.62.25 → 1.62.26) with matching core and keepkey dependencies.
Also applies to: 17-17, 18-18
packages/hdwallet-metamask-multichain/package.json (1)
3-3: Version bump aligns with monorepo release.The package and its core dependency have been updated consistently to version 1.62.26. The exact pinning of
@shapeshiftoss/hdwallet-corematches the monorepo's internal dependency strategy.Also applies to: 20-20
packages/hdwallet-coinbase/package.json (1)
3-3: Version bump aligns with monorepo release.The package and
@shapeshiftoss/hdwallet-coredependency updated consistently to 1.62.26 with exact pinning matching the monorepo pattern.Also applies to: 18-18
packages/hdwallet-native-vault/package.json (1)
3-3: Version bump aligns with monorepo release.Package and
@shapeshiftoss/hdwallet-nativedependency updated consistently to 1.62.26 with exact pinning.Also applies to: 18-18
packages/hdwallet-gridplus/package.json (1)
3-3: Version bump aligns with monorepo release.Package and
@shapeshiftoss/hdwallet-coredependency updated consistently to 1.62.26 with exact pinning. Specialized dependency configuration (nohoist for ethereumjs) preserved.Also applies to: 23-23
packages/hdwallet-native/package.json (1)
3-3: Version bump aligns with monorepo release.Package and
@shapeshiftoss/hdwallet-coredependency updated to 1.62.26 with exact pinning. Other pinned dependencies (@shapeshiftoss/bitcoinjs-lib7.0.0-shapeshift.2,@shapeshiftoss/proto-tx-builder0.10.0) remain appropriately unchanged.Also applies to: 20-20
packages/hdwallet-keepkey-tcp/package.json (1)
3-3: Version and dependency bumps are consistent with the broader release.The patch version bump to 1.62.26 and synchronized dependency updates align with the expected monorepo release workflow. Based on learnings, version bumping before publishing is standard practice in this repository.
Also applies to: 17-18
Jest v26 doesn't properly support package.json exports field for subpath imports. Added explicit mapping for @brandonblack/musig/base_crypto to work around this limitation. This fixes integration test failures where @bitgo/secp256k1 couldn't import from @brandonblack/musig/base_crypto 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
gomesalexandre
force-pushed
the
feat_ledger_zcash
branch
from
a33e729 to
edadf99
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
integration/jest.config.js (1)
10-16: Preferrequire.resolvefor the new musig module alias for robustness.The mapper looks fine, but hard‑coding the
node_modulespath is a bit brittle compared to the existingrequire.resolveusage. Consider aligning this entry with the others for better resilience to layout changes:- "^@brandonblack/musig/base_crypto$": "<rootDir>/../../node_modules/@brandonblack/musig/lib/base_crypto.js", + "^@brandonblack/musig/base_crypto$": require.resolve("@brandonblack/musig/lib/base_crypto.js"),
📜 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 (2)
integration/jest.config.js(1 hunks)integration/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 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: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:
integration/jest.config.js
📚 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:
integration/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:
integration/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 (1)
integration/package.json (1)
14-24: Adding@bitgo/utxo-libto integration looks correct; double‑check version alignment on release.The new dependency here makes sense given the move to
@bitgo/utxo-libfor PSBT construction and keeps the integration package in sync with the Ledger changes. If this release also bumps the monorepo to1.62.26, ensure this package’s own version and its@shapeshiftoss/hdwallet-*dependency versions are updated in the same sweep so integration tests exercise the latest code paths.
Description
This PR:
@bitgo/utxo-libfor Zcash (much saner than@shapeshiftoss/bitcoinjs-libfork/patchgetTrustedInputBIP143method from Ledgerhw-app-btcand ensures my sanity leaves my bodyIssue
Testing
Screenshots
https://jam.dev/c/3c325702-6910-45d8-8eed-6fbd4e69e43c
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.