Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

fix: better network comparison + WC peer tracking #3729

Merged
merged 4 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/components/ConnectButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const onConnectButtonClick = async (): Promise<void> => {
}

const ConnectButton = (props: { 'data-testid': string }): ReactElement => (
<Track {...OVERVIEW_EVENTS.ONBOARD}>
<Track {...OVERVIEW_EVENTS.OPEN_ONBOARD}>
Copy link
Member Author

Choose a reason for hiding this comment

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

This was not the correct event key.

<Button color="primary" minWidth={240} onClick={onConnectButtonClick} variant="contained" {...props}>
Connect
</Button>
Expand Down
76 changes: 57 additions & 19 deletions src/logic/wallets/__tests__/network.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,32 +77,70 @@ describe('src/logic/wallets/utils/network', () => {
})
})
describe('shouldSwitchNetwork', () => {
it('should return true when networks mismatch', () => {
const wallet = {
provider: {
networkVersion: '4',
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(true)
})

it('should return false when wallet is not connected', () => {
const wallet = {
provider: undefined,
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(false)
})

it('should return false when networks are the same', () => {
const wallet = {
provider: {
networkVersion: '1',
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(false)
describe('should return true when networks mismatch', () => {
usame-algan marked this conversation as resolved.
Show resolved Hide resolved
it('for numeric `chainId`s', () => {
const wallet = {
provider: {
networkVersion: 4,
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(true)
})
it('for strict hex `chainId`s', () => {
const wallet = {
provider: {
networkVersion: '0x2',
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(true)
})
it('for string `chainId`s', () => {
const wallet = {
provider: {
networkVersion: '4',
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(true)
})
})
describe('should return false when networks are the same', () => {
it('for numeric `chainIds`', () => {
const wallet = {
provider: {
networkVersion: 1,
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(false)
})
it('for strict hex `chainId`s', () => {
const wallet = {
provider: {
networkVersion: '0x1',
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(false)
})
it('for string `chainId`s', () => {
const wallet = {
provider: {
networkVersion: '1',
},
}

expect(shouldSwitchNetwork(wallet as Wallet)).toBe(false)
})
})
})
})
7 changes: 4 additions & 3 deletions src/logic/wallets/patchedWalletConnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ export const getRpcMap = (): IRPCMap => {
}, {})
}

export const WALLET_CONNECT_MODULE_NAME = 'WalletConnect'
const patchedWalletConnect = (chainId: ChainId): WalletModule => {
return {
name: 'WalletConnect',
name: WALLET_CONNECT_MODULE_NAME,
svg: walletConnectIcon,
wallet: async ({ resetWalletState }: Helpers) => {
const provider = new WalletConnectProvider({
Expand All @@ -56,13 +57,13 @@ const patchedWalletConnect = (chainId: ChainId): WalletModule => {
provider.autoRefreshOnNetworkChange = false

provider.wc.on('disconnect', () => {
resetWalletState({ disconnected: true, walletName: 'WalletConnect' })
resetWalletState({ disconnected: true, walletName: WALLET_CONNECT_MODULE_NAME })
})

return {
provider,
interface: {
name: 'WalletConnect',
name: WALLET_CONNECT_MODULE_NAME,
connect: () =>
new Promise((resolve, reject) => {
provider
Expand Down
65 changes: 36 additions & 29 deletions src/logic/wallets/store/middleware/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Dispatch } from 'redux'
import { Action } from 'redux-actions'
import WalletConnectProvider from '@walletconnect/web3-provider'

import { store as reduxStore } from 'src/store'
import { enhanceSnackbarForAction, NOTIFICATIONS } from 'src/logic/notifications'
Expand All @@ -10,48 +11,45 @@ import { providerSelector } from '../selectors'
import { trackEvent } from 'src/utils/googleTagManager'
import { WALLET_EVENTS } from 'src/utils/events/wallet'
import { instantiateSafeContracts } from 'src/logic/contracts/safeContracts'
import { resetWeb3, setWeb3 } from '../../getWeb3'
import onboard, { removeLastUsedProvider, saveLastUsedProvider } from '../../onboard'
import { resetWeb3, setWeb3 } from 'src/logic/wallets/getWeb3'
import onboard, { removeLastUsedProvider, saveLastUsedProvider } from 'src/logic/wallets/onboard'
import { WALLET_CONNECT_MODULE_NAME } from 'src/logic/wallets/patchedWalletConnect'
import { checksumAddress } from 'src/utils/checksumAddress'
import { shouldSwitchNetwork } from 'src/logic/wallets/utils/network'

let hasWalletName = false
let hasAccount = false
let hasNetwork = false
const UNKNOWN_PEER = 'Unknown'

const providerMiddleware =
(store: ReturnType<typeof reduxStore>) =>
(next: Dispatch) =>
async (action: Action<ProviderPayloads>): Promise<Action<ProviderPayloads>> => {
const handledAction = next(action)

const { type, payload } = action

// Onboard sends provider details via separate subscriptions: wallet, account, network
// Payloads from all three need to be combined to be `loaded` and `available`
if (type === PROVIDER_ACTIONS.WALLET_NAME) {
hasWalletName = !!payload
} else if (type === PROVIDER_ACTIONS.ACCOUNT) {
hasAccount = !!payload
} else if (type === PROVIDER_ACTIONS.NETWORK) {
hasNetwork = !!payload
} else {
// Dispatched actions from reducers unrelated to wallet
const isProviderAction = [
PROVIDER_ACTIONS.WALLET_NAME,
PROVIDER_ACTIONS.ACCOUNT,
PROVIDER_ACTIONS.NETWORK,
].includes(action.type as PROVIDER_ACTIONS)

// Prevent other dispatches from triggering this middleware
if (!isProviderAction) {
return handledAction
}

// No wallet is connected via onboard
if (!hasWalletName && !hasAccount && !hasNetwork) {
const state = store.getState()
const { name, account, network, loaded, available } = providerSelector(state)

// No wallet is connected via onboard, reset provider
if (!name && !account && !network) {
resetWeb3()
removeLastUsedProvider()
}

// Wallet 'partially' connected: only a subset of onboard subscription(s) have fired
if (!hasWalletName || !hasAccount || !hasNetwork) {
if (!name || !account || !network) {
return handledAction
}

const state = store.getState()
const { available, loaded, name, account } = providerSelector(state)

// @TODO: `loaded` flag that is/was always set to true - should be moved to wallet connection catch
// Wallet, account and network did not successfully load
if (!loaded) {
Expand All @@ -64,18 +62,27 @@ const providerMiddleware =
return handledAction
}

const { wallet, address } = onboard().getState()

if (name === wallet.name) {
saveLastUsedProvider(name)
}

// Instantiate web3/contract
const { wallet } = onboard().getState()
if (wallet.provider) {
setWeb3(wallet.provider)
instantiateSafeContracts()
}

if (name) {
saveLastUsedProvider(name)
// Only track when account has been successfully saved to store
if (payload === account) {
trackEvent({ ...WALLET_EVENTS.CONNECT, label: name })
// Only track when store/UI is in sync with onboard
if (account === checksumAddress(address) && !shouldSwitchNetwork(wallet)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

The address retrieved from onboard's state is not checksummed.

trackEvent({ ...WALLET_EVENTS.CONNECT, label: name })
// Track WalletConnect peer wallet
if (name === WALLET_CONNECT_MODULE_NAME) {
trackEvent({
...WALLET_EVENTS.WALLET_CONNECT,
label: (wallet.provider as InstanceType<typeof WalletConnectProvider>)?.wc?.peerMeta?.name || UNKNOWN_PEER,
})
}
}

Expand Down
22 changes: 17 additions & 5 deletions src/logic/wallets/utils/network.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Wallet } from 'bnc-onboard/dist/src/interfaces'
import onboard from 'src/logic/wallets/onboard'
import { numberToHex } from 'web3-utils'
import { hexToNumberString, isHexStrict, numberToHex } from 'web3-utils'

import { getChainInfo, getExplorerUrl, getPublicRpcUrl, _getChainId } from 'src/config'
import { ChainId } from 'src/config/chain.d'
Expand Down Expand Up @@ -85,12 +85,24 @@ export const switchNetwork = async (wallet: Wallet, chainId: ChainId): Promise<v
}

export const shouldSwitchNetwork = (wallet: Wallet): boolean => {
if (!wallet.provider) {
return false
}

// The current network can be stored under one of two keys
const isCurrentNetwork = [wallet?.provider?.networkVersion, wallet?.provider?.chainId].some(
(chainId) => chainId && chainId.toString() !== _getChainId(),
)
const isCurrentNetwork = [wallet?.provider?.networkVersion, wallet?.provider?.chainId].some((chainId) => {
const _chainId = _getChainId()

if (typeof chainId === 'number') {
return chainId.toString() === _chainId
}
if (typeof chainId === 'string') {
return isHexStrict(chainId) ? hexToNumberString(chainId) === _chainId : chainId === _chainId
}
return false
})

return isCurrentNetwork
return !isCurrentNetwork
}

export const switchWalletChain = async (): Promise<void> => {
Expand Down
4 changes: 4 additions & 0 deletions src/utils/events/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ const WALLET = {
event: GTM_EVENT.META,
action: 'Connect wallet',
},
WALLET_CONNECT: {
event: GTM_EVENT.META,
action: 'WalletConnect Peer',
},
}

const WALLET_CATEGORY = 'wallet'
Expand Down