Skip to content

Conversation

@kaladinlight
Copy link
Contributor

@kaladinlight kaladinlight commented Nov 21, 2025

Summary by CodeRabbit

  • New Features

    • Unified Bitcoin script types: introduced BTCScriptType (Legacy, LegacyMultisig, Segwit, SegwitNative).
  • Updates

    • Replaced BTCInputScriptType/BTCOutputScriptType with BTCScriptType across public APIs, tests, and examples.
    • Many method signatures and path-description outputs updated to use BTCScriptType.
  • Improvements

    • Centralized UTXO path description/delegation and simplified network/script-type handling and naming across wallets and integrations.

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

gomesalexandre and others added 30 commits October 8, 2025 00:42
Complete GridPlus Lattice integration with EIP-1559 support, EIP-712 signing, on-device calldata decoding, and connection persistence.

Features:
- Full EIP-1559 transaction support with proper yParity handling
- EIP-712 typed data signing with general signing mode
- On-device calldata decoding for clear signing (FOX, RFOX contracts)
- Connection persistence via privKey for seamless reconnection
- Request deduplication and caching for optimal performance
- Proper transaction construction using @ethereumjs/tx

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

Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Enable Solana address derivation by setting support flags to true.
Solana implementation uses ED25519_PUB flag with path m/44'/501'/account'/0'.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Check that Lattice firmware >= 0.14.0 before attempting Solana address
derivation with ED25519 flag. Add console log to help debug firmware version issues.
Also add iterIdx: 0 parameter explicitly to getAddresses call.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Switch from direct client.getAddresses() to SDK's fetchSolanaAddresses() wrapper
which requires proper global state management. Initialize SDK state in transport
setup with localStorage callbacks for proper client persistence.

Changes:
- Add SDK state initialization (setLoadClient/setSaveClient) in transport.ts
- Use fetchSolanaAddresses() wrapper instead of direct client calls
- Add localStorage getters/setters for client state persistence
- Fetch 10 addresses at once for better performance (SDK default)

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Add bs58 dependency for Solana address encoding
- SDK fetchSolanaAddresses returns Buffer objects
- Convert 32-byte public key buffers to base58 addresses

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Cache all 10 addresses returned from fetchSolanaAddresses
- Check cache before fetching to avoid redundant requests
- Reduces device queries for sequential account indices

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Add Bitcoin support (Legacy, SegWit, Wrapped SegWit)
- Add Dogecoin support (Legacy only)
- Implement Solana signing with SDK wrapper
- Implement Bitcoin/Dogecoin signing with SDK wrappers
- Batch address caching for all chains (10 addresses at once)
- Support flags for BTC and BTCInfo

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Add CosmosWallet and ThorchainWallet interface implementation
- Implement cosmosGetAddress with bech32 encoding (using secp256k1 pubkey)
- Implement thorchainGetAddress with bech32 encoding (thor/tthor prefix)
- Add CosmosWalletInfo and ThorchainWalletInfo support
- Add dependencies: bech32, crypto-js for address derivation
- Cosmos and THORChain signing methods stubbed (to be implemented)

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Downgrade bech32 from ^2.0.0 to ^1.1.4 (match hdwallet-native)
- Remove @types/bech32 devDependency (not needed)
- Fixes TypeScript build errors caused by version mismatch

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

Co-Authored-By: Claude <noreply@anthropic.com>
Add getPublicKeys() method to retrieve xpubs for account discovery:
- Support secp256k1 curve (Bitcoin, Dogecoin) using SECP256K1_XPUB flag
- Support ed25519 curve (Solana) using ED25519_PUB flag
- Uses fetchAddressesByDerivationPath() from GridPlus SDK
- Returns array of PublicKey objects with xpub strings
- Error handling with null fallback for failed requests

This fixes the "GridPlus public key retrieval not implemented yet" error
in UTXO chain adapters for account discovery.

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix getPublicKeys() path parsing (remove "m/" prefix for GridPlus SDK)
- Implement cosmosSignTx() using GridPlus general signing with secp256k1
- Implement thorchainSignTx() using GridPlus general signing with secp256k1
- Add required dependencies: @cosmjs/amino, @cosmjs/stargate, proto-tx-builder, p-lazy, bs58check
- Create signer adapter for proto-tx-builder integration
- Sign Amino documents with SHA256 hash using GridPlus device

This enables Cosmos and THORChain transaction signing on GridPlus Lattice.

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

Co-Authored-By: Claude <noreply@anthropic.com>
…rt isGridPlus

- Fix cosmosGetAddress() to strip 'm/' prefix from path
- Fix thorchainGetAddress() to strip 'm/' prefix from path
- Export isGridPlus() helper function
- Add comprehensive debug logging to getPublicKeys()

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Refactor ethGetAddress to use fetchAddresses with batch caching (fetch 10 at once)
- Add convertXpubVersion utility to convert xpub→dgub for Dogecoin
- Fix Solana cache key format to match msg.addressNList
- Fix Cosmos/THORChain Buffer→hex conversion for createCosmosAddress

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix cosmosGetAddress and thorchainGetAddress to use correct BIP32 paths
  * Extract address index from last path element
  * Pass base path (without index) to fetchAddressesByDerivationPath
  * This fixes wrong address derivation for GridPlus Cosmos/THORChain

- Add complete MAYAchain support to GridPlus wallet
  * Implement mayachainGetAddress() and mayachainSignTx()
  * Add mayachainGetAccountPaths() and mayachainNextAccountPath()
  * MAYAchain uses slip44=931 and "maya" bech32 prefix

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

Co-Authored-By: Claude <noreply@anthropic.com>
Add setupWithoutConnect() method to GridPlusTransport that skips client.connect() call when reconnecting with existing privKey, avoiding unnecessary pairing screen on device.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Track full pairing flow including:
- Transport creation and reuse
- Client initialization and SDK setup
- connect() and pair() calls with errors
- Wallet creation steps

Debugging ephemeralPub error and dead click issue.

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

Co-Authored-By: Claude <noreply@anthropic.com>
When client.connect() fails with "Device Locked", the SDK client is left
in an invalid state without ephemeral keys. This causes pair() to fail
with ephemeralPub error. Retry connect() before pair() to fix.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Remove duplicate Client creation that caused ephemeralPub errors.
Now properly using SDK's setup() + getClient() workflow instead of
creating both SDK client AND our own client instance.

Fixes:
- ephemeralPub error on initial pairing
- Device Locked error handling
- Reconnection flow with SDK's stored state

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

Co-Authored-By: Claude <noreply@anthropic.com>
…P44 paths

GridPlus was incorrectly deriving Cosmos SDK chains (ATOM, THOR, MAYA) from
4-level paths instead of the standard 5-level BIP44 paths.

Root cause: Code was removing the last element from addressNList before
calling fetchAddressesByDerivationPath() with startPathIndex. However, the
SDK function ignores startPathIndex when no wildcard is present in the path,
resulting in derivation from the wrong (truncated) path.

Example bug for m/44'/118'/0'/0/0:
- Before: Derived from m/44'/118'/0'/0 (WRONG)
- After: Derives from m/44'/118'/0'/0/0 (CORRECT)

Fixed functions:
- cosmosGetAddress, cosmosSignTx
- thorchainGetAddress, thorchainSignTx
- mayachainGetAddress, mayachainSignTx

This matches the derivation used by Native, KeepKey, and other wallets.

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

Co-Authored-By: Claude <noreply@anthropic.com>
GridPlus SDK returns uncompressed 65-byte secp256k1 pubkeys, but Cosmos SDK
chains (ATOM, THOR, MAYA) require compressed 33-byte pubkeys for address
derivation. Without compression, completely different addresses are generated.

Changes:
- Add @bitcoinerlab/secp256k1 dependency to hdwallet-gridplus
- Compress pubkeys in cosmosGetAddress, thorchainGetAddress, mayachainGetAddress
- Compress pubkeys in cosmosSignTx, thorchainSignTx, mayachainSignTx
- Add migration 6 in web to clear Cosmos SDK AccountIds for re-derivation

This fixes wrong address generation for Cosmos SDK chains when using GridPlus.

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

Co-Authored-By: Claude <noreply@anthropic.com>
kaladinlight and others added 22 commits October 24, 2025 15:25
Use TransactionFactory with explicit type field for proper transaction type detection.
Legacy (type 0) transactions return arrays that need RLP encoding, while
EIP-1559 (type 2) transactions are pre-encoded.

