-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Noe/scw 3.0 #1310
base: release/3.0.0
Are you sure you want to change the base?
Noe/scw 3.0 #1310
Changes from 25 commits
514754d
082f2d3
36cebd2
b159d5e
3a42530
86fe21c
df7d582
ca1a1eb
99e745d
475b91c
2ad857c
0bd0ad4
71f4ab5
1b360f7
189b209
54a32a7
12ee316
92829e4
d996dbf
848100f
dfb4b74
795003e
a430c74
a31242d
2df37ea
80a0bdb
1f05d86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,28 +1,26 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
import AsyncStorage from "@react-native-async-storage/async-storage"; | ||||||||||||||||||||||||||||||||||||||||||||||
// TODO: move out of ConnectViaWallet | ||||||||||||||||||||||||||||||||||||||||||||||
import { memo, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { memo, useCallback, useEffect, useRef, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { ActivityIndicator } from "react-native"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||||||
useDisconnect as useThirdwebDisconnect, | ||||||||||||||||||||||||||||||||||||||||||||||
useSetActiveWallet as useSetThirdwebActiveWallet, | ||||||||||||||||||||||||||||||||||||||||||||||
useConnect as useThirdwebConnect, | ||||||||||||||||||||||||||||||||||||||||||||||
useActiveWallet as useThirdwebActiveWallet, | ||||||||||||||||||||||||||||||||||||||||||||||
} from "thirdweb/react"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { createWallet } from "thirdweb/wallets"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||||||
InstalledWallet, | ||||||||||||||||||||||||||||||||||||||||||||||
useInstalledWallets, | ||||||||||||||||||||||||||||||||||||||||||||||
} from "./ConnectViaWalletSupportedWallets"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { useInstalledWallets } from "./ConnectViaWalletSupportedWallets"; | ||||||||||||||||||||||||||||||||||||||||||||||
import config from "../../../config"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { getAccountsList } from "../../../data/store/accountsStore"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { useAppStateHandlers } from "../../../hooks/useAppStateHandlers"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { translate } from "../../../i18n"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { getEthOSSigner } from "../../../utils/ethos"; | ||||||||||||||||||||||||||||||||||||||||||||||
import logger from "../../../utils/logger"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { thirdwebClient } from "../../../utils/thirdweb"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { thirdwebClient, thirdwebWallets } from "../../../utils/thirdweb"; | ||||||||||||||||||||||||||||||||||||||||||||||
import TableView, { TableViewItemType } from "../../TableView/TableView"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { TableViewEmoji, TableViewImage } from "../../TableView/TableViewImage"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { RightViewChevron } from "../../TableView/TableViewRightChevron"; | ||||||||||||||||||||||||||||||||||||||||||||||
import { InstalledWallet, ISupportedWalletName } from "@utils/evm/wallets"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
export function getConnectViaWalletTableViewPrivateKeyItem( | ||||||||||||||||||||||||||||||||||||||||||||||
args: Partial<TableViewItemType> | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -79,17 +77,32 @@ export function getConnectViaWalletInstalledWalletTableViewItem(args: { | |||||||||||||||||||||||||||||||||||||||||||||
export const InstalledWalletsTableView = memo( | ||||||||||||||||||||||||||||||||||||||||||||||
function InstalledWalletsTableView(props: { | ||||||||||||||||||||||||||||||||||||||||||||||
onAccountExists: (arg: { address: string }) => void; | ||||||||||||||||||||||||||||||||||||||||||||||
onAccountDoesNotExist: (arg: { address: string }) => void; | ||||||||||||||||||||||||||||||||||||||||||||||
onAccountDoesNotExist: (arg: { address: string; isSCW: boolean }) => void; | ||||||||||||||||||||||||||||||||||||||||||||||
}) { | ||||||||||||||||||||||||||||||||||||||||||||||
const { onAccountExists, onAccountDoesNotExist } = props; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const walletsInstalled = useInstalledWallets(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const { connect: thirdwebConnect } = useThirdwebConnect(); | ||||||||||||||||||||||||||||||||||||||||||||||
const { disconnect: disconnectThirdweb } = useThirdwebDisconnect(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const thirdwebActiveWallet = useThirdwebActiveWallet(); | ||||||||||||||||||||||||||||||||||||||||||||||
const thirdwebActiveWalletRef = useRef(thirdwebActiveWallet); | ||||||||||||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||
thirdwebActiveWalletRef.current = thirdwebActiveWallet; | ||||||||||||||||||||||||||||||||||||||||||||||
}, [thirdwebActiveWallet]); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const setThirdwebActiveWallet = useSetThirdwebActiveWallet(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const disconnectActiveThirdweb = useCallback(async () => { | ||||||||||||||||||||||||||||||||||||||||||||||
if (!thirdwebActiveWalletRef.current) return; | ||||||||||||||||||||||||||||||||||||||||||||||
disconnectThirdweb(thirdwebActiveWalletRef.current); | ||||||||||||||||||||||||||||||||||||||||||||||
// Wait for the disconnect to complete | ||||||||||||||||||||||||||||||||||||||||||||||
while (!!thirdwebActiveWalletRef.current) { | ||||||||||||||||||||||||||||||||||||||||||||||
await new Promise((r) => setTimeout(r, 100)); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
}, [disconnectThirdweb]); | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+97
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve the disconnection polling implementation The current implementation has several concerns:
Consider this improved implementation: const disconnectActiveThirdweb = useCallback(async () => {
if (!thirdwebActiveWalletRef.current) return;
disconnectThirdweb(thirdwebActiveWalletRef.current);
- // Wait for the disconnect to complete
- while (!!thirdwebActiveWalletRef.current) {
- await new Promise((r) => setTimeout(r, 100));
- }
+ // Wait for disconnect with timeout
+ const maxAttempts = 50; // 5 seconds total
+ let attempts = 0;
+ while (thirdwebActiveWalletRef.current && attempts < maxAttempts) {
+ await new Promise((r) => setTimeout(r, 100));
+ attempts++;
+ }
+ if (thirdwebActiveWalletRef.current) {
+ logger.warn('Wallet disconnect timeout exceeded');
+ }
}, [disconnectThirdweb]); 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 101-102: Avoid redundant double-negation. It is not necessary to use double-negation when a value will already be coerced to a boolean. (lint/complexity/noExtraBooleanCast) |
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const [isProcessingWalletId, setIsProcessingWalletId] = useState< | ||||||||||||||||||||||||||||||||||||||||||||||
string | null | ||||||||||||||||||||||||||||||||||||||||||||||
>(null); | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -117,38 +130,35 @@ export const InstalledWalletsTableView = memo( | |||||||||||||||||||||||||||||||||||||||||||||
walletName: wallet.name, | ||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||
action: async () => { | ||||||||||||||||||||||||||||||||||||||||||||||
const isSCW = !!wallet?.isSmartContractWallet; | ||||||||||||||||||||||||||||||||||||||||||||||
logger.debug( | ||||||||||||||||||||||||||||||||||||||||||||||
`[Onboarding] Clicked on wallet ${wallet.name} - opening external app` | ||||||||||||||||||||||||||||||||||||||||||||||
`[Onboarding] Clicked on wallet ${wallet.name} - ${ | ||||||||||||||||||||||||||||||||||||||||||||||
isSCW ? "Opening web page" : "opening external app" | ||||||||||||||||||||||||||||||||||||||||||||||
technoplato marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
}` | ||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
setIsProcessingWalletId(wallet.name); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
if (thirdwebActiveWallet) { | ||||||||||||||||||||||||||||||||||||||||||||||
disconnectThirdweb(thirdwebActiveWallet); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
await disconnectActiveThirdweb(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||
let walletAddress: string = ""; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
// Specific flow for Coinbase Wallet | ||||||||||||||||||||||||||||||||||||||||||||||
if (wallet.name === "Coinbase Wallet") { | ||||||||||||||||||||||||||||||||||||||||||||||
const wallet = await thirdwebConnect(async () => { | ||||||||||||||||||||||||||||||||||||||||||||||
const coinbaseWallet = createWallet("com.coinbase.wallet", { | ||||||||||||||||||||||||||||||||||||||||||||||
appMetadata: config.walletConnectConfig.appMetadata, | ||||||||||||||||||||||||||||||||||||||||||||||
mobileConfig: { | ||||||||||||||||||||||||||||||||||||||||||||||
callbackURL: `https://${config.websiteDomain}/coinbase`, | ||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||
if (wallet.thirdwebId === "com.coinbase.wallet") { | ||||||||||||||||||||||||||||||||||||||||||||||
const thirdwebWallet = await thirdwebConnect(async () => { | ||||||||||||||||||||||||||||||||||||||||||||||
const coinbaseWallet = | ||||||||||||||||||||||||||||||||||||||||||||||
thirdwebWallets[wallet.name as ISupportedWalletName]; | ||||||||||||||||||||||||||||||||||||||||||||||
await coinbaseWallet.connect({ client: thirdwebClient }); | ||||||||||||||||||||||||||||||||||||||||||||||
setThirdwebActiveWallet(coinbaseWallet); | ||||||||||||||||||||||||||||||||||||||||||||||
return coinbaseWallet; | ||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
if (!wallet) { | ||||||||||||||||||||||||||||||||||||||||||||||
if (!thirdwebWallet) { | ||||||||||||||||||||||||||||||||||||||||||||||
throw new Error("No coinbase wallet"); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const account = wallet.getAccount(); | ||||||||||||||||||||||||||||||||||||||||||||||
const account = thirdwebWallet.getAccount(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
if (!account) { | ||||||||||||||||||||||||||||||||||||||||||||||
throw new Error("No coinbase account found"); | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -166,7 +176,8 @@ export const InstalledWalletsTableView = memo( | |||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
// Generic flow for all other wallets | ||||||||||||||||||||||||||||||||||||||||||||||
else if (wallet.thirdwebId) { | ||||||||||||||||||||||||||||||||||||||||||||||
const walletConnectWallet = createWallet(wallet.thirdwebId); | ||||||||||||||||||||||||||||||||||||||||||||||
const walletConnectWallet = | ||||||||||||||||||||||||||||||||||||||||||||||
thirdwebWallets[wallet.name as ISupportedWalletName]; | ||||||||||||||||||||||||||||||||||||||||||||||
const account = await walletConnectWallet.connect({ | ||||||||||||||||||||||||||||||||||||||||||||||
client: thirdwebClient, | ||||||||||||||||||||||||||||||||||||||||||||||
walletConnect: config.walletConnectConfig, | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -179,7 +190,10 @@ export const InstalledWalletsTableView = memo( | |||||||||||||||||||||||||||||||||||||||||||||
if (getAccountsList().includes(walletAddress)) { | ||||||||||||||||||||||||||||||||||||||||||||||
onAccountExists({ address: walletAddress }); | ||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||
onAccountDoesNotExist({ address: walletAddress }); | ||||||||||||||||||||||||||||||||||||||||||||||
onAccountDoesNotExist({ | ||||||||||||||||||||||||||||||||||||||||||||||
address: walletAddress, | ||||||||||||||||||||||||||||||||||||||||||||||
isSCW, | ||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
} catch (e: any) { | ||||||||||||||||||||||||||||||||||||||||||||||
logger.error("Error connecting to wallet:", e); | ||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Update all usages of
onAccountDoesNotExist
to includeisSCW
The
onAccountDoesNotExist
callback now includes an additional parameterisSCW: boolean
. Ensure that all implementations and calls to this function across the codebase are updated to pass this new parameter.