Skip to content

feat: hdwallet-trezor Zcash support#766

Merged
gomesalexandre merged 6 commits intomasterfrom
feat_zcash_trezor
Dec 10, 2025
Merged

feat: hdwallet-trezor Zcash support#766
gomesalexandre merged 6 commits intomasterfrom
feat_zcash_trezor

Conversation

@gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 10, 2025

Description

Does what it says on the box - Claude deets below

Add Zcash-specific transaction parameters to Trezor signing:

  • Add ZCASH_VERSION_GROUP_ID constants (v4, v5)
  • Add ZCASH_CONSENSUS_BRANCH_ID constant (NU6.1)
  • Pass version, versionGroupId, and branchId to Trezor Connect

Implementation follows native wallet pattern (PR #760):

  • Defaults to version 5 (NU5) if not specified
  • Calculates versionGroupId based on transaction version
  • Uses consensus branch ID 0x4dec4df0 for NU6.1

Fixes broadcast error: "transaction uses an incorrect consensus
branch id" when signing Zcash transactions with Trezor.

Requires Trezor firmware v1.11.1+ (T1) or v2.5.1+ (TT) for NU5 support.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Screenshots

https://jam.dev/c/673911a2-c6f6-45f3-baa9-e9d59a0535a4

Issue

Summary by CodeRabbit

  • New Features
    • Added Zcash support to Trezor hardware wallet integration, enabling users to sign Zcash transactions with proper protocol compliance and parameter handling.

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

gomesalexandre and others added 3 commits December 10, 2025 11:13
Add Zcash-specific transaction parameters to Trezor signing:
- Add ZCASH_VERSION_GROUP_ID constants (v4, v5)
- Add ZCASH_CONSENSUS_BRANCH_ID constant (NU6.1)
- Pass version, versionGroupId, and branchId to Trezor Connect

Implementation follows native wallet pattern (PR #760):
- Defaults to version 5 (NU5) if not specified
- Calculates versionGroupId based on transaction version
- Uses consensus branch ID 0x4dec4df0 for NU6.1

Fixes broadcast error: "transaction uses an incorrect consensus
branch id" when signing Zcash transactions with Trezor.

Requires Trezor firmware v1.11.1+ (T1) or v2.5.1+ (TT) for NU5 support.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@gomesalexandre gomesalexandre requested a review from a team as a code owner December 10, 2025 09:48
@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Warning

Rate limit exceeded

@gomesalexandre has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 28 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 603bea6 and 206ff60.

📒 Files selected for processing (28)
  • 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-trezor/src/bitcoin.ts (2 hunks)
  • packages/hdwallet-vultisig/package.json (2 hunks)
  • packages/hdwallet-walletconnect/package.json (2 hunks)
  • packages/hdwallet-walletconnectV2/package.json (2 hunks)
📝 Walkthrough

Walkthrough

The Bitcoin transaction signing module for Trezor devices is extended with Zcash support. New constants map Zcash version numbers to group IDs and branch IDs. The signing function detects Zcash transactions and conditionally adds version, versionGroupId, and branchId fields to the payload.

Changes

Cohort / File(s) Change Summary
Zcash Transaction Signing Support
packages/hdwallet-trezor/src/bitcoin.ts
Adds Zcash constants (ZCASH_VERSION_GROUP_ID, ZCASH_CONSENSUS_BRANCH_ID); modifies btcSignTx to detect Zcash transactions, default version to 5 for Zcash, and include Zcash-specific fields in signTransaction payload via conditional spread operator

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify Zcash version and branch ID constant mappings are correct and align with Zcash protocol specifications
  • Confirm the version default logic (version 5 for Zcash) is appropriate and doesn't conflict with existing transaction types
  • Check conditional spread syntax and ensure Zcash fields are only included when signing Zcash transactions
  • Verify backward compatibility: non-Zcash paths remain unaffected

Poem

🐰 A Zcash trail through Bitcoin's gate,
Version five and branch IDs await,
Constants defined with careful thought,
New signatures that Trezor wrought! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding Zcash support to the hdwallet-trezor package, which is the primary objective of this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 49e30c7 and 603bea6.

📒 Files selected for processing (1)
  • packages/hdwallet-trezor/src/bitcoin.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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-trezor/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-trezor/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-trezor/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 (1)
packages/hdwallet-trezor/src/bitcoin.ts (1)

34-37: No issues identified. The version group ID values match official Zcash protocol specifications: version 4 uses 0x892f2085 (Sapling/Overwinter format per ZIP-2003) and version 5 uses 0x26a7270a (NU5/Orchard format per ZIP-225).

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

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

image

Derivation is fine, signing is fine, I guess we can give this a go!

@gomesalexandre gomesalexandre merged commit 5f368c0 into master Dec 10, 2025
3 checks passed
@gomesalexandre gomesalexandre deleted the feat_zcash_trezor branch December 10, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants