Skip to content

Commit

Permalink
Refactor: onboard auto connect (#2343)
Browse files Browse the repository at this point in the history
* Fix: re-connecting a locked wallet

* Reconnect WC v2

* Refactor: onboard autoConnect

* e2e wallet

* Rm the connectE2EWallet command

* Tracking + WC v2 peer tracking

* Reset last tracked wallet on disconnect

* Lint

* Minor UI adjustments

* Fix: WC v1 detection
  • Loading branch information
katspaugh authored Aug 4, 2023
1 parent 3410911 commit beae29d
Show file tree
Hide file tree
Showing 19 changed files with 58 additions and 121 deletions.
2 changes: 0 additions & 2 deletions cypress/e2e/add_owner.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ const offset = 7

describe('Adding an owner', () => {
before(() => {
cy.connectE2EWallet()

cy.visit(`/${TEST_SAFE}/settings/setup`)
cy.contains('button', 'Accept selection').click()

Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/create_safe.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
describe('Create Safe', () => {
it('should create a new Safe', () => {
cy.connectE2EWallet()

cy.visit('/welcome')

// Close cookie banner
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/non_owner_spending_limit.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const SPENDING_LIMIT_SAFE = 'gor:0xBE3C5aFF7f66c23fe71c3047911f9Aa0026b281B'

describe('Check non-owner spending limit beneficiary modal', () => {
before(() => {
cy.connectE2EWallet()

cy.visit(`/${SPENDING_LIMIT_SAFE}/home`, { failOnStatusCode: false })

cy.contains('Accept selection').click()
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/smoke/create_safe_simple.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const OWNER_ADDRESS = '0xE297437d6b53890cbf004e401F3acc67c8b39665'

describe('Create Safe form', () => {
it('should navigate to the form', () => {
cy.connectE2EWallet()

cy.visit('/welcome')

// Close cookie banner
Expand Down
1 change: 0 additions & 1 deletion cypress/e2e/smoke/create_tx.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const currentNonce = 3

describe('Queue a transaction on 1/N', () => {
before(() => {
cy.connectE2EWallet()
cy.useProdCGW()

cy.visit(`/home?safe=${SAFE}`)
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/smoke/nfts.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ const TEST_SAFE = 'gor:0x97d314157727D517A706B5D08507A1f9B44AaaE9'

describe('Assets > NFTs', () => {
before(() => {
cy.connectE2EWallet()

cy.visit(`/balances/nfts?safe=${TEST_SAFE}`)
cy.contains('button', 'Accept selection').click()
cy.contains(/E2E Wallet @ G(ö|oe)rli/)
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/smoke/pending_actions.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ const SAFE = 'gor:0xCD4FddB8FfA90012DFE11eD4bf258861204FeEAE'

describe('Pending actions', () => {
before(() => {
cy.connectE2EWallet()
cy.useProdCGW()

cy.visit(`/welcome`)
cy.contains('button', 'Accept selection').click()
})
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/spending_limit.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const SPENDING_LIMIT_SAFE = 'gor:0x28F95E682D1dd632b54Dc61740575f49DB39Eb7F'

describe('Check spending limit modal', () => {
before(() => {
cy.connectE2EWallet()

cy.visit(`/${SPENDING_LIMIT_SAFE}/home`, { failOnStatusCode: false })

cy.contains('Accept selection').click()
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/tx_modal.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ const SAFE_NONCE = '6'

describe('Tx Modal', () => {
before(() => {
cy.connectE2EWallet()

// Open the Safe used for testing
cy.visit(`/${TEST_SAFE}`)
cy.contains('a', 'Accept selection').click()
Expand Down
2 changes: 0 additions & 2 deletions cypress/e2e/tx_simulation.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const RECIPIENT_ADDRESS = '0x6a5602335a878ADDCa4BF63a050E34946B56B5bC'

describe('Tx Simulation', () => {
before(() => {
cy.connectE2EWallet()

// Open the Safe used for testing
cy.visit(`/${TEST_SAFE}/home`, { failOnStatusCode: false })
cy.contains('button', 'Accept selection').click()
Expand Down
7 changes: 0 additions & 7 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
Cypress.Commands.add('connectE2EWallet', () => {
cy.on('window:before:load', (window) => {
// Does not work unless `JSON.stringify` is used
window.localStorage.setItem('SAFE_v2__lastWallet', JSON.stringify('E2E Wallet'))
})
})

Cypress.Commands.add('useProdCGW', () => {
cy.on('window:before:load', (window) => {
window.localStorage.setItem('SAFE_v2__debugProdCgw', JSON.stringify(true))
Expand Down
7 changes: 6 additions & 1 deletion src/components/batch/BatchSidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,12 @@ const BatchSidebar = ({ isOpen, onToggle }: { isOpen: boolean; onToggle: (open:
<Divider />

<Track {...BATCH_EVENTS.BATCH_CONFIRM} label={batchTxs.length}>
<Button variant="contained" onClick={onConfirmClick} disabled={!batchTxs.length}>
<Button
variant="contained"
onClick={onConfirmClick}
disabled={!batchTxs.length}
className={css.confirmButton}
>
Confirm batch
</Button>
</Track>
Expand Down
6 changes: 5 additions & 1 deletion src/components/batch/BatchSidebar/styles.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
}

.txs ul {
padding: 0 var(--space-3) var(--space-3);
padding: 0 var(--space-3) var(--space-2);
display: flex;
flex-direction: column;
gap: var(--space-1);
Expand All @@ -43,6 +43,10 @@
height: calc(100% + 31px);
}

.confirmButton {
margin-top: var(--space-1);
}

.txs svg {
color: var(--color-border-main);
transition: color 0.1s ease-in;
Expand Down
3 changes: 1 addition & 2 deletions src/components/common/ConnectWallet/AccountCenter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import css from '@/components/common/ConnectWallet/styles.module.css'
import EthHashInfo from '@/components/common/EthHashInfo'
import ExpandLessIcon from '@mui/icons-material/ExpandLess'
import ExpandMoreIcon from '@mui/icons-material/ExpandMore'
import useOnboard, { forgetLastWallet, switchWallet } from '@/hooks/wallets/useOnboard'
import useOnboard, { switchWallet } from '@/hooks/wallets/useOnboard'
import { useAppSelector } from '@/store'
import { selectChainById } from '@/store/chainsSlice'
import Identicon from '@/components/common/Identicon'
Expand Down Expand Up @@ -36,7 +36,6 @@ const AccountCenter = ({ wallet }: { wallet: ConnectedWallet }) => {
label: wallet.label,
})

forgetLastWallet()
handleClose()
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/common/NavTabs/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const NextLinkComposed = forwardRef<HTMLAnchorElement, Props>(function NextCompo

const NavTabs = ({ tabs }: { tabs: NavItem[] }) => {
const router = useRouter()
const activeTab = tabs.map((tab) => tab.href).indexOf(router.pathname)
const activeTab = Math.max(0, tabs.map((tab) => tab.href).indexOf(router.pathname))
const query = router.query.safe ? { safe: router.query.safe } : undefined

return (
Expand Down
6 changes: 3 additions & 3 deletions src/components/tx-flow/flows/SuccessScreen/StatusMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ const getStep = (status: PendingStatus, error?: Error) => {
case PendingStatus.PROCESSING:
case PendingStatus.RELAYING:
return {
description: 'Transaction is now processing.',
description: 'Transaction is now processing',
instruction: 'The transaction was confirmed and is now being processed.',
}
case PendingStatus.INDEXING:
return {
description: 'Transaction was processed.',
description: 'Transaction was processed',
instruction: 'It is now being indexed.',
}
default:
return {
description: error ? 'Transaction failed' : 'Transaction was successful.',
description: error ? 'Transaction failed' : 'Transaction was successful',
instruction: error ? error.message : '',
}
}
Expand Down
84 changes: 41 additions & 43 deletions src/hooks/wallets/useOnboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import { type ChainInfo } from '@safe-global/safe-gateway-typescript-sdk'
import { getAddress } from 'ethers/lib/utils'
import useChains, { useCurrentChain } from '@/hooks/useChains'
import ExternalStore from '@/services/ExternalStore'
import { localItem } from '@/services/local-storage/local'
import { logError, Errors } from '@/services/exceptions'
import { trackEvent, WALLET_EVENTS } from '@/services/analytics'
import { useInitPairing } from '@/services/pairing/hooks'
import { isWalletUnlocked, WalletNames } from '@/utils/wallets'
import { useAppSelector } from '@/store'
import { type EnvState, selectRpc } from '@/store/settingsSlice'
import { WALLET_KEYS } from './consts'
import { E2E_WALLET_NAME } from '@/tests/e2e-wallet'

const WALLETCONNECT = 'WalletConnect'

export type ConnectedWallet = {
label: string
Expand All @@ -22,12 +22,6 @@ export type ConnectedWallet = {
icon?: string
}

const lastWalletStorage = localItem<string>('lastWallet')

export const forgetLastWallet = () => {
lastWalletStorage.remove()
}

const { getStore, setStore, useStore } = new ExternalStore<OnboardAPI>()

export const initOnboard = async (
Expand Down Expand Up @@ -67,16 +61,15 @@ export const getConnectedWallet = (wallets: WalletState[]): ConnectedWallet | nu
}
}

const getWalletConnectLabel = async ({ label, provider }: ConnectedWallet): Promise<string | undefined> => {
if (label.toUpperCase() !== WALLET_KEYS.WALLETCONNECT.toUpperCase()) return

const getWalletConnectLabel = async (wallet: ConnectedWallet): Promise<string | undefined> => {
const UNKNOWN_PEER = 'Unknown'
const { default: WalletConnect } = await import('@walletconnect/client')

const peerWallet =
((provider as unknown as any).connector as InstanceType<typeof WalletConnect>).peerMeta?.name || UNKNOWN_PEER

return peerWallet ?? UNKNOWN_PEER
const { label } = wallet
const isWalletConnect = label.startsWith(WALLETCONNECT)
if (!isWalletConnect) return
const { connector } = wallet.provider as unknown as any
const peerWalletV2 = connector.session?.peer?.metadata?.name
const peerWalletV1 = connector.peerMeta?.name
return peerWalletV2 || peerWalletV1 || UNKNOWN_PEER
}

const trackWalletType = (wallet: ConnectedWallet) => {
Expand Down Expand Up @@ -118,7 +111,7 @@ export const connectWallet = async (
// On mobile, automatically choose WalletConnect if there is no injected wallet
if (!options && isMobile() && !hasInjectedWallet()) {
options = {
autoSelect: WalletNames.WALLET_CONNECT_V2,
autoSelect: WALLETCONNECT,
}
}

Expand All @@ -133,17 +126,6 @@ export const connectWallet = async (
return
}

// Save the last used wallet and track the wallet type
const newWallet = getConnectedWallet(wallets)

if (newWallet) {
// Save
lastWalletStorage.set(newWallet.label)

// Track
trackWalletType(newWallet)
}

isConnecting = false

return wallets
Expand All @@ -153,7 +135,7 @@ export const switchWallet = (onboard: OnboardAPI) => {
connectWallet(onboard)
}

// Disable/enable wallets according to chain and cache the last used wallet
// Disable/enable wallets according to chain
export const useInitOnboard = () => {
const { configs } = useChains()
const chain = useCurrentChain()
Expand All @@ -178,21 +160,37 @@ export const useInitOnboard = () => {
onboard.state.actions.setWalletModules(supportedWallets)
}

// Connect to the last connected wallet
enableWallets().then(() => {
if (onboard.state.get().wallets.length > 0) return

const label = lastWalletStorage.get()
if (!label) return

isWalletUnlocked(label).then((isUnlocked) => {
isUnlocked &&
connectWallet(onboard, {
autoSelect: { label, disableModals: false },
})
})
// e2e wallet
if (typeof window !== 'undefined' && window.Cypress) {
connectWallet(onboard, {
autoSelect: { label: E2E_WALLET_NAME, disableModals: true },
})
}
})
}, [chain, onboard])

// Track connected wallet
useEffect(() => {
let lastConnectedWallet = ''
if (!onboard) return

const walletSubscription = onboard.state.select('wallets').subscribe((wallets) => {
const newWallet = getConnectedWallet(wallets)
if (newWallet) {
if (newWallet.label !== lastConnectedWallet) {
lastConnectedWallet = newWallet.label
trackWalletType(newWallet)
}
} else {
lastConnectedWallet = ''
}
})

return () => {
walletSubscription.unsubscribe()
}
}, [onboard])
}

export default useStore
3 changes: 1 addition & 2 deletions src/services/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ export const createOnboard = (

connect: {
removeWhereIsMyWalletWarning: true,
autoConnectLastWallet: true,
},

// TODO: Investigate using `autoConnectLastWallet` instead of our `lastWalletStorage`
})

return onboard
Expand Down
42 changes: 0 additions & 42 deletions src/utils/wallets.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
import { ProviderLabel } from '@web3-onboard/injected-wallets'
import { hasValidPairingSession } from '@/services/pairing/utils'
import { PAIRING_MODULE_LABEL } from '@/services/pairing/module'
import { E2E_WALLET_NAME } from '@/tests/e2e-wallet'
import type { EthersError } from '@/utils/ethers-utils'
import { ErrorCode } from '@ethersproject/logger'
import { type ConnectedWallet } from '@/hooks/wallets/useOnboard'
import { getWeb3ReadOnly, isSmartContract } from '@/hooks/wallets/web3'
import { WALLET_KEYS } from '@/hooks/wallets/consts'
import { WALLET_CONNECT_V1_MODULE_NAME } from '@/hooks/wallets/wallets'

const isWCRejection = (err: Error): boolean => {
return /rejected/.test(err?.message)
Expand All @@ -21,43 +16,6 @@ export const isWalletRejection = (err: EthersError | Error): boolean => {
return isEthersRejection(err as EthersError) || isWCRejection(err)
}

export const WalletNames = {
METAMASK: ProviderLabel.MetaMask,
WALLET_CONNECT: WALLET_CONNECT_V1_MODULE_NAME,
WALLET_CONNECT_V2: 'WalletConnect',
SAFE_MOBILE_PAIRING: PAIRING_MODULE_LABEL,
}

/* Check if the wallet is unlocked. */
export const isWalletUnlocked = async (walletName: string): Promise<boolean> => {
if (typeof window === 'undefined') return false

if (window.ethereum?.isConnected?.()) {
return true
}

// Only MetaMask exposes a method to check if the wallet is unlocked
if (walletName === WalletNames.METAMASK) {
return window.ethereum?._metamask?.isUnlocked?.() || false
}

// Wallet connect creates a localStorage entry when connected and removes it when disconnected
if (walletName === WalletNames.WALLET_CONNECT) {
return window.localStorage.getItem('walletconnect') !== null
}

// Our own Safe mobile pairing module
if (walletName === WalletNames.SAFE_MOBILE_PAIRING && hasValidPairingSession()) {
return hasValidPairingSession()
}

if (walletName === E2E_WALLET_NAME) {
return Boolean(window.Cypress)
}

return false
}

export const isHardwareWallet = (wallet: ConnectedWallet): boolean => {
return [WALLET_KEYS.LEDGER, WALLET_KEYS.TREZOR, WALLET_KEYS.KEYSTONE].includes(
wallet.label.toUpperCase() as WALLET_KEYS,
Expand Down

0 comments on commit beae29d

Please sign in to comment.