Skip to content
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

fix: update switch chain logic #1916

Merged
merged 21 commits into from
Oct 3, 2024
Merged

Conversation

fionnachan
Copy link
Member

@fionnachan fionnachan commented Sep 20, 2024

Closes FS-859

Copy link

vercel bot commented Sep 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Oct 2, 2024 5:31pm

@cla-bot cla-bot bot added the cla-signed label Sep 20, 2024
@fionnachan fionnachan marked this pull request as ready for review September 24, 2024 13:12
chrstph-dvx
chrstph-dvx previously approved these changes Sep 26, 2024
@brtkx
Copy link
Contributor

brtkx commented Sep 27, 2024

We should update this code, because useChainId defaults to Ethreum Mainnet if the connected chain is not part of our config.

@fionnachan fionnachan marked this pull request as draft September 30, 2024 13:47
@fionnachan fionnachan marked this pull request as ready for review September 30, 2024 15:13
@@ -105,7 +116,8 @@ export function TransferPanel() {
const { switchNetworkAsync } = useSwitchNetworkWithConfig({
isSwitchingNetworkBeforeTx: true
})
const chainId = useChainId()
// do not use `useChainId` because it won't detect chains outside of our wagmi config
const latestChain = useLatest(useNetwork())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we use it in an async function, we must use useLatest to ensure it's up-to-date

Comment on lines 355 to 357
if (!isTransferAllowed) {
return
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

additional safeguard

Comment on lines 957 to 980
try {
setTransferring(true)
if (isConnectedToTheWrongChain) {
trackEvent('Switch Network and Transfer', {
type: isTeleportMode
? 'Teleport'
: isDepositMode
? 'Deposit'
: 'Withdrawal',
tokenSymbol: selectedToken?.symbol,
assetType: selectedToken ? 'ERC-20' : 'ETH',
accountType: isSmartContractWallet ? 'Smart Contract' : 'EOA',
network: childChainName,
amount: Number(amount),
amount2: isBatchTransfer ? Number(amount2) : undefined,
version: 2
})
await switchNetworkAsync?.(sourceChainId)
}
} catch (error) {
return networkConnectionWarningToast()
} finally {
setTransferring(false)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the chain switching out here so that it is used for all kinds of transfers.

} finally {
setTransferring(false)
}
const sourceChainId = latestNetworks.current.sourceChain.id

if (!isTransferAllowed) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when user reached here, the connected chain must be the source chain, or else it won't proceed.

dewanshparashar
dewanshparashar previously approved these changes Oct 1, 2024
Comment on lines +22 to +29
const isConnectedToTheWrongChain = chain?.id !== networks.sourceChain.id

if (!arbTokenBridgeLoaded) {
return false
}
if (!eth) {
return false
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These conditions were in the while await Promise loop.

Since the while loop is deleted, they are added here to prevent transfers.

@fionnachan fionnachan changed the title fix: add back wrong network toast fix: update switch chain logic Oct 2, 2024
@fionnachan fionnachan merged commit 7c9c055 into master Oct 3, 2024
35 checks passed
@fionnachan fionnachan deleted the hotfix-wrong-network-toast branch October 3, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants