feat: sui Ledger blind signing detection#773
Conversation
📝 WalkthroughWalkthroughAdds a runtime guard in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (28)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (8)📚 Learning: 2025-08-07T15:27:03.179ZApplied to files:
📚 Learning: 2025-08-07T15:23:54.858ZApplied to files:
📚 Learning: 2025-08-07T15:24:19.530ZApplied to files:
📚 Learning: 2025-12-09T13:48:55.139ZApplied to files:
📚 Learning: 2025-08-07T15:47:29.207ZApplied to files:
📚 Learning: 2025-08-07T15:47:26.835ZApplied to files:
📚 Learning: 2025-12-12T11:20:00.907ZApplied to files:
📚 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 (24)
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 |
gomesalexandre
force-pushed
the
feat_sui_blind_sign
branch
from
0bf1908 to
d3ca814
Compare
![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
🧹 Nitpick comments (2)
packages/hdwallet-ledger/src/sui.ts (2)
71-73: Consider creating a custom error class.Using
anyto set the error name works but isn't type-safe. A custom error class would be cleaner and more maintainable.🔎 Proposed custom error class
Add this class at the top of the file after the imports:
class SuiBlindSigningRequiredError extends Error { constructor(message: string = "Sui transaction requires blind signing") { super(message); this.name = "SuiBlindSigningRequired"; Object.setPrototypeOf(this, SuiBlindSigningRequiredError.prototype); } }Then update the error handling:
- const blindSignError = new Error("Sui transaction requires blind signing"); - (blindSignError as any).name = "SuiBlindSigningRequired"; - throw blindSignError; + throw new SuiBlindSigningRequiredError();
71-71: Consider making the error message more actionable.The error message could guide users on how to resolve the issue by mentioning they need to enable blind signing in the Sui Ledger app settings.
🔎 Proposed more descriptive message
- const blindSignError = new Error("Sui transaction requires blind signing"); + const blindSignError = new Error("Sui transaction requires blind signing. Please enable blind signing in your Ledger Sui app settings.");
📜 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 (1)
packages/hdwallet-ledger/src/sui.ts
🧰 Additional context used
🧠 Learnings (4)
📚 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/sui.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/sui.ts
📚 Learning: 2025-08-07T15:47:26.835Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger-webusb/src/transport.ts:12-12
Timestamp: 2025-08-07T15:47:26.835Z
Learning: In the shapeshiftoss/hdwallet monorepo, ts-ignore is used instead of ts-expect-error for Ledger transport imports because the CI environment has different type checking behavior than local development. The code works locally without errors, but CI reports type issues, so ts-ignore is necessary to suppress the inconsistent type checking across environments.
Applied to files:
packages/hdwallet-ledger/src/sui.ts
📚 Learning: 2025-12-12T11:19:53.263Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 769
File: packages/hdwallet-walletconnectV2/src/walletconnectV2.ts:150-152
Timestamp: 2025-12-12T11:19:53.263Z
Learning: In the shapeshift/hdwallet monorepo, remove reliance on ethSupportsNetwork() across wallet implementations. This legacy method is no longer used to determine chain support. Instead, rely on the wallet class flags like _supportsMonad, _supportsPlasma, _supportsHyperEvm. Review all wallet implementations for ethSupportsNetwork() usage and migrate checks to the corresponding _supports* flags, updating tests and any affected logic accordingly.
Applied to files:
packages/hdwallet-ledger/src/sui.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-ledger/src/sui.ts (1)
70-70: Verify the Sui Ledger app status code for blind signing errors.The code checks for status code
0x8to detect blind signing requirements, but this status code is not documented in publicly available Ledger documentation. Since Ledger status codes vary by app and version, confirm with the Sui Ledger app documentation or maintainers (obsidiansystems/ledger-app-sui) that0x8is the correct code for this specific condition. If the code was tested against a particular Sui Ledger app version, document that version requirement.
gomesalexandre
changed the title
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 (1)
packages/hdwallet-ledger/src/sui.ts
🧰 Additional context used
🧠 Learnings (6)
📚 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/sui.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/sui.ts
📚 Learning: 2025-08-07T15:47:26.835Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: packages/hdwallet-ledger-webusb/src/transport.ts:12-12
Timestamp: 2025-08-07T15:47:26.835Z
Learning: In the shapeshiftoss/hdwallet monorepo, ts-ignore is used instead of ts-expect-error for Ledger transport imports because the CI environment has different type checking behavior than local development. The code works locally without errors, but CI reports type issues, so ts-ignore is necessary to suppress the inconsistent type checking across environments.
Applied to files:
packages/hdwallet-ledger/src/sui.ts
📚 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/sui.ts
📚 Learning: 2025-12-12T11:19:53.179Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 769
File: packages/hdwallet-ledger/src/ledger.ts:403-405
Timestamp: 2025-12-12T11:19:53.179Z
Learning: In packages/hdwallet-ledger/src/ethereum.ts, the ethSupportsNetwork function is a legacy/unused function that only returns true for chainId === 1. The Ledger ETH module does not call ethSupportsNetwork to validate chain support during signing operations - it accepts any chainId passed in the ETHSignTx message directly, so chain support flags can be enabled without needing to update ethSupportsNetwork.
Applied to files:
packages/hdwallet-ledger/src/sui.ts
📚 Learning: 2025-12-12T11:19:53.263Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 769
File: packages/hdwallet-walletconnectV2/src/walletconnectV2.ts:150-152
Timestamp: 2025-12-12T11:19:53.263Z
Learning: In the shapeshift/hdwallet monorepo, remove reliance on ethSupportsNetwork() across wallet implementations. This legacy method is no longer used to determine chain support. Instead, rely on the wallet class flags like _supportsMonad, _supportsPlasma, _supportsHyperEvm. Review all wallet implementations for ethSupportsNetwork() usage and migrate checks to the corresponding _supports* flags, updating tests and any affected logic accordingly.
Applied to files:
packages/hdwallet-ledger/src/sui.ts
🪛 Biome (2.1.2)
packages/hdwallet-ledger/src/sui.ts
[error] 69-70: Expected a statement but instead found '<<<<<<< HEAD'.
Expected a statement here.
(parse)
[error] 71-72: Expected a statement but instead found '======='.
Expected a statement here.
(parse)
[error] 73-74: Expected a statement but instead found '>>>>>>> 2d0817b8 (feat: detect Sui Ledger blind signing requirement)'.
Expected a statement here.
(parse)
[error] 74-74: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
⏰ 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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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-metamask-multichain/package.json (1)
1-31: Critical: Verify merge conflict resolution and complete changeset.The AI summary explicitly mentions "unresolved merge conflict markers around this insertion," but no conflict markers are visible in this file. Additionally, the PR objectives describe implementing Sui Ledger blind signing detection (with code changes in
suiSignTx), yet only a MetaMask package version bump is provided for review.This suggests either:
- Merge conflicts exist in other files not included in this review
- The actual Sui blind signing implementation files are missing from the review set
- The summary references an outdated commit state
Please confirm:
- All merge conflicts have been resolved across the entire PR
- The complete changeset includes the actual Sui blind signing detection logic (not just version bumps)
📜 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 (28)
examples/sandbox/package.jsonintegration/package.jsonlerna.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-core/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey-nodewebusb/package.jsonpackages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-ledger/src/sui.tspackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-native-vault/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-portis/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-trezor/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-walletconnectV2/package.json
✅ Files skipped from review due to trivial changes (3)
- packages/hdwallet-portis/package.json
- packages/hdwallet-keepkey-nodewebusb/package.json
- packages/hdwallet-core/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/hdwallet-ledger/src/sui.ts
🧰 Additional context used
🧠 Learnings (8)
📚 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-keepkey-tcp/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-trezor/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-keepkey-tcp/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-trezor/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-keepkey-tcp/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-trezor/package.json
📚 Learning: 2025-12-09T13:48:55.139Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 764
File: packages/hdwallet-gridplus/package.json:0-0
Timestamp: 2025-12-09T13:48:55.139Z
Learning: In the shapeshiftoss/hdwallet monorepo, the dist/ directory is not version controlled (not tracked in git). Build artifacts are generated during the build/publish workflow, not committed to the repository.
Applied to files:
packages/hdwallet-keepkey-tcp/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-trezor/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-keepkey-tcp/package.jsonpackages/hdwallet-walletconnect/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-phantom/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-trezor/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-keepkey-tcp/package.jsonpackages/hdwallet-ledger-webhid/package.jsonpackages/hdwallet-native/package.jsonpackages/hdwallet-keepkey-electron/package.jsonpackages/hdwallet-trezor-connect/package.jsonpackages/hdwallet-coinbase/package.jsonpackages/hdwallet-ledger/package.jsonpackages/hdwallet-gridplus/package.jsonpackages/hdwallet-keepkey-webusb/package.jsonexamples/sandbox/package.jsonpackages/hdwallet-metamask-multichain/package.jsonpackages/hdwallet-keplr/package.jsonpackages/hdwallet-ledger-webusb/package.jsonpackages/hdwallet-native-vault/package.jsonintegration/package.jsonpackages/hdwallet-vultisig/package.jsonpackages/hdwallet-keepkey-chromeusb/package.jsonpackages/hdwallet-walletconnectV2/package.jsonpackages/hdwallet-keepkey-nodehid/package.jsonpackages/hdwallet-keepkey/package.jsonpackages/hdwallet-trezor/package.json
📚 Learning: 2025-12-12T11:20:00.907Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 769
File: packages/hdwallet-walletconnectV2/src/walletconnectV2.ts:150-152
Timestamp: 2025-12-12T11:20:00.907Z
Learning: In the shapeshift/hdwallet monorepo, the `ethSupportsNetwork()` method in wallet implementations is a legacy/relic method that is no longer used. Chain support is determined by the `_supports*` flags (e.g., `_supportsMonad`, `_supportsPlasma`, `_supportsHyperEvm`) on the wallet classes, not by the `ethSupportsNetwork()` method.
Applied to files:
packages/hdwallet-coinbase/package.jsonpackages/hdwallet-trezor/package.json
📚 Learning: 2025-08-07T15:24:34.076Z
Learnt from: gomesalexandre
Repo: shapeshift/hdwallet PR: 726
File: examples/sandbox/package.json:15-31
Timestamp: 2025-08-07T15:24:34.076Z
Learning: Lerna v6+ supports workspace protocol syntax (workspace:*) but does not automatically convert exact versions to workspace protocol during version bumps. It only preserves existing workspace protocol syntax. Teams using Lerna for automated version bumps would need manual conversion to use workspace protocol, which negates automation benefits.
Applied to files:
lerna.json
⏰ 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 (24)
packages/hdwallet-keepkey-webusb/package.json (1)
3-3: Version bumps appear correct, but core PR functionality not visible in this file.The version bumps from 1.62.31 to 1.62.32 are consistent across the package and its dependencies. Based on learnings, this is expected workflow for the monorepo.
However, the PR objectives describe implementing Sui Ledger blind signing detection, but this file only contains routine version bumps for a KeepKey package. The core functionality mentioned in the PR description (detecting blind signing requirements in
suiSignTxand surfacing descriptive errors) is not present in the provided file. Additionally, the AI summary mentions "unresolved merge conflict markers" which are not visible in this file.Please ensure:
- The actual Sui Ledger blind signing implementation is included in the PR
- Any merge conflict markers mentioned in the AI summary are resolved
Also applies to: 17-18
packages/hdwallet-vultisig/package.json (1)
3-3: Version bumps look correct.The package version and dependency version are properly aligned at 1.62.32. Based on learnings, coordinated version bumps across the monorepo before publishing are standard practice in this repository.
However, the AI summary notes that "the diff also contains unresolved merge conflict markers around this insertion." While this specific file appears clean, please verify that no merge conflict markers (e.g.,
<<<<<<<,=======,>>>>>>>) remain in other files in this PR, particularly in the actual Sui blind signing detection code.Also applies to: 20-20
packages/hdwallet-metamask-multichain/package.json (1)
3-3: Version bumps follow established workflow.The package and dependency version bumps are routine and follow the monorepo's release workflow. Based on learnings, version bumps are performed in PRs before publishing, and dependencies may reference unpublished versions during the PR review phase—this is expected behavior.
Also applies to: 20-20
packages/hdwallet-phantom/package.json (1)
3-3: Version bump is routine; note package scope doesn't align with PR purpose.The version bump to 1.62.32 for both the package and
@shapeshiftoss/hdwallet-coredependency follows the expected monorepo release workflow. However, this package (hdwallet-phantom) is unrelated to the Sui Ledger blind signing feature described in the PR—the implementation should be inhdwallet-ledger. This appears to be a coordinated release across multiple packages.packages/hdwallet-walletconnect/package.json (1)
3-3: LGTM! Version alignment is consistent.The version bump to 1.62.32 and the corresponding dependency update to
@shapeshiftoss/hdwallet-core1.62.32 align with the monorepo-wide release. Based on learnings, version bumps before publishing is expected workflow in this repository.Also applies to: 18-18
packages/hdwallet-gridplus/package.json (1)
3-3: LGTM! Version alignment is consistent.The version bump to 1.62.32 and the corresponding dependency update to
@shapeshiftoss/hdwallet-core1.62.32 are properly synchronized with the monorepo release.Also applies to: 23-23
packages/hdwallet-walletconnectV2/package.json (1)
3-3: LGTM! Version alignment is consistent.The version bump to 1.62.32 and dependency update are properly aligned with the monorepo release.
Also applies to: 18-18
packages/hdwallet-keepkey/package.json (1)
3-3: LGTM! Version alignment is consistent.The version bump to 1.62.32 and dependency update to
@shapeshiftoss/hdwallet-core1.62.32 are properly synchronized.Also applies to: 24-24
packages/hdwallet-keepkey-tcp/package.json (1)
3-3: LGTM! Version alignment is consistent.The version bump to 1.62.32 and updates to both
@shapeshiftoss/hdwallet-coreand@shapeshiftoss/hdwallet-keepkeydependencies are properly aligned.Also applies to: 17-18
packages/hdwallet-keepkey-electron/package.json (1)
3-3: LGTM! Version alignment is consistent.The version bump to 1.62.32 and dependency update to
@shapeshiftoss/hdwallet-keepkey1.62.32 are properly synchronized.Also applies to: 17-17
lerna.json (1)
3-3: LGTM! Monorepo version bump is consistent.The lerna version bump to 1.62.32 properly drives the coordinated version updates across all packages in the monorepo.
examples/sandbox/package.json (1)
3-3: LGTM! Comprehensive version alignment across all dependencies.The version bump to 1.62.32 and all 18
@shapeshiftoss/hdwallet-*dependency updates are properly synchronized with the monorepo release. This ensures the sandbox example environment remains consistent with the updated package versions.Also applies to: 15-32
packages/hdwallet-keplr/package.json (1)
3-3: LGTM! Standard version bump.The package version and hdwallet-core dependency are correctly aligned at 1.62.32. Based on learnings, version bumps in PRs before publishing are expected in this monorepo's release workflow.
Also applies to: 20-20
packages/hdwallet-trezor/package.json (1)
3-3: LGTM! Standard version bump.The package version and hdwallet-core dependency are correctly aligned at 1.62.32.
Also applies to: 20-20
packages/hdwallet-trezor-connect/package.json (1)
3-3: LGTM! Standard version bump.The package version and both hdwallet dependencies (core and trezor) are correctly aligned at 1.62.32.
Also applies to: 17-18
packages/hdwallet-coinbase/package.json (1)
3-3: LGTM! Standard version bump.The package version and hdwallet-core dependency are correctly aligned at 1.62.32.
Also applies to: 18-18
packages/hdwallet-native/package.json (1)
3-3: LGTM! Standard version bump.The package version and hdwallet-core dependency are correctly aligned at 1.62.32.
Also applies to: 20-20
packages/hdwallet-keepkey-nodehid/package.json (1)
3-3: LGTM! Standard version bump.The package version and hdwallet-keepkey dependency are correctly aligned at 1.62.32.
Also applies to: 17-17
packages/hdwallet-ledger/package.json (1)
3-3: LGTM! Standard version bump.The package version and hdwallet-core dependency are correctly aligned at 1.62.32. This package contains the Sui blind signing implementation mentioned in the PR objectives, though the actual implementation file (sui.ts) was not included in this review.
Also applies to: 31-31
packages/hdwallet-ledger-webhid/package.json (1)
3-3: LGTM! Standard version bump.The package version and both hdwallet dependencies (core and ledger) are correctly aligned at 1.62.32.
Also applies to: 21-22
packages/hdwallet-keepkey-chromeusb/package.json (1)
3-3: LGTM! Version bumps are consistent.The version bumps from 1.62.31 to 1.62.32 are consistent across the package and its dependencies, aligning with the monorepo release workflow.
Also applies to: 17-18
packages/hdwallet-ledger-webusb/package.json (1)
3-3: LGTM! Version bumps are consistent.The version bumps from 1.62.31 to 1.62.32 are consistent across the package and its dependencies, including @shapeshiftoss/hdwallet-ledger which contains the Sui blind signing implementation.
Also applies to: 23-24
integration/package.json (1)
3-3: LGTM! Version bumps are consistent.The version bumps from 1.62.31 to 1.62.32 are consistent across the integration package and all 10 hdwallet dependencies, properly aligning the test suite with the new release.
Also applies to: 15-24
packages/hdwallet-native-vault/package.json (1)
3-3: The review comment is based on incorrect assumptions about the repository architecture. The Sui blind signing detection implementation is present inpackages/hdwallet-ledger/src/sui.ts(lines 69-72), where it properly checks for error status code 0x8 and throws "Sui transaction requires blind signing." This is the correct location for Ledger-specific implementations.The
packages/hdwallet-native-vaultpackage is a vault/storage abstraction layer, not where Sui signing logic is implemented. The version bump in its package.json is routine and correct, consistent with the monorepo's release workflow.No merge conflict markers exist in the codebase, despite the commit message reference.
Likely an incorrect or invalid review comment.
|
Vercel CI failure unrelated to this PR, will tackle in a follow-up to make green. |
2 participants
Description
Does what it says on thy box
hdwallet fren of shapeshift/web#11508
Issue
#773
Testing
Screenshots
https://jam.dev/c/2bafcadd-be8d-4917-9c7d-936a5c97a257
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.