Key changes:
- Add explicit `type` field (0 for legacy, 2 for EIP-1559) to transaction data
- Conditionally encode payload based on type: Array.isArray(raw) ? encode(raw) : raw
- Use TransactionFactory for both unsigned and signed transactions
- Maintain Kevin's cleaner code structure while fixing the regression

Version: 1.62.4-alpha.238

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

Co-Authored-By: Claude <noreply@anthropic.com>
@kaladinlight kaladinlight requested a review from a team as a code owner November 21, 2025 20:54
@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Warning

Rate limit exceeded

@kaladinlight has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 3 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 a1a7c01 and 9f63742.

📒 Files selected for processing (40)
  • examples/sandbox/index.ts (33 hunks)
  • integration/src/bitcoin/bitcoin.ts (8 hunks)
  • integration/src/bitcoin/litecoin.ts (2 hunks)
  • integration/src/bitcoin/testnet.ts (4 hunks)
  • integration/src/wallets/keepkey.ts (12 hunks)
  • integration/src/wallets/ledger.ts (5 hunks)
  • integration/src/wallets/native.ts (9 hunks)
  • integration/src/wallets/phantom.ts (4 hunks)
  • integration/src/wallets/trezor.ts (5 hunks)
  • integration/src/wallets/vultisig.ts (3 hunks)
  • packages/hdwallet-core/src/bitcoin.ts (14 hunks)
  • packages/hdwallet-core/src/networks.ts (2 hunks)
  • packages/hdwallet-core/src/utxoUtils.ts (1 hunks)
  • packages/hdwallet-core/src/wallet.ts (3 hunks)
  • packages/hdwallet-gridplus/src/bitcoin.ts (3 hunks)
  • packages/hdwallet-gridplus/src/constants.ts (0 hunks)
  • packages/hdwallet-gridplus/src/gridplus.ts (2 hunks)
  • packages/hdwallet-keepkey/src/bitcoin.ts (5 hunks)
  • packages/hdwallet-keepkey/src/keepkey.ts (3 hunks)
  • packages/hdwallet-keepkey/src/utils.ts (1 hunks)
  • packages/hdwallet-ledger/src/bitcoin.ts (8 hunks)
  • packages/hdwallet-ledger/src/ethereum.ts (1 hunks)
  • packages/hdwallet-ledger/src/ledger.ts (2 hunks)
  • packages/hdwallet-ledger/src/utils.ts (1 hunks)
  • packages/hdwallet-metamask-multichain/src/shapeshift-multichain.ts (1 hunks)
  • packages/hdwallet-metamask-multichain/src/utxo.ts (2 hunks)
  • packages/hdwallet-native/src/bitcoin.test.ts (1 hunks)
  • packages/hdwallet-native/src/bitcoin.ts (4 hunks)
  • packages/hdwallet-native/src/native.test.ts (3 hunks)
  • packages/hdwallet-native/src/native.ts (1 hunks)
  • packages/hdwallet-native/src/util.ts (1 hunks)
  • packages/hdwallet-phantom/src/bitcoin.ts (4 hunks)
  • packages/hdwallet-phantom/src/phantom.ts (3 hunks)
  • packages/hdwallet-portis/src/bitcoin.ts (3 hunks)
  • packages/hdwallet-portis/src/portis.ts (2 hunks)
  • packages/hdwallet-trezor/src/bitcoin.ts (4 hunks)
  • packages/hdwallet-trezor/src/trezor.ts (4 hunks)
  • packages/hdwallet-vultisig/src/bitcoin.ts (4 hunks)
  • packages/hdwallet-vultisig/src/vultisig.test.ts (4 hunks)
  • packages/hdwallet-vultisig/src/vultisig.ts (1 hunks)
📝 Walkthrough

Walkthrough

Consolidates Bitcoin script-type enums by replacing BTCInputScriptType/BTCOutputScriptType with a unified BTCScriptType across core, networks, utilities, wallet implementations, tests, and examples; updates signatures, mappings, describe/unknown UTXO path delegation, and network/bip32 handling accordingly.

Changes

