WalletConnect Solana adapter #17360
WalletConnect Solana adapter #17360
Conversation
|
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request |
|
🚀 Expo preview is ready!
|
939fb8c to
9331fff
Compare
WalkthroughThe overall changes span multiple modules, focusing on enhanced fee estimation, transaction processing, UI adjustments, and WalletConnect integration. In the fee estimation modules, an optional fee payer property is added to the ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
suite-common/walletconnect/src/adapters/solana.ts (4)
5-25: Centralizing methods is a good design choice.Defining the
methodsarray for Solana-specific functionality keeps logic tidy and consistent with other adapters.
27-84: Promote clearer error messaging and handle possible unsuccessful fee estimates gracefully.
- If
estimatedFee.successisfalse, the code throws a generic error. Including the actual error fromestimatedFee.payload.error(if present) can aid debugging.- The fallback for
feePayer(line 51) is helpful, but double-check if there's ever a scenario whereestimatedFee.payload.levelsis empty.- throw new Error('Failed to estimate fee'); + throw new Error(`Failed to estimate fee: ${estimatedFee.payload.error ?? 'unknown error'}`);
127-129: Update error messages for Solana push transaction.The error references "eth_sendTransaction", which may be a copy-paste leftover. Rename it to clarify it is a Solana push error.
- console.error('eth_sendTransaction push error', pushResponse); - throw new Error('eth_sendTransaction push error'); + console.error('solana_signAndSendTransaction push error', pushResponse); + throw new Error('solana_signAndSendTransaction push error');
166-201: Robust namespace processing.Checks for unsupported, inactive, and active networks are well structured. Make sure the
hasAccountslogic aligns with future expansions (e.g., multi-account requirements).suite-common/walletconnect/src/walletConnectTypes.ts (1)
5-6: Interfaces and new methods appear coherent.
getChainId,getNamespace, andprocessNamespacesprovide clear hooks for multi-chain context.- Consider clarifying the
requiredparameter's meaning inprocessNamespaces(e.g., rename toareNamespacesMandatory) for improved readability.Also applies to: 14-21
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (1)
71-89: Account ordering logic is coherent.
Sorting by device, then account type, then index is well-structured. Potentially factor it out as a utility for reusability if needed across the codebase.suite-common/walletconnect/src/walletConnectThunks.ts (1)
128-128: Helpful console logging for error diagnostics.
We see an inline console.error; if it’s just for debugging, consider adding a consistent error handling approach.suite-common/walletconnect/src/adapters/ethereum.ts (2)
20-25: Confirm method coverage.
Consider verifying whether other Ethereum methods likeeth_signare needed. Otherwise, keeping only these methods is fine if your dApps and internal logic don't require additional calls.
219-254: Namespace processing approach is generally good.
This adds robust logic to categorize networks asunsupported,active, orinactive. Consider how this might be extended or unified if you add multi-chain or non-EVM support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
packages/blockchain-link-types/src/responses.ts(1 hunks)packages/blockchain-link/src/workers/solana/fee.ts(2 hunks)packages/blockchain-link/src/workers/solana/index.ts(3 hunks)packages/components/src/components/form/Select/Select.tsx(5 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx(2 hunks)suite-common/walletconnect/package.json(1 hunks)suite-common/walletconnect/src/adapters/ethereum.ts(2 hunks)suite-common/walletconnect/src/adapters/index.ts(2 hunks)suite-common/walletconnect/src/adapters/solana.ts(1 hunks)suite-common/walletconnect/src/walletConnectThunks.ts(7 hunks)suite-common/walletconnect/src/walletConnectTypes.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
suite-common/walletconnect/src/adapters/index.ts
[error] 26-26: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
suite-common/walletconnect/src/walletConnectThunks.ts
[error] 279-279: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Releases revision Checks
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: test
- GitHub Check: build-web
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: transport-e2e-test
- GitHub Check: prepare_android_test_app
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (46)
suite-common/walletconnect/package.json (1)
21-22:✅ Verification successful
Dependency addition for Solana support
Adding the
bs58package is appropriate for Solana integration as it's required for Base58 encoding/decoding, which is the standard format for Solana addresses and transaction data.
🏁 Script executed:
#!/bin/bash # Check if bs58 is a stable package and its usage in the codebase # Check latest version and download stats to verify it's a maintained package npm view bs58 --json | jq '{version: .version, downloads: .["dist-tags"], license: .license, maintainers: .maintainers}' # Look for actual usage of bs58 in the codebase to verify it's being utilized rg --type ts "bs58" -A 3 -B 3Length of output: 24341
Dependency Addition Verified: bs58 is appropriately added for Solana support.
After verification, we confirmed that:
- bs58 (v6.0.0) is stable, MIT licensed, and maintained.
- It is actively used in the codebase (e.g., in
suite-common/walletconnect/src/adapters/solana.tsfor encoding signatures) as well as in multiple other modules for Base58 encoding/decoding.The addition of the
bs58package insuite-common/walletconnect/package.jsonis thus correctly justified to support Solana’s standard for address and transaction data encoding.packages/blockchain-link-types/src/responses.ts (1)
101-101:✅ Verification successful
Valid addition of optional feePayer property
Adding the optional
feePayerproperty to the EstimateFee interface is appropriate for Solana support, as Solana transactions require explicit identification of the account that will pay the transaction fees.
🏁 Script executed:
#!/bin/bash # Check if the feePayer property is used in the Solana-related code # Look for usage of feePayer in Solana adapter implementations rg "feePayer" --type ts -A 2 -B 2 "./packages/blockchain-link/src/workers/solana"Length of output: 547
Approved: Addition of optional feePayer property is valid
The new
feePayer?: string;property in the EstimateFee interface is properly justified, as it aligns with Solana’s requirement to explicitly designate the fee payer. Verification confirms that the property is used in the Solana adapter (e.g., inpackages/blockchain-link/src/workers/solana/index.tswherefeePayeris accessed).packages/blockchain-link/src/workers/solana/index.ts (4)
24-25: Import new decompilation function for lookup tablesAdding the
decompileTransactionMessageFetchingLookupTablesimport is aligned with Solana's recent addition of Address Lookup Tables, which helps with transaction space optimization.
491-496: Updated priority fee calculation with decompiledTransactionMessageProperly passing the decompiledTransactionMessage to getPriorityFee ensures accurate fee computation for Solana transactions, handling both the decompiled message and compiled message for different aspects of fee calculation.
515-515: Added feePayer address to fee estimation payloadAdding the feePayer address to the payload provides important information for transaction construction and signing, especially for wallet connect integration where the client needs to know which account will pay the fees.
486-489:✅ Verification successful
Using more robust transaction message decompilation
The switch to
decompileTransactionMessageFetchingLookupTablesis a good improvement as it properly handles Address Lookup Tables, which are important for complex Solana transactions.
🏁 Script executed:
#!/bin/bash # Verify the implementation and usage of the decompilation function # Check how this new function is implemented in @solana/web3.js rg "decompileTransactionMessageFetchingLookupTables" --type ts -A 10 -B 2 "./node_modules/@solana" # Check for other usages of this function in the codebase to ensure consistency rg "decompileTransactionMessageFetchingLookupTables" --type ts -A 2 -B 2Length of output: 1159
Improvement Validated – Transaction Message Decompilation Upgrade Approved
The updated implementation using
decompileTransactionMessageFetchingLookupTablesconsistently handles Address Lookup Tables, ensuring robust decompilation of complex Solana transactions. Our search confirmed its correct usage inpackages/blockchain-link/src/workers/solana/index.tswithout conflicting references. Great work on aligning with best practices.packages/components/src/components/form/Select/Select.tsx (5)
119-119: Added menu width control property to WrapperPropsAdding the
$menuFitContenttransient property to the styled component props is the correct pattern for controlling CSS from component props.
285-285: Added menuFitContent to SelectProps interfaceThe property is correctly added to the SelectProps interface, maintaining type safety for component consumers.
299-299: Updated Select component parametersThe menuFitContent parameter is added to the function signature, allowing consumers to use this feature when needed.
359-359: Passed menuFitContent to styled wrapper componentThe menuFitContent prop is correctly passed to the styled component as a transient prop ($menuFitContent), following React's best practices for passing props to styled components.
234-238:✅ Verification successful
Dynamic menu width styling implementation
The conditional styling for the select menu width is implemented correctly. When
$menuFitContentis true, the menu will size itself according to content instead of filling the available space.Let's examine how this feature works with the Solana account selection in the WalletConnect integration:
🏁 Script executed:
#!/bin/bash # Check how menuFitContent is used in WalletConnect components # Look for usage of menuFitContent in WalletConnect components rg "menuFitContent" --type ts -A 2 -B 2 "./packages/suite/src/components/suite/modals" rg "menuFitContent" --type ts -A 2 -B 2 "./suite-common/walletconnect"Length of output: 1006
Dynamic menu width styling implementation verified in WalletConnect integration
The conditional styling in
Select.tsxworks as expected. When$menuFitContentis true, the menu sizes itself based on its content. Our grep search inWalletConnectProposalModal.tsxconfirms that the WalletConnect integration correctly passes and uses this prop for displaying the Solana account selection menu. No changes are required.packages/blockchain-link/src/workers/solana/fee.ts (2)
4-4: Good adoption of theCompilableTransactionMessagetype.This helps simplify fee calculation by removing the need to call
decompileTransactionMessage.
59-64: Ensure robust handling of empty or undefined instructions.While this approach to retrieve “writable” accounts likely works in most scenarios, consider verifying that
decompiledMessage.instructionsis neither undefined nor empty before processing.suite-common/walletconnect/src/adapters/solana.ts (4)
1-3: Imports look correct.All imported packages are relevant and align with the project's stated dependencies.
136-141: Chain ID logic appears consistent.Mapping testnet/mainnet to distinct chain IDs is straightforward. Verify that these IDs match your supported network references within the app and external docs.
142-164: Efficiently constructing a Solana namespace object.The approach is concise and ensures each chain ID is included only once.
203-209: Adapter structure is well-defined.The
solanaAdapterneatly ties everything together, adhering to theWalletConnectAdapterinterface.suite-common/walletconnect/src/adapters/index.ts (3)
1-2: Imports look good.
No issues spotted; these imports are straightforward.
6-7: Adapter additions are correctly integrated.
The inclusion ofsolanaAdapteris consistent, and relevant types are correctly imported.
28-36: Solid modular design for processing namespaces.
Calls each adapter’sprocessNamespacesto build out the final results in a centralized manner. This is clear and maintainable.packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (12)
1-2: React and React-Select imports are valid.
No issues with the new imports or their usage here.
4-7: Imports from shared modules look consistent.
All references to suite-common and wallet-core types appear in order.
13-14: WalletConnect types imported successfully.
This aligns well with usage in the component.
15-25: UI component imports are correct.
No code smell found; these additions look good.
33-34: Account-related imports are relevant.
No issues with referencing the new components.
40-46: Newly introducedAccountGroupinterface is intuitive.
The design neatly groups accounts by device and metadata; no immediate concerns.
51-53: Storing selected accounts in local state.
Tracking selected accounts here is appropriate for the new selection UI.
56-56: PassingselectedDefaultAccountsto the thunk.
Well-handled approach for user-selected accounts; the logic is straightforward.
63-69: Single-account-per-symbol logic.
Filtering out previously selected accounts of the same symbol ensures only one account is retained per symbol. Verify this aligns with your desired UX when multiple accounts share the same symbol.
91-97: Default account selection effect.
The code synchronizes selected accounts with active networks as intended.
99-122: Comprehensive grouping function.
Groups accounts by device and account type. Good for a multi-device environment.
197-205: Solana-specific handling via Select component.
Smart approach to let the user pick a default account for Solana networks. ThemenuFitContentfeature is a nice enhancement for UI.Also applies to: 209-239, 242-247
suite-common/walletconnect/src/walletConnectThunks.ts (6)
13-13: Network import well-used for chain retrieval logic.
No additional notes; looks fine.
18-23: Centralized adapter imports.
Makes the code more readable and maintainable.
100-101: Namespace processing is cleanly delegated.
LeveragesprocessNamespacesto handle both required and optional. Good reuse of logic.
155-159: Expanded thunk signature for chosen default accounts.
Allows more granular control of the accounts used in the final session.
163-185: Ordering approach for priority accounts.
AppendingselectedDefaultAccountsfirst, then other accounts, meets user preference logic. Well done.
242-256: Chain ID retrieval from adapters.
Ensures correct chain-based events if thechainIdis available. This is a sensible approach for multi-network support.suite-common/walletconnect/src/adapters/ethereum.ts (8)
2-2: Import usage looks appropriate.
This is a straightforward typed import for WalletConnect proposals. No further issues.
5-5: Double-check network availability.
Ensure that all Ethereum networks innetworksCollectionhave validchainIdproperties so calls togetNetworksucceed.
8-8: Standard import, no concerns.
TheAccounttype import is appropriate and aligns with usage in this file.
14-18: Type imports for WalletConnect operations.
These type imports look fine and align with the new methods in this file.
28-28: Handle possible undefined return values.
This thunk might returnstringorundefined. Ensure that callers handle both cases gracefully.
193-193: Validate that the network has a chainId.
Returning\eip155:`is correct for Ethereum networks, but confirm all relevant networks definechainId`.
195-217: Namespace construction is logically sound.
This function correctly collects visible Ethereum accounts. Implementation is clear and consistent.
256-261: Final adapter shape is well-defined.
All the new functions and properties are properly exposed, ensuring consistent usage ofethereumAdapterthroughout the codebase.
9331fff to
8a19fbf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (2)
65-71: The account selection logic requires additional validation.The
handleSelectAccountfunction removes any existing account with the same symbol before adding the new one, which is correct. However, it might be safer to verify that the account being added is valid before adding it to the state.Consider adding validation to ensure the account is valid before updating the state:
const handleSelectAccount = (account: Account) => { + if (!account) return; setSelectedDefaultAccounts(prev => { const newAccounts = prev.filter(prevAccount => prevAccount.symbol !== account.symbol); return [...newAccounts, account]; }); };
105-128: The account grouping function could be optimized.The
buildAccountOptionGroupsfunction effectively groups accounts by device and account type, but it could be more efficient.Consider using a map-based approach for faster lookups:
const buildAccountOptionGroups = (network: PendingConnectionProposalNetwork) => { const groups: AccountGroup[] = []; + const groupMap = new Map<string, AccountGroup>(); orderedAccounts .filter(account => account.symbol === network.symbol) .forEach(account => { const device = devices.find(d => d.state?.staticSessionId === account.deviceState); if (!device) return; const label = `${account.deviceState}-${account.accountType}`; + let group = groupMap.get(label); - const group = groups.find(g => g.label === label); if (group) { group.options.push(account); } else { + group = { label, device, accountType: account.accountType, networkType: account.networkType, options: [account], }; + groupMap.set(label, group); + groups.push(group); - groups.push({ - label, - device, - accountType: account.accountType, - networkType: account.networkType, - options: [account], - }); } }); return groups; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/blockchain-link-types/src/responses.ts(1 hunks)packages/blockchain-link/src/workers/solana/fee.ts(2 hunks)packages/blockchain-link/src/workers/solana/index.ts(3 hunks)packages/components/src/components/form/Select/Select.tsx(5 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/blockchain-link-types/src/responses.ts
- packages/blockchain-link/src/workers/solana/fee.ts
- packages/components/src/components/form/Select/Select.tsx
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: test
- GitHub Check: transport-e2e-test
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (14)
packages/blockchain-link/src/workers/solana/index.ts (4)
24-25: Good addition of the new decompilation function importThe addition of
decompileTransactionMessageFetchingLookupTablesis an improvement over the existingdecompileTransactionMessagefunction as it will allow the system to handle address lookup tables, which are important for complex Solana transactions.
486-489: Appropriate enhancement to transaction message decompilationThis change properly replaces direct decompilation with a version that fetches lookup tables. This is necessary because Solana transactions can reference addresses through lookup tables for efficiency, and WalletConnect needs complete transaction data to function correctly.
491-496: Update to priority fee calculation with new message formatThe priority fee calculation now correctly uses the decompiled transaction message while still maintaining access to the original message and signatures. This ensures proper fee estimation for transactions that utilize address lookup tables.
515-515: Good addition of fee payer information in responseIncluding the fee payer address in the estimation response is important for WalletConnect integration, as it allows the client to clearly identify which account will be responsible for paying transaction fees.
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (10)
1-7: Import additions look appropriate for the new functionality.The added imports (React hooks, GroupBase, and various type imports) align with the new account selection functionality being implemented for Solana networks.
13-25: Component imports are well-organized.The imports from @trezor/components are appropriately structured and include all necessary UI components for the enhanced modal functionality.
32-36: Imports for account-related components are appropriate.The inclusion of
AccountLabelandAccountTypeBadgecomponents supports the account selection UI.
41-47: Well-structured AccountGroup interface.The new interface clearly defines the structure for grouping accounts by device and type, which is necessary for the select dropdown implementation. The interface properly extends the GroupBase type from react-select.
52-55: Good use of selectors for state management.The component properly uses Redux selectors to access accounts, account labels, and devices, which are needed for the account selection functionality.
57-59: The handleAccept function now includes selectedDefaultAccounts.This change ensures that the user's account selections are properly passed to the session proposal approval action.
73-96: Comprehensive account ordering logic.The
orderedAccountsuseMemo correctly filters for visible accounts and sorts them by device state, account type, and index. The sorting logic prioritizes normal accounts and maintains a consistent order based on device and account characteristics.
97-103: Proper initialization of selectedDefaultAccounts.The useEffect hook correctly sets the default accounts based on active networks from the proposal, ensuring that the initial state reflects the networks requested by the dApp.
203-255: Excellent implementation of Solana account selection.The conditional rendering for Solana networks now includes a Select component that allows users to choose from available accounts. The implementation correctly:
- Shows account selection only for active Solana networks
- Displays accounts grouped by device and type
- Shows appropriate labels and badges for each account
- Handles selection changes
The use of the
menuFitContentproperty ensures that the dropdown menu adjusts its width based on content, improving the UI experience.
1-302: Verify that the WalletConnect session correctly works with the selected Solana account.Since this is a new feature for Solana networks, it's important to ensure that the selected account is correctly used in WalletConnect sessions.
Please test the following scenarios:
- Connecting to a Solana dApp with multiple accounts available
- Selecting different accounts and verifying that the correct account is used
- Reconnecting to a previously connected dApp to verify that account selection persists
🧰 Tools
🪛 Biome (1.9.4)
[error] 186-186: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
8a19fbf to
4d657dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
suite-common/walletconnect/src/adapters/index.ts (1)
32-40: Consider adding error handling for adapter processing.If any adapter’s
processNamespacesthrows an error, it may disrupt the entire flow. Handling these failures individually can improve robustness.For instance:
export const processNamespaces = ( accounts: Account[], networks: PendingConnectionProposalNetwork[], namespaces: ProposalTypes.RequiredNamespaces, required: boolean, ) => - adapters.forEach(adapter => - adapter.processNamespaces(accounts, networks, namespaces, required), - ); + adapters.forEach(adapter => { + try { + adapter.processNamespaces(accounts, networks, namespaces, required); + } catch (err) { + console.error(`Failed to process namespaces for adapter: ${adapter.networkType}`, err); + // Optionally handle error further + } + });suite-common/walletconnect/src/adapters/solana.ts (1)
86-139: Fix the leftover “eth_sendTransaction” label in the error message.The console error message may confuse developers; rename it to something more Solana-specific.
- console.error('eth_sendTransaction push error', pushResponse); + console.error('solana_signAndSendTransaction push error', pushResponse);suite-common/walletconnect/src/adapters/ethereum.ts (1)
221-255: Consider refactoring status determination logic.The
processNamespacesfunction efficiently handles namespace processing, but the nested status determination logic could be improved for clarity.Consider extracting the status determination logic to a separate function for better readability:
- const getStatus = () => { - if (!supported) return 'unsupported'; - const hasAccounts = accounts.some( - account => account.symbol === supported?.symbol, - ); - if (hasAccounts) return 'active'; - - return 'inactive'; - }; + const getNetworkStatus = ( + networkSupported: typeof supported, + accountsList: Account[] + ) => { + if (!networkSupported) return 'unsupported'; + const hasAccounts = accountsList.some( + account => account.symbol === networkSupported?.symbol, + ); + return hasAccounts ? 'active' : 'inactive'; + }; networks.push({ namespaceId: chain, symbol: supported?.symbol, name: supported?.name ?? `Unknown (${chain})`, - status: getStatus(), + status: getNetworkStatus(supported, accounts), required, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
packages/components/src/components/form/Select/Select.tsx(5 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx(2 hunks)suite-common/walletconnect/package.json(1 hunks)suite-common/walletconnect/src/adapters/ethereum.ts(2 hunks)suite-common/walletconnect/src/adapters/index.ts(2 hunks)suite-common/walletconnect/src/adapters/solana.ts(1 hunks)suite-common/walletconnect/src/walletConnectThunks.ts(7 hunks)suite-common/walletconnect/src/walletConnectTypes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- suite-common/walletconnect/package.json
- packages/components/src/components/form/Select/Select.tsx
- suite-common/walletconnect/src/walletConnectThunks.ts
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: build-deploy
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: build-web
- GitHub Check: build
- GitHub Check: transport-e2e-test
- GitHub Check: test
🔇 Additional comments (27)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (6)
41-47: Good approach to account grouping with the AccountGroup interfaceThe new interface provides a clear structure for grouping accounts by device and account type, which is essential for the Solana account selection feature. This makes the code more maintainable and type-safe.
55-55: Well-implemented state management for account selectionThe
selectedDefaultAccountsstate andhandleSelectAccountfunction are properly implemented. The approach of filtering existing accounts with the same symbol before adding the new one prevents duplicates, which is a good practice.The modification to include selected accounts in the
sessionProposalApproveThunkcall is correctly implemented, ensuring the user's account preferences are respected.Also applies to: 57-59, 65-71
73-96: Effective account ordering strategyThe
useMemohook for ordering accounts is well-implemented with clear sorting criteria:
- By device state
- By account type (with normal accounts prioritized)
- By account index
This provides a consistent and intuitive ordering for account selection that aligns with user expectations.
97-103: Good initialization of default account selectionThe effect hook appropriately initializes the
selectedDefaultAccountsstate based on active networks from the pending proposal. This ensures that default selections are automatically provided when the proposal changes.
105-128: Well-structured account grouping logicThe
buildAccountOptionGroupsfunction effectively organizes accounts into groups based on device state and account type, creating a more intuitive selection interface for users. The group labeling approach ensures unique groupings.
203-255: Appropriate conditional rendering for Solana networksThe implementation correctly identifies Solana networks and renders a Select component for account selection, while maintaining the existing UI for other networks. The Select component is configured with appropriate props for formatting and interaction.
This directly addresses the PR objective of adding account selection for Solana, which is needed because Solana adapters typically don't allow account switching.
suite-common/walletconnect/src/walletConnectTypes.ts (2)
5-6: All good on these added imports.No issues spotted with importing
NetworkandAccounthere.
14-21: Interface updates look appropriate.These method signatures appear consistent with the rest of the codebase; no concerns.
suite-common/walletconnect/src/adapters/index.ts (2)
1-1: No issues with the new imports.The import statements for
ProposalTypes,solanaAdapter, andPendingConnectionProposalNetworkare valid and necessary.Also applies to: 6-7
23-31: Refactor from object spreading looks good.Using
Object.assign(acc, val)instead of repeatedly spreading objects avoids quadratic time complexity. Nicely done!suite-common/walletconnect/src/adapters/solana.ts (9)
1-3: Initial imports look fine.Everything is in order here.
5-10: All dependency imports look good.No issues with these imports; they align with usage in the file.
12-18: No concerns with constants and type imports.The references to
WALLETCONNECT_MODULEand local types are correct.
19-25: Method set appears coherent.Listing Solana-specific methods is clear and maintainable.
141-146: Chain ID enum declarations appear correct.These enumerations match documented Solana chain references.
148-152: Validate returning multiple mainnet chain IDs.Returning two chain IDs for mainnet is potentially intentional but can be confusing. Please confirm this is the desired approach.
Would you like to confirm that supporting both
MAINNETandMAINNET_LEGACYis required in all usage contexts?
153-177: Namespace generation logic looks good.Dynamically collecting chains and accounts for Solana is well-structured.
179-216: Processing logic for Solana namespaces is solid.Properly categorizing networks as ‘active’, ‘inactive’, or ‘unsupported’ fits the spec.
217-224: Adapter export finalization is well-organized.No issues found; the adapter object aligns with the interface.
suite-common/walletconnect/src/adapters/ethereum.ts (8)
2-2: Import looks good.The addition of
ProposalTypesfrom@walletconnect/typesis necessary for handling WalletConnect proposal namespace types in the newprocessNamespacesfunction.
5-5: Clean import organization.The new imports are properly organized and support the extended functionality needed for enhanced WalletConnect integration. The imports provide access to network utilities, account types, and WalletConnect namespace interfaces essential for multi-chain support.
Also applies to: 8-8, 14-19
20-25: Good code organization.Extracting Ethereum methods into a constant improves maintainability and reusability. This extraction makes it clearer what methods are supported by the Ethereum adapter and facilitates easier updates if supported methods change in the future.
28-28: Return type update correctly reflects function behavior.Updating the return type from
voidtostring | undefinedaccurately represents the actual return values from different method handlers within the thunk (signatures, transaction IDs, and chain IDs).
193-193: Clean utility function implementation.The
getChainIdutility function provides a clean abstraction for generating properly formatted EIP-155 chain IDs. Returning an array makes this function more flexible for potential future extensions where a network might be mapped to multiple chain IDs.
195-219: Well-structured namespace generation.The
getNamespacefunction correctly constructs the WalletConnect namespace for Ethereum by:
- Filtering accounts based on visibility and network type
- Collecting unique chain IDs
- Formatting account addresses according to WalletConnect specifications
The use of array methods and clear variable naming makes the code easy to follow.
257-264: Good adapter implementation.The updated
ethereumAdapterobject properly exposes all the necessary functionality for Ethereum WalletConnect integration. The use ofsatisfiesoperator provides type safety while allowing the object to have additional properties if needed.
1-264:✅ Verification successful
Verify WalletConnect Protocol Version Compatibility.
This implementation appears to be integrating with WalletConnect, but I don't see explicit version handling or compatibility checks. WalletConnect v2 has significant differences from v1.
Please verify that this implementation is compatible with the WalletConnect version being used throughout the application:
🌐 Web query:
What are the major differences between WalletConnect v1 and v2 protocol?💡 Result:
WalletConnect v2.0 introduced significant upgrades over v1.0, addressing scalability, security, and multi-chain interoperability. Here are the key differences:
Multi-Chain Support
- v1.0: Limited to single-chain interactions (primarily EVM-compatible chains) per session, requiring manual switching[1][4][10].
- v2.0: Supports simultaneous multi-chain interactions, allowing dApps to request compatibility across multiple blockchains (e.g., Ethereum, Cosmos, Polkadot) within a single session[1][2][10].
Chain Agnosticism
- v1.0: Assumed EVM compatibility, creating barriers for non-EVM chains[1][4].
- v2.0: Chain-agnostic design enables interoperability with any blockchain, expanding support to ecosystems like Solana and Bitcoin[1][4][13].
Session Management
- v1.0: Sessions persisted indefinitely until manually disconnected, leading to stale connections[7][19].
- v2.0: Introduces 7-day session expiration and 30-day pairings for reusability, with automatic disconnects[3][7][19].
Pairing Process
- v1.0: Required QR code scanning for every new dApp connection[1][4].
- v2.0: Decouples pairing from sessions, allowing cached pairings for faster reconnections without repeated scans[4][7].
Security & Performance
- v1.0: Relied on centralized relays vulnerable to data interception[1][4].
- v2.0: Uses decentralized Waku 2.0 protocol with end-to-end encryption and message rebroadcasting for reliability[4][7][19].
Developer Improvements
- JSON-RPC Permissions: v2.0 requires upfront method declarations to ensure API compatibility between wallets and dApps[4][7].
- Message Tracking: v2.0 adds request history and acknowledgment features, resolving v1.0’s lack of message confirmation[7][19].
User Experience
- v1.0: Frequent reauthorizations needed after app/browser closures[1][3].
- v2.0: Persistent sessions reduce interruptions, and multi-device access is supported[1][10].
Adoption Timeline
- v1.0 was permanently shut down on June 28, 2023, mandating migration to v2.0 for continued functionality[2][10][16].
These upgrades position WalletConnect v2.0 as a more scalable, secure, and flexible protocol for Web3’s evolving multi-chain ecosystem.
Citations:
- 1: https://tanx.fi/blog/wallet-connect-2-0-what-s-it-all-about
- 2: https://walletconnect.network/blog/walletconnect-v1-0-has-now-been-shut-down
- 3: https://secuxtech.com/blogs/blog/the-new-and-improved-walletconnect-2-0
- 4: https://walletconnect.network/blog/walletconnect-network-v2-0-what-s-new
- 5: https://npmjs.com/package/@web3auth/wallet-connect-v1-adapter
- 6: https://www.venly.io/blog/walletconnect-v2-takes-venly-wallets-to-the-next-level
- 7: https://blog.skolem.com/the-great-migration-to-walletconnect-v2-0/
- 8: https://web3auth.io/docs/sdk/pnp/web/adapters/wallet-connect-v1
- 9: https://specs.walletconnect.com/2.0
- 10: https://keyring.app/walletconnect-v2/
- 11: https://wagmi.sh/react/api/connectors/walletConnect
- 12: https://tokeny.com/introducing-walletconnect-v2-discover-the-new-upgrades/
- 13: https://beincrypto.com/learn/walletconnect-guide/
- 14: https://github.com/jemshit/WalletConnect
- 15: WalletConnect 2.0 wevm/wagmi#2582
- 16: https://cryptounicorns.substack.com/p/walletconnect-v20-migration
- 17: https://github.com/WalletConnect/walletconnect-specs
- 18: https://docs.reown.com/api/sign/wallet-usage
- 19: https://www.obvious.technology/blogs/obvious-integrates-wallet-connect-2-0-into-its-app
- 20: https://www.blockchain.com/blog/posts/wallet-connect
Confirm WalletConnect v2 Compliance
Based on the differences between WalletConnect v1 and v2—and on the usage of v2-specific patterns in this file—the Ethereum adapter is implemented using WalletConnect v2 conventions:
- The use of types like
ProposalTypes.RequiredNamespacesand the namespace-based logic (e.g., ingetNamespaceandprocessNamespaces) aligns with v2, which supports multi-chain interactions and explicit namespace definitions.- Although there isn’t an explicit “version check” (as v2’s design assumes correct SDK usage), this is expected given the protocol’s structure.
For added clarity and maintainability, please consider adding an inline comment or documentation note that explicitly states this adapter is intended for WalletConnect v2, ensuring consistency across the codebase.
4d657dd to
ec0cd0a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (2)
203-248: Check for potential accessibility issues with the Select componentThe Select component implementation looks good functionally, but consider adding appropriate ARIA attributes or ensuring the component has proper keyboard navigation support for accessibility.
The component could benefit from adding
aria-labelto improve screen reader support:<Select isSearchable={false} isClearable={false} isClean menuFitContent size="small" + aria-label={`Select account for ${network.name}`} value={selectedDefaultAccounts.find( account => account.symbol === network.symbol, )} options={buildAccountOptionGroups(network)} // rest of the component />
244-246: Consider adding type safety for the onChange handlerThe current implementation uses
(option: Option)which may be too generic. Consider using a more specific type for better type safety.-onChange={(option: Option) => +onChange={(option: Option & Account) => handleSelectAccount(option) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/blockchain-link-types/src/responses.ts(1 hunks)packages/blockchain-link/src/workers/solana/fee.ts(2 hunks)packages/blockchain-link/src/workers/solana/index.ts(3 hunks)packages/components/src/components/form/Select/Select.tsx(5 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/blockchain-link-types/src/responses.ts
- packages/components/src/components/form/Select/Select.tsx
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: run-e2e-suite-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-e2e-suite-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-e2e-suite-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: transport-e2e-test
- GitHub Check: prepare_android_test_app
- GitHub Check: build-web
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: test
🔇 Additional comments (15)
packages/blockchain-link/src/workers/solana/index.ts (4)
25-25: Good addition of necessary importThe import of
decompileTransactionMessageFetchingLookupTablesis added correctly, which will be used to enhance fee estimation by fetching lookup tables.
486-489: Improved transaction message decompilation with lookup tablesThe new async function
decompileTransactionMessageFetchingLookupTablesreplaces the synchronousdecompileTransactionMessage. This is an improvement as it fetches lookup tables during the decompilation process, which should provide more accurate transaction information for Solana transactions.
491-496: Enhanced priority fee calculationThe priority fee calculation has been updated to use the newly decompiled transaction message. This change properly leverages the more comprehensive transaction data obtained from the lookup tables.
515-515: Added feePayer to estimation payloadThe feePayer is now retrieved from the decompiledTransactionMessage, which aligns with the PR objective of enhancing fee handling for Solana transactions and supporting account selection in the WalletConnect adapter.
packages/blockchain-link/src/workers/solana/fee.ts (3)
4-4: Appropriate type import additionAdded import for
CompilableTransactionMessagetype which is necessary for the updated function signature.
58-62: Optimized function signatureThe
getPriorityFeefunction now accepts a pre-decompiled message as a parameter, eliminating the need for redundant decompilation within this function. This is a good optimization that should improve performance.
64-64: Simplified instruction accessNow directly accessing instructions from the decompiled message instead of performing decompilation again within this function. This change makes the code cleaner and more efficient.
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (8)
41-47: Well-structured interface for account groupingThe new
AccountGroupinterface properly extendsGroupBase<Account>and includes all necessary properties for account grouping and display. This is a good way to establish a clear data structure for organizing accounts by device and account type.
55-55: Appropriate state management for account selectionThe introduction of
selectedDefaultAccountsstate is a good approach for tracking user-selected accounts for the Solana adapter. This aligns with the PR objective of adding account selection to the pairing modal specifically for Solana.
57-59: Nice integration with the approval workflowThe modification to include
selectedDefaultAccountsin thesessionProposalApproveThunkdispatch properly incorporates user account selections into the approval process.
65-71: Well-implemented account selection handlerThe
handleSelectAccountfunction correctly preserves selections for other symbols while updating the selection for the current symbol. This ensures a smooth user experience when selecting different accounts.
73-96: Effective account ordering logicThe
orderedAccountsmemoization correctly filters and sorts accounts based on visibility, device state, and account type, with appropriate priority given to normal accounts. This provides a consistent and user-friendly ordering of accounts in the UI.
97-103: Good initialization of default accountsThe useEffect hook properly initializes the
selectedDefaultAccountsstate based on the active networks from the pending proposal, ensuring that default selections are updated whenever the proposal changes.
105-128: Comprehensive group creation functionThe
buildAccountOptionGroupsfunction effectively organizes accounts into groups based on device state and account type, creating a hierarchical structure that improves the user selection experience.
209-209:✅ Verification successful
Verify the implementation of the menuFitContent property
The
menuFitContentproperty is being used here, which according to the summary was newly introduced to allow select menus to adjust their width based on content.
🏁 Script executed:
#!/bin/bash # Check the Select component implementation to verify the menuFitContent property grep -r "menuFitContent" --include="*.tsx" --include="*.ts" packages/components/src/components/form/Select/Length of output: 655
Action: Confirmed — the
menuFitContentproperty is implemented correctly.After verifying the code in the Select component (located at
packages/components/src/components/form/Select/Select.tsx), themenuFitContentproperty is defined, conditionally applied, and properly passed through to the styled component. No further changes are required in theWalletConnectProposalModal.tsxusage.
ec0cd0a to
326f397
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
suite-common/walletconnect/src/adapters/index.ts (1)
32-40: Consider adding error handling for adapter processing.The
processNamespacesfunction iterates through adapters but doesn't include error handling if an adapter's implementation fails.Consider updating the implementation to include error handling:
export const processNamespaces = ( accounts: Account[], networks: PendingConnectionProposalNetwork[], namespaces: ProposalTypes.RequiredNamespaces, required: boolean, ) => - adapters.forEach(adapter => - adapter.processNamespaces(accounts, networks, namespaces, required), - ); + adapters.forEach(adapter => { + try { + adapter.processNamespaces(accounts, networks, namespaces, required); + } catch (error) { + console.error(`Error processing namespaces for ${adapter.networkType}:`, error); + } + });packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (4)
65-71: Account selection logic needs better type safety.The account selection handler logic is sound, but lacks explicit type checking.
Consider adding type annotation for the account parameter:
- const handleSelectAccount = (account: Account) => { + const handleSelectAccount = (account: Account | null) => { + if (!account) return; setSelectedDefaultAccounts(prev => { const newAccounts = prev.filter(prevAccount => prevAccount.symbol !== account.symbol); return [...newAccounts, account]; }); };
105-128: Group building function should handle missing devices more robustly.The buildAccountOptionGroups function skips accounts with missing devices without proper error handling or logging.
Consider adding better handling for missing devices:
const buildAccountOptionGroups = (network: PendingConnectionProposalNetwork) => { const groups: AccountGroup[] = []; orderedAccounts .filter(account => account.symbol === network.symbol) .forEach(account => { const device = devices.find(d => d.state?.staticSessionId === account.deviceState); - if (!device) return; + if (!device) { + console.warn(`Device not found for account ${account.descriptor}`); + return; + } const label = `${account.deviceState}-${account.accountType}`; const group = groups.find(g => g.label === label); if (group) { group.options.push(account); } else { groups.push({ label, device, accountType: account.accountType, networkType: account.networkType, options: [account], }); } }); return groups; };
199-200: Avoid usinganytype casting.Using
anytype casting fornetwork.symbolbypasses TypeScript's type checking, which can lead to runtime errors.If possible, update the CoinLogo component to accept the appropriate symbol type or create a proper type guard:
- symbol={network.symbol as any} + symbol={network.symbol}This would require updating the CoinLogo component to accept the appropriate symbol type.
244-246: Improve type safety in Select onChange handler.The onChange handler casts the option to
Optiontype without validation, which could lead to runtime errors.Consider adding type checking:
-onChange={(option: Option) => - handleSelectAccount(option) -} +onChange={(option: Option | null) => + option && handleSelectAccount(option as Account) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
packages/components/src/components/form/Select/Select.tsx(5 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx(2 hunks)suite-common/walletconnect/package.json(1 hunks)suite-common/walletconnect/src/adapters/ethereum.ts(2 hunks)suite-common/walletconnect/src/adapters/index.ts(2 hunks)suite-common/walletconnect/src/adapters/solana.ts(1 hunks)suite-common/walletconnect/src/walletConnectThunks.ts(7 hunks)suite-common/walletconnect/src/walletConnectTypes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- suite-common/walletconnect/package.json
- suite-common/walletconnect/src/adapters/ethereum.ts
- suite-common/walletconnect/src/adapters/solana.ts
- packages/components/src/components/form/Select/Select.tsx
- suite-common/walletconnect/src/walletConnectThunks.ts
- suite-common/walletconnect/src/walletConnectTypes.ts
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: run-e2e-suite-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-e2e-suite-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-e2e-suite-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: prepare_android_test_app
- GitHub Check: transport-e2e-test
- GitHub Check: build-web
- GitHub Check: test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (9)
suite-common/walletconnect/src/adapters/index.ts (3)
6-7: Well structured imports for Solana adapter implementation.The imports are organized properly, bringing in the necessary Solana adapter and WalletConnect types to support the PR's objective of implementing WalletConnect Solana adapter.
9-13: Proper integration of Solana adapter in the existing structure.Adding the
solanaAdapterto the existing adapters array is a clean approach that follows the established pattern.
23-30: Successfully addressed previous review comment on Object.assign.The implementation has been updated to use
Object.assigninstead of object spreading in the reduce function, which addresses the performance concern from the previous review comment about time complexity.packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/WalletConnectProposalModal.tsx (6)
1-36: Appropriate imports and organization for new account selection functionality.The imports have been updated to support the new account selection functionality for Solana, including React hooks and necessary components.
41-47: Well-structured AccountGroup interface for organizing accounts.The AccountGroup interface provides a clear structure for grouping accounts by device and type, which is essential for the Solana account selection functionality.
57-59: Updated sessionProposalApproveThunk to include selected accounts.Good implementation to include the selectedDefaultAccounts in the session proposal approve action, which supports the PR objective of adding account selection for Solana.
73-96: Well-structured account sorting logic with clear prioritization.The useMemo implementation for ordering accounts is well-organized with clear prioritization rules (device state → account type → index) that make sense for the UI presentation.
97-103: Effective default account selection based on active networks.The useEffect hook appropriately sets default selected accounts based on active networks, which ensures a good user experience by pre-selecting relevant accounts.
203-255: Well-implemented Solana account selection UI.The conditional rendering for Solana networks with a Select component fulfills the PR objective of adding account selection for Solana. The implementation provides a good user experience with grouped account options and clear visual indicators.
Description
Solana support for WalletConnect. Since Solana adapters don't usually allow account switching, this PR also adds account selection to the pairing modal (only for Solana right now).
Only problem is we don't support sign message, since it's not in FW, but most dApps don't rely on it.
Screenshots: