Skip to content

Commit

Permalink
fix: disable tx creation for counterfactual Safes
Browse files Browse the repository at this point in the history
  • Loading branch information
schmanu committed Sep 19, 2024
1 parent bf6ea57 commit f167981
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 16 deletions.
80 changes: 80 additions & 0 deletions src/components/common/CheckWallet/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import useIsWrongChain from '@/hooks/useIsWrongChain'
import useWallet from '@/hooks/wallets/useWallet'
import { chainBuilder } from '@/tests/builders/chains'
import { useIsWalletDelegate } from '@/hooks/useDelegates'
import { faker } from '@faker-js/faker'
import { extendedSafeInfoBuilder } from '@/tests/builders/safe'
import useSafeInfo from '@/hooks/useSafeInfo'

// mock useWallet
jest.mock('@/hooks/wallets/useWallet', () => ({
Expand Down Expand Up @@ -38,6 +42,25 @@ jest.mock('@/hooks/useIsWrongChain', () => ({
default: jest.fn(() => false),
}))

jest.mock('@/hooks/useDelegates', () => ({
__esModule: true,
useIsWalletDelegate: jest.fn(() => false),
}))

jest.mock('@/hooks/useSafeInfo', () => ({
__esModule: true,
default: jest.fn(() => {
const safeAddress = faker.finance.ethereumAddress()
return {
safeAddress,
safe: extendedSafeInfoBuilder()
.with({ address: { value: safeAddress } })
.with({ deployed: true })
.build(),
}
}),
}))

const renderButton = () =>
render(<CheckWallet checkNetwork={false}>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>)

Expand Down Expand Up @@ -110,6 +133,63 @@ describe('CheckWallet', () => {
expect(allowContainer.querySelector('button')).not.toBeDisabled()
})

it('should not disable the button for delegates', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)
;(useIsWalletDelegate as jest.MockedFunction<typeof useIsWalletDelegate>).mockReturnValueOnce(true)

const { container } = renderButton()

expect(container.querySelector('button')).not.toBeDisabled()
})

it('should disable the button for counterfactual Safes', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(true)

const safeAddress = faker.finance.ethereumAddress()
const mockSafeInfo = {
safeAddress,
safe: extendedSafeInfoBuilder()
.with({ address: { value: safeAddress } })
.with({ deployed: false })
.build(),
}

;(useSafeInfo as jest.MockedFunction<typeof useSafeInfo>).mockReturnValueOnce(
mockSafeInfo as unknown as ReturnType<typeof useSafeInfo>,
)

const { container } = renderButton()

expect(container.querySelector('button')).toBeDisabled()
expect(container.querySelector('span[aria-label]')).toHaveAttribute(
'aria-label',
'You need to activate the Safe before transacting',
)
})

it('should enable the button for counterfactual Safes if allowed', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(true)

const safeAddress = faker.finance.ethereumAddress()
const mockSafeInfo = {
safeAddress,
safe: extendedSafeInfoBuilder()
.with({ address: { value: safeAddress } })
.with({ deployed: false })
.build(),
}

;(useSafeInfo as jest.MockedFunction<typeof useSafeInfo>).mockReturnValueOnce(
mockSafeInfo as unknown as ReturnType<typeof useSafeInfo>,
)

const { container } = render(
<CheckWallet allowUndeployedSafe>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
)

expect(container.querySelector('button')).toBeEnabled()
})

it('should allow non-owners if specified', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)

Expand Down
40 changes: 26 additions & 14 deletions src/components/common/CheckWallet/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useIsWalletDelegate } from '@/hooks/useDelegates'
import { type ReactElement } from 'react'
import { useMemo, type ReactElement } from 'react'
import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import useWallet from '@/hooks/wallets/useWallet'
Expand All @@ -14,12 +14,13 @@ type CheckWalletProps = {
allowNonOwner?: boolean
noTooltip?: boolean
checkNetwork?: boolean
allowUndeployedSafe?: boolean
}

enum Message {
WalletNotConnected = 'Please connect your wallet',
NotSafeOwner = 'Your connected wallet is not a signer of this Safe Account',
CounterfactualMultisig = 'You need to activate the Safe before transacting',
SafeNotActivated = 'You need to activate the Safe before transacting',
}

const CheckWallet = ({
Expand All @@ -28,28 +29,39 @@ const CheckWallet = ({
allowNonOwner,
noTooltip,
checkNetwork = false,
allowUndeployedSafe = false,
}: CheckWalletProps): ReactElement => {
const wallet = useWallet()
const isSafeOwner = useIsSafeOwner()
const isSpendingLimit = useIsOnlySpendingLimitBeneficiary()
const isOnlySpendingLimit = useIsOnlySpendingLimitBeneficiary()
const connectWallet = useConnectWallet()
const isWrongChain = useIsWrongChain()
const isDelegate = useIsWalletDelegate()

const { safe } = useSafeInfo()

const isCounterfactualMultiSig = !allowNonOwner && !safe.deployed && safe.threshold > 1
const isUndeployedSafe = !safe.deployed

const message =
wallet &&
(isSafeOwner || allowNonOwner || (isSpendingLimit && allowSpendingLimit) || isDelegate) &&
!isCounterfactualMultiSig
? ''
: !wallet
? Message.WalletNotConnected
: isCounterfactualMultiSig
? Message.CounterfactualMultisig
: Message.NotSafeOwner
const message = useMemo(() => {
if (!wallet) {
return Message.WalletNotConnected
}
if ((!allowNonOwner && !isSafeOwner && !isDelegate) || (isOnlySpendingLimit && !allowSpendingLimit)) {
return Message.NotSafeOwner
}
if (isUndeployedSafe && !allowUndeployedSafe) {
return Message.SafeNotActivated
}
}, [
allowNonOwner,
allowSpendingLimit,
allowUndeployedSafe,
isDelegate,
isOnlySpendingLimit,
isSafeOwner,
isUndeployedSafe,
wallet,
])

if (checkNetwork && isWrongChain) return children(false)
if (!message) return children(true)
Expand Down
2 changes: 1 addition & 1 deletion src/features/counterfactual/ActivateAccountButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const ActivateAccountButton = () => {
return (
<Tooltip title={isProcessing ? 'The safe activation is already in process' : undefined}>
<span>
<CheckWallet allowNonOwner>
<CheckWallet allowNonOwner allowUndeployedSafe>
{(isOk) => (
<Button
data-testid="activate-account-btn-cf"
Expand Down
2 changes: 1 addition & 1 deletion src/features/counterfactual/ActivateAccountFlow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ const ActivateAccountFlow = () => {
<Divider sx={{ mx: -3, mt: 2, mb: 1 }} />

<Box display="flex" flexDirection="row" justifyContent="flex-end" gap={3}>
<CheckWallet checkNetwork={!submitDisabled} allowNonOwner>
<CheckWallet checkNetwork={!submitDisabled} allowNonOwner allowUndeployedSafe>
{(isOk) => (
<Button
data-testid="activate-account-btn"
Expand Down

0 comments on commit f167981

Please sign in to comment.