Skip to content

Commit

Permalink
feat: block/warn about bridges accordingly (#2652)
Browse files Browse the repository at this point in the history
* feat: block/warn about bridges accordingly

* fix: abstract constants + rename variables
  • Loading branch information
iamacook authored Oct 18, 2023
1 parent a38b437 commit 6e45ac2
Show file tree
Hide file tree
Showing 7 changed files with 282 additions and 72 deletions.
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', () => {

Check failure on line 9 in src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts

View workflow job for this annotation

GitHub Actions / Tests annotations (🧪 jest-coverage-report-action)

useCompatibilityWarning > should return an error for a dangerous bridge

TypeError: Cannot destructure property 'message' of '((cov_2igsvcx892(...).s[17]++) , (intermediate value)(intermediate value)(intermediate value))' as it is undefined. at message (/home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:42:11) at mountMemo (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:17225:19) at Object.useMemo (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:17670:16) at useMemo (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react/cjs/react.development.js:1650:21) at useCompatibilityWarning (/home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:38:17) at /home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:17:64 at TestComponent (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:283:27) at renderWithHooks (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:16305:18) at mountIndeterminateComponent (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:20074:13) at beginWork (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:21587:16) at beginWork$1 (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:27426:14) at performUnitOfWork (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:26560:12) at workLoopSync (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:26466:5) at renderRootSync (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:26434:7) at recoverFromConcurrentError (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:25850:20) at performConcurrentWorkOnRoot (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:25750:22) at flushActQueue (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react/cjs/react.development.js:2667:24) at act (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react/cjs/react.development.js:2582:11) at /home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/act-compat.js:63:25 at renderRoot (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:159:26) at render (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:246:10) at renderHook (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:293:7) at customRenderHook (/home/runner/work/safe-wallet-web/safe-wallet-web/src/tests/test-utils.tsx:81:20) at Object.<anonymous> (/home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:17:34) at Promise.then.completed (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/utils.js:298:28) at new Promise (<anonymous>) at callAsyncCircusFn (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/utils.js:231:10) at _callCircusTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:316:40) at _runTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:252:3) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:126:9) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:121:9) at run (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:71:3) at runAndTransformResultsToJestFormat (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21) at jestAdapter (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19) at runTestInternal (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/runTest.js:367:16) at runTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/runTest.js:444:34)
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', () => {

Check failure on line 26 in src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts

View workflow job for this annotation

GitHub Actions / Tests annotations (🧪 jest-coverage-report-action)

useCompatibilityWarning > should return a warning for a risky bridge

TypeError: Cannot destructure property 'message' of '((cov_2igsvcx892(...).s[17]++) , (intermediate value)(intermediate value)(intermediate value))' as it is undefined. at message (/home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:42:11) at mountMemo (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:17225:19) at Object.useMemo (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:17670:16) at useMemo (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react/cjs/react.development.js:1650:21) at useCompatibilityWarning (/home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:38:17) at /home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:35:64 at TestComponent (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:283:27) at renderWithHooks (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:16305:18) at mountIndeterminateComponent (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:20074:13) at beginWork (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:21587:16) at beginWork$1 (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:27426:14) at performUnitOfWork (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:26560:12) at workLoopSync (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:26466:5) at renderRootSync (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:26434:7) at recoverFromConcurrentError (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:25850:20) at performConcurrentWorkOnRoot (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react-dom/cjs/react-dom.development.js:25750:22) at flushActQueue (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react/cjs/react.development.js:2667:24) at act (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/react/cjs/react.development.js:2582:11) at /home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/act-compat.js:63:25 at renderRoot (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:159:26) at render (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:246:10) at renderHook (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/@testing-library/react/dist/pure.js:293:7) at customRenderHook (/home/runner/work/safe-wallet-web/safe-wallet-web/src/tests/test-utils.tsx:81:20) at Object.<anonymous> (/home/runner/work/safe-wallet-web/safe-wallet-web/src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:35:34) at Promise.then.completed (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/utils.js:298:28) at new Promise (<anonymous>) at callAsyncCircusFn (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/utils.js:231:10) at _callCircusTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:316:40) at _runTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:252:3) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:126:9) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:121:9) at run (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:71:3) at runAndTransformResultsToJestFormat (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21) at jestAdapter (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19) at runTestInternal (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/runTest.js:367:16) at runTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/runTest.js:444:34)
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',
]

0 comments on commit 6e45ac2

Please sign in to comment.