Cohort / File(s) Summary
Core: bitcoin
packages/hdwallet-core/src/bitcoin.ts
Removed BTCInputScriptType/BTCOutputScriptType; added BTCScriptType (Legacy, LegacyMultisig, Segwit, SegwitNative). Updated all BTC public types, account constructors, createPayment, describe/unknown UTXO path signatures and logic.
Core: networks & utils
packages/hdwallet-core/src/networks.ts, packages/hdwallet-core/src/utxoUtils.ts, packages/hdwallet-core/src/wallet.ts
Reworked BIP32 shape (public/private), switched BIP32 maps to BTCScriptType keys, updated getNetwork (default Legacy) and scriptType→account type mappings; updated wallet-related interfaces to use BTCScriptType.
Examples & Integration tests
examples/sandbox/index.ts, integration/src/bitcoin/*, integration/src/wallets/*
Replaced all test/example usages and expectations to use core.BTCScriptType variants and updated verbose path strings accordingly.
Wallets: core adapters
packages/hdwallet-*/src/* (KeepKey, Ledger, Trezor, Native, Phantom, GridPlus, Portis, Vultisig, GridPlus constants removal)
Replaced script-type enums/signatures with BTCScriptType, updated btcSupportsScriptType, btcGetAddress, btcSignTx, btcSignMessage, describeUTXOPath delegations, account path builders and internal mappings. Removed or delegated various in-file path parsing utilities to core. packages/hdwallet-gridplus/src/constants.ts removed.
Packages: native/gridplus/others tests
packages/hdwallet-native/src/*, packages/hdwallet-gridplus/src/*, packages/hdwallet-*/src/*/*.test.ts
Adjusted tests and internal helpers to the new enum and changed some defaults (e.g., network resolution no longer defaults to PayToMultisig). Tests updated to expect BTCScriptType members.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant WalletImpl as Wallet Implementation
  participant Core as `@shapeshiftoss/hdwallet-core`
  participant Network as getNetwork / bip32

  Note over WalletImpl,Core: Old flow (prior to this diff)
  WalletImpl->>WalletImpl: describeUTXOPath (manual parsing)
  WalletImpl--xCore: sometimes used core.unknownUTXOPath
  WalletImpl->>Network: getNetwork(coin, BTCOutputScriptType.*)
  WalletImpl-->>WalletImpl: build verbose description

  Note over WalletImpl,Core: New flow (after this diff)
  WalletImpl->>Core: describeUTXOPath(path, coin, BTCScriptType)
  Core-->>WalletImpl: PathDescription (centralized)
  WalletImpl->>Network: getNetwork(coin, BTCScriptType?) 
  Network-->>WalletImpl: bip32 mapped by BTCScriptType
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Areas requiring extra attention:

  • packages/hdwallet-core/src/networks.ts — BIP32 shape and getNetwork lookup changes; verify mappings for all coins and error messages.
  • packages/hdwallet-core/src/bitcoin.ts — many public type changes and Exclude/union updates; confirm type safety and account constructors.
  • wallet adapters delegating describeUTXOPath (ledger/trezor/keepkey/portis/native) — ensure behavior parity after centralizing to core.describeUTXOPath.
  • packages/hdwallet-gridplus/src/constants.ts removal — confirm no remaining references and PSBT/network assumptions.

Possibly related PRs

  • feat: gridplus cleanup #741 — core enum/type refactor and BTCScriptType introduction; directly related to the enum consolidation applied here.
  • feat: hdwallet-gridplus #731 — GridPlus adapter changes overlapping bitcoin utilities and signature updates referenced by this diff.
  • feat: hdwallet-trezor #736 — Trezor/translateScriptType and related script-type translation changes overlap with this PR’s updates.

Suggested reviewers

  • gomesalexandre
  • NeOMakinG

Poem

🐇
I hopped through enums, tidy and spry,
One BTCScriptType beneath the sky,
Legacy, Segwit, Native—four in a row,
Tests and wallets now learn how to flow,
A rabbit's small cheer for a neater code show.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% 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 'chore: unify and cleanup script types' directly reflects the main change: replacing BTCInputScriptType and BTCOutputScriptType enums with a unified BTCScriptType enum throughout the codebase.

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.

Base automatically changed from feat_gridplus_suggestions to master November 25, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants