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

fix: Disable transaction buttons until sdk is initialized #4405

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open
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
68 changes: 40 additions & 28 deletions src/components/common/CheckWallet/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getByLabelText, render } from '@/tests/test-utils'
import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
import { render } from '@/tests/test-utils'
import CheckWallet from '.'
import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
Expand Down Expand Up @@ -61,6 +62,11 @@ jest.mock('@/hooks/useSafeInfo', () => ({
}),
}))

jest.mock('@/hooks/coreSDK/safeCoreSDK', () => ({
__esModule: true,
useSafeSDK: jest.fn(() => ({})),
}))

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

Expand All @@ -70,46 +76,43 @@ describe('CheckWallet', () => {
})

it('renders correctly when the wallet is connected to the right chain and is an owner', () => {
const { container } = renderButton()
const { getByText } = renderButton()

// Check that the button is enabled
expect(container.querySelector('button')).not.toBeDisabled()
expect(getByText('Continue')).not.toBeDisabled()
})

it('should disable the button when the wallet is not connected', () => {
;(useWallet as jest.MockedFunction<typeof useWallet>).mockReturnValueOnce(null)

const { container } = renderButton()
const { getByText, getByLabelText } = renderButton()

// Check that the button is disabled
expect(container.querySelector('button')).toBeDisabled()
expect(getByText('Continue')).toBeDisabled()

// Check the tooltip text
getByLabelText(container, 'Please connect your wallet')
expect(getByLabelText('Please connect your wallet')).toBeInTheDocument()
})

it('should disable the button when the wallet is connected to the right chain but is not an owner', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)

const { container } = renderButton()
const { getByText, getByLabelText } = renderButton()

expect(container.querySelector('button')).toBeDisabled()
expect(container.querySelector('span[aria-label]')).toHaveAttribute(
'aria-label',
`Your connected wallet is not a signer of this Safe Account`,
)
expect(getByText('Continue')).toBeDisabled()
expect(getByLabelText('Your connected wallet is not a signer of this Safe Account')).toBeInTheDocument()
})

it('should be disabled when connected to the wrong network', () => {
;(useIsWrongChain as jest.MockedFunction<typeof useIsWrongChain>).mockReturnValue(true)
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(true)

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

const { container } = renderButtonWithNetworkCheck()
const { getByText } = renderButtonWithNetworkCheck()

expect(container.querySelector('button')).toBeDisabled()
expect(getByText('Continue')).toBeDisabled()
})

it('should not disable the button for non-owner spending limit benificiaries', () => {
Expand All @@ -118,20 +121,20 @@ describe('CheckWallet', () => {
useIsOnlySpendingLimitBeneficiary as jest.MockedFunction<typeof useIsOnlySpendingLimitBeneficiary>
).mockReturnValueOnce(true)

const { container: allowContainer } = render(
const { getByText } = render(
<CheckWallet allowSpendingLimit>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
)

expect(allowContainer.querySelector('button')).not.toBeDisabled()
expect(getByText('Continue')).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()
const { getByText } = renderButton()

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

it('should disable the button for counterfactual Safes', () => {
Expand All @@ -150,10 +153,10 @@ describe('CheckWallet', () => {
mockSafeInfo as unknown as ReturnType<typeof useSafeInfo>,
)

const { container } = renderButton()
const { getByText, getByLabelText } = renderButton()

expect(container.querySelector('button')).toBeDisabled()
getByLabelText(container, 'You need to activate the Safe before transacting')
expect(getByText('Continue')).toBeDisabled()
expect(getByLabelText('You need to activate the Safe before transacting')).toBeInTheDocument()
})

it('should enable the button for counterfactual Safes if allowed', () => {
Expand All @@ -172,21 +175,21 @@ describe('CheckWallet', () => {
mockSafeInfo as unknown as ReturnType<typeof useSafeInfo>,
)

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

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

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

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

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

it('should not allow non-owners that have a spending limit without allowing spending limits', () => {
Expand All @@ -195,10 +198,19 @@ describe('CheckWallet', () => {
useIsOnlySpendingLimitBeneficiary as jest.MockedFunction<typeof useIsOnlySpendingLimitBeneficiary>
).mockReturnValueOnce(true)

const { container: allowContainer } = render(
const { getByText } = render(<CheckWallet>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>)

expect(getByText('Continue')).toBeDisabled()
})

it('should disable the button if SDK is not initialized', () => {
;(useSafeSDK as jest.MockedFunction<typeof useSafeSDK>).mockReturnValue(undefined)

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

expect(allowContainer.querySelector('button')).toBeDisabled()
expect(getByText('Continue')).toBeDisabled()
expect(getByLabelText('SDK is not initialized yet'))
})
})
7 changes: 7 additions & 0 deletions src/components/common/CheckWallet/index.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
import { useIsWalletDelegate } from '@/hooks/useDelegates'
import { useMemo, type ReactElement } from 'react'
import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary'
Expand All @@ -19,6 +20,7 @@ type CheckWalletProps = {

enum Message {
WalletNotConnected = 'Please connect your wallet',
SDKNotInitialized = 'SDK is not initialized yet',
NotSafeOwner = 'Your connected wallet is not a signer of this Safe Account',
SafeNotActivated = 'You need to activate the Safe before transacting',
}
Expand All @@ -37,6 +39,7 @@ const CheckWallet = ({
const connectWallet = useConnectWallet()
const isWrongChain = useIsWrongChain()
const isDelegate = useIsWalletDelegate()
const sdk = useSafeSDK()

const { safe } = useSafeInfo()

Expand All @@ -46,6 +49,9 @@ const CheckWallet = ({
if (!wallet) {
return Message.WalletNotConnected
}
if (!sdk) {
return Message.SDKNotInitialized
usame-algan marked this conversation as resolved.
Show resolved Hide resolved
}
if (isUndeployedSafe && !allowUndeployedSafe) {
return Message.SafeNotActivated
}
Expand All @@ -61,6 +67,7 @@ const CheckWallet = ({
isOnlySpendingLimit,
isSafeOwner,
isUndeployedSafe,
sdk,
wallet,
])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ const NetworkMultiSelector = ({
))
}
renderOption={(props, chain, { selected }) => (
<li key={chain.chainId} {...props}>
<li {...props} key={chain.chainId}>
<Checkbox data-testid="network-checkbox" size="small" checked={selected} />
<ChainIndicator chainId={chain.chainId} inline />
</li>
Expand Down
4 changes: 4 additions & 0 deletions src/components/tx-flow/flows/SignMessage/SignMessage.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type Safe from '@safe-global/protocol-kit'
import { act } from 'react'
import { extendedSafeInfoBuilder } from '@/tests/builders/safe'
import { hexlify, zeroPadValue, toUtf8Bytes } from 'ethers'
Expand All @@ -13,6 +14,7 @@ import * as useChainsHook from '@/hooks/useChains'
import * as sender from '@/services/safe-messages/safeMsgSender'
import * as onboard from '@/hooks/wallets/useOnboard'
import * as useSafeMessage from '@/hooks/messages/useSafeMessage'
import * as sdk from '@/hooks/coreSDK/safeCoreSDK'
import { render, fireEvent, waitFor } from '@/tests/test-utils'
import type { ConnectedWallet } from '@/hooks/wallets/useOnboard'
import type { EIP1193Provider, WalletState, AppState, OnboardAPI } from '@web3-onboard/core'
Expand Down Expand Up @@ -102,6 +104,8 @@ describe('SignMessage', () => {
}))

jest.spyOn(useIsWrongChainHook, 'default').mockImplementation(() => false)

jest.spyOn(sdk, 'useSafeSDK').mockReturnValue({} as unknown as Safe)
})

describe('EIP-191 messages', () => {
Expand Down
Loading