feat: use @shapeshiftoss/bitcoinjs-lib instead of @bitgo/utxo-lib for Ledger Zcash PSBTs #11374
Conversation
Remove ?url suffix from SVG imports to fix module script MIME type error
Update hdwallet packages to 1.62.29-bitgo-removal.0 which removes the @bitgo/utxo-lib dependency and uses @shapeshiftoss/bitcoinjs-lib for Zcash Ledger signing instead. This unifies the Native and Ledger Zcash implementations to both use the ShapeShift fork of bitcoinjs-lib which has native ZIP-244 support.
Fixes 'Outputs are spending more than Inputs' error by using psbt.data.getTransaction() for unsigned transaction instead of extractTransaction()
Access internal PSBT transaction to preserve Zcash consensus fields
- Clean PSBT API usage (no __CACHE.__TX access) - Safe blockHeight handling for unconfirmed Zcash inputs
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR updates Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
NeOMakinG
left a comment
NeOMakinG
left a comment
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!
…ranch ID Updates hdwallet packages to 1.62.31
gomesalexandre
force-pushed
the
feat_bitgo_bitcoinjs_lib
branch
from
7446048 to
e71d175
Compare
Mempool transactions return blockHeight: -1 from unchained-client. When passed to Ledger, getZcashBranchId(-1) falls through to OVERWINTER consensus branch ID (0x5ba81b19) instead of NU6.1 (0x4dec4df0), causing second Zcash transactions to fail with "bad-tx-consensus-branch-id-mismatch". Only pass blockHeight when it's a valid positive number. When undefined, hdwallet-ledger uses fallback 3150000 (above NU6.1 activation). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Documents Step 3.8 for adding asset support detection, which was missing for recent chains (Tron, SUI, etc.) and caused assets not to show up. Also adds note about RelaySwapper mapping completeness. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization 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 (4)
package.jsonpackages/chain-adapters/package.jsonpackages/chain-adapters/src/utxo/UtxoBaseAdapter.tspackages/swapper/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/swapper/package.json
- package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always runyarn lint --fixandyarn type-checkafter making changes
Avoidletvariable assignments - preferconstwith inline IIFE switch statements or extract to functions for conditional logic
Files:
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern:userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables inuseMemoand callbacks inuseCallbackwhere possible
For static JSX icon elements (e.g.,<TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode usinguseColorModeValuehook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook:useTranslate()fromreact-polyglot
UseuseFeatureFlag('FlagName')hook to access feature flag values in components
Prefertypeoverinterfacefor type definitions
Use strict typing - avoidany
UseNominaltypes for domain identifiers (e.g.,WalletId,AccountId)
Import types from@shapeshiftoss/caipfor chain/account/asset IDs
UseuseAppSelectorfor Redux state
UseuseAppDispatchfor Redux actions
Memoize expensive computations withuseMemo
Memoize callbacks withuseCallback
**/*.{ts,tsx}: UseResult<T, E>pattern for error handling in swappers and APIs; ALWAYS useOk()andErr()from@sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from@shapeshiftoss/errorswith meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...
Files:
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Usehandleprefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names withis,has,can,shouldprefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names likedata,item,obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names likefn,func, orcallback
Files:
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10232
File: packages/unchained-client/openapitools.json:61-61
Timestamp: 2025-08-08T10:23:16.843Z
Learning: In shapeshift/web, for temporary “monkey patch” PRs (e.g., packages/unchained-client/openapitools.json using jsDelivr CDN refs like cosmos/mayachain), gomesalexandre is fine with branch-based URLs and does not want SHA pinning. Treat this as a scoped exception to their general preference for pinned dependencies/refs.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:27:02.203Z
Learning: In shapeshift/web, while gomesalexandre generally prefers pinned dependencies for security, semver with a caret range (^7.7.2) is acceptable as an exception since it's already a transient dependency of many packages and is being made explicit for hdwallet support.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11078
File: src/setupVitest.ts:11-15
Timestamp: 2025-11-20T12:00:45.005Z
Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Branch naming: Use descriptive names (e.g., `feat_gridplus`, `fix_wallet_connect`)
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10767
File: package.json:324-324
Timestamp: 2025-11-19T22:20:25.661Z
Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:16:12.898Z
Learning: In shapeshift/web, the semver package must be included in dependencies (not devDependencies) because hdwallet packages have transient dependencies that require semver but don't ship it themselves. This ensures semver is available at runtime for hdwallet functionality.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-11-19T22:20:25.661Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10767
File: package.json:324-324
Timestamp: 2025-11-19T22:20:25.661Z
Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-08-08T20:16:12.898Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:16:12.898Z
Learning: In shapeshift/web, the semver package must be included in dependencies (not devDependencies) because hdwallet packages have transient dependencies that require semver but don't ship it themselves. This ensures semver is available at runtime for hdwallet functionality.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-12-04T22:57:50.850Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 11290
File: packages/chain-adapters/src/utxo/zcash/ZcashChainAdapter.ts:48-51
Timestamp: 2025-12-04T22:57:50.850Z
Learning: In packages/chain-adapters/src/**/*ChainAdapter.ts files, the getName() method uses the pattern `const enumIndex = Object.values(ChainAdapterDisplayName).indexOf(ChainAdapterDisplayName.XXX); return Object.keys(ChainAdapterDisplayName)[enumIndex]` to reverse-lookup the enum key from its value. This is the established pattern used consistently across almost all chain adapters (Bitcoin, Ethereum, Litecoin, Dogecoin, Polygon, Arbitrum, Cosmos, etc.) and should be preserved for consistency when adding new chain adapters.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-08-08T20:27:02.203Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:27:02.203Z
Learning: In shapeshift/web, while gomesalexandre generally prefers pinned dependencies for security, semver with a caret range (^7.7.2) is acceptable as an exception since it's already a transient dependency of many packages and is being made explicit for hdwallet support.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-12-09T21:07:22.474Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11335
File: packages/swapper/src/swappers/CetusSwapper/utils/helpers.ts:3-3
Timestamp: 2025-12-09T21:07:22.474Z
Learning: In packages/swapper/src/swappers/CetusSwapper, mysten/sui types (SuiClient, Transaction) must be imported from the nested path within cetusprotocol/aggregator-sdk (e.g., 'cetusprotocol/aggregator-sdk/node_modules/mysten/sui/client') because the aggregator SDK bundles its own version of mysten/sui. Direct imports from 'mysten/sui' break at runtime even when specified in package.json.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-08-29T18:09:45.982Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 10376
File: vite.config.mts:136-137
Timestamp: 2025-08-29T18:09:45.982Z
Learning: In the ShapeShift web repository vite.config.mts, the commonjsOptions.exclude configuration using bare package name strings like ['shapeshiftoss/caip', 'shapeshiftoss/types'] works correctly for excluding specific packages from CommonJS transformation, despite theoretical concerns about module ID matching patterns.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-11-20T12:00:45.005Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11078
File: src/setupVitest.ts:11-15
Timestamp: 2025-11-20T12:00:45.005Z
Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-08-07T11:20:05.201Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: package.json:0-0
Timestamp: 2025-08-07T11:20:05.201Z
Learning: gomesalexandre prefers pinned dependencies (exact versions without caret ranges) as a general security practice to prevent vulnerabilities from being introduced through automatic version bumps. He referenced the LedgerHQ/connect-kit vulnerability (GitHub issue #29) as an example of security risks from dependency updates.
Applied to files:
packages/chain-adapters/package.json
📚 Learning: 2025-12-17T14:50:01.629Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11449
File: packages/chain-adapters/src/tron/TronChainAdapter.ts:570-596
Timestamp: 2025-12-17T14:50:01.629Z
Learning: In packages/chain-adapters/src/tron/TronChainAdapter.ts, the parseTx method uses `unknown` type for the txHashOrTx parameter intentionally. TRON is a "second-class chain" that works differently from other chains - it accepts either a string hash (to fetch TronTx via unchained client) or a TronTx object directly. The base chain-adapter interface is strongly typed and doesn't accommodate this flexible signature, so `unknown` is used as an appropriate escape hatch rather than a type safety issue.
Applied to files:
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts
🔇 Additional comments (2)
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts (1)
355-357: LGTM!The multiline formatting with trailing comma improves readability and diff clarity.
packages/chain-adapters/package.json (1)
34-35: Version 1.62.35 does not exist in the npm registry for either package.Both
@shapeshiftoss/hdwallet-coreand@shapeshiftoss/hdwallet-ledgershow a maximum published version of 1.62.34. The dependency updates to 1.62.35 cannot be merged unless these versions are published to npm first.⛔ Skipped due to learnings
Learnt from: gomesalexandre Repo: shapeshift/web PR: 10767 File: package.json:324-324 Timestamp: 2025-11-19T22:20:25.661Z Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).Learnt from: gomesalexandre Repo: shapeshift/web PR: 10222 File: package.json:202-202 Timestamp: 2025-08-08T20:16:12.898Z Learning: In shapeshift/web, the semver package must be included in dependencies (not devDependencies) because hdwallet packages have transient dependencies that require semver but don't ship it themselves. This ensures semver is available at runtime for hdwallet functionality.Learnt from: gomesalexandre Repo: shapeshift/web PR: 10222 File: package.json:202-202 Timestamp: 2025-08-08T20:27:02.203Z Learning: In shapeshift/web, while gomesalexandre generally prefers pinned dependencies for security, semver with a caret range (^7.7.2) is acceptable as an exception since it's already a transient dependency of many packages and is being made explicit for hdwallet support.Learnt from: gomesalexandre Repo: shapeshift/web PR: 11170 File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19 Timestamp: 2025-11-25T21:43:10.838Z Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.Learnt from: gomesalexandre Repo: shapeshift/web PR: 10206 File: package.json:0-0 Timestamp: 2025-08-07T11:20:05.201Z Learning: gomesalexandre prefers pinned dependencies (exact versions without caret ranges) as a general security practice to prevent vulnerabilities from being introduced through automatic version bumps. He referenced the LedgerHQ/connect-kit vulnerability (GitHub issue #29) as an example of security risks from dependency updates.Learnt from: gomesalexandre Repo: shapeshift/web PR: 10232 File: packages/unchained-client/openapitools.json:61-61 Timestamp: 2025-08-08T10:23:16.843Z Learning: In shapeshift/web, for temporary “monkey patch” PRs (e.g., packages/unchained-client/openapitools.json using jsDelivr CDN refs like cosmos/mayachain), gomesalexandre is fine with branch-based URLs and does not want SHA pinning. Treat this as a scoped exception to their general preference for pinned dependencies/refs.Learnt from: gomesalexandre Repo: shapeshift/web PR: 11078 File: src/setupVitest.ts:11-15 Timestamp: 2025-11-20T12:00:45.005Z Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.Learnt from: NeOMakinG Repo: shapeshift/web PR: 10231 File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14 Timestamp: 2025-08-08T15:00:22.321Z Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.
|
Ledger/Native retest after bump: https://jam.dev/c/f2d7c786-7f9e-4127-97b0-72a08b890239 |
… Ledger Zcash PSBTs (#11374)
Description
Web fren of shapeshift/hdwallet#768
Does what it says on the box.
Issue (if applicable)
N/A
Risk
Zcash Ledger hardware wallet transactions. Depends on unpublished hdwallet version 1.62.29.
Testing
Engineering
Operations
Test Zcash sends with Ledger hardware wallet. Verify consecutive sends work (spending unconfirmed change).
Screenshots (if applicable)
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.