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.

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

feat: block/warn about bridges accordingly #2652

Merged
merged 2 commits into from
Oct 18, 2023
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
64 changes: 0 additions & 64 deletions src/components/walletconnect/ProposalForm/ChainWarning.tsx

This file was deleted.

43 changes: 43 additions & 0 deletions src/components/walletconnect/ProposalForm/CompatibilityWarning.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { Alert, Typography } from '@mui/material'
import type { ReactElement } from 'react'
import type { Web3WalletTypes } from '@walletconnect/web3wallet'

import ChainIndicator from '@/components/common/ChainIndicator'
import { useCompatibilityWarning } from './useCompatibilityWarning'
import useSafeInfo from '@/hooks/useSafeInfo'

import css from './styles.module.css'

export const CompatibilityWarning = ({
proposal,
chainIds,
}: {
proposal: Web3WalletTypes.SessionProposal
chainIds: Array<string>
}): ReactElement => {
const { safe } = useSafeInfo()
const isUnsupportedChain = !chainIds.includes(safe.chainId)
const { severity, message } = useCompatibilityWarning(proposal, isUnsupportedChain)

return (
<>
<Alert severity={severity} className={css.alert}>
{message}
</Alert>

{isUnsupportedChain && (
<>
<Typography mt={3} mb={1}>
Supported networks
</Typography>

<div>
{chainIds.map((chainId) => (
<ChainIndicator inline chainId={chainId} key={chainId} className={css.chain} />
))}
</div>
</>
)}
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { renderHook } from '@/tests/test-utils'
import type { ChainInfo, SafeInfo } from '@safe-global/safe-gateway-typescript-sdk'
import type { Web3WalletTypes } from '@walletconnect/web3wallet'

import * as bridges from '../bridges'
import { useCompatibilityWarning } from '../useCompatibilityWarning'

describe('useCompatibilityWarning', () => {
it('should return an error for a dangerous bridge', () => {
jest.spyOn(bridges, 'isStrictAddressBridge').mockReturnValue(true)

const proposal = {
params: { proposer: { metadata: { name: 'Fake Bridge' } } },
verifyContext: { verified: { origin: '' } },
} as unknown as Web3WalletTypes.SessionProposal

const { result } = renderHook(() => useCompatibilityWarning(proposal, false))

expect(result.current).toEqual({
message:
'Fake Bridge is a bridge that is unusable in Safe{Wallet} due to the current implementation of WalletConnect — the bridged funds will be lost. Consider using a different bridge.',
severity: 'error',
})
})

it('should return a warning for a risky bridge', () => {
jest.spyOn(bridges, 'isStrictAddressBridge').mockReturnValue(false)
jest.spyOn(bridges, 'isDefaultAddressBridge').mockReturnValue(true)

const proposal = {
params: { proposer: { metadata: { name: 'Fake Bridge' } } },
verifyContext: { verified: { origin: '' } },
} as unknown as Web3WalletTypes.SessionProposal

const { result } = renderHook(() => useCompatibilityWarning(proposal, false))

expect(result.current).toEqual({
message:
'While using Fake Bridge, please make sure that the desination address you send funds to matches the Safe address you have on the respective chain. Otherwise, the funds will be lost.',
severity: 'warning',
})
})

it('should return an error for an unsupported chain', () => {
jest.spyOn(bridges, 'isStrictAddressBridge').mockReturnValue(false)
jest.spyOn(bridges, 'isDefaultAddressBridge').mockReturnValue(false)

const proposal = {
params: { proposer: { metadata: { name: 'Fake dApp' } } },
verifyContext: { verified: { origin: '' } },
} as unknown as Web3WalletTypes.SessionProposal

const { result } = renderHook(() => useCompatibilityWarning(proposal, true))

expect(result.current).toEqual({
message:
'Fake dApp does not support the Safe Account network. If you want to interact with Fake dApp, please switch to a Safe Account on a supported network.',
severity: 'error',
})
})

describe('should otherwise return info', () => {
it('if chains are loaded', () => {
jest.spyOn(bridges, 'isStrictAddressBridge').mockReturnValue(false)
jest.spyOn(bridges, 'isDefaultAddressBridge').mockReturnValue(false)

const proposal = {
params: { proposer: { metadata: { name: 'Fake dApp' } } },
verifyContext: { verified: { origin: '' } },
} as unknown as Web3WalletTypes.SessionProposal

const { result } = renderHook(() => useCompatibilityWarning(proposal, false), {
initialReduxState: {
chains: {
loading: false,
error: undefined,
data: [
{
chainId: '1',
chainName: 'Ethereum',
},
] as unknown as Array<ChainInfo>,
},
safeInfo: {
loading: false,
error: undefined,
data: {
address: {},
chainId: '1',
} as unknown as SafeInfo,
},
},
})

expect(result.current).toEqual({
message: 'Please make sure that the dApp is connected to Ethereum.',
severity: 'info',
})
})

it("if chains aren't loaded", () => {
jest.spyOn(bridges, 'isStrictAddressBridge').mockReturnValue(false)
jest.spyOn(bridges, 'isDefaultAddressBridge').mockReturnValue(false)

const proposal = {
params: { proposer: { metadata: { name: 'Fake dApp' } } },
verifyContext: { verified: { origin: '' } },
} as unknown as Web3WalletTypes.SessionProposal

const { result } = renderHook(() => useCompatibilityWarning(proposal, false))

expect(result.current).toEqual({
message: 'Please make sure that the dApp is connected to this network.',
severity: 'info',
})
})
})
})
11 changes: 11 additions & 0 deletions src/components/walletconnect/ProposalForm/bridges.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { StrictAddressBridges, DefaultAddressBridges } from '../constants'

// Bridge enforces the same address on destination chain
export const isStrictAddressBridge = (origin: string) => {
return StrictAddressBridges.some((bridge) => origin.includes(bridge))
}

// Bridge defaults to same address on destination chain but allows changing it
export const isDefaultAddressBridge = (origin: string) => {
return DefaultAddressBridges.some((bridge) => origin.includes(bridge))
}
17 changes: 9 additions & 8 deletions src/components/walletconnect/ProposalForm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import type { Web3WalletTypes } from '@walletconnect/web3wallet'
import SafeAppIconCard from '@/components/safe-apps/SafeAppIconCard'
import css from './styles.module.css'
import ProposalVerification from './ProposalVerification'
import { ChainWarning } from './ChainWarning'
import { CompatibilityWarning } from './CompatibilityWarning'
import useChains from '@/hooks/useChains'
import useSafeInfo from '@/hooks/useSafeInfo'
import { getSupportedChainIds } from '@/services/walletconnect/utils'
import { isStrictAddressBridge, isDefaultAddressBridge } from './bridges'

type ProposalFormProps = {
proposal: Web3WalletTypes.SessionProposal
Expand All @@ -22,13 +23,13 @@ const ProposalForm = ({ proposal, onApprove, onReject }: ProposalFormProps): Rea
const { safe } = useSafeInfo()
const [understandsRisk, setUnderstandsRisk] = useState(false)
const { proposer } = proposal.params
const { isScam } = proposal.verifyContext.verified
const { isScam, origin } = proposal.verifyContext.verified

const chainIds = useMemo(() => getSupportedChainIds(configs, proposal.params), [configs, proposal.params])
const supportsCurrentChain = chainIds.includes(safe.chainId)
const isUnsupportedChain = !chainIds.includes(safe.chainId)

const isHighRisk = proposal.verifyContext.verified.validation === 'INVALID'
const disabled = !supportsCurrentChain || isScam || (isHighRisk && !understandsRisk)
const isHighRisk = proposal.verifyContext.verified.validation === 'INVALID' || isDefaultAddressBridge(origin)
const disabled = isUnsupportedChain || isScam || isStrictAddressBridge(origin) || (isHighRisk && !understandsRisk)

return (
<div className={css.container}>
Expand All @@ -54,14 +55,14 @@ const ProposalForm = ({ proposal, onApprove, onReject }: ProposalFormProps): Rea
<div className={css.info}>
<ProposalVerification proposal={proposal} />

<ChainWarning proposal={proposal} chainIds={chainIds} />
<CompatibilityWarning proposal={proposal} chainIds={chainIds} />
</div>

{supportsCurrentChain && isHighRisk && (
{!isUnsupportedChain && isHighRisk && (
<FormControlLabel
className={css.checkbox}
control={<Checkbox checked={understandsRisk} onChange={(_, checked) => setUnderstandsRisk(checked)} />}
label="I understand the risks of interacting with this dApp and would like to continue."
label="I understand the risks associated with interacting with this dApp and would like to continue."
/>
)}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { useMemo } from 'react'
import type { AlertColor } from '@mui/material'
import type { Web3WalletTypes } from '@walletconnect/web3wallet'

import useChains from '@/hooks/useChains'
import useSafeInfo from '@/hooks/useSafeInfo'
import { isStrictAddressBridge, isDefaultAddressBridge } from './bridges'

const NAME_PLACEHOLDER = '%%name%%'
const CHAIN_PLACEHOLDER = '%%chain%%'

const Warnings: Record<string, { severity: AlertColor; message: string }> = {
BLOCKED_BRIDGE: {
severity: 'error',
message: `${NAME_PLACEHOLDER} is a bridge that is unusable in Safe{Wallet} due to the current implementation of WalletConnect — the bridged funds will be lost. Consider using a different bridge.`,
},
WARNED_BRIDGE: {
severity: 'warning',
message: `While using ${NAME_PLACEHOLDER}, please make sure that the desination address you send funds to matches the Safe address you have on the respective chain. Otherwise, the funds will be lost.`,
},
UNSUPPORTED_CHAIN: {
severity: 'error',
message: `${NAME_PLACEHOLDER} does not support the Safe Account network. If you want to interact with ${NAME_PLACEHOLDER}, please switch to a Safe Account on a supported network.`,
},
WRONG_CHAIN: {
severity: 'info',
message: `Please make sure that the dApp is connected to ${CHAIN_PLACEHOLDER}.`,
},
}

export const useCompatibilityWarning = (
proposal: Web3WalletTypes.SessionProposal,
isUnsupportedChain: boolean,
): (typeof Warnings)[string] => {
const { configs } = useChains()
const { safe } = useSafeInfo()

return useMemo(() => {
const { origin } = proposal.verifyContext.verified
const { proposer } = proposal.params

let { message, severity } = isStrictAddressBridge(origin)
? Warnings.DANGEROUS_BRIDGE
: isDefaultAddressBridge(origin)
? Warnings.RISKY_BRIDGE
: isUnsupportedChain
? Warnings.UNSUPPORTED_CHAIN
: Warnings.WRONG_CHAIN

if (message.includes(NAME_PLACEHOLDER)) {
message = message.replaceAll(NAME_PLACEHOLDER, proposer.metadata.name)
}

if (message.includes(CHAIN_PLACEHOLDER)) {
const chainName = configs.find((chain) => chain.chainId === safe.chainId)?.chainName ?? 'this network'
message = message.replaceAll(CHAIN_PLACEHOLDER, chainName)
}

return {
message,
severity,
}
}, [configs, isUnsupportedChain, proposal.params, proposal.verifyContext.verified, safe.chainId])
}
37 changes: 37 additions & 0 deletions src/components/walletconnect/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Bridges enforcing same address on destination chains
export const StrictAddressBridges = [
'bridge.arbitrum.io',
'bridge.base.org',
'cbridge.celer.network',
'www.orbiter.finance',
'zksync-era.l2scan.co',
'app.optimism.io',
'www.portalbridge.com',
'wallet.polygon.technology',
'app.rhino.fi',
]

// Bridges that initially select the same address on the destination chain but allow changing it
export const DefaultAddressBridges = [
'across.to',
'app.allbridge.io',
'core.allbridge.io',
'bungee.exchange',
'www.carrier.so',
'app.chainport.io',
'bridge.gnosischain.com',
'app.hop.exchange',
'app.interport.fi',
'jumper.exchange',
'www.layerswap.io',
'meson.fi',
'satellite.money',
'stargate.finance',
'app.squidrouter.com',
'app.symbiosis.finance',
'www.synapseprotocol.com',
'app.thevoyager.io',
'portal.txsync.io',
'bridge.wanchain.org',
'app.xy.finance',
]
Loading