Skip to content

Commit

Permalink
Fix: adjust WalletConnect events (#2717)
Browse files Browse the repository at this point in the history
* Fix: adjust WalletConnect events

* Cut off error details

* Don't swallow chain switch errors

* Fix nested p warning

* Fix tests

* Split errors

* Style wc error

* Connect/disconnect timer to 1s

* Fix: safe logo for WC

* Show unsupported chain error

* Fix: WC url for the origin field

* Redirect WC Safe App to open popup

* Fix error overflow

* Fix tests

* Fix: pass app id to offchain message flow

* A better wrong chain message
  • Loading branch information
katspaugh authored Nov 1, 2023
1 parent 236c541 commit 5d7e2dc
Show file tree
Hide file tree
Showing 26 changed files with 174 additions and 104 deletions.
1 change: 0 additions & 1 deletion src/components/safe-apps/AppFrame/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ const AppFrame = ({ appUrl, allowedFeaturesList, safeAppFromManifest }: AppFrame
<SignMessageOnChainFlow
props={{
app: safeAppFromManifest,
appId: remoteApp?.id,
requestId,
message,
method,
Expand Down
2 changes: 1 addition & 1 deletion src/components/tx-flow/flows/SignMessage/SignMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const MessageHashField = ({ label, hashValue }: { label: string; hashValue: stri
<Typography variant="body2" fontWeight={700} mt={2}>
{label}:
</Typography>
<Typography variant="body2">
<Typography variant="body2" component="div">
<EthHashInfo address={hashValue} showAvatar={false} shortAddress={false} showCopyButton />
</Typography>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ describe('ReviewSignMessageOnChain', () => {
type: SafeAppAccessPolicyTypes.NoRestrictions,
},
}}
appId={73}
requestId="73"
message={{
types: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { SafeTxContext } from '@/components/tx-flow/SafeTxProvider'
import { asError } from '@/services/exceptions/utils'

export type SignMessageOnChainProps = {
appId?: number
app?: SafeAppData
requestId: RequestId
message: string | EIP712TypedData
Expand Down
10 changes: 7 additions & 3 deletions src/components/walletconnect/WcConnectionForm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import WcInput from '../WcInput'
import WcLogoHeader from '../WcLogoHeader'
import css from './styles.module.css'
import useSafeInfo from '@/hooks/useSafeInfo'
import Track from '@/components/common/Track'
import { WALLETCONNECT_EVENTS } from '@/services/analytics/events/walletconnect'

const WC_HINTS_KEY = 'wcHints'

Expand Down Expand Up @@ -44,9 +46,11 @@ export const WcConnectionForm = ({
className={css.infoIcon}
>
<span>
<IconButton onClick={onToggle}>
<SvgIcon component={InfoIcon} inheritViewBox color="border" />
</IconButton>
<Track {...(showHints ? WALLETCONNECT_EVENTS.HINTS_HIDE : WALLETCONNECT_EVENTS.HINTS_SHOW)}>
<IconButton onClick={onToggle}>
<SvgIcon component={InfoIcon} inheritViewBox color="border" />
</IconButton>
</Track>
</span>
</Tooltip>

Expand Down
14 changes: 10 additions & 4 deletions src/components/walletconnect/WcErrorMessage/index.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
import { splitError } from '@/services/walletconnect/utils'
import { Button, Typography } from '@mui/material'
import WcLogoHeader from '../WcLogoHeader'
import css from './styles.module.css'

const WcErrorMessage = ({ error, onClose }: { error: Error; onClose: () => void }) => {
const message = error.message || 'An error occurred'
const [summary, details] = splitError(message)

return (
<div className={css.errorContainer}>
<WcLogoHeader error />
<WcLogoHeader errorMessage={summary} />

<Typography title={error.message} className={css.errorMessage} mb={3}>
{error.message || 'An error occurred'}
</Typography>
{details && (
<Typography mt={1} className={css.details}>
{details}
</Typography>
)}

<Button variant="contained" onClick={onClose} className={css.button}>
OK
Expand Down
14 changes: 8 additions & 6 deletions src/components/walletconnect/WcErrorMessage/styles.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
text-align: center;
}

.errorMessage {
overflow: hidden;
text-overflow: ellipsis;
width: 100%;
}

.button {
padding: var(--space-1) var(--space-4);
margin-top: var(--space-3);
}

.details {
width: 100%;
overflow: hidden;
text-overflow: ellipsis;
hyphens: auto;
}
6 changes: 1 addition & 5 deletions src/components/walletconnect/WcHints/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
ListItemAvatar,
ListItemText,
} from '@mui/material'
import { useEffect, useState } from 'react'
import { useState } from 'react'
import type { ReactElement } from 'react'
import Question from '@/public/images/common/question.svg'
import css from './styles.module.css'
Expand Down Expand Up @@ -84,10 +84,6 @@ const WcHints = (): ReactElement => {
trackEvent(WALLETCONNECT_EVENTS.HINTS_EXPAND)
}

useEffect(() => {
trackEvent(WALLETCONNECT_EVENTS.HINTS_SHOW)
}, [])

return (
<Box display="flex" flexDirection="column" gap={1}>
<HintAccordion
Expand Down
8 changes: 4 additions & 4 deletions src/components/walletconnect/WcLogoHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import WalletConnect from '@/public/images/common/walletconnect.svg'
import Alert from '@/public/images/notifications/alert.svg'
import css from './styles.module.css'

const WcLogoHeader = ({ error }: { error?: boolean }): ReactElement => {
const WcLogoHeader = ({ errorMessage }: { errorMessage?: string }): ReactElement => {
return (
<>
<div>
<SvgIcon component={WalletConnect} inheritViewBox className={css.icon} />
{error && <SvgIcon component={Alert} inheritViewBox className={css.errorBadge} fontSize="small" />}
{errorMessage && <SvgIcon component={Alert} inheritViewBox className={css.errorBadge} fontSize="small" />}
</div>

<Typography variant="h5" mt={2} mb={0.5}>
Connect dApps to {`Safe{Wallet}`}
<Typography variant="h5" mt={2} mb={0.5} className={css.title}>
{errorMessage || 'Connect dApps to Safe{Wallet}'}
</Typography>
</>
)
Expand Down
6 changes: 6 additions & 0 deletions src/components/walletconnect/WcLogoHeader/styles.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@
border-radius: 50%;
border: 1px solid var(--color-background-paper);
}

.title {
width: 100%;
overflow: hidden;
text-overflow: ellipsis;
}
10 changes: 5 additions & 5 deletions src/components/walletconnect/WcSessionList/WcNoSessions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ const WcSampleDapps = ({ onUnload }: { onUnload: () => void }) => {
return (
<Typography variant="body2" display="flex" justifyContent="space-between" alignItems="center" mt={3}>
{SAMPLE_DAPPS.map((item) => (
<ExternalLink href={item.url} key={item.url} noIcon px={1}>
<img src={item.icon} alt={item.name} width={32} height={32} />
<Typography variant="body2" ml={1}>
<Typography variant="body2" ml={1} key={item.url}>
<ExternalLink href={item.url} noIcon px={1}>
<img src={item.icon} alt={item.name} width={32} height={32} />
{item.name}
</Typography>
</ExternalLink>
</ExternalLink>
</Typography>
))}
</Typography>
)
Expand Down
7 changes: 6 additions & 1 deletion src/components/walletconnect/WcSessionList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ type WcSesstionListProps = {
}

const WcSessionListItem = ({ session, onDisconnect }: { session: SessionTypes.Struct; onDisconnect: () => void }) => {
const MAX_NAME_LENGTH = 23
const { safeLoaded } = useSafeInfo()
const name = getPeerName(session.peer) || 'Unknown dApp'
let name = getPeerName(session.peer) || 'Unknown dApp'

if (name.length > MAX_NAME_LENGTH + 1) {
name = `${name.slice(0, MAX_NAME_LENGTH)}…`
}

return (
<ListItem className={css.sessionListItem}>
Expand Down
13 changes: 5 additions & 8 deletions src/components/walletconnect/WcSessionMananger/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import WcProposalForm from '../WcProposalForm'
import WcConnectionState from '../WcConnectionState'
import { trackEvent } from '@/services/analytics'
import { WALLETCONNECT_EVENTS } from '@/services/analytics/events/walletconnect'
import { splitError } from '@/services/walletconnect/utils'

type WcSessionManagerProps = {
sessions: SessionTypes.Struct[]
uri: string
}

const SESSION_INFO_TIMEOUT = 2000
const SESSION_INFO_TIMEOUT = 1000

const WcSessionManager = ({ sessions, uri }: WcSessionManagerProps) => {
const { safe, safeAddress } = useSafeInfo()
Expand Down Expand Up @@ -107,12 +108,8 @@ const WcSessionManager = ({ sessions, uri }: WcSessionManagerProps) => {

setOpen(true)

let timer = setTimeout(() => {
setOpen(false)

timer = setTimeout(() => {
setChangedSession(undefined)
}, 500)
const timer = setTimeout(() => {
setChangedSession(undefined)
}, SESSION_INFO_TIMEOUT)

return () => clearTimeout(timer)
Expand All @@ -121,7 +118,7 @@ const WcSessionManager = ({ sessions, uri }: WcSessionManagerProps) => {
// Track errors
useEffect(() => {
if (error && open) {
trackEvent({ ...WALLETCONNECT_EVENTS.SHOW_ERROR, label: error.message })
trackEvent({ ...WALLETCONNECT_EVENTS.SHOW_ERROR, label: splitError(error.message || '')[0] })
}
}, [error, open])

Expand Down
14 changes: 13 additions & 1 deletion src/pages/apps/open.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { NextPage } from 'next'
import { useRouter } from 'next/router'
import { useCallback } from 'react'
import { useCallback, useContext } from 'react'
import { Box, CircularProgress } from '@mui/material'

import { useSafeAppUrl } from '@/hooks/safe-apps/useSafeAppUrl'
Expand All @@ -15,6 +15,10 @@ import { useBrowserPermissions } from '@/hooks/safe-apps/permissions'
import useChainId from '@/hooks/useChainId'
import { AppRoutes } from '@/config/routes'
import { getOrigin } from '@/components/safe-apps/utils'
import { WalletConnectContext } from '@/services/walletconnect/WalletConnectContext'

// TODO: Remove this once we properly deprecate the WC app
const WC_SAFE_APP = /wallet-connect/

const SafeApps: NextPage = () => {
const chainId = useChainId()
Expand Down Expand Up @@ -42,6 +46,8 @@ const SafeApps: NextPage = () => {
remoteSafeAppsLoading,
})

const { setOpen } = useContext(WalletConnectContext)

const goToList = useCallback(() => {
router.push({
pathname: AppRoutes.apps.index,
Expand All @@ -52,6 +58,12 @@ const SafeApps: NextPage = () => {
// appUrl is required to be present
if (!appUrl || !router.isReady) return null

if (WC_SAFE_APP.test(appUrl)) {
setOpen(true)
goToList()
return null
}

if (isModalVisible) {
return (
<SafeAppsInfoModal
Expand Down
9 changes: 9 additions & 0 deletions src/services/analytics/events/walletconnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export const WALLETCONNECT_EVENTS = {
action: 'WC show hints',
category: WALLETCONNECT_CATEGORY,
},
HINTS_HIDE: {
action: 'WC hide hints',
category: WALLETCONNECT_CATEGORY,
},
HINTS_EXPAND: {
action: 'WC expand hints',
category: WALLETCONNECT_CATEGORY,
Expand Down Expand Up @@ -59,4 +63,9 @@ export const WALLETCONNECT_EVENTS = {
action: 'WC switch from unsupported chain',
category: WALLETCONNECT_CATEGORY,
},
REQUEST: {
action: 'WC request',
category: WALLETCONNECT_CATEGORY,
event: EventType.META,
},
}
2 changes: 0 additions & 2 deletions src/services/exceptions/ErrorCodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ enum ErrorCodes {
_902 = '902: Error loading Safe Apps list',
_903 = '903: Error loading Safe App manifest',
_905 = '905: Third party cookies are disabled',

_910 = '910: WalletConnect failed to switch chain',
}

export default ErrorCodes
2 changes: 1 addition & 1 deletion src/services/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const createOnboard = (
appMetadata: {
name: 'Safe{Wallet}',
// Both heights need be set to correctly size the image in the connecting screen/modal
icon: '<svg height="100%"><image href="/images/safe-logo-green.png" height="100%" /></svg>',
icon: 'https://app.safe.global/images/safe-logo-green.png',
description: 'Please select a wallet to connect to Safe{Wallet}',
recommendedInjectedWallets: getRecommendedInjectedWallets(),
},
Expand Down
1 change: 1 addition & 0 deletions src/services/safe-wallet-provider/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const safe = {
}

const appInfo = {
id: 1,
name: 'test',
description: 'test',
iconUrl: 'test',
Expand Down
1 change: 1 addition & 0 deletions src/services/safe-wallet-provider/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ type SafeSettings = {
}

export type AppInfo = {
id: number
name: string
description: string
url: string
Expand Down
10 changes: 4 additions & 6 deletions src/services/safe-wallet-provider/useSafeWalletProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import * as messages from '@/utils/safe-messages'
import { createStoreHydrator } from '@/store/storeHydrator'

const appInfo = {
id: 1,
name: 'test',
description: 'test',
iconUrl: 'test',
Expand Down Expand Up @@ -90,6 +91,7 @@ describe('useSafeWalletProvider', () => {
name: appInfo.name,
message: 'message',
requestId: expect.any(String),
safeAppId: 1,
})

expect(resp).toBeInstanceOf(Promise)
Expand Down Expand Up @@ -131,7 +133,6 @@ describe('useSafeWalletProvider', () => {
// SignMessageOnChainFlow props
expect(mockSetTxFlow.mock.calls[0][0].props).toStrictEqual({
props: {
appId: undefined,
requestId: expect.any(String),
message: 'message',
method: 'signMessage',
Expand Down Expand Up @@ -205,6 +206,7 @@ describe('useSafeWalletProvider', () => {
name: appInfo.name,
message: typedMessage,
requestId: expect.any(String),
safeAppId: 1,
})

expect(resp).toBeInstanceOf(Promise)
Expand Down Expand Up @@ -252,11 +254,7 @@ describe('useSafeWalletProvider', () => {
expect(mockSetTxFlow.mock.calls[0][0].props).toStrictEqual({
data: {
appId: undefined,
app: {
name: appInfo.name,
url: appInfo.url,
iconUrl: appInfo.iconUrl,
},
app: appInfo,
requestId: expect.any(String),
txs: [
{
Expand Down
Loading

0 comments on commit 5d7e2dc

Please sign in to comment.