QR login and Metamask transfers#715
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors external wallet support: removes seed-based wallet flows and APT/TON/TRON support; adds MetaMask-backed discovery, Hive Snap helpers, EVM/SOL transfer utilities, a new ExternalTransferDialog and useExternalTransfer mutation, MobileLogin QR dialog, dependency updates, and i18n adjustments across locales. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User UI
participant Dialog as ExternalTransferDialog
participant MetaMask as MetaMask / Wallet Standard
participant API as wallets package mutation
participant Cache as React Query Cache
UI->>Dialog: open(currency, username)
Dialog->>MetaMask: discover/request accounts / installSnap
MetaMask-->>Dialog: accounts / hive public keys
Dialog->>Dialog: validate recipient & amount, estimate fee
Dialog->>MetaMask: ensure chain, request sign/send or estimate
MetaMask-->>Dialog: gas estimate / txHash
Dialog->>API: useExternalTransfer.mutateAsync({ to, amount })
API-->>Dialog: { txHash, currency }
Dialog->>Cache: invalidate ["ecency-wallets","external-wallet-balance"]
Dialog-->>UI: show success (explorer URL) or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.ts`:
- Around line 90-96: The transfer currently wraps lamports with Number(lamports)
which converts the bigint returned by parseToLamports back to a Number and can
lose precision for large SOL amounts; update the Transaction.add call that uses
SystemProgram.transfer (with fromPubkey and toPubkey) to pass the lamports value
as a bigint directly (remove the Number(...) wrapper) so SystemProgram.transfer
receives the original bigint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7b38a99-64a5-4f68-ac4e-b63f93ab0b14
⛔ Files ignored due to path filters (7)
packages/wallets/dist/browser/index.d.tsis excluded by!**/dist/**packages/wallets/dist/browser/index.jsis excluded by!**/dist/**packages/wallets/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.cjsis excluded by!**/dist/**packages/wallets/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.mjsis excluded by!**/dist/**packages/wallets/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (4)
apps/web/src/features/i18n/locales/en-US.jsonpackages/wallets/src/modules/wallets/utils/metamask-discovery.tspackages/wallets/src/modules/wallets/utils/metamask-sol-transfer.spec.tspackages/wallets/src/modules/wallets/utils/metamask-sol-transfer.ts
✅ Files skipped from review due to trivial changes (2)
- packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.spec.ts
- apps/web/src/features/i18n/locales/en-US.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.ts`:
- Around line 22-23: The current logic silently truncates fractional SOL beyond
9 decimals by using slice(0, 9); change it to reject over-precision instead:
before padding, check if fraction.length > 9 and if any character after the 9th
is not '0' (i.e., non-zero precision) then throw an error (e.g., "amount has
more than 9 decimal places"); if the extra digits are all zeros you may allow
it, otherwise keep the existing behavior of computing BigInt(whole) *
LAMPORTS_PER_SOL + BigInt(paddedFraction) using paddedFraction created from
fraction.padEnd(9, "0"). Ensure this validation is applied where paddedFraction,
fraction, whole, and LAMPORTS_PER_SOL are used so transfers with over-precision
fail fast instead of being truncated.
- Around line 44-55: Change the loose any types to the proper interfaces from
`@wallet-standard/base`: import Wallet and WalletAccount and update
getMetaMaskSolanaWallet's signature to return Promise<Wallet>, change the find
callback parameter from (w: any) to (w: Wallet), and replace other any usages
for accounts to WalletAccount; for the feature checks/calls (e.g.,
w.features["standard:connect"] and w.features["solana:signAndSendTransaction"])
cast or assert w.features to the appropriate feature interfaces before invoking
their methods so TypeScript knows the methods exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68e6d467-8da1-4008-8241-d7a94a10aa96
⛔ Files ignored due to path filters (6)
packages/wallets/dist/browser/index.jsis excluded by!**/dist/**packages/wallets/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.cjsis excluded by!**/dist/**packages/wallets/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.mjsis excluded by!**/dist/**packages/wallets/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (1)
packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.ts
| async function getMetaMaskSolanaWallet(): Promise<any> { | ||
| const { getWallets } = await import("@wallet-standard/app"); | ||
| const walletsApi = getWallets(); | ||
| const wallets = walletsApi.get(); | ||
|
|
||
| // Only select a MetaMask wallet that supports Solana signing | ||
| const mmWallet = wallets.find( | ||
| (w: any) => | ||
| w.name.toLowerCase().includes("metamask") && | ||
| w.features["standard:connect"] && | ||
| w.features["solana:signAndSendTransaction"] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify strict-mode tsconfig presence
fd 'tsconfig.*json' -t f | xargs -r rg -n '"strict"\s*:\s*true'
# Verify remaining any-typed usages in this file
rg -n '\bany\b' packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.tsRepository: ecency/vision-next
Length of output: 741
🏁 Script executed:
cat -n packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.tsRepository: ecency/vision-next
Length of output: 5016
🏁 Script executed:
# Check if wallet-standard provides type definitions
fd -t f '@wallet-standard' packages/wallets | head -20Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check package.json for wallet-standard dependency and versions
rg -A 5 '"@wallet-standard' packages/wallets/package.jsonRepository: ecency/vision-next
Length of output: 483
🏁 Script executed:
# Check if `@wallet-standard/app` has type definitions available
fd -t f 'wallet-standard' packages/wallets/node_modules 2>/dev/null | head -20Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check if TypeScript types are available for `@wallet-standard/app`
# Look in the package dependencies or lock file
rg -i '@wallet-standard' pnpm-lock.yaml | head -20Repository: ecency/vision-next
Length of output: 291
🌐 Web query:
@wallet-standard/app TypeScript type definitions wallet interface
💡 Result:
The @wallet-standard/app package provides TypeScript type definitions and functions for applications to interact with wallets implementing the Wallet Standard. It primarily exports the getWallets function, which returns a Wallets API for managing registered wallets. Key interfaces and types from the package (in lib/types/wallets.d.ts): export interface Wallets { get: readonly Wallet[]; on(event: 'change', listener: => void): => void; register(...wallets: Wallet[]): => void; } The Wallet interface is defined in @wallet-standard/base: export interface Wallet { readonly version: WalletVersion; // e.g., '1.0.0' readonly name: string; readonly icon: WalletIcon; // data URI for image readonly chains: readonly string[]; // chain identifiers readonly features: Record<string, unknown>; // feature objects readonly accounts: readonly WalletAccount[]; } export interface WalletAccount { readonly address: string; readonly publicKey: ReadonlyUint8Array; readonly chains: readonly string[]; readonly features: Record<string, unknown>; readonly label?: string; readonly icon?: WalletIcon; } Usage example: import { getWallets } from '@wallet-standard/app'; const wallets = getWallets; const walletList = wallets.get; This package helps dApps detect, list, and interact with standard-compliant wallets via window events like 'wallet-standard:app-ready' and 'wallet-standard:register-wallet'. Latest version: 1.1.0 (Oct 2024). Source: https://github.com/wallet-standard/wallet-standard/tree/master/packages/core/app
Citations:
- 1: https://www.npmjs.com/package/@wallet-standard/app
- 2: https://registry.npmjs.org/%40wallet-standard%2Fapp
- 3: https://github.com/wallet-standard/wallet-standard/blob/master/packages/core/base/src/wallet.ts
- 4: https://github.com/wallet-standard/wallet-standard/blob/master/packages/core/app/src/wallets.ts
- 5: https://github.com/wallet-standard/wallet-standard
Import Wallet and WalletAccount from @wallet-standard/base instead of using any.
The code uses any in strict-mode code at lines 44, 51, 71, 75, and 108, which violates the coding guideline that "all new code should include proper types." The @wallet-standard/base package already exports typed interfaces for these shapes—import and use Wallet and WalletAccount directly:
- Line 44: Return type should be
Promise<Wallet> - Line 51: Parameter type should be
Wallet - Line 75: Parameter type should be
WalletAccount - Lines 71 and 108: For feature methods, cast features with specific interfaces (the
featuresproperty isRecord<string, unknown>, so feature-specific interfaces are still needed for method calls)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wallets/src/modules/wallets/utils/metamask-sol-transfer.ts` around
lines 44 - 55, Change the loose any types to the proper interfaces from
`@wallet-standard/base`: import Wallet and WalletAccount and update
getMetaMaskSolanaWallet's signature to return Promise<Wallet>, change the find
callback parameter from (w: any) to (w: Wallet), and replace other any usages
for accounts to WalletAccount; for the feature checks/calls (e.g.,
w.features["standard:connect"] and w.features["solana:signAndSendTransaction"])
cast or assert w.features to the appropriate feature interfaces before invoking
their methods so TypeScript knows the methods exist.
feruzm
added
the
major
1 participant
Summary by CodeRabbit
New Features
Changes
Removed