Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Warning for mismatching mastercopy in safe proxy #3908

Merged
merged 19 commits into from
Jun 21, 2022
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { InvalidMasterCopyError } from './'
import { render, waitFor, fireEvent } from 'src/utils/test-utils'
import * as safeVersion from 'src/logic/safe/utils/safeVersion'
import * as useAsync from 'src/logic/hooks/useAsync'

describe('InvalidMasterCopyError', () => {
it('returns null if valid master copy', async () => {
jest.spyOn(safeVersion, 'isValidMasterCopy')
jest.spyOn(useAsync, 'default').mockImplementationOnce(() => [true, undefined, false])

const { container } = render(<InvalidMasterCopyError />)

await waitFor(() => {
expect(container.firstChild).toBeNull()
})
})

it('returns null if error', async () => {
jest.spyOn(safeVersion, 'isValidMasterCopy')
jest.spyOn(useAsync, 'default').mockImplementationOnce(() => [undefined, new Error(), false])

const { container } = render(<InvalidMasterCopyError />)

await waitFor(() => {
expect(container.firstChild).toBeNull()
})
})

it('displays an error message if not a valid master copy', async () => {
jest.spyOn(safeVersion, 'isValidMasterCopy')
jest.spyOn(useAsync, 'default').mockImplementationOnce(() => [false, undefined, false])

const { getByText } = render(<InvalidMasterCopyError />)

await waitFor(() => {
expect(getByText(/This Safe was created with an unsupported base contract/)).toBeInTheDocument()
})
})

it('hides the error message on close', async () => {
jest.spyOn(safeVersion, 'isValidMasterCopy')
jest.spyOn(useAsync, 'default').mockImplementation(() => [false, undefined, false])

const { getByText, queryByText, getByRole } = render(<InvalidMasterCopyError />)

await waitFor(() => {
expect(getByText(/This Safe was created with an unsupported base contract/)).toBeInTheDocument()
})

fireEvent.click(getByRole('button'))

await waitFor(() => {
expect(queryByText(/This Safe was created with an unsupported base contract/)).not.toBeInTheDocument()
})
})
})
47 changes: 47 additions & 0 deletions src/components/AppLayout/InvalidMasterCopyError/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { Link } from '@gnosis.pm/safe-react-components'
import { useSelector } from 'react-redux'
import { getChainInfo } from 'src/config'
import { Errors, logError } from 'src/logic/exceptions/CodedException'
import useAsync from 'src/logic/hooks/useAsync'
import { currentSafe } from 'src/logic/safe/store/selectors'
import { isValidMasterCopy } from 'src/logic/safe/utils/safeVersion'
import MuiAlert from '@material-ui/lab/Alert'
import { useState } from 'react'

const CLI_LINK = 'https://github.com/5afe/safe-cli'

export const InvalidMasterCopyError = (): React.ReactElement | null => {
const chainInfo = getChainInfo()
const { implementation } = useSelector(currentSafe)
const [showMasterCopyError, setShowMasterCopyError] = useState(true)

const [validMasterCopy, error] = useAsync(async () => {
if (implementation.value) {
return await isValidMasterCopy(chainInfo.chainId, implementation.value)
iamacook marked this conversation as resolved.
Show resolved Hide resolved
}
}, [chainInfo.chainId, implementation.value])

if (!showMasterCopyError) {
return null
}

if (error) {
logError(Errors._620, error.message)
return null
}

if (typeof validMasterCopy === 'undefined' || validMasterCopy) {
return null
}

return (
<MuiAlert severity="error" onClose={() => setShowMasterCopyError(false)}>
This Safe was created with an unsupported base contract. The web interface might not work correctly. We recommend
using the{' '}
<Link href={CLI_LINK} size="xl" target="_blank" rel="noopener noreferrer">
command line interface
</Link>{' '}
instead.
</MuiAlert>
)
}
1 change: 1 addition & 0 deletions src/components/AppLayout/Sidebar/SafeHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const IconContainer = styled.div`
display: flex;
gap: 8px;
justify-content: space-evenly;
align-items: center;
margin: 14px 0;
`
const StyledButton = styled(Button)`
Expand Down
4 changes: 4 additions & 0 deletions src/components/AppLayout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { MobileNotSupported } from './MobileNotSupported'
import { SAFE_APP_LANDING_PAGE_ROUTE, SAFE_ROUTES, WELCOME_ROUTE } from 'src/routes/routes'
import useDarkMode from 'src/logic/hooks/useDarkMode'
import { screenSm } from 'src/theme/variables'
import { InvalidMasterCopyError } from 'src/components/AppLayout/InvalidMasterCopyError'

const Container = styled.div`
height: 100vh;
Expand All @@ -23,6 +24,7 @@ const Container = styled.div`

const HeaderWrapper = styled.nav`
height: 52px;
min-height: 52px;
width: 100%;
z-index: 1299;

Expand Down Expand Up @@ -157,6 +159,8 @@ const Layout: React.FC<Props> = ({
<HeaderWrapper>
<Header />
</HeaderWrapper>
<InvalidMasterCopyError />

<BodyWrapper>
{showSideBar && (
<SidebarWrapper data-testid="sidebar" $expanded={expanded} onClick={onSidebarClick}>
Expand Down
21 changes: 12 additions & 9 deletions src/logic/contracts/safeContracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getFallbackHandlerDeployment,
getMultiSendCallOnlyDeployment,
getSignMessageLibDeployment,
SingletonDeployment,
} from '@gnosis.pm/safe-deployments'
import Web3 from 'web3'
import { AbiItem } from 'web3-utils'
Expand All @@ -22,6 +23,7 @@ import { CompatibilityFallbackHandler } from 'src/types/contracts/compatibility_
import { SignMessageLib } from 'src/types/contracts/sign_message_lib.d'
import { MultiSend } from 'src/types/contracts/multi_send.d'
import { getSafeInfo } from 'src/logic/safe/utils/safeInformation'
import { NonPayableTransactionObject } from 'src/types/contracts/types'

export const SENTINEL_ADDRESS = '0x0000000000000000000000000000000000000001'

Expand All @@ -30,12 +32,13 @@ let safeMaster: GnosisSafe
let fallbackHandler: CompatibilityFallbackHandler
let multiSend: MultiSend

const getSafeContractDeployment = ({ safeVersion }: { safeVersion: string }) => {
const getSafeContractDeployment = ({ safeVersion }: { safeVersion: string }): SingletonDeployment | undefined => {
// We check if version is prior to v1.0.0 as they are not supported but still we want to keep a minimum compatibility
const useOldestContractVersion = semverSatisfies(safeVersion, '<1.0.0')
// We have to check if network is L2
const networkId = _getChainId()
const chainConfig = getChainById(networkId)

// We had L1 contracts in three L2 networks, xDai, EWC and Volta so even if network is L2 we have to check that safe version is after v1.3.0
const useL2ContractVersion = chainConfig.l2 && semverSatisfies(safeVersion, '>=1.3.0')
const getDeployment = useL2ContractVersion ? getSafeL2SingletonDeployment : getSafeSingletonDeployment
Expand Down Expand Up @@ -194,7 +197,7 @@ export const getMasterCopyAddressFromProxyAddress = async (proxyAddress: string)
return masterCopyAddress
}

export const instantiateSafeContracts = () => {
export const instantiateSafeContracts = (): void => {
const web3 = getWeb3()
const chainId = _getChainId()

Expand All @@ -211,32 +214,32 @@ export const instantiateSafeContracts = () => {
multiSend = getMultiSendContractInstance(web3, chainId)
}

export const getSafeMasterContract = () => {
export const getSafeMasterContract = (): GnosisSafe => {
instantiateSafeContracts()
return safeMaster
}

export const getSafeMasterContractAddress = () => {
export const getSafeMasterContractAddress = (): string => {
return safeMaster.options.address
}

export const getFallbackHandlerContractAddress = () => {
export const getFallbackHandlerContractAddress = (): string => {
return fallbackHandler.options.address
}

export const getMultisendContract = () => {
export const getMultisendContract = (): MultiSend => {
return multiSend
}

export const getMultisendContractAddress = () => {
export const getMultisendContractAddress = (): string => {
return multiSend.options.address
}

export const getSafeDeploymentTransaction = (
safeAccounts: string[],
numConfirmations: number,
safeCreationSalt: number,
) => {
): NonPayableTransactionObject<string> => {
const gnosisSafeData = safeMaster.methods
.setup(
safeAccounts,
Expand All @@ -257,7 +260,7 @@ export const estimateGasForDeployingSafe = async (
numConfirmations: number,
userAccount: string,
safeCreationSalt: number,
) => {
): Promise<number> => {
const proxyFactoryData = getSafeDeploymentTransaction(safeAccounts, numConfirmations, safeCreationSalt).encodeABI()

return calculateGasOf({
Expand Down
1 change: 1 addition & 0 deletions src/logic/exceptions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ enum ErrorCodes {
_617 = '617: Error fetching safeTxGas',
_618 = '618: Error fetching fee history',
_619 = '619: Failed to prepare a multisend tx',
_620 = '620: Failed to load master copy information',
_700 = '700: Failed to read from local/session storage',
_701 = '701: Failed to write to local/session storage',
_702 = '702: Failed to remove from local/session storage',
Expand Down
12 changes: 12 additions & 0 deletions src/logic/safe/store/actions/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ describe('extractRemoteSafeInfo', () => {
'SAFE_TX_GAS_OPTIONAL',
'SPENDING_LIMIT',
] as FEATURES[],
implementation: {
value: '0x3E5c63644E683549055b9Be8653de26E0B4CD36E',
name: 'Gnosis Safe: Mastercopy 1.3.0',
logoUri:
'https://safe-transaction-assets.staging.gnosisdev.com/contracts/logos/0x3E5c63644E683549055b9Be8653de26E0B4CD36E.png',
},
}

const remoteSafeInfo = await extractRemoteSafeInfo(remoteSafeInfoWithoutModules as any)
Expand Down Expand Up @@ -131,6 +137,12 @@ describe('extractRemoteSafeInfo', () => {
'SAFE_TX_GAS_OPTIONAL',
'SPENDING_LIMIT',
] as FEATURES[],
implementation: {
value: '0x3E5c63644E683549055b9Be8653de26E0B4CD36E',
name: 'Gnosis Safe: Mastercopy 1.3.0',
logoUri:
'https://safe-transaction-assets.staging.gnosisdev.com/contracts/logos/0x3E5c63644E683549055b9Be8653de26E0B4CD36E.png',
},
}

const remoteSafeInfo = await extractRemoteSafeInfo(remoteSafeInfoWithModules as any)
Expand Down
8 changes: 4 additions & 4 deletions src/logic/safe/store/actions/mocks/safeInformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const remoteSafeInfoWithModules = {
implementation: {
value: '0x3E5c63644E683549055b9Be8653de26E0B4CD36E',
name: 'Gnosis Safe: Mastercopy 1.3.0',
logoUrl:
logoUri:
'https://safe-transaction-assets.staging.gnosisdev.com/contracts/logos/0x3E5c63644E683549055b9Be8653de26E0B4CD36E.png',
},
guard: {
Expand All @@ -43,7 +43,7 @@ export const remoteSafeInfoWithModules = {
fallbackHandler: {
value: '0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4',
name: 'Gnosis Safe: Default Callback Handler 1.3.0',
logoUrl:
logoUri:
'https://safe-transaction-assets.staging.gnosisdev.com/contracts/logos/0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4.png',
},
version: '1.3.0',
Expand Down Expand Up @@ -79,14 +79,14 @@ export const remoteSafeInfoWithoutModules = {
implementation: {
value: '0x3E5c63644E683549055b9Be8653de26E0B4CD36E',
name: 'Gnosis Safe: Mastercopy 1.3.0',
logoUrl:
logoUri:
'https://safe-transaction-assets.staging.gnosisdev.com/contracts/logos/0x3E5c63644E683549055b9Be8653de26E0B4CD36E.png',
},
modules: [],
fallbackHandler: {
value: '0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4',
name: 'Gnosis Safe: Default Callback Handler 1.3.0',
logoUrl:
logoUri:
'https://safe-transaction-assets.staging.gnosisdev.com/contracts/logos/0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4.png',
},
version: '1.3.0',
Expand Down
1 change: 1 addition & 0 deletions src/logic/safe/store/actions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export const extractRemoteSafeInfo = async (remoteSafeInfo: SafeInfo): Promise<P
safeInfo.txQueuedTag = remoteSafeInfo.txQueuedTag
safeInfo.txHistoryTag = remoteSafeInfo.txHistoryTag
safeInfo.chainId = remoteSafeInfo.chainId as ChainId
safeInfo.implementation = remoteSafeInfo.implementation

return safeInfo
}
Expand Down
7 changes: 7 additions & 0 deletions src/logic/safe/store/models/safe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Record, RecordOf } from 'immutable'
import { ChainId } from 'src/config/chain.d'

import { BalanceRecord } from 'src/logic/tokens/store/actions/fetchSafeTokens'
import { AddressEx } from '@gnosis.pm/safe-react-gateway-sdk/dist/types/common'

export type SafeOwner = string

Expand Down Expand Up @@ -33,6 +34,7 @@ export type SafeRecordProps = {
modules?: ModulePair[] | null
spendingLimits?: SpendingLimit[] | null
balances: BalanceRecord[]
implementation: AddressEx
loaded: boolean
nonce: number
recurringUser?: boolean
Expand All @@ -59,6 +61,11 @@ const makeSafe = Record<SafeRecordProps>({
modules: [],
spendingLimits: [],
balances: [],
implementation: {
value: '',
name: null,
logoUri: null,
},
loaded: false,
nonce: 0,
recurringUser: undefined,
Expand Down
3 changes: 3 additions & 0 deletions src/logic/safe/store/reducer/safe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ const updateSafeProps = (prevSafe, safe) => {
List.isList(safe[key])
? record.set(key, safe[key])
: record.update(key, (current) => current.merge(safe[key]))
} else {
// TODO: temporary fix if type is AddressEx because it's neither a Map, nor has a size property
record.set(key, safe[key])
}
} else {
// By default we overwrite the value. This is for strings, numbers and unset values
Expand Down
49 changes: 48 additions & 1 deletion src/logic/safe/utils/__tests__/safeVersion.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
import { checkIfSafeNeedsUpdate, hasFeature } from 'src/logic/safe/utils/safeVersion'
import * as GatewaySDK from '@gnosis.pm/safe-react-gateway-sdk'
DiogoSoaress marked this conversation as resolved.
Show resolved Hide resolved
import { checkIfSafeNeedsUpdate, hasFeature, isValidMasterCopy } from 'src/logic/safe/utils/safeVersion'

describe('Check safe version', () => {
it('Calls checkIfSafeNeedUpdate, should return true if the safe version is bellow the target one', async () => {
Expand Down Expand Up @@ -36,4 +37,50 @@ describe('Check safe version', () => {
expect(hasFeature(FEATURES.SAFE_APPS, '1.1.1')).toBe(true)
})
})

describe('isValidMasterCopy', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add some more tests so that we cover all deployed versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the linter and prettier warnings and added tests for 1.0.0 and 1.2.0!

it('returns false if address is not contained in result', async () => {
jest.spyOn(GatewaySDK, 'getMasterCopies').mockImplementation(() =>
Promise.resolve([
{
address: '0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552',
version: '1.3.0',
deployer: 'Gnosis',
deployedBlockNumber: 12504268,
lastIndexedBlockNumber: 14927028,
l2: false,
},
]),
)

const isValid = await isValidMasterCopy('1', '0x0000000000000000000000000000000000000005')
expect(isValid).toBe(false)
})

it('returns true if address is contained in list', async () => {
jest.spyOn(GatewaySDK, 'getMasterCopies').mockImplementation(() =>
Promise.resolve([
{
address: '0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552',
version: '1.3.0',
deployer: 'Gnosis',
deployedBlockNumber: 12504268,
lastIndexedBlockNumber: 14927028,
l2: false,
},
{
address: '0x3E5c63644E683549055b9Be8653de26E0B4CD36E',
version: '1.3.0+L2',
deployer: 'Gnosis',
deployedBlockNumber: 12504423,
lastIndexedBlockNumber: 14927028,
l2: true,
},
]),
)

const isValid = await isValidMasterCopy('1', '0x3E5c63644E683549055b9Be8653de26E0B4CD36E')
expect(isValid).toBe(true)
})
})
})
Loading