feat: make phantom great again#11416
Conversation
…atibility - Phantom requires transaction in JSON format for signing - Native wallet continues to use intentMessageBytes - Both wallets now work correctly with no regressions - Updated hdwallet packages to 1.62.29-phantom-sui-fix.2
Required property was missing after making transactionJson non-optional in hdwallet-core SuiSignTx interface 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Transaction.toJSON() needs the sui client to resolve coin balances and references. Without it, the method fails with "No sui client passed" error when trying to build the transaction offline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@gomesalexandre has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR updates hdwallet dependencies to 1.62.29 across multiple package.json files, configures a local npm registry in .yarnrc.yml, adds transactionJson serialization to SUI transaction objects in chain adapters and swappers, and implements Phantom wallet-specific chain support guards to disable incompatible chains. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
gomesalexandre
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts (1)
52-233: Phantom-specific chain gating matches stated support matrix, minor consistency nits only
- Importing
isPhantom/PhantomHDWalletand:
- Disabling BCH/DOGE/LTC/ZEC for Phantom,
- Disabling Avalanche, Optimism, BSC, Gnosis, Arbitrum, Arbitrum Nova for Phantom, and
- Allowing Polygon, Base, Monad, and HyperEVM (subject to feature flags),
aligns with the described Phantom support and avoids hitting unsupported or rate‑limited chains.Behaviorally this looks correct and should prevent “chain switch not supported” surfaces on unsupported EVM chains while enabling the intended networks.
Two small follow‑ups you might consider (non‑blocking):
- For BCH/DOGE/LTC/ZEC, the early
if (wallet instanceof PhantomHDWallet) return falsemakes the subsequent!(wallet instanceof PhantomHDWallet)checks redundant; you can safely drop them for clarity.- For consistency and to future‑proof against different Phantom implementations, you could standardize on the
isPhantom(wallet)type guard instead of mixing it withwallet instanceof PhantomHDWallet.Also, please double‑check in manual testing that:
- Phantom correctly shows support for Polygon, Base, Monad, HyperEVM, and
- Phantom no longer attempts to derive/send on the newly disabled chains (BCH/DOGE/LTC/ZEC, Avalanche, Optimism, BSC, Gnosis, Arbitrum, Arbitrum Nova).
📜 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 (7)
.yarnrc.yml(1 hunks)package.json(1 hunks)packages/chain-adapters/package.json(1 hunks)packages/chain-adapters/src/sui/SuiChainAdapter.ts(2 hunks)packages/swapper/package.json(1 hunks)packages/swapper/src/swappers/CetusSwapper/endpoints.ts(2 hunks)src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/swapper/src/swappers/CetusSwapper/endpoints.tspackages/chain-adapters/src/sui/SuiChainAdapter.tssrc/hooks/useWalletSupportsChain/useWalletSupportsChain.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/swapper/src/swappers/CetusSwapper/endpoints.tspackages/chain-adapters/src/sui/SuiChainAdapter.tssrc/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
**/swapper{s,}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
ALWAYS use
makeSwapErrorRightfor swapper errors withTradeQuoteErrorenum for error codes and provide detailed error information
Files:
packages/swapper/src/swappers/CetusSwapper/endpoints.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/swapper/src/swappers/CetusSwapper/endpoints.tspackages/chain-adapters/src/sui/SuiChainAdapter.tssrc/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
packages/swapper/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/swapper.mdc)
packages/swapper/**/*.ts: Use TypeScript with explicit types (e.g., SupportedChainIds) for all code in the Swapper system
Use camelCase for variable and function names in the Swapper system
Use PascalCase for types, interfaces, and enums in the Swapper system
Use kebab-case for filenames in the Swapper system
Files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
packages/swapper/src/swappers/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/swapper.mdc)
packages/swapper/src/swappers/**/*.ts: Adhere to the Swapper directory structure: each swapper resides in packages/swapper/src/swappers// with required files (SwapperName.ts, endpoints.ts, types.ts, utils/constants.ts, utils/helpers.ts)
Validate inputs and log errors for debugging in Swapper system implementations
Swapper files must be located in packages/swapper/src/swappers/ directory structure and not placed outside this location
Avoid side effects in swap logic; ensure swap methods are deterministic and stateless
Files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
packages/swapper/src/swappers/*/*.ts
📄 CodeRabbit inference engine (.cursor/rules/swapper.mdc)
packages/swapper/src/swappers/*/*.ts: All swappers must implement the Swapper interface from packages/swapper/src/types.ts
Implement filterAssetIdsBySellable method to filter assets by supported chain IDs in the sell property
Implement filterBuyAssetsBySellAssetId method to filter assets by supported chain IDs in the buy property
Reuse executeEvmTransaction utility for EVM-based swappers instead of implementing custom transaction execution
Files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
packages/swapper/src/swappers/*/endpoints.ts
📄 CodeRabbit inference engine (.cursor/rules/swapper.mdc)
packages/swapper/src/swappers/*/endpoints.ts: All swapper API implementations must implement the SwapperApi interface from packages/swapper/src/types.ts
Reuse checkEvmSwapStatus utility for checking EVM swap status instead of implementing custom status checks
Files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
🧠 Learnings (38)
📓 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: 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: 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: 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: 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-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.jsonsrc/hooks/useWalletSupportsChain/useWalletSupportsChain.tspackages/swapper/package.jsonpackage.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.jsonpackages/chain-adapters/src/sui/SuiChainAdapter.tspackage.json
📚 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.jsonpackages/swapper/package.jsonpackage.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.jsonpackages/swapper/package.jsonpackage.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.jsonpackages/swapper/package.jsonpackage.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.jsonpackages/swapper/package.jsonpackage.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.jsonsrc/hooks/useWalletSupportsChain/useWalletSupportsChain.tspackages/swapper/package.jsonpackage.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.jsonsrc/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 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-09T21:06:09.903Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11335
File: packages/swapper/src/swappers/CetusSwapper/endpoints.ts:66-68
Timestamp: 2025-12-09T21:06:09.903Z
Learning: In CetusSwapper endpoints.ts, prefer throwing errors directly in transaction-preparation/helper methods (e.g., getUnsignedSuiTransaction) instead of using the Result pattern. Reserve the Result pattern (makeSwapErrorRight/TradeQuoteError) for the main swapper API methods (getTradeQuote, getTradeRate). This guidance applies specifically to this file and similar transaction-preparation code within it.
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
📚 Learning: 2025-12-09T21:07:14.896Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11335
File: packages/swapper/src/swappers/CetusSwapper/utils/helpers.ts:3-3
Timestamp: 2025-12-09T21:07:14.896Z
Learning: When editing CetusSwapper, import Sui types (SuiClient, Transaction) from the aggregator SDK's nested path, e.g., from cetusprotocol/aggregator-sdk/node_modules/mysten/sui/client, rather than importing directly from mysten/sui. Direct imports from mysten/sui can break at runtime because the aggregator SDK bundles its own version. Update imports in CetusSwapper/utils/helpers.ts and review other files under packages/swapper/src/swappers/CetusSwapper for the same pattern.
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
📚 Learning: 2025-12-01T22:01:37.982Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11240
File: packages/swapper/src/swappers/CetusSwapper/endpoints.ts:56-58
Timestamp: 2025-12-01T22:01:37.982Z
Learning: In packages/swapper/src/swappers/CetusSwapper/endpoints.ts, gomesalexandre is comfortable with mutating the shared Cetus SDK singleton instance (sdk.senderAddress = from) when required by the SDK API, preferring pragmatic working code over theoretical statelessness concerns.
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Reuse executeEvmTransaction utility for EVM-based swappers instead of implementing custom transaction execution
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.tspackages/swapper/package.json
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/endpoints.ts : All swapper API implementations must implement the SwapperApi interface from packages/swapper/src/types.ts
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/**/*.ts : Avoid side effects in swap logic; ensure swap methods are deterministic and stateless
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.tspackages/swapper/package.json
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/**/*.ts : Use TypeScript with explicit types (e.g., SupportedChainIds) for all code in the Swapper system
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.tspackages/chain-adapters/src/sui/SuiChainAdapter.tssrc/hooks/useWalletSupportsChain/useWalletSupportsChain.tspackages/swapper/package.json
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/**/*.ts : Use PascalCase for types, interfaces, and enums in the Swapper system
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.tspackages/swapper/package.json
📚 Learning: 2025-12-04T11:05:01.146Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11281
File: packages/swapper/src/swappers/PortalsSwapper/utils/fetchSquidStatus.ts:98-106
Timestamp: 2025-12-04T11:05:01.146Z
Learning: In packages/swapper/src/swappers/PortalsSwapper/utils/fetchSquidStatus.ts, getSquidTrackingLink should return blockchain explorer links (using Asset.explorerTxLink) rather than API endpoints. For non-GMP Squid swaps: return source chain explorer link with sourceTxHash when pending/failed, and destination chain explorer link with destinationTxHash when confirmed.
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/endpoints.ts : Reuse checkEvmSwapStatus utility for checking EVM swap status instead of implementing custom status checks
Applied to files:
packages/swapper/src/swappers/CetusSwapper/endpoints.tspackages/swapper/package.json
📚 Learning: 2025-09-12T13:44:17.019Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:0-0
Timestamp: 2025-09-12T13:44:17.019Z
Learning: gomesalexandre prefers letting chain adapter errors throw naturally in useSimulateEvmTransaction rather than adding explicit error handling for missing adapters, consistent with his fail-fast approach and dismissal of defensive validation as "stale" in WalletConnect transaction simulation flows.
Applied to files:
packages/chain-adapters/src/sui/SuiChainAdapter.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/utils/constants.ts : Define supported chain IDs for each swapper in utils/constants.ts with both 'sell' and 'buy' properties following the pattern: SupportedChainIds type
Applied to files:
packages/chain-adapters/src/sui/SuiChainAdapter.tssrc/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Implement filterBuyAssetsBySellAssetId method to filter assets by supported chain IDs in the buy property
Applied to files:
packages/chain-adapters/src/sui/SuiChainAdapter.ts
📚 Learning: 2025-09-18T23:47:14.810Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts:55-66
Timestamp: 2025-09-18T23:47:14.810Z
Learning: In the useWalletSupportsChain architecture, checkWalletHasRuntimeSupport() determines if the app has runtime capability to interact with a chain type (not actual signing capabilities), while walletSupportsChain() does the actual capabilities detection by checking account IDs. For Ledger read-only mode, checkWalletHasRuntimeSupport should return true since the app can display balances/addresses, with KeyManager being the source of truth rather than wallet instance.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-10-23T14:27:19.073Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10857
File: src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts:101-104
Timestamp: 2025-10-23T14:27:19.073Z
Learning: In WalletConnect wallet_switchEthereumChain and wallet_addEthereumChain requests, the chainId parameter is always present as per the protocol spec. Type guards checking for missing chainId in these handlers (like `if (!evmNetworkIdHex) return`) are solely for TypeScript compiler satisfaction, not real runtime edge cases.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Import types from `shapeshiftoss/caip` for chain/account/asset IDs
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.tspackages/swapper/package.json
📚 Learning: 2025-10-01T07:42:40.195Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10596
File: src/components/Layout/Header/NavBar/WalletConnectedMenu.tsx:77-99
Timestamp: 2025-10-01T07:42:40.195Z
Learning: In WalletConnectedMenu.tsx's handleReconnectWallet handler, gomesalexandre prefers throwing an error for unsupported wallet types in the default case rather than gracefully handling with a fallback. His reasoning: "if we have a problem here, we have bigger problems" - only supported wallets (KeepKey, Ledger, MetaMask, Coinbase, Phantom) should reach the reconnect flow when disconnected/locked, so encountering an unsupported type indicates a larger architectural issue that should be surfaced explicitly rather than masked with graceful degradation.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 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:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.tspackage.json
📚 Learning: 2025-10-15T15:57:39.956Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10810
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:212-0
Timestamp: 2025-10-15T15:57:39.956Z
Learning: gomesalexandre uses discriminated union patterns (e.g., `isEIP1559 ? { max_fee_per_gas, max_priority_fee_per_gas } : { gas_price }`) in WalletConnect flows without additional validation guards, trusting that the runtime data structure ensures mutual exclusivity between EIP-1559 and legacy gas pricing fields.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-08-11T09:45:51.174Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:175-180
Timestamp: 2025-08-11T09:45:51.174Z
Learning: gomesalexandre prefers truthy checks over explicit boolean comparisons (e.g., `walletSupportsSellAssetChain` instead of `walletSupportsSellAssetChain === true`) when dealing with tri-state values (boolean | null) in TypeScript, as the falsy behavior for null/undefined is intentional and acceptable.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-08-13T15:52:25.116Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10272
File: src/context/WalletProvider/MobileWallet/mobileMessageHandlers.ts:61-0
Timestamp: 2025-08-13T15:52:25.116Z
Learning: In the ShapeShift web codebase, specifically in src/context/WalletProvider/MobileWallet/mobileMessageHandlers.ts, message variants in the Message union type do not include inline comments documenting their expected return types. The codebase follows a pattern of keeping these type definitions clean without such documentation comments.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-11-05T23:37:30.632Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10912
File: src/context/WalletProvider/NewWalletViews/NewWalletViewsSwitch.tsx:290-290
Timestamp: 2025-11-05T23:37:30.632Z
Learning: In the ShapeShift web codebase, `isMobile` imported from '@/lib/globals' is a module-level constant (defined as `export const isMobile = Boolean(window?.isShapeShiftMobile)`) that is evaluated once at module load time. It is a stable reference that does not need to be included in useCallback/useMemo/useEffect dependency arrays.
Applied to files:
src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/**/*.ts : Use camelCase for variable and function names in the Swapper system
Applied to files:
packages/swapper/package.json
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : All swappers must implement the Swapper interface from packages/swapper/src/types.ts
Applied to files:
packages/swapper/package.json
📚 Learning: 2025-11-25T21:43:10.838Z
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.
Applied to files:
package.json
📚 Learning: 2025-08-27T13:49:48.668Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10375
File: src/state/migrations/index.ts:214-215
Timestamp: 2025-08-27T13:49:48.668Z
Learning: The ShapeShift web app uses an automated versioning system for Redux Persist migrations. The version is calculated as `Math.max(...Object.keys(clearAssetsMigrations).map(Number))`, which automatically uses the highest migration number as the version. This eliminates the need to manually update persistConfig versions when adding new migrations - the system automatically bumps the version when new migration numbers are added to the migration objects.
Applied to files:
package.json
📚 Learning: 2025-08-27T13:49:48.668Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10375
File: src/state/migrations/index.ts:214-215
Timestamp: 2025-08-27T13:49:48.668Z
Learning: The clearAssetsMigrations system in the ShapeShift web app works independently of the root persistConfig version. Migration numbers (like 171) don't need to match the root persistConfig version field, and the system has been functioning correctly for months/years without manual version bumps.
Applied to files:
package.json
📚 Learning: 2025-08-08T15:00:22.321Z
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.
Applied to files:
package.json
🔇 Additional comments (6)
packages/chain-adapters/package.json (1)
31-37: Aligning chain-adapters with hdwallet 1.62.29 looks correctBumping
@shapeshiftoss/hdwallet-coreand@shapeshiftoss/hdwallet-ledgerto1.62.29keeps this workspace in sync with the root and swapper packages and is needed to consume the new Phantom/Sui behavior.Please ensure CI is exercising Sui + Ledger flows against this hdwallet version (sign + broadcast) before merge.
packages/swapper/package.json (1)
32-46: Swapper hdwallet-core bump matches the rest of the workspaceUpdating
@shapeshiftoss/hdwallet-coreto1.62.29here keeps the swapper in lockstep with root and chain-adapters, which is important for the new Sui + Phantom capabilities.Please confirm
vitest-packagescovers at least one Cetus Sui swap path using Phantom/Native/Ledger with this hdwallet version before merging.package.json (1)
99-121: hdwallet dependency suite correctly unified on 1.62.29All
@shapeshiftoss/hdwallet-*dependencies are consistently bumped to1.62.29, which is required to pick up the Phantom derivation fixes and Sui support across Native, Ledger, and Phantom.Given the breadth of impact, please double‑check that at least a minimal matrix of flows has been exercised on this version (e.g., Phantom Sui send, Native Sui send, Ledger Sui send, plus one EVM and one BTC flow).
packages/swapper/src/swappers/CetusSwapper/endpoints.ts (1)
37-109: AddingtransactionJsonto Cetus unsigned Sui transactions is consistent and safe
getUnsignedSuiTransactionnow computestransactionJsonviatxb.toJSON({ client: suiClient })and returns it alongsideaddressNListandintentMessageBytes. The intent construction and BIP44 path derivation stay unchanged, so existing consumers remain compatible while Phantom (and other Sui wallets) can use the richer JSON form.Please verify:
- The return type for
getUnsignedSuiTransactioninGetUnsignedSuiTransactionArgs/SwapperApiincludestransactionJson.- A Phantom‑backed Cetus Sui swap successfully signs using this JSON (no mismatch between the aggregator SDK’s
Transactionand the newtoJSON({ client })usage).packages/chain-adapters/src/sui/SuiChainAdapter.ts (1)
238-293: SuiChainAdapter now correctly exposestransactionJsonfor Sui SignTx
buildSendApiTransactionnow callsawait tx.toJSON()and returnstransactionJsontogether withaddressNListandintentMessageBytes. This cleanly extends the Sui SignTx payload for wallets (like Phantom) that need a JSON representation, whilesignTransactionandsignAndBroadcastTransactionstill operate on the intent bytes as before.Please confirm:
- The
SignTx<KnownChainIds.SuiMainnet>type has been updated to includetransactionJson.- Phantom, Native, and Ledger Sui signing all still succeed when using this augmented SignTx shape.
.yarnrc.yml (1)
3-17: The code snippet in this review does not match the actual .yarnrc.yml file.The actual file has
npmRegistryServer: "https://registry.npmjs.org"(the safe default), nothttp://127.0.0.1:4873. TheunsafeHttpWhitelistentries for localhost and 127.0.0.1 are present but harmless since the registry isn't configured to use them.Verdaccio is documented for local hdwallet testing via the
hdwallet-verdaccio-local-publish-pipelineskill, not as a committed repo default. No changes to .yarnrc.yml are needed for this workflow.Likely an incorrect or invalid review comment.
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
My record failed because my computed ran out of battery but, everything works fine (did a bunch of swaps on multiple chains)
The only weird thing I noticed is my HYPE balance isn't showing up in the swapper:
while I can see it in the asset list:
I think we are missing something for the hype support probably on web side, that is going to be better after the last PR adding the notion of second class chain in a better way, happy to stamp it and test in the last PR!
2 participants
Description
Web fren of shapeshift/hdwallet#771
This PR:
Changes
Issue (if applicable)
Risk
This PR modifies hdwallet and affects multiple wallet interactions including Phantom, Native, and Ledger Sui transactions. It should be considered High Risk as it touches transaction signing logic.
Testing
Engineering
Test plan
Operations
Test Phantom wallet connection and verify:
Screenshots (if applicable)
develop
this diff
https://jam.dev/c/f5ffdde3-da4f-4848-9b8c-296ec7a0ecc2
https://jam.dev/c/3829ae7a-98b8-44be-a359-2344fa2fc6a6
https://jam.dev/c/e2136755-29b1-4842-9cff-b1f043458a9a
https://jam.dev/c/0d6d38d2-ec17-41e7-ae8e-6d726577e5be
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.