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.

Sign up for GitHub

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

Jump to bottom

FIX: switching networks bug #4770

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

FIX: switching networks bug #4770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2f07fb1
pending chain id change
mateodaza Sep 24, 2024
d9e7460
removing unused variable
mateodaza Sep 24, 2024
f9e2750
Merge branch 'develop' into changing-network-bug
mateodaza Sep 24, 2024
d031056
Merge branch 'develop' into changing-network-bug
mateodaza Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/components/modals/SwitchNetwork.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const defaultNetworks = Object.keys(networksConfig).map(key => ({
networkId: Number(key),
chainType: networksConfig[Number(key)].chainType,
}));

interface ISwitchNetworkModal extends IModal {
desc?: string;
customNetworks?: INetworkIdWithChain[];
Expand All @@ -39,11 +40,14 @@ const SwitchNetwork: FC<ISwitchNetworkModal> = ({
setShowModal,
}) => {
const { isAnimating, closeModal } = useModalAnimation(setShowModal);

const { switchChain } = useSwitchChain();
const { formatMessage } = useIntl();
const { walletChainType, handleSingOutAndSignInWithEVM, chain } =
useGeneralWallet();
const {
walletChainType,
handleSingOutAndSignInWithEVM,
setPendingNetworkId,
chain,
} = useGeneralWallet();
const chainId = (chain as Chain)?.id;
const theme = useAppSelector(state => state.general.theme);

Expand All @@ -55,6 +59,17 @@ const SwitchNetwork: FC<ISwitchNetworkModal> = ({
};
}) || defaultNetworks;

const handleNetworkItemClick = (networkId: number) => {
if (walletChainType === ChainType.SOLANA) {
setPendingNetworkId(networkId);
handleSingOutAndSignInWithEVM();
closeModal(); // Close the modal since we cannot control the wallet modal
} else {
switchChain?.({ chainId: networkId });
closeModal();
}
};

return (
<Modal
headerTitle={formatMessage({ id: 'label.switch_network' })}
Expand All @@ -67,13 +82,7 @@ const SwitchNetwork: FC<ISwitchNetworkModal> = ({
{desc && <P>{desc}</P>}
{networks?.map(({ networkId, chainType }) => (
<NetworkItem
onClick={() => {
if (walletChainType === ChainType.SOLANA) {
handleSingOutAndSignInWithEVM();
}
switchChain?.({ chainId: networkId });
closeModal();
}}
onClick={() => handleNetworkItemClick(networkId)}
$isSelected={networkId === chainId}
key={networkId}
$baseTheme={theme}
Expand Down
16 changes: 15 additions & 1 deletion src/providers/generalWalletProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
Transaction,
SystemProgram,
} from '@solana/web3.js';
import { useBalance, useDisconnect, useAccount } from 'wagmi';
import { useBalance, useDisconnect, useAccount, useSwitchChain } from 'wagmi';
import { getWalletClient } from '@wagmi/core';
import { WalletAdapterNetwork } from '@solana/wallet-adapter-base';
import { useWeb3Modal } from '@web3modal/wagmi/react';
Expand Down Expand Up @@ -58,6 +58,7 @@ interface IGeneralWalletContext {
handleSignOutAndShowWelcomeModal: () => Promise<void>;
isOnSolana: boolean;
isOnEVM: boolean;
setPendingNetworkId: (id: number | null) => void;
}
// Create the context
export const GeneralWalletContext = createContext<IGeneralWalletContext>({
Expand All @@ -76,6 +77,7 @@ export const GeneralWalletContext = createContext<IGeneralWalletContext>({
handleSignOutAndShowWelcomeModal: async () => {},
isOnSolana: false,
isOnEVM: false,
setPendingNetworkId: () => {},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent Function Signature for setPendingNetworkId

The default implementation of setPendingNetworkId in GeneralWalletContext does not match the interface signature. The interface expects (id: number | null) => void, but the default value is setPendingNetworkId: () => {}, which accepts no parameters. This inconsistency could lead to type errors or unexpected behavior.

Apply the following diff to fix the inconsistency:

 export const GeneralWalletContext = createContext<IGeneralWalletContext>({
   // ...other context values
   handleSignOutAndShowWelcomeModal: async () => {},
   isOnSolana: false,
   isOnEVM: false,
-  setPendingNetworkId: () => {},
+  setPendingNetworkId: (id: number | null) => {},
 });

Committable suggestion was skipped due to low confidence.

});

const getPhantomSolanaProvider = () => {
Expand All @@ -93,6 +95,9 @@ export const GeneralWalletProvider: React.FC<{
const [walletChainType, setWalletChainType] = useState<ChainType | null>(
null,
);
const [pendingNetworkId, setPendingNetworkId] = useState<number | null>(
null,
);
const [walletAddress, setWalletAddress] = useState<string | null>(null);
const [balance, setBalance] = useState<string>();
const [isConnected, setIsConnected] = useState<boolean>(false);
Expand All @@ -106,6 +111,7 @@ export const GeneralWalletProvider: React.FC<{
const router = useRouter();
const { token } = useAppSelector(state => state.user);
const { setVisible, visible } = useWalletModal();
const { switchChain } = useSwitchChain();

const isGIVeconomyRoute = useMemo(
() => checkIsGIVeconomyRoute(router.route),
Expand Down Expand Up @@ -266,6 +272,13 @@ export const GeneralWalletProvider: React.FC<{
}
}, [walletChainType, nonFormattedEvBalance, solanaBalance]);

useEffect(() => {
if (walletChainType === ChainType.EVM && pendingNetworkId !== null) {
switchChain?.({ chainId: pendingNetworkId });
setPendingNetworkId(null);
}
}, [walletChainType, pendingNetworkId]);
Comment on lines +275 to +280
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle Asynchronous switchChain Call and Potential Errors

The switchChain function returned by useSwitchChain is asynchronous and may throw an error if the chain switch fails. Currently, it's called without await or error handling inside the useEffect, which could lead to unhandled promise rejections and unexpected behavior.

Consider modifying the code to handle the asynchronous call and catch any errors:

 useEffect(() => {
-  if (walletChainType === ChainType.EVM && pendingNetworkId !== null) {
-    switchChain?.({ chainId: pendingNetworkId });
-    setPendingNetworkId(null);
-  }
+  const performSwitchChain = async () => {
+    if (walletChainType === ChainType.EVM && pendingNetworkId !== null) {
+      try {
+        await switchChain?.({ chainId: pendingNetworkId });
+      } catch (error) {
+        console.error('Error switching chain:', error);
+      } finally {
+        setPendingNetworkId(null);
+      }
+    }
+  };
+  performSwitchChain();
 }, [walletChainType, pendingNetworkId]);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (walletChainType === ChainType.EVM && pendingNetworkId !== null) {
switchChain?.({ chainId: pendingNetworkId });
setPendingNetworkId(null);
}
}, [walletChainType, pendingNetworkId]);
useEffect(() => {
const performSwitchChain = async () => {
if (walletChainType === ChainType.EVM && pendingNetworkId !== null) {
try {
await switchChain?.({ chainId: pendingNetworkId });
} catch (error) {
console.error('Error switching chain:', error);
} finally {
setPendingNetworkId(null);
}
}
};
performSwitchChain();
}, [walletChainType, pendingNetworkId]);


const signMessage = async (
message: string,
): Promise<string | undefined> => {
Expand Down Expand Up @@ -408,6 +421,7 @@ export const GeneralWalletProvider: React.FC<{
handleSignOutAndShowWelcomeModal,
isOnSolana,
isOnEVM,
setPendingNetworkId,
};

// Render the provider component with the provided context value
Expand Down
Loading