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

fix: disconnect previous wallet #2712

Merged
merged 2 commits into from
Oct 31, 2023
Merged

fix: disconnect previous wallet #2712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbe5fd2
fix: disconnect previously connected wallet
schmanu Oct 31, 2023
9c8175d
fix: only disconnect social signer wallet
schmanu Oct 31, 2023
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
254 changes: 254 additions & 0 deletions src/hooks/wallets/__tests__/useOnboard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/SocialLoginModule'
import { faker } from '@faker-js/faker'
import type { EIP1193Provider, OnboardAPI, WalletState } from '@web3-onboard/core'
import { getConnectedWallet, switchWallet } from '../useOnboard'

// mock wallets
jest.mock('@/hooks/wallets/wallets', () => ({
getDefaultWallets: jest.fn(() => []),
getRecommendedInjectedWallets: jest.fn(() => []),
}))

describe('useOnboard', () => {
describe('getConnectedWallet', () => {
it('returns the connected wallet', () => {
const wallets = [
{
label: 'Wallet 1',
icon: 'wallet1.svg',
provider: null as unknown as EIP1193Provider,
chains: [{ id: '0x4' }],
accounts: [
{
address: '0x1234567890123456789012345678901234567890',
ens: null,
balance: null,
},
],
},
{
label: 'Wallet 2',
icon: 'wallet2.svg',
provider: null as unknown as EIP1193Provider,
chains: [{ id: '0x100' }],
accounts: [
{
address: '0x2',
ens: null,
balance: null,
},
],
},
] as WalletState[]

expect(getConnectedWallet(wallets)).toEqual({
label: 'Wallet 1',
icon: 'wallet1.svg',
address: '0x1234567890123456789012345678901234567890',
provider: wallets[0].provider,
chainId: '4',
})
})

it('should return null if the address is invalid', () => {
const wallets = [
{
label: 'Wallet 1',
icon: 'wallet1.svg',
provider: null as unknown as EIP1193Provider,
chains: [{ id: '0x4' }],
accounts: [
{
address: '0xinvalid',
ens: null,
balance: null,
},
],
},
] as WalletState[]

expect(getConnectedWallet(wallets)).toBeNull()
})
})

describe('switchWallet', () => {
it('should keep the social signer wallet connected if switching wallets fails', async () => {
const mockOnboard = {
state: {
get: jest.fn().mockReturnValue({
wallets: [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: ONBOARD_MPC_MODULE_LABEL,
},
],
}),
},
connectWallet: jest.fn().mockRejectedValue('Error'),
disconnectWallet: jest.fn(),
}

await switchWallet(mockOnboard as unknown as OnboardAPI)

expect(mockOnboard.connectWallet).toHaveBeenCalled()
expect(mockOnboard.disconnectWallet).not.toHaveBeenCalled()
})

it('should not disconnect the social signer wallet label if the same label connects', async () => {
const mockNewState = [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: ONBOARD_MPC_MODULE_LABEL,
},
]

const mockOnboard = {
state: {
get: jest.fn().mockReturnValue({
wallets: [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: ONBOARD_MPC_MODULE_LABEL,
},
],
}),
},
connectWallet: jest.fn().mockResolvedValue(mockNewState),
disconnectWallet: jest.fn(),
}

await switchWallet(mockOnboard as unknown as OnboardAPI)

expect(mockOnboard.connectWallet).toHaveBeenCalled()
expect(mockOnboard.disconnectWallet).not.toHaveBeenCalled()
})

it('should not disconnect non social signer wallets if new wallet connects', async () => {
const mockNewState = [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: 'MetaMask',
},
]

const mockOnboard = {
state: {
get: jest.fn().mockReturnValue({
wallets: [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: 'Wallet Connect',
},
],
}),
},
connectWallet: jest.fn().mockResolvedValue(mockNewState),
disconnectWallet: jest.fn(),
}

await switchWallet(mockOnboard as unknown as OnboardAPI)

expect(mockOnboard.connectWallet).toBeCalled()
expect(mockOnboard.disconnectWallet).not.toHaveBeenCalled()
})

it('should disconnect the social signer wallet label if new wallet connects', async () => {
const mockNewState = [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: 'MetaMask',
},
]

const mockOnboard = {
state: {
get: jest.fn().mockReturnValue({
wallets: [
{
accounts: [
{
address: faker.finance.ethereumAddress(),
ens: undefined,
},
],
chains: [
{
id: '5',
},
],
label: ONBOARD_MPC_MODULE_LABEL,
},
],
}),
},
connectWallet: jest.fn().mockResolvedValue(mockNewState),
disconnectWallet: jest.fn(),
}

await switchWallet(mockOnboard as unknown as OnboardAPI)

expect(mockOnboard.connectWallet).toBeCalled()
expect(mockOnboard.disconnectWallet).toHaveBeenCalledWith({ label: ONBOARD_MPC_MODULE_LABEL })
})
})
})
69 changes: 0 additions & 69 deletions src/hooks/wallets/useOnboard.test.ts
View file
Open in desktop

This file was deleted.

16 changes: 14 additions & 2 deletions src/hooks/wallets/useOnboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useInitPairing } from '@/services/pairing/hooks'
import { useAppSelector } from '@/store'
import { type EnvState, selectRpc } from '@/store/settingsSlice'
import { E2E_WALLET_NAME } from '@/tests/e2e-wallet'
import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/SocialLoginModule'

const WALLETCONNECT = 'WalletConnect'

Expand Down Expand Up @@ -130,8 +131,19 @@ export const connectWallet = async (
return wallets
}

export const switchWallet = (onboard: OnboardAPI) => {
connectWallet(onboard)
export const switchWallet = async (onboard: OnboardAPI) => {
const oldWalletLabel = getConnectedWallet(onboard.state.get().wallets)?.label
const newWallets = await connectWallet(onboard)
const newWalletLabel = newWallets ? getConnectedWallet(newWallets)?.label : undefined

// If the wallet actually changed we disconnect the old connected wallet.
if (!newWalletLabel || oldWalletLabel !== ONBOARD_MPC_MODULE_LABEL) {
return
}

if (newWalletLabel !== oldWalletLabel) {
await onboard.disconnectWallet({ label: oldWalletLabel })
Copy link
Member

@katspaugh katspaugh Oct 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

So we disconnect if the old wallet is social login? What if the new wallet is social login and the old one is MM? Will it work fine?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes exactly.
Only the Social signer will be disconnected if switched to another wallet.
We could also remove this behavior once we support every chain. But this is currently causing issues QA found.

}
}

// Disable/enable wallets according to chain
Expand Down
Loading